Skip to content

Upload device keys during initCrypto #2872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Dec 7, 2022
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