crypto: Add new `ClientEvent.ReceivedToDeviceMessage` with proper `OlmEncryptionInfo` support #4891

BillCarsonFr · 2025-06-30T10:29:16Z

Notes for reviewer:

Since the integration of the rust-sdk it was not possible to tell the difference between a successfully decrypted to-device event and a cleartext one. With matrix-sdk-crypto-wasm 15.0.0, it is now possible.

This replaces #4835. As suggested the ClientEvent.ToDeviceEvent is deprecated in favour of a new ClientEvent.ReceivedToDeviceMessage:

client.on(ClientEvent.ReceivedToDeviceMessage, (message, encryptionInfo) => {
    processedToDeviceResolver.resolve([message, encryptionInfo]);
});

I also updated a bit the deprecated ClientEvent.ToDeviceEvent so that at least the event.isEncrypted is correct. See this commit dfd851f. This is optional though, but I thought that it would be good to at least restore this? (Notice that other broken APIs stay broken getSenderKey(), decryptionFailureReason(), ...).

There is a change that was revealed by this test. Now UTD events are not returned by the preprocessToDeviceMessages. I don't really see a reason to return them? But we could return them as is with a null encryption info

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
Sign-off given on the changes (see CONTRIBUTING.md).

refactor rename ProcessedToDeviceEvent to ReceivedToDeviceEvent

richvdh · 2025-07-01T08:47:37Z

Switching this to T-Enhancement so that it shows up in the js-sdk's changelog

andybalaam

I don't fully understand this, but it looks sensible. I'd like someone with more clue to take a look too.

src/sync-accumulator.ts

andybalaam · 2025-07-01T09:04:31Z

src/sync-accumulator.ts

+/**
+ * Successfully processed to-device message.
+ */
+export type IToDeviceMessage = IToDeviceEvent;


What is the purpose of this alias? The compiler won't complain if you use the wrong one, right?

Also, what is the difference in meaning between IToDeviceMessage and IToDeviceEvent.

(Also, do we still use I prefix for interfaces?)

I am trying to match a bit more the rust sdk terminlogy that uses message and not event for to-device (they don't have event_ids, and other properties compared to events).
Unfortunatly the spec uses a mix of message and event for to-device.
So I kept IToDeviceEvent for the one coming down the sync, and IToDeviceMessage for the one succesfully processed (and decrypted if applicable)

I removed it and keep only IToDeviceEvent 7a843ab

andybalaam · 2025-07-01T09:06:36Z

src/sync.ts

-                if (cancelledKeyVerificationTxns.includes(txnId)) {
-                    toDeviceEvent.flagCancelled();
+        .forEach(function (processedEvent) {
+            // For backwards compatibility, we emit the event as a MatrixEvent


Instead of emitting processedEvent? If so, it would help me if you added that to the comment.

richvdh

Looks good, a few suggestions.

src/client.ts

richvdh · 2025-07-01T08:54:27Z

src/client.ts

+    /**
+     * Fires whenever the SDK receives a new to-device message.
+     * @param message - The to-device message which caused this event to fire.
+     * @param encryptionInfo - The encryptionInfo for this message, null if sent in clear.
+     * @example
+     * ```
+     * matrixClient.on("receivedToDeviceMessage", function(message, encryptionInfo){
+     *   var claimed_sender = encryptionInfo ? encryptionInfo.sender : message.sender;
+     *   var isVerified = encryptionInfo ? encryptionInfo.verified : false;
+     *   var type = message.type;
+     * });
+     * ```
+     */


For the record, this documentation is actually in the wrong place: it should be on ClientEvent.ReceivedToDeviceMessage (which is what the application developer will actually use) rather than on the map entry (which is just here for typescript, really). Have a look at CryptoEvent, which does a better job of this.

Of course, that's true of all the ClientEvents entries, so no need to fix it as part of this PR. Just sharing FYI really.

richvdh · 2025-07-01T08:55:16Z

src/crypto-api/index.ts

@@ -1365,6 +1365,24 @@ export interface OwnDeviceKeys {
    curve25519: string;
 }

+export interface OlmEncryptionInfo {


could you give this type a doc-comment explaining what it represents, please.

src/crypto-api/index.ts

richvdh · 2025-07-01T09:13:19Z

src/client.ts

+    [ClientEvent.ReceivedToDeviceMessage]: (
+        message: IToDeviceMessage,
+        encryptionInfo: OlmEncryptionInfo | null,
+    ) => void;


wouldn't it make more sense to use the ReceivedToDeviceMessage type as the payload?

I tried that first, but then I changed to use that. Because I thought it was more natural to do like that in javascript? I may be just wrong. I have no strong feeling, will move to use ReceivedToDeviceMessage

richvdh · 2025-07-01T09:19:44Z

src/sync.ts

@@ -1909,54 +1922,59 @@ export function _createAndReEmitRoom(client: MatrixClient, roomId: string, opts:
 /**
 * Process a list of (decrypted, where possible) received to-device events.
 *
- * Converts the events into `MatrixEvent`s, and emits appropriate {@link ClientEvent.ToDeviceEvent} events.
+ * Converts the to-device message and OlmEncryptionIngo and emits appropriate {@link ClientEvent.ReceivedToDeviceMessage} events.


this sentence doesn't seem to have enough words.

richvdh · 2025-07-01T09:19:56Z

src/sync.ts

@@ -1909,54 +1922,59 @@ export function _createAndReEmitRoom(client: MatrixClient, roomId: string, opts:
 /**
 * Process a list of (decrypted, where possible) received to-device events.
 *
- * Converts the events into `MatrixEvent`s, and emits appropriate {@link ClientEvent.ToDeviceEvent} events.
+ * Converts the to-device message and OlmEncryptionIngo and emits appropriate {@link ClientEvent.ReceivedToDeviceMessage} events.
+ * Converts the events into `MatrixEvent`s, and emits the now deprecated {@link ClientEvent.ToDeviceEvent} events for compatibility.


Suggested change

* Converts the events into `MatrixEvent`s, and emits the now deprecated {@link ClientEvent.ToDeviceEvent} events for compatibility.

* Also converts the events into `MatrixEvent`s, and emits the now deprecated {@link ClientEvent.ToDeviceEvent} events for compatibility.

richvdh · 2025-07-01T09:24:02Z

src/sync.ts

+            {
+                const toDeviceEvent = processedEvent.message;
+                const content = toDeviceEvent.content;
+                const deprecatedCompatibilityEvent = new MatrixEvent(Object.assign({}, toDeviceEvent));


what is the purpose of the Object.assign here? might be worth adding a comment?

richvdh · 2025-07-01T09:26:01Z

spec/unit/rust-crypto/rust-crypto.spec.ts

            expect(res).toEqual(inputs);
        });

-        it("should pass through bad encrypted messages", async () => {
+        it("should fail to process bad encrypted messages", async () => {


There is a change that was revealed by this test. Now UTD events are not returned by the preprocessToDeviceMessages. I don't really see a reason to return them?

Agreed.

BillCarsonFr added the T-Task Tasks for the team like planning label Jun 30, 2025

github-actions bot deployed to PR Documentation Preview June 30, 2025 12:55 View deployment

BillCarsonFr mentioned this pull request Jun 30, 2025

Crypto: Support recognising a clearText vs a decrypted to-device event #4835

Closed

4 tasks

BillCarsonFr added 4 commits June 30, 2025 15:19

crypto: Add new ClientEvent.ReceivedToDeviceMessage

ce5ba96

refactor rename ProcessedToDeviceEvent to ReceivedToDeviceEvent

fix: Restore legacy isEncrypted() for to-device messages

dfd851f

Update test for new preprocessToDeviceMessages API

f0a629b

quick fix on doc

464b9ee

BillCarsonFr force-pushed the valere/crypto/handle_processedToDevice branch from 1b52566 to d37c78f Compare June 30, 2025 13:23

quick update docs and renaming

9819ca1

BillCarsonFr force-pushed the valere/crypto/handle_processedToDevice branch from d37c78f to 9819ca1 Compare June 30, 2025 13:29

github-actions bot deployed to PR Documentation Preview June 30, 2025 13:31 View deployment

BillCarsonFr marked this pull request as ready for review June 30, 2025 13:36

BillCarsonFr requested review from a team as code owners June 30, 2025 13:36

BillCarsonFr requested review from andybalaam and richvdh June 30, 2025 13:36

richvdh changed the title ~~crypto: Add new ClientEvent.ReceivedToDeviceMessage with proper OlmEncryptionInfo support~~ crypto: Add new ClientEvent.ReceivedToDeviceMessage with proper OlmEncryptionInfo support Jul 1, 2025

richvdh added T-Enhancement and removed T-Task Tasks for the team like planning labels Jul 1, 2025

andybalaam approved these changes Jul 1, 2025

View reviewed changes

richvdh reviewed Jul 1, 2025

View reviewed changes

BillCarsonFr added 4 commits July 1, 2025 14:04

review: Better doc and names for OlmEncryptionInfo

f17a536

review: Remove IToDeviceMessage alias and only keep IToDeviceEvent

7a843ab

review: improve comments of processToDeviceMessages

9958240

review: pass up encrypted event when no crypto callbacks

785479c

github-actions bot deployed to PR Documentation Preview July 1, 2025 13:14 View deployment

BillCarsonFr mentioned this pull request Jul 1, 2025

doc: Move emitter doc to the event instead of on the mapping function #4893

Draft

4 tasks

	* Converts the events into `MatrixEvent`s, and emits the now deprecated {@link ClientEvent.ToDeviceEvent} events for compatibility.
	* Also converts the events into `MatrixEvent`s, and emits the now deprecated {@link ClientEvent.ToDeviceEvent} events for compatibility.

Uh oh!

crypto: Add new ClientEvent.ReceivedToDeviceMessage with proper OlmEncryptionInfo support #4891

Are you sure you want to change the base?

crypto: Add new ClientEvent.ReceivedToDeviceMessage with proper OlmEncryptionInfo support #4891

Uh oh!

Conversation

BillCarsonFr commented Jun 30, 2025 • edited by richvdh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

richvdh commented Jul 1, 2025

Uh oh!

andybalaam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillCarsonFr Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crypto: Add new `ClientEvent.ReceivedToDeviceMessage` with proper `OlmEncryptionInfo` support #4891

crypto: Add new `ClientEvent.ReceivedToDeviceMessage` with proper `OlmEncryptionInfo` support #4891

BillCarsonFr commented Jun 30, 2025 •

edited by richvdh

Loading

BillCarsonFr Jul 1, 2025 •

edited

Loading