Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1838bb38d1 | ||
|
|
0361ef35d0 | ||
|
|
f5a29127fb | ||
|
|
c9dccfe4a2 | ||
|
|
903a1d8b76 | ||
|
|
838749df64 | ||
|
|
3958bda489 |
@@ -14,27 +14,32 @@
|
||||
* source, and it is the one that is already correct in any environment where
|
||||
* mail works.
|
||||
*
|
||||
* ## The consequence worth stating plainly
|
||||
* ## Every environment needs its own console entry
|
||||
*
|
||||
* Google refuses a redirect URI whose host is not under an **authorized
|
||||
* domain**, and a domain can only be authorized after ownership has been proved
|
||||
* by DNS in Search Console. `localhost` is the sole exemption.
|
||||
* Whatever this resolves to has to exist, verbatim, under Authorized redirect
|
||||
* URIs for the client this app uses. Google compares the two as strings, and a
|
||||
* mismatch is answered with `redirect_uri_mismatch` — accurate, and silent
|
||||
* about which half is wrong.
|
||||
*
|
||||
* `qa-redefined-designs.bermudalamb.synology.me` therefore **cannot ever be
|
||||
* used**: Synology owns the registrable domain above it, so there is no record
|
||||
* to add and nothing to prove. This is the same wall #285 hit with Cloudflare.
|
||||
* | Environment | Redirect URI |
|
||||
* | --- | --- |
|
||||
* | Local, Vite | `http://localhost:5173/api/auth/google/callback` |
|
||||
* | Local, built | `http://localhost:3000/api/auth/google/callback` |
|
||||
* | QA | `https://qa-redefined-designs.bermudalamb.synology.me/api/auth/google/callback` |
|
||||
* | Production | `https://redefined-designs.com/api/auth/google/callback` |
|
||||
*
|
||||
* | Environment | Redirect URI | Works |
|
||||
* | --- | --- | --- |
|
||||
* | Local | `http://localhost:3000/...` | Yes, by exemption |
|
||||
* | QA on the Synology host | — | **No, and cannot** |
|
||||
* | QA on `qa.redefined-designs.com` | `https://qa.redefined-designs.com/...` | After #313 |
|
||||
* | Production | `https://redefined-designs.com/...` | After #313 |
|
||||
* Local development needs the 5173 one, because that is where the dev server
|
||||
* serves the app; the 3000 one only applies when the backend serves a built
|
||||
* frontend.
|
||||
*
|
||||
* So this feature is built and exercised locally, and QA cannot see it until QA
|
||||
* moves onto a subdomain of the real domain. That is a `PUBLIC_URL` change and
|
||||
* one console entry, not a code change — this module follows `PUBLIC_URL`
|
||||
* wherever it points. See #345.
|
||||
* An earlier version of this comment claimed the QA hostname could never be
|
||||
* registered, because it sits under a domain Synology owns. **That was wrong**,
|
||||
* and it is recorded here rather than quietly deleted: it was asserted from the
|
||||
* shape of #285, which is a related but different problem, and it sent QA
|
||||
* testing of this feature behind #313 for no reason. Adding the URI works.
|
||||
*
|
||||
* This module needs no change in any environment. It follows `PUBLIC_URL`
|
||||
* wherever it points.
|
||||
*/
|
||||
|
||||
/** The callback path. One constant, because it appears in two sentences. */
|
||||
|
||||
+26
-12
@@ -63,6 +63,12 @@
|
||||
# in the notification email (#224). Absent, the email still
|
||||
# sends and simply carries no shortcuts. Its own value, not
|
||||
# production's: a link signed with it acts without a login.
|
||||
# QA_GOOGLE_CLIENT_ID — optional, and all-or-nothing with the secret below:
|
||||
# QA_GOOGLE_CLIENT_SECRET setting one without the other refuses to boot
|
||||
# (#340). Both unset means the Google button is not offered
|
||||
# at all, which is the right answer until QA's callback URL
|
||||
# is registered in the Google Auth Platform. See the note
|
||||
# beside the values themselves for the exact URL (#345).
|
||||
# QA_REMBG_URL — optional. The background-removal sidecar, e.g.
|
||||
# http://rembg-syn:7000. Unset turns the feature off rather
|
||||
# than breaking anything. The sidecar must be on the same
|
||||
@@ -195,20 +201,28 @@ services:
|
||||
# rotating it revokes every outstanding link, which is the intended way to
|
||||
# deal with a leak.
|
||||
- INTAKE_ACTION_SECRET=${QA_INTAKE_ACTION_SECRET:-}
|
||||
# Deliberately left empty, and it is not an oversight (#340, #345).
|
||||
# Read from the stack like every other QA secret, rather than hardcoded
|
||||
# empty as they were in #340. Leaving them unreadable made this file the
|
||||
# odd one out and cost a QA deploy: the variables were set on the stack,
|
||||
# nothing read them, and the button stayed missing with no explanation.
|
||||
#
|
||||
# Google refuses a redirect URI whose host is not under a domain whose
|
||||
# ownership has been proved by DNS, and nobody can prove ownership of
|
||||
# *.bermudalamb.synology.me because Synology owns the registrable domain
|
||||
# above it. Same wall as #285. So QA cannot run Google sign-in at all
|
||||
# while it lives on this hostname, and setting these would only produce a
|
||||
# button that fails at Google.
|
||||
# Setting these needs one thing done first: the QA callback registered
|
||||
# under Authorized redirect URIs for this client in the Google Auth
|
||||
# Platform, exactly as it appears below. Google compares the two as
|
||||
# strings and answers a mismatch with redirect_uri_mismatch.
|
||||
#
|
||||
# It becomes possible when #313 moves QA to qa.redefined-designs.com:
|
||||
# set both here, set PUBLIC_URL to the new host, and add the matching
|
||||
# callback in the Google Auth Platform. No code change either way.
|
||||
- GOOGLE_CLIENT_ID=
|
||||
- GOOGLE_CLIENT_SECRET=
|
||||
# https://qa-redefined-designs.bermudalamb.synology.me/api/auth/google/callback
|
||||
#
|
||||
# An earlier version of this comment said that URI could never be
|
||||
# registered, because Synology owns the domain above it. That was wrong,
|
||||
# and the correction is left here rather than removed: it was inferred
|
||||
# from #285, which is a related but different problem, and it put QA
|
||||
# testing of this feature behind #313 for no reason.
|
||||
#
|
||||
# When #313 moves QA to qa.redefined-designs.com, point PUBLIC_URL at the
|
||||
# new host and register that callback too. No code change either way.
|
||||
- GOOGLE_CLIENT_ID=${QA_GOOGLE_CLIENT_ID:-}
|
||||
- GOOGLE_CLIENT_SECRET=${QA_GOOGLE_CLIENT_SECRET:-}
|
||||
volumes:
|
||||
# Separate uploads directory. Sharing production's would let a QA run
|
||||
# write into, and a QA teardown delete, real product images.
|
||||
|
||||
+52
-28
@@ -18,33 +18,42 @@ screen, so what appears there is the production identity even while testing.
|
||||
| Data Access | Exactly `openid`, `email`, `profile` |
|
||||
| Verification Center | Nothing to submit, and it should stay that way |
|
||||
|
||||
## The constraint that shapes everything
|
||||
## Redirect URIs, one per environment
|
||||
|
||||
**Google refuses a redirect URI whose host is not under an authorized domain,
|
||||
and a domain can only be authorized after ownership is proved by DNS in Search
|
||||
Console.** `localhost` is the only exemption.
|
||||
Every environment sends a redirect URI derived from its own `PUBLIC_URL`, and
|
||||
each one has to exist verbatim under **Authorized redirect URIs** on the client
|
||||
this app uses. Google compares them as strings and answers a mismatch with
|
||||
`redirect_uri_mismatch`, which is accurate and says nothing about which half is
|
||||
wrong.
|
||||
|
||||
`qa-redefined-designs.bermudalamb.synology.me` therefore cannot ever be used:
|
||||
Synology owns the registrable domain above it, so there is no record to add and
|
||||
nothing to prove. This is the same wall #285 hit with Cloudflare.
|
||||
|
||||
The consequence, stated plainly because it changes how the feature is worked on:
|
||||
|
||||
| Environment | Google sign-in |
|
||||
| Environment | Redirect URI |
|
||||
| --- | --- |
|
||||
| Local, on `localhost` | Works, by exemption |
|
||||
| QA on the Synology hostname | **Impossible**, not merely unconfigured |
|
||||
| QA on `qa.redefined-designs.com` | Works, after the cutover |
|
||||
| Production on `redefined-designs.com` | Works, after the cutover |
|
||||
| Local, Vite dev server | `http://localhost:5173/api/auth/google/callback` |
|
||||
| Local, backend serving a build | `http://localhost:3000/api/auth/google/callback` |
|
||||
| QA | `https://qa-redefined-designs.bermudalamb.synology.me/api/auth/google/callback` |
|
||||
| Production | `https://redefined-designs.com/api/auth/google/callback` |
|
||||
|
||||
So this feature is built and exercised locally, and QA cannot see it at all
|
||||
until QA moves onto a subdomain of the real domain. The Search Console property
|
||||
for `redefined-designs.com` already covers `qa.redefined-designs.com`, because
|
||||
a Domain property covers every subdomain.
|
||||
Local development needs the 5173 entry, because that is where the dev server
|
||||
serves the app. The 3000 one applies only when the backend serves a built
|
||||
frontend, which local development does not produce.
|
||||
|
||||
`docker-compose.qa.yml` sets both credentials to empty deliberately, with a
|
||||
comment saying so, and the storefront then offers no button rather than one that
|
||||
fails at Google.
|
||||
### A correction
|
||||
|
||||
An earlier version of this document said the QA hostname **could never be
|
||||
registered**, because it sits under a domain Synology owns rather than one we
|
||||
do. That was wrong. Adding the URI works.
|
||||
|
||||
The claim is recorded here rather than quietly removed, because of what it
|
||||
cost. It was inferred from #285, where Cloudflare genuinely cannot be applied to
|
||||
that hostname, and asserted with far more confidence than the inference
|
||||
supported. On the strength of it, QA testing of Google sign-in was documented as
|
||||
blocked behind #313, the QA compose file hardcoded its credentials to empty, and
|
||||
two issues recorded it as fact.
|
||||
|
||||
What is true, and is all that was ever established: `localhost` is exempt from
|
||||
the authorized-domain rules, and a domain listed as an authorized domain has to
|
||||
be verified in Search Console. Whether either of those actually applied to this
|
||||
hostname, and how, was never checked.
|
||||
|
||||
## Scopes, and why publishing needs no review
|
||||
|
||||
@@ -60,20 +69,35 @@ walkthrough and a wait measured in weeks.** Nothing in this feature needs one.
|
||||
Uploading an app logo also triggers a brand review, which is why Branding has
|
||||
none.
|
||||
|
||||
## Turning it on in QA
|
||||
|
||||
Already done, and recorded here because the order matters.
|
||||
|
||||
1. Register the QA callback under **Clients**, Authorized redirect URIs:
|
||||
`https://qa-redefined-designs.bermudalamb.synology.me/api/auth/google/callback`
|
||||
2. Set `QA_GOOGLE_CLIENT_ID` and `QA_GOOGLE_CLIENT_SECRET` on the QA stack. Both
|
||||
or neither — the backend refuses to start on one without the other, because
|
||||
the failure would otherwise arrive the moment a customer presses the button.
|
||||
3. Redeploy.
|
||||
|
||||
Registering first is the point. Setting the variables makes the button appear,
|
||||
and a button that appears before its callback exists fails at Google rather than
|
||||
in the storefront, where nothing in the logs explains it.
|
||||
|
||||
## The cutover checklist, for #313
|
||||
|
||||
1. Point QA at `qa.redefined-designs.com` and set its `PUBLIC_URL` to match.
|
||||
2. Set `GOOGLE_CLIENT_ID` and `GOOGLE_CLIENT_SECRET` in the QA stack. Both or
|
||||
neither — the backend refuses to start on one without the other, because the
|
||||
failure would otherwise arrive the moment a customer presses the button.
|
||||
3. In **Clients**, add the QA callback:
|
||||
2. In **Clients**, add the new QA callback:
|
||||
`https://qa.redefined-designs.com/api/auth/google/callback`
|
||||
4. Confirm the production callback is registered:
|
||||
3. Confirm the production callback is registered:
|
||||
`https://redefined-designs.com/api/auth/google/callback`
|
||||
5. In **Audience**, move the publishing status from Testing to **In production**.
|
||||
4. In **Audience**, move the publishing status from Testing to **In production**.
|
||||
Do it once the domain resolves, so the home page and privacy links Google
|
||||
shows actually answer.
|
||||
|
||||
The old QA callback can be left registered until the hostname is retired. An
|
||||
extra entry costs nothing and removing it early breaks QA for no gain.
|
||||
|
||||
No code changes at any step. The redirect URI is derived from `PUBLIC_URL`, so
|
||||
the environment variable and the console entry are the whole of it.
|
||||
|
||||
|
||||
@@ -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`.
|
||||
@@ -233,6 +233,27 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
||||
By creating an account you agree to our{' '}
|
||||
<a href="/privacy" target="_blank" rel="noopener noreferrer">Privacy Policy</a>.
|
||||
</Text>
|
||||
|
||||
{/* On this tab too, and its absence here was a bug (#345).
|
||||
A passkey belongs only on Log In, because you cannot
|
||||
register an account with one — but creating an account is
|
||||
exactly what a new customer reaches for Google to do, so
|
||||
leaving it off the sign-up tab hid the feature from the
|
||||
people it helps most.
|
||||
|
||||
The two consent boxes above are not carried across. Google
|
||||
takes the customer off this site entirely, and a tick that
|
||||
survived that round trip would be a consent recorded from a
|
||||
form nobody submitted. They are asked again, with the same
|
||||
wording, on the step they land on (#342). */}
|
||||
{googleEnabled && (
|
||||
<>
|
||||
<Divider plain style={{ marginBlock: 16 }}>
|
||||
<Text type="secondary" style={{ fontSize: 12 }}>or</Text>
|
||||
</Divider>
|
||||
<GoogleSignInButton returnTo={returnTo} intent="sign-up" />
|
||||
</>
|
||||
)}
|
||||
</Form>
|
||||
)
|
||||
},
|
||||
|
||||
@@ -37,6 +37,17 @@ function GoogleMark() {
|
||||
type Props = Readonly<{
|
||||
/** Where to send the customer back to. Validated again on the server. */
|
||||
returnTo: string;
|
||||
/**
|
||||
* Which tab this sits on, which changes only the wording.
|
||||
*
|
||||
* One endpoint serves both: it signs in a known identity, links a verified
|
||||
* address, or creates an account. The customer does not know or care which
|
||||
* of those will happen, so the label matches what they came to the tab to
|
||||
* do rather than what the server ends up doing.
|
||||
*
|
||||
* Both spellings are in Google identity guidelines alongside the mark.
|
||||
*/
|
||||
intent?: 'sign-in' | 'sign-up';
|
||||
}>;
|
||||
|
||||
/**
|
||||
@@ -51,7 +62,7 @@ type Props = Readonly<{
|
||||
* here. It has to be, since anyone can type the URL, and doing it in one place
|
||||
* beats doing it in two languages. See `google/returnTo.ts`.
|
||||
*/
|
||||
export default function GoogleSignInButton({ returnTo }: Props) {
|
||||
export default function GoogleSignInButton({ returnTo, intent = 'sign-in' }: Props) {
|
||||
return (
|
||||
<Button
|
||||
block
|
||||
@@ -62,7 +73,7 @@ export default function GoogleSignInButton({ returnTo }: Props) {
|
||||
window.location.assign(`/api/auth/google/start?returnTo=${encodeURIComponent(returnTo)}`);
|
||||
}}
|
||||
>
|
||||
Sign in with Google
|
||||
{intent === 'sign-up' ? 'Sign up with Google' : 'Sign in with Google'}
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -124,6 +124,23 @@ test.describe('Customer accounts', () => {
|
||||
// So the button being ABSENT is the behaviour under test here, and it is the
|
||||
// one that matters: a control that appears and then fails at Google is worse
|
||||
// than one that was never offered.
|
||||
// The sign-up tab, which #345 left it off entirely. A passkey belongs only on
|
||||
// Log In, because you cannot register an account with one — but creating an
|
||||
// account is exactly what a new customer reaches for Google to do, so its
|
||||
// absence there hid the feature from the people it helps most.
|
||||
//
|
||||
// Asserted as absent for the same reason as the login one: local and QA have
|
||||
// no credentials, so absence is the behaviour that actually runs here.
|
||||
test('offers no Google button on the sign-up tab either, when unconfigured', async ({
|
||||
authModal
|
||||
}) => {
|
||||
await authModal.gotoRegister();
|
||||
|
||||
await expect(
|
||||
authModal.registerDialog.getByRole('button', { name: /with Google/i })
|
||||
).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('offers no Google button when the environment is not configured for it', async ({
|
||||
authModal,
|
||||
accountModal,
|
||||
|
||||
Reference in New Issue
Block a user