Skip to content

sdk: share room history when we send an invite #4946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 23, 2025
Merged

sdk: share room history when we send an invite #4946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96bdd91
sdk: share room history when we send an invite
richvdh Apr 16, 2025
18f20a7
sdk: only share history if cross-signing is set up
richvdh Apr 16, 2025
6e96391
sdk: clean up imports
richvdh Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/matrix-sdk/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ uniffi = ["dep:uniffi", "matrix-sdk-base/uniffi", "dep:matrix-sdk-ffi-macros"]
experimental-widgets = ["dep:uuid"]

docsrs = ["e2e-encryption", "sqlite", "indexeddb", "sso-login", "qrcode"]
experimental-share-history-on-invite = []

[dependencies]
anyhow = { workspace = true, optional = true }
Expand Down
25 changes: 6 additions & 19 deletions crates/matrix-sdk/src/room/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,7 @@ use crate::{
BaseRoom, Client, Error, HttpResult, Result, RoomState, TransmissionProgress,
};
#[cfg(feature = "e2e-encryption")]
use crate::{
crypto::types::events::CryptoContextInfo, encryption::backups::BackupState,
room::shared_room_history::share_room_history,
};
use crate::{crypto::types::events::CryptoContextInfo, encryption::backups::BackupState};

pub mod edit;
pub mod futures;
Expand Down Expand Up @@ -1480,6 +1477,11 @@ impl Room {
/// * `user_id` - The `UserId` of the user to invite to the room.
#[instrument(skip_all)]
pub async fn invite_user_by_id(&self, user_id: &UserId) -> Result<()> {
#[cfg(all(feature = "experimental-share-history-on-invite", feature = "e2e-encryption"))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other ways to invite a user, don't we need to take care of them as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like what? invite_user_by_3pid? Ideally yes, but this flow isn't going to work there, because we don't have a matrix ID for the target user. We ought to think about it at some point, but I think it's well out of scope for this prototyping effort. I've added a note at element-hq/element-meta#2685 (comment).

{
shared_room_history::share_room_history(self, user_id.to_owned()).await?;
}

Comment on lines +1480 to +1484
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are we certain that people won't want to have a way to say "Ok, invite this person, but absolutely don't share the room history with them"?

I can see that we might want to do this by default, but I can also see that some people might want to opt out.

This might be a nice place to introduce a named future, which would allow us to:

  1. Make this configurable
  2. Not break the API despite having newly added configuration options.

I don't mean that we should do this right now, but perhaps down the line as we stabilize support for this feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we've discussed this in the past, we have talked about not wanting different people to have different views of a room, especially because if the person with incomplete history then invites another person, they also have incomplete history, and it gets very confusing.

So I think we don't want this option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess that weakens my argument towards "the SDK shouldn't be as opinionated as the Element clients".

But ok, I think we can revisit this and discuss later if the SDK should make this optional.

let recipient = InvitationRecipient::UserId { user_id: user_id.to_owned() };
let request = invite_user::v3::Request::new(self.room_id().to_owned(), recipient);
self.client.send(request).await?;
Expand Down Expand Up @@ -1806,21 +1808,6 @@ impl Room {
Ok(())
}

/// Share any shareable E2EE history in this room with the given recipient,
/// as per [MSC4268].
///
/// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268
///
/// This is temporarily exposed for integration testing as part of
/// experimental work on history sharing. In future, it will be combined
/// with sending an invite.
#[cfg(feature = "e2e-encryption")]
#[doc(hidden)]
#[instrument(skip_all, fields(room_id = ?self.room_id(), ?user_id))]
pub async fn share_history<'a>(&'a self, user_id: &UserId) -> Result<()> {
share_room_history(self, user_id.to_owned()).await
}

/// Wait for the room to be fully synced.
///
/// This method makes sure the room that was returned when joining a room
Expand Down
19 changes: 16 additions & 3 deletions crates/matrix-sdk/src/room/shared_room_history.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,30 @@
use std::iter;

use ruma::OwnedUserId;
use tracing::{info, instrument, warn};

use crate::{crypto::types::events::room_key_bundle::RoomKeyBundleContent, Error, Result, Room};

/// Share any shareable E2EE history in the given room with the given recipient,
/// as per [MSC4268].
///
/// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268
#[instrument(skip(room), fields(room_id = ?room.room_id()))]
pub async fn share_room_history(room: &Room, user_id: OwnedUserId) -> Result<()> {
tracing::info!("Sharing message history in {} with {}", room.room_id(), user_id);
let client = &room.client;

// 0. We can only share room history if our user has set up cross signing
let own_identity = match client.user_id() {
Some(own_user) => client.encryption().get_user_identity(own_user).await?,
None => None,
};
if own_identity.is_none() {
warn!("Not sharing message history as cross-signing is not set up");
return Ok(());
}

info!("Sharing message history");

// 1. Construct the key bundle
let bundle = {
let olm_machine = client.olm_machine().await;
Expand All @@ -34,7 +47,7 @@ pub async fn share_room_history(room: &Room, user_id: OwnedUserId) -> Result<()>
};

if bundle.is_empty() {
tracing::info!("No keys to share");
info!("No keys to share");
return Ok(());
}

Expand All @@ -43,7 +56,7 @@ pub async fn share_room_history(room: &Room, user_id: OwnedUserId) -> Result<()>
let upload =
client.upload_encrypted_file(&mime::APPLICATION_JSON, &mut (json.as_slice())).await?;

tracing::info!(
info!(
media_url = ?upload.url,
shared_keys = bundle.room_keys.len(),
withheld_keys = bundle.withheld.len(),
Expand Down
2 changes: 1 addition & 1 deletion labs/multiverse/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ crossterm = "0.28.1"
futures-util = { workspace = true }
imbl = { workspace = true }
itertools = { workspace = true }
matrix-sdk = { path = "../../crates/matrix-sdk", features = ["sso-login"] }
matrix-sdk = { path = "../../crates/matrix-sdk", features = ["sso-login", "experimental-share-history-on-invite"] }
matrix-sdk-base = { path = "../../crates/matrix-sdk-base" }
matrix-sdk-common = { path = "../../crates/matrix-sdk-common" }
matrix-sdk-ui = { path = "../../crates/matrix-sdk-ui" }
Expand Down
2 changes: 1 addition & 1 deletion testing/matrix-sdk-integration-testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ futures = { version = "0.3.29", features = ["executor"] }
futures-core = { workspace = true }
futures-util = { workspace = true }
http = { workspace = true }
matrix-sdk = {workspace = true, default-features = true, features = ["testing", "qrcode"] }
matrix-sdk = {workspace = true, default-features = true, features = ["testing", "qrcode", "experimental-share-history-on-invite"] }
matrix-sdk-base = { workspace = true, default-features = true, features = ["testing", "qrcode"] }
matrix-sdk-ui = { workspace = true, default-features = true }
matrix-sdk-test = { workspace = true }
Expand Down
3 changes: 1 addition & 2 deletions testing/matrix-sdk-integration-testing/src/tests/e2ee.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,8 +1289,7 @@ async fn test_history_share_on_invite() -> Result<()> {
.expect("We should be able to send a message to the room");

// Alice invites Bob to the room
// TODO: invite Bob rather than just call `share_history`
alice_room.share_history(bob.user_id().unwrap()).await?;
alice_room.invite_user_by_id(bob.user_id().unwrap()).await?;

let bob_response = bob.sync_once().instrument(bob_span.clone()).await?;

Expand Down
Loading