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 passkeyLoginRouter from './routes/passkeyLogin';
|
||||
import googleAuthRouter from './routes/googleAuth';
|
||||
import { googleConfig } from './google/config';
|
||||
import publicRouter from './routes/public';
|
||||
import cartRouter from './routes/cart';
|
||||
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
|
||||
// browsing is ever reported, and there is no flag anyone can forget to
|
||||
// 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
|
||||
* 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. */
|
||||
|
||||
@@ -730,3 +730,54 @@ describe('GET /api/customers/me/identities', () => {
|
||||
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
|
||||
# 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.
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
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> {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useState } from 'react';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useSearchParams } from 'react-router-dom';
|
||||
import Form from 'antd/es/form';
|
||||
import Input from 'antd/es/input';
|
||||
@@ -10,6 +10,8 @@ import Typography from 'antd/es/typography';
|
||||
import Divider from 'antd/es/divider';
|
||||
import { registerCustomer, loginCustomer, signInWithPasskey, passkeysSupported } from './customerApi';
|
||||
import { useCustomerAuth } from './CustomerAuthContext';
|
||||
import GoogleSignInButton from './GoogleSignInButton';
|
||||
import { fetchConfig } from '../api';
|
||||
|
||||
const { Text } = Typography;
|
||||
|
||||
@@ -42,6 +44,15 @@ type Props = Readonly<{
|
||||
// route closes back to the page behind it, while the cart and favorite
|
||||
// prompts resume the action the customer was interrupted doing.
|
||||
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
|
||||
// prompt shown when a signed-out visitor adds to the cart — which had already
|
||||
// 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 notice = googleNotice(searchParams.get('auth'));
|
||||
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.
|
||||
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>) {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
@@ -208,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>
|
||||
)
|
||||
},
|
||||
@@ -238,11 +284,13 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
||||
works for everyone. Absent entirely where WebAuthn is not
|
||||
available, rather than shown disabled: a greyed button
|
||||
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 && (
|
||||
<>
|
||||
<Divider plain style={{ marginBlock: 16 }}>
|
||||
<Text type="secondary" style={{ fontSize: 12 }}>or</Text>
|
||||
</Divider>
|
||||
<Button
|
||||
block
|
||||
loading={passkeyLoading}
|
||||
@@ -258,6 +306,17 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
||||
</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>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -37,6 +37,12 @@ export default function AuthPromptModal({ open, onClose, onSuccess }: Props) {
|
||||
onClose();
|
||||
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}
|
||||
/>
|
||||
</Modal>
|
||||
|
||||
@@ -7,6 +7,15 @@ type Props = Readonly<{
|
||||
// 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.
|
||||
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> = {
|
||||
@@ -18,7 +27,7 @@ const TITLES: Record<AuthMode, string> = {
|
||||
// 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
|
||||
// bookmarks.
|
||||
export default function AuthRouteModal({ mode, onClose, onNavigate }: Props) {
|
||||
export default function AuthRouteModal({ mode, onClose, onNavigate, returnTo }: Props) {
|
||||
return (
|
||||
<Modal
|
||||
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
|
||||
// their account page.
|
||||
onSuccess={onClose}
|
||||
returnTo={returnTo}
|
||||
/>
|
||||
</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.
|
||||
const background = state?.background;
|
||||
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() {
|
||||
// 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" />}
|
||||
{modalPath === '/account' && <Account onClose={closeModal} />}
|
||||
{modalPath === '/login' && (
|
||||
<AuthRouteModal mode="login" onClose={closeModal} onNavigate={goWithinAuth} />
|
||||
<AuthRouteModal mode="login" onClose={closeModal} onNavigate={goWithinAuth} returnTo={returnTo} />
|
||||
)}
|
||||
{modalPath === '/register' && (
|
||||
<AuthRouteModal mode="register" onClose={closeModal} onNavigate={goWithinAuth} />
|
||||
<AuthRouteModal mode="register" onClose={closeModal} onNavigate={goWithinAuth} returnTo={returnTo} />
|
||||
)}
|
||||
{modalPath === '/forgot-password' && (
|
||||
<ForgotPassword onClose={closeModal} onBackToSignIn={() => goWithinAuth('/login')} />
|
||||
|
||||
@@ -119,6 +119,47 @@ test.describe('Customer accounts', () => {
|
||||
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 }) => {
|
||||
await accountModal.openAndLogOut();
|
||||
await expect(header.logInButton).toBeVisible();
|
||||
|
||||
@@ -25,14 +25,18 @@ test.describe('Editing the customer emails', () => {
|
||||
'Favorited item sold',
|
||||
'Favorited item withdrawn',
|
||||
'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
|
||||
// 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);
|
||||
});
|
||||
|
||||
|
||||
@@ -1,5 +1,29 @@
|
||||
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.
|
||||
*
|
||||
@@ -30,13 +54,30 @@ export class AdminEmails {
|
||||
return this.page.getByRole('button', { name: `Insert {{${name}}}` });
|
||||
}
|
||||
|
||||
/** One template's entry in the rail. */
|
||||
railTab(label: string | RegExp): Locator {
|
||||
return this.page.getByRole('tab', { name: label });
|
||||
/**
|
||||
* One template's entry in the rail, matched on its whole accessible name.
|
||||
*
|
||||
* 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 {
|
||||
return this.page.getByRole('tab', { name: new RegExp(`${label}.*Customised`) });
|
||||
return this.page.getByRole('tab', { name: nameMatching(label, { customised: true }) });
|
||||
}
|
||||
|
||||
subject(label: string): Locator {
|
||||
@@ -65,7 +106,7 @@ export class AdminEmails {
|
||||
* resolved mid-swap finds the outgoing one.
|
||||
*/
|
||||
async openTemplate(label: string): Promise<void> {
|
||||
await this.railTab(new RegExp(label)).click();
|
||||
await this.railTab(label).click();
|
||||
await expect(this.subject(label)).toBeVisible();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user