Skip to content
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.

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

sdk: Fix double bootstrap #3019

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,11 @@ impl Client {
device_id: Option<String>,
) -> Result<(), ClientError> {
RUNTIME.block_on(async move {
let mut builder = self.inner.matrix_auth().login_username(&username, &password);
let mut builder = self
.inner
.matrix_auth()
.with_initialize_e2e_by_default(false)
.login_username(&username, &password);
if let Some(initial_device_name) = initial_device_name.as_ref() {
builder = builder.initial_device_display_name(initial_device_name);
}
Expand Down
32 changes: 29 additions & 3 deletions crates/matrix-sdk/src/encryption/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1224,15 +1224,41 @@ impl Encryption {
Ok(olm_machine.uploaded_key_count().await?)
}

/// Enables event listeners for the E2EE support.
/// Bootstrap encryption and enables event listeners for the E2EE support.
///
/// For now enables only listeners for backups. Should be called once we
/// Based on the `EncryptionSettings`, this call might:
/// - Boostrap cross-signing if needed (POST `/device_signing/upload`)
/// - Create a key backup if needed (POST `/room_keys/version`)
/// - Create a secret storage if needed (PUT `/account_data/{type}`)
///
/// As part of this process, and if needed, the current device keys would be
/// uploaded to the server, new account data would be added, and cross
/// signing keys and signatures might be uploaded.
///
/// Should be called once we
/// created a [`OlmMachine`], i.e. after logging in.
pub(crate) async fn run_initialization_tasks(&self) -> Result<()> {
///
/// # Arguments
///
/// * `auth_data` - Some requests might require re-authentication, to avoid
/// asking the user to enter
/// their password (or any other methods) again, we can pass the auth data
/// here. This is required for uploading cross-signing keys if there are
/// none yet. Notice that there is an MSC proposal to remove this
/// requirement `MSC3967`, allowing to upload cross-signing keys without
/// authentication when for the first time, thus making this parameter
/// useless.
pub(crate) async fn run_initialization_tasks(&self, auth_data: Option<AuthData>) -> Result<()> {
let mut tasks = self.client.inner.tasks.lock().unwrap();

let this = self.clone();
tasks.setup_e2ee = Some(spawn(async move {
if this.settings().auto_enable_cross_signing {
if let Err(e) = this.bootstrap_cross_signing_if_needed(auth_data).await {
error!("Couldn't bootstrap cross signing {e:?}");
}
}

if let Err(e) = this.backups().setup_and_resume().await {
error!("Couldn't setup and resume backups {e:?}");
}
Expand Down
19 changes: 1 addition & 18 deletions crates/matrix-sdk/src/matrix_auth/login_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,24 +183,7 @@ impl LoginBuilder {
});

let response = client.send(request, Some(RequestConfig::short_retry())).await?;
self.auth.receive_login_response(&response).await?;

// This may block login for a while, but the user asked for it!
// TODO: (#2763) put this into a background task.
#[cfg(feature = "e2e-encryption")]
{
use ruma::api::client::uiaa::{AuthData, Password};

let auth_data = match login_info {
login::v3::LoginInfo::Password(p) => {
Some(AuthData::Password(Password::new(p.identifier, p.password)))
}
// Other methods can't be immediately translated to an auth.
_ => None,
};

client.post_login_cross_signing(auth_data).await;
}
self.auth.receive_login_response(&response, Some(login_info)).await?;

Ok(response)
}
Expand Down
77 changes: 51 additions & 26 deletions crates/matrix-sdk/src/matrix_auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@ use eyeball::SharedObservable;
use futures_core::Stream;
use futures_util::StreamExt;
use matrix_sdk_base::SessionMeta;
#[cfg(feature = "e2e-encryption")]
use ruma::api::client::uiaa::{AuthData as RumaUiaaAuthData, Password as RumaUiaaPassword};
use ruma::{
api::{
client::{
account::register,
session::{
get_login_types, login, logout, refresh_token, sso_login, sso_login_with_provider,
get_login_types,
login::{
self,
v3::{LoginInfo, Password},
},
logout, refresh_token, sso_login, sso_login_with_provider,
},
uiaa::UserIdentifier,
},
Expand Down Expand Up @@ -73,17 +76,34 @@ impl fmt::Debug for MatrixAuthData {
#[derive(Debug, Clone)]
pub struct MatrixAuth {
client: Client,
initialize_e2e_by_default: bool,
}

impl MatrixAuth {
pub(crate) fn new(client: Client) -> Self {
Self { client }
Self { client, initialize_e2e_by_default: true }
}

fn data(&self) -> Option<&MatrixAuthData> {
self.client.inner.auth_ctx.auth_data.get()?.as_matrix()
}

///
/// If set to `true`, the client will automatically initialize end-to-end
/// encryption (bootstrap cross signing, create a backup, etc.)
/// when logging in for the first time.
/// If set to `false`, the caller will have to manually call
/// `run_initialization_tasks` in order to bootstrap encryption. Default
/// is `true`.
///
/// # Arguments
/// * `initialize_e2e_by_default` - Whether end-to-end encryption should be
/// initialized by default.
pub fn with_initialize_e2e_by_default(mut self, initialize_e2e_by_default: bool) -> MatrixAuth {
self.initialize_e2e_by_default = initialize_e2e_by_default;
self
}

/// Gets the homeserver’s supported login types.
///
/// This should be the first step when trying to log in so you can call the
Expand Down Expand Up @@ -547,19 +567,18 @@ impl MatrixAuth {
pub async fn register(&self, request: register::v3::Request) -> Result<register::v3::Response> {
let homeserver = self.client.homeserver();
info!("Registering to {homeserver}");
#[cfg(feature = "e2e-encryption")]
let auth_data = match (&request.username, &request.password) {
(Some(u), Some(p)) => Some(RumaUiaaAuthData::Password(RumaUiaaPassword::new(
UserIdentifier::UserIdOrLocalpart(u.clone()),

let login_info = match (&request.username, &request.password) {
(Some(u), Some(p)) => Some(LoginInfo::Password(Password::new(
UserIdentifier::UserIdOrLocalpart(u.into()),
p.clone(),
))),
_ => None,
};

let response = self.client.send(request, None).await?;
if let Some(session) = MatrixSession::from_register_response(&response) {
let _ = self.set_session(session).await;
#[cfg(feature = "e2e-encryption")]
self.client.post_login_cross_signing(auth_data).await;
let _ = self.set_session(session, login_info).await;
}
Ok(response)
}
Expand Down Expand Up @@ -819,7 +838,7 @@ impl MatrixAuth {
#[instrument(skip_all)]
pub async fn restore_session(&self, session: MatrixSession) -> Result<()> {
debug!("Restoring Matrix auth session");
self.set_session(session).await?;
self.set_session(session, None).await?;
debug!("Done restoring Matrix auth session");
Ok(())
}
Expand All @@ -833,35 +852,41 @@ impl MatrixAuth {
pub(crate) async fn receive_login_response(
&self,
response: &login::v3::Response,
login_info: Option<LoginInfo>,
) -> Result<()> {
self.client.maybe_update_login_well_known(response.well_known.as_ref());

self.set_session(response.into()).await?;
self.set_session(response.into(), login_info).await?;

Ok(())
}

async fn set_session(&self, session: MatrixSession) -> Result<()> {
async fn set_session(
&self,
session: MatrixSession,
login_info: Option<LoginInfo>,
) -> Result<()> {
self.set_session_tokens(session.tokens);
self.client.set_session_meta(session.meta).await?;

#[cfg(feature = "e2e-encryption")]
self.client.encryption().run_initialization_tasks().await?;
{
use ruma::api::client::uiaa::{AuthData, Password};

Ok(())
}
}
let auth_data = match login_info {
Some(login::v3::LoginInfo::Password(p)) => {
Some(AuthData::Password(Password::new(p.identifier, p.password)))
}
// Other methods can't be immediately translated to an auth.
_ => None,
};

// Internal client helpers
impl Client {
#[cfg(feature = "e2e-encryption")]
pub(crate) async fn post_login_cross_signing(&self, auth_data: Option<RumaUiaaAuthData>) {
let encryption = self.encryption();
if encryption.settings().auto_enable_cross_signing {
if let Err(err) = encryption.bootstrap_cross_signing_if_needed(auth_data).await {
tracing::warn!("cross-signing bootstrapping failed: {err}");
if self.initialize_e2e_by_default {
self.client.encryption().run_initialization_tasks(auth_data).await?;
}
}

Ok(())
}
}

Expand Down
9 changes: 2 additions & 7 deletions crates/matrix-sdk/src/oidc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ impl Oidc {
}

#[cfg(feature = "e2e-encryption")]
self.client.encryption().run_initialization_tasks().await?;
self.client.encryption().run_initialization_tasks(None).await?;

Ok(())
}
Expand Down Expand Up @@ -920,11 +920,6 @@ impl Oidc {
// Enable the cross-process lock for refreshes, if needs be.
self.deferred_enable_cross_process_refresh_lock().await?;

// Bootstrap cross signing, if needs be.
// TODO: (#2763) put this into a background task.
#[cfg(feature = "e2e-encryption")]
self.client.post_login_cross_signing(None).await;

if let Some(cross_process_manager) = self.ctx().cross_process_token_refresh_manager.get() {
if let Some(tokens) = self.session_tokens() {
let mut cross_process_guard = cross_process_manager
Expand All @@ -950,7 +945,7 @@ impl Oidc {
}

#[cfg(feature = "e2e-encryption")]
self.client.encryption().run_initialization_tasks().await?;
self.client.encryption().run_initialization_tasks(None).await?;

Ok(())
}
Expand Down
6 changes: 6 additions & 0 deletions crates/matrix-sdk/tests/integration/matrix_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ async fn test_login_with_cross_signing_bootstrapping() {
assert!(client.logged_in(), "Client should be logged in");
assert!(auth.logged_in(), "Client should be logged in with the MatrixAuth API");

client.encryption().wait_for_e2ee_initialization_tasks().await;

let me = client.user_id().expect("we are now logged in");
let own_identity =
client.encryption().get_user_identity(me).await.expect("succeeds").expect("is present");
Expand Down Expand Up @@ -503,6 +505,8 @@ async fn test_login_with_cross_signing_bootstrapping() {
assert!(client.logged_in(), "Client should be logged in");
assert!(auth.logged_in(), "Client should be logged in with the MatrixAuth API");

client.encryption().wait_for_e2ee_initialization_tasks().await;

let me = client.user_id().expect("we are now logged in");
let own_identity =
client.encryption().get_user_identity(me).await.expect("succeeds").expect("is present");
Expand Down Expand Up @@ -579,6 +583,8 @@ async fn test_login_doesnt_fail_if_cross_signing_bootstrapping_failed() {

let me = client.user_id().expect("we are now logged in");

client.encryption().wait_for_e2ee_initialization_tasks().await;

let own_identity = client.encryption().get_user_identity(me).await.expect("succeeds");
let identity = own_identity.expect("created local default identity");
assert!(identity.is_verified());
Expand Down