feat(auth): the Google sign-in round trip (#341)
Two routes, and a customer whose Google identity is already linked can sign in. Creating accounts and linking them are deliberately held back to the next two issues, so this change is about the protocol alone and can be reviewed as such. The authorization code flow, with PKCE. The browser is sent to Google, comes back carrying a code, and this server exchanges it over its own TLS connection, so nothing that can read the page ever holds a token. PKCE goes in even though this is a confidential client with a secret: it costs one hash and closes code interception outright rather than resting the whole flow on the secret staying secret. No JWKS fetch, and the reasoning is written into the module rather than left to be rediscovered. The id token arrives in the response to a request this server made, over TLS, directly to Google's token endpoint, which is exactly the case OpenID Connect permits skipping signature verification for. That removes a key fetch, a cache and a rotation path from the part of the codebase least worth having moving parts in. It removes none of the claim checks, and the comment says plainly that the moment an id token reaches this code from anywhere else, the reasoning stops holding. So the claim checks are load-bearing rather than belt and braces, and each has a test naming what accepting it blindly would allow. A wrong audience is a token minted for another application being replayed here. A wrong nonce is a token from an earlier attempt. A missing subject is an identity row keyed on nothing. Both spellings of the issuer are accepted because Google really does send both, and taking only one produces sign-ins that fail for some customers and not others. email_verified is compared to the boolean and never merely tested for truthiness. The string "false" is truthy, and the linking policy turns entirely on this flag, so that one line is the difference between a policy and an account-takeover path. The attempt cookie is the whole security of the callback, which is a plain GET anyone on the internet can invoke. It carries three secrets, minted separately because they are checked by different parties at different moments: state proves the callback belongs to the request this browser started, nonce proves the token was minted for this attempt, and the verifier proves the code is being spent by whoever asked for it. It is cleared on every path through the callback, so one attempt cannot be replayed even once. SameSite is Lax and not Strict, and that line has the longest comment in the file because it is the most expensive thing here to get wrong. The callback arrives as a cross-site top-level navigation; Strict withholds the cookie, the state check then fails, and every sign-in is refused with an error that looks exactly like tampering. Where the customer returns to survives the round trip in that cookie, and it is a value an attacker can propose. Unchecked, the start route is an open redirect wearing a sign-in flow as a disguise: a link on our own domain, with our own certificate, that lands somewhere else. Its own module, so it can be tested without a database and so the next path needing the same question has an obvious place to ask it. Its first draft used a regex that inverted its own character class and rejected every path, which passed every other test and would have broken every real sign-in — there is now a test for exactly that. Declining at Google's consent screen is a cancellation rather than a failure. The customer goes back where they were with nothing said, the same distinction #41 drew for a dismissed passkey prompt. A disabled account is refused here as well, because enforcing it on some sign-in routes and not others is how a disabled account keeps a way in. Signing in calls the shared function, not a third implementation that agrees today. There is a test that the resulting session is accepted by an unrelated route, which is what makes that sharing worth something rather than merely tidy. Verified: backend tsc clean for src and tests, 590 unit tests pass, lint back to the seven warnings that predate this branch. The integration suite covers the routes end to end with only the token exchange stubbed, and needs a database this machine has no Docker for. Closes #341 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
607881b711
commit
87f07baaff
@@ -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, 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]
|
||||
);
|
||||
}
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user