Prevent setting multiple room key backup listeners

If key backups are enabled on a client that already has them enabled,
don't re-add the listener for to-device room key messages.

src/e2ee/CryptoClient.ts

@@ -301,13 +301,17 @@ export class CryptoClient {
      * Enable backing up of room keys.
      * @param {IKeyBackupInfoRetrieved} info The configuration for key backup behaviour,
      * as returned by {@link MatrixClient#getKeyBackupVersion}.
-     * @returns {Promise<void>} Resolves when complete.
+     * @returns {Promise<void>} Resolves once backups have been enabled.
      */
     @requiresReady()
     public async enableKeyBackup(info: IKeyBackupInfoRetrieved): Promise<void> {
-        this.client.on("to_device.decrypted", this.onToDeviceMessage);
+        if (!this.engine.isBackupEnabled()) {
+            // Only add the listener if we didn't add it already
+            this.client.on("to_device.decrypted", this.onToDeviceMessage);
+        }
         await this.engine.enableKeyBackup(info);
-        this.engine.backupRoomKeys();
+        // Back up any pending keys now, but asynchronously
+        void this.engine.backupRoomKeys();
     }
 
     /**

src/e2ee/RustEngine.ts

@@ -35,7 +35,11 @@ export class RustEngine {
 
     private keyBackupVersion: KeyBackupVersion|undefined;
     private keyBackupWaiter = Promise.resolve();
-    private isBackupEnabled = false;
+
+    private backupEnabled = false;
+    public isBackupEnabled() {
+        return this.backupEnabled;
+    }
 
     public constructor(public readonly machine: OlmMachine, private client: MatrixClient) {
     }
@@ -154,7 +158,7 @@ export class RustEngine {
 
     public enableKeyBackup(info: IKeyBackupInfoRetrieved): Promise<void> {
         this.keyBackupWaiter = this.keyBackupWaiter.then(async () => {
-            if (this.isBackupEnabled) {
+            if (this.backupEnabled) {
                 // Finish any pending backups before changing the backup version/pubkey
                 await this.actuallyDisableKeyBackup();
             }
@@ -168,7 +172,7 @@ export class RustEngine {
             }
             await this.machine.enableBackupV1(publicKey, info.version);
             this.keyBackupVersion = info.version;
-            this.isBackupEnabled = true;
+            this.backupEnabled = true;
         });
         return this.keyBackupWaiter;
     }
@@ -183,12 +187,12 @@ export class RustEngine {
     private async actuallyDisableKeyBackup(): Promise<void> {
         await this.machine.disableBackup();
         this.keyBackupVersion = undefined;
-        this.isBackupEnabled = false;
+        this.backupEnabled = false;
     }
 
     public backupRoomKeys(): Promise<void> {
         this.keyBackupWaiter = this.keyBackupWaiter.then(async () => {
-            if (!this.isBackupEnabled) {
+            if (!this.backupEnabled) {
                 throw new Error("Key backup error: attempted to create a backup before having enabled backups");
             }
             await this.actuallyBackupRoomKeys();
@@ -202,7 +206,7 @@ export class RustEngine {
 
     private backupRoomKeysIfEnabled(): Promise<void> {
         this.keyBackupWaiter = this.keyBackupWaiter.then(async () => {
-            if (this.isBackupEnabled) {
+            if (this.backupEnabled) {
                 await this.actuallyBackupRoomKeys();
             }
         });

test/encryption/KeyBackupTest.ts

@@ -1,3 +1,4 @@
+import * as simple from "simple-mock";
 import HttpBackend from 'matrix-mock-request';
 
 import {
@@ -8,7 +9,7 @@ import {
     IKeyBackupUpdateResponse,
     KeyBackupEncryptionAlgorithm,
 } from "../../src/models/KeyBackup";
-import { RoomEncryptionAlgorithm, ICryptoRoomInformation, MatrixClient, MembershipEvent, RoomTracker } from "../../src";
+import { ICryptoRoomInformation, IToDeviceMessage, MatrixClient, MembershipEvent, RoomEncryptionAlgorithm, RoomTracker } from "../../src";
 import { bindNullEngine, createTestClient, testCryptoStores, TEST_DEVICE_ID, generateCurve25519PublicKey, bindNullQuery } from "../TestUtils";
 
 const USER_ID = "@alice:example.org";
@@ -208,36 +209,75 @@ describe('KeyBackups', () => {
         };
 
         const knownSessions: Set<string> = new Set();
+        let expectedSessions = 0;
         let etagCount = 0;
 
+        const onBackupRequest = (path, obj: Record<string, unknown>): IKeyBackupUpdateResponse => {
+            const sessions = obj?.rooms[roomId]?.sessions;
+            expect(sessions).toBeDefined();
+
+            Object.keys(sessions).forEach(session => { knownSessions.add(session); });
+            return {
+                count: knownSessions.size,
+                etag: `etag${++etagCount}`,
+            };
+        };
+
         const expectToPutRoomKey = () => {
-            http.when("PUT", "/room_keys/keys").respond(200, (path, obj: Record<string, unknown>): IKeyBackupUpdateResponse => {
-                const sessions = obj?.rooms[roomId]?.sessions;
-                expect(sessions).toBeDefined();
-
-                Object.keys(sessions).forEach(session => { knownSessions.add(session); });
-                return {
-                    count: knownSessions.size,
-                    etag: `etag${++etagCount}`,
-                };
-            });
+            http.when("PUT", "/room_keys/keys").respond(200, onBackupRequest);
         };
 
         expectToPutRoomKey();
         await Promise.all([
             client.enableKeyBackup(keyBackupInfo),
             http.flushAllExpected(),
         ]);
-        expect(knownSessions.size).toStrictEqual(1);
+        expect(knownSessions.size).toBe(++expectedSessions);
+
+        // --- Test that it's safe to re-enable backups
+
+        // Re-enabling backups replays all existing keys, so expect another request to be made
+        expectToPutRoomKey();
+        await Promise.all([
+            client.enableKeyBackup(keyBackupInfo),
+            http.flushAllExpected(),
+        ]);
+        // No new session expected this time
+        expect(knownSessions.size).toBe(expectedSessions);
 
         // --- Back up a new room key by generating one while backups are enabled
 
         expectToPutRoomKey();
         await encryptRoomEvent();
-        expect(knownSessions.size).toStrictEqual(2);
+        expect(knownSessions.size).toBe(++expectedSessions);
 
         // --- Back up a room key received via a to-device message
-        // TODO: use updateSyncData to send an *encrypted* "m.room_key" event.
+
+        const onRoomKeySpy = simple.mock((client.crypto as any).engine, "backupRoomKeys");
+
+        // TODO: Encrypt this so that it will actually be included in the backup.
+        // Until then, no backup request or new session are expected.
+        const toDeviceMessage: IToDeviceMessage = {
+            type: "m.room_key",
+            sender: USER_ID,
+            content: {
+                algorithm: RoomEncryptionAlgorithm.MegolmV1AesSha2,
+                room_id: roomId,
+                session_id: "abc",
+                session_key: "def",
+            },
+        };
+
+        bindNullEngine(http);
+        await Promise.all([
+            client.crypto.updateSyncData(
+                [toDeviceMessage],
+                {}, [], [], [],
+            ),
+            http.flushAllExpected(),
+        ]);
+        expect(knownSessions.size).toBe(expectedSessions);
+        expect(onRoomKeySpy.callCount).toBe(1);
 
         // --- Export a room key
         // TODO: consider moving this to a test dedicated to key exports