[mforce]

This PR adds the decision record for two-factor authentication (epic #1273). TOTP becomes both a login second factor and a step-up factor, and every Owner must enrol after a grace period. The PR has no code. Slices #1276 to #1281 build from this record, and each one adds its rules to src/AGENTS.md when it lands.

Closes #1274

Where to start

The PR changes two files:

• docs/decisions/1274-two-factor-authentication.md holds the whole design.
• docs/decisions/README.md gains one index row.

Read the record from "The rule" through "The shape" first. Its sections then give the decisions, the existing tests the design touches, the guards each slice adds, and the glossary terms. The branch has 39 commits: the initial draft and index, review fixes, recorded decisions, and one merge of main. Read the final file, not each commit, because the PR merges as one squash commit.

The design in short

• Login has two steps for an enrolled user. A correct password mints nothing. It answers 401 Auth.SecondFactorRequired with a signed 5-minute challenge, and only POST /api/v1/auth/login/second-factor mints the session. Login for an unenrolled user is byte-identical to today.
• One class owns all two-factor state. TwoFactorCredentials is the only code that checks an authenticator code or a recovery code, at login, at step-up and at change-password. It locks the user row, so its checks are exact under concurrency.
• Replay protection lives on the user row in Postgres, not in Redis. A per-user watermark stops a TOTP step from being used twice, and recovery codes are single-use rows.
• Limits. A wrong code is charged to the same five-attempt lockout as a wrong password. For an enrolled user, no password check resets that counter.
• Storage. The authenticator key is encrypted with the Data Protection has no persisted key ring, so Identity's token providers break across restarts and replicas #794 Data Protection ring. Recovery codes are 16 characters (about 75 bits), and each is stored as one row holding a SHA-256 digest of the user id and the code.
• Recovery paths. A user can turn two-factor off, an Owner can reset another user's, and recover-admin clears it for an Owner. Each path that clears a factor ends the user's other sessions and connected apps.
• The Owner rule. After a per-person grace period, a gate refuses an unenrolled Owner's tokens, connected-app tokens included, until the Owner enrols. Sign-in itself is never gated.

My decisions

These are the answers I gave, quoted as I gave them.

On 2026-10-20:

1. Step-up for an enrolled user asks for the password and a code: "Password + code (Recommended)". An unlocked phone holds both the session and the authenticator app, so the code alone would let whoever holds the phone create an Owner or approve a connected app.
2. Previous-certificate support for the Data Protection ring (Data Protection: keep reading keys protected by the previous certificate after a rotation #1338) does not need to ship before 2FA slice 8: every Owner must use two-factor authentication #1281: "No, don't wait (Recommended)". Review round 12 later found that, after a certificate change, a signed-out user needs two unused recovery codes to recover alone, and the record says so.
3. recover-admin forces a password change: "Force a change (Recommended)". Today it clears the flag, so the temporary password it prints is never forced to change.
4. The limits stay as written: "Keep as written (Recommended)". A user gets 20 wrong authenticator codes a day, which keeps a password holder's yearly chance of guessing the factor near 3.6%, against about 58% with lockout alone.
5. The login challenge is single-use: "Single use challenge".

On 2026-10-21:

6. 2FA slice 8: every Owner must use two-factor authentication #1281 has no hard cutover: "no hard cutover". I then asked for "a buffer of x days (14 days in this case)" in place of a date, and approved a per-person grace counted from each Owner's first sign-in after 2FA slice 8: every Owner must use two-factor authentication #1281 ships, or from their promotion: "Yes, per-person buffer (Recommended)". If the write that starts an Owner's grace fails, the request still goes through and the app logs an alert: "Log an alert event (Recommended)".
7. Each farm's entry in the authenticator app is named "Cluckwork (Farm name) (Recommended)", and a name that has to be cut adds the farm code: "Add farm code if cut (Recommended)". Two farms still show the same label when their names match after the : replacement and trimming and are short enough not to be cut.
8. Enrolment still signs out other sessions and connected apps, with a warning first: "Keep, warn first (Recommended)".
9. Each privileged action needs its own code: "One code per action (Recommended)".
10. A two-factor change reaches only the audit log for now: "Audit only, issue later (Recommended)" (Two-factor: tell a user at sign-in when someone else changed or bypassed their two-factor #1337).
11. The Account page warns when few recovery codes are left: "Warn at 2 or fewer (Recommended)".
12. The code step's per-address limit becomes configurable with a higher default: "add an app config and set default to 20?". The record sets RateLimiting:SecondFactor:PermitLimit to 20 per 15 minutes.
13. An enrolled user's session needs a full sign-in every 30 days: "Yes, 30 days, configurable (Recommended)".
14. The wrong-code message gets a clock hint: "Add the hint (Recommended)".
15. Connected apps stay outside that 30-day limit: "Leave connected apps (Recommended)", with a configurable forced reconnect filed as Connected apps: configurable approval lifetime that forces reconnect #1335. Two-factor for other roles is Two-factor: let an Owner require it for other roles #1336.
16. An enrolled user who changes their own password also gives a code: "Ask for a code (Recommended)". A wrong current password charges the lockout and a locked user is refused, as at step-up: "Same as step-up (Recommended)". A failed commit keeps today's behaviour, so the rare case stays ambiguous for the user, as it is today: "Keep today's 500 (Recommended)".
17. The slice order below: "Ok,".

Choices the agents made, which I did not decide

The coordinator agent and the worker agent made these choices while carrying out the review rounds. Each line names its source.

• The SPA reads a two_factor token claim (the coordinator, round 3). The gate later moved to the credential check (the coordinator, round 14).
• The slice order (proposed by the coordinator, round 4). I approved it later with "Ok,".
• The single-use rule uses a clock-free count of completed sign-ins (the coordinator asked for a clock-free rule and the worker chose the count, round 9). Its cost is that completing one challenge cancels the user's other outstanding challenges.
• The grace starts on the Owner's first authenticated request, through the credential check that already runs on every request. It also starts for an enrolled Owner, and a role change or a reset never restarts it (the coordinator, round 14; "enrolled or not" was the worker's choice).
• The clock hint shows only when the user typed a 6-digit code (the worker, round 14). Seeded test users stay unenrolled by default (the coordinator, round 14).
• The label uses the same issuer in the parameter and the prefix (the coordinator, carrying out my label answer). When an email is cut, the label adds the full user id, and the account part is capped at 174 escaped bytes (the coordinator, round 18).
• In round 14, the coordinator chose to copy the acting session's sign-in time through password change and turn-off. My later requirement for a code replaced that design.
• A missing user at change-password answers Users.CurrentPasswordIncorrect (the worker, round 17).
• From round 17 to round 23, change-password's commit failure had a durability probe and two new 503 codes (the coordinator and the worker). I replaced that design with "Keep today's 500 (Recommended)".
• The coordinator rated GLM's round 27 warning as a nit.

Slice order

The record changes the epic's original order to #1276, #1278, #1279, #1280, #1277, #1281. #1277, the slice that lets users enrol, now comes after login, step-up enforcement and every recovery path. No deployable build on main then lets a user enrol while step-up enforcement or a way back is missing. #1278 builds the enrolment methods without routes, so its tests enrol users through the one real writer. The coordinator proposed this order, and I approved it on 2026-10-21.

Every two-factor column ships in #1276, so #1278 to #1281 ship no database migration. Rolling one of those slices back is an image rollback. Rolling back past #1276 also needs its migration reversed.

Existing tests

The record's "Existing tests the design touches" section lists each test group by grep.

• Kept unchanged. The refresh, MustChangePassword gate, lockout, rate-limit, security-event, bootstrap-admin, provision-account and operator-verb tests.
• Kept, then extended. The login contract tests (2FA slice 5: TOTP code as a second step at login #1278 extends the not-cacheable pin and the SPA tests), the step-up tests (2FA slice 6: TOTP code as a step-up factor, including connected-app consent #1279), the change-password tests (2FA slice 6: TOTP code as a step-up factor, including connected-app consent #1279 adds enrolled cases) and the OAuth consent tests.
• Changed. Each slice changes the tests that read what it changes.
  • In 2FA slice 3: tenant guard on AspNetUserTokens and TOTP secret storage #1276, the AspNetUserTokens tenancy tests copy the UserRole* set.
  • In 2FA slice 5: TOTP code as a second step at login #1278, the direct LoginAsync callers unwrap the new result type.
  • In 2FA slice 6: TOTP code as a step-up factor, including connected-app consent #1279, the direct IssueAsync callers pass code: null, and the two test decorators that override ChangeOwnPasswordAsync take its new parameters.
  • In 2FA slice 7: turn off and reset TOTP, by the user, an Owner, or recover-admin #1280, the recover-admin tests assert MustChangePassword = true.
  • In 2FA slice 8: every Owner must use two-factor authentication #1281, the three credential-check test files follow the verifier's new result type.
  • Each slice that adds an audit action updates the audit-vocabulary tests.
• 2FA slice 8: every Owner must use two-factor authentication #1281 owns a test-factor policy. Seeded users stay unenrolled by default. Two-factor tests seed enrolled users and get codes from recovery codes or a controlled clock, never a wall-clock wait. One shared helper completes the second factor.

Review

The record went through 11 local review rounds before the first push. Rounds 1 to 5 read the whole record. Rounds 6 to 11 read only the change since the round before. The reviewers were Claude Opus 5.5 (A), Claude Fable 5.1 (B), Claude Sonnet 5.5 (C), Codex gpt-6-astra (D) and GLM-5.3-Flash on my homelab vLLM (E). The syntheses, reviews and deltas are in /home/mforce/.cluckwork-slices/totp/interrogate-1274/.

There is no code yet, so the reviewers traced every guard and mutation by hand against the design and the existing code. Nothing in the table proves a test runs.

The "Synthesized" column gives the initial synthesis count for each round, without items that GLM addenda added later. "Raw" counts each reviewer's findings separately, and only rounds 7 to 11 reported it.

Round	Scope	Reviewers	Raw	Synthesized	What it found, and how the record answers it
1	whole record	A, B, C, D, E	not reported	18, plus 6 to decide	Per-user limits sat on a shared counter that falls back to per-process. The record moved them to a durable count on the locked user row and put all state in one owner class.
2	whole record	A, B, C, D, E	not reported	11, plus 6 cheap	All fixed, and the challenge is stated as a credential.
3	whole record	A, B, C, D, E	not reported	9, plus 6 cheap	How to tie the Owner gate to enrolment. The coordinator chose a two_factor token claim and proposed the slice reorder.
4	whole record	A, B, C, D	not reported	10, plus 7 cheap	Nothing critical. The slice order above was settled, and each guard was placed in its slice.
5	whole record	A, B, C, D, E	not reported	7, plus 4 cheap	One critical finding. A rewrite had dropped IgnoresAmbientPrincipalAttribute from the code step, which reopened the #532 lockout bypass. The record restored it with a guard modelled on the #532 test, and gave ClearAsync a three-state result.
6	delta	A, B, C, D	not reported	5, plus 5 cheap	Nothing critical. The no-op paths were defined, the no-exemption rule restored, and each open question written to stand alone.
7	delta	B, D	9	4	Mostly the #1281 test harness. It became a test-factor policy, and my four answers were recorded.
8	delta	B, D	8	5	The test clock against the JWT lifetime, helpers named by list, and two weak mutations. All fixed, and my single-use answer was added.
9	delta	B, D	7	5	All came from a single-use floor based on time. It became a clock-free sign-in count.
10	delta	B, D	3	3	Small items. The password step may read the count, and one mutation was fixed.
11	delta	D	0	0	Clean, nothing above a nit.

After the first push, every round read only the change since the round before:

• Rounds 12 and 13 were narrow checks by D. Round 12 found that self-service recovery needs two codes when signed out, and round 13 was clean.
• Round 14 ran the full panel, A to E, and found one critical issue. Connected-app tokens never refresh, so a grace clock carried in a token claim would never start for an Owner who only uses an app. The coordinator moved the grace start to the credential check.
• Round 15 ran A to D, with GLM's review arriving later. It found two critical issues in the password-change rule, and my "Ask for a code (Recommended)" answer closed both.
• Rounds 16 to 27 found no critical issue. Most ran A to E. The exceptions were rounds 16, 19 and 23, which had A to D, with GLM's review of the round before arriving as an addendum; round 21, which had B and D only; and round 26, where GLM stalled three times, was stopped, and left no completed report.
• Round 27. A, B, C and D found nothing above a nit. GLM rated one wording item a warning. The coordinator counted it as a nit, because the read order it described was correct and only the stated reason was wrong.
• A narrow check by Codex and Fable reviewed ea975c11, which held round 27's nits, and found one nit. A separate one-line check by Codex reviewed its fix, 8f70b8d9, and was clean.

Guards

Both guards walk tracked files. I ran them at 1fe72f3f, the merge of main, after rebuilding both test projects:

dotnet test tests/Cluckwork.Api.IntegrationTests --no-build --filter "FullyQualifiedName~ImagePin_IsOneIdenticalString"
Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2

dotnet test tests/Cluckwork.Application.Tests --no-build --filter "FullyQualifiedName~Documentation"
Passed!  - Failed:     0, Passed:    21, Skipped:     0, Total:    21