fix(web): dedicated Google re-auth page + PR #39 review follow-ups - #40
Open
ajinkyasraj wants to merge 1 commit into
Open
fix(web): dedicated Google re-auth page + PR #39 review follow-ups#40ajinkyasraj wants to merge 1 commit into
ajinkyasraj wants to merge 1 commit into
Conversation
The inline "Re-authenticate" control on a Google connection was a <details>
disclosure inside a cramped, right-aligned table cell: clicking it only expanded
a form instead of doing anything, so it read as broken, and its placement/purpose
were unclear. Replace it with a proper page (mirrors the Edit page).
- New GET /connections/{id}/reauth → connection_reauth.html: a clear single
primary "Re-authenticate with Google" button (uses the connection's stored
client), plus an optional "use a different OAuth client" disclosure for
same-account client migration. The table now shows a plain Re-authenticate
link (reauth_required = amber, active = subtle) that navigates to it — no more
form-in-<details>. reauth_required connections reach the same page (Copilot).
Review follow-ups from PR #39:
- Both-or-neither validation on add + reauth: a google_client_id without a
client_secret builds a non-refreshing config (gmail needs both), so the token
would die at expiry. Reject the half-specified case with 400.
- Reauth uses writeConnectionError for keyring errors (covers ErrKeyringRotating
with Retry-After), instead of the hand-rolled ErrKeyringNotLoaded-only block.
- Correct the reauth-migration wording (UI + oauth-pkce.md): re-auth requires
the same account, so migration is to a different client for the SAME account
(e.g. a new GCP project in the same org), NOT cross-org. Cross-org = delete +
re-add.
- Tests: reauth page renders; default reauth uses the stored client; half-
specified client rejected on both add and reauth.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #39 (now merged): fixes the Google Re-authenticate UX and addresses the Copilot review comments.
The re-auth UX problem
On a Google connection, "Re-authenticate" was a
<details>disclosure inside a cramped, right-alignedwhitespace-nowraptable cell. Clicking it only expanded a form rather than doing anything — so it read as broken, and its placement/purpose were unclear.Fix: a proper re-auth page (mirrors the Edit page):
GET /connections/{id}/reauth→connection_reauth.html: one clear primary "Re-authenticate with Google" button (uses the connection's stored client), plus an optional "use a different OAuth client" disclosure for same-account client migration. Shows the bound account so the operator knows what they're re-authing.reauth_required, subtle foractive) that navigates to the page — no more form-in-<details>, no CSRF/JS-injection edge.reauth_requiredconnections now reach the same page too.Review follow-ups (Copilot on #39)
google_client_idwith noclient_secretbuilds a config the gmail connector can't refresh (it needs both), so the token would silently die at expiry. Now a400.writeConnectionErroron reauth instead of the hand-rolledErrKeyringNotLoaded-only block — soErrKeyringRotatingis handled (503 +Retry-After), no drift.oauth-pkce.md): re-auth requires signing in as the same account, so migration is to a different client for the same account (e.g. a new GCP project in the same org) — not cross-org. A different org's Internal client won't admit the account (org_internal); cross-org = delete + re-add.Tests
go build/vet,gofmt -lclean;go test ./internal/web/green, including new cases: reauth page renders (shows bound account); default reauth redirects to Google using the stored client; half-specified client rejected (400) on both add and reauth.🤖 Generated with Claude Code