feat(auth): create an account from a Google identity, then ask about consent (#342)
Linting / lint (pull_request) Successful in 2m59s
SonarQube Analysis / sonarqube (pull_request) Failing after 32m1s

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:04:57 -05:00
co-authored by Claude Opus 5
parent 9177b54dea
commit 828ee62bef
5 changed files with 481 additions and 15 deletions
+105
View File
@@ -0,0 +1,105 @@
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 do
*
* It refuses 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 #343 rather than falling out of an INSERT here.
* Refusing is the safe half of that decision and the only one available until
* the policy is written down.
*
* ## 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();
}
}
+70 -11
View File
@@ -5,6 +5,9 @@ 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 { issueVerificationEmail } from '../customerVerification';
import type { AttemptSecrets } from '../google/oauth';
import { googleSignInLimiter } from '../rateLimit';
import { safeReturnTo } from '../google/returnTo';
@@ -18,12 +21,16 @@ const router = Router();
* 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
* ## What this 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.
* It signs in a customer whose Google identity is already linked, and creates
* an account for one nobody here has seen (#342).
*
* It refuses 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 #343 rather than falling out of an INSERT.
* Refusing is the safe half of that decision and the only half available until
* the policy is written down.
*
* ## The cookie, and why it is the whole security of the callback
*
@@ -62,6 +69,15 @@ interface IdentityRow {
*/
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';
function setAttemptCookie(res: Response, attempt: Attempt): void {
res.cookie(ATTEMPT_COOKIE, Buffer.from(JSON.stringify(attempt)).toString('base64url'), {
httpOnly: true,
@@ -112,6 +128,49 @@ function secretsMatch(a: string, b: string): boolean {
return crypto.timingSafeEqual(digest(a), digest(b));
}
/**
* Creates an account for a Google identity nobody here has seen, and signs in.
*
* 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 return path is deliberately dropped for a brand-new customer, who lands
* on the consent step instead. That step 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 signUp(res: Response, identity: GoogleIdentity): Promise<void> {
const outcome = await createCustomerFromGoogle(identity);
if (outcome.kind === 'email-taken') {
// An account already uses this address, and joining them is #343. Refusing
// is the safe half of that decision: linking on an address is exactly the
// takeover path the policy exists to reason about carefully.
console.warn('[google] refused a sign-up: that address already has an account');
res.redirect(FAILURE_PATH);
return;
}
// 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);
}
router.get(
'/start',
googleSignInLimiter,
@@ -178,11 +237,11 @@ router.get(
);
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);
// 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 signUp(res, identity);
// 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,
@@ -208,6 +267,6 @@ router.get(
);
/** Exported for the tests; nothing else needs the cookie's name. */
export { ATTEMPT_COOKIE, ATTEMPT_TTL_MS, FAILURE_PATH };
export { ATTEMPT_COOKIE, ATTEMPT_TTL_MS, FAILURE_PATH, WELCOME_PATH };
export default router;