From db19be1208af79666a7f028cf510f0880cd8ef18 Mon Sep 17 00:00:00 2001 From: Ory Medina Date: Mon, 20 Jul 2026 17:50:16 -0600 Subject: [PATCH] [TOL-2311] Add --force to logout paths in case revoke on server fails --- README.md | 2 + internal/cli/auth.go | 37 ++++-- internal/cli/cli_test.go | 62 +++++++++ internal/credentials/agenttoken/clear.go | 32 ++++- internal/credentials/agenttoken/identity.go | 22 ++-- .../credentials/agenttoken/manager_test.go | 121 +++++++++++++++++- 6 files changed, 246 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 0778c54..9402a17 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,8 @@ tollbit auth logout tollbit auth logout --all ``` +Logging in again (`tollbit auth login`) mints a new token and revokes all previous ones — use it if you believe a token was exposed. `auth logout` revokes the current token; `auth logout --force` clears local credentials even if the server can't be reached (the token is then revoked at your next login or expires within 30 days). + `TOLLBIT_AGENT_DEFAULT_NAME` and `TOLLBIT_AGENT_DEFAULT_USER_AGENT` set fallback profile defaults. Saved profile overrides those defaults. `search` and `content` accept `--user-agent` as a per-request override. `auth status --check` exits `0` when the token is valid, `1` when invalid/expired, and `2` when missing (no stdout). diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 7888dda..c08bd99 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -31,7 +31,8 @@ type ( } authLogoutOptions struct { - all bool + all bool + force bool } ) @@ -93,6 +94,7 @@ func NewAuthLogoutCommand(factory app.Factory) *cobra.Command { }, } cmd.Flags().BoolVar(&opts.all, "all", false, "also clear the persisted agent profile") + cmd.Flags().BoolVar(&opts.force, "force", false, "clear local credentials even if the server token could not be revoked") return cmd } @@ -201,18 +203,33 @@ func runAuthLogout(cmd *cobra.Command, factory app.Factory, opts authLogoutOptio return RuntimeError(err) } ctx := cmd.Context() + + var clearErr error + successMsg := "Cleared agent token." if opts.all { - if err := credentials.ClearIdentity(ctx); err != nil { - return RuntimeError(err) - } - fmt.Fprintln(cmd.OutOrStdout(), "Cleared agent profile and token.") - return nil + clearErr = credentials.ClearIdentity(ctx, opts.force) + successMsg = "Cleared agent profile and token." + } else { + clearErr = credentials.ClearAgentTokens(ctx, opts.force) } - if err := credentials.ClearAgentTokens(ctx); err != nil { - return RuntimeError(err) + + switch { + case clearErr == nil: + fmt.Fprintln(cmd.OutOrStdout(), successMsg) + return nil + case errors.Is(clearErr, agenttoken.ErrRevokeFailed) && opts.force: + fmt.Fprintln(cmd.OutOrStdout(), successMsg) + fmt.Fprintln(cmd.ErrOrStderr(), + "warning: could not revoke the token on the server. It will be revoked the next time you log in, or expires within 30 days.") + return nil + case errors.Is(clearErr, agenttoken.ErrRevokeFailed): + return RuntimeError(errors.New( + "could not reach the server to revoke your token; you are still logged in. " + + "Check your connection and run `tollbit auth logout` again. " + + "To clear local credentials without revoking, use --force (the token is revoked at your next login or expires within 30 days).")) + default: + return RuntimeError(clearErr) } - fmt.Fprintln(cmd.OutOrStdout(), "Cleared agent token.") - return nil } func runAuthStatus(cmd *cobra.Command, factory app.Factory, opts authStatusOptions) error { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index cef8db6..ae51915 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -364,6 +364,68 @@ func TestRunAuthLoginStatusAndLogout(t *testing.T) { } } +func TestRunAuthLogoutFailClosedAndForce(t *testing.T) { + storageDir := t.TempDir() + tokenPath := filepath.Join(storageDir, "agent-token.jwt") + refreshPath := filepath.Join(storageDir, "refresh-token.json") + if err := os.WriteFile(tokenPath, []byte(testAgentJWT(t)), 0o600); err != nil { + t.Fatal(err) + } + refreshRaw, err := json.Marshal(map[string]string{"refresh_token": "agrt_cli"}) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(refreshPath, refreshRaw, 0o600); err != nil { + t.Fatal(err) + } + + authSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.RequestURI() != "/agent/v1/tokens/refresh/revoke" { + t.Fatalf("unexpected auth request: %s %s", r.Method, r.URL.RequestURI()) + } + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]any{"detail": "revoke failed"}) + })) + defer authSrv.Close() + + t.Setenv(testAuthBaseURLEnvVar, authSrv.URL) + t.Setenv(testCredentialsStorageDirEnvVar, storageDir) + + var stdout, stderr bytes.Buffer + code := executeTestCommand([]string{"auth", "logout"}, nil, &stdout, &stderr) + if code == 0 { + t.Fatalf("expected non-zero exit on revoke failure, got 0 stdout=%q stderr=%q", stdout.String(), stderr.String()) + } + if !strings.Contains(stderr.String(), "still logged in") { + t.Fatalf("expected fail-closed message, got stderr=%q", stderr.String()) + } + if _, err := os.Stat(tokenPath); err != nil { + t.Fatalf("expected agent token to remain, got err=%v", err) + } + if _, err := os.Stat(refreshPath); err != nil { + t.Fatalf("expected refresh token to remain, got err=%v", err) + } + + stdout.Reset() + stderr.Reset() + code = executeTestCommand([]string{"auth", "logout", "--force"}, nil, &stdout, &stderr) + if code != 0 { + t.Fatalf("auth logout --force failed: code=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String()) + } + if !strings.Contains(stdout.String(), "Cleared agent token.") { + t.Fatalf("unexpected force logout stdout: %q", stdout.String()) + } + if !strings.Contains(stderr.String(), "warning: could not revoke the token on the server") { + t.Fatalf("expected force warning on stderr, got %q", stderr.String()) + } + if _, err := os.Stat(tokenPath); !os.IsNotExist(err) { + t.Fatalf("expected agent token removed under force, got err=%v", err) + } + if _, err := os.Stat(refreshPath); !os.IsNotExist(err) { + t.Fatalf("expected refresh token removed under force, got err=%v", err) + } +} + func testAgentJWT(t *testing.T) string { t.Helper() claims := struct { diff --git a/internal/credentials/agenttoken/clear.go b/internal/credentials/agenttoken/clear.go index b97b1e7..3745a4c 100644 --- a/internal/credentials/agenttoken/clear.go +++ b/internal/credentials/agenttoken/clear.go @@ -2,32 +2,50 @@ package agenttoken import ( "context" + "errors" + "fmt" "github.com/rs/zerolog" ) +// ErrRevokeFailed indicates the refresh token could not be revoked on the +// server. Without force, ClearAuthTokens leaves local credentials intact so the +// user can retry logout. With force, local credentials are cleared and this is +// returned as a non-fatal signal so callers can warn about residual validity. +var ErrRevokeFailed = errors.New("refresh token could not be revoked on the server") + func (m *CredentialManager) Clear(ctx context.Context) error { return m.clearAgentToken(ctx) } -func (m *CredentialManager) ClearAgentTokens(ctx context.Context) error { - return m.ClearAuthTokens(ctx) +func (m *CredentialManager) ClearAgentTokens(ctx context.Context, force bool) error { + return m.ClearAuthTokens(ctx, force) } -func (m *CredentialManager) ClearAuthTokens(ctx context.Context) error { +func (m *CredentialManager) ClearAuthTokens(ctx context.Context, force bool) error { refresh, exists, err := m.readRefreshCredential(ctx) if err != nil { return err } + var revokeErr error if exists { - if err := m.revokeRefreshToken(ctx, refresh); err != nil { - return err - } + revokeErr = m.revokeRefreshToken(ctx, refresh) + } + if revokeErr != nil && !force { + // Fail closed: keep local credentials so logout can be retried. + return fmt.Errorf("%w: %w", ErrRevokeFailed, revokeErr) } if err := m.clearAgentToken(ctx); err != nil { return err } - return m.clearRefreshCredential(ctx) + if err := m.clearRefreshCredential(ctx); err != nil { + return err + } + if revokeErr != nil { + // force: local credentials cleared, but revocation failed — signal it. + return fmt.Errorf("%w: %w", ErrRevokeFailed, revokeErr) + } + return nil } func (m *CredentialManager) revokeRefreshToken(ctx context.Context, cred refreshCredential) error { diff --git a/internal/credentials/agenttoken/identity.go b/internal/credentials/agenttoken/identity.go index 69fd5b9..0b75e0a 100644 --- a/internal/credentials/agenttoken/identity.go +++ b/internal/credentials/agenttoken/identity.go @@ -15,7 +15,7 @@ func (m *CredentialManager) SaveIdentity(ctx context.Context, identity auth.Agen if err := m.WriteIdentity(ctx, identity); err != nil { return err } - return m.ClearAuthTokens(ctx) + return m.ClearAuthTokens(ctx, false) } func (m *CredentialManager) WriteIdentity(ctx context.Context, identity auth.AgentIdentity) error { @@ -84,21 +84,25 @@ func (m *CredentialManager) GetStoredIdentity(ctx context.Context) (auth.AgentId return identity, true, nil } -func (m *CredentialManager) ClearIdentity(ctx context.Context) error { +func (m *CredentialManager) ClearIdentity(ctx context.Context, force bool) error { if err := ctx.Err(); err != nil { return err } - if err := m.ClearAuthTokens(ctx); err != nil { - return err + clearErr := m.ClearAuthTokens(ctx, force) + // Fail closed: revoke failed without force → tokens were left intact, so + // leave the identity in place too and let the whole logout be retried. + if clearErr != nil && errors.Is(clearErr, ErrRevokeFailed) && !force { + return clearErr } - err := os.Remove(m.identityPath) - if os.IsNotExist(err) { - err = nil + // A non-revoke error is a real filesystem failure — surface it. + if clearErr != nil && !errors.Is(clearErr, ErrRevokeFailed) { + return clearErr } - if err != nil { + // Tokens were cleared (success, or force): remove the identity file. + if err := os.Remove(m.identityPath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("clear agent identity credential: %w", err) } - return nil + return clearErr // nil on full success; ErrRevokeFailed signal on force } func validIdentity(id auth.AgentIdentity) (auth.AgentIdentity, error) { diff --git a/internal/credentials/agenttoken/manager_test.go b/internal/credentials/agenttoken/manager_test.go index 8fcef7a..af266a7 100644 --- a/internal/credentials/agenttoken/manager_test.go +++ b/internal/credentials/agenttoken/manager_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -233,7 +234,7 @@ func TestClearAuthTokensRemovesAgentAndRefreshTokens(t *testing.T) { t.Fatal(err) } - if err := mgr.ClearAuthTokens(context.Background()); err != nil { + if err := mgr.ClearAuthTokens(context.Background(), false); err != nil { t.Fatal(err) } if _, err := os.Stat(path); !os.IsNotExist(err) { @@ -264,9 +265,13 @@ func TestClearAuthTokensKeepsRefreshTokenWhenRevokeFails(t *testing.T) { t.Fatal(err) } - if err := mgr.ClearAuthTokens(context.Background()); err == nil { + err := mgr.ClearAuthTokens(context.Background(), false) + if err == nil { t.Fatal("expected revoke error") } + if !errors.Is(err, ErrRevokeFailed) { + t.Fatalf("expected ErrRevokeFailed, got %v", err) + } if _, err := os.Stat(path); err != nil { t.Fatalf("expected agent token to remain after revoke failure, got err=%v", err) } @@ -275,6 +280,38 @@ func TestClearAuthTokensKeepsRefreshTokenWhenRevokeFails(t *testing.T) { } } +func TestClearAuthTokensForceClearsWhenRevokeFails(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, tokenFilename) + if err := os.WriteFile(path, []byte(testJWT(t, validClaims())), 0o600); err != nil { + t.Fatal(err) + } + mgr := newTestManager(t, dir, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/agent/v1/tokens/refresh/revoke" { + t.Fatalf("unexpected auth request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]any{"detail": "revoke failed"}) + })) + if err := mgr.writeJSON(context.Background(), mgr.refreshPath, refreshCredential{RefreshToken: "agrt_old"}); err != nil { + t.Fatal(err) + } + + err := mgr.ClearAuthTokens(context.Background(), true) + if err == nil { + t.Fatal("expected ErrRevokeFailed signal") + } + if !errors.Is(err, ErrRevokeFailed) { + t.Fatalf("expected ErrRevokeFailed, got %v", err) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("expected agent token removed under force, got err=%v", err) + } + if _, err := os.Stat(mgr.refreshPath); !os.IsNotExist(err) { + t.Fatalf("expected refresh token removed under force, got err=%v", err) + } +} + func TestClearSucceedsWhenTokenIsMissing(t *testing.T) { dir := t.TempDir() mgr := newTestManager(t, dir, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) @@ -581,7 +618,7 @@ func TestCanonicalAgentTokenRoundTrip(t *testing.T) { t.Fatalf("expected token file mode 0600, got %#o", got) } - if err := mgr.ClearAgentTokens(context.Background()); err != nil { + if err := mgr.ClearAgentTokens(context.Background(), false); err != nil { t.Fatal(err) } if _, exists, err := mgr.CurrentAgentToken(context.Background()); err != nil || exists { @@ -884,7 +921,7 @@ func TestClearIdentityRemovesIdentityAndToken(t *testing.T) { if err := os.WriteFile(filepath.Join(dir, tokenFilename), []byte(testJWT(t, validClaims())), 0o600); err != nil { t.Fatal(err) } - if err := mgr.ClearIdentity(context.Background()); err != nil { + if err := mgr.ClearIdentity(context.Background(), false); err != nil { t.Fatal(err) } if _, err := os.Stat(filepath.Join(dir, identityFilename)); !os.IsNotExist(err) { @@ -895,6 +932,82 @@ func TestClearIdentityRemovesIdentityAndToken(t *testing.T) { } } +func TestClearIdentityKeepsFilesWhenRevokeFails(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, tokenFilename) + mgr := newTestManager(t, dir, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/agent/v1/tokens/refresh/revoke" { + t.Fatalf("unexpected auth request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]any{"detail": "revoke failed"}) + })) + if err := mgr.WriteIdentity(context.Background(), testAgentIdentity()); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(testJWT(t, validClaims())), 0o600); err != nil { + t.Fatal(err) + } + if err := mgr.writeJSON(context.Background(), mgr.refreshPath, refreshCredential{RefreshToken: "agrt_old"}); err != nil { + t.Fatal(err) + } + + err := mgr.ClearIdentity(context.Background(), false) + if err == nil { + t.Fatal("expected revoke error") + } + if !errors.Is(err, ErrRevokeFailed) { + t.Fatalf("expected ErrRevokeFailed, got %v", err) + } + if _, err := os.Stat(filepath.Join(dir, identityFilename)); err != nil { + t.Fatalf("expected identity to remain, got err=%v", err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("expected agent token to remain, got err=%v", err) + } + if _, err := os.Stat(mgr.refreshPath); err != nil { + t.Fatalf("expected refresh token to remain, got err=%v", err) + } +} + +func TestClearIdentityForceClearsWhenRevokeFails(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, tokenFilename) + mgr := newTestManager(t, dir, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/agent/v1/tokens/refresh/revoke" { + t.Fatalf("unexpected auth request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]any{"detail": "revoke failed"}) + })) + if err := mgr.WriteIdentity(context.Background(), testAgentIdentity()); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(testJWT(t, validClaims())), 0o600); err != nil { + t.Fatal(err) + } + if err := mgr.writeJSON(context.Background(), mgr.refreshPath, refreshCredential{RefreshToken: "agrt_old"}); err != nil { + t.Fatal(err) + } + + err := mgr.ClearIdentity(context.Background(), true) + if err == nil { + t.Fatal("expected ErrRevokeFailed signal") + } + if !errors.Is(err, ErrRevokeFailed) { + t.Fatalf("expected ErrRevokeFailed, got %v", err) + } + if _, err := os.Stat(filepath.Join(dir, identityFilename)); !os.IsNotExist(err) { + t.Fatalf("expected identity removed under force, got err=%v", err) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("expected agent token removed under force, got err=%v", err) + } + if _, err := os.Stat(mgr.refreshPath); !os.IsNotExist(err) { + t.Fatalf("expected refresh token removed under force, got err=%v", err) + } +} + func newTestManager(t *testing.T, dir string, handler http.Handler) *CredentialManager { t.Helper() return newTestManagerWithConfig(t, dir, CredentialManagerConfig{}, handler)