amadan
Plan §13 step 2: sign-in, second factor, recovery codes
Seven tasks, TDD throughout, modelled on 2026-08-30-sheets-core.md. Every
signature in it was read at source in rastrillo v0.22.0, chi v5.3.2 or
keymaildev/signin v0.1.0 rather than recalled — one identifier is
flagged in the self-review as the exception.
Three things the plan cannot decide, stated up front rather than
discovered mid-execution:
The app has no secrets. carlos secrets list --account bds --app home
returns empty where sheets has SHEETS_DEK, SHEETS_INSTANCE_KEY and
CARLOS_SMTP_PASS. Task 1 cannot run without HOME_DEK and task 4 cannot
send a link without SMTP. Minting a DEK is a one-way door, so Paul
provisions it.
Passkeys need JavaScript and section 11 forbids it. A WebAuthn ceremony
is navigator.credentials by construction, and rastrillo ships no client
JS for it — the string appears only in its docs and webauthn/
browser_test.go at v0.22.0. Section 11 says "no surface here has any
reason to require script, and the day one does is the day this section
is wrong". Task 6 is blocked on that ruling and says so; everything else
is scriptless and unaffected. Recovery-code redemption is already a
plain form POST reading r.PostFormValue("code"), with
recoveryCodesPerSet = 10 — matching section 5.2 exactly — so redemption
could ship without passkeys if that is the ruling.
There is still no main on this remote.
Two spec corrections the plan needs and does not make silently. Section
5.1 says to use rastrillo/auth for address classification, which cannot
be done: auth's classification is reachable only through auth.Begin,
which writes auth_links.address TEXT NOT NULL — the thing ruling 14
exists to avoid. The usable seam is keymaildev/signin's Classifier, a
plain struct with exported fields and no stores, so the plan depends on
signin directly. And section 3.1 says the blind index uses an expensive
KDF rather than a bare HMAC while ruling 3 and section 6 both specify
HMAC(pepper, lowercased address); that must be settled before the pepper
is built, and is already filed as task 2.
The plan carries gates 1 and 2 rather than deferring them. Step 2 opens
the first database, which is what makes those two buildable, so they are
not optional extras at the end. Task 4 ends with a step that plants an
unsealed address and requires the gate to fail before reverting — a gate
never seen to fail is a gate written to pass, which is the finding that
came back twice on gate 9.
🤖 Generated with Claude Opus 5, at the direction of Paul Campbell.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfmrCe9McDH9dnqkuEMrjF
1 file changed,
+646
−0
docs/superpowers/plans/2026-09-03-home-signin.md+646 −0
diff --git a/docs/superpowers/plans/2026-09-03-home-signin.md b/docs/superpowers/plans/2026-09-03-home-signin.md| new file mode 100644 |
| index 0000000..73fb385 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-09-03-home-signin.md |
| @@ -0,0 +1,646 @@ |
| +# Oficina home — §13 step 2: sign-in, second factor, recovery codes |
| + |
| +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| + |
| +**Goal:** A person can sign in to the Oficina home with a magic link, optionally add a passkey as a second factor, and recover with a one-time code — with no readable email address anywhere at rest. |
| + |
| +**Architecture:** Hand-rolled sign-in on the credential origin. `rastrillo/auth` is used only for address classification and the Keymail upgrade, never for its link or session stores, because those hold readable addresses (ruling 14). The session Subject is an opaque person ref minted at first sign-in; link state is sealed before it is written. This is the first step that opens a database, so it is also the step that makes gates 1 and 2 buildable — and therefore mandatory. |
| + |
| +**Tech Stack:** Go 1.25.0, rastrillo v0.22.0, chi v5, modernc.org/sqlite (cgo-free), zero JavaScript. |
| + |
| +**Spec:** `docs/superpowers/specs/2026-08-30-home-design.md` — §5.1, §5.2, §10, §12, and §13 step 2. Read it alongside this plan; every task argues from it. |
| + |
| +## Global Constraints |
| + |
| +Every task's requirements implicitly include this section. Values are copied verbatim from the spec or AGENTS.md. |
| + |
| +- **No address column exists.** `people` holds person ref, blind index, created_at — "**No address column.**" (§10) |
| +- **The session Subject is an opaque person ref, never an address.** Sessions, credentials and recovery codes all key on it. (§5.1) |
| +- **Link state is sealed before it is written** and short-lived — "never cleaned up afterwards", because deleting a row does not erase bytes from a continuously replicated file. (§5.1, §3.2) |
| +- **`PRAGMA secure_delete` is on.** (§5.1) |
| +- **Migrations are additive-only.** (§10) |
| +- **The credential origin never renders an account-authored string, ever.** It is the RP. "a permanent constraint with its own test, not a naming choice." (§4) |
| +- **The RP ID is the full host** — `id.oficina.works` — "**Never widen the RP ID to a registrable suffix**", asserted by a test. (§4, ruling 16) |
| +- **Zero JS.** "Every screen works with scripts off. **No surface here has any reason to require script**, and the day one does is the day this section is wrong." (§11) |
| +- **Write `class="rst-*"`**, `day` theme, rastrillo `server` shape. (§11) |
| +- **Recovery is recovery codes**: "ten, shown once, redeemed as a plain form POST with no JS. Sign-in only — step-up still demands a real assertion." (§5.2, ruling 6) |
| +- **Passkey is optional per person.** Per-team enforcement is **not in slice one** (ruling 15) and no operator reset exists in slice one (§5.2) — do not build either. |
| +- **No address, team name, or queue payload in any log line, error string, job status message or metric label.** Enforced by gate 9, already in the tree. (AGENTS.md; §3.4) |
| +- **One error string** for a hashing failure — "*failed to hash*" (§5.1's surrounding text; check §7 before inventing a second). |
| +- **One binary, `CGO_ENABLED=0`, cgo-free SQLite.** (AGENTS.md) |
| +- **Every branch gets a discussion, and that discussion is the review record.** §Review wants a pass by a model that did not write the code. (AGENTS.md) |
| + |
| +### The gate escalation this step triggers |
| + |
| +§13 step 1 is "Skeleton, `AGENTS.md`, gates 1, 2 and 9". Gate 9 landed in `feat/gate-9-no-content-in-logs`. Gates 1 and 2 could not be built because the app had no database. **This step opens the first database, so this step owes gates 1 and 2** — they are not optional extras at the end: |
| + |
| +- **Gate 1, plaintext grep** — "raw SQLite file and WAL plus this app's store objects, against a seeded corpus of known addresses." (§12) |
| +- **Gate 2, no-address-at-rest** — "the schema has no address column, and a full sign-in plus directory upsert plus rekey leaves no readable address anywhere, WAL included." (§12) |
| + |
| +Sheets already has the helper this needs: `internal/sheets/sealtest` in `oficina/sheets-core`. Port it the way `internal/guardstest` was ported, rather than reinventing it. |
| + |
| +### Three things to settle before Task 5 — not the executor's to decide |
| + |
| +1. **`home` has no secrets.** `carlos secrets list --account bds --app home` returns empty; sheets has `SHEETS_DEK`, `SHEETS_INSTANCE_KEY`, `CARLOS_SMTP_PASS`. Task 1 cannot run until `HOME_DEK` exists; Task 4 cannot send a link without SMTP. Minting a DEK is a one-way door — lose it and every sealed link is unreadable — so **Paul provisions these, not the executor.** |
| +2. **Passkey needs JavaScript, and §11 forbids it.** §11: "Every screen works with scripts off. **No surface here has any reason to require script**, and the day one does is the day this section is wrong." A WebAuthn ceremony is `navigator.credentials.create/get` — script by construction — and **rastrillo ships no client JS for it** (`navigator.credentials` appears only in its docs and `webauthn/browser_test.go`, verified at v0.22.0). Task 6 is blocked on a ruling: §11 gains an explicit carve-out for the one optional enhancement, or passkeys leave slice one. Everything else here is scriptless and unaffected. |
| +3. **No `main` on this remote.** The cadence says branch from and squash-merge to `main`; `git ls-remote --heads origin` shows `design`, `skeleton`, `domain/oficina-works` and feature branches. Work off `skeleton`. |
| + |
| +### A spec-vs-reality note the executor must not "fix" silently |
| + |
| +§5.1 says "We still use `rastrillo/auth` for **address classification and Keymail upgrade** — the part that is hard to get right and holds nothing." **`auth` cannot be used that way.** Its classification is reachable only through `auth.Begin`, which writes `auth_links.address TEXT NOT NULL` — verified at source, and the very thing ruling 14 exists to avoid. |
| + |
| +The usable seam is one level down: `github.com/keymaildev/signin` exports `Classifier` as a plain struct with exported fields (`HTTP`, `LookupTXT`, `Scheme`, `Now`, `TTL`, `NegTTL`) and a `Classify(ctx, address) Class` method. No constructor, no database, no stores. **Depend on `signin` directly** (rastrillo already requires v0.1.1). This plan does that; the spec's sentence should be corrected to match — filed, not silently diverged from. |
| + |
| +--- |
| + |
| +## File Structure |
| + |
| +``` |
| +cmd/home/main.go Resolve -> db.Open -> migrate.Apply -> Serve. |
| +internal/home/ |
| + app.go App struct and wiring. Gains db, sealer, sessions, classifier. |
| + keys.go LoadSealer: HOME_DEK from env, base64, exactly 32 bytes, refuse to boot otherwise. |
| + seal/seal.go DEK, HKDF purpose keys, AES-GCM. Ported from sheets-core. |
| + sealtest/sealtest.go Gate 1: AssertNoPlaintext over raw file, WAL and SHM. Ported. |
| + models.go Schema. people, signin_links. No address column, anywhere. |
| + migrations/0001_people.sql |
| + migrations/0002_signin_links.sql |
| + personref.go Mint and validate the opaque person ref. |
| + signin.go Begin/Verify: sealed link state, one-time redemption, session mint. |
| + classify.go signin.Classifier wrapper. Fails open. |
| + render.go One parsed tree per page, ui.Funcs plus an asset func. |
| + templates/ Scriptless screens. class="rst-*". |
| + logtest/ Gate 9. Already on feat/gate-9-no-content-in-logs. |
| +internal/guardstest/ |
| + logsinks_test.go Already landed. Forbids non-slog sinks. |
| + noaddress_test.go Gate 2's schema half. |
| + rpid_test.go Rulings 13/16: the RP ID is the full host. |
| +``` |
| + |
| +--- |
| + |
| +### Task 1: The seal package, the DEK, and gate 1 |
| + |
| +The gate goes in with the mechanism, as it did in Sheets. Once this lands, every later task is checked by it. |
| + |
| +**Files:** |
| +- Create: `internal/home/seal/seal.go`, `internal/home/seal/seal_test.go` |
| +- Create: `internal/home/sealtest/sealtest.go` |
| +- Create: `internal/home/keys.go`, `internal/home/keys_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: nothing. |
| +- Produces: |
| + - `seal.New(dek []byte) (*Sealer, error)` — DEK must be exactly 32 bytes. |
| + - `(*Sealer).Seal(purpose string, plaintext []byte) ([]byte, error)` |
| + - `(*Sealer).Open(purpose string, ciphertext []byte) ([]byte, error)` |
| + - `home.LoadSealer() (*seal.Sealer, error)` — reads `HOME_DEK`. |
| + - `sealtest.Canaries [][]byte`, `sealtest.AssertNoPlaintext(t *testing.T, dbPath string)` |
| + - Purposes: `"signin.address"`, `"team.name"`, `"queue.payload"`. |
| + |
| +Port `seal.go` from `oficina/sheets-core/internal/sheets/seal/seal.go` **unchanged except the HKDF info string**, which becomes `"oficina/home:"+purpose`. Do not redesign it: two apps disagreeing about their envelope format is a bug waiting to differ. Likewise `sealtest.go`, keeping its `leaked`/`findLeak` split — a failing subtest always fails its parent, so the scan must be unit-testable without a live `*testing.T`. |
| + |
| +- [ ] **Step 1: Write the failing round-trip test** |
| + |
| +```go |
| +package seal |
| + |
| +import ( |
| + "bytes" |
| + "testing" |
| +) |
| + |
| +func testDEK() []byte { |
| + d := make([]byte, 32) |
| + for i := range d { |
| + d[i] = byte(i) |
| + } |
| + return d |
| +} |
| + |
| +func TestSealOpenRoundTrip(t *testing.T) { |
| + s, err := New(testDEK()) |
| + if err != nil { |
| + t.Fatalf("New: %v", err) |
| + } |
| + ct, err := s.Seal("signin.address", []byte("person@example.invalid")) |
| + if err != nil { |
| + t.Fatalf("Seal: %v", err) |
| + } |
| + if bytes.Contains(ct, []byte("example.invalid")) { |
| + t.Fatal("ciphertext contains the plaintext") |
| + } |
| + pt, err := s.Open("signin.address", ct) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + if string(pt) != "person@example.invalid" { |
| + t.Fatalf("Open = %q", pt) |
| + } |
| +} |
| + |
| +// A value sealed under one purpose must not open under another. These are |
| +// adjacent columns of the same row; a bug that swapped them would be |
| +// invisible without this. |
| +func TestWrongPurposeFails(t *testing.T) { |
| + s, _ := New(testDEK()) |
| + ct, _ := s.Seal("signin.address", []byte("secret")) |
| + if _, err := s.Open("team.name", ct); err == nil { |
| + t.Fatal("opened under the wrong purpose") |
| + } |
| +} |
| + |
| +func TestRejectsShortDEK(t *testing.T) { |
| + if _, err := New(make([]byte, 31)); err == nil { |
| + t.Fatal("accepted a 31-byte DEK") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/home/seal/` |
| +Expected: FAIL, build error — `undefined: New`. |
| + |
| +- [ ] **Step 3: Port seal.go** |
| + |
| +Copy `oficina/sheets-core/internal/sheets/seal/seal.go` to `internal/home/seal/seal.go`. Change only the package doc and the HKDF info string: |
| + |
| +```go |
| +r := hkdf.New(sha256.New, s.dek, nil, []byte("oficina/home:"+purpose)) |
| +``` |
| + |
| +Add `golang.org/x/crypto` with `go get golang.org/x/crypto`. |
| + |
| +- [ ] **Step 4: Run the tests** |
| + |
| +Run: `go test ./internal/home/seal/` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Write LoadSealer and its test** |
| + |
| +```go |
| +// LoadSealer reads HOME_DEK and returns the process Sealer. |
| +// |
| +// It refuses to boot rather than generating a key. A generated key would |
| +// let the app start with every previously sealed link unreadable, which |
| +// looks like data loss and is discovered late. |
| +func LoadSealer() (*seal.Sealer, error) { |
| + raw := os.Getenv("HOME_DEK") |
| + if raw == "" { |
| + return nil, errors.New("HOME_DEK is not set") |
| + } |
| + dek, err := base64.StdEncoding.DecodeString(raw) |
| + if err != nil { |
| + return nil, fmt.Errorf("HOME_DEK is not valid base64: %w", err) |
| + } |
| + return seal.New(dek) |
| +} |
| +``` |
| + |
| +Test that unset, malformed and 31-byte values each error, and a valid one returns a Sealer. Use `t.Setenv`. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git add internal/home/seal internal/home/sealtest internal/home/keys.go internal/home/keys_test.go go.mod go.sum |
| +git commit -m "Seal values at rest, and the plaintext gate that proves it" |
| +``` |
| + |
| +--- |
| + |
| +### Task 2: The database, the schema, and gate 2's schema half |
| + |
| +**Files:** |
| +- Modify: `cmd/home/main.go` |
| +- Create: `internal/home/models.go`, `internal/home/migrations/0001_people.sql` |
| +- Create: `internal/guardstest/noaddress_test.go` |
| +- Modify: `internal/guardstest/repo_test.go` (add `migrationFiles`, `readFile`) |
| + |
| +**Interfaces:** |
| +- Produces: `home.Schema *migrate.Set`, `home.BootSchema *migrate.Set`, and an `App` holding `*db.DB`. |
| + |
| +Follow sheets-core's boot shape exactly — `Resolve`, open the DB yourself, `migrate.Apply`, then blank `DBPath` before `Serve`, because `Serve` would otherwise open a second handle: |
| + |
| +```go |
| +opts, err := rastrillo.Resolve(rastrillo.Options{Logger: logger}) |
| +d, err := rdb.Open(opts.DBPath, logger) |
| +_, err = migrate.Apply(context.Background(), d, home.BootSchema) |
| +opts.Mux = mux |
| +opts.DBPath = "" |
| +rastrillo.Serve(opts) |
| +``` |
| + |
| +```go |
| +//go:embed migrations/*.sql |
| +var migrationsFS embed.FS |
| + |
| +var Schema = migrate.MustFromFS(migrationsFS, "home") |
| +var BootSchema = migrate.Merge(sessions.Schema, Schema) |
| +``` |
| + |
| +`auth.Schema` is **deliberately absent**: this app does not use `rastrillo/auth`, and its address-bearing tables must never be created. Migration files are `migrations/NNNN_lowercase_name.sql`, forward-only, ledgered by ID and checksum. |
| + |
| +- [ ] **Step 1: Write gate 2's schema half** |
| + |
| +```go |
| +package guards |
| + |
| +import ( |
| + "regexp" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// addressish matches a column name that would hold an email address. |
| +// §10: "people — person ref, blind index, created_at. No address column." |
| +var addressish = regexp.MustCompile(`(?i)\b(address|email|e_mail|mail_to|recipient)\b`) |
| + |
| +// TestNoAddressColumn reads every migration and fails on a column whose |
| +// name says it holds an address. This is the cheap half of gate 2; the |
| +// expensive half — a full sign-in leaves no readable address anywhere, |
| +// WAL included — is Task 4 and needs a running sign-in. |
| +func TestNoAddressColumn(t *testing.T) { |
| + root := repoRoot(t) |
| + for _, path := range migrationFiles(t, root) { |
| + rel := relPath(t, root, path) |
| + for i, line := range strings.Split(readFile(t, path), "\n") { |
| + // A sealed blob is not an address, and says so in its name. |
| + if strings.Contains(strings.ToLower(line), "sealed") { |
| + continue |
| + } |
| + if addressish.MatchString(line) { |
| + t.Errorf("%s:%d: %q names an address column; §10 says there is none", |
| + rel, i+1, strings.TrimSpace(line)) |
| + } |
| + } |
| + } |
| +} |
| +``` |
| + |
| +Add `migrationFiles` and `readFile` beside the existing helpers in `repo_test.go`, following their shape (`t.Helper()`, `t.Fatalf` on error). |
| + |
| +- [ ] **Step 2: Run it** |
| + |
| +Run: `go test ./internal/guardstest/ -run TestNoAddressColumn` |
| +Expected: PASS trivially — no migrations yet. That is correct; it is a tripwire, like gate 9. |
| + |
| +- [ ] **Step 3: Write the first migration** |
| + |
| +`internal/home/migrations/0001_people.sql`: |
| + |
| +```sql |
| +CREATE TABLE people ( |
| + person_ref TEXT PRIMARY KEY, |
| + blind_index TEXT NOT NULL UNIQUE, |
| + created_at TEXT NOT NULL |
| +); |
| +CREATE INDEX idx_people_blind_index ON people(blind_index); |
| +``` |
| + |
| +No address column, no email column. The blind index is `HMAC(pepper, lowercased address)` per ruling 3 — **but §3.1 says the index "uses an expensive KDF rather than a bare HMAC", contradicting ruling 3 and §6.** Filed as task 2 on `skeleton`; it **must be settled before the pepper is implemented in step 4**. This task creates the column; nothing writes it yet. |
| + |
| +- [ ] **Step 4: Verify** |
| + |
| +Run: `go test ./...` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Turn on secure_delete** |
| + |
| +`PRAGMA secure_delete` must be on, because "deleting a row does not remove bytes from a file that is continuously replicated" (§5.1). **Verify whether `rastrillo/db.Open` already sets it**; if not, set it immediately after open. Either way add a test that reads `PRAGMA secure_delete` back and asserts it is 1. Do not assume — this is the riskiest assumption in the plan. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git add internal/home/models.go internal/home/migrations cmd/home/main.go internal/guardstest |
| +git commit -m "Open the database, and the schema that has no address column" |
| +``` |
| + |
| +--- |
| + |
| +### Task 3: The person ref |
| + |
| +**Files:** |
| +- Create: `internal/home/personref.go`, `internal/home/personref_test.go` |
| + |
| +**Interfaces:** |
| +- Produces: `home.NewPersonRef() (string, error)`, `home.ValidPersonRef(s string) bool` |
| + |
| +The person ref is the subject of every session, credential and recovery code, and later the assertion's `sub` and the directory's join key. Opaque, stable across an email change, and **never derived from the address** — a derived ref puts the address back at rest in a reversible form under another name. |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +```go |
| +func TestPersonRefIsOpaqueAndUnique(t *testing.T) { |
| + seen := map[string]bool{} |
| + for i := 0; i < 1000; i++ { |
| + ref, err := NewPersonRef() |
| + if err != nil { |
| + t.Fatalf("NewPersonRef: %v", err) |
| + } |
| + if seen[ref] { |
| + t.Fatalf("duplicate person ref %q after %d draws", ref, i) |
| + } |
| + seen[ref] = true |
| + if !ValidPersonRef(ref) { |
| + t.Fatalf("NewPersonRef produced %q, which ValidPersonRef rejects", ref) |
| + } |
| + } |
| +} |
| + |
| +func TestPersonRefDoesNotEmbedInput(t *testing.T) { |
| + ref, _ := NewPersonRef() |
| + if strings.Contains(ref, "@") { |
| + t.Fatalf("person ref %q contains an @", ref) |
| + } |
| +} |
| + |
| +func TestValidPersonRefRejectsRubbish(t *testing.T) { |
| + for _, bad := range []string{"", "person@example.invalid", "../etc", strings.Repeat("a", 500)} { |
| + if ValidPersonRef(bad) { |
| + t.Errorf("ValidPersonRef(%q) = true", bad) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run, watch it fail** |
| + |
| +Run: `go test ./internal/home/ -run PersonRef` |
| +Expected: FAIL — `undefined: NewPersonRef`. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +```go |
| +// personRefBytes is 16 bytes of crypto/rand, base32 without padding. |
| +// Long enough that collisions are not a concern at any plausible number |
| +// of people; short enough to read in a log line, which it is allowed to |
| +// appear in — §3.4: "Errors carry person refs and codes." |
| +const personRefBytes = 16 |
| + |
| +func NewPersonRef() (string, error) { |
| + b := make([]byte, personRefBytes) |
| + if _, err := rand.Read(b); err != nil { |
| + return "", err |
| + } |
| + enc := base32.StdEncoding.WithPadding(base32.NoPadding) |
| + return "pr_" + strings.ToLower(enc.EncodeToString(b)), nil |
| +} |
| +``` |
| + |
| +`ValidPersonRef` checks the `pr_` prefix, the exact expected length, and the alphabet. |
| + |
| +- [ ] **Step 4: Run** |
| + |
| +Run: `go test ./internal/home/ -run PersonRef` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/home/personref.go internal/home/personref_test.go |
| +git commit -m "The person ref: opaque, minted, never derived from an address" |
| +``` |
| + |
| +--- |
| + |
| +### Task 4: Magic-link sign-in, and gate 2 in full |
| + |
| +The task the whole step exists for, and where the blindness claim is either kept or quietly broken. |
| + |
| +**Files:** |
| +- Create: `internal/home/signin.go`, `internal/home/signin_test.go` |
| +- Create: `internal/home/migrations/0002_signin_links.sql` |
| +- Create: `internal/home/templates/signin.html`, `internal/home/templates/check_your_mail.html` |
| +- Modify: `internal/home/app.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `seal.Sealer`, `NewPersonRef`, `sessions.Sessions`, `mail.Sender`. |
| +- Produces: `(*App).handleSigninBegin`, `(*App).handleSigninVerify`, routed at `POST /signin` and `GET /signin/verify`. |
| + |
| +`0002_signin_links.sql`: |
| + |
| +```sql |
| +CREATE TABLE signin_links ( |
| + token_hash TEXT PRIMARY KEY, |
| + sealed_address BLOB NOT NULL, |
| + expires_at TEXT NOT NULL, |
| + redeemed_at TEXT |
| +); |
| +CREATE INDEX idx_signin_links_expires_at ON signin_links(expires_at); |
| +``` |
| + |
| +The column holds `sealer.Seal("signin.address", []byte(addr))`. **Seal before the insert, never "insert then clean up"** — Litestream ships WAL frames continuously, so a plaintext write reaches the bucket before any delete could remove it (§5.1, §3.2). |
| + |
| +Use `rastrillo/sessions` directly, **never `rastrillo/auth`**: `sessions.Session.Subject` is an opaque string the app chooses, so it takes the person ref. `auth` sets `Subject: id.Address` and stores `auth_links.address TEXT NOT NULL` — both verified at source, and precisely what ruling 14 forbids. |
| + |
| +- [ ] **Step 1: Write gate 2 in full, as a failing test** |
| + |
| +```go |
| +// TestFullSigninLeavesNoReadableAddress is §12 gate 2: "a full sign-in |
| +// plus directory upsert plus rekey leaves no readable address anywhere, |
| +// WAL included." The directory and rekey halves arrive in step 4; this |
| +// asserts the sign-in half and must be EXTENDED, not replaced, then. |
| +func TestFullSigninLeavesNoReadableAddress(t *testing.T) { |
| + dbPath := filepath.Join(t.TempDir(), "home.db") |
| + app := newTestApp(t, dbPath) |
| + |
| + addr := string(sealtest.Canaries[0]) + "@example.invalid" |
| + |
| + form := url.Values{"address": {addr}} |
| + req := httptest.NewRequest(http.MethodPost, "/signin", strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, req) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("POST /signin = %d, want 303", rec.Code) |
| + } |
| + |
| + // Redeem the link the mailer captured, so a session row exists too. |
| + app.Router().ServeHTTP(httptest.NewRecorder(), |
| + httptest.NewRequest(http.MethodGet, capturedLink(t, app), nil)) |
| + |
| + sealtest.AssertNoPlaintext(t, dbPath) |
| +} |
| +``` |
| + |
| +`newTestApp` wires a `mail.Sender` that captures rather than sends (`mail.Sender` is a one-method interface: `Send(ctx, to, subject, body) error`). |
| + |
| +- [ ] **Step 2: Run it** |
| + |
| +Run: `go test ./internal/home/ -run TestFullSignin` |
| +Expected: FAIL — no `/signin` route. |
| + |
| +- [ ] **Step 3: Implement Begin** |
| + |
| +`sessions.NewToken()` returns `(token, hash, err)`; mail the raw token, store the hash. Seal the address, insert, send, then **303** to a "check your mail" page. **Always 303, always the same page**, whether or not the address is known — a different response for a known address is an account-enumeration oracle. |
| + |
| +- [ ] **Step 4: Implement Verify** |
| + |
| +Look up by `sessions.HashToken(r.URL.Query().Get("t"))`. Reject if missing, expired, or `redeemed_at` is non-null — single use. Open the sealed address, classify it (Task 5), then mint or find the person row. **The blind index needs step 4's pepper, which is not settled** (see Task 2 step 3); until it is, mint a person row keyed on the ref alone and **file a task rather than leaving a TODO in the code**. Mint the session with `Subject: personRef`, and mark the link redeemed **in the same transaction** as the session mint. |
| + |
| +- [ ] **Step 5: Run the gate** |
| + |
| +Run: `go test ./internal/home/ -run TestFullSignin` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 6: Prove the gate bites** |
| + |
| +Temporarily change the insert to store the address unsealed. Run the test; it must FAIL naming the canary. Revert. **Do not skip this.** A gate never seen to fail is a gate written to pass — the finding that came back twice on gate 9. |
| + |
| +- [ ] **Step 7: Commit** |
| + |
| +```bash |
| +git add internal/home/signin.go internal/home/signin_test.go internal/home/migrations/0002_signin_links.sql internal/home/templates internal/home/app.go |
| +git commit -m "Magic-link sign-in, with the address sealed before it is written" |
| +``` |
| + |
| +--- |
| + |
| +### Task 5: Address classification and the Keymail upgrade |
| + |
| +**Files:** |
| +- Create: `internal/home/classify.go`, `internal/home/classify_test.go` |
| +- Modify: `go.mod` — add `github.com/keymaildev/signin` |
| + |
| +**Interfaces:** |
| +- Produces: `(*App).classify(ctx context.Context, address string) signin.Class` |
| + |
| +Use `signin.Classifier` directly — a plain struct, no constructor, no database. See the spec-vs-reality note above: `rastrillo/auth` cannot serve this purpose without its address-bearing tables. |
| + |
| +**Classification fails open** (§5.1): "classification fails open, so every address has a working path — which matters more here than in a normal app, because guests arrive from outside the suite entirely." A classifier error, timeout or DNS failure must yield the magic-link path, never an error page. |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +```go |
| +func TestClassifyFailsOpen(t *testing.T) { |
| + for _, tc := range []struct { |
| + name string |
| + lookup func(ctx context.Context, name string) ([]string, error) |
| + }{ |
| + {"dns error", func(context.Context, string) ([]string, error) { return nil, errors.New("servfail") }}, |
| + {"empty", func(context.Context, string) ([]string, error) { return nil, nil }}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + app := newTestApp(t, filepath.Join(t.TempDir(), "home.db")) |
| + app.classifier.LookupTXT = tc.lookup |
| + got := app.classify(context.Background(), "someone@example.invalid") |
| + if got == signin.ClassKeymail { |
| + t.Fatalf("classify = %v, want the magic-link path on failure", got) |
| + } |
| + }) |
| + } |
| +} |
| +``` |
| + |
| +**Check `signin`'s actual `Class` constant names before writing this** — the package is at `/home/paulca/go/pkg/mod/github.com/keymaildev/signin@v0.1.0/classify.go`. Do not invent them; `ClassKeymail` above is a placeholder for whatever it actually declares, and is the one identifier in this plan not verified at source. |
| + |
| +- [ ] **Step 2: Run, fail, implement, run** |
| + |
| +Wrap `Classifier.Classify` with a bounded `context.WithTimeout` and return the magic-link class on anything unexpected. |
| + |
| +- [ ] **Step 3: Commit** |
| + |
| +```bash |
| +git add internal/home/classify.go internal/home/classify_test.go go.mod go.sum |
| +git commit -m "Classify addresses, and fail open so every address has a path" |
| +``` |
| + |
| +--- |
| + |
| +### Task 6: Recovery codes — BLOCKED |
| + |
| +**Blocked on the §11 ruling** in "Three things to settle". `passkey.RegenerateRecoveryCodes` lives in the same package as the WebAuthn ceremony, and enrolling a passkey needs client JavaScript rastrillo does not ship. |
| + |
| +Verified at v0.22.0 so the executor need not re-derive it: |
| +- `recoveryCodesPerSet = 10` — matches §5.2's "ten" exactly. |
| +- Redemption is `if r.Method != http.MethodPost` then `r.PostFormValue("code")` — **a plain form POST with no JS**, matching §5.2 exactly. |
| +- Codes are base32, single-use, hashed with `sessions.HashToken`. |
| + |
| +So **redemption is scriptless and could ship alone**; only *issuing* codes alongside a passkey drags JS in. If passkeys leave slice one, this task becomes "recovery codes as a second sign-in path" and stays scriptless. If §11 gains a carve-out, this task also owns writing the WebAuthn JS the framework does not provide. |
| + |
| +**Do not start until that ruling exists.** |
| + |
| +--- |
| + |
| +### Task 7: The scriptless screens, and the RP-ID guard |
| + |
| +**Files:** |
| +- Create: `internal/home/render.go`, `internal/home/templates/*.html` |
| +- Create: `internal/guardstest/rpid_test.go` |
| +- Create: `static/tokens.css`, `static/theme.css` — copied from rastrillo, then frozen |
| + |
| +**Interfaces:** |
| +- Produces: `(*App).render(w http.ResponseWriter, name string, data any)` |
| + |
| +Follow sheets-core's `internal/sheets/render.go`: one parsed tree per page, `ui.Funcs()` plus an `asset` func, `ParseFS(ui.Templates(), "*.html")` then the app's own layout and page. |
| + |
| +**Write `class="rst-*"`** — and for the right reason. tokens.css at v0.22.0 ships **both** spellings, paired: 130 distinct bare `rst-*` attributes alongside their class twins. Its own header says "TWO SPELLINGS, ON PURPOSE, FOR ONE RELEASE … The attribute spelling is the vocabulary this library is moving to; the class spelling is what every app and every shipped partial writes today." Write classes because that is what the partials you render alongside write — not because the attribute grammar is unreleased. It is released. |
| + |
| +Copy `static/tokens.css` and `static/theme.css` (the `day` theme) deliberately, and **re-copy on every rastrillo upgrade** until `rastrillo doctor` lands. |
| + |
| +- [ ] **Step 1: The RP-ID guard** |
| + |
| +```go |
| +// Rulings 13 and 16: "The RP ID is the full host, asserted by a test." |
| +// passkey.Config has NO RPID field — RPID is derived internally as |
| +// url.Parse(Origin).Hostname() — so the thing to assert is the Origin |
| +// this app configures. |
| +func TestRPIDIsTheFullHost(t *testing.T) { |
| + for _, origin := range configuredOrigins(t) { |
| + u, err := url.Parse(origin) |
| + if err != nil { |
| + t.Fatalf("parse %q: %v", origin, err) |
| + } |
| + host := u.Hostname() |
| + if strings.Count(host, ".") < 2 { |
| + t.Errorf("origin %q derives RP ID %q, a registrable suffix: an RP ID "+ |
| + "valid at every host in the zone collapses the credential/directory "+ |
| + "origin split (ruling 2)", origin, host) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Every screen works with scripts off** |
| + |
| +§11 is enforceable mechanically and should be. |
| + |
| +```go |
| +func TestNoScriptTagInAnyTemplate(t *testing.T) { |
| + for _, name := range templateNames(t) { |
| + var buf bytes.Buffer |
| + renderTo(t, &buf, name, sampleData(name)) |
| + if bytes.Contains(bytes.ToLower(buf.Bytes()), []byte("<script")) { |
| + t.Errorf("%s renders a <script> tag; §11 says every screen works with scripts off", name) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 3: Commit** |
| + |
| +```bash |
| +git add internal/home/render.go internal/home/templates static internal/guardstest/rpid_test.go |
| +git commit -m "Scriptless sign-in screens, and the guard that the RP ID is a full host" |
| +``` |
| + |
| +--- |
| + |
| +## Deferred, with triggers |
| + |
| +| Thing | Trigger | |
| +|---|---| |
| +| Passkey enrollment and step-up | Paul rules on §11's zero-JS absolute | |
| +| The pepper and the blind index | §3.1's "expensive KDF" vs ruling 3's `HMAC` is settled — task 2 on `skeleton` | |
| +| Assertion minting, `jti`, the redirect protocol | §13 step 3 | |
| +| The directory, calls 2 and 3 | §13 step 4 | |
| +| Operator reset | §5.2: "the first real person locked out with passkey and codes both lost" | |
| + |
| +## Self-review notes |
| + |
| +- **Spec coverage:** §5.1 is Tasks 1–5; §5.2 is Task 6, blocked and said so; §10's `people`, `signin_links` and `sessions` are Tasks 2–4; §12 gates 1 and 2 are Tasks 1 and 4; §11 is Task 7. §5.3–§5.6 are step 3, out of scope and listed above. |
| +- **Deliberately not covered:** `prefs`, `directory`, `teams`, `provisioning_queue`, `rekey_journal`, `consumers`, `assertion_jti` — later steps. |
| +- **Two spec corrections needed, neither made silently:** §5.1's "use `rastrillo/auth` for classification" is not achievable; §3.1 contradicts ruling 3 on HMAC-vs-KDF. |
| +- **The one unverified identifier:** `signin.ClassKeymail` in Task 5. Every other signature in this plan was read at source. |
| +- **The riskiest assumption:** that `rastrillo/db.Open` either sets `PRAGMA secure_delete` or lets the app set it. Task 2 step 5 verifies rather than assumes; if neither holds, that is a blocker and an upstream issue. |