From 81312b8a787c9a25ce897f361ba7720cdc763e9e Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 2 Mar 2021 11:40:20 +0000 Subject: [PATCH] Return the current OTK count on an empty upload request (#1774) * Always return OTK counts * Fix parameter ordering * Send IDs over to keyserver internal API * Review comments * Fix syntax error * Fix panic, hopefully * Require user ID to be set * Fix user API call --- clientapi/routing/keys.go | 5 ++++- keyserver/api/api.go | 2 ++ keyserver/internal/internal.go | 29 +++++++++++++++++++++++------ sytest-blacklist | 5 ++++- sytest-whitelist | 1 - userapi/internal/api.go | 2 ++ 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/clientapi/routing/keys.go b/clientapi/routing/keys.go index ba03a352f..e22336428 100644 --- a/clientapi/routing/keys.go +++ b/clientapi/routing/keys.go @@ -38,7 +38,10 @@ func UploadKeys(req *http.Request, keyAPI api.KeyInternalAPI, device *userapi.De return *resErr } - uploadReq := &api.PerformUploadKeysRequest{} + uploadReq := &api.PerformUploadKeysRequest{ + DeviceID: device.ID, + UserID: device.UserID, + } if r.DeviceKeys != nil { uploadReq.DeviceKeys = []api.DeviceKeys{ { diff --git a/keyserver/api/api.go b/keyserver/api/api.go index 442af8715..5cb287bc1 100644 --- a/keyserver/api/api.go +++ b/keyserver/api/api.go @@ -108,6 +108,8 @@ type OneTimeKeysCount struct { // PerformUploadKeysRequest is the request to PerformUploadKeys type PerformUploadKeysRequest struct { + UserID string // Required - User performing the request + DeviceID string // Optional - Device performing the request, for fetching OTK count DeviceKeys []DeviceKeys OneTimeKeys []OneTimeKeys // OnlyDisplayNameUpdates should be `true` if ALL the DeviceKeys are present to update diff --git a/keyserver/internal/internal.go b/keyserver/internal/internal.go index 53afe0a60..f53a07619 100644 --- a/keyserver/internal/internal.go +++ b/keyserver/internal/internal.go @@ -513,6 +513,23 @@ func (a *KeyInternalAPI) uploadLocalDeviceKeys(ctx context.Context, req *api.Per } func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.PerformUploadKeysRequest, res *api.PerformUploadKeysResponse) { + if req.UserID == "" { + res.Error = &api.KeyError{ + Err: "user ID missing", + } + } + if req.DeviceID != "" && len(req.OneTimeKeys) == 0 { + counts, err := a.DB.OneTimeKeysCount(ctx, req.UserID, req.DeviceID) + if err != nil { + res.Error = &api.KeyError{ + Err: fmt.Sprintf("a.DB.OneTimeKeysCount: %s", err), + } + } + if counts != nil { + res.OneTimeKeyCounts = append(res.OneTimeKeyCounts, *counts) + } + return + } for _, key := range req.OneTimeKeys { // grab existing keys based on (user/device/algorithm/key ID) keyIDsWithAlgorithms := make([]string, len(key.KeyJSON)) @@ -521,9 +538,9 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform keyIDsWithAlgorithms[i] = keyIDWithAlgo i++ } - existingKeys, err := a.DB.ExistingOneTimeKeys(ctx, key.UserID, key.DeviceID, keyIDsWithAlgorithms) + existingKeys, err := a.DB.ExistingOneTimeKeys(ctx, req.UserID, req.DeviceID, keyIDsWithAlgorithms) if err != nil { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ Err: "failed to query existing one-time keys: " + err.Error(), }) continue @@ -531,8 +548,8 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform for keyIDWithAlgo := range existingKeys { // if keys exist and the JSON doesn't match, error out as the key already exists if !bytes.Equal(existingKeys[keyIDWithAlgo], key.KeyJSON[keyIDWithAlgo]) { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ - Err: fmt.Sprintf("%s device %s: algorithm / key ID %s one-time key already exists", key.UserID, key.DeviceID, keyIDWithAlgo), + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ + Err: fmt.Sprintf("%s device %s: algorithm / key ID %s one-time key already exists", req.UserID, req.DeviceID, keyIDWithAlgo), }) continue } @@ -540,8 +557,8 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform // store one-time keys counts, err := a.DB.StoreOneTimeKeys(ctx, key) if err != nil { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ - Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", key.UserID, key.DeviceID, err.Error()), + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ + Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", req.UserID, req.DeviceID, err.Error()), }) continue } diff --git a/sytest-blacklist b/sytest-blacklist index 601a3f705..b635c9f0e 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -66,4 +66,7 @@ A prev_batch token from incremental sync can be used in the v1 messages API Forgotten room messages cannot be paginated # Blacklisted due to flakiness -Can re-join room if re-invited \ No newline at end of file +Can re-join room if re-invited + +# Blacklisted due to flakiness after #1774 +Local device key changes get to remote servers with correct prev_id \ No newline at end of file diff --git a/sytest-whitelist b/sytest-whitelist index d53fa899d..1e4442a09 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -143,7 +143,6 @@ Local new device changes appear in v2 /sync Local update device changes appear in v2 /sync Get left notifs for other users in sync and /keys/changes when user leaves Local device key changes get to remote servers -Local device key changes get to remote servers with correct prev_id Server correctly handles incoming m.device_list_update If remote user leaves room, changes device and rejoins we see update in sync If remote user leaves room, changes device and rejoins we see update in /keys/changes diff --git a/userapi/internal/api.go b/userapi/internal/api.go index d8af5433d..4abf908d4 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -161,6 +161,7 @@ func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) er var uploadRes keyapi.PerformUploadKeysResponse a.KeyAPI.PerformUploadKeys(context.Background(), &keyapi.PerformUploadKeysRequest{ + UserID: userID, DeviceKeys: deviceKeys, }, &uploadRes) if uploadRes.Error != nil { @@ -217,6 +218,7 @@ func (a *UserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api.Perf // display name has changed: update the device key var uploadRes keyapi.PerformUploadKeysResponse a.KeyAPI.PerformUploadKeys(context.Background(), &keyapi.PerformUploadKeysRequest{ + UserID: req.RequestingUserID, DeviceKeys: []keyapi.DeviceKeys{ { DeviceID: dev.ID,