Skip to content

security(users): /users/:id status, role and delete act on the global account, not on the caller's membership #807

Description

@alex-dembele

Problem

An administrator of organization A can lock a person out of every organization they belong to, or delete their account everywhere. They can also "change their role" and get a success message for a change that has no effect. All three go through routes that write the global users row instead of the caller's membership.

Routes: backend/cmd/server/main.go:2052-2054. Handlers: backend/internal/handler/user_handler.go.

Route What it writes Consequence
PATCH /users/:id/status users.is_active (l.157-168) Sign-in rejects an inactive user before it reads the membership (internal/application/auth/login.go:182). Deactivating someone in A locks them out of B.
DELETE /users/:id deletes the users row (l.289) The account disappears from every organization, not just the caller's.
PATCH /users/:id/role users.role_id (l.227-241) Sessions take their role from organization_members.role (the GetOrganizationMember block in login.go). The write is ignored, but the response says "User role updated".

The admin check inside these handlers (currentUser.Role.Name != "admin", l.146, 212, 269) reads the global role. #702 has the same root cause.

userInTenant() (l.70) only checks that the target is a member of the caller's tenant. It does not limit the effect to that tenant.

The UI no longer calls these routes. Settings uses /organization/members/:memberId/{role,status} (membership service, main.go around l.2545). useUsers() in frontend/src/features/settings/adminData.ts:24 is imported nowhere. The routes can still be reached by any admin API caller.

Per CLAUDE.md rule 2, a write whose effect crosses the tenant boundary is a P0 security defect.

Found during the direction audit of 2026-09-25 (code read, not yet reproduced live; criterion 2 is the reproduction).

Acceptance criteria

  1. PATCH /users/:id/status, PATCH /users/:id/role and DELETE /users/:id are removed from the router. If a live consumer turns up, they are reimplemented as membership operations on the caller's tenant only. The PR records which of the two was done.
  2. An integration test takes a user who is an active member of tenants A and B. After an admin of A deactivates or removes them, the user can still sign in to B and their B membership (role, status) is unchanged.
  3. No authorization decision reads users.role_id. Admin checks go through RequirePermission or the membership role. A test proves that setting users.role_id = admin grants nothing.
  4. The dead useUsers() hook and the AdminUser type are removed from frontend/src/features/settings/adminData.ts, and npm run type-check is green.
  5. CLAUDE.md rule 4 tests: Success, NotFound (a foreign or unknown member gets a 404 with the same body either way) and Unauthorized (a non-admin gets a 403).
  6. docs/openapi.yaml no longer lists the removed routes, and internal/handler/authz_route_coverage_test.go is updated to match.

Definition of Done

No request made in the context of tenant A can change whether a person can sign in to tenant B, or what they can do there. The cross-tenant test output is pasted on this issue.

Out of scope

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:backendGo, /internal, /pkgarea:securityCybersecurity and threat intelligencepriority:P0Blocking: nothing else ships until this closespriority:P0-criticalProduction broken or exposed — work nowstatus:readyMeets the ready definitiontier:0-trustTrust: security, isolation, evidence integritytrustEvidence a buyer's CISO tests before features mattertype:securitySecurity defect or hardening

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions