accounts: sign in with Google, on a shared provider-agnostic OAuth engine #122

Merged
admin_emi merged 1 commit from worktree-google-oauth into dev 2026-07-27 00:56:44 +00:00
Owner

Adds Google as a second identity provider — sign in with it, and connect it to an existing account.

Why this is a refactor and not an addition

The obvious move is a second copy of the Discord flow. That would duplicate the signed state, the code exchange, and — critically — the account-resolution logic, which is the security-sensitive part. A parallel hand-rolled copy is exactly where a subtle divergence becomes a takeover bug.

So the flow moves to accounts/oauth.py and both providers become configurations of it. A third provider is now a PROVIDERS entry and nothing else. notifications/discord_link.py drops from 342 lines to a thin alias layer plus the DM toggle.

The tests are parametrised over every provider, so Discord and Google are held to the same rules rather than one being tested and the other assumed.

Schema

Identity moves to an oauth_account table keyed (provider, subject). Resolution is on the provider's own stable id, never email — an address can be changed or reassigned upstream, a subject can't.

user.discord_id deliberately stays. It is the DM delivery address notify.py:166 reads, not merely an identity, so the Discord link keeps writing it. That asymmetry is documented on both the model and the link helper.

discord_onlyoauth_only, and the lockout guard is now about the last credential rather than about Discord: with two providers linked to a password-less account, either may be unlinked; the survivor can't be.

Migration 0005

Renames the flag and backfills an oauth_account row for every existing discord_id. Without that backfill an already-linked user would stop being recognised at login and would silently get a second account on their next sign-in.

It also drops a vestigial discord_only: revision 0003 re-adds that column on a database whose 0001 already built oauth_only from current model metadata, which would otherwise leave fresh and upgraded databases with different schemas. Handled in 0005 rather than by editing 0003, which is already applied on dev and beta.

Exercised on SQLite across all three paths:

  • fresh database → oauth_only present, no vestigial column
  • database shaped like beta today (stamped 0004, live discord_only values, one Discord-linked user) → values preserved, exactly one row backfilled, none for the unlinked user
  • re-run twice → no duplicate rows (the NOT EXISTS guard)

Compatibility

FE and BE deploy independently, so a newer backend must keep serving an older frontend:

  • /discord/link/callback keeps answering — that URL is registered in Discord's developer portal, so moving it would break every Discord login until an operator edited the portal.
  • The other /discord/* routes stay because the deployed frontend calls them.
  • The callback still emits ?discord= alongside the new ?oauth=&provider=.
  • UserRead still carries discord_only as a computed alias of oauth_only.

tests/notifications/test_discord_link.py was rewritten to pin exactly these promises rather than to re-test the flow.

Security properties (all tested, per provider)

  • No forced linking — a login state carries no user id, so it's replayable against whoever is signed in; it therefore never links.
  • Email matching requires the provider's verified flag, tested from the attacker's side.
  • No account moves between identities — an email already tied to a different identity from that provider is refused.
  • A state is bound to its provider as well as its purpose, so a Discord state is worthless at Google's callback.
  • No lockout — the last provider on a password-less account can't be unlinked.

Deployment

No changes to existing secrets. Google needs new ones:

  • THERMOGRAPH_GOOGLE_CLIENT_ID / THERMOGRAPH_GOOGLE_CLIENT_SECRET
  • redirect URI https://<host>/api/v2/oauth/google/callback registered on the OAuth client. Google matches redirect URIs exactly, so each environment needs its own entry.
  • consent screen needs only openid + email, both non-sensitive — no Google verification review.

Unset, Google reports enabled: false, shows no UI, and its routes bounce to unavailable — same fail-closed behaviour Discord has. Both providers therefore ship dark on dev and beta, since only prod's vault holds the Discord pair and no vault yet holds Google's.

Verification

  • Backend: 470 passed / 8 skipped. New tests/accounts/test_oauth.py (parametrised over both providers) replaces test_discord_login.py.
  • Frontend: gofmt -l clean, go vet + go test ./... green; account.js parses as an ES module.
  • Generated Google authorize URL inspected directly: correct endpoint, response_type=code, scope=openid email, access_type=online, matching redirect_uri.

Not covered: account.js still has no test runner in this repo, so the new provider UI is unexercised by CI.

Adds **Google** as a second identity provider — sign in with it, and connect it to an existing account. ## Why this is a refactor and not an addition The obvious move is a second copy of the Discord flow. That would duplicate the signed state, the code exchange, and — critically — the account-resolution logic, which is the security-sensitive part. A parallel hand-rolled copy is exactly where a subtle divergence becomes a takeover bug. So the flow moves to `accounts/oauth.py` and both providers become configurations of it. A third provider is now a `PROVIDERS` entry and nothing else. `notifications/discord_link.py` drops from 342 lines to a thin alias layer plus the DM toggle. The tests are parametrised over every provider, so Discord and Google are held to the same rules rather than one being tested and the other assumed. ## Schema Identity moves to an `oauth_account` table keyed `(provider, subject)`. Resolution is on the provider's own stable id, never email — an address can be changed or reassigned upstream, a subject can't. `user.discord_id` **deliberately stays.** It is the DM delivery address `notify.py:166` reads, not merely an identity, so the Discord link keeps writing it. That asymmetry is documented on both the model and the link helper. `discord_only` → `oauth_only`, and the lockout guard is now about the *last* credential rather than about Discord: with two providers linked to a password-less account, either may be unlinked; the survivor can't be. ## Migration 0005 Renames the flag and **backfills an `oauth_account` row for every existing `discord_id`**. Without that backfill an already-linked user would stop being recognised at login and would silently get a *second* account on their next sign-in. It also drops a vestigial `discord_only`: revision `0003` re-adds that column on a database whose `0001` already built `oauth_only` from current model metadata, which would otherwise leave fresh and upgraded databases with different schemas. Handled in `0005` rather than by editing `0003`, which is already applied on dev and beta. Exercised on SQLite across all three paths: - fresh database → `oauth_only` present, no vestigial column - database shaped like beta today (stamped `0004`, live `discord_only` values, one Discord-linked user) → values preserved, exactly one row backfilled, none for the unlinked user - re-run twice → no duplicate rows (the `NOT EXISTS` guard) ## Compatibility FE and BE deploy independently, so a newer backend must keep serving an older frontend: - `/discord/link/callback` keeps answering — **that URL is registered in Discord's developer portal**, so moving it would break every Discord login until an operator edited the portal. - The other `/discord/*` routes stay because the deployed frontend calls them. - The callback still emits `?discord=` alongside the new `?oauth=&provider=`. - `UserRead` still carries `discord_only` as a computed alias of `oauth_only`. `tests/notifications/test_discord_link.py` was rewritten to pin exactly these promises rather than to re-test the flow. ## Security properties (all tested, per provider) - **No forced linking** — a login state carries no user id, so it's replayable against whoever is signed in; it therefore never links. - **Email matching requires the provider's verified flag**, tested from the attacker's side. - **No account moves between identities** — an email already tied to a different identity from that provider is refused. - **A state is bound to its provider** as well as its purpose, so a Discord state is worthless at Google's callback. - **No lockout** — the last provider on a password-less account can't be unlinked. ## Deployment No changes to existing secrets. Google needs new ones: - `THERMOGRAPH_GOOGLE_CLIENT_ID` / `THERMOGRAPH_GOOGLE_CLIENT_SECRET` - redirect URI `https://<host>/api/v2/oauth/google/callback` registered on the OAuth client. **Google matches redirect URIs exactly**, so each environment needs its own entry. - consent screen needs only `openid` + `email`, both non-sensitive — no Google verification review. Unset, Google reports `enabled: false`, shows no UI, and its routes bounce to `unavailable` — same fail-closed behaviour Discord has. **Both providers therefore ship dark on dev and beta**, since only prod's vault holds the Discord pair and no vault yet holds Google's. ## Verification - Backend: **470 passed / 8 skipped**. New `tests/accounts/test_oauth.py` (parametrised over both providers) replaces `test_discord_login.py`. - Frontend: `gofmt -l` clean, `go vet` + `go test ./...` green; `account.js` parses as an ES module. - Generated Google authorize URL inspected directly: correct endpoint, `response_type=code`, `scope=openid email`, `access_type=online`, matching `redirect_uri`. Not covered: `account.js` still has no test runner in this repo, so the new provider UI is unexercised by CI.
admin_emi added 1 commit 2026-07-26 19:07:33 +00:00
accounts: sign in with Google, on a shared provider-agnostic OAuth engine
All checks were successful
PR build (required check) / changes (pull_request) Successful in 6s
secrets-guard / encrypted (pull_request) Successful in 5s
shell-lint / shellcheck (pull_request) Successful in 8s
PR build (required check) / validate-observability (pull_request) Has been skipped
PR build (required check) / build-frontend (pull_request) Successful in 1m1s
PR build (required check) / build-backend (pull_request) Successful in 1m29s
PR build (required check) / gate (pull_request) Successful in 1s
c17a4c3dd7
Adds Google as a second identity provider. Rather than a second copy of the
Discord flow, the flow itself moves to accounts/oauth.py and both providers
become configurations of it — account resolution is the security-critical
part, and a parallel hand-rolled copy is where a subtle divergence becomes a
takeover. A third provider is now a PROVIDERS entry and nothing else.

Identity moves to an oauth_account table keyed (provider, subject), so a
login resolves on the provider's own stable id rather than an email that can
be changed or reassigned. user.discord_id deliberately stays: it is the DM
delivery address notify.py reads, not merely an identity, and the Discord
link keeps writing it. discord_only becomes oauth_only, and the lockout guard
now refuses to unlink the *last* provider from a password-less account rather
than singling out Discord — with two linked, either may go.

Migration 0005 renames the flag and backfills an oauth_account row for every
existing discord_id. Without that backfill an already-linked user would stop
being recognised at login and would silently get a second account on their
next sign-in. It also drops a vestigial discord_only that 0003 re-adds on a
database whose 0001 already built oauth_only from current metadata, which
otherwise left fresh and upgraded databases with different schemas.

Compatibility, since the frontend deploys independently of this service:
/discord/link/callback keeps answering because that URL is registered in
Discord's developer portal, the other /discord/* routes stay because the
deployed frontend calls them, the callback still emits ?discord= alongside
?oauth=, and UserRead still carries discord_only as a computed alias.

Google needs THERMOGRAPH_GOOGLE_CLIENT_ID/_CLIENT_SECRET and its own
registered redirect URI; unset, it reports disabled and shows no UI.
admin_emi force-pushed worktree-google-oauth from c17a4c3dd7 to 81135c1be5 2026-07-27 00:54:07 +00:00 Compare
admin_emi merged commit deb039ee24 into dev 2026-07-27 00:56:44 +00:00
admin_emi deleted branch worktree-google-oauth 2026-07-27 00:56:44 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Jinemi/thermograph#122
No description provided.