feat(authz): Settings → Roles management UI (placeholder hook)#358
Draft
mcharles-square wants to merge 5 commits into
Draft
feat(authz): Settings → Roles management UI (placeholder hook)#358mcharles-square wants to merge 5 commits into
mcharles-square wants to merge 5 commits into
Conversation
github-actions
Bot
added
javascript
🔐 Codex Security Review
Review SummaryOverall Risk: HIGH Findings[HIGH] Selected member role is silently dropped, creating ADMIN users
[HIGH] Roles UI mutates only in-memory placeholder state
NotesNo cryptostealing, SQL injection, command injection, network discovery, plugin, Rust, or infrastructure issues were visible in the reviewed diff. Generated by Codex Security Review | |
7 tasks
mcharles-square
force-pushed
the
feat/rbac-roles-management-ui
branch
from
4f84fff to
9d6373d
Compare
U11 from docs/plans/2026-05-19-001-feat-granular-rbac-plan.md. New
Settings → Roles page where admins (anyone holding role:manage) can:
- List built-in and custom roles, with member counts and permission
counts.
- Edit any role except Owner (SUPER_ADMIN is immutable server-side).
- Create custom roles via a grouped checkbox builder over the full
permission catalog: search, collapse-by-resource, automatic
inclusion of read-pairing dependencies, "Required" lock icons on
reads still held by other selected actions.
- Delete custom roles, gated on memberCount = 0 so members never end
up orphaned mid-deletion.
Also wires role selection into Settings → Team:
- AddTeamMemberModal grows a role Select populated from
useRoleManagement. Owner is excluded (ownership transfer is a
separate, deliberate flow). FIELD_TECH is the default when present.
- useUserManagement.createUser accepts a roleId param (currently
voided with a TODO; CreateUserRequest needs a role_id field — see
inline comment).
Adds /settings/roles route + nav entry (gated on role:manage),
prefetch wiring, and stories for the three new components.
Notes on placeholders (each marked TODO inline):
- api/useRoleManagement.ts serves an in-memory dataset derived from
the catalog. The proto package authz.v1 currently exposes only the
Permission / PermissionGroup messages; the ListRoles / CreateRole /
UpdateRole / DeleteRole RPCs and AuthzService client land in a
follow-up. The hook's signature mirrors useUserManagement so the
settings components consume it identically.
- features/settings/utils/permissionCatalog.ts is a hand-maintained
mirror of server/internal/domain/authz/catalog.go, replaced by a
AuthzService.GetPermissionCatalog fetch once that RPC ships.
- useUserManagement.createUser accepts roleId but does not forward it
yet — CreateUserRequest has no role_id field; add it then wire the
forward in the authClient.createUser call.
formatRole gains a FIELD_TECH → "Field Tech" mapping so the role badge
on the Team table renders consistently with the role label in the
Roles list.
- CreateEditRoleModal: drop the prevKey/openKey render-phase reset
pattern. Callers now remount the modal via key={role?.roleId ??
"create"}, so the form state is seeded from useState defaults
exactly once per open. Removes three inner key props on the input
fields that existed only to remount them under the prior scheme.
- Roles.tsx: consolidate the two CreateEditRoleModal mounts (create vs
edit) into one, gated on `open={showCreateModal || !!editRole}` with
`role={editRole}`. Replaces the two handleX callbacks with a single
handleEditor pair.
- useRoleManagement: extract a RoleCallbacks base for the shared
onError/onFinally pair across the four *Props types. Drops ~8 lines
of duplication.
- Type RoleItem.builtinKey as the union "SUPER_ADMIN" | "ADMIN" |
"FIELD_TECH" (new BuiltinRoleKey export) instead of plain string —
catches consumer typos at compile time.
- CreateEditRoleModal.toggleKey: when unchecking a locked read whose
withRequiredReads expansion produces an identical set, return the
previous reference so React doesn't schedule a no-op re-render of
the permission tree.
…der UI - Consolidate 12 permission groups into 6: hide fleet:read (auto-dependency), merge sites + racks into "Sites, buildings & racks", merge admin singletons (server logs, API keys, team, roles, fleet nodes) into "Administration" - Simplify permission rows to description-only (remove action label + key) - Remove lock badge UX; unchecking a read key now cascades removal - Use standard modal width, design tokens, and textOnly Button for CTAs - Drop role description field and built-in role callout banner - Generalize edit title to "Edit role" with disabled name input for built-ins Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mcharles-square
force-pushed
the
feat/rbac-roles-management-ui
branch
from
9d6373d to
10dbae0
Compare
Bug fixes visible today: - CreateEditRoleModal no longer wipes role descriptions on save — description is optional in CreateRoleProps/UpdateRoleProps and updateRole preserves the existing value when omitted. - Read-key checkboxes that still have action dependents render disabled with a "(required)" suffix via lockedReadKeys, replacing the misleading toggleKey cascade comment that promised behavior the code didn't implement. - Roles page renders CreateEditRoleModal conditionally with a bump counter so two consecutive opens always get a fresh instance, fixing stale name/selection/query leak. - Edit action is now hidden for all built-in roles (was only SUPER_ADMIN); server unconditionally rejects built-in mutation, so the UI must not expose it. Catalog and copy alignment with server: - Added activity:read to client permissionCatalog with the same description and resource as server catalog.go. - role:manage description now matches the server: "Create, edit, and delete custom roles. Built-in roles cannot be modified." - fleet is intentionally excluded from RESOURCE_TO_GROUP so fleet:read remains dependency-floor only — documented inline. Land role_id end-to-end (was the unresolved P1 Codex finding): - Added optional string role_id = 2 to auth.v1.CreateUserRequest; regenerated Go and TS. - New domain helper resolveCreateUserRole: empty role_id keeps the legacy ADMIN-default; non-empty validates org membership, rejects SUPER_ADMIN, and runs the parity check used by authorizeCallerForNewUserWithRole before persisting the assignment via the existing CreateUserOrganizationRole path. - useUserManagement.createUser forwards roleId on the wire. - New TestParseInt64RoleID and TestResolveCreateUserRole_ ValidationBranches cover the pre-resolver rejection paths. - Extended UserManagementStore interface with GetRoleByID; regenerated mock. Swap-target clarifications: - TODO comments renamed to authzClient.createCustomRole / updateCustomRole / deleteCustomRole to match real method names. - Added RoleItem-as-client-model comment plus pbToRoleItem adapter notes at swap sites: proto Role uses permissionKeys (not permissions), numeric BuiltinKey enum (not string union), and Timestamp (not Date | null) for updatedAt. Maintainability: - isImmutable(role) helper consolidates the SUPER_ADMIN / builtin predicate split. - ALL_KEYS inlined into builtin-admin; ADMIN_KEYS alias removed. - Migration-number comment in useRoleManagement replaced with intent-focused description.
The E2E teamAccounts spec failed every "add member" path with
"FleetError: invalid_argument (Common: 0) invalid role_id" because
the placeholder useRoleManagement emits non-numeric ids
("builtin-field-tech", "role-<ts>") that the server's int64 parser
rejects. Strip non-numeric roleId in useUserManagement.createUser so
the server applies its default role until the real AuthzService.ListRoles
swap provides numeric ids; the guard removes itself naturally once those
ids are on the wire.
5 tasks
Collaborator
Author
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
2 participants
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.
Summary
U11 from
docs/plans/2026-05-19-001-feat-granular-rbac-plan.md. NewSettings → Rolespage where admins (anyone holdingrole:manage) can manage built-in and custom roles, plus a role Select onAddTeamMemberModalso new members can be assigned a role at creation time.What it does
Roles page (
/settings/roles)memberCount === 0so members can't be orphaned mid-deletion.role:manage. Direct visits redirect to/settings/generalfor callers without the key.Create/Edit role modal
:readpartner, and any miner action requiresfleet:read).(required)suffix vialockedReadKeys, so the UI never holds a selection the server would reject on save.descriptionfrom the payload when no description input is surfaced; the hook leaves the existing role description untouched.AddTeamMemberModal
useRoleManagement.listRoles.roleIdis now forwarded end-to-end toauth.v1.CreateUser(see "Server work" below).Server work (in this PR)
proto/auth/v1/auth.proto—CreateUserRequestgainsoptional string role_id = 2.server/internal/domain/auth/service.go— resolves the role for the new user: emptyrole_idkeeps the legacy ADMIN-default; non-empty validates org membership, rejects SUPER_ADMIN (ownership transfer is a separate flow), and runs the same parity check used byauthorizeCallerForNewUserWithRolebefore persisting the assignment through the existingCreateUserOrganizationRolewrite.TestParseInt64RoleID,TestResolveCreateUserRole_ValidationBranches) cover the resolver's validation branches. DB-backed integration coverage for the success path is left for the existing CreateUser integration suite to grow into (see test plan).Placeholders & blocking deps
Two placeholders remain. Each is marked
TODO(rbac):inline with the exact next step:api/useRoleManagement.ts— in-memory datasetAuthzServicewithListRoles/CreateCustomRole/UpdateCustomRole/DeleteCustomRoleRPCs are now merged onmain; swap each placeholder for the realauthzClient.*call. The swap requires apbToRoleItemadapter at the wire boundary — protoRoleusespermissionKeys(notpermissions), a numericBuiltinKeyenum (not the string union the client model uses), andTimestamp(notDate | null) forupdatedAt. Inline comments at the swap sites describe the adapter shape.features/settings/utils/permissionCatalog.ts— hand-maintained mirror ofcatalog.goAuthzService.GetPermissionCatalogRPC. Replace the constant with a fetch + cache; the helper functions (requiredReadsFor,withRequiredReads,lockedReadKeys) stay as-is.Deferred from U11 plan
These U11 surfaces are intentionally not in this PR — they should land as separate follow-up work:
Team.tsx. Plan calls for a "Roles" cell with a modal that lets admins add/remove assignments on existing users. This PR only surfaces role at user creation time. Depends onAssignRole/UnassignRole/ListUserAssignmentswhich are stillUnimplementedserver-side.memberCount > 0. UI behavior matches server semantics but doesn't match plan UX.Files
New:
client/src/protoFleet/api/useRoleManagement.tsclient/src/protoFleet/features/settings/components/Roles.tsxclient/src/protoFleet/features/settings/components/CreateEditRoleModal.tsxclient/src/protoFleet/features/settings/components/DeleteRoleDialog.tsxclient/src/protoFleet/features/settings/utils/permissionCatalog.tsModified:
proto/auth/v1/auth.proto—CreateUserRequest.role_idserver/internal/domain/auth/service.go(+ tests) — role resolution and validationserver/internal/domain/stores/interfaces/user.go+sqlstores/user.go+ mocks —GetRoleByIDclient/src/protoFleet/api/useUserManagement.ts— forwardsroleIdclient/src/protoFleet/features/settings/components/AddTeamMemberModal.tsx— role Selectclient/src/protoFleet/features/settings/utils/formatRole.ts+ test — FIELD_TECH → "Field Tech"client/src/protoFleet/config/navItems.ts—/settings/rolesentry gated onrole:manageclient/src/protoFleet/routePrefetch.ts+router.tsx— lazy import + routeTest plan
tsc --noEmitclean.eslintclean.vitest run src/protoFleet/features/settings src/protoFleet/api— 331 passed.go build ./...clean;go test ./internal/domain/auth/...passes (including newTestParseInt64RoleIDandTestResolveCreateUserRole_ValidationBranches).role:manage).AuthzServiceRPCs are wired: replace the placeholder hook body via thepbToRoleItemadapter, drop thepermissionCatalog.tsmirror, confirm tests still pass.