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:
parent
714dae9a2c
commit
12635a85b2
|
|
@ -72,12 +72,22 @@ impl StoredCredentials {
|
||||||
|
|
||||||
fn from_response(t: TokenResponse, issuer: String) -> Self {
|
fn from_response(t: TokenResponse, issuer: String) -> Self {
|
||||||
let expires_at = t.expires_in.map(|s| now_unix() + s);
|
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 {
|
Self {
|
||||||
schema_version: Self::SCHEMA_VERSION,
|
schema_version: Self::SCHEMA_VERSION,
|
||||||
access_token: t.access_token,
|
access_token: t.access_token,
|
||||||
refresh_token: t.refresh_token,
|
refresh_token: t.refresh_token,
|
||||||
expires_at,
|
expires_at,
|
||||||
scope: t.scope,
|
scope,
|
||||||
account_email: t.account_email,
|
account_email: t.account_email,
|
||||||
issuer,
|
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 {
|
fn now_unix() -> i64 {
|
||||||
std::time::SystemTime::now()
|
std::time::SystemTime::now()
|
||||||
.duration_since(std::time::UNIX_EPOCH)
|
.duration_since(std::time::UNIX_EPOCH)
|
||||||
|
|
@ -381,3 +423,87 @@ mod tests {
|
||||||
let _ = std::fs::remove_dir_all(&dir);
|
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
|
//! 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.
|
//! 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
|
//! Verified against real production tokens: the claim set is exactly
|
||||||
//! intended — one account, every Cognitum product — so this is by design, not a
|
//! `typ, sub, account_id, org_id, workspace_id, client_id, scope, family_id,
|
||||||
//! defect. It does mean **scope is the only capability boundary**: `client_id`
|
//! jti, iat, exp, setup, workload`. No audience. No issuer.
|
||||||
//! cannot serve as one, because clients borrow each other's registrations
|
//!
|
||||||
//! (musica ships `DEFAULT_CLIENT_ID = "meta-proxy"`). Hence `required_scope`
|
//! **What binds a token to its issuer, then?** The JWKS. We accept only
|
||||||
//! below is not optional garnish; it is the boundary.
|
//! 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 jsonwebtoken::{decode, decode_header, Algorithm, Validation};
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
|
|
@ -70,8 +80,6 @@ pub enum VerifyError {
|
||||||
/// attack, and an operator needs to be able to tell those apart.
|
/// attack, and an operator needs to be able to tell those apart.
|
||||||
#[error("token is expired or not yet valid (check host clock sync)")]
|
#[error("token is expired or not yet valid (check host clock sync)")]
|
||||||
ExpiredOrNotYetValid,
|
ExpiredOrNotYetValid,
|
||||||
#[error("token issuer is not {expected}")]
|
|
||||||
WrongIssuer { expected: String },
|
|
||||||
#[error("token type {found:?} is not an interactive access token")]
|
#[error("token type {found:?} is not an interactive access token")]
|
||||||
WrongTokenType { found: Option<String> },
|
WrongTokenType { found: Option<String> },
|
||||||
#[error("long-lived setup/workload credentials are not accepted (unverifiable revocation)")]
|
#[error("long-lived setup/workload credentials are not accepted (unverifiable revocation)")]
|
||||||
|
|
@ -115,9 +123,12 @@ struct AccessTokenClaims {
|
||||||
|
|
||||||
/// Verifier configuration.
|
/// Verifier configuration.
|
||||||
pub struct VerifierConfig {
|
pub struct VerifierConfig {
|
||||||
/// Expected `iss`, compared **verbatim**. RFC 8414 §2 requires the issuer to
|
/// The authorization server's origin, e.g. `https://auth.cognitum.one`.
|
||||||
/// match the origin exactly — a trailing slash or an http/https mismatch is
|
///
|
||||||
/// a failure, not a near-miss to be normalised away.
|
/// **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,
|
pub issuer: String,
|
||||||
/// The scope a caller must hold for the route being served.
|
/// The scope a caller must hold for the route being served.
|
||||||
pub required_scope: String,
|
pub required_scope: String,
|
||||||
|
|
@ -143,12 +154,14 @@ pub fn verify_access_token(
|
||||||
let key = jwks.decoding_key_for(&kid)?;
|
let key = jwks.decoding_key_for(&kid)?;
|
||||||
|
|
||||||
let mut validation = Validation::new(Algorithm::ES256);
|
let mut validation = Validation::new(Algorithm::ES256);
|
||||||
validation.set_issuer(&[config.issuer.as_str()]);
|
|
||||||
validation.leeway = CLOCK_LEEWAY_SECS;
|
validation.leeway = CLOCK_LEEWAY_SECS;
|
||||||
validation.validate_exp = true;
|
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.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 data = decode::<AccessTokenClaims>(token, &key, &validation).map_err(map_jwt_error)?;
|
||||||
let claims = data.claims;
|
let claims = data.claims;
|
||||||
|
|
@ -222,9 +235,6 @@ fn map_jwt_error(e: jsonwebtoken::errors::Error) -> VerifyError {
|
||||||
ErrorKind::ExpiredSignature | ErrorKind::ImmatureSignature => {
|
ErrorKind::ExpiredSignature | ErrorKind::ImmatureSignature => {
|
||||||
VerifyError::ExpiredOrNotYetValid
|
VerifyError::ExpiredOrNotYetValid
|
||||||
}
|
}
|
||||||
ErrorKind::InvalidIssuer => VerifyError::WrongIssuer {
|
|
||||||
expected: String::new(),
|
|
||||||
},
|
|
||||||
ErrorKind::InvalidAlgorithm | ErrorKind::InvalidAlgorithmName => {
|
ErrorKind::InvalidAlgorithm | ErrorKind::InvalidAlgorithmName => {
|
||||||
VerifyError::WrongAlgorithm
|
VerifyError::WrongAlgorithm
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -111,7 +111,10 @@ fn valid_claims() -> serde_json::Value {
|
||||||
"exp": now() + 900, // identity's real 15-minute TTL
|
"exp": now() + 900, // identity's real 15-minute TTL
|
||||||
"setup": false,
|
"setup": false,
|
||||||
"workload": 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 ────────────────────────
|
// ────────────────────── issuer / type ────────────────────────
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_token_from_another_issuer_is_rejected() {
|
fn a_token_with_no_iss_claim_is_accepted_because_cognitum_issues_none() {
|
||||||
let mut c = valid_claims();
|
// THE regression test for this module's worst bug to date.
|
||||||
c["iss"] = json!("https://evil.example");
|
//
|
||||||
assert!(matches!(
|
// An earlier revision required and validated `iss`. Cognitum access tokens
|
||||||
verify(&sign(&c), scope::SENSING_READ),
|
// have no `iss` claim, so that rejected every real token — while the suite
|
||||||
Err(VerifyError::WrongIssuer { .. })
|
// 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]
|
#[test]
|
||||||
fn an_issuer_differing_only_by_a_trailing_slash_is_rejected() {
|
fn an_unrelated_iss_claim_does_not_change_the_outcome() {
|
||||||
// RFC 8414 §2: the issuer is compared verbatim. Normalising this away is a
|
// If identity ever starts emitting `iss`, we neither require nor reject on
|
||||||
// small kindness that quietly widens who we trust.
|
// it — the JWKS is the issuer binding. This pins that adding the claim
|
||||||
|
// cannot silently start failing tokens.
|
||||||
let mut c = valid_claims();
|
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!(
|
assert!(matches!(
|
||||||
verify(&sign(&c), scope::SENSING_READ),
|
verify(&token, scope::SENSING_READ),
|
||||||
Err(VerifyError::WrongIssuer { .. })
|
Err(VerifyError::BadSignature)
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue