fix(server): refuse an unadmitted OIDC subject without a sign-in challenge

changelog.d/oidc-not-admitted.fixed.md

@@ -0,0 +1,12 @@
+- A server with an OIDC resource identity no longer answers 401
+  `error="invalid_token"` to a valid access token whose subject the admission
+  file does not name. That answer told OAuth and MCP clients such as Claude to
+  sign in again, which issues a token for the same subject, so they looped or
+  reported a failed sign-in. The server now answers 403 with the new error
+  code `not_admitted` and no challenge. A valid token while the admission
+  snapshot is expired gets 503 `service_unavailable` with `Retry-After`,
+  instead of the same 401. A forged, expired or misaddressed token still gets
+  401 `invalid_token`, and the token's own checks still run first. See
+  [MCP][oidc-not-admitted-guide].
+
+[oidc-not-admitted-guide]: ../docs/user/operations/mcp.md

crates/omnigraph-api-types/src/lib.rs

@@ -2290,6 +2290,10 @@ error_codes! {
     /// The credential's actor may not do this: policy denies the action, or
     /// the action needs a policy permit the server has not been given.
     Forbidden = "forbidden", [403];
+    /// The OIDC access token is valid, but the server admits no principal for
+    /// its subject. Signing in again issues a token for the same subject and
+    /// cannot help; the server's operator must admit the identity.
+    NotAdmitted = "not_admitted", [403];
     /// The request is malformed or invalid: a body, query string, path
     /// parameter or header that does not decode, a query the compiler
     /// refuses (see `diagnostic`), or a value refused before any effect.
@@ -2426,6 +2430,7 @@ impl ErrorCode {
             }
             // The request itself is wrong; no state it could read changes that.
             Self::Unauthorized
+            | Self::NotAdmitted
             | Self::BadRequest
             | Self::ApiContractMismatch
             | Self::ActorMismatch

crates/omnigraph-cli/src/command_outcome.rs

@@ -455,7 +455,9 @@ fn admission_refusal(status: StatusCode, code: Option<&ErrorCode>) -> Option<Act
         | (StatusCode::UNAUTHORIZED, code @ ErrorCode::Unauthorized)
         | (StatusCode::BAD_REQUEST, code @ ErrorCode::ApiContractMismatch)
         | (StatusCode::PRECONDITION_FAILED, code @ ErrorCode::ActorMismatch)
-        | (StatusCode::FORBIDDEN, code @ ErrorCode::Forbidden) => Some(code.action()),
+        | (StatusCode::FORBIDDEN, code @ (ErrorCode::Forbidden | ErrorCode::NotAdmitted)) => {
+            Some(code.action())
+        }
         _ => None,
     }
 }

crates/omnigraph-server/src/handlers.rs

@@ -422,11 +422,20 @@ pub(crate) async fn require_bearer_auth(
     }
 
     let provided_token = crate::challenge::bearer_token(request.headers())?;
-    let Some(actor) = state.authenticate_bearer_token(provided_token) else {
-        return Err(ApiError::unauthorized(
-            Unauthenticated::Refused,
-            "invalid bearer token",
-        ));
+    let actor = match state.authenticate_bearer_token(provided_token) {
+        Ok(actor) => actor,
+        Err(crate::oidc_identity::OidcRefusal::Invalid) => {
+            return Err(ApiError::unauthorized(
+                Unauthenticated::Refused,
+                "invalid bearer token",
+            ));
+        }
+        Err(crate::oidc_identity::OidcRefusal::NotAdmitted) => {
+            return Err(ApiError::not_admitted());
+        }
+        Err(crate::oidc_identity::OidcRefusal::Unavailable) => {
+            return Err(ApiError::identity_admission_unavailable());
+        }
     };
     request.extensions_mut().insert(actor.actor().clone());
     request.extensions_mut().insert(actor);

crates/omnigraph-server/src/lib.rs

@@ -1080,7 +1080,14 @@ impl AppState {
         snapshot.any_per_graph_policy
     }
 
-    fn authenticate_bearer_token(&self, provided_token: &str) -> Option<AuthenticatedActor> {
+    /// The actor a bearer token resolves to: a static token, then a signed
+    /// data token, then OIDC. Only OIDC tells a refused token from an
+    /// unadmitted or temporarily unverifiable identity; every other miss is
+    /// an invalid token.
+    fn authenticate_bearer_token(
+        &self,
+        provided_token: &str,
+    ) -> std::result::Result<AuthenticatedActor, oidc_identity::OidcRefusal> {
         // Hash the incoming token and compare against every stored digest in
         // constant time. Iterate all entries unconditionally so total work —
         // and therefore response timing — doesn't depend on which slot matches.
@@ -1091,20 +1098,27 @@ impl AppState {
                 matched = Some(Arc::clone(actor));
             }
         }
-        matched.map(AuthenticatedActor::cluster_static).or_else(|| {
-            let now = std::time::SystemTime::now()
-                .duration_since(std::time::UNIX_EPOCH)
-                .ok()?
-                .as_secs();
-            self.data_token_trust
-                .as_ref()
-                .and_then(|trust| trust.verify_authenticated_at(provided_token, now))
-                .or_else(|| {
-                    self.oidc_identity_trust
-                        .as_ref()?
-                        .verify_at(provided_token, i64::try_from(now).ok()?)
-                })
-        })
+        if let Some(actor) = matched {
+            return Ok(AuthenticatedActor::cluster_static(actor));
+        }
+        let Some(now) = std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .ok()
+            .map(|elapsed| elapsed.as_secs())
+        else {
+            return Err(oidc_identity::OidcRefusal::Invalid);
+        };
+        if let Some(actor) = self
+            .data_token_trust
+            .as_ref()
+            .and_then(|trust| trust.verify_authenticated_at(provided_token, now))
+        {
+            return Ok(actor);
+        }
+        match (self.oidc_identity_trust.as_ref(), i64::try_from(now)) {
+            (Some(trust), Ok(now)) => trust.check_at(provided_token, now),
+            _ => Err(oidc_identity::OidcRefusal::Invalid),
+        }
     }
 }
 
@@ -1152,6 +1166,25 @@ impl ApiError {
         Self::new(ErrorCode::Forbidden, message)
     }
 
+    /// HTTP 403 for a valid OIDC token whose subject this server does not
+    /// admit. No challenge: signing in again cannot help.
+    pub(crate) fn not_admitted() -> Self {
+        Self::new(
+            ErrorCode::NotAdmitted,
+            "the signed-in identity is not admitted to this server",
+        )
+    }
+
+    /// HTTP 503 for a valid OIDC token while the identity admission snapshot
+    /// is expired or the clock lies outside it. Access returns with the next
+    /// publication of the snapshot; signing in again cannot help.
+    pub(crate) fn identity_admission_unavailable() -> Self {
+        Self::new(
+            ErrorCode::ServiceUnavailable,
+            "OIDC identity admission is unavailable: the server's identity snapshot has expired; retry after its next publication",
+        )
+    }
+
     pub fn bad_request(message: impl Into<String>) -> Self {
         Self::new(ErrorCode::BadRequest, message)
     }
@@ -1445,6 +1478,7 @@ impl ApiError {
             let refused_before_start = matches!(
                 self.code,
                 ErrorCode::Unauthorized
+                    | ErrorCode::NotAdmitted
                     | ErrorCode::ApiContractMismatch
                     | ErrorCode::ActorMismatch
                     | ErrorCode::Forbidden

crates/omnigraph-server/src/oidc_identity.rs

@@ -305,73 +305,113 @@ impl OidcIdentityTrust {
         });
     }
 
+    #[cfg(test)]
     pub(crate) fn verify_at(&self, token: &str, now: i64) -> Option<AuthenticatedActor> {
-        if token.is_empty() || token.len() > MAX_TOKEN_BYTES {
-            return None;
-        }
+        self.check_at(token, now).ok()
+    }
+
+    /// Admit a bearer token, or say why not. The token's own checks come
+    /// first, against the snapshot's published keys and binding; only a token
+    /// that passes them learns whether the snapshot is current and whether its
+    /// subject is admitted. A stale snapshot's keys classify a refusal and
+    /// never admit.
+    pub(crate) fn check_at(
+        &self,
+        token: &str,
+        now: i64,
+    ) -> Result<AuthenticatedActor, OidcRefusal> {
         let snapshot = self.current.load();
+        let subject = accepted_subject(&snapshot, token, now).ok_or(OidcRefusal::Invalid)?;
         if now < 0
             || snapshot.expires_at <= now
             || snapshot.generated_at > now.saturating_add(CLOCK_SKEW_SECONDS)
         {
-            return None;
-        }
-        let header = decode_header(token).ok()?;
-        if header.alg != Algorithm::RS256
-            || header.crit.is_some()
-            || header.enc.is_some()
-            || header.zip.is_some()
-            || header
-                .typ
-                .as_deref()
-                .is_some_and(|t| !matches!(t, "JWT" | "at+jwt"))
-        {
-            return None;
+            return Err(OidcRefusal::Unavailable);
         }
-        let key = snapshot.keys.get(header.kid.as_deref()?)?;
-        let mut validation = Validation::new(Algorithm::RS256);
-        validation.leeway = 0;
-        // Explicit clock makes expiry tests deterministic and shares one instant
-        // with the snapshot check; signature/issuer/audience still use the library.
-        validation.validate_exp = false;
-        validation.validate_nbf = false;
-        validation.set_required_spec_claims(&["iss", "sub", "aud", "iat", "exp"]);
-        validation.set_issuer(&[&snapshot.binding.issuer]);
-        validation.set_audience(&[&snapshot.binding.audience]);
-        let claims = decode::<Value>(token, key, &validation).ok()?.claims;
-        if claims.get("iss")?.as_str()? != snapshot.binding.issuer
-            || claims.get("aud")?.as_str()? != snapshot.binding.audience
-            || claims.get("org_id")?.as_str()? != snapshot.binding.organization_id
-            || !bounded(claims.get("client_id")?.as_str()?, 512)
-            || claims.get("act").is_some_and(|v| !v.is_null())
-            || claims.get("impersonator").is_some_and(|v| !v.is_null())
-            || claims
-                .get("sub_profile")
-                .is_some_and(|v| v.as_str() != Some("user"))
-        {
-            return None;
-        }
-        let iat = claims.get("iat")?.as_i64()?;
-        let exp = claims.get("exp")?.as_i64()?;
-        if iat < 0
-            || iat > now.saturating_add(CLOCK_SKEW_SECONDS)
-            || exp <= now
-            || exp <= iat
-            || exp.checked_sub(iat)? > MAX_LIFETIME_SECONDS
-            || claims
-                .get("nbf")
-                .is_some_and(|v| v.as_i64().is_none_or(|n| n > now))
-        {
-            return None;
-        }
-        let subject = claims.get("sub")?.as_str()?;
-        let principal_id = snapshot.principals.get(subject)?.clone();
-        Some(AuthenticatedActor::oidc_identity(VerifiedIdentity {
+        let principal_id = snapshot
+            .principals
+            .get(&subject)
+            .ok_or(OidcRefusal::NotAdmitted)?
+            .clone();
+        Ok(AuthenticatedActor::oidc_identity(VerifiedIdentity {
             principal_id,
         }))
     }
 }
 
+/// Why an OIDC bearer token admits no actor.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum OidcRefusal {
+    /// The token fails its own checks: signature, issuer, audience,
+    /// organization, client, delegation or lifetime. A new token can help
+    /// (RFC 6750 `invalid_token`).
+    Invalid,
+    /// The token passes, but the admission snapshot names no principal for
+    /// its subject. A new token for the same subject cannot help.
+    NotAdmitted,
+    /// The token passes, but the admission snapshot has expired or the
+    /// server's clock lies outside it. Access returns with the next
+    /// publication; a new token cannot help.
+    Unavailable,
+}
+
+/// The subject of a token that passes every check of its own against the
+/// snapshot's keys and binding, whatever the snapshot's lifetime.
+fn accepted_subject(snapshot: &Snapshot, token: &str, now: i64) -> Option<String> {
+    if token.is_empty() || token.len() > MAX_TOKEN_BYTES {
+        return None;
+    }
+    let header = decode_header(token).ok()?;
+    if header.alg != Algorithm::RS256
+        || header.crit.is_some()
+        || header.enc.is_some()
+        || header.zip.is_some()
+        || header
+            .typ
+            .as_deref()
+            .is_some_and(|t| !matches!(t, "JWT" | "at+jwt"))
+    {
+        return None;
+    }
+    let key = snapshot.keys.get(header.kid.as_deref()?)?;
+    let mut validation = Validation::new(Algorithm::RS256);
+    validation.leeway = 0;
+    // Explicit clock makes expiry tests deterministic and shares one instant
+    // with the snapshot check; signature/issuer/audience still use the library.
+    validation.validate_exp = false;
+    validation.validate_nbf = false;
+    validation.set_required_spec_claims(&["iss", "sub", "aud", "iat", "exp"]);
+    validation.set_issuer(&[&snapshot.binding.issuer]);
+    validation.set_audience(&[&snapshot.binding.audience]);
+    let claims = decode::<Value>(token, key, &validation).ok()?.claims;
+    if claims.get("iss")?.as_str()? != snapshot.binding.issuer
+        || claims.get("aud")?.as_str()? != snapshot.binding.audience
+        || claims.get("org_id")?.as_str()? != snapshot.binding.organization_id
+        || !bounded(claims.get("client_id")?.as_str()?, 512)
+        || claims.get("act").is_some_and(|v| !v.is_null())
+        || claims.get("impersonator").is_some_and(|v| !v.is_null())
+        || claims
+            .get("sub_profile")
+            .is_some_and(|v| v.as_str() != Some("user"))
+    {
+        return None;
+    }
+    let iat = claims.get("iat")?.as_i64()?;
+    let exp = claims.get("exp")?.as_i64()?;
+    if iat < 0
+        || iat > now.saturating_add(CLOCK_SKEW_SECONDS)
+        || exp <= now
+        || exp <= iat
+        || exp.checked_sub(iat)? > MAX_LIFETIME_SECONDS
+        || claims
+            .get("nbf")
+            .is_some_and(|v| v.as_i64().is_none_or(|n| n > now))
+    {
+        return None;
+    }
+    Some(claims.get("sub")?.as_str()?.to_owned())
+}
+
 #[cfg(test)]
 #[path = "oidc_identity_tests.rs"]
 mod tests;

crates/omnigraph-server/src/oidc_identity_tests.rs

@@ -296,3 +296,50 @@ fn public_snapshot_bounds_and_ambiguous_admission_refuse_before_boot() {
     let bad = encode(&h, &claims(), &EncodingKey::from_secret(b"not an RSA key")).unwrap();
     assert!(trust.verify_at(&bad, NOW).is_none());
 }
+
+#[test]
+fn refusals_say_whether_a_new_token_can_help_issue_1172() {
+    let (_dir, trust) = setup(&document());
+    assert!(trust.check_at(&sign(&claims()), NOW).is_ok());
+    // A token that fails its own checks is invalid: a new one can help.
+    let mut expired = claims();
+    expired["exp"] = json!(NOW);
+    let tampered = tamper_signature(&sign(&claims()));
+    for token in [sign(&expired), tampered, "not-a-jwt".into()] {
+        assert_eq!(
+            trust.check_at(&token, NOW).err(),
+            Some(OidcRefusal::Invalid)
+        );
+    }
+    // A valid token for a subject the snapshot does not name is not admitted.
+    let mut stranger = claims();
+    stranger["sub"] = json!("user_unadmitted");
+    assert_eq!(
+        trust.check_at(&sign(&stranger), NOW).err(),
+        Some(OidcRefusal::NotAdmitted)
+    );
+    // Past the snapshot's expiry a valid token is unverifiable, not invalid;
+    // a token that fails its own checks stays invalid even then.
+    let mut late = claims();
+    late["iat"] = json!(NOW + 250);
+    late["exp"] = json!(NOW + 400);
+    assert_eq!(
+        trust.check_at(&sign(&late), NOW + 300).err(),
+        Some(OidcRefusal::Unavailable)
+    );
+    assert_eq!(
+        trust.check_at("not-a-jwt", NOW + 300).err(),
+        Some(OidcRefusal::Invalid)
+    );
+}
+
+/// The token with one character changed in the middle of its signature. The
+/// last base64url character can carry only padding bits, so changing it may
+/// leave the signature bytes unchanged.
+fn tamper_signature(token: &str) -> String {
+    let (head, signature) = token.rsplit_once('.').unwrap();
+    let mut signature: Vec<char> = signature.chars().collect();
+    let middle = signature.len() / 2;
+    signature[middle] = if signature[middle] == 'A' { 'B' } else { 'A' };
+    format!("{head}.{}", signature.into_iter().collect::<String>())
+}