Oauth scopes (graze-social#49)

Author: ngerakines

Cargo.lock

@@ -19,11 +19,11 @@ path = "src/lib.rs"
 [dependencies]
 axum = { version = "0.8" }
 
-atproto-identity = { version = "0.11.0", features = ["lru", "zeroize", "hickory-dns"] }
-atproto-oauth = { version = "0.11.0", features = ["lru", "zeroize", "hickory-dns"] }
-atproto-oauth-axum = { version = "0.11.0", features = ["zeroize"] }
-atproto-client = { version = "0.11.0" }
-atproto-xrpcs = { version = "0.11.0", features = ["hickory-dns"] }
+atproto-identity = { version = "0.11.3", features = ["lru", "zeroize", "hickory-dns"] }
+atproto-oauth = { version = "0.11.3", features = ["lru", "zeroize", "hickory-dns"] }
+atproto-oauth-axum = { version = "0.11.3", features = ["zeroize"] }
+atproto-client = { version = "0.11.3" }
+atproto-xrpcs = { version = "0.11.3", features = ["hickory-dns"] }
 
 axum-template = { version = "3.0", features = ["minijinja"] }
 minijinja = { version = "2.7", features = ["builtins"] }

Cargo.toml

@@ -2,10 +2,16 @@
 
 use anyhow::Result;
 use atproto_identity::key::{KeyData, identify_key, generate_key, KeyType};
+use atproto_oauth::scopes::Scope;
 use std::time::Duration;
 
 use crate::errors::ConfigError;
 
+/// ATProtocol OAuth client metadata endpoint path
+/// This is the path where the ATProtocol client metadata document is served.
+/// The full URL is constructed by prepending the external_base URL.
+pub const ATPROTO_CLIENT_METADATA_PATH: &str = "/oauth-client-metadata.json";
+
 /// HTTP server port configuration
 #[derive(Clone)]
 pub struct HttpPort(u16);
@@ -28,7 +34,7 @@ pub struct PrivateKeys(Vec<KeyData>);
 
 /// OAuth supported scopes configuration
 #[derive(Clone)]
-pub struct OAuthSupportedScopes(Vec<String>);
+pub struct OAuthSupportedScopes(Vec<Scope>);
 
 /// Client default access token expiration configuration
 #[derive(Clone)]
@@ -115,7 +121,7 @@ impl Config {
         let oauth_signing_keys: PrivateKeys = optional_env("OAUTH_SIGNING_KEYS").try_into()?;
         let oauth_supported_scopes: OAuthSupportedScopes = default_env(
             "OAUTH_SUPPORTED_SCOPES",
-            "openid profile email atproto:atproto atproto:transition:generic atproto:transition:email",
+            "openid profile email atproto transition:generic transition:email",
         )
         .try_into()?;
         let plc_hostname = default_env("PLC_HOSTNAME", "plc.directory");
@@ -370,17 +376,23 @@ impl TryFrom<Option<String>> for OAuthSupportedScopes {
     fn try_from(value: Option<String>) -> Result<Self, Self::Error> {
         let value = value.unwrap_or_default();
         if value.is_empty() {
-            return Ok(Self(vec![
-                "atproto:atproto".to_string(),
-                "atproto:transition:generic".to_string(),
-                "atproto:transition:email".to_string(),
-            ]));
+            // Parse default scopes
+            let default_scopes = "atproto transition:generic transition:email";
+            let scopes = Scope::parse_multiple_reduced(default_scopes).map_err(|e| {
+                ConfigError::InvalidScope(format!("Failed to parse default scopes: {}", e))
+            })?;
+            return Ok(Self(scopes));
         }
 
-        let scopes = value
-            .split_whitespace()
-            .map(|s| s.to_string())
-            .collect::<Vec<String>>();
+        // Apply compat_scopes to normalize scope format before parsing
+        let normalized_value = crate::oauth::scope_validation::compat_scopes(&value);
+        
+        // Parse the provided scopes
+        let scopes = Scope::parse_multiple(&normalized_value)
+            .map_err(|e| ConfigError::InvalidScope(format!("Failed to parse scopes: {}", e)))?;
+
+        // Validate scope requirements
+        Self::validate_scope_requirements(&scopes)?;
 
         Ok(Self(scopes))
     }
@@ -394,12 +406,26 @@ impl TryFrom<String> for OAuthSupportedScopes {
     }
 }
 
-impl AsRef<Vec<String>> for OAuthSupportedScopes {
-    fn as_ref(&self) -> &Vec<String> {
+impl AsRef<Vec<Scope>> for OAuthSupportedScopes {
+    fn as_ref(&self) -> &Vec<Scope> {
         &self.0
     }
 }
 
+impl OAuthSupportedScopes {
+    /// Validate that scopes contain required AT Protocol scopes when certain OAuth scopes are present
+    /// This delegates to the centralized validation in scope_validation module
+    pub fn validate_scope_requirements(scopes: &[Scope]) -> Result<(), ConfigError> {
+        crate::oauth::scope_validation::validate_oauth_scope_requirements(scopes)
+            .map_err(|e| ConfigError::InvalidScope(e.to_string()))
+    }
+
+    /// Get scopes as a Vec of strings for serialization
+    pub fn as_strings(&self) -> Vec<String> {
+        self.0.iter().map(|s| s.to_string_normalized()).collect()
+    }
+}
+
 impl TryFrom<String> for ClientDefaultAccessTokenExpiration {
     type Error = anyhow::Error;
 
@@ -491,6 +517,109 @@ impl TryFrom<String> for AtprotoClientName {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_oauth_supported_scopes_validation() {
+        // Test 0: Invalid scopes - missing required atproto scope
+        let missing_atproto =
+            OAuthSupportedScopes::try_from("openid transition:generic".to_string());
+        assert!(
+            missing_atproto.is_err(),
+            "Configuration without atproto scope should fail"
+        );
+        if let Err(e) = missing_atproto {
+            let error_msg = e.to_string();
+            assert!(error_msg.contains("atproto") && error_msg.contains("required"));
+        }
+
+        // Test 1: Valid scopes with openid (no transition:generic required)
+        let valid_openid =
+            OAuthSupportedScopes::try_from("atproto openid".to_string());
+        if let Err(ref e) = valid_openid {
+            eprintln!("Test 1 failed with error: {}", e);
+        }
+        assert!(
+            valid_openid.is_ok(),
+            "openid with atproto should be valid"
+        );
+
+        // Test 2: Valid scopes with email (requires openid and email capability)
+        let valid_email =
+            OAuthSupportedScopes::try_from("atproto openid email transition:email".to_string());
+        assert!(
+            valid_email.is_ok(),
+            "email with openid and transition:email should be valid"
+        );
+
+        // Test 3: Valid scopes - profile with openid
+        let valid_profile =
+            OAuthSupportedScopes::try_from("atproto openid profile".to_string());
+        assert!(
+            valid_profile.is_ok(),
+            "profile with openid should be valid"
+        );
+
+        // Test 4: Invalid scopes - email and profile without openid
+        let invalid_email = OAuthSupportedScopes::try_from("atproto email profile".to_string());
+        assert!(
+            invalid_email.is_err(),
+            "email and profile without openid should fail"
+        );
+        if let Err(e) = invalid_email {
+            let error_msg = e.to_string();
+            // Should fail on profile or email requiring openid
+            assert!(
+                error_msg.contains("openid"),
+                "Expected error about openid requirement, got: {}",
+                error_msg
+            );
+        }
+
+        // Test 5: Invalid scopes - email without openid
+        let invalid_email_no_openid = OAuthSupportedScopes::try_from("atproto email transition:email".to_string());
+        assert!(
+            invalid_email_no_openid.is_err(),
+            "email without openid should be invalid"
+        );
+        if let Err(e) = invalid_email_no_openid {
+            let error_msg = e.to_string();
+            assert!(error_msg.contains("email") && error_msg.contains("openid"));
+        }
+
+        // Test 6: Valid email with account:email?action=read
+        let valid_email_alt =
+            OAuthSupportedScopes::try_from("atproto openid email account:email?action=read".to_string());
+        assert!(
+            valid_email_alt.is_ok(),
+            "email with openid and account:email?action=read should be valid"
+        );
+
+        // Test 6: Valid scopes with both openid and email with all requirements
+        let valid_both = OAuthSupportedScopes::try_from(
+            "atproto openid email transition:generic transition:email".to_string(),
+        );
+        assert!(
+            valid_both.is_ok(),
+            "openid and email with all requirements should be valid"
+        );
+
+        // Test 7: Invalid scopes - email with transition:generic (doesn't grant email)
+        let invalid_email_with_generic =
+            OAuthSupportedScopes::try_from("atproto openid email transition:generic".to_string());
+        assert!(
+            invalid_email_with_generic.is_err(),
+            "email with only transition:generic should be invalid (doesn't grant email access)"
+        );
+        if let Err(e) = invalid_email_with_generic {
+            let error_msg = e.to_string();
+            assert!(error_msg.contains("email") && error_msg.contains("requires"));
+        }
+    }
+}
+
 impl AsRef<String> for AtprotoClientName {
     fn as_ref(&self) -> &String {
         &self.0

src/config.rs

@@ -36,6 +36,10 @@ pub enum ConfigError {
         "error-aip-config-7 Failed to parse boolean '{0}': expected true/false/1/0/yes/no/on/off"
     )]
     BoolParsingFailed(String),
+
+    /// Error when OAuth scopes don't meet requirements
+    #[error("error-aip-config-8 Invalid scope configuration: {0}")]
+    InvalidScope(String),
 }
 
 /// HTTP server errors

src/errors.rs

@@ -1,9 +1,11 @@
-//! Handles GET /oauth/atp/client-metadata - Provides ATProtocol OAuth client metadata per RFC 7591
+//! Handles GET /oauth-client-metadata.json - Provides ATProtocol OAuth client metadata per RFC 7591
 
+use atproto_oauth::scopes::Scope;
 use atproto_oauth_axum::{handler_metadata::handle_oauth_metadata, state::OAuthClientConfig};
 use axum::{extract::State, response::IntoResponse};
 
 use super::context::AppState;
+use crate::config::ATPROTO_CLIENT_METADATA_PATH;
 
 /// Handles requests for ATProtocol OAuth client metadata.
 ///
@@ -13,10 +15,31 @@ pub async fn handle_atpoauth_client_metadata(
     State(app_state): State<AppState>,
 ) -> impl IntoResponse {
     // Convert AppState configuration to OAuthClientConfig
+    // Filter scopes to only include ATProtocol-compatible scopes for client metadata
+    // Parse the configured scopes
+    let all_scopes = app_state
+        .config
+        .oauth_supported_scopes
+        .as_ref()
+        .clone();
+    
+    // Filter the scopes using the same function as in atprotocol_bridge
+    // This removes non-ATProtocol scopes and validates requirements
+    let filtered_scopes = match crate::oauth::scope_validation::filter_atprotocol_scopes(&all_scopes) {
+        Ok(scopes) => scopes,
+        Err(_) => {
+            // If filtering fails (e.g., missing required scopes), default to just atproto
+            vec![Scope::Atproto]
+        }
+    };
+    
+    // Serialize the filtered scopes
+    let scopes = Scope::serialize_multiple(&filtered_scopes);
+
     let oauth_client_config = OAuthClientConfig {
         client_id: format!(
-            "{}/oauth/atp/client-metadata",
-            app_state.config.external_base
+            "{}{}",
+            app_state.config.external_base, ATPROTO_CLIENT_METADATA_PATH
         ),
         redirect_uris: format!("{}/oauth/atp/callback", app_state.config.external_base),
         jwks_uri: None, // Use inline JWKS instead of external URI
@@ -26,7 +49,7 @@ pub async fn handle_atpoauth_client_metadata(
         logo_uri: app_state.config.atproto_client_logo.as_ref().clone(),
         tos_uri: app_state.config.atproto_client_tos.as_ref().clone(),
         policy_uri: app_state.config.atproto_client_policy.as_ref().clone(),
-        scope: Some("atproto transition:generic transition:email".to_string()),
+        scope: Some(scopes),
     };
 
     // Use the atproto-oauth-axum handler

src/http/handler_atprotocol_client_metadata.rs

@@ -174,11 +174,14 @@ async fn process_authorization_query(
         None
     };
 
+    // Apply compat_scopes to normalize scope format if present
+    let normalized_scope = query.scope.as_ref().map(|s| crate::oauth::scope_validation::compat_scopes(s));
+    
     let request = AuthorizationRequest {
         response_type: vec![crate::oauth::types::ResponseType::Code],
         client_id: query.client_id.clone(),
         redirect_uri,
-        scope: query.scope.clone(),
+        scope: normalized_scope,
         state: query.state.clone(),
         code_challenge: query.code_challenge.clone(),
         code_challenge_method: query.code_challenge_method.clone(),
@@ -190,7 +193,7 @@ async fn process_authorization_query(
     if let Some(ref requested_scope) = request.scope {
         let requested_scopes = crate::oauth::types::parse_scope(requested_scope);
         let supported_scopes =
-            crate::oauth::types::parse_scope(&config.oauth_supported_scopes.as_ref().join(" "));
+            crate::oauth::types::parse_scope(&config.oauth_supported_scopes.as_strings().join(" "));
 
         if !requested_scopes.is_subset(&supported_scopes) {
             return Err(serde_json::json!({
@@ -284,7 +287,7 @@ mod tests {
             atproto_oauth_signing_keys: Default::default(),
             oauth_signing_keys: Default::default(),
             oauth_supported_scopes: crate::config::OAuthSupportedScopes::try_from(
-                "read write atproto:atproto".to_string(),
+                "atproto transition:generic transition:email".to_string(),
             )
             .unwrap(),
             dpop_nonce_seed: "seed".to_string(),
@@ -312,7 +315,7 @@ mod tests {
             client_id: "test-client".to_string(),
             redirect_uri: Some("https://example.com/callback".to_string()),
             response_type: Some("code".to_string()),
-            scope: Some("read write".to_string()),
+            scope: Some("atproto transition:generic".to_string()),
             state: Some("test-state".to_string()),
             code_challenge: None,
             code_challenge_method: None,
@@ -346,7 +349,7 @@ mod tests {
             client_id: "test-client".to_string(),
             redirect_uri: Some("https://example.com/callback".to_string()),
             response_type: Some("code".to_string()),
-            scope: Some("read".to_string()),
+            scope: Some("atproto".to_string()),
             state: Some("test-state".to_string()),
             code_challenge: Some("test-challenge".to_string()),
             code_challenge_method: Some("S256".to_string()),

src/http/handler_atprotocol_oauth_authorize.rs

@@ -73,7 +73,7 @@ mod tests {
             atproto_oauth_signing_keys: Default::default(),
             oauth_signing_keys: Default::default(),
             oauth_supported_scopes: crate::config::OAuthSupportedScopes::try_from(
-                "read write atproto:atproto".to_string(),
+                "atproto transition:generic transition:email".to_string(),
             )
             .unwrap(),
             dpop_nonce_seed: "seed".to_string(),

src/http/handler_index.rs

@@ -166,7 +166,7 @@ mod tests {
             atproto_oauth_signing_keys: Default::default(),
             oauth_signing_keys: Default::default(),
             oauth_supported_scopes: crate::config::OAuthSupportedScopes::try_from(
-                "read write atproto:atproto".to_string(),
+                "atproto transition:generic transition:email".to_string(),
             )
             .unwrap(),
             dpop_nonce_seed: "seed".to_string(),