mirror of
https://github.com/ruvnet/RuView
synced 2026-07-22 17:23:19 +00:00
fix(auth): stop requiring an iss claim Cognitum never issues (G-3 blocker)
The verifier called `Validation::set_issuer` and listed `iss` in `set_required_spec_claims`. Cognitum access tokens have **no `iss` claim** — the real claim set is typ, sub, account_id, org_id, workspace_id, client_id, scope, family_id, jti, iat, exp, setup, workload. So the verifier rejected every genuine token with a flat 401. The whole 41-test suite was green throughout, because the test fixtures included an `iss` the real thing does not have. The tests validated my assumption instead of the platform. Found only by pointing a real token at a real server; no amount of additional unit testing against the same wrong fixture would have caught it. `valid_claims()` now mirrors production exactly, with a comment saying why, so the next person cannot reintroduce the drift by "fixing" an incomplete-looking fixture. 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 a valid signature IS proof of issuer. meta-llm's verifier — the org's only other resource-side verifier — checks neither `iss` nor `aud`, for the same reason. `VerifierConfig.issuer` stays, now documented as JWKS-derivation and logging rather than a claim assertion. Three tests replace the two that asserted issuer behaviour that cannot exist: a real-shaped (iss-less) token verifies; an unrelated `iss` changes nothing (so identity adding one later cannot silently start failing tokens); and a token signed by a different key is still refused — the complement that proves removing the issuer check did not remove the boundary. Also: report the granted scope from the token's `scope` claim when the /oauth/token envelope omits it, which identity's does. `login` and `whoami` said "(not reported by the server)" while the authoritative answer sat inside the token they had just stored. The claim-peek helper is display-only and documented at length as NOT verification — a client reading its own freshly issued token is a different situation from a server reading a stranger's. VERIFIED END TO END against production (gate G-3, previously open): no credential GET /api/v1/models -> 401 garbage bearer GET /api/v1/models -> 401 real sensing:read token GET /api/v1/models -> 200 real sensing:read token GET /api/v1/recording/list-> 200 real sensing:read token POST /api/v1/train/start -> 401 real sensing:read token POST /api/v1/train/stop -> 401 Token obtained by a human through `wifi-densepose login` against auth.cognitum.one; server ran with RUVIEW_OAUTH_ISSUER set, JWKS fetched live at boot (key_count=1). Tests: 82 with --features login (58 unit + 22 matrix + 2 doctests). Co-Authored-By: Ruflo & AQE
This commit is contained in:
Dragan Spiridonov
@@ -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<String> {
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String> },
|
||||
#[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::<AccessTokenClaims>(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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user