Revert "Process m.room.encryption events before emitting RoomMember events" (#2913)

This reverts commit aaf3702c66.

spec/integ/megolm-integ.spec.ts

@@ -21,19 +21,16 @@ 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,
-    IUploadKeysRequest,
+    IDownloadKeyResult,
     MatrixEvent,
     MatrixEventEvent,
+    IndexedDBCryptoStore,
     Room,
-    RoomMember,
-    RoomStateEvent,
 } from "../../src/matrix";
 import { IDeviceKeys } from "../../src/crypto/dehydration";
 import { DeviceInfo } from "../../src/crypto/deviceinfo";
@@ -330,9 +327,7 @@ 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');
     });
 
@@ -878,12 +873,7 @@ 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();
 
@@ -1022,9 +1012,7 @@ 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();
@@ -1376,87 +1364,4 @@ describe("megolm", () => {
 
         await beccaTestClient.stop();
     });
-
-    it("allows enabling encryption in the createRoom call", async () => {
-        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,28 +362,22 @@ export class MockStorageApi {
  * @param {MatrixEvent} event
  * @returns {Promise} promise which resolves (to `event`) when the event has been decrypted
  */
-export async function awaitDecryption(
+export async function awaitDecryption(event: MatrixEvent): Promise<MatrixEvent> {
-    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) {
-        if (waitOnDecryptionFailure && event.isDecryptionFailure()) {
+        return event;
-            logger.log(`${Date.now()} event ${event.getId()} got decryption error; waiting`);
-        } else {
-            return event;
-        }
     } else {
-        logger.log(`${Date.now()} event ${event.getId()} is not yet decrypted; waiting`);
+        logger.log(`${Date.now()} event ${event.getId()} is being decrypted; waiting`);
-    }
 
-    return new Promise((resolve) => {
+        return new Promise((resolve) => {
-        event.once(MatrixEventEvent.Decrypted, (ev) => {
+            event.once(MatrixEventEvent.Decrypted, (ev) => {
-            logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
+                logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
-            resolve(ev);
+                resolve(ev);
+            });
         });
-    });
+    }
 }
 
 export const emitPromise = (e: EventEmitter, k: string): Promise<any> => new Promise(r => e.once(k, r));

src/crypto/index.ts

@@ -2815,7 +2815,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
             // MatrixClient has already checked that this room should be encrypted,
             // so this is an unexpected situation.
             throw new Error(
-                "Room " + roomId + " was previously configured to use encryption, but is " +
+                "Room was previously configured to use encryption, but is " +
                 "no longer. Perhaps the homeserver is hiding the " +
                 "configuration event.",
             );

src/sync.ts

@@ -207,7 +207,6 @@ export class SyncApi {
             this.onMarkerStateEvent(room, markerEvent, markerFoundOptions);
         });
 
-        this.client.store.storeRoom(room);
         return room;
     }
 
@@ -338,6 +337,7 @@ export class SyncApi {
             await this.injectRoomEvents(room, stateEvents, events);
 
             room.recalculate();
+            client.store.storeRoom(room);
             client.emit(ClientEvent.Room, room);
 
             this.processEventsForNotifs(room, events);
@@ -1231,6 +1231,7 @@ export class SyncApi {
 
             if (inviteObj.isBrandNewRoom) {
                 room.recalculate();
+                client.store.storeRoom(room);
                 client.emit(ClientEvent.Room, room);
             } else {
                 // Update room state for invite->reject->invite cycles
@@ -1361,18 +1362,6 @@ export class SyncApi {
                 }
             }
 
-            // process any crypto events *before* emitting the RoomStateEvent events. This
-            // avoids a race condition if the application tries to send a message after the
-            // state event is processed, but before crypto is enabled, which then causes the
-            // crypto layer to complain.
-            if (this.opts.crypto) {
-                for (const e of stateEvents.concat(events)) {
-                    if (e.isState() && e.getType() === EventType.RoomEncryption && e.getStateKey() === "") {
-                        await this.opts.crypto.onCryptoEvent(e);
-                    }
-                }
-            }
-
             try {
                 await this.injectRoomEvents(room, stateEvents, events, syncEventData.fromCache);
             } catch (e) {
@@ -1394,16 +1383,27 @@ export class SyncApi {
 
             room.recalculate();
             if (joinObj.isBrandNewRoom) {
+                client.store.storeRoom(room);
                 client.emit(ClientEvent.Room, room);
             }
 
             this.processEventsForNotifs(room, events);
 
-            const emitEvent = (e: MatrixEvent): boolean => client.emit(ClientEvent.Event, e);
+            const processRoomEvent = async (e): Promise<void> => {
-            stateEvents.forEach(emitEvent);
+                client.emit(ClientEvent.Event, e);
-            events.forEach(emitEvent);
+                if (e.isState() && e.getType() == "m.room.encryption" && this.opts.crypto) {
-            ephemeralEvents.forEach(emitEvent);
+                    await this.opts.crypto.onCryptoEvent(e);
-            accountDataEvents.forEach(emitEvent);
+                }
+            };
+
+            await utils.promiseMapSeries(stateEvents, processRoomEvent);
+            await utils.promiseMapSeries(events, processRoomEvent);
+            ephemeralEvents.forEach(function(e) {
+                client.emit(ClientEvent.Event, e);
+            });
+            accountDataEvents.forEach(function(e) {
+                client.emit(ClientEvent.Event, e);
+            });
 
             // Decrypt only the last message in all rooms to make sure we can generate a preview
             // And decrypt all events after the recorded read receipt to ensure an accurate
@@ -1423,6 +1423,7 @@ export class SyncApi {
 
             room.recalculate();
             if (leaveObj.isBrandNewRoom) {
+                client.store.storeRoom(room);
                 client.emit(ClientEvent.Room, room);
             }