From d34e01fc67a9b5e9feda4c8f74c1e2ef0de60d81 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 19 Apr 2023 16:32:41 +0200 Subject: [PATCH] Provision and delete Matrix devices in OAuth sessions --- crates/data-model/src/compat/device.rs | 27 ++++++++++------- .../src/oauth2/authorization/complete.rs | 7 +++-- crates/handlers/src/oauth2/consent.rs | 5 ++-- crates/handlers/src/oauth2/revoke.rs | 30 +++++++++++++++++-- crates/handlers/src/oauth2/token.rs | 19 ++++++++++-- crates/oauth2-types/src/scope.rs | 8 ++++- 6 files changed, 75 insertions(+), 21 deletions(-) diff --git a/crates/data-model/src/compat/device.rs b/crates/data-model/src/compat/device.rs index cac0f242..2f384e12 100644 --- a/crates/data-model/src/compat/device.rs +++ b/crates/data-model/src/compat/device.rs @@ -20,7 +20,8 @@ use rand::{ use serde::{Deserialize, Serialize}; use thiserror::Error; -static DEVICE_ID_LENGTH: usize = 10; +static GENERATED_DEVICE_ID_LENGTH: usize = 10; +static DEVICE_SCOPE_PREFIX: &str = "urn:matrix:org.matrix.msc2967.client:device:"; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(transparent)] @@ -30,9 +31,6 @@ pub struct Device { #[derive(Debug, Error)] pub enum InvalidDeviceID { - #[error("Device ID does not have the right size")] - InvalidLength, - #[error("Device ID contains invalid characters")] InvalidCharacters, } @@ -42,14 +40,24 @@ impl Device { #[must_use] pub fn to_scope_token(&self) -> ScopeToken { // SAFETY: the inner id should only have valid scope characters - format!("urn:matrix:org.matrix.msc2967.client:device:{}", self.id) + format!("{DEVICE_SCOPE_PREFIX}:{}", self.id) .parse() .unwrap() } + /// Get the corresponding [`Device`] from a [`ScopeToken`] + /// + /// Returns `None` if the [`ScopeToken`] is not a device scope + #[must_use] + pub fn from_scope_token(token: &ScopeToken) -> Option { + let id = token.as_str().strip_prefix(DEVICE_SCOPE_PREFIX)?; + // XXX: we might be silently ignoring errors here, but it's probably fine? + Device::try_from(id.to_owned()).ok() + } + /// Generate a random device ID pub fn generate(rng: &mut R) -> Self { - let id: String = Alphanumeric.sample_string(rng, DEVICE_ID_LENGTH); + let id: String = Alphanumeric.sample_string(rng, GENERATED_DEVICE_ID_LENGTH); Self { id } } @@ -65,11 +73,8 @@ impl TryFrom for Device { /// Create a [`Device`] out of an ID, validating the ID has the right shape fn try_from(id: String) -> Result { - if id.len() != DEVICE_ID_LENGTH { - return Err(InvalidDeviceID::InvalidLength); - } - - if !id.chars().all(|c| c.is_ascii_alphanumeric()) { + // This matches the regex in the policy + if !id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { return Err(InvalidDeviceID::InvalidCharacters); } diff --git a/crates/handlers/src/oauth2/authorization/complete.rs b/crates/handlers/src/oauth2/authorization/complete.rs index 41912dbe..ccbee9f6 100644 --- a/crates/handlers/src/oauth2/authorization/complete.rs +++ b/crates/handlers/src/oauth2/authorization/complete.rs @@ -21,7 +21,7 @@ use axum::{ use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::SessionInfoExt; -use mas_data_model::{AuthorizationGrant, BrowserSession, Client}; +use mas_data_model::{AuthorizationGrant, BrowserSession, Client, Device}; use mas_keystore::{Encrypter, Keystore}; use mas_policy::PolicyFactory; use mas_router::{PostAuthAction, Route, UrlBuilder}; @@ -217,9 +217,10 @@ pub(crate) async fn complete( let lacks_consent = grant .scope .difference(¤t_consent) - .any(|scope| !scope.starts_with("urn:matrix:org.matrix.msc2967.client:device:")); + .filter(|scope| Device::from_scope_token(scope).is_none()) + .any(|_| true); - // Check if the client lacks consent *or* if consent was explicitely asked + // Check if the client lacks consent *or* if consent was explicitly asked if lacks_consent || grant.requires_consent { repo.save().await?; return Err(GrantCompletionError::RequiresConsent); diff --git a/crates/handlers/src/oauth2/consent.rs b/crates/handlers/src/oauth2/consent.rs index e69e4770..5f8c9d23 100644 --- a/crates/handlers/src/oauth2/consent.rs +++ b/crates/handlers/src/oauth2/consent.rs @@ -24,7 +24,7 @@ use mas_axum_utils::{ csrf::{CsrfExt, ProtectedForm}, SessionInfoExt, }; -use mas_data_model::AuthorizationGrantStage; +use mas_data_model::{AuthorizationGrantStage, Device}; use mas_keystore::Encrypter; use mas_policy::PolicyFactory; use mas_router::{PostAuthAction, Route}; @@ -190,9 +190,10 @@ pub(crate) async fn post( let scope_without_device = grant .scope .iter() - .filter(|s| !s.starts_with("urn:matrix:org.matrix.msc2967.client:device:")) + .filter(|s| Device::from_scope_token(s).is_none()) .cloned() .collect(); + repo.oauth2_client() .give_consent_for_user( &mut rng, diff --git a/crates/handlers/src/oauth2/revoke.rs b/crates/handlers/src/oauth2/revoke.rs index 81ce49f0..f19d7ce0 100644 --- a/crates/handlers/src/oauth2/revoke.rs +++ b/crates/handlers/src/oauth2/revoke.rs @@ -18,10 +18,14 @@ use mas_axum_utils::{ client_authorization::{ClientAuthorization, CredentialsVerificationError}, http_client_factory::HttpClientFactory, }; -use mas_data_model::TokenType; +use mas_data_model::{Device, TokenType}; use mas_iana::oauth::OAuthTokenTypeHint; use mas_keystore::Encrypter; -use mas_storage::{BoxClock, BoxRepository}; +use mas_storage::{ + job::{DeleteDeviceJob, JobRepositoryExt}, + user::BrowserSessionRepository, + BoxClock, BoxRepository, RepositoryAccess, +}; use oauth2_types::{ errors::{ClientError, ClientErrorCode}, requests::RevocationRequest, @@ -196,6 +200,28 @@ pub(crate) async fn post( return Err(RouteError::UnauthorizedClient); } + // Fetch the user session + let user_session = repo + .browser_session() + .lookup(session.user_session_id) + .await? + .ok_or(RouteError::UnknownToken)?; + + // Scan the scopes of the session to find if there is any device that should be + // deleted from the Matrix server. + // TODO: this should be moved in a higher level "end oauth session" method. + // XXX: this might not be the right semantic, but it's the best we + // can do for now, since we're not explicitly storing devices for OAuth2 + // sessions. + for scope in session.scope.iter() { + if let Some(device) = Device::from_scope_token(scope) { + // Schedule a job to delete the device. + repo.job() + .schedule_job(DeleteDeviceJob::new(&user_session.user, &device)) + .await?; + } + } + // Now that we checked everything, we can end the session. repo.oauth2_session().finish(&clock, session).await?; diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 235f7500..b42e7658 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -20,16 +20,17 @@ use mas_axum_utils::{ client_authorization::{ClientAuthorization, CredentialsVerificationError}, http_client_factory::HttpClientFactory, }; -use mas_data_model::{AuthorizationGrantStage, Client}; +use mas_data_model::{AuthorizationGrantStage, Client, Device}; use mas_keystore::{Encrypter, Keystore}; use mas_router::UrlBuilder; use mas_storage::{ + job::{JobRepositoryExt, ProvisionDeviceJob}, oauth2::{ OAuth2AccessTokenRepository, OAuth2AuthorizationGrantRepository, OAuth2RefreshTokenRepository, OAuth2SessionRepository, }, user::BrowserSessionRepository, - BoxClock, BoxRepository, BoxRng, Clock, + BoxClock, BoxRepository, BoxRng, Clock, RepositoryAccess, }; use oauth2_types::{ errors::{ClientError, ClientErrorCode}, @@ -328,6 +329,20 @@ async fn authorization_code_grant( params = params.with_id_token(id_token); } + // Look for device to provision + for scope in session.scope.iter() { + if let Some(device) = Device::from_scope_token(scope) { + // Note that we're not waiting for the job to finish, we just schedule it. We + // might get in a situation where the provisioning job is not finished when the + // client does its first request to the Homeserver. This is fine for now, since + // Synapse still provision devices on-the-fly if it doesn't find them in the + // database. + repo.job() + .schedule_job(ProvisionDeviceJob::new(&browser_session.user, &device)) + .await?; + } + } + repo.oauth2_authorization_grant() .exchange(clock, authz_grant) .await?; diff --git a/crates/oauth2-types/src/scope.rs b/crates/oauth2-types/src/scope.rs index ab6299b0..c4b75d7b 100644 --- a/crates/oauth2-types/src/scope.rs +++ b/crates/oauth2-types/src/scope.rs @@ -39,6 +39,12 @@ impl ScopeToken { pub const fn from_static(token: &'static str) -> Self { Self(Cow::Borrowed(token)) } + + /// Get the scope token as a string slice. + #[must_use] + pub fn as_str(&self) -> &str { + self.0.as_ref() + } } /// `openid`. @@ -115,7 +121,7 @@ impl std::fmt::Display for ScopeToken { #[derive(Debug, Clone, PartialEq, Eq)] pub struct Scope(BTreeSet); -impl std::ops::Deref for Scope { +impl Deref for Scope { type Target = BTreeSet; fn deref(&self) -> &Self::Target {