Compare commits
21
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1838bb38d1 | ||
|
|
0361ef35d0 | ||
|
|
f5a29127fb | ||
|
|
c9dccfe4a2 | ||
|
|
903a1d8b76 | ||
|
|
838749df64 | ||
|
|
3958bda489 | ||
|
|
b82b989cc8 | ||
|
|
c2ee0b0c5d | ||
|
|
843c51dd91 | ||
|
|
2c6ac4d2be | ||
|
|
0014a8db8a | ||
|
|
dcb3c7c91b | ||
|
|
c5014d5342 | ||
|
|
44df0bd2d9 | ||
|
|
8a5f6eb08c | ||
|
|
25078417e5 | ||
|
|
79a2b606ea | ||
|
|
022ab8edcf | ||
|
|
9177b54dea | ||
|
|
87f07baaff |
+16
-1
@@ -19,6 +19,8 @@ import filtersRouter from './routes/filters';
|
||||
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';
|
||||
@@ -71,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
|
||||
});
|
||||
});
|
||||
|
||||
@@ -105,6 +116,10 @@ app.use('/api/customers/me/passkeys', passkeysRouter);
|
||||
// Unauthenticated, unlike the router above: this is how a customer becomes
|
||||
// signed in, so it cannot sit behind requireCustomer (#39).
|
||||
app.use('/api/customers/passkeys', passkeyLoginRouter);
|
||||
// Its own prefix rather than under /api/customers: this is the one route a
|
||||
// third party redirects a browser into, and the callback path is registered
|
||||
// verbatim in Google's console (#341).
|
||||
app.use('/api/auth/google', googleAuthRouter);
|
||||
app.use('/api/customers', customersRouter);
|
||||
app.use('/api/client-errors', clientErrorsRouter);
|
||||
app.use('/', publicRouter);
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
import { pool } from '../db';
|
||||
import type { GoogleIdentity } from './oauth';
|
||||
|
||||
/**
|
||||
* Joining a Google identity to an account that already exists (#343).
|
||||
*
|
||||
* The smallest module in this feature and the one to read most carefully. It is
|
||||
* the point where somebody who has proved nothing to *this* shop is handed an
|
||||
* account that belongs to somebody who did.
|
||||
*
|
||||
* ## The rule, and why it is defensible
|
||||
*
|
||||
* Link only when Google asserts `email_verified` and the address matches an
|
||||
* existing customer exactly. Refuse otherwise.
|
||||
*
|
||||
* Google asserting the address means whoever completed that sign-in
|
||||
* demonstrably controls the mailbox. That mailbox is already the root of trust
|
||||
* for every other route into the account: it is where a password reset goes,
|
||||
* and following a reset link is enough to take the account over completely. So
|
||||
* linking on it grants nothing that was not already reachable, and it spares
|
||||
* the customer who came to Google precisely because they forgot the password.
|
||||
*
|
||||
* **Never link on an unverified address.** That is not a degraded version of the
|
||||
* same thing — it is an account takeover with extra steps, since the assertion
|
||||
* would be one nobody has checked. It is why this is a written rule rather than
|
||||
* a default that arrived with a library.
|
||||
*
|
||||
* ## Why the identity lookup happens before any of this
|
||||
*
|
||||
* The caller matches on `(provider, provider_sub)` first, and only reaches here
|
||||
* when that finds nothing. An identity that has signed in before keeps working
|
||||
* even if the address on either side has since changed, which is the whole
|
||||
* reason the subject claim is what gets stored.
|
||||
*/
|
||||
|
||||
export type LinkOutcome =
|
||||
| { kind: 'linked'; customerId: number }
|
||||
/** Google did not vouch for the address, or nothing matched it. */
|
||||
| { kind: 'refused' };
|
||||
|
||||
interface CustomerRow {
|
||||
id: number;
|
||||
disabled_at: Date | null;
|
||||
}
|
||||
|
||||
export async function linkToExistingCustomer(identity: GoogleIdentity): Promise<LinkOutcome> {
|
||||
// The first thing checked, and it is the whole policy. Everything below is
|
||||
// bookkeeping; this line is the security.
|
||||
if (!identity.emailVerified) return { kind: 'refused' };
|
||||
|
||||
const { rows } = await pool.query<CustomerRow>(
|
||||
// Compared exactly, against an address the caller has already lowercased
|
||||
// and trimmed the way registration does. A stricter comparison here would
|
||||
// silently fail to match and produce a second account for one person
|
||||
// instead of an error anybody sees.
|
||||
`SELECT id, disabled_at FROM customers WHERE email = $1`,
|
||||
[identity.email]
|
||||
);
|
||||
const customer = rows[0];
|
||||
if (!customer) return { kind: 'refused' };
|
||||
|
||||
// Refused here as well as at sign-in. Linking to a disabled account and then
|
||||
// refusing the session would leave the identity attached, so the next attempt
|
||||
// would take the sign-in path instead — turning a disabled account into one
|
||||
// that is merely inconvenient to reach.
|
||||
if (customer.disabled_at !== null) return { kind: 'refused' };
|
||||
|
||||
await pool.query(
|
||||
`INSERT INTO customer_identities (customer_id, provider, provider_sub, last_used_at)
|
||||
VALUES ($1, 'google', $2, now())
|
||||
ON CONFLICT (provider, provider_sub) DO NOTHING`,
|
||||
[customer.id, identity.sub]
|
||||
);
|
||||
|
||||
return { kind: 'linked', customerId: customer.id };
|
||||
}
|
||||
@@ -0,0 +1,106 @@
|
||||
import crypto from 'node:crypto';
|
||||
import { pool } from '../db';
|
||||
import type { GoogleIdentity } from './oauth';
|
||||
|
||||
/**
|
||||
* Creating a customer from a Google identity (#342).
|
||||
*
|
||||
* ## What this deliberately does not decide
|
||||
*
|
||||
* It reports `email-taken` when the address already belongs to a customer, and
|
||||
* stops there. Whether to join those two accounts is linking, which is the most
|
||||
* security-sensitive decision in this project and lives in `linkIdentity.ts`
|
||||
* (#343). Deciding it here would mean an account is handed over as a side
|
||||
* effect of an INSERT failing, which is exactly the shape that decision must
|
||||
* never take.
|
||||
*
|
||||
* ## Consent, which is the actual problem in this issue
|
||||
*
|
||||
* Registration captures two consents and stores their wording verbatim, and
|
||||
* marketing consent must start unticked (#56). A customer arriving through
|
||||
* Google has never seen those checkboxes and **cannot have**: the redirect to
|
||||
* Google happens before anyone knows whether they are new.
|
||||
*
|
||||
* So the account is created with both false and no stored wording, which is
|
||||
* legally correct — nobody has agreed to anything, and nothing is recorded as
|
||||
* though they had. What makes it honest rather than merely lawful is that the
|
||||
* customer is then asked, on a step that shows the same two sentences, through
|
||||
* the same endpoints registration uses. That is what keeps the stored text
|
||||
* byte-identical, which is the whole point of storing it.
|
||||
*
|
||||
* Skipping that step is allowed and leaves both false. A consent nobody gave is
|
||||
* the correct default and a perfectly fine resting state.
|
||||
*/
|
||||
|
||||
export type SignUpOutcome =
|
||||
| { kind: 'created'; customerId: number }
|
||||
/** The address is already an account's. #343 decides whether to link. */
|
||||
| { kind: 'email-taken' };
|
||||
|
||||
interface IdRow {
|
||||
id: number;
|
||||
}
|
||||
|
||||
export async function createCustomerFromGoogle(identity: GoogleIdentity): Promise<SignUpOutcome> {
|
||||
const client = await pool.connect();
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
|
||||
const { rows: existing } = await client.query<IdRow>(
|
||||
`SELECT id FROM customers WHERE email = $1`,
|
||||
[identity.email]
|
||||
);
|
||||
if (existing.length) {
|
||||
await client.query('ROLLBACK');
|
||||
return { kind: 'email-taken' };
|
||||
}
|
||||
|
||||
const { rows } = await client.query<IdRow>(
|
||||
`INSERT INTO customers (email, password_hash, first_name, last_name, email_verified, unsubscribe_token)
|
||||
VALUES ($1, NULL, $2, $3, $4, $5)
|
||||
RETURNING id`,
|
||||
[
|
||||
identity.email,
|
||||
// Hints rather than requirements. Registration demands both names
|
||||
// because every email greets by first name, but Google may return
|
||||
// neither and refusing the sign-in over it would be absurd — the
|
||||
// greeting already has a fallback for exactly this.
|
||||
identity.firstName,
|
||||
identity.lastName,
|
||||
// Only on Google's word, never assumed. An unverified assertion is
|
||||
// worth nothing, and the caller sends the usual confirmation email when
|
||||
// this is false.
|
||||
identity.emailVerified,
|
||||
crypto.randomBytes(16).toString('hex')
|
||||
]
|
||||
);
|
||||
// The INSERT above has a RETURNING clause, so no row means the statement
|
||||
// did not do what it says.
|
||||
const customer = rows[0];
|
||||
if (!customer) throw new Error('the customer INSERT returned no row');
|
||||
|
||||
// In the same transaction, deliberately. A customer row with no identity is
|
||||
// an account nobody can sign in to and nobody can recover, because it has
|
||||
// no password either — the worst possible thing to leave behind.
|
||||
await client.query(
|
||||
`INSERT INTO customer_identities (customer_id, provider, provider_sub, last_used_at)
|
||||
VALUES ($1, 'google', $2, now())`,
|
||||
[customer.id, identity.sub]
|
||||
);
|
||||
|
||||
await client.query('COMMIT');
|
||||
return { kind: 'created', customerId: customer.id };
|
||||
} catch (err) {
|
||||
await client.query('ROLLBACK');
|
||||
// Two sign-ins racing for the same brand-new address. The SELECT above
|
||||
// cannot see the other transaction's uncommitted row, so the unique index
|
||||
// is what actually holds — and losing that race means the account now
|
||||
// exists, which is 'email-taken' rather than an error.
|
||||
if ((err as { code?: string }).code === '23505') {
|
||||
return { kind: 'email-taken' };
|
||||
}
|
||||
throw err;
|
||||
} finally {
|
||||
client.release();
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,254 @@
|
||||
import crypto from 'node:crypto';
|
||||
import type { GoogleConfig } from './config';
|
||||
|
||||
/**
|
||||
* The OpenID Connect authorization code flow, as far as Google implements it (#341).
|
||||
*
|
||||
* ## Why the code flow, and not the one with a token in the browser
|
||||
*
|
||||
* The browser is sent to Google, comes back carrying a code, and *this server*
|
||||
* exchanges that code for tokens over its own TLS connection. The customer's
|
||||
* browser never holds a token, so nothing that can read the page can steal one.
|
||||
*
|
||||
* PKCE goes in as well, even though this is a confidential client that holds a
|
||||
* secret. It costs one hash and it closes code interception outright rather
|
||||
* than resting the whole flow on the secret staying secret.
|
||||
*
|
||||
* ## Why there is no JWKS fetch here
|
||||
*
|
||||
* The id token arrives on a direct TLS connection to Google's token endpoint,
|
||||
* in the response to a request this server made. OpenID Connect Core §3.1.3.7
|
||||
* says signature verification MAY be skipped in exactly that case, because TLS
|
||||
* has already established who answered and that nothing altered the reply.
|
||||
*
|
||||
* That removes a key fetch, a cache and a rotation path from the auth code,
|
||||
* which is a real saving in the place least worth having moving parts. It
|
||||
* removes none of the claim checks: those are what stop a token minted for
|
||||
* another application, or for another attempt, being accepted here. See
|
||||
* `verifiedIdentity`, where every one of them is enforced and none is optional.
|
||||
*
|
||||
* The moment an id token reaches this code from anywhere other than that
|
||||
* response — a redirect fragment, a request body, a header — this reasoning
|
||||
* stops holding and signature verification becomes mandatory. Nothing does that
|
||||
* today, and nothing should.
|
||||
*/
|
||||
|
||||
const AUTH_ENDPOINT = 'https://accounts.google.com/o/oauth2/v2/auth';
|
||||
const TOKEN_ENDPOINT = 'https://oauth2.googleapis.com/token';
|
||||
|
||||
/**
|
||||
* The only scopes this asks for, and the reason publishing needs no review.
|
||||
*
|
||||
* `openid` produces the id token, `email` carries the address and the
|
||||
* `email_verified` flag the linking policy turns on, and `profile` carries the
|
||||
* names used when an account is created. All three are non-sensitive; adding a
|
||||
* sensitive one turns publishing into a verification review with a video
|
||||
* walkthrough and a wait measured in weeks.
|
||||
*/
|
||||
const SCOPES = 'openid email profile';
|
||||
|
||||
/**
|
||||
* Both spellings Google issues for the issuer claim.
|
||||
*
|
||||
* It really does use both, and accepting only one produces sign-ins that fail
|
||||
* for some customers and not others — which is about the least diagnosable
|
||||
* failure this flow can have.
|
||||
*/
|
||||
const ISSUERS = new Set(['https://accounts.google.com', 'accounts.google.com']);
|
||||
|
||||
/** A little slack for clock skew between this host and Google. */
|
||||
const CLOCK_SKEW_SECONDS = 60;
|
||||
|
||||
/** Who Google says signed in. Everything here has been checked. */
|
||||
export interface GoogleIdentity {
|
||||
/** The subject claim: opaque, stable, and the only safe identifier. */
|
||||
sub: string;
|
||||
email: string;
|
||||
/** Whether Google asserts the address. The linking policy turns on this. */
|
||||
emailVerified: boolean;
|
||||
firstName: string | null;
|
||||
lastName: string | null;
|
||||
}
|
||||
|
||||
/** The claims this cares about. Google sends more; none of it is wanted. */
|
||||
interface IdTokenClaims {
|
||||
iss?: unknown;
|
||||
aud?: unknown;
|
||||
exp?: unknown;
|
||||
sub?: unknown;
|
||||
nonce?: unknown;
|
||||
email?: unknown;
|
||||
email_verified?: unknown;
|
||||
given_name?: unknown;
|
||||
family_name?: unknown;
|
||||
}
|
||||
|
||||
/** One attempt's secrets, minted at the start and spent at the callback. */
|
||||
export interface AttemptSecrets {
|
||||
state: string;
|
||||
nonce: string;
|
||||
codeVerifier: string;
|
||||
}
|
||||
|
||||
function randomToken(): string {
|
||||
return crypto.randomBytes(32).toString('base64url');
|
||||
}
|
||||
|
||||
/**
|
||||
* Fresh secrets for one sign-in attempt.
|
||||
*
|
||||
* `state` proves the callback belongs to the request this browser started.
|
||||
* `nonce` is echoed inside the id token and proves the token was minted for
|
||||
* this attempt rather than replayed from another. `codeVerifier` is PKCE.
|
||||
*
|
||||
* Three separate values rather than one reused three times: they are checked by
|
||||
* different parties at different moments, and a single value would mean
|
||||
* anything that learned it from one check could satisfy the others.
|
||||
*/
|
||||
export function newAttempt(): AttemptSecrets {
|
||||
return { state: randomToken(), nonce: randomToken(), codeVerifier: randomToken() };
|
||||
}
|
||||
|
||||
/** The S256 challenge for a verifier. Google supports S256; plain is not offered. */
|
||||
export function codeChallenge(verifier: string): string {
|
||||
return crypto.createHash('sha256').update(verifier).digest('base64url');
|
||||
}
|
||||
|
||||
/** Where to send the browser to begin. */
|
||||
export function authorizationUrl(config: GoogleConfig, attempt: AttemptSecrets): string {
|
||||
const url = new URL(AUTH_ENDPOINT);
|
||||
url.searchParams.set('client_id', config.clientId);
|
||||
url.searchParams.set('redirect_uri', config.redirectUri);
|
||||
url.searchParams.set('response_type', 'code');
|
||||
url.searchParams.set('scope', SCOPES);
|
||||
url.searchParams.set('state', attempt.state);
|
||||
url.searchParams.set('nonce', attempt.nonce);
|
||||
url.searchParams.set('code_challenge', codeChallenge(attempt.codeVerifier));
|
||||
url.searchParams.set('code_challenge_method', 'S256');
|
||||
// No `access_type=offline` and no `prompt=consent`, deliberately. Those ask
|
||||
// for a refresh token, and Google is being used to answer one question once —
|
||||
// a stored refresh token would be a long-lived credential with nothing to
|
||||
// spend it on and everything to lose if it leaked.
|
||||
return url.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Trades the code for an id token.
|
||||
*
|
||||
* Returns the raw token rather than parsed claims, so the exchange and the
|
||||
* checking stay separable: the checking is pure and can be tested exhaustively
|
||||
* without a network, which is where the security actually lives.
|
||||
*/
|
||||
export async function exchangeCode(config: GoogleConfig, code: string, codeVerifier: string): Promise<string> {
|
||||
const response = await fetch(TOKEN_ENDPOINT, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: new URLSearchParams({
|
||||
code,
|
||||
client_id: config.clientId,
|
||||
client_secret: config.clientSecret,
|
||||
redirect_uri: config.redirectUri,
|
||||
grant_type: 'authorization_code',
|
||||
code_verifier: codeVerifier
|
||||
})
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
// Logged rather than returned. The body names the client id and can carry
|
||||
// the secret back in an error description, and none of it means anything to
|
||||
// the customer.
|
||||
const detail = await response.text().catch(() => '');
|
||||
console.warn(`[google] token exchange failed: ${response.status} ${detail.slice(0, 300)}`);
|
||||
throw new Error('the Google token exchange was refused');
|
||||
}
|
||||
|
||||
const body = (await response.json()) as { id_token?: unknown };
|
||||
if (typeof body.id_token !== 'string' || body.id_token === '') {
|
||||
throw new Error('Google returned no id token');
|
||||
}
|
||||
return body.id_token;
|
||||
}
|
||||
|
||||
/**
|
||||
* The claims inside an id token, without verifying its signature.
|
||||
*
|
||||
* Named for what it does. Anywhere the token has not come straight back from
|
||||
* the token endpoint over TLS, this function is the wrong one to call, and the
|
||||
* name is meant to make that obvious at the call site.
|
||||
*/
|
||||
function decodeClaims(idToken: string): IdTokenClaims {
|
||||
const [, payload] = idToken.split('.');
|
||||
if (!payload) throw new Error('the id token is not a JWT');
|
||||
try {
|
||||
return JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')) as IdTokenClaims;
|
||||
} catch {
|
||||
throw new Error('the id token payload is not JSON');
|
||||
}
|
||||
}
|
||||
|
||||
function asString(value: unknown): string | null {
|
||||
return typeof value === 'string' && value !== '' ? value : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Who signed in, or a thrown error saying which check failed.
|
||||
*
|
||||
* Every check here is mandatory, and each one closes something specific:
|
||||
*
|
||||
* | Claim | What accepting it blindly would allow |
|
||||
* | --- | --- |
|
||||
* | `iss` | A token from an issuer we never chose to trust |
|
||||
* | `aud` | A token minted for a different application, replayed here |
|
||||
* | `exp` | A token captured once and reused indefinitely |
|
||||
* | `nonce` | A token from an earlier attempt, replayed into this one |
|
||||
* | `sub` | An identity row keyed on nothing |
|
||||
*
|
||||
* The messages name the failing check because they are logged, never shown. A
|
||||
* customer sees one refusal for every cause, exactly as the passkey path does.
|
||||
*/
|
||||
export function verifiedIdentity(
|
||||
idToken: string,
|
||||
expected: { clientId: string; nonce: string },
|
||||
now: Date = new Date()
|
||||
): GoogleIdentity {
|
||||
const claims = decodeClaims(idToken);
|
||||
|
||||
if (typeof claims.iss !== 'string' || !ISSUERS.has(claims.iss)) {
|
||||
throw new Error(`unexpected issuer: ${String(claims.iss)}`);
|
||||
}
|
||||
if (claims.aud !== expected.clientId) {
|
||||
throw new Error('the id token was minted for a different client');
|
||||
}
|
||||
|
||||
const exp = typeof claims.exp === 'number' ? claims.exp : NaN;
|
||||
if (!Number.isFinite(exp)) throw new Error('the id token has no expiry');
|
||||
if (exp + CLOCK_SKEW_SECONDS < Math.floor(now.getTime() / 1000)) {
|
||||
throw new Error('the id token has expired');
|
||||
}
|
||||
|
||||
// Compared in constant time. The nonce is a secret this server minted, and a
|
||||
// byte-by-byte comparison that stops early is a timing oracle for it.
|
||||
const nonce = asString(claims.nonce) ?? '';
|
||||
const supplied = Buffer.from(nonce);
|
||||
const wanted = Buffer.from(expected.nonce);
|
||||
if (supplied.length !== wanted.length || !crypto.timingSafeEqual(supplied, wanted)) {
|
||||
throw new Error('the id token belongs to a different sign-in attempt');
|
||||
}
|
||||
|
||||
const sub = asString(claims.sub);
|
||||
if (sub === null) throw new Error('the id token carries no subject');
|
||||
|
||||
const email = asString(claims.email);
|
||||
if (email === null) throw new Error('the id token carries no email');
|
||||
|
||||
return {
|
||||
sub,
|
||||
email: email.toLowerCase().trim(),
|
||||
// Strictly true, never merely truthy. Google sends a boolean, and treating
|
||||
// the string "false" as a verified address is the exact mistake that turns
|
||||
// the linking policy into an account-takeover path.
|
||||
emailVerified: claims.email_verified === true,
|
||||
firstName: asString(claims.given_name),
|
||||
lastName: asString(claims.family_name)
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* Where a customer is sent back to after signing in with Google (#341).
|
||||
*
|
||||
* An OAuth flow leaves this application entirely and comes back, so where the
|
||||
* customer was has to survive the round trip — and the value doing that is one
|
||||
* an attacker can propose, by handing somebody a link to our own start route
|
||||
* with their destination attached.
|
||||
*
|
||||
* Unchecked, that makes the start route an open redirect wearing a sign-in flow
|
||||
* as a disguise: a link on our real domain, with our real certificate, that
|
||||
* deposits the customer somewhere else entirely. It is precisely the shape a
|
||||
* credible phishing page wants, and it is worth more to an attacker than most
|
||||
* bugs in the flow it hides behind.
|
||||
*
|
||||
* Its own module rather than a helper inside the route, so it can be tested
|
||||
* without a database connection and so the next path that needs the same
|
||||
* question has somewhere obvious to ask it.
|
||||
*/
|
||||
|
||||
/** Where anyone goes when the answer is "not that". */
|
||||
export const DEFAULT_RETURN_TO = '/';
|
||||
|
||||
/**
|
||||
* A path inside this site, or the home page.
|
||||
*
|
||||
* Everything that is not plainly a local path is replaced rather than rejected.
|
||||
* A refusal would mean a customer who signed in successfully sees an error
|
||||
* about a query parameter they never typed, which helps nobody — the storefront
|
||||
* is a fine place to land.
|
||||
*
|
||||
* The cases worth naming, because each is a way of writing "somewhere else"
|
||||
* that still starts with a slash or looks like it might:
|
||||
*
|
||||
* - `//evil.test` is protocol-relative, and browsers treat it as absolute
|
||||
* - `/\evil.test` is treated as protocol-relative by several browsers
|
||||
* - `https://evil.test` does not start with a slash at all
|
||||
* - a backslash anywhere in the authority position is normalised to a slash
|
||||
*/
|
||||
export function safeReturnTo(value: unknown): string {
|
||||
if (typeof value !== 'string' || value === '') return DEFAULT_RETURN_TO;
|
||||
if (!value.startsWith('/')) return DEFAULT_RETURN_TO;
|
||||
// Both slashes, because browsers disagree about which they normalise.
|
||||
if (value.startsWith('//') || value.startsWith('/\\')) return DEFAULT_RETURN_TO;
|
||||
// A control character can truncate or split the Location header a browser
|
||||
// reads. Checked by code point rather than by a regex, because a regex that
|
||||
// matches control characters trips a lint rule existing for good reasons of
|
||||
// its own, and this is clearer than an exemption from it.
|
||||
for (const character of value) {
|
||||
const code = character.codePointAt(0) ?? 0;
|
||||
if (code < 0x20 || code === 0x7f) return DEFAULT_RETURN_TO;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
@@ -184,3 +184,26 @@ export const intakeSubmitLimiter = rateLimit({
|
||||
legacyHeaders: false,
|
||||
message: { error: 'too many submissions — please try again later' }
|
||||
});
|
||||
|
||||
/**
|
||||
* Starting a Google sign-in (#341).
|
||||
*
|
||||
* The route mints three secrets and issues a redirect, which is cheap but not
|
||||
* free, and it is reachable without a session by anyone who knows the URL.
|
||||
*
|
||||
* Generous, because a customer who bounces off Google's consent screen and
|
||||
* tries again is doing something entirely reasonable and must never be told to
|
||||
* wait. The limit exists so a loop cannot spend the server's entropy and fill
|
||||
* the log, not to police customers.
|
||||
*
|
||||
* Keyed on the caller alone: this endpoint carries no email, which is the
|
||||
* distinction the comment on the client-error limiter above draws.
|
||||
*/
|
||||
export const googleSignInLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
limit: 60,
|
||||
keyGenerator: keyByCaller,
|
||||
standardHeaders: 'draft-7',
|
||||
legacyHeaders: false,
|
||||
message: { error: 'too many sign-in attempts — please try again shortly' }
|
||||
});
|
||||
|
||||
@@ -181,6 +181,13 @@ function publicCustomer(c: CustomerRecord) {
|
||||
// neither, and the UI has to be able to show that honestly.
|
||||
analytics_consent: analyticsConsent(c),
|
||||
favorite_alerts: c.favorite_alerts,
|
||||
// Whether, not what (#344). A customer who signed up with Google has none,
|
||||
// and the account page has to be able to say so — offering "change your
|
||||
// password" to somebody who has never had one is a dead end, and saying
|
||||
// nothing leaves them unable to see a credential they are entitled to
|
||||
// manage. A boolean is the whole of what the UI needs, and the hash itself
|
||||
// must never leave this function.
|
||||
has_password: c.password_hash !== null,
|
||||
created_at: c.created_at
|
||||
};
|
||||
}
|
||||
@@ -519,9 +526,26 @@ router.post('/change-password', requireCustomer, asyncRoute(async (req: Request,
|
||||
}
|
||||
const { rows } = await pool.query<CustomerRecord>(`SELECT * FROM customers WHERE id = $1`, [req.customerId]);
|
||||
const customer = requireRow(rows, 'the signed-in customer');
|
||||
if (!(await passwordMatches(currentPassword, customer.password_hash))) {
|
||||
return res.status(401).json({ error: 'current password is incorrect' });
|
||||
|
||||
// Setting the first password and changing an existing one, in one route
|
||||
// rather than two (#344).
|
||||
//
|
||||
// A customer who signed up with Google has no password, so there is nothing
|
||||
// to compare against and asking for one would be a dead end — they cannot
|
||||
// supply a value that was never set. What authorises the change is the
|
||||
// session they are already holding, which is the same thing that authorises
|
||||
// every other setting on the account page.
|
||||
//
|
||||
// One route because two would be two places to get the guard wrong, and the
|
||||
// one that would be forgotten is whichever is not on the path exercised by
|
||||
// hand. The branch is on the stored hash rather than on anything the caller
|
||||
// sends, so a request cannot talk its way into the first-password case.
|
||||
if (customer.password_hash !== null) {
|
||||
if (!(await passwordMatches(currentPassword, customer.password_hash))) {
|
||||
return res.status(401).json({ error: 'current password is incorrect' });
|
||||
}
|
||||
}
|
||||
|
||||
const newHash = await bcrypt.hash(newPassword, PASSWORD_HASH_ROUNDS);
|
||||
await pool.query(`UPDATE customers SET password_hash = $1 WHERE id = $2`, [newHash, req.customerId]);
|
||||
|
||||
@@ -552,6 +576,24 @@ router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Re
|
||||
const { rows } = await pool.query<CustomerRecord>(`SELECT * FROM customers WHERE id = $1`, [req.customerId]);
|
||||
const customer = requireRow(rows, 'the signed-in customer');
|
||||
|
||||
// A customer with no password is refused here rather than waved through, and
|
||||
// the asymmetry with change-password above is deliberate (#344).
|
||||
//
|
||||
// Setting a first password is a change to a credential the customer already
|
||||
// controls. Changing the email address is a change to *where recovery goes* —
|
||||
// whoever holds the new address can reset the password and own the account
|
||||
// outright. That is why this route has always demanded more than a live
|
||||
// session, and dropping the demand for the accounts that cannot meet it would
|
||||
// remove the protection from exactly the ones that need it.
|
||||
//
|
||||
// So the message says the real thing and gives them the route out, rather
|
||||
// than claiming a password was wrong when there is no password at all.
|
||||
if (customer.password_hash === null) {
|
||||
return res.status(409).json({
|
||||
error: 'this account has no password — set one first, then you can change your email address'
|
||||
});
|
||||
}
|
||||
|
||||
if (!(await passwordMatches(currentPassword, customer.password_hash))) {
|
||||
return res.status(401).json({ error: 'current password is incorrect' });
|
||||
}
|
||||
@@ -626,6 +668,36 @@ router.post('/me/analytics-consent', requireCustomer, asyncRoute(async (req: Req
|
||||
res.status(204).end();
|
||||
}));
|
||||
|
||||
/**
|
||||
* Which identity providers this account is signed in with (#343).
|
||||
*
|
||||
* Linking happens automatically when Google vouches for an address that already
|
||||
* has an account, which is defensible but not obvious. A customer who signed up
|
||||
* with a password and later used Google has had two credentials joined without
|
||||
* being asked, and a silent link is indistinguishable from a bug when they
|
||||
* later wonder why the password is no longer needed.
|
||||
*
|
||||
* So it is shown, beside the passkeys, for the reason the passkey list exists
|
||||
* at all: a customer cannot manage credentials they cannot see.
|
||||
*
|
||||
* No unlinking yet. Removing the only way into an account is the question #344
|
||||
* settles, and offering the button before that check runs would be the fastest
|
||||
* possible way to lock somebody out of their own orders.
|
||||
*/
|
||||
router.get('/me/identities', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
|
||||
const { rows } = await pool.query<{ provider: string; created_at: Date; last_used_at: Date | null }>(
|
||||
// No provider_sub. The customer cannot act on it, and it is the one value
|
||||
// that identifies them to the provider — the same reasoning that keeps
|
||||
// credential ids out of the passkey list.
|
||||
`SELECT provider, created_at, last_used_at
|
||||
FROM customer_identities
|
||||
WHERE customer_id = $1
|
||||
ORDER BY created_at`,
|
||||
[req.customerId]
|
||||
);
|
||||
res.json(rows);
|
||||
}));
|
||||
|
||||
router.get('/me/orders', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
|
||||
const { rows } = await pool.query<CustomerOrderRow>(
|
||||
`SELECT o.id, o.processor, o.amount_cents, o.status, o.created_at, i.name AS item_name
|
||||
|
||||
@@ -0,0 +1,303 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import crypto from 'node:crypto';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { signIn } from '../customerSession';
|
||||
import { googleConfig } from '../google/config';
|
||||
import { newAttempt, authorizationUrl, exchangeCode, verifiedIdentity } from '../google/oauth';
|
||||
import type { GoogleIdentity } from '../google/oauth';
|
||||
import { createCustomerFromGoogle } from '../google/newCustomer';
|
||||
import { linkToExistingCustomer } from '../google/linkIdentity';
|
||||
import { issueVerificationEmail } from '../customerVerification';
|
||||
import type { AttemptSecrets } from '../google/oauth';
|
||||
import { googleSignInLimiter } from '../rateLimit';
|
||||
import { safeReturnTo } from '../google/returnTo';
|
||||
|
||||
const router = Router();
|
||||
|
||||
/**
|
||||
* Signing in with Google (#341).
|
||||
*
|
||||
* Unauthenticated by design — this is how a customer becomes authenticated —
|
||||
* and mounted at `/api/auth/google`, away from `/api/customers`, because it is
|
||||
* the first route in this application that a third party redirects into.
|
||||
*
|
||||
* ## What this does and does not do
|
||||
*
|
||||
* It signs in a customer whose Google identity is already linked, and creates
|
||||
* an account for one nobody here has seen (#342).
|
||||
*
|
||||
* It also joins a Google identity to an account that already holds the same
|
||||
* address — but only when Google vouches for that address (#343). The whole of
|
||||
* that policy lives in `google/linkIdentity.ts`, which is the smallest module
|
||||
* in this feature and the one to read most carefully.
|
||||
*
|
||||
* ## The cookie, and why it is the whole security of the callback
|
||||
*
|
||||
* The callback is a plain GET that anyone on the internet can invoke. What
|
||||
* makes it safe is that it can only complete for a browser holding a cookie
|
||||
* this server set moments earlier, carrying three secrets:
|
||||
*
|
||||
* - **state** proves the callback belongs to the request this browser started
|
||||
* - **nonce** proves the id token was minted for this attempt
|
||||
* - **code verifier** proves the code is being spent by whoever asked for it
|
||||
*
|
||||
* The cookie is cleared on every path through the callback, success or failure,
|
||||
* so one attempt cannot be replayed even once.
|
||||
*/
|
||||
|
||||
/** Ten minutes. Long enough to sign in, short enough that a stolen one is stale. */
|
||||
const ATTEMPT_TTL_MS = 10 * 60 * 1000;
|
||||
|
||||
const ATTEMPT_COOKIE = 'rd_oauth';
|
||||
|
||||
interface Attempt extends AttemptSecrets {
|
||||
returnTo: string;
|
||||
}
|
||||
|
||||
interface IdentityRow {
|
||||
customer_id: number;
|
||||
disabled_at: Date | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where the customer is sent when this ends.
|
||||
*
|
||||
* Always a redirect, never JSON. The browser arrives here by following Google's
|
||||
* redirect, so whatever this responds with is rendered as a page — and a bare
|
||||
* JSON error is a dead end with no way back to the storefront.
|
||||
*/
|
||||
const FAILURE_PATH = '/login?auth=google-failed';
|
||||
|
||||
/**
|
||||
* Where a customer who has just been created lands.
|
||||
*
|
||||
* A route rather than a flag on the storefront, so it is a page with an address
|
||||
* — reachable again, linkable from the account page later, and rendered by the
|
||||
* same modal-route machinery every other auth screen uses.
|
||||
*/
|
||||
const WELCOME_PATH = '/welcome';
|
||||
|
||||
/**
|
||||
* Where a customer goes when they have an account this sign-in cannot reach.
|
||||
*
|
||||
* Its own destination rather than the generic failure, because it is the one
|
||||
* refusal a customer can act on: the login form reads this and says to sign in
|
||||
* with the password they already have.
|
||||
*/
|
||||
const USE_PASSWORD_PATH = '/login?auth=google-use-password';
|
||||
|
||||
function setAttemptCookie(res: Response, attempt: Attempt): void {
|
||||
res.cookie(ATTEMPT_COOKIE, Buffer.from(JSON.stringify(attempt)).toString('base64url'), {
|
||||
httpOnly: true,
|
||||
secure: process.env.NODE_ENV === 'production',
|
||||
// Lax, and NOT Strict. The callback arrives as a top-level navigation from
|
||||
// Google, which is cross-site. Strict withholds the cookie, the state check
|
||||
// then fails, and every sign-in is refused with an error that looks exactly
|
||||
// like tampering. This one line is the single most expensive thing to get
|
||||
// wrong in the whole flow.
|
||||
sameSite: 'lax',
|
||||
maxAge: ATTEMPT_TTL_MS,
|
||||
path: '/'
|
||||
});
|
||||
}
|
||||
|
||||
function readAttemptCookie(req: Request): Attempt | null {
|
||||
const raw = req.cookies?.[ATTEMPT_COOKIE];
|
||||
if (typeof raw !== 'string' || raw === '') return null;
|
||||
try {
|
||||
const parsed = JSON.parse(Buffer.from(raw, 'base64url').toString('utf8')) as Partial<Attempt>;
|
||||
if (
|
||||
typeof parsed.state !== 'string' ||
|
||||
typeof parsed.nonce !== 'string' ||
|
||||
typeof parsed.codeVerifier !== 'string'
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
state: parsed.state,
|
||||
nonce: parsed.nonce,
|
||||
codeVerifier: parsed.codeVerifier,
|
||||
returnTo: typeof parsed.returnTo === 'string' ? parsed.returnTo : '/'
|
||||
};
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Compares two secrets without leaking where they first differ.
|
||||
*
|
||||
* `timingSafeEqual` throws on buffers of unequal length, and a length check
|
||||
* before it would leak the length, so both are hashed to a fixed 32 bytes
|
||||
* first — the same trick `adminGate` uses, for the same reason.
|
||||
*/
|
||||
function secretsMatch(a: string, b: string): boolean {
|
||||
const digest = (value: string) => crypto.createHash('sha256').update(value, 'utf8').digest();
|
||||
return crypto.timingSafeEqual(digest(a), digest(b));
|
||||
}
|
||||
|
||||
/**
|
||||
* What happens when the identity lookup found nothing: create, link, or refuse.
|
||||
*
|
||||
* A named function rather than an inline block for the reason
|
||||
* `routesAreWrapped.test.ts` cares about, and because the callback is already
|
||||
* the longest handler in this file.
|
||||
*
|
||||
* The order below is the policy from #343, and it is an order rather than a set
|
||||
* of independent checks:
|
||||
*
|
||||
* 1. Nobody has this address — create the account, and land on the consent step
|
||||
* 2. Somebody does, and Google vouches for it — link, and sign in
|
||||
* 3. Somebody does, and Google does not vouch — refuse, and say to use the
|
||||
* password
|
||||
*
|
||||
* The return path is deliberately dropped in case 1 only. That customer lands
|
||||
* on the consent step, which is worth interrupting for: it is the only moment
|
||||
* the two consent sentences can honestly be shown, because the redirect to
|
||||
* Google happened before anyone knew this person was new.
|
||||
*
|
||||
* Carrying the path through as a query parameter was the alternative, and it
|
||||
* was rejected. The consent page would then have to redirect somewhere a URL
|
||||
* told it to, which is the open-redirect question `safeReturnTo` already
|
||||
* answers on the server — asked a second time, in a second language, on a page
|
||||
* an attacker can link to directly. One new customer occasionally landing on
|
||||
* the storefront rather than back at their cart is the cheaper of the two.
|
||||
*/
|
||||
async function signUpOrLink(res: Response, identity: GoogleIdentity, returnTo: string): Promise<void> {
|
||||
const outcome = await createCustomerFromGoogle(identity);
|
||||
|
||||
if (outcome.kind === 'created') {
|
||||
// Only when Google did not vouch for the address. When it did, the customer
|
||||
// has already demonstrated they receive mail there — which is precisely
|
||||
// what the confirmation email exists to establish — so sending one would
|
||||
// ask them to do a thing that is done.
|
||||
if (!identity.emailVerified) {
|
||||
await issueVerificationEmail(outcome.customerId, identity.email, identity.firstName, identity.lastName);
|
||||
}
|
||||
await signIn(res, outcome.customerId);
|
||||
res.redirect(WELCOME_PATH);
|
||||
return;
|
||||
}
|
||||
|
||||
// The address belongs to somebody. Whether that is the same person is the
|
||||
// question #343 exists to answer, and `linkToExistingCustomer` holds the
|
||||
// whole of the answer.
|
||||
const link = await linkToExistingCustomer(identity);
|
||||
if (link.kind === 'refused') {
|
||||
// Deliberately its own destination rather than the generic failure. This is
|
||||
// the one refusal a customer can act on: they have an account, they simply
|
||||
// cannot reach it this way, and telling them to use the password they
|
||||
// already have is more useful than "that did not work".
|
||||
//
|
||||
// It reveals nothing they did not already supply. They arrived holding a
|
||||
// Google account for this address, so being told the address has an account
|
||||
// here tells them about themselves.
|
||||
console.warn('[google] refused a link: the address is taken and Google did not verify it');
|
||||
res.redirect(USE_PASSWORD_PATH);
|
||||
return;
|
||||
}
|
||||
|
||||
await signIn(res, link.customerId);
|
||||
res.redirect(returnTo);
|
||||
}
|
||||
|
||||
router.get(
|
||||
'/start',
|
||||
googleSignInLimiter,
|
||||
asyncRoute(async (req: Request, res: Response) => {
|
||||
const config = googleConfig();
|
||||
if (!config.enabled) {
|
||||
// Not a 404 and not an error page. Nothing offers this link when Google
|
||||
// sign-in is switched off, so reaching it means a stale bookmark or a
|
||||
// hand-typed URL, and the storefront is the right answer to both.
|
||||
return res.redirect('/');
|
||||
}
|
||||
|
||||
const attempt: Attempt = { ...newAttempt(), returnTo: safeReturnTo(req.query.returnTo) };
|
||||
setAttemptCookie(res, attempt);
|
||||
res.redirect(authorizationUrl(config, attempt));
|
||||
})
|
||||
);
|
||||
|
||||
router.get(
|
||||
'/callback',
|
||||
asyncRoute(async (req: Request, res: Response) => {
|
||||
const config = googleConfig();
|
||||
const attempt = readAttemptCookie(req);
|
||||
|
||||
// Cleared before anything is decided, on every path. A cookie that survives
|
||||
// a failed attempt is a second try at the same state and nonce.
|
||||
res.clearCookie(ATTEMPT_COOKIE, { path: '/' });
|
||||
|
||||
if (!config.enabled || attempt === null) return res.redirect(FAILURE_PATH);
|
||||
|
||||
// Google sends `error=access_denied` when the customer declines at the
|
||||
// consent screen. That is a cancellation rather than a failure, and it goes
|
||||
// back to the storefront with nothing said — the same distinction #41 draws
|
||||
// for a dismissed passkey prompt.
|
||||
if (typeof req.query.error === 'string') {
|
||||
return res.redirect(attempt.returnTo);
|
||||
}
|
||||
|
||||
const state = typeof req.query.state === 'string' ? req.query.state : '';
|
||||
const code = typeof req.query.code === 'string' ? req.query.code : '';
|
||||
if (state === '' || code === '' || !secretsMatch(state, attempt.state)) {
|
||||
console.warn('[google] callback refused: state did not match the attempt cookie');
|
||||
return res.redirect(FAILURE_PATH);
|
||||
}
|
||||
|
||||
let identity;
|
||||
try {
|
||||
const idToken = await exchangeCode(config, code, attempt.codeVerifier);
|
||||
identity = verifiedIdentity(idToken, { clientId: config.clientId, nonce: attempt.nonce });
|
||||
} catch (err) {
|
||||
// Logged, never returned. These messages name which check failed, which
|
||||
// is exactly what the person reading the logs needs and exactly what an
|
||||
// attacker would like to be told.
|
||||
console.warn(`[google] callback refused: ${(err as Error).message}`);
|
||||
return res.redirect(FAILURE_PATH);
|
||||
}
|
||||
|
||||
const { rows } = await pool.query<IdentityRow>(
|
||||
`SELECT i.customer_id, c.disabled_at
|
||||
FROM customer_identities i
|
||||
JOIN customers c ON c.id = i.customer_id
|
||||
WHERE i.provider = 'google' AND i.provider_sub = $1`,
|
||||
[identity.sub]
|
||||
);
|
||||
const linked = rows[0];
|
||||
|
||||
// Nobody this shop has seen through Google before. Either they are new, or
|
||||
// they already have an account under this address — and joining those two
|
||||
// is linking, which is #343 and is refused here until its policy is
|
||||
// written down rather than falling out of an INSERT.
|
||||
if (!linked) return signUpOrLink(res, identity, attempt.returnTo);
|
||||
|
||||
// Refused here as well as on the password and passkey paths. Enforcing it
|
||||
// on some routes and not others is how a disabled account keeps a way in,
|
||||
// which is the reason #39 called this out for passkeys.
|
||||
if (linked.disabled_at !== null) {
|
||||
console.warn(`[google] refused a disabled account: customer ${linked.customer_id}`);
|
||||
return res.redirect(FAILURE_PATH);
|
||||
}
|
||||
|
||||
await pool.query(
|
||||
`UPDATE customer_identities SET last_used_at = now()
|
||||
WHERE provider = 'google' AND provider_sub = $1`,
|
||||
[identity.sub]
|
||||
);
|
||||
|
||||
// The same call password login and passkey login make. Not a third
|
||||
// implementation that agrees today — the same one, so cookie flags, expiry
|
||||
// and logout behave identically however a customer got here.
|
||||
await signIn(res, linked.customer_id);
|
||||
|
||||
res.redirect(attempt.returnTo);
|
||||
})
|
||||
);
|
||||
|
||||
/** Exported for the tests; nothing else needs the cookie's name. */
|
||||
export { ATTEMPT_COOKIE, ATTEMPT_TTL_MS, FAILURE_PATH, WELCOME_PATH, USE_PASSWORD_PATH };
|
||||
|
||||
export default router;
|
||||
@@ -82,7 +82,10 @@ describe('POST /api/customers/register', () => {
|
||||
|
||||
expect(Object.keys(res.body).sort()).toEqual([
|
||||
'analytics_consent', 'created_at', 'email', 'email_verified', 'favorite_alerts',
|
||||
'first_name', 'id', 'last_name', 'marketing_consent'
|
||||
// Whether, never what. Added in #344 so the account page can offer to set
|
||||
// a first password rather than to change one that does not exist; the
|
||||
// hash itself must never appear in this list.
|
||||
'first_name', 'has_password', 'id', 'last_name', 'marketing_consent'
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,783 @@
|
||||
import request from 'supertest';
|
||||
import app from '../../src/app';
|
||||
import { pool, requireRow } from '../../src/db';
|
||||
import { createSession } from '../../src/customerSession';
|
||||
import { resetDb, closeDb } from './setup/testDb';
|
||||
|
||||
const CLIENT_ID = 'test-client.apps.googleusercontent.com';
|
||||
const SUB = 'google-subject-1234567890';
|
||||
|
||||
let fetchSpy: jest.SpyInstance;
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetDb();
|
||||
process.env.GOOGLE_CLIENT_ID = CLIENT_ID;
|
||||
process.env.GOOGLE_CLIENT_SECRET = 'test-secret';
|
||||
process.env.PUBLIC_URL = 'http://localhost:3000';
|
||||
// Nothing here talks to Google. The exchange is the only network call in the
|
||||
// flow, so stubbing it leaves every decision this suite cares about — the
|
||||
// cookie, the state check, the claim checks, the lookup — running for real.
|
||||
fetchSpy = jest.spyOn(globalThis, 'fetch');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchSpy.mockRestore();
|
||||
delete process.env.GOOGLE_CLIENT_ID;
|
||||
delete process.env.GOOGLE_CLIENT_SECRET;
|
||||
delete process.env.PUBLIC_URL;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await pool.end();
|
||||
await closeDb();
|
||||
});
|
||||
|
||||
/** An unsigned id token. Nothing in this flow reads a signature — see google/oauth.ts. */
|
||||
function idToken(claims: Record<string, unknown>): string {
|
||||
const part = (value: unknown) => Buffer.from(JSON.stringify(value)).toString('base64url');
|
||||
return `${part({ alg: 'RS256' })}.${part(claims)}.not-a-signature`;
|
||||
}
|
||||
|
||||
function claimsFor(nonce: string, overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
iss: 'https://accounts.google.com',
|
||||
aud: CLIENT_ID,
|
||||
exp: Math.floor(Date.now() / 1000) + 3600,
|
||||
sub: SUB,
|
||||
nonce,
|
||||
email: 'customer@example.com',
|
||||
email_verified: true,
|
||||
given_name: 'Test',
|
||||
family_name: 'Customer',
|
||||
...overrides
|
||||
};
|
||||
}
|
||||
|
||||
function respondWithToken(token: string): void {
|
||||
fetchSpy.mockResolvedValue(
|
||||
new Response(JSON.stringify({ id_token: token }), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' }
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/** The attempt cookie the start route set, as a header for the callback. */
|
||||
function cookieHeader(setCookie: string[]): string {
|
||||
const attempt = setCookie.find((c) => c.startsWith('rd_oauth='));
|
||||
if (!attempt) throw new Error('the start route set no attempt cookie');
|
||||
// Non-null: the header was just matched, so it has at least one segment.
|
||||
return attempt.split(';')[0] as string;
|
||||
}
|
||||
|
||||
/** One Set-Cookie value, as a request header. Throws rather than typing around its absence. */
|
||||
function requireCookie(setCookie: string[], prefix: string): string {
|
||||
const found = setCookie.find((c) => c.startsWith(prefix));
|
||||
if (!found) throw new Error(`no ${prefix} cookie was set`);
|
||||
return found.split(';')[0] as string;
|
||||
}
|
||||
|
||||
/** Reads the secrets back out of the cookie, which is the only place they exist. */
|
||||
function attemptFrom(setCookie: string[]): { state: string; nonce: string; returnTo: string } {
|
||||
const value = cookieHeader(setCookie).slice('rd_oauth='.length);
|
||||
return JSON.parse(Buffer.from(decodeURIComponent(value), 'base64url').toString('utf8'));
|
||||
}
|
||||
|
||||
/** Starts a sign-in and hands back what the callback needs to finish it. */
|
||||
async function startSignIn(returnTo?: string) {
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/start')
|
||||
.query(returnTo === undefined ? {} : { returnTo });
|
||||
const setCookie = res.headers['set-cookie'] as unknown as string[];
|
||||
return { res, cookie: cookieHeader(setCookie), ...attemptFrom(setCookie) };
|
||||
}
|
||||
|
||||
async function createCustomer(email: string): Promise<number> {
|
||||
const { rows } = await pool.query<{ id: number }>(
|
||||
`INSERT INTO customers (email, password_hash, first_name, last_name, unsubscribe_token)
|
||||
VALUES ($1, 'not-a-real-hash', 'Test', 'Customer', $2) RETURNING id`,
|
||||
[email, `unsub-${email}`]
|
||||
);
|
||||
return requireRow(rows, 'the customer this test just created').id;
|
||||
}
|
||||
|
||||
async function linkGoogle(customerId: number, sub = SUB): Promise<void> {
|
||||
await pool.query(
|
||||
`INSERT INTO customer_identities (customer_id, provider, provider_sub)
|
||||
VALUES ($1, 'google', $2)`,
|
||||
[customerId, sub]
|
||||
);
|
||||
}
|
||||
|
||||
/** A session cookie for a customer, without going through any sign-in flow. */
|
||||
async function sessionFor(customerId: number): Promise<string> {
|
||||
return `rd_session=${await createSession(customerId)}`;
|
||||
}
|
||||
|
||||
describe('GET /api/auth/google/start', () => {
|
||||
it('sends the customer to Google with the code flow and PKCE', async () => {
|
||||
const { res } = await startSignIn();
|
||||
|
||||
expect(res.status).toBe(302);
|
||||
const target = new URL(res.headers.location as string);
|
||||
expect(target.origin).toBe('https://accounts.google.com');
|
||||
expect(target.searchParams.get('response_type')).toBe('code');
|
||||
expect(target.searchParams.get('code_challenge_method')).toBe('S256');
|
||||
});
|
||||
|
||||
it('sets an httpOnly attempt cookie, which is the whole security of the callback', async () => {
|
||||
const res = await request(app).get('/api/auth/google/start');
|
||||
const attempt = (res.headers['set-cookie'] as unknown as string[]).find((c) =>
|
||||
c.startsWith('rd_oauth=')
|
||||
);
|
||||
|
||||
expect(attempt).toMatch(/HttpOnly/i);
|
||||
// Lax and not Strict. Strict withholds the cookie on the cross-site
|
||||
// top-level navigation back from Google, and every sign-in then fails the
|
||||
// state check in a way that looks exactly like tampering.
|
||||
expect(attempt).toMatch(/SameSite=Lax/i);
|
||||
});
|
||||
|
||||
it('never puts the client secret anywhere the browser can see', async () => {
|
||||
const res = await request(app).get('/api/auth/google/start');
|
||||
|
||||
expect(res.headers.location).not.toContain('test-secret');
|
||||
expect(JSON.stringify(res.headers['set-cookie'])).not.toContain('test-secret');
|
||||
});
|
||||
|
||||
it('carries a local return path through the round trip', async () => {
|
||||
const { returnTo } = await startSignIn('/?max_price=50000');
|
||||
|
||||
expect(returnTo).toBe('/?max_price=50000');
|
||||
});
|
||||
|
||||
it('refuses an off-site return path rather than becoming an open redirect', async () => {
|
||||
const { returnTo } = await startSignIn('//evil.test');
|
||||
|
||||
expect(returnTo).toBe('/');
|
||||
});
|
||||
|
||||
it('sends the customer to the storefront when Google sign-in is switched off', async () => {
|
||||
delete process.env.GOOGLE_CLIENT_ID;
|
||||
delete process.env.GOOGLE_CLIENT_SECRET;
|
||||
|
||||
const res = await request(app).get('/api/auth/google/start');
|
||||
|
||||
// A stale bookmark or a hand-typed URL, and the storefront answers both.
|
||||
expect(res.status).toBe(302);
|
||||
expect(res.headers.location).toBe('/');
|
||||
expect(res.headers['set-cookie']).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/auth/google/callback', () => {
|
||||
it('signs in a customer whose Google identity is already linked', async () => {
|
||||
const customerId = await createCustomer('linked@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state, nonce } = await startSignIn('/cart');
|
||||
respondWithToken(idToken(claimsFor(nonce)));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
expect(res.status).toBe(302);
|
||||
expect(res.headers.location).toBe('/cart');
|
||||
const session = (res.headers['set-cookie'] as unknown as string[]).find((c) =>
|
||||
c.startsWith('rd_session=')
|
||||
);
|
||||
expect(session).toBeDefined();
|
||||
});
|
||||
|
||||
it('produces a session the rest of the application accepts', async () => {
|
||||
const customerId = await createCustomer('session@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce)));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
const setCookie = res.headers['set-cookie'] as unknown as string[];
|
||||
const session = requireCookie(setCookie, 'rd_session=');
|
||||
|
||||
// The point of sharing signIn with the password and passkey paths: the
|
||||
// session is not merely present, it is the same kind of session.
|
||||
const me = await request(app).get('/api/customers/me').set('Cookie', session);
|
||||
expect(me.status).toBe(200);
|
||||
expect(me.body.email).toBe('session@example.com');
|
||||
});
|
||||
|
||||
it('stamps last_used_at, which is what tells two identities apart', async () => {
|
||||
const customerId = await createCustomer('stamped@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce)));
|
||||
|
||||
await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
const { rows } = await pool.query<{ last_used_at: Date | null }>(
|
||||
`SELECT last_used_at FROM customer_identities WHERE provider_sub = $1`,
|
||||
[SUB]
|
||||
);
|
||||
expect(requireRow(rows, 'the identity just used').last_used_at).not.toBeNull();
|
||||
});
|
||||
|
||||
it('clears the attempt cookie, so one attempt cannot be replayed', async () => {
|
||||
const customerId = await createCustomer('once@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce)));
|
||||
|
||||
const first = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
const cleared = (first.headers['set-cookie'] as unknown as string[]).find((c) =>
|
||||
c.startsWith('rd_oauth=')
|
||||
);
|
||||
expect(cleared).toMatch(/rd_oauth=;/);
|
||||
});
|
||||
|
||||
it('refuses a state that does not match the attempt cookie', async () => {
|
||||
const { cookie } = await startSignIn();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state: 'not-the-state-we-issued' })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
// Refused before any network call: a mismatched state is not worth a token
|
||||
// exchange, and spending the code would be handing it to whoever forged it.
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses a callback carrying no attempt cookie at all', async () => {
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state: 'anything' });
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('treats a declined consent screen as a cancellation, not a failure', async () => {
|
||||
const { cookie } = await startSignIn('/cart');
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ error: 'access_denied' })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
// Back where they were, with nothing said. A customer who changed their
|
||||
// mind has not encountered an error, which is the distinction #41 draws
|
||||
// for a dismissed passkey prompt.
|
||||
expect(res.headers.location).toBe('/cart');
|
||||
});
|
||||
|
||||
it('refuses an id token minted for a different application', async () => {
|
||||
const customerId = await createCustomer('wrongaud@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce, { aud: 'someone-else.apps.googleusercontent.com' })));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
expect(res.headers['set-cookie'] ?? []).not.toContainEqual(expect.stringContaining('rd_session='));
|
||||
});
|
||||
|
||||
it('refuses an id token from a different sign-in attempt', async () => {
|
||||
const customerId = await createCustomer('wrongnonce@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const { cookie, state } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor('a-nonce-from-somewhere-else')));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
});
|
||||
|
||||
it('refuses when Google declines the token exchange', async () => {
|
||||
const { cookie, state } = await startSignIn();
|
||||
fetchSpy.mockResolvedValue(new Response('{"error":"invalid_grant"}', { status: 400 }));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'a-spent-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
});
|
||||
|
||||
it('refuses a disabled account, as the password and passkey paths do', async () => {
|
||||
const customerId = await createCustomer('disabled@example.com');
|
||||
await linkGoogle(customerId);
|
||||
await pool.query(`UPDATE customers SET disabled_at = now() WHERE id = $1`, [customerId]);
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce)));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
// Enforcing it on some sign-in routes and not others is how a disabled
|
||||
// account keeps a way in.
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
expect(res.headers['set-cookie'] ?? []).not.toContainEqual(expect.stringContaining('rd_session='));
|
||||
});
|
||||
|
||||
it('refuses when the address already belongs to an account, and creates nothing', async () => {
|
||||
await createCustomer('unlinked@example.com');
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce, { email: 'unlinked@example.com' })));
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
// Joining those two accounts is linking, which is #343. Doing it here on
|
||||
// the strength of a matching address is the takeover path that decision
|
||||
// exists to reason about carefully.
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
const { rows } = await pool.query<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM customer_identities`
|
||||
);
|
||||
expect(requireRow(rows, 'a count of identities').n).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #342. A Google account nobody here has seen becomes a customer.
|
||||
*
|
||||
* The consent behaviour is most of what these assert, because it is the part
|
||||
* that is easy to get quietly wrong: an account created with a consent nobody
|
||||
* gave, or with wording that does not match what the customer was shown, is
|
||||
* still an account that works.
|
||||
*/
|
||||
describe('signing up with Google', () => {
|
||||
async function signUpWith(overrides: Record<string, unknown> = {}) {
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
respondWithToken(idToken(claimsFor(nonce, overrides)));
|
||||
return request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
}
|
||||
|
||||
async function customerBy(email: string) {
|
||||
const { rows } = await pool.query<{
|
||||
id: number;
|
||||
first_name: string | null;
|
||||
last_name: string | null;
|
||||
email_verified: boolean;
|
||||
password_hash: string | null;
|
||||
marketing_consent: boolean;
|
||||
marketing_consent_text: string | null;
|
||||
analytics_consent: boolean;
|
||||
analytics_consent_text: string | null;
|
||||
}>(`SELECT * FROM customers WHERE email = $1`, [email]);
|
||||
return requireRow(rows, `the customer for ${email}`);
|
||||
}
|
||||
|
||||
async function countOf(table: 'customers' | 'customer_identities'): Promise<number> {
|
||||
const { rows } = await pool.query<{ n: number }>(`SELECT count(*)::int AS n FROM ${table}`);
|
||||
return requireRow(rows, `a count of ${table}`).n;
|
||||
}
|
||||
|
||||
async function verificationTokens(customerId: number): Promise<number> {
|
||||
const { rows } = await pool.query<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM customer_tokens WHERE customer_id = $1 AND kind = 'verify_email'`,
|
||||
[customerId]
|
||||
);
|
||||
return requireRow(rows, 'a count of verification tokens').n;
|
||||
}
|
||||
|
||||
it('creates a customer and an identity, and signs them in', async () => {
|
||||
const res = await signUpWith({ email: 'brandnew@example.com' });
|
||||
|
||||
expect(res.status).toBe(302);
|
||||
const customer = await customerBy('brandnew@example.com');
|
||||
const { rows } = await pool.query<{ customer_id: number }>(
|
||||
`SELECT customer_id FROM customer_identities WHERE provider_sub = $1`,
|
||||
[SUB]
|
||||
);
|
||||
expect(requireRow(rows, 'the new identity').customer_id).toBe(customer.id);
|
||||
expect(res.headers['set-cookie'] as unknown as string[]).toContainEqual(
|
||||
expect.stringContaining('rd_session=')
|
||||
);
|
||||
});
|
||||
|
||||
it('lands the new customer on the consent step rather than the storefront', async () => {
|
||||
// The one moment the two consent sentences can honestly be shown: the
|
||||
// redirect to Google happened before anyone knew this person was new.
|
||||
const res = await signUpWith({ email: 'consentstep@example.com' });
|
||||
|
||||
expect(res.headers.location).toBe('/welcome');
|
||||
});
|
||||
|
||||
it('creates the account with no password at all', async () => {
|
||||
await signUpWith({ email: 'nopassword@example.com' });
|
||||
|
||||
// The first accounts in this project's history without one. #344 is where
|
||||
// the routes that assumed otherwise learn to cope.
|
||||
expect((await customerBy('nopassword@example.com')).password_hash).toBeNull();
|
||||
});
|
||||
|
||||
it('gives both consents as false, with no stored wording', async () => {
|
||||
await signUpWith({ email: 'noconsent@example.com' });
|
||||
const customer = await customerBy('noconsent@example.com');
|
||||
|
||||
// Nobody agreed to anything, so nothing is recorded as though they had. A
|
||||
// stored wording against a false consent would be a record of a
|
||||
// conversation that never happened.
|
||||
expect(customer.marketing_consent).toBe(false);
|
||||
expect(customer.analytics_consent).toBe(false);
|
||||
expect(customer.marketing_consent_text).toBeNull();
|
||||
expect(customer.analytics_consent_text).toBeNull();
|
||||
});
|
||||
|
||||
it('takes the names from the Google profile', async () => {
|
||||
await signUpWith({ email: 'named@example.com' });
|
||||
const customer = await customerBy('named@example.com');
|
||||
|
||||
expect(customer.first_name).toBe('Test');
|
||||
expect(customer.last_name).toBe('Customer');
|
||||
});
|
||||
|
||||
it('creates the account anyway when Google sends no names', async () => {
|
||||
// Registration demands both because every email greets by first name, but
|
||||
// Google may return neither and refusing over it would be absurd — the
|
||||
// greeting already has a fallback for exactly this.
|
||||
const { cookie, state, nonce } = await startSignIn();
|
||||
const claims = claimsFor(nonce, { email: 'nameless@example.com' }) as Record<string, unknown>;
|
||||
delete claims.given_name;
|
||||
delete claims.family_name;
|
||||
respondWithToken(idToken(claims));
|
||||
|
||||
await request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
|
||||
const customer = await customerBy('nameless@example.com');
|
||||
expect(customer.first_name).toBeNull();
|
||||
expect(customer.last_name).toBeNull();
|
||||
});
|
||||
|
||||
describe('the verified address', () => {
|
||||
it('is marked verified, and sends no confirmation email, when Google vouches', async () => {
|
||||
await signUpWith({ email: 'vouched@example.com', email_verified: true });
|
||||
const customer = await customerBy('vouched@example.com');
|
||||
|
||||
expect(customer.email_verified).toBe(true);
|
||||
// The confirmation email exists to prove the customer receives mail at
|
||||
// the address. Google has just proved exactly that, so sending one would
|
||||
// ask them to do a thing that is already done.
|
||||
expect(await verificationTokens(customer.id)).toBe(0);
|
||||
});
|
||||
|
||||
it('is unverified, and does send one, when Google does not', async () => {
|
||||
await signUpWith({ email: 'unvouched@example.com', email_verified: false });
|
||||
const customer = await customerBy('unvouched@example.com');
|
||||
|
||||
// An unverified assertion is worth nothing, so this account goes through
|
||||
// the ordinary confirmation exactly as a password sign-up would.
|
||||
expect(customer.email_verified).toBe(false);
|
||||
expect(await verificationTokens(customer.id)).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it('reaches the same customer on a second sign-in, not a second account', async () => {
|
||||
await signUpWith({ email: 'returning@example.com' });
|
||||
const first = await customerBy('returning@example.com');
|
||||
|
||||
const second = await signUpWith({ email: 'returning@example.com' });
|
||||
|
||||
// Signed in, and back to the storefront rather than the consent step —
|
||||
// which is shown once, to somebody who has just been created.
|
||||
expect(second.headers.location).toBe('/');
|
||||
expect(await countOf('customers')).toBe(1);
|
||||
expect((await customerBy('returning@example.com')).id).toBe(first.id);
|
||||
});
|
||||
|
||||
it('reaches the same customer even after they change their Google address', async () => {
|
||||
await signUpWith({ email: 'was@example.com' });
|
||||
const original = await customerBy('was@example.com');
|
||||
|
||||
// Matched on the subject, which is the whole reason that column exists. An
|
||||
// email match would have created a second account here — and an address
|
||||
// that had since been reassigned would have handed this one to a stranger.
|
||||
const second = await signUpWith({ email: 'now@example.com' });
|
||||
|
||||
expect(second.headers.location).toBe('/');
|
||||
expect(await countOf('customers')).toBe(1);
|
||||
expect((await customerBy('was@example.com')).id).toBe(original.id);
|
||||
});
|
||||
|
||||
it('still refuses a disabled account, which a sign-up must not route around', async () => {
|
||||
const customerId = await createCustomer('blocked@example.com');
|
||||
await linkGoogle(customerId, SUB);
|
||||
await pool.query(`UPDATE customers SET disabled_at = now() WHERE id = $1`, [customerId]);
|
||||
|
||||
const res = await signUpWith({ email: 'blocked@example.com' });
|
||||
|
||||
// The identity exists, so this takes the sign-in path and is refused
|
||||
// there. No second account is created as a way around it.
|
||||
expect(res.headers.location).toBe('/login?auth=google-failed');
|
||||
expect(await countOf('customers')).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #343. The most security-sensitive phase of this feature.
|
||||
*
|
||||
* Every test here is about the same question asked from a different angle: when
|
||||
* is it right to hand somebody an account they have not proved they own?
|
||||
*/
|
||||
describe('linking a Google identity to an existing account', () => {
|
||||
async function attempt(overrides: Record<string, unknown> = {}, returnTo?: string) {
|
||||
const { cookie, state, nonce } = await startSignIn(returnTo);
|
||||
respondWithToken(idToken(claimsFor(nonce, overrides)));
|
||||
return request(app)
|
||||
.get('/api/auth/google/callback')
|
||||
.query({ code: 'an-auth-code', state })
|
||||
.set('Cookie', cookie);
|
||||
}
|
||||
|
||||
async function identityCount(customerId: number): Promise<number> {
|
||||
const { rows } = await pool.query<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM customer_identities WHERE customer_id = $1`,
|
||||
[customerId]
|
||||
);
|
||||
return requireRow(rows, 'a count of identities').n;
|
||||
}
|
||||
|
||||
it('links when Google vouches for an address an account already holds', async () => {
|
||||
const customerId = await createCustomer('haspassword@example.com');
|
||||
|
||||
const res = await attempt({ email: 'haspassword@example.com', email_verified: true }, '/cart');
|
||||
|
||||
// Whoever completed that sign-in demonstrably controls the mailbox, which
|
||||
// is already the root of trust for a password reset on this account. So
|
||||
// linking grants nothing that was not already reachable.
|
||||
expect(res.headers.location).toBe('/cart');
|
||||
expect(await identityCount(customerId)).toBe(1);
|
||||
expect(res.headers['set-cookie'] as unknown as string[]).toContainEqual(
|
||||
expect.stringContaining('rd_session=')
|
||||
);
|
||||
});
|
||||
|
||||
it('signs the linked customer into their existing account, not a new one', async () => {
|
||||
const customerId = await createCustomer('same@example.com');
|
||||
|
||||
const res = await attempt({ email: 'same@example.com', email_verified: true });
|
||||
const session = requireCookie(res.headers['set-cookie'] as unknown as string[], 'rd_session=');
|
||||
const me = await request(app).get('/api/customers/me').set('Cookie', session);
|
||||
|
||||
expect(me.body.id).toBe(customerId);
|
||||
const { rows } = await pool.query<{ n: number }>(`SELECT count(*)::int AS n FROM customers`);
|
||||
expect(requireRow(rows, 'a count of customers').n).toBe(1);
|
||||
});
|
||||
|
||||
it('refuses when Google does not vouch for the address', async () => {
|
||||
const customerId = await createCustomer('unverified@example.com');
|
||||
|
||||
const res = await attempt({ email: 'unverified@example.com', email_verified: false });
|
||||
|
||||
// The whole policy in one assertion. Linking on an unverified assertion is
|
||||
// not a degraded version of the same thing — it is an account takeover with
|
||||
// extra steps, because nobody has checked the claim.
|
||||
expect(res.headers.location).toBe('/login?auth=google-use-password');
|
||||
expect(await identityCount(customerId)).toBe(0);
|
||||
expect(res.headers['set-cookie'] ?? []).not.toContainEqual(
|
||||
expect.stringContaining('rd_session=')
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses on a merely truthy email_verified, which is the trap', async () => {
|
||||
// The string "false" is truthy. If this check ever becomes a truthiness
|
||||
// test, every unverified Google account links to whatever account holds
|
||||
// its address.
|
||||
const customerId = await createCustomer('trap@example.com');
|
||||
|
||||
const res = await attempt({ email: 'trap@example.com', email_verified: 'false' });
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-use-password');
|
||||
expect(await identityCount(customerId)).toBe(0);
|
||||
});
|
||||
|
||||
it('sends the refused customer somewhere they can act on', async () => {
|
||||
// They have an account and simply cannot reach it this way. Telling them to
|
||||
// use the password they already have beats "that did not work", and reveals
|
||||
// nothing: they arrived holding a Google account for this address.
|
||||
await createCustomer('actionable@example.com');
|
||||
|
||||
const res = await attempt({ email: 'actionable@example.com', email_verified: false });
|
||||
|
||||
expect(res.headers.location).toBe('/login?auth=google-use-password');
|
||||
});
|
||||
|
||||
it('refuses to link to a disabled account', async () => {
|
||||
const customerId = await createCustomer('disabledlink@example.com');
|
||||
await pool.query(`UPDATE customers SET disabled_at = now() WHERE id = $1`, [customerId]);
|
||||
|
||||
const res = await attempt({ email: 'disabledlink@example.com', email_verified: true });
|
||||
|
||||
// Linking and then refusing the session would leave the identity attached,
|
||||
// so the next attempt would take the sign-in path instead — turning a
|
||||
// disabled account into one that is merely inconvenient to reach.
|
||||
expect(await identityCount(customerId)).toBe(0);
|
||||
expect(res.headers['set-cookie'] ?? []).not.toContainEqual(
|
||||
expect.stringContaining('rd_session=')
|
||||
);
|
||||
});
|
||||
|
||||
it('matches the address case-insensitively, as registration stores it', async () => {
|
||||
const customerId = await createCustomer('mixedcase@example.com');
|
||||
|
||||
const res = await attempt({ email: 'MixedCase@Example.COM', email_verified: true });
|
||||
|
||||
// A stricter comparison than registration's would silently fail to match
|
||||
// and produce a second account for one person, rather than an error anyone
|
||||
// sees.
|
||||
expect(await identityCount(customerId)).toBe(1);
|
||||
expect(res.headers.location).toBe('/');
|
||||
});
|
||||
|
||||
it('prefers the identity over the address once linked', async () => {
|
||||
const withIdentity = await createCustomer('theirs@example.com');
|
||||
await linkGoogle(withIdentity, SUB);
|
||||
// A second customer now holds the address this Google account reports.
|
||||
const withAddress = await createCustomer('moved@example.com');
|
||||
|
||||
const res = await attempt({ email: 'moved@example.com', email_verified: true });
|
||||
const session = requireCookie(res.headers['set-cookie'] as unknown as string[], 'rd_session=');
|
||||
const me = await request(app).get('/api/customers/me').set('Cookie', session);
|
||||
|
||||
// The identity lookup runs first and nothing else is consulted. An identity
|
||||
// that has signed in before keeps working even when the address on either
|
||||
// side has since changed — and the account matching the address is somebody
|
||||
// else's, which is exactly why the order matters.
|
||||
expect(me.body.id).toBe(withIdentity);
|
||||
expect(await identityCount(withAddress)).toBe(0);
|
||||
});
|
||||
|
||||
it('does not link twice when the same customer signs in again', async () => {
|
||||
const customerId = await createCustomer('twice@example.com');
|
||||
|
||||
await attempt({ email: 'twice@example.com', email_verified: true });
|
||||
await attempt({ email: 'twice@example.com', email_verified: true });
|
||||
|
||||
expect(await identityCount(customerId)).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/customers/me/identities', () => {
|
||||
it('shows the customer what they are linked to', async () => {
|
||||
const customerId = await createCustomer('shown@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const session = await sessionFor(customerId);
|
||||
|
||||
const res = await request(app).get('/api/customers/me/identities').set('Cookie', session);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toHaveLength(1);
|
||||
expect(res.body[0].provider).toBe('google');
|
||||
});
|
||||
|
||||
it('never returns the provider subject', async () => {
|
||||
const customerId = await createCustomer('opaque@example.com');
|
||||
await linkGoogle(customerId);
|
||||
const session = await sessionFor(customerId);
|
||||
|
||||
const res = await request(app).get('/api/customers/me/identities').set('Cookie', session);
|
||||
|
||||
// The customer cannot act on it, and it is the one value that identifies
|
||||
// them to Google — the same reasoning that keeps credential ids out of the
|
||||
// passkey list.
|
||||
expect(JSON.stringify(res.body)).not.toContain(SUB);
|
||||
expect(res.body[0].provider_sub).toBeUndefined();
|
||||
});
|
||||
|
||||
it('is empty for a customer who has never used a provider', async () => {
|
||||
const customerId = await createCustomer('none@example.com');
|
||||
const session = await sessionFor(customerId);
|
||||
|
||||
const res = await request(app).get('/api/customers/me/identities').set('Cookie', session);
|
||||
|
||||
expect(res.body).toEqual([]);
|
||||
});
|
||||
|
||||
it('refuses without a session', async () => {
|
||||
const res = await request(app).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');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,363 @@
|
||||
import request from 'supertest';
|
||||
import bcrypt from 'bcryptjs';
|
||||
import app from '../../src/app';
|
||||
import { pool, requireRow } from '../../src/db';
|
||||
import { createSession } from '../../src/customerSession';
|
||||
import { PASSWORD_HASH_ROUNDS } from '../../src/passwordHashing';
|
||||
import { resetDb, closeDb } from './setup/testDb';
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetDb();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await pool.end();
|
||||
await closeDb();
|
||||
});
|
||||
|
||||
const PASSWORD = 'supersecret123';
|
||||
|
||||
/**
|
||||
* A customer who signed up with Google: no password at all (#344).
|
||||
*
|
||||
* Inserted rather than driven through the OAuth flow, because what these tests
|
||||
* are about is the state, not how it was reached. The flow that produces it has
|
||||
* its own suite.
|
||||
*/
|
||||
async function passwordlessCustomer(email: string): Promise<number> {
|
||||
const { rows } = await pool.query<{ id: number }>(
|
||||
`INSERT INTO customers (email, password_hash, first_name, last_name, email_verified, unsubscribe_token)
|
||||
VALUES ($1, NULL, 'Test', 'Customer', true, $2) RETURNING id`,
|
||||
[email, `unsub-${email}`]
|
||||
);
|
||||
const id = requireRow(rows, 'the passwordless customer').id;
|
||||
await pool.query(
|
||||
`INSERT INTO customer_identities (customer_id, provider, provider_sub) VALUES ($1, 'google', $2)`,
|
||||
[id, `sub-${email}`]
|
||||
);
|
||||
return id;
|
||||
}
|
||||
|
||||
async function customerWithPassword(email: string): Promise<number> {
|
||||
const { rows } = await pool.query<{ id: number }>(
|
||||
`INSERT INTO customers (email, password_hash, first_name, last_name, email_verified, unsubscribe_token)
|
||||
VALUES ($1, $2, 'Test', 'Customer', true, $3) RETURNING id`,
|
||||
[email, await bcrypt.hash(PASSWORD, PASSWORD_HASH_ROUNDS), `unsub-${email}`]
|
||||
);
|
||||
return requireRow(rows, 'the customer with a password').id;
|
||||
}
|
||||
|
||||
async function sessionFor(customerId: number): Promise<string> {
|
||||
return `rd_session=${await createSession(customerId)}`;
|
||||
}
|
||||
|
||||
async function storedHash(customerId: number): Promise<string | null> {
|
||||
const { rows } = await pool.query<{ password_hash: string | null }>(
|
||||
`SELECT password_hash FROM customers WHERE id = $1`,
|
||||
[customerId]
|
||||
);
|
||||
return requireRow(rows, 'the customer').password_hash;
|
||||
}
|
||||
|
||||
describe('an account with no password', () => {
|
||||
describe('setting a first one', () => {
|
||||
it('takes no current password, because there is none to give', async () => {
|
||||
const id = await passwordlessCustomer('first@example.com');
|
||||
const session = await sessionFor(id);
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', session)
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
// Asking for a value that was never set is a dead end. The session they
|
||||
// are already holding is what authorises this, exactly as it authorises
|
||||
// every other setting on the account page.
|
||||
expect(res.status).toBe(204);
|
||||
expect(await storedHash(id)).not.toBeNull();
|
||||
});
|
||||
|
||||
it('lets them sign in with it afterwards', async () => {
|
||||
const id = await passwordlessCustomer('cansignin@example.com');
|
||||
|
||||
await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', await sessionFor(id))
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
const login = await request(app)
|
||||
.post('/api/customers/login')
|
||||
.send({ email: 'cansignin@example.com', password: 'a-brand-new-password' });
|
||||
expect(login.status).toBe(200);
|
||||
});
|
||||
|
||||
it('enforces the same minimum length as registration', async () => {
|
||||
const id = await passwordlessCustomer('short@example.com');
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', await sessionFor(id))
|
||||
.send({ newPassword: 'short' });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(await storedHash(id)).toBeNull();
|
||||
});
|
||||
|
||||
it('still demands the current one from an account that has a password', async () => {
|
||||
// The branch is on the stored hash, never on what the caller sends, so a
|
||||
// request cannot talk its way into the first-password case by omitting a
|
||||
// field.
|
||||
const id = await customerWithPassword('haspassword@example.com');
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', await sessionFor(id))
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
});
|
||||
|
||||
describe('signing in with a password', () => {
|
||||
it('is refused exactly as a wrong password is', async () => {
|
||||
await passwordlessCustomer('oracle@example.com');
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/login')
|
||||
.send({ email: 'oracle@example.com', password: 'anything-at-all' });
|
||||
|
||||
// Answering "this account has no password" would turn the login form into
|
||||
// an oracle for which customers use Google. One refusal for every cause,
|
||||
// and the account page is where a signed-in customer learns what they
|
||||
// have.
|
||||
expect(res.status).toBe(401);
|
||||
expect(res.body.error).toBe('invalid email or password');
|
||||
});
|
||||
|
||||
it('is refused for a blank password too, rather than matching an absent hash', async () => {
|
||||
await passwordlessCustomer('blank@example.com');
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/login')
|
||||
.send({ email: 'blank@example.com', password: '' });
|
||||
|
||||
// Both sides missing is the combination most tempting to call a match,
|
||||
// and calling it one would let anyone sign in as any Google-only customer.
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
});
|
||||
|
||||
describe('changing the email address', () => {
|
||||
it('is refused, and says why rather than claiming a password was wrong', async () => {
|
||||
const id = await passwordlessCustomer('moving@example.com');
|
||||
|
||||
const res = await request(app)
|
||||
.put('/api/customers/me/email')
|
||||
.set('Cookie', await sessionFor(id))
|
||||
.send({ email: 'somewhere-else@example.com' });
|
||||
|
||||
// Changing the address is a change to where recovery goes: whoever holds
|
||||
// the new one can reset the password and own the account outright. That is
|
||||
// why this route has always demanded more than a live session, and
|
||||
// dropping the demand for accounts that cannot meet it would remove the
|
||||
// protection from exactly the ones that need it.
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toMatch(/no password/);
|
||||
});
|
||||
|
||||
it('works once they have set one', async () => {
|
||||
const id = await passwordlessCustomer('thenmoving@example.com');
|
||||
const session = await sessionFor(id);
|
||||
await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', session)
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
const res = await request(app)
|
||||
.put('/api/customers/me/email')
|
||||
.set('Cookie', session)
|
||||
.send({ email: 'moved@example.com', currentPassword: 'a-brand-new-password' });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleting the account', () => {
|
||||
it('works, because deletion never asked for a password', async () => {
|
||||
const id = await passwordlessCustomer('deleting@example.com');
|
||||
|
||||
const res = await request(app).delete('/api/customers/me').set('Cookie', await sessionFor(id));
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
const { rows } = await pool.query<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM customers WHERE id = $1`,
|
||||
[id]
|
||||
);
|
||||
expect(requireRow(rows, 'a count of customers').n).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the passkey lockout guard, which becomes reachable here', () => {
|
||||
async function givePasskey(customerId: number, credentialId: string): Promise<number> {
|
||||
const { rows } = await pool.query<{ id: number }>(
|
||||
`INSERT INTO customer_credentials (customer_id, credential_id, public_key, name)
|
||||
VALUES ($1, $2, 'not-a-real-key', 'Phone') RETURNING id`,
|
||||
[customerId, credentialId]
|
||||
);
|
||||
return requireRow(rows, 'the credential just created').id;
|
||||
}
|
||||
|
||||
it('refuses to remove the last way into an account with no password', async () => {
|
||||
// Written in #40 against the condition rather than the schema, and
|
||||
// unreachable until now because password_hash was NOT NULL. This is the
|
||||
// first test that actually exercises it.
|
||||
const id = await passwordlessCustomer('lastway@example.com');
|
||||
const credentialId = await givePasskey(id, 'only-credential');
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/customers/me/passkeys/${credentialId}`)
|
||||
.set('Cookie', await sessionFor(id));
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toMatch(/only way you can sign in/);
|
||||
});
|
||||
|
||||
it('allows it when a second passkey remains', async () => {
|
||||
const id = await passwordlessCustomer('twokeys@example.com');
|
||||
const first = await givePasskey(id, 'credential-one');
|
||||
await givePasskey(id, 'credential-two');
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/customers/me/passkeys/${first}`)
|
||||
.set('Cookie', await sessionFor(id));
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
});
|
||||
|
||||
it('allows it once a password has been set', async () => {
|
||||
const id = await passwordlessCustomer('nowhaspassword@example.com');
|
||||
const credentialId = await givePasskey(id, 'credential-with-password');
|
||||
const session = await sessionFor(id);
|
||||
await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', session)
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/customers/me/passkeys/${credentialId}`)
|
||||
.set('Cookie', session);
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resetting a password that was never set', () => {
|
||||
it('gives them one, which is a reasonable answer rather than an error', async () => {
|
||||
await passwordlessCustomer('resetting@example.com');
|
||||
await request(app)
|
||||
.post('/api/customers/request-password-reset')
|
||||
.send({ email: 'resetting@example.com' });
|
||||
const { rows } = await pool.query<{ token: string }>(
|
||||
`SELECT t.token FROM customer_tokens t JOIN customers c ON c.id = t.customer_id
|
||||
WHERE c.email = $1 AND t.kind = 'password_reset'`,
|
||||
['resetting@example.com']
|
||||
);
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/reset-password')
|
||||
.send({ token: requireRow(rows, 'the reset token').token, password: 'a-brand-new-password' });
|
||||
|
||||
// The reset path sets a hash and does not care whether one was there
|
||||
// before. A customer who reaches for "forgot password" without ever
|
||||
// having had one gets a working password, which is what they were asking
|
||||
// for.
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('removes their passkeys, which is worth knowing rather than assuming', async () => {
|
||||
// #42 made a reset remove every passkey, on the reasoning that recovery
|
||||
// has to be complete. That still holds here: nothing about this path
|
||||
// identifies who asked, and a Google-only customer resetting a password
|
||||
// they never had is not obviously in a better position than one who did.
|
||||
const id = await passwordlessCustomer('resetkeys@example.com');
|
||||
await pool.query(
|
||||
`INSERT INTO customer_credentials (customer_id, credential_id, public_key, name)
|
||||
VALUES ($1, 'reset-credential', 'not-a-real-key', 'Phone')`,
|
||||
[id]
|
||||
);
|
||||
await request(app)
|
||||
.post('/api/customers/request-password-reset')
|
||||
.send({ email: 'resetkeys@example.com' });
|
||||
const { rows } = await pool.query<{ token: string }>(
|
||||
`SELECT t.token FROM customer_tokens t JOIN customers c ON c.id = t.customer_id
|
||||
WHERE c.email = $1 AND t.kind = 'password_reset'`,
|
||||
['resetkeys@example.com']
|
||||
);
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/customers/reset-password')
|
||||
.send({ token: requireRow(rows, 'the reset token').token, password: 'a-brand-new-password' });
|
||||
|
||||
expect(res.body.passkeysRemoved).toBe(1);
|
||||
});
|
||||
|
||||
it('leaves the Google identity attached, so they keep both ways in', async () => {
|
||||
const id = await passwordlessCustomer('keepsgoogle@example.com');
|
||||
await request(app)
|
||||
.post('/api/customers/request-password-reset')
|
||||
.send({ email: 'keepsgoogle@example.com' });
|
||||
const { rows } = await pool.query<{ token: string }>(
|
||||
`SELECT t.token FROM customer_tokens t JOIN customers c ON c.id = t.customer_id
|
||||
WHERE c.email = $1 AND t.kind = 'password_reset'`,
|
||||
['keepsgoogle@example.com']
|
||||
);
|
||||
|
||||
await request(app)
|
||||
.post('/api/customers/reset-password')
|
||||
.send({ token: requireRow(rows, 'the reset token').token, password: 'a-brand-new-password' });
|
||||
|
||||
// Deliberately not removed alongside the passkeys. A passkey is a
|
||||
// credential this shop issued and can revoke; a Google identity is one
|
||||
// Google holds, and severing it would leave the customer unable to use
|
||||
// the button they signed up with for no gain — whoever completed the
|
||||
// reset controls the mailbox either way.
|
||||
const { rows: identities } = await pool.query<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM customer_identities WHERE customer_id = $1`,
|
||||
[id]
|
||||
);
|
||||
expect(requireRow(identities, 'a count of identities').n).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('what the account page is told', () => {
|
||||
it('reports has_password false for a Google-only customer', async () => {
|
||||
const id = await passwordlessCustomer('told@example.com');
|
||||
|
||||
const res = await request(app).get('/api/customers/me').set('Cookie', await sessionFor(id));
|
||||
|
||||
expect(res.body.has_password).toBe(false);
|
||||
});
|
||||
|
||||
it('reports it true once one is set', async () => {
|
||||
const id = await passwordlessCustomer('nowtrue@example.com');
|
||||
const session = await sessionFor(id);
|
||||
await request(app)
|
||||
.post('/api/customers/change-password')
|
||||
.set('Cookie', session)
|
||||
.send({ newPassword: 'a-brand-new-password' });
|
||||
|
||||
const res = await request(app).get('/api/customers/me').set('Cookie', session);
|
||||
|
||||
expect(res.body.has_password).toBe(true);
|
||||
});
|
||||
|
||||
it('never returns the hash itself', async () => {
|
||||
const id = await customerWithPassword('nohash@example.com');
|
||||
|
||||
const res = await request(app).get('/api/customers/me').set('Cookie', await sessionFor(id));
|
||||
|
||||
expect(res.body.password_hash).toBeUndefined();
|
||||
expect(JSON.stringify(res.body)).not.toContain('$2');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,221 @@
|
||||
import crypto from 'node:crypto';
|
||||
import { authorizationUrl, codeChallenge, newAttempt, verifiedIdentity } from '../../src/google/oauth';
|
||||
import type { GoogleConfig } from '../../src/google/config';
|
||||
|
||||
const CONFIG: GoogleConfig = {
|
||||
clientId: 'id.apps.googleusercontent.com',
|
||||
clientSecret: 'shh',
|
||||
redirectUri: 'https://redefined-designs.com/api/auth/google/callback',
|
||||
enabled: true
|
||||
};
|
||||
|
||||
const NONCE = 'the-nonce-for-this-attempt';
|
||||
|
||||
/** An id token with the given claims. Unsigned, because nothing here reads a signature. */
|
||||
function idToken(claims: Record<string, unknown>): string {
|
||||
const part = (value: unknown) => Buffer.from(JSON.stringify(value)).toString('base64url');
|
||||
return `${part({ alg: 'RS256' })}.${part(claims)}.not-a-signature`;
|
||||
}
|
||||
|
||||
const VALID = {
|
||||
iss: 'https://accounts.google.com',
|
||||
aud: CONFIG.clientId,
|
||||
exp: Math.floor(Date.now() / 1000) + 3600,
|
||||
sub: '1234567890',
|
||||
nonce: NONCE,
|
||||
email: 'Customer@Example.com',
|
||||
email_verified: true,
|
||||
given_name: 'Test',
|
||||
family_name: 'Customer'
|
||||
};
|
||||
|
||||
/** VALID minus one claim, for the tests about a claim being absent. */
|
||||
function without(claim: keyof typeof VALID): Record<string, unknown> {
|
||||
const copy: Record<string, unknown> = { ...VALID };
|
||||
delete copy[claim];
|
||||
return copy;
|
||||
}
|
||||
|
||||
|
||||
describe('authorizationUrl', () => {
|
||||
const attempt = { state: 'st', nonce: 'no', codeVerifier: 'ver' };
|
||||
|
||||
it('sends the customer to Google with the code flow and PKCE', () => {
|
||||
const url = new URL(authorizationUrl(CONFIG, attempt));
|
||||
|
||||
expect(url.origin + url.pathname).toBe('https://accounts.google.com/o/oauth2/v2/auth');
|
||||
expect(url.searchParams.get('response_type')).toBe('code');
|
||||
expect(url.searchParams.get('client_id')).toBe(CONFIG.clientId);
|
||||
expect(url.searchParams.get('redirect_uri')).toBe(CONFIG.redirectUri);
|
||||
expect(url.searchParams.get('state')).toBe('st');
|
||||
expect(url.searchParams.get('nonce')).toBe('no');
|
||||
expect(url.searchParams.get('code_challenge_method')).toBe('S256');
|
||||
expect(url.searchParams.get('code_challenge')).toBe(codeChallenge('ver'));
|
||||
});
|
||||
|
||||
it('asks for exactly the three non-sensitive scopes', () => {
|
||||
// Anything beyond these turns publishing into a verification review with a
|
||||
// video walkthrough and a wait measured in weeks.
|
||||
const url = new URL(authorizationUrl(CONFIG, attempt));
|
||||
|
||||
expect(url.searchParams.get('scope')?.split(' ').sort()).toEqual(['email', 'openid', 'profile']);
|
||||
});
|
||||
|
||||
it('never asks for offline access', () => {
|
||||
// A refresh token would be a long-lived credential with nothing to spend it
|
||||
// on. Google issues one only when asked, so the check is that we do not ask.
|
||||
const url = new URL(authorizationUrl(CONFIG, attempt));
|
||||
|
||||
expect(url.searchParams.get('access_type')).toBeNull();
|
||||
expect(url.searchParams.get('prompt')).toBeNull();
|
||||
});
|
||||
|
||||
it('never puts the client secret in a URL the browser follows', () => {
|
||||
expect(authorizationUrl(CONFIG, attempt)).not.toContain(CONFIG.clientSecret);
|
||||
});
|
||||
});
|
||||
|
||||
describe('newAttempt', () => {
|
||||
it('mints three different secrets', () => {
|
||||
// Three rather than one reused: they are checked by different parties at
|
||||
// different moments, and a single value would mean anything learning it
|
||||
// from one check satisfies the others.
|
||||
const { state, nonce, codeVerifier } = newAttempt();
|
||||
|
||||
expect(new Set([state, nonce, codeVerifier]).size).toBe(3);
|
||||
});
|
||||
|
||||
it('does not repeat itself', () => {
|
||||
expect(newAttempt().state).not.toBe(newAttempt().state);
|
||||
});
|
||||
});
|
||||
|
||||
describe('codeChallenge', () => {
|
||||
it('is the base64url SHA-256 of the verifier, which is what S256 means', () => {
|
||||
const verifier = 'a-verifier';
|
||||
|
||||
expect(codeChallenge(verifier)).toBe(
|
||||
crypto.createHash('sha256').update(verifier).digest('base64url')
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The security of the whole flow lives here.
|
||||
*
|
||||
* No signature is verified, because the token arrives on a direct TLS
|
||||
* connection to Google's token endpoint — the case OpenID Connect explicitly
|
||||
* permits skipping it. That makes every one of these claim checks load-bearing
|
||||
* rather than belt-and-braces, so each has a test naming what accepting it
|
||||
* blindly would allow.
|
||||
*/
|
||||
describe('verifiedIdentity', () => {
|
||||
const expected = { clientId: CONFIG.clientId, nonce: NONCE };
|
||||
|
||||
it('accepts a well-formed token and reports who signed in', () => {
|
||||
const identity = verifiedIdentity(idToken(VALID), expected);
|
||||
|
||||
expect(identity.sub).toBe('1234567890');
|
||||
expect(identity.emailVerified).toBe(true);
|
||||
expect(identity.firstName).toBe('Test');
|
||||
expect(identity.lastName).toBe('Customer');
|
||||
});
|
||||
|
||||
it('normalises the email the way registration does', () => {
|
||||
// A stricter comparison than registration's would silently fail to match an
|
||||
// existing customer and produce a duplicate account instead (#343).
|
||||
expect(verifiedIdentity(idToken(VALID), expected).email).toBe('customer@example.com');
|
||||
});
|
||||
|
||||
it('accepts both spellings of the issuer, because Google sends both', () => {
|
||||
// Accepting only one produces sign-ins that fail for some customers and not
|
||||
// others, which is about the least diagnosable failure this flow can have.
|
||||
for (const iss of ['https://accounts.google.com', 'accounts.google.com']) {
|
||||
expect(verifiedIdentity(idToken({ ...VALID, iss }), expected).sub).toBe('1234567890');
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses an issuer it was not told to trust', () => {
|
||||
expect(() => verifiedIdentity(idToken({ ...VALID, iss: 'https://evil.test' }), expected)).toThrow(
|
||||
/unexpected issuer/
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses a token minted for a different application', () => {
|
||||
// Without this, a token obtained by any other Google app could be replayed
|
||||
// here and would sign somebody in.
|
||||
expect(() =>
|
||||
verifiedIdentity(idToken({ ...VALID, aud: 'someone-else.apps.googleusercontent.com' }), expected)
|
||||
).toThrow(/different client/);
|
||||
});
|
||||
|
||||
it('refuses an expired token', () => {
|
||||
const expired = { ...VALID, exp: Math.floor(Date.now() / 1000) - 3600 };
|
||||
|
||||
expect(() => verifiedIdentity(idToken(expired), expected)).toThrow(/expired/);
|
||||
});
|
||||
|
||||
it('allows a minute of clock skew, so a healthy host does not refuse valid tokens', () => {
|
||||
const justExpired = { ...VALID, exp: Math.floor(Date.now() / 1000) - 5 };
|
||||
|
||||
expect(verifiedIdentity(idToken(justExpired), expected).sub).toBe('1234567890');
|
||||
});
|
||||
|
||||
it('refuses a token with no expiry at all', () => {
|
||||
expect(() => verifiedIdentity(idToken(without('exp')), expected)).toThrow(/no expiry/);
|
||||
});
|
||||
|
||||
it('refuses a token from a different sign-in attempt', () => {
|
||||
// This is what stops a token captured from one attempt being replayed into
|
||||
// another, which is the whole reason the nonce exists.
|
||||
expect(() => verifiedIdentity(idToken({ ...VALID, nonce: 'someone-elses' }), expected)).toThrow(
|
||||
/different sign-in attempt/
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses a token carrying no nonce', () => {
|
||||
expect(() => verifiedIdentity(idToken(without('nonce')), expected)).toThrow(
|
||||
/different sign-in attempt/
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses a token with no subject, which is the identity itself', () => {
|
||||
expect(() => verifiedIdentity(idToken(without('sub')), expected)).toThrow(/no subject/);
|
||||
});
|
||||
|
||||
it('refuses a token with no email', () => {
|
||||
expect(() => verifiedIdentity(idToken(without('email')), expected)).toThrow(/no email/);
|
||||
});
|
||||
|
||||
describe('email_verified', () => {
|
||||
it('is true only for the boolean, never a truthy string', () => {
|
||||
// The linking policy turns entirely on this flag (#343). Treating the
|
||||
// string "false" as verified is exactly the mistake that would make
|
||||
// auto-linking an account-takeover path.
|
||||
expect(verifiedIdentity(idToken({ ...VALID, email_verified: 'false' }), expected).emailVerified)
|
||||
.toBe(false);
|
||||
expect(verifiedIdentity(idToken({ ...VALID, email_verified: 'true' }), expected).emailVerified)
|
||||
.toBe(false);
|
||||
});
|
||||
|
||||
it('is false when the claim is missing', () => {
|
||||
expect(verifiedIdentity(idToken(without('email_verified')), expected).emailVerified).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
it('tolerates a profile with no names, because Google may send none', () => {
|
||||
const anonymous = { ...without('given_name') };
|
||||
delete anonymous.family_name;
|
||||
const identity = verifiedIdentity(idToken(anonymous), expected);
|
||||
|
||||
expect(identity.firstName).toBeNull();
|
||||
expect(identity.lastName).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['not a JWT', 'nonsense'],
|
||||
['a JWT whose payload is not JSON', 'aGVhZGVy.bm90LWpzb24.sig']
|
||||
])('refuses %s', (_label, token) => {
|
||||
expect(() => verifiedIdentity(token, expected)).toThrow();
|
||||
});
|
||||
});
|
||||
Binary file not shown.
+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.
|
||||
@@ -0,0 +1,155 @@
|
||||
# Google Sign-In Implementation Plan
|
||||
|
||||
> **Status: complete.** All six phases merged between 2026-09-10 and 2026-09-11. Boxes are checked as a record of what landed, not as work outstanding. Two claims in the original plan turned out to be false and are marked inline rather than deleted — see [Corrections](#corrections).
|
||||
|
||||
**Goal:** A customer can sign in with Google and arrive at exactly the session a password login produces; a returning customer reaches the same account rather than a second one; and an account with no password can still manage itself.
|
||||
|
||||
**Architecture:** Server-side OpenID Connect authorization code flow with PKCE, mounted at `/api/auth/google`. The browser never holds a token. A `customer_identities` table keyed on the provider's subject claim links a Google account to a customer, and `customers.password_hash` becomes nullable so a Google-only account can exist. Everything after the identity is established — account creation, linking, the disabled-account refusal, session creation — is shared with the paths that already existed.
|
||||
|
||||
**Tech Stack:** Express + TypeScript, Postgres, `node-pg-migrate`, React + antd, Jest (unit and integration), Playwright (e2e).
|
||||
|
||||
**Parent issue:** #332. **Phases:** #340 through #345.
|
||||
|
||||
**Ops reference:** `docs/ops/google-sign-in.md` — the console setup, the per-environment redirect URIs, and the cutover checklist.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- **One session implementation.** A social sign-in must end in the same `rd_session` cookie with the same flags, expiry and logout behaviour. It calls `signIn` from `customerSession.ts`, which password and passkey login already share. A second path that agrees today is a path that gets changed alone.
|
||||
- **Keyed on the subject claim, never the email.** An email is a display value its owner can change and a provider may reassign. Matching on it strands a customer who changes theirs and hands their account to whoever inherits the old address.
|
||||
- **`email_verified` is compared to the boolean, never tested for truthiness.** The string `"false"` is truthy, and the linking policy turns entirely on this flag.
|
||||
- **The redirect URI derives from `PUBLIC_URL`.** Google compares it as an exact string. One source, and it is the one already correct wherever email links work.
|
||||
- **Absent, not disabled, where unconfigured.** Being unconfigured is the normal state for local development, so the button must not appear at all rather than appear and fail.
|
||||
- **Disabled accounts are refused here too.** Enforcing it on one sign-in path and not another is how a disabled account keeps a way in.
|
||||
- **Consent wording is stored verbatim and must stay byte-identical** to what the customer saw (#56). A Google sign-up captures consent through the endpoints registration already uses.
|
||||
- **Commit style:** Conventional Commits, subject ending `(#34N)`, no hard wrapping in bodies.
|
||||
|
||||
## File Structure
|
||||
|
||||
**Created:**
|
||||
- `backend/migrations/1788200000000_add-social-identities.js` — the identities table, and the nullable password hash
|
||||
- `backend/src/google/config.ts` — client credentials and the derived redirect URI
|
||||
- `backend/src/google/oauth.ts` — the protocol: authorization URL, token exchange, claim verification
|
||||
- `backend/src/google/returnTo.ts` — the open-redirect guard, its own module so it is testable without a database
|
||||
- `backend/src/google/newCustomer.ts` — account creation from an identity
|
||||
- `backend/src/google/linkIdentity.ts` — the linking policy, and nothing else
|
||||
- `backend/src/routes/googleAuth.ts` — the two routes and the attempt cookie
|
||||
- `frontend/src/customer/GoogleSignInButton.tsx` — the button and Google's mark
|
||||
- `frontend/src/customer/Welcome.tsx` — the one-time consent step
|
||||
- `frontend/src/customer/ConnectedAccounts.tsx` — what the account is linked to
|
||||
- `docs/ops/google-sign-in.md` — the console setup and cutover checklist
|
||||
|
||||
**Modified:**
|
||||
- `backend/src/app.ts` — mounts the router, adds `googleSignIn` to the public config
|
||||
- `backend/src/routes/customers.ts` — null-safe password comparisons, first-password setting, `has_password`, the identities endpoint
|
||||
- `backend/src/passwordHashing.ts` — `passwordMatches`, which answers false for an absent hash instead of throwing
|
||||
- `backend/src/envValidation.ts` — the credentials are all-or-nothing
|
||||
- `backend/src/rateLimit.ts` — a limiter for the start route
|
||||
- `backend/src/db-kysely/schema.ts`, `backend/tests/integration/setup/testDb.ts` — the hand-maintained mirror and reset lists
|
||||
- `frontend/src/customer/AuthForm.tsx` — the button on both tabs, and the notice from a refused sign-in
|
||||
- `frontend/src/customer/AccountDetails.tsx` — set a first password rather than change one
|
||||
- `frontend/src/main.tsx`, `AuthRouteModal.tsx`, `AuthPromptModal.tsx` — the welcome route and the return path
|
||||
- `docker-compose.prod.yml`, `docker-compose.qa.yml` — credentials read from the stack
|
||||
|
||||
**Tests:** `googleConfig`, `googleOauth`, `googleReturnTo`, `passwordMatches` (unit); `googleSignIn`, `passwordlessAccounts` (integration); `auth.spec.ts` (e2e).
|
||||
|
||||
---
|
||||
|
||||
## Phase 0: Google Auth Platform setup
|
||||
|
||||
Console work, in the order of the left-hand nav. Full detail in `docs/ops/google-sign-in.md`.
|
||||
|
||||
- [x] Verify `redefined-designs.com` in Search Console, as a **Domain** property, with the same Google account used for the Cloud project
|
||||
- [x] **Branding** — app name, support email, authorized domain, home page and privacy links; no logo, which would trigger a brand review
|
||||
- [x] **Audience** — External, Testing, own account as a test user
|
||||
- [x] **Clients** — Web application, one redirect URI per environment
|
||||
- [x] **Data Access** — exactly `openid`, `email`, `profile`; anything sensitive turns publishing into a review
|
||||
- [x] **Verification Center** — confirm there is nothing to submit
|
||||
|
||||
## Phase 1: Groundwork (#340)
|
||||
|
||||
- [x] Make `customers.password_hash` nullable
|
||||
- [x] Add `customer_identities`, unique across `(provider, provider_sub)`
|
||||
- [x] Update the Kysely mirror, `REQUIRED_TABLES`, the truncate list and the schema-loss count
|
||||
- [x] Derive the redirect URI from `PUBLIC_URL`, with an `enabled` flag
|
||||
- [x] Add the credentials to environment validation, all-or-nothing
|
||||
- [x] Make the three bcrypt comparisons null-safe through one shared function
|
||||
|
||||
**The point of the shared function:** `bcrypt.compare` throws on a null hash rather than returning false, so a forgotten check answers a sign-in with a 500. On the login route that is also an oracle, because it happens for exactly the accounts that have no password.
|
||||
|
||||
## Phase 2: The round trip (#341)
|
||||
|
||||
- [x] Authorization code flow with PKCE
|
||||
- [x] State, nonce and verifier in one `httpOnly` cookie, `SameSite=Lax`, cleared on every path
|
||||
- [x] Verify the id token by its claims without a JWKS fetch, with the reasoning and its boundary written into the module
|
||||
- [x] Refuse a disabled account
|
||||
- [x] Guard the return path against becoming an open redirect
|
||||
- [x] Rate limit the start route
|
||||
|
||||
**`SameSite=Lax`, never `Strict`.** The callback is a cross-site top-level navigation. `Strict` withholds the cookie, the state check fails, and every sign-in is refused with an error that looks exactly like tampering.
|
||||
|
||||
## Phase 3: New accounts and consent (#342)
|
||||
|
||||
- [x] Create the customer and the identity in one transaction
|
||||
- [x] Both consents false, with no stored wording
|
||||
- [x] Land a new customer on `/welcome`, which asks with the same two sentences
|
||||
- [x] Take names from the profile as hints, tolerating their absence
|
||||
- [x] Mark the address verified only when Google asserts it; otherwise send the usual confirmation
|
||||
|
||||
**The consent problem:** a customer arriving through Google has never seen the checkboxes and could not have, because the redirect happens before anyone knows they are new. Creating the account with both false is lawful; asking immediately afterwards is what makes it honest.
|
||||
|
||||
## Phase 4: Linking (#343)
|
||||
|
||||
- [x] Link only when Google asserts `email_verified` and the address matches exactly
|
||||
- [x] Refuse otherwise, to a destination the customer can act on
|
||||
- [x] Refuse to link to a disabled account
|
||||
- [x] Show the linked account beside the passkeys
|
||||
|
||||
**The order is the policy.** The identity lookup runs first and nothing else is consulted when it matches, so an identity that has signed in before keeps working after the address changes on either side.
|
||||
|
||||
## Phase 5: Life without a password (#344)
|
||||
|
||||
- [x] One route sets a first password and changes an existing one, branching on the stored hash
|
||||
- [x] Refuse an email change until a password exists
|
||||
- [x] Leave login's single refusal exactly as it was
|
||||
- [x] Exercise the passkey lockout guard, unreachable since #40 and now live
|
||||
- [x] Report `has_password` to the account page, and nothing more
|
||||
|
||||
## Phase 6: The button (#345)
|
||||
|
||||
- [x] On both tabs, below the password form and the passkey option
|
||||
- [x] Google's mark inlined, per their identity guidelines
|
||||
- [x] The return path supplied by the caller, validated on the server
|
||||
- [x] Absent where unconfigured, from the public config flag
|
||||
|
||||
---
|
||||
|
||||
## Corrections
|
||||
|
||||
Two things the original plan asserted turned out to be false. Both are recorded rather than removed, because the reasoning errors are the useful part.
|
||||
|
||||
### QA could never run this
|
||||
|
||||
**Claimed:** `qa-redefined-designs.bermudalamb.synology.me` cannot carry a redirect URI, because Google requires the host to sit under a domain whose ownership is proved by DNS and Synology owns the domain above it. Therefore the feature could only be built locally until #313.
|
||||
|
||||
**Actually:** registering the URI works. QA has run Google sign-in since.
|
||||
|
||||
The claim was inferred from #285, where Cloudflare's free tier genuinely cannot be applied to that hostname, and stated as fact without being checked. On the strength of it, QA testing was documented as blocked, `docker-compose.qa.yml` hardcoded its credentials to empty rather than reading the stack, and a QA deploy was spent discovering otherwise.
|
||||
|
||||
What was actually established is narrower: `localhost` is exempt from the authorized-domain requirement, and a domain listed under Authorized domains has to be verified in Search Console. Whether either applied here was never tested.
|
||||
|
||||
### The local redirect URI
|
||||
|
||||
**Claimed:** register `http://localhost:3000/api/auth/google/callback` for local development.
|
||||
|
||||
**Actually:** local development browses the Vite dev server on 5173, so that is the URI the server sends and the one that must be registered. The 3000 entry applies only when the backend serves a built frontend, which local development does not produce. The omission cost an hour of `redirect_uri_mismatch`.
|
||||
|
||||
### Also corrected during the work
|
||||
|
||||
- **Account deletion does not confirm with a password.** #332 recorded that it does. The route takes none, so it needed no change, and there is now a test pinning that.
|
||||
- **The button was missing from the sign-up tab.** #345 rendered it inside the Log In tab only, following the passkey button too closely. A passkey belongs only on Log In because you cannot register an account with one; creating an account is precisely what a customer reaches for Google to do. Fixed before the feature was used in anger.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- **Unlinking a Google account** is not offered. Removing the only way into an account is guarded for passkeys and would need the same guard here.
|
||||
- **Apple** is decided against on #332: a paid programme, a client secret expiring every six months, no `localhost` redirect URIs, and a name and email returned exactly once.
|
||||
- **Google One Tap** has its own plan and a recommendation to wait. The two blockers are that a browser-supplied id token makes signature verification mandatory, and that its script must load for signed-out visitors, which collides with the consent rule in `frontend/src/brevo.ts`.
|
||||
@@ -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> {
|
||||
|
||||
@@ -12,6 +12,7 @@ import { setFavoriteAlerts } from './favoritesApi';
|
||||
import { useCustomerAuth } from './CustomerAuthContext';
|
||||
import AccountDetails from './AccountDetails';
|
||||
import Passkeys from './Passkeys';
|
||||
import ConnectedAccounts from './ConnectedAccounts';
|
||||
|
||||
const { Text } = Typography;
|
||||
|
||||
@@ -178,6 +179,8 @@ export default function Account({ onClose }: Props) {
|
||||
cannot do this is not offered a button that fails (#40). */}
|
||||
<Passkeys />
|
||||
|
||||
<ConnectedAccounts />
|
||||
|
||||
<Divider />
|
||||
<Space wrap>
|
||||
{/* Order history is a page of its own now. The link stays here because
|
||||
|
||||
@@ -30,6 +30,10 @@ export default function AccountDetails({ customer, onChanged }: Props) {
|
||||
const [passwordForm] = Form.useForm();
|
||||
const [emailForm] = Form.useForm();
|
||||
|
||||
// A customer who signed up with Google has none, which changes the wording,
|
||||
// the button, and whether a current-password field exists at all (#344).
|
||||
const hasPassword = customer.has_password;
|
||||
|
||||
async function saveName(values: { firstName: string; lastName: string }) {
|
||||
setBusy('name');
|
||||
setNameError(null);
|
||||
@@ -59,15 +63,21 @@ export default function AccountDetails({ customer, onChanged }: Props) {
|
||||
}
|
||||
}
|
||||
|
||||
async function savePassword(values: { currentPassword: string; newPassword: string }) {
|
||||
async function savePassword(values: { currentPassword?: string; newPassword: string }) {
|
||||
setBusy('password');
|
||||
setPasswordError(null);
|
||||
try {
|
||||
await changeMyPassword(values.currentPassword, values.newPassword);
|
||||
// Nothing to refresh: this session is deliberately the one kept alive.
|
||||
// Clearing the fields matters more, since they hold both passwords.
|
||||
await changeMyPassword(values.currentPassword ?? '', values.newPassword);
|
||||
// Clearing the fields matters more than anything else here, since they
|
||||
// hold both passwords. Setting a first one does refresh, because
|
||||
// has_password has just changed and this panel renders from it.
|
||||
passwordForm.resetFields();
|
||||
message.success('Password changed. Other devices have been signed out.');
|
||||
if (hasPassword) {
|
||||
message.success('Password changed. Other devices have been signed out.');
|
||||
} else {
|
||||
onChanged();
|
||||
message.success('Password set. You can now sign in with it as well as with Google.');
|
||||
}
|
||||
} catch (err) {
|
||||
setPasswordError((err as Error).message);
|
||||
} finally {
|
||||
@@ -151,23 +161,33 @@ export default function AccountDetails({ customer, onChanged }: Props) {
|
||||
},
|
||||
{
|
||||
key: 'password',
|
||||
label: 'Change your password',
|
||||
// Named for what it is for this customer. Offering to change a
|
||||
// password to somebody who signed up with Google and has never had
|
||||
// one is a dead end (#344).
|
||||
label: hasPassword ? 'Change your password' : 'Set a password',
|
||||
children: (
|
||||
<>
|
||||
<Paragraph type="secondary">
|
||||
Signing in elsewhere will end. You will stay signed in on this device.
|
||||
{hasPassword
|
||||
? 'Signing in elsewhere will end. You will stay signed in on this device.'
|
||||
: 'You signed up without a password. Setting one gives you a second way in, alongside the accounts listed below.'}
|
||||
</Paragraph>
|
||||
{passwordError && (
|
||||
<Alert type="error" showIcon message={passwordError} style={{ marginBottom: 16 }} />
|
||||
)}
|
||||
<Form layout="vertical" form={passwordForm} onFinish={savePassword}>
|
||||
<Form.Item
|
||||
name="currentPassword"
|
||||
label="Current password"
|
||||
rules={[{ required: true, message: 'Your current password is required' }]}
|
||||
>
|
||||
<Input.Password autoComplete="current-password" />
|
||||
</Form.Item>
|
||||
{/* Absent, not disabled, for an account that has none. The
|
||||
server branches on the stored hash rather than on anything
|
||||
sent, so there is nothing for this field to carry. */}
|
||||
{hasPassword && (
|
||||
<Form.Item
|
||||
name="currentPassword"
|
||||
label="Current password"
|
||||
rules={[{ required: true, message: 'Your current password is required' }]}
|
||||
>
|
||||
<Input.Password autoComplete="current-password" />
|
||||
</Form.Item>
|
||||
)}
|
||||
<Form.Item
|
||||
name="newPassword"
|
||||
label="New password"
|
||||
@@ -193,7 +213,7 @@ export default function AccountDetails({ customer, onChanged }: Props) {
|
||||
</Form.Item>
|
||||
<Form.Item style={{ marginBottom: 0 }}>
|
||||
<Button type="primary" htmlType="submit" loading={busy === 'password'}>
|
||||
Change password
|
||||
{hasPassword ? 'Change password' : 'Set password'}
|
||||
</Button>
|
||||
</Form.Item>
|
||||
</Form>
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
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';
|
||||
import Button from 'antd/es/button';
|
||||
@@ -9,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;
|
||||
|
||||
@@ -41,13 +44,49 @@ 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;
|
||||
}>;
|
||||
|
||||
/**
|
||||
* What a Google sign-in that ended badly wants the login form to say (#343).
|
||||
*
|
||||
* Read from the query string because the callback is a redirect: it cannot
|
||||
* return a body, and the customer's browser arrives here having been sent by
|
||||
* Google. A parameter is the only channel there is.
|
||||
*
|
||||
* `google-use-password` is the interesting one. It means the customer has an
|
||||
* account and simply cannot reach it this way, which is the single refusal in
|
||||
* this flow they can act on — so it says what to do rather than what failed.
|
||||
*
|
||||
* It reveals nothing they did not already supply. They arrived holding a Google
|
||||
* account for this address, so being told the address has an account here tells
|
||||
* them only about themselves.
|
||||
*/
|
||||
function googleNotice(reason: string | null): string | null {
|
||||
if (reason === 'google-use-password') {
|
||||
return 'You already have an account with this email address. Log in with your password below.';
|
||||
}
|
||||
if (reason === 'google-failed') {
|
||||
return 'That Google sign-in did not work. You can log in with your password instead.';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// The one implementation of signing in and registering. It was previously
|
||||
// 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);
|
||||
const [loading, setLoading] = useState(false);
|
||||
// Separate from `loading`, so the password button does not sit disabled and
|
||||
@@ -61,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);
|
||||
@@ -107,6 +160,12 @@ export default function AuthForm({ mode, onModeChange, onForgotPassword, onSucce
|
||||
|
||||
return (
|
||||
<>
|
||||
{/* The notice sits above the tabs and below any live error, because it
|
||||
describes how the customer arrived rather than what they just did. An
|
||||
error from this form supersedes it. */}
|
||||
{!error && notice && (
|
||||
<Alert type="info" showIcon message={notice} style={{ marginBottom: 16 }} />
|
||||
)}
|
||||
{error && <Alert type="error" showIcon message={error} style={{ marginBottom: 16 }} />}
|
||||
<Tabs
|
||||
activeKey={mode}
|
||||
@@ -174,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>
|
||||
)
|
||||
},
|
||||
@@ -204,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}
|
||||
@@ -224,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,70 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import Typography from 'antd/es/typography';
|
||||
import Tag from 'antd/es/tag';
|
||||
import Spin from 'antd/es/spin';
|
||||
import { fetchIdentities, Identity } from './customerApi';
|
||||
|
||||
const { Title, Text, Paragraph } = Typography;
|
||||
|
||||
const PROVIDER_NAMES: Record<string, string> = { google: 'Google' };
|
||||
|
||||
/**
|
||||
* Which identity providers this account can be signed in with (#343).
|
||||
*
|
||||
* Linking happens automatically when Google vouches for an address that already
|
||||
* has an account here. That is defensible — whoever completed the sign-in
|
||||
* demonstrably controls the mailbox, which is already the root of trust for a
|
||||
* password reset — but it is not obvious, and a customer who signed up with a
|
||||
* password has had two credentials joined without being asked.
|
||||
*
|
||||
* A silent link is indistinguishable from a bug when somebody later wonders why
|
||||
* the password is no longer needed. So it is shown, beside the passkeys, for the
|
||||
* reason the passkey list exists at all: a customer cannot manage credentials
|
||||
* they cannot see.
|
||||
*
|
||||
* Read-only for now. Removing the only way into an account is the question #344
|
||||
* settles, and offering an unlink button before that check runs would be the
|
||||
* fastest way to lock somebody out of their own orders.
|
||||
*/
|
||||
export default function ConnectedAccounts() {
|
||||
const [identities, setIdentities] = useState<Identity[]>([]);
|
||||
const [loading, setLoading] = useState(true);
|
||||
|
||||
useEffect(() => {
|
||||
fetchIdentities()
|
||||
.then(setIdentities)
|
||||
// Silent. This is a supplementary panel on a page whose real content is
|
||||
// elsewhere, and a red error over the account settings because one extra
|
||||
// read failed would be worse than the panel simply not appearing.
|
||||
.catch(() => setIdentities([]))
|
||||
.finally(() => setLoading(false));
|
||||
}, []);
|
||||
|
||||
// Nothing at all for the overwhelming majority, who have never used a
|
||||
// provider. An empty state here would appear on every account page to say
|
||||
// that nothing had happened.
|
||||
if (loading) return <Spin />;
|
||||
if (identities.length === 0) return null;
|
||||
|
||||
return (
|
||||
<>
|
||||
<Title level={5}>Connected accounts</Title>
|
||||
<Paragraph type="secondary" style={{ fontSize: 13 }}>
|
||||
You can sign in with these as well as with your password.
|
||||
</Paragraph>
|
||||
{identities.map((identity) => (
|
||||
<div key={identity.provider} style={{ marginBottom: 8 }}>
|
||||
<Tag color="blue">{PROVIDER_NAMES[identity.provider] ?? identity.provider}</Tag>
|
||||
<Text type="secondary" style={{ fontSize: 12 }}>
|
||||
{/* Last used rather than connected, for the reason the passkey list
|
||||
shows it: it is what tells a customer whether something is still
|
||||
theirs, where a connection date says only that it happened. */}
|
||||
{identity.last_used_at
|
||||
? `last used ${new Date(identity.last_used_at).toLocaleDateString()}`
|
||||
: 'never used to sign in'}
|
||||
</Text>
|
||||
</div>
|
||||
))}
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -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>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,114 @@
|
||||
import { useState } from 'react';
|
||||
import Modal from 'antd/es/modal';
|
||||
import Checkbox from 'antd/es/checkbox';
|
||||
import Button from 'antd/es/button';
|
||||
import Space from 'antd/es/space';
|
||||
import Alert from 'antd/es/alert';
|
||||
import Typography from 'antd/es/typography';
|
||||
import { updateConsent, updateAnalyticsConsent } from './customerApi';
|
||||
import { MARKETING_CONSENT_TEXT, ANALYTICS_CONSENT_TEXT } from './AuthForm';
|
||||
|
||||
const { Paragraph, Title } = Typography;
|
||||
|
||||
type Props = Readonly<{ onClose: () => void }>;
|
||||
|
||||
/**
|
||||
* The consent step a customer sees once, right after signing up with Google (#342).
|
||||
*
|
||||
* ## Why this screen has to exist
|
||||
*
|
||||
* Registration asks for two consents and stores their wording verbatim, and
|
||||
* marketing consent must start unticked (#56). Somebody who arrived through
|
||||
* Google has never seen those checkboxes and could not have: the redirect
|
||||
* happened before anyone knew whether they were new.
|
||||
*
|
||||
* Their account is created with both false, which is legally correct — nobody
|
||||
* agreed to anything and nothing is recorded as though they had. But leaving it
|
||||
* there would mean a Google sign-up is never asked at all, and a silent no is
|
||||
* still a decision made on someone else's behalf.
|
||||
*
|
||||
* ## Why the wording is imported rather than written here
|
||||
*
|
||||
* These two constants are the same strings the server stores against the
|
||||
* consent. The record is meant to say what the customer actually saw, so a
|
||||
* second copy of the sentence that drifted by a word would quietly defeat that.
|
||||
* Three wordings were already in circulation once before this was shared.
|
||||
*
|
||||
* ## Why skipping is a real option, not a soft refusal
|
||||
*
|
||||
* Consent has to be as easy to withhold as to give. "Not now" leaves both false
|
||||
* and closes, and nothing is sent. Both can be changed later from the account
|
||||
* page, which is where a customer who changes their mind will look.
|
||||
*/
|
||||
export default function Welcome({ onClose }: Props) {
|
||||
const [marketing, setMarketing] = useState(false);
|
||||
const [analytics, setAnalytics] = useState(false);
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
async function save() {
|
||||
setSaving(true);
|
||||
setError(null);
|
||||
try {
|
||||
// Two calls to two endpoints, which is the point rather than an
|
||||
// inefficiency: they are separate consents with separate purposes, and
|
||||
// the server stores the wording for each independently.
|
||||
await updateConsent(marketing);
|
||||
await updateAnalyticsConsent(analytics);
|
||||
onClose();
|
||||
} catch (err) {
|
||||
// The account exists and they are signed in either way, so this is not a
|
||||
// failure to recover from — only a preference that did not save.
|
||||
setError(`Those preferences didn't save — ${(err as Error).message}. You can set them on your account page.`);
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<Modal
|
||||
title="Welcome to Redefined Designs"
|
||||
open
|
||||
onCancel={onClose}
|
||||
footer={null}
|
||||
style={{ maxWidth: 'calc(100vw - 32px)' }}
|
||||
destroyOnHidden
|
||||
>
|
||||
<Paragraph type="secondary">
|
||||
Your account is ready and you are signed in. Two optional things, and you can change
|
||||
either of them later on your account page.
|
||||
</Paragraph>
|
||||
|
||||
{error && <Alert type="warning" showIcon message={error} style={{ marginBottom: 16 }} />}
|
||||
|
||||
<Space direction="vertical" size="middle" style={{ display: 'flex' }}>
|
||||
<Checkbox checked={marketing} onChange={(e) => setMarketing(e.target.checked)}>
|
||||
{MARKETING_CONSENT_TEXT}
|
||||
</Checkbox>
|
||||
{/* Its own checkbox and independently refusable. Someone has to be able
|
||||
to take the emails and refuse the tracking, or the consent is not
|
||||
granular and is not valid. Unticked, and never pre-ticked: Quebec's
|
||||
Law 25 requires profiling to be off until the person switches it on. */}
|
||||
<Checkbox checked={analytics} onChange={(e) => setAnalytics(e.target.checked)}>
|
||||
{ANALYTICS_CONSENT_TEXT}
|
||||
</Checkbox>
|
||||
</Space>
|
||||
|
||||
<Space style={{ marginTop: 24 }}>
|
||||
<Button type="primary" loading={saving} onClick={save}>
|
||||
Save preferences
|
||||
</Button>
|
||||
{/* As prominent as it needs to be. Withholding consent has to be as
|
||||
easy as giving it, and a "Not now" hidden in small print is the
|
||||
pattern that makes a consent invalid. */}
|
||||
<Button onClick={onClose} disabled={saving}>
|
||||
Not now
|
||||
</Button>
|
||||
</Space>
|
||||
|
||||
<Title level={5} style={{ marginTop: 24, fontSize: 13, opacity: 0.65 }}>
|
||||
Leaving both unticked is fine — we will not email you or share what you browse.
|
||||
</Title>
|
||||
</Modal>
|
||||
);
|
||||
}
|
||||
@@ -18,6 +18,14 @@ export interface Customer {
|
||||
*/
|
||||
analytics_consent: boolean;
|
||||
favorite_alerts: boolean;
|
||||
/**
|
||||
* Whether this account has a password at all (#344).
|
||||
*
|
||||
* False for anyone who signed up with Google. The account page reads it to
|
||||
* decide between offering to change a password and offering to set a first
|
||||
* one, which are different things to somebody who has never had one.
|
||||
*/
|
||||
has_password: boolean;
|
||||
created_at: string;
|
||||
}
|
||||
|
||||
@@ -183,6 +191,17 @@ export function changeMyEmail(currentPassword: string, email: string): Promise<C
|
||||
}).then(res => handle<Customer>(res));
|
||||
}
|
||||
|
||||
/** One identity provider this account can sign in with (#343). */
|
||||
export interface Identity {
|
||||
provider: string;
|
||||
created_at: string;
|
||||
last_used_at: string | null;
|
||||
}
|
||||
|
||||
export function fetchIdentities(): Promise<Identity[]> {
|
||||
return fetch('/api/customers/me/identities').then(res => handle<Identity[]>(res));
|
||||
}
|
||||
|
||||
/** A registered passkey, as the account page lists it (#40). */
|
||||
export interface Passkey {
|
||||
id: number;
|
||||
|
||||
+12
-3
@@ -13,6 +13,7 @@ import ErrorFallback from './components/ErrorFallback';
|
||||
import DevThrow from './components/DevThrow';
|
||||
import Admin from './admin/Admin';
|
||||
import AuthRouteModal from './customer/AuthRouteModal';
|
||||
import Welcome from './customer/Welcome';
|
||||
import Account from './customer/Account';
|
||||
import PrivacyPolicy from './customer/PrivacyPolicy';
|
||||
import Submit from './intake/Submit';
|
||||
@@ -45,7 +46,7 @@ const STOREFRONT_BACKDROP: Partial<Location> = { pathname: '/', search: '', hash
|
||||
// than as a page of their own. Each stays a real, linkable URL — bookmarkable,
|
||||
// refreshable, and closed by the browser's Back button — while never being
|
||||
// somewhere with no way out.
|
||||
const MODAL_ROUTES = ['/account', '/login', '/register', '/forgot-password', '/reset-password'];
|
||||
const MODAL_ROUTES = ['/account', '/login', '/register', '/forgot-password', '/reset-password', '/welcome'];
|
||||
|
||||
// Respects the OS-level "reduce motion" accessibility setting by turning off
|
||||
// antd's transitions. Beyond the accessibility win, animated popups are a
|
||||
@@ -147,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
|
||||
@@ -192,14 +197,18 @@ 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')} />
|
||||
)}
|
||||
{/* One-time, right after a Google sign-up (#342). A route rather than
|
||||
a flag so it has an address and uses the same modal machinery as
|
||||
every other auth screen. */}
|
||||
{modalPath === '/welcome' && <Welcome onClose={closeModal} />}
|
||||
{modalPath === '/reset-password' && (
|
||||
<ResetPassword
|
||||
onClose={closeModal}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
+29
-4
@@ -280,11 +280,36 @@ Reserved ranges: netsh interface ipv4 show excludedportrange protocol=tcp
|
||||
function Install-IfMissing {
|
||||
param([string]$Directory)
|
||||
$name = Split-Path -Leaf $Directory
|
||||
if (Test-Path (Join-Path $Directory 'node_modules')) {
|
||||
Write-Note "$name dependencies already installed"
|
||||
return
|
||||
|
||||
# Whether node_modules matches the lockfile, not merely whether it exists.
|
||||
#
|
||||
# This used to ask only `Test-Path node_modules`, which meant a branch that
|
||||
# ADDED a dependency never installed it for anyone who already had the
|
||||
# directory — and almost everyone always does. The build then failed on
|
||||
# "Cannot find module", naming a package that is right there in
|
||||
# package.json, which reads as a broken checkout rather than a missing
|
||||
# install. It cost an afternoon the first time #37 added @simplewebauthn.
|
||||
#
|
||||
# npm writes node_modules/.package-lock.json describing exactly what it put
|
||||
# there, so comparing its timestamp against package-lock.json answers the
|
||||
# real question: is what is installed what is currently asked for. A pull
|
||||
# that changes dependencies makes the lockfile newer, and this notices.
|
||||
$lockfile = Join-Path $Directory 'package-lock.json'
|
||||
$installed = Join-Path $Directory 'node_modules/.package-lock.json'
|
||||
|
||||
if ((Test-Path $installed) -and (Test-Path $lockfile)) {
|
||||
$lockTime = (Get-Item $lockfile).LastWriteTimeUtc
|
||||
$installedTime = (Get-Item $installed).LastWriteTimeUtc
|
||||
if ($installedTime -ge $lockTime) {
|
||||
Write-Note "$name dependencies are up to date"
|
||||
return
|
||||
}
|
||||
Write-Step "Installing $name dependencies (the lockfile has changed)"
|
||||
}
|
||||
Write-Step "Installing $name dependencies"
|
||||
else {
|
||||
Write-Step "Installing $name dependencies"
|
||||
}
|
||||
|
||||
Push-Location $Directory
|
||||
try { Invoke-Checked { npm install } "$name npm install" } finally { Pop-Location }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user