Skip to content

Commit

Permalink
Upload device keys during initCrypto (#2872)
Browse files Browse the repository at this point in the history
Rather than waiting for the application to call `.startClient`, upload the
device keys during `initCrypto()`. Element-R is going to approach this slightly
differently (it wants to manage the decision on key uploads itself), so this
lays some groundwork by collecting the libolm-specific bits together.
  • Loading branch information
richvdh authored Dec 7, 2022
1 parent 4a7365f commit a9e7a46
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 18 deletions.
13 changes: 9 additions & 4 deletions spec/integ/matrix-client-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ type OlmPayload = ReturnType<Session["encrypt"]>;

async function bobUploadsDeviceKeys(): Promise<void> {
bobTestClient.expectDeviceKeyUpload();
await Promise.all([
bobTestClient.client.uploadKeys(),
bobTestClient.httpBackend.flushAllExpected(),
]);
await bobTestClient.httpBackend.flushAllExpected();
expect(Object.keys(bobTestClient.deviceKeys!).length).not.toEqual(0);
}

Expand Down Expand Up @@ -383,6 +380,14 @@ describe("MatrixClient crypto", () => {

it("Bob uploads device keys", bobUploadsDeviceKeys);

it("handles failures to upload device keys", async () => {
// since device keys are uploaded asynchronously, there's not really much to do here other than fail the
// upload.
bobTestClient.httpBackend.when("POST", "/keys/upload")
.fail(0, new Error("bleh"));
await bobTestClient.httpBackend.flushAllExpected();
});

it("Ali downloads Bobs device keys", async () => {
await bobUploadsDeviceKeys();
await aliDownloadsKeys();
Expand Down
14 changes: 13 additions & 1 deletion spec/integ/matrix-client-methods.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,12 @@ describe("MatrixClient", function() {
}

beforeEach(function() {
return client!.initCrypto();
// running initCrypto should trigger a key upload
httpBackend!.when("POST", "/keys/upload").respond(200, {});
return Promise.all([
client!.initCrypto(),
httpBackend!.flush("/keys/upload", 1),
]);
});

afterEach(() => {
Expand Down Expand Up @@ -1371,6 +1376,13 @@ describe("MatrixClient", function() {
await prom;
});
});

describe("uploadKeys", () => {
// uploadKeys() is a no-op nowadays, so there's not much to test here.
it("should complete successfully", async () => {
await client!.uploadKeys();
});
});
});

function withThreadId(event: MatrixEvent, newThreadId: string): MatrixEvent {
Expand Down
8 changes: 7 additions & 1 deletion spec/unit/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,13 @@ describe("Crypto", function() {
});

client = new TestClient("@alice:example.org", "aliceweb");
await client.client.initCrypto();

// running initCrypto should trigger a key upload
client.httpBackend.when("POST", "/keys/upload").respond(200, {});
await Promise.all([
client.client.initCrypto(),
client.httpBackend.flush("/keys/upload", 1),
]);

encryptedPayload = {
algorithm: "m.olm.v1.curve25519-aes-sha2",
Expand Down
11 changes: 10 additions & 1 deletion spec/unit/crypto/backup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ function makeTestClient(cryptoStore: CryptoStore) {
const scheduler = makeTestScheduler();
const store = new StubStore();

return new MatrixClient({
const client = new MatrixClient({
baseUrl: "https://my.home.server",
idBaseUrl: "https://identity.server",
accessToken: "my.access.token",
Expand All @@ -147,6 +147,10 @@ function makeTestClient(cryptoStore: CryptoStore) {
cryptoStore: cryptoStore,
cryptoCallbacks: { getCrossSigningKey, saveCrossSigningKeys },
});

// initialising the crypto library will trigger a key upload request, which we can stub out
client.uploadKeysRequest = jest.fn();
return client;
}

describe("MegolmBackup", function() {
Expand Down Expand Up @@ -509,6 +513,8 @@ describe("MegolmBackup", function() {
deviceId: "device",
cryptoStore: cryptoStore,
});
// initialising the crypto library will trigger a key upload request, which we can stub out
client.uploadKeysRequest = jest.fn();

megolmDecryption = new MegolmDecryption({
userId: '@user:id',
Expand Down Expand Up @@ -724,6 +730,9 @@ describe("MegolmBackup", function() {
deviceId: "device",
cryptoStore,
});
// initialising the crypto library will trigger a key upload request, which we can stub out
client.uploadKeysRequest = jest.fn();

await client.initCrypto();

cryptoStore.countSessionsNeedingBackup = jest.fn().mockReturnValue(6);
Expand Down
19 changes: 8 additions & 11 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1210,10 +1210,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
this.store.storeUser(new User(userId));
}

if (this.crypto) {
this.crypto.uploadDeviceKeys();
}

// periodically poll for turn servers if we support voip
if (this.canSupportVoip) {
this.checkTurnServersIntervalID = setInterval(() => {
Expand Down Expand Up @@ -1864,6 +1860,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// if crypto initialisation was successful, tell it to attach its event handlers.
crypto.registerEventHandlers(this as Parameters<Crypto["registerEventHandlers"]>[0]);
this.crypto = crypto;

// upload our keys in the background
this.crypto.uploadDeviceKeys().catch((e) => {
// TODO: throwing away this error is a really bad idea.
logger.error("Error uploading device keys", e);
});
}

/**
Expand Down Expand Up @@ -1895,15 +1897,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
}

/**
* Upload the device keys to the homeserver.
* @return {Promise<void>} A promise that will resolve when the keys are uploaded.
* @deprecated Does nothing.
*/
public async uploadKeys(): Promise<void> {
if (!this.crypto) {
throw new Error("End-to-end encryption disabled");
}

await this.crypto.uploadDeviceKeys();
logger.warn("MatrixClient.uploadKeys is deprecated");
}

/**
Expand Down

0 comments on commit a9e7a46

Please sign in to comment.