diff --git a/v2/crates/ruview-auth/src/login/store.rs b/v2/crates/ruview-auth/src/login/store.rs index 71bf4a53..c7990365 100644 --- a/v2/crates/ruview-auth/src/login/store.rs +++ b/v2/crates/ruview-auth/src/login/store.rs @@ -72,12 +72,22 @@ impl StoredCredentials { fn from_response(t: TokenResponse, issuer: String) -> Self { let expires_at = t.expires_in.map(|s| now_unix() + s); + // Identity's /oauth/token response has no top-level `scope` field, but + // the access token itself carries a `scope` claim — and that claim is + // the authoritative one, since it is what a resource server actually + // gates on. Falling back to it turns "(not reported by the server)" + // into the real answer. Envelope first on the off chance a future + // response does carry one. + let scope = t + .scope + .clone() + .or_else(|| scope_from_access_token(&t.access_token)); Self { schema_version: Self::SCHEMA_VERSION, access_token: t.access_token, refresh_token: t.refresh_token, expires_at, - scope: t.scope, + scope, account_email: t.account_email, issuer, } @@ -95,6 +105,38 @@ impl StoredCredentials { } } +/// Read the `scope` claim out of an access token **for display only**. +/// +/// # This is NOT verification +/// +/// It base64-decodes the JWT payload and does not check the signature, the +/// issuer, `exp`, `typ`, or anything else. Its only legitimate use is telling a +/// user what they just consented to, for a token this process received over TLS +/// directly from the issuer moments ago. +/// +/// Never use it to make an authorization decision. Anything that gates access +/// must go through [`crate::verify::verify_access_token`], which checks the +/// signature against identity's published JWKS. A client reading its own freshly +/// issued token is a fundamentally different situation from a server reading a +/// token a stranger handed it. +/// +/// Returns `None` rather than guessing if the token is not a well-formed JWT — +/// an unreadable scope must present as unknown, never as empty (which would +/// read as "you were granted nothing"). +fn scope_from_access_token(jwt: &str) -> Option { + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use base64::Engine; + + let payload_b64 = jwt.split('.').nth(1)?; + let bytes = URL_SAFE_NO_PAD.decode(payload_b64).ok()?; + let claims: serde_json::Value = serde_json::from_slice(&bytes).ok()?; + claims + .get("scope")? + .as_str() + .filter(|s| !s.is_empty()) + .map(str::to_owned) +} + fn now_unix() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -381,3 +423,87 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } } + +/// The scope-from-token fallback. Split out so its "display only, never an +/// authorization input" contract is pinned by name. +#[cfg(test)] +mod scope_display_tests { + use super::*; + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use base64::Engine; + + fn jwt_with_payload(payload: serde_json::Value) -> String { + // Header and signature are irrelevant here — that is the whole point: + // this path never inspects them, so the test must not imply it does. + format!( + "eyJhbGciOiJFUzI1NiJ9.{}.not-a-real-signature", + URL_SAFE_NO_PAD.encode(serde_json::to_vec(&payload).unwrap()) + ) + } + + #[test] + fn reads_the_scope_claim_when_the_envelope_omits_it() { + // The live behaviour that motivated this: identity's /oauth/token + // response carries no top-level `scope`, but the token does. + let t = jwt_with_payload(serde_json::json!({"scope": "sensing:read"})); + assert_eq!(scope_from_access_token(&t).as_deref(), Some("sensing:read")); + } + + #[test] + fn reads_a_multi_scope_claim_intact() { + let t = jwt_with_payload(serde_json::json!({"scope": "sensing:read sensing:admin"})); + assert_eq!( + scope_from_access_token(&t).as_deref(), + Some("sensing:read sensing:admin") + ); + } + + #[test] + fn an_unparseable_token_reads_as_unknown_not_as_empty() { + // "" would render as "you were granted nothing", which is a different + // and wrong claim. + assert_eq!(scope_from_access_token("not-a-jwt"), None); + assert_eq!(scope_from_access_token(""), None); + assert_eq!(scope_from_access_token("a.!!!not-base64!!!.c"), None); + } + + #[test] + fn an_empty_scope_claim_reads_as_unknown() { + let t = jwt_with_payload(serde_json::json!({"scope": ""})); + assert_eq!(scope_from_access_token(&t), None); + } + + #[test] + fn a_token_with_no_scope_claim_reads_as_unknown() { + let t = jwt_with_payload(serde_json::json!({"sub": "u1"})); + assert_eq!(scope_from_access_token(&t), None); + } + + #[test] + fn the_response_envelope_wins_when_it_does_carry_a_scope() { + let token = TokenResponse { + access_token: jwt_with_payload(serde_json::json!({"scope": "from:token"})), + token_type: None, + account_email: None, + refresh_token: None, + expires_in: Some(900), + scope: Some("from:envelope".into()), + }; + let c = StoredCredentials::from_response(token, "https://auth.test".into()); + assert_eq!(c.scope.as_deref(), Some("from:envelope")); + } + + #[test] + fn the_token_claim_is_used_when_the_envelope_is_silent() { + let token = TokenResponse { + access_token: jwt_with_payload(serde_json::json!({"scope": "sensing:read"})), + token_type: None, + account_email: None, + refresh_token: None, + expires_in: Some(900), + scope: None, + }; + let c = StoredCredentials::from_response(token, "https://auth.test".into()); + assert_eq!(c.scope.as_deref(), Some("sensing:read")); + } +} diff --git a/v2/crates/ruview-auth/src/verify.rs b/v2/crates/ruview-auth/src/verify.rs index a8b9071a..d81e83a0 100644 --- a/v2/crates/ruview-auth/src/verify.rs +++ b/v2/crates/ruview-auth/src/verify.rs @@ -24,14 +24,24 @@ //! any realistic revocation propagates; a 365-day one does. Accepting one would //! mean honouring a credential that may already have been revoked, so we don't. //! -//! ## There is no `aud` claim +//! ## There is no `aud` claim, and no `iss` claim either //! -//! Cognitum access tokens carry no audience. Cross-product *identity* is -//! intended — one account, every Cognitum product — so this is by design, not a -//! defect. It does mean **scope is the only capability boundary**: `client_id` -//! cannot serve as one, because clients borrow each other's registrations -//! (musica ships `DEFAULT_CLIENT_ID = "meta-proxy"`). Hence `required_scope` -//! below is not optional garnish; it is the boundary. +//! Verified against real production tokens: the claim set is exactly +//! `typ, sub, account_id, org_id, workspace_id, client_id, scope, family_id, +//! jti, iat, exp, setup, workload`. No audience. No issuer. +//! +//! **What binds a token to its issuer, then?** The JWKS. We accept only +//! signatures made by a key served from the configured `jwks_uri`, so +//! possession of a valid signature *is* proof of issuer. Adding an `iss` claim +//! check on top would not strengthen that — and requiring a claim identity does +//! not emit rejects every genuine token, which is exactly what an earlier +//! revision of this module did. +//! +//! The missing `aud` has a real consequence: **scope is the only capability +//! boundary**. `client_id` cannot serve as one, because clients borrow each +//! other's registrations (musica shipped as `meta-proxy` while its own +//! registration was pending). Hence `required_scope` below is not optional +//! garnish; it is the boundary. use jsonwebtoken::{decode, decode_header, Algorithm, Validation}; use serde::Deserialize; @@ -70,8 +80,6 @@ pub enum VerifyError { /// attack, and an operator needs to be able to tell those apart. #[error("token is expired or not yet valid (check host clock sync)")] ExpiredOrNotYetValid, - #[error("token issuer is not {expected}")] - WrongIssuer { expected: String }, #[error("token type {found:?} is not an interactive access token")] WrongTokenType { found: Option }, #[error("long-lived setup/workload credentials are not accepted (unverifiable revocation)")] @@ -115,9 +123,12 @@ struct AccessTokenClaims { /// Verifier configuration. pub struct VerifierConfig { - /// Expected `iss`, compared **verbatim**. RFC 8414 §2 requires the issuer to - /// match the origin exactly — a trailing slash or an http/https mismatch is - /// a failure, not a near-miss to be normalised away. + /// The authorization server's origin, e.g. `https://auth.cognitum.one`. + /// + /// **Not validated against a claim** — Cognitum access tokens carry no + /// `iss` (see module docs); the JWKS provides the issuer binding. This is + /// here for logging and for deriving the default `jwks_uri`, so that the + /// configured issuer and the keys we trust cannot drift apart silently. pub issuer: String, /// The scope a caller must hold for the route being served. pub required_scope: String, @@ -143,12 +154,14 @@ pub fn verify_access_token( let key = jwks.decoding_key_for(&kid)?; let mut validation = Validation::new(Algorithm::ES256); - validation.set_issuer(&[config.issuer.as_str()]); validation.leeway = CLOCK_LEEWAY_SECS; validation.validate_exp = true; - // No audience validation: these tokens carry no `aud` (see module docs). + // No `aud` and no `iss` validation — Cognitum access tokens carry neither. + // See "What binds a token to its issuer" in the module docs: the JWKS is + // the binding, and requiring a claim identity does not emit rejects every + // real token. meta-llm's verifier makes the same two omissions. validation.validate_aud = false; - validation.set_required_spec_claims(&["exp", "iss"]); + validation.set_required_spec_claims(&["exp"]); let data = decode::(token, &key, &validation).map_err(map_jwt_error)?; let claims = data.claims; @@ -222,9 +235,6 @@ fn map_jwt_error(e: jsonwebtoken::errors::Error) -> VerifyError { ErrorKind::ExpiredSignature | ErrorKind::ImmatureSignature => { VerifyError::ExpiredOrNotYetValid } - ErrorKind::InvalidIssuer => VerifyError::WrongIssuer { - expected: String::new(), - }, ErrorKind::InvalidAlgorithm | ErrorKind::InvalidAlgorithmName => { VerifyError::WrongAlgorithm } diff --git a/v2/crates/ruview-auth/tests/verifier_matrix.rs b/v2/crates/ruview-auth/tests/verifier_matrix.rs index 159c8313..7db59f74 100644 --- a/v2/crates/ruview-auth/tests/verifier_matrix.rs +++ b/v2/crates/ruview-auth/tests/verifier_matrix.rs @@ -111,7 +111,10 @@ fn valid_claims() -> serde_json::Value { "exp": now() + 900, // identity's real 15-minute TTL "setup": false, "workload": false, - "iss": TEST_ISSUER, + // NOTE: no `iss`. Real Cognitum access tokens carry none — verified + // against production. An earlier fixture added one, the verifier was + // built to require it, and the whole suite passed while rejecting every + // genuine token. Fixtures mirror production or they prove nothing. }) } @@ -262,24 +265,43 @@ fn a_token_expired_beyond_the_leeway_is_rejected() { // ────────────────────── issuer / type ──────────────────────── #[test] -fn a_token_from_another_issuer_is_rejected() { - let mut c = valid_claims(); - c["iss"] = json!("https://evil.example"); - assert!(matches!( - verify(&sign(&c), scope::SENSING_READ), - Err(VerifyError::WrongIssuer { .. }) - )); +fn a_token_with_no_iss_claim_is_accepted_because_cognitum_issues_none() { + // THE regression test for this module's worst bug to date. + // + // An earlier revision required and validated `iss`. Cognitum access tokens + // have no `iss` claim, so that rejected every real token — while the suite + // stayed green, because the fixtures had an `iss` the real thing lacks. + // `valid_claims()` now mirrors production, so this passing means the + // verifier accepts the shape that actually exists. + let claims = valid_claims(); + assert!( + claims.get("iss").is_none(), + "fixture must mirror production, which emits no iss" + ); + verify(&sign(&claims), scope::SENSING_READ).expect("a real-shaped token must verify"); } #[test] -fn an_issuer_differing_only_by_a_trailing_slash_is_rejected() { - // RFC 8414 §2: the issuer is compared verbatim. Normalising this away is a - // small kindness that quietly widens who we trust. +fn an_unrelated_iss_claim_does_not_change_the_outcome() { + // If identity ever starts emitting `iss`, we neither require nor reject on + // it — the JWKS is the issuer binding. This pins that adding the claim + // cannot silently start failing tokens. let mut c = valid_claims(); - c["iss"] = json!(format!("{TEST_ISSUER}/")); + c["iss"] = json!("https://something.else.example"); + verify(&sign(&c), scope::SENSING_READ) + .expect("issuer binding is the JWKS, not a claim"); +} + +#[test] +fn the_jwks_is_what_actually_binds_a_token_to_its_issuer() { + // The complement of the two above: with no `iss` check, the signature is + // the ONLY thing standing between us and a forged token. A different key + // must therefore be refused — otherwise removing the issuer check would + // have removed the boundary entirely. + let token = sign_with(&valid_claims(), other_key()); assert!(matches!( - verify(&sign(&c), scope::SENSING_READ), - Err(VerifyError::WrongIssuer { .. }) + verify(&token, scope::SENSING_READ), + Err(VerifyError::BadSignature) )); }