Make the introspection endpoint normalise stable and unstable scopes

Author: sandhose

crates/handlers/src/graphql/query/session.rs

@@ -11,7 +11,6 @@ use mas_storage::{
     compat::{CompatSessionFilter, CompatSessionRepository},
     oauth2::OAuth2SessionFilter,
 };
-use oauth2_types::scope::Scope;
 
 use crate::graphql::{
     UserId,
@@ -77,20 +76,11 @@ impl SessionQuery {
             ))));
         }
 
-        // Then, try to find an OAuth 2.0 session. Because we don't have any dedicated
-        // device column, we're looking up using the device scope.
-        // All device IDs can't necessarily be encoded as a scope. If it's not the case,
-        // we'll skip looking for OAuth 2.0 sessions.
-        let Ok(scope_token) = device.to_scope_token() else {
-            repo.cancel().await?;
-
-            return Ok(None);
-        };
-        let scope = Scope::from_iter([scope_token]);
+        // Then, try to find an OAuth 2.0 session.
         let filter = OAuth2SessionFilter::new()
             .for_user(&user)
             .active_only()
-            .with_scope(&scope);
+            .for_device(&device);
         let sessions = repo.oauth2_session().list(filter, pagination).await?;
 
         // It's possible to have multiple active OAuth 2.0 sessions. For now, we just

crates/handlers/src/oauth2/introspection.rs

@@ -4,7 +4,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 // Please see LICENSE in the repository root for full details.
 
-use std::sync::LazyLock;
+use std::{collections::BTreeSet, sync::LazyLock};
 
 use axum::{Json, extract::State, http::HeaderValue, response::IntoResponse};
 use hyper::{HeaderMap, StatusCode};
@@ -24,7 +24,7 @@ use mas_storage::{
 use oauth2_types::{
     errors::{ClientError, ClientErrorCode},
     requests::{IntrospectionRequest, IntrospectionResponse},
-    scope::ScopeToken,
+    scope::{Scope, ScopeToken},
 };
 use opentelemetry::{Key, KeyValue, metrics::Counter};
 use thiserror::Error;
@@ -190,9 +190,33 @@ const INACTIVE: IntrospectionResponse = IntrospectionResponse {
     device_id: None,
 };
 
-const API_SCOPE: ScopeToken = ScopeToken::from_static("urn:matrix:org.matrix.msc2967.client:api:*");
+const UNSTABLE_API_SCOPE: ScopeToken =
+    ScopeToken::from_static("urn:matrix:org.matrix.msc2967.client:api:*");
+const STABLE_API_SCOPE: ScopeToken = ScopeToken::from_static("urn:matrix:client:api:*");
 const SYNAPSE_ADMIN_SCOPE: ScopeToken = ScopeToken::from_static("urn:synapse:admin:*");
 
+/// Normalize a scope by adding the stable and unstable API scopes equivalents
+/// if missing
+fn normalize_scope(mut scope: Scope) -> Scope {
+    // Here we abuse the fact that the scope is a BTreeSet to not care about
+    // duplicates
+    let mut to_add = BTreeSet::new();
+    for token in &*scope {
+        if token == &STABLE_API_SCOPE {
+            to_add.insert(UNSTABLE_API_SCOPE);
+        } else if token == &UNSTABLE_API_SCOPE {
+            to_add.insert(STABLE_API_SCOPE);
+        } else if let Some(device) = Device::from_scope_token(token) {
+            let tokens = device
+                .to_scope_token()
+                .expect("from/to scope token rountrip should never fail");
+            to_add.extend(tokens);
+        }
+    }
+    scope.append(&mut to_add);
+    scope
+}
+
 #[tracing::instrument(
     name = "handlers.oauth2.introspection.post",
     fields(client.id = client_authorization.client_id()),
@@ -311,9 +335,11 @@ pub(crate) async fn post(
                 ],
             );
 
+            let scope = normalize_scope(session.scope);
+
             IntrospectionResponse {
                 active: true,
-                scope: Some(session.scope),
+                scope: Some(scope),
                 client_id: Some(session.client_id.to_string()),
                 username,
                 token_type: Some(OAuthTokenTypeHint::AccessToken),
@@ -382,9 +408,11 @@ pub(crate) async fn post(
                 ],
             );
 
+            let scope = normalize_scope(session.scope);
+
             IntrospectionResponse {
                 active: true,
-                scope: Some(session.scope),
+                scope: Some(scope),
                 client_id: Some(session.client_id.to_string()),
                 username,
                 token_type: Some(OAuthTokenTypeHint::RefreshToken),
@@ -446,9 +474,9 @@ pub(crate) async fn post(
                     .transpose()?
             };
 
-            let scope = [API_SCOPE]
+            let scope = [STABLE_API_SCOPE, UNSTABLE_API_SCOPE]
                 .into_iter()
-                .chain(device_scope_opt)
+                .chain(device_scope_opt.into_iter().flatten())
                 .chain(synapse_admin_scope_opt)
                 .collect();
 
@@ -530,9 +558,9 @@ pub(crate) async fn post(
                     .transpose()?
             };
 
-            let scope = [API_SCOPE]
+            let scope = [STABLE_API_SCOPE, UNSTABLE_API_SCOPE]
                 .into_iter()
-                .chain(device_scope_opt)
+                .chain(device_scope_opt.into_iter().flatten())
                 .chain(synapse_admin_scope_opt)
                 .collect();
 
@@ -879,7 +907,7 @@ mod tests {
         let refresh_token = response["refresh_token"].as_str().unwrap();
         let device_id = response["device_id"].as_str().unwrap();
         let expected_scope: Scope =
-            format!("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:{device_id}")
+            format!("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:{device_id} urn:matrix:client:api:* urn:matrix:client:device:{device_id}")
                 .parse()
                 .unwrap();
 
@@ -912,7 +940,7 @@ mod tests {
         assert_eq!(response.token_type, Some(OAuthTokenTypeHint::AccessToken));
         assert_eq!(
             response.scope.map(|s| s.to_string()),
-            Some("urn:matrix:org.matrix.msc2967.client:api:*".to_owned())
+            Some("urn:matrix:client:api:* urn:matrix:org.matrix.msc2967.client:api:*".to_owned())
         );
         assert_eq!(response.device_id.as_deref(), Some(device_id));
 

crates/oauth2-types/src/scope.rs

@@ -10,7 +10,13 @@
 
 #![allow(clippy::module_name_repetitions)]
 
-use std::{borrow::Cow, collections::BTreeSet, iter::FromIterator, ops::Deref, str::FromStr};
+use std::{
+    borrow::Cow,
+    collections::BTreeSet,
+    iter::FromIterator,
+    ops::{Deref, DerefMut},
+    str::FromStr,
+};
 
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
@@ -121,6 +127,12 @@ impl Deref for Scope {
     }
 }
 
+impl DerefMut for Scope {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
 impl FromStr for Scope {
     type Err = InvalidScope;
 
@@ -248,6 +260,7 @@ mod tests {
         );
 
         assert!(Scope::from_str("http://example.com").is_ok());
-        assert!(Scope::from_str("urn:matrix:org.matrix.msc2967.client:*").is_ok());
+        assert!(Scope::from_str("urn:matrix:client:api:*").is_ok());
+        assert!(Scope::from_str("urn:matrix:org.matrix.msc2967.client:api:*").is_ok());
     }
 }