From 374e73cfca19bb51829c1b292f7f1959c38dce46 Mon Sep 17 00:00:00 2001 From: piekstra Date: Fri, 24 Jul 2026 14:09:31 -0400 Subject: [PATCH] fix(init): re-auth instead of hard-failing when the stored token is expired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expired/revoked refresh token fails during token refresh inside the oauth2 transport: the token endpoint returns HTTP 400 invalid_grant, surfaced as *oauth2.RetrieveError wrapped in *url.Error — the request never reaches the API, so no googleapi 401 and no "401" substring exists for isAuthError to match. tryExistingToken then propagated the error and init aborted: Error: getting profile: Get "https://gmail.googleapis.com/...": oauth2: "invalid_grant" "Token has been expired or revoked." forcing users to manually delete the keyring entry before init would run — the exact remediation init exists to provide. Testing-mode OAuth apps hit this every 7 days. isAuthError now recognizes *oauth2.RetrieveError with ErrorCode invalid_grant (and the corresponding strings in wrapped/legacy shapes, which only the token endpoint can produce), routing verification failures into the existing re-auth confirm + fresh OAuth flow. Also: under --auth-code-stdin the re-auth confirmation prompt is skipped (with a notice) — the two-phase install has no TTY and stdin is reserved for the auth code, so the huh prompt could only fail. --- initcmd/init.go | 58 ++++++++++++++++------ initcmd/init_test.go | 114 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 16 deletions(-) diff --git a/initcmd/init.go b/initcmd/init.go index 3e869d4..4029713 100644 --- a/initcmd/init.go +++ b/initcmd/init.go @@ -509,7 +509,7 @@ func tryExistingToken(ctx context.Context, d initDeps, opts *initOptions) (bool, if msg := auth.CheckScopesMigration(cfg.GrantedScopes); msg != "" { d.View.Error("Recorded scopes are stale.") d.View.Println(msg) - if err := promptAndDeleteForReauth(d); err != nil { + if err := promptAndDeleteForReauth(d, opts); err != nil { return false, err } return false, nil @@ -527,7 +527,7 @@ func tryExistingToken(ctx context.Context, d initDeps, opts *initOptions) (bool, if err != nil { if isAuthError(err) { d.View.Error("Stored token is expired or revoked.") - if err := promptAndDeleteForReauth(d); err != nil { + if err := promptAndDeleteForReauth(d, opts); err != nil { return false, err } return false, nil @@ -547,7 +547,7 @@ func tryExistingToken(ctx context.Context, d initDeps, opts *initOptions) (bool, if err != nil { if people.IsInsufficientScopeError(err) { d.View.Error("Token is missing People API scope.") - if err := promptAndDeleteForReauth(d); err != nil { + if err := promptAndDeleteForReauth(d, opts); err != nil { return false, err } return false, nil @@ -561,14 +561,23 @@ func tryExistingToken(ctx context.Context, d initDeps, opts *initOptions) (bool, // promptAndDeleteForReauth asks the user whether to re-auth and clears the // stored token if they confirm. Returns nil (caller should fall through to // the OAuth flow) on confirm; returns an error on decline. -func promptAndDeleteForReauth(d initDeps) error { - confirm, err := d.Prompter.ConfirmReauth() - if err != nil { - return err - } - if !confirm { - d.View.Info("Run '%s config clear' to remove the stored token.", config.ProductName()) - return errors.New("re-authentication declined") +// +// Under --auth-code-stdin (two-phase install) there is no TTY to prompt on +// and stdin is reserved for the auth code, so the confirmation is skipped: +// the caller only reaches here when the stored token is already unusable, +// and a scripted install asking for a fresh token is its own consent. +func promptAndDeleteForReauth(d initDeps, opts *initOptions) error { + if opts.authCodeStdin { + d.View.Info("Re-authenticating (--auth-code-stdin; skipping confirmation).") + } else { + confirm, err := d.Prompter.ConfirmReauth() + if err != nil { + return err + } + if !confirm { + d.View.Info("Run '%s config clear' to remove the stored token.", config.ProductName()) + return errors.New("re-authentication declined") + } } if err := d.DeleteToken(); err != nil { return fmt.Errorf("clearing token: %w", err) @@ -760,20 +769,37 @@ func extractAuthCode(input string) string { return input } -// isAuthError is preserved from the original implementation. +// isAuthError reports whether err means the stored token no longer +// authenticates — i.e. re-auth is the fix — as opposed to a transient +// network/API failure. func isAuthError(err error) bool { if err == nil { return false } + // An expired/revoked refresh token fails inside the oauth2 transport + // (the request never reaches the API): the token endpoint returns HTTP + // 400 with error code "invalid_grant", surfaced as *oauth2.RetrieveError. + // Without this branch, init hard-fails on the exact scenario it exists + // to fix — Testing-mode OAuth apps expire their refresh tokens every 7 + // days, and the resulting error carries no 401 for the fallback below. + var retrieveErr *oauth2.RetrieveError + if ok := errorAs(err, &retrieveErr); ok && retrieveErr.ErrorCode == "invalid_grant" { + return true + } var apiErr *googleapi.Error if ok := errorAs(err, &apiErr); ok { return apiErr.Code == http.StatusUnauthorized } + // String fallback for wrapped/legacy error shapes. "invalid_grant" and + // "Token has been expired or revoked" only ever come from the OAuth + // token endpoint's error response, so they are safe to treat as + // definitive without a status code; a bare 401 still needs corroboration. errStr := err.Error() - return strings.Contains(errStr, "401") && - (strings.Contains(errStr, "Invalid Credentials") || - strings.Contains(errStr, "invalid_grant") || - strings.Contains(errStr, "Token has been expired or revoked")) + if strings.Contains(errStr, "invalid_grant") || + strings.Contains(errStr, "Token has been expired or revoked") { + return true + } + return strings.Contains(errStr, "401") && strings.Contains(errStr, "Invalid Credentials") } var errorAs = errors.As diff --git a/initcmd/init_test.go b/initcmd/init_test.go index fb63be2..3b1c65a 100644 --- a/initcmd/init_test.go +++ b/initcmd/init_test.go @@ -4,7 +4,9 @@ import ( "bytes" "context" "errors" + "fmt" "net/http" + "net/url" "os" "path/filepath" "strings" @@ -114,6 +116,24 @@ func TestIsAuthError(t *testing.T) { {"text 401 + invalid_grant", errors.New("oauth2: 401 invalid_grant: Token has been expired"), true}, {"text Token has been expired or revoked", errors.New("401: Token has been expired or revoked"), true}, {"text 401 alone", errors.New("HTTP 401 response"), false}, + // An expired/revoked refresh token fails token *refresh* (HTTP 400 + // invalid_grant from the token endpoint), so it carries no 401 — + // this is the shape a Testing-mode OAuth app produces every 7 days. + {"RetrieveError invalid_grant", &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusBadRequest}, + ErrorCode: "invalid_grant", + ErrorDescription: "Token has been expired or revoked.", + }, true}, + {"RetrieveError invalid_grant wrapped like production", fmt.Errorf("getting profile: %w", + &url.Error{Op: "Get", URL: "https://gmail.googleapis.com/gmail/v1/users/me/profile", Err: &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusBadRequest}, + ErrorCode: "invalid_grant", + }}), true}, + {"RetrieveError other code", &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusServiceUnavailable}, + ErrorCode: "temporarily_unavailable", + }, false}, + {"text invalid_grant without 401", errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`), true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -600,6 +620,100 @@ func TestRunWithExpiredTokenPromptsReauth(t *testing.T) { } } +// TestRunWithExpiredRefreshTokenPromptsReauth is the regression test for the +// invalid_grant hard-fail: verification of a stored token whose *refresh* +// fails (oauth2.RetrieveError wrapped by the HTTP client, no 401 anywhere) +// must route into the re-auth flow, not abort init. +func TestRunWithExpiredRefreshTokenPromptsReauth(t *testing.T) { + t.Parallel() + fs := newFakeFS() + d := baseDeps(t, fs) + d.HasStoredToken = func() bool { return true } + calls := 0 + d.GmailVerify = func(_ context.Context) (string, error) { + calls++ + if calls == 1 { + return "", fmt.Errorf("getting profile: %w", &url.Error{ + Op: "Get", + URL: "https://gmail.googleapis.com/gmail/v1/users/me/profile", + Err: &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusBadRequest}, + ErrorCode: "invalid_grant", + ErrorDescription: "Token has been expired or revoked.", + }, + }) + } + return "ada@example.com", nil + } + deleteCalled := false + d.DeleteToken = func() error { deleteCalled = true; return nil } + + srcDir := t.TempDir() + src := filepath.Join(srcDir, "downloaded.json") + if err := os.WriteFile(src, []byte(validOAuthJSON), 0644); err != nil { + t.Fatal(err) + } + + stub := &stubPrompter{credChoice: "paste", pasteJSON: validOAuthJSON, redirectURL: "http://localhost/?code=ABC", reauth: true} + d.Prompter = stub + + if err := runWith(context.Background(), d, &initOptions{credentialsFile: src}); err != nil { + t.Fatalf("runWith: %v", err) + } + if !deleteCalled { + t.Errorf("expected DeleteToken to be called after re-auth confirm") + } + if !contains(stub.calls, "reauth") { + t.Errorf("expected reauth prompt") + } +} + +// TestRunWith_AuthCodeStdinExpiredTokenSkipsReauthPrompt: the two-phase +// install has no TTY and its stdin carries the auth code, so an expired +// stored token must fall through to the fresh OAuth flow without the +// interactive re-auth confirmation. +func TestRunWith_AuthCodeStdinExpiredTokenSkipsReauthPrompt(t *testing.T) { + t.Parallel() + fs := newFakeFS() + d := baseDeps(t, fs) + d.HasStoredToken = func() bool { return true } + calls := 0 + d.GmailVerify = func(_ context.Context) (string, error) { + calls++ + if calls == 1 { + return "", fmt.Errorf("getting profile: %w", &oauth2.RetrieveError{ + Response: &http.Response{StatusCode: http.StatusBadRequest}, + ErrorCode: "invalid_grant", + }) + } + return "ada@example.com", nil + } + deleteCalled := false + d.DeleteToken = func() error { deleteCalled = true; return nil } + d.StdinReadAll = func() (string, error) { return "http://localhost/?code=STDIN-CODE\n", nil } + + srcDir := t.TempDir() + src := filepath.Join(srcDir, "downloaded.json") + if err := os.WriteFile(src, []byte(validOAuthJSON), 0644); err != nil { + t.Fatal(err) + } + + stub := &stubPrompter{credChoice: "paste", pasteJSON: validOAuthJSON} + d.Prompter = stub + + err := runWith(context.Background(), d, + &initOptions{credentialsFile: src, authCodeStdin: true, noBrowser: true}) + if err != nil { + t.Fatalf("runWith: %v", err) + } + if !deleteCalled { + t.Errorf("expected DeleteToken to be called") + } + if contains(stub.calls, "reauth") || contains(stub.calls, "redirect") || contains(stub.calls, "browser") { + t.Fatalf("--auth-code-stdin must not prompt, calls=%v", stub.calls) + } +} + // TestRunWithConfirmOpenBrowserTrueInvokesOpener exercises the noBrowser=false // branch: when the user confirms, OpenBrowser must be called with the auth URL. func TestRunWithConfirmOpenBrowserTrueInvokesOpener(t *testing.T) {