From 0a5d06e98b982379b8c1bdb04af130db0563d85e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 09:15:19 +0100 Subject: [PATCH] fix(auth): tolerate unreadable file keyring tokens --- CHANGELOG.md | 1 + README.md | 2 + docs/spec.md | 3 +- internal/cmd/auth_accounts.go | 173 +---------------- internal/cmd/auth_cmd_test.go | 72 ++++++- internal/cmd/auth_list_helpers.go | 304 ++++++++++++++++++++++++++++++ internal/cmd/auth_tokens.go | 35 +++- 7 files changed, 411 insertions(+), 179 deletions(-) create mode 100644 internal/cmd/auth_list_helpers.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4c761..1ed8608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ ### Fixed - Backup: split Gmail checkpoint commits by row count and plaintext byte size so large messages stay below GitHub's blob limit. +- Auth: keep `gog auth list` and `gog auth tokens list` useful when one file-keyring token cannot be decrypted; unreadable entries are now reported instead of aborting the whole listing. (#377) - Auth: time out Linux D-Bus keyring write operations and report when OAuth completed but saving the refresh token failed, so manual auth no longer looks like a stuck paste when token persistence is blocked. (#130) - Install docs: document Windows release ZIP/PATH setup and clarify that source builds require the Go version declared in `go.mod`, not Ubuntu 24.04's Go 1.22 package. (#157, #135) - CI: pin GitHub Actions workflow dependencies to immutable commit SHAs. (#288) diff --git a/README.md b/README.md index f647ddf..4d6cbc5 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,8 @@ Verify tokens are usable (helps spot revoked/expired tokens): gog auth list --check ``` +If one file-keyring token cannot be decrypted, `gog auth list` still reports the readable accounts and prints the unreadable account with an error. Use `gog auth tokens list` to show token keys without decrypting them, then `gog auth tokens delete ` after confirming the affected account. + Diagnose keyring/password drift and refresh-token failures: ```bash diff --git a/docs/spec.md b/docs/spec.md index 0bcff55..4d17e09 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -102,8 +102,9 @@ Implementation: `internal/config/*`. Current minimal management commands (implemented): -- `gog auth tokens list` (keys only) +- `gog auth tokens list` (keys only; does not decrypt token payloads) - `gog auth tokens delete ` +- `gog auth list` reports unreadable token entries instead of failing the whole listing, so one bad file-keyring entry does not hide other accounts. Implementation: `internal/secrets/store.go`. diff --git a/internal/cmd/auth_accounts.go b/internal/cmd/auth_accounts.go index 41ad0c2..40fb987 100644 --- a/internal/cmd/auth_accounts.go +++ b/internal/cmd/auth_accounts.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "sort" "strings" "time" @@ -121,7 +120,7 @@ func (c *AuthListCmd) Run(ctx context.Context, _ *RootFlags) error { if err != nil { return err } - tokens, err := store.ListTokens() + tokens, tokenReadErrors, err := listAuthTokensWithFallback(store) if err != nil { return err } @@ -131,180 +130,18 @@ func (c *AuthListCmd) Run(ctx context.Context, _ *RootFlags) error { return err } - sort.Slice(tokens, func(i, j int) bool { return tokens[i].Email < tokens[j].Email }) - - type tokenByEmail struct { - tok secrets.Token - ok bool - } - tokMap := make(map[string]tokenByEmail, len(tokens)) - for _, t := range tokens { - email := normalizeEmail(t.Email) - if email == "" { - continue - } - tokMap[email] = tokenByEmail{tok: t, ok: true} - } - - type entry struct { - Email string - Token *secrets.Token - SA bool - } - entries := make([]entry, 0, len(tokens)+len(serviceAccountEmails)) - seen := make(map[string]struct{}) - for _, email := range serviceAccountEmails { - email = normalizeEmail(email) - if email == "" { - continue - } - if _, ok := seen[email]; ok { - continue - } - seen[email] = struct{}{} - te := tokMap[email] - var tok *secrets.Token - if te.ok { - t := te.tok - tok = &t - } - entries = append(entries, entry{Email: email, Token: tok, SA: true}) - } - for _, t := range tokens { - email := normalizeEmail(t.Email) - if email == "" { - continue - } - if _, ok := seen[email]; ok { - continue - } - seen[email] = struct{}{} - t2 := t - entries = append(entries, entry{Email: email, Token: &t2, SA: false}) - } - sort.Slice(entries, func(i, j int) bool { return entries[i].Email < entries[j].Email }) + entries := buildAuthListEntries(tokens, tokenReadErrors, serviceAccountEmails) if outfmt.IsJSON(ctx) { - type item struct { - Email string `json:"email"` - Subject string `json:"subject,omitempty"` - Client string `json:"client,omitempty"` - Services []string `json:"services,omitempty"` - Scopes []string `json:"scopes,omitempty"` - CreatedAt string `json:"created_at,omitempty"` - Auth string `json:"auth"` - Valid *bool `json:"valid,omitempty"` - Error string `json:"error,omitempty"` - } - out := make([]item, 0, len(entries)) - for _, e := range entries { - auth := authTypeOAuth - if e.SA { - auth = authTypeServiceAccount - } - if e.Token != nil && e.SA { - auth = authTypeOAuthServiceAccount - } - - created := "" - services := []string(nil) - scopes := []string(nil) - - if e.Token != nil { - if !e.Token.CreatedAt.IsZero() { - created = e.Token.CreatedAt.UTC().Format("2006-01-02T15:04:05Z07:00") - } - services = e.Token.Services - scopes = e.Token.Scopes - } else if e.SA { - if _, mtime, ok := bestServiceAccountPathAndMtime(e.Email); ok { - created = mtime.UTC().Format("2006-01-02T15:04:05Z07:00") - } - services = []string{"service-account"} - } - - it := item{ - Email: e.Email, - Client: "", - Services: services, - Scopes: scopes, - CreatedAt: created, - Auth: auth, - } - if e.Token != nil { - it.Client = e.Token.Client - it.Subject = e.Token.Subject - } - if c.Check { - if e.Token == nil { - valid := true - it.Valid = &valid - it.Error = "service account (not checked)" - } else { - err := checkRefreshToken(ctx, e.Token.Client, e.Token.RefreshToken, e.Token.Scopes, c.Timeout) - valid := err == nil - it.Valid = &valid - if err != nil { - it.Error = err.Error() - } - } - } - out = append(out, it) - } - return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"accounts": out}) + return c.writeAuthListJSON(ctx, entries) } + if len(entries) == 0 { u.Err().Println("No tokens stored") return nil } - for _, e := range entries { - auth := authTypeOAuth - if e.SA { - auth = authTypeServiceAccount - } - if e.Token != nil && e.SA { - auth = authTypeOAuthServiceAccount - } - - client := "" - if e.Token != nil { - client = e.Token.Client - } - created := "" - servicesCSV := "" - - if e.Token != nil { - if !e.Token.CreatedAt.IsZero() { - created = e.Token.CreatedAt.UTC().Format("2006-01-02T15:04:05Z07:00") - } - servicesCSV = strings.Join(e.Token.Services, ",") - } else if e.SA { - if _, mtime, ok := bestServiceAccountPathAndMtime(e.Email); ok { - created = mtime.UTC().Format("2006-01-02T15:04:05Z07:00") - } - servicesCSV = "service-account" - } - - if c.Check { - if e.Token == nil { - u.Out().Printf("%s\t%s\t%s\t%s\t%t\t%s\t%s", e.Email, client, servicesCSV, created, true, "service account (not checked)", auth) - continue - } - - err := checkRefreshToken(ctx, e.Token.Client, e.Token.RefreshToken, e.Token.Scopes, c.Timeout) - valid := err == nil - msg := "" - if err != nil { - msg = err.Error() - } - u.Out().Printf("%s\t%s\t%s\t%s\t%t\t%s\t%s", e.Email, client, servicesCSV, created, valid, msg, auth) - continue - } - - u.Out().Printf("%s\t%s\t%s\t%s\t%s", e.Email, client, servicesCSV, created, auth) - } - return nil + return c.writeAuthListText(ctx, u, entries) } func bestServiceAccountPathAndMtime(email string) (string, time.Time, bool) { diff --git a/internal/cmd/auth_cmd_test.go b/internal/cmd/auth_cmd_test.go index f7251fd..3732fa6 100644 --- a/internal/cmd/auth_cmd_test.go +++ b/internal/cmd/auth_cmd_test.go @@ -212,6 +212,30 @@ func TestAuthTokensList_FiltersNonTokenKeys(t *testing.T) { } } +func TestAuthTokensList_ListsKeysWithoutDecrypting(t *testing.T) { + origOpen := openSecretsStore + t.Cleanup(func() { openSecretsStore = origOpen }) + + openSecretsStore = func() (secrets.Store, error) { + return &errorTokenStore{ + keys: []string{secrets.TokenKey(config.DefaultClientName, "a@b.com")}, + err: errors.New("read token: aes.KeyUnwrap(): integrity check failed"), + }, nil + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"auth", "tokens", "list"}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + if strings.TrimSpace(out) != "token:default:a@b.com" { + t.Fatalf("unexpected output: %q", out) + } +} + func TestAuthStatus_JSON(t *testing.T) { t.Setenv("HOME", t.TempDir()) t.Setenv("XDG_CONFIG_HOME", t.TempDir()) @@ -301,7 +325,7 @@ func (s *errorTokenStore) GetToken(string, string) (secrets.Token, error) { func (s *errorTokenStore) DeleteToken(string, string) error { return nil } -func (s *errorTokenStore) ListTokens() ([]secrets.Token, error) { return nil, nil } +func (s *errorTokenStore) ListTokens() ([]secrets.Token, error) { return nil, s.err } func (s *errorTokenStore) GetDefaultAccount(string) (string, error) { return "", nil } @@ -356,6 +380,52 @@ func TestAuthDoctor_JSON_ClassifiesFileKeyringIntegrity(t *testing.T) { } } +func TestAuthList_JSON_ReportsUnreadableToken(t *testing.T) { + origOpen := openSecretsStore + t.Cleanup(func() { openSecretsStore = origOpen }) + + t.Setenv("HOME", t.TempDir()) + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + openSecretsStore = func() (secrets.Store, error) { + return &errorTokenStore{ + keys: []string{secrets.TokenKey(config.DefaultClientName, "a@b.com")}, + err: errors.New("read token: aes.KeyUnwrap(): integrity check failed"), + }, nil + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"--json", "auth", "list"}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + var payload struct { + Accounts []struct { + Email string `json:"email"` + Client string `json:"client"` + Auth string `json:"auth"` + Error string `json:"error"` + Hint string `json:"hint"` + } `json:"accounts"` + } + if err := json.Unmarshal([]byte(out), &payload); err != nil { + t.Fatalf("json parse: %v\nout=%q", err, out) + } + if len(payload.Accounts) != 1 { + t.Fatalf("accounts=%#v, want one unreadable token row", payload.Accounts) + } + account := payload.Accounts[0] + if account.Email != "a@b.com" || account.Client != config.DefaultClientName || account.Auth != authTypeOAuth { + t.Fatalf("unexpected account row: %#v", account) + } + if !strings.Contains(account.Error, "integrity check failed") || !strings.Contains(account.Hint, "password mismatch") { + t.Fatalf("missing classified unreadable-token details: %#v", account) + } +} + func TestAuthDoctor_JSON_CheckClassifiesInvalidRAPT(t *testing.T) { origOpen := openSecretsStore origCheck := checkRefreshToken diff --git a/internal/cmd/auth_list_helpers.go b/internal/cmd/auth_list_helpers.go new file mode 100644 index 0000000..2dced9b --- /dev/null +++ b/internal/cmd/auth_list_helpers.go @@ -0,0 +1,304 @@ +package cmd + +import ( + "context" + "fmt" + "os" + "sort" + "strings" + + "github.com/steipete/gogcli/internal/outfmt" + "github.com/steipete/gogcli/internal/secrets" + "github.com/steipete/gogcli/internal/ui" +) + +type authTokenReadError struct { + Client string + Email string + Err error +} + +type authListEntry struct { + Email string + Client string + Token *secrets.Token + SA bool + ReadErr error + ReadHint string +} + +type authListJSONItem struct { + Email string `json:"email"` + Subject string `json:"subject,omitempty"` + Client string `json:"client,omitempty"` + Services []string `json:"services,omitempty"` + Scopes []string `json:"scopes,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + Auth string `json:"auth"` + Valid *bool `json:"valid,omitempty"` + Error string `json:"error,omitempty"` + Hint string `json:"hint,omitempty"` +} + +func listAuthTokensWithFallback(store secrets.Store) ([]secrets.Token, []authTokenReadError, error) { + tokens, err := store.ListTokens() + if err == nil { + return tokens, nil, nil + } + + return readableTokens(store) +} + +type tokenByEmail struct { + tok secrets.Token + ok bool +} + +func buildAuthListEntries(tokens []secrets.Token, tokenReadErrors []authTokenReadError, serviceAccountEmails []string) []authListEntry { + sort.Slice(tokens, func(i, j int) bool { return tokens[i].Email < tokens[j].Email }) + + tokMap := make(map[string]tokenByEmail, len(tokens)) + for _, t := range tokens { + email := normalizeEmail(t.Email) + if email == "" { + continue + } + tokMap[email] = tokenByEmail{tok: t, ok: true} + } + + readErrMap := make(map[string]authTokenReadError, len(tokenReadErrors)) + for _, readErr := range tokenReadErrors { + email := normalizeEmail(readErr.Email) + if email == "" { + continue + } + readErrMap[email] = readErr + } + + entries := make([]authListEntry, 0, len(tokens)+len(serviceAccountEmails)+len(tokenReadErrors)) + seen := make(map[string]struct{}) + for _, email := range serviceAccountEmails { + email = normalizeEmail(email) + if email == "" { + continue + } + if _, ok := seen[email]; ok { + continue + } + seen[email] = struct{}{} + entries = append(entries, authListEntryForServiceAccount(email, tokMap[email], readErrMap[email])) + } + for _, t := range tokens { + email := normalizeEmail(t.Email) + if email == "" { + continue + } + if _, ok := seen[email]; ok { + continue + } + seen[email] = struct{}{} + tok := t + entries = append(entries, authListEntry{Email: email, Token: &tok}) + } + for _, readErr := range tokenReadErrors { + email := normalizeEmail(readErr.Email) + if email == "" { + continue + } + if _, ok := seen[email]; ok { + continue + } + seen[email] = struct{}{} + entries = append(entries, authListEntryForReadError(email, readErr)) + } + + sort.Slice(entries, func(i, j int) bool { return entries[i].Email < entries[j].Email }) + + return entries +} + +func authListEntryForServiceAccount(email string, te tokenByEmail, readErr authTokenReadError) authListEntry { + var tok *secrets.Token + if te.ok { + t := te.tok + tok = &t + } + entry := authListEntry{Email: email, Client: readErr.Client, Token: tok, SA: true} + if readErr.Err != nil { + entry.ReadErr = readErr.Err + _, entry.ReadHint = classifyAuthDoctorError(readErr.Err) + } + + return entry +} + +func authListEntryForReadError(email string, readErr authTokenReadError) authListEntry { + entry := authListEntry{Email: email, Client: readErr.Client, ReadErr: readErr.Err} + _, entry.ReadHint = classifyAuthDoctorError(readErr.Err) + + return entry +} + +func (e authListEntry) authType() string { + if e.SA && (e.Token != nil || e.ReadErr != nil) { + return authTypeOAuthServiceAccount + } + if e.SA { + return authTypeServiceAccount + } + + return authTypeOAuth +} + +func (e authListEntry) details() (client string, created string, services []string, scopes []string) { + client = e.Client + if e.Token != nil { + if !e.Token.CreatedAt.IsZero() { + created = e.Token.CreatedAt.UTC().Format("2006-01-02T15:04:05Z07:00") + } + return e.Token.Client, created, e.Token.Services, e.Token.Scopes + } + if e.SA { + if _, mtime, ok := bestServiceAccountPathAndMtime(e.Email); ok { + created = mtime.UTC().Format("2006-01-02T15:04:05Z07:00") + } + return client, created, []string{"service-account"}, nil + } + + return client, created, nil, nil +} + +func (c *AuthListCmd) writeAuthListJSON(ctx context.Context, entries []authListEntry) error { + out := make([]authListJSONItem, 0, len(entries)) + for _, e := range entries { + client, created, services, scopes := e.details() + it := authListJSONItem{ + Email: e.Email, + Client: client, + Services: services, + Scopes: scopes, + CreatedAt: created, + Auth: e.authType(), + } + if e.Token != nil { + it.Subject = e.Token.Subject + } + if e.ReadErr != nil { + it.Error = e.ReadErr.Error() + it.Hint = e.ReadHint + } + if c.Check { + c.annotateAuthListCheck(ctx, e, &it) + } + out = append(out, it) + } + + return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"accounts": out}) +} + +func (c *AuthListCmd) annotateAuthListCheck(ctx context.Context, e authListEntry, it *authListJSONItem) { + switch { + case e.ReadErr != nil: + valid := false + it.Valid = &valid + case e.Token == nil: + valid := true + it.Valid = &valid + it.Error = "service account (not checked)" + default: + err := checkRefreshToken(ctx, e.Token.Client, e.Token.RefreshToken, e.Token.Scopes, c.Timeout) + valid := err == nil + it.Valid = &valid + if err != nil { + it.Error = err.Error() + } + } +} + +func (c *AuthListCmd) writeAuthListText(ctx context.Context, u *ui.UI, entries []authListEntry) error { + for _, e := range entries { + writeAuthListReadWarning(u, e) + + client, created, services, _ := e.details() + servicesCSV := strings.Join(services, ",") + if c.Check { + c.writeAuthListCheckRow(ctx, u, e, client, servicesCSV, created) + continue + } + + u.Out().Printf("%s\t%s\t%s\t%s\t%s", e.Email, client, servicesCSV, created, e.authType()) + } + + return nil +} + +func writeAuthListReadWarning(u *ui.UI, e authListEntry) { + if e.ReadErr == nil { + return + } + u.Err().Printf("WARN\t%s\t%s", e.Email, e.ReadErr.Error()) + if e.ReadHint != "" { + u.Err().Printf("hint\t%s\t%s", e.Email, e.ReadHint) + } +} + +func (c *AuthListCmd) writeAuthListCheckRow(ctx context.Context, u *ui.UI, e authListEntry, client string, servicesCSV string, created string) { + switch { + case e.ReadErr != nil: + u.Out().Printf("%s\t%s\t%s\t%s\t%t\t%s\t%s", e.Email, client, servicesCSV, created, false, e.ReadErr.Error(), e.authType()) + case e.Token == nil: + u.Out().Printf("%s\t%s\t%s\t%s\t%t\t%s\t%s", e.Email, client, servicesCSV, created, true, "service account (not checked)", e.authType()) + default: + err := checkRefreshToken(ctx, e.Token.Client, e.Token.RefreshToken, e.Token.Scopes, c.Timeout) + valid := err == nil + msg := "" + if err != nil { + msg = err.Error() + } + u.Out().Printf("%s\t%s\t%s\t%s\t%t\t%s\t%s", e.Email, client, servicesCSV, created, valid, msg, e.authType()) + } +} + +func readableTokens(store secrets.Store) ([]secrets.Token, []authTokenReadError, error) { + keys, err := store.Keys() + if err != nil { + return nil, nil, fmt.Errorf("list tokens: %w", err) + } + + out := make([]secrets.Token, 0) + readErrors := make([]authTokenReadError, 0) + seen := make(map[string]struct{}) + + for _, key := range keys { + client, email, ok := secrets.ParseTokenKey(key) + if !ok { + continue + } + keyID := client + "\n" + email + if _, ok := seen[keyID]; ok { + continue + } + seen[keyID] = struct{}{} + + tok, err := store.GetToken(client, email) + if err != nil { + readErrors = append(readErrors, authTokenReadError{ + Client: client, + Email: email, + Err: fmt.Errorf("read token for %s: %w", email, err), + }) + continue + } + + if tok.Subject != "" { + subjectID := tok.Client + "\nsub:" + tok.Subject + if _, ok := seen[subjectID]; ok { + continue + } + seen[subjectID] = struct{}{} + } + out = append(out, tok) + } + + return out, readErrors, nil +} diff --git a/internal/cmd/auth_tokens.go b/internal/cmd/auth_tokens.go index c70e213..b0eadae 100644 --- a/internal/cmd/auth_tokens.go +++ b/internal/cmd/auth_tokens.go @@ -32,18 +32,10 @@ func (c *AuthTokensListCmd) Run(ctx context.Context, _ *RootFlags) error { if err != nil { return err } - tokens, err := store.ListTokens() + filtered, err := storedTokenKeys(store) if err != nil { return err } - filtered := make([]string, 0, len(tokens)) - for _, tok := range tokens { - if strings.TrimSpace(tok.Email) == "" { - continue - } - filtered = append(filtered, secrets.TokenKey(tok.Client, tok.Email)) - } - sort.Strings(filtered) if len(filtered) == 0 { if outfmt.IsJSON(ctx) { @@ -61,6 +53,31 @@ func (c *AuthTokensListCmd) Run(ctx context.Context, _ *RootFlags) error { return nil } +func storedTokenKeys(store secrets.Store) ([]string, error) { + keys, err := store.Keys() + if err != nil { + return nil, err + } + + filtered := make([]string, 0, len(keys)) + seen := make(map[string]struct{}, len(keys)) + for _, key := range keys { + client, email, ok := secrets.ParseTokenKey(key) + if !ok { + continue + } + tokenKey := secrets.TokenKey(client, email) + if _, ok := seen[tokenKey]; ok { + continue + } + seen[tokenKey] = struct{}{} + filtered = append(filtered, tokenKey) + } + sort.Strings(filtered) + + return filtered, nil +} + type AuthTokensDeleteCmd struct { Email string `arg:"" name:"email" help:"Email"` }