Central auth: TOTP MFA with recovery codes + audit-viewing surface - #36
Open
swimmesberger wants to merge 13 commits into
Open
Central auth: TOTP MFA with recovery codes + audit-viewing surface#36swimmesberger wants to merge 13 commits into
swimmesberger wants to merge 13 commits into
Conversation
Two [RequireRole("Admin")] readers over the rows every other module already
writes: audit.list (keyset paging on the primary key, plus kind/user/route
filters) and audit.kinds (the distinct kinds present, so the filter offers what
is there rather than what a frontend constant remembers).
Paging is a cursor rather than an offset because the trail is append-only and
written while it is read; ordering is by id rather than CreatedAt because the
SQLite provider cannot ORDER BY a DateTimeOffset. Both optional references are
projected null-safely: the FKs are SET NULL, so a row about a deleted account is
the normal end state of a long-lived trail.
No writers, no migration, no retention.
Extends WatchtowerUserStore with the three Identity two-factor stores, adds the User.TwoFactorEnabled/AuthenticatorKey columns and a user_recovery_codes table whose rows hold SHA-256 hashes (AuthSessionService.HashToken) and are deleted on redemption, so a code is single-use by the delete's affected-row count. AuthSessionService gains the pending-MFA record (SessionKind.MfaPending, 5-minute lifetime, hashed at rest) and ValidateAnyAsync now excludes that kind explicitly — the other two validate paths already match on Sso/App, so a half-finished login can never become an identity. UserMfaService is the single implementation of enrol/confirm/verify/disable/reissue that the login endpoint, the self-service endpoints and the admin reset all drive; Identity's own RFC 6238 AuthenticatorTokenProvider does the code validation, so no TOTP package is added.
New admin-gated `audit` module, sidebar order 57 — between Groups and Settings, because the trail is the record of what was done to the entries above it. Read-only: no mutations at all. Filters (event kind, user, app) are part of the query key, so changing one starts a fresh first page; "Load more" follows the server's keyset cursor and Refresh drops the loaded pages and re-reads from the newest row. The kind dropdown is fed by audit.kinds rather than a frontend constant, so a kind a future writer introduces becomes filterable with no edit here. The user and app pickers are gated on their modules being enabled.
…etMfa
POST /api/auth/login stops short of the SSO session for a two-factor account and
answers { mfaRequired, mfaToken } — a pending record in the body, never a cookie.
POST /api/auth/login/mfa finishes the login from a TOTP or a recovery code, with
the same response and hand-over semantics as a single-factor login; a wrong code
keeps the challenge but counts against the lockout, and the login rate limiter
covers the route.
The self-service surface (/api/auth/mfa/*) is REST rather than RPC because every
management handler is gated to the operator realm and protecting your own account
is not management. Enrolment is refused while two-factor is already on: minting a
new key for an enabled account would lock its owner out without ever asking them
for anything.
users.resetMfa is the administrator's one-directional counterpart — it can remove
a second factor, never add one.
The 'No audit-viewing UI' known limitation is retired; what remains true of it — no retention, no export, no deletion — is stated in its place. design.md §7 and §8 gain the Audit module and page, with the two reasons the paging looks the way it does: an append-only table being written while it is read (cursor, not offset), and SQLite refusing to ORDER BY a DateTimeOffset (Id, not CreatedAt).
…for MFA TotpCodes is an independent RFC 6238 implementation (Identity's is internal), which is what makes the verification tests meaningful: a code produced by the verifier's own arithmetic would agree with it however wrong both were. Linked into the API test project rather than duplicated. MfaPendingSessionTests pins the invariant the design rests on — a pending token is refused by ValidateAsync, ValidateAppSessionAsync and ValidateAnyAsync alike — plus expiry, single-use consumption and the disabled-account case. MfaEndpointTests drives the real pipeline: enrolment through the shipped endpoints, the challenge answering without a cookie, the pending token failing as an RPC credential, wrong codes counting against the same five-attempt lockout the password uses, recovery-code login, disable and regenerate, and the audit row for every new kind.
LoginPage gains a code step when the password answers with a challenge, with a recovery-code toggle. The expired-challenge case is decided client-side: the backend answers a wrong code and a lapsed one identically (saying which would tell a caller holding a stolen password whether the window is worth grinding), but this client knows how long ago it asked, so it can offer the right recovery. SecurityPage is platform rather than a feature module because every account may protect its own credentials — including one outside the operator realm, for whom the shell renders the applications portal instead of routes. The shell therefore lets exactly this path through, and the portal grows a link to it. Enrolment is a QR code (qrcode, code-split into the page's own chunk) with a manual-key fallback, then confirmation, then a recovery-code sheet whose Done button is gated on an explicit acknowledgement — the codes cannot be shown again.
- Sidebar order 58, not 57: Realms already claims 57, and equal orders fall back to an alphabetical tie-break, which would have slotted Audit between Groups and Realms and split the three screens meant to be read together. - Refresh invalidates the kinds dropdown too — it is derived from the same rows, so a newly recorded kind would otherwise stay unfilterable until a reload. - Refresh no longer spins during the initial load; the skeleton rows already report that, and the foot button reports "load more". - ArgumentNullException.ThrowIfNull(query) in ListAuthEventKinds, matching ListAuthEvents.
Review blocker: WATCHTOWER__AUTH__RESETPASSWORD now disables 2FA, clears the authenticator key and deletes the recovery codes, and says so in both the audit detail and the warning log. The commonest reason to reach for this hook is a lost authenticator, so a recovery that restored only the password restored nothing — and whoever can set the variable and restart already owns the deployment, so the factor was never a barrier to them. The self-service routes now carry the login rate limiter and drive the account lockout on a refused code: holding a session is not a reason to relax either, since a borrowed session is exactly the case where someone grinds six digits to turn the factor off. The status read stays unlimited — it judges nothing. Confirming enrolment additionally requires the account password. The code proves possession of the new authenticator, which whoever holds a borrowed session also has; the password is the one thing that session does not carry. Also: login/mfa binds the challenge to the realm of the host it arrives on; ValidateAnyAsync filters by allow-list so a new SessionKind cannot drift in; DisableAsync is one user write inside a transaction; ConfirmTotpAsync rolls the flag back rather than leaving an account demanding a factor with no way around it; the password step no longer clears the failure budget before the 2FA branch; and every claim that stamp rotation invalidates a session is corrected — nothing reads the stamp yet, it is bookkeeping for a later hook.
The setup dialog's confirm step now asks for the account password alongside the code, matching the backend: the code proves the new authenticator works, which is something whoever borrowed a signed-in browser also has. The recovery-code sheet can no longer be dismissed by Escape or an outside click until the acknowledgement is ticked — the codes cannot be shown again, so a stray key press there costs every code just issued. The login page grows the missing branch for a non-'signed-in' outcome rather than leaving the button spinning. UserDto carries TwoFactorEnabled (a policy fact, not a secret), the Users page shows a 2FA column and offers Reset two-factor behind a confirm dialog, and the row action appears only where there is something to clear. Docs: README, the central-auth operator guide and design.md now describe the shipped surface — including that break-glass clears the second factor — and mark per-realm enforcement as the part still outstanding.
Reconciles the audit-view merge: both README feature bullets kept, securityRoute + audit.routes both registered, rpc-schema regenerated on the combination (108 methods, byte-identical to the auto-merge). Also removes an orphaned JSDoc block in AppsPage.tsx (review nit #14).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #18 (§4 TOTP, audit-view follow-up) and #31 (R7 MFA baseline, R8 audit surface) — the two pieces shared by both issues.
TOTP MFA (
feat/auth-totp)WatchtowerUserStoreimplements the 2FA/authenticator-key/recovery-code store interfaces; validation is Identity's built-in RFC 6238 provider.users.two_factor_enabled+users.authenticator_key, newuser_recovery_codestable — 10 codes per issue, SHA-256-hashed at rest, delete-on-redeem (single-use under concurrency).{ mfaRequired, mfaToken }(single-use pending record, 5-min lifetime, hashed at rest, excluded from every session-validation path by an allow-list) —POST /api/auth/login/mfafinishes with a TOTP or recovery code. Wrong codes count toward the account lockout; the per-IP login limiter covers the route; the pending token is realm-host-bound./api/auth/mfa/*(status / begin / confirm / disable / regenerate) — deliberately REST rather than RPC so accounts of any realm can protect themselves (RPC handlers are system-realm-gated). Enrollment confirm requires the account password (step-up), all judging endpoints are rate-limited and drive the lockout.users.resetMfahandler + 2FA column and Reset two-factor action on the Users page.RESETPASSWORD) now also clears the second factor — recovery stays a way in for an admin who lost both factors.qrcodedep), recovery-code sheet gated on explicit acknowledgement.mfa.totp.*,mfa.recovery.*,login.mfa.*).Audit view (
feat/auth-audit-view)Auditmodule:audit.list(keyset pagination onId DESC— stable under concurrent appends and sidesteps the SQLiteORDER BY DateTimeOffsetlimitation; kind/user/route filters; limit clamped 100/500) andaudit.kinds(distinct values, so new kinds — including this PR's MFA kinds — become filterable with no frontend change).SET NULLFKs: the trail outlives its subjects), no secrets in the DTO.Process & verification
Each feature was implemented in an isolated worktree, went through an independent review loop (TOTP: 1 blocker — break-glass couldn't recover a 2FA'd admin — plus 3 majors, all fixed and re-verified; audit view: 1 minor + 3 nits, fixed), and a final architecture gate before merging.
dotnet build: 0 warnings, 0 errorsDeliberate deferrals (tracked)
Realmpolicy seam, Central identity: Watchtower as OIDC provider + tenant registry (realms, invitations, device/session management, MFA) #31 R7)auth_eventsfilter indexes + retention story