Don't send device list updates upon registration (#3307)

Fixes https://github.com/matrix-org/dendrite/issues/3273

As we otherwise send down device list updates which are merely useful
for the user and causes tests to be flakey:

```
 TestPushSync/Adding_a_push_rule_wakes_up_an_incremental_/sync (10ms)
      push_test.go:57: no pushrules found in sync response: {"next_batch":"s0_0_0_0_0_1_1_0_1","device_lists":{"changed":["@user-1:hs1"]}}
```

What this does: If a `PerformDeviceCreation` request is coming from
registering an account, it does **not** send device list updates, as
they are merely useful (no joined rooms, no one to inform) . In all
other cases, the behavior is unchanged and device list updates are sent
as usual.
This commit is contained in:
Till 2024-01-20 21:20:37 +01:00 committed by GitHub
parent bebf701dce
commit d357615452
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 26 additions and 9 deletions

View file

@ -7,7 +7,7 @@ coverage:
project: project:
default: default:
target: auto target: auto
threshold: 0% threshold: 0.1%
base: auto base: auto
flags: flags:
- unittests - unittests

View file

@ -630,6 +630,7 @@ func handleGuestRegistration(
AccessToken: token, AccessToken: token,
IPAddr: req.RemoteAddr, IPAddr: req.RemoteAddr,
UserAgent: req.UserAgent(), UserAgent: req.UserAgent(),
FromRegistration: true,
}, &devRes) }, &devRes)
if err != nil { if err != nil {
return util.JSONResponse{ return util.JSONResponse{
@ -919,6 +920,7 @@ func completeRegistration(
DeviceID: deviceID, DeviceID: deviceID,
IPAddr: ipAddr, IPAddr: ipAddr,
UserAgent: userAgent, UserAgent: userAgent,
FromRegistration: true,
}, &devRes) }, &devRes)
if err != nil { if err != nil {
return util.JSONResponse{ return util.JSONResponse{

View file

@ -379,6 +379,10 @@ type PerformDeviceCreationRequest struct {
// update for this account. Generally the only reason to do this is if the account // update for this account. Generally the only reason to do this is if the account
// is an appservice account. // is an appservice account.
NoDeviceListUpdate bool NoDeviceListUpdate bool
// FromRegistration determines if this request comes from registering a new account
// and is in most cases false.
FromRegistration bool
} }
// PerformDeviceCreationResponse is the response for PerformDeviceCreation // PerformDeviceCreationResponse is the response for PerformDeviceCreation
@ -803,6 +807,10 @@ type PerformUploadKeysRequest struct {
// itself doesn't change but it's easier to pretend upload new keys and reuse the same code paths. // itself doesn't change but it's easier to pretend upload new keys and reuse the same code paths.
// Without this flag, requests to modify device display names would delete device keys. // Without this flag, requests to modify device display names would delete device keys.
OnlyDisplayNameUpdates bool OnlyDisplayNameUpdates bool
// FromRegistration is set if this key upload comes right after creating an account
// and determines if we need to inform downstream components.
FromRegistration bool
} }
// PerformUploadKeysResponse is the response to PerformUploadKeys // PerformUploadKeysResponse is the response to PerformUploadKeys

View file

@ -711,9 +711,15 @@ func (a *UserInternalAPI) uploadLocalDeviceKeys(ctx context.Context, req *api.Pe
} }
return return
} }
err = emitDeviceKeyChanges(a.KeyChangeProducer, existingKeys, keysToStore, req.OnlyDisplayNameUpdates)
if err != nil { // If the request does _not_ come right after registering an account
util.GetLogger(ctx).Errorf("Failed to emitDeviceKeyChanges: %s", err) // inform downstream components. However, we're fine with just creating the
// database entries above in other cases.
if !req.FromRegistration {
err = emitDeviceKeyChanges(a.KeyChangeProducer, existingKeys, keysToStore, req.OnlyDisplayNameUpdates)
if err != nil {
util.GetLogger(ctx).Errorf("Failed to emitDeviceKeyChanges: %s", err)
}
} }
} }

View file

@ -316,7 +316,7 @@ func (a *UserInternalAPI) PerformDeviceCreation(ctx context.Context, req *api.Pe
return nil return nil
} }
// create empty device keys and upload them to trigger device list changes // create empty device keys and upload them to trigger device list changes
return a.deviceListUpdate(dev.UserID, []string{dev.ID}) return a.deviceListUpdate(dev.UserID, []string{dev.ID}, req.FromRegistration)
} }
func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.PerformDeviceDeletionRequest, res *api.PerformDeviceDeletionResponse) error { func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.PerformDeviceDeletionRequest, res *api.PerformDeviceDeletionResponse) error {
@ -356,10 +356,10 @@ func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.Pe
return fmt.Errorf("a.KeyAPI.PerformDeleteKeys: %w", err) return fmt.Errorf("a.KeyAPI.PerformDeleteKeys: %w", err)
} }
// create empty device keys and upload them to delete what was once there and trigger device list changes // create empty device keys and upload them to delete what was once there and trigger device list changes
return a.deviceListUpdate(req.UserID, deletedDeviceIDs) return a.deviceListUpdate(req.UserID, deletedDeviceIDs, false)
} }
func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) error { func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string, fromRegistration bool) error {
deviceKeys := make([]api.DeviceKeys, len(deviceIDs)) deviceKeys := make([]api.DeviceKeys, len(deviceIDs))
for i, did := range deviceIDs { for i, did := range deviceIDs {
deviceKeys[i] = api.DeviceKeys{ deviceKeys[i] = api.DeviceKeys{
@ -371,8 +371,9 @@ func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) er
var uploadRes api.PerformUploadKeysResponse var uploadRes api.PerformUploadKeysResponse
if err := a.PerformUploadKeys(context.Background(), &api.PerformUploadKeysRequest{ if err := a.PerformUploadKeys(context.Background(), &api.PerformUploadKeysRequest{
UserID: userID, UserID: userID,
DeviceKeys: deviceKeys, DeviceKeys: deviceKeys,
FromRegistration: fromRegistration,
}, &uploadRes); err != nil { }, &uploadRes); err != nil {
return err return err
} }