Process m.room.encryption events before emitting RoomMember events (

#2914)

element-hq/element-web#23819 is an intermittent failure to correctly initiate a user verification process. The
root cause is as follows:

* In matrix-react-sdk, ensureDMExists tries to create an encrypted DM room, and assumes it is ready for use
  (including sending encrypted events) as soon as it receives a RoomStateEvent.NewMember notification
  indicating that the other user has been invited or joined. 

* However, in sync.ts, we process the membership events in a /sync response (including emitting
  RoomStateEvent.NewMember notifications), which is long before we process any m.room.encryption event.
    
* The upshot is that we can end up trying to send an encrypted event in the new room before processing
  the m.room.encryption event, which causes the crypto layer to blow up with an error of "Room was 
  previously configured to use encryption, but is no longer".

Strictly speaking, ensureDMExists probably ought to be listening for ClientEvent.Room as well as RoomStateEvent.NewMember; but that doesn't help us, because ClientEvent.Room is also emitted
before we process the crypto event.

So, we need to process the crypto event before we start emitting these other events; but a corollary of that 
is that we need to do so before we store the new room in the client's store. That makes things tricky, because
currently the crypto layer expects the room to have been stored in the client first.

So... we have to rearrange everything to pass the newly-created Room object into the crypto layer, rather than
just the room id, so that it doesn't need to rely on getting the Room from the client's store.

spec/integ/megolm-integ.spec.ts

@@ -21,16 +21,19 @@ import * as testUtils from "../test-utils/test-utils";
 import { TestClient } from "../TestClient";
 import { logger } from "../../src/logger";
 import {
+    IClaimOTKsResult,
     IContent,
+    IDownloadKeyResult,
     IEvent,
-    IClaimOTKsResult,
     IJoinedRoom,
+    IndexedDBCryptoStore,
     ISyncResponse,
-    IDownloadKeyResult,
+    IUploadKeysRequest,
     MatrixEvent,
     MatrixEventEvent,
-    IndexedDBCryptoStore,
     Room,
+    RoomMember,
+    RoomStateEvent,
 } from "../../src/matrix";
 import { IDeviceKeys } from "../../src/crypto/dehydration";
 import { DeviceInfo } from "../../src/crypto/deviceinfo";
@@ -327,7 +330,9 @@ describe("megolm", () => {
         const room = aliceTestClient.client.getRoom(ROOM_ID)!;
         const event = room.getLiveTimeline().getEvents()[0];
         expect(event.isEncrypted()).toBe(true);
-        const decryptedEvent = await testUtils.awaitDecryption(event);
+
+        // it probably won't be decrypted yet, because it takes a while to process the olm keys
+        const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true });
         expect(decryptedEvent.getContent().body).toEqual('42');
     });
 
@@ -873,7 +878,12 @@ describe("megolm", () => {
 
         const room = aliceTestClient.client.getRoom(ROOM_ID)!;
         await room.decryptCriticalEvents();
-        expect(room.getLiveTimeline().getEvents()[0].getContent().body).toEqual('42');
+
+        // it probably won't be decrypted yet, because it takes a while to process the olm keys
+        const decryptedEvent = await testUtils.awaitDecryption(
+            room.getLiveTimeline().getEvents()[0], { waitOnDecryptionFailure: true },
+        );
+        expect(decryptedEvent.getContent().body).toEqual('42');
 
         const exported = await aliceTestClient.client.exportRoomKeys();
 
@@ -1012,7 +1022,9 @@ describe("megolm", () => {
         const room = aliceTestClient.client.getRoom(ROOM_ID)!;
         const event = room.getLiveTimeline().getEvents()[0];
         expect(event.isEncrypted()).toBe(true);
-        const decryptedEvent = await testUtils.awaitDecryption(event);
+
+        // it probably won't be decrypted yet, because it takes a while to process the olm keys
+        const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true });
         expect(decryptedEvent.getRoomId()).toEqual(ROOM_ID);
         expect(decryptedEvent.getContent()).toEqual({});
         expect(decryptedEvent.getClearContent()).toBeUndefined();
@@ -1364,4 +1376,90 @@ describe("megolm", () => {
 
         await beccaTestClient.stop();
     });
+
+    it("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";
+        await aliceTestClient.start();
+
+        aliceTestClient.httpBackend.when("POST", "/keys/query")
+            .respond(200, function(_path, content: IUploadKeysRequest) {
+                return { device_keys: {} };
+            });
+
+        /* Alice makes the /createRoom call */
+        aliceTestClient.httpBackend.when("POST", "/createRoom")
+            .respond(200, { room_id: testRoomId });
+        await Promise.all([
+            aliceTestClient.client.createRoom({
+                initial_state: [{
+                    type: 'm.room.encryption',
+                    state_key: '',
+                    content: { algorithm: 'm.megolm.v1.aes-sha2' },
+                }],
+            }),
+            aliceTestClient.httpBackend.flushAllExpected(),
+        ]);
+
+        /* The sync arrives in two parts; first the m.room.create... */
+        aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
+            rooms: { join: {
+                [testRoomId]: {
+                    timeline: { events: [
+                        {
+                            type: 'm.room.create',
+                            state_key: '',
+                            event_id: "$create",
+                        },
+                        {
+                            type: 'm.room.member',
+                            state_key: aliceTestClient.getUserId(),
+                            content: { membership: "join" },
+                            event_id: "$alijoin",
+                        },
+                    ] },
+                },
+            } },
+        });
+        await aliceTestClient.flushSync();
+
+        // ... and then the e2e event and an invite ...
+        aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
+            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: "invite" },
+                            event_id: "$otherinvite",
+                        },
+                    ] },
+                },
+            } },
+        });
+
+        // as soon as the roomMember arrives, try to send a message
+        aliceTestClient.client.on(RoomStateEvent.NewMember, (_e, _s, member: RoomMember) => {
+            if (member.userId == "@other:user") {
+                aliceTestClient.client.sendMessage(testRoomId, { msgtype: "m.text", body: "Hello, World" });
+            }
+        });
+
+        // flush the sync and wait for the /send/ request.
+        aliceTestClient.httpBackend.when("PUT", "/send/m.room.encrypted/")
+            .respond(200, (_path, _content) => ({ event_id: "asdfgh" }));
+        await Promise.all([
+            aliceTestClient.flushSync(),
+            aliceTestClient.httpBackend.flush("/send/m.room.encrypted/", 1),
+        ]);
+    });
 });

spec/test-utils/test-utils.ts

@@ -362,22 +362,28 @@ export class MockStorageApi {
  * @param {MatrixEvent} event
  * @returns {Promise} promise which resolves (to `event`) when the event has been decrypted
  */
-export async function awaitDecryption(event: MatrixEvent): Promise<MatrixEvent> {
+export async function awaitDecryption(
+    event: MatrixEvent, { waitOnDecryptionFailure = false } = {},
+): Promise<MatrixEvent> {
     // An event is not always decrypted ahead of time
     // getClearContent is a good signal to know whether an event has been decrypted
     // already
     if (event.getClearContent() !== null) {
-        return event;
+        if (waitOnDecryptionFailure && event.isDecryptionFailure()) {
+            logger.log(`${Date.now()} event ${event.getId()} got decryption error; waiting`);
+        } else {
+            return event;
+        }
     } else {
-        logger.log(`${Date.now()} event ${event.getId()} is being decrypted; waiting`);
+        logger.log(`${Date.now()} event ${event.getId()} is not yet decrypted; waiting`);
+    }
 
-        return new Promise((resolve) => {
-            event.once(MatrixEventEvent.Decrypted, (ev) => {
-                logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
-                resolve(ev);
-            });
+    return new Promise((resolve) => {
+        event.once(MatrixEventEvent.Decrypted, (ev) => {
+            logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
+            resolve(ev);
         });
-    }
+    });
 }
 
 export const emitPromise = (e: EventEmitter, k: string): Promise<any> => new Promise(r => e.once(k, r));

spec/unit/crypto.spec.ts

@@ -19,6 +19,7 @@ import { MemoryStore } from "../../src";
 import { RoomKeyRequestState } from '../../src/crypto/OutgoingRoomKeyRequestManager';
 import { RoomMember } from '../../src/models/room-member';
 import { IStore } from '../../src/store';
+import { IRoomEncryption, RoomList } from "../../src/crypto/RoomList";
 
 const Olm = global.Olm;
 
@@ -1140,4 +1141,48 @@ describe("Crypto", function() {
             await client!.client.crypto!.start();
         });
     });
+
+    describe("setRoomEncryption", () => {
+        let mockClient: MatrixClient;
+        let mockRoomList: RoomList;
+        let clientStore: IStore;
+        let crypto: Crypto;
+
+        beforeEach(async function() {
+            mockClient = {} as MatrixClient;
+            const mockStorage = new MockStorageApi() as unknown as Storage;
+            clientStore = new MemoryStore({ localStorage: mockStorage }) as unknown as IStore;
+            const cryptoStore = new MemoryCryptoStore();
+
+            mockRoomList = {
+                getRoomEncryption: jest.fn().mockReturnValue(null),
+                setRoomEncryption: jest.fn().mockResolvedValue(undefined),
+            } as unknown as RoomList;
+
+            crypto = new Crypto(
+                mockClient,
+                "@alice:home.server",
+                "FLIBBLE",
+                clientStore,
+                cryptoStore,
+                mockRoomList,
+                [],
+            );
+        });
+
+        it("should set the algorithm if called for a known room", async () => {
+            const room = new Room("!room:id", mockClient, "@my.user:id");
+            await clientStore.storeRoom(room);
+            await crypto.setRoomEncryption("!room:id", { algorithm: "m.megolm.v1.aes-sha2" } as IRoomEncryption);
+            expect(mockRoomList!.setRoomEncryption).toHaveBeenCalledTimes(1);
+            expect(jest.mocked(mockRoomList!.setRoomEncryption).mock.calls[0][0]).toEqual("!room:id");
+        });
+
+        it("should raise if called for an unknown room", async () => {
+            await expect(async () => {
+                await crypto.setRoomEncryption("!room:id", { algorithm: "m.megolm.v1.aes-sha2" } as IRoomEncryption);
+            }).rejects.toThrow(/unknown room/);
+            expect(mockRoomList!.setRoomEncryption).not.toHaveBeenCalled();
+        });
+    });
 });