Skip to content

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

Open
ragnorc wants to merge 1 commit into
ModernRelay:mainfrom
ragnorc:fix/1172-not-admitted
Open

ragnorc wants to merge 1 commit into
ModernRelay:mainfrom
ragnorc:fix/1172-not-admitted

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #1172. The issue is still needs-triage; this PR should merge only after it is accepted.

A server with --oidc-identity-trust answered 401 error="invalid_token" in three cases: to a forged or expired token, to a valid token whose subject the admission file does not name, and to a valid token while the admission snapshot was expired. For the last two, signing in again issues a token for the same subject, so an OAuth or MCP client that follows RFC 6750 §3.1 (Claude, for example) loops on sign-in or reports a failed sign-in.

What changes

Token Before After
Fails its own checks (signature, issuer, audience, organization, client, delegation, lifetime) 401 invalid_token 401 invalid_token (unchanged)
Passes, subject not in the admission file 401 invalid_token 403 not_admitted, no WWW-Authenticate, disposition fix
Passes, admission snapshot expired (or clock outside it) 401 invalid_token 503 service_unavailable, no challenge, Retry-After
  • OidcIdentityTrust::check_at returns a typed refusal (Invalid, NotAdmitted, Unavailable). The token's own checks run first, against the snapshot's published keys and binding. So a caller without a validly signed token still gets 401 and learns nothing about the admission file. A stale snapshot's keys only classify a refusal; they never admit. One refinement to the issue's sketch: a token that fails its own checks stays 401 even while the snapshot is expired, so a mistyped static token doesn't turn into a misleading 503.
  • Static and signed data tokens are unchanged. Only OIDC distinguishes an unadmitted or temporarily unverifiable identity; every other miss stays an invalid token.
  • not_admitted is a new entry in the ErrorCode registry (403, action fix). It counts as refused before start in the server's disposition, and the CLI's admission-refusal table accepts it beside forbidden. The 403 body names no subject.
  • verify_at remains as a test-only wrapper, so the existing OIDC unit tests are unchanged.
  • The client_id that server: an OIDC actor drops the token's client_id, so every client acting for one person is the same actor #1173 asks to keep is not touched here.

Compatibility

ErrorCode is a closed enum and contract 0.14 has shipped. A client that decodes code with the 0.14.0 enum will not parse a not_admitted body. Only an OIDC server that refuses an unadmitted subject sends it, and the status (403, with no challenge) is what stops the OAuth loop. MCP clients that read only the status and headers are unaffected. openapi.json is regenerated.

Tests

  • auth_policy::issue_1172_an_unadmitted_subject_is_refused_without_a_sign_in_challenge (the regression test the Fix Regression Gate keys on), on GET /graphs/discovery and POST /mcp:
    • an unadmitted subject gets 403 not_admitted with no WWW-Authenticate, disposition {effects: none, action: fix}, and the subject not echoed;
    • an admitted subject succeeds;
    • a tampered signature and a wrong audience still get 401 with error="invalid_token" and resource_metadata.
  • oidc_identity::tests::refusals_say_whether_a_new_token_can_help_issue_1172 covers the three refusal kinds, including garbage while the snapshot is expired, which stays Invalid.
  • The MCP test that grouped an unknown subject with the invalid_token cases now expects 403 with no challenge for it.
  • The OpenAPI error-code list gains not_admitted.

The failing behaviour was reproduced on main in the issue's comments: the same request shape got 401 invalid_token for an unadmitted subject.

Local runs, with RUST_MIN_STACK=16777216 and CARGO_INCREMENTAL=0:

  • cargo test -p omnigraph-server -p omnigraph-api-types --locked: server lib 219 passed, 2 ignored; integration 287 passed; boot_settings 43 passed, 1 ignored; multi_graph 8 passed; omnigraph-api-types 16 passed. Run on Rust 1.98.0.
  • cargo test -p omnigraph-cli --locked --lib: 367 passed, 9 ignored. Run on Rust 1.98.0.
  • On the pinned toolchain (cargo +1.97.1): cargo clippy -p omnigraph-server -p omnigraph-api-types -p omnigraph-cli --all-targets --locked -- -D warnings -W clippy::dbg_macro is clean, oidc_identity unit tests 8 passed, and integration auth_policy::/mcp::/openapi:: 163 passed. Clippy 0.1.98, the local default, also flags existing code in omnigraph-catalog (chunks_exact with a constant size); that is outside this change and the pinned toolchain does not flag it.
  • cargo fmt --all --check, python3 scripts/check-docs.py, bash scripts/check-agents-md.sh, typos: clean.

Not run locally: the workspace-wide test and clippy graphs, the CLI integration suites, GQT and DST.

Docs

The MCP guide's sign-in section, the server.md auth paragraph (kept at the 350-line cap), the troubleshooting status table (403 and 503 rows), RFC 0003's challenge section and its decision log, and a changelog.d fixed note.

ragnorc commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Recommendation: ready.

A valid OIDC access token for a subject the admission file does not name used to get HTTP 401 with error="invalid_token". RFC 6750 tells the client to get a new token. The new token names the same subject, so the client signs in again and the server refuses it again. The same 401 was sent when the admission snapshot had expired.

The server now returns one of three results. A token that fails its own checks still gets 401 invalid_token. A valid token whose subject is not admitted gets 403 not_admitted and no WWW-Authenticate header. A valid token while the snapshot is expired, or while the clock is outside the snapshot, gets 503 service_unavailable with Retry-After and no challenge. The token checks run first, against that snapshot's keys and binding. A stale snapshot never admits the caller.

OidcIdentityTrust::check_at owns the three results. require_bearer_auth is the one gate for protected routes and for /mcp, and it maps each result. The gate inserts an actor only for an admitted token. Policy still applies to that actor. Static bearer tokens and signed data tokens still fail as invalid tokens. They have no admission file.

The sign-in loop stops. The operator admits the identity, or publishes a fresh snapshot. A caller with no valid signature learns nothing about the admission file. The response names no subject. A client that decodes the shipped 0.14 ErrorCode enum cannot parse a not_admitted body. The status and the absent challenge are what stop the loop.

No blocking findings.

Optional:

  1. In RFC 0003, the Challenge section still says that a bearer token that does not authenticate is a refused one. The new paragraph narrows that sentence. Edit the earlier sentence so the record states one rule.
  2. service_unavailable registry text still describes only closed operation admission and an owner-change race. The troubleshooting page now also names an expired OIDC snapshot. Add that cause to the ErrorCode doc comment and regenerate openapi.json.

Inline on docs/rfcs/0003-mcp-server-surface.md line 373: The Challenge section above still says that a bearer token that does not authenticate is a refused one. This paragraph narrows that rule. Edit the earlier sentence so the RFC states one rule. Only a token that fails its own checks is a refused token, and only that refusal carries invalid_token.

Inline on crates/omnigraph-server/src/lib.rs line 1181: This constructor uses ErrorCode::ServiceUnavailable. The registry text for that code still describes closed operation admission and an owner-change race. OpenAPI copies that text. The troubleshooting page now also names an expired OIDC snapshot. Add that cause to the code's doc comment and regenerate openapi.json, so both places name the same cause.

Tradeoff and liability. The check uses the snapshot keys after expires_at. Those keys only choose the refusal. A token that fails verification stays 401 while the snapshot is expired, including a kid the snapshot does not hold. A key rotation during a publisher outage can still look like invalid_token. That choice keeps a bad or mistyped token from being reported as an outage. The signature check now runs on the expired path. A normal request already pays that cost.

not_admitted is one new public code on contract 0.14. The status and the challenge both change, so a separate code is the right cost. A later change that only changes a message should not add another code. OidcRefusal is the owner of the three results. A server-wide auth-result type would be a second primitive. Add it when a second verifier needs the same three results. The success path still builds VerifiedIdentity from principal_id only, so issue 1173 can add client_id there.

Protected routes and /mcp share require_bearer_auth. Policy denial of an admitted subject was already 403 forbidden, with no challenge. The CLI MCP bridge reads the credential again only after HTTP 401. Open PRs 1146 and 1154 keep that rule. This diff does not touch Lance storage.

Validation. Head d739db6943e47cb681d3d2bf71188f7590ea2b42. Merge base bdddcbb153415d575711d7aa7eaf5999af27a8a2. GQT cannot express a bearer challenge, so the owner is the server integration test. The temporary test on the merge base was the PR test, and it was removed after the run.

Merge base, predict 401 for an unadmitted subject:

cargo test -p omnigraph-server --locked --test integration auth_policy::issue_1172_an_unadmitted_subject_is_refused_without_a_sign_in_challenge -- --exact --nocapture

Result: fail. GET /graphs/discovery returned 401, and the test requires 403. 0 passed and 1 failed.

Head, same test, separate target directory:

Result: pass. 1 passed and 0 failed.

Head unit tests oidc_identity::tests::: 8 passed and 0 failed, including refusals_say_whether_a_new_token_can_help_issue_1172.

Head mcp::mcp_authenticates_every_request_and_enforces_resource_and_http_bounds: pass.

Head openapi::error_code_schema_has_expected_variants: pass.

CI on this head is green, including Test Workspace, Lint (clippy), Format, and Fix Regression Gate. Local clippy and the CLI test binary were not rerun.

Limits. The HTTP 503 answer was not driven through the integration suite. Snapshot parse refuses an already-expired file, and the server clock is not injectable. The unit test covers the Unavailable classification. The HTTP answer uses ErrorCode::ServiceUnavailable, which already sets status 503 and Retry-After when the action is retry. The challenge layer adds WWW-Authenticate only on 401.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

server: a signed-in person the admission file does not name gets 401 invalid_token, so OAuth clients sign in again

1 participant