1 Commits
Author SHA1 Message Date
synAdminandClaude Opus 5 47dbd3ec65 fix(auth): offer Google on the sign-up tab, not only on Log In (#345)
SonarQube Analysis / sonarqube (pull_request) Canceled after 7s
Linting / lint (pull_request) Canceled after 0s
The button was rendered inside the Log In tab's form only, so a visitor on Create Account saw no social option at all. Reported from a local run.

The mistake came from following the passkey button too closely. A passkey belongs only on Log In, and correctly so: you cannot register an account with one, since registration requires an account to register it against. Google is the opposite case. Creating an account is precisely what a new customer reaches for it to do, so leaving it off the sign-up tab hid the feature from the people it helps most — and hid it on the tab the modal opens on by default.

The label differs by tab and nothing else does. One endpoint serves both: it signs in a known identity, links a verified address, or creates an account, and the customer neither knows nor cares which will happen. So the wording matches what they came to that tab to do rather than what the server ends up doing. Both spellings are given in Google's identity guidelines alongside the mark.

The two consent checkboxes above it are deliberately 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 and through the same endpoints, on the step they land on afterwards — which is what #342 built that step for.

The end-to-end test asserts absence, like the one beside it, because local and QA have no credentials and absence is the behaviour that actually runs there.

Verified: frontend tsc, lint and build clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-11 10:01:26 -05:00
5 changed files with 65 additions and 153 deletions
+17 -22
View File
@@ -14,32 +14,27 @@
* source, and it is the one that is already correct in any environment where
* mail works.
*
* ## Every environment needs its own console entry
* ## The consequence worth stating plainly
*
* 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.
* 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.
*
* | 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` |
* `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.
*
* 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.
* | 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 |
*
* 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.
* 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.
*/
/** The callback path. One constant, because it appears in two sentences. */
+12 -26
View File
@@ -63,12 +63,6 @@
# 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
@@ -201,28 +195,20 @@ services:
# rotating it revokes every outstanding link, which is the intended way to
# deal with a leak.
- INTAKE_ACTION_SECRET=${QA_INTAKE_ACTION_SECRET:-}
# 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.
# Deliberately left empty, and it is not an oversight (#340, #345).
#
# 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.
# 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.
#
# 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:-}
# 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=
volumes:
# Separate uploads directory. Sharing production's would let a QA run
# write into, and a QA teardown delete, real product images.
+28 -52
View File
@@ -18,42 +18,33 @@ screen, so what appears there is the production identity even while testing.
| Data Access | Exactly `openid`, `email`, `profile` |
| Verification Center | Nothing to submit, and it should stay that way |
## Redirect URIs, one per environment
## The constraint that shapes everything
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.
**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.
| Environment | Redirect URI |
`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, 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, on `localhost` | Works, by exemption |
| QA on the Synology hostname | **Impossible**, not merely unconfigured |
| QA on `qa.redefined-designs.com` | Works, after the cutover |
| Production on `redefined-designs.com` | Works, after the cutover |
Local 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.
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.
### 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.
`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
@@ -69,35 +60,20 @@ 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:
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`
3. Confirm the production callback is registered:
4. 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**.
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.
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.
+3 -7
View File
@@ -25,18 +25,14 @@ test.describe('Editing the customer emails', () => {
'Favorited item sold',
'Favorited item withdrawn',
'Cart reminder',
'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'
'Email address changed'
]) {
await expect(adminEmails.railTab(label)).toBeVisible();
await expect(adminEmails.railTab(new RegExp(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);
});
+5 -46
View File
@@ -1,29 +1,5 @@
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.
*
@@ -54,30 +30,13 @@ export class AdminEmails {
return this.page.getByRole('button', { name: `Insert {{${name}}}` });
}
/**
* 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) });
/** One template's entry in the rail. */
railTab(label: string | RegExp): Locator {
return this.page.getByRole('tab', { name: label });
}
customisedTab(label: string): Locator {
return this.page.getByRole('tab', { name: nameMatching(label, { customised: true }) });
return this.page.getByRole('tab', { name: new RegExp(`${label}.*Customised`) });
}
subject(label: string): Locator {
@@ -106,7 +65,7 @@ export class AdminEmails {
* resolved mid-swap finds the outgoing one.
*/
async openTemplate(label: string): Promise<void> {
await this.railTab(label).click();
await this.railTab(new RegExp(label)).click();
await expect(this.subject(label)).toBeVisible();
}
}