Fix double-caching of OAuth tokens in Azure client secret credentials#1573
Merged
renaudhartert-db merged 1 commit intomainfrom Mar 24, 2026
Merged
Conversation
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 22, 2026
## Changes Fixes #1549. Related: #1550, #1573. Google's Cloud Go libraries (`impersonate.IDTokenSource`, `impersonate.CredentialsTokenSource`, `idtoken.NewTokenSource`, `google.CredentialsFromJSON`) all internally wrap their token sources in `oauth2.ReuseTokenSource`. This means the SDK's `cachedTokenSource` async refresh is swallowed by the inner `ReuseTokenSource` cache, making it entirely wasted work. This is the same double-caching bug as M2M OAuth (#1549). Though, unlike M2M (#1550) and Azure Client Secret (#1573), Google's libraries don't expose an uncached `Token(ctx)` method -- the inner sources are unexported types. Recreating the token source on each call is expensive and will require further considerations. The pragmatic fix is to disable async refresh for both GCP providers: - `GoogleDefaultCredentials` (`config/auth_gcp_google_id.go`) - `GoogleCredentials` (`config/auth_gcp_google_credentials.go`) Google's own `ReuseTokenSource` (with a 10-second early refresh window) still provides token renewal before expiry. Thorough comments explain the rationale and warn against re-enabling async refresh. ## Tests Existing `config` tests pass (`go test ./config/ -v -count=1`). No new tests needed -- this is a configuration change (appending `auth.WithAsyncRefresh(false)` to cache options) and the async refresh mechanism itself is already well-tested. --------- Signed-off-by: Ubuntu <renaud.hartert@databricks.com> Co-authored-by: simon <simon.faltum@databricks.com>
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
The tokenSourceFor method previously used clientcredentials.Config.TokenSource which wraps the result in oauth2.ReuseTokenSource with a 10s expiryDelta. This inner cache swallowed proactive refresh attempts from azureReuseTokenSource and serviceToServiceVisitor, causing bursts of 401s at token expiry. Use auth.TokenSourceFn with clientcredentials.Config.Token instead, so that caching is purely controlled by the outer layers. Fixes #1549. Co-authored-by: Isaac Signed-off-by: Ubuntu <renaud.hartert@databricks.com>
renaudhartert-db
force-pushed
the
fix-azure-client-secret-double-caching
branch
from
f8ca20e to
555ecd6
Compare
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
mihaimitrea-db
approved these changes
Mar 24, 2026
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
2 participants
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.
Summary
Fixes the double-caching of OAuth tokens in Azure client secret credentials (#1549) and unifies the internal token source infrastructure on
auth.TokenSource.Why
AzureClientSecretCredentials.tokenSourceForcalledclientcredentials.Config{}.TokenSource(ctx), which returns anoauth2.ReuseTokenSourcewith a hardcoded 10-secondexpiryDelta. This token source was then wrapped inazureReuseTokenSource(acachedTokenSource) and again inserviceToServiceVisitor(anothercachedTokenSource). The innerReuseTokenSourceswallowed proactive refresh attempts from the outer layers -- it kept returning its cached token until only ~10 seconds remained before expiry, defeating the 20-minute async refresh window and causing bursts of 401s at token expiry.This is the same class of bug as #1550 (M2M OAuth), but in the Azure client secret path.
What changed
Interface changes
None. All changes are to unexported types and functions.
Behavioral changes
AzureClientSecretCredentialsno longer double-caches tokens. Each call to the underlying token source results in an HTTP request to the token endpoint; caching is purely controlled by the outerazureReuseTokenSourceandserviceToServiceVisitorlayers.Internal changes
azureHostResolverinterface now returnsauth.TokenSourceinstead ofoauth2.TokenSource, and no longer takes acontext.Contextparameter. Context is passed per-call viaToken(ctx).serviceToServiceVisitorandrefreshableVisitornow acceptauth.TokenSourcedirectly, removing theauthconv.AuthTokenSourcewrapping at call sites.azureReuseTokenSourceandwrapoperate onauth.TokenSourceinstead ofoauth2.TokenSource, eliminating theauthconvround-trip.AzureClientSecretCredentials: removed the duplicatetokenSourceFor(which usedclientcredentials.Config.TokenSourcewith inner caching). The formerauthTokenSourceFor-- which callsclientcredentials.Config.Token(ctx)without inner caching -- is now the soletokenSourceFor.AzureCliCredentials.tokenSourceFor: returns anauth.TokenSourcethat creates a freshazureCliTokenSourceper call, properly threading the caller's context.AzureMsiCredentials.tokenSourceFor: wrapsNewAzureMsiTokenSourcewithauthconv.AuthTokenSource.oauth2.TokenSourcewithauthconv.AuthTokenSourcebefore passing to the visitor functions.How is this tested?
TestAzureClientSecretCredentials_tokenSourceFor_noCaching: callsToken()N times and asserts N HTTP requests hit the token endpoint. Fails when the fix is reverted (inner caching collapses N calls into 1).TestAzureClientSecretCredentials_Configure: happy-path test for the full Azure client secret credential flow.config/tests pass.