Skip to content

Commit d269fdb

Browse files
aauschclauderenaudhartert-db
authored
fix(auth/m2m): remove double-caching to enable proactive token refresh (#1550)
Fixes a double-caching bug in M2M OAuth that caused the proactive async token refresh to have no effect, resulting in bursts of HTTP 401 or 403 errors at each token rotation boundary (~every hour). Closes #1549. ## Why `M2mCredentials.Configure` previously called `clientcredentials.Config.TokenSource(ctx)`, which returns an `oauth2.ReuseTokenSource`. That source was then passed to `refreshableVisitor`, which wraps it in a `cachedTokenSource`. The resulting stack is: ``` cachedTokenSource ← Databricks async cache (proactive, T-20min) └── authTokenSource ← discards context └── oauth2.ReuseTokenSource ← Go stdlib cache, expiryDelta=10s └── clientcredentials.tokenSource ← HTTP /token endpoint ``` When `cachedTokenSource` triggers its async refresh at T−20 min, it calls through to `ReuseTokenSource.Token()`. Because the token still has 20 minutes of life, `ReuseTokenSource` considers it valid and returns the cached token without an HTTP call. The async refresh fires repeatedly (with an accelerating schedule) but each call hits the same inner cache — until only ~10 s remain, at which point `ReuseTokenSource`'s `expiryDelta` window is crossed and a real network call is finally made. Any request that receives the about-to-expire token and whose round-trip to Databricks completes after the token's expiry time gets an auth error. In production this manifests as a burst of errors at precisely one-hour intervals, correlated with pod startup times. **Note on status codes**: the exact HTTP status code returned for an expired M2M OAuth token varies by workspace and request type. A completely invalid token returns 401 on all endpoints. A naturally expired token has been observed to return 403 `"Invalid Token"` on both management endpoints (`GET /api/2.0/serving-endpoints/{name}`) and inference endpoints (`POST /serving-endpoints/{name}/invocations`). Local workarounds that intercept auth errors at the transport layer need to handle both 401 and 403. ## What changed ### Interface changes None. ### Behavioral changes - M2M OAuth token refresh now makes a real HTTP call to the token endpoint at T−20 min (the intended proactive window) rather than at T−10 s. - Callers will see one additional token endpoint call per hour per process (the proactive refresh), offset by eliminating the burst of auth errors at expiry. ### Internal changes - `auth_m2m.go`: replaced `clientcredentials.Config.TokenSource(ctx)` (which returns a `ReuseTokenSource`) with a `TokenSourceFn` closure that calls `ccfg.Token(ctx)` directly. `cachedTokenSource` is now the sole caching layer. - `cache_test.go`: added `reuseTokenSource` test helper and two new tests: - `TestCachedTokenSource_AsyncRefreshBlockedByInnerCache` — documents the double-caching behaviour using a mock clock. - `TestCachedTokenSource_AsyncRefreshWithDirectSource` — verifies that a direct (non-caching) inner source triggers an HTTP fetch at the proactive window. - `NEXT_CHANGELOG.md`: updated. ## How is this tested? - `TestCachedTokenSource_AsyncRefreshBlockedByInnerCache`: uses a fake clock and a controlled `reuseTokenSource` helper to confirm that inner caching delays the HTTP fetch to T−10 s rather than T−20 min. - `TestCachedTokenSource_AsyncRefreshWithDirectSource`: uses the same fake clock with a direct token source to confirm the fetch occurs at T−20 min after the fix. - Both tests operate at the token-source level using a mock clock — they count real HTTP fetches to the token endpoint rather than checking the status code returned by Databricks API calls. This means they validate the fix regardless of whether the downstream auth failure manifests as 401 or 403. - Existing `TestM2mHappyFlow` and `TestM2mHappyFlowForAccount` continue to pass. --------- Signed-off-by: Alex Ausch <alex@ausch.name> Signed-off-by: Ubuntu <renaud.hartert@databricks.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Renaud Hartert <renaud.hartert@databricks.com>
1 parent 4415065 commit d269fdb

4 files changed

Lines changed: 103 additions & 5 deletions

File tree

NEXT_CHANGELOG.md

100755100644
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515

1616
* Fix double-caching of OAuth tokens in Azure client secret credentials ([#1549](https://github.com/databricks/databricks-sdk-go/issues/1549)).
1717
* Disable async token refresh for GCP credential providers to avoid wasted refresh attempts caused by double-caching with Google's internal `oauth2.ReuseTokenSource` ([#1549](https://github.com/databricks/databricks-sdk-go/issues/1549)).
18+
* Fixed double-caching in M2M OAuth that prevented the proactive async token refresh from reaching the HTTP endpoint until ~10s before expiry, causing bursts of 401 errors at token rotation boundaries ([#1549](https://github.com/databricks/databricks-sdk-go/issues/1549)).
1819

1920
### Documentation
2021

2122
### Internal Changes
2223

2324
* Normalize internal token sources on `auth.TokenSource` for proper context propagation ([#1577](https://github.com/databricks/databricks-sdk-go/pull/1577)).
25+
* Fix `TestAzureGithubOIDCCredentials` hang caused by missing `HTTPTransport` stub: `EnsureResolved` now calls `resolveHostMetadata`, which makes a real network request when no transport is set ([#1550](https://github.com/databricks/databricks-sdk-go/pull/1550)).
2426
* Bump golang.org/x/crypto from 0.21.0 to 0.45.0 in /examples/slog ([#1566](https://github.com/databricks/databricks-sdk-go/pull/1566)).
2527
* Bump golang.org/x/net from 0.23.0 to 0.33.0 in /examples/slog ([#1127](https://github.com/databricks/databricks-sdk-go/pull/1127)).
2628
* Bump golang.org/x/oauth2 from 0.20.0 to 0.27.0 ([#1563](https://github.com/databricks/databricks-sdk-go/pull/1563)).

config/auth_azure_github_oidc_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func TestAzureGithubOIDCCredentials(t *testing.T) {
4949
AzureTenantID: "test-tenant-id",
5050
ActionsIDTokenRequestURL: "http://endpoint.com/test?version=1",
5151
ActionsIDTokenRequestToken: "token-1337",
52+
HTTPTransport: fixtures.MappingTransport{},
5253
},
5354
wantErrPrefix: errPrefix("github-oidc-azure auth: not configured"),
5455
},
@@ -79,6 +80,7 @@ func TestAzureGithubOIDCCredentials(t *testing.T) {
7980
AzureClientID: "test-client-id",
8081
ActionsIDTokenRequestURL: "http://endpoint.com/test?version=1",
8182
ActionsIDTokenRequestToken: "token-1337",
83+
HTTPTransport: fixtures.MappingTransport{},
8284
},
8385
wantErrPrefix: errPrefix("github-oidc-azure auth: not configured"),
8486
},
@@ -94,6 +96,7 @@ func TestAzureGithubOIDCCredentials(t *testing.T) {
9496
AzureTenantID: "test-tenant-id",
9597
Host: "http://host.com/test",
9698
ActionsIDTokenRequestURL: "http://endpoint.com/test?version=1",
99+
HTTPTransport: fixtures.MappingTransport{},
97100
},
98101
wantErrPrefix: errPrefix("github-oidc-azure auth: not configured"),
99102
},
@@ -109,6 +112,7 @@ func TestAzureGithubOIDCCredentials(t *testing.T) {
109112
AzureTenantID: "test-tenant-id",
110113
Host: "http://host.com/test",
111114
ActionsIDTokenRequestToken: "token-1337",
115+
HTTPTransport: fixtures.MappingTransport{},
112116
},
113117
wantErrPrefix: errPrefix("github-oidc-azure auth: not configured"),
114118
},

config/auth_m2m.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/databricks/databricks-sdk-go/config/credentials"
1111
"github.com/databricks/databricks-sdk-go/config/experimental/auth"
12-
"github.com/databricks/databricks-sdk-go/config/experimental/auth/authconv"
1312
"github.com/databricks/databricks-sdk-go/logger"
1413
)
1514

@@ -28,16 +27,29 @@ func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials
2827
return nil, fmt.Errorf("oidc: %w", err)
2928
}
3029
logger.Debugf(ctx, "Generating Databricks OAuth token for Service Principal (%s)", cfg.ClientID)
31-
ts := (&clientcredentials.Config{
30+
ccfg := &clientcredentials.Config{
3231
ClientID: cfg.ClientID,
3332
ClientSecret: cfg.ClientSecret,
3433
AuthStyle: oauth2.AuthStyleInHeader,
3534
TokenURL: endpoints.TokenEndpoint,
3635
Scopes: cfg.GetScopes(),
37-
}).TokenSource(ctx)
36+
}
3837

39-
authTs := authconv.AuthTokenSource(ts)
38+
// Use a direct (non-caching) token source so that cachedTokenSource is the
39+
// single cache layer. clientcredentials.Config.TokenSource returns an
40+
// oauth2.ReuseTokenSource which adds a second cache with a 10 s expiryDelta.
41+
// With double-caching the async refresh in cachedTokenSource calls through to
42+
// ReuseTokenSource, which returns its own cached token without making an HTTP
43+
// request until only ~10 s remain — defeating the proactive 20-min refresh
44+
// window and causing a burst of 401s at token expiry.
45+
// See https://github.com/databricks/databricks-sdk-go/issues/1549.
46+
//
47+
// ctx is captured from Configure; the oauth2 HTTP client is bound to it via
48+
// InContextForOAuth2 and must be used for all token requests.
49+
directTS := auth.TokenSourceFn(func(_ context.Context) (*oauth2.Token, error) {
50+
return ccfg.Token(ctx)
51+
})
4052
return credentials.NewOAuthCredentialsProviderFromTokenSource(
41-
auth.NewCachedTokenSource(authTs, cacheOptions(cfg)...),
53+
auth.NewCachedTokenSource(directTS, cacheOptions(cfg)...),
4254
), nil
4355
}

config/auth_m2m_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package config
22

33
import (
4+
"context"
45
"errors"
56
"net/http"
67
"net/url"
8+
"sync/atomic"
79
"testing"
810

11+
"github.com/databricks/databricks-sdk-go/config/credentials"
912
"github.com/databricks/databricks-sdk-go/credentials/u2m"
1013
"github.com/databricks/databricks-sdk-go/httpclient/fixtures"
1114
"golang.org/x/oauth2"
@@ -96,6 +99,83 @@ func TestM2mNotSupported(t *testing.T) {
9699
}
97100
}
98101

102+
// TestM2mCredentials_DirectTokenSource verifies that M2mCredentials.Configure
103+
// plumbs a direct token source (ccfg.Token) through cachedTokenSource rather
104+
// than wrapping clientcredentials.Config.TokenSource, which returns an
105+
// oauth2.ReuseTokenSource that adds a second cache layer. With double-caching,
106+
// the proactive async refresh in cachedTokenSource is silently suppressed until
107+
// ~10 s before expiry, causing bursts of 401 errors at token rotation boundaries.
108+
// See https://github.com/databricks/databricks-sdk-go/issues/1549.
109+
func TestM2mCredentials_DirectTokenSource(t *testing.T) {
110+
var tokenCalls int32
111+
transport := &postCountingTransport{
112+
calls: &tokenCalls,
113+
inner: fixtures.MappingTransport{
114+
"GET /oidc/.well-known/oauth-authorization-server": {
115+
Response: u2m.OAuthAuthorizationServer{
116+
TokenEndpoint: "https://localhost/token",
117+
},
118+
},
119+
"POST /token": {
120+
Response: oauth2.Token{
121+
TokenType: "Bearer",
122+
AccessToken: "test-token",
123+
},
124+
},
125+
},
126+
}
127+
128+
cfg := &Config{
129+
Host: "a",
130+
ClientID: "b",
131+
ClientSecret: "c",
132+
AuthType: "oauth-m2m",
133+
ConfigFile: "/dev/null",
134+
HTTPTransport: transport,
135+
}
136+
137+
err := cfg.EnsureResolved()
138+
if err != nil {
139+
t.Fatalf("EnsureResolved(): %v", err)
140+
}
141+
142+
ctx := cfg.refreshClient.InContextForOAuth2(cfg.refreshCtx)
143+
provider, err := M2mCredentials{}.Configure(ctx, cfg)
144+
if err != nil {
145+
t.Fatalf("Configure(): %v", err)
146+
}
147+
148+
oauthProvider := provider.(credentials.OAuthCredentialsProvider)
149+
150+
// Token() goes through cachedTokenSource, which fetches once and caches.
151+
// Verify the endpoint is reached (not short-circuited by an inner cache).
152+
tok, err := oauthProvider.Token(context.Background())
153+
if err != nil {
154+
t.Fatalf("Token(): %v", err)
155+
}
156+
if tok.AccessToken == "" {
157+
t.Fatalf("Token(): empty access token")
158+
}
159+
160+
if got := int(atomic.LoadInt32(&tokenCalls)); got != 1 {
161+
t.Errorf("token endpoint calls = %d, want 1", got)
162+
}
163+
}
164+
165+
type postCountingTransport struct {
166+
calls *int32
167+
inner http.RoundTripper
168+
}
169+
170+
func (t *postCountingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
171+
if req.Method == "POST" {
172+
atomic.AddInt32(t.calls, 1)
173+
}
174+
return t.inner.RoundTrip(req)
175+
}
176+
177+
func (t *postCountingTransport) SkipRetryOnIO() bool { return true }
178+
99179
func TestM2M_Scopes(t *testing.T) {
100180
tests := []struct {
101181
name string

0 commit comments

Comments
 (0)