Warn when a pre-provisioned OAuth client is created without an issuer - #3435
Open
maxisbey wants to merge 2 commits into
Open
Warn when a pre-provisioned OAuth client is created without an issuer#3435maxisbey wants to merge 2 commits into
maxisbey wants to merge 2 commits into
Conversation
ClientCredentialsOAuthProvider and PrivateKeyJWTOAuthProvider send fixed credentials to whichever authorization server discovery yields unless `issuer=` names the one they belong to. Leaving it out stays allowed, but the provider now says so at construction with a UserWarning that names the server URL and the keyword to pass, so the choice is visible rather than silent. Nothing else changes: with `issuer=` set there is no warning, and a value that is not an http(s) URL is still a ValueError. The example story and the interaction tests pass `issuer=` (their authorization server is known); the extension tests that exercise the no-issuer path opt in to the warning explicitly.
Contributor
📚 Documentation preview
|
maxisbey
marked this pull request as ready for review
September 3, 2026 14:12
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The warning is already attributed to the caller's constructor line, so it does not need the provider name or server URL; _checked_issuer keeps its single argument.
Contributor
Author
|
test2 |
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
1 participant
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.
ClientCredentialsOAuthProviderandPrivateKeyJWTOAuthProvidernow emit aUserWarningat construction whenissuer=is not given.Motivation and Context
#3398 added the optional
issuer=keyword to the two pre-provisioned providers: with it, token requests are only ever built from metadata for that authorization server. Without it the provider follows whichever authorization server the MCP server advertises, which is the behaviour these providers have always had, and it stays allowed. The difference between the two is easy to miss when reading a constructor call, so the provider now says which one you picked:stacklevelpoints the warning at the caller's constructor line. Passingissuer=silences it, and there is no other change in behaviour.How Has This Been Tested?
tests/client/auth/extensions/test_client_credentials.py: a parametrised test asserts the warning text and that it is attributed to the calling file for both providers, and one test keeps exercising the no-issuer exchange path underpytest.warns. The older tests in that file, the interaction tests and thePrivateKeyJWTOAuthProviderdocs test now passissuer=matching the authorization server they already mock or run. Theoauth_client_credentialsexample story passesissuer=too.Breaking Changes
None. Code that constructs either provider without
issuer=keeps working and now sees oneUserWarningper call site (test suites running with warnings as errors will want to passissuer=or filter it).Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
The conformance client script constructs these providers from the harness's context, which does not carry an issuer yet, so the client-conformance job will print the warning; that is expected.
AI Disclaimer