From 91a5ce6e5d5f6312f058bf6f2020e13097b930df Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Fri, 17 Jul 2026 23:43:46 +1000 Subject: [PATCH] auth: stop retrying invalid_grant refresh tokens, surface actionable 401s A rotating refresh token consumed by the provider mid-outage (response lost to a connection drop) can never succeed again. The store now records the rejected token, fails fast with ErrReauthRequired instead of calling the token endpoint on every request, and resumes automatically when a new credential is written (panda auth login, or another process's rotation). Request paths that previously sent unauthenticated requests when the token source failed (server proxy relay, proxy client query) now fail fast with 'run panda auth login' instead of relaying the proxy's opaque 'missing or invalid Authorization header' 401. The CLI also attaches a re-auth hint to any 401, which covers older servers. --- pkg/auth/store/store.go | 68 +++++++++++++++++++++++++++-- pkg/auth/store/store_test.go | 85 ++++++++++++++++++++++++++++++++++++ pkg/cli/serverclient.go | 2 + pkg/proxy/client.go | 12 ++++- pkg/server/api.go | 13 +++++- 5 files changed, 173 insertions(+), 7 deletions(-) diff --git a/pkg/auth/store/store.go b/pkg/auth/store/store.go index a710d81a..97a6e2ab 100644 --- a/pkg/auth/store/store.go +++ b/pkg/auth/store/store.go @@ -42,6 +42,12 @@ var ErrCredentialDowngrade = errors.New("refusing to overwrite a refreshable cre // lock wait. The caller should retry shortly rather than clobber the rotation. var ErrCredentialBusy = errors.New("credentials are being refreshed by another process; retry shortly") +// ErrReauthRequired is returned once the provider has rejected the stored +// refresh token as invalid_grant. With a rotating provider this is terminal: +// no retry with the same token can succeed, so the store stops calling the +// token endpoint until a new credential is written (e.g. `panda auth login`). +var ErrReauthRequired = errors.New("re-authentication required: run 'panda auth login'") + // Store manages local credential storage. type Store interface { // Path returns the resolved credentials file path. @@ -107,6 +113,13 @@ type store struct { tokens *client.Tokens forceRefresh bool refreshMu sync.Mutex + + // deadRefreshToken is the refresh token the provider last rejected with + // invalid_grant. While the on-disk credential still carries this token, + // refresh attempts fail fast with ErrReauthRequired instead of calling the + // token endpoint again; a credential written with any other refresh token + // clears it. + deadRefreshToken string } // New creates a new credential store. @@ -247,6 +260,7 @@ func (s *store) writeTokens(tokens *client.Tokens) error { } s.tokens = tokens + s.deadRefreshToken = "" s.log.Debug("Saved credentials") return nil @@ -294,6 +308,7 @@ func (s *store) Clear() error { } s.tokens = nil + s.deadRefreshToken = "" s.log.Debug("Cleared credentials") return nil @@ -349,6 +364,33 @@ func (s *store) forceRefreshRequested() bool { return s.forceRefresh } +// deadRefreshTokenValue returns the refresh token last rejected as +// invalid_grant, or "" when none is recorded. +func (s *store) deadRefreshTokenValue() string { + s.mu.RLock() + defer s.mu.RUnlock() + + return s.deadRefreshToken +} + +// markDeadRefreshToken records a refresh token the provider rejected as +// invalid_grant so later refresh attempts fail fast until a new credential +// is written. +func (s *store) markDeadRefreshToken(refreshToken string) { + s.mu.Lock() + defer s.mu.Unlock() + + s.deadRefreshToken = refreshToken +} + +// clearDeadRefreshToken forgets a recorded invalid_grant rejection. +func (s *store) clearDeadRefreshToken() { + s.mu.Lock() + defer s.mu.Unlock() + + s.deadRefreshToken = "" +} + // clearForceRefresh clears the forced-refresh flag after a successful refresh. func (s *store) clearForceRefresh() { s.mu.Lock() @@ -486,6 +528,21 @@ func (s *store) refresh(prior *client.Tokens) (*client.Tokens, error) { prior = reloaded } + // A refresh token the provider already rejected as invalid_grant can never + // succeed again under rotation; fail fast without another token-endpoint + // call. Any different token on disk means a new credential landed (fresh + // login, or another process won the rotation), which supersedes the + // rejection. + if dead := s.deadRefreshTokenValue(); dead != "" { + if prior.RefreshToken == dead { + s.log.Debug("Refresh token was already rejected as invalid_grant; waiting for re-authentication") + + return nil, ErrReauthRequired + } + + s.clearDeadRefreshToken() + } + priorIssuedAt := prior.RefreshTokenIssuedAt s.log.WithField("expires_at", prior.ExpiresAt.Format(time.RFC3339)).Debug("Refreshing access token") @@ -493,15 +550,18 @@ func (s *store) refresh(prior *client.Tokens) (*client.Tokens, error) { newTokens, err := s.cfg.AuthClient.Refresh(context.Background(), prior.RefreshToken) if err != nil { if isInvalidGrant(err) { + s.markDeadRefreshToken(prior.RefreshToken) s.log.WithError(err).Warn( - "Refresh token rejected (invalid_grant); it was likely rotated by another " + - "refresher sharing these credentials, or revoked — re-authentication may be " + + "Refresh token rejected (invalid_grant); it was rotated by another " + + "refresher sharing these credentials, or revoked — re-authentication is " + "required (panda auth login)", ) - } else { - s.log.WithError(err).Warn("Failed to refresh access token") + + return nil, fmt.Errorf("%w: %v", ErrReauthRequired, err) } + s.log.WithError(err).Warn("Failed to refresh access token") + return nil, fmt.Errorf("refreshing token: %w", err) } diff --git a/pkg/auth/store/store_test.go b/pkg/auth/store/store_test.go index ff6af86b..2f6d53bb 100644 --- a/pkg/auth/store/store_test.go +++ b/pkg/auth/store/store_test.go @@ -331,6 +331,91 @@ func TestInvalidateForcesRefreshThenClears(t *testing.T) { } } +func TestGetAccessTokenStopsRefreshingAfterInvalidGrant(t *testing.T) { + t.Parallel() + + client := &stubAuthClient{refreshErr: errors.New( + `token endpoint returned status 400: {"error": "invalid_grant"}`, + )} + store := New(logrus.New(), Config{ + Path: filepath.Join(t.TempDir(), "creds.json"), + AuthClient: client, + RefreshBuffer: 5 * time.Minute, + }).(*store) + store.tokens = &authclient.Tokens{ + AccessToken: "expired", + RefreshToken: "burned-token", + ExpiresAt: time.Now().Add(-time.Minute), + } + + if _, err := store.GetAccessToken(); !errors.Is(err, ErrReauthRequired) { + t.Fatalf("expected ErrReauthRequired, got %v", err) + } + + if client.refreshCalls != 1 { + t.Fatalf("expected 1 refresh attempt, got %d", client.refreshCalls) + } + + // Retrying with the same rejected token must not hit the provider again. + if _, err := store.GetAccessToken(); !errors.Is(err, ErrReauthRequired) { + t.Fatalf("expected ErrReauthRequired on retry, got %v", err) + } + + if client.refreshCalls != 1 { + t.Fatalf("expected no further refresh attempts, got %d", client.refreshCalls) + } +} + +func TestGetAccessTokenRecoversWhenNewCredentialWritten(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "creds.json") + + client := &stubAuthClient{refreshErr: errors.New( + `token endpoint returned status 400: {"error": "invalid_grant"}`, + )} + store := New(logrus.New(), Config{ + Path: path, + AuthClient: client, + RefreshBuffer: 5 * time.Minute, + }).(*store) + store.tokens = &authclient.Tokens{ + AccessToken: "expired", + RefreshToken: "burned-token", + ExpiresAt: time.Now().Add(-time.Minute), + } + + if _, err := store.GetAccessToken(); !errors.Is(err, ErrReauthRequired) { + t.Fatalf("expected ErrReauthRequired, got %v", err) + } + + // Another process (e.g. the host CLI running `panda auth login`) writes a + // fresh credential to the shared file. + other := New(logrus.New(), Config{Path: path}) + if err := other.Save(&authclient.Tokens{ + AccessToken: "expired-too", + RefreshToken: "fresh-token", + ExpiresAt: time.Now().Add(-time.Minute), + }); err != nil { + t.Fatalf("saving replacement credential: %v", err) + } + + client.refreshErr = nil + + token, err := store.GetAccessToken() + if err != nil { + t.Fatalf("GetAccessToken after new credential: %v", err) + } + + if token != "refreshed-token" { + t.Fatalf("expected refreshed token, got %q", token) + } + + if client.refreshCalls != 2 { + t.Fatalf("expected refresh to resume with the new token, got %d calls", client.refreshCalls) + } +} + type stubAuthClient struct { refreshCalls int refreshErr error diff --git a/pkg/cli/serverclient.go b/pkg/cli/serverclient.go index 73e58b7f..6a5ce2a8 100644 --- a/pkg/cli/serverclient.go +++ b/pkg/cli/serverclient.go @@ -296,6 +296,8 @@ func serverErrorHint(status int, message string) string { } switch status { + case http.StatusUnauthorized: + return "the panda proxy rejected or is missing your credential; run 'panda auth login' to re-authenticate, then retry" case http.StatusNotFound: return "the requested module, operation, datasource, or resource is not available on this server; check 'panda datasources' and 'panda resources'" case http.StatusBadGateway: diff --git a/pkg/proxy/client.go b/pkg/proxy/client.go index 9f2e317e..69177bb7 100644 --- a/pkg/proxy/client.go +++ b/pkg/proxy/client.go @@ -380,8 +380,16 @@ func (c *proxyClient) ClickHouseQuery(ctx context.Context, datasource, sql strin req.Header.Set(attribution.Header, v) } - if token := c.RegisterToken(); token != "" && token != NoAuthToken { - req.Header.Set("Authorization", "Bearer "+token) + // Fail fast when auth is configured but no token is available: an + // unauthenticated request would only surface the proxy's opaque 401 + // instead of the actionable credential error. + tok, err := c.accessToken(ctx) + if err != nil { + return 0, nil, err + } + + if tok != "" { + req.Header.Set("Authorization", "Bearer "+tok) } resp, err := c.queryHTTPClient.Do(req) diff --git a/pkg/server/api.go b/pkg/server/api.go index 7e9b138b..3f9918c1 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -850,7 +850,18 @@ func (s *service) proxyRequestWithService( req.Header.Set(attribution.Header, v) } - if token := proxySvc.RegisterToken(); token != "" && token != proxy.NoAuthToken { + // An empty token means auth is configured but no credential is usable + // (expired, revoked, or never logged in). Forwarding unauthenticated + // would only surface the proxy's opaque 401; fail fast with the action + // the user can take instead. + switch token := proxySvc.RegisterToken(); token { + case proxy.NoAuthToken: + case "": + return nil, http.StatusUnauthorized, nil, fmt.Errorf( + "%w: credentials are missing or expired — run 'panda auth login'", + proxy.ErrAuthenticationRequired, + ) + default: req.Header.Set("Authorization", "Bearer "+token) }