Compare commits
10
Commits
0014a8db8a
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0361ef35d0 | ||
|
|
f5a29127fb | ||
|
|
c9dccfe4a2 | ||
|
|
903a1d8b76 | ||
|
|
838749df64 | ||
|
|
3958bda489 | ||
|
|
b82b989cc8 | ||
|
|
c2ee0b0c5d | ||
|
|
843c51dd91 | ||
|
|
2c6ac4d2be |
+11
-1
@@ -20,6 +20,7 @@ import customersRouter from './routes/customers';
|
|||||||
import passkeysRouter from './routes/passkeys';
|
import passkeysRouter from './routes/passkeys';
|
||||||
import passkeyLoginRouter from './routes/passkeyLogin';
|
import passkeyLoginRouter from './routes/passkeyLogin';
|
||||||
import googleAuthRouter from './routes/googleAuth';
|
import googleAuthRouter from './routes/googleAuth';
|
||||||
|
import { googleConfig } from './google/config';
|
||||||
import publicRouter from './routes/public';
|
import publicRouter from './routes/public';
|
||||||
import cartRouter from './routes/cart';
|
import cartRouter from './routes/cart';
|
||||||
import shippingAddressesRouter from './routes/shippingAddresses';
|
import shippingAddressesRouter from './routes/shippingAddresses';
|
||||||
@@ -72,7 +73,16 @@ app.get('/api/config', (_req, res) => {
|
|||||||
// keeps QA out of production's Brevo account: QA sets no key, so no QA
|
// keeps QA out of production's Brevo account: QA sets no key, so no QA
|
||||||
// browsing is ever reported, and there is no flag anyone can forget to
|
// browsing is ever reported, and there is no flag anyone can forget to
|
||||||
// turn off. Same shape as paypalClientId above.
|
// turn off. Same shape as paypalClientId above.
|
||||||
brevoTrackerKey: process.env.BREVO_TRACKER_KEY?.trim() || null
|
brevoTrackerKey: process.env.BREVO_TRACKER_KEY?.trim() || null,
|
||||||
|
// Whether to offer the Google button at all (#345). A boolean, never the
|
||||||
|
// client id: the browser does not need it, because the whole flow is a
|
||||||
|
// redirect this server builds.
|
||||||
|
//
|
||||||
|
// Absent rather than disabled is the point. A developer with no credentials
|
||||||
|
// gets a storefront that works and simply does not offer the option, the
|
||||||
|
// same choice #41 made for a browser without WebAuthn — and QA, which
|
||||||
|
// cannot have credentials until #313, gets the same.
|
||||||
|
googleSignIn: googleConfig().enabled
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -14,27 +14,32 @@
|
|||||||
* source, and it is the one that is already correct in any environment where
|
* source, and it is the one that is already correct in any environment where
|
||||||
* mail works.
|
* 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
|
* Whatever this resolves to has to exist, verbatim, under Authorized redirect
|
||||||
* domain**, and a domain can only be authorized after ownership has been proved
|
* URIs for the client this app uses. Google compares the two as strings, and a
|
||||||
* by DNS in Search Console. `localhost` is the sole exemption.
|
* 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
|
* | Environment | Redirect URI |
|
||||||
* 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.
|
* | 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 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
|
||||||
* | Local | `http://localhost:3000/...` | Yes, by exemption |
|
* frontend.
|
||||||
* | 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 |
|
|
||||||
*
|
*
|
||||||
* So this feature is built and exercised locally, and QA cannot see it until QA
|
* An earlier version of this comment claimed the QA hostname could never be
|
||||||
* moves onto a subdomain of the real domain. That is a `PUBLIC_URL` change and
|
* registered, because it sits under a domain Synology owns. **That was wrong**,
|
||||||
* one console entry, not a code change — this module follows `PUBLIC_URL`
|
* and it is recorded here rather than quietly deleted: it was asserted from the
|
||||||
* wherever it points. See #345.
|
* 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. */
|
/** The callback path. One constant, because it appears in two sentences. */
|
||||||
|
|||||||
@@ -730,3 +730,54 @@ describe('GET /api/customers/me/identities', () => {
|
|||||||
expect(res.status).toBe(401);
|
expect(res.status).toBe(401);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the storefront offers a Google button at all (#345).
|
||||||
|
*
|
||||||
|
* A boolean and never the client id: the browser does not need one, because
|
||||||
|
* the whole flow is a redirect the server builds.
|
||||||
|
*/
|
||||||
|
describe('GET /api/config, google sign-in', () => {
|
||||||
|
// The suite-wide beforeEach configures Google so the flow above can run.
|
||||||
|
// These tests are about the unconfigured case too, so they start from clean.
|
||||||
|
beforeEach(() => {
|
||||||
|
delete process.env.GOOGLE_CLIENT_ID;
|
||||||
|
delete process.env.GOOGLE_CLIENT_SECRET;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is false when the environment has no credentials', async () => {
|
||||||
|
const res = await request(app).get('/api/config');
|
||||||
|
|
||||||
|
// Which is the state of local development, and of QA until #313 moves it
|
||||||
|
// off a hostname whose domain nobody can prove they own.
|
||||||
|
expect(res.body.googleSignIn).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is true when both credentials are set', async () => {
|
||||||
|
process.env.GOOGLE_CLIENT_ID = 'id.apps.googleusercontent.com';
|
||||||
|
process.env.GOOGLE_CLIENT_SECRET = 'shh';
|
||||||
|
|
||||||
|
const res = await request(app).get('/api/config');
|
||||||
|
|
||||||
|
expect(res.body.googleSignIn).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is false with only one of the pair, matching what the backend refuses to boot on', async () => {
|
||||||
|
process.env.GOOGLE_CLIENT_ID = 'id.apps.googleusercontent.com';
|
||||||
|
|
||||||
|
const res = await request(app).get('/api/config');
|
||||||
|
|
||||||
|
expect(res.body.googleSignIn).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never sends the client id or secret to the browser', async () => {
|
||||||
|
process.env.GOOGLE_CLIENT_ID = 'id.apps.googleusercontent.com';
|
||||||
|
process.env.GOOGLE_CLIENT_SECRET = 'a-real-looking-secret';
|
||||||
|
|
||||||
|
const res = await request(app).get('/api/config');
|
||||||
|
|
||||||
|
const body = JSON.stringify(res.body);
|
||||||
|
expect(body).not.toContain('a-real-looking-secret');
|
||||||
|
expect(body).not.toContain('googleusercontent');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
+26
-12
@@ -63,6 +63,12 @@
|
|||||||
# in the notification email (#224). Absent, the email still
|
# in the notification email (#224). Absent, the email still
|
||||||
# sends and simply carries no shortcuts. Its own value, not
|
# sends and simply carries no shortcuts. Its own value, not
|
||||||
# production's: a link signed with it acts without a login.
|
# 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.
|
# QA_REMBG_URL — optional. The background-removal sidecar, e.g.
|
||||||
# http://rembg-syn:7000. Unset turns the feature off rather
|
# http://rembg-syn:7000. Unset turns the feature off rather
|
||||||
# than breaking anything. The sidecar must be on the same
|
# 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
|
# rotating it revokes every outstanding link, which is the intended way to
|
||||||
# deal with a leak.
|
# deal with a leak.
|
||||||
- INTAKE_ACTION_SECRET=${QA_INTAKE_ACTION_SECRET:-}
|
- 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
|
# Setting these needs one thing done first: the QA callback registered
|
||||||
# ownership has been proved by DNS, and nobody can prove ownership of
|
# under Authorized redirect URIs for this client in the Google Auth
|
||||||
# *.bermudalamb.synology.me because Synology owns the registrable domain
|
# Platform, exactly as it appears below. Google compares the two as
|
||||||
# above it. Same wall as #285. So QA cannot run Google sign-in at all
|
# strings and answers a mismatch with redirect_uri_mismatch.
|
||||||
# while it lives on this hostname, and setting these would only produce a
|
|
||||||
# button that fails at Google.
|
|
||||||
#
|
#
|
||||||
# It becomes possible when #313 moves QA to qa.redefined-designs.com:
|
# https://qa-redefined-designs.bermudalamb.synology.me/api/auth/google/callback
|
||||||
# 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.
|
# An earlier version of this comment said that URI could never be
|
||||||
- GOOGLE_CLIENT_ID=
|
# registered, because Synology owns the domain above it. That was wrong,
|
||||||
- GOOGLE_CLIENT_SECRET=
|
# 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:
|
volumes:
|
||||||
# Separate uploads directory. Sharing production's would let a QA run
|
# Separate uploads directory. Sharing production's would let a QA run
|
||||||
# write into, and a QA teardown delete, real product images.
|
# write into, and a QA teardown delete, real product images.
|
||||||
|
|||||||
@@ -0,0 +1,130 @@
|
|||||||
|
# Google sign-in
|
||||||
|
|
||||||
|
What has to be true outside the repository for the Google button to work, and
|
||||||
|
what to do at the domain cutover. The code side is #332 and the six issues under
|
||||||
|
it; this is only the parts that live in a browser tab at Google.
|
||||||
|
|
||||||
|
## Where it is configured
|
||||||
|
|
||||||
|
The **Google Auth Platform** in the Google Cloud Console, in one project. There
|
||||||
|
is one consent screen per project and every OAuth client in it shares that
|
||||||
|
screen, so what appears there is the production identity even while testing.
|
||||||
|
|
||||||
|
| Section | What it holds |
|
||||||
|
| --- | --- |
|
||||||
|
| Branding | App name, support email, authorized domains, the three app links |
|
||||||
|
| Audience | External, publishing status, test users |
|
||||||
|
| Clients | The OAuth client, its redirect URIs, the id and secret |
|
||||||
|
| Data Access | Exactly `openid`, `email`, `profile` |
|
||||||
|
| Verification Center | Nothing to submit, and it should stay that way |
|
||||||
|
|
||||||
|
## Redirect URIs, one per environment
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
| Environment | Redirect URI |
|
||||||
|
| --- | --- |
|
||||||
|
| 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` |
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
### 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
|
||||||
|
|
||||||
|
`openid` produces the id token carrying the subject claim, which is the identity
|
||||||
|
stored. `email` carries the address and the `email_verified` flag the linking
|
||||||
|
policy turns on. `profile` carries the names used when an account is created.
|
||||||
|
|
||||||
|
All three are non-sensitive. Requesting only them is what lets the app publish
|
||||||
|
without verification and without customers seeing an unverified-app warning.
|
||||||
|
**Add one sensitive scope and publishing becomes a review with a video
|
||||||
|
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. In **Clients**, add the new QA callback:
|
||||||
|
`https://qa.redefined-designs.com/api/auth/google/callback`
|
||||||
|
3. Confirm the production callback is registered:
|
||||||
|
`https://redefined-designs.com/api/auth/google/callback`
|
||||||
|
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.
|
||||||
|
|
||||||
|
**Leaving it in Testing is the failure to watch for.** Only listed test users can
|
||||||
|
sign in, and the refusal happens on Google's own page, so nothing reaches the
|
||||||
|
storefront and nothing appears in its logs. A customer reports a broken button
|
||||||
|
and the logs are silent.
|
||||||
|
|
||||||
|
## The production smoke test
|
||||||
|
|
||||||
|
The consent screen, the redirect and the domain are all environment-specific, so
|
||||||
|
QA proves the flow and not the configuration. After the cutover:
|
||||||
|
|
||||||
|
1. Sign in with a Google account that has never been used on the site. A new
|
||||||
|
customer is created and lands on the consent step.
|
||||||
|
2. Sign in again with the same account. It reaches the same customer rather than
|
||||||
|
a second one.
|
||||||
|
3. Check the account page lists Google under connected accounts.
|
||||||
|
|
||||||
|
## What is not offered, and why
|
||||||
|
|
||||||
|
**Unlinking.** A customer cannot detach their Google account. Removing the only
|
||||||
|
way into an account is guarded for passkeys and the same guard would be needed
|
||||||
|
here first. Worth its own issue when somebody actually asks.
|
||||||
|
|
||||||
|
**Apple.** A separate decision with a materially different cost, set out on
|
||||||
|
#332: a paid developer programme, a client secret that expires every six months,
|
||||||
|
no `localhost` redirect URIs at all, and a name and email returned exactly once.
|
||||||
|
Apple is required for iOS apps offering third-party sign-in, and this is a
|
||||||
|
website, so that rule does not apply here.
|
||||||
@@ -61,6 +61,14 @@ export interface SiteConfig {
|
|||||||
* A key alone does not start tracking — see brevo.ts.
|
* A key alone does not start tracking — see brevo.ts.
|
||||||
*/
|
*/
|
||||||
brevoTrackerKey: string | null;
|
brevoTrackerKey: string | null;
|
||||||
|
/**
|
||||||
|
* Whether Google sign-in is configured in this environment (#345).
|
||||||
|
*
|
||||||
|
* False locally without credentials, and false in QA until #313 moves it off
|
||||||
|
* the Synology hostname — Google refuses a redirect URI whose domain nobody
|
||||||
|
* can prove they own. The button is then absent rather than disabled.
|
||||||
|
*/
|
||||||
|
googleSignIn: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function fetchConfig(): Promise<SiteConfig> {
|
export async function fetchConfig(): Promise<SiteConfig> {
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useState } from 'react';
|
import { useEffect, useState } from 'react';
|
||||||
import { useSearchParams } from 'react-router-dom';
|
import { useSearchParams } from 'react-router-dom';
|
||||||
import Form from 'antd/es/form';
|
import Form from 'antd/es/form';
|
||||||
import Input from 'antd/es/input';
|
import Input from 'antd/es/input';
|
||||||
@@ -10,6 +10,8 @@ import Typography from 'antd/es/typography';
|
|||||||
import Divider from 'antd/es/divider';
|
import Divider from 'antd/es/divider';
|
||||||
import { registerCustomer, loginCustomer, signInWithPasskey, passkeysSupported } from './customerApi';
|
import { registerCustomer, loginCustomer, signInWithPasskey, passkeysSupported } from './customerApi';
|
||||||
import { useCustomerAuth } from './CustomerAuthContext';
|
import { useCustomerAuth } from './CustomerAuthContext';
|
||||||
|
import GoogleSignInButton from './GoogleSignInButton';
|
||||||
|
import { fetchConfig } from '../api';
|
||||||
|
|
||||||
const { Text } = Typography;
|
const { Text } = Typography;
|
||||||
|
|
||||||
@@ -42,6 +44,15 @@ type Props = Readonly<{
|
|||||||
// route closes back to the page behind it, while the cart and favorite
|
// route closes back to the page behind it, while the cart and favorite
|
||||||
// prompts resume the action the customer was interrupted doing.
|
// prompts resume the action the customer was interrupted doing.
|
||||||
onSuccess: () => void;
|
onSuccess: () => void;
|
||||||
|
/**
|
||||||
|
* Where a Google sign-in should return the customer (#345).
|
||||||
|
*
|
||||||
|
* Supplied by the caller because only the caller knows: the route modal has a
|
||||||
|
* page behind it, and the cart prompt has the page it interrupted. An OAuth
|
||||||
|
* redirect leaves the application entirely, so this cannot be recovered
|
||||||
|
* afterwards the way onSuccess recovers it for every other path.
|
||||||
|
*/
|
||||||
|
returnTo?: string;
|
||||||
}>;
|
}>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -73,7 +84,7 @@ function googleNotice(reason: string | null): string | null {
|
|||||||
// written twice — once as the /login and /register pages, once inside the
|
// written twice — once as the /login and /register pages, once inside the
|
||||||
// prompt shown when a signed-out visitor adds to the cart — which had already
|
// prompt shown when a signed-out visitor adds to the cart — which had already
|
||||||
// drifted in consent wording and in which links each offered.
|
// drifted in consent wording and in which links each offered.
|
||||||
export default function AuthForm({ mode, onModeChange, onForgotPassword, onSuccess }: Props) {
|
export default function AuthForm({ mode, onModeChange, onForgotPassword, onSuccess, returnTo = '/' }: Props) {
|
||||||
const [searchParams] = useSearchParams();
|
const [searchParams] = useSearchParams();
|
||||||
const notice = googleNotice(searchParams.get('auth'));
|
const notice = googleNotice(searchParams.get('auth'));
|
||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
@@ -89,6 +100,20 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
|||||||
// rather than whether pressing it works.
|
// rather than whether pressing it works.
|
||||||
const canUsePasskeys = passkeysSupported();
|
const canUsePasskeys = passkeysSupported();
|
||||||
|
|
||||||
|
// Whether this environment has Google credentials at all. Fetched rather than
|
||||||
|
// built in, because one image serves every environment — and false is the
|
||||||
|
// right starting value: a button that appears a moment late is better than
|
||||||
|
// one that appears and then vanishes.
|
||||||
|
const [googleEnabled, setGoogleEnabled] = useState(false);
|
||||||
|
useEffect(() => {
|
||||||
|
fetchConfig()
|
||||||
|
.then((config) => setGoogleEnabled(config.googleSignIn))
|
||||||
|
// Silent, and the button simply never appears. The password form behind
|
||||||
|
// it works regardless, which is the whole reason it is below rather than
|
||||||
|
// above.
|
||||||
|
.catch(() => setGoogleEnabled(false));
|
||||||
|
}, []);
|
||||||
|
|
||||||
async function submit(action: () => Promise<unknown>) {
|
async function submit(action: () => Promise<unknown>) {
|
||||||
setLoading(true);
|
setLoading(true);
|
||||||
setError(null);
|
setError(null);
|
||||||
@@ -208,6 +233,27 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
|||||||
By creating an account you agree to our{' '}
|
By creating an account you agree to our{' '}
|
||||||
<a href="/privacy" target="_blank" rel="noopener noreferrer">Privacy Policy</a>.
|
<a href="/privacy" target="_blank" rel="noopener noreferrer">Privacy Policy</a>.
|
||||||
</Text>
|
</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>
|
</Form>
|
||||||
)
|
)
|
||||||
},
|
},
|
||||||
@@ -238,11 +284,13 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
|||||||
works for everyone. Absent entirely where WebAuthn is not
|
works for everyone. Absent entirely where WebAuthn is not
|
||||||
available, rather than shown disabled: a greyed button
|
available, rather than shown disabled: a greyed button
|
||||||
invites a customer to wonder what they are missing (#41). */}
|
invites a customer to wonder what they are missing (#41). */}
|
||||||
|
{(canUsePasskeys || googleEnabled) && (
|
||||||
|
<Divider plain style={{ marginBlock: 16 }}>
|
||||||
|
<Text type="secondary" style={{ fontSize: 12 }}>or</Text>
|
||||||
|
</Divider>
|
||||||
|
)}
|
||||||
{canUsePasskeys && (
|
{canUsePasskeys && (
|
||||||
<>
|
<>
|
||||||
<Divider plain style={{ marginBlock: 16 }}>
|
|
||||||
<Text type="secondary" style={{ fontSize: 12 }}>or</Text>
|
|
||||||
</Divider>
|
|
||||||
<Button
|
<Button
|
||||||
block
|
block
|
||||||
loading={passkeyLoading}
|
loading={passkeyLoading}
|
||||||
@@ -258,6 +306,17 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
|||||||
</Text>
|
</Text>
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
|
{/* Below the passkey button, which is below the password form.
|
||||||
|
The order is deliberate and it is not about preference: a
|
||||||
|
passkey is already on the device in front of the customer,
|
||||||
|
while Google is a round trip to somebody else's site. Absent
|
||||||
|
rather than disabled where it is not configured, for the
|
||||||
|
same reason as the one above (#345). */}
|
||||||
|
{googleEnabled && (
|
||||||
|
<div style={{ marginTop: canUsePasskeys ? 16 : 0 }}>
|
||||||
|
<GoogleSignInButton returnTo={returnTo} />
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</Form>
|
</Form>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,6 +37,12 @@ export default function AuthPromptModal({ open, onClose, onSuccess }: Props) {
|
|||||||
onClose();
|
onClose();
|
||||||
navigate('/forgot-password', { state: { background: location } });
|
navigate('/forgot-password', { state: { background: location } });
|
||||||
}}
|
}}
|
||||||
|
// The page the customer was on when this interrupted them, which is
|
||||||
|
// where a Google round trip should put them back (#345). Unlike
|
||||||
|
// onSuccess it cannot resume the interrupted action — the redirect
|
||||||
|
// leaves the application — so it returns them to the page and they
|
||||||
|
// press the button again.
|
||||||
|
returnTo={`${location.pathname}${location.search}`}
|
||||||
onSuccess={onSuccess}
|
onSuccess={onSuccess}
|
||||||
/>
|
/>
|
||||||
</Modal>
|
</Modal>
|
||||||
|
|||||||
@@ -7,6 +7,15 @@ type Props = Readonly<{
|
|||||||
// Moving between the auth routes, supplied by the router so the rule about
|
// Moving between the auth routes, supplied by the router so the rule about
|
||||||
// keeping the whole detour to one history entry lives in one place.
|
// keeping the whole detour to one history entry lives in one place.
|
||||||
onNavigate: (path: string) => void;
|
onNavigate: (path: string) => void;
|
||||||
|
/**
|
||||||
|
* The page behind this modal, for a Google sign-in to return to (#345).
|
||||||
|
*
|
||||||
|
* Supplied by the router, which is the only thing that knows it: this modal
|
||||||
|
* renders over a backdrop location, and its own path is /login, so reading
|
||||||
|
* the current URL here would send the customer back to the form they just
|
||||||
|
* left.
|
||||||
|
*/
|
||||||
|
returnTo: string;
|
||||||
}>;
|
}>;
|
||||||
|
|
||||||
const TITLES: Record<AuthMode, string> = {
|
const TITLES: Record<AuthMode, string> = {
|
||||||
@@ -18,7 +27,7 @@ const TITLES: Record<AuthMode, string> = {
|
|||||||
// clicks Log in while browsing and changes their mind is not stranded. Both
|
// clicks Log in while browsing and changes their mind is not stranded. Both
|
||||||
// stay real routes: /reset-password links to /login, and customers may have
|
// stay real routes: /reset-password links to /login, and customers may have
|
||||||
// bookmarks.
|
// bookmarks.
|
||||||
export default function AuthRouteModal({ mode, onClose, onNavigate }: Props) {
|
export default function AuthRouteModal({ mode, onClose, onNavigate, returnTo }: Props) {
|
||||||
return (
|
return (
|
||||||
<Modal
|
<Modal
|
||||||
title={TITLES[mode]}
|
title={TITLES[mode]}
|
||||||
@@ -36,6 +45,7 @@ export default function AuthRouteModal({ mode, onClose, onNavigate }: Props) {
|
|||||||
// in while browsing wants to carry on browsing rather than be moved to
|
// in while browsing wants to carry on browsing rather than be moved to
|
||||||
// their account page.
|
// their account page.
|
||||||
onSuccess={onClose}
|
onSuccess={onClose}
|
||||||
|
returnTo={returnTo}
|
||||||
/>
|
/>
|
||||||
</Modal>
|
</Modal>
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -0,0 +1,79 @@
|
|||||||
|
import Button from 'antd/es/button';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Google's own mark, inlined as SVG (#345).
|
||||||
|
*
|
||||||
|
* Their identity guidelines specify the four colours and the geometry, and a
|
||||||
|
* hand-drawn approximation of somebody else's trademark is a compliance problem
|
||||||
|
* rather than a style choice. These are the published values.
|
||||||
|
*
|
||||||
|
* Inlined rather than fetched, for the reason every other asset in this app is:
|
||||||
|
* a second origin is a second thing that can be down, blocked, or slow, and
|
||||||
|
* this one sits on the sign-in path.
|
||||||
|
*/
|
||||||
|
function GoogleMark() {
|
||||||
|
return (
|
||||||
|
<svg width="18" height="18" viewBox="0 0 18 18" aria-hidden="true" focusable="false">
|
||||||
|
<path
|
||||||
|
fill="#4285F4"
|
||||||
|
d="M17.64 9.2c0-.64-.06-1.25-.16-1.84H9v3.48h4.84a4.14 4.14 0 0 1-1.8 2.72v2.26h2.92c1.7-1.57 2.68-3.88 2.68-6.62z"
|
||||||
|
/>
|
||||||
|
<path
|
||||||
|
fill="#34A853"
|
||||||
|
d="M9 18c2.43 0 4.47-.8 5.96-2.18l-2.92-2.26c-.8.54-1.84.86-3.04.86-2.34 0-4.32-1.58-5.03-3.7H.96v2.34A9 9 0 0 0 9 18z"
|
||||||
|
/>
|
||||||
|
<path
|
||||||
|
fill="#FBBC05"
|
||||||
|
d="M3.97 10.72a5.4 5.4 0 0 1 0-3.44V4.94H.96a9 9 0 0 0 0 8.12l3.01-2.34z"
|
||||||
|
/>
|
||||||
|
<path
|
||||||
|
fill="#EA4335"
|
||||||
|
d="M9 3.58c1.32 0 2.5.45 3.44 1.35l2.58-2.59C13.46.9 11.43 0 9 0A9 9 0 0 0 .96 4.94l3.01 2.34C4.68 5.16 6.66 3.58 9 3.58z"
|
||||||
|
/>
|
||||||
|
</svg>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
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';
|
||||||
|
}>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Signing in with Google (#345).
|
||||||
|
*
|
||||||
|
* A navigation rather than a fetch, which is what makes this different from
|
||||||
|
* every other control on the auth form. The flow leaves this application
|
||||||
|
* entirely, so there is no promise to await and no error to catch here — the
|
||||||
|
* server's callback decides what happens and redirects accordingly.
|
||||||
|
*
|
||||||
|
* `returnTo` is sent as a query parameter and **validated on the server**, not
|
||||||
|
* 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, intent = 'sign-in' }: Props) {
|
||||||
|
return (
|
||||||
|
<Button
|
||||||
|
block
|
||||||
|
icon={<GoogleMark />}
|
||||||
|
onClick={() => {
|
||||||
|
// assign rather than the router: this is a full page departure to
|
||||||
|
// another origin, and react-router would try to match it as a route.
|
||||||
|
window.location.assign(`/api/auth/google/start?returnTo=${encodeURIComponent(returnTo)}`);
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
{intent === 'sign-up' ? 'Sign up with Google' : 'Sign in with Google'}
|
||||||
|
</Button>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -148,6 +148,10 @@ function AppRoutes() {
|
|||||||
// the storefront, so closing always lands somewhere real.
|
// the storefront, so closing always lands somewhere real.
|
||||||
const background = state?.background;
|
const background = state?.background;
|
||||||
const backdrop = modalPath ? background ?? { ...location, ...STOREFRONT_BACKDROP } : location;
|
const backdrop = modalPath ? background ?? { ...location, ...STOREFRONT_BACKDROP } : location;
|
||||||
|
// Where a Google sign-in should land the customer: the page behind the modal,
|
||||||
|
// not the modal's own path. Built here because the backdrop is only known
|
||||||
|
// here, and validated again on the server (#345).
|
||||||
|
const returnTo = `${backdrop.pathname}${backdrop.search ?? ''}`;
|
||||||
|
|
||||||
function closeModal() {
|
function closeModal() {
|
||||||
// Back, when there is somewhere to go back to, so closing the modal and
|
// Back, when there is somewhere to go back to, so closing the modal and
|
||||||
@@ -193,10 +197,10 @@ function AppRoutes() {
|
|||||||
{import.meta.env.DEV && <DevThrow scope="modal" />}
|
{import.meta.env.DEV && <DevThrow scope="modal" />}
|
||||||
{modalPath === '/account' && <Account onClose={closeModal} />}
|
{modalPath === '/account' && <Account onClose={closeModal} />}
|
||||||
{modalPath === '/login' && (
|
{modalPath === '/login' && (
|
||||||
<AuthRouteModal mode="login" onClose={closeModal} onNavigate={goWithinAuth} />
|
<AuthRouteModal mode="login" onClose={closeModal} onNavigate={goWithinAuth} returnTo={returnTo} />
|
||||||
)}
|
)}
|
||||||
{modalPath === '/register' && (
|
{modalPath === '/register' && (
|
||||||
<AuthRouteModal mode="register" onClose={closeModal} onNavigate={goWithinAuth} />
|
<AuthRouteModal mode="register" onClose={closeModal} onNavigate={goWithinAuth} returnTo={returnTo} />
|
||||||
)}
|
)}
|
||||||
{modalPath === '/forgot-password' && (
|
{modalPath === '/forgot-password' && (
|
||||||
<ForgotPassword onClose={closeModal} onBackToSignIn={() => goWithinAuth('/login')} />
|
<ForgotPassword onClose={closeModal} onBackToSignIn={() => goWithinAuth('/login')} />
|
||||||
|
|||||||
@@ -119,6 +119,47 @@ test.describe('Customer accounts', () => {
|
|||||||
await header.waitForSignedIn();
|
await header.waitForSignedIn();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #345. Local development has no Google credentials, and neither does QA
|
||||||
|
// until #313 moves it off a hostname whose domain nobody can prove they own.
|
||||||
|
// 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,
|
||||||
|
customer,
|
||||||
|
header
|
||||||
|
}) => {
|
||||||
|
await accountModal.openAndLogOut();
|
||||||
|
await expect(header.logInButton).toBeVisible();
|
||||||
|
await authModal.gotoLogIn();
|
||||||
|
|
||||||
|
const google = authModal.logInDialog.getByRole('button', { name: /Sign in with Google/i });
|
||||||
|
await expect(google).toHaveCount(0);
|
||||||
|
|
||||||
|
// And the password form is untouched by its absence, which is the whole
|
||||||
|
// reason the alternatives sit below it rather than above.
|
||||||
|
await authModal.logIn(customer.email, customer.password);
|
||||||
|
await header.waitForSignedIn();
|
||||||
|
});
|
||||||
|
|
||||||
test('rejects login with the wrong password', async ({ page, customer, accountModal, authModal, header }) => {
|
test('rejects login with the wrong password', async ({ page, customer, accountModal, authModal, header }) => {
|
||||||
await accountModal.openAndLogOut();
|
await accountModal.openAndLogOut();
|
||||||
await expect(header.logInButton).toBeVisible();
|
await expect(header.logInButton).toBeVisible();
|
||||||
|
|||||||
@@ -25,14 +25,18 @@ test.describe('Editing the customer emails', () => {
|
|||||||
'Favorited item sold',
|
'Favorited item sold',
|
||||||
'Favorited item withdrawn',
|
'Favorited item withdrawn',
|
||||||
'Cart reminder',
|
'Cart reminder',
|
||||||
'Email address changed'
|
'Email address changed',
|
||||||
|
// Added in #337, and the reason these are matched on the whole
|
||||||
|
// accessible name rather than as substrings: it extends the label above
|
||||||
|
// it, so an unanchored match resolved to both tabs.
|
||||||
|
'Email address changed by the shop'
|
||||||
]) {
|
]) {
|
||||||
await expect(adminEmails.railTab(new RegExp(label))).toBeVisible();
|
await expect(adminEmails.railTab(label)).toBeVisible();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Only a customised template is marked, so which ones have been changed is
|
// Only a customised template is marked, so which ones have been changed is
|
||||||
// visible without opening each one. An untouched template carries nothing.
|
// visible without opening each one. An untouched template carries nothing.
|
||||||
await expect(adminEmails.railTab(/Password reset/)).toBeVisible();
|
await expect(adminEmails.railTab('Password reset')).toBeVisible();
|
||||||
await expect(adminEmails.customisedTab('Password reset')).toHaveCount(0);
|
await expect(adminEmails.customisedTab('Password reset')).toHaveCount(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,29 @@
|
|||||||
import { FrameLocator, Locator, Page, expect } from '@playwright/test';
|
import { FrameLocator, Locator, Page, expect } from '@playwright/test';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A matcher for one template label, against a tab's *whole* accessible name.
|
||||||
|
*
|
||||||
|
* A tab is named for its template, plus the word "Customised" once it has been
|
||||||
|
* edited — the dot beside it carries that as an aria-label, so the state is not
|
||||||
|
* colour-only.
|
||||||
|
*
|
||||||
|
* Anchored at both ends, which is the entire point of this function. The
|
||||||
|
* locators here used to build an unanchored regex from the label, so a template
|
||||||
|
* whose name merely *began* with another's matched both. Adding "Email address
|
||||||
|
* changed by the shop" alongside "Email address changed" broke a passing test
|
||||||
|
* with a strict-mode violation naming the assertion rather than the new
|
||||||
|
* template — the same shape as the switch locator that silently retargeted in
|
||||||
|
* #317, and the same cost to diagnose.
|
||||||
|
*
|
||||||
|
* The escape matters for the same reason: these labels are copy, and copy
|
||||||
|
* acquires brackets and full stops eventually.
|
||||||
|
*/
|
||||||
|
function nameMatching(label: string, options: { customised?: boolean } = {}): RegExp {
|
||||||
|
const escaped = label.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||||
|
const suffix = options.customised ? '\\s+Customised' : '(?:\\s+Customised)?';
|
||||||
|
return new RegExp(`^${escaped}${suffix}$`);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The Emails tab: a vertical rail of template types and one editor at a time.
|
* The Emails tab: a vertical rail of template types and one editor at a time.
|
||||||
*
|
*
|
||||||
@@ -30,13 +54,30 @@ export class AdminEmails {
|
|||||||
return this.page.getByRole('button', { name: `Insert {{${name}}}` });
|
return this.page.getByRole('button', { name: `Insert {{${name}}}` });
|
||||||
}
|
}
|
||||||
|
|
||||||
/** One template's entry in the rail. */
|
/**
|
||||||
railTab(label: string | RegExp): Locator {
|
* One template's entry in the rail, matched on its whole accessible name.
|
||||||
return this.page.getByRole('tab', { name: label });
|
*
|
||||||
|
* A tab's accessible name is the template's label, plus the word "Customised"
|
||||||
|
* when it has been edited — the dot beside it carries that as an aria-label so
|
||||||
|
* the state is not colour-only.
|
||||||
|
*
|
||||||
|
* Anchored at both ends, which is the point of this helper rather than a bare
|
||||||
|
* substring match. These locators used to build an unanchored regex from the
|
||||||
|
* label, so a template whose name merely *began* with another's matched both.
|
||||||
|
* Adding "Email address changed by the shop" beside "Email address changed"
|
||||||
|
* broke a passing test with a strict-mode violation, and the failure named the
|
||||||
|
* assertion rather than the new template — the same shape as the switch
|
||||||
|
* locator that silently retargeted in #317.
|
||||||
|
*
|
||||||
|
* The escape matters for the same reason: a label is copy, and copy acquires
|
||||||
|
* brackets and full stops eventually.
|
||||||
|
*/
|
||||||
|
railTab(label: string): Locator {
|
||||||
|
return this.page.getByRole('tab', { name: nameMatching(label) });
|
||||||
}
|
}
|
||||||
|
|
||||||
customisedTab(label: string): Locator {
|
customisedTab(label: string): Locator {
|
||||||
return this.page.getByRole('tab', { name: new RegExp(`${label}.*Customised`) });
|
return this.page.getByRole('tab', { name: nameMatching(label, { customised: true }) });
|
||||||
}
|
}
|
||||||
|
|
||||||
subject(label: string): Locator {
|
subject(label: string): Locator {
|
||||||
@@ -65,7 +106,7 @@ export class AdminEmails {
|
|||||||
* resolved mid-swap finds the outgoing one.
|
* resolved mid-swap finds the outgoing one.
|
||||||
*/
|
*/
|
||||||
async openTemplate(label: string): Promise<void> {
|
async openTemplate(label: string): Promise<void> {
|
||||||
await this.railTab(new RegExp(label)).click();
|
await this.railTab(label).click();
|
||||||
await expect(this.subject(label)).toBeVisible();
|
await expect(this.subject(label)).toBeVisible();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user