OAuth2 authentication + account linking (#12) #73

Merged

james merged 3 commits from 12-oauth into main 2026-06-17 15:02:35 +00:00

Conversation 0 Commits 3 Files changed 28 +2179 -25

james commented 2026-06-17 01:41:35 +00:00

Owner

Copy link

Closes #12.

Summary

• Minimal in-app OAuth (no Auth.js). Authorization-Code + PKCE, S256 challenge. Single load-bearing file (lib/auth/oauth.ts) for the linking decision tree; provider definitions in lib/auth/oauth-providers.ts; cookie helpers in lib/auth/oauth-cookies.ts. ADR-0014 records the deviation from the CLAUDE.md Auth.js default and enumerates the threat model the implementation handles (state, PKCE, mix-up / RFC 9207, replay via in-same-response cookie clear, redirect-URI lock-in, no token storage).
• GitHub provider ships v1. Env-driven enablement: enabled iff GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET are set. Adding a provider is one registry entry + a fetchProfile function + tests.
• Routes: GET /api/auth/oauth/start sets three short-lived cookies (state / PKCE verifier / signed meta) and 302s to the provider; GET /api/auth/oauth/callback/[provider] validates, exchanges, fetches profile, runs the decision tree, applies the result. Every callback exit clears the OAuth cookies in the same response — captured callback URLs can't be replayed.
• Linking decision tree (five branches): login_existing / signup_new / linked_to_current / refused_email_in_use / refused_belongs_to_other. Each branch is unit-tested.
• "Do not auto-merge by email" is the load-bearing security invariant. If OAuth returns email X and a User with that email already exists, the new signup is refused (/login?error=email_in_use). The published account-takeover attack against several Sign-In-With-X integrations is exactly auto-link-by-email; we don't ship it.
• Email verification required at the OAuth boundary. fetchProfile throws NoVerifiedEmailError if the provider can't produce a verified primary email. GitHub's /user/emails returns the {primary, verified} flag per address.
• DB layer: new oauth_identities table (migration 005). UNIQUE (provider, provider_user_id) is the durable lookup key; INDEX (user_id) backs the /account listing. FK + cascade matches every other per-user table.
• UI:
  • /login and /register mount <OAuthButtons> below the password form with a divider. Both pages also surface ?error=email_in_use and ?error=no_verified_email as a banner above the form.
  • New /account page lives under the authed shell. Lists the user's local + OAuth identities, with "Unlink" form-buttons (disabled when removing the row would leave the user with zero sign-in methods) and a "Connect another" section showing OAuth providers that aren't yet linked.
  • unlinkOAuthIdentityAction server action mirrors the logout-action pattern from PR #71: ownership check + last-method invariant + redirect-in-same-response.
  • Top nav gains an "Account" link.
• safeNext at ingress. /api/auth/oauth/start runs safeNext(rawNext) before storing in the meta cookie, so a tampered ?next= can't smuggle an off-origin redirect into the callback.
• CLAUDE.md "Stack defaults" auth line updated to point at ADR-0014. lib/auth/public-routes.ts's "Auth.js callback routes" comment replaced with a reference to ADR-0014.
• Follow-up filed: #72 — verified-email recovery flow for the email_in_use lockout. Without password reset, a user who forgot their password and tries OAuth sign-in is locked out; #72 builds the magic-link path that resolves it. Documented in ADR-0014.

Build / route summary

○  /                  static
○  /offline           static (force-static, precache intact)
○  /profile, /skills, /experience, /projects, /network  static placeholders
ƒ  /login, /register  dynamic (server session gate + OAuth buttons)
ƒ  /account           dynamic (server lookup of identities)
ƒ  /notes             dynamic
ƒ  /api/auth/oauth/start
ƒ  /api/auth/oauth/callback/[provider]
ƒ  Proxy (Middleware)

/ and /offline staying ○ (Static) is the load-bearing thing: ADR-0008's precache invariant is preserved.

Test plan

• npm run typecheck, npm run lint — clean.
• npm test — 173 passed / 38 skipped (211 total). 49 new tests:
  • tests/db/oauth-identities.test.ts — dual-engine, 6 tests (12 with the skipped pg leg). UNIQUE + cascade + delete-bounded-by-ownership.
  • tests/auth/oauth-providers.test.ts — 11 tests. Registry shape, env-driven enablement, GitHub fetchProfile round-trip + the no-verified-email throw + non-200 paths.
  • tests/auth/oauth-decision.test.ts — 7 tests. Every branch of resolveOAuthIdentity including first-user-is-admin.
  • tests/api/oauth.test.ts — 12 tests. /start parameter validation + cookie set, safeNext at ingress, every callback branch (signin-new, signin-existing-identity, signin-email-conflict, link-success, link-conflict, no-verified-email), and the replay defence (cookies cleared on bail-out responses).
• npm run build — succeeds. Routes as expected above.
• CI: build, dual-engine tests, security scans, gitleaks.
• Manual browser (requires GITHUB_CLIENT_ID + GITHUB_CLIENT_SECRET and an OAuth app configured with callback http://localhost:3000/api/auth/oauth/callback/github):
  • Fresh DB → /login → "Sign in with GitHub" → authorise → land on /profile. /account shows the GitHub identity, no Unlink (only one method).
  • Local registration with email X → sign out → "Sign in with GitHub" with email X → redirect to /login?error=email_in_use with the documented copy. No new User created.
  • Sign in as existing user → /account → "Connect GitHub" → OAuth dance → returns to /account. Two identities, both Unlink buttons enabled. Unlink GitHub → row removed, Unlink button vanishes.

What didn't change

• proxy.ts policy (ADR-0013). The new routes ride under the existing /api/auth/ public prefix; the proxy's HTML/API split is untouched.
• Auth backend pipeline (registerUser, attemptLogin, createSession). OAuth callback layers on top.
• lib/dto/user.ts's hand-rolled parsers stay; zod migration is the follow-up the ADR-0012 note already names.

ADR

ADR-0014 lives at docs/adr/0014-oauth-account-linking.md. Notable: it explicitly enumerates the threats handled — without that enumeration, "rolled minimal OAuth" reads as a security shortcut instead of a considered choice.

🤖 Generated with Claude Code

Closes #12. ## Summary - **Minimal in-app OAuth (no Auth.js).** Authorization-Code + PKCE, S256 challenge. Single load-bearing file (`lib/auth/oauth.ts`) for the linking decision tree; provider definitions in `lib/auth/oauth-providers.ts`; cookie helpers in `lib/auth/oauth-cookies.ts`. ADR-0014 records the deviation from the CLAUDE.md Auth.js default and enumerates the threat model the implementation handles (state, PKCE, mix-up / RFC 9207, replay via in-same-response cookie clear, redirect-URI lock-in, no token storage). - **GitHub provider** ships v1. Env-driven enablement: enabled iff `GITHUB_CLIENT_ID` and `GITHUB_CLIENT_SECRET` are set. Adding a provider is one registry entry + a `fetchProfile` function + tests. - **Routes:** `GET /api/auth/oauth/start` sets three short-lived cookies (state / PKCE verifier / signed meta) and 302s to the provider; `GET /api/auth/oauth/callback/[provider]` validates, exchanges, fetches profile, runs the decision tree, applies the result. Every callback exit clears the OAuth cookies in the same response — captured callback URLs can't be replayed. - **Linking decision tree (five branches):** `login_existing` / `signup_new` / `linked_to_current` / `refused_email_in_use` / `refused_belongs_to_other`. Each branch is unit-tested. - **"Do not auto-merge by email"** is the load-bearing security invariant. If OAuth returns email X and a User with that email already exists, the new signup is refused (`/login?error=email_in_use`). The published account-takeover attack against several Sign-In-With-X integrations is exactly auto-link-by-email; we don't ship it. - **Email verification required at the OAuth boundary.** `fetchProfile` throws `NoVerifiedEmailError` if the provider can't produce a verified primary email. GitHub's `/user/emails` returns the `{primary, verified}` flag per address. - **DB layer:** new `oauth_identities` table (migration 005). UNIQUE `(provider, provider_user_id)` is the durable lookup key; INDEX `(user_id)` backs the /account listing. FK + cascade matches every other per-user table. - **UI:** - `/login` and `/register` mount `<OAuthButtons>` below the password form with a divider. Both pages also surface `?error=email_in_use` and `?error=no_verified_email` as a banner above the form. - **New `/account` page** lives under the authed shell. Lists the user's local + OAuth identities, with "Unlink" form-buttons (disabled when removing the row would leave the user with zero sign-in methods) and a "Connect another" section showing OAuth providers that aren't yet linked. - **`unlinkOAuthIdentityAction`** server action mirrors the logout-action pattern from PR #71: ownership check + last-method invariant + redirect-in-same-response. - Top nav gains an "Account" link. - **`safeNext` at ingress.** `/api/auth/oauth/start` runs `safeNext(rawNext)` before storing in the meta cookie, so a tampered `?next=` can't smuggle an off-origin redirect into the callback. - **CLAUDE.md "Stack defaults"** auth line updated to point at ADR-0014. `lib/auth/public-routes.ts`'s "Auth.js callback routes" comment replaced with a reference to ADR-0014. - **Follow-up filed: #72** — verified-email recovery flow for the `email_in_use` lockout. Without password reset, a user who forgot their password and tries OAuth sign-in is locked out; #72 builds the magic-link path that resolves it. Documented in ADR-0014. ## Build / route summary ``` ○ / static ○ /offline static (force-static, precache intact) ○ /profile, /skills, /experience, /projects, /network static placeholders ƒ /login, /register dynamic (server session gate + OAuth buttons) ƒ /account dynamic (server lookup of identities) ƒ /notes dynamic ƒ /api/auth/oauth/start ƒ /api/auth/oauth/callback/[provider] ƒ Proxy (Middleware) ``` `/` and `/offline` staying `○ (Static)` is the load-bearing thing: ADR-0008's precache invariant is preserved. ## Test plan - [x] `npm run typecheck`, `npm run lint` — clean. - [x] `npm test` — **173 passed / 38 skipped (211 total).** 49 new tests: - `tests/db/oauth-identities.test.ts` — dual-engine, 6 tests (12 with the skipped pg leg). UNIQUE + cascade + delete-bounded-by-ownership. - `tests/auth/oauth-providers.test.ts` — 11 tests. Registry shape, env-driven enablement, GitHub `fetchProfile` round-trip + the no-verified-email throw + non-200 paths. - `tests/auth/oauth-decision.test.ts` — 7 tests. Every branch of `resolveOAuthIdentity` including first-user-is-admin. - `tests/api/oauth.test.ts` — 12 tests. /start parameter validation + cookie set, `safeNext` at ingress, every callback branch (signin-new, signin-existing-identity, signin-email-conflict, link-success, link-conflict, no-verified-email), and the replay defence (cookies cleared on bail-out responses). - [x] `npm run build` — succeeds. Routes as expected above. - [ ] CI: build, dual-engine tests, security scans, gitleaks. - [ ] **Manual browser** (requires `GITHUB_CLIENT_ID` + `GITHUB_CLIENT_SECRET` and an OAuth app configured with callback `http://localhost:3000/api/auth/oauth/callback/github`): - Fresh DB → /login → "Sign in with GitHub" → authorise → land on /profile. /account shows the GitHub identity, no Unlink (only one method). - Local registration with email X → sign out → "Sign in with GitHub" with email X → redirect to /login?error=email_in_use with the documented copy. No new User created. - Sign in as existing user → /account → "Connect GitHub" → OAuth dance → returns to /account. Two identities, both Unlink buttons enabled. Unlink GitHub → row removed, Unlink button vanishes. ## What didn't change - `proxy.ts` policy (ADR-0013). The new routes ride under the existing `/api/auth/` public prefix; the proxy's HTML/API split is untouched. - Auth backend pipeline (`registerUser`, `attemptLogin`, `createSession`). OAuth callback layers on top. - `lib/dto/user.ts`'s hand-rolled parsers stay; zod migration is the follow-up the ADR-0012 note already names. ## ADR ADR-0014 lives at `docs/adr/0014-oauth-account-linking.md`. Notable: it explicitly enumerates the threats handled — without that enumeration, "rolled minimal OAuth" reads as a security shortcut instead of a considered choice. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james added 1 commit 2026-06-17 01:41:35 +00:00

OAuth2 authentication + account linking (#12) ... 579f7d5a4f

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

james added 1 commit 2026-06-17 13:17:08 +00:00

build(release): add cosign public key 8dbedc47e8

james force-pushed 12-oauth from 8dbedc47e8 to 86f77d3143 2026-06-17 14:14:18 +00:00 Compare

james added 1 commit 2026-06-17 14:35:19 +00:00

Harden OAuth redirect helper against open-redirect (#12) ... 7fe3bd9e9e

The semgrep open-redirect pattern matched `NextResponse.redirect()` in
the callback's helper. The existing code was structurally safe (every
caller passed a hardcoded path or a safeNext()-sanitised string) but
relied on caller diligence. Enforce same-origin inside the helper so a
future caller can't break it; document the intentional off-origin
redirect in /start (going to the provider authorize URL is the whole
point of OAuth) with a justified nosemgrep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

james merged commit 4715359cef into main 2026-06-17 15:02:35 +00:00

james referenced this pull request from a commit 2026-06-17 15:02:37 +00:00

Merge pull request 'OAuth2 authentication + account linking (#12)' (#73) from 12-oauth into main

james referenced this pull request 2026-06-18 00:40:24 +00:00

Add generic OIDC provider support (Authentik, Keycloak, Zitadel, Google as OIDC, …) #85

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:auth

Authentication and authorization

  

area:ci

Forgejo Actions, build, test, security, release

  

area:db

Database abstraction, migrations, persistence

  

area:infra

Containerization, deployment, foundational tooling

  

area:native

React Native app (Android + Linux Flatpak)

  

area:pwa

PWA / web frontend

  

area:service

Backend service, API routes, persistence

  

epic

High-level tracker; links several feature tickets

  

feature

A user-facing feature

  

foundation

Foundational work that other tickets build on

No labels

area:auth

area:ci

area:db

area:infra

area:native

area:pwa

area:service

epic

feature

foundation

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

james/carol!73

No description provided.