refactor: tidy auth token validation and scope handling (#764)

* refactor: lift JWT claims and algorithm mapping out of validate

* refactor: return (key, issuer) tuple instead of VerificationKey struct

* test: cover jwt_algorithm mapping and edge cases

* refactor: extract ScopeMode::is_satisfied_by for scope checks

* refactor: render scope_mode via ScopeMode::as_str

Author: DaleSeo

crates/apollo-mcp-server/src/auth.rs

@@ -47,6 +47,30 @@ pub enum ScopeMode {
     RequireAny,
 }
 
+impl ScopeMode {
+    /// Whether the `present` scopes satisfy the `required` scopes under this
+    /// mode. Callers skip this check when no scopes are configured, so
+    /// `required` is expected to be non-empty.
+    fn is_satisfied_by(self, required: &[String], present: &[String]) -> bool {
+        match self {
+            ScopeMode::Disabled => true,
+            ScopeMode::RequireAll => required.iter().all(|req| present.contains(req)),
+            ScopeMode::RequireAny => required.iter().any(|req| present.contains(req)),
+        }
+    }
+
+    /// The wire string for this mode, matching the serde `snake_case` rename.
+    /// Used when rendering the `scope_mode` hint in the `WWW-Authenticate`
+    /// header.
+    fn as_str(self) -> &'static str {
+        match self {
+            ScopeMode::Disabled => "disabled",
+            ScopeMode::RequireAll => "require_all",
+            ScopeMode::RequireAny => "require_any",
+        }
+    }
+}
+
 /// Errors that can occur when building a TLS-configured HTTP client
 #[derive(Debug, thiserror::Error)]
 pub enum TlsConfigError {
@@ -521,17 +545,9 @@ async fn oauth_validate(
 
     // Scope validation: only applies when scopes are configured
     if !auth_config.scopes.is_empty() {
-        let sufficient = match auth_config.scope_mode {
-            ScopeMode::Disabled => true,
-            ScopeMode::RequireAll => auth_config
-                .scopes
-                .iter()
-                .all(|req| valid_token.scopes.contains(req)),
-            ScopeMode::RequireAny => auth_config
-                .scopes
-                .iter()
-                .any(|req| valid_token.scopes.contains(req)),
-        };
+        let sufficient = auth_config
+            .scope_mode
+            .is_satisfied_by(&auth_config.scopes, &valid_token.scopes);
 
         if !sufficient {
             // Compute missing scopes for diagnostic logging
@@ -696,19 +712,32 @@ mod tests {
         }
     }
 
+    mod scope_mode {
+        use super::*;
+
+        #[test]
+        fn as_str_matches_serde_rename() {
+            for mode in [
+                ScopeMode::Disabled,
+                ScopeMode::RequireAll,
+                ScopeMode::RequireAny,
+            ] {
+                let serde = serde_json::to_string(&mode).expect("serialize scope mode");
+                assert_eq!(
+                    format!("\"{}\"", mode.as_str()),
+                    serde,
+                    "as_str drifted from the serde rename for {mode:?}"
+                );
+            }
+        }
+    }
+
     mod scope_validation {
         use super::*;
         use rstest::rstest;
 
         fn is_sufficient(mode: ScopeMode, required: &[String], present: &[String]) -> bool {
-            if required.is_empty() {
-                return true;
-            }
-            match mode {
-                ScopeMode::Disabled => true,
-                ScopeMode::RequireAll => required.iter().all(|req| present.contains(req)),
-                ScopeMode::RequireAny => required.iter().any(|req| present.contains(req)),
-            }
+            required.is_empty() || mode.is_satisfied_by(required, present)
         }
 
         fn s(vals: &[&str]) -> Vec<String> {

crates/apollo-mcp-server/src/auth/networked_key_resolver.rs

@@ -2,12 +2,12 @@ use std::str::FromStr;
 use std::time::Duration;
 
 use jsonwebtoken::jwk::KeyAlgorithm;
-use jwks::Jwks;
+use jwks::{Jwk, Jwks};
 use serde::Deserialize;
 use tracing::{error, info, trace, warn};
 use url::Url;
 
-use super::valid_token::{KeyResolver, VerificationKey};
+use super::valid_token::KeyResolver;
 
 /// [`KeyResolver`] that fetches signing keys from the network via OIDC/OAuth
 /// discovery.
@@ -187,17 +187,14 @@ impl KeyResolver for NetworkedKeyResolver<'_> {
     /// `discovery_timeout` on the happy path. The JWKS fetch does not fall
     /// back to alternate discovery URLs on failure; real providers advertise
     /// the same `jwks_uri` from every well-known path.
-    async fn resolve_key(&self, server: &Url, key_id: &str) -> Option<VerificationKey> {
+    async fn resolve_key(&self, server: &Url, key_id: &str) -> Option<(Jwk, String)> {
         let metadata = discover_metadata(self.client, server, self.discovery_timeout).await?;
         let mut jwks = fetch_jwks(self.client, &metadata.jwks_uri, self.discovery_timeout).await?;
         let mut jwk = jwks.keys.remove(key_id)?;
         if jwk.alg.is_none() {
             jwk.alg = resolve_alg(&metadata.id_token_signing_alg_values_supported, server);
         }
-        Some(VerificationKey {
-            jwk,
-            issuer: metadata.issuer,
-        })
+        Some((jwk, metadata.issuer))
     }
 }