From 6e50aae0ebecaa237fa75d688169ffbc62557f2e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 22:38:02 +0000 Subject: [PATCH 1/3] feat(ogar-oidc): provider-agnostic OIDC ID-token verifier (PR-01) New crate ogar-oidc implementing ogar_auth::federation::AssertionVerifier for compact JWS ID tokens: - ES256 (p256 from the AdaWorldAPI/elliptic-curves fork; ecdsa patched to the AdaWorldAPI/signatures fork) and EdDSA (ogar-encryption). none, HS* and crit are refused; RS256 is not implemented (no RSA fork wired). - Keys are looked up only in the token issuer's own KeySet, so one issuer's key never verifies a token naming another. - iss byte-equal and trusted, sub, aud/azp, exp/iat/nbf with leeway, nonce; strength from amr; email only when email_verified. - No network I/O: Discovery::from_json and KeySet::from_jwks_json take bytes the caller fetched; the clock and nonce come in with the token. 16 integration tests; nine disable runs (key search across issuers, unknown alg defaulting to ES256, no azp rule, no trust check, exp off by one, nonce ignored, unverified email, key type not matched to alg, crit accepted) each turn their test red. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMQJzQD5KvNm8BtbsWG --- Cargo.toml | 6 + crates/ogar-oidc/Cargo.toml | 18 + crates/ogar-oidc/src/lib.rs | 567 +++++++++++++++++++++++++++++ crates/ogar-oidc/tests/verifier.rs | 523 ++++++++++++++++++++++++++ docs/AUTH-EXTERNAL-IDENTITY.md | 31 +- docs/UPSTREAM-DEPS.md | 1 + 6 files changed, 1140 insertions(+), 6 deletions(-) create mode 100644 crates/ogar-oidc/Cargo.toml create mode 100644 crates/ogar-oidc/src/lib.rs create mode 100644 crates/ogar-oidc/tests/verifier.rs diff --git a/Cargo.toml b/Cargo.toml index af6a8054..02bd4f5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ members = [ "crates/ogar-adapter-python", "crates/ogar-adapter-csharp", "crates/ogar-auth", + "crates/ogar-oidc", "crates/ogar-encryption", "crates/ogar-doc-ir", "crates/ogar-a2ui-frame", @@ -129,3 +130,8 @@ surrealdb-core = { git = "https://github.com/AdaWorldAPI/surrealdb", branch = # temporary edit, not a reason to keep the default expensive. [profile.dev] debug = 0 + +# ecdsa (pulled by p256 for ogar-oidc's ES256) resolves to the AdaWorldAPI fork +# of RustCrypto/signatures, at the same 0.17.0 version crates.io carries. +[patch.crates-io] +ecdsa = { git = "https://github.com/AdaWorldAPI/signatures", rev = "5595c3b1bfa14022b46ec3cc662a87e49b954266" } diff --git a/crates/ogar-oidc/Cargo.toml b/crates/ogar-oidc/Cargo.toml new file mode 100644 index 00000000..d94b1ba7 --- /dev/null +++ b/crates/ogar-oidc/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "ogar-oidc" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +description = "Provider-agnostic OIDC ID-token verifier for ogar-auth: implements `AssertionVerifier` over compact JWS ID tokens (ES256, EdDSA) against per-issuer JWKS the caller supplies. No network I/O, no provider-specific claims." + +[dependencies] +ogar-auth = { path = "../ogar-auth" } +# Ed25519 verification — reused from OGAR's encryption surface, never re-implemented. +ogar-encryption = { path = "../ogar-encryption" } +# ES256 (P-256 ECDSA). AdaWorldAPI fork of RustCrypto/elliptic-curves, the rev the +# ndarray `encryption` crate already pins for x448. `ecdsa` itself is redirected to +# the AdaWorldAPI/signatures fork by the workspace `[patch.crates-io]`. +p256 = { git = "https://github.com/AdaWorldAPI/elliptic-curves", rev = "739304e026fdf06cd1a31606e4db487d3f47c5ae", default-features = false, features = ["ecdsa", "alloc"] } +base64 = "0.22" +serde_json = "1" diff --git a/crates/ogar-oidc/src/lib.rs b/crates/ogar-oidc/src/lib.rs new file mode 100644 index 00000000..17a1bb42 --- /dev/null +++ b/crates/ogar-oidc/src/lib.rs @@ -0,0 +1,567 @@ +//! Provider-agnostic OIDC ID-token verifier for `ogar-auth` (PR-01). +//! +//! [`OidcVerifier`] implements [`ogar_auth::federation::AssertionVerifier`]: it +//! takes a compact JWS ID token and returns [`VerifiedClaims`] that +//! [`ogar_auth::federation::establish`] turns into a `VerifiedLogin`. Nothing in +//! here is specific to one identity provider; ZITADEL, Keycloak or Entra ID are +//! all a [`Client`] registration away. +//! +//! # What this crate does NOT do +//! +//! - **No network I/O.** The caller fetches the discovery document and the JWKS +//! (and refreshes them on key rotation) and hands the bytes in. That keeps the +//! verifier deterministic, testable and wasm-clean. +//! - **No authorization.** It establishes *who* signed in, never what they may +//! do. Roles stay with `ogar-rbac`. +//! - **No account linking.** `email` is returned as a display hint only, and +//! only when the provider marks it verified. +//! +//! # Checks, in order +//! +//! 1. The token is three base64url segments; the header `alg` is `ES256` or +//! `EdDSA` (everything else, `none` and `HS*` included, is refused); a header +//! with `crit` is refused. +//! 2. The payload `iss` is read **unverified** only to pick the [`Client`]; it +//! must byte-equal a registered issuer and be in [`TrustedIssuers`]. +//! 3. The signing key is looked up **only in that issuer's key set**, by `kid`, +//! and its type must match `alg`. A key registered for another issuer can +//! never verify a token naming this one. +//! 4. The signature is checked over `header.payload`. +//! 5. Claims: `sub` non-empty; `aud` contains the client id; with several +//! audiences `azp` is required, and `azp` when present must equal the client +//! id; `exp` required and in the future; `iat` required and not in the +//! future; `nbf` if present has passed (all with [`OidcVerifier::leeway_secs`]); +//! `nonce` equals the expected nonce when one is given. +//! +//! RS256 is not implemented yet: no AdaWorldAPI fork of an RSA crate is wired. +//! A provider must sign ID tokens with a P-256 or Ed25519 key. + +use std::collections::BTreeMap; +use std::fmt; + +use base64::Engine as _; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use ogar_auth::federation::{AssertionVerifier, TrustedIssuers, VerifiedClaims}; +use ogar_auth::identity::{ExternalIdentity, ExternalSubject, Issuer}; +use ogar_auth::user::AuthStrength; +use p256::ecdsa::signature::Verifier as _; +use serde_json::Value; + +/// One verification key from a JWKS. +#[derive(Clone)] +enum Key { + Es256(p256::ecdsa::VerifyingKey), + Ed25519([u8; ogar_encryption::sign::PUBLIC_KEY_LEN]), +} + +impl Key { + fn alg(&self) -> Alg { + match self { + Self::Es256(_) => Alg::Es256, + Self::Ed25519(_) => Alg::EdDsa, + } + } +} + +impl fmt::Debug for Key { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Es256(_) => "Key::Es256", + Self::Ed25519(_) => "Key::Ed25519", + }) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Alg { + Es256, + EdDsa, +} + +/// Why a JWKS document could not be read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum KeySetError { + /// The document is not JSON or has no `keys` array. + Malformed, + /// A supported key (EC P-256 or OKP Ed25519) has an invalid encoding. + InvalidKey, + /// Two signing keys share a `kid`. + DuplicateKid, +} + +impl fmt::Display for KeySetError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Malformed => "JWKS document is malformed", + Self::InvalidKey => "JWKS contains an invalid key", + Self::DuplicateKid => "JWKS contains two signing keys with the same kid", + }) + } +} + +impl std::error::Error for KeySetError {} + +/// The signing keys of one issuer, read from its JWKS document. +#[derive(Debug, Clone, Default)] +pub struct KeySet { + keys: BTreeMap, + unkeyed: Vec, + skipped: usize, +} + +impl KeySet { + /// Parse a JWKS document (`{"keys":[...]}`). + /// + /// EC P-256 and OKP Ed25519 signing keys are kept. Keys of other types + /// (RSA included) and keys whose `use` is not `sig` are skipped and counted + /// in [`KeySet::skipped`], so a mixed JWKS still loads. + /// + /// # Errors + /// + /// [`KeySetError`] for a malformed document, an invalid supported key, or a + /// duplicate `kid`. + pub fn from_jwks_json(json: &[u8]) -> Result { + let doc: Value = serde_json::from_slice(json).map_err(|_| KeySetError::Malformed)?; + let keys = doc + .get("keys") + .and_then(Value::as_array) + .ok_or(KeySetError::Malformed)?; + let mut set = Self::default(); + for jwk in keys { + if jwk + .get("use") + .and_then(Value::as_str) + .is_some_and(|u| u != "sig") + { + set.skipped += 1; + continue; + } + let Some(key) = parse_jwk(jwk)? else { + set.skipped += 1; + continue; + }; + match jwk.get("kid").and_then(Value::as_str) { + Some(kid) => { + if set.keys.insert(kid.to_owned(), key).is_some() { + return Err(KeySetError::DuplicateKid); + } + } + None => set.unkeyed.push(key), + } + } + Ok(set) + } + + /// Number of usable signing keys. + #[must_use] + pub fn len(&self) -> usize { + self.keys.len() + self.unkeyed.len() + } + + /// Whether no usable signing key was found. + #[must_use] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Keys present in the document but not usable here (unsupported type, or + /// not a signing key). + #[must_use] + pub fn skipped(&self) -> usize { + self.skipped + } + + /// The key a token header selects. With a `kid` only that key; without one + /// only when the set holds exactly one key. + fn select(&self, kid: Option<&str>) -> Option<&Key> { + match kid { + Some(kid) => self.keys.get(kid), + None if self.len() == 1 => self.keys.values().chain(&self.unkeyed).next(), + None => None, + } + } +} + +fn parse_jwk(jwk: &Value) -> Result, KeySetError> { + let field = |name: &str| jwk.get(name).and_then(Value::as_str); + let alg = field("alg"); + match (field("kty"), field("crv")) { + (Some("EC"), Some("P-256")) => { + if alg.is_some_and(|a| a != "ES256") { + return Ok(None); + } + let x = b64_fixed::<32>(field("x")).ok_or(KeySetError::InvalidKey)?; + let y = b64_fixed::<32>(field("y")).ok_or(KeySetError::InvalidKey)?; + let mut sec1 = [0u8; 65]; + sec1[0] = 0x04; + sec1[1..33].copy_from_slice(&x); + sec1[33..].copy_from_slice(&y); + let key = p256::ecdsa::VerifyingKey::from_sec1_bytes(&sec1) + .map_err(|_| KeySetError::InvalidKey)?; + Ok(Some(Key::Es256(key))) + } + (Some("OKP"), Some("Ed25519")) => { + if alg.is_some_and(|a| a != "EdDSA") { + return Ok(None); + } + let x = b64_fixed::<32>(field("x")).ok_or(KeySetError::InvalidKey)?; + Ok(Some(Key::Ed25519(x))) + } + _ => Ok(None), + } +} + +fn b64_fixed(s: Option<&str>) -> Option<[u8; N]> { + URL_SAFE_NO_PAD.decode(s?).ok()?.try_into().ok() +} + +/// An OIDC client registration: the issuer, this application's client id at +/// that issuer, and the issuer's current signing keys. +#[derive(Debug, Clone)] +pub struct Client { + issuer: Issuer, + client_id: String, + keys: KeySet, +} + +impl Client { + /// Register `client_id` at `issuer` with its JWKS. + #[must_use] + pub fn new(issuer: Issuer, client_id: impl Into, keys: KeySet) -> Self { + Self { + issuer, + client_id: client_id.into(), + keys, + } + } + + /// The issuer this registration belongs to. + #[must_use] + pub fn issuer(&self) -> &Issuer { + &self.issuer + } + + /// Replace the key set (after the caller re-fetched the JWKS on rotation). + pub fn set_keys(&mut self, keys: KeySet) { + self.keys = keys; + } +} + +/// The issuer and JWKS location read from an OIDC discovery document. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Discovery { + /// The `issuer` the document states; checked byte-equal to the expected one. + pub issuer: Issuer, + /// Where the caller fetches the JWKS from. + pub jwks_uri: String, +} + +/// Why a discovery document was refused. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DiscoveryError { + /// Not JSON, or `issuer` / `jwks_uri` missing. + Malformed, + /// The document names a different issuer than the one it was fetched for. + IssuerMismatch, + /// `jwks_uri` is not https. + InsecureJwksUri, +} + +impl fmt::Display for DiscoveryError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Malformed => "discovery document is malformed", + Self::IssuerMismatch => "discovery document names a different issuer", + Self::InsecureJwksUri => "discovery jwks_uri is not https", + }) + } +} + +impl std::error::Error for DiscoveryError {} + +impl Discovery { + /// Read `/.well-known/openid-configuration` fetched for `expected`. + /// + /// # Errors + /// + /// [`DiscoveryError`] when the document is malformed, its `issuer` is not + /// byte-equal to `expected` (OIDC Discovery §4.3), or `jwks_uri` is not https. + pub fn from_json(expected: &Issuer, json: &[u8]) -> Result { + let doc: Value = serde_json::from_slice(json).map_err(|_| DiscoveryError::Malformed)?; + let issuer = doc + .get("issuer") + .and_then(Value::as_str) + .ok_or(DiscoveryError::Malformed)?; + if issuer != expected.as_str() { + return Err(DiscoveryError::IssuerMismatch); + } + let jwks_uri = doc + .get("jwks_uri") + .and_then(Value::as_str) + .ok_or(DiscoveryError::Malformed)?; + if !jwks_uri.starts_with("https://") { + return Err(DiscoveryError::InsecureJwksUri); + } + Ok(Self { + issuer: expected.clone(), + jwks_uri: jwks_uri.to_owned(), + }) + } +} + +/// The input to one verification: the raw ID token, the current time and the +/// nonce the login request sent (if any). +#[derive(Debug, Clone)] +pub struct IdToken { + /// The compact JWS (`header.payload.signature`). + pub token: String, + /// Seconds since the Unix epoch, supplied by the caller. + pub now_unix: u64, + /// The nonce this login sent; when set, the token must carry the same one. + pub nonce: Option, +} + +/// Why an ID token was refused. `Display` never includes token contents. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OidcError { + /// Not three base64url segments, or a segment is not JSON. + Malformed, + /// The header `alg` is not ES256 / EdDSA, or the header carries `crit`. + UnsupportedAlgorithm, + /// No [`Client`] is registered for the token's `iss`. + UnknownIssuer, + /// The issuer has a client but is not in [`TrustedIssuers`]. + UntrustedIssuer, + /// No key of this issuer matches the header `kid` and `alg`. + UnknownKey, + /// The signature does not verify. + BadSignature, + /// A required claim is missing or has the wrong type. + MissingClaim, + /// `aud` / `azp` do not name this client. + AudienceMismatch, + /// `exp` has passed. + Expired, + /// `nbf` or `iat` lies in the future. + NotYetValid, + /// The nonce does not match the one the login sent. + NonceMismatch, +} + +impl fmt::Display for OidcError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Malformed => "ID token is malformed", + Self::UnsupportedAlgorithm => "ID token uses an unsupported algorithm", + Self::UnknownIssuer => "ID token issuer has no client registration", + Self::UntrustedIssuer => "ID token issuer is not trusted", + Self::UnknownKey => "no signing key of this issuer matches the ID token", + Self::BadSignature => "ID token signature does not verify", + Self::MissingClaim => "ID token lacks a required claim", + Self::AudienceMismatch => "ID token is not addressed to this client", + Self::Expired => "ID token has expired", + Self::NotYetValid => "ID token is not valid yet", + Self::NonceMismatch => "ID token nonce does not match", + }) + } +} + +impl std::error::Error for OidcError {} + +/// The OIDC [`AssertionVerifier`]: one [`Client`] per issuer. +#[derive(Debug, Clone)] +pub struct OidcVerifier { + clients: BTreeMap, + /// Clock skew tolerated on `exp`, `nbf` and `iat`, in seconds. + pub leeway_secs: u64, +} + +impl Default for OidcVerifier { + fn default() -> Self { + Self { + clients: BTreeMap::new(), + leeway_secs: 60, + } + } +} + +impl OidcVerifier { + /// An empty verifier with 60 s leeway. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Register a client, replacing an earlier registration for the same issuer. + pub fn add_client(&mut self, client: Client) { + self.clients.insert(client.issuer.clone(), client); + } + + /// The registration for `issuer`, for key rotation. + pub fn client_mut(&mut self, issuer: &Issuer) -> Option<&mut Client> { + self.clients.get_mut(issuer) + } +} + +fn decode_json(segment: &str) -> Result { + let bytes = URL_SAFE_NO_PAD + .decode(segment) + .map_err(|_| OidcError::Malformed)?; + let value: Value = serde_json::from_slice(&bytes).map_err(|_| OidcError::Malformed)?; + if value.is_object() { + Ok(value) + } else { + Err(OidcError::Malformed) + } +} + +fn verify_signature(key: &Key, message: &[u8], signature: &[u8]) -> bool { + match key { + Key::Es256(vk) => p256::ecdsa::Signature::from_slice(signature) + .is_ok_and(|sig| vk.verify(message, &sig).is_ok()), + Key::Ed25519(pk) => <&[u8; ogar_encryption::sign::SIGNATURE_LEN]>::try_from(signature) + .is_ok_and(|sig| ogar_encryption::sign::verify(pk, message, sig)), + } +} + +/// `MultiFactor` when the provider says so (`amr` contains `mfa`) or lists at +/// least two distinct recognised factors; otherwise `SingleFactor`. +fn strength(claims: &Value) -> AuthStrength { + const FACTORS: [&str; 10] = [ + "pwd", "otp", "hwk", "swk", "user", "pop", "sms", "tel", "fpt", "face", + ]; + let Some(amr) = claims.get("amr").and_then(Value::as_array) else { + return AuthStrength::SingleFactor; + }; + let values: Vec<&str> = amr.iter().filter_map(Value::as_str).collect(); + if values.contains(&"mfa") { + return AuthStrength::MultiFactor; + } + let distinct = FACTORS.iter().filter(|f| values.contains(f)).count(); + if distinct >= 2 { + AuthStrength::MultiFactor + } else { + AuthStrength::SingleFactor + } +} + +fn check_audience(claims: &Value, client_id: &str) -> Result<(), OidcError> { + let aud: Vec<&str> = match claims.get("aud") { + Some(Value::String(s)) => vec![s.as_str()], + Some(Value::Array(a)) => a.iter().filter_map(Value::as_str).collect(), + _ => return Err(OidcError::MissingClaim), + }; + if !aud.contains(&client_id) { + return Err(OidcError::AudienceMismatch); + } + match claims.get("azp") { + Some(Value::String(azp)) if azp == client_id => Ok(()), + Some(_) => Err(OidcError::AudienceMismatch), + None if aud.len() > 1 => Err(OidcError::AudienceMismatch), + None => Ok(()), + } +} + +fn check_time(claims: &Value, now: u64, leeway: u64) -> Result<(), OidcError> { + let num = |name: &str| { + claims + .get(name) + .map(|v| v.as_u64().ok_or(OidcError::MissingClaim)) + }; + let exp = num("exp").ok_or(OidcError::MissingClaim)??; + if now >= exp.saturating_add(leeway) { + return Err(OidcError::Expired); + } + let iat = num("iat").ok_or(OidcError::MissingClaim)??; + if iat > now.saturating_add(leeway) { + return Err(OidcError::NotYetValid); + } + if let Some(nbf) = num("nbf").transpose()? + && nbf > now.saturating_add(leeway) + { + return Err(OidcError::NotYetValid); + } + Ok(()) +} + +impl AssertionVerifier for OidcVerifier { + type Input = IdToken; + type Error = OidcError; + const ID: &'static str = "ogar-oidc/id-token"; + + fn verify( + &self, + trusted: &TrustedIssuers, + input: &IdToken, + ) -> Result { + let mut parts = input.token.split('.'); + let (Some(h), Some(p), Some(s), None) = + (parts.next(), parts.next(), parts.next(), parts.next()) + else { + return Err(OidcError::Malformed); + }; + let header = decode_json(h)?; + if header.get("crit").is_some() { + return Err(OidcError::UnsupportedAlgorithm); + } + let alg = match header.get("alg").and_then(Value::as_str) { + Some("ES256") => Alg::Es256, + Some("EdDSA") => Alg::EdDsa, + _ => return Err(OidcError::UnsupportedAlgorithm), + }; + + // Unverified read, used only to choose whose keys to try. + let claims = decode_json(p)?; + let iss = claims + .get("iss") + .and_then(Value::as_str) + .ok_or(OidcError::MissingClaim)?; + let issuer = Issuer::new(iss).map_err(|_| OidcError::UnknownIssuer)?; + let client = self.clients.get(&issuer).ok_or(OidcError::UnknownIssuer)?; + if trusted.provider_of(&issuer).is_none() { + return Err(OidcError::UntrustedIssuer); + } + + let kid = header.get("kid").and_then(Value::as_str); + let key = client + .keys + .select(kid) + .filter(|k| k.alg() == alg) + .ok_or(OidcError::UnknownKey)?; + let signature = URL_SAFE_NO_PAD + .decode(s) + .map_err(|_| OidcError::Malformed)?; + let signed = &input.token[..h.len() + 1 + p.len()]; + if !verify_signature(key, signed.as_bytes(), &signature) { + return Err(OidcError::BadSignature); + } + + // From here the claims are authenticated. + let sub = claims + .get("sub") + .and_then(Value::as_str) + .ok_or(OidcError::MissingClaim)?; + let subject = ExternalSubject::new(sub).map_err(|_| OidcError::MissingClaim)?; + check_audience(&claims, &client.client_id)?; + check_time(&claims, input.now_unix, self.leeway_secs)?; + if let Some(expected) = &input.nonce + && claims.get("nonce").and_then(Value::as_str) != Some(expected.as_str()) + { + return Err(OidcError::NonceMismatch); + } + let email = (claims.get("email_verified").and_then(Value::as_bool) == Some(true)) + .then(|| { + claims + .get("email") + .and_then(Value::as_str) + .map(str::to_owned) + }) + .flatten(); + + Ok(VerifiedClaims { + identity: ExternalIdentity::new(issuer, subject), + strength: strength(&claims), + email, + }) + } +} diff --git a/crates/ogar-oidc/tests/verifier.rs b/crates/ogar-oidc/tests/verifier.rs new file mode 100644 index 00000000..93fc48a6 --- /dev/null +++ b/crates/ogar-oidc/tests/verifier.rs @@ -0,0 +1,523 @@ +//! Integration tests for `ogar-oidc`: the public surface only. Tokens are +//! signed in-test with fixed keys; every failure asserts the exact variant. + +use base64::Engine as _; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use ogar_auth::federation::{AssertionVerifier, LoginError, TrustedIssuers, establish}; +use ogar_auth::identity::{ExternalIdentity, ExternalSubject, Issuer}; +use ogar_auth::user::{AuthStrength, KeyRef, LocalUserStore, ProviderId, User, UserId}; +use ogar_encryption::sign::Keypair; +use ogar_oidc::{ + Client, Discovery, DiscoveryError, IdToken, KeySet, KeySetError, OidcError, OidcVerifier, +}; +use p256::ecdsa::SigningKey; +use p256::ecdsa::signature::Signer; +use serde_json::{Value, json}; + +const NOW: u64 = 1_800_000_000; +const ISS_A: &str = "https://idp-a.example"; +const ISS_B: &str = "https://idp-b.example"; +const CLIENT: &str = "client-a"; +const PROV_A: ProviderId = ProviderId("idp-a"); +const PROV_B: ProviderId = ProviderId("idp-b"); + +// ── helpers ───────────────────────────────────────────────────────────────── + +fn b64(bytes: &[u8]) -> String { + URL_SAFE_NO_PAD.encode(bytes) +} + +fn es_key(seed_byte: u8) -> SigningKey { + SigningKey::from_slice(&[seed_byte; 32]).expect("valid scalar") +} + +fn jwk_es256(sk: &SigningKey, kid: &str) -> Value { + // Uncompressed SEC1: 0x04 || x(32) || y(32). + let sec1 = sk.verifying_key().to_sec1_bytes(); + assert_eq!(sec1.len(), 65); + json!({ + "kty": "EC", + "crv": "P-256", + "kid": kid, + "use": "sig", + "x": b64(&sec1[1..33]), + "y": b64(&sec1[33..]), + }) +} + +fn jwk_ed25519(kp: &Keypair, kid: &str) -> Value { + json!({ + "kty": "OKP", + "crv": "Ed25519", + "kid": kid, + "use": "sig", + "x": b64(&kp.public_key()), + }) +} + +fn without(mut jwk: Value, field: &str) -> Value { + jwk.as_object_mut().expect("object").remove(field); + jwk +} + +fn keyset(jwks: Vec) -> KeySet { + let doc = json!({ "keys": jwks }); + KeySet::from_jwks_json(doc.to_string().as_bytes()).expect("keyset") +} + +fn signing_input(header: &Value, claims: &Value) -> String { + format!( + "{}.{}", + b64(header.to_string().as_bytes()), + b64(claims.to_string().as_bytes()) + ) +} + +fn sign_es256(sk: &SigningKey, header: Value, claims: Value) -> String { + let input = signing_input(&header, &claims); + let sig: p256::ecdsa::Signature = sk.sign(input.as_bytes()); + format!("{input}.{}", b64(&sig.to_bytes())) +} + +fn sign_ed25519(kp: &Keypair, header: Value, claims: Value) -> String { + let input = signing_input(&header, &claims); + let sig = kp.sign(input.as_bytes()); + format!("{input}.{}", b64(&sig)) +} + +fn es_header(kid: &str) -> Value { + json!({ "alg": "ES256", "typ": "JWT", "kid": kid }) +} + +fn base_claims(iss: &str) -> Value { + json!({ + "iss": iss, + "sub": "user-123", + "aud": CLIENT, + "iat": NOW, + "exp": NOW + 300, + }) +} + +fn drop_claim(mut claims: Value, name: &str) -> Value { + claims.as_object_mut().expect("object").remove(name); + claims +} + +fn issuer(s: &str) -> Issuer { + Issuer::new(s).expect("issuer") +} + +fn trusted_both() -> TrustedIssuers { + let mut t = TrustedIssuers::new(); + t.insert(issuer(ISS_A), PROV_A).unwrap(); + t.insert(issuer(ISS_B), PROV_B).unwrap(); + t +} + +fn verifier_a(keys: KeySet) -> OidcVerifier { + let mut v = OidcVerifier::new(); + v.add_client(Client::new(issuer(ISS_A), CLIENT, keys)); + v +} + +fn input(token: String) -> IdToken { + IdToken { + token, + now_unix: NOW, + nonce: None, + } +} + +fn run( + v: &OidcVerifier, + token: String, +) -> Result { + v.verify(&trusted_both(), &input(token)) +} + +/// ES256 happy-path scaffold: key "a1", verifier for idp-a, token over `claims`. +fn es_run(claims: Value) -> Result { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + run(&v, sign_es256(&sk, es_header("a1"), claims)) +} + +fn user(id: u64, subject: &str) -> User { + User { + id: UserId(id), + subject: subject.to_string(), + tenant: 7, + roles: vec!["physician".to_string()], + memberships: vec![], + bindings: vec![], + key_refs: vec![KeyRef(format!("kms://user/{id}"))], + } +} + +fn ident(iss: &str, sub: &str) -> ExternalIdentity { + ExternalIdentity::new(issuer(iss), ExternalSubject::new(sub).unwrap()) +} + +// ── 1-2: valid tokens ─────────────────────────────────────────────────────── + +#[test] +fn valid_es256_token_is_accepted() { + let claims = es_run(base_claims(ISS_A)).expect("valid token"); + assert_eq!(claims.identity, ident(ISS_A, "user-123")); + assert_eq!(claims.strength, AuthStrength::SingleFactor); + assert_eq!(claims.email, None); +} + +#[test] +fn valid_eddsa_token_is_accepted() { + let kp = Keypair::from_seed(&[9u8; 32]); + let v = verifier_a(keyset(vec![jwk_ed25519(&kp, "e1")])); + let header = json!({ "alg": "EdDSA", "kid": "e1" }); + let claims = run(&v, sign_ed25519(&kp, header, base_claims(ISS_A))).expect("valid token"); + assert_eq!(claims.identity, ident(ISS_A, "user-123")); +} + +// ── 3: end-to-end through establish ───────────────────────────────────────── + +#[test] +fn establish_end_to_end_bound_and_unbound() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + let token = sign_es256(&sk, es_header("a1"), base_claims(ISS_A)); + let inp = input(token); + + let mut store = LocalUserStore::new(); + store.insert(user(1, "alice")); + store + .bind_external(UserId(1), ident(ISS_A, "user-123")) + .unwrap(); + let login = establish(&v, &trusted_both(), &inp, &store).expect("login"); + assert_eq!(login.identity(), &ident(ISS_A, "user-123")); + assert_eq!(login.authenticated_user().user.id, UserId(1)); + assert_eq!(login.authenticated_user().auth.provider, PROV_A); + + let mut empty = LocalUserStore::new(); + empty.insert(user(1, "alice")); + let r = establish(&v, &trusted_both(), &inp, &empty); + assert!(matches!(r, Err(LoginError::Unbound)), "got {r:?}"); +} + +// ── 4: algorithm confusion ────────────────────────────────────────────────── + +#[test] +fn alg_none_hs256_and_crit_are_unsupported() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + let claims = base_claims(ISS_A); + + let none_header = json!({ "alg": "none", "kid": "a1" }); + let unsigned = format!("{}.", signing_input(&none_header, &claims)); + assert_eq!(run(&v, unsigned), Err(OidcError::UnsupportedAlgorithm)); + + let hs_header = json!({ "alg": "HS256", "kid": "a1" }); + let hs = format!("{}.{}", signing_input(&hs_header, &claims), b64(&[1u8; 32])); + assert_eq!(run(&v, hs), Err(OidcError::UnsupportedAlgorithm)); + + let crit_header = json!({ "alg": "ES256", "kid": "a1", "crit": ["exp"] }); + let crit = sign_es256(&sk, crit_header, claims); + assert_eq!(run(&v, crit), Err(OidcError::UnsupportedAlgorithm)); +} + +// ── 5: cross-issuer key confusion ─────────────────────────────────────────── + +#[test] +fn key_of_one_issuer_never_verifies_another() { + let key_a = es_key(7); + let key_b = es_key(8); + let mut v = OidcVerifier::new(); + v.add_client(Client::new( + issuer(ISS_A), + CLIENT, + keyset(vec![jwk_es256(&key_a, "a1")]), + )); + v.add_client(Client::new( + issuer(ISS_B), + CLIENT, + keyset(vec![jwk_es256(&key_b, "b1")]), + )); + + // Signed by A, kid of A, but naming issuer B: B has no kid "a1". + let token = sign_es256(&key_a, es_header("a1"), base_claims(ISS_B)); + assert_eq!(run(&v, token), Err(OidcError::UnknownKey)); + + // Same signer, kid of B: B's key is found but the signature is A's. + let token = sign_es256(&key_a, es_header("b1"), base_claims(ISS_B)); + assert_eq!(run(&v, token), Err(OidcError::BadSignature)); + + // Control: B signing for B verifies. + let token = sign_es256(&key_b, es_header("b1"), base_claims(ISS_B)); + assert!(run(&v, token).is_ok()); +} + +// ── 6: tampering ──────────────────────────────────────────────────────────── + +#[test] +fn tampered_payload_fails_signature() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + let token = sign_es256(&sk, es_header("a1"), base_claims(ISS_A)); + let mut parts: Vec<&str> = token.split('.').collect(); + let mut forged = base_claims(ISS_A); + forged["sub"] = json!("attacker"); + let forged_payload = b64(forged.to_string().as_bytes()); + parts[1] = &forged_payload; + assert_eq!(run(&v, parts.join(".")), Err(OidcError::BadSignature)); +} + +// ── 7: issuer matching and trust ──────────────────────────────────────────── + +#[test] +fn issuer_is_byte_exact_and_must_be_trusted() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + + let slash = sign_es256(&sk, es_header("a1"), base_claims("https://idp-a.example/")); + assert_eq!(run(&v, slash), Err(OidcError::UnknownIssuer)); + + let mut only_b = TrustedIssuers::new(); + only_b.insert(issuer(ISS_B), PROV_B).unwrap(); + let token = sign_es256(&sk, es_header("a1"), base_claims(ISS_A)); + assert_eq!( + v.verify(&only_b, &input(token)), + Err(OidcError::UntrustedIssuer) + ); +} + +// ── 8: time ───────────────────────────────────────────────────────────────── + +#[test] +fn time_claims_and_leeway_boundaries() { + let with = |name: &str, value: u64| { + let mut c = base_claims(ISS_A); + c[name] = json!(value); + c + }; + // leeway is 60: exp + 60 <= now is expired. + assert_eq!(es_run(with("exp", NOW - 61)), Err(OidcError::Expired)); + assert!(es_run(with("exp", NOW - 59)).is_ok()); + assert_eq!(es_run(with("iat", NOW + 61)), Err(OidcError::NotYetValid)); + assert!(es_run(with("iat", NOW + 59)).is_ok()); + assert_eq!(es_run(with("nbf", NOW + 61)), Err(OidcError::NotYetValid)); + assert!(es_run(with("nbf", NOW + 59)).is_ok()); + assert_eq!( + es_run(drop_claim(base_claims(ISS_A), "exp")), + Err(OidcError::MissingClaim) + ); +} + +// ── 9: audience ───────────────────────────────────────────────────────────── + +#[test] +fn audience_and_authorized_party_rules() { + let mut other = base_claims(ISS_A); + other["aud"] = json!("client-b"); + assert_eq!(es_run(other), Err(OidcError::AudienceMismatch)); + + let mut multi = base_claims(ISS_A); + multi["aud"] = json!(["client-a", "client-b"]); + assert_eq!(es_run(multi.clone()), Err(OidcError::AudienceMismatch)); + multi["azp"] = json!("client-a"); + assert!(es_run(multi).is_ok()); + + let mut wrong_azp = base_claims(ISS_A); + wrong_azp["azp"] = json!("client-b"); + assert_eq!(es_run(wrong_azp), Err(OidcError::AudienceMismatch)); +} + +// ── 10: nonce ─────────────────────────────────────────────────────────────── + +#[test] +fn nonce_must_match_when_expected() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + let with_nonce = |claim: Option<&str>| { + let mut c = base_claims(ISS_A); + if let Some(n) = claim { + c["nonce"] = json!(n); + } + let token = sign_es256(&sk, es_header("a1"), c); + IdToken { + token, + now_unix: NOW, + nonce: Some("n1".to_string()), + } + }; + assert!(v.verify(&trusted_both(), &with_nonce(Some("n1"))).is_ok()); + assert_eq!( + v.verify(&trusted_both(), &with_nonce(Some("n2"))), + Err(OidcError::NonceMismatch) + ); + assert_eq!( + v.verify(&trusted_both(), &with_nonce(None)), + Err(OidcError::NonceMismatch) + ); +} + +// ── 11: strength ──────────────────────────────────────────────────────────── + +#[test] +fn amr_decides_strength() { + let with_amr = |amr: Value| { + let mut c = base_claims(ISS_A); + c["amr"] = amr; + es_run(c).expect("valid").strength + }; + assert_eq!(with_amr(json!(["pwd", "otp"])), AuthStrength::MultiFactor); + assert_eq!(with_amr(json!(["mfa"])), AuthStrength::MultiFactor); + assert_eq!(with_amr(json!(["pwd"])), AuthStrength::SingleFactor); +} + +// ── 12: email ─────────────────────────────────────────────────────────────── + +#[test] +fn email_is_returned_only_when_verified() { + let email = |verified: Option| { + let mut c = base_claims(ISS_A); + c["email"] = json!("a@b.example"); + if let Some(v) = verified { + c["email_verified"] = json!(v); + } + es_run(c).expect("valid").email + }; + assert_eq!(email(Some(true)), Some("a@b.example".to_string())); + assert_eq!(email(Some(false)), None); + assert_eq!(email(None), None); +} + +// ── 13: kid handling ──────────────────────────────────────────────────────── + +#[test] +fn kid_selection_rules() { + let k1 = es_key(7); + let k2 = es_key(8); + let no_kid_header = json!({ "alg": "ES256" }); + + // Two keys, token without kid: ambiguous. + let v = verifier_a(keyset(vec![jwk_es256(&k1, "a1"), jwk_es256(&k2, "a2")])); + let token = sign_es256(&k1, no_kid_header.clone(), base_claims(ISS_A)); + assert_eq!(run(&v, token), Err(OidcError::UnknownKey)); + + // One unkeyed key, token without kid: accepted. + let v = verifier_a(keyset(vec![without(jwk_es256(&k1, "x"), "kid")])); + let token = sign_es256(&k1, no_kid_header, base_claims(ISS_A)); + assert!(run(&v, token).is_ok()); + + // kid points at an Ed25519 key but the header says ES256. + let kp = Keypair::from_seed(&[9u8; 32]); + let v = verifier_a(keyset(vec![jwk_ed25519(&kp, "e1")])); + let token = sign_es256(&k1, es_header("e1"), base_claims(ISS_A)); + assert_eq!(run(&v, token), Err(OidcError::UnknownKey)); +} + +// ── 14: JWKS parsing ──────────────────────────────────────────────────────── + +#[test] +fn jwks_parsing_skips_and_rejects() { + let sk = es_key(7); + let ec = jwk_es256(&sk, "k1"); + let rsa = json!({ "kty": "RSA", "n": "AQAB", "e": "AQAB", "kid": "r1" }); + let parse = + |keys: Vec| KeySet::from_jwks_json(json!({ "keys": keys }).to_string().as_bytes()); + + let set = parse(vec![rsa, ec.clone()]).expect("mixed"); + assert_eq!(set.len(), 1); + assert_eq!(set.skipped(), 1); + + let mut enc = jwk_es256(&es_key(8), "k2"); + enc["use"] = json!("enc"); + let set = parse(vec![ec.clone(), enc]).expect("enc skipped"); + assert_eq!(set.len(), 1); + assert_eq!(set.skipped(), 1); + + let dup = jwk_es256(&es_key(8), "k1"); + assert!(matches!( + parse(vec![ec, dup]), + Err(KeySetError::DuplicateKid) + )); + + let zero = b64(&[0u8; 32]); + let bad_point = json!({ "kty": "EC", "crv": "P-256", "kid": "z", "x": zero, "y": zero }); + assert!(matches!( + parse(vec![bad_point]), + Err(KeySetError::InvalidKey) + )); + + assert!(matches!( + KeySet::from_jwks_json(b"{}"), + Err(KeySetError::Malformed) + )); + assert!(matches!( + KeySet::from_jwks_json(b"not json"), + Err(KeySetError::Malformed) + )); +} + +// ── 15: discovery ─────────────────────────────────────────────────────────── + +#[test] +fn discovery_checks_issuer_and_jwks_scheme() { + let expected = issuer(ISS_A); + let doc = |iss: &str, uri: &str| { + json!({ "issuer": iss, "jwks_uri": uri }) + .to_string() + .into_bytes() + }; + + let d = Discovery::from_json(&expected, &doc(ISS_A, "https://idp-a.example/jwks")) + .expect("valid discovery"); + assert_eq!(d.issuer, expected); + assert_eq!(d.jwks_uri, "https://idp-a.example/jwks"); + + assert_eq!( + Discovery::from_json( + &expected, + &doc("https://idp-a.example/", "https://idp-a.example/jwks") + ), + Err(DiscoveryError::IssuerMismatch) + ); + assert_eq!( + Discovery::from_json(&expected, &doc(ISS_A, "http://idp-a.example/jwks")), + Err(DiscoveryError::InsecureJwksUri) + ); + assert_eq!( + Discovery::from_json(&expected, b"{}"), + Err(DiscoveryError::Malformed) + ); +} + +// ── 16: errors never leak the subject or the token ────────────────────────── + +#[test] +fn error_display_never_leaks_subject_or_token() { + let sk = es_key(7); + let v = verifier_a(keyset(vec![jwk_es256(&sk, "a1")])); + let mut bad_aud = base_claims(ISS_A); + bad_aud["aud"] = json!("client-b"); + let mut expired = base_claims(ISS_A); + expired["exp"] = json!(NOW - 100); + + let tokens = vec![ + sign_es256(&sk, es_header("a1"), bad_aud), + sign_es256(&sk, es_header("a1"), expired), + sign_es256(&sk, es_header("zz"), base_claims(ISS_A)), + sign_es256(&es_key(8), es_header("a1"), base_claims(ISS_A)), + sign_es256(&sk, es_header("a1"), base_claims("https://other.example")), + format!( + "{}.", + signing_input(&json!({ "alg": "none" }), &base_claims(ISS_A)) + ), + "garbage".to_string(), + ]; + for token in tokens { + let err = run(&v, token.clone()).expect_err("must fail"); + let shown = format!("{err} / {err:?}"); + assert!(!shown.contains("user-123"), "leaked subject: {shown}"); + assert!(!shown.contains(&token), "leaked token: {shown}"); + } +} diff --git a/docs/AUTH-EXTERNAL-IDENTITY.md b/docs/AUTH-EXTERNAL-IDENTITY.md index 5ccd5277..c52b2a22 100644 --- a/docs/AUTH-EXTERNAL-IDENTITY.md +++ b/docs/AUTH-EXTERNAL-IDENTITY.md @@ -67,9 +67,28 @@ which code may call RBAC with a hand-built user, is not enforceable by types. unscoped subject) **cannot be auto-migrated**: the issuer is not recorded in them. Re-bind explicitly with `bind_external`. -## 6. PR-01 handoff - -The next piece is a reusable OIDC adapter implementing `AssertionVerifier` -(signature, audience, expiry, JWKS) that returns `VerifiedClaims`. It must stay -provider-agnostic: provider differences are data in `TrustedIssuers`, not match -arms. +## 6. The OIDC adapter (`ogar-oidc`, PR-01) + +`ogar_oidc::OidcVerifier` implements `AssertionVerifier` for compact JWS ID +tokens. It is provider-agnostic: a provider is a `Client` (issuer, client id, +JWKS), never a match arm. + +- **No network I/O.** The caller fetches discovery and JWKS and passes the + bytes (`Discovery::from_json`, `KeySet::from_jwks_json`); key rotation is + `Client::set_keys`. The clock and the expected nonce come in with the token + (`IdToken`), so verification is deterministic. +- **Algorithms:** ES256 and EdDSA. `none`, `HS*` and a header with `crit` are + refused. RS256 is not implemented: no AdaWorldAPI fork of an RSA crate is + wired yet, so a provider must sign ID tokens with a P-256 or Ed25519 key. +- **Keys are per issuer.** The unverified `iss` only chooses which issuer's keys + to try; a key registered for one issuer never verifies a token naming + another. +- **Claims:** `iss` byte-equal and in `TrustedIssuers`; `sub` non-empty; `aud` + contains the client id; several audiences require `azp`; `azp` when present + equals the client id; `exp` and `iat` required, `nbf` optional, with a + leeway (default 60 s); `nonce` equal when the login sent one. +- **Strength:** `MultiFactor` when `amr` contains `mfa` or two distinct + recognised factors, otherwise `SingleFactor`. +- **Email** is returned only when `email_verified` is `true`, and stays a + display hint (section 3). +- Errors (`OidcError`) carry no token contents. diff --git a/docs/UPSTREAM-DEPS.md b/docs/UPSTREAM-DEPS.md index a261acd4..678476f8 100644 --- a/docs/UPSTREAM-DEPS.md +++ b/docs/UPSTREAM-DEPS.md @@ -22,6 +22,7 @@ | 6 (`lance-graph-ontology` cache) | manifest-watch event API for cache invalidation | LOCKED | | 7 (`lance-graph-callcenter`) | dataset transactional commit boundary; tag-based version retention | LOCKED | | `ogar-auth` (identity / federation) | `lance_graph_contract::auth::ActorContext` (canonical identity envelope) + `sla::TenantId`; git dep, branch `main`, unpinned. Measured: OGAR base `5b82ae0`; resolved lance-graph-contract at `f859d484c46dda9725ace2dc870c73e95203edde` | CURRENT-CONTRACT (unpinned `main`; re-measure the resolved rev when it matters) | +| `ogar-oidc` (OIDC ID-token verifier) | `p256` from AdaWorldAPI/elliptic-curves rev `739304e026fdf06cd1a31606e4db487d3f47c5ae` (the rev ndarray `encryption` pins); `ecdsa` 0.17.0 redirected by workspace `[patch.crates-io]` to AdaWorldAPI/signatures rev `5595c3b1bfa14022b46ec3cc662a87e49b954266`; Ed25519 via `ogar-encryption`. RS256: no RSA fork wired | CURRENT-CONTRACT (rev-pinned) | **Coordination notes**: - OGAR's `Identity::class_identity_versioned(prefix, class, n)` MUST produce a string that `NiblePath` can split losslessly. From 38c7b3724676fc2d94900c0cb0137809dfc50a29 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 22:42:18 +0000 Subject: [PATCH 2/3] fix(ogar-oidc): MFA needs two independent factor categories (Codex P2) amr ["hwk","user"] described one hardware-key authentication with a presence check, yet counted as two factors. Strength now requires mfa or two of knowledge / possession / inherence; user (presence) is not a factor. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMQJzQD5KvNm8BtbsWG --- crates/ogar-oidc/src/lib.rs | 21 ++++++++++++++------- crates/ogar-oidc/tests/verifier.rs | 6 ++++++ docs/AUTH-EXTERNAL-IDENTITY.md | 5 +++-- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/crates/ogar-oidc/src/lib.rs b/crates/ogar-oidc/src/lib.rs index 17a1bb42..6dc25b69 100644 --- a/crates/ogar-oidc/src/lib.rs +++ b/crates/ogar-oidc/src/lib.rs @@ -424,12 +424,15 @@ fn verify_signature(key: &Key, message: &[u8], signature: &[u8]) -> bool { } } -/// `MultiFactor` when the provider says so (`amr` contains `mfa`) or lists at -/// least two distinct recognised factors; otherwise `SingleFactor`. +/// `MultiFactor` when the provider says so (`amr` contains `mfa`) or when the +/// `amr` values (RFC 8176) cover at least two independent factor categories: +/// knowledge, possession, inherence. Several methods of one category (e.g. +/// `otp` + `sms`) are one factor, and `user` (presence) or other method labels +/// that are not a factor count for nothing. Otherwise `SingleFactor`. fn strength(claims: &Value) -> AuthStrength { - const FACTORS: [&str; 10] = [ - "pwd", "otp", "hwk", "swk", "user", "pop", "sms", "tel", "fpt", "face", - ]; + const KNOWLEDGE: [&str; 3] = ["pwd", "pin", "kba"]; + const POSSESSION: [&str; 7] = ["hwk", "swk", "otp", "sms", "tel", "pop", "sc"]; + const INHERENCE: [&str; 5] = ["fpt", "face", "iris", "retina", "vbm"]; let Some(amr) = claims.get("amr").and_then(Value::as_array) else { return AuthStrength::SingleFactor; }; @@ -437,8 +440,12 @@ fn strength(claims: &Value) -> AuthStrength { if values.contains(&"mfa") { return AuthStrength::MultiFactor; } - let distinct = FACTORS.iter().filter(|f| values.contains(f)).count(); - if distinct >= 2 { + let covers = |category: &[&str]| category.iter().any(|m| values.contains(m)); + let categories = [&KNOWLEDGE[..], &POSSESSION[..], &INHERENCE[..]] + .into_iter() + .filter(|c| covers(c)) + .count(); + if categories >= 2 { AuthStrength::MultiFactor } else { AuthStrength::SingleFactor diff --git a/crates/ogar-oidc/tests/verifier.rs b/crates/ogar-oidc/tests/verifier.rs index 93fc48a6..f578f051 100644 --- a/crates/ogar-oidc/tests/verifier.rs +++ b/crates/ogar-oidc/tests/verifier.rs @@ -371,6 +371,12 @@ fn amr_decides_strength() { assert_eq!(with_amr(json!(["pwd", "otp"])), AuthStrength::MultiFactor); assert_eq!(with_amr(json!(["mfa"])), AuthStrength::MultiFactor); assert_eq!(with_amr(json!(["pwd"])), AuthStrength::SingleFactor); + // Hardware key with user presence: one factor (RFC 8176: `user` is presence). + assert_eq!(with_amr(json!(["hwk", "user"])), AuthStrength::SingleFactor); + // Two methods of one category (possession) are still one factor. + assert_eq!(with_amr(json!(["otp", "sms"])), AuthStrength::SingleFactor); + // Possession + inherence (e.g. a passkey unlocked by fingerprint). + assert_eq!(with_amr(json!(["hwk", "fpt"])), AuthStrength::MultiFactor); } // ── 12: email ─────────────────────────────────────────────────────────────── diff --git a/docs/AUTH-EXTERNAL-IDENTITY.md b/docs/AUTH-EXTERNAL-IDENTITY.md index c52b2a22..b720e77d 100644 --- a/docs/AUTH-EXTERNAL-IDENTITY.md +++ b/docs/AUTH-EXTERNAL-IDENTITY.md @@ -87,8 +87,9 @@ JWKS), never a match arm. contains the client id; several audiences require `azp`; `azp` when present equals the client id; `exp` and `iat` required, `nbf` optional, with a leeway (default 60 s); `nonce` equal when the login sent one. -- **Strength:** `MultiFactor` when `amr` contains `mfa` or two distinct - recognised factors, otherwise `SingleFactor`. +- **Strength:** `MultiFactor` when `amr` contains `mfa` or covers two + independent factor categories (knowledge, possession, inherence; RFC 8176), + otherwise `SingleFactor`. `user` (presence) is not a factor. - **Email** is returned only when `email_verified` is `true`, and stays a display hint (section 3). - Errors (`OidcError`) carry no token contents. From 539a7dbec7504135a0f2d25bb0b1757808482616 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 22:45:56 +0000 Subject: [PATCH 3/3] fix(ogar-oidc): honour JWK key_ops when importing verification keys (Codex P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A key whose key_ops is present but lacks "verify" (or is not an array) is skipped, per RFC 7517 §4.3, instead of being imported as a verification key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XAqKMQJzQD5KvNm8BtbsWG --- crates/ogar-oidc/src/lib.rs | 15 ++++++++++++++- crates/ogar-oidc/tests/verifier.rs | 13 +++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/ogar-oidc/src/lib.rs b/crates/ogar-oidc/src/lib.rs index 6dc25b69..17aab080 100644 --- a/crates/ogar-oidc/src/lib.rs +++ b/crates/ogar-oidc/src/lib.rs @@ -113,7 +113,8 @@ impl KeySet { /// Parse a JWKS document (`{"keys":[...]}`). /// /// EC P-256 and OKP Ed25519 signing keys are kept. Keys of other types - /// (RSA included) and keys whose `use` is not `sig` are skipped and counted + /// (RSA included), keys whose `use` is not `sig` and keys whose `key_ops` + /// does not include `verify` are skipped and counted /// in [`KeySet::skipped`], so a mixed JWKS still loads. /// /// # Errors @@ -132,6 +133,7 @@ impl KeySet { .get("use") .and_then(Value::as_str) .is_some_and(|u| u != "sig") + || !permits_verify(jwk) { set.skipped += 1; continue; @@ -182,6 +184,17 @@ impl KeySet { } } +/// RFC 7517 §4.3: when `key_ops` is present, the key may be used only for the +/// listed operations. A key whose `key_ops` is not an array, or does not list +/// `verify`, is not a verification key. +fn permits_verify(jwk: &Value) -> bool { + match jwk.get("key_ops") { + None => true, + Some(Value::Array(ops)) => ops.iter().any(|op| op.as_str() == Some("verify")), + Some(_) => false, + } +} + fn parse_jwk(jwk: &Value) -> Result, KeySetError> { let field = |name: &str| jwk.get(name).and_then(Value::as_str); let alg = field("alg"); diff --git a/crates/ogar-oidc/tests/verifier.rs b/crates/ogar-oidc/tests/verifier.rs index f578f051..4884117e 100644 --- a/crates/ogar-oidc/tests/verifier.rs +++ b/crates/ogar-oidc/tests/verifier.rs @@ -441,6 +441,19 @@ fn jwks_parsing_skips_and_rejects() { assert_eq!(set.len(), 1); assert_eq!(set.skipped(), 1); + // key_ops restricts usage (RFC 7517 §4.3): without "verify" it is skipped. + let mut derive = jwk_es256(&es_key(8), "k3"); + derive.as_object_mut().expect("object").remove("use"); + derive["key_ops"] = json!(["deriveKey"]); + let set = parse(vec![ec.clone(), derive]).expect("deriveKey skipped"); + assert_eq!(set.len(), 1); + assert_eq!(set.skipped(), 1); + let mut verify = jwk_es256(&es_key(8), "k4"); + verify["key_ops"] = json!(["verify"]); + let set = parse(vec![ec.clone(), verify]).expect("verify kept"); + assert_eq!(set.len(), 2); + assert_eq!(set.skipped(), 0); + let dup = jwk_es256(&es_key(8), "k1"); assert!(matches!( parse(vec![ec, dup]),