From 0547fd7344d86cf0d0f56489be241ddfad648eeb Mon Sep 17 00:00:00 2001 From: Dragan Spiridonov Date: Thu, 23 Jul 2026 09:31:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(auth):=20enforce=20client=5Fid=20as=20the?= =?UTF-8?q?=20audience=20=E2=80=94=20Cognitum's=20stand-in=20for=20`aud`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by reading cognitum-one/freetokens, a live sibling service whose browser OAuth landed while this PR was open. Its integration contract states the platform rule outright: "Cognitum access tokens intentionally use custom `client_id` rather than a registered JWT `aud` claim." -- freetokens docs/AUTH_INTEGRATION.md and `src/auth/oauth.ts` enforces it on every sign-in: payload.client_id !== config.OAUTH_CLIENT_ID -> reject RuView did not. An earlier revision here removed the `client_id` check and kept it only for logging, reasoning that clients borrow one another's registrations (musica shipped as `meta-proxy` while its own was pending) and that scope alone must therefore carry the boundary. That reasoned from a TRANSITIONAL state: RuView has its own registered client (identity migration 0017), and the platform does have an audience mechanism — it is simply spelled `client_id`. Consequence of the old behaviour: a Cognitum access token minted for ANY product — meta-proxy, musica, metaharness, freetokens — was accepted by a RuView server provided it carried a sensing scope. Scope was the only thing standing between another product's token and this one. Now there are two boundaries, audience and capability, which is what the platform intends. - `VerifierConfig.allowed_client_ids`; empty = accept any (explicit opt-out). - `RUVIEW_OAUTH_CLIENT_IDS` env, default `ruview`, `*` to disable with a loud warning naming what is being given up. Comma-separated for the migration case where a borrowed registration must be accepted alongside our own. - New `VerifyError::WrongAudience`, checked BEFORE scope, so the failure names the real reason rather than blaming the scope. The existing cross-product test now asserts `WrongAudience` rather than `MissingScope` — the token is refused for the stronger reason. Three new tests: a correctly-scoped token from another product is still refused; the empty-list opt-out accepts anything (pinned so it stays deliberate); multiple allowed clients work. This also corrects the module docs and ADR-271, which claimed "scope is the ONLY capability boundary" — true of the code as written, but not of the platform. Tests: 85 ruview-auth, 533 sensing-server. Co-Authored-By: Ruflo & AQE --- v2/crates/ruview-auth/src/lib.rs | 2 + v2/crates/ruview-auth/src/verify.rs | 49 +++++++++++++++++-- .../ruview-auth/tests/verifier_matrix.rs | 47 +++++++++++++++++- .../src/bearer_auth.rs | 39 ++++++++++++++- 4 files changed, 130 insertions(+), 7 deletions(-) diff --git a/v2/crates/ruview-auth/src/lib.rs b/v2/crates/ruview-auth/src/lib.rs index d31a09f3..be356502 100644 --- a/v2/crates/ruview-auth/src/lib.rs +++ b/v2/crates/ruview-auth/src/lib.rs @@ -38,6 +38,8 @@ //! let config = VerifierConfig { //! issuer: "https://auth.cognitum.one".to_string(), //! required_scope: scope::SENSING_READ.to_string(), +//! // Audience: Cognitum has no `aud`, so `client_id` carries it. +//! allowed_client_ids: vec!["ruview".to_string()], //! }; //! //! let principal = verify_access_token("", &jwks, &config)?; diff --git a/v2/crates/ruview-auth/src/verify.rs b/v2/crates/ruview-auth/src/verify.rs index d81e83a0..e5bd5c7b 100644 --- a/v2/crates/ruview-auth/src/verify.rs +++ b/v2/crates/ruview-auth/src/verify.rs @@ -37,11 +37,20 @@ //! 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. +//! **`client_id` is Cognitum's stand-in for `aud`.** `cognitum-one/freetokens` +//! (live) documents the contract — *"Cognitum access tokens intentionally use +//! custom `client_id` rather than a registered JWT `aud` claim"* — and rejects +//! any token whose `client_id` is not its own. This verifier does the same via +//! [`VerifierConfig::allowed_client_ids`]. +//! +//! An earlier revision treated `client_id` as unusable because clients borrow +//! each other's registrations (musica shipped as `meta-proxy` while its own was +//! pending) and relied on scope alone. That was reasoning from a transitional +//! state: RuView has its own registered client, and accepting a token minted for +//! any Cognitum product is a weaker position than the platform intends. +//! +//! So there are now TWO boundaries, not one: audience (`client_id`) and +//! capability (`scope`). Neither is optional garnish. use jsonwebtoken::{decode, decode_header, Algorithm, Validation}; use serde::Deserialize; @@ -88,6 +97,10 @@ pub enum VerifyError { MissingAccountId, #[error("token does not carry the required scope {required:?}")] MissingScope { required: String }, + /// Minted for a different Cognitum product. `client_id` is the platform's + /// audience mechanism in the absence of `aud`. + #[error("token was issued to client {found:?}, which this server does not accept")] + WrongAudience { found: String }, } /// Identity's access-token claims. Mirrors `AccessTokenClaims` in @@ -132,6 +145,21 @@ pub struct VerifierConfig { pub issuer: String, /// The scope a caller must hold for the route being served. pub required_scope: String, + /// `client_id` values whose tokens this server accepts — the AUDIENCE check. + /// + /// Cognitum tokens carry no `aud`; the platform uses `client_id` for this + /// instead. `freetokens` (cognitum-one/freetokens, live) states the contract + /// plainly — *"Cognitum access tokens intentionally use custom `client_id` + /// rather than a registered JWT `aud` claim"* — and enforces + /// `payload.client_id !== OAUTH_CLIENT_ID` on every request. + /// + /// Empty means accept any client, which is what an earlier revision did on + /// the reasoning that clients borrow each other's registrations (musica + /// shipped as `meta-proxy` while its own was pending). That was a + /// transitional state, not the model: RuView has its own registered client, + /// so leaving this empty means accepting a token minted for ANY Cognitum + /// product. Configure it. + pub allowed_client_ids: Vec, } /// Verify a raw JWT and produce a [`Principal`]. @@ -171,6 +199,17 @@ pub fn verify_access_token( if claims.typ.as_deref() != Some(TYP_ACCESS) { return Err(VerifyError::WrongTokenType { found: claims.typ }); } + // AUDIENCE. Cognitum's stand-in for `aud` (see VerifierConfig docs). + if !config.allowed_client_ids.is_empty() + && !config + .allowed_client_ids + .iter() + .any(|c| c == &claims.client_id) + { + return Err(VerifyError::WrongAudience { + found: claims.client_id, + }); + } if claims.setup || claims.workload { // Belt and braces alongside the `typ` check: identity stamps these as // booleans as well, and a credential that sets either must never be diff --git a/v2/crates/ruview-auth/tests/verifier_matrix.rs b/v2/crates/ruview-auth/tests/verifier_matrix.rs index 7db59f74..e376d837 100644 --- a/v2/crates/ruview-auth/tests/verifier_matrix.rs +++ b/v2/crates/ruview-auth/tests/verifier_matrix.rs @@ -133,6 +133,8 @@ fn config_for(required_scope: &str) -> VerifierConfig { VerifierConfig { issuer: TEST_ISSUER.to_string(), required_scope: required_scope.to_string(), + // Mirrors production: RuView accepts only tokens minted for itself. + allowed_client_ids: vec!["ruview".to_string()], } } @@ -388,12 +390,55 @@ fn g2_a_genuinely_valid_token_from_another_cognitum_product_cannot_reach_the_sen c["client_id"] = json!("meta-proxy"); c["scope"] = json!("inference"); + // Rejected on AUDIENCE now (client_id), which is the stronger of the two + // reasons — it fires before scope is even considered. assert!(matches!( verify(&sign(&c), scope::SENSING_READ), - Err(VerifyError::MissingScope { .. }) + Err(VerifyError::WrongAudience { .. }) )); } +#[test] +fn a_token_minted_for_another_cognitum_product_is_refused_even_with_the_right_scope() { + // The audience check standing alone. Same user, same signature, correct + // sensing:read scope — but minted for freetokens, so not for this server. + // `cognitum-one/freetokens` enforces the mirror image of this. + let mut c = valid_claims(); + c["client_id"] = json!("freetokens"); + assert!(matches!( + verify(&sign(&c), scope::SENSING_READ), + Err(VerifyError::WrongAudience { .. }) + )); +} + +#[test] +fn an_empty_audience_list_accepts_any_client() { + // The documented opt-out (RUVIEW_OAUTH_CLIENT_IDS=*). Pinned so the + // behaviour is deliberate rather than accidental. + let mut c = valid_claims(); + c["client_id"] = json!("some-other-product"); + let cfg = VerifierConfig { + issuer: TEST_ISSUER.to_string(), + required_scope: scope::SENSING_READ.to_string(), + allowed_client_ids: vec![], + }; + verify_access_token(&sign(&c), &jwks_serving_test_key(), &cfg) + .expect("an empty allowlist means accept any client"); +} + +#[test] +fn multiple_allowed_clients_are_honoured() { + // Migration case: accepting a borrowed registration alongside our own. + let mut c = valid_claims(); + c["client_id"] = json!("meta-proxy"); + let cfg = VerifierConfig { + issuer: TEST_ISSUER.to_string(), + required_scope: scope::SENSING_READ.to_string(), + allowed_client_ids: vec!["ruview".into(), "meta-proxy".into()], + }; + verify_access_token(&sign(&c), &jwks_serving_test_key(), &cfg).expect("both accepted"); +} + #[test] fn g2_a_read_scoped_session_cannot_reach_the_admin_surface() { // The routine case the least-scope rule exists for: a dashboard streaming diff --git a/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs b/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs index b57783c6..0cea8bad 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs @@ -71,6 +71,30 @@ pub const OAUTH_JWKS_URL_ENV: &str = "RUVIEW_OAUTH_JWKS_URL"; /// The production Cognitum issuer, for operators who just want it on. pub const COGNITUM_ISSUER: &str = "https://auth.cognitum.one"; +/// Comma-separated `client_id` values whose tokens this server accepts — the +/// AUDIENCE control. Defaults to [`DEFAULT_CLIENT_ID`]. +/// +/// Cognitum tokens carry no `aud`; `client_id` is the platform's stand-in, and +/// `cognitum-one/freetokens` enforces exactly this. Set to `*` to accept any +/// Cognitum client — only sensible while borrowing another product's +/// registration, and it means any Cognitum token opens this server. +pub const OAUTH_CLIENT_IDS_ENV: &str = "RUVIEW_OAUTH_CLIENT_IDS"; + +/// RuView's own registered OAuth client (identity migration `0017`). +pub const DEFAULT_CLIENT_ID: &str = "ruview"; + +fn allowed_client_ids() -> Vec { + match std::env::var(OAUTH_CLIENT_IDS_ENV) { + Ok(v) if v.trim() == "*" => Vec::new(), // explicit opt-out + Ok(v) if !v.trim().is_empty() => v + .split(',') + .map(|c| c.trim().to_string()) + .filter(|c| !c.is_empty()) + .collect(), + _ => vec![DEFAULT_CLIENT_ID.to_string()], + } +} + /// Path prefix the middleware protects when auth is enabled. pub const PROTECTED_PREFIX: &str = "/api/v1/"; @@ -140,6 +164,7 @@ fn is_ws_path(path: &str) -> bool { pub struct OAuthState { jwks: JwksCache, issuer: String, + allowed_client_ids: Vec, } impl std::fmt::Debug for OAuthState { @@ -233,7 +258,17 @@ impl AuthState { key_count, "Cognitum OAuth enabled for /api/v1/*" ); - Some(Arc::new(OAuthState { jwks, issuer })) + let allowed_client_ids = allowed_client_ids(); + if allowed_client_ids.is_empty() { + tracing::warn!( + "{OAUTH_CLIENT_IDS_ENV}=* — this server accepts a Cognitum token minted \ + for ANY product, not just RuView. `client_id` is the platform's stand-in \ + for `aud`; disabling it leaves scope as the only boundary." + ); + } else { + tracing::info!(accepted_clients = ?allowed_client_ids, "OAuth audience restricted"); + } + Some(Arc::new(OAuthState { jwks, issuer, allowed_client_ids })) } Ok(_) => return Err(OAuthConfigError::EmptyIssuer), Err(_) => None, @@ -445,6 +480,7 @@ pub async fn require_bearer( let config = VerifierConfig { issuer: oauth.issuer.clone(), required_scope: required.to_string(), + allowed_client_ids: oauth.allowed_client_ids.clone(), }; match verify_access_token(supplied, &oauth.jwks, &config) { Ok(principal) => { @@ -851,6 +887,7 @@ mod oauth_tests { Arc::new(OAuthState { jwks: JwksCache::new("https://stub/jwks.json", Box::new(StaticJwks(doc))), issuer: ISSUER.to_string(), + allowed_client_ids: vec!["ruview".to_string()], }) }