fix(adk): resolve OAuth persistence, ID token extraction, and auth flow crashes#535
Open
anubhav756 wants to merge 6 commits intoanubhav-adkfrom
Open
fix(adk): resolve OAuth persistence, ID token extraction, and auth flow crashes#535anubhav756 wants to merge 6 commits intoanubhav-adkfrom
anubhav756 wants to merge 6 commits intoanubhav-adkfrom
Conversation
48e8128 to
fb00b17
Compare
8b30dc4 to
ccce1f1
Compare
e22b74d to
9795ba4
Compare
102b2ec to
850d006
Compare
…ow crashes
This PR addresses several authentication bugs within the `toolbox-adk` wrapper that were preventing successful OIDC flows, causing repeated login prompts, and crashing during tool execution.
`adk-python` natively drops the `id_token` during OAuth2 exchange (`update_credential_with_tokens`), which is strictly required by the MCP Toolbox for `USER_IDENTITY` authentication.
#### Solution
Added a temporary monkey patch to hook into the `adk-python` credential update process and explicitly preserve `tokens.get("id_token")`.
> [!NOTE]
> Tracked by `TODO`s to remove once upstream PR google/adk-python#4402 is merged.
Tools repeatedly prompted users to authenticate because the exchanged credentials were not being appropriately persisted across sessions (`exchanged_auth_credential` was left unpopulated/None).
Explicitly assigned the newly fetched user credentials to `auth_config_adk.exchanged_auth_credential` and synced them to storage using `tool_context._invocation_context.credential_service.save_credential()`.
Passing an empty list of scopes triggered an `AttributeError` deep inside the `adk-python` `auth_handler.py` due to a falsy chain evaluation on empty dictionaries.
Added default OIDC fallback scopes (`["openid", "profile", "email"]`) for `USER_IDENTITY` flows when no explicit scopes are provided.
If a tool required the same authentication service across multiple parameters, the `add_auth_token_getter` logic would attempt to register it twice, causing a `ValueError: already registered`.
Collected `needed_services` into a deduplicated `set` and added a protective check before registration (`if not hasattr(...) or s not in ...`) to ensure idempotency.
- [x] Verified that submitting empty scopes successfully defaults to `openid profile email` and resolves without crashing `adk-python`.
- [x] Verified that the `id_token` is successfully propagated bounds and retrieved via `getattr(creds.oauth2, "id_token", ...)` during tool execution.
- [x] Verified that subsequent tool executions load the saved credentials seamlessly from the `CredentialService` without triggering continuous Google OAuth consent screens.
9795ba4 to
467aa0c
Compare
…roxy private auth params
…ore bindings validation error
…t_auth_response successfully
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.
Overview
This PR addresses several authentication bugs within the
toolbox-adkwrapper that were preventing successful OIDC flows, causing repeated login prompts, and crashing during tool execution.Changes
1. ID Token Retention (Temporary Monkey Patch)
Problem
adk-pythonnatively drops theid_tokenduring OAuth2 exchange (update_credential_with_tokens), which is strictly required by the MCP Toolbox forUSER_IDENTITYauthentication.Solution
Added a temporary monkey patch to hook into the
adk-pythoncredential update process and explicitly preservetokens.get("id_token").Note
Tracked by
TODOs to remove once upstream PR google/adk-python#4402 is merged.2. Credential Persistence Fix
Problem
Tools repeatedly prompted users to authenticate because the exchanged credentials were not being appropriately persisted across sessions (
exchanged_auth_credentialwas left unpopulated/None).Solution
Explicitly assigned the newly fetched user credentials to
auth_config_adk.exchanged_auth_credentialand synced them to storage usingtool_context._invocation_context.credential_service.save_credential().3. Safe Default OIDC Scopes
Problem
Passing an empty list of scopes triggered an
AttributeErrordeep inside theadk-pythonauth_handler.pydue to a falsy chain evaluation on empty dictionaries.Solution
Added default OIDC fallback scopes (
["openid", "profile", "email"]) forUSER_IDENTITYflows when no explicit scopes are provided.4. Auth Token Getter Deduplication
Problem
If a tool required the same authentication service across multiple parameters, the
add_auth_token_getterlogic would attempt to register it twice, causing aValueError: already registered.Solution
Collected
needed_servicesinto a deduplicatedsetand added a protective check before registration (if not hasattr(...) or s not in ...) to ensure idempotency.Testing
openid profile emailand resolves without crashingadk-python.id_tokenis successfully propagated bounds and retrieved viagetattr(creds.oauth2, "id_token", ...)during tool execution.CredentialServicewithout triggering continuous Google OAuth consent screens.