feat(widget): Send custom to-device messages in widgets in e2ee rooms #5252

BillCarsonFr · 2025-06-19T08:53:16Z

Adds the capability for widgets to send encrytped to-device messages properly.

It is getting rid of the old encrypted boolean that was used by widget to request encryption. As said by the base MSC
for widget send/receive messages: Widgets should not be aware of encryption.
If the widget is in an encrytped room then to-device traffic will be encrypted.

Fixes #4859

Public API changes documented in changelogs (optional)

Signed-off-by:

codecov · 2025-06-19T09:23:26Z

Codecov Report

Attention: Patch coverage is 92.98246% with 16 lines in your changes missing coverage. Please review.

Project coverage is 88.75%. Comparing base (1aa933c) to head (76a9446).
Report is 43 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/widget/matrix.rs	88.99%	5 Missing and 7 partials ⚠️
...ates/matrix-sdk/src/test_utils/mocks/encryption.rs	98.07%	1 Missing and 1 partial ⚠️
...rates/matrix-sdk/src/widget/machine/from_widget.rs	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5252      +/-   ##
==========================================
- Coverage   90.20%   88.75%   -1.45%     
==========================================
  Files         334      330       -4     
  Lines      105133    88874   -16259     
  Branches   105133    88874   -16259     
==========================================
- Hits        94831    78884   -15947     
+ Misses       6238     6176      -62     
+ Partials     4064     3814     -250

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

BillCarsonFr · 2025-06-19T09:23:43Z

crates/matrix-sdk/src/widget/matrix.rs

+        if room_encrypted {
+            trace!("Sending to-device message in encrypted room <{}>", self.room.room_id());
+
+            //  Attempt to re-batch these by content to batch them up into a single request


This part is a bit annoying, and the root cause is I believe a bad design choice on the widget API that wants the same structure than /sendToDevice HTTP API.
It is annoying because the crypto API allows to encrypt a same content for a list of recipients (not a bag of different contents). I added a comment on the MSC matrix-org/matrix-spec-proposals#3819 (comment)

but anyhow, for now we have to invert this map and re-group per similar content (have to serialise for checking that)

BillCarsonFr · 2025-06-19T09:24:58Z

crates/matrix-sdk/src/widget/machine/from_widget.rs

+    /// It is possible that sending to some device failed (failed to encrypt or
+    /// failed to send)
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub failures: Option<BTreeMap<String, Vec<String>>>,


For now there is no reason/code for the failures. Will be handled in a follow-up PR

poljar

I think this looks mostly good, there is some surprising logic that we could make a bit more obvious and perhaps even more performant.

crates/matrix-sdk/src/test_utils/mocks/encryption.rs

crates/matrix-sdk/src/widget/machine/from_widget.rs

crates/matrix-sdk/src/widget/matrix.rs

poljar · 2025-06-24T14:28:53Z

crates/matrix-sdk/src/widget/matrix.rs

+            let mut content_grouped: BTreeMap<
+                &str,
+                BTreeMap<OwnedUserId, Vec<DeviceIdOrAllDevices>>,
+            > = BTreeMap::new();
+
+            for (user_id, device_map) in messages.iter() {
+                for (device_id, content) in device_map.iter() {
+                    content_grouped
+                        .entry(content.json().get())
+                        .or_default()
+                        .entry(user_id.clone())
+                        .or_default()
+                        .push(device_id.to_owned());
+                }
+            }


This one is a bit trippy, you are reversing the mapping from user -> device -> content to content -> user -> device.

As far as I understand this is an optimization so we can use the function that encrypts the same content for multiple devices.

Could you document this fact, it isn't immediately obvious why this is needed.

Renaming the variable to content_to_recipients_map or some similar more obvious name might help as well.

poljar · 2025-06-24T14:30:35Z

crates/matrix-sdk/src/widget/matrix.rs


-        response.map_err(Into::into)
+            // Encrypt and send


This sounds like a perfect place where we could introduce a helper function.

This is getting a bit long I would say.

poljar · 2025-06-24T14:36:09Z

crates/matrix-sdk/src/widget/matrix.rs

+                                    client.encryption().get_user_devices(&user_id).await?;
+                                let devices = user_devices.devices();
+                                devices.for_each(|device| {
+                                    device_set.insert((
+                                        device.user_id().to_owned(),
+                                        device.device_id().to_owned(),


This is a bit weird, you are doing the following:

Get all the devices of the user

Record the user/device id

Throw the devices away

Iterate through each recorded user/device id and load each device individually from the store.

It seems to me that it would be much better to:

Get all the devices of a user

If an alias was used, you have your set.

If a device IDs were used, filter out any device that isn't part of the recipient set.

Update here 159a7a1

I haven't found the simple way of finding failure like you suggested let failures = recipient_list - recipient_devices, I tried something else

This still feels pretty suboptimal:

You fetch all devices for each DeviceId entry in the list.

You collect a bunch of duplicates, which wastes memory until...

You deduplicate the list of recipient devices.

What I had in mind would be something like the following diff:

Commit ID: d205fe05c3577be6ca56d4a994dccbc00be52b7b Change ID: unuvrlxqtqmnuptxnkxukwuoxmknkktl Author : Damir Jelić <[email protected]> (2025-06-26 12:22:21) Committer: Damir Jelić <[email protected]> (2025-06-26 13:05:20) (no description set) diff --git a/crates/matrix-sdk/src/widget/matrix.rs b/crates/matrix-sdk/src/widget/matrix.rs index 8fd210baa7..8b1df37c11 100644 --- a/crates/matrix-sdk/src/widget/matrix.rs +++ b/crates/matrix-sdk/src/widget/matrix.rs @@ -15,7 +15,7 @@ //! Matrix driver implementation that exposes Matrix functionality //! that is relevant for the widget API. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use as_variant::as_variant; use matrix_sdk_base::deserialized_responses::{EncryptionInfo, RawAnySyncOrStrippedState}; @@ -436,37 +436,48 @@ let client = self.room.client(); let mut recipient_devices = Vec::<Device>::new(); - for (user_id, device_id_or_alias) in user_to_list_of_device_id_or_all { + for (user_id, recipient_device_ids) in user_to_list_of_device_id_or_all { let user_devices = client.encryption().get_user_devices(&user_id).await?; - if device_id_or_alias.contains(&DeviceIdOrAllDevices::AllDevices) { - recipient_devices.extend(user_devices.devices()); + let user_devices: Vec<Device> = if recipient_device_ids + .contains(&DeviceIdOrAllDevices::AllDevices) + { + // If the user wants to send to all devices there's nothing to filter and no + // need to inspect other entries in the users device list. + user_devices.devices().collect() } else { - recipient_devices.extend(user_devices.devices().filter(|d| { - device_id_or_alias - .contains(&DeviceIdOrAllDevices::DeviceId(d.device_id().to_owned())) - })); - - let missing_devices = device_id_or_alias - .iter() - .filter_map(|d| { - as_variant!(d, DeviceIdOrAllDevices::DeviceId) - .map(|device_id| device_id.to_owned()) - }) - .filter(|device_id| user_devices.get(device_id).is_none()) - .collect::<Vec<_>>(); - - if !missing_devices.is_empty() { - failures.entry(user_id).or_default().extend(missing_devices); - } - } + // If the user wants to send to only some devices, filter out any devices that + // aren't part of the recipient_device_ids list. + let filtered_devices = user_devices + .devices() + .map(|device| (device.device_id().to_owned(), device)) + .filter(|(device_id, _)| { + recipient_device_ids + .contains(&DeviceIdOrAllDevices::DeviceId(device_id.clone())) + }); + + let (found_device_ids, devices): (BTreeSet<_>, Vec<_>) = filtered_devices.unzip(); + + let list_of_devices: BTreeSet<_> = recipient_device_ids + .into_iter() + .filter_map(|d| as_variant!(d, DeviceIdOrAllDevices::DeviceId)) + .collect(); + + // Let's now find any devices that are part of the recipient_device_ids list but + // were not found in our store. + // TODO: Do we want to do this for the case where all devices are part of the + // recipient list, in case someone specified all devices as well as some devices + // explicitly? + let missing_devices = + list_of_devices.difference(&found_device_ids).map(|d| d.to_owned()).collect(); + failures.insert(user_id, missing_devices); + + devices + }; + + recipient_devices.extend(user_devices); } - // The user-provided list of devices may contain duplicates, ensure no - // duplicates are sent. - recipient_devices - .dedup_by(|da, db| da.user_id().eq(db.user_id()) && da.device_id().eq(db.device_id())); - if !recipient_devices.is_empty() { // need to group by content let encrypt_and_send_failures = client

I haven't found the simple way of finding failure like you suggested let failures = recipient_list - recipient_devices, I tried something else

Sorry, I meant BTreeSet::difference() but couldn't remember the name of the method.

poljar · 2025-06-24T14:38:09Z

crates/matrix-sdk/src/widget/matrix.rs

+                for (u, d) in device_set {
+                    let found_device = client.encryption().get_device(&u, &d).await?;
+                    let Some(device) = found_device else {
+                        warn!("Device {} not found for user {}", d, u);
+                        // device not found, skip it and log
+                        failures.entry(u).or_default().push(d);
+                        continue;
+                    };
+                    recipients.push(device);
+                }


Once you have all the recipient devices collected, getting the failures would be a simple set operation.

let failures = recipient_list - recipient_devices

poljar

Still not quite to my liking. I left a diff that I think makes things clearer and doesn't load all devices for each user multiple times.

Hopefully it helps out a bit.

BillCarsonFr changed the title ~~Valere/crypto/widget send encrypted to device2~~ feat(widget): Send custom to-device messages in widgets in e2ee rooms Jun 19, 2025

BillCarsonFr added 6 commits June 19, 2025 11:12

widget: to-device remove deprecated encrypted field

25b70c8

feat(widget): Send encrypted to-device

474af4e

test: add test for send encrypted to-device with wildcard

8cebc55

post rebase fix

b420a41

Improve test by checking to-device decryption on reception

78a74f1

new test for widget to-device error handling for server error

0b565d0

BillCarsonFr force-pushed the valere/crypto/widget_send_encrypted_to_device2 branch from 29cabcc to 0b565d0 Compare June 19, 2025 09:12

BillCarsonFr commented Jun 19, 2025

View reviewed changes

BillCarsonFr mentioned this pull request Jun 19, 2025

feat(widget): Send custom to-device messages in widgets in e2ee rooms #5229

Closed

1 task

BillCarsonFr marked this pull request as ready for review June 19, 2025 09:25

BillCarsonFr requested a review from a team as a code owner June 19, 2025 09:25

BillCarsonFr requested review from poljar and removed request for a team June 19, 2025 09:25

poljar requested changes Jun 24, 2025

View reviewed changes

BillCarsonFr added 5 commits June 25, 2025 10:10

Merge branch 'main' into valere/crypto/widget_send_encrypted_to_device2

1f126bd

review: cleanup recipient collection code

159a7a1

review: extract helper to reduce method size

8fe836b

review: fix un-needed Option for failures

6647da2

review improve comment

c651937

BillCarsonFr requested a review from poljar June 25, 2025 13:16

BillCarsonFr added 2 commits June 25, 2025 20:22

review: move update_tracked_users to a better place

912c7ef

fix compilation when e2e disabled

2f206c8

poljar requested changes Jun 26, 2025

View reviewed changes

BillCarsonFr added 3 commits June 26, 2025 16:41

review: further improvement of collect recipient code

7fb86c7

review: added todo and logs for some edge cases

98b4a99

fix doc

76a9446

feat(widget): Send custom to-device messages in widgets in e2ee rooms #5252

Are you sure you want to change the base?

feat(widget): Send custom to-device messages in widgets in e2ee rooms #5252

Conversation

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

Uh oh!

Uh oh!

codecov bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar Jun 24, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BillCarsonFr commented Jun 19, 2025 •

edited

Loading

codecov bot commented Jun 19, 2025 •

edited

Loading

poljar Jun 24, 2025 •

edited

Loading