diff --git a/spec/TestClient.js b/spec/TestClient.js index edd15df80f5..f7a584eb4ee 100644 --- a/spec/TestClient.js +++ b/spec/TestClient.js @@ -52,21 +52,21 @@ export default function TestClient(userId, deviceId, accessToken) { /** * start the client, and wait for it to initialise. * - * @param {object?} existingDevices the list of our existing devices to return from - * the /query request. Defaults to empty device list * @return {Promise} */ -TestClient.prototype.start = function(existingDevices) { +TestClient.prototype.start = function() { this.httpBackend.when("GET", "/pushrules").respond(200, {}); this.httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" }); - this.expectKeyUpload(existingDevices); + this.expectKeyUpload(); this.client.startClient({ // set this so that we can get hold of failed events pendingEventOrdering: 'detached', }); - return this.httpBackend.flush(); + return this.httpBackend.flush().then(() => { + console.log('TestClient[' + this.userId + ']: started'); + }); }; /** @@ -78,22 +78,9 @@ TestClient.prototype.stop = function() { /** * Set up expectations that the client will upload device and one-time keys. - * - * @param {object?} existingDevices the list of our existing devices to return from - * the /query request. Defaults to empty device list */ -TestClient.prototype.expectKeyUpload = function(existingDevices) { +TestClient.prototype.expectKeyUpload = function() { const self = this; - this.httpBackend.when('POST', '/keys/query').respond(200, function(path, content) { - expect(Object.keys(content.device_keys)).toEqual([self.userId]); - expect(content.device_keys[self.userId]).toEqual({}); - let res = existingDevices; - if (!res) { - res = { device_keys: {} }; - res.device_keys[self.userId] = {}; - } - return res; - }); this.httpBackend.when("POST", "/keys/upload").respond(200, function(path, content) { expect(content.one_time_keys).toBe(undefined); expect(content.device_keys).toBeTruthy(); @@ -111,6 +98,24 @@ TestClient.prototype.expectKeyUpload = function(existingDevices) { }); }; +/** + * Set up expectations that the client will query device keys. + * + * We check that the query contains each of the users in `response`. + * + * @param {Object} response response to the query. + */ +TestClient.prototype.expectKeyQuery = function(response) { + this.httpBackend.when('POST', '/keys/query').respond( + 200, (path, content) => { + Object.keys(response.device_keys).forEach((userId) => { + expect(content.device_keys[userId]).toEqual({}); + }); + return response; + }); +}; + + /** * get the uploaded curve25519 device key * diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index f25ec6725cd..96638539443 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -393,7 +393,9 @@ describe("MatrixClient crypto", function() { afterEach(function() { aliTestClient.stop(); + aliTestClient.httpBackend.verifyNoOutstandingExpectation(); bobTestClient.stop(); + bobTestClient.httpBackend.verifyNoOutstandingExpectation(); }); it('Ali knows the difference between a new user and one with no devices', @@ -620,16 +622,13 @@ describe("MatrixClient crypto", function() { .then(function() { aliTestClient.client.setDeviceBlocked(bobUserId, bobDeviceId, true); const p1 = sendMessage(aliTestClient.client); - const p2 = expectAliQueryKeys() - .then(expectAliClaimKeys) - .then(function() { - return expectSendMessageRequest(aliTestClient.httpBackend); - }).then(function(sentContent) { - // no unblocked devices, so the ciphertext should be empty - expect(sentContent.ciphertext).toEqual({}); - }); + const p2 = expectSendMessageRequest(aliTestClient.httpBackend) + .then(function(sentContent) { + // no unblocked devices, so the ciphertext should be empty + expect(sentContent.ciphertext).toEqual({}); + }); return q.all([p1, p2]); - }).catch(testUtils.failTest).nodeify(done); + }).nodeify(done); }); it("Bob receives two pre-key messages", function(done) { @@ -685,4 +684,44 @@ describe("MatrixClient crypto", function() { }).then(expectAliQueryKeys) .nodeify(done); }); + + it("Ali does a key query when encryption is enabled", function(done) { + // enabling encryption in the room should make alice download devices + // for both members. + q() + .then(() => startClient(aliTestClient)) + .then(() => { + const syncData = { + next_batch: '2', + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomId] = { + state: { + events: [ + testUtils.mkEvent({ + type: 'm.room.encryption', + skey: '', + content: { + algorithm: 'm.olm.v1.curve25519-aes-sha2', + }, + }), + ], + }, + }; + + aliTestClient.httpBackend.when('GET', '/sync').respond( + 200, syncData); + return aliTestClient.httpBackend.flush('/sync', 1); + }).then(() => { + aliTestClient.expectKeyQuery({ + device_keys: { + [aliUserId]: {}, + [bobUserId]: {}, + }, + }); + return aliTestClient.httpBackend.flush('/keys/query', 1); + }).nodeify(done); + }); }); diff --git a/spec/integ/matrix-client-methods.spec.js b/spec/integ/matrix-client-methods.spec.js index 31c4a3da00b..76a92ab9e62 100644 --- a/spec/integ/matrix-client-methods.spec.js +++ b/spec/integ/matrix-client-methods.spec.js @@ -351,7 +351,6 @@ describe("MatrixClient", function() { httpBackend.when("POST", "/keys/query").check(function(req) { expect(req.data).toEqual({device_keys: { - '@alice:localhost': {}, 'boris': {}, 'chaz': {}, }}); diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js index 099ab55286b..a692cdca61e 100644 --- a/spec/integ/megolm.spec.js +++ b/spec/integ/megolm.spec.js @@ -672,9 +672,7 @@ describe("megolm", function() { let inboundGroupSession; let decrypted; - return aliceTestClient.start( - getTestKeysQueryResponse(aliceTestClient.userId), - ).then(function() { + return aliceTestClient.start().then(function() { // an encrypted room with just alice const syncResponse = { next_batch: 1, @@ -701,6 +699,13 @@ describe("megolm", function() { }; aliceTestClient.httpBackend.when('GET', '/sync').respond(200, syncResponse); + // the completion of the first initialsync hould make Alice + // invalidate the device cache for all members in e2e rooms (ie, + // herself), and do a key query. + aliceTestClient.expectKeyQuery( + getTestKeysQueryResponse(aliceTestClient.userId), + ); + return aliceTestClient.httpBackend.flush(); }).then(function() { aliceTestClient.client.setDeviceKnown(aliceTestClient.userId, 'DEVICE_ID'); diff --git a/spec/mock-request.js b/spec/mock-request.js index bbd2e97f3d2..bf1a1b04e30 100644 --- a/spec/mock-request.js +++ b/spec/mock-request.js @@ -49,17 +49,17 @@ HttpBackend.prototype = { const tryFlush = function() { // if there's more real requests and more expected requests, flush 'em. console.log( - " trying to flush queue => reqs=%s expected=%s [%s]", - self.requests.length, self.expectedRequests.length, path, + " trying to flush queue => reqs=[%s] expected=[%s]", + self.requests, self.expectedRequests.map((er) => er.path), ); if (self._takeFromQueue(path)) { // try again on the next tick. - console.log(" flushed. Trying for more. [%s]", path); flushed += 1; if (numToFlush && flushed === numToFlush) { - console.log(" [%s] Flushed assigned amount: %s", path, numToFlush); + console.log(" Flushed assigned amount: %s", numToFlush); defer.resolve(); } else { + console.log(" flushed. Trying for more."); setTimeout(tryFlush, 0); } } else if (flushed === 0 && !triedWaiting) { @@ -68,7 +68,7 @@ HttpBackend.prototype = { setTimeout(tryFlush, 5); triedWaiting = true; } else { - console.log(" no more flushes. [%s]", path); + console.log(" no more flushes."); defer.resolve(); } }; diff --git a/src/crypto/DeviceList.js b/src/crypto/DeviceList.js index fc775cf5ff1..ebc0a59deec 100644 --- a/src/crypto/DeviceList.js +++ b/src/crypto/DeviceList.js @@ -64,7 +64,13 @@ export default class DeviceList { // just wait for the existing download to complete promises.push(this._keyDownloadsInProgressByUser[u]); } else { - if (forceDownload || !this.getStoredDevicesForUser(u)) { + if (forceDownload) { + console.log("Invalidating device list for " + u + + " for forceDownload"); + this.invalidateUserDeviceList(u); + } else if (!this.getStoredDevicesForUser(u)) { + console.log("Invalidating device list for " + u + + " due to empty cache"); this.invalidateUserDeviceList(u); } if (this._pendingUsersWithNewDevices[u]) { diff --git a/src/crypto/algorithms/olm.js b/src/crypto/algorithms/olm.js index b98dccde5dd..99fb64c2996 100644 --- a/src/crypto/algorithms/olm.js +++ b/src/crypto/algorithms/olm.js @@ -64,7 +64,7 @@ OlmEncryption.prototype._ensureSession = function(roomMembers) { } const self = this; - this._prepPromise = self._crypto.downloadKeys(roomMembers, true).then(function(res) { + this._prepPromise = self._crypto.downloadKeys(roomMembers).then(function(res) { return self._crypto.ensureOlmSessionsForUsers(roomMembers); }).then(function() { self._sessionPrepared = true; diff --git a/src/crypto/index.js b/src/crypto/index.js index 72db6662102..4f43eafb229 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -86,9 +86,6 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId, ); if (!myDevices) { - // we don't yet have a list of our own devices; make sure we - // get one when we flush the pendingUsersWithNewDevices. - this._deviceList.invalidateUserDeviceList(this._userId); myDevices = {}; } @@ -545,6 +542,23 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { config: 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. + } }; @@ -769,6 +783,8 @@ Crypto.prototype._onSyncCompleted = function(syncData) { } else { // otherwise, we have to invalidate all devices for all users we // share a room with. + console.log("Completed first initialsync; invalidating all " + + "device list caches"); this._invalidateDeviceListForAllActiveUsers(); } }