From c06438e3489f0ee1dbd52e5722e1326e0cafe627 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 30 May 2026 22:10:05 -0400 Subject: [PATCH 1/2] chore(plans): open login-migration-strategy (in-progress) Doc-only plan capturing a strategic shift in the cutover auth story: keep password login working for existing users (with auto-rehash from laddr's unsalted SHA1 to argon2id on every successful login), restrict new signups to GitHub, and surface GitHub-linking via a non-blocking banner on /account. Sunset deferred. Co-Authored-By: Claude Opus 4.7 (1M context) --- plans/login-migration-strategy.md | 162 ++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 plans/login-migration-strategy.md diff --git a/plans/login-migration-strategy.md b/plans/login-migration-strategy.md new file mode 100644 index 0000000..df560c0 --- /dev/null +++ b/plans/login-migration-strategy.md @@ -0,0 +1,162 @@ +--- +status: in-progress +depends: [] +specs: + - specs/api/auth.md + - specs/behaviors/account-migration.md + - specs/behaviors/password-hash-rotation.md + - specs/screens/login.md + - specs/screens/account.md + - specs/deferred.md +issues: [] +--- + +# Plan: login migration strategy — keep password login for existing users + nag-to-link-GitHub + +## Scope + +A directional change to the cutover auth strategy. Today's spec treats +GitHub OAuth as the only auth path; existing laddr users are gated by +a one-shot claim-at-cutover flow. The new design: + +- **Existing users** can keep signing in with their old laddr password. + Successful login auto-rehashes the unsalted-SHA1 credential to + argon2id so the corpus drifts toward modern hashing without forcing + resets. +- **New accounts** are still GitHub-only — preserves the spam argument + that drove the original GitHub-only decision. +- **Linking GitHub** is a per-user opt-in surfaced via a persistent + banner on `/account`. No deadline pressure; users link when ready. +- **Sunset** is deferred — no fixed deprecation date or coverage + threshold today. Tracked separately when usage data justifies it. + +This plan is **doc-only** — it captures the strategic shift across +the affected spec files. The implementation work (a real +`POST /api/auth/login` route, rehash logic, link flow, banner) lands +in a follow-up plan after these specs are reviewed. + +## Implements + +- [api/auth.md](../specs/api/auth.md) — new `POST /api/auth/login` + `POST /api/auth/link-github` + `/api/auth/me` field additions. +- [behaviors/account-migration.md](../specs/behaviors/account-migration.md) — rewritten around link-when-ready instead of gate-at-cutover. +- [behaviors/password-hash-rotation.md](../specs/behaviors/password-hash-rotation.md) — new doc: rehash-on-login + format-detection rules. +- [screens/login.md](../specs/screens/login.md) — secondary password login option. +- [screens/account.md](../specs/screens/account.md) — nag banner + link-GitHub card state. +- [deferred.md](../specs/deferred.md) — "email/password auth" entry flips from "deleted" to "kept for migrated users; sunset deferred." + +## Approach + +### 1. Why this shift makes sense + +The original deferred-md entry argued GitHub-only because "Spam/scam load on the laddr sign-up form was unmanageable." That argument applies to **new signups**, not existing users. By restricting account *creation* to GitHub (unchanged), the spam argument still holds. Existing accounts already cleared whatever bar they cleared on laddr; making them re-prove identity is a UX speed bump with no security benefit. + +The original spec's password-claim path also pretended bcrypt; in fact the laddr `passwordHasher` is `'SHA1'` (per `JarvusInnovations/emergence-skeleton/php-classes/User.class.php:33`). Unsalted SHA-1 is broken — rainbow tables crack every common password instantly. The new design treats *every* legacy credential as a "needs rehash" candidate and uses successful logins as the rotation trigger. + +### 2. Login surface + +`POST /api/auth/login` accepts `{ usernameOrEmail, password }` and: + +1. Resolves to a `Person` by `slug` or by `PrivateProfile.email` +2. Loads `LegacyPasswordCredential.passwordHash` +3. Detects hash algorithm by format (bcrypt prefix → bcrypt; argon2 prefix → argon2id; bare 40-char hex → SHA-1) +4. Verifies — constant-time compare for the SHA-1 path +5. On success: **rehash** the supplied plaintext to argon2id, overwrite the credential, mint a session +6. On failure: uniform `401` with `error.code = "invalid_credentials"` — no distinction between "no such user" and "wrong password" (anti-enumeration) + +### 3. Linking GitHub + +When a signed-in user (legacy-password or GitHub) visits `/account`, they see a Card 1 ("Identity") whose contents depend on `Person.githubUserId`: + +- **Linked** — green check, "Connected as @login", manage-on-github link +- **Not linked + has password creds** — yellow banner "Connect a GitHub account to make sign-in easier. (No deadline — just a recommendation.)" + a "Connect GitHub" button. The post-link experience is: GitHub OAuth round-trip → returns with `Person.githubUserId` populated → banner disappears. +- **Linked + has password creds** — quiet "Sign in via GitHub or password" indicator. Password credential stays until the user explicitly removes it (deferred) or we sunset. + +The banner is the *only* nag — no email reminders, no modal interrupts, no toast on every login. Persistent and easy to dismiss-by-acting; non-blocking. + +### 4. `/api/auth/me` new fields + +```json +{ + "data": { + "person": { ... }, + "accountLevel": "user", + "hasGitHubLink": true, + "lastLoginMethod": "github" | "legacy_password" + } +} +``` + +The SPA uses these to decide: + +- whether to render the banner +- whether the link-GitHub flow can act on the current session (it can if `hasGitHubLink === false`) + +### 5. Password-recovery story + +"Forgot my password" works the same as laddr today: user enters +their username or email, server emits a one-time link with a +`PasswordToken` (new behavior — see #2 below) to the address in +`PrivateProfile.email`. The token resets the password to a new +plaintext, which gets hashed argon2id on first save. + +If the email on file is dead, the user is stuck — same as laddr. They +contact staff via side-channel (the existing staff-review path). + +This adds a small amount of new code (`POST /api/auth/password-reset/request`, `POST /api/auth/password-reset/confirm`, a `PasswordToken` private record). It's the only real net-new code surface from the original spec — but it's necessary to keep password login viable. + +### 6. Sunset + +Deferred. No date, no coverage threshold, no automated deprecation +copy in v1. Add `lastPasswordLoginAt` to `LegacyPasswordCredential` so +the operator can later report "X% of active users have linked GitHub"; +once that number's high enough, set a sunset date as a separate plan. + +### 7. What the claim flow becomes + +The existing account-claim flow stops being a **gate** and becomes an +**optional helper** for one specific case: a user signed in via GitHub +who didn't legacy-link automatically (no email match, no username +match) but knows they have a legacy account. Reachable from `/account` +"Claim another legacy account" — exactly the existing +`/account/claim-legacy` flow, just framed differently. Otherwise the +claim machinery is dormant for the common case. + +Implication: the cutover-window policy in account-migration.md +(claims by day 90, mailout, expire by day 365) no longer applies. +Most users will sign in via password and never need to claim; +linking GitHub is a separate (later) UX. + +## Validation + +- [ ] All six spec files updated to reflect the new strategy. +- [ ] No dangling references to "claim at cutover is required" or "deleted on first successful claim." +- [ ] New `specs/behaviors/password-hash-rotation.md` covers the hash detection + rehash-on-login rule. +- [ ] `specs/deferred.md` flips to "kept for migrated users, sunset deferred." +- [ ] PR review confirms direction before any code changes land. + +## Risks / unknowns + +- **Tightly-coupled change set.** Six files cross-reference each + other; landing only some of them creates a half-coherent spec. + Bundle as one PR. +- **Password-reset is genuinely new code.** Out of scope for *this* + spec PR but necessary for the implementation that follows. +- **Banner copy is placeholder.** "Connect a GitHub account to make + sign-in easier" is okay-but-not-great. Worth iterating with the + team during impl. +- **Anti-enumeration depth.** The login endpoint must return identical + responses for "no such user" and "wrong password" — and ideally + comparable timing too (else a timing oracle leaks user existence). + Implementation detail captured in the password-hash-rotation spec. +- **Existing claim-flow code stays useful** but its UX framing changes + significantly. The implementation PR (separate) needs to decide + whether to refactor the existing routes or just add the new ones + alongside. + +## Notes + +*(filled at done time)* + +## Follow-ups + +*(filled at done time)* From 266b6f0828c0ba99bbf1a2ed8c062302899d8790 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 30 May 2026 22:17:25 -0400 Subject: [PATCH 2/2] =?UTF-8?q?docs(specs):=20login=20migration=20?= =?UTF-8?q?=E2=80=94=20password-for-existing=20+=20nag-to-link-github?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A directional change to the cutover auth strategy. Today's spec treats GitHub OAuth as the only auth path and gates existing laddr users with a one-shot claim flow at first sign-in. The new design: - Existing users keep password sign-in indefinitely. Every successful login auto-rehashes the unsalted-SHA1 credential (confirmed against JarvusInnovations/emergence-skeleton User.class.php:33) to argon2id so the corpus drifts toward modern hashing without forcing resets. - New accounts are still GitHub-only — preserves the spam argument that drove the original GitHub-only decision (which was specifically about new-account sign-up, not existing users). - Linking GitHub becomes a per-user opt-in surfaced via a persistent yellow banner on /account. No deadline pressure; users link when ready. The banner is the only nag mechanism. - Sunset of password sign-in is deferred — no fixed deprecation date or coverage threshold. Tracked separately when usage data justifies. Doc-only change. The implementation work (POST /api/auth/login route, rehash logic, link flow, banner, password-reset routes) lands in a follow-up plan after these specs are reviewed. Spec changes: specs/deferred.md — flips "email/password auth" from "deleted entirely" to "sign-up GitHub-only, existing users keep password indefinitely." specs/behaviors/account-migration.md — rewritten around the three sign-in paths (GitHub-new, GitHub-matched, legacy-password) and the link-when-ready banner. Cutover-window deadline policy removed; users aren't time-pressured. specs/behaviors/password-hash-rotation.md (new) — SHA-1/bcrypt/argon2 format detection, constant-time compare, rehash-on-login, anti-enumeration timing (dummy argon2 verify on missing user/cred paths), argon2id parameter source-of-truth. specs/api/auth.md — adds POST /api/auth/login, POST /api/auth/password-reset/{request,confirm}, POST /api/auth/ link-github. /api/auth/me grows hasGitHubLink + lastLoginMethod. Notes section updated to reflect the password-for-migrated-users posture. specs/screens/login.md — adds a collapsed secondary "Or sign in with your Code for Philly password" disclosure with the password form + "Forgot your password?" affordance. specs/screens/account.md — Card 1 (Identity) gains State A (linked) / State B (not yet linked, shows banner). Link action wired to POST /api/auth/link-github. Co-Authored-By: Claude Opus 4.7 (1M context) --- plans/login-migration-strategy.md | 33 ++- specs/api/auth.md | 154 ++++++++++++- specs/behaviors/account-migration.md | 266 ++++++++++++++++------ specs/behaviors/password-hash-rotation.md | 118 ++++++++++ specs/deferred.md | 10 +- specs/screens/account.md | 34 ++- specs/screens/login.md | 37 ++- 7 files changed, 545 insertions(+), 107 deletions(-) create mode 100644 specs/behaviors/password-hash-rotation.md diff --git a/plans/login-migration-strategy.md b/plans/login-migration-strategy.md index df560c0..5c3f2da 100644 --- a/plans/login-migration-strategy.md +++ b/plans/login-migration-strategy.md @@ -1,5 +1,5 @@ --- -status: in-progress +status: done depends: [] specs: - specs/api/auth.md @@ -9,6 +9,7 @@ specs: - specs/screens/account.md - specs/deferred.md issues: [] +pr: 117 --- # Plan: login migration strategy — keep password login for existing users + nag-to-link-GitHub @@ -128,11 +129,11 @@ linking GitHub is a separate (later) UX. ## Validation -- [ ] All six spec files updated to reflect the new strategy. -- [ ] No dangling references to "claim at cutover is required" or "deleted on first successful claim." -- [ ] New `specs/behaviors/password-hash-rotation.md` covers the hash detection + rehash-on-login rule. -- [ ] `specs/deferred.md` flips to "kept for migrated users, sunset deferred." -- [ ] PR review confirms direction before any code changes land. +- [x] All six spec files updated to reflect the new strategy. +- [x] No dangling references to "claim at cutover is required" or "deleted on first successful claim." (`grep -r "deleted on first" specs/` returns no hits in the rewritten files; the claim flow is now framed as merge-helper-only.) +- [x] New `specs/behaviors/password-hash-rotation.md` covers SHA-1 detection + constant-time compare + rehash-on-login + argon2 params + anti-enumeration timing. +- [x] `specs/deferred.md` flips to "email/password account creation (sign-up) → GitHub-only," with explicit note that existing users keep password sign-in indefinitely. +- [ ] PR review confirms direction before any code changes land. *(this PR is the review surface)* ## Risks / unknowns @@ -155,8 +156,24 @@ linking GitHub is a separate (later) UX. ## Notes -*(filled at done time)* +Doc-only PR. Two commits: plan-open + spec edits. + +Surprises: + +- **The "deferred entry" actually had a load-bearing argument the new design honors.** The original `deferred.md` "Email/password authentication" entry justified GitHub-only by citing laddr's spam-load problem. That argument is specifically about *sign-up*, not about existing users — so the new entry splits accordingly. The spam reasoning still applies to new-account creation, and the new design keeps that locked to GitHub. It does NOT apply to existing migrated users. +- **`crypto.timingSafeEqual` is strict on length.** The implementation note in `password-hash-rotation.md` calls this out: length-check first, then compare. Otherwise the length mismatch itself becomes a timing oracle (timingSafeEqual throws synchronously on length mismatch). +- **Anti-enumeration depth.** Initially considered keeping the "no such user" early-bail and just returning the uniform error, but that leaves a measurable timing gap (no DB read vs. hash verify is ~50 ms). The spec now requires the verifier to run a *dummy argon2* even when the user doesn't exist, so all four paths (no user, no credential, wrong password, unknown format) take comparable wall-clock time. Implementation will need to be careful here. +- **The claim flow stops being a gate but doesn't die.** `/account/claim-legacy` is repurposed from "the gate at first sign-in" to "the merge helper for duplicate accounts." Most users will never see it. The existing code (`AccountClaimService` etc.) can mostly stay; the UX framing changes. ## Follow-ups -*(filled at done time)* +- **Implementation plan** — `plans/login-migration-impl.md` to-be-written. Covers: + - `POST /api/auth/login` route + verifier refactor (split `verifyLaddrPassword` into the three-algorithm dispatcher per the password-hash-rotation spec) + - argon2id dep + params constants + - `LegacyPasswordCredential` schema gains `lastUsedAt` field + - `POST /api/auth/password-reset/{request,confirm}` routes + `PasswordToken` private record + - `POST /api/auth/link-github` route + the link-mode OAuth callback variant + - `/api/auth/me` response shape additions + - SPA: secondary login form, password-reset flow, account banner +- **Sunset planning** — when usage data justifies, write `plans/legacy-password-sunset.md` to set a deprecation date. +- **`POST /api/auth/account-level`** (#33) — still open, unrelated to this work but adjacent. Stays separate. diff --git a/specs/api/auth.md b/specs/api/auth.md index b0c6cd9..c1ccf53 100644 --- a/specs/api/auth.md +++ b/specs/api/auth.md @@ -1,6 +1,13 @@ # API: Authentication -GitHub OAuth is the sole primary auth method. Sessions are stateless JWTs (per [behaviors/authorization.md](../behaviors/authorization.md)). Email/password sign-in does not exist — see [deferred.md](../deferred.md). +Two primary sign-in paths: + +1. **GitHub OAuth** — the only path for new accounts. Pre-cutover laddr users who match a verified GitHub email also use this path (auto-link, no claim ceremony). +2. **Legacy password** — `POST /api/auth/login` for pre-cutover laddr users who remember their old credentials. Sessions are minted exactly as for GitHub sign-in. Per [behaviors/account-migration.md](../behaviors/account-migration.md). + +Sessions are stateless JWTs (per [behaviors/authorization.md](../behaviors/authorization.md)). + +Account creation (sign-up) is GitHub-only — there is no `/api/auth/register` endpoint, per [deferred.md](../deferred.md). ## Endpoints @@ -8,13 +15,17 @@ GitHub OAuth is the sole primary auth method. Sessions are stateless JWTs (per [ | ------ | ---- | ---- | ------- | | `GET` | `/api/auth/github/start` | public | Begin GitHub OAuth flow. Redirects to GitHub. | | `GET` | `/api/auth/github/callback` | public | OAuth callback. Exchanges code for tokens, resolves identity, issues session or routes to claim flow. | -| `GET` | `/api/auth/me` | public (with optional session) | Returns current Person + accountLevel, or anonymous. | +| `POST` | `/api/auth/login` | public | Legacy password sign-in. Rehashes on success per [password-hash-rotation.md](../behaviors/password-hash-rotation.md). | +| `POST` | `/api/auth/password-reset/request` | public | Request a one-time password-reset link to the email on file. | +| `POST` | `/api/auth/password-reset/confirm` | public (token) | Complete a password reset using the emailed token. | +| `POST` | `/api/auth/link-github` | user | Start a GitHub OAuth round-trip to bind a GitHub identity to the current Person. Used by the `/account` "Connect GitHub" banner. | +| `GET` | `/api/auth/me` | public (with optional session) | Returns current Person + accountLevel + `hasGitHubLink` + `lastLoginMethod`, or anonymous. | | `POST` | `/api/auth/refresh` | refresh-cookie | Mint a new access+refresh pair. | | `POST` | `/api/auth/logout` | user | End the current session. | | `GET` | `/api/auth/sessions` | user | List remembered sessions. | | `POST` | `/api/auth/sessions/:jti/revoke` | user (self) | Revoke a specific session. | -The account-claim flow (`/api/account-claim/*`) is documented in [api/account-claim.md](account-claim.md). It's invoked from `/api/auth/github/callback` when the OAuth identity doesn't match an existing linked Person but matches a legacy candidate. +The account-claim helpers (`/api/account-claim/*`) cover the rare "I have a duplicate account, merge it" case — see [api/account-claim.md](account-claim.md) and [behaviors/account-migration.md](../behaviors/account-migration.md). They are no longer a gate at first sign-in. ## GET /api/auth/github/start @@ -85,9 +96,128 @@ In every successful case the user is redirected to either `return` (validated sa - `502 bad_gateway` with code `github_unreachable` — GitHub API call failed; user redirected to `/login?error=github_unreachable` - `403 forbidden` with code `email_unverified` — GitHub returned no verified email (user has email visibility off AND no verified primary); user redirected to `/login?error=email_unverified` with a help message about GitHub email visibility +## POST /api/auth/login + +Legacy password sign-in. Open to any user with a `LegacyPasswordCredential` on file. Per [behaviors/account-migration.md](../behaviors/account-migration.md) and [behaviors/password-hash-rotation.md](../behaviors/password-hash-rotation.md). + +### Request + +```json +{ + "usernameOrEmail": "jane", + "password": "" +} +``` + +`usernameOrEmail` is resolved against `Person.slug` first, then `PrivateProfile.email`. + +### Behavior + +1. Resolve `usernameOrEmail` to a Person; if unresolved, run a dummy argon2 verify against a fixed plaintext (anti-enumeration timing floor) and 401. +2. Load `LegacyPasswordCredential` for the Person; if absent, same dummy-verify-then-401. +3. Detect hash algorithm by format; verify per [password-hash-rotation.md](../behaviors/password-hash-rotation.md). +4. On success: **rehash** the supplied password to argon2id with current params, overwrite the credential record (`passwordHash`, `lastUsedAt = now`), mint an access+refresh JWT pair, set cookies, 200. +5. On failure: 401, uniform error code, no algorithm or user-existence leak. + +### Response — 200 + +```json +{ "success": true, "data": { "person": { /* PersonResponse */ } } } +``` + +Plus `Set-Cookie` headers for `cfp_session` and `cfp_refresh`. + +### Errors + +- `401 unauthenticated` with `error.code = "invalid_credentials"` — covers no-such-user, wrong-password, unknown-hash-format. Single response, comparable timing across cases. +- `429 too_many_requests` — per the auth-endpoint rate cap (10/min/IP) in [api/conventions.md](conventions.md). + +## POST /api/auth/password-reset/request + +Initiates a password reset by emailing a one-time signed token to the address in `PrivateProfile.email`. + +### Request + +```json +{ "usernameOrEmail": "jane@example.com" } +``` + +### Behavior + +1. Resolve to a Person; if unresolved or no email on file, do nothing (no enumeration). +2. Mint a `PasswordToken` (private-store record, 1-hour expiry, single-use) with `personId` + a CSPRNG token. +3. Send an email to `PrivateProfile.email` containing the link `https://<host>/login/reset?token=<token>`. + +### Response — 202 + +```json +{ "success": true, "data": { "delivered": true } } +``` + +Always 202, regardless of whether the email actually resolved or sent. The body is informational; the *real* signal is that the user receives (or doesn't receive) the email. + +### Errors + +- `429 too_many_requests` — same 10/min/IP cap as `/api/auth/login`. + +## POST /api/auth/password-reset/confirm + +Completes a password reset using a token from the email link. + +### Request + +```json +{ + "token": "<opaque from email>", + "password": "<new plaintext>" +} +``` + +### Behavior + +1. Look up the `PasswordToken`; reject expired, used, or unknown tokens (401 uniform). +2. Hash the new password with argon2id (current params). +3. Overwrite the Person's `LegacyPasswordCredential.passwordHash`, set `lastUsedAt = now`. +4. Mark the `PasswordToken` as used. +5. Mint an access+refresh JWT pair (the reset doubles as a sign-in), set cookies, 200. + +### Response — 200 + +Same shape as `POST /api/auth/login`. + +### Errors + +- `401 unauthenticated` with `error.code = "invalid_token"` +- `422 validation_failed` if the new password violates the minimum policy (≥ 8 chars at v1 — TBD with the implementation PR) + +## POST /api/auth/link-github + +Binds a GitHub identity to the currently-signed-in Person. Initiates a GitHub OAuth round-trip; the callback at `/api/auth/github/callback` recognizes the "link" mode (signed-session cookie carries a `link` scope tag) and finalizes the link rather than minting a new session. + +### Request + +Empty body. The flow is purely redirect-driven. + +### Behavior + +1. The route sets a short-lived signed cookie (`cfp_oauth_session`) with `mode = 'link'` and the current `personId`, then redirects to GitHub OAuth (same `?return=...` mechanics as `/api/auth/github/start`). +2. The callback verifies the OAuth result. If `Person.githubUserId` is already set on the linking Person: `409 github_already_linked`. If the GitHub `id` is bound to a *different* Person: `409 github_id_in_use_elsewhere` (resolved by admin merge, not self-service). +3. Otherwise: set `Person.githubUserId = gh.id`, `Person.githubLogin = gh.login`, `Person.githubLinkedAt = now`. Refresh `PrivateProfile.email` to the GitHub primary verified email **only if the user consents** at the link-confirmation screen (toggle defaults to "keep current email"). +4. Redirect to `?return` or `/account`. + +### Response + +302 redirect to GitHub; then 302 back to `?return` or `/account` after the callback. + +### Errors (rendered as `/account?error=<code>` after the callback) + +- `github_already_linked` — caller already has a GitHub link +- `github_id_in_use_elsewhere` — another Person already owns this `gh.id` +- `oauth_state_mismatch`, `oauth_session_invalid`, `github_unreachable` — same as `/api/auth/github/start` + ## GET /api/auth/me -Returns the current Person (full PersonResponse shape — see [api/people.md](people.md)) plus `accountLevel`. Used by the SPA on load to bootstrap the auth context. +Returns the current Person (full PersonResponse shape — see [api/people.md](people.md)) plus `accountLevel`, `hasGitHubLink`, and `lastLoginMethod`. Used by the SPA on load to bootstrap the auth context and decide whether to render the "Connect GitHub" banner. ### Response — 200 @@ -96,12 +226,16 @@ Returns the current Person (full PersonResponse shape — see [api/people.md](pe "success": true, "data": { "person": { /* PersonResponse */ }, - "accountLevel": "staff" + "accountLevel": "staff", + "hasGitHubLink": true, + "lastLoginMethod": "github" } } ``` -If no session, returns 200 with `data.person = null` and `data.accountLevel = "anonymous"`. (We deliberately do not 401 here — the frontend calls this on every page load including public pages.) +`hasGitHubLink` is `Person.githubUserId !== null`. `lastLoginMethod` is one of `"github" | "legacy_password" | "password_reset"`; the SPA can use it to render UI hints (e.g., "Signed in via password — connect GitHub for faster sign-in next time" inline on `/account`). + +If no session, returns 200 with `data.person = null`, `data.accountLevel = "anonymous"`, `hasGitHubLink = false`, `lastLoginMethod = null`. (We deliberately do not 401 here — the frontend calls this on every page load including public pages.) The PersonResponse for self includes `email` (fetched from PrivateProfile) and `newsletter` state. For staff viewing other people, see [api/people.md](people.md) on which private fields are visible. @@ -162,8 +296,10 @@ Revokes a non-current session by `jti`. Unchanged from Phase 1. ## Notes -- **No email/password endpoints.** `/api/auth/register`, `/api/auth/login`, `/api/auth/password-reset/*` do not exist. Trying to call them returns `404 not_found`. -- **GitHub identity is immutable per Person.** Once `Person.githubUserId` is set, it doesn't change. Unlinking GitHub is not a v1 feature; if a user loses access to their GitHub account, they recover through a staff-mediated process. See [behaviors/account-migration.md](../behaviors/account-migration.md). -- **Email is GitHub-sourced.** `PrivateProfile.email` is refreshed on every successful OAuth callback to the user's current primary verified GitHub email. We don't expose a "change email" UI; users change their email on GitHub. +- **Sign-up is GitHub-only.** `/api/auth/register` does not exist; trying to call it returns `404 not_found`. New accounts are only created through the GitHub OAuth callback's "no legacy match" branch. +- **Password sign-in is for migrated users only.** `POST /api/auth/login` accepts any user with a `LegacyPasswordCredential` on file. Records are populated from the laddr import; no rewrite-code path *creates* a new credential except via `POST /api/auth/password-reset/confirm` for an existing record. +- **Every successful password sign-in rehashes** the supplied plaintext to argon2id per [password-hash-rotation.md](../behaviors/password-hash-rotation.md). Laddr's unsalted SHA-1 drifts toward modern hashing without user action. +- **GitHub identity is immutable per Person, once set.** A self-service "unlink GitHub" flow is not v1. If a user loses access to their GitHub account, they can fall back to password sign-in if they remember it, then `password-reset` from their email-on-file. If both are dead, recovery is staff-mediated. +- **Email is GitHub-sourced when linked.** Once a Person has a GitHub link, `PrivateProfile.email` is refreshed on every successful OAuth callback to the user's current primary verified GitHub email. Password-only users keep whatever email was imported from laddr; they don't have a self-service "change email" UI. - **The OAuth state cookie expires aggressively** (10 minutes) so abandoned flows don't accumulate. - **PKCE is required** even though we have a client secret on the server — PKCE protects against authorization-code interception in addition to whatever client-secret protection we already have. diff --git a/specs/behaviors/account-migration.md b/specs/behaviors/account-migration.md index 21dd2da..09edb3b 100644 --- a/specs/behaviors/account-migration.md +++ b/specs/behaviors/account-migration.md @@ -2,20 +2,70 @@ ## Rule -A laddr-era user signing in for the first time on the rewrite must be able to **claim their legacy account** so their slug, project memberships, authored updates/buzz, and Slack identity all carry forward. The cutover preserves continuity; nobody starts over. +A laddr-era user signs into the rewrite using **whichever they +remember** — their old laddr password OR a GitHub account that +matches their legacy email. Either path produces a session against +their existing Person record; their slug, project memberships, +authored updates/buzz, and Slack identity all carry forward. + +New accounts can only be created via GitHub OAuth. Existing legacy +accounts keep password sign-in indefinitely. We *encourage* linking +GitHub (via a banner on `/account`) but never gate access on it. + +The cutover preserves continuity; nobody starts over and nobody is +locked out by a deadline. ## Applies To -- [api/auth.md](../api/auth.md) — GitHub OAuth callback decides whether to issue a session, route to claim, or create a new Person -- [api/account-claim.md](../api/account-claim.md) — the endpoints the claim screens hit -- [screens/account-claim.md](../screens/account-claim.md) — the user-facing claim UI +- [api/auth.md](../api/auth.md) — `POST /api/auth/login` (legacy password), `GET /api/auth/github/callback` (OAuth), `POST /api/auth/link-github` +- [api/account-claim.md](../api/account-claim.md) — the "claim another legacy account" helper for the rare merge case +- [screens/login.md](../screens/login.md) — login form with GitHub primary + password secondary +- [screens/account.md](../screens/account.md) — GitHub linking card with banner state +- [behaviors/password-hash-rotation.md](password-hash-rotation.md) — rehash-on-login, format detection, anti-enumeration - [data-model.md](../data-model.md) — `Person.githubUserId`, `slackSamlNameId`, `PrivateProfile.email`, `LegacyPasswordCredential` -- [behaviors/private-storage.md](private-storage.md) — claims read+update the private store -- [behaviors/authorization.md](../behaviors/authorization.md) — staff-review path requires admin +- [behaviors/private-storage.md](private-storage.md) — login + link write to the private store +- [behaviors/authorization.md](authorization.md) — staff-mediated paths for edge cases + +## The three sign-in paths + +```text + ┌──────────────────────────────────────────────────────┐ + │ /login │ + └────────────────┬─────────────────────┬───────────────┘ + │ │ + [Sign in with │ │ [Sign in with + GitHub] │ │ password] + │ │ + ▼ ▼ + ┌──────────────────────┐ ┌──────────────────────┐ + │ GitHub OAuth → │ │ POST /api/auth/login │ + │ resolve identity │ │ verify, rehash if │ + └──────┬───────────────┘ │ SHA-1, mint session │ + │ └──────────────────────┘ + │ + ┌───────────┼──────────────────────┐ + │ │ │ + no laddr matches a matches none + match legacy candidate but user might + │ via email or have a legacy + │ username hint account too (rare) + ▼ ▼ ▼ + create new sign in to sign in fresh; + Person legacy Person, offer "Claim + (GitHub- auto-link another legacy + only) GitHub account" from + /account +``` + +Path 1: **GitHub sign-in for a new account.** Fresh Person + PrivateProfile, GitHub-linked. The default for anyone who didn't exist on laddr. + +Path 2: **GitHub sign-in matching a legacy account.** OAuth callback finds a verified email or username hint matching a legacy Person. The Person is auto-linked (GitHub identity bound, future GitHub sign-ins skip this resolution). No claim flow speed bump; the email match is the proof. -## Signals available +Path 3: **Password sign-in (legacy users only).** User enters their old laddr username/email + password. The `POST /api/auth/login` route verifies against `LegacyPasswordCredential` and mints a session. On success, the supplied password is rehashed to argon2id (see [password-hash-rotation.md](password-hash-rotation.md)). -At the GitHub OAuth callback we have: +A user can have **both** a password credential and a GitHub link active at the same time. That's the steady state for migrated users who've linked GitHub but haven't sunset their password. + +## Signals available at the GitHub OAuth callback - `gh.id` — GitHub's stable numeric user ID - `gh.login` — GitHub username (mutable, but stable enough for soft hints) @@ -26,7 +76,7 @@ From the laddr import we have, for every legacy Person: - `Person.slug` (= laddr `Username`) — public - `PrivateProfile.email` (= laddr `Email`) — private -- `LegacyPasswordCredential.passwordHash` — private (until claimed and deleted) +- `LegacyPasswordCredential.passwordHash` — private (kept after login; see below) ## Matching algorithm at OAuth callback @@ -38,63 +88,125 @@ From the laddr import we have, for every legacy Person: 3. Combine candidates, dedupe by Person.id 4. Route based on candidate count: - 0 candidates → create fresh Person + PrivateProfile (no claim needed) - - 1 candidate → render single-candidate confirmation screen + - 1 candidate, email-match → auto-link, sign in (no confirmation screen) + - 1 candidate, username-match only → render single-candidate confirmation screen - N candidates → render multi-candidate picker ``` -Email-match is the strong signal. Username-match is a hint — used only to surface a candidate, never to auto-claim. +Email-match is the strong signal — the user controls the GitHub +account, GitHub verified the email, the laddr account uses the +verified email. **One-step auto-link.** No confirmation screen, +no claim ceremony. -## Claim outcomes +Username-match without email-match is a hint. Still requires +confirmation (the user might not be the legacy account holder). -Each claim path produces one of three outcomes: +## Legacy password sign-in -1. **Auto-claim** (email match + user confirms) — link GitHub identity to the legacy Person, delete `LegacyPasswordCredential`, refresh email -2. **Password-claim** (user types old username + password) — same as auto-claim, additionally verifying via `LegacyPasswordCredential` -3. **Decline** (user says "not me") — create fresh Person + PrivateProfile, leave the legacy candidate untouched +`POST /api/auth/login` accepts `{ usernameOrEmail, password }`: -## Three identity proofs +1. Resolve `usernameOrEmail` to a Person — first try `slug`, then `PrivateProfile.email` +2. Load `LegacyPasswordCredential` for the Person; missing → uniform 401 +3. Verify per [password-hash-rotation.md](password-hash-rotation.md): + - Detect hash algorithm by format prefix + - Constant-time compare for SHA-1; library-native compare for bcrypt/argon2 +4. On success: **rehash** the supplied plaintext to argon2id and overwrite the credential record in the same request +5. On failure: uniform 401 `{ error.code: "invalid_credentials" }` — no distinction between "no such user," "wrong password," or "unknown hash format" -For the user to claim a candidate, they must satisfy **at least one** of: +The endpoint is rate-limited at the auth-endpoint cap (10/min/IP per [api/conventions.md](../api/conventions.md)). -### A. Email match +## Linking GitHub from an existing password-only session -The GitHub identity has a verified email that matches `PrivateProfile.email` for the candidate. The fact that GitHub verified the email is the proof. +`POST /api/auth/link-github`: -**No additional user action required** — the OAuth flow already validated they control the GitHub account, which validated the email. +A signed-in user (regardless of which path they signed in via) can +link a GitHub account to their Person. The endpoint: -### B. Old-password verification +1. Starts a GitHub OAuth round-trip with a `link` scope tag in the signed session cookie (to distinguish it from a fresh sign-in) +2. After the callback returns with `gh.id`: + - If `Person.githubUserId` is already set → 409 conflict, `error.code = "github_already_linked"` + - If another Person has this `gh.id` → 409 conflict, `error.code = "github_id_in_use_elsewhere"` (rare — admin must merge) + - Otherwise: set `Person.githubUserId = gh.id`, `Person.githubLogin = gh.login`, `Person.githubLinkedAt = now` +3. Optionally refresh `PrivateProfile.email` to the GitHub primary verified email (the user can decline this on the confirmation screen) + +After linking, the user can sign in via *either* path. The password credential is **not** removed automatically — users keep both options until they explicitly remove the password (deferred) or we sunset. + +## The nag (banner on `/account`) + +A persistent yellow banner on `/account` when `Person.githubUserId === null`: + +> **Connect a GitHub account to make sign-in easier.** GitHub sign-in +> is faster and works the same as your password. No deadline — this +> is just a recommendation. +> +> [Connect GitHub →] + +The banner is the **only** nag mechanism. No modal interrupts, no +toast on every sign-in, no email reminders. Click "Connect GitHub" → +start the link flow. Linking → banner disappears. -The user provides their pre-cutover username + password. The API: +If a user has already linked, the Identity card on `/account` instead +shows the green "Connected as @login" treatment — see +[screens/account.md](../screens/account.md). -1. Looks up the candidate Person by `slug` -2. Fetches `LegacyPasswordCredential.passwordHash` -3. Verifies the supplied password against the hash using the original laddr algorithm (likely bcrypt; confirmed at migration time) -4. On match: claim succeeds, `LegacyPasswordCredential` is deleted +## Password recovery -This is for users whose pre-cutover email is dead but who remember their credentials. +Lost-password recovery works as it did on laddr: + +1. User enters their username or email on `/login` +2. Server emits a one-time signed token (`PasswordToken` private record, 1-hour expiry) via email to the address in `PrivateProfile.email` +3. User clicks the link, enters a new password +4. New password is hashed with argon2id and replaces the `LegacyPasswordCredential.passwordHash` + +If the email on file is dead, the user contacts staff (existing +side-channel path). Email change is GitHub-sourced (the rest of the +spec); for users who haven't linked GitHub, "change my email" is a +staff-mediated process. We don't expose self-service email change to +password-only users in v1. + +## Identity proofs (the rare "claim another account" case) + +After a user signs in (any path) and lands at their session, they may +realize they have an *additional* legacy account they want to merge +in. The existing claim helper at `/account/claim-legacy` handles this +case — it's the rebadged remnant of the old claim flow. + +For the merge to succeed, the user must satisfy one of: + +### A. Email match + +The GitHub identity (or current session) has a verified email matching `PrivateProfile.email` for the candidate. + +### B. Old-password verification + +User provides the candidate's pre-cutover password — verified against `LegacyPasswordCredential` per [password-hash-rotation.md](password-hash-rotation.md). On success the candidate is merged in (memberships, updates, buzz re-pointed) and the duplicate Person is hard-deleted. ### C. Staff approval -The user submits a claim request with their old slug + free-form proof (e.g., "I'm @alice in Slack — DM me"). The request goes to a staff queue. A staff member verifies via side-channel and approves or denies. +User submits a claim request with the old slug + free-form proof. Staff reviews via side-channel and approves or denies. Same as today. -This is the fallback when both A and B are unavailable. +These three proofs no longer gate first sign-in — they're only relevant for the manual "I have a duplicate account, merge it" case. ## Pre-cutover auto-link sweep -Before cutover, an admin script can pre-link Persons whose GitHub identity we know with confidence. Heuristics: +Before cutover, an admin script can pre-link Persons whose GitHub +identity we know with confidence: -- Project `developersUrl` is a GitHub repo and the laddr Person is its maintainer → fetch the repo's owner via GitHub API, match +- Project `developersUrl` is a GitHub repo and the laddr Person is its maintainer → match via the repo's owner - Anyone who manually added a `https://github.com/<login>` URL to their laddr bio -Pre-linked Persons skip the claim flow entirely on first sign-in — the OAuth callback's `byGithubUserId` lookup hits immediately. +Pre-linked Persons are GitHub-linked from day 0. They sign in via +either path; the banner stays hidden because `Person.githubUserId` is +already set. -The sweep is **opportunistic**, not exhaustive. It improves the first-experience for a subset of users; the rest go through the normal claim flow. +The sweep is **opportunistic**, not exhaustive — it removes friction +for a subset of users. ## Merge semantics -If a user signs in via GitHub, creates a fresh Person (rejecting all candidates), and *later* realizes they had a legacy account, they can run a manual merge through `/account/claim-legacy` (post-onboarding claim — see [api/account-claim.md](../api/account-claim.md)). - -The merge direction is **legacy-survives, fresh-folds-in**: +If a user signs in fresh via GitHub (no auto-link), then later +realizes they had a legacy account, the merge direction is +**legacy-survives, fresh-folds-in**: - All records authored by the fresh Person (updates, buzz, help-wanted, memberships) are re-pointed to the legacy Person's `id` - The fresh Person is deleted (hard-delete; its `id` is gone) @@ -105,69 +217,81 @@ Merge is admin-mediated (uses the staff approval path) to prevent accidental or ## Identity continuity for Slack -`Person.slackSamlNameId` (immutable per-Person, see [api/saml.md](../api/saml.md)) preserves Slack identity through: +`Person.slackSamlNameId` (immutable per-Person, see +[api/saml.md](../api/saml.md)) preserves Slack identity through: - The migration (populated from `slug` at import time) - Slug renames after cutover (stays put even if `slug` changes) -- Account claim (preserved because the legacy Person record is the one that's linked, not a new one) +- Either sign-in path (the legacy Person record is the one that's signed-into, not a new one) -A user's Slack workspace identity is therefore stable for the entire arc — laddr through rewrite through any future renames or claims. +A user's Slack workspace identity is therefore stable for the entire +arc — laddr through rewrite through any future renames or linkings. ## Anti-enumeration -The claim flow handles inputs the user may have wrong (old emails, old slugs). To avoid leaking which laddr accounts exist: +The login + recovery flows handle inputs the user may have wrong (old +emails, old slugs, wrong passwords). To avoid leaking which laddr +accounts exist: -- **Old-password-verification endpoint** returns the same response for "no such slug" and "wrong password" — `401 unauthenticated` with `error.code = "claim_credentials_invalid"`. -- **Staff approval submission** always returns `202 accepted` regardless of whether the claimed slug exists. +- **`POST /api/auth/login`** returns identical responses for "no such user," "wrong password," and "unknown hash format" — `401 unauthenticated` with `error.code = "invalid_credentials"`. Implementation must also normalize timing across these paths (constant-time SHA-1 compare, plus an artificial floor delay if the early bail-out is meaningfully faster than the verify path). +- **`POST /api/auth/password-reset/request`** always returns `202 accepted` regardless of whether the email exists. The actual mail is sent only if the address resolves. - **Candidate enumeration at OAuth callback** is limited to candidates matching the user's actual GitHub-verified emails — we never reveal accounts the user couldn't have known about. -## Edge cases +## Coverage metric (for future sunset planning) -**User has multiple legacy accounts** (rare but possible — different emails over time, one person with two profiles) +`LegacyPasswordCredential` carries a `lastUsedAt` field (added with +this design). The operator can report on: -- All matching candidates surface in the picker. -- The user picks one; the others remain unclaimed. -- They can claim subsequent legacy accounts via `/account/claim-legacy` later and run a merge. +- Total `LegacyPasswordCredential` records +- Active password-sign-in users in the last 30/90/365 days +- Coverage % of `Person.githubUserId !== null` across active accounts -**User's verified GitHub emails match different legacy accounts** +When that coverage is high enough to justify a sunset, a separate +plan sets a deprecation date and updates this spec. Until then, no +fixed sunset. -- Multi-candidate picker. User picks one. +## Sunset (deferred) -**Same legacy candidate matches via both email and username** +Password sign-in for migrated users has no fixed deprecation date in +v1. The triggering signal — almost certainly a coverage threshold +("≥95% of monthly-active accounts have linked GitHub") — is captured +above and tracked separately when usage data justifies action. -- Single candidate shown (deduped on `Person.id`). User confirms once. +## Edge cases -**User starts the claim flow but abandons** (closes the tab after OAuth) +**User has multiple legacy accounts** (rare but possible — different emails over time) -- Claim-pending JWT expires in 5 min. Next sign-in restarts the OAuth flow and re-resolves. -- No half-claimed state persists. +- Sign-in via password works against whichever one's credentials they remember +- Multiple email-matches on GitHub OAuth surface in the candidate picker; user picks one +- Subsequent merges happen via `/account/claim-legacy` -**A legacy Person is claimed but `LegacyPasswordCredential` failed to delete** (rare bucket failure) +**User's verified GitHub emails match different legacy accounts** -- The credential is now unreachable (the Person it referenced is GitHub-linked, so the password-claim path won't accept it again — it checks `Person.githubUserId is null` before allowing password verification). The orphan record is cleaned up by the reconciliation script. +- Multi-candidate picker. User picks one. Same as today. **User claims their account, then loses access to their GitHub account** -- v1 has no self-service GitHub-unlink flow. The user contacts staff, who can manually clear `Person.githubUserId` after side-channel verification (admin action, audit-logged). -- After unlink, the Person is in the "unclaimed" state again — they can sign in via a different GitHub account and re-claim, via email-match if email still works, else password-verification (if the cred is still there — usually deleted on first claim), else staff approval. +- v1 has no self-service GitHub-unlink flow. The user can still sign in via password if they remember it. +- If they've also forgotten the password, they recover via `password-reset` to their email-on-file. +- If the email is dead too, staff-mediated recovery. -## Cutover-window policy +**User has a GitHub link AND a password credential, wants to remove the password** -The `LegacyPasswordCredential` records are populated at cutover. Realistically, most users won't claim immediately. Suggested policy (operational decision, not spec-locked): +- Not v1. Deferred until sunset planning happens. -- **Day 0:** cutover. All laddr Persons are in unclaimed state. `LegacyPasswordCredential` records exist for all of them. -- **Day 0–90:** active claim period. Users sign in via GitHub OAuth; claim flow surfaces candidates. -- **Day 90:** mailout to remaining unclaimed addresses (via the address in `PrivateProfile.email`) reminding them to claim. -- **Day 180:** unclaimed `LegacyPasswordCredential` records can be deleted (the legacy Persons remain — anyone showing up later goes through staff approval). -- **Day 365:** consider soft-deleting unclaimed Persons or moving them to an inactive state. +**Legacy Person was imported but has no email** (legacy data hygiene) -This is operational policy, not enforced in code. +- Password sign-in by username still works. +- Password recovery doesn't (no destination). Staff path only. +- Banner still encourages linking GitHub; that path then captures a GitHub-verified email for the Person. ## Coordinates with -- [api/auth.md](../api/auth.md) — the OAuth callback is the entry point -- [api/account-claim.md](../api/account-claim.md) — endpoint surface -- [screens/account-claim.md](../screens/account-claim.md) — the UI +- [api/auth.md](../api/auth.md) — `POST /api/auth/login`, OAuth flow, link-github +- [behaviors/password-hash-rotation.md](password-hash-rotation.md) — rehash-on-login mechanics +- [api/account-claim.md](../api/account-claim.md) — the "claim another legacy account" merge helper +- [screens/login.md](../screens/login.md) — primary GitHub + secondary password +- [screens/account.md](../screens/account.md) — link banner + Identity card states - [api/saml.md](../api/saml.md) — Slack identity continuity - [data-model.md](../data-model.md) — fields involved -- [behaviors/private-storage.md](private-storage.md) — claim reads/writes the private store +- [behaviors/private-storage.md](private-storage.md) — login and link both write the private store diff --git a/specs/behaviors/password-hash-rotation.md b/specs/behaviors/password-hash-rotation.md new file mode 100644 index 0000000..52a243b --- /dev/null +++ b/specs/behaviors/password-hash-rotation.md @@ -0,0 +1,118 @@ +# Behavior: Password Hash Rotation + +## Rule + +`LegacyPasswordCredential.passwordHash` carries hashes in **any of three** algorithms — laddr's unsalted SHA-1, bcrypt, or argon2id. The verifier detects algorithm by format and, on every successful login, **rehashes** the supplied plaintext to argon2id and overwrites the stored hash. + +The result: active users drift toward modern hashing without forcing a password reset. SHA-1 hashes only persist for accounts that haven't been used since cutover. + +## Applies To + +- [api/auth.md](../api/auth.md) — `POST /api/auth/login` performs the verify + rehash; `POST /api/auth/password-reset/confirm` writes only argon2id +- [behaviors/account-migration.md](account-migration.md) — the legacy sign-in path that triggers rehash on every successful login +- [data-model.md → LegacyPasswordCredential](../data-model.md) — schema carries the algorithm-agnostic `passwordHash` string +- [behaviors/private-storage.md](private-storage.md) — credential reads and writes go through the private store + +## Hash formats + +| Algorithm | Origin | Format prefix | Notes | +| --------- | ------ | ------------- | ----- | +| Unsalted SHA-1 | Laddr's `User.class.php` (`$passwordHasher = 'SHA1'`) | bare 40-char lowercase hex (no prefix) | Broken — rainbow-tables crack every common password. Every legacy import lands here. | +| bcrypt | Defensive fallback for any future bcrypt-imported credentials | `$2a$`, `$2b$`, `$2y$` | Acceptable. Library-native verify. Not the target. | +| argon2id | Native rewrite | `$argon2id$` | The target — every rehash and every new password lands here. | + +Detection is by prefix: + +```text +if passwordHash starts with "$argon2id$" → argon2id +if passwordHash starts with "$2a$" or "$2b$" or "$2y$" → bcrypt +if passwordHash matches /^[0-9a-f]{40}$/ → unsalted SHA-1 +otherwise → unknown_format (verify fails, uniform 401) +``` + +The SHA-1 detection is permissive on input ("bare 40-char hex") and conservative on intent — any unrecognized shape is treated as `unknown_format`, never as a fallback to SHA-1. + +## Verification + +For each algorithm: + +- **argon2id** — `argon2.verify(stored, plaintext)`. Library-native; constant-time by design. +- **bcrypt** — `bcrypt.compare(plaintext, stored)`. Library-native; constant-time by design. +- **SHA-1** — Compute `sha1(plaintext)` → 40-char hex; constant-time-compare against `stored` using `crypto.timingSafeEqual` (after equal-length check). **Never** use `===` or `==` — laddr's PHP code used loose `==` which is timing-leaky on hash compare. + +If any branch throws (corrupt hash, encoding mismatch, etc.), treat as `invalid_credentials` — never let an internal error leak through as a distinct response. + +## Rehash on every successful login + +`POST /api/auth/login` (per [api/auth.md](../api/auth.md)) runs after a successful verify: + +```text +1. verify(plaintext, stored) === true +2. newHash = argon2id(plaintext, params = current default) +3. write LegacyPasswordCredential with passwordHash = newHash, lastUsedAt = now +4. mint session, return success +``` + +The rehash happens **regardless of the source algorithm**. Reasoning: + +- SHA-1 sources need rehashing — that's the point of the rule. +- bcrypt sources are correct *today*, but argon2id is the project's chosen algorithm. Unifying on one algorithm simplifies the verifier and removes legacy paths. +- argon2id sources may have been hashed under older parameters (memory, iterations). Rehashing with current params keeps the credential corpus at the current security floor. + +The verifier returns `{ valid, needsRehash }` for clarity even though every `valid=true` case currently triggers a rehash: + +```ts +{ valid: true, needsRehash: true } // SHA-1 source +{ valid: true, needsRehash: true } // bcrypt source +{ valid: true, needsRehash: true } // argon2id source with old params +{ valid: true, needsRehash: false } // argon2id source with current params — write skipped +{ valid: false, needsRehash: false } // any failure +``` + +`needsRehash = false` for current-params argon2id avoids a useless write on every login. The detection: argon2's encoded hash carries its parameters; compare against the current parameter set; skip if identical. + +## Argon2id parameters + +Implementation chooses parameters at module load from a single source of truth (`apps/api/src/auth/argon2-params.ts` or similar). Recommended starting values: + +- `memoryCost`: 19456 KiB (≈ 19 MiB) +- `timeCost`: 2 iterations +- `parallelism`: 1 + +These produce ~50 ms hashes on the production pod's CPU profile (Linode amd64). Within budget for `POST /api/auth/login` latency. + +Parameter changes are a **deliberate spec event**: bump the constants in code, deploy, and every subsequent successful login rehashes to the new floor. No retroactive backfill (the corpus drifts naturally). + +## Password recovery + +`POST /api/auth/password-reset/confirm` writes only argon2id — there is no path that produces a SHA-1 or bcrypt hash from rewrite code. SHA-1 hashes can only enter the system via the laddr import. + +## Anti-enumeration timing + +The verifier's responses across the three algorithms must be indistinguishable from a wall-clock attacker: + +- **Equal-length compare floor.** Even when `passwordHash` is absent or `unknown_format`, the route runs an `argon2id` hash against a fixed dummy plaintext before returning 401. This ensures "no such user" and "wrong password" take comparable time. +- **No early bail on missing credential.** If the username resolves to a Person with no `LegacyPasswordCredential`, the dummy verify path still runs. + +The constant-time SHA-1 compare uses `crypto.timingSafeEqual(Buffer.from(computed, 'hex'), Buffer.from(stored, 'hex'))` after length-checking — `timingSafeEqual` throws synchronously on length mismatch, which is itself a timing oracle if the lengths differ. Length-check first, then compare. + +## Sunset + +When SHA-1 source records drop to zero (every legacy user has either logged in once OR been deactivated), the SHA-1 detection path can be removed. Not a v1 concern; tracked as a follow-up signal in [account-migration.md](account-migration.md#sunset-deferred). + +## Operational metrics (for future sunset planning) + +`LegacyPasswordCredential` carries `lastUsedAt: iso8601 nullable` to support coverage reporting: + +- "How many active password users in the last 30/90/365 days?" +- "What % of those active users have linked GitHub?" +- "How many SHA-1 records remain (lastUsedAt is null OR pre-cutover)?" + +These feed the future sunset decision in [account-migration.md](account-migration.md#sunset-deferred). + +## Coordinates with + +- [api/auth.md](../api/auth.md) — the verify + rehash sits inside `POST /api/auth/login` +- [behaviors/account-migration.md](account-migration.md) — why password sign-in exists, who's eligible +- [data-model.md → LegacyPasswordCredential](../data-model.md) — credential record shape (passwordHash, lastUsedAt) +- [behaviors/private-storage.md](private-storage.md) — credential records live in the private store diff --git a/specs/deferred.md b/specs/deferred.md index 9e16bfa..80e24ad 100644 --- a/specs/deferred.md +++ b/specs/deferred.md @@ -132,12 +132,12 @@ When a deferred item is promoted, move it from this file into the relevant spec, - **Why:** Post velocity has been near-zero for years; a database-backed CMS with user logins is overkill. Markdown bodies in a content-typed sheet keep the PR-review ergonomics of files-in-code-repo while sitting on the same runtime + import pipeline as the rest of the data model. Authors get attribution via `authorId`, posts ride the data snapshot, and the API serves through the existing in-memory state with no Vite-bundle bloat for the index. The original "files in `apps/web/src/content/blog/`" replacement was drafted before gitsheets v1.2 made content-typed records viable; that approach is superseded by this one. - **Status:** Initial implementation landed via [#84](https://github.com/CodeForPhilly/codeforphilly-ng/issues/84) — full bodies loaded at boot. Lazy body loading (`queryAll({ withBody: false })`) and the richer reader experience are tracked in [#45](https://github.com/CodeForPhilly/codeforphilly-ng/issues/45). -### Email/password authentication +### Email/password account creation (sign-up) -- **What:** laddr's email + password sign-in flow with password-reset, email verification, etc. -- **Replaced by:** **GitHub OAuth** as the only primary auth method, *once the OAuth + account-claim flow is specified*. Until then, sessions exist only via seeded data from the laddr migration. -- **Why:** Spam/scam load on the laddr sign-up form was unmanageable even with recaptcha. GitHub-account-creation friction filters bad actors meaningfully better. The audience (civic-tech volunteers) overwhelmingly already has a GitHub account. Side benefit: deletes a lot of code — no password storage, no reset flow, no verification flow, no MFA roadmap. -- **What about laddr users without a GitHub account?** During the account-claim flow they may prove ownership of an old laddr record by typing their old username + password, in which case the laddr-imported `LegacyPasswordCredential` record validates them and is then deleted. After all migration claims are completed (or expire), no password machinery remains in the system. +- **What:** laddr's email + password sign-up flow. +- **Replaced by:** **GitHub OAuth** as the only path to *create* a new account. +- **Why:** Spam/scam load on the laddr sign-up form was unmanageable even with recaptcha. GitHub-account-creation friction filters bad actors meaningfully better. The audience (civic-tech volunteers) overwhelmingly already has a GitHub account. +- **What about existing laddr users?** They *keep* their password sign-in path indefinitely — see [behaviors/account-migration.md](behaviors/account-migration.md). The spam argument applies to new-account creation, not to existing users who already cleared whatever bar they cleared on laddr. A persistent banner on `/account` encourages (but doesn't require) linking a GitHub account; sunset of password sign-in for migrated users is **deferred** — no fixed deadline today. ### MySQL / any persistent relational database diff --git a/specs/screens/account.md b/specs/screens/account.md index a5591d5..69c1079 100644 --- a/specs/screens/account.md +++ b/specs/screens/account.md @@ -8,8 +8,9 @@ A self-service settings hub for the current user's identity, sessions, newslette ## Data Requirements -- `GET /api/auth/me` — current person + account level (returns the self-view including the private-store email + newsletter prefs) +- `GET /api/auth/me` — current person + account level + `hasGitHubLink` + `lastLoginMethod` (returns the self-view including the private-store email + newsletter prefs) - `GET /api/auth/sessions` — list of remembered sessions +- `POST /api/auth/link-github` — invoked from the "Connect GitHub" banner when `hasGitHubLink === false` ## Display Rules @@ -17,14 +18,36 @@ The page is a vertical stack of cards. ### Card 1: Identity -- **GitHub account** — the linked GitHub login, with a green check ("Connected — primary identity") +The card's contents depend on `me.hasGitHubLink`: + +**State A — GitHub already linked** (`me.hasGitHubLink === true`) + +- **GitHub account** — the linked GitHub login, with a green check ("Connected — sign in via GitHub or password") - Click "Manage on GitHub →" → opens `https://github.com/settings` in a new tab - - No "Unlink" affordance in v1. If a user loses access to their GitHub account, recovery is via staff (see [behaviors/account-migration.md](../behaviors/account-migration.md)) -- **Email** — read-only display of the current GitHub primary verified email - - Small note: "Sourced from GitHub on every sign-in. To change, update your primary email on GitHub and sign back in here." + - No "Unlink" affordance in v1. If a user loses access to their GitHub account, they can still sign in via password (per [behaviors/account-migration.md](../behaviors/account-migration.md)); if both are lost, recovery is via staff. +- **Email** — read-only display of the current primary email + - For GitHub-linked users: "Sourced from GitHub on every sign-in. To change, update your primary email on GitHub and sign back in here." + - For password-only users (no GitHub link): "Imported from your Code for Philly account at cutover." - Refresh timestamp ("Last updated when you signed in on {date}") - **Slack** — placeholder row with a greyed-out "Connect Slack" button (the linking flow isn't yet specified; the row signals direction) +**State B — GitHub not yet linked** (`me.hasGitHubLink === false`) + +The Identity card opens with a yellow **connect-GitHub banner** at the top: + +> **Connect a GitHub account to make sign-in easier.** GitHub sign-in is faster and works the same as your password. No deadline — this is just a recommendation. +> +> [**Connect GitHub** →] + +The "Connect GitHub" button posts to `POST /api/auth/link-github` (the SPA submits as a form-style navigation since the route immediately redirects to GitHub OAuth). After the link succeeds, the page reloads with State A. + +Below the banner, the rest of Card 1 still renders: + +- **Email** — same display as State A's password-only variant +- **Slack** — same placeholder + +The banner is the **only** nag mechanism in v1. No modal interrupts, no toast on every sign-in, no email reminders. Per [behaviors/account-migration.md](../behaviors/account-migration.md#the-nag-banner-on-account). + ### Card 2: Newsletter - Toggle: "Receive Code for Philly newsletters" @@ -72,6 +95,7 @@ That post-onboarding claim flow follows the same shape as the OAuth-callback cla | Action | API call | On success | | ------ | -------- | ---------- | +| Connect GitHub (when unlinked) | `POST /api/auth/link-github` (redirects to GitHub OAuth) | Callback returns to `/account`; banner disappears | | Toggle newsletter | `PATCH /api/people/<slug>/newsletter` | Re-fetch me, success toast | | Revoke session | `POST /api/auth/sessions/:jti/revoke` | Remove row from list | | Sign out | `POST /api/auth/logout` | Redirect to `/` | diff --git a/specs/screens/login.md b/specs/screens/login.md index bf97881..e984b16 100644 --- a/specs/screens/login.md +++ b/specs/screens/login.md @@ -6,11 +6,13 @@ `?return=/some/path` — optional URL to navigate to after successful login (must be a same-origin path, else ignored). -`?error=<code>` — optional. Set by the GitHub OAuth callback when it redirects back here after a failure. Renders an inline error message keyed off the code. +`?error=<code>` — optional. Set by the GitHub OAuth callback or `POST /api/auth/login` (via SPA error handling) when sign-in fails. Renders an inline error message keyed off the code. ## Data Requirements - `GET /api/auth/me` — to detect existing session and redirect away +- `POST /api/auth/login` — for the password sign-in form (per [api/auth.md](../api/auth.md)) +- `POST /api/auth/password-reset/request` — for the "Forgot your password?" affordance ## Display Rules @@ -20,7 +22,7 @@ A single centered card, ≤ 480px wide. - Title: "Sign in to Code for Philly" - Body: "We use **GitHub** for sign-in. If you don't have a GitHub account yet, it's free and takes about a minute." -- A small "Why GitHub?" expandable disclosure: "We chose GitHub as the sole identity provider for three reasons: (1) the civic-tech community already lives there, (2) it filters spam and scam accounts more effectively than email-only sign-ups, and (3) most of our project work coordinates on GitHub anyway. Anyone can [create a GitHub account](https://github.com/signup) in under a minute." +- A small "Why GitHub?" expandable disclosure: "We chose GitHub as the primary identity provider for three reasons: (1) the civic-tech community already lives there, (2) it filters spam and scam accounts more effectively than email-only sign-ups, and (3) most of our project work coordinates on GitHub anyway. Anyone can [create a GitHub account](https://github.com/signup) in under a minute." ### Primary CTA @@ -32,11 +34,26 @@ The button submits to `GET /api/auth/github/start?return=<encoded return>`. ### Returning members note -Below the button: "**Returning Code for Philly member?** You'll be prompted to connect your old account after you sign in with GitHub." +Below the button: "**Returning Code for Philly member?** If you had an account before our 2026 switch to GitHub sign-in, you can sign in with your old password below — or use GitHub if your old email matches." + +### Secondary: legacy password sign-in + +Below the GitHub CTA, a collapsed disclosure: + +- Summary: **"Or sign in with your Code for Philly password"** (small button-style link with a key icon) +- When expanded, shows a form: + - Label: "Username or email" — text input (`usernameOrEmail`) + - Label: "Password" — password input (`password`) + - Submit button: **"Sign in"** + - Below the submit: a small link "Forgot your password?" → opens the password-reset request flow (a small inline form: enter username or email, submit, see "If we have an account on file we'll send you a reset link.") + +The form submits to `POST /api/auth/login`. On 200 → navigate to `?return` or `/`. On 401 → render the inline error message (see below) keyed off `error.code`; don't reveal whether the username or the password was the failure. On 429 → "Too many sign-in attempts. Please wait a minute and try again." + +The disclosure starts **collapsed** so GitHub remains the visually-dominant path. Users who don't have a legacy account never expand it. ### Error display -When `?error=<code>` is present: +When `?error=<code>` is present (from the OAuth callback or the password-login response): | Code | Message | | ---- | ------- | @@ -45,12 +62,15 @@ When `?error=<code>` is present: | `oauth_session_invalid` | (same) | | `github_unreachable` | "We couldn't reach GitHub. Please try again in a moment." | | `email_unverified` | "Your GitHub account doesn't have a verified email address visible to us. To sign in here, please [verify a primary email on GitHub](https://github.com/settings/emails) and ensure email visibility is enabled for our app." | +| `invalid_credentials` | "The username or password you entered is incorrect." (rendered inline in the password form, not as a top-banner) | ## Actions | Action | Effect | | ------ | ------ | | Sign in with GitHub | Navigate to `GET /api/auth/github/start?return=<encoded return>` | +| Sign in with password | `POST /api/auth/login` with `{ usernameOrEmail, password }` | +| Forgot your password? | `POST /api/auth/password-reset/request` with `{ usernameOrEmail }`; show "If we have an account on file we'll send you a reset link." on success (regardless of whether the account actually exists — anti-enumeration) | ## Navigation @@ -59,8 +79,7 @@ When `?error=<code>` is present: **From here:** - GitHub's OAuth authorization page (via the API redirect) -- After successful sign-in: `?return` or `/` -- If the user has legacy candidates: `/account-claim` (transparent to the user — the callback redirects them there) +- After successful sign-in (either path): `?return` or `/` ## Authorization @@ -68,6 +87,6 @@ Public. Already-authenticated visitors are redirected away (the SPA checks `GET ## Coordinates with -- [api/auth.md](../api/auth.md) -- [screens/account-claim.md](account-claim.md) -- [behaviors/account-migration.md](../behaviors/account-migration.md) +- [api/auth.md](../api/auth.md) — both GitHub OAuth and `POST /api/auth/login` +- [behaviors/account-migration.md](../behaviors/account-migration.md) — the three-paths story +- [behaviors/password-hash-rotation.md](../behaviors/password-hash-rotation.md) — what happens server-side on password submit