Skip to content

fix(auth): populate OAuth scopes from server metadata discovery - #7267

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-oauth-scope-discovery-7266
Open

chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-oauth-scope-discovery-7266

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Summary

CredentialsManager._populate_auth_scheme() auto-discovers OAuth2/OIDC
server metadata (RFC 8414 / RFC 9728, e.g. as served by FastMCP) and uses
it to fill in the authorizationUrl / tokenUrl fields of an
ExtendedOAuth2 auth scheme's flows when they are missing. The discovery
response also carries scopes_supported, but that field was discarded —
callers had to duplicate the server's supported scopes by hand in their
AuthScheme configuration even though auto-discovery already had them.

This adds the same "fill in only if currently empty" treatment to
scopes for each OAuth flow (implicit, password, clientCredentials,
authorizationCode), mirroring the existing URL-population logic.

Fixes #7266

Test plan

Added an assertion to the existing
test_populate_auth_scheme_success test verifying that
flows.authorizationCode.scopes is populated from
AuthorizationServerMetadata.scopes_supported after a successful
discovery.

Verified the test fails without the fix (by reverting only the source
change and re-running):

$ .venv/bin/python -m pytest tests/unittests/auth/test_credential_manager.py -k test_populate_auth_scheme_success -q
...
FAILED tests/unittests/auth/test_credential_manager.py::TestCredentialManager::test_populate_auth_scheme_success - AssertionError: assert {} == {'read': '', 'write': ''}
1 failed, 41 deselected, 1 warning in 3.05s

And passes with the fix applied:

$ .venv/bin/python -m pytest tests/unittests/auth/test_credential_manager.py -q
..........................................                               [100%]
42 passed, 2 warnings in 1.45s

Full tests/unittests/auth suite (252 tests) also passes:

$ .venv/bin/python -m pytest tests/unittests/auth -q
...
252 passed, 85 warnings in 2.74s

pre-commit run (ruff, isort, pyink, addlicense, ADK compliance checks,
codespell) passes on both changed files.

AI assistance disclosure

This change was authored with the assistance of an AI coding agent
(Claude Code), which implemented the fix, added the test, and verified
it against the existing test suite.

🤖 Generated with Claude Code

CredentialsManager._populate_auth_scheme() only copied the discovered
authorization/token URLs onto the auth scheme's OAuth flows, ignoring
the scopes_supported field that RFC8414 discovery documents (and
FastMCP's discovery endpoint) also return. Callers had to duplicate
those scopes manually in the AuthScheme even though auto-discovery
already had them.

Fixes google#7266
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.

CredentialsManager should also extract scopes when populating auth schemes

1 participant