bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

feat(auth): create an account from a Google identity, then ask about consent (#342)

Browse Source 
A Google account nobody here has seen now becomes a customer. The OAuth part of this was the easy half; the problem worth the issue is consent.

Registration asks for two consents and stores their wording verbatim, and marketing consent must start unticked. Somebody arriving through Google has never seen those checkboxes and could not have, because the redirect happened before anyone knew whether they were new.

Creating the account with both false is legally correct: nobody agreed to anything, and nothing is recorded as though they had. There is no stored wording either, because a wording saved against a false consent is a record of a conversation that never happened. But stopping there would mean a Google sign-up is never asked at all, and a silent no is still a decision made on somebody else's behalf.

So the account is created, the customer is signed in, and they land on a step that shows the same two sentences with the same two unticked boxes. It saves through the endpoints registration already uses, which is what keeps the stored text byte-identical rather than merely similar. Not now is offered as an equal option, because consent has to be as easy to withhold as to give, and both can be changed later from the account page.

The wording on that screen is imported from the shared constants rather than retyped. Three different wordings were already in circulation once before that was shared, and the record is meant to say what the customer actually saw.

The return path is deliberately dropped for a new customer, who lands on the consent step instead. Carrying it through as a query parameter was the alternative and was rejected: the consent page would then redirect somewhere a URL told it to, which is the open-redirect question already answered 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 much the cheaper of the two.

The customer and the identity are inserted in one transaction. A customer row with no identity is an account nobody can sign in to and nobody can recover, because it has no password either.

Signing up is refused when the address already belongs to a customer. Joining those two accounts is linking, it is the most security-sensitive decision in this project, and it belongs to the next issue rather than falling out of an INSERT here. Refusing is the safe half of that decision and the only half available until the policy is written down. The unique index rather than the preceding SELECT is what actually holds when two sign-ins race, so losing that race is treated as the address being taken rather than as an error.

Google's assertion about the address is taken only when it is the boolean true. When it holds, the account is marked verified and no confirmation email is sent, because that email exists to prove the customer receives mail at the address and Google has just proved exactly that. When it does not, the account is unverified and goes through the ordinary confirmation, because an unverified assertion is worth nothing.

Names from the profile are hints. Registration demands both because every email greets by first name, but Google may return neither and refusing a sign-in over it would be absurd — the greeting already has a fallback for exactly this case.

The tests worth reading are the two about a returning customer. One signs in again and reaches the same account; the other changes their Google address first and still reaches it. That second one is the whole reason the identity is keyed on the subject claim: an email match would have created a second account there, and an address that had since been reassigned would have handed the first one to a stranger.

Verified: backend tsc clean for src and tests, 590 unit tests pass, lint at the seven warnings that predate this branch, frontend tsc, lint and build clean. The integration suite needs a database this machine has no Docker for.

Closes #342

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
synAdmin
2026-09-10 10:11:16 -05:00
committed by bermudalamb
co-authored by Claude Opus 5
parent 79a2b606ea
commit 25078417e5
5 changed files with 481 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/integration/googleSignIn.integration.test.ts
+186 -3
View File
@@ -335,7 +335,7 @@ describe('GET /api/auth/google/callback', () => {
expect(res.headers['set-cookie'] ?? []).not.toContainEqual(expect.stringContaining('rd_session='));
});
it('does not sign in an unlinked Google account, and creates nothing', async () => {
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' })));
@@ -345,8 +345,9 @@ describe('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.
// 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`
@@ -354,3 +355,185 @@ describe('GET /api/auth/google/callback', () => {
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);
});
});