From 21e0c79f7d8f447b79cf3e334ae54b4c7bf17d71 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 24 Aug 2018 10:55:59 +0200 Subject: [PATCH] Revert "Revert "Lazy loading: don't block on setting up room crypto"" This reverts commit 5cf2ebea4fc2728e5492c108b04120daa81bc610. --- spec/unit/room.spec.js | 3 + src/crypto/RoomList.js | 3 + src/crypto/index.js | 138 +++++++++++++++++++++++++---------------- src/models/room.js | 5 ++ src/sync.js | 9 +-- 5 files changed, 102 insertions(+), 56 deletions(-) diff --git a/spec/unit/room.spec.js b/spec/unit/room.spec.js index eb5ba5a51..b98559ecf 100644 --- a/spec/unit/room.spec.js +++ b/spec/unit/room.spec.js @@ -1318,6 +1318,9 @@ describe("Room", function() { // events should already be MatrixEvents return function(event) {return event;}; }, + isRoomEncrypted: function() { + return false; + }, _http: { serverResponse, authedRequest: function() { diff --git a/src/crypto/RoomList.js b/src/crypto/RoomList.js index 5bb437fb7..eb5b469c6 100644 --- a/src/crypto/RoomList.js +++ b/src/crypto/RoomList.js @@ -71,6 +71,9 @@ export default class RoomList { } async setRoomEncryption(roomId, roomInfo) { + // important that this happens before calling into the store + // as it prevents the Crypto::setRoomEncryption for calling + // this twice for consecutive m.room.encryption events this._roomEncryption[roomId] = roomInfo; await this._cryptoStore.doTxn( 'readwrite', [IndexedDBCryptoStore.STORE_ROOMS], (txn) => { diff --git a/src/crypto/index.js b/src/crypto/index.js index 77268cf2c..756fdb3e9 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -106,6 +106,10 @@ function Crypto(baseApis, sessionStore, userId, deviceId, this._receivedRoomKeyRequestCancellations = []; // true if we are currently processing received room key requests this._processingRoomKeyRequests = false; + // track if an initial tracking of all the room members + // has happened for a given room. This is delayed + // to avoid loading room members as long as possible. + this._roomDeviceTrackingState = {}; } utils.inherits(Crypto, EventEmitter); @@ -612,9 +616,6 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) { * @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 = async function(roomId, config, inhibitDeviceQuery) { // if state is being replayed from storage, we might already have a configuration @@ -670,25 +671,42 @@ Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDevic if (storeConfigPromise) { await storeConfigPromise; } - - // 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); - if (!room) { - throw new Error(`Unable to enable encryption in unknown room ${roomId}`); - } - - const members = await room.getEncryptionTargetMembers(); - members.forEach((m) => { - this._deviceList.startTrackingDeviceList(m.userId); - }); - if (!inhibitDeviceQuery) { - this._deviceList.refreshOutdatedDeviceLists(); - } + console.log("Enabling encryption in " + roomId); }; +/** + * Make sure we are tracking the device lists for all users in this room. + * + * @param {string} roomId The room ID to start tracking devices in. + * @returns {Promise} when all devices for the room have been fetched and marked to track + */ +Crypto.prototype.trackRoomDevices = function(roomId) { + const trackAndRefresh = async () => { + // not an encrypted room + if (!this._roomEncryptors[roomId]) { + return; + } + const room = this._clientStore.getRoom(roomId); + if (!room) { + throw new Error(`Unable to start tracking devices in unknown room ${roomId}`); + } + console.log(`Starting to track devices for room ${roomId} ...`); + const members = await room.getEncryptionTargetMembers(); + members.forEach((m) => { + this._deviceList.startTrackingDeviceList(m.userId); + }); + return this._deviceList.refreshOutdatedDeviceLists(); + }; + + let promise = this._roomDeviceTrackingState[roomId]; + if (!promise) { + promise = trackAndRefresh(); + this._roomDeviceTrackingState[roomId] = promise; + } + return promise; +}; + /** * @typedef {Object} module:crypto~OlmSessionResult * @property {module:crypto/deviceinfo} device device info @@ -779,7 +797,7 @@ Crypto.prototype.importRoomKeys = function(keys) { }, ); }; - +/* eslint-disable valid-jsdoc */ //https://github.com/eslint/eslint/issues/7307 /** * Encrypt an event according to the configuration of the room. * @@ -790,7 +808,8 @@ Crypto.prototype.importRoomKeys = function(keys) { * @return {module:client.Promise?} Promise which resolves when the event has been * encrypted, or null if nothing was needed */ -Crypto.prototype.encryptEvent = function(event, room) { +/* eslint-enable valid-jsdoc */ +Crypto.prototype.encryptEvent = async function(event, room) { if (!room) { throw new Error("Cannot send encrypted messages in unknown rooms"); } @@ -808,6 +827,12 @@ Crypto.prototype.encryptEvent = function(event, room) { ); } + if (!this._roomDeviceTrackingState[roomId]) { + this.trackRoomDevices(roomId); + } + // wait for all the room devices to be loaded + await this._roomDeviceTrackingState[roomId]; + let content = event.getContent(); // If event has an m.relates_to then we need // to put this on the wrapping event instead @@ -818,20 +843,19 @@ Crypto.prototype.encryptEvent = function(event, room) { delete content['m.relates_to']; } - return alg.encryptMessage( - room, event.getType(), content, - ).then((encryptedContent) => { - if (mRelatesTo) { - encryptedContent['m.relates_to'] = mRelatesTo; - } + const encryptedContent = await alg.encryptMessage( + room, event.getType(), content); - event.makeEncrypted( - "m.room.encrypted", - encryptedContent, - this._olmDevice.deviceCurve25519Key, - this._olmDevice.deviceEd25519Key, - ); - }); + if (mRelatesTo) { + encryptedContent['m.relates_to'] = mRelatesTo; + } + + event.makeEncrypted( + "m.room.encrypted", + encryptedContent, + this._olmDevice.deviceCurve25519Key, + this._olmDevice.deviceEd25519Key, + ); }; /** @@ -924,9 +948,7 @@ Crypto.prototype.onCryptoEvent = async function(event) { const content = event.getContent(); try { - // inhibit the device list refresh for now - it will happen once we've - // finished processing the sync, in onSyncCompleted. - await this.setRoomEncryption(roomId, content, true); + await this.setRoomEncryption(roomId, content); } catch (e) { console.error("Error configuring encryption in room " + roomId + ":", e); @@ -946,6 +968,7 @@ Crypto.prototype.onSyncWillProcess = async function(syncData) { // at which point we'll start tracking all the users of that room. console.log("Initial sync performed - resetting device tracking state"); this._deviceList.stopTrackingAllDeviceLists(); + this._roomDeviceTrackingState = {}; } }; @@ -991,11 +1014,12 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) { }); } - if (deviceLists.left && Array.isArray(deviceLists.left)) { + if (deviceLists.left && Array.isArray(deviceLists.left) && + deviceLists.left.length) { // Check we really don't share any rooms with these users // any more: the server isn't required to give us the // exact correct set. - const e2eUserIds = new Set(await this._getE2eUsers()); + const e2eUserIds = new Set(await this._getTrackedE2eUsers()); deviceLists.left.forEach((u) => { if (!e2eUserIds.has(u)) { @@ -1007,12 +1031,13 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) { /** * Get a list of all the IDs of users we share an e2e room with + * for which we are tracking devices already * * @returns {string[]} List of user IDs */ -Crypto.prototype._getE2eUsers = async function() { +Crypto.prototype._getTrackedE2eUsers = async function() { const e2eUserIds = []; - for (const room of this._getE2eRooms()) { + for (const room of this._getTrackedE2eRooms()) { const members = await room.getEncryptionTargetMembers(); for (const member of members) { e2eUserIds.push(member.userId); @@ -1022,17 +1047,21 @@ Crypto.prototype._getE2eUsers = async function() { }; /** - * Get a list of the e2e-enabled rooms we are members of + * Get a list of the e2e-enabled rooms we are members of, + * and for which we are already tracking the devices * * @returns {module:models.Room[]} */ -Crypto.prototype._getE2eRooms = function() { +Crypto.prototype._getTrackedE2eRooms = function() { return this._clientStore.getRooms().filter((room) => { // check for rooms with encryption enabled const alg = this._roomEncryptors[room.roomId]; if (!alg) { return false; } + if (!this._roomDeviceTrackingState[room.roomId]) { + return false; + } // ignore any rooms which we have left const myMembership = room.getMyMembership(); @@ -1101,15 +1130,20 @@ Crypto.prototype._onRoomMembership = function(event, member, oldMembership) { // not encrypting in this room 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); - } else if (member.membership == 'invite' && - this._clientStore.getRoom(roomId).shouldEncryptForInvitedMembers()) { - console.log('Invite event for ' + member.userId + ' in ' + roomId); - this._deviceList.startTrackingDeviceList(member.userId); + // only mark users in this room as tracked if we already started tracking in this room + // this way we don't start device queries after sync on behalf of this room which we won't use + // the result of anyway, as we'll need to do a query again once all the members are fetched + // by calling _trackRoomDevices + if (this._roomDeviceTrackingState[roomId]) { + 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); + } else if (member.membership == 'invite' && + this._clientStore.getRoom(roomId).shouldEncryptForInvitedMembers()) { + console.log('Invite event for ' + member.userId + ' in ' + roomId); + this._deviceList.startTrackingDeviceList(member.userId); + } } alg.onRoomMembership(event, member, oldMembership); diff --git a/src/models/room.js b/src/models/room.js index 94115ef45..dc1052ea4 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -368,6 +368,11 @@ Room.prototype.loadMembersIfNeeded = function() { }); this._membersPromise = promise; + // now the members are loaded, start to track the e2e devices if needed + if (this._client.isRoomEncrypted(this.roomId)) { + this._client._crypto.trackRoomDevices(this.roomId); + } + return this._membersPromise; }; diff --git a/src/sync.js b/src/sync.js index 4aa2d2482..d8dffe6b3 100644 --- a/src/sync.js +++ b/src/sync.js @@ -1085,15 +1085,16 @@ SyncApi.prototype._processSyncResponse = async function( self._processEventsForNotifs(room, timelineEvents); - async function processRoomEvent(e) { + function processRoomEvent(e) { client.emit("event", e); if (e.isState() && e.getType() == "m.room.encryption" && self.opts.crypto) { - await self.opts.crypto.onCryptoEvent(e); + self.opts.crypto.onCryptoEvent(e); } } - await Promise.mapSeries(stateEvents, processRoomEvent); - await Promise.mapSeries(timelineEvents, processRoomEvent); + stateEvents.forEach(processRoomEvent); + timelineEvents.forEach(processRoomEvent); + ephemeralEvents.forEach(function(e) { client.emit("event", e); });