Skip to content

fix: 1152 - vetters cannot reassign member roles - #1162

Draft
leahpeker wants to merge 4 commits into
mainfrom
fix-vetters-cannot-set-admin-1152
Draft

fix: 1152 - vetters cannot reassign member roles#1162
leahpeker wants to merge 4 commits into
mainfrom
fix-vetters-cannot-set-admin-1152

Conversation

@leahpeker

@leahpeker leahpeker commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Overview

Vetters (any role with MANAGE_USERS but not full admin) could reassign roles — including the built-in admin role — on any existing member via /admin/members/:id. The role-update endpoint (PATCH /api/auth/users/{id}/roles/) only checked MANAGE_USERS, and the member detail screen rendered an unrestricted role checklist to anyone who could reach that page.

Per Issue #1152: vetters should only be able to add members — any other role change should go through a full admin. So this gates role reassignment on is_admin (holding the built-in admin role), not a grantable permission — MANAGE_ROLES is itself assignable to a non-admin role, which would reopen the same escalation path.

  • Backend: update_user_roles in backend/users/_management.py now requires _is_admin(request.auth) instead of PermissionKey.MANAGE_ROLES.
  • Frontend: added isAdmin() to frontend/src/models/permissions.ts (mirrors backend _is_admin); MemberRolesSection in frontend/src/screens/admin/MemberDetailScreen.tsx now hides the editable role checkboxes from non-admins, showing the member's current roles read-only instead.

Closes #1152

Test plan

  • backend/tests/users/test_users_admin_extended.py::TestUpdateUserRoles — a MANAGE_USERS-only user is denied (403); a non-admin role holding MANAGE_ROLES is also denied; only the built-in admin succeeds
  • frontend/src/screens/admin/MemberDetailScreen.test.tsx — role checklist hidden for non-admins (with or without manage_roles), shown for the built-in admin role
  • make agent-lint (ruff) — clean
  • make agent-typecheck (backend ty) — clean
  • make agent-frontend-typecheck (tsc) — clean
  • make agent-frontend-lint (eslint) — clean
  • prettier on changed files — clean

update_user_roles only checked MANAGE_USERS, so a vetter-tier role
(member management without full admin) could reassign any role on an
existing member, including the built-in admin role via the member
detail screen. Gate the endpoint behind MANAGE_ROLES instead — the
same permission that already gates the role editor. MANAGE_USERS
still covers adding/managing members.

Frontend: hide the editable role checklist on the member detail screen
from users without manage_roles, showing their current roles read-only
instead.
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.

Vetters should not be able to set admin

1 participant