feat(auth): the Google sign-in round trip (#341) #347

Merged
bermudalamb merged 1 commits from feature/341-google-round-trip into main 2026-09-10 08:53:01 -05:00
Owner

Closes #341. Phase 2 of six under #332.

Two routes, and a customer whose Google identity is already linked can sign in. Account creation (#342) and linking (#343) are deliberately held back, so this is about the protocol alone.

The flow

Authorization code with PKCE. The browser is sent to Google, comes back carrying a code, and this server exchanges it over its own TLS connection — nothing that can read the page ever holds a token. PKCE goes in even though this is a confidential client: one hash, and it closes code interception rather than resting the flow on the secret staying secret.

No JWKS fetch, and why that is sound

The id token arrives in the response to a request this server made, over TLS, directly to Google's token endpoint. That is exactly the case OpenID Connect permits skipping signature verification for, and it removes a key fetch, a cache and a rotation path from the part of the codebase least worth having moving parts in.

The reasoning is written into google/oauth.ts rather than left to be rediscovered, including the boundary: the moment an id token reaches this code from anywhere other than that response, the reasoning stops holding and signature verification becomes mandatory.

It removes none of the claim checks. Each is load-bearing, and each has a test naming what accepting it blindly would allow:

Claim What blind acceptance allows
iss A token from an issuer never chosen
aud A token minted for another app, replayed here
exp A token captured once and reused forever
nonce A token from an earlier attempt
sub An identity row keyed on nothing

Both spellings of iss are accepted, because Google sends both and taking only one fails for some customers and not others.

email_verified is compared to the boolean, never tested for truthiness. The string "false" is truthy, and the linking policy in #343 turns entirely on this flag, so that one line is the difference between a policy and a takeover path.

The attempt cookie

The callback is a plain GET anyone can invoke. What makes it safe is a cookie set moments earlier carrying three secrets, minted separately because they are checked by different parties at different moments. It is cleared on every path through the callback, so an attempt cannot be replayed even once.

SameSite=Lax, not Strict, with the longest comment in the file. The callback is a cross-site top-level navigation; Strict withholds the cookie, the state check fails, and every sign-in is refused with an error that looks exactly like tampering.

The open redirect that would have been

Where the customer returns to survives the round trip in that cookie, and it is a value an attacker proposes. 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, landing somewhere else.

google/returnTo.ts is its own module so it can be tested without a database. Worth knowing: its first draft used a regex that inverted its own character class and rejected every path. It passed every other test and would have broken every real sign-in. There is now a test for exactly that.

Also

  • Declining at Google's consent screen is a cancellation, not a failure. Back where they were, nothing said — the distinction #41 drew for a dismissed passkey prompt.
  • A disabled account is refused here too. Enforcing it on some sign-in routes and not others is how a disabled account keeps a way in.
  • Sign-in calls the shared signIn, and there is a test that the resulting session is accepted by an unrelated route. That is what makes the sharing worth something rather than merely tidy.

Testing

39 new unit tests across the protocol and the return path, plus an integration suite covering both routes end to end with only the token exchange stubbed — so the cookie, the state check, the claim checks and the lookup all run for real.

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

🤖 Generated with Claude Code

Closes #341. Phase 2 of six under #332. Two routes, and a customer whose Google identity is **already linked** can sign in. Account creation (#342) and linking (#343) are deliberately held back, so this is about the protocol alone. ## The flow Authorization code with PKCE. The browser is sent to Google, comes back carrying a code, and this server exchanges it over its own TLS connection — nothing that can read the page ever holds a token. PKCE goes in even though this is a confidential client: one hash, and it closes code interception rather than resting the flow on the secret staying secret. ## No JWKS fetch, and why that is sound The id token arrives in the response to a request *this server* made, over TLS, directly to Google's token endpoint. That is exactly the case OpenID Connect permits skipping signature verification for, and it removes a key fetch, a cache and a rotation path from the part of the codebase least worth having moving parts in. The reasoning is written into `google/oauth.ts` rather than left to be rediscovered, including the boundary: **the moment an id token reaches this code from anywhere other than that response, the reasoning stops holding and signature verification becomes mandatory.** It removes none of the claim checks. Each is load-bearing, and each has a test naming what accepting it blindly would allow: | Claim | What blind acceptance allows | |---|---| | `iss` | A token from an issuer never chosen | | `aud` | A token minted for another app, replayed here | | `exp` | A token captured once and reused forever | | `nonce` | A token from an earlier attempt | | `sub` | An identity row keyed on nothing | Both spellings of `iss` are accepted, because Google sends both and taking only one fails for some customers and not others. **`email_verified` is compared to the boolean, never tested for truthiness.** The string `"false"` is truthy, and the linking policy in #343 turns entirely on this flag, so that one line is the difference between a policy and a takeover path. ## The attempt cookie The callback is a plain GET anyone can invoke. What makes it safe is a cookie set moments earlier carrying three secrets, minted separately because they are checked by different parties at different moments. It is cleared on every path through the callback, so an attempt cannot be replayed even once. **`SameSite=Lax`, not `Strict`**, with the longest comment in the file. The callback is a cross-site top-level navigation; `Strict` withholds the cookie, the state check fails, and every sign-in is refused with an error that looks exactly like tampering. ## The open redirect that would have been Where the customer returns to survives the round trip in that cookie, and it is a value an attacker proposes. 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, landing somewhere else. `google/returnTo.ts` is its own module so it can be tested without a database. Worth knowing: its first draft used a regex that inverted its own character class and rejected **every** path. It passed every other test and would have broken every real sign-in. There is now a test for exactly that. ## Also - Declining at Google's consent screen is a **cancellation**, not a failure. Back where they were, nothing said — the distinction #41 drew for a dismissed passkey prompt. - A **disabled account** is refused here too. Enforcing it on some sign-in routes and not others is how a disabled account keeps a way in. - Sign-in calls the shared `signIn`, and there is a test that the resulting session is accepted by an unrelated route. That is what makes the sharing worth something rather than merely tidy. ## Testing 39 new unit tests across the protocol and the return path, plus an integration suite covering both routes end to end with only the token exchange stubbed — so the cookie, the state check, the claim checks and the lookup all run for real. Verified locally: backend `tsc` clean for src and tests, 590 unit tests pass, lint back to the seven warnings that predate this branch. The integration suite needs a database this machine has no Docker for. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
bermudalamb added 1 commit 2026-09-10 08:50:05 -05:00
feat(auth): the Google sign-in round trip (#341)
SonarQube Analysis / sonarqube (pull_request) Failing after 28m35s
Linting / lint (pull_request) Successful in 3m39s
87f07baaff
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>
bermudalamb merged commit 9177b54dea into main 2026-09-10 08:53:01 -05:00
bermudalamb deleted branch feature/341-google-round-trip 2026-09-10 08:53:02 -05:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#347