Repository navigation
client_credentials sends the credential to an unverified issuer unless issuer= is passed (4.1.2) #3850
Description
On fastmcp 4.1.2, ClientCredentialsOAuthProvider without issuer= still posts to the token endpoint named in the protected resource's own metadata, so a server can point it at any authorization server it likes. Passing issuer= rejects it before the token call. 4.1.1 has no issuer argument at all, so the check can't be exercised there. The deprecation warning says the argument will be required in 3.0; the concern is the window before that, where the default is still to follow the server.
Activity
Shanmukh-l33t commentedon Nov 12, 2026
Root cause (current
main,77001c9)Without
issuer=,ClientCredentialsOAuthProviderposts the client credential to a token endpoint that is never verified to belong to any authorization server:_require_metadata_for_configured_issuer(src/mcp/client/auth/extensions/client_credentials.py:49) returns immediately whenissuer is None, so nothing guards the exchange._get_token_endpoint(src/mcp/client/auth/oauth2.py:445-451) falls back to the 2025-03-26 defaulturljoin(<resource origin>, "/token")when discovery found no authorization server metadata — and_exchange_token_client_credentialsthen postsclient_id/client_secret(Basic header or body) there. A resource whose protected resource metadata names an authorization server that serves no metadata receives the credential at its own/token, with no issuer verified anywhere.issueris only checked at discovery time (validate_metadata_issuer,src/mcp/client/auth/oauth2.py:716); the exchange itself would build a request from whatever metadata is in the context.(1)+(2) is the reported hole, and it matches the report exactly: passing
issuer=makes_require_metadata_for_configured_issuerstop the flow before the token call. (The class lives in this repo,src/mcp/client/auth/extensions/client_credentials.py; fastmcp surfaces it.)Proposed fix (branch below, no PR — I know unassigned PRs get auto-closed)
Narrowest change that closes the unverified-issuer window without changing documented behavior: a client-credentials token request is now only built from authorization server metadata for a verified issuer — the configured
issuer=when there is one, else the authorization server root the client resolved (the PRM-advertised server, or the resource origin on the legacy path), i.e. the same root discovery already requires the document'sissuerto name (RFC 8414 §3.3 / SEP-2468, #3398). Anything else stops withOAuthFlowErrorbefore the credential is attached, and the refused metadata and tokens are dropped so the next request rediscovers instead of refreshing against what was refused.What this deliberately does not change: a resource can still steer an
issuer=-less client to an authorization server of its choosing as long as that server serves self-consistent metadata — the documented deprecation-window default, pinned bytest_without_issuer_the_exchange_follows_whichever_server_was_discovered. Closing that window means makingissuer=required now instead of in 3.0, i.e. flipping the documented default early — a breaking change, so that call is yours. If you want it early, it is essentially making_checked_issuerraise instead of warn and updating the tests pinned to the follow-the-server default.Tests (regression-first,
tests/client/auth/extensions/test_client_credentials.py; 2 tests × {client-secret, private-key-jwt})test_without_issuer_the_exchange_refuses_when_no_metadata_was_discovered— full auth flow: 401 → PRM advertiseshttps://elsewhere.example.com→ every AS well-known 404s. Before: the secret provider buildsPOST https://api.example.com/tokencarrying the secret; after:OAuthFlowErrorbefore any request is built.test_without_issuer_the_exchange_refuses_metadata_for_an_unresolved_issuer— exchange-level guard: metadata whoseissuernames an issuer other than the resolved root. Before: the request is built (and the jwt assertion is minted for that issuer); after: refused, metadata + tokens dropped.RED on unmodified
main— 4 failed (2 tests × 2 providers):..._refuses_when_no_metadata_was_discovered[secret]—Failed: DID NOT RAISE OAuthFlowError: the token request carrying the credential was built for the resource origin's/token..._refuses_metadata_for_an_unresolved_issuer[secret]—Failed: DID NOT RAISE OAuthFlowError: request built for the unverified issuer's token endpoint..._refuses_metadata_for_an_unresolved_issuer[jwt]—Failed: DID NOT RAISE OAuthFlowError: assertion minted with the unverified issuer as audience..._refuses_when_no_metadata_was_discovered[jwt]—AssertionError: raisedMissing OAuth metadata for private_key_jwt flowinstead of the precise refusalGREEN with the fix:
tests/client/auth/extensions/test_client_credentials.py— 33 passed (29 pre-existing + 4 new). Targeted neighbors (tests/client tests/shared tests/docs_src) — 2753 passed, 1 xfailed. Full suite (coverage run -m pytest, serial — the-n 2run loses xdist worker coverage data in this sandbox) — 6236 passed, 1 failed, 8 skipped, 1 xfailed; the one failure istests/server/mcpserver/resources/test_file_resources.py::test_permission_error, which fails identically on pristinemainin this sandbox (it runs as root, sochmod 0o000doesn't block the read).coverage report— exit 0, TOTAL 100.00% (54333 statements / 0 missed, 4258 branches / 0 partial; the repo gate is branch coveragefail_under = 100).strict-no-cover— clean. Lint:ruff check+ruff format --checkclean on both changed files.Compare: main...Shanmukh-l33t:python-sdk:fix/3850-verify-issuer-before-token-request
This fix and its tests were written with AI assistance (Hermes Agent).
media-ctrl commentedon Nov 13, 2026
Checked this against main (77001c9), since the release you are on and main have diverged here.
The constructor already accepts issuer (src/mcp/client/auth/extensions/client_credentials.py:96) and the flow enforces it before a token request is built:
_require_metadata_for_configured_issuer(:49-64) raises on missing metadata or on an issuer mismatch, and_select_authorization_servernarrows the advertised list throughissuers_matchfirst. So the argument you are asking for exists on main, and the practical ask is a backport plus a clear note that the current release is unsafe by default rather than a new feature.What remains after that is the default path, and it is worth stating precisely: with no issuer,
_preferred_authorization_server(advertised, None)returnsadvertised[0](:42-46). A protected resource document that advertises several authorization servers therefore lets the resource choose which one receives the client credentials, and the client has no way to notice it was pointed somewhere else. Making issuer required in 3.0 closes it, but that window is open today.A narrower change would close most of it without breaking the common deployment: refuse the flow when issuer is None and the resource advertises more than one authorization server. The ambiguity is the attack surface. A single advertised authorization server is the normal case and keeps working unchanged, so the compatibility cost is limited to deployments that were already choosing arbitrarily.
That is one condition in
_preferred_authorization_server, and it is testable with a fixture protected resource metadata document listing two authorization servers where the first is not the credential's issuer: today the flow proceeds, with the change it errors before any token request is constructed.Shanmukh-l33t commentedon Nov 14, 2026
Agreed on the parts that matter, and one correction to my earlier comment: the constructor accepting
issuerI did not state, and on77001c9it is there (client_credentials.py:96), CI-covered, and enforced before a token request is built. The ask is a backport plus a default-path note, not a new argument.On the narrower change, one boundary worth stating before anyone implements it.
_preferred_authorization_serverpicksadvertised[0]whenissuer is None(client_credentials.py:42-46), and_require_metadata_for_configured_issuerreturns immediately in that case (:55-56), so refusing when more than one authorization server is advertised closes the choice the client cannot audit. It does not close provenance for the single-server case: with one advertised server and noissuer, the flow still accepts whatevertoken_endpointthat document names (oauth2.py:445-451), including the resource origin's own/tokenvia theurljoinfallback when discovery produced no metadata. A protected resource document that names exactly one server of its own choosing is still unverified; it is just not ambiguous.So the two changes have different failure modes: the multi-server refusal removes a decision the client cannot defend, and required-issuer-in-3.0 removes the unverified binding. The fixture you describe (two advertised servers, the first not the credential's issuer) exercises the first and would pass through the second unchanged, which is probably right if the 3.0 removal is already planned, but worth saying in the same place so the gap does not read as closed.
media-ctrl commentedon Nov 14, 2026
Both of those read correctly against main, and the split is the useful part of the framing.
Confirmed on
_get_token_endpoint(oauth2.py:446-451): the metadatatoken_endpointis used verbatim when present, andurljoin(get_authorization_base_url(server_url), "/token")is the fallback when discovery produced nothing. So with one advertised server and noissuer, the credential still follows a document the protected resource chose. Unambiguous and unverified, as you put it.One piece of the ask is already in the code, but only at runtime:
_checked_issuer(client_credentials.py:28-38) raisesMCPDeprecationWarningat construction and its text says exactly that the MCP server decides which authorization server receives the credentials. The gap is visibility, so the docs and CHANGELOG half of the backport is worth doing independently of the 3.0 removal. That half is cheap, and it is the half that reaches people who never read this thread.On the fixture, one more case is worth pinning explicitly: single advertised server and no
issuer, asserting the current behavior of posting the credential to that endpoint, so the provenance gap is tested rather than implied. Onceissuerbecomes required, that case becomes the regression that proves the change landed, which is more useful than deleting it.Worth noting for whoever implements:
_preferred_authorization_serverstill falls back toadvertised[0]when anissueris configured and nothing matches (:42-46). It is safe today only because_require_metadata_for_configured_issueris the first statement in_exchange_token_client_credentials(:149) and drops metadata plus tokens on mismatch. That is ordering, not selection, so a raise in the selector would keep it from regressing silently.