-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Bug #4458 #4467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bug #4458 #4467
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Response from ADK Triaging Agent Hello @P-nishant, thank you for creating this PR! Before we can merge this PR, could you please sign the Contributor License Agreement (CLA)? You can find more information at https://cla.developers.google.com/. In addition, could you please provide the manual end-to-end (E2E) evidence as described in your "Manual E2E Evidence (MCP + Cloud Run auth)" section? This information will help reviewers to verify your changes more efficiently. Thanks! |
Summary of ChangesHello @P-nishant, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an authentication issue where Cloud Run and other identity-aware services, which require OIDC ID tokens, were incorrectly receiving OAuth2 access tokens from the service account credential exchanger. The changes introduce new configuration options to explicitly request an ID token, ensuring proper authentication for these services while maintaining backward compatibility for existing access token flows. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for OIDC ID tokens for service account authentication, which is crucial for services like Cloud Run. The changes are well-structured, introducing new fields to the ServiceAccount model and a new path in the ServiceAccountCredentialExchanger to handle ID token fetching. The existing access token flow remains untouched, ensuring backward compatibility. The new functionality is also well-covered by unit tests. I have one suggestion to improve code clarity and maintainability.
| f"Failed to exchange service account token: {e}" | ||
| ) from e | ||
|
|
||
| def _fetch_id_token(self, sa_config) -> AuthCredential: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code clarity and to leverage static type checking, please add a type hint for the sa_config parameter. It should be of type ServiceAccount.
You will also need to import ServiceAccount from .....auth.auth_credential.
| def _fetch_id_token(self, sa_config) -> AuthCredential: | |
| def _fetch_id_token(self, sa_config: ServiceAccount) -> AuthCredential: |
|
Hi @P-nishant , Thank you for your contribution! It appears you haven't yet signed the Contributor License Agreement (CLA). Please visit https://cla.developers.google.com/ to complete the signing process. Once the CLA is signed, we'll be able to proceed with the review of your PR. Thank you! |
Manual E2E Evidence (local, Cloud Run-like ID-token behavior)Because I don’t have a Cloud Run MCP service in this repo, I validated the fix end-to-end locally using a streamable-http MCP server that enforces the Cloud Run-relevant contract:
This does not verify signatures (local test only), but it validates the behavior the fix targets: using an ID-token-like JWT (with correct audience) succeeds; an opaque token fails. Setup
Command used Terminal Apython e2e_local\jwt_aud_mcp_server.py Terminal B$env:E2E_MCP_URL="http://localhost:/mcp" logs |
Fix: ID Token support for Cloud Run MCP auth
Why
Cloud Run (and Cloud Functions, IAP, etc.) with "Require authentication" expects
an OIDC ID Token, not an OAuth2 access token. The
ServiceAccountCredentialExchangeralways fetched an access token viacredentials.token, so calls to authenticated Cloud Run services failed with401 Unauthorized.There was no way for users to ask for an ID token — the only workaround was
monkey-patching the exchanger at runtime.
What changed
ServiceAccountmodel (src/google/adk/auth/auth_credential.py)Two new optional fields:
use_id_token(bool, defaultFalse) — whenTrue, fetch an ID tokeninstead of an access token.
audience(str) — the target audience for the ID token, typically theservice URL. Required when
use_id_tokenisTrue.scopesalso got a default ([]) so callers using ID tokens don't have to passan empty list.
ServiceAccountCredentialExchanger(src/google/adk/tools/openapi_tool/auth/credential_exchangers/service_account_exchanger.py)Added a
_fetch_id_tokenhelper and a branch at the top ofexchange_credentialthat calls it when
use_id_tokenis set. The existing access-token path isuntouched.
For default credentials it uses
google.oauth2.id_token.fetch_id_token(). Forexplicit service-account JSON keys it uses
service_account.IDTokenCredentials.from_service_account_info().Tests
Four new tests covering: default-credential ID token, explicit-SA ID token,
missing audience validation, and fetch failure handling. All existing tests still
pass.
Usage
PR Description (ready to paste)
Summary
Cloud Run-protected MCP endpoints require an OIDC ID token, but service-account
exchange always returned an OAuth access token. This change adds an opt-in ID
token path for service account auth and keeps existing access-token behavior as
the default.
Testing Plan
autoformat.shequivalent on Windows shell):.\.venv\Scripts\python.exe -m isort src\google\adk\auth\auth_credential.py src\google\adk\tools\openapi_tool\auth\credential_exchangers\service_account_exchanger.py tests\unittests\tools\openapi_tool\auth\credential_exchangers\test_service_account_exchanger.py.\.venv\Scripts\python.exe -m pyink --config pyproject.toml src\google\adk\auth\auth_credential.py src\google\adk\tools\openapi_tool\auth\credential_exchangers\service_account_exchanger.py tests\unittests\tools\openapi_tool\auth\credential_exchangers\test_service_account_exchanger.py.\.venv\Scripts\python.exe -m pytest tests\unittests\tools\openapi_tool\auth\credential_exchangers\test_service_account_exchanger.py -q.\.venv\Scripts\python.exe -m pytest tests\unittests\tools\openapi_tool\auth\ tests\unittests\auth\test_credential_manager.py tests\unittests\tools\mcp_tool\test_mcp_tool.py -qUnit Test Evidence
Manual E2E Evidence (MCP + Cloud Run auth)
Please attach in the PR:
adk webprompt/response where the MCP tool call succeeds.use_id_token=True,audience=<cloud-run-url>).Suggested log snippet to include:
Docs Impact
use_id_token,audience) forservice-account auth flow.
google/adk-docsto documentCloud Run authenticated MCP setup with ID token usage.
Review Request
audienceis missing.