feat(users): deactivate members, and make the membership the authority - #534
Merged
Conversation
Adds the primitive that removes a person's access to a workspace in one action — dashboard sessions, MCP OAuth tokens, MCP API keys and the membership itself — and an admin Deactivate/Reactivate action on top of it. SCIM `active: false` will call the same primitive, so the directory and the admin can never disagree about what "deactivated" means. `OrganizationMember.deactivatedAt` marks the state. The row is kept rather than deleted so that role and joinedAt survive a reactivation and the member list can say "deactivated" instead of making a leaver indistinguishable from someone who was never here. Reactivation restores the membership only: revoked keys stay revoked and old sessions stay dead. Four pre-existing authorization defects made the primitive untrustworthy and are fixed here: - PUT /api/users/:id/role wrote `users.role` — the active-org cache — and never `organization_members.role`, which is what tool authorization reads. A demoted admin therefore kept UNRESTRICTED MCP tools. Role changes now write the membership, refresh the cache, and revoke sessions on a demotion. - Nothing guarded the last admin: an admin could delete or demote the only other admin. A shared `assertNotLastAdmin` (active admins only) now guards demotion, removal and deactivation. - GET /api/users listed from the cache column, so a multi-workspace member was invisible to every workspace's admins but one. It now lists memberships. DELETE /api/users/:id deleted the GLOBAL user row, letting one workspace's admin destroy someone's access everywhere; it now removes only this membership when the user belongs to other workspaces. - JwtStrategy resolved the role from the cache and only self-healed a NULL active org, so a removed or deactivated member kept a working session. It now reads the active membership, repoints to another active one, and fails closed with 401 when none remains. The same fail-closed rule is applied in getAllowedToolIds (no org → no tools), the global /mcp listing, MCP API key resolution, per-server tenant checks and SSO membership checks. The migration is additive and reconciles cache/membership role drift before the code starts treating the membership as authoritative. Production was audited first: 0 of 1380 cloud users have an active org without a membership, 0 role mismatches, 0 orphaned keys. Verified end to end on the local stack: an MCP key, an old JWT and a fresh login all answer 401 after deactivation; the member list shows Deactivated; reactivation restores sign-in while the old key stays revoked; deactivating an admin with another active admin succeeds and their JWT dies; demoting an admin writes membership.role and kills their pre-demotion session. Through the UI: status pill, Deactivate/Reactivate actions and the reactivation audit row.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
First of the SCIM series. Adds the primitive that removes a person's access to a workspace in one action, an admin Deactivate / Reactivate on top of it, and fixes four pre-existing authorization defects that made such a primitive untrustworthy. Cloud-relevant: none of this is SSO-gated.
The primitive
UserLifecycleService.deactivateInOrganizationcloses every path at once — dashboard sessions, MCP OAuth tokens, MCP API keys, and the membership — because each is checked by different code and a leaver who keeps any one of them has not left. SCIMactive: false(next PR) calls the same method, so the directory and the admin can never disagree about what "deactivated" means.OrganizationMember.deactivatedAtmarks the state. The row is kept: role andjoinedAtsurvive reactivation, and the member list can say "Deactivated" instead of making a leaver indistinguishable from someone who was never here. Reactivation restores the membership only — revoked keys stay revoked, old sessions stay dead.Last admin: an admin-initiated deactivation of the only active admin is refused before any write. A directory-initiated one (SCIM) revokes sessions and keys but keeps the membership, so the workspace keeps one way back in; audited as
LAST_ADMIN_PROTECTION_TRIGGERED.Defects fixed
PUT /api/users/:id/roleusers.role(the cache) only;organization_members.roleuntouched → a demoted admin kept unrestricted MCP toolsassertNotLastAdmin(active admins only) on demotion, removal, deactivationGET /api/usersactive/deactivatedAtDELETE /api/users/:idJwtStrategyNo active workspaceThe same fail-closed rule now applies in
getAllowedToolIds(no org → no tools; deactivated → no tools), the global/mcplisting, MCP API key resolution, per-server tenant checks and SSO membership checks.Migration
Additive nullable column plus a reconcile of cache/membership role drift, run before the code treats the membership as authoritative. Production audited first: 0 of 1380 cloud users have an active org without a membership row, 0 role mismatches, 0 orphaned keys.
Verified
End to end on the local stack:
Through the UI: status pill, Deactivate/Reactivate actions (both users pages), reactivation audit row. 3889 tests pass.
Behaviour changes to note in the release
GET /api/usersreturns memberships (multi-org members now visible);DELETE /api/users/:idremoves the membership rather than the account when the user is in other workspaces.