From ab19b9bcbd32c6db573e15c32f74f603400758df Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Apr 2017 12:55:22 +0100 Subject: [PATCH] Rework device list tracking logic Yet another attempt at fixing https://github.com/vector-im/riot-web/issues/2305. This now implements the algorithm described at http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#tracking-the-device-list-for-a-user: * We now keep a flag to tell us which users' device lists we are tracking. That makes it much easier to figure out whether we should care about device-update notifications from /sync (thereby fixing https://github.com/vector-im/riot-web/issues/3588). * We use the same flag to indicate whether the device list for a particular user is out of date. Previously we did this implicitly by only updating the stored sync token when the list had been updated, but that was somewhat complicated, and in any case didn't help in cases where we initiated the key download due to a user joining an encrypted room. Also fixes https://github.com/vector-im/riot-web/issues/3310. --- spec/TestClient.js | 1 + spec/integ/matrix-client-crypto.spec.js | 44 ++- spec/integ/megolm.spec.js | 77 ++--- src/crypto/DeviceList.js | 360 +++++++++++++++--------- src/crypto/index.js | 111 +++----- src/store/session/webstorage.js | 9 + 6 files changed, 331 insertions(+), 271 deletions(-) diff --git a/spec/TestClient.js b/spec/TestClient.js index ceee409fa4e..7eceea395d8 100644 --- a/spec/TestClient.js +++ b/spec/TestClient.js @@ -60,6 +60,7 @@ TestClient.prototype.toString = function() { * @return {Promise} */ TestClient.prototype.start = function() { + console.log(this + ': starting'); this.httpBackend.when("GET", "/pushrules").respond(200, {}); this.httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" }); this.expectDeviceKeyUpload(); diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index a9a486fcb2c..ffc06dd3e72 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -403,29 +403,6 @@ describe("MatrixClient crypto", function() { bobTestClient.httpBackend.verifyNoOutstandingExpectation(); }); - it('Ali knows the difference between a new user and one with no devices', - function(done) { - aliTestClient.httpBackend.when('POST', '/keys/query').respond(200, { - device_keys: { - '@bob:id': {}, - }, - }); - - const p1 = aliTestClient.client.downloadKeys(['@bob:id']); - const p2 = aliTestClient.httpBackend.flush('/keys/query', 1); - - q.all([p1, p2]).then(function() { - const devices = aliTestClient.storage.getEndToEndDevicesForUser( - '@bob:id', - ); - expect(utils.keys(devices).length).toEqual(0); - - // request again: should be no more requests - return aliTestClient.client.downloadKeys(['@bob:id']); - }).nodeify(done); - }, - ); - it("Bob uploads device keys", function() { return q() .then(bobUploadsDeviceKeys); @@ -673,6 +650,27 @@ describe("MatrixClient crypto", function() { return q() .then(() => aliTestClient.start()) .then(() => firstSync(aliTestClient)) + + // ali will only care about bob's new_device if she is tracking + // bob's devices, which she will do if we enable encryption + .then(aliEnablesEncryption) + + .then(() => { + aliTestClient.expectKeyQuery({ + device_keys: { + [aliUserId]: {}, + [bobUserId]: {}, + }, + }); + return aliTestClient.httpBackend.flush('/keys/query', 1); + }) + + // make sure that the initial key download has completed + // (downloadKeys will wait until it does) + .then(() => { + return aliTestClient.client.downloadKeys([bobUserId]); + }) + .then(function() { const syncData = { next_batch: '2', diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js index f07f483fb37..ce2869ae1df 100644 --- a/spec/integ/megolm.spec.js +++ b/spec/integ/megolm.spec.js @@ -1001,7 +1001,19 @@ describe("megolm", function() { return aliceTestClient.httpBackend.flush('/keys/query', 1); }).then((flushed) => { expect(flushed).toEqual(0); - expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1); + const bobStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@bob:xyz']; + if (bobStat != 1 && bobStat != 2) { + throw new Error('Unexpected status for bob: wanted 1 or 2, got ' + + bobStat); + } + + const chrisStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@chris:abc']; + if (chrisStat != 1 && chrisStat != 2) { + throw new Error('Unexpected status for chris: wanted 1 or 2, got ' + + bobStat); + } // now add an expectation for a query for bob's devices, and let // it complete. @@ -1020,7 +1032,15 @@ describe("megolm", function() { // wait for the client to stop processing the response return aliceTestClient.client.downloadKeys(['@bob:xyz']); }).then(() => { - expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(2); + const bobStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@bob:xyz']; + expect(bobStat).toEqual(3); + const chrisStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@chris:abc']; + if (chrisStat != 1 && chrisStat != 2) { + throw new Error('Unexpected status for chris: wanted 1 or 2, got ' + + bobStat); + } // now let the query for chris's devices complete. return aliceTestClient.httpBackend.flush('/keys/query', 1); @@ -1030,56 +1050,17 @@ describe("megolm", function() { // wait for the client to stop processing the response return aliceTestClient.client.downloadKeys(['@chris:abc']); }).then(() => { + const bobStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@bob:xyz']; + const chrisStat = aliceTestClient.storage + .getEndToEndDeviceTrackingStatus()['@chris:abc']; + + expect(bobStat).toEqual(3); + expect(chrisStat).toEqual(3); expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(3); }); }); - it("Device list downloads before /changes shouldn't affect sync token", - () => { - // https://github.com/vector-im/riot-web/issues/3126#issuecomment-279374939 - aliceTestClient.storage.storeEndToEndDeviceSyncToken(0); - aliceTestClient.storage.storeEndToEndRoom(ROOM_ID, { - algorithm: 'm.megolm.v1.aes-sha2', - }); - - return aliceTestClient.start().then(() => { - aliceTestClient.httpBackend.when('GET', '/sync').respond( - 200, getSyncResponse([aliceTestClient.userId, '@bob:xyz'])); - return aliceTestClient.httpBackend.flush('/sync', 1); - }).then(() => { - aliceTestClient.httpBackend.when('POST', '/keys/query').respond( - 200, {device_keys: {'@bob:xyz': {}}}, - ); - return q.all([ - aliceTestClient.client.downloadKeys(['@bob:xyz']), - aliceTestClient.httpBackend.flush('/keys/query', 1), - ]); - }).then(() => { - expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(0); - - aliceTestClient.httpBackend.when( - 'GET', '/keys/changes', - ).check((req) => { - expect(req.queryParams.from).toEqual(0); - expect(req.queryParams.to).toEqual(1); - }).respond(200, {changed: ['@bob:xyz']}); - - return aliceTestClient.httpBackend.flush('/keys/changes'); - }).then((flushed) => { - aliceTestClient.httpBackend.when('POST', '/keys/query').respond( - 200, {device_keys: {'@bob:xyz': {}}}, - ); - return aliceTestClient.httpBackend.flush('/keys/query'); - }).then((flushed) => { - expect(flushed).toEqual(1); - - // let the client finish processing the keys - return q.delay(10); - }).then(() => { - expect(aliceTestClient.storage.getEndToEndDeviceSyncToken()).toEqual(1); - }); - }); - it("Alice exports megolm keys and imports them to a new device", function(done) { let messageEncrypted; diff --git a/src/crypto/DeviceList.js b/src/crypto/DeviceList.js index 128afdb093e..c1f8787f59b 100644 --- a/src/crypto/DeviceList.js +++ b/src/crypto/DeviceList.js @@ -26,35 +26,35 @@ import q from 'q'; import DeviceInfo from './deviceinfo'; import olmlib from './olmlib'; +// constants for DeviceList._deviceTrackingStatus +// const TRACKING_STATUS_NOT_TRACKED = 0; +const TRACKING_STATUS_PENDING_DOWNLOAD = 1; +const TRACKING_STATUS_DOWNLOAD_IN_PROGRESS = 2; +const TRACKING_STATUS_UP_TO_DATE = 3; + /** * @alias module:crypto/DeviceList */ export default class DeviceList { constructor(baseApis, sessionStore, olmDevice) { - this._baseApis = baseApis; this._sessionStore = sessionStore; - this._olmDevice = olmDevice; + this._serialiser = new DeviceListUpdateSerialiser( + baseApis, sessionStore, olmDevice, + ); - // users with outdated device lists - // userId -> true - this._pendingUsersWithNewDevices = {}; + // which users we are tracking device status for. + // userId -> TRACKING_STATUS_* + this._deviceTrackingStatus = sessionStore.getEndToEndDeviceTrackingStatus() || {}; + for (const u of Object.keys(this._deviceTrackingStatus)) { + // if a download was in progress when we got shut down, it isn't any more. + if (this._deviceTrackingStatus[u] == TRACKING_STATUS_DOWNLOAD_IN_PROGRESS) { + this._deviceTrackingStatus[u] = TRACKING_STATUS_PENDING_DOWNLOAD; + } + } - // userId -> true + // userId -> promise this._keyDownloadsInProgressByUser = {}; - // deferred which is resolved when the current device query resolves. - // (null if there is no current request). - this._currentQueryDeferred = null; - - // deferred which is resolved when the *next* device query resolves. - // - // Normally it is meaningless for this to be non-null when - // _currentQueryDeferred is null, but it can happen if the previous - // query has finished but the next one has not yet started (because the - // previous query failed, in which case we deliberately delay starting - // the next query to avoid tight-looping). - this._queuedQueryDeferred = null; - this.lastKnownSyncToken = null; } @@ -68,43 +68,27 @@ export default class DeviceList { * module:crypto/deviceinfo|DeviceInfo}. */ downloadKeys(userIds, forceDownload) { - let needsRefresh = false; - let waitForCurrentQuery = false; + const usersToDownload = []; + const promises = []; userIds.forEach((u) => { - if (this._pendingUsersWithNewDevices[u]) { - // we already know this user's devices are outdated - needsRefresh = true; - } else if (this._keyDownloadsInProgressByUser[u]) { - // already a download in progress - just wait for it. - // (even if forceDownload is true) - waitForCurrentQuery = true; - } else if (forceDownload) { - console.log("Invalidating device list for " + u + - " for forceDownload"); - this.invalidateUserDeviceList(u); - needsRefresh = true; - } else if (!this.getStoredDevicesForUser(u)) { - console.log("Invalidating device list for " + u + - " due to empty cache"); - this.invalidateUserDeviceList(u); - needsRefresh = true; + const trackingStatus = this._deviceTrackingStatus[u]; + if (this._keyDownloadsInProgressByUser[u]) { + // already a key download in progress/queued for this user; its results + // will be good enough for us. + promises.push(this._keyDownloadsInProgressByUser[u]); + } else if (forceDownload || trackingStatus != TRACKING_STATUS_UP_TO_DATE) { + usersToDownload.push(u); } }); - let promise; - if (needsRefresh) { - console.log("downloadKeys: waiting for next key query"); - promise = this._startOrQueueDeviceQuery(); - } else if(waitForCurrentQuery) { - console.log("downloadKeys: waiting for in-flight query to complete"); - promise = this._currentQueryDeferred.promise; - } else { - // we're all up-to-date. - promise = q(); + if (usersToDownload.length != 0) { + console.log("downloadKeys: downloading for", usersToDownload); + const downloadPromise = this._doKeyDownload(usersToDownload); + promises.push(downloadPromise); } - return promise.then(() => { + return q.all(promises).then(() => { return this._getDevicesFromStore(userIds); }); } @@ -216,14 +200,15 @@ export default class DeviceList { } /** - * Mark the cached device list for the given user outdated. + * flag the given user for device-list tracking, if they are not already. * - * This doesn't set off an update, so that several users can be batched - * together. Call refreshOutdatedDeviceLists() for that. + * This will mean that a subsequent call to refreshOutdatedDeviceLists() + * will download the device list for the user, and that subsequent calls to + * invalidateUserDeviceList will trigger more updates. * * @param {String} userId */ - invalidateUserDeviceList(userId) { + startTrackingDeviceList(userId) { // sanity-check the userId. This is mostly paranoia, but if synapse // can't parse the userId we give it as an mxid, it 500s the whole // request and we can never update the device lists again (because @@ -234,124 +219,218 @@ export default class DeviceList { if (typeof userId !== 'string') { throw new Error('userId must be a string; was '+userId); } - this._pendingUsersWithNewDevices[userId] = true; + if (!this._deviceTrackingStatus[userId]) { + console.log('Now tracking device list for ' + userId); + this._deviceTrackingStatus[userId] = TRACKING_STATUS_PENDING_DOWNLOAD; + } + // we don't yet persist the tracking status, since there may be a lot + // of calls; instead we wait for the forthcoming + // refreshOutdatedDeviceLists. } /** - * If there is not already a device list query in progress, and we have - * users who have outdated device lists, start a query now. + * Mark the cached device list for the given user outdated. + * + * If we are not tracking this user's devices, we'll do nothing. Otherwise + * we flag the user as needing an update. + * + * This doesn't actually set off an update, so that several users can be + * batched together. Call refreshOutdatedDeviceLists() for that. + * + * @param {String} userId */ - refreshOutdatedDeviceLists() { - if (this._currentQueryDeferred) { - // request already in progress - do nothing. (We will automatically - // make another request if there are more users with outdated - // device lists when the current request completes). - return; + invalidateUserDeviceList(userId) { + if (this._deviceTrackingStatus[userId]) { + console.log("Marking device list outdated for", userId); + this._deviceTrackingStatus[userId] = TRACKING_STATUS_PENDING_DOWNLOAD; } - - this._startDeviceQuery(); + // we don't yet persist the tracking status, since there may be a lot + // of calls; instead we wait for the forthcoming + // refreshOutdatedDeviceLists. } /** - * If there is currently a device list query in progress, returns a promise - * which will resolve when the *next* query completes. Otherwise, starts - * a new query, and returns a promise which resolves when it completes. + * Mark all tracked device lists as outdated. * - * @return {Promise} + * This will flag each user whose devices we are tracking as in need of an + * update. */ - _startOrQueueDeviceQuery() { - if (!this._currentQueryDeferred) { - this._startDeviceQuery(); - if (!this._currentQueryDeferred) { - return q(); - } - - return this._currentQueryDeferred.promise; + invalidateAllDeviceLists() { + for (const userId of Object.keys(this._deviceTrackingStatus)) { + this.invalidateUserDeviceList(userId); } + } - if (!this._queuedQueryDeferred) { - this._queuedQueryDeferred = q.defer(); + /** + * If we have users who have outdated device lists, start key downloads for them + */ + refreshOutdatedDeviceLists() { + const usersToDownload = []; + for (const userId of Object.keys(this._deviceTrackingStatus)) { + const stat = this._deviceTrackingStatus[userId]; + if (stat == TRACKING_STATUS_PENDING_DOWNLOAD) { + usersToDownload.push(userId); + } } + if (usersToDownload.length == 0) { + return; + } + + // we didn't persist the tracking status during + // invalidateUserDeviceList, so do it now. + this._persistDeviceTrackingStatus(); - return this._queuedQueryDeferred.promise; + this._doKeyDownload(usersToDownload); } + /** - * kick off a new device query + * Fire off download update requests for the given users, and update the + * device list tracking status for them, and the + * _keyDownloadsInProgressByUser map for them. + * + * @param {String[]} users list of userIds * - * Throws if there is already a query in progress. + * @return {module:client.Promise} resolves when all the users listed have + * been updated. rejects if there was a problem updating any of the + * users. */ - _startDeviceQuery() { - if (this._currentQueryDeferred) { - throw new Error("DeviceList._startDeviceQuery called with request active"); - } - - this._currentQueryDeferred = this._queuedQueryDeferred || q.defer(); - this._queuedQueryDeferred = null; - - const users = Object.keys(this._pendingUsersWithNewDevices); + _doKeyDownload(users) { if (users.length === 0) { // nothing to do - this._currentQueryDeferred.resolve(); - this._currentQueryDeferred = null; - - // that means we're up-to-date with the lastKnownSyncToken. - const token = this.lastKnownSyncToken; - if (token !== null) { - this._sessionStore.storeEndToEndDeviceSyncToken(token); - } - - return; + return q(); } - this._doKeyDownloadForUsers(users).done(() => { - users.forEach((u) => { - delete this._keyDownloadsInProgressByUser[u]; - }); - - this._currentQueryDeferred.resolve(); - this._currentQueryDeferred = null; - - // flush out any more requests that were blocked up while that - // was going on. - this._startDeviceQuery(); + const prom = this._serialiser.updateDevicesForUsers( + users, this.lastKnownSyncToken, + ).then(() => { + finished(true); }, (e) => { console.error( - 'Error updating device key cache for ' + users + ":", e, + 'Error downloading keys for ' + users + ":", e, ); + finished(false); + throw e; + }); - // reinstate the pending flags on any users which failed; this will - // mean that we will do another download in the future (actually on - // the next /sync). + users.forEach((u) => { + this._keyDownloadsInProgressByUser[u] = prom; + const stat = this._deviceTrackingStatus[u]; + if (stat == TRACKING_STATUS_PENDING_DOWNLOAD) { + this._deviceTrackingStatus[u] = TRACKING_STATUS_DOWNLOAD_IN_PROGRESS; + } + }); + + const finished = (success) => { users.forEach((u) => { delete this._keyDownloadsInProgressByUser[u]; - this._pendingUsersWithNewDevices[u] = true; + const stat = this._deviceTrackingStatus[u]; + if (stat == TRACKING_STATUS_DOWNLOAD_IN_PROGRESS) { + if (success) { + // we didn't get any new invalidations since this download started: + // this user's device list is now up to date. + this._deviceTrackingStatus[u] = TRACKING_STATUS_UP_TO_DATE; + console.log("Device list for", u, "now up to date"); + } else { + this._deviceTrackingStatus[u] = TRACKING_STATUS_PENDING_DOWNLOAD; + } + } }); + this._persistDeviceTrackingStatus(); + }; - this._currentQueryDeferred.reject(e); - this._currentQueryDeferred = null; - }); + return prom; + } - users.forEach((u) => { - delete this._pendingUsersWithNewDevices[u]; - this._keyDownloadsInProgressByUser[u] = true; - }); + _persistDeviceTrackingStatus() { + this._sessionStore.storeEndToEndDeviceTrackingStatus(this._deviceTrackingStatus); + } +} + +/** + * Serialises updates to device lists + * + * Ensures that results from /keys/query are not overwritten if a second call + * completes *before* an earlier one. + * + * It currently does this by ensuring only one call to /keys/query happens at a + * time (and queuing other requests up). + */ +class DeviceListUpdateSerialiser { + constructor(baseApis, sessionStore, olmDevice) { + this._baseApis = baseApis; + this._sessionStore = sessionStore; + this._olmDevice = olmDevice; + + this._downloadInProgress = false; + + // users which are queued for download + // userId -> true + this._keyDownloadsQueuedByUser = {}; + + // deferred which is resolved when the queued users are downloaded. + // + // non-null indicates that we have users queued for download. + this._queuedQueryDeferred = null; + + // sync token to be used for the next query: essentially the + // most recent one we know about + this._nextSyncToken = null; } /** - * @param {string[]} downloadUsers list of userIds + * Make a key query request for the given users + * + * @param {String[]} users list of user ids * - * @return {Promise} + * @param {String} syncToken sync token to pass in the query request, to + * help the HS give the most recent results + * + * @return {module:client.Promise} resolves when all the users listed have + * been updated. rejects if there was a problem updating any of the + * users. */ - _doKeyDownloadForUsers(downloadUsers) { - console.log('Starting key download for ' + downloadUsers); + updateDevicesForUsers(users, syncToken) { + users.forEach((u) => { + this._keyDownloadsQueuedByUser[u] = true; + }); + this._nextSyncToken = syncToken; + + if (!this._queuedQueryDeferred) { + this._queuedQueryDeferred = q.defer(); + } + + if (this._downloadInProgress) { + // just queue up these users + console.log('Queued key download for', users); + return this._queuedQueryDeferred.promise; + } + + // start a new download. + return this._doQueuedQueries(); + } + + _doQueuedQueries() { + if (this._downloadInProgress) { + throw new Error( + "DeviceListUpdateSerialiser._doQueuedQueries called with request active", + ); + } + + const downloadUsers = Object.keys(this._keyDownloadsQueuedByUser); + this._keyDownloadsQueuedByUser = {}; + const deferred = this._queuedQueryDeferred; + this._queuedQueryDeferred = null; + + console.log('Starting key download for', downloadUsers); + this._downloadInProgress = true; - const token = this.lastKnownSyncToken; const opts = {}; - if (token) { - opts.token = token; + if (this._nextSyncToken) { + opts.token = this._nextSyncToken; } - return this._baseApis.downloadKeysForUsers( + + this._baseApis.downloadKeysForUsers( downloadUsers, opts, ).then((res) => { const dk = res.device_keys || {}; @@ -369,12 +448,23 @@ export default class DeviceList { } return prom; - }).then(() => { - if (token !== null) { - this._sessionStore.storeEndToEndDeviceSyncToken(token); - } + }).done(() => { console.log('Completed key download for ' + downloadUsers); + + this._downloadInProgress = false; + deferred.resolve(); + + // if we have queued users, fire off another request. + if (this._queuedQueryDeferred) { + this._doQueuedQueries(); + } + }, (e) => { + console.warn('Error downloading keys for ' + downloadUsers + ':', e); + this._downloadInProgressInProgress = false; + deferred.reject(e); }); + + return deferred.promise; } _processQueryResponseForUser(userId, response) { diff --git a/src/crypto/index.js b/src/crypto/index.js index 41e85b2eecc..37a108b2e34 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -61,7 +61,7 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId, this._olmDevice = new OlmDevice(sessionStore); this._deviceList = new DeviceList(baseApis, sessionStore, this._olmDevice); - this._initialDeviceListInvalidationDone = false; + this._initialDeviceListInvalidationPending = false; this._clientRunning = false; @@ -558,9 +558,13 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) { * Configure a room to use encryption (ie, save a flag in the sessionstore). * * @param {string} roomId The room ID to enable encryption in. + * * @param {object} config The encryption config for the room. + * + * @param {boolean=} inhibitDeviceQuery true to suppress device list query for + * users in the room (for now) */ -Crypto.prototype.setRoomEncryption = function(roomId, config) { +Crypto.prototype.setRoomEncryption = function(roomId, config, inhibitDeviceQuery) { // if we already have encryption in this room, we should ignore this event // (for now at least. maybe we should alert the user somehow?) const existingConfig = this._sessionStore.getEndToEndRoom(roomId); @@ -590,21 +594,16 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { }); this._roomEncryptors[roomId] = alg; - // if encryption was not previously enabled in this room, we will have been - // ignoring new device events for these users so far. We may well have - // up-to-date lists for some users, for instance if we were sharing other - // e2e rooms with them, so there is room for optimisation here, but for now - // we just invalidate everyone in the room. - if (!existingConfig) { - console.log("Enabling encryption in " + roomId + " for the first time; " + - "invalidating device lists for all users therein"); - const room = this._clientStore.getRoom(roomId); - const members = room.getJoinedMembers(); - members.forEach((m) => { - this._deviceList.invalidateUserDeviceList(m.userId); - }); - // the actual refresh happens once we've finished processing the sync, - // in _onSyncCompleted. + // make sure we are tracking the device lists for all users in this room. + console.log("Enabling encryption in " + roomId + "; " + + "starting to track device lists for all users therein"); + const room = this._clientStore.getRoom(roomId); + const members = room.getJoinedMembers(); + members.forEach((m) => { + this._deviceList.startTrackingDeviceList(m.userId); + }); + if (!inhibitDeviceQuery) { + this._deviceList.refreshOutdatedDeviceLists(); } }; @@ -795,7 +794,9 @@ Crypto.prototype._onCryptoEvent = function(event) { const content = event.getContent(); try { - this.setRoomEncryption(roomId, content); + // inhibit the device list refresh for now - it will happen once we've + // finished processing the sync, in _onSyncCompleted. + this.setRoomEncryption(roomId, content, true); } catch (e) { console.error("Error configuring encryption in room " + roomId + ":", e); @@ -814,6 +815,8 @@ Crypto.prototype._onSyncCompleted = function(syncData) { const nextSyncToken = syncData.nextSyncToken; if (!syncData.oldSyncToken) { + console.log("Completed initial sync"); + // an initialsync. this._sendNewDeviceEvents(); @@ -821,37 +824,40 @@ Crypto.prototype._onSyncCompleted = function(syncData) { // invalidate devices which have changed since then. const oldSyncToken = this._sessionStore.getEndToEndDeviceSyncToken(); if (oldSyncToken !== null) { + this._initialDeviceListInvalidationPending = true; this._invalidateDeviceListsSince( oldSyncToken, nextSyncToken, ).catch((e) => { // if that failed, we fall back to invalidating everyone. console.warn("Error fetching changed device list", e); - this._invalidateDeviceListForAllActiveUsers(); + this._deviceList.invalidateAllDeviceLists(); }).done(() => { - this._initialDeviceListInvalidationDone = true; + this._initialDeviceListInvalidationPending = false; this._deviceList.lastKnownSyncToken = nextSyncToken; this._deviceList.refreshOutdatedDeviceLists(); }); } else { // otherwise, we have to invalidate all devices for all users we - // share a room with. + // are tracking. console.log("Completed first initialsync; invalidating all " + "device list caches"); - this._invalidateDeviceListForAllActiveUsers(); - this._initialDeviceListInvalidationDone = true; + this._deviceList.invalidateAllDeviceLists(); } } - if (this._initialDeviceListInvalidationDone) { - // if we've got an up-to-date list of users with outdated device lists, - // tell the device list about the new sync token (but not otherwise, because - // otherwise we'll start thinking we're more in sync than we are.) - this._deviceList.lastKnownSyncToken = nextSyncToken; - - // catch up on any new devices we got told about during the sync. - this._deviceList.refreshOutdatedDeviceLists(); + if (!this._initialDeviceListInvalidationPending) { + // we can now store our sync token so that we can get an update on + // restart rather than having to invalidate everyone. + // + // (we don't really need to do this on every sync - we could just + // do it periodically) + this._sessionStore.storeEndToEndDeviceSyncToken(nextSyncToken); } + // catch up on any new devices we got told about during the sync. + this._deviceList.lastKnownSyncToken = nextSyncToken; + this._deviceList.refreshOutdatedDeviceLists(); + // we don't start uploading one-time keys until we've caught up with // to-device messages, to help us avoid throwing away one-time-keys that we // are about to receive messages for @@ -928,49 +934,18 @@ Crypto.prototype._invalidateDeviceListsSince = function( return this._baseApis.getKeyChanges( oldSyncToken, lastKnownSyncToken, ).then((r) => { + console.log("got key changes since", oldSyncToken, ":", r.changed); + if (!r.changed || !Array.isArray(r.changed)) { return; } - // only invalidate users we share an e2e room with - we don't - // care about users in non-e2e rooms. - const filteredUserIds = this._getE2eRoomMembers(); r.changed.forEach((u) => { - if (u in filteredUserIds) { - this._deviceList.invalidateUserDeviceList(u); - } + this._deviceList.invalidateUserDeviceList(u); }); }); }; -/** - * Invalidate any stored device list for any users we share an e2e room with - * - * @private - */ -Crypto.prototype._invalidateDeviceListForAllActiveUsers = function() { - Object.keys(this._getE2eRoomMembers()).forEach((m) => { - this._deviceList.invalidateUserDeviceList(m); - }); -}; - -/** - * get the users we share an e2e-enabled room with - * - * @returns {Object} userid->userid map (should be a Set but argh ES6) - */ -Crypto.prototype._getE2eRoomMembers = function() { - const userIds = Object.create(null); - - const rooms = this._getE2eRooms(); - for (const r of rooms) { - const members = r.getJoinedMembers(); - members.forEach((m) => { userIds[m.userId] = m.userId; }); - } - - return userIds; -}; - /** * Get a list of the e2e-enabled rooms we are members of * @@ -1039,6 +1014,12 @@ Crypto.prototype._onRoomMembership = function(event, member, oldMembership) { return; } + if (member.membership == 'join') { + console.log('Join event for ' + member.userId + ' in ' + roomId); + // make sure we are tracking the deviceList for this user + this._deviceList.startTrackingDeviceList(member.userId); + } + alg.onRoomMembership(event, member, oldMembership); }; diff --git a/src/store/session/webstorage.js b/src/store/session/webstorage.js index 5e9bffe558e..07bfa74eeba 100644 --- a/src/store/session/webstorage.js +++ b/src/store/session/webstorage.js @@ -99,6 +99,14 @@ WebStorageSessionStore.prototype = { return getJsonItem(this.store, keyEndToEndDevicesForUser(userId)); }, + storeEndToEndDeviceTrackingStatus: function(statusMap) { + setJsonItem(this.store, KEY_END_TO_END_DEVICE_LIST_TRACKING_STATUS, statusMap); + }, + + getEndToEndDeviceTrackingStatus: function() { + return getJsonItem(this.store, KEY_END_TO_END_DEVICE_LIST_TRACKING_STATUS); + }, + /** * Store the sync token corresponding to the device list. * @@ -202,6 +210,7 @@ WebStorageSessionStore.prototype = { const KEY_END_TO_END_ACCOUNT = E2E_PREFIX + "account"; const KEY_END_TO_END_ANNOUNCED = E2E_PREFIX + "announced"; const KEY_END_TO_END_DEVICE_SYNC_TOKEN = E2E_PREFIX + "device_sync_token"; +const KEY_END_TO_END_DEVICE_LIST_TRACKING_STATUS = E2E_PREFIX + "device_tracking"; function keyEndToEndDevicesForUser(userId) { return E2E_PREFIX + "devices/" + userId;