From 439f96ce45fbd5ef8e339a56c68b552621f0ff28 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 14:58:41 +0100 Subject: [PATCH 1/7] test(crypto): remove `newBackendOnly` test closure --- .eslintrc.cjs | 2 +- spec/integ/crypto/cross-signing.spec.ts | 8 +- spec/integ/crypto/crypto.spec.ts | 267 +++++++++++------------- spec/integ/crypto/megolm-backup.spec.ts | 45 ++-- spec/integ/crypto/verification.spec.ts | 71 +++---- 5 files changed, 174 insertions(+), 219 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 7bad5c8177a..86aebd68cb3 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -91,7 +91,7 @@ module.exports = { "jest/no-standalone-expect": [ "error", { - additionalTestBlockFunctions: ["beforeAll", "beforeEach", "oldBackendOnly", "newBackendOnly"], + additionalTestBlockFunctions: ["beforeAll", "beforeEach", "oldBackendOnly"], }, ], }, diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index 840970aebc2..f046fc2f0ba 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -56,10 +56,6 @@ const TEST_DEVICE_ID = "xzcvb"; * to provide the most effective integration tests possible. */ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: string, initCrypto: InitCrypto) => { - // newBackendOnly is the opposite to `oldBackendOnly`: it will skip the test if we are running against the legacy - // backend. Once we drop support for legacy crypto, it will go away. - const newBackendOnly = backend === "rust-sdk" ? test : test.skip; - let aliceClient: MatrixClient; /** an object which intercepts `/sync` requests from {@link #aliceClient} */ @@ -163,7 +159,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s ); }); - newBackendOnly("get cross signing keys from secret storage and import them", async () => { + it("get cross signing keys from secret storage and import them", async () => { // Return public cross signing keys e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA); @@ -264,7 +260,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s expect(calls.length).toEqual(0); }); - newBackendOnly("will upload existing cross-signing keys to an established secret storage", async () => { + it("will upload existing cross-signing keys to an established secret storage", async () => { // This rather obscure codepath covers the case that: // - 4S is set up and working // - our device has private cross-signing keys, but has not published them to 4S diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index b241975af70..cc3c3ffb1ea 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -222,7 +222,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // oldBackendOnly is an alternative to `it` or `test` which will skip the test if we are running against the // Rust backend. Once we have full support in the rust sdk, it will go away. const oldBackendOnly = backend === "rust-sdk" ? test.skip : test; - const newBackendOnly = backend !== "rust-sdk" ? test.skip : test; const Olm = globalThis.Olm; @@ -548,7 +547,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, return await awaitDecryption; } - newBackendOnly("fails with HISTORICAL_MESSAGE_BACKUP_NO_BACKUP when there is no backup", async () => { + it("fails with HISTORICAL_MESSAGE_BACKUP_NO_BACKUP when there is no backup", async () => { fetchMock.get("path:/_matrix/client/v3/room_keys/version", { status: 404, body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, @@ -560,7 +559,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); }); - newBackendOnly("fails with HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED when the backup is broken", async () => { + it("fails with HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED when the backup is broken", async () => { fetchMock.get("path:/_matrix/client/v3/room_keys/version", {}); expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); await startClientAndAwaitFirstSync(); @@ -571,7 +570,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, ); }); - newBackendOnly("fails with HISTORICAL_MESSAGE_WORKING_BACKUP when backup is working", async () => { + it("fails with HISTORICAL_MESSAGE_WORKING_BACKUP when backup is working", async () => { // The test backup data is signed by a dummy device. We'll need to tell Alice about the device, and // later, tell her to trust it, so that she trusts the backup. const e2eResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl()); @@ -604,7 +603,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP); }); - newBackendOnly("fails with NOT_JOINED if user is not member of room", async () => { + it("fails with NOT_JOINED if user is not member of room", async () => { fetchMock.get("path:/_matrix/client/v3/room_keys/version", { status: 404, body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, @@ -620,148 +619,125 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED); }); - newBackendOnly( - "fails with NOT_JOINED if user is not member of room (MSC4115 unstable prefix)", - async () => { - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, - }); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); + it("fails with NOT_JOINED if user is not member of room (MSC4115 unstable prefix)", async () => { + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); - const ev = await sendEventAndAwaitDecryption({ - unsigned: { - [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "leave", - }, - }); - expect(ev.decryptionFailureReason).toEqual( - DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED, - ); - }, - ); + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "leave", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED); + }); - newBackendOnly( - "fails with another error when the server reports user was a member of the room", - async () => { - // This tests that when the server reports that the user - // was invited at the time the event was sent, then we - // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, - // and instead get some other error, since the user should - // have gotten the key for the event. - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, - }); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); + it("fails with another error when the server reports user was a member of the room", async () => { + // This tests that when the server reports that the user + // was invited at the time the event was sent, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, + // and instead get some other error, since the user should + // have gotten the key for the event. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); - const ev = await sendEventAndAwaitDecryption({ - unsigned: { - [UNSIGNED_MEMBERSHIP_FIELD.name]: "invite", - }, - }); - expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); - }, - ); + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.name]: "invite", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }); - newBackendOnly( - "fails with another error when the server reports user was a member of the room (MSC4115 unstable prefix)", - async () => { - // This tests that when the server reports that the user - // was invited at the time the event was sent, then we - // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, - // and instead get some other error, since the user should - // have gotten the key for the event. - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, - }); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); + it("fails with another error when the server reports user was a member of the room (MSC4115 unstable prefix)", async () => { + // This tests that when the server reports that the user + // was invited at the time the event was sent, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, + // and instead get some other error, since the user should + // have gotten the key for the event. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); - const ev = await sendEventAndAwaitDecryption({ - unsigned: { - [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "invite", - }, - }); - expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); - }, - ); + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "invite", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }); - newBackendOnly( - "fails with another error when the server reports user was a member of the room", - async () => { - // This tests that when the server reports the user's - // membership, and reports that the user was joined, then we - // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, and - // instead get some other error. - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, - }); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); + it("fails with another error when the server reports user was a member of the room", async () => { + // This tests that when the server reports the user's + // membership, and reports that the user was joined, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, and + // instead get some other error. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); - const ev = await sendEventAndAwaitDecryption({ - unsigned: { - [UNSIGNED_MEMBERSHIP_FIELD.name]: "join", - }, - }); - expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); - }, - ); + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.name]: "join", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }); - newBackendOnly( - "fails with another error when the server reports user was a member of the room (MSC4115 unstable prefix)", - async () => { - // This tests that when the server reports the user's - // membership, and reports that the user was joined, then we - // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, and - // instead get some other error. - fetchMock.get("path:/_matrix/client/v3/room_keys/version", { - status: 404, - body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, - }); - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); + it("fails with another error when the server reports user was a member of the room (MSC4115 unstable prefix)", async () => { + // This tests that when the server reports the user's + // membership, and reports that the user was joined, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, and + // instead get some other error. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); - const ev = await sendEventAndAwaitDecryption({ - unsigned: { - [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "join", - }, - }); - expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); - }, - ); + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.altName!]: "join", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }); }); describe("IsolationMode decryption tests", () => { - newBackendOnly( - "OnlySigned mode - fails with an error when cross-signed sender is required but sender is not cross-signed", - async () => { - const decryptedEvent = await setUpTestAndDecrypt(new OnlySignedDevicesIsolationMode()); - - // It will error as an unknown device because we haven't fetched - // the sender's device keys. - expect(decryptedEvent.isDecryptionFailure()).toBe(true); - expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE); - }, - ); + it("OnlySigned mode - fails with an error when cross-signed sender is required but sender is not cross-signed", async () => { + const decryptedEvent = await setUpTestAndDecrypt(new OnlySignedDevicesIsolationMode()); - newBackendOnly( - "NoIsolation mode - Decrypts with warning when cross-signed sender is required but sender is not cross-signed", - async () => { - const decryptedEvent = await setUpTestAndDecrypt(new AllDevicesIsolationMode(false)); + // It will error as an unknown device because we haven't fetched + // the sender's device keys. + expect(decryptedEvent.isDecryptionFailure()).toBe(true); + expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE); + }); - expect(decryptedEvent.isDecryptionFailure()).toBe(false); + it("NoIsolation mode - Decrypts with warning when cross-signed sender is required but sender is not cross-signed", async () => { + const decryptedEvent = await setUpTestAndDecrypt(new AllDevicesIsolationMode(false)); - expect(await aliceClient.getCrypto()!.getEncryptionInfoForEvent(decryptedEvent)).toEqual({ - shieldColour: EventShieldColour.RED, - shieldReason: EventShieldReason.UNKNOWN_DEVICE, - }); - }, - ); + expect(decryptedEvent.isDecryptionFailure()).toBe(false); + + expect(await aliceClient.getCrypto()!.getEncryptionInfoForEvent(decryptedEvent)).toEqual({ + shieldColour: EventShieldColour.RED, + shieldReason: EventShieldReason.UNKNOWN_DEVICE, + }); + }); async function setUpTestAndDecrypt(isolationMode: DeviceIsolationMode): Promise { // This tests that a message will not be decrypted if the sender @@ -1086,7 +1062,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, return encryptedMessage; } - newBackendOnly("should rotate the session after 2 messages", async () => { + it("should rotate the session after 2 messages", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); await startClientAndAwaitFirstSync(); const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount); @@ -1133,7 +1109,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(thirdSessionId).not.toBe(sessionId); }); - newBackendOnly("should rotate the session after 1h", async () => { + it("should rotate the session after 1h", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); await startClientAndAwaitFirstSync(); const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount); @@ -1186,7 +1162,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); }); - newBackendOnly("should rotate the session when the history visibility changes", async () => { + it("should rotate the session when the history visibility changes", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); await startClientAndAwaitFirstSync(); const p2pSession = await establishOlmSession(aliceClient, keyReceiver, syncResponder, testOlmAccount); @@ -2102,16 +2078,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, describe("bootstrapSecretStorage", () => { // Doesn't work with legacy crypto, which will try to bootstrap even without private key, which is buggy. - newBackendOnly( - "should throw an error if we are unable to create a key because createSecretStorageKey is not set", - async () => { - await expect( - aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }), - ).rejects.toThrow("unable to create a new secret storage key, createSecretStorageKey is not set"); - - expect(await aliceClient.getCrypto()!.isSecretStorageReady()).toStrictEqual(false); - }, - ); + it("should throw an error if we are unable to create a key because createSecretStorageKey is not set", async () => { + await expect( + aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }), + ).rejects.toThrow("unable to create a new secret storage key, createSecretStorageKey is not set"); + + expect(await aliceClient.getCrypto()!.isSecretStorageReady()).toStrictEqual(false); + }); it("Should create a 4S key", async () => { accountDataAccumulator.interceptGetAccountData(); @@ -2257,7 +2230,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(signatures![aliceClient.getUserId()!][`ed25519:${mskId}`]).toBeDefined(); }); - newBackendOnly("should upload existing megolm backup key to a new 4S store", async () => { + it("should upload existing megolm backup key to a new 4S store", async () => { const backupKeyTo4SPromise = awaitMegolmBackupKeyUpload(); // we need these to set up the mocks but we don't actually care whether they @@ -2476,7 +2449,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(verificationStatus.needsUserApproval).toBe(false); }); - newBackendOnly("An unverified user changes identity", async () => { + it("An unverified user changes identity", async () => { // We have to be tracking Bob's keys, which means we need to share a room with him syncResponder.sendOrQueueSyncResponse({ ...getSyncResponse([BOB_TEST_USER_ID]), diff --git a/spec/integ/crypto/megolm-backup.spec.ts b/spec/integ/crypto/megolm-backup.spec.ts index 7a20d4e6904..2e39c6d0334 100644 --- a/spec/integ/crypto/megolm-backup.spec.ts +++ b/spec/integ/crypto/megolm-backup.spec.ts @@ -114,10 +114,6 @@ function mockUploadEmitter( } describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backend: string, initCrypto: InitCrypto) => { - // oldBackendOnly is an alternative to `it` or `test` which will skip the test if we are running against the - // Rust backend. Once we have full support in the rust sdk, it will go away. - const newBackendOnly = backend === "libolm" ? test.skip : test; - const isNewBackend = backend === "rust-sdk"; let aliceClient: MatrixClient; @@ -530,33 +526,30 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe expect(result.imported).toStrictEqual(expectedTotal - decryptionFailureCount); }); - newBackendOnly( - "Should get the decryption key from the secret storage and restore the key backup", - async function () { - // @ts-ignore - mock a private method for testing purpose - jest.spyOn(aliceCrypto.secretStorage, "get").mockResolvedValue(testData.BACKUP_DECRYPTION_KEY_BASE64); + it("Should get the decryption key from the secret storage and restore the key backup", async function () { + // @ts-ignore - mock a private method for testing purpose + jest.spyOn(aliceCrypto.secretStorage, "get").mockResolvedValue(testData.BACKUP_DECRYPTION_KEY_BASE64); - const fullBackup = { - rooms: { - [ROOM_ID]: { - sessions: { - [testData.MEGOLM_SESSION_DATA.session_id]: testData.CURVE25519_KEY_BACKUP_DATA, - }, + const fullBackup = { + rooms: { + [ROOM_ID]: { + sessions: { + [testData.MEGOLM_SESSION_DATA.session_id]: testData.CURVE25519_KEY_BACKUP_DATA, }, }, - }; - fetchMock.get("express:/_matrix/client/v3/room_keys/keys", fullBackup); + }, + }; + fetchMock.get("express:/_matrix/client/v3/room_keys/keys", fullBackup); - await aliceCrypto.loadSessionBackupPrivateKeyFromSecretStorage(); - const decryptionKey = await aliceCrypto.getSessionBackupPrivateKey(); - expect(encodeBase64(decryptionKey!)).toStrictEqual(testData.BACKUP_DECRYPTION_KEY_BASE64); + await aliceCrypto.loadSessionBackupPrivateKeyFromSecretStorage(); + const decryptionKey = await aliceCrypto.getSessionBackupPrivateKey(); + expect(encodeBase64(decryptionKey!)).toStrictEqual(testData.BACKUP_DECRYPTION_KEY_BASE64); - const result = await aliceCrypto.restoreKeyBackup(); - expect(result.imported).toStrictEqual(1); - }, - ); + const result = await aliceCrypto.restoreKeyBackup(); + expect(result.imported).toStrictEqual(1); + }); - newBackendOnly("Should throw an error if the decryption key is not found in cache", async () => { + it("Should throw an error if the decryption key is not found in cache", async () => { await expect(aliceCrypto.restoreKeyBackup()).rejects.toThrow("No decryption key found in crypto store"); }); }); @@ -865,7 +858,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe expect(backupStatus).toStrictEqual(testData.SIGNED_BACKUP_DATA.version); }); - newBackendOnly("getKeyBackupInfo() should not return a backup if the active backup has been deleted", async () => { + it("getKeyBackupInfo() should not return a backup if the active backup has been deleted", async () => { // 404 means that there is no active backup fetchMock.get("express:/_matrix/client/v3/room_keys/version", 404); fetchMock.delete(`express:/_matrix/client/v3/room_keys/version/${testData.SIGNED_BACKUP_DATA.version}`, {}); diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index 47d92d3e96e..0b69d4121a5 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -118,10 +118,6 @@ const TEST_HOMESERVER_URL = "https://alice-server.com"; */ // we test with both crypto stacks... describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: string, initCrypto: InitCrypto) => { - // newBackendOnly is the opposite to `oldBackendOnly`: it will skip the test if we are running against the legacy - // backend. Once we drop support for legacy crypto, it will go away. - const newBackendOnly = backend === "rust-sdk" ? test : test.skip; - /** the client under test */ let aliceClient: MatrixClient; @@ -568,7 +564,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(qrCodeBuffer).toBeUndefined(); }); - newBackendOnly("can verify another by scanning their QR code", async () => { + it("can verify another by scanning their QR code", async () => { aliceClient = await startTestClient(); // we need cross-signing keys for a QR code verification e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA); @@ -1149,43 +1145,40 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(request?.otherUserId).toBe("@bob:xyz"); }); - newBackendOnly( - "If the verification request is not decrypted within 5 minutes, the request is ignored", - async () => { - const p2pSession = await createOlmSession(testOlmAccount, e2eKeyReceiver); - const groupSession = new Olm.OutboundGroupSession(); - groupSession.create(); + it("If the verification request is not decrypted within 5 minutes, the request is ignored", async () => { + const p2pSession = await createOlmSession(testOlmAccount, e2eKeyReceiver); + const groupSession = new Olm.OutboundGroupSession(); + groupSession.create(); - // make the room_key event, but don't send it yet - const toDeviceEvent = encryptGroupSessionKeyForAlice(groupSession, p2pSession); + // make the room_key event, but don't send it yet + const toDeviceEvent = encryptGroupSessionKeyForAlice(groupSession, p2pSession); - // Add verification request from Bob to Alice in the DM between them - returnRoomMessageFromSync(TEST_ROOM_ID, createEncryptedVerificationRequest(groupSession)); + // Add verification request from Bob to Alice in the DM between them + returnRoomMessageFromSync(TEST_ROOM_ID, createEncryptedVerificationRequest(groupSession)); - // Wait for the sync response to be processed - await syncPromise(aliceClient); + // Wait for the sync response to be processed + await syncPromise(aliceClient); - const room = aliceClient.getRoom(TEST_ROOM_ID)!; - const matrixEvent = room.getLiveTimeline().getEvents()[0]; + const room = aliceClient.getRoom(TEST_ROOM_ID)!; + const matrixEvent = room.getLiveTimeline().getEvents()[0]; - // wait for a first attempt at decryption: should fail - await awaitDecryption(matrixEvent); - expect(matrixEvent.getContent().msgtype).toEqual("m.bad.encrypted"); + // wait for a first attempt at decryption: should fail + await awaitDecryption(matrixEvent); + expect(matrixEvent.getContent().msgtype).toEqual("m.bad.encrypted"); - // Advance time by 5mins, the verification request should be ignored after that - jest.advanceTimersByTime(5 * 60 * 1000); + // Advance time by 5mins, the verification request should be ignored after that + jest.advanceTimersByTime(5 * 60 * 1000); - // Send Bob the room keys - returnToDeviceMessageFromSync(toDeviceEvent); + // Send Bob the room keys + returnToDeviceMessageFromSync(toDeviceEvent); - // Wait for the message to be decrypted - await awaitDecryption(matrixEvent, { waitOnDecryptionFailure: true }); + // Wait for the message to be decrypted + await awaitDecryption(matrixEvent, { waitOnDecryptionFailure: true }); - const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); - // the request should not be present - expect(request).not.toBeDefined(); - }, - ); + const request = aliceClient.getCrypto()!.findVerificationRequestDMInProgress(TEST_ROOM_ID, "@bob:xyz"); + // the request should not be present + expect(request).not.toBeDefined(); + }); }); describe("Secrets are gossiped after verification", () => { @@ -1257,7 +1250,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st fetchMock.mockReset(); }); - newBackendOnly("Should request cross signing keys after verification", async () => { + it("Should request cross signing keys after verification", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); @@ -1268,7 +1261,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st await requestPromises.get("m.cross_signing.self_signing"); }); - newBackendOnly("Should accept the backup decryption key gossip if valid", async () => { + it("Should accept the backup decryption key gossip if valid", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); @@ -1287,7 +1280,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(encodeBase64(cachedKey!)).toEqual(BACKUP_DECRYPTION_KEY_BASE64); }); - newBackendOnly("Should not accept the backup decryption key gossip if private key do not match", async () => { + it("Should not accept the backup decryption key gossip if private key do not match", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); @@ -1308,7 +1301,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(cachedKey).toBeNull(); }); - newBackendOnly("Should not accept the backup decryption key gossip if backup not trusted", async () => { + it("Should not accept the backup decryption key gossip if backup not trusted", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); @@ -1332,7 +1325,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(cachedKey).toBeNull(); }); - newBackendOnly("Should not accept the backup decryption key gossip if backup algorithm unknown", async () => { + it("Should not accept the backup decryption key gossip if backup algorithm unknown", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); @@ -1357,7 +1350,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st expect(cachedKey).toBeNull(); }); - newBackendOnly("Should not accept an invalid backup decryption key", async () => { + it("Should not accept an invalid backup decryption key", async () => { const requestPromises = mockSecretRequestAndGetPromises(); await doInteractiveVerification(); From 6a67f39de85f5fb755bdeac475a0aeda56099b49 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 14:59:29 +0100 Subject: [PATCH 2/7] test(crypto): fix duplicate test name --- spec/integ/crypto/crypto.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index cc3c3ffb1ea..35d7d8fc975 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -635,7 +635,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED); }); - it("fails with another error when the server reports user was a member of the room", async () => { + it("fails with another error when the server reports user was invited in the room", async () => { // This tests that when the server reports that the user // was invited at the time the event was sent, then we // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, @@ -656,7 +656,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); }); - it("fails with another error when the server reports user was a member of the room (MSC4115 unstable prefix)", async () => { + it("fails with another error when the server reports user was invited in the room (MSC4115 unstable prefix)", async () => { // This tests that when the server reports that the user // was invited at the time the event was sent, then we // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, From ee5f03ab2a517389932db613312079eee69c72f4 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 15:05:58 +0100 Subject: [PATCH 3/7] test(crypto): remove `oldBackendOnly` test closure --- .eslintrc.cjs | 3 +- spec/integ/crypto/crypto.spec.ts | 166 ------------------------------- 2 files changed, 1 insertion(+), 168 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 86aebd68cb3..f679304bc39 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -86,12 +86,11 @@ module.exports = { // Disabled tests are a reality for now but as soon as all of the xits are // eliminated, we should enforce this. "jest/no-disabled-tests": "off", - // Also treat "oldBackendOnly" as a test function. // Used in some crypto tests. "jest/no-standalone-expect": [ "error", { - additionalTestBlockFunctions: ["beforeAll", "beforeEach", "oldBackendOnly"], + additionalTestBlockFunctions: ["beforeAll", "beforeEach"], }, ], }, diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 35d7d8fc975..1ebc4705190 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -58,10 +58,7 @@ import { MatrixClient, MatrixEvent, MatrixEventEvent, - MsgType, PendingEventOrdering, - RoomMember, - RoomStateEvent, } from "../../../src/matrix"; import { E2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; import { ISyncResponder, SyncResponder } from "../../test-utils/SyncResponder"; @@ -219,10 +216,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, return; } - // oldBackendOnly is an alternative to `it` or `test` which will skip the test if we are running against the - // Rust backend. Once we have full support in the rust sdk, it will go away. - const oldBackendOnly = backend === "rust-sdk" ? test.skip : test; - const Olm = globalThis.Olm; let testOlmAccount = {} as unknown as Olm.Account; @@ -1273,97 +1266,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(decryptedEvent.getClearContent()).toBeUndefined(); }); - oldBackendOnly("allows sending an encrypted event as soon as room state arrives", async () => { - /* Empirically, clients expect to be able to send encrypted events as soon as the - * RoomStateEvent.NewMember notification is emitted, so test that works correctly. - */ - const testRoomId = "!testRoom:id"; - expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); - await startClientAndAwaitFirstSync(); - - /* Alice makes the /createRoom call */ - fetchMock.postOnce(new RegExp("/createRoom"), { room_id: testRoomId }); - await aliceClient.createRoom({ - initial_state: [ - { - type: "m.room.encryption", - state_key: "", - content: { algorithm: "m.megolm.v1.aes-sha2" }, - }, - ], - }); - - /* The sync arrives in two parts; first the m.room.create... */ - syncResponder.sendOrQueueSyncResponse({ - rooms: { - join: { - [testRoomId]: { - timeline: { - events: [ - { - type: "m.room.create", - state_key: "", - event_id: "$create", - }, - { - type: "m.room.member", - state_key: aliceClient.getUserId(), - content: { membership: KnownMembership.Join }, - event_id: "$alijoin", - }, - ], - }, - }, - }, - }, - }); - await syncPromise(aliceClient); - - // ... and then the e2e event and an invite ... - syncResponder.sendOrQueueSyncResponse({ - rooms: { - join: { - [testRoomId]: { - timeline: { - events: [ - { - type: "m.room.encryption", - state_key: "", - content: { algorithm: "m.megolm.v1.aes-sha2" }, - event_id: "$e2e", - }, - { - type: "m.room.member", - state_key: "@other:user", - content: { membership: KnownMembership.Invite }, - event_id: "$otherinvite", - }, - ], - }, - }, - }, - }, - }); - - // as soon as the roomMember arrives, try to send a message - expectAliceKeyQuery({ device_keys: { "@other:user": {} }, failures: {} }); - aliceClient.on(RoomStateEvent.NewMember, (_e, _s, member: RoomMember) => { - if (member.userId == "@other:user") { - aliceClient.sendMessage(testRoomId, { msgtype: MsgType.Text, body: "Hello, World" }); - } - }); - - // flush the sync and wait for the /send/ request. - const sendEventPromise = new Promise((resolve) => { - fetchMock.putOnce(new RegExp("/send/m.room.encrypted/"), () => { - resolve(undefined); - return { event_id: "asdfgh" }; - }); - }); - await syncPromise(aliceClient); - await sendEventPromise; - }); - describe("getEncryptionInfoForEvent", () => { it("handles outgoing events", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); @@ -1614,74 +1516,6 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); }, ); - - oldBackendOnly("does not block decryption on an 'm.unavailable' report", async function () { - // there may be a key downloads for alice - expectAliceKeyQuery({ device_keys: {}, failures: {} }); - - await startClientAndAwaitFirstSync(); - - // encrypt a message with a group session. - const groupSession = new Olm.OutboundGroupSession(); - groupSession.create(); - const messageEncryptedEvent = encryptMegolmEvent({ - senderKey: testSenderKey, - groupSession: groupSession, - room_id: ROOM_ID, - }); - - // Alice gets the room message, but not the key - syncResponder.sendOrQueueSyncResponse({ - next_batch: 1, - rooms: { - join: { [ROOM_ID]: { timeline: { events: [messageEncryptedEvent] } } }, - }, - }); - await syncPromise(aliceClient); - - // alice will (eventually) send a room-key request - fetchMock.putOnce(new RegExp("/sendToDevice/m.room_key_request/"), {}); - - // at this point, the message should be a decryption failure - const room = aliceClient.getRoom(ROOM_ID)!; - const event = room.getLiveTimeline().getEvents()[0]; - expect(event.isDecryptionFailure()).toBeTruthy(); - - // we want to wait for the message to be updated, so create a promise for it - const retryPromise = new Promise((resolve) => { - event.once(MatrixEventEvent.Decrypted, (ev) => { - resolve(ev); - }); - }); - - // alice gets back a room-key-withheld notification - syncResponder.sendOrQueueSyncResponse({ - next_batch: 2, - to_device: { - events: [ - { - type: "m.room_key.withheld", - sender: "@bob:example.com", - content: { - algorithm: "m.megolm.v1.aes-sha2", - room_id: ROOM_ID, - session_id: groupSession.session_id(), - sender_key: testSenderKey, - code: "m.unavailable", - reason: "", - }, - }, - ], - }, - }); - await syncPromise(aliceClient); - - // the withheld notification should trigger a retry; wait for it - await retryPromise; - - // finally: the message should still be a regular decryption failure, not a withheld notification. - expect(event.getContent().body).not.toContain("withheld"); - }); }); describe("key upload request", () => { From c109e058411ba065c0b668cb9f1da4e06825a448 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 15:13:27 +0100 Subject: [PATCH 4/7] test(crypto): remove `rust-sdk` comparison --- spec/integ/crypto/crypto.spec.ts | 18 +++++------------ spec/integ/crypto/megolm-backup.spec.ts | 26 +++++-------------------- spec/integ/crypto/verification.spec.ts | 16 +++++++-------- 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 1ebc4705190..58c7c0cfe42 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -1347,19 +1347,11 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // now check getEncryptionInfoForEvent again const encInfo2 = await aliceClient.getCrypto()!.getEncryptionInfoForEvent(lastEvent); - let expectedEncryptionInfo; - if (backend === "rust-sdk") { - // rust crypto does not trust its own device until it is cross-signed. - expectedEncryptionInfo = { - shieldColour: EventShieldColour.RED, - shieldReason: EventShieldReason.UNSIGNED_DEVICE, - }; - } else { - expectedEncryptionInfo = { - shieldColour: EventShieldColour.NONE, - shieldReason: null, - }; - } + // rust crypto does not trust its own device until it is cross-signed. + const expectedEncryptionInfo = { + shieldColour: EventShieldColour.RED, + shieldReason: EventShieldReason.UNSIGNED_DEVICE, + }; expect(encInfo2).toEqual(expectedEncryptionInfo); }); }); diff --git a/spec/integ/crypto/megolm-backup.spec.ts b/spec/integ/crypto/megolm-backup.spec.ts index 2e39c6d0334..63592a5e987 100644 --- a/spec/integ/crypto/megolm-backup.spec.ts +++ b/spec/integ/crypto/megolm-backup.spec.ts @@ -114,8 +114,6 @@ function mockUploadEmitter( } describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backend: string, initCrypto: InitCrypto) => { - const isNewBackend = backend === "rust-sdk"; - let aliceClient: MatrixClient; /** an object which intercepts `/sync` requests on the test homeserver */ let syncResponder: SyncResponder; @@ -242,11 +240,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe // On the first decryption attempt, decryption fails. await awaitDecryption(event); - expect(event.decryptionFailureReason).toEqual( - isNewBackend - ? DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP - : DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID, - ); + expect(event.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP); // Eventually, decryption succeeds. await awaitDecryption(event, { waitOnDecryptionFailure: true }); @@ -378,13 +372,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe it("Should import full backup in chunks", async function () { const importMockImpl = jest.fn(); - if (isNewBackend) { - // @ts-ignore - mock a private method for testing purpose - jest.spyOn(aliceCrypto.backupManager, "importBackedUpRoomKeys").mockImplementation(importMockImpl); - } else { - // @ts-ignore - mock a private method for testing purpose - jest.spyOn(aliceCrypto, "importBackedUpRoomKeys").mockImplementation(importMockImpl); - } + // @ts-ignore - mock a private method for testing purpose + jest.spyOn(aliceCrypto.backupManager, "importBackedUpRoomKeys").mockImplementation(importMockImpl); // We need several rooms with several sessions to test chunking const { response, expectedTotal } = createBackupDownloadResponse([45, 300, 345, 12, 130]); @@ -444,13 +433,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe // Ok for other chunks .mockResolvedValue(undefined); - if (isNewBackend) { - // @ts-ignore - mock a private method for testing purpose - jest.spyOn(aliceCrypto.backupManager, "importBackedUpRoomKeys").mockImplementation(importMockImpl); - } else { - // @ts-ignore - mock a private method for testing purpose - jest.spyOn(aliceCrypto, "importBackedUpRoomKeys").mockImplementation(importMockImpl); - } + // @ts-ignore - mock a private method for testing purpose + jest.spyOn(aliceCrypto.backupManager, "importBackedUpRoomKeys").mockImplementation(importMockImpl); const { response, expectedTotal } = createBackupDownloadResponse([100, 300]); diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index 0b69d4121a5..db698392ca5 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -989,15 +989,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st // Rust crypto requires the sender's device keys before it accepts a // verification request. - if (backend === "rust-sdk") { - const crypto = aliceClient.getCrypto()!; - - const bobDeviceKeys = getTestOlmAccountKeys(testOlmAccount, BOB_TEST_USER_ID, "BobDevice"); - e2eKeyResponder.addDeviceKeys(bobDeviceKeys); - syncResponder.sendOrQueueSyncResponse({ device_lists: { changed: [BOB_TEST_USER_ID] } }); - await syncPromise(aliceClient); - await crypto.getUserDeviceInfo([BOB_TEST_USER_ID]); - } + const crypto = aliceClient.getCrypto()!; + + const bobDeviceKeys = getTestOlmAccountKeys(testOlmAccount, BOB_TEST_USER_ID, "BobDevice"); + e2eKeyResponder.addDeviceKeys(bobDeviceKeys); + syncResponder.sendOrQueueSyncResponse({ device_lists: { changed: [BOB_TEST_USER_ID] } }); + await syncPromise(aliceClient); + await crypto.getUserDeviceInfo([BOB_TEST_USER_ID]); }); /** From 36a58e91086c6ba315030c1fcf9d84ffd7ba4ce6 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 15:24:27 +0100 Subject: [PATCH 5/7] test(crypto): remove iteration on `CRYPTO_BACKEND` --- spec/integ/crypto/cross-signing.spec.ts | 6 +++--- spec/integ/crypto/crypto.spec.ts | 8 +++----- spec/integ/crypto/megolm-backup.spec.ts | 12 +++--------- spec/integ/crypto/to-device-messages.spec.ts | 6 +++--- spec/integ/crypto/verification.spec.ts | 13 +++---------- spec/test-utils/test-utils.ts | 9 --------- 6 files changed, 15 insertions(+), 39 deletions(-) diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index f046fc2f0ba..61a4c0011aa 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -18,7 +18,7 @@ import fetchMock from "fetch-mock-jest"; import "fake-indexeddb/auto"; import { IDBFactory } from "fake-indexeddb"; -import { CRYPTO_BACKENDS, InitCrypto, syncPromise } from "../../test-utils/test-utils"; +import { syncPromise } from "../../test-utils/test-utils"; import { AuthDict, createClient, MatrixClient } from "../../../src"; import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; import encryptAESSecretStorageItem from "../../../src/utils/encryptAESSecretStorageItem.ts"; @@ -55,7 +55,7 @@ const TEST_DEVICE_ID = "xzcvb"; * These tests work by intercepting HTTP requests via fetch-mock rather than mocking out bits of the client, so as * to provide the most effective integration tests possible. */ -describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: string, initCrypto: InitCrypto) => { +describe("cross-signing", () => { let aliceClient: MatrixClient; /** an object which intercepts `/sync` requests from {@link #aliceClient} */ @@ -104,7 +104,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s body: { errcode: "M_NOT_FOUND" }, }); - await initCrypto(aliceClient); + await aliceClient.initRustCrypto(); }, /* it can take a while to initialise the crypto library on the first pass, so bump up the timeout. */ 10000, diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 58c7c0cfe42..f707d29fc8e 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -24,10 +24,8 @@ import Olm from "@matrix-org/olm"; import * as testUtils from "../../test-utils/test-utils"; import { - CRYPTO_BACKENDS, emitPromise, getSyncResponse, - InitCrypto, mkEventCustom, mkMembershipCustom, syncPromise, @@ -209,7 +207,7 @@ async function expectSendMegolmMessage( return JSON.parse(r.plaintext); } -describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, initCrypto: InitCrypto) => { +describe("crypto", () => { if (!globalThis.Olm) { // currently we use libolm to implement the crypto in the tests, so need it to be present. logger.warn("not running megolm tests: Olm not present"); @@ -369,7 +367,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, keyReceiver = new E2EKeyReceiver(homeserverUrl); syncResponder = new SyncResponder(homeserverUrl); - await initCrypto(aliceClient); + await aliceClient.initRustCrypto(); // create a test olm device which we will use to communicate with alice. We use libolm to implement this. testOlmAccount = await createOlmAccount(); @@ -2441,7 +2439,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, // For legacy crypto, these tests only work with a proper persistent cryptoStore. cryptoStore: new IndexedDBCryptoStore(indexedDB, "test"), }); - await initCrypto(client); + await client.initRustCrypto(); mockInitialApiRequests(client.getHomeserverUrl()); return client; } diff --git a/spec/integ/crypto/megolm-backup.spec.ts b/spec/integ/crypto/megolm-backup.spec.ts index 63592a5e987..5063d8814f0 100644 --- a/spec/integ/crypto/megolm-backup.spec.ts +++ b/spec/integ/crypto/megolm-backup.spec.ts @@ -32,13 +32,7 @@ import { SyncResponder } from "../../test-utils/SyncResponder"; import { E2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; import { mockInitialApiRequests } from "../../test-utils/mockEndpoints"; -import { - advanceTimersUntil, - awaitDecryption, - CRYPTO_BACKENDS, - InitCrypto, - syncPromise, -} from "../../test-utils/test-utils"; +import { advanceTimersUntil, awaitDecryption, syncPromise } from "../../test-utils/test-utils"; import * as testData from "../../test-utils/test-data"; import { KeyBackupInfo, KeyBackupSession } from "../../../src/crypto-api/keybackup"; import { flushPromises } from "../../test-utils/flushPromises"; @@ -113,7 +107,7 @@ function mockUploadEmitter( return emitter; } -describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backend: string, initCrypto: InitCrypto) => { +describe("megolm-keys backup", () => { let aliceClient: MatrixClient; /** an object which intercepts `/sync` requests on the test homeserver */ let syncResponder: SyncResponder; @@ -159,7 +153,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe deviceId: TEST_DEVICE_ID, ...opts, }); - await initCrypto(client); + await client.initRustCrypto(); return client; } diff --git a/spec/integ/crypto/to-device-messages.spec.ts b/spec/integ/crypto/to-device-messages.spec.ts index ee15266ce98..7614e34a9ae 100644 --- a/spec/integ/crypto/to-device-messages.spec.ts +++ b/spec/integ/crypto/to-device-messages.spec.ts @@ -18,7 +18,7 @@ import fetchMock from "fetch-mock-jest"; import "fake-indexeddb/auto"; import { IDBFactory } from "fake-indexeddb"; -import { CRYPTO_BACKENDS, getSyncResponse, InitCrypto, syncPromise } from "../../test-utils/test-utils"; +import { getSyncResponse, syncPromise } from "../../test-utils/test-utils"; import { createClient, MatrixClient } from "../../../src"; import * as testData from "../../test-utils/test-data"; import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; @@ -38,7 +38,7 @@ afterEach(() => { * These tests work by intercepting HTTP requests via fetch-mock rather than mocking out bits of the client, so as * to provide the most effective integration tests possible. */ -describe.each(Object.entries(CRYPTO_BACKENDS))("to-device-messages (%s)", (backend: string, initCrypto: InitCrypto) => { +describe("to-device-messages", () => { let aliceClient: MatrixClient; /** an object which intercepts `/keys/query` requests on the test homeserver */ @@ -81,7 +81,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("to-device-messages (%s)", (backe { filter_id: "fid" }, ); - await initCrypto(aliceClient); + await aliceClient.initRustCrypto(); }, /* it can take a while to initialise the crypto library on the first pass, so bump up the timeout. */ 10000, diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index db698392ca5..74d6eeea4a2 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -44,14 +44,7 @@ import { VerifierEvent, } from "../../../src/crypto-api/verification"; import { defer, escapeRegExp } from "../../../src/utils"; -import { - awaitDecryption, - CRYPTO_BACKENDS, - emitPromise, - getSyncResponse, - InitCrypto, - syncPromise, -} from "../../test-utils/test-utils"; +import { awaitDecryption, emitPromise, getSyncResponse, syncPromise } from "../../test-utils/test-utils"; import { SyncResponder } from "../../test-utils/SyncResponder"; import { BACKUP_DECRYPTION_KEY_BASE64, @@ -117,7 +110,7 @@ const TEST_HOMESERVER_URL = "https://alice-server.com"; * to provide the most effective integration tests possible. */ // we test with both crypto stacks... -describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: string, initCrypto: InitCrypto) => { +describe("verification", () => { /** the client under test */ let aliceClient: MatrixClient; @@ -1470,7 +1463,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st deviceId: "device_under_test", ...opts, }); - await initCrypto(client); + await client.initRustCrypto(); await client.startClient(); return client; } diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index dbdbeb4333c..115c1eaacc3 100644 --- a/spec/test-utils/test-utils.ts +++ b/spec/test-utils/test-utils.ts @@ -552,15 +552,6 @@ export const mkPusher = (extra: Partial = {}): IPusher => ({ ...extra, }); -/** - * a list of the supported crypto implementations, each with a callback to initialise that implementation - * for the given client - */ -export const CRYPTO_BACKENDS: Record = {}; -export type InitCrypto = (_: MatrixClient) => Promise; - -CRYPTO_BACKENDS["rust-sdk"] = (client: MatrixClient) => client.initRustCrypto(); - export const emitPromise = (e: EventEmitter, k: string): Promise => new Promise((r) => e.once(k, r)); /** From 004a3b098637df8f82d82d603b17d6a794937573 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 3 Feb 2025 16:00:45 +0100 Subject: [PATCH 6/7] test(crypto): remove old legacy comments and tests --- spec/TestClient.ts | 10 ----- spec/integ/crypto/cross-signing.spec.ts | 6 +-- spec/integ/crypto/crypto.spec.ts | 47 +++++++++--------------- spec/integ/crypto/verification.spec.ts | 7 +--- spec/integ/matrix-client-syncing.spec.ts | 6 +-- 5 files changed, 23 insertions(+), 53 deletions(-) diff --git a/spec/TestClient.ts b/spec/TestClient.ts index 66c63064e4d..93276e056bb 100644 --- a/spec/TestClient.ts +++ b/spec/TestClient.ts @@ -26,12 +26,10 @@ import MockHttpBackend from "matrix-mock-request"; import type { IDeviceKeys, IOneTimeKey } from "../src/@types/crypto"; import type { IE2EKeyReceiver } from "./test-utils/E2EKeyReceiver"; -import { LocalStorageCryptoStore } from "../src/crypto/store/localStorage-crypto-store"; import { logger } from "../src/logger"; import { syncPromise } from "./test-utils/test-utils"; import { createClient, IStartClientOpts } from "../src/matrix"; import { ICreateClientOpts, IDownloadKeyResult, MatrixClient, PendingEventOrdering } from "../src/client"; -import { MockStorageApi } from "./MockStorageApi"; import { IKeysUploadResponse, IUploadKeysRequest } from "../src/client"; import { ISyncResponder } from "./test-utils/SyncResponder"; @@ -55,10 +53,6 @@ export class TestClient implements IE2EKeyReceiver, ISyncResponder { sessionStoreBackend?: Storage, options?: Partial, ) { - if (sessionStoreBackend === undefined) { - sessionStoreBackend = new MockStorageApi() as unknown as Storage; - } - this.httpBackend = new MockHttpBackend(); const fullOptions: ICreateClientOpts = { @@ -69,10 +63,6 @@ export class TestClient implements IE2EKeyReceiver, ISyncResponder { fetchFn: this.httpBackend.fetchFn as typeof globalThis.fetch, ...options, }; - if (!fullOptions.cryptoStore) { - // expose this so the tests can get to it - fullOptions.cryptoStore = new LocalStorageCryptoStore(sessionStoreBackend); - } this.client = createClient(fullOptions); this.deviceKeys = null; diff --git a/spec/integ/crypto/cross-signing.spec.ts b/spec/integ/crypto/cross-signing.spec.ts index 61a4c0011aa..cbf2f4a1881 100644 --- a/spec/integ/crypto/cross-signing.spec.ts +++ b/spec/integ/crypto/cross-signing.spec.ts @@ -417,9 +417,8 @@ describe("cross-signing", () => { function awaitCrossSigningKeysUpload() { return new Promise((resolve) => { fetchMock.post( - // legacy crypto uses /unstable/; /v3/ is correct { - url: new RegExp("/_matrix/client/(unstable|v3)/keys/device_signing/upload"), + url: new RegExp("/_matrix/client/v3/keys/device_signing/upload"), name: "upload-keys", }, (url, options) => { @@ -472,9 +471,6 @@ describe("cross-signing", () => { await aliceClient.startClient(); await syncPromise(aliceClient); - // Wait for legacy crypto to find the device - await jest.advanceTimersByTimeAsync(10); - const devices = await aliceClient.getCrypto()!.getUserDeviceInfo([aliceClient.getSafeUserId()]); expect(devices.get(aliceClient.getSafeUserId())!.has(testData.TEST_DEVICE_ID)).toBeTruthy(); }); diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index f707d29fc8e..34ba2d89ce9 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -51,7 +51,6 @@ import { IContent, IDownloadKeyResult, IEvent, - IndexedDBCryptoStore, IStartClientOpts, MatrixClient, MatrixEvent, @@ -1901,7 +1900,6 @@ describe("crypto", () => { }); describe("bootstrapSecretStorage", () => { - // Doesn't work with legacy crypto, which will try to bootstrap even without private key, which is buggy. it("should throw an error if we are unable to create a key because createSecretStorageKey is not set", async () => { await expect( aliceClient.getCrypto()!.bootstrapSecretStorage({ setupNewSecretStorage: true }), @@ -1947,8 +1945,6 @@ describe("crypto", () => { }); it("should do nothing if an AES key is already in the secret storage and setupNewSecretStorage is not set", async () => { - const awaitAccountDataClientUpdate = awaitAccountDataUpdate("m.secret_storage.default_key"); - const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey }); // Wait for the key to be uploaded in the account data @@ -1960,9 +1956,6 @@ describe("crypto", () => { // Wait for bootstrapSecretStorage to finished await bootstrapPromise; - // On legacy crypto we need to wait for ClientEvent.AccountData before calling bootstrap again. - await awaitAccountDataClientUpdate; - // Call again bootstrapSecretStorage await aliceClient.getCrypto()!.bootstrapSecretStorage({ createSecretStorageKey }); @@ -2210,7 +2203,6 @@ describe("crypto", () => { await aliceClient.getCrypto()!.deleteKeyBackupVersion(nextVersion!); await aliceClient.getCrypto()!.checkKeyBackupAndEnable(); - // XXX Legacy crypto does not update 4S when doing that; should ensure that rust implem does it. expect(await aliceClient.getCrypto()!.getActiveSessionBackupVersion()).toBeNull(); }); }); @@ -2313,7 +2305,7 @@ describe("crypto", () => { /** Guards against downgrade attacks from servers hiding or manipulating the crypto settings. */ describe("Persistent encryption settings", () => { - let persistentStoreClient: MatrixClient; + let client1: MatrixClient; let client2: MatrixClient; beforeEach(async () => { @@ -2326,12 +2318,13 @@ describe("crypto", () => { // For legacy crypto, these tests only work properly with a proper (indexeddb-based) CryptoStore, so // rather than using the existing `aliceClient`, create a new client. Once we drop legacy crypto, we can // just use `aliceClient` here. - persistentStoreClient = await makeNewClient(homeserverurl, userId, "persistentStoreClient"); - await persistentStoreClient.startClient({}); + // XXX: Even with the rust-crypto, we need to create to a new client. The tests fail with a timeout error. + client1 = await makeNewClient(homeserverurl, userId, "client1"); + await client1.startClient({}); }); afterEach(async () => { - persistentStoreClient.stopClient(); + client1.stopClient(); client2?.stopClient(); }); @@ -2339,13 +2332,13 @@ describe("crypto", () => { // Alice is in an encrypted room const encryptionState = mkEncryptionEvent({ algorithm: "m.megolm.v1.aes-sha2" }); syncResponder.sendOrQueueSyncResponse(getSyncResponseWithState([encryptionState])); - await syncPromise(persistentStoreClient); + await syncPromise(client1); // Send a message, and expect to get an `m.room.encrypted` event. - await Promise.all([persistentStoreClient.sendTextMessage(ROOM_ID, "test"), expectEncryptedSendMessage()]); + await Promise.all([client1.sendTextMessage(ROOM_ID, "test"), expectEncryptedSendMessage()]); // We now replace the client, and allow the new one to resync, *without* the encryption event. - client2 = await replaceClient(persistentStoreClient); + client2 = await replaceClient(client1); syncResponder.sendOrQueueSyncResponse(getSyncResponseWithState([])); await client2.startClient({}); await syncPromise(client2); @@ -2358,11 +2351,11 @@ describe("crypto", () => { // Alice is in an encrypted room, where the rotation period is set to 2 messages const encryptionState = mkEncryptionEvent({ algorithm: "m.megolm.v1.aes-sha2", rotation_period_msgs: 2 }); syncResponder.sendOrQueueSyncResponse(getSyncResponseWithState([encryptionState])); - await syncPromise(persistentStoreClient); + await syncPromise(client1); // Send a message, and expect to get an `m.room.encrypted` event. const [, msg1Content] = await Promise.all([ - persistentStoreClient.sendTextMessage(ROOM_ID, "test1"), + client1.sendTextMessage(ROOM_ID, "test1"), expectEncryptedSendMessage(), ]); @@ -2376,17 +2369,17 @@ describe("crypto", () => { next_batch: "1", rooms: { join: { [TEST_ROOM_ID]: { timeline: { events: [encryptionState2], prev_batch: "" } } } }, }); - await syncPromise(persistentStoreClient); + await syncPromise(client1); // Send two more messages. The first should use the same megolm session as the first; the second should // use a different one. const [, msg2Content] = await Promise.all([ - persistentStoreClient.sendTextMessage(ROOM_ID, "test2"), + client1.sendTextMessage(ROOM_ID, "test2"), expectEncryptedSendMessage(), ]); expect(msg2Content.session_id).toEqual(msg1Content.session_id); const [, msg3Content] = await Promise.all([ - persistentStoreClient.sendTextMessage(ROOM_ID, "test3"), + client1.sendTextMessage(ROOM_ID, "test3"), expectEncryptedSendMessage(), ]); expect(msg3Content.session_id).not.toEqual(msg1Content.session_id); @@ -2396,13 +2389,13 @@ describe("crypto", () => { // Alice is in an encrypted room, where the rotation period is set to 2 messages const encryptionState = mkEncryptionEvent({ algorithm: "m.megolm.v1.aes-sha2", rotation_period_msgs: 2 }); syncResponder.sendOrQueueSyncResponse(getSyncResponseWithState([encryptionState])); - await syncPromise(persistentStoreClient); + await syncPromise(client1); // Send a message, and expect to get an `m.room.encrypted` event. - await Promise.all([persistentStoreClient.sendTextMessage(ROOM_ID, "test1"), expectEncryptedSendMessage()]); + await Promise.all([client1.sendTextMessage(ROOM_ID, "test1"), expectEncryptedSendMessage()]); // We now replace the client, and allow the new one to resync with a *different* encryption event. - client2 = await replaceClient(persistentStoreClient); + client2 = await replaceClient(client1); const encryptionState2 = mkEncryptionEvent({ algorithm: "m.megolm.v1.aes-sha2", rotation_period_msgs: 100, @@ -2433,11 +2426,7 @@ describe("crypto", () => { userId: userId, accessToken: "akjgkrgjs", deviceId: "xzcvb", - cryptoCallbacks: createCryptoCallbacks(), logger: logger.getChild(loggerPrefix), - - // For legacy crypto, these tests only work with a proper persistent cryptoStore. - cryptoStore: new IndexedDBCryptoStore(indexedDB, "test"), }); await client.initRustCrypto(); mockInitialApiRequests(client.getHomeserverUrl()); @@ -2446,7 +2435,7 @@ describe("crypto", () => { function mkEncryptionEvent(content: object) { return mkEventCustom({ - sender: persistentStoreClient.getSafeUserId(), + sender: client1.getSafeUserId(), type: "m.room.encryption", state_key: "", content: content, @@ -2463,7 +2452,7 @@ describe("crypto", () => { events: [ mkMembershipCustom({ membership: KnownMembership.Join, - sender: persistentStoreClient.getSafeUserId(), + sender: client1.getSafeUserId(), }), ...stateEvents, ], diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index 74d6eeea4a2..a655f8ac978 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -420,9 +420,8 @@ describe("verification", () => { expect(requests[0].transactionId).toEqual(transactionId); } - // legacy crypto picks devices individually; rust crypto uses a broadcast message - const toDeviceMessage = - requestBody.messages[TEST_USER_ID]["*"] ?? requestBody.messages[TEST_USER_ID][TEST_DEVICE_ID]; + // rust crypto uses a broadcast message + const toDeviceMessage = requestBody.messages[TEST_USER_ID]["*"]; expect(toDeviceMessage.from_device).toEqual(aliceClient.deviceId); expect(toDeviceMessage.transaction_id).toEqual(transactionId); }); @@ -510,10 +509,8 @@ describe("verification", () => { reciprocateQRCodeCallbacks.confirm(); await sendToDevicePromise; - // at this point, on legacy crypto, the master key is already marked as trusted, and the request is "Done". // Rust crypto, on the other hand, waits for the 'done' to arrive from the other side. if (request.phase === VerificationPhase.Done) { - // legacy crypto: we're all done const userVerificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(TEST_USER_ID); // eslint-disable-next-line jest/no-conditional-expect expect(userVerificationStatus.isCrossSigningVerified()).toBeTruthy(); diff --git a/spec/integ/matrix-client-syncing.spec.ts b/spec/integ/matrix-client-syncing.spec.ts index 59d29b82b7f..dd499317dbb 100644 --- a/spec/integ/matrix-client-syncing.spec.ts +++ b/spec/integ/matrix-client-syncing.spec.ts @@ -27,7 +27,6 @@ import { UNSTABLE_MSC2716_MARKER, MatrixClient, ClientEvent, - IndexedDBCryptoStore, ISyncResponse, IRoomEvent, IJoinedRoom, @@ -2571,9 +2570,8 @@ describe("MatrixClient syncing (IndexedDB version)", () => { }; it("should emit ClientEvent.Room when invited while using indexeddb crypto store", async () => { - const idbTestClient = new TestClient(selfUserId, "DEVICE", selfAccessToken, undefined, { - cryptoStore: new IndexedDBCryptoStore(globalThis.indexedDB, "tests"), - }); + // rust crypto uses by default indexeddb + const idbTestClient = new TestClient(selfUserId, "DEVICE", selfAccessToken); const idbHttpBackend = idbTestClient.httpBackend; const idbClient = idbTestClient.client; idbHttpBackend.when("GET", "/versions").respond(200, {}); From 8678df71f6f3f7eea1853b9e49788e3faf76d67d Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 4 Feb 2025 15:50:52 +0100 Subject: [PATCH 7/7] test(crypto): fix documentations and removed unused expect --- spec/integ/crypto/crypto.spec.ts | 2 +- spec/integ/crypto/verification.spec.ts | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 34ba2d89ce9..091a9a490a7 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -2318,7 +2318,7 @@ describe("crypto", () => { // For legacy crypto, these tests only work properly with a proper (indexeddb-based) CryptoStore, so // rather than using the existing `aliceClient`, create a new client. Once we drop legacy crypto, we can // just use `aliceClient` here. - // XXX: Even with the rust-crypto, we need to create to a new client. The tests fail with a timeout error. + // XXX: Even with the rust-crypto, we need to create a new client. The tests fail with a timeout error. client1 = await makeNewClient(homeserverurl, userId, "client1"); await client1.startClient({}); }); diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index a655f8ac978..a85430289f9 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -509,16 +509,12 @@ describe("verification", () => { reciprocateQRCodeCallbacks.confirm(); await sendToDevicePromise; - // Rust crypto, on the other hand, waits for the 'done' to arrive from the other side. + // Rust crypto waits for the 'done' to arrive from the other side. if (request.phase === VerificationPhase.Done) { const userVerificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(TEST_USER_ID); // eslint-disable-next-line jest/no-conditional-expect expect(userVerificationStatus.isCrossSigningVerified()).toBeTruthy(); await verificationPromise; - } else { - // rust crypto: still in flight - // eslint-disable-next-line jest/no-conditional-expect - expect(request.phase).toEqual(VerificationPhase.Started); } // the dummy device replies with its own 'done'