From b03dc6ac43a5a9bc478603cf710ac7e7657aea3f Mon Sep 17 00:00:00 2001
From: Andy Balaam <andy.balaam@matrix.org>
Date: Mon, 11 Dec 2023 10:30:27 +0000
Subject: [PATCH] Move roomList out of MatrixClient, into legacy Crypto (#3944)

* Comment explaining the purpose of RoomList

* Fix incorrect return type declaration on RoomList.getRoomEncryption

* Move RoomList out of MatrixClient, into legacy Crypto

* Initialise RoomList inside Crypto.init to allow us to await it
---
 spec/unit/crypto.spec.ts | 24 ++++--------------------
 src/client.ts            | 25 ++++---------------------
 src/crypto/RoomList.ts   |  9 ++++++++-
 src/crypto/index.ts      | 27 +++++++++++++++++++++++++--
 4 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/spec/unit/crypto.spec.ts b/spec/unit/crypto.spec.ts
index bb21efd34..ac1eec9d2 100644
--- a/spec/unit/crypto.spec.ts
+++ b/spec/unit/crypto.spec.ts
@@ -356,7 +356,6 @@ describe("Crypto", function () {
 
         let crypto: Crypto;
         let mockBaseApis: MatrixClient;
-        let mockRoomList: RoomList;
 
         let fakeEmitter: EventEmitter;
 
@@ -390,19 +389,10 @@ describe("Crypto", function () {
                 isGuest: jest.fn(),
                 emit: jest.fn(),
             } as unknown as MatrixClient;
-            mockRoomList = {} as unknown as RoomList;
 
             fakeEmitter = new EventEmitter();
 
-            crypto = new Crypto(
-                mockBaseApis,
-                "@alice:home.server",
-                "FLIBBLE",
-                clientStore,
-                cryptoStore,
-                mockRoomList,
-                [],
-            );
+            crypto = new Crypto(mockBaseApis, "@alice:home.server", "FLIBBLE", clientStore, cryptoStore, []);
             crypto.registerEventHandlers(fakeEmitter as any);
             await crypto.init();
         });
@@ -1341,15 +1331,9 @@ describe("Crypto", function () {
                 setRoomEncryption: jest.fn().mockResolvedValue(undefined),
             } as unknown as RoomList;
 
-            crypto = new Crypto(
-                mockClient,
-                "@alice:home.server",
-                "FLIBBLE",
-                clientStore,
-                cryptoStore,
-                mockRoomList,
-                [],
-            );
+            crypto = new Crypto(mockClient, "@alice:home.server", "FLIBBLE", clientStore, cryptoStore, []);
+            // @ts-ignore we are injecting a mock into a private property
+            crypto.roomList = mockRoomList;
         });
 
         it("should set the algorithm if called for a known room", async () => {
diff --git a/src/client.ts b/src/client.ts
index 7225af05a..9ef4550d4 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -51,7 +51,7 @@ import { decodeBase64, encodeBase64 } from "./base64";
 import { IExportedDevice as IExportedOlmDevice } from "./crypto/OlmDevice";
 import { IOlmDevice } from "./crypto/algorithms/megolm";
 import { TypedReEmitter } from "./ReEmitter";
-import { IRoomEncryption, RoomList } from "./crypto/RoomList";
+import { IRoomEncryption } from "./crypto/RoomList";
 import { logger, Logger } from "./logger";
 import { SERVICE_TYPES } from "./service-types";
 import {
@@ -1272,7 +1272,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     protected cryptoStore?: CryptoStore;
     protected verificationMethods?: VerificationMethod[];
     protected fallbackICEServerAllowed = false;
-    protected roomList: RoomList;
     protected syncApi?: SlidingSyncSdk | SyncApi;
     public roomNameGenerator?: ICreateClientOpts["roomNameGenerator"];
     public pushRules?: IPushRules;
@@ -1428,10 +1427,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
 
         this.livekitServiceURL = opts.livekitServiceURL;
 
-        // List of which rooms have encryption enabled: separate from crypto because
-        // we still want to know which rooms are encrypted even if crypto is disabled:
-        // we don't want to start sending unencrypted events to them.
-        this.roomList = new RoomList(this.cryptoStore);
         this.roomNameGenerator = opts.roomNameGenerator;
 
         this.toDeviceMessageQueue = new ToDeviceMessageQueue(this);
@@ -2233,10 +2228,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         this.logger.debug("Crypto: Starting up crypto store...");
         await this.cryptoStore.startup();
 
-        // initialise the list of encrypted rooms (whether or not crypto is enabled)
-        this.logger.debug("Crypto: initialising roomlist...");
-        await this.roomList.init();
-
         const userId = this.getUserId();
         if (userId === null) {
             throw new Error(
@@ -2251,15 +2242,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             );
         }
 
-        const crypto = new Crypto(
-            this,
-            userId,
-            this.deviceId,
-            this.store,
-            this.cryptoStore,
-            this.roomList,
-            this.verificationMethods!,
-        );
+        const crypto = new Crypto(this, userId, this.deviceId, this.store, this.cryptoStore, this.verificationMethods!);
 
         this.reEmitter.reEmit(crypto, [
             CryptoEvent.KeyBackupFailed,
@@ -3283,7 +3266,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         // we don't have an m.room.encrypted event, but that might be because
         // the server is hiding it from us. Check the store to see if it was
         // previously encrypted.
-        return this.roomList.isRoomEncrypted(roomId);
+        return this.crypto?.isRoomEncrypted(roomId) ?? false;
     }
 
     /**
@@ -4019,7 +4002,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             throw new Error("End-to-end encryption disabled");
         }
 
-        const roomEncryption = this.roomList.getRoomEncryption(roomId);
+        const roomEncryption = this.crypto?.getRoomEncryption(roomId);
         if (!roomEncryption) {
             // unknown room, or unencrypted room
             this.logger.error("Unknown room.  Not sharing decryption keys");
diff --git a/src/crypto/RoomList.ts b/src/crypto/RoomList.ts
index a73efcd55..b100bd302 100644
--- a/src/crypto/RoomList.ts
+++ b/src/crypto/RoomList.ts
@@ -29,6 +29,13 @@ export interface IRoomEncryption {
 }
 /* eslint-enable camelcase */
 
+/**
+ * Information about the encryption settings of rooms. Loads this information
+ * from the supplied crypto store when `init()` is called, and saves it to the
+ * crypto store whenever it is updated via `setRoomEncryption()`. Can supply
+ * full information about a room's encryption via `getRoomEncryption()`, or just
+ * answer whether or not a room has encryption via `isRoomEncrypted`.
+ */
 export class RoomList {
     // Object of roomId -> room e2e info object (body of the m.room.encryption event)
     private roomEncryption: Record<string, IRoomEncryption> = {};
@@ -43,7 +50,7 @@ export class RoomList {
         });
     }
 
-    public getRoomEncryption(roomId: string): IRoomEncryption {
+    public getRoomEncryption(roomId: string): IRoomEncryption | null {
         return this.roomEncryption[roomId] || null;
     }
 
diff --git a/src/crypto/index.ts b/src/crypto/index.ts
index 2a3c371ac..1b3cc228c 100644
--- a/src/crypto/index.ts
+++ b/src/crypto/index.ts
@@ -64,7 +64,7 @@ import {
     IUploadKeySignaturesResponse,
     MatrixClient,
 } from "../client";
-import type { IRoomEncryption, RoomList } from "./RoomList";
+import { IRoomEncryption, RoomList } from "./RoomList";
 import { IKeyBackupInfo } from "./keybackup";
 import { ISyncStateData } from "../sync";
 import { CryptoStore } from "./store/base";
@@ -385,6 +385,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
     public readonly dehydrationManager: DehydrationManager;
     public readonly secretStorage: LegacySecretStorage;
 
+    private readonly roomList: RoomList;
     private readonly reEmitter: TypedReEmitter<CryptoEvent, CryptoEventHandlerMap>;
     private readonly verificationMethods: Map<VerificationMethod, typeof VerificationBase>;
     public readonly supportedAlgorithms: string[];
@@ -473,10 +474,13 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         private readonly deviceId: string,
         private readonly clientStore: IStore,
         public readonly cryptoStore: CryptoStore,
-        private readonly roomList: RoomList,
         verificationMethods: Array<VerificationMethod | (typeof VerificationBase & { NAME: string })>,
     ) {
         super();
+
+        logger.debug("Crypto: initialising roomlist...");
+        this.roomList = new RoomList(cryptoStore);
+
         this.reEmitter = new TypedReEmitter(this);
 
         if (verificationMethods) {
@@ -626,6 +630,9 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         // (this is important for key backups & things)
         this.deviceList.startTrackingDeviceList(this.userId);
 
+        logger.debug("Crypto: initialising roomlist...");
+        await this.roomList.init();
+
         logger.log("Crypto: checking for key backup...");
         this.backupManager.checkAndStart();
     }
@@ -4244,6 +4251,22 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         obj.signatures = recursiveMapToObject(sigs);
         if (unsigned !== undefined) obj.unsigned = unsigned;
     }
+
+    /**
+     * @returns true if the room with the supplied ID is encrypted. False if the
+     * room is not encrypted, or is unknown to us.
+     */
+    public isRoomEncrypted(roomId: string): boolean {
+        return this.roomList.isRoomEncrypted(roomId);
+    }
+
+    /**
+     * @returns information about the encryption on the room with the supplied
+     * ID, or null if the room is not encrypted or unknown to us.
+     */
+    public getRoomEncryption(roomId: string): IRoomEncryption | null {
+        return this.roomList.getRoomEncryption(roomId);
+    }
 }
 
 /**