diff --git a/backend/src/app.ts b/backend/src/app.ts index 9f06e67..469b3c3 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -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 }); }); diff --git a/backend/tests/integration/googleSignIn.integration.test.ts b/backend/tests/integration/googleSignIn.integration.test.ts index de0bae2..0c7f16f 100644 --- a/backend/tests/integration/googleSignIn.integration.test.ts +++ b/backend/tests/integration/googleSignIn.integration.test.ts @@ -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'); + }); +}); diff --git a/docs/ops/google-sign-in.md b/docs/ops/google-sign-in.md new file mode 100644 index 0000000..ad603f8 --- /dev/null +++ b/docs/ops/google-sign-in.md @@ -0,0 +1,106 @@ +# 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 | + +## The constraint that shapes everything + +**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. + +`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 | +| --- | --- | +| 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 | + +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. + +`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. + +## 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. + +## 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: + `https://qa.redefined-designs.com/api/auth/google/callback` +4. 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**. + Do it once the domain resolves, so the home page and privacy links Google + shows actually answer. + +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. diff --git a/frontend/src/api.ts b/frontend/src/api.ts index 1463440..8a92414 100755 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -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 { diff --git a/frontend/src/customer/AuthForm.tsx b/frontend/src/customer/AuthForm.tsx index b70a598..b357f35 100644 --- a/frontend/src/customer/AuthForm.tsx +++ b/frontend/src/customer/AuthForm.tsx @@ -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(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) { setLoading(true); setError(null); @@ -238,11 +263,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) && ( + + or + + )} {canUsePasskeys && ( <> - - or - + ); +} diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 87e6142..5b48b6e 100755 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -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 && } {modalPath === '/account' && } {modalPath === '/login' && ( - + )} {modalPath === '/register' && ( - + )} {modalPath === '/forgot-password' && ( goWithinAuth('/login')} /> diff --git a/frontend/tests/e2e/auth.spec.ts b/frontend/tests/e2e/auth.spec.ts index e6f51db..2cd733d 100755 --- a/frontend/tests/e2e/auth.spec.ts +++ b/frontend/tests/e2e/auth.spec.ts @@ -119,6 +119,30 @@ 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. + 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();