docs(plans): record the Google sign-in plan where the other plans live
The plan for this feature existed as a published page and nowhere in the repository, which made it the only sizeable piece of work here without a record beside its siblings in docs/superpowers/plans. Written to that folder's conventions — dated filename, goal and architecture up front, global constraints, the file structure, then the phases as tasks. The boxes are checked rather than open, because all six phases merged before this was written and an implementation plan full of unticked work that is already done would read as a to-do list nobody had started. It is a record rather than a reconstruction. The file list is taken from the commits themselves rather than from memory, and a check confirms every path it names exists. The corrections section is the part worth keeping. Two things the plan asserted turned out to be false, and both are written down rather than quietly fixed: that QA could never run this feature, which cost a deploy and put the wrong constraint into a compose file and two issues; and that local development should register the port 3000 callback, when the dev server it actually browses is on 5173. A plan that silently stops saying something teaches nobody why it said it. Also records the two smaller corrections made during the work — that account deletion never asked for a password, and that the button was initially missing from the sign-up tab — and the three follow-ups that were deliberately not built. Verified: every file path named in the document exists in the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0361ef35d0
commit
1838bb38d1
@@ -0,0 +1,155 @@
|
||||
# Google Sign-In Implementation Plan
|
||||
|
||||
> **Status: complete.** All six phases merged between 2026-09-10 and 2026-09-11. Boxes are checked as a record of what landed, not as work outstanding. Two claims in the original plan turned out to be false and are marked inline rather than deleted — see [Corrections](#corrections).
|
||||
|
||||
**Goal:** A customer can sign in with Google and arrive at exactly the session a password login produces; a returning customer reaches the same account rather than a second one; and an account with no password can still manage itself.
|
||||
|
||||
**Architecture:** Server-side OpenID Connect authorization code flow with PKCE, mounted at `/api/auth/google`. The browser never holds a token. A `customer_identities` table keyed on the provider's subject claim links a Google account to a customer, and `customers.password_hash` becomes nullable so a Google-only account can exist. Everything after the identity is established — account creation, linking, the disabled-account refusal, session creation — is shared with the paths that already existed.
|
||||
|
||||
**Tech Stack:** Express + TypeScript, Postgres, `node-pg-migrate`, React + antd, Jest (unit and integration), Playwright (e2e).
|
||||
|
||||
**Parent issue:** #332. **Phases:** #340 through #345.
|
||||
|
||||
**Ops reference:** `docs/ops/google-sign-in.md` — the console setup, the per-environment redirect URIs, and the cutover checklist.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- **One session implementation.** A social sign-in must end in the same `rd_session` cookie with the same flags, expiry and logout behaviour. It calls `signIn` from `customerSession.ts`, which password and passkey login already share. A second path that agrees today is a path that gets changed alone.
|
||||
- **Keyed on the subject claim, never the email.** An email is a display value its owner can change and a provider may reassign. Matching on it strands a customer who changes theirs and hands their account to whoever inherits the old address.
|
||||
- **`email_verified` is compared to the boolean, never tested for truthiness.** The string `"false"` is truthy, and the linking policy turns entirely on this flag.
|
||||
- **The redirect URI derives from `PUBLIC_URL`.** Google compares it as an exact string. One source, and it is the one already correct wherever email links work.
|
||||
- **Absent, not disabled, where unconfigured.** Being unconfigured is the normal state for local development, so the button must not appear at all rather than appear and fail.
|
||||
- **Disabled accounts are refused here too.** Enforcing it on one sign-in path and not another is how a disabled account keeps a way in.
|
||||
- **Consent wording is stored verbatim and must stay byte-identical** to what the customer saw (#56). A Google sign-up captures consent through the endpoints registration already uses.
|
||||
- **Commit style:** Conventional Commits, subject ending `(#34N)`, no hard wrapping in bodies.
|
||||
|
||||
## File Structure
|
||||
|
||||
**Created:**
|
||||
- `backend/migrations/1788200000000_add-social-identities.js` — the identities table, and the nullable password hash
|
||||
- `backend/src/google/config.ts` — client credentials and the derived redirect URI
|
||||
- `backend/src/google/oauth.ts` — the protocol: authorization URL, token exchange, claim verification
|
||||
- `backend/src/google/returnTo.ts` — the open-redirect guard, its own module so it is testable without a database
|
||||
- `backend/src/google/newCustomer.ts` — account creation from an identity
|
||||
- `backend/src/google/linkIdentity.ts` — the linking policy, and nothing else
|
||||
- `backend/src/routes/googleAuth.ts` — the two routes and the attempt cookie
|
||||
- `frontend/src/customer/GoogleSignInButton.tsx` — the button and Google's mark
|
||||
- `frontend/src/customer/Welcome.tsx` — the one-time consent step
|
||||
- `frontend/src/customer/ConnectedAccounts.tsx` — what the account is linked to
|
||||
- `docs/ops/google-sign-in.md` — the console setup and cutover checklist
|
||||
|
||||
**Modified:**
|
||||
- `backend/src/app.ts` — mounts the router, adds `googleSignIn` to the public config
|
||||
- `backend/src/routes/customers.ts` — null-safe password comparisons, first-password setting, `has_password`, the identities endpoint
|
||||
- `backend/src/passwordHashing.ts` — `passwordMatches`, which answers false for an absent hash instead of throwing
|
||||
- `backend/src/envValidation.ts` — the credentials are all-or-nothing
|
||||
- `backend/src/rateLimit.ts` — a limiter for the start route
|
||||
- `backend/src/db-kysely/schema.ts`, `backend/tests/integration/setup/testDb.ts` — the hand-maintained mirror and reset lists
|
||||
- `frontend/src/customer/AuthForm.tsx` — the button on both tabs, and the notice from a refused sign-in
|
||||
- `frontend/src/customer/AccountDetails.tsx` — set a first password rather than change one
|
||||
- `frontend/src/main.tsx`, `AuthRouteModal.tsx`, `AuthPromptModal.tsx` — the welcome route and the return path
|
||||
- `docker-compose.prod.yml`, `docker-compose.qa.yml` — credentials read from the stack
|
||||
|
||||
**Tests:** `googleConfig`, `googleOauth`, `googleReturnTo`, `passwordMatches` (unit); `googleSignIn`, `passwordlessAccounts` (integration); `auth.spec.ts` (e2e).
|
||||
|
||||
---
|
||||
|
||||
## Phase 0: Google Auth Platform setup
|
||||
|
||||
Console work, in the order of the left-hand nav. Full detail in `docs/ops/google-sign-in.md`.
|
||||
|
||||
- [x] Verify `redefined-designs.com` in Search Console, as a **Domain** property, with the same Google account used for the Cloud project
|
||||
- [x] **Branding** — app name, support email, authorized domain, home page and privacy links; no logo, which would trigger a brand review
|
||||
- [x] **Audience** — External, Testing, own account as a test user
|
||||
- [x] **Clients** — Web application, one redirect URI per environment
|
||||
- [x] **Data Access** — exactly `openid`, `email`, `profile`; anything sensitive turns publishing into a review
|
||||
- [x] **Verification Center** — confirm there is nothing to submit
|
||||
|
||||
## Phase 1: Groundwork (#340)
|
||||
|
||||
- [x] Make `customers.password_hash` nullable
|
||||
- [x] Add `customer_identities`, unique across `(provider, provider_sub)`
|
||||
- [x] Update the Kysely mirror, `REQUIRED_TABLES`, the truncate list and the schema-loss count
|
||||
- [x] Derive the redirect URI from `PUBLIC_URL`, with an `enabled` flag
|
||||
- [x] Add the credentials to environment validation, all-or-nothing
|
||||
- [x] Make the three bcrypt comparisons null-safe through one shared function
|
||||
|
||||
**The point of the shared function:** `bcrypt.compare` throws on a null hash rather than returning false, so a forgotten check answers a sign-in with a 500. On the login route that is also an oracle, because it happens for exactly the accounts that have no password.
|
||||
|
||||
## Phase 2: The round trip (#341)
|
||||
|
||||
- [x] Authorization code flow with PKCE
|
||||
- [x] State, nonce and verifier in one `httpOnly` cookie, `SameSite=Lax`, cleared on every path
|
||||
- [x] Verify the id token by its claims without a JWKS fetch, with the reasoning and its boundary written into the module
|
||||
- [x] Refuse a disabled account
|
||||
- [x] Guard the return path against becoming an open redirect
|
||||
- [x] Rate limit the start route
|
||||
|
||||
**`SameSite=Lax`, never `Strict`.** The callback is a cross-site top-level navigation. `Strict` withholds the cookie, the state check fails, and every sign-in is refused with an error that looks exactly like tampering.
|
||||
|
||||
## Phase 3: New accounts and consent (#342)
|
||||
|
||||
- [x] Create the customer and the identity in one transaction
|
||||
- [x] Both consents false, with no stored wording
|
||||
- [x] Land a new customer on `/welcome`, which asks with the same two sentences
|
||||
- [x] Take names from the profile as hints, tolerating their absence
|
||||
- [x] Mark the address verified only when Google asserts it; otherwise send the usual confirmation
|
||||
|
||||
**The consent problem:** a customer arriving through Google has never seen the checkboxes and could not have, because the redirect happens before anyone knows they are new. Creating the account with both false is lawful; asking immediately afterwards is what makes it honest.
|
||||
|
||||
## Phase 4: Linking (#343)
|
||||
|
||||
- [x] Link only when Google asserts `email_verified` and the address matches exactly
|
||||
- [x] Refuse otherwise, to a destination the customer can act on
|
||||
- [x] Refuse to link to a disabled account
|
||||
- [x] Show the linked account beside the passkeys
|
||||
|
||||
**The order is the policy.** The identity lookup runs first and nothing else is consulted when it matches, so an identity that has signed in before keeps working after the address changes on either side.
|
||||
|
||||
## Phase 5: Life without a password (#344)
|
||||
|
||||
- [x] One route sets a first password and changes an existing one, branching on the stored hash
|
||||
- [x] Refuse an email change until a password exists
|
||||
- [x] Leave login's single refusal exactly as it was
|
||||
- [x] Exercise the passkey lockout guard, unreachable since #40 and now live
|
||||
- [x] Report `has_password` to the account page, and nothing more
|
||||
|
||||
## Phase 6: The button (#345)
|
||||
|
||||
- [x] On both tabs, below the password form and the passkey option
|
||||
- [x] Google's mark inlined, per their identity guidelines
|
||||
- [x] The return path supplied by the caller, validated on the server
|
||||
- [x] Absent where unconfigured, from the public config flag
|
||||
|
||||
---
|
||||
|
||||
## Corrections
|
||||
|
||||
Two things the original plan asserted turned out to be false. Both are recorded rather than removed, because the reasoning errors are the useful part.
|
||||
|
||||
### QA could never run this
|
||||
|
||||
**Claimed:** `qa-redefined-designs.bermudalamb.synology.me` cannot carry a redirect URI, because Google requires the host to sit under a domain whose ownership is proved by DNS and Synology owns the domain above it. Therefore the feature could only be built locally until #313.
|
||||
|
||||
**Actually:** registering the URI works. QA has run Google sign-in since.
|
||||
|
||||
The claim was inferred from #285, where Cloudflare's free tier genuinely cannot be applied to that hostname, and stated as fact without being checked. On the strength of it, QA testing was documented as blocked, `docker-compose.qa.yml` hardcoded its credentials to empty rather than reading the stack, and a QA deploy was spent discovering otherwise.
|
||||
|
||||
What was actually established is narrower: `localhost` is exempt from the authorized-domain requirement, and a domain listed under Authorized domains has to be verified in Search Console. Whether either applied here was never tested.
|
||||
|
||||
### The local redirect URI
|
||||
|
||||
**Claimed:** register `http://localhost:3000/api/auth/google/callback` for local development.
|
||||
|
||||
**Actually:** local development browses the Vite dev server on 5173, so that is the URI the server sends and the one that must be registered. The 3000 entry applies only when the backend serves a built frontend, which local development does not produce. The omission cost an hour of `redirect_uri_mismatch`.
|
||||
|
||||
### Also corrected during the work
|
||||
|
||||
- **Account deletion does not confirm with a password.** #332 recorded that it does. The route takes none, so it needed no change, and there is now a test pinning that.
|
||||
- **The button was missing from the sign-up tab.** #345 rendered it inside the Log In tab only, following the passkey button too closely. A passkey belongs only on Log In because you cannot register an account with one; creating an account is precisely what a customer reaches for Google to do. Fixed before the feature was used in anger.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- **Unlinking a Google account** is not offered. Removing the only way into an account is guarded for passkeys and would need the same guard here.
|
||||
- **Apple** is decided against on #332: a paid programme, a client secret expiring every six months, no `localhost` redirect URIs, and a name and email returned exactly once.
|
||||
- **Google One Tap** has its own plan and a recommendation to wait. The two blockers are that a browser-supplied id token makes signature verification mandatory, and that its script must load for signed-out visitors, which collides with the consent rule in `frontend/src/brevo.ts`.
|
||||
Reference in New Issue
Block a user