diff --git a/Cargo.lock b/Cargo.lock index 0621c6ecd..c79fc1f8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3116,6 +3116,7 @@ dependencies = [ "matrix-sdk-ffi-macros", "matrix-sdk-ui", "mime", + "oauth2", "once_cell", "paranoid-android", "ruma", diff --git a/Cargo.toml b/Cargo.toml index b5cdd9945..a5c595088 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ insta = { version = "1.42.1", features = ["json", "redactions"] } itertools = "0.14.0" js-sys = "0.3.69" mime = "0.3.17" +oauth2 = { version = "5.0.0", default-features = false, features = ["reqwest", "timing-resistant-secret-traits"] } once_cell = "1.20.2" pbkdf2 = { version = "0.12.2" } pin-project-lite = "0.2.16" diff --git a/bindings/matrix-sdk-ffi/CHANGELOG.md b/bindings/matrix-sdk-ffi/CHANGELOG.md index f24fa817f..664ac6fdd 100644 --- a/bindings/matrix-sdk-ffi/CHANGELOG.md +++ b/bindings/matrix-sdk-ffi/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. ### Breaking changes: +- `Client::url_for_oidc` now allows requesting additional scopes for the OAuth2 authorization code grant. + ([#5395](https://github.com/matrix-org/matrix-rust-sdk/pull/5395)) - `Client::url_for_oidc` now allows passing an optional existing device id from a previous login call. ([#5394](https://github.com/matrix-org/matrix-rust-sdk/pull/5394)) - `ClientBuilder::build_with_qr_code` has been removed. Instead, the Client should be built by passing diff --git a/bindings/matrix-sdk-ffi/Cargo.toml b/bindings/matrix-sdk-ffi/Cargo.toml index 9d57cc7b1..a75ae6aa7 100644 --- a/bindings/matrix-sdk-ffi/Cargo.toml +++ b/bindings/matrix-sdk-ffi/Cargo.toml @@ -78,6 +78,7 @@ tracing-subscriber = { workspace = true, features = ["env-filter"] } url.workspace = true uuid = { version = "1.4.1", features = ["v4"] } zeroize.workspace = true +oauth2.workspace = true [target.'cfg(target_family = "wasm")'.dependencies] console_error_panic_hook = "0.1.7" diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index c4b7b0165..357b34127 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -51,6 +51,7 @@ use matrix_sdk_ui::{ unable_to_decrypt_hook::UtdHookManager, }; use mime::Mime; +use oauth2::Scope; use ruma::{ api::client::{alias::get_alias, error::ErrorKind, uiaa::UserIdentifier}, events::{ @@ -462,20 +463,34 @@ impl Client { /// If not set, a random one will be generated. It can be an existing /// device ID from a previous login call. Note that this should be done /// only if the client also holds the corresponding encryption keys. + /// + /// * `additional_scopes` - Additional scopes to request from the + /// authorization server, e.g. "urn:matrix:client:com.example.msc9999.foo". + /// The scopes for API access and the device ID according to the + /// [specification](https://spec.matrix.org/v1.15/client-server-api/#allocated-scope-tokens) + /// are always requested. pub async fn url_for_oidc( &self, oidc_configuration: &OidcConfiguration, prompt: Option, login_hint: Option, device_id: Option, + additional_scopes: Option>, ) -> Result, OidcError> { let registration_data = oidc_configuration.registration_data()?; let redirect_uri = oidc_configuration.redirect_uri()?; let device_id = device_id.map(OwnedDeviceId::from); - let mut url_builder = - self.inner.oauth().login(redirect_uri, device_id, Some(registration_data)); + let additional_scopes = + additional_scopes.map(|scopes| scopes.into_iter().map(Scope::new).collect::>()); + + let mut url_builder = self.inner.oauth().login( + redirect_uri, + device_id, + Some(registration_data), + additional_scopes, + ); if let Some(prompt) = prompt { url_builder = url_builder.prompt(vec![prompt.into()]); diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 56c51adcb..f1ee44af7 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -6,6 +6,11 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - ReleaseDate +### Breaking changes: + +- `OAuth::login` now allows requesting additional scopes for the authorization code grant. + ([#5395](https://github.com/matrix-org/matrix-rust-sdk/pull/5395)) + ## [0.13.0] - 2025-07-10 ### Security Fixes diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml index 92e8ec0ad..b1077fac5 100644 --- a/crates/matrix-sdk/Cargo.toml +++ b/crates/matrix-sdk/Cargo.toml @@ -94,7 +94,7 @@ matrix-sdk-sqlite = { workspace = true, optional = true } matrix-sdk-test = { workspace = true, optional = true } mime.workspace = true mime2ext = "0.1.53" -oauth2 = { version = "5.0.0", default-features = false, features = ["reqwest", "timing-resistant-secret-traits"] } +oauth2.workspace = true once_cell.workspace = true percent-encoding = "2.3.1" pin-project-lite.workspace = true diff --git a/crates/matrix-sdk/src/authentication/oauth/mod.rs b/crates/matrix-sdk/src/authentication/oauth/mod.rs index 3f4ae46a7..4d6b6fb22 100644 --- a/crates/matrix-sdk/src/authentication/oauth/mod.rs +++ b/crates/matrix-sdk/src/authentication/oauth/mod.rs @@ -854,7 +854,10 @@ impl OAuth { } /// The scopes to request for logging in and the corresponding device ID. - fn login_scopes(device_id: Option) -> ([Scope; 2], OwnedDeviceId) { + fn login_scopes( + device_id: Option, + additional_scopes: Option>, + ) -> (Vec, OwnedDeviceId) { /// Scope to grand full access to the client-server API. const SCOPE_MATRIX_CLIENT_SERVER_API_FULL_ACCESS: &str = "urn:matrix:org.matrix.msc2967.client:api:*"; @@ -864,13 +867,16 @@ impl OAuth { // Generate the device ID if it is not provided. let device_id = device_id.unwrap_or_else(DeviceId::new); - ( - [ - Scope::new(SCOPE_MATRIX_CLIENT_SERVER_API_FULL_ACCESS.to_owned()), - Scope::new(format!("{SCOPE_MATRIX_DEVICE_ID_PREFIX}{device_id}")), - ], - device_id, - ) + let mut scopes = vec![ + Scope::new(SCOPE_MATRIX_CLIENT_SERVER_API_FULL_ACCESS.to_owned()), + Scope::new(format!("{SCOPE_MATRIX_DEVICE_ID_PREFIX}{device_id}")), + ]; + + if let Some(extra_scopes) = additional_scopes { + scopes.extend(extra_scopes); + } + + (scopes, device_id) } /// Log in via OAuth 2.0 with the Authorization Code flow. @@ -903,6 +909,12 @@ impl OAuth { /// [`OAuth::register_client()`] or [`OAuth::restore_registered_client()`] /// was called previously. /// + /// * `additional_scopes` - Additional scopes to request from the + /// authorization server, e.g. "urn:matrix:client:com.example.msc9999.foo". + /// The scopes for API access and the device ID according to the + /// [specification](https://spec.matrix.org/v1.15/client-server-api/#allocated-scope-tokens) + /// are always requested. + /// /// # Example /// /// ```no_run @@ -921,7 +933,7 @@ impl OAuth { /// let client_metadata: Raw = client_metadata(); /// let registration_data = client_metadata.into(); /// - /// let auth_data = oauth.login(redirect_uri, None, Some(registration_data)) + /// let auth_data = oauth.login(redirect_uri, None, Some(registration_data), None) /// .build() /// .await?; /// @@ -942,8 +954,9 @@ impl OAuth { redirect_uri: Url, device_id: Option, registration_data: Option, + additional_scopes: Option>, ) -> OAuthAuthCodeUrlBuilder { - let (scopes, device_id) = Self::login_scopes(device_id); + let (scopes, device_id) = Self::login_scopes(device_id, additional_scopes); OAuthAuthCodeUrlBuilder::new( self.clone(), @@ -1125,7 +1138,7 @@ impl OAuth { device_id: Option, ) -> Result { - let (scopes, _) = Self::login_scopes(device_id); + let (scopes, _) = Self::login_scopes(device_id, None); let client_id = self.client_id().ok_or(OAuthError::NotRegistered)?.clone(); diff --git a/crates/matrix-sdk/src/authentication/oauth/tests.rs b/crates/matrix-sdk/src/authentication/oauth/tests.rs index 11f3c2f82..55bf3dba8 100644 --- a/crates/matrix-sdk/src/authentication/oauth/tests.rs +++ b/crates/matrix-sdk/src/authentication/oauth/tests.rs @@ -2,7 +2,7 @@ use anyhow::Context as _; use assert_matches::assert_matches; use matrix_sdk_base::store::RoomLoadSettings; use matrix_sdk_test::async_test; -use oauth2::{ClientId, CsrfToken, PkceCodeChallenge, RedirectUrl}; +use oauth2::{ClientId, CsrfToken, PkceCodeChallenge, RedirectUrl, Scope}; use ruma::{ api::client::discovery::get_authorization_server_metadata::v1::Prompt, device_id, owned_device_id, user_id, DeviceId, ServerName, @@ -56,6 +56,7 @@ async fn check_authorization_url( device_id: Option<&DeviceId>, expected_prompt: Option<&str>, expected_login_hint: Option<&str>, + additional_scopes: Option>, ) { tracing::debug!("authorization data URL = {}", authorization_data.url); @@ -85,15 +86,45 @@ async fn check_authorization_url( num_expected -= 1; } "scope" => { - let expected_start = "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:"; - assert!(val.starts_with(expected_start)); - assert!(val.len() > expected_start.len()); + let actual_scopes: Vec = val.split(' ').map(String::from).collect(); + + assert!(actual_scopes.len() >= 2, "Expected at least two scopes"); + + assert!( + actual_scopes + .contains(&"urn:matrix:org.matrix.msc2967.client:api:*".to_owned()), + "Expected Matrix API scope not found in scopes" + ); // Only check the device ID if we know it. If it's generated randomly we don't // know it. if let Some(device_id) = device_id { - assert!(val.ends_with(device_id.as_str())); - assert_eq!(val.len(), expected_start.len() + device_id.as_str().len()); + let device_id_scope = + format!("urn:matrix:org.matrix.msc2967.client:device:{device_id}"); + assert!( + actual_scopes.contains(&device_id_scope), + "Expected device ID scope not found in scopes" + ) + } else { + assert!( + actual_scopes + .iter() + .any(|s| s.starts_with("urn:matrix:org.matrix.msc2967.client:device:")), + "Expected device ID scope not found in scopes" + ); + } + + if let Some(additional_scopes) = &additional_scopes { + // Check if the additional scopes are present in the actual scopes. + let expected_len = 2 + additional_scopes.len(); + assert_eq!(actual_scopes.len(), expected_len, "Expected {expected_len} scopes",); + + for scope in additional_scopes { + assert!( + actual_scopes.contains(scope), + "Expected additional scope not found in scopes: {scope:?}", + ); + } } num_expected -= 1; @@ -146,7 +177,7 @@ async fn test_high_level_login() -> anyhow::Result<()> { // When getting the OIDC login URL. let authorization_data = oauth - .login(redirect_uri.clone(), None, Some(registration_data)) + .login(redirect_uri.clone(), None, Some(registration_data), None) .prompt(vec![Prompt::Create]) .build() .await @@ -169,12 +200,16 @@ async fn test_high_level_login_cancellation() -> anyhow::Result<()> { // Given a client ready to complete login. let (oauth, server, mut redirect_uri, registration_data) = mock_environment().await.unwrap(); - let authorization_data = - oauth.login(redirect_uri.clone(), None, Some(registration_data)).build().await.unwrap(); + let authorization_data = oauth + .login(redirect_uri.clone(), None, Some(registration_data), None) + .build() + .await + .unwrap(); assert_eq!(oauth.client_id().map(|id| id.as_str()), Some("test_client_id")); - check_authorization_url(&authorization_data, &oauth, &server.uri(), None, None, None).await; + check_authorization_url(&authorization_data, &oauth, &server.uri(), None, None, None, None) + .await; // When completing login with a cancellation callback. redirect_uri.set_query(Some(&format!( @@ -200,12 +235,16 @@ async fn test_high_level_login_invalid_state() -> anyhow::Result<()> { // Given a client ready to complete login. let (oauth, server, mut redirect_uri, registration_data) = mock_environment().await.unwrap(); - let authorization_data = - oauth.login(redirect_uri.clone(), None, Some(registration_data)).build().await.unwrap(); + let authorization_data = oauth + .login(redirect_uri.clone(), None, Some(registration_data), None) + .build() + .await + .unwrap(); assert_eq!(oauth.client_id().map(|id| id.as_str()), Some("test_client_id")); - check_authorization_url(&authorization_data, &oauth, &server.uri(), None, None, None).await; + check_authorization_url(&authorization_data, &oauth, &server.uri(), None, None, None, None) + .await; // When completing login with an old/tampered state. redirect_uri.set_query(Some("code=42&state=imposter_alert")); @@ -229,7 +268,7 @@ async fn test_login_url() -> anyhow::Result<()> { let server_uri = server.uri(); let oauth_server = server.oauth(); - oauth_server.mock_server_metadata().ok().expect(3).mount().await; + oauth_server.mock_server_metadata().ok().expect(4).mount().await; let client = server.client_builder().registered_with_oauth().build().await; let oauth = client.oauth(); @@ -239,15 +278,26 @@ async fn test_login_url() -> anyhow::Result<()> { let redirect_uri_str = REDIRECT_URI_STRING; let redirect_uri = Url::parse(redirect_uri_str)?; + let additional_scopes = + vec![Scope::new("urn:test:scope1".to_owned()), Scope::new("urn:test:scope2".to_owned())]; + // No extra parameters. let authorization_data = - oauth.login(redirect_uri.clone(), Some(device_id.clone()), None).build().await?; - check_authorization_url(&authorization_data, &oauth, &server_uri, Some(&device_id), None, None) - .await; + oauth.login(redirect_uri.clone(), Some(device_id.clone()), None, None).build().await?; + check_authorization_url( + &authorization_data, + &oauth, + &server_uri, + Some(&device_id), + None, + None, + None, + ) + .await; // With prompt parameter. let authorization_data = oauth - .login(redirect_uri.clone(), Some(device_id.clone()), None) + .login(redirect_uri.clone(), Some(device_id.clone()), None, None) .prompt(vec![Prompt::Create]) .build() .await?; @@ -258,12 +308,13 @@ async fn test_login_url() -> anyhow::Result<()> { Some(&device_id), Some("create"), None, + None, ) .await; // With user_id_hint parameter. let authorization_data = oauth - .login(redirect_uri.clone(), Some(device_id.clone()), None) + .login(redirect_uri.clone(), Some(device_id.clone()), None, None) .user_id_hint(user_id!("@joe:example.org")) .build() .await?; @@ -274,6 +325,23 @@ async fn test_login_url() -> anyhow::Result<()> { Some(&device_id), None, Some("mxid:@joe:example.org"), + None, + ) + .await; + + // With additional scopes. + let authorization_data = oauth + .login(redirect_uri.clone(), Some(device_id.clone()), None, Some(additional_scopes.clone())) + .build() + .await?; + check_authorization_url( + &authorization_data, + &oauth, + &server_uri, + Some(&device_id), + None, + None, + Some(additional_scopes), ) .await; diff --git a/examples/oauth_cli/src/main.rs b/examples/oauth_cli/src/main.rs index f3ef04871..5d255be96 100644 --- a/examples/oauth_cli/src/main.rs +++ b/examples/oauth_cli/src/main.rs @@ -187,8 +187,10 @@ impl OAuthCli { // the redirect when the custom URI scheme is opened. let (redirect_uri, server_handle) = LocalServerBuilder::new().spawn().await?; - let OAuthAuthorizationData { url, .. } = - oauth.login(redirect_uri, None, Some(client_metadata().into())).build().await?; + let OAuthAuthorizationData { url, .. } = oauth + .login(redirect_uri, None, Some(client_metadata().into()), None) + .build() + .await?; let query_string = use_auth_url(&url, server_handle).await.map(|query| query.0).unwrap_or_default();