summaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-09-20 17:44:02 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-09-20 17:44:02 +0000
commit6431cad59bda3da4bafb58fdf91f6a946e42af43 (patch)
tree1c5b6dc9ebbe3e9b34119b3c80812ba28770bb1c /internal
parent464f9e09c6cde9a2fe66a18500736206ae233d0a (diff)
Surface a dead OAuth grant instead of reporting it as connected
With the cancelled-context bug fixed, the refresh actually reaches Google and the real state of the Google Tasks grant becomes visible: it answers invalid_grant. The stored refresh token has been dead since roughly 2026-08-16. The earlier bug had been masking this — the refresh never got far enough to be rejected. Settings treated the presence of a token row as "Connected", so it reported Google Tasks as connected for a month while every fetch failed. A refresh failure is now persisted (migration 035 adds last_error/last_error_at to oauth_tokens) and cleared on the next success, and settings reports "connected" only when there is no recorded failure. invalid_grant is classified specially, because it is the one failure retrying cannot fix: the message tells the user to reconnect, and notes that a weekly recurrence means the OAuth client is still in Testing publishing status, where Google expires refresh tokens after seven days. Other errors — a network blip, a 5xx — are left as transient rather than pushing the user through consent needlessly. Reconnecting at /settings is still a manual step; nothing here can perform consent on the user's behalf. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'internal')
-rw-r--r--internal/api/google_tasks_oauth.go25
-rw-r--r--internal/api/google_tasks_oauth_test.go91
-rw-r--r--internal/handlers/settings.go21
-rw-r--r--internal/store/oauth_tokens.go48
-rw-r--r--internal/store/oauth_tokens_test.go4
5 files changed, 182 insertions, 7 deletions
diff --git a/internal/api/google_tasks_oauth.go b/internal/api/google_tasks_oauth.go
index 6edba21..c6f816c 100644
--- a/internal/api/google_tasks_oauth.go
+++ b/internal/api/google_tasks_oauth.go
@@ -3,6 +3,7 @@ package api
import (
"context"
"fmt"
+ "strings"
"time"
"golang.org/x/oauth2"
@@ -18,6 +19,21 @@ import (
type TokenStore interface {
GetOAuthToken(source string) (*oauth2.Token, error)
SaveOAuthToken(source string, tok *oauth2.Token) error
+ // RecordOAuthTokenError persists why a refresh failed, so the settings page
+ // can distinguish "connected" from "has a row but the grant is dead".
+ RecordOAuthTokenError(source string, message string) error
+}
+
+// NeedsReconnect reports whether an error means the stored grant is finished
+// and only a fresh consent will fix it.
+//
+// Google answers invalid_grant when a refresh token has been revoked,
+// superseded, or has simply aged out — which for an OAuth client still in
+// "Testing" publishing status happens after seven days. Retrying cannot help;
+// the user has to reconnect. Any other error (a network blip, a 5xx) is worth
+// retrying and must not push the user through consent needlessly.
+func NeedsReconnect(err error) bool {
+ return err != nil && strings.Contains(err.Error(), "invalid_grant")
}
// GoogleTasksOAuthSource is the key GoogleTasksClient's token is stored
@@ -77,6 +93,15 @@ func (d *dbTokenSource) Token() (*oauth2.Token, error) {
refreshed, err := d.config.TokenSource(ctx, current).Token()
if err != nil {
+ // Record it: a failed refresh is invisible otherwise, and an
+ // invalid_grant in particular means no amount of waiting will fix it.
+ msg := err.Error()
+ if NeedsReconnect(err) {
+ msg = "Google rejected the saved authorisation (invalid_grant) — reconnect to continue. " +
+ "If this recurs weekly, the OAuth client is still in Testing publishing status, " +
+ "which expires refresh tokens after 7 days."
+ }
+ _ = d.store.RecordOAuthTokenError(d.source, msg)
return nil, fmt.Errorf("refreshing token: %w", err)
}
diff --git a/internal/api/google_tasks_oauth_test.go b/internal/api/google_tasks_oauth_test.go
index 93acd76..222baab 100644
--- a/internal/api/google_tasks_oauth_test.go
+++ b/internal/api/google_tasks_oauth_test.go
@@ -15,9 +15,10 @@ import (
// memTokenStore is an in-memory TokenStore.
type memTokenStore struct {
- mu sync.Mutex
- token *oauth2.Token
- saved int
+ mu sync.Mutex
+ token *oauth2.Token
+ saved int
+ lastError string
}
func (m *memTokenStore) GetOAuthToken(string) (*oauth2.Token, error) {
@@ -30,6 +31,13 @@ func (m *memTokenStore) GetOAuthToken(string) (*oauth2.Token, error) {
return &cp, nil
}
+func (m *memTokenStore) RecordOAuthTokenError(_ string, message string) error {
+ m.mu.Lock()
+ defer m.mu.Unlock()
+ m.lastError = message
+ return nil
+}
+
func (m *memTokenStore) SaveOAuthToken(_ string, tok *oauth2.Token) error {
m.mu.Lock()
defer m.mu.Unlock()
@@ -153,3 +161,80 @@ func TestDBTokenSource_NotConnected(t *testing.T) {
t.Errorf("error = %q, want it to mention not being connected", err)
}
}
+
+// invalid_grant means the saved authorisation is finished — retrying cannot
+// help, and the user has to reconnect. Anything else is worth retrying, so
+// misclassifying it would push the user through consent for a network blip.
+func TestNeedsReconnect(t *testing.T) {
+ cases := []struct {
+ err error
+ want bool
+ }{
+ {errors.New(`oauth2: "invalid_grant" "Bad Request"`), true},
+ {errors.New("refreshing token: oauth2: \"invalid_grant\" \"Token has been expired or revoked.\""), true},
+ {errors.New("Post \"https://oauth2.googleapis.com/token\": dial tcp: i/o timeout"), false},
+ {errors.New("oauth2: cannot fetch token: 503 Service Unavailable"), false},
+ {nil, false},
+ }
+ for _, c := range cases {
+ if got := NeedsReconnect(c.err); got != c.want {
+ t.Errorf("NeedsReconnect(%v) = %v, want %v", c.err, got, c.want)
+ }
+ }
+}
+
+// A failed refresh must be recorded, or settings keeps showing "connected"
+// while every background fetch fails — which is how Google Tasks appeared
+// healthy for a month while returning nothing.
+func TestDBTokenSource_RecordsRefreshFailure(t *testing.T) {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ w.WriteHeader(http.StatusBadRequest)
+ _, _ = w.Write([]byte(`{"error":"invalid_grant","error_description":"Token has been expired or revoked."}`))
+ }))
+ defer srv.Close()
+
+ store := &memTokenStore{token: expiredToken()}
+ src := &dbTokenSource{
+ store: store,
+ source: GoogleTasksOAuthSource,
+ config: &oauth2.Config{
+ ClientID: "id",
+ ClientSecret: "secret",
+ Endpoint: oauth2.Endpoint{TokenURL: srv.URL},
+ },
+ }
+
+ if _, err := src.Token(); err == nil {
+ t.Fatal("expected the refresh to fail")
+ }
+ if store.lastError == "" {
+ t.Fatal("the failure was not recorded")
+ }
+ if !strings.Contains(store.lastError, "reconnect") {
+ t.Errorf("recorded error should tell the user to reconnect, got %q", store.lastError)
+ }
+}
+
+// A successful refresh must clear a previously recorded failure; the store's
+// SaveOAuthToken does that, so this pins that saving is what happens on success.
+func TestDBTokenSource_SuccessSavesToken(t *testing.T) {
+ hits := 0
+ srv := tokenServer(t, &hits)
+ store := &memTokenStore{token: expiredToken()}
+ src := &dbTokenSource{
+ store: store,
+ source: GoogleTasksOAuthSource,
+ config: &oauth2.Config{
+ ClientID: "id",
+ ClientSecret: "secret",
+ Endpoint: oauth2.Endpoint{TokenURL: srv.URL},
+ },
+ }
+ if _, err := src.Token(); err != nil {
+ t.Fatalf("Token(): %v", err)
+ }
+ if store.saved == 0 {
+ t.Error("a refreshed token should be persisted")
+ }
+}
diff --git a/internal/handlers/settings.go b/internal/handlers/settings.go
index d6d78c0..fd63518 100644
--- a/internal/handlers/settings.go
+++ b/internal/handlers/settings.go
@@ -23,6 +23,10 @@ func (h *Handler) HandleSettingsPage(w http.ResponseWriter, r *http.Request) {
}
googleTasksToken, _ := h.store.GetOAuthToken(api.GoogleTasksOAuthSource)
+ // A stored row is not the same as a working grant: an invalid_grant refresh
+ // failure leaves the row in place while every fetch fails. Treat a recorded
+ // error as "needs reconnecting" rather than showing a reassuring "Connected".
+ googleTasksStatus, _ := h.store.GetOAuthTokenStatus(api.GoogleTasksOAuthSource)
icsFeeds, _ := h.store.GetICSFeeds()
data := struct {
@@ -44,9 +48,12 @@ func (h *Handler) HandleSettingsPage(w http.ResponseWriter, r *http.Request) {
CSRFToken: auth.GetCSRFTokenFromContext(r.Context()),
WebAuthnEnabled: h.WebAuthnEnabled,
GoogleTasksOAuthReady: h.googleTasksOAuthConfig != nil,
- GoogleTasksConnected: googleTasksToken != nil,
+ GoogleTasksConnected: googleTasksToken != nil && googleTasksStatus.LastError == "",
Feeds: toICSFeedViews(icsFeeds),
- GoogleTasksError: r.URL.Query().Get("google_tasks_error"),
+ GoogleTasksError: firstNonEmpty(
+ r.URL.Query().Get("google_tasks_error"),
+ googleTasksStatus.LastError,
+ ),
}
if err := h.renderer.Render(w, "settings.html", data); err != nil {
@@ -160,3 +167,13 @@ func (h *Handler) HandleToggleSourceConfig(w http.ResponseWriter, r *http.Reques
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]bool{"enabled": enabled})
}
+
+// firstNonEmpty returns the first non-empty string, or "".
+func firstNonEmpty(values ...string) string {
+ for _, v := range values {
+ if v != "" {
+ return v
+ }
+ }
+ return ""
+}
diff --git a/internal/store/oauth_tokens.go b/internal/store/oauth_tokens.go
index 12df515..d216269 100644
--- a/internal/store/oauth_tokens.go
+++ b/internal/store/oauth_tokens.go
@@ -50,11 +50,57 @@ func (s *Store) SaveOAuthToken(source string, tok *oauth2.Token) error {
refresh_token = excluded.refresh_token,
token_type = excluded.token_type,
expiry = excluded.expiry,
- updated_at = CURRENT_TIMESTAMP
+ updated_at = CURRENT_TIMESTAMP,
+ -- A successful save means the grant works again; drop any recorded
+ -- failure so settings stops telling the user to reconnect.
+ last_error = NULL,
+ last_error_at = NULL
`, source, tok.AccessToken, tok.RefreshToken, tok.TokenType, expiry)
return err
}
+// RecordOAuthTokenError stores why a token stopped working.
+//
+// A stored row used to mean "connected" on its own, so a dead refresh token
+// showed as connected while every background fetch failed. Persisting the
+// reason is what lets settings say "reconnect" instead.
+func (s *Store) RecordOAuthTokenError(source string, message string) error {
+ _, err := s.db.Exec(`
+ UPDATE oauth_tokens
+ SET last_error = ?, last_error_at = CURRENT_TIMESTAMP
+ WHERE source = ?
+ `, message, source)
+ return err
+}
+
+// OAuthTokenStatus is a stored grant plus whether it is currently working.
+type OAuthTokenStatus struct {
+ Connected bool
+ LastError string
+ LastErrorAt *time.Time
+}
+
+// GetOAuthTokenStatus reports whether a source is usable, not merely present.
+func (s *Store) GetOAuthTokenStatus(source string) (OAuthTokenStatus, error) {
+ var lastError sql.NullString
+ var lastErrorAt sql.NullTime
+ err := s.db.QueryRow(
+ `SELECT COALESCE(last_error, ''), last_error_at FROM oauth_tokens WHERE source = ?`,
+ source).Scan(&lastError, &lastErrorAt)
+ if err == sql.ErrNoRows {
+ return OAuthTokenStatus{}, nil
+ }
+ if err != nil {
+ return OAuthTokenStatus{}, err
+ }
+ st := OAuthTokenStatus{Connected: true, LastError: lastError.String}
+ if lastErrorAt.Valid {
+ t := lastErrorAt.Time
+ st.LastErrorAt = &t
+ }
+ return st, nil
+}
+
// DeleteOAuthToken removes a source's stored token (disconnect).
func (s *Store) DeleteOAuthToken(source string) error {
_, err := s.db.Exec(`DELETE FROM oauth_tokens WHERE source = ?`, source)
diff --git a/internal/store/oauth_tokens_test.go b/internal/store/oauth_tokens_test.go
index 20c4a79..435a757 100644
--- a/internal/store/oauth_tokens_test.go
+++ b/internal/store/oauth_tokens_test.go
@@ -25,7 +25,9 @@ func newOAuthTokensTestStore(t *testing.T) *Store {
refresh_token TEXT NOT NULL,
token_type TEXT NOT NULL DEFAULT 'Bearer',
expiry DATETIME,
- updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
+ updated_at DATETIME DEFAULT CURRENT_TIMESTAMP,
+ last_error TEXT,
+ last_error_at DATETIME
)
`); err != nil {
t.Fatal(err)