Remove MCP direct identity-provider mode - #1350
Open
jiaenren wants to merge 1 commit into
Open
Conversation
MCP shipped two ways to authenticate a caller: the Gateway validating a bearer and MCP trusting injected headers, or FastMCP verifying the caller itself. Only one can be the answer, and the published documentation already recommended the second. This removes the first. request_context.py loses the header trust boundary entirely: the ASGI middleware, the per-request state, the header parsing, and the user_name field that no consumer read. get_request_credentials collapses to the FastMCP access token, which is now the only source of caller identity, so identity can no longer come from a header a caller could set. The module goes from 355 lines to 194. auth_enabled disappears with it. Authentication is not a mode any more, so its configuration is required rather than conditionally required, and a deployment that cannot reach an identity provider fails at load instead of starting up unauthenticated. The Gateway stops synthesising what FastMCP serves: the protected-resource route forwards unconditionally, and the local_reply_config that rewrote the /mcp 401 challenge is gone because FastMCP emits it. services.mcp loses authorizationServers and scopes, and oidcProxy loses its enabled toggle, which could only ever be true. Tests authenticate the way the service does. protocol_harness gains a static token verifier and a service_config helper, so suites that exercise the tool pipeline get a real auth provider instead of trusted headers. The smoke checks come with it, because deploying proxy mode for the first time showed the suite failing before it reached any assertion: it authenticates with the OSMO-issued OETF token, which FastMCP rejects. The unauthenticated surface a client uses to bootstrap OAuth needs no token and is now its own test -- the 401 challenge and its resource_metadata pointer, the RFC 9728 document, that FastMCP advertises its authorization endpoint under /mcp, and that the health endpoints the /mcp prefix would otherwise publish return 404. The authenticated catalog and workflow checks name OSMO_MCP_ACCESS_TOKEN, which runs them when an identity-provider token is supplied. Two smoke-check fixes ride along. The expected catalog was missing osmo_list_tasks, added by #1311, so its exact-equality assertion would have failed at 25 versus 26 had it ever run, and the two counts in the service README were stale for the same reason. The suite also carried an `auth` tag that the kind environment's exclude_tags dropped, so it never ran anywhere -- which is why the staleness went unnoticed. It is tagged by what it needs deployed now, with kind excluding `mcp` explicitly. Two behaviours changed and are recorded rather than hidden. An unauthenticated request is now rejected after RequestBodyLimitMiddleware has buffered its bounded body rather than before, because FastMCP's auth runs inside the route. And whether a follow-up request succeeds after a cancelled one is FastMCP's to decide; verified against the pre-change tree that an authenticated deployment already behaved this way, so this change does not introduce it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This was referenced Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack (6): #1328 → #1329 → #1350 → #1335 → #1342 → #1347
Stacked on #1329. The structural core of the MCP authentication simplification stack.
Issue #None
+251 / -1,285.
Why
MCP shipped two ways to authenticate: the Gateway validating a bearer and MCP trusting injected headers, or FastMCP verifying the caller itself. Only one can be the answer, and the published docs already recommended the second. Every route, matcher, config field and code path was conditional on which one was active.
Smoke checks come with it
Deploying proxy mode for the first time showed the MCP smoke suite failing before it reached any assertion: it authenticates with the OSMO-issued OETF token, which FastMCP rejects because it verifies caller tokens against the identity provider.
The unauthenticated surface a client uses to bootstrap OAuth needs no token and is now its own test — the 401 challenge and its
resource_metadatapointer, the RFC 9728 document, that FastMCP advertises its authorization endpoint under/mcp, and that the health endpoints the/mcpprefix would otherwise publish return 404. The authenticated catalog and workflow checks nameOSMO_MCP_ACCESS_TOKEN, which runs them when an identity-provider token is supplied.Two fixes ride along: the expected catalog was missing
osmo_list_tasks(added by #1311), so its exact-equality assertion would have failed at 25 versus 26 had it ever run; and the suite carried anauthtag that the kind environment'sexclude_tagsdropped, so it never ran anywhere — which is why that staleness went unnoticed.Change
request_context.py355 → 194 lines. The ASGI middleware, per-request state, header parsing, and theuser_namefield no consumer read are gone.get_request_credentials()collapses to FastMCP's access token — so caller identity can no longer come from a header a caller could set. That is now structural rather than validated.auth_enabledgoes with it. Authentication isn't a mode, so its configuration is required rather than conditionally required: a deployment that cannot reach an identity provider fails at load instead of starting unauthenticated.The Gateway stops synthesising what FastMCP serves — the protected-resource route forwards unconditionally, and the
local_reply_configthat rewrote the/mcp401 is gone because FastMCP emits it.authorizationServers,scopes, andoidcProxy.enabled(which could only ever be true) are removed, along with the direct-mode CI fixture and the validator half that tested it.Tests authenticate the way the service does:
protocol_harnessgainsAnyTokenVerifierand aservice_confighelper, so suites exercising the tool pipeline get a real auth provider instead of trusted headers.Two behaviour changes, recorded not hidden
1. Unauthenticated requests are rejected after the body is buffered, not before. The deleted middleware enforced fail-before-body; FastMCP's auth runs inside the route, so
RequestBodyLimitMiddleware— which is outermost — buffers the bounded body first. The request is still rejected and the body still bounded; only the order changed. A test asserts this explicitly rather than leaving it undiscovered.2. After a cancelled request, a follow-up on the same client is FastMCP's to decide. I verified against the pre-change tree, with an auth provider installed, that an authenticated deployment already behaved this way — so this PR does not introduce it. It is pre-existing in OIDC-proxy mode and worth its own investigation; a development deployment has it today.
Verification
bazel test //src/service/mcp/... //test/smoke/...— 76/76 passrender-tests.sh,validate-mcp-chart.sh— pass🤖 Generated with Claude Code
Checklist