refactor(store): wave B.12 — user repo (#267)

[PaulDotterer]

Summary

Biggest single-domain migration in Wave B. `UserRepo` covers the core user-projection reads consumed across 21 files. Branches off main directly.

Methods

• `Get(id)` / `GetByEmail(email)`
• `SessionInfo(userID)` — narrow shape for hot refresh-token validation
• `Permissions(userID)` — flattened direct + user-group inherited permissions
• `NextLinuxUID()` — UID allocator
• `List(filter)` / `Count()` / `ListAllNonDeleted()`

Call sites migrated (~70)

• api (12 handlers): `user_handler`, `auth_handler`, `sso_handler`, `totp_handler`, `settings_handler`, `system_actions` + tests, `terminal_handler`, `token_handler`, `identity_link_handler`, `assignment_handler`, `user_group_handler`
• scim (6 files): `users`, `users_create`, `users_helpers`, `users_mutate`, `users_translation`, `handler` (interface)
• idp: `linker` + `linker_test`
• cmd/control: `admin_user`

Signatures shifted

• `userToProto`, `userToSCIM` — both now take `store.User`
• `SystemActionsCleaner.CleanupDeletedUserActions` (+ test fake)
• 6 system-action helpers + `systemTtyUserParams`

Field renames

`LinuxUid` → `LinuxUID` at the domain type. `SshPublicKeys` stays as `json.RawMessage` at the boundary per the JSONB normalize plan.

Non-goals

• SCIM user reads (`FindSCIMUser*`, `ListSCIMUsers`, `CountSCIMUsers`) — return user-projection-shaped rows but conceptually SCIM domain; SCIM follow-up.
• Write-side projector queries — stay on `Queries()` per pattern.

Acceptance

• Build / vet / staticcheck / gofmt: clean.
• Tests pass: api (6m30s), scim (1m12s), idp.

Part of #242. Closes #267.

Summary by CodeRabbit

• Refactor
  • Switched internal user-data access to a new repository-backed implementation across many services and tests, consolidating user models and repositories. No changes to public APIs or user-facing behavior—existing features, responses, and workflows remain the same.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7f437f12-9a6b-4e3a-a913-c767d52e5554

📥 Commits

Reviewing files that changed from the base of the PR and between ecd4beb and f93af48.

📒 Files selected for processing (26)

• cmd/control/admin_user.go
• internal/api/assignment_handler.go
• internal/api/auth_handler.go
• internal/api/identity_link_handler.go
• internal/api/settings_handler.go
• internal/api/sso_handler.go
• internal/api/system_actions.go
• internal/api/system_actions_manager_test.go
• internal/api/system_actions_params_canary_test.go
• internal/api/terminal_handler.go
• internal/api/token_handler.go
• internal/api/totp_handler.go
• internal/api/user_group_handler.go
• internal/api/user_handler.go
• internal/scim/groups_helpers.go
• internal/scim/handler.go
• internal/scim/handler_test.go
• internal/scim/users.go
• internal/scim/users_create.go
• internal/scim/users_helpers.go
• internal/scim/users_mutate.go
• internal/scim/users_translation.go
• internal/store/postgres/user.go
• internal/store/postgres/wire.go
• internal/store/repos.go
• internal/store/user.go

✅ Files skipped from review due to trivial changes (1)

• internal/api/token_handler.go

🚧 Files skipped from review as they are similar to previous changes (25)

• internal/scim/users_helpers.go
• internal/store/repos.go
• internal/scim/handler_test.go
• internal/api/user_group_handler.go
• internal/store/postgres/wire.go
• internal/api/terminal_handler.go
• cmd/control/admin_user.go
• internal/scim/users.go
• internal/api/identity_link_handler.go
• internal/api/settings_handler.go
• internal/api/assignment_handler.go
• internal/scim/groups_helpers.go
• internal/scim/users_create.go
• internal/scim/users_translation.go
• internal/api/sso_handler.go
• internal/api/system_actions_manager_test.go
• internal/api/auth_handler.go
• internal/api/totp_handler.go
• internal/scim/users_mutate.go
• internal/store/user.go
• internal/api/system_actions_params_canary_test.go
• internal/scim/handler.go
• internal/store/postgres/user.go
• internal/api/user_handler.go
• internal/api/system_actions.go

---

📝 Walkthrough

Walkthrough

This PR migrates user data access from query-based projections (store.Queries()) to a new repository abstraction (store.Repos().User): adds store.User types and UserRepo interface, implements a Postgres-backed UserRepo, wires it into store.Repos, and updates handlers, SCIM code, system actions, and tests to use the repository methods.

Changes

User Repository Migration

Layer / File(s)	Summary
User Repository contract and domain types internal/store/user.go	Introduces store.User, store.UserSessionInfo, ListUsersFilter, and the UserRepo interface with Get/GetByEmail/SessionInfo/Permissions/NextLinuxUID/List/Count/ListAllNonDeleted.
Postgres UserRepo implementation internal/store/postgres/user.go	Implements Postgres-backed User repo wrapping sqlc queries; adds methods to fetch users, session info, permissions, allocate Linux UIDs, list/count users, and convert sqlc rows to store.User.
Store registry wiring internal/store/repos.go, internal/store/postgres/wire.go	Adds User UserRepo to store.Repos and initializes it in NewRepos via NewUser(q).
API handler user data access migration internal/api/* (auth_handler.go, user_handler.go, sso_handler.go, totp_handler.go, settings_handler.go, assignment_handler.go, token_handler.go, terminal_handler.go, identity_link_handler.go, user_group_handler.go)	Replaces query-layer user reads with repository calls (Get/GetByEmail/SessionInfo/Permissions/NextLinuxUID/List/Count) across many handlers; preserves existing error handling and control flow.
SCIM migration and translations internal/scim/* (users.go, users_create.go, users_helpers.go, users_mutate.go, users_translation.go, groups_helpers.go, handler.go, handler_test.go)	Updates SCIM handlers and helpers to accept and work with store.User, uses repository reads for existence/read-back and UID allocation, and updates test fakes accordingly.
System actions refactor internal/api/system_actions.go, cmd/control/admin_user.go	Switches system-action syncing/cleanup to use Repos().User (ListAllNonDeleted/Get/Permissions), updates helper signatures to accept store.User, and renames LinuxUid usages to LinuxUID.
Tests and canary updates internal/api/system_actions_manager_test.go, internal/api/system_actions_params_canary_test.go, internal/scim/handler_test.go	Adjusts tests to load users via Repos().User.Get, updates fixtures and fakes to use store.User, and renames UID field usage in expectations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• manchtools/power-manage-server#231: Refactors SCIM user translation helpers that are adapted here to accept store.User.
• manchtools/power-manage-server#83: Related changes to system-action syncing/cleanup that interact with the updated CleanupDeletedUserActions contract.
• manchtools/power-manage-server#69: Overlaps on TTY/system-action parameter changes and UID mapping that this PR also adjusts.

"🐰 I hopped through code so neat,
New UserRepo makes reads complete,
Queries rest, Repos lead the way,
UIDs and types now bright as day,
Hooray for hops in CI's beat!"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(store): wave B.12 — user repo (#267)' clearly and specifically identifies the main change as implementing a user repository refactor in wave B.12, directly aligning with the pull request's primary objective.
Linked Issues check	✅ Passed	The pull request successfully implements all core UserRepo methods (Get, GetByEmail, SessionInfo, Permissions, NextLinuxUID, List, Count, ListAllNonDeleted) and migrates approximately 70 call sites across 21 files including API handlers, SCIM handlers, and CLI tools as required by issue #267.
Out of Scope Changes check	✅ Passed	All changes are within scope: UserRepo interface and implementation, migration of call sites to use repository methods, signature updates for userToProto and userToSCIM, SystemActionsCleaner contract changes, and the LinuxUid to LinuxUID field rename—all explicitly required by issue #267.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/267-user-repo

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/control/admin_user.go (1)

23-27: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle non-not-found lookup errors before creating the bootstrap admin.

Line 23 currently treats every GetByEmail error as “user does not exist” and continues with creation. That can create duplicate bootstrap attempts or mask DB/repo failures. Only proceed on explicit not-found; return other errors.

Proposed fix

 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	urlpkg "net/url"
@@
 	_, err := st.Repos().User.GetByEmail(ctx, email)
 	if err == nil {
 		logger.Info("admin user already exists", "email", email)
 		return nil
 	}
+	if !errors.Is(err, store.ErrNotFound) {
+		return fmt.Errorf("lookup admin user by email: %w", err)
+	}
 
 	// Create admin user via event sourcing

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/control/admin_user.go` around lines 23 - 27, The code assumes any error
from st.Repos().User.GetByEmail(ctx, email) means the user is missing; change
the logic to first check the error and only proceed to create the bootstrap
admin when the error is an explicit "not found" case (use the repository's
not-found sentinel or errors.Is check against the repo/sql not-found value used
in your project); if err != nil and is not a not-found error, return that error
immediately instead of continuing to creation; keep the existing
logger.Info("admin user already exists", "email", email) path when GetByEmail
returns nil and reference GetByEmail and the repo's not-found sentinel in your
check.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cmd/control/admin_user.go`:
- Around line 23-27: The code assumes any error from
st.Repos().User.GetByEmail(ctx, email) means the user is missing; change the
logic to first check the error and only proceed to create the bootstrap admin
when the error is an explicit "not found" case (use the repository's not-found
sentinel or errors.Is check against the repo/sql not-found value used in your
project); if err != nil and is not a not-found error, return that error
immediately instead of continuing to creation; keep the existing
logger.Info("admin user already exists", "email", email) path when GetByEmail
returns nil and reference GetByEmail and the repo's not-found sentinel in your
check.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1951261d-2b90-4ca4-a848-64c4fbcb73e2

📥 Commits

Reviewing files that changed from the base of the PR and between 8e6bf70 and ecd4beb.

📒 Files selected for processing (26)

• cmd/control/admin_user.go
• internal/api/assignment_handler.go
• internal/api/auth_handler.go
• internal/api/identity_link_handler.go
• internal/api/settings_handler.go
• internal/api/sso_handler.go
• internal/api/system_actions.go
• internal/api/system_actions_manager_test.go
• internal/api/system_actions_params_canary_test.go
• internal/api/terminal_handler.go
• internal/api/token_handler.go
• internal/api/totp_handler.go
• internal/api/user_group_handler.go
• internal/api/user_handler.go
• internal/scim/groups_helpers.go
• internal/scim/handler.go
• internal/scim/handler_test.go
• internal/scim/users.go
• internal/scim/users_create.go
• internal/scim/users_helpers.go
• internal/scim/users_mutate.go
• internal/scim/users_translation.go
• internal/store/postgres/user.go
• internal/store/postgres/wire.go
• internal/store/repos.go
• internal/store/user.go