Compare commits
20
Commits
607881b711
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
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.
|
||||
@@ -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