security(users): stop /users/:id writes and login from crossing the tenant boundary (#807)

[alex-dembele]

Closes #807

What changed

Criterion 1: removed, not reimplemented. PATCH /users/:id/status, PATCH /users/:id/role and DELETE /users/:id are gone from cmd/server/main.go, along with their handlers in internal/handler/user_handler.go. No live consumer exists. Settings uses /organization/members/:memberId/{role,status}, and the only client, useUsers(), was imported nowhere.

A second route to the same lockout, fixed in login. Sign-in only ever tried the user's default organization. If that organization deactivated or revoked the membership, sign-in was refused, even when another organization still granted access. So an admin of A could still lock someone out of B through the legitimate membership route, just by A being their default. internal/application/auth/login.go now falls back to the earliest-joined active membership in another active organization. When there is none, the refusal is unchanged.

Criterion 3. POST /users dropped its in-handler users.role_id check. The RequireRole("admin") guard already authorizes it from the session's role in the active organization. GET /users had already been cleaned the same way.

Criterion 4. useUsers() and AdminUser were removed from frontend/src/features/settings/adminData.ts.

Criterion 6. docs/openapi.yaml never listed the three routes (it has only /users/me, /users/me/avatar and /users/{id}/avatar), so it needed no change. authz_route_coverage_test.go gains TestProtectedRoutes_GlobalAccountWritesStayRemoved, which fails if any of the three is mounted again under any guard. I checked that it fails against the old main.go.

Verification

Cross-tenant test (criterion 2) and the users.role_id test (criterion 3). This is the real membership route, the real login use case and the same database:

$ go test ./internal/handler/ -run 'TestCrossTenant_' -count=1 -v
=== RUN   TestCrossTenant_AdminOfAWithdrawingAccessLeavesBUntouched/deactivated
    user_cross_tenant_test.go:167: after A set deactivated: sign-in lands in B (577b0845-…) with org_roles=map[577b0845-…:admin]
=== RUN   TestCrossTenant_AdminOfAWithdrawingAccessLeavesBUntouched/revoked
    user_cross_tenant_test.go:167: after A set revoked: sign-in lands in B (47fe8861-…) with org_roles=map[47fe8861-…:admin]
--- PASS: TestCrossTenant_AdminOfAWithdrawingAccessLeavesBUntouched (0.20s)
--- PASS: TestCrossTenant_SoleMembershipWithdrawnStillRefusesSignIn (0.12s)
--- PASS: TestCrossTenant_GlobalRoleIDGrantsNothing (0.15s)
ok  	github.com/opendefender/openrisk/internal/handler	0.515s

With the login fix reverted, the same test fails with sign-in of dual@both.io refused: your access to this organization has been revoked.

Wider suite:

$ go test ./internal/handler/... ./internal/application/... ./internal/middleware/... ./cmd/... -count=1
ok  internal/handler 22.4s · ok internal/handler/auth · ok internal/application/auth 12.3s · ok internal/application/membership
ok  internal/middleware · ok cmd/server · (all 33 application packages ok)

Live run on Postgres 16 (throwaway containers). One person is user in A (their default) and root in B.

On master, with an A admin whose users.role_id points at a role named admin:

PATCH /users/:id/status {is_active:false}  -> 200 {"message":"User status updated"}
dual signs in                              -> {"error":"Authentication failed"}
users.is_active=false · OpenDefender | user | active · Org B Corp | root | active
PATCH /users/:id/role {role:admin}         -> 200 {"message":"User role updated"}

On this branch:

PATCH  /users/:id/status  -> 404
PATCH  /users/:id/role    -> 404
DELETE /users/:id         -> 404
PUT /organization/members/:id/status {deactivated}  -> {"status":"deactivated","is_active":false}
dual signs in -> {"org":"Org B Corp"}  token {"tenant_id":"18bcb883-…","org_roles":{"18bcb883-…":"root"}}
users.is_active=true · OpenDefender | user | deactivated · Org B Corp | root | active

Frontend: npx eslint and npx prettier --check are clean on adminData.ts.

Honest remainders

• Criterion 4, npm run type-check, is not green, and the cause is on master, not this branch. Master itself fails on two missing imports from the Premium visual overhaul — RareUI + transitions.dev #751 merges: MembersView.tsx(487,30): Cannot find name 'DeleteButton' and CreateRiskModal.tsx(241,16): Cannot find name 'ScrollProgress'. With this PR's change stashed, the same two errors appear. This PR adds none. I did not fix them here because they belong to Premium visual overhaul — RareUI + transitions.dev #751.
• Exploitability on a stock install. The seeded legacy role is named Admin, and the handlers compared against admin. So on a default install these routes answered 403, or 500 for the seeded root admin (nil Role). The defect was reachable wherever a lowercase admin role row exists, as shown above. It was latent, not dead.
• Criterion 5. The three removed routes have no use case left to test. The surviving membership route already has Success, NotFound (a foreign id and an invented id give byte-identical 404s) and Unauthorized (a plain member gets 403) in organization_member_e2e_test.go. The login fallback adds TestLoginFallback_Success_…, _NotFound_… and _Unauthorized_….
• Criterion 3, /teams. The seven /teams handlers still contain a users.role_id check. It is never reached today, because they resolve the caller by token id and 404 first. Removing it would switch on a feature nobody has reviewed for tenant isolation. Tracked in security(teams): /teams handlers still read the global users.role_id and resolve the caller by token id #830.
• Sessions in B. Withdrawing access in A still revokes the person's refresh tokens in every organization. They can sign straight back in to B, so the DoD holds, but an action in A does sign them out of B. Tracked in security(membership): withdrawing access in one organization revokes the person's sessions in every organization #831. It needs a tech-lead review because it touches session revocation.
• Stages skipped: no tech-lead design pass (no structural change), and no separate devsecops agent run. The security reasoning and the live reproduction are above.

[alex-dembele]

Update: added a fix for TestNoStaleDecisions (a stale isolation-registry entry for the removed /users/{id} routes). Criterion 4 (type-check) is fixed on master by #843. All four PRs (#832, #843, #844, #846) merge together without conflicts, and the combined tests pass. Details are on #807.