Skip to content

feat(auth): local user accounts and session foundation - #857

Closed
Sanjin-Maker wants to merge 1 commit into
srcfl:masterfrom
Sanjin-Maker:agent/localauth
Closed

feat(auth): local user accounts and session foundation#857
Sanjin-Maker wants to merge 1 commit into
srcfl:masterfrom
Sanjin-Maker:agent/localauth

Conversation

@Sanjin-Maker

Copy link
Copy Markdown
Contributor

Summary

First of the commercial-readiness auth slice (branches off master, no file overlap with draft #744 — that PR extends the read-guard path list; this one adds accounts, and the enforcement PR that follows will compose with whichever lands first).

  • users table in state (STRICT, role CHECK constraint): operator/viewer roles, argon2id hashes in PHC string format, disabled flag, CountOperators so callers can refuse to remove the last enabled operator.
  • go/internal/localauth: HashPassword/VerifyPassword (argon2id at OWASP-minimum params — subsecond on a Pi, expensive on GPUs; constant-time compare; malformed PHC strings rejected), and Sessions — 32-byte random bearer tokens, 24 h expiry, logout, and RevokeUser for password change/disable.
  • Sessions are deliberately in-memory: a restart logs everyone out (the safe failure for a control system) and no session secret is ever persisted.
  • state.User marshals with the hash omitted, so listing endpoints can return rows directly.
  • x/crypto promoted to a direct dependency (already in the tree).

No API wiring yet — api.auth.mode enforcement, audit log and login endpoints follow in the next PR. Zero behavior change for existing installs.

Verification

go test ./internal/localauth/ ./internal/state/ -count=1 — hash/verify round-trip with salt randomness, short-password and malformed-PHC rejection, session lifecycle/expiry/forgery/per-user revocation, user CRUD round-trip incl. duplicate, role CHECK, operator counting with disable. DCO signed; minor changeset.

🤖 Generated with Claude Code

users table (operator/viewer, argon2id PHC hashes) in state, plus
go/internal/localauth: constant-time password verification at OWASP
argon2id parameters and in-memory bearer sessions with expiry and
per-user revocation. Sessions are memory-only on purpose — restart
logs everyone out, and no session secret touches the database. API
enforcement comes separately; nothing changes for existing installs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Sanjin Naidu <sanjin@sanrowconsulting.com>
@frahlg

frahlg commented Aug 7, 2026

Copy link
Copy Markdown
Member

Security and availability blocker: VerifyPassword passes PHC-controlled t, m, p, and derived-key length straight into argon2.IDKey. The argon2 package panics when t is 0 or p is 0, and a very large m or key length can consume excessive memory. The current malformed-PHC tests do not cover valid-looking parameter fields, so a corrupt or imported user row can crash or exhaust the service during login.

Reject an unsupported PHC version and validate strict bounds for t, m, p, salt length, and key length before calling argon2.IDKey. Add tests for t=0, p=0, oversized memory, and an unexpected version. This PR also changes go.mod, which older #732 and #797 already own, so it remains behind them in the queue.

frahlg commented Aug 8, 2026

Copy link
Copy Markdown
Member

Thank you @Sanjin-Maker for a detailed prototype and for covering storage, API and UI rather than treating authentication as one handler.

We are closing #857#859 because local password hashes and box-owned browser sessions change an explicit FTW trust boundary. Today the app pairs with Noise keys, the box stores no user credential, and public browser access relies on an operator-managed HTTPS/auth boundary. We need a product and architecture decision before changing that model.

This version also needs strict Argon2 parameter bounds and version checks before stored or attacker-controlled values reach the allocator. The later API and UI PRs inherit larger session, revocation, rate-limit, audit and access-order problems.

If you want to help continue this discussion, please start with a written issue, not replacement code. It should explain the user need, why app pairing or the reverse-proxy boundary is not enough, the threat model, credential recovery, revocation, rate limits, audit access and safe local recovery. We can decide the architecture together before any implementation starts.

Thank you for exploring the space and making the trade-offs visible.

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.

2 participants