Skip to content

fix(auth): omit scope from OAuth2 token exchange and refresh requests#5874

Closed
doughayden wants to merge 1 commit into
google:mainfrom
doughayden:fix/oauth2-omit-scope-from-token-requests
Closed

fix(auth): omit scope from OAuth2 token exchange and refresh requests#5874
doughayden wants to merge 1 commit into
google:mainfrom
doughayden:fix/oauth2-omit-scope-from-token-requests

Conversation

@doughayden

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Solution:

create_oauth2_session in oauth2_credential_util.py is the shared helper behind both token exchange and token refresh, and it sets scope on the OAuth2Session. authlib then carries that scope onto the refresh request (if "scope" not in kwargs and self.scope), and some providers like Salesforce reject scope on refresh, so the refresh fails.

Neither token operation needs scope. The authorization code already encodes the granted scopes (RFC 6749 section 4.1.3), and on refresh scope is optional, with the server reusing the originally granted scope when it is absent (RFC 6749 section 6). This change stops setting scope in the shared helper, so it is dropped from both exchange and refresh.

Authorization URL construction uses a separate OAuth2Session in auth_handler.py and is unchanged, so scope still appears on the consent request where it is required. This is the "use scope only during authorization" outcome discussed on the issue, implemented at the token-request helper rather than by splitting scopes out of the auth scheme.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added two tests in tests/unittests/auth/test_oauth2_credential_util.py that drive refresh_token and fetch_token through a stubbed OAuth2Session.post and assert the outgoing request body contains no scope. Asserting the wire body keeps the regression independent of authlib's internal scope handling.

tests/unittests/auth/test_oauth2_credential_util.py    11 passed
tests/unittests/auth                                  183 passed

Manual End-to-End (E2E) Tests:

A runnable reproduction is at doughayden/adk-issue-examples / 02-scope_in_refresh: it stands up a token server that rejects scope on refresh and drives an ADK refresh through it. Refresh fails before this change and succeeds after. The same fix has been running as a monkey-patch against a live Salesforce integration, where token refresh succeeds.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules. (N/A)

Additional context

This is currently carried as a monkey-patch in a downstream project; the fix lets that be removed. Fixing the shared helper rather than passing scope at each call site also covers the token exchange path, keeping the two token operations consistent.

@rohityan rohityan self-assigned this May 28, 2026
@rohityan rohityan added core [Component] This issue is related to the core interface and implementation request clarification [Status] The maintainer need clarification or more information from the author labels May 28, 2026
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @doughayden , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors by running autoformat.sh

@doughayden

Copy link
Copy Markdown
Contributor Author

Thanks @rohityan. autoformat.sh was removed in 533776e ("chore: Switch to pre-commit and cleanup redundant tools") and replaced by pre-commit, so I ran pre-commit run --all-files against the merge head to reproduce CI locally.

The auto-fix changes are all in files this PR does not touch: contributing/samples/models/interactions_api/{agent,main}.py, src/google/adk/tools/skill_toolset.py, and tests/unittests/models/test_interactions_utils.py. The diffs are mechanical (quote style, import order, line wrapping). Happy to rebase once those land on main, or to open a separate cleanup PR if that's easier on your end.

- Stop setting scope on the shared create_oauth2_session helper
- Drops scope from both token exchange and refresh requests
- Neither needs scope (RFC 6749 4.1.3, 6); some providers reject it
- Auth URL construction in auth_handler.py keeps scope, unchanged
- Add body-level tests asserting refresh and exchange omit scope
@boyangsvl boyangsvl force-pushed the fix/oauth2-omit-scope-from-token-requests branch from 8d5a4c2 to 74a609e Compare May 29, 2026 18:04
@boyangsvl boyangsvl assigned boyangsvl and unassigned rohityan May 29, 2026
copybara-service Bot pushed a commit that referenced this pull request May 29, 2026
- Stop setting scope on the shared create_oauth2_session helper
- Drops scope from both token exchange and refresh requests
- Neither needs scope (RFC 6749 4.1.3, 6); some providers reject it
- Auth URL construction in auth_handler.py keeps scope, unchanged
- Add body-level tests asserting refresh and exchange omit scope

Merge #5874

Change-Id: I78e7e1e69076afcc13460b7965ca05cae734814e
@boyangsvl

Copy link
Copy Markdown
Collaborator

I've merged the PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation request clarification [Status] The maintainer need clarification or more information from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth2 token refresh fails for providers that reject scope parameter (e.g. Salesforce)

3 participants