diff --git a/backend/src/app.ts b/backend/src/app.ts index 46957b5..9f06e67 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -19,6 +19,7 @@ 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 publicRouter from './routes/public'; import cartRouter from './routes/cart'; import shippingAddressesRouter from './routes/shippingAddresses'; @@ -105,6 +106,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); diff --git a/backend/src/google/oauth.ts b/backend/src/google/oauth.ts new file mode 100644 index 0000000..6f72b23 --- /dev/null +++ b/backend/src/google/oauth.ts @@ -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 { + 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) + }; +} diff --git a/backend/src/google/returnTo.ts b/backend/src/google/returnTo.ts new file mode 100644 index 0000000..d330ab8 --- /dev/null +++ b/backend/src/google/returnTo.ts @@ -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; +} diff --git a/backend/src/rateLimit.ts b/backend/src/rateLimit.ts index 4498cc9..2fdd1f6 100644 --- a/backend/src/rateLimit.ts +++ b/backend/src/rateLimit.ts @@ -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' } +}); diff --git a/backend/src/routes/googleAuth.ts b/backend/src/routes/googleAuth.ts new file mode 100644 index 0000000..0985f76 --- /dev/null +++ b/backend/src/routes/googleAuth.ts @@ -0,0 +1,213 @@ +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 { 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 phase does and does not do + * + * It signs in a customer whose Google identity is **already linked**. A + * successful sign-in by somebody with no identity row does nothing yet: account + * creation is #342 and the linking policy is #343, and holding them back keeps + * this change about the protocol alone. + * + * ## 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'; + +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; + 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)); +} + +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( + `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]; + + // No identity row means a customer this shop has never seen through Google. + // Creating one is #342 and linking to an existing account is #343; until + // those land there is nothing to do, and doing nothing must not look like a + // protocol failure. + if (!linked) return res.redirect(FAILURE_PATH); + + // 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 }; + +export default router; diff --git a/backend/tests/integration/googleSignIn.integration.test.ts b/backend/tests/integration/googleSignIn.integration.test.ts new file mode 100644 index 0000000..023d2aa --- /dev/null +++ b/backend/tests/integration/googleSignIn.integration.test.ts @@ -0,0 +1,356 @@ +import request from 'supertest'; +import app from '../../src/app'; +import { pool, requireRow } from '../../src/db'; +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 { + 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 = {}) { + 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 { + 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 { + await pool.query( + `INSERT INTO customer_identities (customer_id, provider, provider_sub) + VALUES ($1, 'google', $2)`, + [customerId, sub] + ); +} + +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('does not sign in an unlinked Google 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); + + // Account creation is #342 and linking is #343. Matching on the address + // here would be the takeover path both of those exist to decide 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); + }); +}); diff --git a/backend/tests/unit/googleOauth.test.ts b/backend/tests/unit/googleOauth.test.ts new file mode 100644 index 0000000..8a41b00 --- /dev/null +++ b/backend/tests/unit/googleOauth.test.ts @@ -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 { + 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 { + const copy: Record = { ...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(); + }); +}); diff --git a/backend/tests/unit/googleReturnTo.test.ts b/backend/tests/unit/googleReturnTo.test.ts new file mode 100644 index 0000000..c20fa5b Binary files /dev/null and b/backend/tests/unit/googleReturnTo.test.ts differ