ElementR: Process all verification events, not just requests (#3650)

* Process all verification event * Add test for `isVerificationEvent` * Review changes * Remove null comparison and add doc to remote echo * review changes
matrix-org · Aug 9, 2023 · 3a5d660 · 3a5d660
1 parent 3f7af18
commit 3a5d660
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 11 deletions.
diff --git a/spec/unit/rust-crypto/verification.spec.ts b/spec/unit/rust-crypto/verification.spec.ts
@@ -17,8 +17,9 @@ limitations under the License.
 import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-wasm";
 import { Mocked } from "jest-mock";
 
-import { RustVerificationRequest } from "../../../src/rust-crypto/verification";
+import { isVerificationEvent, RustVerificationRequest } from "../../../src/rust-crypto/verification";
 import { OutgoingRequestProcessor } from "../../../src/rust-crypto/OutgoingRequestProcessor";
+import { EventType, MatrixEvent, MsgType } from "../../../src";
 
 describe("VerificationRequest", () => {
     describe("pending", () => {
@@ -80,6 +81,40 @@ describe("VerificationRequest", () => {
     });
 });
 
+describe("isVerificationEvent", () => {
+    it.each([
+        [EventType.KeyVerificationCancel],
+        [EventType.KeyVerificationDone],
+        [EventType.KeyVerificationMac],
+        [EventType.KeyVerificationStart],
+        [EventType.KeyVerificationKey],
+        [EventType.KeyVerificationReady],
+        [EventType.KeyVerificationAccept],
+    ])("should return true with %s event", (eventType) => {
+        const event = new MatrixEvent({
+            type: eventType,
+        });
+        expect(isVerificationEvent(event)).toBe(true);
+    });
+
+    it("should return true with EventType.RoomMessage and MsgType.KeyVerificationRequest", () => {
+        const event = new MatrixEvent({
+            type: EventType.RoomMessage,
+            content: {
+                msgtype: MsgType.KeyVerificationRequest,
+            },
+        });
+        expect(isVerificationEvent(event)).toBe(true);
+    });
+
+    it("should return false with a non verification event", () => {
+        const event = new MatrixEvent({
+            type: EventType.RoomName,
+        });
+        expect(isVerificationEvent(event)).toBe(false);
+    });
+});
+
 /** build a RustVerificationRequest with default parameters */
 function makeTestRequest(
     inner?: RustSdkCryptoJs.VerificationRequest,

diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts
@@ -55,8 +55,8 @@ import { secretStorageContainsCrossSigningKeys } from "./secret-storage";
 import { keyFromPassphrase } from "../crypto/key_passphrase";
 import { encodeRecoveryKey } from "../crypto/recoverykey";
 import { crypto } from "../crypto/crypto";
-import { RustVerificationRequest, verificationMethodIdentifierToMethod } from "./verification";
-import { EventType, MsgType } from "../@types/event";
+import { isVerificationEvent, RustVerificationRequest, verificationMethodIdentifierToMethod } from "./verification";
+import { EventType } from "../@types/event";
 import { CryptoEvent } from "../crypto";
 import { TypedEventEmitter } from "../models/typed-event-emitter";
 import { RustBackupCryptoEventMap, RustBackupCryptoEvents, RustBackupManager } from "./backup";
@@ -1054,15 +1054,13 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
      * @param event - live event
      */
     public async onLiveEventFromSync(event: MatrixEvent): Promise<void> {
-        // Ignore state event
-        if (event.isState()) return;
+        // Ignore state event or remote echo
+        // transaction_id is provided in case of remote echo {@link https://spec.matrix.org/v1.7/client-server-api/#local-echo}
+        if (event.isState() || !!event.getUnsigned().transaction_id) return;
 
         const processEvent = async (evt: MatrixEvent): Promise<void> => {
-            // Process only key validation request
-            if (
-                evt.getType() === EventType.RoomMessage &&
-                evt.getContent().msgtype === MsgType.KeyVerificationRequest
-            ) {
+            // Process only verification event
+            if (isVerificationEvent(event)) {
                 await this.onKeyVerificationRequest(evt);
             }
         };
@@ -1072,6 +1070,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
             // 5 mins
             const TIMEOUT_DELAY = 5 * 60 * 1000;
 
+            // After 5mins, we are not expecting the event to be decrypted
             const timeoutId = setTimeout(() => event.off(MatrixEventEvent.Decrypted, onDecrypted), TIMEOUT_DELAY);
 
             const onDecrypted = (decryptedEvent: MatrixEvent, error?: Error): void => {
@@ -1081,7 +1080,6 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
                 event.off(MatrixEventEvent.Decrypted, onDecrypted);
                 processEvent(decryptedEvent);
             };
-            // After 5mins, we are not expecting the event to be decrypted
 
             event.on(MatrixEventEvent.Decrypted, onDecrypted);
         } else {

diff --git a/src/rust-crypto/verification.ts b/src/rust-crypto/verification.ts
@@ -31,6 +31,8 @@ import {
 import { TypedEventEmitter } from "../models/typed-event-emitter";
 import { OutgoingRequest, OutgoingRequestProcessor } from "./OutgoingRequestProcessor";
 import { TypedReEmitter } from "../ReEmitter";
+import { MatrixEvent } from "../models/event";
+import { EventType, MsgType } from "../@types/event";
 
 /**
  * An incoming, or outgoing, request to verify a user or a device via cross-signing.
@@ -700,3 +702,28 @@ export function verificationMethodIdentifierToMethod(method: string): RustSdkCry
     }
     return meth;
 }
+
+/**
+ * Return true if the event's type matches that of an in-room verification event
+ *
+ * @param event - MatrixEvent
+ * @returns
+ *
+ * @internal
+ */
+export function isVerificationEvent(event: MatrixEvent): boolean {
+    switch (event.getType()) {
+        case EventType.KeyVerificationCancel:
+        case EventType.KeyVerificationDone:
+        case EventType.KeyVerificationMac:
+        case EventType.KeyVerificationStart:
+        case EventType.KeyVerificationKey:
+        case EventType.KeyVerificationReady:
+        case EventType.KeyVerificationAccept:
+            return true;
+        case EventType.RoomMessage:
+            return event.getContent().msgtype === MsgType.KeyVerificationRequest;
+        default:
+            return false;
+    }
+}