mirror of
https://github.com/ruvnet/RuView
synced 2026-07-24 17:43:20 +00:00
fix(auth): enforce client_id as the audience — Cognitum's stand-in for aud
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
This commit is contained in:
@@ -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("<jwt>", &jwks, &config)?;
|
||||
|
||||
@@ -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<String>,
|
||||
}
|
||||
|
||||
/// 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<String> {
|
||||
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<String>,
|
||||
}
|
||||
|
||||
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()],
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user