Skip to content

feat(auth): sign in with GitHub and let the identity be the account - #443

Merged
argszero merged 2 commits into
mainfrom
feat/github-device-login
Nov 9, 2026
Merged

argszero merged 2 commits into
mainfrom
feat/github-device-login

Conversation

@argszero

@argszero argszero commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Sign in with GitHub, for both kinds of client the platform has. Browserless clients (desktop app, CLI, agent) use the device-code flow; the web login page gets a "Sign in with GitHub" button on the ordinary authorisation-code flow. Both meet at one function, land_identity, so whichever entrance a GitHub account uses it lands on the same user row and receives the same existing API key.

Addresses the work order filed as rant 2026-11-09T14:43:55 ("support GitHub sign-in and registration, and let the desktop client obtain an API key in one step").

The contract browserless clients already implement

Endpoint Request Success Intermediate
POST /api/auth/github/device/code none 200 {device_code, user_code, verification_uri, expires_in, interval} —
POST /api/auth/github/device/token {device_code} 200 {api_key, user_id, login} 202 authorization_pending / 429 slow_down / 410 expired_token / 403 access_denied

Field names and status codes are as given in the order; the success body is the same shape as email login (api_key + user_id), plus login. No intermediate state issues a key — those four branches return before the database is touched.

The web half

Endpoint Request Success
GET /api/auth/github/authorize none 200 {authorize_url} + Set-Cookie: atp_gh_state=…
POST /api/auth/github/exchange {code, state} 200 {api_key, user_id, login}

state must match the HttpOnly cookie authorize set; a mismatch answers 400 github_state_mismatch before any upstream call or database write. Without that binding, a code minted by an attacker could sign a victim's browser into the attacker's account. The cookie is cleared (Max-Age=0) on success, so a used state has no second chance. The front end strips code/state from the address bar immediately on return, then enters the app through the same restoreSession path email login uses.

redirect_uri is derived from server.public_url (trailing slash trimmed) rather than added as a third configuration key, and the same value is sent when exchanging the code, as GitHub requires both to match.

The acceptance items

  1. Device-code login — the two endpoints above; the web half added here.
  2. Identity is the account — keyed on GitHub's stable numeric id, never on login (which can be renamed and would otherwise create a second account). An existing key is reused through dao::get_or_create_api_key, the same issuance path email login uses; the success body is api_key + user_id as in email login.
  3. Registration is already verified — no email code, no email dependency; verified is set on creation and scope stays empty, so a GitHub account with no public email still registers.
  4. New accounts get the same treatment — a quotas row is created with the account and the daily gift is grantable on day one; asserted in the gate.
  5. Web login page entry — the button plus the authorisation-code flow above, sharing land_identity with the device flow.
  6. Exactly two config keys — [github] client_id / client_secret, empty meaning disabled, where the endpoints answer 503 github_login_not_enabled (naming what to set) and never 500. config/config.example.toml carries both, and states the two switches an OAuth App needs: device flow enabled, and the callback URL set to public_url.

Schema

SCHEMA_VERSION 17 → 18. users gains github_id TEXT NOT NULL DEFAULT '' (via ensure_column, so an existing database upgrades in place), a github_login column, and a partial unique index on github_id that excludes '', so email-only accounts are unaffected and no second row can claim an identity. email and password_hash stay NOT NULL (GitHub accounts store an empty string plus an unusable hash), which is why no table rebuild is needed. The migration is described in CHANGELOG.md.

Tests

cargo test 605 passed, 0 failed (590 at the base commit 405f078); cargo fmt --check clean; cargo clippy --all-targets -- -D warnings clean.

The new src/github_oauth_gate.rs is test-only (registered as #[cfg(test)] mod in main.rs) and adds 15 assertions, of which four are the web half: the authorise URL is derived from public_url and the state cookie is HttpOnly + SameSite=Lax; a mismatched state buys no credentials and does not reach upstream; the web flow and the device flow land on the same user row and the same key; the login page carries the button with its key in both language packs.

Tests never call github.com: the upstream base is injectable (AppState::github_api), pointed at a local mock. Every assertion carries a non-empty positive control, so a broken extractor turns red instead of passing on an empty set.

Not in this pull request

Nothing from the order — all six items are here. The OAuth App itself is the host's to create; the code answers 503 with an actionable message until then, which is why an unconfigured deployment is not a merge blocker.

Clients with no browser callback (desktop app, CLI, agent) could not register
at all: registration is email plus a six-digit code, and the code only travels
by mail. Such a client cannot read the user's GitHub email either, so "one click
and the model is configured" was impossible in principle.

Add the GitHub device flow, with the field names and status codes fixed as the
client already implements them:

  POST /api/auth/github/device/code
    -> 200 {device_code, user_code, verification_uri, expires_in, interval}
  POST /api/auth/github/device/token {device_code}
    -> 200 {api_key, user_id, login}
    -> 202 authorization_pending / 429 slow_down / 410 expired_token /
       403 access_denied

An intermediate state never issues a key: that branch returns without touching
the database.

The GitHub identity is the account. First login registers, and the same GitHub
account always returns to the same account afterwards. The credential fields are
byte-identical to email login (api_key + user_id), and an existing key is reused
through the same dao::get_or_create_api_key -- there is no second issuance path.
A new account is verified on creation (a GitHub identity is already verified, so
no mail code), gets its quotas row with the account, and the daily newcomer gift
is granted as usual.

The identity key is GitHub's stable numeric id, not the login: a login can be
renamed, and keying on it would create a second account the day it happens.

Migration: users gains github_id / github_login plus a partial unique index
(github_id <> ''), all idempotent; schema 17 -> 18. Columns are added rather than
rebuilding the table: email and password_hash are NOT NULL, and making them
nullable would force every `SELECT ... email FROM users` reader in the tree
(admin user list, ops reports) to tolerate NULL -- one migration turning into a
cross-module rewrite. A GitHub account therefore still writes an email, using
GitHub's published noreply form <id>+<login>@users.noreply.github.com, and an
empty password_hash, which auth::verify_password always rejects -- such an
account cannot be signed into with email plus password.

Config is exactly two keys, [github] client_id / client_secret. Left empty the
endpoints answer 503 with the code github_login_not_enabled, not 500: an
unconfigured deployment is a state, not an internal fault. The OAuth App must
have Device Flow enabled; the example config says so.

Tests never reach github.com: the upstream base URLs live on
AppState::github_api and the gate points them at a local fake, so the two
config keys stay the only config keys. The new gate asserts the contract fields,
the four intermediate states against their status codes, and that the example
config declares only the two credentials; four behavioural tests cover the
no-key-on-pending rule, key reuse across two logins with one row each, the quota
row and grantable daily gift, and the 503-not-500 answer. Every assertion carries
a non-empty positive control so a decoder that went blind cannot pass on an empty
set.

cargo test 590 -> 601.

the suite also gains five contract tests and six behavioural tests, all with
non-empty positive controls; cargo test 590 -> 601, fmt and clippy clean.
@argszero
argszero force-pushed the feat/github-device-login branch from 151934d to 914f057 Compare November 9, 2026 07:29
The login page had no GitHub entry, so the web route into the platform was
still email plus a mail-borne code. This adds the authorisation-code half of
the same feature: a "Sign in with GitHub" button, `GET
/api/auth/github/authorize` which hands back an `authorize_url`, and `POST
/api/auth/github/exchange` which turns the returned code into platform
credentials.

Both flows now meet at one function, `land_identity`, so a GitHub account
reaches the same user row and the same existing key whichever entrance it
used. `redirect_uri` is derived from `server.public_url` rather than added
as a third config key, and `state` is checked against a short-lived HttpOnly
cookie set by `authorize` -- without that binding a code minted by an
attacker could sign the victim's browser into the attacker's account. A
mismatched state answers 400 before any upstream call or database write.

The example config now states the two switches an OAuth App needs: device
flow enabled, and the callback URL set to `public_url`. cargo test 601 ->
605, fmt and clippy clean.
@argszero
argszero merged commit 149297b into main Nov 9, 2026
2 checks passed
@argszero
argszero deleted the feat/github-device-login branch November 9, 2026 08:00
@argszero argszero mentioned this pull request Nov 9, 2026
10 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant