feat(secret storage): keyId in SecretStorage.setDefaultKeyId can be set at null in order to delete an exising recovery key

Author: florianduros

spec/unit/secret-storage.spec.ts

@@ -23,12 +23,14 @@ import {
     SecretStorageCallbacks,
     SecretStorageKeyDescriptionAesV1,
     SecretStorageKeyDescriptionCommon,
+    ServerSideSecretStorage,
     ServerSideSecretStorageImpl,
     trimTrailingEquals,
 } from "../../src/secret-storage";
 import { randomString } from "../../src/randomstring";
 import { SecretInfo } from "../../src/secret-storage.ts";
-import { AccountDataEvents } from "../../src";
+import { AccountDataEvents, ClientEvent, MatrixEvent, TypedEventEmitter } from "../../src";
+import { defer, IDeferred } from "../../src/utils";
 
 declare module "../../src/@types/event" {
     interface SecretStorageAccountDataEvents {
@@ -273,6 +275,78 @@ describe("ServerSideSecretStorageImpl", function () {
             expect(console.warn).toHaveBeenCalledWith(expect.stringContaining("unknown algorithm"));
         });
     });
+
+    describe("setDefaultKeyId", function () {
+        let secretStorage: ServerSideSecretStorage;
+        let accountDataAdapter: Mocked<AccountDataClient>;
+        let accountDataPromise: IDeferred<void>;
+        beforeEach(() => {
+            accountDataAdapter = mockAccountDataClient();
+            accountDataPromise = defer();
+            accountDataAdapter.setAccountData.mockImplementation(() => {
+                accountDataPromise.resolve();
+                return Promise.resolve({});
+            });
+
+            secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {});
+        });
+
+        it("should set the default key id", async function () {
+            const setDefaultPromise = secretStorage.setDefaultKeyId("keyId");
+            await accountDataPromise.promise;
+
+            expect(accountDataAdapter.setAccountData).toHaveBeenCalledWith("m.secret_storage.default_key", {
+                key: "keyId",
+            });
+
+            accountDataAdapter.emit(
+                ClientEvent.AccountData,
+                new MatrixEvent({
+                    type: "m.secret_storage.default_key",
+                    content: { key: "keyId" },
+                }),
+            );
+            await setDefaultPromise;
+        });
+
+        it("should set the default key id with a null key id", async function () {
+            const setDefaultPromise = secretStorage.setDefaultKeyId(null);
+            await accountDataPromise.promise;
+
+            expect(accountDataAdapter.setAccountData).toHaveBeenCalledWith("m.secret_storage.default_key", {});
+
+            accountDataAdapter.emit(
+                ClientEvent.AccountData,
+                new MatrixEvent({
+                    type: "m.secret_storage.default_key",
+                    content: {},
+                }),
+            );
+            await setDefaultPromise;
+        });
+    });
+
+    describe("getDefaultKeyId", function () {
+        it("should return null when there is no key", async function () {
+            const accountDataAdapter = mockAccountDataClient();
+            const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {});
+            expect(await secretStorage.getDefaultKeyId()).toBe(null);
+        });
+
+        it("should return the key id when there is a key", async function () {
+            const accountDataAdapter = mockAccountDataClient();
+            accountDataAdapter.getAccountDataFromServer.mockResolvedValue({ key: "keyId" });
+            const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {});
+            expect(await secretStorage.getDefaultKeyId()).toBe("keyId");
+        });
+
+        it("should return null when an empty object is in the account data", async function () {
+            const accountDataAdapter = mockAccountDataClient();
+            accountDataAdapter.getAccountDataFromServer.mockResolvedValue({});
+            const secretStorage = new ServerSideSecretStorageImpl(accountDataAdapter, {});
+            expect(await secretStorage.getDefaultKeyId()).toBe(null);
+        });
+    });
 });
 
 describe("trimTrailingEquals", () => {
@@ -291,8 +365,13 @@ describe("trimTrailingEquals", () => {
 });
 
 function mockAccountDataClient(): Mocked<AccountDataClient> {
+    const eventEmitter = new TypedEventEmitter();
     return {
         getAccountDataFromServer: jest.fn().mockResolvedValue(null),
         setAccountData: jest.fn().mockResolvedValue({}),
+        on: eventEmitter.on.bind(eventEmitter),
+        off: eventEmitter.off.bind(eventEmitter),
+        removeListener: eventEmitter.removeListener.bind(eventEmitter),
+        emit: eventEmitter.emit.bind(eventEmitter),
     } as unknown as Mocked<AccountDataClient>;
 }

src/@types/event.ts

@@ -373,7 +373,7 @@ export interface AccountDataEvents extends SecretStorageAccountDataEvents {
     [EventType.PushRules]: IPushRules;
     [EventType.Direct]: { [userId: string]: string[] };
     [EventType.IgnoredUserList]: { [userId: string]: {} };
-    "m.secret_storage.default_key": { key: string };
+    "m.secret_storage.default_key": { key: string } | {};
     "m.identity_server": { base_url: string | null };
     [key: `${typeof LOCAL_NOTIFICATION_SETTINGS_PREFIX.name}.${string}`]: LocalNotificationSettings;
     [key: `m.secret_storage.key.${string}`]: SecretStorageKeyDescription;

src/secret-storage.ts

@@ -315,10 +315,12 @@ export interface ServerSideSecretStorage {
 
     /**
      * Set the default key ID for encrypting secrets.
+     * If keyId is `null`, the default key id value in the account data will be set to an empty object.
+     * This is considered as "disabling" the default key.
      *
      * @param keyId - The new default key ID
      */
-    setDefaultKeyId(keyId: string): Promise<void>;
+    setDefaultKeyId(keyId: string | null): Promise<void>;
 }
 
 /**
@@ -352,26 +354,31 @@ export class ServerSideSecretStorageImpl implements ServerSideSecretStorage {
      */
     public async getDefaultKeyId(): Promise<string | null> {
         const defaultKey = await this.accountDataAdapter.getAccountDataFromServer("m.secret_storage.default_key");
-        if (!defaultKey) return null;
-        return defaultKey.key ?? null;
+        if (!defaultKey || !Object.keys(defaultKey).length) return null;
+        return (defaultKey as { key: string }).key ?? null;
     }
 
     /**
-     * Set the default key ID for encrypting secrets.
-     *
-     * @param keyId - The new default key ID
+     * Implementation of {@link ServerSideSecretStorage#setDefaultKeyId}.
      */
-    public setDefaultKeyId(keyId: string): Promise<void> {
+    public setDefaultKeyId(keyId: string | null): Promise<void> {
         return new Promise<void>((resolve, reject) => {
             const listener = (ev: MatrixEvent): void => {
-                if (ev.getType() === "m.secret_storage.default_key" && ev.getContent().key === keyId) {
+                const content = ev.getContent();
+                const isSameKey = keyId === null ? !Object.keys(content).length : content.key === keyId;
+                if (ev.getType() === "m.secret_storage.default_key" && isSameKey) {
                     this.accountDataAdapter.removeListener(ClientEvent.AccountData, listener);
                     resolve();
                 }
             };
             this.accountDataAdapter.on(ClientEvent.AccountData, listener);
 
-            this.accountDataAdapter.setAccountData("m.secret_storage.default_key", { key: keyId }).catch((e) => {
+            // The spec says that the key should be an object with a `key` property
+            // https://spec.matrix.org/v1.13/client-server-api/#key-storage
+            // To delete the default key, we send an empty object like the rust sdk does
+            // (see https://docs.rs/matrix-sdk/latest/matrix_sdk/encryption/recovery/struct.Recovery.html#method.reset_identity)
+            const newValue = keyId === null ? {} : { key: keyId };
+            this.accountDataAdapter.setAccountData("m.secret_storage.default_key", newValue).catch((e) => {
                 this.accountDataAdapter.removeListener(ClientEvent.AccountData, listener);
                 reject(e);
             });