accounts: sign in with Discord, sharing the existing link callback #115

Merged

admin_emi merged 1 commit from worktree-discord-oauth into dev

2026-07-26 18:16:56 +00:00

admin_emi commented

2026-07-26 17:43:58 +00:00

Owner

Discord was already linkable from a signed-in session, but it could not authenticate one. This adds sign in with Discord and connects the resulting identity to an account.

What's new

GET /api/v2/discord/login/start — unauthenticated by design; this is how you get a session. Someone already signed in who lands there is sent down the link flow instead, so it never silently switches accounts.

Both flows return to the existing /discord/link/callback. Discord only honours redirect URIs registered in the developer portal, so a second callback path would have blocked this behind an operator change in the portal. Instead the signed state now carries a purpose; because that is inside the HMAC, a state can only verify under the flow it was minted for. Scopes are requested per-call, not registered, so the added email scope needs no portal change either.

Account resolution, in order:

An existing discord_id — the durable key, needs no email at all.
Otherwise the Discord email, only when Discord reports it verified. That flag is the sole evidence the person owns the address; matching an existing account on an unverified one would hand it over to whoever typed it into Discord. Unverified is refused outright.
Failing both, a new account is created (verified, since Discord vouched for the address — so no confirmation email is sent).

Security properties worth reviewing

No forced linking. A login state carries no user id — it can't, there is no user yet — so it is replayable against whoever happens to be signed in. It therefore never links. If it did, an attacker who got a victim to open the callback with a code from the attacker's own Discord would attach their identity to the victim's account and then sign in as them. Only link/start, whose state is bound to a specific user id, may link. Covered by test_a_login_state_cannot_link_the_signed_in_account.
Email matching is gated on Discord's verified flag, tested from the attacker's side in test_login_refuses_an_unverified_discord_email.
An account is never moved between Discord identities — an email whose account is already linked elsewhere is refused (mismatch).
No lockout. Accounts created this way have no password their owner has ever seen, and no reset-password router is mounted, so unlinking would be unrecoverable. user.discord_only records this and unlink returns 409; the UI drops the button and explains.

Migration

0003_user_discord_only adds the column conditionally. Revision 0001 builds the schema with Base.metadata.create_all off live model metadata, so a database created after this column reached the ORM already has it and an unconditional add_column would fail there. Verified both ways against SQLite: a fresh DB no-ops and stamps; a DB rewound to 0002 gains the column with existing rows backfilled to false.

Drive-by fixes to the existing link flow

Three things that were already broken before this change:

The callback redirected to {BASE}/subscriptions, which no route serves — the frontend serves that page at /alerts. A completed link landed on a dead path.
Nothing ever read the ?discord= status the callback has always sent, so linking gave no feedback at all despite the code comment claiming the page "surfaces the link state". Every outcome is now a toast.
Linking a Discord account another user already owns hit the discord_id UNIQUE constraint and 500'd; it now returns a taken status and explains itself.

Frontend

"Continue with Discord" in the sign-in modal, shown only when the server reports Discord configured — the /discord/config probe now runs regardless of sign-in state, since the modal needs the answer precisely when nobody is signed in.

Deployment note

No new secrets. THERMOGRAPH_DISCORD_APP_ID and THERMOGRAPH_DISCORD_CLIENT_SECRET already gate linking and now gate both flows; the redirect URI is already registered. Only prod's vault has that pair, so this ships dark on dev and beta — enabled() is false there, the UI hides itself, and the endpoints bounce to unavailable. Adding the two keys to those vaults (plus each host's redirect URI in the portal) is what would turn it on.

Verification

Backend: full suite, 442 passed / 8 skipped. 13 new tests in tests/notifications/test_discord_login.py.
Frontend: gofmt -l clean, go vet ./... and go test ./... green; account.js parses as an ES module.
Migration exercised on both a fresh and a rewound SQLite database.

Not covered: account.js has no test runner in this repo, so the new UI is unexercised by CI — same as the auth UI it sits next to.

Discord was already linkable from a signed-in session, but it could not authenticate one. This adds **sign in with Discord** and connects the resulting identity to an account. ## What's new `GET /api/v2/discord/login/start` — unauthenticated by design; this is how you *get* a session. Someone already signed in who lands there is sent down the link flow instead, so it never silently switches accounts. **Both flows return to the existing `/discord/link/callback`.** Discord only honours redirect URIs registered in the developer portal, so a second callback path would have blocked this behind an operator change in the portal. Instead the signed `state` now carries a purpose; because that is inside the HMAC, a state can only verify under the flow it was minted for. Scopes are requested per-call, not registered, so the added `email` scope needs no portal change either. **Account resolution**, in order: 1. An existing `discord_id` — the durable key, needs no email at all. 2. Otherwise the Discord email, **only when Discord reports it verified**. That flag is the sole evidence the person owns the address; matching an existing account on an unverified one would hand it over to whoever typed it into Discord. Unverified is refused outright. 3. Failing both, a new account is created (verified, since Discord vouched for the address — so no confirmation email is sent). ## Security properties worth reviewing - **No forced linking.** A login state carries no user id — it can't, there is no user yet — so it is replayable against whoever happens to be signed in. It therefore *never* links. If it did, an attacker who got a victim to open the callback with a code from the attacker's own Discord would attach their identity to the victim's account and then sign in as them. Only `link/start`, whose state is bound to a specific user id, may link. Covered by `test_a_login_state_cannot_link_the_signed_in_account`. - **Email matching is gated on Discord's `verified` flag**, tested from the attacker's side in `test_login_refuses_an_unverified_discord_email`. - **An account is never moved between Discord identities** — an email whose account is already linked elsewhere is refused (`mismatch`). - **No lockout.** Accounts created this way have no password their owner has ever seen, and no reset-password router is mounted, so unlinking would be unrecoverable. `user.discord_only` records this and unlink returns 409; the UI drops the button and explains. ## Migration `0003_user_discord_only` adds the column **conditionally**. Revision `0001` builds the schema with `Base.metadata.create_all` off live model metadata, so a database created after this column reached the ORM already has it and an unconditional `add_column` would fail there. Verified both ways against SQLite: a fresh DB no-ops and stamps; a DB rewound to `0002` gains the column with existing rows backfilled to false. ## Drive-by fixes to the existing link flow Three things that were already broken before this change: - The callback redirected to `{BASE}/subscriptions`, which **no route serves** — the frontend serves that page at `/alerts`. A completed link landed on a dead path. - **Nothing ever read the `?discord=` status** the callback has always sent, so linking gave no feedback at all despite the code comment claiming the page "surfaces the link state". Every outcome is now a toast. - Linking a Discord account another user already owns hit the `discord_id` UNIQUE constraint and **500'd**; it now returns a `taken` status and explains itself. ## Frontend "Continue with Discord" in the sign-in modal, shown only when the server reports Discord configured — the `/discord/config` probe now runs regardless of sign-in state, since the modal needs the answer precisely when nobody is signed in. ## Deployment note No new secrets. `THERMOGRAPH_DISCORD_APP_ID` and `THERMOGRAPH_DISCORD_CLIENT_SECRET` already gate linking and now gate both flows; the redirect URI is already registered. **Only prod's vault has that pair**, so this ships dark on dev and beta — `enabled()` is false there, the UI hides itself, and the endpoints bounce to `unavailable`. Adding the two keys to those vaults (plus each host's redirect URI in the portal) is what would turn it on. ## Verification - Backend: full suite, **442 passed / 8 skipped**. 13 new tests in `tests/notifications/test_discord_login.py`. - Frontend: `gofmt -l` clean, `go vet ./...` and `go test ./...` green; `account.js` parses as an ES module. - Migration exercised on both a fresh and a rewound SQLite database. Not covered: `account.js` has no test runner in this repo, so the new UI is unexercised by CI — same as the auth UI it sits next to.

admin_emi added 1 commit 2026-07-26 17:43:59 +00:00

accounts: sign in with Discord, sharing the existing link callback

PR build (required check) / changes (pull_request) Successful in 8s

Details

secrets-guard / encrypted (pull_request) Successful in 5s

Details

shell-lint / shellcheck (pull_request) Successful in 8s

Details

PR build (required check) / validate-observability (pull_request) Has been skipped

Details

PR build (required check) / build-frontend (pull_request) Successful in 1m11s

Details

PR build (required check) / build-backend (pull_request) Successful in 1m35s

Details

PR build (required check) / gate (pull_request) Successful in 1s

Details

8b70a87555

Discord was already linkable from a signed-in session; it could not
authenticate one. Adds /discord/login/start, which resolves a Discord
identity to an account and issues a session.

Both flows return to the existing /discord/link/callback. Discord only
honours redirect URIs registered in the developer portal, so a second
callback path would have blocked this behind an operator change; the
signed state now carries a purpose, and since that is inside the HMAC a
state can only verify under the flow it was minted for.

Account resolution, in order: an existing discord_id (the durable key,
no email needed); otherwise the Discord email, but only when Discord
reports it verified — that flag is the sole evidence the person owns the
address, and matching on an unverified one would hand over the account.
Failing both, a new account is created.

A login state carries no user id, so it is replayable against whoever is
signed in. It therefore never links: doing so would be a forced-linking
takeover. Only link/start, whose state is bound to a user id, may link.

Accounts created this way have no password their owner has ever seen,
so user.discord_only records that and unlink is refused for them — no
reset-password router is mounted, so unlinking would be unrecoverable.
Migration 0003 adds the column conditionally: 0001 builds the schema
from live model metadata, so a fresh database already has it.

Also fixes two latent bugs in the link flow: the callback redirected to
/subscriptions, which no route serves (it is /alerts), and nothing ever
read the ?discord= status it has always sent, so a completed link gave
no feedback. Both now surface as a toast. Linking a Discord account that
another account already owns returned a 500 from the unique constraint;
it now explains itself.