Skip to content

feat(admin): disable/remove admins, TOTP QR code, audited sign-ins and failed 2FA - #37

Merged
jayleaton merged 5 commits into
mainfrom
feat/admin-followups
Oct 22, 2026
Merged

jayleaton merged 5 commits into
mainfrom
feat/admin-followups

Conversation

@jayleaton

Copy link
Copy Markdown
Owner

Follow-ups to #26. Admins can now be disabled, enabled and removed. The 2FA setup page shows a QR code. Admin sign-ins, sign-outs and wrong 2FA codes are audited. The bootstrap admin's first CLI invite is no longer recorded as a resend.

Summary

 services.AdminService
+  DisableAdmin / EnableAdmin / RemoveAdmin   policy → not yourself → not the last active admin → audit
   InviteAdmin                                "resent" only if an earlier admin.invite is on record
+services.AuthEventService                     admin.sign_in (method) · admin.sign_out · admin.two_factor_failed (failures)
 server
+  AuthEvents   wraps UIAuth and the JSON auth endpoints; serializes 2FA checks and attributes
+               a failure via the admin's failure counter (adminrepo.Admins.FailedSince, read-only)
+  AdminAccounts.Disable = auth BanUser(permanent, revokes sessions) · Enable = UnbanUser · Remove = adminrepo.Admins.Remove
 api   + POST /admins/{id}/disable · POST /admins/{id}/enable · DELETE /admins/{id}
 ui    + row actions (no actions on your own row, confirm before remove) · inline-SVG TOTP QR · new audit labels (en/ja)
  • Disable bans the admin permanently: their sessions die and their sign-in is refused. Enable reverses that. Remove deletes the admin row; their sessions cascade, and audit entries keep the id and email.
  • Nobody can disable or remove themselves. The last active admin can't be disabled or removed either. Pending invites and disabled admins don't count as active.
  • QR code: rendered server-side with github.com/boombuler/barcode (pure Go, no dependencies, MIT, v1.1.0) as a single <path>. There is no external service and no image URL, and the otpauth URI and secret are never logged.
  • Failed 2FA attribution: packages/auth has no event hooks, and a wrong code doesn't say whose challenge it was. Auth does bump that admin's failedVerificationCount and updatedAt. AuthEvents serializes verifications within the process, so after a failure the counter that moved since the check started is its own. That is exact with one admin instance, which is the template's setup. With several instances, two simultaneous failures could be attributed to both admins. An auth.Config event hook would be the cleaner long-term source; this PR stays inside apps/admin and adminrepo as asked.
  • Code fix: if an admin is removed between the lookup and the change, the result is now a 404 instead of a 500.

Evidence

Before (#26): there was no way to disable or remove an admin, and no QR code (manual key only). Sign-ins and 2FA failures weren't audited. The bootstrap admin's first CLI invite was logged with resent: true.

After:

  • Real servers: web on :3200 and admin on :3201 over one temporary Postgres container (since removed), with ADMIN_REQUIRE_2FA=true, driven by headless Chromium. The QR code was decoded with zbarimg from the rendered SVG, and the code entered came from the decoded secret:

    [e2e] 01-root: QR decodes to otpauth://totp/Admin:admin@example.com?algorithm=SHA1&digits=6&issuer=Admin&period=30&secret=<redacted>
    [e2e] root: wrong code refused: "That code is not valid. Try again."
    [e2e] root: signed in with the right code
    [e2e] 02-eve: QR decodes to otpauth://totp/Admin:eve@example.com?...
    [e2e] eve: disabled session redirected to /login
    [e2e] eve: disabled sign-in refused: "Invalid credentials"
    [e2e] root: eve enabled
    [e2e] confirm dialog: "Remove eve@example.com? They lose access to the dashboard right away. This cannot be undone."
    [e2e] root: own row has 0 action buttons
    [e2e] ALL OK
    

    The audit log then read, newest first: Removed admin, Enabled admin, Disabled admin, Signed in (eve, password + two-factor), Invited admin, Signed in, Failed two-factor code (1 in a row), Signed out, Signed in, Invited admin by cli with no "resent", Created first admin. The server log contained no otpauth or secret, and request paths show /api/auth/reset-password/{token}.

    The 7 screenshots, including the QR enrolment page, the admins page with eve disabled, and the audit log in en and ja, are on the dev box at ~/Documents/spark-work/zig-dsv41-scratch/admin-shots/followups/. They are not uploaded to GitHub.

  • Local checks (the COMPUTE RULE limits them to gofmt, vet and plain go test of the touched packages):

    Package Result Passing tests (incl. subtests)
    services 100% 38
    api 100% 118
    ui 90.0% 127
    adminrepo 83.7% —
    server all pass —

    The new DB-backed tests are TestAdminManagement, TestAuthEvents, TestAdminsRemove and TestAdminsFailedSince. TestAuthEvents covers JSON sign-in → wrong TOTP → right TOTP → sign-out, and asserts the exact audit trail.

  • Full make ci (lint, -race, coverage floors, probes): see this PR's CI run.

Merge Danger

Door: two-way.

There are no migrations; the 2FA counter query is read-only. Removing an admin deletes their row in the admin database, but that is an explicit, confirmed and audited admin action, not something deploying this PR does. A code revert removes the features.

Blast Radius: service.

Only apps/admin and packages/database/adminrepo change, plus one new Go dependency (boombuler/barcode). The web app is untouched. The audit log gains a few sign-in and sign-out rows per admin session.

🤖 Generated with Claude Code

jayleaton and others added 5 commits October 22, 2026 16:22
…dmin database

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… last active admin); audit sign-ins, sign-outs and failed 2FA; first invite of a seeded admin is no resend

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in removal

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, labels for the new audit actions

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jayleaton
jayleaton marked this pull request as ready for review October 22, 2026 09:24
@jayleaton
jayleaton merged commit dc345be into main Oct 22, 2026
10 checks passed
@jayleaton
jayleaton deleted the feat/admin-followups branch October 22, 2026 09:27
jayleaton added a commit that referenced this pull request Oct 22, 2026
…admin actions)

- The admin auth allowlist (audited by AuthEvents) is mounted with the
  opt-in JSON API, behind apiOnly, since scripts need it for bearer tokens;
  by default cmd/admin still mounts nothing under /api/.
- Crawl covers the new disable/enable/remove routes; their loading labels
  drop inline styles for the CSP.
- Wiring tests read invite tokens from the /set-password link and call
  auth the way scripts do (Expo-Origin, not a browser Origin).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jayleaton added a commit that referenced this pull request Oct 22, 2026
…y server-side (#38)

* feat(security): browsers only see the UI; API, secrets and errors stay server-side

- JSON API (/api/auth, /api/core) and /healthz answer any browser request
  (Sec-Fetch-*, Origin, Referer, Accept: text/html) with a bare 404, no CORS,
  no 405/Allow. Admin app mounts no JSON API and no auth endpoints.
- Emailed auth links for the UIs open UI pages (auth.Config.VerifyEmailPath,
  relative redirectTo); deep-link callbacks keep the redirecting endpoints.
- UI responses carry a nonce CSP without unsafe-*, nosniff, XFO, strict-origin
  referrer, Permissions-Policy, COOP/CORP, no-store, HSTS on https; inline
  styles removed; static has no directory listing; unrouted paths are 404.
- Config credentials are secrets.Secret; Reveal only in cmd (forbidigo);
  redacting slog handler for tokens, cookies, auth headers, emailed links.
- Generic errors: plain 500 for UI panics, JSON-typed validation messages.
- Admin session cookies use __Host-; locale cookies are HttpOnly.
- Exposure crawl tests over every UI route in both apps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(security): close review findings (open redirect, link exception, crawler blind spot)

- web safeNext rejects control characters and backslashes (/%09/evil).
- Deep-link exception only for deep-link or other-origin callbacks, works
  without Fetch Metadata, and turns auth JSON into the bare 404.
- /api is a 404 (no 307); web non-UI responses get nosniff/XFO/no-store.
- Crawler checks SSE bodies (live streams cut after their first event),
  probes for markup injection and off-site redirects; TestNoUnescapedOutput
  bans templ.Raw/Unsafe* in UI sources.
- Redactor covers header/value maps and struct JSON; theme cookie is secure
  on https; docs say templ escaping, not the CSP, is the XSS defence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(admin): reconcile the lockdown with #37 (audited auth endpoints, admin actions)

- The admin auth allowlist (audited by AuthEvents) is mounted with the
  opt-in JSON API, behind apiOnly, since scripts need it for bearer tokens;
  by default cmd/admin still mounts nothing under /api/.
- Crawl covers the new disable/enable/remove routes; their loading labels
  drop inline styles for the CSP.
- Wiring tests read invite tokens from the /set-password link and call
  auth the way scripts do (Expo-Origin, not a browser Origin).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(admin): set invite passwords through the audited endpoint again

TestAuthEvents expects the set_password sign-in in the audit trail; the
helper reads the token from the /set-password link and posts it to the
auth endpoint the opt-in API mounts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant