Skip to content

Offer re-consent when an extension's refresh token is rejected (invalid_grant) - #455

Closed
CaseyLabs wants to merge 3 commits into
hkdb:mainfrom
clabs-public-forks:fix/reauth-on-invalid-grant
Closed

CaseyLabs wants to merge 3 commits into
hkdb:mainfrom
clabs-public-forks:fix/reauth-on-invalid-grant

Conversation

@CaseyLabs

@CaseyLabs CaseyLabs commented Oct 6, 2026 •

Copy link
Copy Markdown

Problem

If the refresh token for an extension OAuth slot (e.g. google-calendar) is rejected by the provider with invalid_grant, there is no way to recover from the UI.

This happens routinely with self-created Google OAuth clients (see #336): while the app is in Testing, Google expires refresh tokens after 7 days. Mail keeps working once re-authorized, but the calendar slot's token dies separately.

Repro:

  1. Have a google-calendar token whose refresh token is expired/revoked.
  2. Remove the Google calendar source (or have none).
  3. Calendar → Add Google Calendar → pick the account.

Result: the dialog shows ... auth broker: refresh: token refresh failed: invalid_grant - Bad Request and no Grant Calendar Access button. That button (in AddGoogleCalendarDialog, AddMicrosoftCalendarDialog and both account hook panels) only appears when the error contains additional consent required, which the broker only raises when the token row is missing or lacks scopes, not when it exists but is dead. The per-source Reauthorize button in Calendar Settings doesn't help when there is no source. The only workaround is deleting the oauth_tokens row by hand.

Fix

  • internal/oauth2: RefreshTokenWithProvider wraps invalid_grant responses in a new ErrInvalidGrant sentinel. The error message text is unchanged.
  • internal/extensions/auth: when a refresh fails with ErrInvalidGrant, bearerRefreshTransport returns *coreapi.ErrAdditionalConsentRequired for that slot, with the scopes the client was issued for. The existing UI then offers the grant flow, and StartIncrementalConsent overwrites the dead token. The error carries a new optional Reason (refresh token expired or revoked), so its message no longer claims scopes are missing. It still starts with additional consent required, which is the text the calendar dialogs match.
  • This only applies to slots an extension grant flow can re-grant: the *-calendar and *-contacts slots with credentials configured. A new oauth2.IncrementalConsentProvider makes that decision, and StartIncrementalConsent now uses the same function, so the two can't drift apart. Mail slots (custom-mail for bring-your-own-app accounts, and google-mail / microsoft-mail that Contacts uses for its read scopes) keep the original refresh error. Only mail re-authorization can replace those tokens, so a Grant button there would just fail again.

Other refresh failures (network errors, invalid_client, etc.) keep the existing error.

Tests

  • TestRefreshTokenWithProvider_InvalidGrant / _OtherErrorIsNotInvalidGrant: sentinel matching against an httptest token endpoint.
  • TestTransport_InvalidGrantOnMailSlotKeepsRefreshError: end to end through HTTPClientForExtension for a custom account, with stub API (401) and token (invalid_grant) servers. The custom-mail slot must return the ErrInvalidGrant refresh error, not a consent error.
  • TestErrAdditionalConsentRequired_Error: the message with and without Reason.
  • TestCanRegrant: which slots get the consent error. An extension slot with credentials does; an extension slot without credentials and the mail slots don't.

The calendar and contacts slots refresh against Google's and Microsoft's real token URLs, so there's no end-to-end test for the consent conversion itself; TestCanRegrant covers which slots qualify.

go test ./internal/... ./extensions/... passes, except TestSerializeVEVENT_AllDay. That test also fails on main when run outside UTC; it's unrelated to this change.

Notes

  • Broker now builds every extension HTTP client through one newClient helper instead of three copies of the same setup. The helper copies the scopes list, so the client doesn't share the caller's slice.
  • For a source that already exists, background sync now records additional consent required for account … under google-calendar: refresh token expired or revoked as its last_error instead of the raw invalid_grant text. The Reauthorize button was already shown for those sources.
  • Nothing starts a consent flow automatically. Only the user-initiated dialogs react to this error.

When a provider rejects an extension slot's refresh token with
invalid_grant (expired, e.g. Google's 7-day limit for OAuth apps in
Testing, or revoked), the broker transport returned a plain
"auth broker: refresh: token refresh failed: invalid_grant" error. The
Calendar add-source dialogs and account hook panels only show the
Grant Calendar Access button for "additional consent required", so a
user whose calendar token had died (and who had no existing source to
Reauthorize) had no way to recover in the UI.

- oauth2: wrap invalid_grant refresh failures in ErrInvalidGrant
  (message text unchanged).
- auth broker: map ErrInvalidGrant to ErrAdditionalConsentRequired,
  carrying the scopes the client was issued for, but only for slots an
  extension grant flow can re-grant (the *-calendar and *-contacts slots
  StartIncrementalConsent resolves via oauth2.GetProvider). Mail slots
  (custom-mail, and google-/microsoft-mail used for core-routed scopes)
  keep the original refresh error: only mail re-authorization can
  replace those tokens, and a Grant button there would loop.
@CaseyLabs
CaseyLabs force-pushed the fix/reauth-on-invalid-grant branch from b856eef to b147877 Compare October 6, 2026 16:13
…setup

- Add oauth2.IncrementalConsentProvider as the single source of truth for
  whether a slot can be re-granted; StartIncrementalConsent and the auth
  transport both use it.
- Factor the three bearerRefreshTransport constructions into Broker.newClient.
- Trim redundant test assertions and narrow the slot table test.
- Add an optional Reason to ErrAdditionalConsentRequired; the transport sets
  it on invalid_grant so the message no longer claims scopes are missing.
- Clone the scopes slice when building a broker client so the transport
  doesn't alias the caller's slice.
@CaseyLabs
CaseyLabs marked this pull request as ready for review October 6, 2026 17:04
@CaseyLabs CaseyLabs closed this Oct 9, 2026
@CaseyLabs
CaseyLabs deleted the fix/reauth-on-invalid-grant branch October 9, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant