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) }