From 9679c0612dee43789c63269a1ad729e4071d7917 Mon Sep 17 00:00:00 2001 From: Yasuhiro Inami Date: Sun, 9 Aug 2026 16:27:47 +0100 Subject: [PATCH 1/4] feat(auth): auto-reauth on expired/revoked refresh tokens (invalid_grant) When the stored OAuth refresh token is expired or revoked (invalid_grant), gog currently fails with a hard error and requires the user to manually re-run 'gog auth add'. This is in contrast to other CLI tools (e.g. Rust's yup-oauth2 InstalledFlowAuthenticator) which automatically fall back to a browser-based re-authorization flow when the refresh token is invalid. This PR adds auto-reauth support: - In interactive sessions (TTY stdin, --no-input not set), gog detects invalid_grant during token refresh, launches a browser-based OAuth flow (with --force-consent to ensure a new refresh token), persists it to the keyring, resets the in-memory token source, and retries the original API request. - In non-interactive sessions (--no-input or non-TTY stdin), gog surfaces a clear error message with the manual 'gog auth add' command instead. - Excluded for ADC, service accounts, and direct access tokens. - The reauth preserves the stored token's full scope/service set, preventing silent grant narrowing. - The authorized email is verified to match the expected account before persisting. - The in-memory token source (resettableOAuthTokenSource) is rebuilt with the new refresh token so the retried request doesn't reuse the revoked token. Design inspired by yup-oauth2's InstalledFlowAuthenticator.find_token_info() fallback pattern: https://github.com/dermesser/yup-oauth2/blob/master/src/authenticator.rs Co-authored-by: Yasuhiro Inami --- CHANGELOG.md | 2 +- docs/auto-reauth-issue-draft.md | 92 +++++ internal/cmd/root.go | 23 ++ internal/googleapi/auth_dependencies.go | 35 ++ internal/googleapi/auto_reauth_test.go | 461 ++++++++++++++++++++++++ internal/googleapi/client.go | 75 ++++ internal/googleapi/client_auth.go | 46 +++ internal/googleapi/reauth_glue_test.go | 276 ++++++++++++++ internal/googleapi/transport.go | 29 ++ internal/googleauth/reauth.go | 224 ++++++++++++ internal/googleauth/reauth_test.go | 363 +++++++++++++++++++ 11 files changed, 1625 insertions(+), 1 deletion(-) create mode 100644 docs/auto-reauth-issue-draft.md create mode 100644 internal/googleapi/auto_reauth_test.go create mode 100644 internal/googleapi/reauth_glue_test.go create mode 100644 internal/googleauth/reauth.go create mode 100644 internal/googleauth/reauth_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ed8a337a..86ee9c152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.35.1 - Unreleased -- No unreleased changes. +- Auth: re-authorize expired or revoked stored OAuth refresh tokens interactively, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. ## 0.35.0 - 2026-08-09 diff --git a/docs/auto-reauth-issue-draft.md b/docs/auto-reauth-issue-draft.md new file mode 100644 index 000000000..c2e5c888a --- /dev/null +++ b/docs/auto-reauth-issue-draft.md @@ -0,0 +1,92 @@ +# Feature: Auto-reauth on expired/revoked refresh tokens + +## Summary + +When a stored OAuth refresh token is expired or revoked (`invalid_grant`), +`gog` fails with a hard error and requires the user to manually re-run +`gog auth add`. This is in contrast to other CLI tools (e.g. Rust's +`yup-oauth2` `InstalledFlowAuthenticator`) which automatically fall back to +a browser-based re-authorization flow when the refresh token is invalid, +making the failure transparent to the user. + +## Problem + +When the refresh token becomes invalid (Google revokes it for various +reasons: 6-month inactivity, password change, token limit reached, app in +Testing mode with 7-day expiry, user manually revoking access), any +`gog` command that needs authentication fails: + +``` +$ gog calendar events --today +Error: refresh access token: oauth2: "invalid_grant" "Token has been expired or revoked." +``` + +The user must then manually run: + +``` +gog auth add you@example.com --services gmail,calendar,drive +``` + +This is particularly frustrating because: + +1. **It's a hard stop** — no graceful degradation or recovery hint beyond the + raw OAuth error. +2. **The error message doesn't tell the user what to do** — it's a wrapped + `golang.org/x/oauth2` error, not a user-facing diagnostic. +3. **It happens repeatedly** — especially for users whose OAuth app is in + Testing mode (7-day refresh token expiry) or who don't use `gog` daily. +4. **Other tools handle this transparently** — `yup-oauth2` (used by the + `today` Rust CLI) detects `invalid_grant` during token refresh and + automatically falls back to the Installed Flow (opens a browser, obtains a + new refresh token, persists it, and retries the original request). + +## Proposed behavior + +When `gog` detects `invalid_grant` during a token refresh attempt: + +1. **If running interactively** (stdin is a terminal, `--no-input` is not set): + - Print a message to stderr: "Refresh token expired or revoked. Re-authorizing…" + - Automatically launch the browser-based OAuth flow (same as `gog auth add`) + using the stored account's services and client. + - On success, persist the new refresh token to the keyring and retry the + original API request. + - On failure (user denies, browser doesn't open, etc.), surface a clear + error with the re-auth command to run manually. + +2. **If running non-interactively** (`--no-input`, CI, pipes): + - Do NOT auto-launch a browser. + - Surface a clear error message with the exact `gog auth add` command to run. + +## Design considerations + +- **Security**: Auto-reauth should only trigger when the refresh token is + specifically revoked/expired (`invalid_grant`), not for other OAuth errors. + The browser flow uses the same PKCE + state validation as `gog auth add`. +- **Scope preservation**: The reauth should request the same services/scopes + as the stored token, not a broader or narrower set. +- `--force-consent` should be used during auto-reauth to ensure Google + returns a new refresh token (without it, Google may omit the refresh token + for returning users). +- **Keychain access**: The reauth flow needs keychain write access to persist + the new token. On macOS, this may trigger a Keychain permission prompt. +- **Timeout**: The auto-reauth browser flow should have a reasonable timeout + (e.g. 2 minutes) to avoid hanging indefinitely in CI-like environments. + +## Prior art + +- `yup-o-auth2` (Rust): `InstalledFlowAuthenticator::find_token_info()` — + on refresh failure, falls back to `auth_flow.token()` which opens a browser. + Source: [authenticator.rs](https://github.com/dermesser/yup-oauth2/blob/master/src/authenticator.rs) + +- gogcli already has partial auth resilience: + - v0.31.0: Recover from corrupt token payloads (#872) + - v0.32.0: Retry on 403 insufficient scopes by refreshing credentials (#889) + - v0.33.0: Trust Developer-ID-signed binaries for Keychain access + + Auto-reauth on `invalid_grant` is the next gap in this progression. + +## Environment + +- gog: v0.34.0 (Homebrew) +- macOS: 15.x (also reproduced on macOS 27 Tahoe beta) +- Keyring: macOS Keychain (also affects file backend) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 6891e1c85..77e4deadc 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -220,6 +220,9 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { ctx := context.Background() ctx = app.WithRuntime(ctx, runtime) ctx = googleapi.WithReadOnly(ctx, cli.ReadOnly) + if cli.NoInput || !stdinIsTerminal(ctx) { + ctx = googleapi.WithNoInput(ctx) + } runtimeContext := ctx serviceAccounts := func() (*config.ServiceAccountStore, error) { return commandServiceAccountStore(runtimeContext) @@ -247,6 +250,25 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { resolveClient := func(email string, override string) (string, error) { return resolveRuntimeClient(runtime, email, override) } + + // reauthFn is the auto-reauth closure called when the stored refresh + // token is expired or revoked (invalid_grant). It launches a browser- + // based OAuth flow and persists the new token, mirroring `gog auth add`. + reauthFn := func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (string, error) { + opts := googleauth.ReauthOptions{ + Email: email, + Client: client, + Services: services, + Scopes: scopes, + StoredToken: storedToken, + OpenSecretsStore: openTokens, + EnsureKeychainAccess: ensureKeychainAccessIfNeeded, + AuthorizeFunc: authorizeGoogleAccount, + FetchIdentityFunc: fetchAuthIdentity, + } + return googleauth.Reauth(ctx, opts) + } + authDependencies := googleapi.AuthDependencies{ ResolveClient: resolveClient, ReadCredentials: readCredentials, @@ -256,6 +278,7 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { Mode: cli.authMode, ADCTokenSource: googleapi.DefaultADCTokenSource, ServiceAccountTokenSource: googleapi.DefaultServiceAccountTokenSource, + Reauth: reauthFn, } ctx = googleapi.WithAuthDependencies(ctx, authDependencies) composeRuntimeGoogleServices(runtime, googleapi.NewFactory(authDependencies, googleapi.FactoryOptions{ diff --git a/internal/googleapi/auth_dependencies.go b/internal/googleapi/auth_dependencies.go index d716209ea..f1190beab 100644 --- a/internal/googleapi/auth_dependencies.go +++ b/internal/googleapi/auth_dependencies.go @@ -27,6 +27,17 @@ type ( ServiceAccountTokenSourceFunc func(context.Context, []byte, string, []string) (oauth2.TokenSource, error) ) +// ReauthFunc attempts to re-authorize the given account by launching a +// browser-based OAuth flow and persisting the new refresh token. It is +// called automatically when the stored refresh token is expired or revoked +// (invalid_grant) and the session is interactive. +// +// storedToken, if non-nil, carries the full scope/service set from the +// original authorization so the reauth can preserve the grant width. +// Returns the new refresh token so the caller can update any in-memory +// token source that still holds the revoked token. +type ReauthFunc func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (string, error) + type AuthDependencies struct { ResolveClient authclient.ClientResolver ReadCredentials authclient.CredentialsReader @@ -36,6 +47,7 @@ type AuthDependencies struct { Mode AuthMode ADCTokenSource ADCTokenSourceFunc ServiceAccountTokenSource ServiceAccountTokenSourceFunc + Reauth ReauthFunc } var ( @@ -183,3 +195,26 @@ func DefaultADCTokenSource(ctx context.Context, scopes ...string) (oauth2.TokenS return tokenSource, nil } + +// noInputContextKey controls whether auto-reauth is suppressed. When +// --no-input is set (or stdin is not a terminal), the auto-reauth fallback +// must not launch a browser. +type noInputContextKey struct{} + +// WithNoInput marks the context as non-interactive. Auto-reauth will be +// suppressed and a clear error message with manual instructions is returned +// instead. +func WithNoInput(ctx context.Context) context.Context { + return context.WithValue(ctx, noInputContextKey{}, true) +} + +// NoInputFromContext reports whether the context was marked non-interactive. +func NoInputFromContext(ctx context.Context) bool { + if ctx == nil { + return false + } + + enabled, _ := ctx.Value(noInputContextKey{}).(bool) + + return enabled +} diff --git a/internal/googleapi/auto_reauth_test.go b/internal/googleapi/auto_reauth_test.go new file mode 100644 index 000000000..48e9ef779 --- /dev/null +++ b/internal/googleapi/auto_reauth_test.go @@ -0,0 +1,461 @@ +package googleapi + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "golang.org/x/oauth2" +) + +func TestIsInvalidGrantError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "nil error", + err: nil, + want: false, + }, + { + name: "unrelated error", + err: errors.New("network timeout"), + want: false, + }, + { + name: `invalid_grant with "Token has been expired or revoked"`, + err: errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`), + want: true, + }, + { + name: "invalid_grant Bad Request", + err: errors.New(`oauth2: "invalid_grant" "Bad Request"`), + want: true, + }, + { + name: "wrapped invalid_grant", + err: fmt.Errorf("refresh access token: %w", errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`)), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isInvalidGrantError(tt.err); got != tt.want { + t.Errorf("isInvalidGrantError() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRetryTransportAutoReauthOnInvalidGrant(t *testing.T) { + tokenSource := &refreshableTestTokenSource{token: "stale-token"} + reauthCalls := 0 + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, fmt.Errorf("read body: %w", err) + } + _ = body + + switch calls { + case 1: + // First call: the oauth2 transport will fail with invalid_grant + // because the refresh token is stale. We simulate this by + // returning an error from the base transport. + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + case 2: + // After reauth, the token should be fresh. + if got := req.Header.Get("Authorization"); got != "Bearer fresh-token" { + t.Fatalf("second authorization = %q, want %q", got, "Bearer fresh-token") + } + + return newTestResponse(http.StatusOK, "ok"), nil + default: + t.Fatalf("unexpected call %d", calls) + return nil, errUnexpectedRequestBody + } + }) + + rt := &RetryTransport{ + Base: &oauth2TransportWrapper{ + source: tokenSource, + base: base, + }, + MaxRetries429: 0, + MaxRetries5xx: 0, + BaseDelay: 0, + Reauth: func(ctx context.Context) error { + reauthCalls++ + tokenSource.token = "fresh-token" + + return nil + }, + } + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "http://example.com", io.NopCloser(strings.NewReader("payload"))) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.ContentLength = int64(len("payload")) + + resp, err := rt.RoundTrip(req) + if err != nil { + t.Fatalf("round trip: %v", err) + } + _ = resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status: %d", resp.StatusCode) + } + + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } + + if reauthCalls != 1 { + t.Fatalf("reauth calls = %d, want 1", reauthCalls) + } +} + +func TestRetryTransportAutoReauthFailureSurfacesError(t *testing.T) { + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + _ = req.Body + + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + }) + + rt := &RetryTransport{ + Base: base, + Reauth: func(context.Context) error { + return errors.New("browser did not open") + }, + } + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://example.com", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + + _, err = rt.RoundTrip(req) + if err == nil { + t.Fatalf("expected error") + } + + if !strings.Contains(err.Error(), "invalid_grant") { + t.Fatalf("error should mention invalid_grant: %v", err) + } + + if !strings.Contains(err.Error(), "re-authentication failed") { + t.Fatalf("error should mention re-authentication failed: %v", err) + } + + if calls != 1 { + t.Fatalf("calls = %d, want 1", calls) + } +} + +func TestRetryTransportNoReauthWhenNil(t *testing.T) { + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + }) + + rt := &RetryTransport{ + Base: base, + Reauth: nil, // No reauth function + } + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://example.com", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + + _, err = rt.RoundTrip(req) + if err == nil { + t.Fatalf("expected error") + } + + if !strings.Contains(err.Error(), "invalid_grant") { + t.Fatalf("error should contain invalid_grant: %v", err) + } + + if !strings.Contains(err.Error(), "gog auth add") { + t.Fatalf("error should contain 'gog auth add' hint: %v", err) + } + + if calls != 1 { + t.Fatalf("calls = %d, want 1", calls) + } +} + +func TestRetryTransportNoReauthWhenRetriesDisabled(t *testing.T) { + reauthCalls := 0 + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + }) + + rt := &RetryTransport{ + Base: base, + Reauth: func(context.Context) error { + reauthCalls++ + return nil + }, + } + + ctx := WithoutRetries(context.Background()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://example.com", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + + _, err = rt.RoundTrip(req) + if err == nil { + t.Fatalf("expected error") + } + + if !strings.Contains(err.Error(), "invalid_grant") { + t.Fatalf("error should contain invalid_grant: %v", err) + } + + if !strings.Contains(err.Error(), "gog auth add") { + t.Fatalf("error should contain 'gog auth add' hint: %v", err) + } + + if reauthCalls != 0 { + t.Fatalf("reauth should not be called when retries are disabled, got %d calls", reauthCalls) + } + + if calls != 1 { + t.Fatalf("calls = %d, want 1", calls) + } +} + +func TestRetryTransportNoReauthForNonReplayableBody(t *testing.T) { + reauthCalls := 0 + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + body, _ := io.ReadAll(req.Body) + _ = body + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + }) + + rt := &RetryTransport{ + Base: base, + Reauth: func(context.Context) error { + reauthCalls++ + return nil + }, + } + + // Non-replayable: large body with unknown content length + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "http://example.com", io.NopCloser(strings.NewReader("payload"))) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.ContentLength = maxBufferedReplayBodyBytes + 1 + + _, err = rt.RoundTrip(req) + if err == nil { + t.Fatalf("expected error") + } + + if !strings.Contains(err.Error(), "invalid_grant") { + t.Fatalf("error should contain invalid_grant: %v", err) + } + + if !strings.Contains(err.Error(), "gog auth add") { + t.Fatalf("error should contain 'gog auth add' hint: %v", err) + } + + if reauthCalls != 0 { + t.Fatalf("reauth should not be called for non-replayable body, got %d calls", reauthCalls) + } +} + +func TestRetryTransportReauthOnlyOnce(t *testing.T) { + reauthCalls := 0 + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + // Always return invalid_grant, even after reauth + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + }) + + rt := &RetryTransport{ + Base: base, + Reauth: func(context.Context) error { + reauthCalls++ + return nil + }, + } + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://example.com", nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + + _, err = rt.RoundTrip(req) + if err == nil { + t.Fatalf("expected error") + } + + if !strings.Contains(err.Error(), "invalid_grant") { + t.Fatalf("error should contain invalid_grant: %v", err) + } + + if !strings.Contains(err.Error(), "gog auth add") { + t.Fatalf("error should contain 'gog auth add' hint: %v", err) + } + + // Should only reauth once, not loop + if reauthCalls != 1 { + t.Fatalf("reauth calls = %d, want 1", reauthCalls) + } + + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } +} + +func TestNoInputFromContext(t *testing.T) { + if NoInputFromContext(context.Background()) { + t.Fatal("default context should not be no-input") + } + + ctx := WithNoInput(context.Background()) + if !NoInputFromContext(ctx) { + t.Fatal("WithNoInput context should report NoInput") + } +} + +// TestRetryTransportReauthResetsTokenSource verifies that after a successful +// reauth, the retried request uses the NEW refresh token, not the stale one. +// This is the critical test that exposes the "store-only" hole: if the +// in-memory token source is not reset, the retried request reuses the +// revoked refresh token and fails again. +func TestRetryTransportReauthResetsTokenSource(t *testing.T) { + // simulateResettableSource is a token source that tracks which refresh + // token it uses, simulating resettableOAuthTokenSource behavior. + source := &simulResettableSource{refreshToken: "revoked-token"} + + reauthCalled := false + calls := 0 + + base := roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + body, _ := io.ReadAll(req.Body) + _ = body + + switch calls { + case 1: + // First call: the oauth2 transport tries to refresh using the + // revoked token and fails with invalid_grant. + return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + case 2: + // After reauth, the token source should have been reset. + // If it wasn't, this test would fail because the reauth + // function only updates the store, not the source. + if source.refreshToken == "revoked-token" { + t.Fatal("token source was not reset after reauth; retried request would use revoked token") + } + return newTestResponse(http.StatusOK, "ok"), nil + default: + t.Fatalf("unexpected call %d", calls) + return nil, nil + } + }) + + rt := &RetryTransport{ + Base: &oauth2TransportWrapper{ + source: source, + base: base, + }, + Reauth: func(ctx context.Context) error { + reauthCalled = true + // Simulate: reauth gets a new token from the browser and + // persists it to the store. The critical step is resetting + // the in-memory token source. + source.ResetRefreshToken("new-refresh-token") + return nil + }, + } + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "http://example.com", io.NopCloser(strings.NewReader("payload"))) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.ContentLength = int64(len("payload")) + + resp, err := rt.RoundTrip(req) + if err != nil { + t.Fatalf("round trip: %v", err) + } + _ = resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status: %d", resp.StatusCode) + } + + if !reauthCalled { + t.Fatal("reauth was not called") + } + + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } +} + +// simulResettableSource is a minimal token source that simulates +// resettableOAuthTokenSource for testing the reset-after-reauth behavior. +type simulResettableSource struct { + refreshToken string +} + +func (s *simulResettableSource) Token() (*oauth2.Token, error) { + return &oauth2.Token{AccessToken: "access-" + s.refreshToken}, nil +} + +func (s *simulResettableSource) ResetRefreshToken(token string) { + s.refreshToken = token +} + +// oauth2TransportWrapper is a minimal oauth2.Transport-like wrapper for +// testing. It calls the token source to get a token and sets the +// Authorization header before delegating to the base transport. +type oauth2TransportWrapper struct { + source oauth2.TokenSource + base http.RoundTripper +} + +func (w *oauth2TransportWrapper) RoundTrip(req *http.Request) (*http.Response, error) { + tok, err := w.source.Token() + if err != nil { + return nil, err + } + + req.Header.Set("Authorization", "Bearer "+tok.AccessToken) + + return w.base.RoundTrip(req) +} diff --git a/internal/googleapi/client.go b/internal/googleapi/client.go index 101626bed..baef62952 100644 --- a/internal/googleapi/client.go +++ b/internal/googleapi/client.go @@ -13,6 +13,7 @@ import ( "github.com/openclaw/gogcli/internal/authclient" "github.com/openclaw/gogcli/internal/googleauth" + "github.com/openclaw/gogcli/internal/secrets" ) const ( @@ -129,6 +130,7 @@ func authenticatedTransportWithStoredScopeCheck( requireStoredGrant bool, ) (http.RoundTripper, error) { var ts oauth2.TokenSource + var isStoredOAuth bool if dependencies, ok := authDependenciesFromContext(ctx); ok && dependencies.Mode == AuthModeADC { slog.Debug("using Application Default Credentials (GOG_AUTH_MODE=adc)", "serviceLabel", serviceLabel) @@ -146,6 +148,13 @@ func authenticatedTransportWithStoredScopeCheck( if err != nil { return nil, err } + + // Only stored OAuth tokens (wrapped in persistingTokenSource) can + // be auto-reauthorized via a browser flow. Direct access tokens + // (StaticTokenSource) and service-account sources never return + // invalid_grant through the refresh-token path, but even if they + // did, launching a user browser flow would be nonsensical. + _, isStoredOAuth = ts.(*persistingTokenSource) } retryTransport := NewRetryTransport(&oauth2.Transport{ @@ -159,6 +168,15 @@ func authenticatedTransportWithStoredScopeCheck( retryTransport.RefreshAuth = refresher.ForceRefresh } + // Wire auto-reauth for invalid_grant (expired/revoked refresh token). + // Only enabled for stored OAuth (not ADC, not direct access tokens, + // not service accounts) and only when the session is interactive. + if isStoredOAuth && !NoInputFromContext(ctx) { + if reauthFn := reauthFunctionFromContext(ctx, serviceLabel, email, scopes, ts); reauthFn != nil { + retryTransport.Reauth = reauthFn + } + } + return readOnlyTransportFromContext(ctx, retryTransport), nil } @@ -298,3 +316,60 @@ func newBaseTransport() *http.Transport { return transport } + +// reauthFunctionFromContext builds a Reauth closure from the auth +// dependencies stored in the context. Returns nil if the dependencies are +// not available or the Reauth function is not configured, in which case +// auto-reauth is disabled and invalid_grant errors are surfaced directly. +// +// The token source is captured so that after a successful reauth, the +// in-memory token source can be reset with the new refresh token — +// otherwise the retried request would reuse the revoked token. +func reauthFunctionFromContext(ctx context.Context, serviceLabel string, email string, scopes []string, ts oauth2.TokenSource) func(context.Context) error { + dependencies, ok := authDependenciesFromContext(ctx) + if !ok || dependencies.Reauth == nil { + return nil + } + + resolvedEmail := email + + // Derive the service list from the single service label when available. + var services []string + if serviceLabel != "" { + services = []string{serviceLabel} + } + + return func(ctx context.Context) error { + // Resolve the client at call time so it reflects the latest config. + client, err := dependencies.resolveClient(resolvedEmail, authclient.ClientOverrideFromContext(ctx)) + if err != nil { + return fmt.Errorf("resolve client for reauth: %w", err) + } + + // Load the stored token to preserve the full scope/service set + // from the original authorization, preventing silent grant narrowing. + var storedToken *secrets.Token + if store, storeErr := dependencies.openTokens(); storeErr == nil { + if tok, getErr := store.GetToken(client, resolvedEmail); getErr == nil { + storedToken = &tok + } + } + + newRefreshToken, err := dependencies.Reauth(ctx, resolvedEmail, client, services, scopes, storedToken) + if err != nil { + return err + } + + // Reset the in-memory token source so the retried request uses + // the new refresh token instead of the revoked one. Guard against + // an empty token to avoid rebuilding the source with no refresh + // token, which would produce a confusing refresh error. + if newRefreshToken != "" { + if resetter, ok := ts.(refreshTokenResetter); ok { + resetter.ResetRefreshToken(newRefreshToken) + } + } + + return nil + } +} diff --git a/internal/googleapi/client_auth.go b/internal/googleapi/client_auth.go index 3729e7dfb..b71fd7b1b 100644 --- a/internal/googleapi/client_auth.go +++ b/internal/googleapi/client_auth.go @@ -44,6 +44,26 @@ var ( errBaseTokenSourceReturnedNilToken = errors.New("base token source returned nil token") ) +// isInvalidGrantError checks whether an OAuth2 token refresh error is an +// invalid_grant failure, meaning the stored refresh token is no longer +// usable and must be re-obtained via a full browser-based authorization. +func isInvalidGrantError(err error) bool { + if err == nil { + return false + } + + msg := strings.ToLower(err.Error()) + + // golang.org/x/oauth2 wraps the Google response in a *RetrieveError + // whose Error() contains the raw JSON body. Google's response for a + // revoked/expired refresh token is: + // {"error":"invalid_grant","error_description":"Token has been expired or revoked."} + // + // We match on "invalid_grant" to catch all variants (expired, revoked, + // bad request) without being brittle to the exact error_description. + return strings.Contains(msg, "invalid_grant") +} + type resettableOAuthTokenSource struct { mu sync.Mutex source oauth2.TokenSource @@ -95,6 +115,18 @@ func (r *resettableOAuthTokenSource) ForceRefresh(context.Context) (*oauth2.Toke return t, nil } +// ResetRefreshToken replaces the stored refresh token and rebuilds the +// underlying oauth2.TokenSource so the next Token() call uses the new +// token. This is called after a successful auto-reauth to ensure the +// retried request doesn't reuse the revoked refresh token. +func (r *resettableOAuthTokenSource) ResetRefreshToken(refreshToken string) { + r.mu.Lock() + defer r.mu.Unlock() + + r.refreshToken = strings.TrimSpace(refreshToken) + r.source = r.newSource(&oauth2.Token{RefreshToken: r.refreshToken}) +} + func (r *resettableOAuthTokenSource) rememberRefreshTokenLocked(t *oauth2.Token) { if t == nil { return @@ -148,6 +180,20 @@ func (p *persistingTokenSource) ForceRefresh(ctx context.Context) error { return err } +// ResetRefreshToken delegates to the base token source to replace the +// stored refresh token after a successful auto-reauth. +func (p *persistingTokenSource) ResetRefreshToken(refreshToken string) { + if resetter, ok := p.base.(*resettableOAuthTokenSource); ok { + resetter.ResetRefreshToken(refreshToken) + } +} + +// refreshTokenResetter is implemented by token sources that can replace +// their stored refresh token at runtime (used after auto-reauth). +type refreshTokenResetter interface { + ResetRefreshToken(refreshToken string) +} + func (p *persistingTokenSource) persistTokenLocked(t *oauth2.Token) (*oauth2.Token, error) { if t == nil { return nil, errBaseTokenSourceReturnedNilToken diff --git a/internal/googleapi/reauth_glue_test.go b/internal/googleapi/reauth_glue_test.go new file mode 100644 index 000000000..c52490dfb --- /dev/null +++ b/internal/googleapi/reauth_glue_test.go @@ -0,0 +1,276 @@ +package googleapi + +import ( + "context" + "errors" + "testing" + + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" + + "github.com/openclaw/gogcli/internal/config" + "github.com/openclaw/gogcli/internal/secrets" +) + +// TestReauthFunctionFromContextResetsTokenSource verifies that the production +// closure returned by reauthFunctionFromContext actually calls +// ResetRefreshToken on the real persistingTokenSource chain after a +// successful reauth. This is the integration test for the glue code that +// was broken in the initial implementation. +func TestReauthFunctionFromContextResetsTokenSource(t *testing.T) { + // Build a real resettableOAuthTokenSource → persistingTokenSource chain. + // The oauth2.Config's TokenSource will use the refresh token we give it. + // We simulate a token endpoint that returns invalid_grant for the old + // token and success for the new one. + + var reauthRefreshToken string + + // Create the auth dependencies with a fake Reauth that returns a new + // refresh token (simulating a successful browser flow). + deps := AuthDependencies{ + ResolveClient: func(string, string) (string, error) { return "default", nil }, + ReadCredentials: func(string) (config.ClientCredentials, error) { + return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil + }, + OpenTokens: func() (secrets.Store, error) { + return &fakeStore{}, nil + }, + Reauth: func(ctx context.Context, email, client string, services, scopes []string, storedToken *secrets.Token) (string, error) { + reauthRefreshToken = "new-refresh-token" + return "new-refresh-token", nil + }, + } + + ctx := WithAuthDependencies(context.Background(), deps) + + // Build a real token source chain as the transport layer would. + cfg := oauth2.Config{ + ClientID: "id", + ClientSecret: "secret", + Endpoint: google.Endpoint, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + } + + // The transport construction captures the token source; we simulate + // what tokenSourceForAccountScopesWithStoredScopeCheck does. + baseSource := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { + return cfg.TokenSource(context.Background(), t) + }, &oauth2.Token{ + RefreshToken: "revoked-token", + }) + + ts := newPersistingTokenSource(baseSource, &fakeStore{}, "default", "user@example.com", secrets.Token{ + Email: "user@example.com", + RefreshToken: "revoked-token", + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + }, "calendar", nil) + + // Get the reauth closure. + reauthFn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"https://www.googleapis.com/auth/calendar"}, ts) + if reauthFn == nil { + t.Fatal("expected non-nil reauth function") + } + + // Call the reauth closure. + if err := reauthFn(context.Background()); err != nil { + t.Fatalf("reauth closure: %v", err) + } + + if reauthRefreshToken != "new-refresh-token" { + t.Fatalf("Reauth was not called or returned unexpected token: %q", reauthRefreshToken) + } + + // Verify the token source was actually reset by checking the inner + // resettableOAuthTokenSource's refresh token. + persisting, ok := ts.(*persistingTokenSource) + if !ok { + t.Fatalf("expected *persistingTokenSource, got %T", ts) + } + + resettable, ok := persisting.base.(*resettableOAuthTokenSource) + if !ok { + t.Fatalf("expected *resettableOAuthTokenSource, got %T", persisting.base) + } + + if resettable.refreshToken != "new-refresh-token" { + t.Fatalf("token source was not reset: expected new-refresh-token, got %q", resettable.refreshToken) + } +} + +// TestReauthFunctionFromContextNilWhenNoDeps verifies that the closure is +// nil when auth dependencies are not in the context. +func TestReauthFunctionFromContextNilWhenNoDeps(t *testing.T) { + fn := reauthFunctionFromContext(context.Background(), "calendar", "user@example.com", []string{"scope"}, nil) + if fn != nil { + t.Fatal("expected nil reauth function when no deps in context") + } +} + +// TestReauthFunctionFromContextNilWhenNoReauthFunc verifies that the closure +// is nil when the Reauth field is not set on the dependencies. +func TestReauthFunctionFromContextNilWhenNoReauthFunc(t *testing.T) { + deps := AuthDependencies{ + ResolveClient: func(string, string) (string, error) { return "default", nil }, + ReadCredentials: func(string) (config.ClientCredentials, error) { return config.ClientCredentials{}, nil }, + OpenTokens: func() (secrets.Store, error) { return &fakeStore{}, nil }, + // Reauth is nil + } + ctx := WithAuthDependencies(context.Background(), deps) + fn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope"}, nil) + if fn != nil { + t.Fatal("expected nil reauth function when Reauth is nil") + } +} + +// TestResettableOAuthTokenSourceResetRefreshToken verifies that +// ResetRefreshToken replaces the refresh token and rebuilds the source. +func TestResettableOAuthTokenSourceResetRefreshToken(t *testing.T) { + var tokenRequests []string + + source := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { + // Track which refresh token is being used. + tokenRequests = append(tokenRequests, t.RefreshToken) + // Return a stub token source that always succeeds. + return &stubTokenSource{token: &oauth2.Token{AccessToken: "access"}} + }, &oauth2.Token{ + RefreshToken: "old-token", + }) + + // Verify initial state + if source.refreshToken != "old-token" { + t.Fatalf("expected old-token, got %q", source.refreshToken) + } + + // Reset + source.ResetRefreshToken("new-token") + + if source.refreshToken != "new-token" { + t.Fatalf("expected new-token, got %q", source.refreshToken) + } + + // Verify the source was rebuilt — calling Token() should use the new token + _, _ = source.Token() + + if len(tokenRequests) == 0 { + t.Fatal("Token() did not create a new source") + } + + if tokenRequests[len(tokenRequests)-1] != "new-token" { + t.Fatalf("expected new-token in request, got %q", tokenRequests[len(tokenRequests)-1]) + } +} + +// TestPersistingTokenSourceResetRefreshToken verifies delegation. +func TestPersistingTokenSourceResetRefreshToken(t *testing.T) { + base := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { + return (&oauth2.Config{ + ClientID: "id", + ClientSecret: "secret", + Endpoint: oauth2.Endpoint{ + AuthURL: "https://example.com/auth", + TokenURL: "https://example.com/token", + }, + Scopes: []string{"scope"}, + }).TokenSource(context.Background(), t) + }, &oauth2.Token{RefreshToken: "old"}) + + persisting := newPersistingTokenSource(base, &fakeStore{}, "default", "user@example.com", secrets.Token{ + Email: "user@example.com", + RefreshToken: "old", + }, "calendar", nil) + + // Reset via the persisting wrapper + persisting.(*persistingTokenSource).ResetRefreshToken("new-refresh") + + // Verify the inner source was updated + if base.refreshToken != "new-refresh" { + t.Fatalf("expected new-refresh, got %q", base.refreshToken) + } +} + +// TestReauthClosureLoadsStoredToken verifies the closure loads the stored +// token and passes it through to Reauth for scope preservation. +func TestReauthClosureLoadsStoredToken(t *testing.T) { + store := &fakeStore{ + token: &secrets.Token{ + Email: "user@example.com", + Scopes: []string{"scope1", "scope2", "scope3"}, + Services: []string{"calendar", "gmail", "drive"}, + }, + } + + var passedStoredToken *secrets.Token + + deps := AuthDependencies{ + ResolveClient: func(string, string) (string, error) { return "default", nil }, + ReadCredentials: func(string) (config.ClientCredentials, error) { + return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil + }, + OpenTokens: func() (secrets.Store, error) { return store, nil }, + Reauth: func(ctx context.Context, email, client string, services, scopes []string, storedToken *secrets.Token) (string, error) { + passedStoredToken = storedToken + return "new-token", nil + }, + } + + ctx := WithAuthDependencies(context.Background(), deps) + + // Build a real token source so the reset path works + baseSource := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { + return (&oauth2.Config{ + ClientID: "id", + ClientSecret: "secret", + Endpoint: google.Endpoint, + Scopes: []string{"scope"}, + }).TokenSource(context.Background(), t) + }, &oauth2.Token{RefreshToken: "old"}) + + ts := newPersistingTokenSource(baseSource, store, "default", "user@example.com", secrets.Token{ + Email: "user@example.com", + RefreshToken: "old", + }, "calendar", nil) + + reauthFn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope1"}, ts) + if reauthFn == nil { + t.Fatal("expected non-nil reauth function") + } + + if err := reauthFn(context.Background()); err != nil { + t.Fatalf("reauth: %v", err) + } + + if passedStoredToken == nil { + t.Fatal("expected stored token to be passed to Reauth") + } + + if len(passedStoredToken.Scopes) != 3 { + t.Fatalf("expected 3 scopes from stored token, got %d: %v", len(passedStoredToken.Scopes), passedStoredToken.Scopes) + } +} + +// stubTokenSource returns a fixed token without any network calls. +type stubTokenSource struct { + token *oauth2.Token +} + +func (s *stubTokenSource) Token() (*oauth2.Token, error) { + return s.token, nil +} + +// fakeStore is a minimal secrets.Store for testing. +type fakeStore struct { + token *secrets.Token +} + +func (s *fakeStore) Keys() ([]string, error) { return nil, nil } +func (s *fakeStore) GetToken(client, email string) (secrets.Token, error) { + if s.token != nil { + return *s.token, nil + } + return secrets.Token{}, errors.New("not found") +} +func (s *fakeStore) SetToken(client, email string, tok secrets.Token) error { s.token = &tok; return nil } +func (s *fakeStore) DeleteToken(client, email string) error { return nil } +func (s *fakeStore) ListTokens() ([]secrets.Token, error) { return nil, nil } +func (s *fakeStore) GetDefaultAccount(string) (string, error) { return "", nil } +func (s *fakeStore) SetDefaultAccount(string, string) error { return nil } diff --git a/internal/googleapi/transport.go b/internal/googleapi/transport.go index 78e265f61..5afd30612 100644 --- a/internal/googleapi/transport.go +++ b/internal/googleapi/transport.go @@ -49,6 +49,11 @@ type RetryTransport struct { BaseDelay time.Duration CircuitBreaker *CircuitBreaker RefreshAuth func(context.Context) error + // Reauth is called when the OAuth refresh token is expired or revoked + // (invalid_grant). It should launch a browser-based re-authorization + // flow, persist the new refresh token, and return nil on success. + // If Reauth is nil, invalid_grant errors are surfaced without retry. + Reauth func(context.Context) error } // NewRetryTransport creates a RetryTransport with sensible defaults. @@ -82,6 +87,7 @@ func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { retries429 := 0 retries5xx := 0 retriedAuth := false + retriedReauth := false for { // Reset body for retry @@ -99,6 +105,29 @@ func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { resp, err = t.Base.RoundTrip(req) if err != nil { + // Detect invalid_grant from the OAuth2 transport layer. + // This happens when the stored refresh token is expired or + // revoked. If a Reauth function is available and we haven't + // already retried, attempt auto-reauth and retry the request. + if isInvalidGrantError(err) { + if t.Reauth != nil && !retriedReauth && !retryDisabled && replayable { + slog.Debug("refresh token expired or revoked, attempting auto-reauth") + + if reauthErr := t.Reauth(req.Context()); reauthErr != nil { + slog.Debug("auto-reauth failed", "err", reauthErr) + return nil, fmt.Errorf("refresh token expired or revoked: %w; re-authentication failed: %v; run 'gog auth add' to re-authorize manually", err, reauthErr) + } + + retriedReauth = true + continue + } + + // Reauth is not available (suppressed by --no-input, not + // stored OAuth, or already retried). Surface a clear, + // actionable error instead of the raw OAuth2 message. + return nil, fmt.Errorf("refresh token expired or revoked: %w; run 'gog auth add' to re-authorize", err) + } + return nil, fmt.Errorf("round trip: %w", err) } diff --git a/internal/googleauth/reauth.go b/internal/googleauth/reauth.go new file mode 100644 index 000000000..ff59bd694 --- /dev/null +++ b/internal/googleauth/reauth.go @@ -0,0 +1,224 @@ +package googleauth + +import ( + "context" + "fmt" + "log/slog" + "os" + "sort" + "strings" + "time" + + "github.com/openclaw/gogcli/internal/secrets" +) + +// ReauthOptions configures an automatic re-authorization flow triggered +// when the stored refresh token is expired or revoked. +type ReauthOptions struct { + Email string + Client string + Services []string + Scopes []string + // StoredToken, if non-nil, provides the full scope/service set from the + // original authorization. When present, its Scopes and Services are used + // for the re-authorization instead of the triggering request's narrower + // scopes, preventing silent grant narrowing. + StoredToken *secrets.Token + // OpenSecretsStore opens the token store for persisting the new token. + OpenSecretsStore func() (secrets.Store, error) + // EnsureKeychainAccess ensures the keychain is accessible for writes. + // May be nil if keychain access is not needed (e.g. file backend). + EnsureKeychainAccess func(context.Context) error + // AuthorizeFunc performs the OAuth authorization flow. If nil, + // googleauth.Authorize is used. + AuthorizeFunc func(context.Context, AuthorizeOptions) (string, error) + // FetchIdentityFunc fetches the authorized identity. If nil, + // googleauth.IdentityForRefreshToken is used. + FetchIdentityFunc func(context.Context, string, string, []string, time.Duration) (Identity, error) + // Timeout for the browser flow. If zero, defaults to 2 minutes. + Timeout time.Duration + // Stderr is where progress messages are written. If nil, os.Stderr is used. + Stderr interface { + Write([]byte) (int, error) + } +} + +// Reauth performs an automatic re-authorization when the stored refresh +// token is expired or revoked (invalid_grant). It launches a browser-based +// OAuth flow using the same client, services, and scopes as the original +// authorization, then persists the new refresh token to the secret store. +// +// It returns the new refresh token so the caller can update any in-memory +// token source that still holds the revoked token. +// +// This is the auto-reauth counterpart to `gog auth add`, designed to be +// called from the retry transport when an invalid_grant error is detected +// during an API call. +func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { + if opts.Email == "" { + return "", fmt.Errorf("reauth: email is required") + } + if opts.Client == "" { + return "", fmt.Errorf("reauth: client is required") + } + if len(opts.Scopes) == 0 { + return "", fmt.Errorf("reauth: scopes are required") + } + + stderr := opts.Stderr + if stderr == nil { + stderr = os.Stderr + } + + timeout := opts.Timeout + if timeout == 0 { + timeout = 2 * time.Minute + } + + authorizeFn := opts.AuthorizeFunc + if authorizeFn == nil { + authorizeFn = Authorize + } + + fetchIdentityFn := opts.FetchIdentityFunc + if fetchIdentityFn == nil { + fetchIdentityFn = IdentityForRefreshToken + } + + if opts.EnsureKeychainAccess != nil { + if err := opts.EnsureKeychainAccess(ctx); err != nil { + return "", fmt.Errorf("reauth: keychain access: %w", err) + } + } + + // Determine the scopes and services to request. Prefer the stored + // token's full scope set to prevent silent grant narrowing (e.g. a + // calendar command narrowing a gmail+calendar+drive grant). + reauthScopes := opts.Scopes + reauthServices := opts.Services + if opts.StoredToken != nil { + if len(opts.StoredToken.Scopes) > 0 { + reauthScopes = opts.StoredToken.Scopes + } + if len(opts.StoredToken.Services) > 0 { + reauthServices = opts.StoredToken.Services + } + } + + // Convert service strings to Service types. + services := make([]Service, 0, len(reauthServices)) + for _, s := range reauthServices { + svc, err := ParseService(s) + if err != nil { + // If we can't parse the service label, use the scopes directly. + slog.Debug("reauth: could not parse service label, using scopes only", "service", s, "err", err) + continue + } + services = append(services, svc) + } + + // If we couldn't parse any services, derive them from the scopes. + if len(services) == 0 { + services = servicesFromScopes(reauthScopes) + } + + fmt.Fprintln(stderr, "Refresh token expired or revoked. Re-authorizing…") + + authorizeOpts := AuthorizeOptions{ + Services: services, + Scopes: reauthScopes, + ForceConsent: true, // Ensure Google returns a new refresh token + Timeout: timeout, + Client: opts.Client, + } + + refreshToken, err := authorizeFn(ctx, authorizeOpts) + if err != nil { + return "", fmt.Errorf("reauth: authorization failed: %w", err) + } + + // Fetch the authorized identity to verify the email matches. + identity, err := fetchIdentityFn(ctx, opts.Client, refreshToken, reauthScopes, 15*time.Second) + if err != nil { + return "", fmt.Errorf("reauth: fetch authorized identity: %w", err) + } + + authorizedEmail := identity.Email + if authorizedEmail == "" { + authorizedEmail = opts.Email + } + + // Verify the authorized account matches the expected email. + if !strings.EqualFold(strings.TrimSpace(authorizedEmail), strings.TrimSpace(opts.Email)) { + return "", fmt.Errorf("reauth: authorized as %s, expected %s", authorizedEmail, opts.Email) + } + + // Persist the new refresh token. + if opts.OpenSecretsStore == nil { + return "", fmt.Errorf("reauth: secret store opener is required") + } + + store, err := opts.OpenSecretsStore() + if err != nil { + return "", fmt.Errorf("reauth: open secret store: %w", err) + } + + serviceNames := make([]string, 0, len(services)) + for _, svc := range services { + serviceNames = append(serviceNames, string(svc)) + } + sort.Strings(serviceNames) + + if err := store.SetToken(opts.Client, authorizedEmail, secrets.Token{ + Client: opts.Client, + Subject: identity.Subject, + Email: authorizedEmail, + Services: serviceNames, + Scopes: reauthScopes, + RefreshToken: refreshToken, + }); err != nil { + return "", fmt.Errorf("reauth: persist new refresh token: %w", err) + } + + fmt.Fprintln(stderr, "Re-authorization successful. Retrying request…") + + return refreshToken, nil +} + +// servicesFromScopes attempts to derive the service list from a set of +// OAuth scopes. This is a best-effort fallback when the service label +// is not available or cannot be parsed. +func servicesFromScopes(scopes []string) []Service { + scopeSet := make(map[string]struct{}, len(scopes)) + for _, s := range scopes { + scopeSet[s] = struct{}{} + } + + var matched []Service + for _, svc := range serviceOrder { + info, ok := serviceInfoByService[svc] + if !ok || !info.user { + continue + } + + // Check if all scopes for this service are present in the scope set. + allPresent := true + for _, svcScope := range info.scopes { + if _, ok := scopeSet[svcScope]; !ok { + allPresent = false + break + } + } + + if allPresent { + matched = append(matched, svc) + } + } + + if len(matched) == 0 { + // Fallback: return empty, Authorize will use the scopes directly. + return nil + } + + return matched +} diff --git a/internal/googleauth/reauth_test.go b/internal/googleauth/reauth_test.go new file mode 100644 index 000000000..260b53d34 --- /dev/null +++ b/internal/googleauth/reauth_test.go @@ -0,0 +1,363 @@ +package googleauth + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/openclaw/gogcli/internal/config" + "github.com/openclaw/gogcli/internal/secrets" +) + +// mockSecretStore is a minimal secrets.Store for testing Reauth. +type mockSecretStore struct { + tokens map[string]secrets.Token +} + +func (s *mockSecretStore) GetToken(client string, email string) (secrets.Token, error) { + key := client + ":" + email + if tok, ok := s.tokens[key]; ok { + return tok, nil + } + return secrets.Token{}, errors.New("not found") +} + +func (s *mockSecretStore) SetToken(client string, email string, token secrets.Token) error { + key := client + ":" + email + s.tokens[key] = token + return nil +} + +func (s *mockSecretStore) DeleteToken(client string, email string) error { + key := client + ":" + email + delete(s.tokens, key) + return nil +} + + + +func (s *mockSecretStore) ListTokens() ([]secrets.Token, error) { + var out []secrets.Token + for _, tok := range s.tokens { + out = append(out, tok) + } + return out, nil +} + +func (s *mockSecretStore) Keys() ([]string, error) { + keys := make([]string, 0, len(s.tokens)) + for k := range s.tokens { + keys = append(keys, k) + } + return keys, nil +} + +func (s *mockSecretStore) GetDefaultAccount(string) (string, error) { + return "", errors.New("not implemented") +} + +func (s *mockSecretStore) SetDefaultAccount(string, string) error { + return errors.New("not implemented") +} + +func (s *mockSecretStore) Close() error { return nil } + +func TestReauthSuccess(t *testing.T) { + origRead := readClientCredentials + origEndpoint := oauthEndpoint + t.Cleanup(func() { + readClientCredentials = origRead + oauthEndpoint = origEndpoint + }) + + readClientCredentials = func(string) (config.ClientCredentials, error) { + return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil + } + + store := &mockSecretStore{tokens: make(map[string]secrets.Token)} + + authorizeCalled := false + identityCalled := false + + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + OpenSecretsStore: func() (secrets.Store, error) { + return store, nil + }, + EnsureKeychainAccess: func(context.Context) error { return nil }, + AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { + authorizeCalled = true + if authOpts.ForceConsent != true { + t.Fatalf("expected ForceConsent=true") + } + if authOpts.Client != "default" { + t.Fatalf("expected client 'default', got %q", authOpts.Client) + } + return "new-refresh-token", nil + }, + FetchIdentityFunc: func(ctx context.Context, client string, refreshToken string, scopes []string, timeout time.Duration) (Identity, error) { + identityCalled = true + if refreshToken != "new-refresh-token" { + t.Fatalf("expected new-refresh-token, got %q", refreshToken) + } + return Identity{Subject: "sub123", Email: "user@example.com"}, nil + }, + Stderr: &bytesBuffer{}, + } + + if _, err := Reauth(context.Background(), opts); err != nil { + t.Fatalf("Reauth: %v", err) + } + + if !authorizeCalled { + t.Fatal("AuthorizeFunc was not called") + } + + if !identityCalled { + t.Fatal("FetchIdentityFunc was not called") + } + + // Verify token was persisted + tok, err := store.GetToken("default", "user@example.com") + if err != nil { + t.Fatalf("GetToken: %v", err) + } + + if tok.RefreshToken != "new-refresh-token" { + t.Fatalf("expected new-refresh-token, got %q", tok.RefreshToken) + } + + if tok.Email != "user@example.com" { + t.Fatalf("expected user@example.com, got %q", tok.Email) + } + + if tok.Subject != "sub123" { + t.Fatalf("expected sub123, got %q", tok.Subject) + } +} + +func TestReauthAuthorizeFailure(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + OpenSecretsStore: func() (secrets.Store, error) { + return &mockSecretStore{tokens: make(map[string]secrets.Token)}, nil + }, + EnsureKeychainAccess: func(context.Context) error { return nil }, + AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { + return "", errors.New("user denied access") + }, + FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { + t.Fatal("FetchIdentityFunc should not be called on authorize failure") + return Identity{}, nil + }, + Stderr: &bytesBuffer{}, + } + + _, err := Reauth(context.Background(), opts) + if err == nil { + t.Fatal("expected error") + } + + if !strings.Contains(err.Error(), "authorization failed") { + t.Fatalf("error should mention authorization failed: %v", err) + } +} + +func TestReauthMissingEmail(t *testing.T) { + opts := ReauthOptions{ + Client: "default", + Scopes: []string{"scope"}, + OpenSecretsStore: func() (secrets.Store, error) { + return &mockSecretStore{}, nil + }, + } + + _, err := Reauth(context.Background(), opts) + if err == nil { + t.Fatal("expected error for missing email") + } +} + +func TestReauthMissingScopes(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + OpenSecretsStore: func() (secrets.Store, error) { + return &mockSecretStore{}, nil + }, + } + + _, err := Reauth(context.Background(), opts) + if err == nil { + t.Fatal("expected error for missing scopes") + } +} + +func TestReauthKeychainAccessFailure(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Scopes: []string{"scope"}, + OpenSecretsStore: func() (secrets.Store, error) { + return &mockSecretStore{}, nil + }, + EnsureKeychainAccess: func(context.Context) error { + return errors.New("keychain locked") + }, + } + + _, err := Reauth(context.Background(), opts) + if err == nil { + t.Fatal("expected error") + } + + if !strings.Contains(err.Error(), "keychain") { + t.Fatalf("error should mention keychain: %v", err) + } +} + +func TestReauthEmailMismatch(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + OpenSecretsStore: func() (secrets.Store, error) { + return &mockSecretStore{tokens: make(map[string]secrets.Token)}, nil + }, + EnsureKeychainAccess: func(context.Context) error { return nil }, + AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { + return "new-refresh-token", nil + }, + FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { + // Return a *different* email than requested + return Identity{Subject: "sub456", Email: "other@example.com"}, nil + }, + Stderr: &bytesBuffer{}, + } + + _, err := Reauth(context.Background(), opts) + if err == nil { + t.Fatal("expected error for email mismatch") + } + + if !strings.Contains(err.Error(), "authorized as other@example.com") { + t.Fatalf("error should mention email mismatch: %v", err) + } +} + +func TestReauthPreservesStoredScopes(t *testing.T) { + store := &mockSecretStore{tokens: map[string]secrets.Token{ + "default:user@example.com": { + Scopes: []string{ + "https://www.googleapis.com/auth/calendar", + "https://www.googleapis.com/auth/gmail.modify", + "https://www.googleapis.com/auth/drive", + }, + Services: []string{"calendar", "gmail", "drive"}, + }, + }} + + var requestedScopes []string + var requestedServices []string + + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, // narrowed — only the triggering request's service + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, // narrowed + StoredToken: &secrets.Token{ + Scopes: []string{ + "https://www.googleapis.com/auth/calendar", + "https://www.googleapis.com/auth/gmail.modify", + "https://www.googleapis.com/auth/drive", + }, + Services: []string{"calendar", "gmail", "drive"}, + }, + OpenSecretsStore: func() (secrets.Store, error) { + return store, nil + }, + EnsureKeychainAccess: func(context.Context) error { return nil }, + AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { + requestedScopes = authOpts.Scopes + requestedServices = make([]string, len(authOpts.Services)) + for i, svc := range authOpts.Services { + requestedServices[i] = string(svc) + } + return "new-refresh-token", nil + }, + FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { + return Identity{Subject: "sub123", Email: "user@example.com"}, nil + }, + Stderr: &bytesBuffer{}, + } + + if _, err := Reauth(context.Background(), opts); err != nil { + t.Fatalf("Reauth: %v", err) + } + + // Should request the stored token's full scopes, not the narrowed set + if len(requestedScopes) != 3 { + t.Fatalf("expected 3 scopes (preserved from stored token), got %d: %v", len(requestedScopes), requestedScopes) + } + + // Should request the stored token's full services + if len(requestedServices) != 3 { + t.Fatalf("expected 3 services (preserved from stored token), got %d: %v", len(requestedServices), requestedServices) + } + + // Verify persisted token has the full scope set + tok, err := store.GetToken("default", "user@example.com") + if err != nil { + t.Fatalf("GetToken: %v", err) + } + if len(tok.Scopes) != 3 { + t.Fatalf("persisted token should have 3 scopes, got %d: %v", len(tok.Scopes), tok.Scopes) + } +} + +func TestServicesFromScopes(t *testing.T) { + // Calendar scope should match calendar service + services := servicesFromScopes([]string{"https://www.googleapis.com/auth/calendar"}) + if len(services) != 1 || services[0] != ServiceCalendar { + t.Fatalf("expected [calendar], got %v", services) + } + + // Gmail scopes should match gmail service + services = servicesFromScopes([]string{ + "https://www.googleapis.com/auth/gmail.modify", + "https://www.googleapis.com/auth/gmail.settings.basic", + "https://www.googleapis.com/auth/gmail.settings.sharing", + }) + if len(services) != 1 || services[0] != ServiceGmail { + t.Fatalf("expected [gmail], got %v", services) + } + + // Unknown scopes should return empty + services = servicesFromScopes([]string{"https://unknown.example.com/scope"}) + if len(services) != 0 { + t.Fatalf("expected empty, got %v", services) + } +} + +// bytesBuffer is a minimal io.Writer for capturing stderr output in tests. +type bytesBuffer struct { + data []byte +} + +func (b *bytesBuffer) Write(p []byte) (int, error) { + b.data = append(b.data, p...) + return len(p), nil +} + +func (b *bytesBuffer) String() string { + return string(b.data) +} From e45ddcbf73dc36044c484ae3c45bdeffec221079 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 18:58:14 -0700 Subject: [PATCH 2/4] fix(auth): harden revoked-token reauthorization --- CHANGELOG.md | 3 +- docs/auto-reauth-issue-draft.md | 92 ------ internal/cmd/auth.go | 18 ++ internal/cmd/root.go | 6 +- internal/googleapi/auth_dependencies.go | 29 +- internal/googleapi/auto_reauth_test.go | 91 ++++-- internal/googleapi/client.go | 48 +-- internal/googleapi/client_auth.go | 89 ++++-- internal/googleapi/reauth_glue_test.go | 369 +++++++++++------------- internal/googleapi/transport.go | 4 +- internal/googleauth/reauth.go | 92 +++--- internal/googleauth/reauth_test.go | 218 ++++++-------- 12 files changed, 516 insertions(+), 543 deletions(-) delete mode 100644 docs/auto-reauth-issue-draft.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 86ee9c152..e0c1c1b6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## 0.35.1 - Unreleased -- Auth: re-authorize expired or revoked stored OAuth refresh tokens interactively, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. +- Auth: offer one-time re-authorization for expired or revoked stored OAuth refresh tokens after interactive confirmation, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. +- Dependencies: update Kong, Cloudflare Workers types, and pnpm to their latest releases. ## 0.35.0 - 2026-08-09 diff --git a/docs/auto-reauth-issue-draft.md b/docs/auto-reauth-issue-draft.md deleted file mode 100644 index c2e5c888a..000000000 --- a/docs/auto-reauth-issue-draft.md +++ /dev/null @@ -1,92 +0,0 @@ -# Feature: Auto-reauth on expired/revoked refresh tokens - -## Summary - -When a stored OAuth refresh token is expired or revoked (`invalid_grant`), -`gog` fails with a hard error and requires the user to manually re-run -`gog auth add`. This is in contrast to other CLI tools (e.g. Rust's -`yup-oauth2` `InstalledFlowAuthenticator`) which automatically fall back to -a browser-based re-authorization flow when the refresh token is invalid, -making the failure transparent to the user. - -## Problem - -When the refresh token becomes invalid (Google revokes it for various -reasons: 6-month inactivity, password change, token limit reached, app in -Testing mode with 7-day expiry, user manually revoking access), any -`gog` command that needs authentication fails: - -``` -$ gog calendar events --today -Error: refresh access token: oauth2: "invalid_grant" "Token has been expired or revoked." -``` - -The user must then manually run: - -``` -gog auth add you@example.com --services gmail,calendar,drive -``` - -This is particularly frustrating because: - -1. **It's a hard stop** — no graceful degradation or recovery hint beyond the - raw OAuth error. -2. **The error message doesn't tell the user what to do** — it's a wrapped - `golang.org/x/oauth2` error, not a user-facing diagnostic. -3. **It happens repeatedly** — especially for users whose OAuth app is in - Testing mode (7-day refresh token expiry) or who don't use `gog` daily. -4. **Other tools handle this transparently** — `yup-oauth2` (used by the - `today` Rust CLI) detects `invalid_grant` during token refresh and - automatically falls back to the Installed Flow (opens a browser, obtains a - new refresh token, persists it, and retries the original request). - -## Proposed behavior - -When `gog` detects `invalid_grant` during a token refresh attempt: - -1. **If running interactively** (stdin is a terminal, `--no-input` is not set): - - Print a message to stderr: "Refresh token expired or revoked. Re-authorizing…" - - Automatically launch the browser-based OAuth flow (same as `gog auth add`) - using the stored account's services and client. - - On success, persist the new refresh token to the keyring and retry the - original API request. - - On failure (user denies, browser doesn't open, etc.), surface a clear - error with the re-auth command to run manually. - -2. **If running non-interactively** (`--no-input`, CI, pipes): - - Do NOT auto-launch a browser. - - Surface a clear error message with the exact `gog auth add` command to run. - -## Design considerations - -- **Security**: Auto-reauth should only trigger when the refresh token is - specifically revoked/expired (`invalid_grant`), not for other OAuth errors. - The browser flow uses the same PKCE + state validation as `gog auth add`. -- **Scope preservation**: The reauth should request the same services/scopes - as the stored token, not a broader or narrower set. -- `--force-consent` should be used during auto-reauth to ensure Google - returns a new refresh token (without it, Google may omit the refresh token - for returning users). -- **Keychain access**: The reauth flow needs keychain write access to persist - the new token. On macOS, this may trigger a Keychain permission prompt. -- **Timeout**: The auto-reauth browser flow should have a reasonable timeout - (e.g. 2 minutes) to avoid hanging indefinitely in CI-like environments. - -## Prior art - -- `yup-o-auth2` (Rust): `InstalledFlowAuthenticator::find_token_info()` — - on refresh failure, falls back to `auth_flow.token()` which opens a browser. - Source: [authenticator.rs](https://github.com/dermesser/yup-oauth2/blob/master/src/authenticator.rs) - -- gogcli already has partial auth resilience: - - v0.31.0: Recover from corrupt token payloads (#872) - - v0.32.0: Retry on 403 insufficient scopes by refreshing credentials (#889) - - v0.33.0: Trust Developer-ID-signed binaries for Keychain access - - Auto-reauth on `invalid_grant` is the next gap in this progression. - -## Environment - -- gog: v0.34.0 (Homebrew) -- macOS: 15.x (also reproduced on macOS 27 Tahoe beta) -- Keyring: macOS Keychain (also affects file backend) diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 296579979..3036d4982 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -2,16 +2,34 @@ package cmd import ( "context" + "errors" "fmt" + "io" + "os" "strings" "time" "github.com/openclaw/gogcli/internal/app" "github.com/openclaw/gogcli/internal/config" "github.com/openclaw/gogcli/internal/googleauth" + "github.com/openclaw/gogcli/internal/input" "github.com/openclaw/gogcli/internal/secrets" ) +func confirmReauthorization(ctx context.Context, email string) (bool, error) { + prompt := fmt.Sprintf("Refresh token for %s expired or was revoked. Re-authorize now? [y/N]: ", strings.TrimSpace(email)) + line, err := input.PromptLineFrom(ctx, prompt, stdinReader(ctx)) + if err != nil { + if errors.Is(err, io.EOF) || errors.Is(err, os.ErrClosed) { + return false, nil + } + return false, fmt.Errorf("read confirmation: %w", err) + } + + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes", nil +} + func openAuthSecretsStore(ctx context.Context) (secrets.Store, error) { if runtime, ok := app.FromContext(ctx); ok && runtime.Auth.OpenSecretsStore != nil { return runtime.Auth.OpenSecretsStore() diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 77e4deadc..60a8a2f11 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -254,17 +254,18 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { // reauthFn is the auto-reauth closure called when the stored refresh // token is expired or revoked (invalid_grant). It launches a browser- // based OAuth flow and persists the new token, mirroring `gog auth add`. - reauthFn := func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (string, error) { + reauthFn := func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (secrets.Token, error) { opts := googleauth.ReauthOptions{ Email: email, Client: client, Services: services, Scopes: scopes, StoredToken: storedToken, - OpenSecretsStore: openTokens, EnsureKeychainAccess: ensureKeychainAccessIfNeeded, AuthorizeFunc: authorizeGoogleAccount, FetchIdentityFunc: fetchAuthIdentity, + Confirm: confirmReauthorization, + Stderr: runtimeIO.Err, } return googleauth.Reauth(ctx, opts) } @@ -279,6 +280,7 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { ADCTokenSource: googleapi.DefaultADCTokenSource, ServiceAccountTokenSource: googleapi.DefaultServiceAccountTokenSource, Reauth: reauthFn, + ReauthCoordinator: googleapi.NewReauthCoordinator(), } ctx = googleapi.WithAuthDependencies(ctx, authDependencies) composeRuntimeGoogleServices(runtime, googleapi.NewFactory(authDependencies, googleapi.FactoryOptions{ diff --git a/internal/googleapi/auth_dependencies.go b/internal/googleapi/auth_dependencies.go index f1190beab..272f62a5e 100644 --- a/internal/googleapi/auth_dependencies.go +++ b/internal/googleapi/auth_dependencies.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" "golang.org/x/oauth2" "golang.org/x/oauth2/google" @@ -27,6 +28,27 @@ type ( ServiceAccountTokenSourceFunc func(context.Context, []byte, string, []string) (oauth2.TokenSource, error) ) +// ReauthCoordinator serializes interactive recovery across every Google +// service client created for one command invocation. +type ReauthCoordinator struct { + mu sync.Mutex +} + +func NewReauthCoordinator() *ReauthCoordinator { + return &ReauthCoordinator{} +} + +func (c *ReauthCoordinator) run(fn func() error) error { + if c == nil { + return fn() + } + + c.mu.Lock() + defer c.mu.Unlock() + + return fn() +} + // ReauthFunc attempts to re-authorize the given account by launching a // browser-based OAuth flow and persisting the new refresh token. It is // called automatically when the stored refresh token is expired or revoked @@ -34,9 +56,9 @@ type ( // // storedToken, if non-nil, carries the full scope/service set from the // original authorization so the reauth can preserve the grant width. -// Returns the new refresh token so the caller can update any in-memory -// token source that still holds the revoked token. -type ReauthFunc func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (string, error) +// Returns the replacement token metadata. The token-source owner persists it +// and swaps the in-memory refresh token as one serialized operation. +type ReauthFunc func(ctx context.Context, email string, client string, services []string, scopes []string, storedToken *secrets.Token) (secrets.Token, error) type AuthDependencies struct { ResolveClient authclient.ClientResolver @@ -48,6 +70,7 @@ type AuthDependencies struct { ADCTokenSource ADCTokenSourceFunc ServiceAccountTokenSource ServiceAccountTokenSourceFunc Reauth ReauthFunc + ReauthCoordinator *ReauthCoordinator } var ( diff --git a/internal/googleapi/auto_reauth_test.go b/internal/googleapi/auto_reauth_test.go index 48e9ef779..77b265070 100644 --- a/internal/googleapi/auto_reauth_test.go +++ b/internal/googleapi/auto_reauth_test.go @@ -12,6 +12,12 @@ import ( "golang.org/x/oauth2" ) +var ( + errAutoReauthNetworkTimeout = errors.New("network timeout") + errAutoReauthUntypedInvalidGrant = errors.New(`oauth2: "invalid_grant" "Bad Request"`) + errAutoReauthBrowserNotOpen = errors.New("browser did not open") +) + func TestIsInvalidGrantError(t *testing.T) { tests := []struct { name string @@ -25,22 +31,22 @@ func TestIsInvalidGrantError(t *testing.T) { }, { name: "unrelated error", - err: errors.New("network timeout"), + err: errAutoReauthNetworkTimeout, want: false, }, { name: `invalid_grant with "Token has been expired or revoked"`, - err: errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`), + err: invalidGrantError(), want: true, }, { - name: "invalid_grant Bad Request", - err: errors.New(`oauth2: "invalid_grant" "Bad Request"`), - want: true, + name: "untyped invalid_grant text", + err: errAutoReauthUntypedInvalidGrant, + want: false, }, { name: "wrapped invalid_grant", - err: fmt.Errorf("refresh access token: %w", errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`)), + err: fmt.Errorf("refresh access token: %w", invalidGrantError()), want: true, }, } @@ -54,6 +60,13 @@ func TestIsInvalidGrantError(t *testing.T) { } } +func invalidGrantError() error { + return &oauth2.RetrieveError{ + ErrorCode: "invalid_grant", + ErrorDescription: "Token has been expired or revoked.", + } +} + func TestRetryTransportAutoReauthOnInvalidGrant(t *testing.T) { tokenSource := &refreshableTestTokenSource{token: "stale-token"} reauthCalls := 0 @@ -73,7 +86,7 @@ func TestRetryTransportAutoReauthOnInvalidGrant(t *testing.T) { // First call: the oauth2 transport will fail with invalid_grant // because the refresh token is stale. We simulate this by // returning an error from the base transport. - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() case 2: // After reauth, the token should be fresh. if got := req.Header.Get("Authorization"); got != "Bearer fresh-token" { @@ -135,13 +148,13 @@ func TestRetryTransportAutoReauthFailureSurfacesError(t *testing.T) { calls++ _ = req.Body - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() }) rt := &RetryTransport{ Base: base, Reauth: func(context.Context) error { - return errors.New("browser did not open") + return errAutoReauthBrowserNotOpen }, } @@ -150,7 +163,11 @@ func TestRetryTransportAutoReauthFailureSurfacesError(t *testing.T) { t.Fatalf("new request: %v", err) } - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { t.Fatalf("expected error") } @@ -173,12 +190,12 @@ func TestRetryTransportNoReauthWhenNil(t *testing.T) { base := roundTripFunc(func(req *http.Request) (*http.Response, error) { calls++ - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() }) rt := &RetryTransport{ - Base: base, - Reauth: nil, // No reauth function + Base: base, + Reauth: nil, // No reauth function } req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://example.com", nil) @@ -186,7 +203,11 @@ func TestRetryTransportNoReauthWhenNil(t *testing.T) { t.Fatalf("new request: %v", err) } - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { t.Fatalf("expected error") } @@ -210,7 +231,7 @@ func TestRetryTransportNoReauthWhenRetriesDisabled(t *testing.T) { base := roundTripFunc(func(req *http.Request) (*http.Response, error) { calls++ - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() }) rt := &RetryTransport{ @@ -222,12 +243,17 @@ func TestRetryTransportNoReauthWhenRetriesDisabled(t *testing.T) { } ctx := WithoutRetries(context.Background()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://example.com", nil) if err != nil { t.Fatalf("new request: %v", err) } - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { t.Fatalf("expected error") } @@ -257,7 +283,8 @@ func TestRetryTransportNoReauthForNonReplayableBody(t *testing.T) { calls++ body, _ := io.ReadAll(req.Body) _ = body - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + + return nil, invalidGrantError() }) rt := &RetryTransport{ @@ -275,7 +302,11 @@ func TestRetryTransportNoReauthForNonReplayableBody(t *testing.T) { } req.ContentLength = maxBufferedReplayBodyBytes + 1 - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { t.Fatalf("expected error") } @@ -300,7 +331,7 @@ func TestRetryTransportReauthOnlyOnce(t *testing.T) { base := roundTripFunc(func(req *http.Request) (*http.Response, error) { calls++ // Always return invalid_grant, even after reauth - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() }) rt := &RetryTransport{ @@ -316,7 +347,11 @@ func TestRetryTransportReauthOnlyOnce(t *testing.T) { t.Fatalf("new request: %v", err) } - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { t.Fatalf("expected error") } @@ -372,7 +407,7 @@ func TestRetryTransportReauthResetsTokenSource(t *testing.T) { case 1: // First call: the oauth2 transport tries to refresh using the // revoked token and fails with invalid_grant. - return nil, errors.New(`oauth2: "invalid_grant" "Token has been expired or revoked."`) + return nil, invalidGrantError() case 2: // After reauth, the token source should have been reset. // If it wasn't, this test would fail because the reauth @@ -380,10 +415,12 @@ func TestRetryTransportReauthResetsTokenSource(t *testing.T) { if source.refreshToken == "revoked-token" { t.Fatal("token source was not reset after reauth; retried request would use revoked token") } + return newTestResponse(http.StatusOK, "ok"), nil default: t.Fatalf("unexpected call %d", calls) - return nil, nil + + return nil, errUnexpectedRequestBody } }) @@ -398,6 +435,7 @@ func TestRetryTransportReauthResetsTokenSource(t *testing.T) { // persists it to the store. The critical step is resetting // the in-memory token source. source.ResetRefreshToken("new-refresh-token") + return nil }, } @@ -452,10 +490,15 @@ type oauth2TransportWrapper struct { func (w *oauth2TransportWrapper) RoundTrip(req *http.Request) (*http.Response, error) { tok, err := w.source.Token() if err != nil { - return nil, err + return nil, fmt.Errorf("get OAuth token: %w", err) } req.Header.Set("Authorization", "Bearer "+tok.AccessToken) - return w.base.RoundTrip(req) + resp, err := w.base.RoundTrip(req) + if err != nil { + return nil, fmt.Errorf("base round trip: %w", err) + } + + return resp, nil } diff --git a/internal/googleapi/client.go b/internal/googleapi/client.go index baef62952..c95234fe7 100644 --- a/internal/googleapi/client.go +++ b/internal/googleapi/client.go @@ -130,7 +130,7 @@ func authenticatedTransportWithStoredScopeCheck( requireStoredGrant bool, ) (http.RoundTripper, error) { var ts oauth2.TokenSource - var isStoredOAuth bool + var storedOAuth *persistingTokenSource if dependencies, ok := authDependenciesFromContext(ctx); ok && dependencies.Mode == AuthModeADC { slog.Debug("using Application Default Credentials (GOG_AUTH_MODE=adc)", "serviceLabel", serviceLabel) @@ -154,7 +154,7 @@ func authenticatedTransportWithStoredScopeCheck( // (StaticTokenSource) and service-account sources never return // invalid_grant through the refresh-token path, but even if they // did, launching a user browser flow would be nonsensical. - _, isStoredOAuth = ts.(*persistingTokenSource) + storedOAuth, _ = ts.(*persistingTokenSource) } retryTransport := NewRetryTransport(&oauth2.Transport{ @@ -171,8 +171,8 @@ func authenticatedTransportWithStoredScopeCheck( // Wire auto-reauth for invalid_grant (expired/revoked refresh token). // Only enabled for stored OAuth (not ADC, not direct access tokens, // not service accounts) and only when the session is interactive. - if isStoredOAuth && !NoInputFromContext(ctx) { - if reauthFn := reauthFunctionFromContext(ctx, serviceLabel, email, scopes, ts); reauthFn != nil { + if storedOAuth != nil && !NoInputFromContext(ctx) { + if reauthFn := reauthFunctionFromContext(ctx, serviceLabel, email, scopes, storedOAuth); reauthFn != nil { retryTransport.Reauth = reauthFn } } @@ -325,14 +325,12 @@ func newBaseTransport() *http.Transport { // The token source is captured so that after a successful reauth, the // in-memory token source can be reset with the new refresh token — // otherwise the retried request would reuse the revoked token. -func reauthFunctionFromContext(ctx context.Context, serviceLabel string, email string, scopes []string, ts oauth2.TokenSource) func(context.Context) error { +func reauthFunctionFromContext(ctx context.Context, serviceLabel string, email string, scopes []string, ts *persistingTokenSource) func(context.Context) error { dependencies, ok := authDependenciesFromContext(ctx) if !ok || dependencies.Reauth == nil { return nil } - resolvedEmail := email - // Derive the service list from the single service label when available. var services []string if serviceLabel != "" { @@ -340,36 +338,10 @@ func reauthFunctionFromContext(ctx context.Context, serviceLabel string, email s } return func(ctx context.Context) error { - // Resolve the client at call time so it reflects the latest config. - client, err := dependencies.resolveClient(resolvedEmail, authclient.ClientOverrideFromContext(ctx)) - if err != nil { - return fmt.Errorf("resolve client for reauth: %w", err) - } - - // Load the stored token to preserve the full scope/service set - // from the original authorization, preventing silent grant narrowing. - var storedToken *secrets.Token - if store, storeErr := dependencies.openTokens(); storeErr == nil { - if tok, getErr := store.GetToken(client, resolvedEmail); getErr == nil { - storedToken = &tok - } - } - - newRefreshToken, err := dependencies.Reauth(ctx, resolvedEmail, client, services, scopes, storedToken) - if err != nil { - return err - } - - // Reset the in-memory token source so the retried request uses - // the new refresh token instead of the revoked one. Guard against - // an empty token to avoid rebuilding the source with no refresh - // token, which would produce a confusing refresh error. - if newRefreshToken != "" { - if resetter, ok := ts.(refreshTokenResetter); ok { - resetter.ResetRefreshToken(newRefreshToken) - } - } - - return nil + return dependencies.ReauthCoordinator.run(func() error { + return ts.Reauthorize(ctx, func(ctx context.Context, storedToken secrets.Token) (secrets.Token, error) { + return dependencies.Reauth(ctx, email, ts.client, services, scopes, &storedToken) + }) + }) } } diff --git a/internal/googleapi/client_auth.go b/internal/googleapi/client_auth.go index b71fd7b1b..fa3bf41d2 100644 --- a/internal/googleapi/client_auth.go +++ b/internal/googleapi/client_auth.go @@ -42,6 +42,9 @@ type forceRefreshTokenSource interface { var ( errBaseTokenSourceCannotForceRefresh = errors.New("base token source cannot force refresh") errBaseTokenSourceReturnedNilToken = errors.New("base token source returned nil token") + errBaseTokenSourceCannotReplaceToken = errors.New("base token source cannot replace refresh token") + errReauthEmptyRefreshToken = errors.New("reauthorization returned an empty refresh token") + errReauthAccountMismatch = errors.New("reauthorization returned a different account") ) // isInvalidGrantError checks whether an OAuth2 token refresh error is an @@ -52,16 +55,9 @@ func isInvalidGrantError(err error) bool { return false } - msg := strings.ToLower(err.Error()) + var retrieveErr *oauth2.RetrieveError - // golang.org/x/oauth2 wraps the Google response in a *RetrieveError - // whose Error() contains the raw JSON body. Google's response for a - // revoked/expired refresh token is: - // {"error":"invalid_grant","error_description":"Token has been expired or revoked."} - // - // We match on "invalid_grant" to catch all variants (expired, revoked, - // bad request) without being brittle to the exact error_description. - return strings.Contains(msg, "invalid_grant") + return errors.As(err, &retrieveErr) && strings.EqualFold(strings.TrimSpace(retrieveErr.ErrorCode), "invalid_grant") } type resettableOAuthTokenSource struct { @@ -180,18 +176,73 @@ func (p *persistingTokenSource) ForceRefresh(ctx context.Context) error { return err } -// ResetRefreshToken delegates to the base token source to replace the -// stored refresh token after a successful auto-reauth. -func (p *persistingTokenSource) ResetRefreshToken(refreshToken string) { - if resetter, ok := p.base.(*resettableOAuthTokenSource); ok { - resetter.ResetRefreshToken(refreshToken) +// Reauthorize serializes account recovery with ordinary token refreshes. A +// second caller rechecks the token after taking the lock, so concurrent API +// requests cannot launch duplicate browser flows for the same revoked token. +func (p *persistingTokenSource) Reauthorize( + ctx context.Context, + reauthorize func(context.Context, secrets.Token) (secrets.Token, error), +) error { + p.mu.Lock() + defer p.mu.Unlock() + + // Another request may have repaired the shared source while this request + // was waiting. In that case the token succeeds now and no browser is needed. + if t, err := p.base.Token(); err == nil { + _, persistErr := p.persistTokenLocked(t) + return persistErr + } else if !isInvalidGrantError(err) { + return fmt.Errorf("recheck refresh token before reauthorization: %w", err) } -} -// refreshTokenResetter is implemented by token sources that can replace -// their stored refresh token at runtime (used after auto-reauth). -type refreshTokenResetter interface { - ResetRefreshToken(refreshToken string) + resetter, ok := p.base.(*resettableOAuthTokenSource) + if !ok { + return errBaseTokenSourceCannotReplaceToken + } + + // A different service client may have completed reauthorization while this + // source waited on the command-wide coordinator. Adopt that stored token + // before deciding another browser flow is necessary. + if latest, storeErr := p.store.GetToken(p.client, p.email); storeErr == nil { + latestRefreshToken := strings.TrimSpace(latest.RefreshToken) + if latestRefreshToken != "" && latestRefreshToken != strings.TrimSpace(p.tok.RefreshToken) { + resetter.ResetRefreshToken(latestRefreshToken) + p.tok = latest + + if t, tokenErr := p.base.Token(); tokenErr == nil { + _, persistErr := p.persistTokenLocked(t) + return persistErr + } else if !isInvalidGrantError(tokenErr) { + return fmt.Errorf("use concurrently reauthorized token: %w", tokenErr) + } + } + } + + updated, err := reauthorize(ctx, p.tok) + if err != nil { + return err + } + + updated.RefreshToken = strings.TrimSpace(updated.RefreshToken) + if updated.RefreshToken == "" { + return errReauthEmptyRefreshToken + } + + persistEmail := strings.TrimSpace(p.email) + if updatedEmail := strings.TrimSpace(updated.Email); updatedEmail != "" && !strings.EqualFold(updatedEmail, persistEmail) { + return fmt.Errorf("%w: got %s, expected %s", errReauthAccountMismatch, updatedEmail, persistEmail) + } + updated.Client = p.client + updated.Email = persistEmail + + if err := p.store.SetToken(p.client, persistEmail, updated); err != nil { + return fmt.Errorf("persist reauthorized token: %w", err) + } + + resetter.ResetRefreshToken(updated.RefreshToken) + p.tok = updated + + return nil } func (p *persistingTokenSource) persistTokenLocked(t *oauth2.Token) (*oauth2.Token, error) { diff --git a/internal/googleapi/reauth_glue_test.go b/internal/googleapi/reauth_glue_test.go index c52490dfb..42c3c05c7 100644 --- a/internal/googleapi/reauth_glue_test.go +++ b/internal/googleapi/reauth_glue_test.go @@ -3,274 +3,247 @@ package googleapi import ( "context" "errors" + "fmt" + "sync" "testing" "golang.org/x/oauth2" - "golang.org/x/oauth2/google" "github.com/openclaw/gogcli/internal/config" "github.com/openclaw/gogcli/internal/secrets" ) -// TestReauthFunctionFromContextResetsTokenSource verifies that the production -// closure returned by reauthFunctionFromContext actually calls -// ResetRefreshToken on the real persistingTokenSource chain after a -// successful reauth. This is the integration test for the glue code that -// was broken in the initial implementation. -func TestReauthFunctionFromContextResetsTokenSource(t *testing.T) { - // Build a real resettableOAuthTokenSource → persistingTokenSource chain. - // The oauth2.Config's TokenSource will use the refresh token we give it. - // We simulate a token endpoint that returns invalid_grant for the old - // token and success for the new one. - - var reauthRefreshToken string - - // Create the auth dependencies with a fake Reauth that returns a new - // refresh token (simulating a successful browser flow). +var ( + errReauthTestMissingRefreshToken = errors.New("missing refresh token") + errReauthTestTokenNotFound = errors.New("token not found") +) + +func TestReauthFunctionFromContextPersistsAndResetsTokenSource(t *testing.T) { + store := &fakeStore{token: &secrets.Token{ + Email: "user@example.com", + RefreshToken: "revoked-token", + Scopes: []string{"scope"}, + }} + base := reauthTestTokenSource() + persisting := newPersistingTokenSource(base, store, "default", "user@example.com", *store.token, "calendar", nil).(*persistingTokenSource) + deps := AuthDependencies{ ResolveClient: func(string, string) (string, error) { return "default", nil }, ReadCredentials: func(string) (config.ClientCredentials, error) { return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil }, - OpenTokens: func() (secrets.Store, error) { - return &fakeStore{}, nil - }, - Reauth: func(ctx context.Context, email, client string, services, scopes []string, storedToken *secrets.Token) (string, error) { - reauthRefreshToken = "new-refresh-token" - return "new-refresh-token", nil + OpenTokens: func() (secrets.Store, error) { return store, nil }, + Reauth: func(context.Context, string, string, []string, []string, *secrets.Token) (secrets.Token, error) { + return secrets.Token{ + Email: "user@example.com", + RefreshToken: "new-refresh-token", + Scopes: []string{"scope"}, + }, nil }, } - ctx := WithAuthDependencies(context.Background(), deps) - // Build a real token source chain as the transport layer would. - cfg := oauth2.Config{ - ClientID: "id", - ClientSecret: "secret", - Endpoint: google.Endpoint, - Scopes: []string{"https://www.googleapis.com/auth/calendar"}, - } - - // The transport construction captures the token source; we simulate - // what tokenSourceForAccountScopesWithStoredScopeCheck does. - baseSource := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { - return cfg.TokenSource(context.Background(), t) - }, &oauth2.Token{ - RefreshToken: "revoked-token", - }) - - ts := newPersistingTokenSource(baseSource, &fakeStore{}, "default", "user@example.com", secrets.Token{ - Email: "user@example.com", - RefreshToken: "revoked-token", - Scopes: []string{"https://www.googleapis.com/auth/calendar"}, - }, "calendar", nil) - - // Get the reauth closure. - reauthFn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"https://www.googleapis.com/auth/calendar"}, ts) + reauthFn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope"}, persisting) if reauthFn == nil { t.Fatal("expected non-nil reauth function") } - // Call the reauth closure. if err := reauthFn(context.Background()); err != nil { t.Fatalf("reauth closure: %v", err) } - if reauthRefreshToken != "new-refresh-token" { - t.Fatalf("Reauth was not called or returned unexpected token: %q", reauthRefreshToken) + if got := base.refreshToken; got != "new-refresh-token" { + t.Fatalf("in-memory refresh token = %q, want new-refresh-token", got) } - // Verify the token source was actually reset by checking the inner - // resettableOAuthTokenSource's refresh token. - persisting, ok := ts.(*persistingTokenSource) - if !ok { - t.Fatalf("expected *persistingTokenSource, got %T", ts) + if got := store.token.RefreshToken; got != "new-refresh-token" { + t.Fatalf("stored refresh token = %q, want new-refresh-token", got) } - resettable, ok := persisting.base.(*resettableOAuthTokenSource) - if !ok { - t.Fatalf("expected *resettableOAuthTokenSource, got %T", persisting.base) + // A normal refresh response commonly omits RefreshToken. Persisting its + // access-token metadata must retain the replacement refresh token. + if _, err := persisting.Token(); err != nil { + t.Fatalf("post-reauth token: %v", err) } - if resettable.refreshToken != "new-refresh-token" { - t.Fatalf("token source was not reset: expected new-refresh-token, got %q", resettable.refreshToken) + if got := store.token.RefreshToken; got != "new-refresh-token" { + t.Fatalf("stored refresh token after refresh = %q, want new-refresh-token", got) } } -// TestReauthFunctionFromContextNilWhenNoDeps verifies that the closure is -// nil when auth dependencies are not in the context. -func TestReauthFunctionFromContextNilWhenNoDeps(t *testing.T) { - fn := reauthFunctionFromContext(context.Background(), "calendar", "user@example.com", []string{"scope"}, nil) - if fn != nil { - t.Fatal("expected nil reauth function when no deps in context") - } -} - -// TestReauthFunctionFromContextNilWhenNoReauthFunc verifies that the closure -// is nil when the Reauth field is not set on the dependencies. -func TestReauthFunctionFromContextNilWhenNoReauthFunc(t *testing.T) { +func TestPersistingTokenSourceReauthorizeCoalescesConcurrentCalls(t *testing.T) { + store := &fakeStore{token: &secrets.Token{Email: "user@example.com", RefreshToken: "revoked-token"}} + original := *store.token + firstBase := reauthTestTokenSource() + secondBase := reauthTestTokenSource() + first := newPersistingTokenSource( + firstBase, + store, + "default", + "user@example.com", + original, + "calendar", + nil, + ).(*persistingTokenSource) + second := newPersistingTokenSource( + secondBase, + store, + "default", + "user@example.com", + original, + "calendar", + nil, + ).(*persistingTokenSource) + + started := make(chan struct{}) + release := make(chan struct{}) + var mu sync.Mutex + calls := 0 deps := AuthDependencies{ - ResolveClient: func(string, string) (string, error) { return "default", nil }, - ReadCredentials: func(string) (config.ClientCredentials, error) { return config.ClientCredentials{}, nil }, - OpenTokens: func() (secrets.Store, error) { return &fakeStore{}, nil }, - // Reauth is nil - } - ctx := WithAuthDependencies(context.Background(), deps) - fn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope"}, nil) - if fn != nil { - t.Fatal("expected nil reauth function when Reauth is nil") - } -} + ReauthCoordinator: NewReauthCoordinator(), + Reauth: func(ctx context.Context, _ string, _ string, _ []string, _ []string, stored *secrets.Token) (secrets.Token, error) { + mu.Lock() + calls++ -// TestResettableOAuthTokenSourceResetRefreshToken verifies that -// ResetRefreshToken replaces the refresh token and rebuilds the source. -func TestResettableOAuthTokenSourceResetRefreshToken(t *testing.T) { - var tokenRequests []string - - source := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { - // Track which refresh token is being used. - tokenRequests = append(tokenRequests, t.RefreshToken) - // Return a stub token source that always succeeds. - return &stubTokenSource{token: &oauth2.Token{AccessToken: "access"}} - }, &oauth2.Token{ - RefreshToken: "old-token", - }) - - // Verify initial state - if source.refreshToken != "old-token" { - t.Fatalf("expected old-token, got %q", source.refreshToken) - } + if calls == 1 { + close(started) + } - // Reset - source.ResetRefreshToken("new-token") + mu.Unlock() + <-release - if source.refreshToken != "new-token" { - t.Fatalf("expected new-token, got %q", source.refreshToken) - } + if err := ctx.Err(); err != nil { + return secrets.Token{}, fmt.Errorf("reauthorization context: %w", err) + } - // Verify the source was rebuilt — calling Token() should use the new token - _, _ = source.Token() + updated := *stored + updated.RefreshToken = "new-refresh-token" - if len(tokenRequests) == 0 { - t.Fatal("Token() did not create a new source") + return updated, nil + }, } + ctx := WithAuthDependencies(context.Background(), deps) + firstReauth := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope"}, first) + secondReauth := reauthFunctionFromContext(ctx, "drive", "user@example.com", []string{"scope"}, second) - if tokenRequests[len(tokenRequests)-1] != "new-token" { - t.Fatalf("expected new-token in request, got %q", tokenRequests[len(tokenRequests)-1]) - } -} + errs := make(chan error, 2) + go func() { errs <- firstReauth(context.Background()) }() -// TestPersistingTokenSourceResetRefreshToken verifies delegation. -func TestPersistingTokenSourceResetRefreshToken(t *testing.T) { - base := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { - return (&oauth2.Config{ - ClientID: "id", - ClientSecret: "secret", - Endpoint: oauth2.Endpoint{ - AuthURL: "https://example.com/auth", - TokenURL: "https://example.com/token", - }, - Scopes: []string{"scope"}, - }).TokenSource(context.Background(), t) - }, &oauth2.Token{RefreshToken: "old"}) - - persisting := newPersistingTokenSource(base, &fakeStore{}, "default", "user@example.com", secrets.Token{ - Email: "user@example.com", - RefreshToken: "old", - }, "calendar", nil) + <-started - // Reset via the persisting wrapper - persisting.(*persistingTokenSource).ResetRefreshToken("new-refresh") + go func() { errs <- secondReauth(context.Background()) }() - // Verify the inner source was updated - if base.refreshToken != "new-refresh" { - t.Fatalf("expected new-refresh, got %q", base.refreshToken) - } -} + close(release) -// TestReauthClosureLoadsStoredToken verifies the closure loads the stored -// token and passes it through to Reauth for scope preservation. -func TestReauthClosureLoadsStoredToken(t *testing.T) { - store := &fakeStore{ - token: &secrets.Token{ - Email: "user@example.com", - Scopes: []string{"scope1", "scope2", "scope3"}, - Services: []string{"calendar", "gmail", "drive"}, - }, + for range 2 { + if err := <-errs; err != nil { + t.Fatalf("reauthorize: %v", err) + } } - var passedStoredToken *secrets.Token + mu.Lock() + defer mu.Unlock() - deps := AuthDependencies{ - ResolveClient: func(string, string) (string, error) { return "default", nil }, - ReadCredentials: func(string) (config.ClientCredentials, error) { - return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil - }, - OpenTokens: func() (secrets.Store, error) { return store, nil }, - Reauth: func(ctx context.Context, email, client string, services, scopes []string, storedToken *secrets.Token) (string, error) { - passedStoredToken = storedToken - return "new-token", nil - }, + if calls != 1 { + t.Fatalf("browser reauthorization calls = %d, want 1", calls) } - ctx := WithAuthDependencies(context.Background(), deps) + if got := firstBase.refreshToken; got != "new-refresh-token" { + t.Fatalf("first source refresh token = %q, want new-refresh-token", got) + } + + if got := secondBase.refreshToken; got != "new-refresh-token" { + t.Fatalf("second source refresh token = %q, want new-refresh-token", got) + } +} - // Build a real token source so the reset path works - baseSource := newResettableOAuthTokenSource(func(t *oauth2.Token) oauth2.TokenSource { - return (&oauth2.Config{ - ClientID: "id", - ClientSecret: "secret", - Endpoint: google.Endpoint, - Scopes: []string{"scope"}, - }).TokenSource(context.Background(), t) - }, &oauth2.Token{RefreshToken: "old"}) - - ts := newPersistingTokenSource(baseSource, store, "default", "user@example.com", secrets.Token{ +func TestReauthClosureLoadsStoredToken(t *testing.T) { + store := &fakeStore{token: &secrets.Token{ Email: "user@example.com", - RefreshToken: "old", - }, "calendar", nil) + RefreshToken: "revoked-token", + Scopes: []string{"scope1", "scope2", "scope3"}, + Services: []string{"calendar", "gmail", "drive"}, + }} + persisting := newPersistingTokenSource( + reauthTestTokenSource(), + store, + "default", + "user@example.com", + *store.token, + "calendar", + nil, + ).(*persistingTokenSource) + + var passed *secrets.Token + deps := AuthDependencies{ + Reauth: func(_ context.Context, _, _ string, _, _ []string, stored *secrets.Token) (secrets.Token, error) { + snapshot := *stored + passed = &snapshot + snapshot.RefreshToken = "new-refresh-token" - reauthFn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope1"}, ts) - if reauthFn == nil { - t.Fatal("expected non-nil reauth function") + return snapshot, nil + }, } + fn := reauthFunctionFromContext(WithAuthDependencies(context.Background(), deps), "calendar", "user@example.com", []string{"scope1"}, persisting) - if err := reauthFn(context.Background()); err != nil { + if err := fn(context.Background()); err != nil { t.Fatalf("reauth: %v", err) } - if passedStoredToken == nil { - t.Fatal("expected stored token to be passed to Reauth") + if passed == nil || len(passed.Scopes) != 3 { + t.Fatalf("stored token scopes = %#v, want 3 preserved scopes", passed) } +} - if len(passedStoredToken.Scopes) != 3 { - t.Fatalf("expected 3 scopes from stored token, got %d: %v", len(passedStoredToken.Scopes), passedStoredToken.Scopes) +func TestReauthFunctionFromContextUnavailable(t *testing.T) { + if fn := reauthFunctionFromContext(context.Background(), "calendar", "user@example.com", []string{"scope"}, nil); fn != nil { + t.Fatal("expected nil without auth dependencies") } -} -// stubTokenSource returns a fixed token without any network calls. -type stubTokenSource struct { - token *oauth2.Token + ctx := WithAuthDependencies(context.Background(), AuthDependencies{}) + if fn := reauthFunctionFromContext(ctx, "calendar", "user@example.com", []string{"scope"}, nil); fn != nil { + t.Fatal("expected nil without reauth dependency") + } } -func (s *stubTokenSource) Token() (*oauth2.Token, error) { - return s.token, nil +func reauthTestTokenSource() *resettableOAuthTokenSource { + return newResettableOAuthTokenSource(func(token *oauth2.Token) oauth2.TokenSource { + refreshToken := token.RefreshToken + + return tokenSourceFunc(func() (*oauth2.Token, error) { + if refreshToken == "revoked-token" { + return nil, &oauth2.RetrieveError{ErrorCode: "invalid_grant"} + } + + if refreshToken == "" { + return nil, errReauthTestMissingRefreshToken + } + + return &oauth2.Token{AccessToken: "fresh-access-token"}, nil + }) + }, &oauth2.Token{RefreshToken: "revoked-token"}) } -// fakeStore is a minimal secrets.Store for testing. type fakeStore struct { token *secrets.Token } -func (s *fakeStore) Keys() ([]string, error) { return nil, nil } -func (s *fakeStore) GetToken(client, email string) (secrets.Token, error) { - if s.token != nil { - return *s.token, nil +func (s *fakeStore) Keys() ([]string, error) { return nil, nil } +func (s *fakeStore) GetToken(string, string) (secrets.Token, error) { + if s.token == nil { + return secrets.Token{}, errReauthTestTokenNotFound } - return secrets.Token{}, errors.New("not found") + + return *s.token, nil +} + +func (s *fakeStore) SetToken(_ string, _ string, token secrets.Token) error { + tokenCopy := token + s.token = &tokenCopy + + return nil } -func (s *fakeStore) SetToken(client, email string, tok secrets.Token) error { s.token = &tok; return nil } -func (s *fakeStore) DeleteToken(client, email string) error { return nil } -func (s *fakeStore) ListTokens() ([]secrets.Token, error) { return nil, nil } -func (s *fakeStore) GetDefaultAccount(string) (string, error) { return "", nil } -func (s *fakeStore) SetDefaultAccount(string, string) error { return nil } +func (s *fakeStore) DeleteToken(string, string) error { return nil } +func (s *fakeStore) ListTokens() ([]secrets.Token, error) { return nil, nil } +func (s *fakeStore) GetDefaultAccount(string) (string, error) { return "", nil } +func (s *fakeStore) SetDefaultAccount(string, string) error { return nil } diff --git a/internal/googleapi/transport.go b/internal/googleapi/transport.go index 5afd30612..481e4a39a 100644 --- a/internal/googleapi/transport.go +++ b/internal/googleapi/transport.go @@ -115,10 +115,12 @@ func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { if reauthErr := t.Reauth(req.Context()); reauthErr != nil { slog.Debug("auto-reauth failed", "err", reauthErr) - return nil, fmt.Errorf("refresh token expired or revoked: %w; re-authentication failed: %v; run 'gog auth add' to re-authorize manually", err, reauthErr) + + return nil, fmt.Errorf("refresh token expired or revoked: %w; re-authentication failed: %w; run 'gog auth add' to re-authorize manually", err, reauthErr) } retriedReauth = true + continue } diff --git a/internal/googleauth/reauth.go b/internal/googleauth/reauth.go index ff59bd694..4c37193a4 100644 --- a/internal/googleauth/reauth.go +++ b/internal/googleauth/reauth.go @@ -2,6 +2,7 @@ package googleauth import ( "context" + "errors" "fmt" "log/slog" "os" @@ -12,6 +13,16 @@ import ( "github.com/openclaw/gogcli/internal/secrets" ) +var ( + errReauthEmailRequired = errors.New("reauth: email is required") + errReauthClientRequired = errors.New("reauth: client is required") + errReauthScopesRequired = errors.New("reauth: scopes are required") + errReauthConfirmationRequired = errors.New("reauth: confirmation callback is required") + errReauthCancelled = errors.New("reauth: cancelled") + errReauthIdentityEmailMissing = errors.New("reauth: authorized identity did not include an email") + errReauthAuthorizedAsMismatch = errors.New("reauth: authorized account does not match expected account") +) + // ReauthOptions configures an automatic re-authorization flow triggered // when the stored refresh token is expired or revoked. type ReauthOptions struct { @@ -24,8 +35,6 @@ type ReauthOptions struct { // for the re-authorization instead of the triggering request's narrower // scopes, preventing silent grant narrowing. StoredToken *secrets.Token - // OpenSecretsStore opens the token store for persisting the new token. - OpenSecretsStore func() (secrets.Store, error) // EnsureKeychainAccess ensures the keychain is accessible for writes. // May be nil if keychain access is not needed (e.g. file backend). EnsureKeychainAccess func(context.Context) error @@ -35,6 +44,9 @@ type ReauthOptions struct { // FetchIdentityFunc fetches the authorized identity. If nil, // googleauth.IdentityForRefreshToken is used. FetchIdentityFunc func(context.Context, string, string, []string, time.Duration) (Identity, error) + // Confirm obtains explicit user consent before opening a browser. Auto- + // reauthorization fails closed when no confirmation callback is provided. + Confirm func(context.Context, string) (bool, error) // Timeout for the browser flow. If zero, defaults to 2 minutes. Timeout time.Duration // Stderr is where progress messages are written. If nil, os.Stderr is used. @@ -46,23 +58,26 @@ type ReauthOptions struct { // Reauth performs an automatic re-authorization when the stored refresh // token is expired or revoked (invalid_grant). It launches a browser-based // OAuth flow using the same client, services, and scopes as the original -// authorization, then persists the new refresh token to the secret store. +// authorization and returns replacement token metadata to the caller. The +// token-source owner is responsible for persistence and the in-memory swap. // -// It returns the new refresh token so the caller can update any in-memory -// token source that still holds the revoked token. +// It returns replacement token metadata so the caller can persist it and +// update any in-memory token source that still holds the revoked token. // // This is the auto-reauth counterpart to `gog auth add`, designed to be // called from the retry transport when an invalid_grant error is detected // during an API call. -func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { +func Reauth(ctx context.Context, opts ReauthOptions) (secrets.Token, error) { if opts.Email == "" { - return "", fmt.Errorf("reauth: email is required") + return secrets.Token{}, errReauthEmailRequired } + if opts.Client == "" { - return "", fmt.Errorf("reauth: client is required") + return secrets.Token{}, errReauthClientRequired } + if len(opts.Scopes) == 0 { - return "", fmt.Errorf("reauth: scopes are required") + return secrets.Token{}, errReauthScopesRequired } stderr := opts.Stderr @@ -85,9 +100,22 @@ func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { fetchIdentityFn = IdentityForRefreshToken } + if opts.Confirm == nil { + return secrets.Token{}, errReauthConfirmationRequired + } + + confirmed, err := opts.Confirm(ctx, opts.Email) + if err != nil { + return secrets.Token{}, fmt.Errorf("reauth: confirmation: %w", err) + } + + if !confirmed { + return secrets.Token{}, errReauthCancelled + } + if opts.EnsureKeychainAccess != nil { - if err := opts.EnsureKeychainAccess(ctx); err != nil { - return "", fmt.Errorf("reauth: keychain access: %w", err) + if keychainErr := opts.EnsureKeychainAccess(ctx); keychainErr != nil { + return secrets.Token{}, fmt.Errorf("reauth: keychain access: %w", keychainErr) } } @@ -95,11 +123,13 @@ func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { // token's full scope set to prevent silent grant narrowing (e.g. a // calendar command narrowing a gmail+calendar+drive grant). reauthScopes := opts.Scopes + reauthServices := opts.Services if opts.StoredToken != nil { if len(opts.StoredToken.Scopes) > 0 { reauthScopes = opts.StoredToken.Scopes } + if len(opts.StoredToken.Services) > 0 { reauthServices = opts.StoredToken.Services } @@ -108,12 +138,14 @@ func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { // Convert service strings to Service types. services := make([]Service, 0, len(reauthServices)) for _, s := range reauthServices { - svc, err := ParseService(s) - if err != nil { + svc, parseErr := ParseService(s) + if parseErr != nil { // If we can't parse the service label, use the scopes directly. - slog.Debug("reauth: could not parse service label, using scopes only", "service", s, "err", err) + slog.Debug("reauth: could not parse service label, using scopes only", "service", s, "err", parseErr) + continue } + services = append(services, svc) } @@ -122,7 +154,7 @@ func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { services = servicesFromScopes(reauthScopes) } - fmt.Fprintln(stderr, "Refresh token expired or revoked. Re-authorizing…") + fmt.Fprintln(stderr, "Re-authorizing…") authorizeOpts := AuthorizeOptions{ Services: services, @@ -134,55 +166,45 @@ func Reauth(ctx context.Context, opts ReauthOptions) (string, error) { refreshToken, err := authorizeFn(ctx, authorizeOpts) if err != nil { - return "", fmt.Errorf("reauth: authorization failed: %w", err) + return secrets.Token{}, fmt.Errorf("reauth: authorization failed: %w", err) } // Fetch the authorized identity to verify the email matches. identity, err := fetchIdentityFn(ctx, opts.Client, refreshToken, reauthScopes, 15*time.Second) if err != nil { - return "", fmt.Errorf("reauth: fetch authorized identity: %w", err) + return secrets.Token{}, fmt.Errorf("reauth: fetch authorized identity: %w", err) } - authorizedEmail := identity.Email + authorizedEmail := strings.TrimSpace(identity.Email) if authorizedEmail == "" { - authorizedEmail = opts.Email + return secrets.Token{}, errReauthIdentityEmailMissing } // Verify the authorized account matches the expected email. if !strings.EqualFold(strings.TrimSpace(authorizedEmail), strings.TrimSpace(opts.Email)) { - return "", fmt.Errorf("reauth: authorized as %s, expected %s", authorizedEmail, opts.Email) - } - - // Persist the new refresh token. - if opts.OpenSecretsStore == nil { - return "", fmt.Errorf("reauth: secret store opener is required") - } - - store, err := opts.OpenSecretsStore() - if err != nil { - return "", fmt.Errorf("reauth: open secret store: %w", err) + return secrets.Token{}, fmt.Errorf("%w: authorized as %s, expected %s", errReauthAuthorizedAsMismatch, authorizedEmail, opts.Email) } serviceNames := make([]string, 0, len(services)) for _, svc := range services { serviceNames = append(serviceNames, string(svc)) } + sort.Strings(serviceNames) - if err := store.SetToken(opts.Client, authorizedEmail, secrets.Token{ + updated := secrets.Token{ Client: opts.Client, Subject: identity.Subject, Email: authorizedEmail, Services: serviceNames, Scopes: reauthScopes, + CreatedAt: time.Now().UTC(), RefreshToken: refreshToken, - }); err != nil { - return "", fmt.Errorf("reauth: persist new refresh token: %w", err) } fmt.Fprintln(stderr, "Re-authorization successful. Retrying request…") - return refreshToken, nil + return updated, nil } // servicesFromScopes attempts to derive the service list from a set of @@ -195,6 +217,7 @@ func servicesFromScopes(scopes []string) []Service { } var matched []Service + for _, svc := range serviceOrder { info, ok := serviceInfoByService[svc] if !ok || !info.user { @@ -203,6 +226,7 @@ func servicesFromScopes(scopes []string) []Service { // Check if all scopes for this service are present in the scope set. allPresent := true + for _, svcScope := range info.scopes { if _, ok := scopeSet[svcScope]; !ok { allPresent = false diff --git a/internal/googleauth/reauth_test.go b/internal/googleauth/reauth_test.go index 260b53d34..6372e87cd 100644 --- a/internal/googleauth/reauth_test.go +++ b/internal/googleauth/reauth_test.go @@ -7,110 +7,52 @@ import ( "testing" "time" - "github.com/openclaw/gogcli/internal/config" "github.com/openclaw/gogcli/internal/secrets" ) -// mockSecretStore is a minimal secrets.Store for testing Reauth. -type mockSecretStore struct { - tokens map[string]secrets.Token -} - -func (s *mockSecretStore) GetToken(client string, email string) (secrets.Token, error) { - key := client + ":" + email - if tok, ok := s.tokens[key]; ok { - return tok, nil - } - return secrets.Token{}, errors.New("not found") -} - -func (s *mockSecretStore) SetToken(client string, email string, token secrets.Token) error { - key := client + ":" + email - s.tokens[key] = token - return nil -} - -func (s *mockSecretStore) DeleteToken(client string, email string) error { - key := client + ":" + email - delete(s.tokens, key) - return nil -} - - - -func (s *mockSecretStore) ListTokens() ([]secrets.Token, error) { - var out []secrets.Token - for _, tok := range s.tokens { - out = append(out, tok) - } - return out, nil -} - -func (s *mockSecretStore) Keys() ([]string, error) { - keys := make([]string, 0, len(s.tokens)) - for k := range s.tokens { - keys = append(keys, k) - } - return keys, nil -} - -func (s *mockSecretStore) GetDefaultAccount(string) (string, error) { - return "", errors.New("not implemented") -} - -func (s *mockSecretStore) SetDefaultAccount(string, string) error { - return errors.New("not implemented") -} - -func (s *mockSecretStore) Close() error { return nil } +var ( + errReauthTestUserDenied = errors.New("user denied access") + errReauthTestKeychainLocked = errors.New("keychain locked") +) func TestReauthSuccess(t *testing.T) { - origRead := readClientCredentials - origEndpoint := oauthEndpoint - t.Cleanup(func() { - readClientCredentials = origRead - oauthEndpoint = origEndpoint - }) - - readClientCredentials = func(string) (config.ClientCredentials, error) { - return config.ClientCredentials{ClientID: "id", ClientSecret: "secret"}, nil - } - - store := &mockSecretStore{tokens: make(map[string]secrets.Token)} - authorizeCalled := false identityCalled := false opts := ReauthOptions{ - Email: "user@example.com", - Client: "default", - Services: []string{"calendar"}, - Scopes: []string{"https://www.googleapis.com/auth/calendar"}, - OpenSecretsStore: func() (secrets.Store, error) { - return store, nil - }, + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, EnsureKeychainAccess: func(context.Context) error { return nil }, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { authorizeCalled = true + if authOpts.ForceConsent != true { t.Fatalf("expected ForceConsent=true") } + if authOpts.Client != "default" { t.Fatalf("expected client 'default', got %q", authOpts.Client) } + return "new-refresh-token", nil }, FetchIdentityFunc: func(ctx context.Context, client string, refreshToken string, scopes []string, timeout time.Duration) (Identity, error) { identityCalled = true + if refreshToken != "new-refresh-token" { t.Fatalf("expected new-refresh-token, got %q", refreshToken) } + return Identity{Subject: "sub123", Email: "user@example.com"}, nil }, Stderr: &bytesBuffer{}, } - if _, err := Reauth(context.Background(), opts); err != nil { + tok, err := Reauth(context.Background(), opts) + if err != nil { t.Fatalf("Reauth: %v", err) } @@ -122,12 +64,6 @@ func TestReauthSuccess(t *testing.T) { t.Fatal("FetchIdentityFunc was not called") } - // Verify token was persisted - tok, err := store.GetToken("default", "user@example.com") - if err != nil { - t.Fatalf("GetToken: %v", err) - } - if tok.RefreshToken != "new-refresh-token" { t.Fatalf("expected new-refresh-token, got %q", tok.RefreshToken) } @@ -143,16 +79,14 @@ func TestReauthSuccess(t *testing.T) { func TestReauthAuthorizeFailure(t *testing.T) { opts := ReauthOptions{ - Email: "user@example.com", - Client: "default", - Services: []string{"calendar"}, - Scopes: []string{"https://www.googleapis.com/auth/calendar"}, - OpenSecretsStore: func() (secrets.Store, error) { - return &mockSecretStore{tokens: make(map[string]secrets.Token)}, nil - }, + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, EnsureKeychainAccess: func(context.Context) error { return nil }, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { - return "", errors.New("user denied access") + return "", errReauthTestUserDenied }, FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { t.Fatal("FetchIdentityFunc should not be called on authorize failure") @@ -171,13 +105,36 @@ func TestReauthAuthorizeFailure(t *testing.T) { } } +func TestReauthRequiresConfirmation(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Scopes: []string{"scope"}, + } + if _, err := Reauth(context.Background(), opts); err == nil || !strings.Contains(err.Error(), "confirmation callback is required") { + t.Fatalf("Reauth() error = %v, want missing confirmation error", err) + } + + authorized := false + opts.Confirm = func(context.Context, string) (bool, error) { return false, nil } + + opts.AuthorizeFunc = func(context.Context, AuthorizeOptions) (string, error) { + authorized = true + return "token", nil + } + if _, err := Reauth(context.Background(), opts); err == nil || !strings.Contains(err.Error(), "cancelled") { + t.Fatalf("Reauth() error = %v, want cancellation", err) + } + + if authorized { + t.Fatal("authorization started after confirmation was declined") + } +} + func TestReauthMissingEmail(t *testing.T) { opts := ReauthOptions{ - Client: "default", - Scopes: []string{"scope"}, - OpenSecretsStore: func() (secrets.Store, error) { - return &mockSecretStore{}, nil - }, + Client: "default", + Scopes: []string{"scope"}, } _, err := Reauth(context.Background(), opts) @@ -190,9 +147,6 @@ func TestReauthMissingScopes(t *testing.T) { opts := ReauthOptions{ Email: "user@example.com", Client: "default", - OpenSecretsStore: func() (secrets.Store, error) { - return &mockSecretStore{}, nil - }, } _, err := Reauth(context.Background(), opts) @@ -203,14 +157,12 @@ func TestReauthMissingScopes(t *testing.T) { func TestReauthKeychainAccessFailure(t *testing.T) { opts := ReauthOptions{ - Email: "user@example.com", - Client: "default", - Scopes: []string{"scope"}, - OpenSecretsStore: func() (secrets.Store, error) { - return &mockSecretStore{}, nil - }, + Email: "user@example.com", + Client: "default", + Scopes: []string{"scope"}, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, EnsureKeychainAccess: func(context.Context) error { - return errors.New("keychain locked") + return errReauthTestKeychainLocked }, } @@ -226,14 +178,12 @@ func TestReauthKeychainAccessFailure(t *testing.T) { func TestReauthEmailMismatch(t *testing.T) { opts := ReauthOptions{ - Email: "user@example.com", - Client: "default", - Services: []string{"calendar"}, - Scopes: []string{"https://www.googleapis.com/auth/calendar"}, - OpenSecretsStore: func() (secrets.Store, error) { - return &mockSecretStore{tokens: make(map[string]secrets.Token)}, nil - }, + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, EnsureKeychainAccess: func(context.Context) error { return nil }, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { return "new-refresh-token", nil }, @@ -254,25 +204,35 @@ func TestReauthEmailMismatch(t *testing.T) { } } -func TestReauthPreservesStoredScopes(t *testing.T) { - store := &mockSecretStore{tokens: map[string]secrets.Token{ - "default:user@example.com": { - Scopes: []string{ - "https://www.googleapis.com/auth/calendar", - "https://www.googleapis.com/auth/gmail.modify", - "https://www.googleapis.com/auth/drive", - }, - Services: []string{"calendar", "gmail", "drive"}, +func TestReauthRejectsIdentityWithoutEmail(t *testing.T) { + opts := ReauthOptions{ + Email: "user@example.com", + Client: "default", + Services: []string{"calendar"}, + Scopes: []string{"https://www.googleapis.com/auth/calendar"}, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, + AuthorizeFunc: func(context.Context, AuthorizeOptions) (string, error) { + return "new-refresh-token", nil + }, + FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { + return Identity{Subject: "sub123"}, nil }, - }} + Stderr: &bytesBuffer{}, + } + + if _, err := Reauth(context.Background(), opts); err == nil || !strings.Contains(err.Error(), "did not include an email") { + t.Fatalf("Reauth() error = %v, want missing email error", err) + } +} +func TestReauthPreservesStoredScopes(t *testing.T) { var requestedScopes []string var requestedServices []string opts := ReauthOptions{ Email: "user@example.com", Client: "default", - Services: []string{"calendar"}, // narrowed — only the triggering request's service + Services: []string{"calendar"}, // narrowed — only the triggering request's service Scopes: []string{"https://www.googleapis.com/auth/calendar"}, // narrowed StoredToken: &secrets.Token{ Scopes: []string{ @@ -282,16 +242,16 @@ func TestReauthPreservesStoredScopes(t *testing.T) { }, Services: []string{"calendar", "gmail", "drive"}, }, - OpenSecretsStore: func() (secrets.Store, error) { - return store, nil - }, EnsureKeychainAccess: func(context.Context) error { return nil }, + Confirm: func(context.Context, string) (bool, error) { return true, nil }, AuthorizeFunc: func(ctx context.Context, authOpts AuthorizeOptions) (string, error) { requestedScopes = authOpts.Scopes requestedServices = make([]string, len(authOpts.Services)) + for i, svc := range authOpts.Services { requestedServices[i] = string(svc) } + return "new-refresh-token", nil }, FetchIdentityFunc: func(context.Context, string, string, []string, time.Duration) (Identity, error) { @@ -300,7 +260,8 @@ func TestReauthPreservesStoredScopes(t *testing.T) { Stderr: &bytesBuffer{}, } - if _, err := Reauth(context.Background(), opts); err != nil { + tok, err := Reauth(context.Background(), opts) + if err != nil { t.Fatalf("Reauth: %v", err) } @@ -314,13 +275,8 @@ func TestReauthPreservesStoredScopes(t *testing.T) { t.Fatalf("expected 3 services (preserved from stored token), got %d: %v", len(requestedServices), requestedServices) } - // Verify persisted token has the full scope set - tok, err := store.GetToken("default", "user@example.com") - if err != nil { - t.Fatalf("GetToken: %v", err) - } if len(tok.Scopes) != 3 { - t.Fatalf("persisted token should have 3 scopes, got %d: %v", len(tok.Scopes), tok.Scopes) + t.Fatalf("returned token should have 3 scopes, got %d: %v", len(tok.Scopes), tok.Scopes) } } From 2db5fb8e29d928ec3282e26fbf84cace8ac37af7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 19:00:21 -0700 Subject: [PATCH 3/4] chore(deps): refresh dependency pins --- go.mod | 2 +- go.sum | 4 ++-- internal/tracking/worker/package.json | 4 ++-- internal/tracking/worker/pnpm-lock.yaml | 16 ++++++++-------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 628356fa8..c5d314362 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( cloud.google.com/go/pubsub/v2 v2.6.1 filippo.io/age v1.3.1 github.com/99designs/keyring v1.2.2 - github.com/alecthomas/kong v1.16.0 + github.com/alecthomas/kong v1.16.1 github.com/mark3labs/mcp-go v0.57.0 github.com/muesli/termenv v0.16.0 github.com/stretchr/testify v1.11.1 diff --git a/go.sum b/go.sum index 73ea6126f..cdef0e460 100644 --- a/go.sum +++ b/go.sum @@ -24,8 +24,8 @@ github.com/99designs/keyring v1.2.2/go.mod h1:wes/FrByc8j7lFOAGLGSNEg8f/PaI3cgTB github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= -github.com/alecthomas/kong v1.16.0 h1:g92/kUxBcdcTPOM79yE63viJgtcp5dNyrB3/O2cjYT4= -github.com/alecthomas/kong v1.16.0/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I= +github.com/alecthomas/kong v1.16.1 h1:ixhCt93XkJ98kGposQ54+bl0IK6XwqB40AsMynU7Z8E= +github.com/alecthomas/kong v1.16.1/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I= github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs= github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= diff --git a/internal/tracking/worker/package.json b/internal/tracking/worker/package.json index 4f9fcb1bf..b83f53a94 100644 --- a/internal/tracking/worker/package.json +++ b/internal/tracking/worker/package.json @@ -13,7 +13,7 @@ "test": "vitest run" }, "devDependencies": { - "@cloudflare/workers-types": "^5.20260808.1", + "@cloudflare/workers-types": "^5.20260809.1", "@typescript/native": "npm:typescript@^7.0.2", "oxfmt": "^0.62.0", "oxlint": "^1.77.0", @@ -22,5 +22,5 @@ "vitest": "^4.1.10", "wrangler": "^4.120.0" }, - "packageManager": "pnpm@11.20.0" + "packageManager": "pnpm@11.21.0" } diff --git a/internal/tracking/worker/pnpm-lock.yaml b/internal/tracking/worker/pnpm-lock.yaml index d3fdc0be0..be780e900 100644 --- a/internal/tracking/worker/pnpm-lock.yaml +++ b/internal/tracking/worker/pnpm-lock.yaml @@ -16,8 +16,8 @@ importers: .: devDependencies: '@cloudflare/workers-types': - specifier: ^5.20260808.1 - version: 5.20260808.1 + specifier: ^5.20260809.1 + version: 5.20260809.1 '@typescript/native': specifier: npm:typescript@^7.0.2 version: typescript@7.0.2 @@ -38,7 +38,7 @@ importers: version: 4.1.10(vite@8.2.0(esbuild@0.28.1)) wrangler: specifier: ^4.120.0 - version: 4.120.0(@cloudflare/workers-types@5.20260808.1) + version: 4.120.0(@cloudflare/workers-types@5.20260809.1) packages: @@ -85,8 +85,8 @@ packages: cpu: [x64] os: [win32] - '@cloudflare/workers-types@5.20260808.1': - resolution: {integrity: sha512-DN7G9SMyeOq031YhQexoExFAK78ms74cFiFF1teDlTK4+LjHgIc5Z8VbvXQtcCIP//o38btxzxW0b9MGPA83CA==} + '@cloudflare/workers-types@5.20260809.1': + resolution: {integrity: sha512-sBM+0I5lCY9LgTnorn/N2UyrA6KVbUzj9tncxwH8v6sH8tVeAyJj+i0z3xXWY4l4fd1oDcwt8CGf50vGwVISYQ==} '@cspotcode/source-map-support@0.8.1': resolution: {integrity: sha512-IchNf6dN4tHoMFIn/7OE8LWZ19Y6q/67Bmf6vnGREv8RSbBVb9LPJxEcnwrcwX6ixSvaiGoomAUvu4YSxXrVgw==} @@ -1409,7 +1409,7 @@ snapshots: '@cloudflare/workerd-windows-64@1.20260801.1': optional: true - '@cloudflare/workers-types@5.20260808.1': {} + '@cloudflare/workers-types@5.20260809.1': {} '@cspotcode/source-map-support@0.8.1': dependencies: @@ -2318,7 +2318,7 @@ snapshots: '@cloudflare/workerd-linux-arm64': 1.20260801.1 '@cloudflare/workerd-windows-64': 1.20260801.1 - wrangler@4.120.0(@cloudflare/workers-types@5.20260808.1): + wrangler@4.120.0(@cloudflare/workers-types@5.20260809.1): dependencies: '@cloudflare/kv-asset-handler': 0.5.0 '@cloudflare/unenv-preset': 2.16.1(unenv@2.0.0-rc.24)(workerd@1.20260801.1) @@ -2329,7 +2329,7 @@ snapshots: unenv: 2.0.0-rc.24 workerd: 1.20260801.1 optionalDependencies: - '@cloudflare/workers-types': 5.20260808.1 + '@cloudflare/workers-types': 5.20260809.1 fsevents: 2.3.3 transitivePeerDependencies: - bufferutil From 3a23e66aae2cccfdf122cfdbafcbb50f6aeb78a4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 19:04:11 -0700 Subject: [PATCH 4/4] ci: align worker pnpm version --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64854a0c5..2bf4914be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,7 @@ jobs: - name: Enable Corepack (pnpm) run: | corepack enable - corepack prepare pnpm@11.20.0 --activate + corepack prepare pnpm@11.21.0 --activate - name: Install dependencies run: pnpm -C internal/tracking/worker install --frozen-lockfile - name: Lint