fix(auth/m2m): remove double-caching to enable proactive token refresh

[aausch]

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.

[renaudhartert-db]

Thanks for the thorough analysis and the fix! The double-caching problem is well explained and the root cause is clearly correct.

I think the tests need to change though. The two tests in cache_test.go demonstrate the bug at the cachedTokenSource level, but they don't exercise the fix in auth_m2m.go. Could we rather have a test that goes through M2mCredentials.Configure and verifies the returned token source is direct (no inner caching)?

We can mock the HTTP layer through Config.HTTPTransport. Here's what I have in mind — I've tested locally, it passes with the fix and fails without:

func TestM2mCredentials_DirectTokenSource(t *testing.T) {
	var tokenCalls int32
	transport := &tokenCountingTransport{
		calls: &tokenCalls,
		inner: fixtures.MappingTransport{
			"GET /oidc/.well-known/oauth-authorization-server": {
				Response: u2m.OAuthAuthorizationServer{
					TokenEndpoint: "https://localhost/token",
				},
			},
			"POST /token": {
				Response: oauth2.Token{
					TokenType:   "Bearer",
					AccessToken: "test-token",
				},
			},
		},
	}

	cfg := &Config{
		Host:                     "a",
		ClientID:                 "b",
		ClientSecret:             "c",
		AuthType:                 "oauth-m2m",
		ConfigFile:               "/dev/null",
		DisableAsyncTokenRefresh: true,
		HTTPTransport:            transport,
	}

	err := cfg.EnsureResolved()
	if err != nil {
		t.Fatalf("EnsureResolved(): %v", err)
	}

	ctx := cfg.refreshClient.InContextForOAuth2(cfg.refreshCtx)
	provider, err := M2mCredentials{}.Configure(ctx, cfg)
	if err != nil {
		t.Fatalf("Configure(): %v", err)
	}

	oauthProvider := provider.(credentials.OAuthCredentialsProvider)

	// Call Token() 3 times. With the fix, each call hits the endpoint.
	// Without the fix (ReuseTokenSource), only the first call does.
	const nCalls = 3
	for i := 0; i < nCalls; i++ {
		tok, err := oauthProvider.Token(context.Background())
		if err != nil {
			t.Fatalf("Token() call %d: %v", i+1, err)
		}
		if tok.AccessToken == "" {
			t.Fatalf("Token() call %d: empty access token", i+1)
		}
	}

	got := int(atomic.LoadInt32(&tokenCalls))
	if got != nCalls {
		t.Errorf("token endpoint calls = %d, want %d", got, nCalls)
	}
}

type tokenCountingTransport struct {
	calls *int32
	inner http.RoundTripper
}

func (t *tokenCountingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	if req.Method == "POST" {
		atomic.AddInt32(t.calls, 1)
	}
	return t.inner.RoundTrip(req)
}

func (t *tokenCountingTransport) SkipRetryOnIO() bool { return true }

Would you prefer to adjust the tests, or would it be easier for us to take over the PR from here? Happy either way.

[aausch]

Would you prefer to adjust the tests, or would it be easier for us to take over the PR from here? Happy either way.

Thanks for the review and for catching that! I gave the fix a whirl. I think the tests are correct now?

[renaudhartert-db]

LGTM! Could you please handle the fmt issue and the merge conflict? I'll trigger the integration test and complete the merge afterward. Thanks!

[aausch]

Added a fourth commit (f3dc334) that fixes a pre-existing test hang I found while running the full suite locally.

Root cause: EnsureResolved now calls resolveHostMetadata (introduced in #1572), which makes a real network request when no HTTPTransport is set. Four test cases in TestAzureGithubOIDCCredentials had a Host but no HTTPTransport, so they would hang indefinitely waiting for a connection to http://host.com/.well-known/databricks-config.

Fix: Add HTTPTransport: fixtures.MappingTransport{} to those four cases. An empty MappingTransport returns a fast "missing stub" error, which resolveHostMetadata already handles gracefully (logs a warning and falls back to user config). The tests now complete in milliseconds.

This is unrelated to the M2M double-caching fix but reproducible on the current main.

(also rebased on main and force-pushed, to keep the commit logs clean)

[github-actions]

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-go

Inputs:

• PR number: 1550
• Commit SHA: ce9e0d6ff5c4a84163141fa5431bf9e07c8a7e3c

Checks will be approved automatically on success.