Skip to content

Commit

Permalink
Rework device list tracking logic (#425)
Browse files Browse the repository at this point in the history 
Yet another attempt at fixing
element-hq/element-web#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
  element-hq/element-web#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 element-hq/element-web#3310.
  • Loading branch information
@richvdh
richvdh authored Apr 25, 2017
1 parent 9693c30 commit 5020d4e
Show file tree
Hide file tree
Showing 6 changed files with 331 additions and 271 deletions.
1 change: 1 addition & 0 deletions spec/TestClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
44 changes: 21 additions & 23 deletions spec/integ/matrix-client-crypto.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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',
Expand Down
77 changes: 29 additions & 48 deletions spec/integ/megolm.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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;

Expand Down
Loading

0 comments on commit 5020d4e

Please sign in to comment.