review (dave)

Author: toger5

spec/unit/matrixrtc/MatrixRTCSession.spec.ts

@@ -835,7 +835,7 @@ describe("MatrixRTCSession", () => {
             it("rotates key if a member leaves", async () => {
                 jest.useFakeTimers();
                 try {
-                    const KEY_DALAY = 3000;
+                    const KEY_DELAY = 3000;
                     const member2 = Object.assign({}, membershipTemplate, {
                         device_id: "BBBBBBB",
                     });
@@ -856,7 +856,7 @@ describe("MatrixRTCSession", () => {
                         sendEventMock.mockImplementation((_roomId, _evType, payload) => resolve(payload));
                     });
 
-                    sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true, makeKeyDelay: KEY_DALAY });
+                    sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true, makeKeyDelay: KEY_DELAY });
                     const sendKeySpy = jest.spyOn((sess as unknown as any).encryptionManager.transport, "sendKey");
                     const firstKeysPayload = await keysSentPromise1;
                     expect(firstKeysPayload.keys).toHaveLength(1);
@@ -874,7 +874,7 @@ describe("MatrixRTCSession", () => {
                         .mockReturnValue(makeMockRoomState([membershipTemplate], mockRoom.roomId));
                     sess.onRTCSessionMemberUpdate();
 
-                    jest.advanceTimersByTime(3000);
+                    jest.advanceTimersByTime(KEY_DELAY);
                     expect(sendKeySpy).toHaveBeenCalledTimes(1);
                     // check that we send the key with index 1 even though the send gets delayed when leaving.
                     // this makes sure we do not use an index that is one too old.

spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts

@@ -33,7 +33,7 @@ describe("ToDeviceKeyTransport", () => {
     let mockLogger: Mocked<Logger>;
     let transport: ToDeviceKeyTransport;
 
-    function setup() {
+    beforeEach(() => {
         mockClient = getMockClientWithEventEmitter({
             encryptAndSendToDevice: jest.fn(),
         });
@@ -54,11 +54,9 @@ describe("ToDeviceKeyTransport", () => {
         transport = new ToDeviceKeyTransport("@alice:example.org", "MYDEVICE", roomId, mockClient, statistics, {
             getChild: jest.fn().mockReturnValue(mockLogger),
         } as unknown as Mocked<Logger>);
-    }
+    });
 
     it("should send my keys on via to device", async () => {
-        setup();
-
         transport.start();
 
         const keyBase64Encoded = "ABCDEDF";
@@ -110,8 +108,6 @@ describe("ToDeviceKeyTransport", () => {
     });
 
     it("should emit when a key is received", async () => {
-        setup();
-
         const deferred = defer<{ userId: string; deviceId: string; keyBase64Encoded: string; index: number }>();
         transport.on(KeyTransportEvents.ReceivedKeys, (userId, deviceId, keyBase64Encoded, index, timestamp) => {
             deferred.resolve({ userId, deviceId, keyBase64Encoded, index });
@@ -150,8 +146,6 @@ describe("ToDeviceKeyTransport", () => {
     });
 
     it("should not sent to ourself", async () => {
-        setup();
-
         const keyBase64Encoded = "ABCDEDF";
         const keyIndex = 2;
         await transport.sendKey(keyBase64Encoded, keyIndex, [
@@ -168,8 +162,6 @@ describe("ToDeviceKeyTransport", () => {
     });
 
     it("should warn when there is a room mismatch", () => {
-        setup();
-
         transport.start();
 
         const testEncoded = "ABCDEDF";
@@ -243,8 +235,6 @@ describe("ToDeviceKeyTransport", () => {
         ];
 
         test.each(MALFORMED_EVENT)("should warn on malformed event %j", (event) => {
-            setup();
-
             transport.start();
 
             mockClient.emit(

src/client.ts

@@ -7946,9 +7946,9 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      * This will encrypt the payload for all devices in the list and will queue it.
      * The type of the sent to-device message will be `m.room.encrypted`.
      * @param eventType - The type of event to send
-     * @param devices - The list of devices to send the event to. Each device is
+     * @param devices - The list of devices to send the event to.
      * @param payload - The payload to send. This will be encrypted.
-     * @returns Promise which resolves once queued.
+     * @returns Promise which resolves once queued there is no error feedback when sending fails.
      */
     public async encryptAndSendToDevice(
         eventType: string,

src/matrixrtc/EncryptionManager.ts

@@ -76,10 +76,6 @@ export class EncryptionManager implements IEncryptionManager {
     // if it looks like a membership has been updated.
     private lastMembershipFingerprints: Set<string> | undefined;
 
-    // TODO: remove this value since I think this is not helpful to use
-    // it represents what index we actually sent over to ElementCall (which we do in a delayed manner)
-    // but it has no releavnt information for the encryption manager.
-    private mediaTrailerKeyIndexInUse = -1;
     private latestGeneratedKeyIndex = -1;
     private joinConfig: EncryptionConfig | undefined;
 
@@ -266,12 +262,7 @@ export class EncryptionManager implements IEncryptionManager {
         }
 
         const keyIndexToSend = indexToSend ?? this.latestGeneratedKeyIndex;
-        // TODO remove this debug log. it just shows then when sending mediaTrailerKeyIndexInUse contained the wrong index.
-        logger.debug(
-            `Compare key in use to last generated key\n`,
-            `latestGeneratedKeyIndex: ${this.latestGeneratedKeyIndex}\n`,
-            `mediaTrailerKeyIndexInUse: ${this.mediaTrailerKeyIndexInUse}`,
-        );
+
         logger.info(
             `Try sending encryption keys event. keyIndexToSend=${keyIndexToSend} (method parameter: ${indexToSend})`,
         );
@@ -382,16 +373,11 @@ export class EncryptionManager implements IEncryptionManager {
             const useKeyTimeout = setTimeout(() => {
                 this.setNewKeyTimeouts.delete(useKeyTimeout);
                 logger.info(`Delayed-emitting key changed event for ${participantId} index ${encryptionKeyIndex}`);
-                if (userId === this.userId && deviceId === this.deviceId) {
-                    this.mediaTrailerKeyIndexInUse = encryptionKeyIndex;
-                }
+
                 this.onEncryptionKeysChanged(keyBin, encryptionKeyIndex, participantId);
             }, this.useKeyDelay);
             this.setNewKeyTimeouts.add(useKeyTimeout);
         } else {
-            if (userId === this.userId && deviceId === this.deviceId) {
-                this.mediaTrailerKeyIndexInUse = encryptionKeyIndex;
-            }
             this.onEncryptionKeysChanged(keyBin, encryptionKeyIndex, participantId);
         }
     }

src/matrixrtc/IKeyTransport.ts

@@ -47,7 +47,7 @@ export interface IKeyTransport {
 
     /** Subscribe to keys from this transport. */
     on(event: KeyTransportEvents.ReceivedKeys, listener: KeyTransportEventListener): this;
-    /** Unsubscribe to keys from this transport. */
+    /** Unsubscribe from keys from this transport. */
     off(event: KeyTransportEvents.ReceivedKeys, listener: KeyTransportEventListener): this;
 
     /** Once start is called the underlying transport will subscribe to its transport system.

src/matrixrtc/ToDeviceKeyTransport.ts

@@ -23,6 +23,10 @@ import { ClientEvent, type MatrixClient } from "../client.ts";
 import type { MatrixEvent } from "../models/event.ts";
 import { EventType } from "../@types/event.ts";
 
+/**
+ * ToDeviceKeyTransport is used to send MatrixRTC keys to other devices using the
+ * to-device CS-API.
+ */
 export class ToDeviceKeyTransport
     extends TypedEventEmitter<KeyTransportEvents, KeyTransportEventsHandlerMap>
     implements IKeyTransport
@@ -42,7 +46,7 @@ export class ToDeviceKeyTransport
     }
 
     public start(): void {
-        this.client.on(ClientEvent.ToDeviceEvent, (ev) => this.onToDeviceEvent(ev));
+        this.client.on(ClientEvent.ToDeviceEvent, this.onToDeviceEvent);
     }
 
     public stop(): void {
@@ -110,7 +114,7 @@ export class ToDeviceKeyTransport
             content.member.claimed_device_id!,
             content.keys.key,
             content.keys.index,
-            age,
+            now,
         );
     }
 
@@ -159,11 +163,7 @@ export class ToDeviceKeyTransport
             return;
         }
 
-        // TODO session is not used so far
-        // if (!content.session || !content.session.call_id || !content.session.scope || !content.session.application) {
-        //     this.prefixedLogger.warn("Malformed Event: Missing/Malformed content.session", content.session);
-        //     return;
-        // }
+        // TODO check for session related fields once the to-device encryption uses the new format.
         return content as EncryptionKeysToDeviceEventContent;
     }
 }