Skip to content

fix: clarify user identities and restore editing and recovery controls - #11

Merged
velixio merged 1 commit into
mainfrom
fix/user-management
Nov 3, 2026
Merged

velixio merged 1 commit into
mainfrom
fix/user-management

Conversation

@velixio

@velixio velixio commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

The Users table does not distinguish usernames from display names, and its Edit dropdown can be hidden by table overflow. Replace that dropdown with a dedicated edit page, label identities explicitly, and link manageable panel accounts to their password and two-factor controls.

Admins retain the existing permission ceiling: they can recover eligible accounts but cannot change owner credentials. Password and two-factor controls are visible on the account Security card with labeled inputs and confirmations. Two-factor reset removes the current authenticator and backup codes, revokes sessions, and requires enrollment again at the next sign-in.

Credential changes and session/backup-code cleanup now commit in one transaction. Username editing reads the current Headscale user list rather than relying on a stale poller snapshot, and linked account names are updated without rewriting unrelated credentials.

Validation:

  • make lint, go test -race ./..., and make size passed (30 MB).
  • HTTP tests cover username editing, account links, admin recovery, session revocation, reenrollment, invalid passwords, and protected owner accounts.
  • Database tests force session-cleanup failures and verify credential changes roll back.
  • Browser tests exercised Users → Edit → Security, username save, password reset and 2FA reset against a live local test server. Layouts passed at desktop, mobile and landscape widths, including dark theme and reduced motion.
  • All 15 touched named Go functions have documentation comments.

Summary by CodeRabbit

  • New Features
    • Added user detail pages for renaming Headscale users, viewing linked accounts and device counts, and managing users according to available permissions.
    • Added password and two-factor reset controls for eligible accounts. Password resets sign out existing sessions; two-factor resets clear backup codes and require setup again at the next sign-in.
    • Updated people and user lists to show usernames and display names more clearly.
  • Documentation
    • Clarified the distinction between Headscale usernames and panel sign-in names, and documented account recovery and permission limits.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The change adds a Headscale user edit page, updates linked account references after user renames, and adds permission-checked account password and two-factor resets. Credential resets revoke sessions, and two-factor resets remove backup codes.

Changes: User and account management

Layer / File(s) Summary
User editing and account linking
internal/web/server.go, internal/web/handlers_users.go, internal/db/accounts.go, internal/web/templates/pages/user.html, internal/web/templates/pages/users.html, internal/web/templates/pages/people.html, internal/web/pages_test.go, internal/web/user_management_test.go
Adds an authenticated user edit route and page with rename and deletion controls. User actions load users from Headscale, skip the tagged-devices user, update linked account references on rename, and refresh the poller after successful rename or deletion. Account links appear as management links only when the actor has permission.
Account credential resets
internal/db/account_security.go, internal/db/account_security_test.go, internal/web/handlers_users.go, internal/web/templates/pages/person.html, internal/web/user_management_test.go, README.md
Password and two-factor resets use database transactions that also revoke sessions. Two-factor resets clear TOTP state and backup codes. The security page and README describe reset effects and account recovery limits.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  actor Admin
  participant personAction
  participant DB
  Admin->>personAction: Submit password or two-factor reset
  personAction->>DB: Call SetAccountPassword or ResetAccountTwoFactor
  DB->>DB: Update credentials and delete account sessions in a transaction
  DB-->>personAction: Commit result
  personAction-->>Admin: Report success or error
Loading
Merge Risk: Minimal · up to bd83c

No blocking issue remains from this review; the change is ready to merge after normal checks.

Security architecture risk: Low · up to bd83c

The changes strengthen account recovery while preserving administrator permission limits. Recovery still has existing concurrency limitations, and deployment exposure was not established, so the assessment is not minimal risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Credential-reset effects are scoped to the selected account and its sessions and backup codes. Headscale rename affects matching linked-account ownership references. These are privileged administrative operations; the new page does not grant credential-reset authority to users who only have Headscale editing permission.

Security Findings and Attack Paths

  • inferred — Reset transactions do not serialize the entire authentication lifecycle: an already-running login can create a session after revocation, and a stale broad account update can restore credential fields. The same administrative reset actions, authentication ordering, and broad writers existed at the comparison base. These are pre-existing recovery limitations, not established introduced or materially worsened PR attack paths.

Trust Boundaries and Controls

  • observed — Dashboard identity comes from server-side session lookup. The authenticated wrapper requires MFA completion, TOTP enrollment, and UIAccess. Central middleware applies CSRF checks to state-changing requests, and account actions independently enforce their capability ceiling before reaching credential mutations.

Resilience and Maintainability Implications

  • observed — Cross-store rename failure can still leave ownership links referring to the old username; no compensation or durable reconciliation is shown. That split transition predates the PR. The change improves containment by reporting the partial failure and avoids the former broad account rewrite that could overwrite unrelated credential fields.

Hardening Proposals

  • proposed — For stronger recovery guarantees, consider credential-generation checks during session issuance and field-specific or version-checked account updates. These would address the existing concurrent-login and stale-writer limitations beyond the transaction improvements in this PR.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main changes: clarifying user identities and restoring user editing and account recovery controls.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 7 files. (5 skipped: 5 unsupported.)

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 78d4442c-30dc-42a9-a249-1ec785247d44
Commits

Reviewing files that changed from the base of the PR and between 85a08db and bd83c8a.

Files selected for processing: 12
  • README.md
  • internal/db/account_security.go
  • internal/db/account_security_test.go
  • internal/db/accounts.go
  • internal/web/handlers_users.go
  • internal/web/pages_test.go
  • internal/web/server.go
  • internal/web/templates/pages/people.html
  • internal/web/templates/pages/person.html
  • internal/web/templates/pages/user.html
  • internal/web/templates/pages/users.html
  • internal/web/user_management_test.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

@velixio
velixio merged commit 64babfb into main Nov 3, 2026
5 checks passed
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