diff --git a/spec/unit/room.spec.js b/spec/unit/room.spec.js index b98559ecf..eb5ba5a51 100644 --- a/spec/unit/room.spec.js +++ b/spec/unit/room.spec.js @@ -1318,9 +1318,6 @@ 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 eb5b469c6..5bb437fb7 100644 --- a/src/crypto/RoomList.js +++ b/src/crypto/RoomList.js @@ -71,9 +71,6 @@ 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 c512b514e..8b35bc0a4 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -106,10 +106,6 @@ 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); @@ -616,8 +612,11 @@ 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) { +Crypto.prototype.setRoomEncryption = async 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._roomList.getRoomEncryption(roomId); @@ -645,42 +644,24 @@ Crypto.prototype.setRoomEncryption = async function(roomId, config) { }); this._roomEncryptors[roomId] = alg; - 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; + // 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(); } - return promise; }; + /** * @typedef {Object} module:crypto~OlmSessionResult * @property {module:crypto/deviceinfo} device device info @@ -771,7 +752,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. * @@ -782,8 +763,7 @@ Crypto.prototype.importRoomKeys = function(keys) { * @return {module:client.Promise?} Promise which resolves when the event has been * encrypted, or null if nothing was needed */ -/* eslint-enable valid-jsdoc */ -Crypto.prototype.encryptEvent = async function(event, room) { +Crypto.prototype.encryptEvent = function(event, room) { if (!room) { throw new Error("Cannot send encrypted messages in unknown rooms"); } @@ -801,12 +781,6 @@ Crypto.prototype.encryptEvent = async 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 @@ -817,19 +791,20 @@ Crypto.prototype.encryptEvent = async function(event, room) { delete content['m.relates_to']; } - const encryptedContent = await alg.encryptMessage( - room, event.getType(), content); + return alg.encryptMessage( + room, event.getType(), content, + ).then((encryptedContent) => { + if (mRelatesTo) { + encryptedContent['m.relates_to'] = mRelatesTo; + } - if (mRelatesTo) { - encryptedContent['m.relates_to'] = mRelatesTo; - } - - event.makeEncrypted( - "m.room.encrypted", - encryptedContent, - this._olmDevice.deviceCurve25519Key, - this._olmDevice.deviceEd25519Key, - ); + event.makeEncrypted( + "m.room.encrypted", + encryptedContent, + this._olmDevice.deviceCurve25519Key, + this._olmDevice.deviceEd25519Key, + ); + }); }; /** @@ -922,7 +897,9 @@ Crypto.prototype.onCryptoEvent = async function(event) { const content = event.getContent(); try { - await this.setRoomEncryption(roomId, content); + // 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); } catch (e) { console.error("Error configuring encryption in room " + roomId + ":", e); @@ -942,7 +919,6 @@ 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 = {}; } }; @@ -988,12 +964,11 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) { }); } - if (deviceLists.left && Array.isArray(deviceLists.left) && - deviceLists.left.length) { + if (deviceLists.left && Array.isArray(deviceLists.left)) { // 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._getTrackedE2eUsers()); + const e2eUserIds = new Set(await this._getE2eUsers()); deviceLists.left.forEach((u) => { if (!e2eUserIds.has(u)) { @@ -1005,13 +980,12 @@ 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._getTrackedE2eUsers = async function() { +Crypto.prototype._getE2eUsers = async function() { const e2eUserIds = []; - for (const room of this._getTrackedE2eRooms()) { + for (const room of this._getE2eRooms()) { const members = await room.getEncryptionTargetMembers(); for (const member of members) { e2eUserIds.push(member.userId); @@ -1021,21 +995,17 @@ Crypto.prototype._getTrackedE2eUsers = async function() { }; /** - * Get a list of the e2e-enabled rooms we are members of, - * and for which we are already tracking the devices + * Get a list of the e2e-enabled rooms we are members of * * @returns {module:models.Room[]} */ -Crypto.prototype._getTrackedE2eRooms = function() { +Crypto.prototype._getE2eRooms = 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(); @@ -1104,20 +1074,15 @@ Crypto.prototype._onRoomMembership = function(event, member, oldMembership) { // not encrypting in this room return; } - // 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); - } + + 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 dc1052ea4..94115ef45 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -368,11 +368,6 @@ 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 d8dffe6b3..4aa2d2482 100644 --- a/src/sync.js +++ b/src/sync.js @@ -1085,16 +1085,15 @@ SyncApi.prototype._processSyncResponse = async function( self._processEventsForNotifs(room, timelineEvents); - function processRoomEvent(e) { + async function processRoomEvent(e) { client.emit("event", e); if (e.isState() && e.getType() == "m.room.encryption" && self.opts.crypto) { - self.opts.crypto.onCryptoEvent(e); + await self.opts.crypto.onCryptoEvent(e); } } - stateEvents.forEach(processRoomEvent); - timelineEvents.forEach(processRoomEvent); - + await Promise.mapSeries(stateEvents, processRoomEvent); + await Promise.mapSeries(timelineEvents, processRoomEvent); ephemeralEvents.forEach(function(e) { client.emit("event", e); });