Provision and delete Matrix devices in OAuth sessions

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<Self> {
+        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<R: RngCore + ?Sized>(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<String> for Device {
 
     /// Create a [`Device`] out of an ID, validating the ID has the right shape
     fn try_from(id: String) -> Result<Self, Self::Error> {
-        if id.len() != DEVICE_ID_LENGTH {
+        // This matches the regex in the policy
-            return Err(InvalidDeviceID::InvalidLength);
+        if !id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') {
-        }
-
-        if !id.chars().all(|c| c.is_ascii_alphanumeric()) {
             return Err(InvalidDeviceID::InvalidCharacters);
         }
 

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(&current_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);

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,

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?;
 

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?;

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<ScopeToken>);
 
-impl std::ops::Deref for Scope {
+impl Deref for Scope {
     type Target = BTreeSet<ScopeToken>;
 
     fn deref(&self) -> &Self::Target {