diff --git a/cmd/auth.go b/cmd/auth.go index bdb5f42..2059027 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -3,6 +3,7 @@ package cmd import ( "github.com/MakeNowJust/heredoc" "github.com/gleanwork/glean-cli/internal/auth" + "github.com/gleanwork/glean-cli/internal/client" "github.com/spf13/cobra" ) @@ -64,7 +65,7 @@ func newAuthStatusCmd() *cobra.Command { Use: "status", Short: "Show current authentication status", RunE: func(cmd *cobra.Command, args []string) error { - return auth.Status(cmd.Context()) + return auth.Status(cmd.Context(), client.ValidateToken) }, SilenceUsage: true, } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 90739b5..5b885af 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -130,7 +130,14 @@ func Login(ctx context.Context) error { // persistLoginState stores the resolved host in config and persists OAuth tokens. // Saving the host here ensures a successful `glean auth login` remains usable // even when the host originally came from an environment variable. +// +// It also clears any existing API token from storage so that the new OAuth +// credentials take effect immediately — a stale API token in config would +// otherwise shadow the fresh OAuth token (ResolveToken prefers API tokens). func persistLoginState(host string, tok *StoredTokens) error { + if err := config.ClearTokenFromStorage(); err != nil { + return fmt.Errorf("clearing stale API token: %w", err) + } if err := config.SaveHostToFile(host); err != nil { return fmt.Errorf("saving host: %w", err) } @@ -160,8 +167,13 @@ func Logout(ctx context.Context) error { return nil } +// TokenValidator validates credentials in a config against the Glean backend. +// It returns nil when the token is accepted, or an error describing the failure. +type TokenValidator func(ctx context.Context, cfg *config.Config) error + // Status prints the current authentication state. -func Status(ctx context.Context) error { +// validateToken is used to verify API tokens against the backend (typically client.ValidateToken). +func Status(ctx context.Context, validateToken TokenValidator) error { cfg, _ := config.LoadConfig() if cfg == nil || cfg.GleanHost == "" { fmt.Println("Not configured.") @@ -171,6 +183,11 @@ func Status(ctx context.Context) error { if cfg.GleanToken != "" { masked := config.MaskToken(cfg.GleanToken) + if err := validateToken(ctx, cfg); err != nil { + fmt.Printf("✗ API token is invalid or expired\n Host: %s\n Token: %s\n Error: %v\n", cfg.GleanHost, masked, err) + fmt.Println("Run 'glean auth login' to re-authenticate.") + return nil + } fmt.Printf("✓ Authenticated via API token\n Host: %s\n Token: %s\n", cfg.GleanHost, masked) return nil } diff --git a/internal/auth/auth_persistence_test.go b/internal/auth/auth_persistence_test.go index 21f2000..a8b86a7 100644 --- a/internal/auth/auth_persistence_test.go +++ b/internal/auth/auth_persistence_test.go @@ -88,6 +88,37 @@ func TestShortFormHostNormalizesConsistently(t *testing.T) { assert.Equal(t, "OAUTH", authType) } +func TestStaleAPITokenClearedOnOAuthLogin(t *testing.T) { + authtest.IsolateAuthState(t) + + const host = "acme-be.glean.com" + + // Simulate a stale API token in config. + require.NoError(t, config.SaveConfig(host, "stale-api-token")) + + cfg, err := config.LoadConfig() + require.NoError(t, err) + assert.Equal(t, "stale-api-token", cfg.GleanToken, "precondition: stale token exists") + + // Simulate what persistLoginState does (called during OAuth login). + // We can't call Login() directly since it requires a browser, but + // persistLoginState is the function that should clear stale tokens. + require.NoError(t, config.ClearTokenFromStorage()) + require.NoError(t, config.SaveHostToFile(host)) + require.NoError(t, auth.SaveTokens(host, oauthToken())) + + // After OAuth login, the stale API token should be gone. + cfg, err = config.LoadConfig() + require.NoError(t, err) + assert.Empty(t, cfg.GleanToken, "stale API token should be cleared after OAuth login") + assert.Equal(t, host, cfg.GleanHost, "host should remain") + + // OAuth token should now be resolvable. + token, authType := gleanClient.ResolveToken(cfg) + assert.Equal(t, "oauth-access-token", token) + assert.Equal(t, "OAUTH", authType) +} + func TestLogoutClearsPersistedHostAndOAuthTokens(t *testing.T) { authtest.IsolateAuthState(t) diff --git a/internal/client/client.go b/internal/client/client.go index 8683e1e..87644fc 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -4,9 +4,12 @@ package client import ( + "context" "fmt" + "io" "net/http" "strings" + "time" glean "github.com/gleanwork/api-client-go" "github.com/gleanwork/glean-cli/internal/auth" @@ -31,6 +34,47 @@ func ResolveToken(cfg *config.Config) (token, authType string) { return "", "" } +// ValidateToken makes a lightweight POST /rest/api/v1/search request to +// verify that the resolved token is accepted by the Glean backend. Returns nil +// if the token is valid, or an error describing the failure. +func ValidateToken(ctx context.Context, cfg *config.Config) error { + token, authType := ResolveToken(cfg) + if token == "" { + return fmt.Errorf("no token available") + } + + url := "https://" + cfg.GleanHost + "/rest/api/v1/search" + body := strings.NewReader(`{"query":"","pageSize":1}`) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, body) + if err != nil { + return fmt.Errorf("creating request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + if authType != "" { + req.Header.Set("X-Glean-Auth-Type", authType) + } + + resp, err := httputil.NewHTTPClient(10 * time.Second).Do(req) + if err != nil { + return fmt.Errorf("validating token: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + // Try to surface the server's error message (e.g. "Token has expired"). + respBody, _ := io.ReadAll(resp.Body) + if msg := strings.TrimSpace(string(respBody)); msg != "" { + return fmt.Errorf("%s (HTTP %d)", msg, resp.StatusCode) + } + return fmt.Errorf("token rejected by server (HTTP %d)", resp.StatusCode) + } + if resp.StatusCode >= 400 { + return fmt.Errorf("unexpected status validating token (HTTP %d)", resp.StatusCode) + } + return nil +} + // New creates an authenticated Glean SDK client from the loaded configuration. // // Authentication priority: diff --git a/internal/client/client_test.go b/internal/client/client_test.go index ed62f54..4c462f8 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -1,6 +1,7 @@ package client import ( + "context" "io" "net/http" "strings" @@ -92,6 +93,20 @@ func TestExtractInstance(t *testing.T) { } } +func TestValidateToken_NoToken(t *testing.T) { + cfg := &config.Config{GleanHost: "test-be.glean.com", GleanToken: ""} + err := ValidateToken(context.Background(), cfg) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no token available") +} + +func TestValidateToken_Unreachable(t *testing.T) { + cfg := &config.Config{GleanHost: "localhost:1", GleanToken: "some-token"} + err := ValidateToken(context.Background(), cfg) + assert.Error(t, err) + assert.Contains(t, err.Error(), "validating token") +} + func TestNew_EmptyHost(t *testing.T) { cfg := &config.Config{GleanHost: "", GleanToken: "some-token"} _, err := New(cfg) diff --git a/internal/config/config.go b/internal/config/config.go index 963a5a8..9692b80 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -179,6 +179,29 @@ func SaveHostToFile(host string) error { return saveToFile(cfg) } +// ClearTokenFromStorage removes only the API token from keyring and config file, +// leaving the host and other settings intact. This is used during OAuth login to +// prevent a stale API token from shadowing newly obtained OAuth credentials. +func ClearTokenFromStorage() error { + // Remove token from keyring (ignore not-found). + if err := keyringImpl.Delete(ServiceName, tokenKey); err != nil && err != keyring.ErrNotFound { + return fmt.Errorf("error clearing token from keyring: %w", err) + } + + // Remove token from config file while preserving other fields. + cfg, err := loadFromFile() + if err != nil { + return nil // no file to update + } + if cfg.GleanToken != "" { + cfg.GleanToken = "" + if err := saveToFile(cfg); err != nil { + return fmt.Errorf("error clearing token from config file: %w", err) + } + } + return nil +} + // ClearConfig removes all stored configuration from both keyring and file storage. func ClearConfig() error { var keyringErr error diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 811e4e8..0d9a202 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -112,6 +112,27 @@ func setupTestConfig(t *testing.T) (string, func()) { } } +// isolateAuthState redirects config path, service name, and keyring to +// temporary locations so tests never touch real credentials. +// This mirrors authtest.IsolateAuthState but lives in the config package +// to avoid a circular import (authtest imports config). +func isolateAuthState(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + + oldConfigPath := ConfigPath + ConfigPath = filepath.Join(home, ".glean", "config.json") + t.Cleanup(func() { ConfigPath = oldConfigPath }) + + oldServiceName := ServiceName + ServiceName = "glean-cli-test-isolated" + t.Cleanup(func() { ServiceName = oldServiceName }) + + keyring.MockInit() +} + func TestValidateAndTransformHost(t *testing.T) { tests := []struct { name string @@ -257,11 +278,41 @@ func TestConfigOperations(t *testing.T) { }) } +func TestClearTokenFromStorage(t *testing.T) { + isolateAuthState(t) + + t.Run("clears token but preserves host", func(t *testing.T) { + require.NoError(t, SaveConfig("linkedin", "stale-api-token")) + + cfg, err := LoadConfig() + require.NoError(t, err) + assert.Equal(t, "stale-api-token", cfg.GleanToken) + + require.NoError(t, ClearTokenFromStorage()) + + cfg, err = LoadConfig() + require.NoError(t, err) + assert.Empty(t, cfg.GleanToken, "token should be cleared") + assert.Equal(t, "linkedin-be.glean.com", cfg.GleanHost, "host should be preserved") + }) + + t.Run("no-op when no token exists", func(t *testing.T) { + // Clear state from previous subtest. + _ = ClearConfig() + + require.NoError(t, SaveHostToFile("acme")) + + require.NoError(t, ClearTokenFromStorage()) + + cfg, err := LoadConfig() + require.NoError(t, err) + assert.Empty(t, cfg.GleanToken) + assert.Equal(t, "acme-be.glean.com", cfg.GleanHost) + }) +} + func TestLoadConfigEnvPriority(t *testing.T) { - _, cleanupKeyring := setupTestKeyring(t) - _, cleanupConfig := setupTestConfig(t) - defer cleanupKeyring() - defer cleanupConfig() + isolateAuthState(t) t.Run("GLEAN_API_TOKEN overrides keyring", func(t *testing.T) { t.Setenv("GLEAN_API_TOKEN", "env-token") @@ -283,10 +334,7 @@ func TestLoadConfigEnvPriority(t *testing.T) { } func TestLoadConfig_EnvTokenWithKeyringHost(t *testing.T) { - _, cleanupKeyring := setupTestKeyring(t) - _, cleanupConfig := setupTestConfig(t) - defer cleanupKeyring() - defer cleanupConfig() + isolateAuthState(t) err := SaveConfig("myhost.glean.com", "") require.NoError(t, err) @@ -299,10 +347,7 @@ func TestLoadConfig_EnvTokenWithKeyringHost(t *testing.T) { } func TestLoadConfig_EnvHostWithFileToken(t *testing.T) { - _, cleanupKeyring := setupTestKeyring(t) - _, cleanupConfig := setupTestConfig(t) - defer cleanupKeyring() - defer cleanupConfig() + isolateAuthState(t) err := saveToFile(&Config{GleanToken: "file-token"}) require.NoError(t, err)