Skip to content

fix(init): re-auth instead of hard-failing when the stored token is expired #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
piekstra wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(init): re-auth instead of hard-failing when the stored token is expired #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 42 additions & 16 deletions initcmd/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions initcmd/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"bytes"
"context"
"errors"
"fmt"
"net/http"
"net/url"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Loading