From 84e6bb428d9716d1ca8c2d13ed0a4466a6ce3e7d Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Thu, 23 Jul 2026 23:59:06 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(dashboard):=20identity=E2=86=92tier=20?= =?UTF-8?q?grants=20for=20IdPs=20that=20mint=20no=20tier=20claim=20(JEF-50?= =?UTF-8?q?1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloudflare Access over GitHub relays no `tier` claim (GitHub emits none), so every operator floors to `redacted` and the forensic/raw debugging surface is unreachable. Adds an operator-configured identity→tier grant (PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS, format `tier=id1,id2;tier=id3`) that resolves the ceiling from the VERIFIED sub/email when no tier claim is present. Precedence: (1) an explicit, recognized `tier` claim always wins — the IdP's own statement is authoritative, even over a grant; (2) else the highest tier granted to the verified identity (sub exact match, email case-insensitive); (3) else the unchanged Redacted floor. Parsing is strict (mirrors the MIN_TIER parser): an unknown tier name or malformed entry is a loud ConfigError so build_dashboard_auth/build_mcp_verifier refuse to serve rather than silently drop or silently over-grant. Threads an optional `email` claim through Claims/Identity for the match. The chart gains a `tierGrants` value (empty by default — actual grants are wired in the separate cluster repo). Closes JEF-501 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP --- charts/protector/templates/deployment.yaml | 7 + charts/protector/values.yaml | 10 + engine/src/engine/dashboard/auth/claims.rs | 91 +++++- engine/src/engine/dashboard/auth/mod.rs | 75 ++++- .../src/engine/dashboard/auth/test_support.rs | 1 + engine/src/engine/dashboard/auth/tests.rs | 264 +++++++++++++++++- engine/src/engine/run_loop/tests.rs | 50 ++++ 7 files changed, 489 insertions(+), 9 deletions(-) diff --git a/charts/protector/templates/deployment.yaml b/charts/protector/templates/deployment.yaml index 54b0862..d820f59 100644 --- a/charts/protector/templates/deployment.yaml +++ b/charts/protector/templates/deployment.yaml @@ -313,6 +313,13 @@ spec: value: {{ .audience | quote }} - name: PROTECTOR_DASHBOARD_OIDC_TIER_CLAIM value: {{ .tierClaim | quote }} + {{- if .tierGrants }} + # Identity→tier grants (JEF-501): resolves the forensic/raw ceiling from the VERIFIED + # sub/email when the IdP mints no `tier` claim (e.g. Cloudflare Access over GitHub). + # An explicit `tier` claim, when present, still takes precedence over a grant. + - name: PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS + value: {{ .tierGrants | quote }} + {{- end }} {{- end }} {{- end }} {{- end }} diff --git a/charts/protector/values.yaml b/charts/protector/values.yaml index 1850262..846d55a 100644 --- a/charts/protector/values.yaml +++ b/charts/protector/values.yaml @@ -327,6 +327,16 @@ engine: # already-read-only view an identity sees; JEF-489). Default `tier`; supports a flat namespaced # key or a dotted path (e.g. `authz.tier`). tierClaim: "tier" + # Operator identity→tier grants (JEF-501): resolves the forensic/raw ceiling from the + # VERIFIED token identity (`sub`/`email`) when the IdP mints no `tier` claim at all — e.g. + # Cloudflare Access relaying GitHub, which emits neither. Format `tier=id1,id2;tier=id3`, + # e.g. `raw=alice@example.com;forensic=bob@example.com`. An explicit `tier` claim (when the + # IdP does mint one) always takes precedence over a grant. EMPTY = no grants (every identity + # floors to `redacted`, today's behavior) — the actual grant for this cluster is wired in the + # separate cluster repo, not here. A malformed value or an unrecognized tier name is a loud + # misconfiguration: the dashboard/MCP server is NOT served rather than silently dropping the + # grant. + tierGrants: "" # Two exploitation-intel feeds drive the engine's exploit intel + model reasoning # (ADR-0015): the CISA KEV catalogue (actively-exploited CVEs, known NOW) and the FIRST.org # EPSS scores (the PREDICTIVE per-CVE exploitation probability — JEF-243). The engine NEVER diff --git a/engine/src/engine/dashboard/auth/claims.rs b/engine/src/engine/dashboard/auth/claims.rs index aa318f4..ac0e292 100644 --- a/engine/src/engine/dashboard/auth/claims.rs +++ b/engine/src/engine/dashboard/auth/claims.rs @@ -63,18 +63,99 @@ impl Tier { _ => Tier::Redacted, } } + + /// Resolve the CEILING tier for a verified identity (JEF-501), with this precedence: + /// + /// 1. An explicit, **recognized** `tier` claim wins — the IdP's own statement is authoritative, + /// even over a configured grant (e.g. a claim of `forensic` beats a `raw` grant for the same + /// identity: "the IdP's explicit statement wins"). + /// 2. Else, the highest tier the operator's [`TierGrants`] awards this identity, matched by the + /// verified `sub` (exact) or `email` (case-insensitive) — this is what lets an operator grant + /// forensic/raw access when the IdP (e.g. Cloudflare Access over GitHub) mints no `tier` + /// claim at all. + /// 3. Else, the most-restricted [`Tier::Redacted`] floor (ADR-0030 §6 — never a permissive + /// default). + /// + /// Step 1 uses the STRICT [`Tier::parse_config`], not the lenient [`Tier::from_claim_str`]: an + /// unrecognized claim value (garbage, not merely absent) is deliberately NOT treated as an + /// explicit "redacted" statement — it falls through to the grant lookup instead, so a malformed + /// IdP claim can never shadow a legitimate operator grant. + pub fn from_claims_with_grants(claims: &Claims, path: &str, grants: &TierGrants) -> Tier { + if let Some(tier) = Tier::recognized_claim(claims, path) { + return tier; + } + grants.resolve(&claims.sub, claims.email.as_deref()) + } + + /// The claim at `path`, if present, a string, and a RECOGNIZED tier label — `None` for + /// absent/empty/non-string/unrecognized (see [`Tier::from_claims_with_grants`] for why + /// "unrecognized" must be `None`, not `Some(Redacted)`, here). + fn recognized_claim(claims: &Claims, path: &str) -> Option { + let value = claims.lookup(path).and_then(Value::as_str)?; + Tier::parse_config(value) + } +} + +/// Operator-configured identity→tier grants (`PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS`, JEF-501): +/// resolves the tier ceiling from the VERIFIED token identity (`sub`/`email`) when the IdP mints no +/// `tier` claim at all — e.g. Cloudflare Access relaying GitHub, which emits neither. A grant is a +/// CEILING like the claim it stands in for: it can only be READ here, never combined additively, and +/// [`TierGrants::default`] (no entries) reproduces today's behavior (every identity floors to +/// `Redacted` absent a `tier` claim). +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TierGrants { + /// Identifiers (matched as `sub` exact / `email` case-insensitive) granted [`Tier::Raw`]. + raw: Vec, + /// Identifiers granted [`Tier::Forensic`]. + forensic: Vec, +} + +impl TierGrants { + /// Grant `tier` to every identifier in `ids`. A [`Tier::Redacted`] grant is accepted (the + /// config syntax names a real tier) but is a documented no-op: `Redacted` is already the floor + /// every identity gets absent any grant, so there is nothing to record. + pub fn grant(&mut self, tier: Tier, ids: impl IntoIterator) { + match tier { + Tier::Raw => self.raw.extend(ids), + Tier::Forensic => self.forensic.extend(ids), + Tier::Redacted => {} + } + } + + /// The highest tier granted to a verified identity: `sub` is matched EXACTLY; `email` (if the + /// token carried one) is matched CASE-INSENSITIVELY. Neither matching ⇒ [`Tier::Redacted`] (an + /// unlisted/absent identity stays at the floor — a grant never widens beyond what's configured). + pub fn resolve(&self, sub: &str, email: Option<&str>) -> Tier { + if Self::matches(&self.raw, sub, email) { + Tier::Raw + } else if Self::matches(&self.forensic, sub, email) { + Tier::Forensic + } else { + Tier::Redacted + } + } + + fn matches(ids: &[String], sub: &str, email: Option<&str>) -> bool { + ids.iter() + .any(|id| id == sub || email.is_some_and(|email| id.eq_ignore_ascii_case(email))) + } } -/// The decoded token claims the verifier reads: the required `sub`, plus every other claim -/// captured flat in `extra` so the operator-configured tier claim can be looked up from it -/// without this struct having to name the IdP's claim schema (ADR-0030 §1: protector reads the -/// tier claim, it does not define it). +/// The decoded token claims the verifier reads: the required `sub`, the optional `email` +/// (JEF-501 — used, together with `sub`, to match an operator-configured tier grant), plus every +/// other claim captured flat in `extra` so the operator-configured tier claim can be looked up +/// from it without this struct having to name the IdP's claim schema (ADR-0030 §1: protector reads +/// the tier claim, it does not define it). #[derive(Debug, Deserialize)] pub struct Claims { /// The subject — the normalized identity's principal. Required (enforced by the verifier's /// `set_required_spec_claims`). pub sub: String, - /// Every non-`sub` claim, flattened, so a configurable tier path resolves against it. + /// The verified token's `email` claim, if the IdP includes one. Optional — many machine + /// tokens (ID-JAG, service principals) carry no email at all. + #[serde(default)] + pub email: Option, + /// Every non-`sub`/`email` claim, flattened, so a configurable tier path resolves against it. #[serde(flatten)] pub extra: Map, } diff --git a/engine/src/engine/dashboard/auth/mod.rs b/engine/src/engine/dashboard/auth/mod.rs index 9e05f66..bd471fd 100644 --- a/engine/src/engine/dashboard/auth/mod.rs +++ b/engine/src/engine/dashboard/auth/mod.rs @@ -43,7 +43,7 @@ use axum::middleware::Next; use axum::response::{IntoResponse, Response}; use jsonwebtoken::{Algorithm, Validation, decode, decode_header}; -use claims::{Claims, Tier}; +use claims::{Claims, Tier, TierGrants}; use jwks::{HttpJwksFetcher, JwksFetcher, JwksStore}; /// `PROTECTOR_DASHBOARD_OIDC_ISSUER` — the configured issuer. Its ABSENCE is what makes the @@ -57,6 +57,11 @@ const ENV_AUDIENCE: &str = "PROTECTOR_DASHBOARD_OIDC_AUDIENCE"; const ENV_TIER_CLAIM: &str = "PROTECTOR_DASHBOARD_OIDC_TIER_CLAIM"; /// `PROTECTOR_DASHBOARD_OIDC_ALGORITHM` — the pinned asymmetric algorithm (`RS256` | `ES256`). const ENV_ALGORITHM: &str = "PROTECTOR_DASHBOARD_OIDC_ALGORITHM"; +/// `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS` — operator identity→tier grants (JEF-501): resolves the +/// ceiling from the VERIFIED `sub`/`email` when the IdP mints no `tier` claim at all (e.g. +/// Cloudflare Access relaying GitHub). Format `tier=id1,id2;tier=id3`, e.g. +/// `raw=alice@example.com;forensic=bob@example.com`. Unset/empty = no grants (unchanged behavior). +const ENV_TIER_GRANTS: &str = "PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS"; /// The default tier claim path when `PROTECTOR_DASHBOARD_OIDC_TIER_CLAIM` is unset. const DEFAULT_TIER_CLAIM: &str = "tier"; @@ -111,6 +116,9 @@ pub struct OidcConfig { pub tier_claim: String, /// The pinned asymmetric algorithm. pub algorithm: SigningAlgorithm, + /// Operator identity→tier grants (JEF-501): resolves the ceiling from the verified `sub`/ + /// `email` when no `tier` claim is present. Empty (default) = no grants — current behavior. + pub tier_grants: TierGrants, } /// Why [`OidcConfig::from_env`] could not build a config even though an issuer was present. This @@ -129,6 +137,19 @@ pub enum ConfigError { "PROTECTOR_DASHBOARD_OIDC_MIN_TIER `{0}` is not a recognized tier (redacted, forensic, raw)" )] UnsupportedTier(String), + /// A `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS` entry names a tier that is not one of the + /// recognized tiers — a loud misconfiguration (JEF-501), never a silently-dropped grant. + #[error( + "PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS names an unrecognized tier `{0}` \ + (redacted, forensic, raw)" + )] + UnsupportedTierGrant(String), + /// `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS` is not well-formed: every `;`-separated entry must be + /// `tier=id1,id2,...` with at least one non-empty identifier. + #[error( + "PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS entry `{0}` is malformed (expected `tier=id1,id2`)" + )] + MalformedTierGrants(String), } impl OidcConfig { @@ -148,15 +169,53 @@ impl OidcConfig { } None => SigningAlgorithm::Rs256, }; + let tier_grants = match non_empty_env(ENV_TIER_GRANTS) { + Some(value) => parse_tier_grants(&value)?, + None => TierGrants::default(), + }; Ok(Some(OidcConfig { issuer, audience, tier_claim, algorithm, + tier_grants, })) } } +/// Strictly parse `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS`: `;`-separated entries of +/// `tier=id1,id2,...`. Every entry must name a RECOGNIZED tier ([`Tier::parse_config`] — the same +/// strict parser `PROTECTOR_DASHBOARD_OIDC_MIN_TIER` uses, see [`enforce`]) and carry at least one +/// non-empty identifier, or this fails loud with a [`ConfigError`] — a typo here must never +/// silently drop a grant or, worse, silently admit an unintended identifier (fail loud, exactly +/// like a mistyped `MIN_TIER`). +fn parse_tier_grants(value: &str) -> Result { + let mut grants = TierGrants::default(); + for entry in value.split(';') { + let entry = entry.trim(); + if entry.is_empty() { + return Err(ConfigError::MalformedTierGrants(value.to_string())); + } + let (tier_name, ids) = entry + .split_once('=') + .ok_or_else(|| ConfigError::MalformedTierGrants(entry.to_string()))?; + let tier_name = tier_name.trim(); + let tier = Tier::parse_config(tier_name) + .ok_or_else(|| ConfigError::UnsupportedTierGrant(tier_name.to_string()))?; + let ids: Vec = ids + .split(',') + .map(str::trim) + .filter(|id| !id.is_empty()) + .map(str::to_string) + .collect(); + if ids.is_empty() { + return Err(ConfigError::MalformedTierGrants(entry.to_string())); + } + grants.grant(tier, ids); + } + Ok(grants) +} + /// A trimmed, non-empty environment value, or `None` if unset/blank. pub(super) fn non_empty_env(key: &str) -> Option { std::env::var(key) @@ -172,8 +231,13 @@ pub(super) fn non_empty_env(key: &str) -> Option { pub struct Identity { /// The token subject (`sub`) — the principal (a human for a browser/ID-JAG token). pub subject: String, - /// The resolved authorization tier (most-restricted when the claim is absent/empty/unknown). + /// The resolved authorization tier — see [`Tier::from_claims_with_grants`] for the precedence + /// (an explicit recognized `tier` claim, else an identity→tier grant, else the floor). pub tier: Tier, + /// The verified token's `email` claim, if present (JEF-501). Not an identity in its own + /// right — `subject` remains the principal — but threaded through so callers/logs that want it + /// (and the tier-grant resolution above) can read it without re-decoding the token. + pub email: Option, } /// Every distinct way verification can fail. Each acceptance-critical failure — tampered @@ -323,10 +387,15 @@ impl Verifier { let key = self.jwks.decoding_key(header.kid.as_deref()).await?; let data = decode::(token, &key, &self.validation).map_err(|e| AuthError::from_jwt(&e))?; - let tier = Tier::from_claims(&data.claims, &self.config.tier_claim); + let tier = Tier::from_claims_with_grants( + &data.claims, + &self.config.tier_claim, + &self.config.tier_grants, + ); Ok(Identity { subject: data.claims.sub, tier, + email: data.claims.email, }) } } diff --git a/engine/src/engine/dashboard/auth/test_support.rs b/engine/src/engine/dashboard/auth/test_support.rs index 7a9405c..071ac2c 100644 --- a/engine/src/engine/dashboard/auth/test_support.rs +++ b/engine/src/engine/dashboard/auth/test_support.rs @@ -130,6 +130,7 @@ pub(crate) fn test_config() -> OidcConfig { audience: AUDIENCE.into(), tier_claim: "tier".into(), algorithm: SigningAlgorithm::Rs256, + tier_grants: super::claims::TierGrants::default(), } } diff --git a/engine/src/engine/dashboard/auth/tests.rs b/engine/src/engine/dashboard/auth/tests.rs index 0059d23..08fd662 100644 --- a/engine/src/engine/dashboard/auth/tests.rs +++ b/engine/src/engine/dashboard/auth/tests.rs @@ -10,7 +10,7 @@ use base64::Engine as _; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use serde_json::json; -use super::claims::{Claims, Tier}; +use super::claims::{Claims, Tier, TierGrants}; use super::jwks::{HttpJwksFetcher, JwksFetcher, JwksStore}; use super::test_support::{ AUDIENCE, E, ISSUER, KEY_A_N, KEY_A_PEM, KEY_B_N, KEY_B_PEM, KID_A, KID_B, TestFetcher, @@ -393,6 +393,166 @@ fn tier_ordering_is_redacted_lt_forensic_lt_raw_with_redacted_default() { assert_eq!(Tier::default(), Tier::Redacted); } +// --------------------------------------------------------------------------------------------- +// Acceptance (JEF-501): identity→tier grants resolve the ceiling when the IdP mints no `tier` +// claim — the Cloudflare-Access-over-GitHub case, verified live, that motivated this ticket. +// --------------------------------------------------------------------------------------------- + +/// A grants table with `raw` for one identity-pair and `forensic` for another, used across the +/// claim-level unit tests below. +fn sample_grants() -> TierGrants { + let mut grants = TierGrants::default(); + grants.grant( + Tier::Raw, + ["thejefflarson@gmail.com".to_string(), "raw-sub".to_string()], + ); + grants.grant( + Tier::Forensic, + ["alice@x.com".to_string(), "forensic-sub".to_string()], + ); + grants +} + +#[test] +fn grant_resolves_the_ceiling_by_email_or_sub_when_no_tier_claim_is_present() { + let grants = sample_grants(); + // No `tier` claim at all — the case this ticket exists for. + let claims: Claims = + serde_json::from_value(json!({ "sub": "someone", "email": "thejefflarson@gmail.com" })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Raw + ); + + let claims: Claims = + serde_json::from_value(json!({ "sub": "someone", "email": "alice@x.com" })).unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Forensic, + "granted forensic, never widened to raw" + ); + + // Not in either list — stays at the floor. + let claims: Claims = + serde_json::from_value(json!({ "sub": "someone", "email": "nobody@x.com" })).unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted + ); +} + +#[test] +fn an_explicit_recognized_tier_claim_wins_over_a_grant() { + // The identity has a `raw` grant, but the IdP's own token asserts `forensic` — the explicit + // claim wins ("the IdP's explicit statement wins"), so the ceiling is forensic, not raw. + let grants = sample_grants(); + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "tier": "forensic", + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Forensic, + "an explicit recognized claim takes precedence over a grant" + ); +} + +#[test] +fn an_unrecognized_tier_claim_falls_through_to_the_grant_not_the_floor() { + // A garbage claim value (not merely absent) must not shadow a legitimate grant: it is not an + // "explicit recognized statement", so resolution falls through to the grant lookup. + let grants = sample_grants(); + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "tier": "superuser", + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Raw + ); +} + +#[test] +fn grant_email_match_is_case_insensitive_sub_match_is_exact() { + let grants = sample_grants(); + // Email: case-insensitive. + assert_eq!( + grants.resolve("irrelevant", Some("THEJEFFLARSON@GMAIL.COM")), + Tier::Raw + ); + // Sub: exact only — a case-differing sub does not match. + assert_eq!(grants.resolve("RAW-SUB", None), Tier::Redacted); + assert_eq!(grants.resolve("raw-sub", None), Tier::Raw); + // A grant entry matching neither is inert. + assert_eq!( + grants.resolve("someone-else", Some("someone-else@x.com")), + Tier::Redacted + ); +} + +#[test] +fn missing_or_empty_email_still_resolves_a_sub_based_grant() { + let grants = sample_grants(); + assert_eq!(grants.resolve("forensic-sub", None), Tier::Forensic); + assert_eq!(grants.resolve("forensic-sub", Some("")), Tier::Forensic); +} + +#[tokio::test] +async fn verifier_end_to_end_resolves_raw_and_forensic_grants_from_a_signed_token() { + // Mints real signed tokens (no `tier` claim) through the JEF-485 scaffolding and drives the + // FULL verify() path — proving the grant wiring works end to end, not just at the claims layer. + let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); + let config = OidcConfig { + tier_grants: sample_grants(), + ..test_config() + }; + let verifier = Verifier::with_fetcher(config, fetcher); + + // Raw grant, matched by email; token carries no `tier` claim. + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["email"] = json!("thejefflarson@gmail.com"); + let token = sign(KEY_A_PEM, KID_A, &claims); + let identity = verifier.verify(&token).await.expect("token verifies"); + assert_eq!(identity.tier, Tier::Raw); + assert_eq!(identity.email.as_deref(), Some("thejefflarson@gmail.com")); + + // Forensic grant, matched by sub, unlisted identity stays redacted. + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["sub"] = json!("forensic-sub"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); + + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["sub"] = json!("nobody-in-particular"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Redacted); +} + +#[tokio::test] +async fn verifier_end_to_end_explicit_claim_still_wins_over_a_raw_grant() { + let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); + let config = OidcConfig { + tier_grants: sample_grants(), + ..test_config() + }; + let verifier = Verifier::with_fetcher(config, fetcher); + + // This identity has a `raw` grant, but the token itself asserts `forensic` — the claim wins. + let mut claims = base_claims(); + claims["tier"] = json!("forensic"); + claims["email"] = json!("thejefflarson@gmail.com"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); +} + // --------------------------------------------------------------------------------------------- // Acceptance: one path reads a token from BOTH lanes; the mountable layer denies fail-closed. // --------------------------------------------------------------------------------------------- @@ -512,6 +672,7 @@ fn from_env_models_unconfigured_configured_and_errors() { super::ENV_AUDIENCE, super::ENV_TIER_CLAIM, super::ENV_ALGORITHM, + super::ENV_TIER_GRANTS, ]; let clear = || unsafe { for key in keys { @@ -534,6 +695,11 @@ fn from_env_models_unconfigured_configured_and_errors() { assert_eq!(config.audience, AUDIENCE); assert_eq!(config.tier_claim, "tier"); assert_eq!(config.algorithm, SigningAlgorithm::Rs256); + assert_eq!( + config.tier_grants, + super::claims::TierGrants::default(), + "TIER_GRANTS unset ⇒ no grants (unchanged behavior)" + ); // Configurable tier claim + ES256. unsafe { @@ -554,6 +720,102 @@ fn from_env_models_unconfigured_configured_and_errors() { clear(); } +// --------------------------------------------------------------------------------------------- +// PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS (JEF-501): strict parsing, fail loud on malformed/unknown. +// --------------------------------------------------------------------------------------------- + +#[test] +fn tier_grants_config_parses_multiple_tiers_and_fails_loud_on_malformed_or_unknown() { + let _env = super::test_support::ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let clear = || unsafe { + for key in [ + super::ENV_ISSUER, + super::ENV_AUDIENCE, + super::ENV_TIER_GRANTS, + ] { + std::env::remove_var(key); + } + }; + clear(); + unsafe { + std::env::set_var(super::ENV_ISSUER, ISSUER); + std::env::set_var(super::ENV_AUDIENCE, AUDIENCE); + } + + // A well-formed multi-tier grant list parses and resolves as documented: `sub` exact, + // `email` case-insensitive, an unlisted identity floors to Redacted. + unsafe { + std::env::set_var( + super::ENV_TIER_GRANTS, + "raw=thejefflarson@gmail.com;forensic=alice@x.com,BOB-SUB", + ); + } + let config = OidcConfig::from_env().unwrap().unwrap(); + assert_eq!( + config + .tier_grants + .resolve("someone", Some("THEJEFFLARSON@GMAIL.COM")), + Tier::Raw, + "email match is case-insensitive" + ); + assert_eq!( + config.tier_grants.resolve("someone", Some("alice@x.com")), + Tier::Forensic + ); + assert_eq!( + config.tier_grants.resolve("BOB-SUB", None), + Tier::Forensic, + "sub match is exact and works with no email at all" + ); + assert_eq!( + config.tier_grants.resolve("bob-sub", None), + Tier::Redacted, + "sub match is exact — a case-differing sub is inert" + ); + assert_eq!( + config.tier_grants.resolve("nobody", Some("nobody@x.com")), + Tier::Redacted, + "an identity matching neither list stays at the floor" + ); + + // An unknown tier name (not redacted/forensic/raw) fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "admin=alice@x.com") }; + assert_eq!( + OidcConfig::from_env(), + Err(ConfigError::UnsupportedTierGrant("admin".into())) + ); + + // Malformed syntax (no `=`) fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw-alice@x.com") }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + // An entry with no identifiers fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw=") }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + // A stray `;` (empty entry) fails loud rather than being silently skipped. + unsafe { + std::env::set_var( + super::ENV_TIER_GRANTS, + "raw=alice@x.com;;forensic=bob@x.com", + ) + }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + clear(); +} + // --------------------------------------------------------------------------------------------- // HTTP JWKS fetch: response-body size cap over a loopback OIDC server (no external egress). The // loopback server helper lives in `super::test_support::spawn_oidc_server`. diff --git a/engine/src/engine/run_loop/tests.rs b/engine/src/engine/run_loop/tests.rs index 130bcd8..65bfa2e 100644 --- a/engine/src/engine/run_loop/tests.rs +++ b/engine/src/engine/run_loop/tests.rs @@ -34,6 +34,7 @@ fn dashboard_oidc_min_tier_config_fails_loud_but_serves_on_valid_or_absent() { "PROTECTOR_DASHBOARD_OIDC_TIER_CLAIM", "PROTECTOR_DASHBOARD_OIDC_ALGORITHM", "PROTECTOR_DASHBOARD_OIDC_LOGIN_URL", + "PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS", ] { std::env::remove_var(key); } @@ -68,6 +69,55 @@ fn dashboard_oidc_min_tier_config_fails_loud_but_serves_on_valid_or_absent() { clear(); } +/// JEF-501: a malformed/unrecognized `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS` must fail LOUD — +/// neither `build_dashboard_auth` nor `build_mcp_verifier` serve on a misconfigured grant table — +/// exactly like a mistyped `MIN_TIER`; a valid grant table serves normally. +#[test] +fn tier_grants_config_fails_loud_but_serves_on_valid_or_absent() { + let _env = crate::engine::dashboard::auth::test_support::ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + + const ISSUER: &str = "PROTECTOR_DASHBOARD_OIDC_ISSUER"; + const AUDIENCE: &str = "PROTECTOR_DASHBOARD_OIDC_AUDIENCE"; + const TIER_GRANTS: &str = "PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS"; + let clear = || unsafe { + for key in [ISSUER, AUDIENCE, TIER_GRANTS] { + std::env::remove_var(key); + } + }; + clear(); + unsafe { + std::env::set_var(ISSUER, "https://issuer.example"); + std::env::set_var(AUDIENCE, "protector"); + } + + // An unrecognized tier name in the grants table → neither builder serves. + unsafe { std::env::set_var(TIER_GRANTS, "admin=alice@x.com") }; + assert!( + super::build_dashboard_auth().is_none(), + "an unrecognized TIER_GRANTS tier must fail loud (dashboard not served)" + ); + assert!( + super::build_mcp_verifier().is_none(), + "an unrecognized TIER_GRANTS tier must fail loud (mcp not served)" + ); + + // Malformed syntax → neither builder serves. + unsafe { std::env::set_var(TIER_GRANTS, "not-a-valid-entry") }; + assert!(super::build_dashboard_auth().is_none()); + assert!(super::build_mcp_verifier().is_none()); + + // A VALID grant table serves normally on both surfaces. + unsafe { std::env::set_var(TIER_GRANTS, "raw=alice@x.com;forensic=bob@x.com") }; + let (auth, mode) = super::build_dashboard_auth().expect("a valid grant table serves"); + assert!(auth.is_some()); + assert_eq!(mode, crate::engine::dashboard::AuthMode::Oidc); + assert!(super::build_mcp_verifier().is_some()); + + clear(); +} + /// The reflected element type asks the apiserver for metadata only. `metadata_api()` /// is what drives both `watcher(Api::>, _)` and /// `Api::::list_metadata` to issue `.../secrets` requests that return From 0e05e71fa84670469b29951213b838de7c92541f Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Fri, 24 Jul 2026 00:13:43 -0700 Subject: [PATCH 2/2] fix(dashboard): require verified email + type grant identifiers (JEF-501) Security review of PR #268 found two HIGH over-grant paths in the identity->tier grant matching: 1. An email-typed grant matched claims.email with no ownership check. A valid signature only proves the IdP minted the token, not that the subject owns the email it self-asserts (social login, a self-service directory) -- an attacker could set their account email to a granted operator's address and get elevated. Fixed by adding Claims.email_verified (bool, #[serde(default)] so absent means unverified, the safe default) and requiring it be true before an email-typed grant can match. 2. matches() compared every identifier against BOTH sub (exact) and email (case-insensitive) with a single OR, so an email-shaped grant could also match an opaque sub that happened to equal the same string, and vice versa -- silently widening the granted set. Fixed by typing each grant identifier at parse time (GrantId::Email vs GrantId::Sub, split on '@' presence, which is unambiguous for a sub) so it only ever matches its own field. Added tests for both: an unverified email equal to a grant resolves to Redacted while the same verified email resolves to the granted tier; a sub equal to a granted email string (and vice versa) does not match. Split the growing JEF-501 test surface into a new tier_grants_tests.rs to keep both files well under the repo's 1,000-line cap. Re-verified: cargo fmt, cargo clippy --all-targets -D warnings (clean), full cargo test green (907 passed). Addresses JEF-501 security review findings on PR #268. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP --- engine/src/engine/dashboard/auth/claims.rs | 118 +++-- engine/src/engine/dashboard/auth/mod.rs | 2 + engine/src/engine/dashboard/auth/tests.rs | 258 +---------- .../dashboard/auth/tier_grants_tests.rs | 406 ++++++++++++++++++ 4 files changed, 497 insertions(+), 287 deletions(-) create mode 100644 engine/src/engine/dashboard/auth/tier_grants_tests.rs diff --git a/engine/src/engine/dashboard/auth/claims.rs b/engine/src/engine/dashboard/auth/claims.rs index ac0e292..15e38ef 100644 --- a/engine/src/engine/dashboard/auth/claims.rs +++ b/engine/src/engine/dashboard/auth/claims.rs @@ -70,9 +70,9 @@ impl Tier { /// even over a configured grant (e.g. a claim of `forensic` beats a `raw` grant for the same /// identity: "the IdP's explicit statement wins"). /// 2. Else, the highest tier the operator's [`TierGrants`] awards this identity, matched by the - /// verified `sub` (exact) or `email` (case-insensitive) — this is what lets an operator grant - /// forensic/raw access when the IdP (e.g. Cloudflare Access over GitHub) mints no `tier` - /// claim at all. + /// verified `sub` (exact) or a **verified** `email` (case-insensitive) — this is what lets an + /// operator grant forensic/raw access when the IdP (e.g. Cloudflare Access over GitHub) + /// mints no `tier` claim at all. /// 3. Else, the most-restricted [`Tier::Redacted`] floor (ADR-0030 §6 — never a permissive /// default). /// @@ -80,11 +80,18 @@ impl Tier { /// unrecognized claim value (garbage, not merely absent) is deliberately NOT treated as an /// explicit "redacted" statement — it falls through to the grant lookup instead, so a malformed /// IdP claim can never shadow a legitimate operator grant. + /// + /// Step 2 passes `claims.email_verified` through to [`TierGrants::resolve`]: a signature only + /// proves the IdP MINTED the token, not that the subject OWNS the `email` claim it carries (a + /// provider that self-asserts `email` — social login, a self-service directory — lets an + /// attacker set their account email to a granted operator's address). An email-typed grant is + /// therefore never a match candidate unless the token asserts `email_verified: true`; `sub` + /// grants are unaffected (a `sub` is the IdP-assigned principal, not self-asserted). pub fn from_claims_with_grants(claims: &Claims, path: &str, grants: &TierGrants) -> Tier { if let Some(tier) = Tier::recognized_claim(claims, path) { return tier; } - grants.resolve(&claims.sub, claims.email.as_deref()) + grants.resolve(&claims.sub, claims.email.as_deref(), claims.email_verified) } /// The claim at `path`, if present, a string, and a RECOGNIZED tier label — `None` for @@ -96,25 +103,68 @@ impl Tier { } } +/// A single grant identifier, TYPED at parse time by how the operator wrote it, so it can only +/// ever match its own field: an identifier containing `@` is an **email** (matched only against a +/// **verified** `email` claim, case-insensitively); one without is a **sub** (matched only against +/// `sub`, exactly). This closes a cross-field collision a single untyped OR would otherwise allow +/// (JEF-501 HIGH fix): without typing, an operator's `raw=alice@example.com` (meant as an email) +/// would ALSO match a token whose opaque `sub` happened to equal that exact string, silently +/// widening the granted set beyond what was configured — and symmetrically for a bare `sub` +/// identifier that happens to collide with someone's `email`. `@`-presence is an unambiguous split +/// (an email always contains `@`; a `sub` — a GitHub id, a UUID, a service-account name — never +/// does in practice), so no identifier is ever ambiguous between the two. +#[derive(Debug, Clone, PartialEq, Eq)] +enum GrantId { + /// Matched against a **verified** `email` claim only, case-insensitively. + Email(String), + /// Matched against `sub` only, exactly. + Sub(String), +} + +impl GrantId { + fn parse(id: String) -> Self { + if id.contains('@') { + GrantId::Email(id) + } else { + GrantId::Sub(id) + } + } + + /// Whether this identifier matches the verified identity. An [`GrantId::Email`] NEVER matches + /// unless `email_verified` is `true` (JEF-501 HIGH fix: a self-asserted, unverified `email` + /// claim proves nothing about ownership — only that the IdP minted *a* token, not that the + /// subject controls that address). + fn matches(&self, sub: &str, email: Option<&str>, email_verified: bool) -> bool { + match self { + GrantId::Sub(id) => id == sub, + GrantId::Email(id) => { + email_verified && email.is_some_and(|email| email.eq_ignore_ascii_case(id)) + } + } + } +} + /// Operator-configured identity→tier grants (`PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS`, JEF-501): -/// resolves the tier ceiling from the VERIFIED token identity (`sub`/`email`) when the IdP mints no -/// `tier` claim at all — e.g. Cloudflare Access relaying GitHub, which emits neither. A grant is a -/// CEILING like the claim it stands in for: it can only be READ here, never combined additively, and -/// [`TierGrants::default`] (no entries) reproduces today's behavior (every identity floors to -/// `Redacted` absent a `tier` claim). +/// resolves the tier ceiling from the VERIFIED token identity (`sub`/verified-`email`) when the +/// IdP mints no `tier` claim at all — e.g. Cloudflare Access relaying GitHub, which emits neither. +/// A grant is a CEILING like the claim it stands in for: it can only be READ here, never combined +/// additively, and [`TierGrants::default`] (no entries) reproduces today's behavior (every +/// identity floors to `Redacted` absent a `tier` claim). #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct TierGrants { - /// Identifiers (matched as `sub` exact / `email` case-insensitive) granted [`Tier::Raw`]. - raw: Vec, + /// Identifiers granted [`Tier::Raw`]. + raw: Vec, /// Identifiers granted [`Tier::Forensic`]. - forensic: Vec, + forensic: Vec, } impl TierGrants { - /// Grant `tier` to every identifier in `ids`. A [`Tier::Redacted`] grant is accepted (the - /// config syntax names a real tier) but is a documented no-op: `Redacted` is already the floor - /// every identity gets absent any grant, so there is nothing to record. + /// Grant `tier` to every identifier in `ids`, each classified as email/sub by + /// [`GrantId::parse`]. A [`Tier::Redacted`] grant is accepted (the config syntax names a real + /// tier) but is a documented no-op: `Redacted` is already the floor every identity gets absent + /// any grant, so there is nothing to record. pub fn grant(&mut self, tier: Tier, ids: impl IntoIterator) { + let ids = ids.into_iter().map(GrantId::parse); match tier { Tier::Raw => self.raw.extend(ids), Tier::Forensic => self.forensic.extend(ids), @@ -122,30 +172,31 @@ impl TierGrants { } } - /// The highest tier granted to a verified identity: `sub` is matched EXACTLY; `email` (if the - /// token carried one) is matched CASE-INSENSITIVELY. Neither matching ⇒ [`Tier::Redacted`] (an - /// unlisted/absent identity stays at the floor — a grant never widens beyond what's configured). - pub fn resolve(&self, sub: &str, email: Option<&str>) -> Tier { - if Self::matches(&self.raw, sub, email) { + /// The highest tier granted to a verified identity. `sub` is matched EXACTLY against a + /// sub-typed grant only; `email` is matched CASE-INSENSITIVELY against an email-typed grant + /// only, and only when `email_verified` is `true` (JEF-501 — an unverified `email` claim is + /// never a match candidate). Neither matching ⇒ [`Tier::Redacted`] (an unlisted/absent identity + /// stays at the floor — a grant never widens beyond what's configured). + pub fn resolve(&self, sub: &str, email: Option<&str>, email_verified: bool) -> Tier { + if Self::matches(&self.raw, sub, email, email_verified) { Tier::Raw - } else if Self::matches(&self.forensic, sub, email) { + } else if Self::matches(&self.forensic, sub, email, email_verified) { Tier::Forensic } else { Tier::Redacted } } - fn matches(ids: &[String], sub: &str, email: Option<&str>) -> bool { - ids.iter() - .any(|id| id == sub || email.is_some_and(|email| id.eq_ignore_ascii_case(email))) + fn matches(ids: &[GrantId], sub: &str, email: Option<&str>, email_verified: bool) -> bool { + ids.iter().any(|id| id.matches(sub, email, email_verified)) } } -/// The decoded token claims the verifier reads: the required `sub`, the optional `email` -/// (JEF-501 — used, together with `sub`, to match an operator-configured tier grant), plus every -/// other claim captured flat in `extra` so the operator-configured tier claim can be looked up -/// from it without this struct having to name the IdP's claim schema (ADR-0030 §1: protector reads -/// the tier claim, it does not define it). +/// The decoded token claims the verifier reads: the required `sub`, the optional `email` + +/// `email_verified` (JEF-501 — used, together with `sub`, to match an operator-configured tier +/// grant), plus every other claim captured flat in `extra` so the operator-configured tier claim +/// can be looked up from it without this struct having to name the IdP's claim schema (ADR-0030 +/// §1: protector reads the tier claim, it does not define it). #[derive(Debug, Deserialize)] pub struct Claims { /// The subject — the normalized identity's principal. Required (enforced by the verifier's @@ -155,7 +206,14 @@ pub struct Claims { /// tokens (ID-JAG, service principals) carry no email at all. #[serde(default)] pub email: Option, - /// Every non-`sub`/`email` claim, flattened, so a configurable tier path resolves against it. + /// The verified token's `email_verified` claim. **Absent ⇒ `false`** — the safe default + /// (JEF-501 HIGH fix): a signature only proves the IdP minted the token, never that the + /// subject owns the `email` it carries, unless the IdP itself asserts it verified that + /// ownership. An email-typed [`TierGrants`] entry never matches without this being `true`. + #[serde(default)] + pub email_verified: bool, + /// Every non-`sub`/`email`/`email_verified` claim, flattened, so a configurable tier path + /// resolves against it. #[serde(flatten)] pub extra: Map, } diff --git a/engine/src/engine/dashboard/auth/mod.rs b/engine/src/engine/dashboard/auth/mod.rs index bd471fd..5c24731 100644 --- a/engine/src/engine/dashboard/auth/mod.rs +++ b/engine/src/engine/dashboard/auth/mod.rs @@ -33,6 +33,8 @@ mod enforce_tests; pub(crate) mod test_support; #[cfg(test)] mod tests; +#[cfg(test)] +mod tier_grants_tests; use std::sync::Arc; diff --git a/engine/src/engine/dashboard/auth/tests.rs b/engine/src/engine/dashboard/auth/tests.rs index 08fd662..2062147 100644 --- a/engine/src/engine/dashboard/auth/tests.rs +++ b/engine/src/engine/dashboard/auth/tests.rs @@ -393,166 +393,6 @@ fn tier_ordering_is_redacted_lt_forensic_lt_raw_with_redacted_default() { assert_eq!(Tier::default(), Tier::Redacted); } -// --------------------------------------------------------------------------------------------- -// Acceptance (JEF-501): identity→tier grants resolve the ceiling when the IdP mints no `tier` -// claim — the Cloudflare-Access-over-GitHub case, verified live, that motivated this ticket. -// --------------------------------------------------------------------------------------------- - -/// A grants table with `raw` for one identity-pair and `forensic` for another, used across the -/// claim-level unit tests below. -fn sample_grants() -> TierGrants { - let mut grants = TierGrants::default(); - grants.grant( - Tier::Raw, - ["thejefflarson@gmail.com".to_string(), "raw-sub".to_string()], - ); - grants.grant( - Tier::Forensic, - ["alice@x.com".to_string(), "forensic-sub".to_string()], - ); - grants -} - -#[test] -fn grant_resolves_the_ceiling_by_email_or_sub_when_no_tier_claim_is_present() { - let grants = sample_grants(); - // No `tier` claim at all — the case this ticket exists for. - let claims: Claims = - serde_json::from_value(json!({ "sub": "someone", "email": "thejefflarson@gmail.com" })) - .unwrap(); - assert_eq!( - Tier::from_claims_with_grants(&claims, "tier", &grants), - Tier::Raw - ); - - let claims: Claims = - serde_json::from_value(json!({ "sub": "someone", "email": "alice@x.com" })).unwrap(); - assert_eq!( - Tier::from_claims_with_grants(&claims, "tier", &grants), - Tier::Forensic, - "granted forensic, never widened to raw" - ); - - // Not in either list — stays at the floor. - let claims: Claims = - serde_json::from_value(json!({ "sub": "someone", "email": "nobody@x.com" })).unwrap(); - assert_eq!( - Tier::from_claims_with_grants(&claims, "tier", &grants), - Tier::Redacted - ); -} - -#[test] -fn an_explicit_recognized_tier_claim_wins_over_a_grant() { - // The identity has a `raw` grant, but the IdP's own token asserts `forensic` — the explicit - // claim wins ("the IdP's explicit statement wins"), so the ceiling is forensic, not raw. - let grants = sample_grants(); - let claims: Claims = serde_json::from_value(json!({ - "sub": "someone", - "email": "thejefflarson@gmail.com", - "tier": "forensic", - })) - .unwrap(); - assert_eq!( - Tier::from_claims_with_grants(&claims, "tier", &grants), - Tier::Forensic, - "an explicit recognized claim takes precedence over a grant" - ); -} - -#[test] -fn an_unrecognized_tier_claim_falls_through_to_the_grant_not_the_floor() { - // A garbage claim value (not merely absent) must not shadow a legitimate grant: it is not an - // "explicit recognized statement", so resolution falls through to the grant lookup. - let grants = sample_grants(); - let claims: Claims = serde_json::from_value(json!({ - "sub": "someone", - "email": "thejefflarson@gmail.com", - "tier": "superuser", - })) - .unwrap(); - assert_eq!( - Tier::from_claims_with_grants(&claims, "tier", &grants), - Tier::Raw - ); -} - -#[test] -fn grant_email_match_is_case_insensitive_sub_match_is_exact() { - let grants = sample_grants(); - // Email: case-insensitive. - assert_eq!( - grants.resolve("irrelevant", Some("THEJEFFLARSON@GMAIL.COM")), - Tier::Raw - ); - // Sub: exact only — a case-differing sub does not match. - assert_eq!(grants.resolve("RAW-SUB", None), Tier::Redacted); - assert_eq!(grants.resolve("raw-sub", None), Tier::Raw); - // A grant entry matching neither is inert. - assert_eq!( - grants.resolve("someone-else", Some("someone-else@x.com")), - Tier::Redacted - ); -} - -#[test] -fn missing_or_empty_email_still_resolves_a_sub_based_grant() { - let grants = sample_grants(); - assert_eq!(grants.resolve("forensic-sub", None), Tier::Forensic); - assert_eq!(grants.resolve("forensic-sub", Some("")), Tier::Forensic); -} - -#[tokio::test] -async fn verifier_end_to_end_resolves_raw_and_forensic_grants_from_a_signed_token() { - // Mints real signed tokens (no `tier` claim) through the JEF-485 scaffolding and drives the - // FULL verify() path — proving the grant wiring works end to end, not just at the claims layer. - let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); - let config = OidcConfig { - tier_grants: sample_grants(), - ..test_config() - }; - let verifier = Verifier::with_fetcher(config, fetcher); - - // Raw grant, matched by email; token carries no `tier` claim. - let mut claims = base_claims(); - claims.as_object_mut().unwrap().remove("tier"); - claims["email"] = json!("thejefflarson@gmail.com"); - let token = sign(KEY_A_PEM, KID_A, &claims); - let identity = verifier.verify(&token).await.expect("token verifies"); - assert_eq!(identity.tier, Tier::Raw); - assert_eq!(identity.email.as_deref(), Some("thejefflarson@gmail.com")); - - // Forensic grant, matched by sub, unlisted identity stays redacted. - let mut claims = base_claims(); - claims.as_object_mut().unwrap().remove("tier"); - claims["sub"] = json!("forensic-sub"); - let token = sign(KEY_A_PEM, KID_A, &claims); - assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); - - let mut claims = base_claims(); - claims.as_object_mut().unwrap().remove("tier"); - claims["sub"] = json!("nobody-in-particular"); - let token = sign(KEY_A_PEM, KID_A, &claims); - assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Redacted); -} - -#[tokio::test] -async fn verifier_end_to_end_explicit_claim_still_wins_over_a_raw_grant() { - let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); - let config = OidcConfig { - tier_grants: sample_grants(), - ..test_config() - }; - let verifier = Verifier::with_fetcher(config, fetcher); - - // This identity has a `raw` grant, but the token itself asserts `forensic` — the claim wins. - let mut claims = base_claims(); - claims["tier"] = json!("forensic"); - claims["email"] = json!("thejefflarson@gmail.com"); - let token = sign(KEY_A_PEM, KID_A, &claims); - assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); -} - // --------------------------------------------------------------------------------------------- // Acceptance: one path reads a token from BOTH lanes; the mountable layer denies fail-closed. // --------------------------------------------------------------------------------------------- @@ -697,7 +537,7 @@ fn from_env_models_unconfigured_configured_and_errors() { assert_eq!(config.algorithm, SigningAlgorithm::Rs256); assert_eq!( config.tier_grants, - super::claims::TierGrants::default(), + TierGrants::default(), "TIER_GRANTS unset ⇒ no grants (unchanged behavior)" ); @@ -720,102 +560,6 @@ fn from_env_models_unconfigured_configured_and_errors() { clear(); } -// --------------------------------------------------------------------------------------------- -// PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS (JEF-501): strict parsing, fail loud on malformed/unknown. -// --------------------------------------------------------------------------------------------- - -#[test] -fn tier_grants_config_parses_multiple_tiers_and_fails_loud_on_malformed_or_unknown() { - let _env = super::test_support::ENV_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); - let clear = || unsafe { - for key in [ - super::ENV_ISSUER, - super::ENV_AUDIENCE, - super::ENV_TIER_GRANTS, - ] { - std::env::remove_var(key); - } - }; - clear(); - unsafe { - std::env::set_var(super::ENV_ISSUER, ISSUER); - std::env::set_var(super::ENV_AUDIENCE, AUDIENCE); - } - - // A well-formed multi-tier grant list parses and resolves as documented: `sub` exact, - // `email` case-insensitive, an unlisted identity floors to Redacted. - unsafe { - std::env::set_var( - super::ENV_TIER_GRANTS, - "raw=thejefflarson@gmail.com;forensic=alice@x.com,BOB-SUB", - ); - } - let config = OidcConfig::from_env().unwrap().unwrap(); - assert_eq!( - config - .tier_grants - .resolve("someone", Some("THEJEFFLARSON@GMAIL.COM")), - Tier::Raw, - "email match is case-insensitive" - ); - assert_eq!( - config.tier_grants.resolve("someone", Some("alice@x.com")), - Tier::Forensic - ); - assert_eq!( - config.tier_grants.resolve("BOB-SUB", None), - Tier::Forensic, - "sub match is exact and works with no email at all" - ); - assert_eq!( - config.tier_grants.resolve("bob-sub", None), - Tier::Redacted, - "sub match is exact — a case-differing sub is inert" - ); - assert_eq!( - config.tier_grants.resolve("nobody", Some("nobody@x.com")), - Tier::Redacted, - "an identity matching neither list stays at the floor" - ); - - // An unknown tier name (not redacted/forensic/raw) fails loud. - unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "admin=alice@x.com") }; - assert_eq!( - OidcConfig::from_env(), - Err(ConfigError::UnsupportedTierGrant("admin".into())) - ); - - // Malformed syntax (no `=`) fails loud. - unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw-alice@x.com") }; - assert!(matches!( - OidcConfig::from_env(), - Err(ConfigError::MalformedTierGrants(_)) - )); - - // An entry with no identifiers fails loud. - unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw=") }; - assert!(matches!( - OidcConfig::from_env(), - Err(ConfigError::MalformedTierGrants(_)) - )); - - // A stray `;` (empty entry) fails loud rather than being silently skipped. - unsafe { - std::env::set_var( - super::ENV_TIER_GRANTS, - "raw=alice@x.com;;forensic=bob@x.com", - ) - }; - assert!(matches!( - OidcConfig::from_env(), - Err(ConfigError::MalformedTierGrants(_)) - )); - - clear(); -} - // --------------------------------------------------------------------------------------------- // HTTP JWKS fetch: response-body size cap over a loopback OIDC server (no external egress). The // loopback server helper lives in `super::test_support::spawn_oidc_server`. diff --git a/engine/src/engine/dashboard/auth/tier_grants_tests.rs b/engine/src/engine/dashboard/auth/tier_grants_tests.rs new file mode 100644 index 0000000..e014b7c --- /dev/null +++ b/engine/src/engine/dashboard/auth/tier_grants_tests.rs @@ -0,0 +1,406 @@ +//! Unit tests for `PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS` (JEF-501): identity→tier grants that +//! resolve the ceiling from a VERIFIED `sub`/`email` when the IdP mints no `tier` claim at all — +//! the Cloudflare-Access-over-GitHub case, verified live, that motivated this ticket. Split out of +//! `tests.rs` to keep both files well under the repo's 1,000-line cap (CLAUDE.md). +//! +//! Also covers the two HIGH findings from the PR #268 security review: an email-typed grant +//! requires `email_verified: true` (a signature only proves the IdP minted the token, never that +//! the subject owns a self-asserted `email`), and a grant identifier is typed (email vs sub) at +//! parse time so it can only ever match its own field — no cross-field collision. + +use std::sync::Arc; + +use serde_json::json; + +use super::claims::{Claims, Tier, TierGrants}; +use super::test_support::{ + AUDIENCE, ISSUER, KEY_A_N, KEY_A_PEM, KID_A, TestFetcher, base_claims, jwk_set, sign, + test_config, +}; +use super::{ConfigError, OidcConfig, Verifier}; + +// --------------------------------------------------------------------------------------------- +// Acceptance: identity→tier grants resolve the ceiling when the IdP mints no `tier` claim. +// --------------------------------------------------------------------------------------------- + +/// A grants table with `raw` for one identity-pair and `forensic` for another, used across the +/// claim-level unit tests below. +fn sample_grants() -> TierGrants { + let mut grants = TierGrants::default(); + grants.grant( + Tier::Raw, + ["thejefflarson@gmail.com".to_string(), "raw-sub".to_string()], + ); + grants.grant( + Tier::Forensic, + ["alice@x.com".to_string(), "forensic-sub".to_string()], + ); + grants +} + +#[test] +fn grant_resolves_the_ceiling_by_verified_email_or_sub_when_no_tier_claim_is_present() { + let grants = sample_grants(); + // No `tier` claim at all — the case this ticket exists for. A VERIFIED email matches. + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "email_verified": true, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Raw + ); + + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "alice@x.com", + "email_verified": true, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Forensic, + "granted forensic, never widened to raw" + ); + + // Not in either list — stays at the floor. + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "nobody@x.com", + "email_verified": true, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted + ); +} + +// --------------------------------------------------------------------------------------------- +// HIGH fix 1 (security review, PR #268): an email grant requires `email_verified: true`. A +// signature only proves the IdP MINTED the token, not that the subject OWNS the `email` it +// carries — a provider that self-asserts `email` (social login, a self-service directory) would +// otherwise let an attacker set their account email to a granted operator's address and be +// elevated. `sub` grants are unaffected (a `sub` is IdP-assigned, never self-asserted). +// --------------------------------------------------------------------------------------------- + +#[test] +fn an_unverified_email_equal_to_a_grant_does_not_match_a_verified_email_does() { + let grants = sample_grants(); + + // `email_verified` absent ⇒ false (the safe default) ⇒ the grant does NOT apply. + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted, + "an unverified email must NOT elevate, even if it equals a raw grant" + ); + + // `email_verified: false` explicitly ⇒ same denial. + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "email_verified": false, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted + ); + + // `email_verified: true` ⇒ the SAME email now matches. + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "email_verified": true, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Raw + ); +} + +// --------------------------------------------------------------------------------------------- +// HIGH fix 2 (security review, PR #268): a grant identifier is TYPED (email vs sub) at parse +// time and matches ONLY its own field — no cross-field OR. Without this, an email-shaped grant +// would also match a token whose opaque `sub` happened to equal that string, and vice versa, +// silently widening the granted set past what the operator configured. +// --------------------------------------------------------------------------------------------- + +#[test] +fn a_sub_equal_to_a_granted_email_string_does_not_match_no_cross_field() { + let grants = sample_grants(); + // `sub` == the exact string of the `raw` email grant, but a `sub` is only ever compared + // against sub-typed grants — an email-typed grant never considers `sub` at all. + let claims: Claims = + serde_json::from_value(json!({ "sub": "thejefflarson@gmail.com" })).unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted, + "a sub matching an email-typed grant string must not match — no cross-field comparison" + ); +} + +#[test] +fn an_email_equal_to_a_granted_sub_string_does_not_match_no_cross_field() { + let grants = sample_grants(); + // `email` (verified) == the exact string of the `raw` sub grant ("raw-sub"), but a sub-typed + // grant never considers `email` at all. + let claims: Claims = serde_json::from_value(json!({ + "sub": "irrelevant", + "email": "raw-sub", + "email_verified": true, + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Redacted, + "an email matching a sub-typed grant string must not match — no cross-field comparison" + ); +} + +#[test] +fn a_sub_typed_grant_matches_only_sub_exactly() { + let grants = sample_grants(); + assert_eq!(grants.resolve("raw-sub", None, false), Tier::Raw); + assert_eq!( + grants.resolve("RAW-SUB", None, false), + Tier::Redacted, + "sub match is exact — a case-differing sub is inert" + ); +} + +#[test] +fn an_explicit_recognized_tier_claim_wins_over_a_grant() { + // The identity has a `raw` grant, but the IdP's own token asserts `forensic` — the explicit + // claim wins ("the IdP's explicit statement wins"), so the ceiling is forensic, not raw. + let grants = sample_grants(); + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "email_verified": true, + "tier": "forensic", + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Forensic, + "an explicit recognized claim takes precedence over a grant" + ); +} + +#[test] +fn an_unrecognized_tier_claim_falls_through_to_the_grant_not_the_floor() { + // A garbage claim value (not merely absent) must not shadow a legitimate grant: it is not an + // "explicit recognized statement", so resolution falls through to the grant lookup. + let grants = sample_grants(); + let claims: Claims = serde_json::from_value(json!({ + "sub": "someone", + "email": "thejefflarson@gmail.com", + "email_verified": true, + "tier": "superuser", + })) + .unwrap(); + assert_eq!( + Tier::from_claims_with_grants(&claims, "tier", &grants), + Tier::Raw + ); +} + +#[test] +fn grant_email_match_is_case_insensitive_when_verified() { + let grants = sample_grants(); + assert_eq!( + grants.resolve("irrelevant", Some("THEJEFFLARSON@GMAIL.COM"), true), + Tier::Raw + ); + // A grant entry matching neither field is inert. + assert_eq!( + grants.resolve("someone-else", Some("someone-else@x.com"), true), + Tier::Redacted + ); +} + +#[test] +fn missing_or_empty_email_still_resolves_a_sub_based_grant() { + let grants = sample_grants(); + assert_eq!(grants.resolve("forensic-sub", None, false), Tier::Forensic); + assert_eq!( + grants.resolve("forensic-sub", Some(""), false), + Tier::Forensic + ); +} + +#[tokio::test] +async fn verifier_end_to_end_resolves_raw_and_forensic_grants_from_a_signed_token() { + // Mints real signed tokens (no `tier` claim) through the JEF-485 scaffolding and drives the + // FULL verify() path — proving the grant wiring works end to end, not just at the claims layer. + let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); + let config = OidcConfig { + tier_grants: sample_grants(), + ..test_config() + }; + let verifier = Verifier::with_fetcher(config, fetcher); + + // Raw grant, matched by a VERIFIED email; token carries no `tier` claim. + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["email"] = json!("thejefflarson@gmail.com"); + claims["email_verified"] = json!(true); + let token = sign(KEY_A_PEM, KID_A, &claims); + let identity = verifier.verify(&token).await.expect("token verifies"); + assert_eq!(identity.tier, Tier::Raw); + assert_eq!(identity.email.as_deref(), Some("thejefflarson@gmail.com")); + + // The SAME email, UNVERIFIED, must NOT elevate — the end-to-end HIGH fix 1 close. + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["email"] = json!("thejefflarson@gmail.com"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Redacted); + + // Forensic grant, matched by sub, unlisted identity stays redacted. + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["sub"] = json!("forensic-sub"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); + + let mut claims = base_claims(); + claims.as_object_mut().unwrap().remove("tier"); + claims["sub"] = json!("nobody-in-particular"); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Redacted); +} + +#[tokio::test] +async fn verifier_end_to_end_explicit_claim_still_wins_over_a_raw_grant() { + let fetcher = Arc::new(TestFetcher::new(jwk_set(&[(KID_A, KEY_A_N)]))); + let config = OidcConfig { + tier_grants: sample_grants(), + ..test_config() + }; + let verifier = Verifier::with_fetcher(config, fetcher); + + // This identity has a `raw` grant, but the token itself asserts `forensic` — the claim wins. + let mut claims = base_claims(); + claims["tier"] = json!("forensic"); + claims["email"] = json!("thejefflarson@gmail.com"); + claims["email_verified"] = json!(true); + let token = sign(KEY_A_PEM, KID_A, &claims); + assert_eq!(verifier.verify(&token).await.unwrap().tier, Tier::Forensic); +} + +// --------------------------------------------------------------------------------------------- +// PROTECTOR_DASHBOARD_OIDC_TIER_GRANTS: strict parsing, fail loud on malformed/unknown. +// --------------------------------------------------------------------------------------------- + +#[test] +fn tier_grants_config_parses_multiple_tiers_and_fails_loud_on_malformed_or_unknown() { + let _env = super::test_support::ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let clear = || unsafe { + for key in [ + super::ENV_ISSUER, + super::ENV_AUDIENCE, + super::ENV_TIER_GRANTS, + ] { + std::env::remove_var(key); + } + }; + clear(); + unsafe { + std::env::set_var(super::ENV_ISSUER, ISSUER); + std::env::set_var(super::ENV_AUDIENCE, AUDIENCE); + } + + // A well-formed multi-tier grant list parses and resolves as documented: a VERIFIED email is + // case-insensitive, `sub` is exact, an unlisted identity floors to Redacted. + unsafe { + std::env::set_var( + super::ENV_TIER_GRANTS, + "raw=thejefflarson@gmail.com;forensic=alice@x.com,BOB-SUB", + ); + } + let config = OidcConfig::from_env().unwrap().unwrap(); + assert_eq!( + config + .tier_grants + .resolve("someone", Some("THEJEFFLARSON@GMAIL.COM"), true), + Tier::Raw, + "a verified email matches case-insensitively" + ); + assert_eq!( + config + .tier_grants + .resolve("someone", Some("thejefflarson@gmail.com"), false), + Tier::Redacted, + "the SAME email, unverified, does not match" + ); + assert_eq!( + config + .tier_grants + .resolve("someone", Some("alice@x.com"), true), + Tier::Forensic + ); + assert_eq!( + config.tier_grants.resolve("BOB-SUB", None, false), + Tier::Forensic, + "sub match is exact and works with no email at all" + ); + assert_eq!( + config.tier_grants.resolve("bob-sub", None, false), + Tier::Redacted, + "sub match is exact — a case-differing sub is inert" + ); + assert_eq!( + config + .tier_grants + .resolve("nobody", Some("nobody@x.com"), true), + Tier::Redacted, + "an identity matching neither list stays at the floor" + ); + + // An unknown tier name (not redacted/forensic/raw) fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "admin=alice@x.com") }; + assert_eq!( + OidcConfig::from_env(), + Err(ConfigError::UnsupportedTierGrant("admin".into())) + ); + + // Malformed syntax (no `=`) fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw-alice@x.com") }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + // An entry with no identifiers fails loud. + unsafe { std::env::set_var(super::ENV_TIER_GRANTS, "raw=") }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + // A stray `;` (empty entry) fails loud rather than being silently skipped. + unsafe { + std::env::set_var( + super::ENV_TIER_GRANTS, + "raw=alice@x.com;;forensic=bob@x.com", + ) + }; + assert!(matches!( + OidcConfig::from_env(), + Err(ConfigError::MalformedTierGrants(_)) + )); + + clear(); +}