From a8bf66d8af74ff0f237f2d172e9cf28109ec6502 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 6 Aug 2018 17:18:36 +0200 Subject: [PATCH] Make Room.getEncryptionTargetMembers async, as members might be loading --- src/crypto/algorithms/megolm.js | 38 ++++++++-------- src/crypto/algorithms/olm.js | 78 +++++++++++++++++---------------- src/crypto/index.js | 10 ++--- src/models/room.js | 9 ++-- 4 files changed, 70 insertions(+), 65 deletions(-) diff --git a/src/crypto/algorithms/megolm.js b/src/crypto/algorithms/megolm.js index d8a807092..a4eb32ab0 100644 --- a/src/crypto/algorithms/megolm.js +++ b/src/crypto/algorithms/megolm.js @@ -535,8 +535,9 @@ MegolmEncryption.prototype._checkForUnknownDevices = function(devicesInRoom) { * @return {module:client.Promise} Promise which resolves to a map * from userId to deviceId to deviceInfo */ -MegolmEncryption.prototype._getDevicesInRoom = function(room) { - const roomMembers = utils.map(room.getEncryptionTargetMembers(), function(u) { +MegolmEncryption.prototype._getDevicesInRoom = async function(room) { + const members = await room.getEncryptionTargetMembers(); + const roomMembers = utils.map(members, function(u) { return u.userId; }); @@ -555,29 +556,28 @@ MegolmEncryption.prototype._getDevicesInRoom = function(room) { // common and then added new devices before joining this one? --Matthew // // yup, see https://github.com/vector-im/riot-web/issues/2305 --richvdh - return this._crypto.downloadKeys(roomMembers, false).then((devices) => { - // remove any blocked devices - for (const userId in devices) { - if (!devices.hasOwnProperty(userId)) { + const devices = await this._crypto.downloadKeys(roomMembers, false); + // remove any blocked devices + for (const userId in devices) { + if (!devices.hasOwnProperty(userId)) { + continue; + } + + const userDevices = devices[userId]; + for (const deviceId in userDevices) { + if (!userDevices.hasOwnProperty(deviceId)) { continue; } - const userDevices = devices[userId]; - for (const deviceId in userDevices) { - if (!userDevices.hasOwnProperty(deviceId)) { - continue; - } - - if (userDevices[deviceId].isBlocked() || - (userDevices[deviceId].isUnverified() && isBlacklisting) - ) { - delete userDevices[deviceId]; - } + if (userDevices[deviceId].isBlocked() || + (userDevices[deviceId].isUnverified() && isBlacklisting) + ) { + delete userDevices[deviceId]; } } + } - return devices; - }); + return devices; }; /** diff --git a/src/crypto/algorithms/olm.js b/src/crypto/algorithms/olm.js index 26a4d90f8..69390b064 100644 --- a/src/crypto/algorithms/olm.js +++ b/src/crypto/algorithms/olm.js @@ -83,60 +83,62 @@ OlmEncryption.prototype._ensureSession = function(roomMembers) { * * @return {module:client.Promise} Promise which resolves to the new event body */ -OlmEncryption.prototype.encryptMessage = function(room, eventType, content) { +OlmEncryption.prototype.encryptMessage = async function(room, eventType, content) { // pick the list of recipients based on the membership list. // // TODO: there is a race condition here! What if a new user turns up // just as you are sending a secret message? - const users = utils.map(room.getEncryptionTargetMembers(), function(u) { + const members = await room.getEncryptionTargetMembers(); + + const users = utils.map(members, function(u) { return u.userId; }); const self = this; - return this._ensureSession(users).then(function() { - const payloadFields = { - room_id: room.roomId, - type: eventType, - content: content, - }; + await this._ensureSession(users); - const encryptedContent = { - algorithm: olmlib.OLM_ALGORITHM, - sender_key: self._olmDevice.deviceCurve25519Key, - ciphertext: {}, - }; + const payloadFields = { + room_id: room.roomId, + type: eventType, + content: content, + }; - const promises = []; + const encryptedContent = { + algorithm: olmlib.OLM_ALGORITHM, + sender_key: self._olmDevice.deviceCurve25519Key, + ciphertext: {}, + }; - for (let i = 0; i < users.length; ++i) { - const userId = users[i]; - const devices = self._crypto.getStoredDevicesForUser(userId); + const promises = []; - for (let j = 0; j < devices.length; ++j) { - const deviceInfo = devices[j]; - const key = deviceInfo.getIdentityKey(); - if (key == self._olmDevice.deviceCurve25519Key) { - // don't bother sending to ourself - continue; - } - if (deviceInfo.verified == DeviceVerification.BLOCKED) { - // don't bother setting up sessions with blocked users - continue; - } + for (let i = 0; i < users.length; ++i) { + const userId = users[i]; + const devices = self._crypto.getStoredDevicesForUser(userId); - promises.push( - olmlib.encryptMessageForDevice( - encryptedContent.ciphertext, - self._userId, self._deviceId, self._olmDevice, - userId, deviceInfo, payloadFields, - ), - ); + for (let j = 0; j < devices.length; ++j) { + const deviceInfo = devices[j]; + const key = deviceInfo.getIdentityKey(); + if (key == self._olmDevice.deviceCurve25519Key) { + // don't bother sending to ourself + continue; + } + if (deviceInfo.verified == DeviceVerification.BLOCKED) { + // don't bother setting up sessions with blocked users + continue; } - } - return Promise.all(promises).return(encryptedContent); - }); + promises.push( + olmlib.encryptMessageForDevice( + encryptedContent.ciphertext, + self._userId, self._deviceId, self._olmDevice, + userId, deviceInfo, payloadFields, + ), + ); + } + } + + return await Promise.all(promises).return(encryptedContent); }; /** diff --git a/src/crypto/index.js b/src/crypto/index.js index 273dcce70..8b35bc0a4 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -652,7 +652,7 @@ Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDevic throw new Error(`Unable to enable encryption in unknown room ${roomId}`); } - const members = room.getEncryptionTargetMembers(); + const members = await room.getEncryptionTargetMembers(); members.forEach((m) => { this._deviceList.startTrackingDeviceList(m.userId); }); @@ -852,7 +852,7 @@ Crypto.prototype.handleDeviceListChanges = async function(syncData, syncDeviceLi // If we didn't make this assumption, we'd have to use the /keys/changes API // to get key changes between the sync token in the device list and the 'old' // sync token used here to make sure we didn't miss any. - this._evalDeviceListChanges(syncDeviceLists); + await this._evalDeviceListChanges(syncDeviceLists); }; /** @@ -968,7 +968,7 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) { // 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(this._getE2eUsers()); + const e2eUserIds = new Set(await this._getE2eUsers()); deviceLists.left.forEach((u) => { if (!e2eUserIds.has(u)) { @@ -983,10 +983,10 @@ Crypto.prototype._evalDeviceListChanges = async function(deviceLists) { * * @returns {string[]} List of user IDs */ -Crypto.prototype._getE2eUsers = function() { +Crypto.prototype._getE2eUsers = async function() { const e2eUserIds = []; for (const room of this._getE2eRooms()) { - const members = room.getEncryptionTargetMembers(); + const members = await room.getEncryptionTargetMembers(); for (const member of members) { e2eUserIds.push(member.userId); } diff --git a/src/models/room.js b/src/models/room.js index 7d6449fd1..ba0fc5b8c 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -569,10 +569,13 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline, /** * Get a list of members we should be encrypting for in this room - * @return {RoomMember[]} A list of members who we should encrypt messages for - * in this room. + * @return {Promise} A list of members who + * we should encrypt messages for in this room. */ - Room.prototype.getEncryptionTargetMembers = function() { + Room.prototype.getEncryptionTargetMembers = async function() { + if (this._oobMembersPromise) { + await _oobMembersPromise; + } let members = this.getMembersWithMembership("join"); if (this.shouldEncryptForInvitedMembers()) { members = members.concat(this.getMembersWithMembership("invite"));