From 45000a4e347f7bf0fbbe8fd4011dd9e8eb7beaf4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 25 Jun 2025 14:03:34 +0100 Subject: [PATCH] fix(sdk): correctly import e2ee history in `join_room_by_id` It turns out that downstream clients can and do call `Client::join_room_by_id()` rather than `Room::join`, so we need to do the room key history import in the lower-level method. --- crates/matrix-sdk/CHANGELOG.md | 10 +++- crates/matrix-sdk/src/client/mod.rs | 60 ++++++++++++++++++- crates/matrix-sdk/src/room/mod.rs | 35 +---------- .../src/room/shared_room_history.rs | 2 +- .../src/tests/e2ee/shared_history.rs | 6 +- 5 files changed, 70 insertions(+), 43 deletions(-) diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 947b78ea8..1ddcbf5cc 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -6,8 +6,14 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - ReleaseDate -- Add logging to `Room::join`. - ([#5260](https://github.com/matrix-org/matrix-rust-sdk/pull/5260)) +### Bug fixes + +- When joining a room via `Client::join_room_by_id()`, if the client has `enable_share_history_on_invite` enabled, + we will correctly check for received room key bundles. Previously this was only done when calling `Room::join`. + ([#5043](https://github.com/matrix-org/matrix-rust-sdk/pull/5043)) + +### Refactor + - `ClientServerCapabilities` has been renamed to `ClientServerInfo`. Alongside this, `Client::reset_server_info` is now `Client::reset_server_info` and `Client::fetch_server_capabilities` is now `Client::fetch_server_versions`, returning the server versions response directly. diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 67d4f5671..030e4cb28 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -93,6 +93,7 @@ use crate::{ http_client::HttpClient, media::MediaError, notification_settings::NotificationSettings, + room::RoomMember, room_preview::RoomPreview, send_queue::SendQueueData, sliding_sync::Version as SlidingSyncVersion, @@ -1429,11 +1430,42 @@ impl Client { } } + /// Prepare to join a room by ID, by getting the current details about it + async fn prepare_join_room_by_id(&self, room_id: &RoomId) -> Option { + let Some(room) = self.get_room(room_id) else { + // We know nothing about this room. + return None; + }; + + let inviter = match room.invite_details().await { + Ok(details) => details.inviter, + Err(Error::WrongRoomState(_)) => None, + Err(e) => { + warn!("Error fetching invite details for room: {e:?}"); + None + } + }; + + Some(PreJoinRoomInfo { inviter }) + } + /// Finish joining a room. /// /// If the room was an invite that should be marked as a DM, will include it /// in the DM event after creating the joined room. - async fn finish_join_room(&self, room_id: &RoomId) -> Result { + /// + /// If encrypted history sharing is enabled, will check to see if we have a + /// key bundle, and import it if so. + /// + /// # Arguments + /// + /// * `room_id` - The `RoomId` of the room that was joined. + /// * `pre_join_room_info` - Information about the room before we joined. + async fn finish_join_room( + &self, + room_id: &RoomId, + pre_join_room_info: Option, + ) -> Result { let mark_as_dm = if let Some(room) = self.get_room(room_id) { room.state() == RoomState::Invited && room.is_direct().await.unwrap_or_else(|e| { @@ -1451,6 +1483,16 @@ impl Client { room.set_is_direct(true).await?; } + #[cfg(feature = "e2e-encryption")] + if self.inner.enable_share_history_on_invite { + if let Some(inviter) = + pre_join_room_info.as_ref().and_then(|info| info.inviter.as_ref()) + { + crate::room::shared_room_history::maybe_accept_key_bundle(&room, inviter.user_id()) + .await?; + } + } + Ok(room) } @@ -1461,10 +1503,15 @@ impl Client { /// # Arguments /// /// * `room_id` - The `RoomId` of the room to be joined. + #[instrument(skip(self))] pub async fn join_room_by_id(&self, room_id: &RoomId) -> Result { + // See who invited us to this room, if anyone. Note we have to do this before + // making the `/join` request, otherwise we could race against the sync. + let pre_join_info = self.prepare_join_room_by_id(room_id).await; + let request = join_room_by_id::v3::Request::new(room_id.to_owned()); let response = self.send(request).await?; - self.finish_join_room(&response.room_id).await + self.finish_join_room(&response.room_id, pre_join_info).await } /// Join a room by `RoomOrAliasId`. @@ -1486,7 +1533,7 @@ impl Client { via: server_names.to_owned(), }); let response = self.send(request).await?; - self.finish_join_room(&response.room_id).await + self.finish_join_room(&response.room_id, None).await } /// Search the homeserver's directory of public rooms. @@ -2730,6 +2777,13 @@ impl CachedValue { } } +/// Information about the state of a room before we joined it. +#[derive(Debug, Clone, Default)] +struct PreJoinRoomInfo { + /// The user that invited us to the room, if any + pub inviter: Option, +} + // The http mocking library is not supported for wasm32 #[cfg(all(test, not(target_family = "wasm")))] pub(crate) mod tests { diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 46c59cd14..7ccd458cb 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -176,7 +176,7 @@ pub mod reply; pub mod privacy_settings; #[cfg(feature = "e2e-encryption")] -mod shared_room_history; +pub(crate) mod shared_room_history; /// A struct containing methods that are common for Joined, Invited and Left /// Rooms @@ -375,7 +375,6 @@ impl Room { /// /// Only invited and left rooms can be joined via this method. #[doc(alias = "accept_invitation")] - #[instrument(skip_all, fields(room_id = ?self.inner.room_id()))] pub async fn join(&self) -> Result<()> { let prev_room_state = self.inner.state(); @@ -386,40 +385,8 @@ impl Room { )))); } - let inviter = if prev_room_state == RoomState::Invited { - match self.invite_details().await { - Ok(details) => details.inviter, - Err(e) => { - warn!("No invite details were found, can't attempt to find a room key bundle to accept: {e:?}"); - None - } - } - } else { - None - }; - - #[cfg(feature = "e2e-encryption")] - let enable_share_history_on_invite = self.client.inner.enable_share_history_on_invite; - - #[cfg(not(feature = "e2e-encryption"))] - let enable_share_history_on_invite = false; - - debug!( - ?prev_room_state, - inviter=?inviter.as_ref().map(|room_member| room_member.user_id()), - enable_share_history_on_invite, - "Joining room", - ); - self.client.join_room_by_id(self.room_id()).await?; - #[cfg(feature = "e2e-encryption")] - if enable_share_history_on_invite { - if let Some(inviter) = inviter { - shared_room_history::maybe_accept_key_bundle(self, inviter.user_id()).await?; - } - } - Ok(()) } diff --git a/crates/matrix-sdk/src/room/shared_room_history.rs b/crates/matrix-sdk/src/room/shared_room_history.rs index 25827d8ed..a1a9ba979 100644 --- a/crates/matrix-sdk/src/room/shared_room_history.rs +++ b/crates/matrix-sdk/src/room/shared_room_history.rs @@ -112,7 +112,7 @@ pub(super) async fn share_room_history(room: &Room, user_id: OwnedUserId) -> Res /// /// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268 #[instrument(skip(room), fields(room_id = ?room.room_id(), bundle_sender))] -pub(super) async fn maybe_accept_key_bundle(room: &Room, inviter: &UserId) -> Result<()> { +pub(crate) async fn maybe_accept_key_bundle(room: &Room, inviter: &UserId) -> Result<()> { // TODO: retry this if it gets interrupted or it fails. // TODO: do this in the background. diff --git a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs index d88d24b80..1763c290b 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs @@ -86,10 +86,10 @@ async fn test_history_share_on_invite() -> Result<()> { "io.element.msc4268.room_key_bundle" ); - let bob_room = bob.get_room(alice_room.room_id()).expect("Bob should have received the invite"); + bob.get_room(alice_room.room_id()).expect("Bob should have received the invite"); - bob_room - .join() + let bob_room = bob + .join_room_by_id(alice_room.room_id()) .instrument(bob_span.clone()) .await .expect("Bob should be able to accept the invitation from Alice");