From eec7339cf76e8daa3445e09a62daa6c47c7cc214 Mon Sep 17 00:00:00 2001 From: Matteo Date: Wed, 9 Sep 2026 10:43:46 +0200 Subject: [PATCH] feat(users): deactivate members, and make the membership the authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../migration.sql | 15 ++ packages/backend/prisma/schema.prisma | 6 + .../src/audit/security-event.service.ts | 5 + packages/backend/src/auth/auth.controller.ts | 17 +- .../backend/src/auth/jwt.strategy.spec.ts | 49 ++++ packages/backend/src/auth/jwt.strategy.ts | 54 +++- .../src/auth/sso-enforcement.service.ts | 4 +- .../src/ee/cloud/onboarding-cron.service.ts | 2 +- .../src/identity-providers/sso.service.ts | 2 +- .../src/mcp-server/mcp-endpoint.controller.ts | 9 +- .../src/mcp-servers/mcp-servers.service.ts | 2 +- .../src/organizations/last-admin.exception.ts | 20 ++ .../organizations.service.spec.ts | 167 ++++++++++++ .../organizations/organizations.service.ts | 156 +++++++++-- .../src/roles/mcp-api-keys.service.spec.ts | 34 +++ .../backend/src/roles/mcp-api-keys.service.ts | 11 + .../backend/src/roles/roles.service.spec.ts | 46 ++-- packages/backend/src/roles/roles.service.ts | 30 ++- .../src/users/user-lifecycle.service.spec.ts | 212 +++++++++++++++ .../src/users/user-lifecycle.service.ts | 251 ++++++++++++++++++ .../backend/src/users/users.controller.ts | 112 +++++++- packages/backend/src/users/users.module.ts | 7 +- .../backend/src/users/users.service.spec.ts | 103 +++++-- packages/backend/src/users/users.service.ts | 123 +++++++-- .../frontend/src/app/admin/users/page.tsx | 67 ++++- .../frontend/src/app/settings/users/page.tsx | 62 ++++- packages/frontend/src/lib/api.ts | 6 + 27 files changed, 1424 insertions(+), 148 deletions(-) create mode 100644 packages/backend/prisma/migrations/20260909083050_add_member_deactivation/migration.sql create mode 100644 packages/backend/src/organizations/last-admin.exception.ts create mode 100644 packages/backend/src/organizations/organizations.service.spec.ts create mode 100644 packages/backend/src/users/user-lifecycle.service.spec.ts create mode 100644 packages/backend/src/users/user-lifecycle.service.ts diff --git a/packages/backend/prisma/migrations/20260909083050_add_member_deactivation/migration.sql b/packages/backend/prisma/migrations/20260909083050_add_member_deactivation/migration.sql new file mode 100644 index 00000000..7e3967cd --- /dev/null +++ b/packages/backend/prisma/migrations/20260909083050_add_member_deactivation/migration.sql @@ -0,0 +1,15 @@ +-- AlterTable +ALTER TABLE "organization_members" ADD COLUMN "deactivated_at" TIMESTAMP(3); + +-- Reconcile role drift BEFORE the application starts reading the membership +-- role as authoritative. Until this release, PUT /api/users/:id/role wrote +-- users.role only, so for a user's ACTIVE organization the cached column holds +-- the admin's intent and the membership row may be stale. Role sync — the one +-- writer that updates the membership only — is self-hosted-only and re-asserts +-- itself on the next SSO sign-in, so "cache wins" is safe there too. +UPDATE "organization_members" m +SET "role" = u."role" +FROM "users" u +WHERE m."user_id" = u."id" + AND m."organization_id" = u."organization_id" + AND m."role" <> u."role"; diff --git a/packages/backend/prisma/schema.prisma b/packages/backend/prisma/schema.prisma index c16f8356..a61acfd7 100644 --- a/packages/backend/prisma/schema.prisma +++ b/packages/backend/prisma/schema.prisma @@ -58,6 +58,12 @@ model OrganizationMember { organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) role UserRole @default(EDITOR) joinedAt DateTime @default(now()) @map("joined_at") + /// null = active. Set by `UserLifecycleService.deactivateInOrganization` + /// (an admin's Deactivate, or `active: false` pushed by SCIM). The row is + /// KEPT rather than deleted so that role and joinedAt survive a + /// reactivation, and so the member list can say "deactivated" instead of + /// making a leaver indistinguishable from someone who was never here. + deactivatedAt DateTime? @map("deactivated_at") @@unique([userId, organizationId]) @@index([userId]) diff --git a/packages/backend/src/audit/security-event.service.ts b/packages/backend/src/audit/security-event.service.ts index e22c5844..f88a3e62 100644 --- a/packages/backend/src/audit/security-event.service.ts +++ b/packages/backend/src/audit/security-event.service.ts @@ -31,6 +31,11 @@ export const SecurityEvents = { ROLE_CHANGED: 'ROLE_CHANGED', LAST_ADMIN_PROTECTION_TRIGGERED: 'LAST_ADMIN_PROTECTION_TRIGGERED', MEMBERSHIP_REMOVED_BY_SYNC: 'MEMBERSHIP_REMOVED_BY_SYNC', + /** An admin or a directory push removed this member's access to one workspace. */ + USER_DEACTIVATED: 'USER_DEACTIVATED', + USER_REACTIVATED: 'USER_REACTIVATED', + /** An admin removed a multi-workspace user from ONE workspace (account kept). */ + MEMBERSHIP_REMOVED: 'MEMBERSHIP_REMOVED', /** A sign-in rewrote the user's roles from the directory's claims. */ ROLE_SYNC_APPLIED: 'ROLE_SYNC_APPLIED', /** Claims were incomplete, so roles were deliberately left untouched. */ diff --git a/packages/backend/src/auth/auth.controller.ts b/packages/backend/src/auth/auth.controller.ts index 9200ce20..6dbc6a72 100644 --- a/packages/backend/src/auth/auth.controller.ts +++ b/packages/backend/src/auth/auth.controller.ts @@ -570,7 +570,14 @@ export class AuthController { if (existing) { const membership = await this.organizationsService.getMembership(existing.id, req.user.organizationId); if (membership) { - throw new ConflictException('This user is already a member of your organization'); + // A deactivated member is still a member: inviting them again must + // not become a back door that re-creates access. Reactivation is an + // explicit action under Users. + throw new ConflictException( + membership.deactivatedAt + ? 'This user is deactivated in your organization. Reactivate them from Users instead of inviting them again.' + : 'This user is already a member of your organization', + ); } // User exists but not in this org — allow invitation for multi-org membership } @@ -697,7 +704,13 @@ export class AuthController { // Existing user — add to the new organization (multi-org) const alreadyMember = await this.organizationsService.getMembership(existing.id, invite.organizationId); if (alreadyMember) { - throw new ConflictException('You are already a member of this organization'); + // Includes deactivated memberships: accepting an invite must never + // silently reactivate someone an admin or the directory removed. + throw new ConflictException( + alreadyMember.deactivatedAt + ? 'Your access to this organization was deactivated. Ask an administrator to reactivate it.' + : 'You are already a member of this organization', + ); } await this.organizationsService.addMember(existing.id, invite.organizationId, invite.role); // Switch their active org to the newly joined one diff --git a/packages/backend/src/auth/jwt.strategy.spec.ts b/packages/backend/src/auth/jwt.strategy.spec.ts index b05af070..4fba0dba 100644 --- a/packages/backend/src/auth/jwt.strategy.spec.ts +++ b/packages/backend/src/auth/jwt.strategy.spec.ts @@ -18,11 +18,15 @@ describe('JwtStrategy', () => { let strategy: JwtStrategy; let prisma: { user: { findUnique: jest.Mock; update: jest.Mock }; organizationMember: { findFirst: jest.Mock } }; + // The strategy now loads the user WITH their active memberships; the + // membership is the authoritative role. const dbUser = { id: 'user-cuid-1', email: 'alice@example.com', role: 'EDITOR', organizationId: 'org-1', + sessionsValidFrom: null, + memberships: [{ organizationId: 'org-1', role: 'EDITOR' }], }; beforeEach(() => { @@ -172,4 +176,49 @@ describe('JwtStrategy', () => { ).rejects.toThrow('User no longer exists'); }); }); + + describe('membership is authoritative', () => { + const dashboard = { sub: 'user-cuid-1', email: 'alice@example.com', tokenUse: 'dashboard' } as any; + + it('returns the MEMBERSHIP role and repairs a stale cache', async () => { + prisma.user.findUnique.mockResolvedValue({ + ...dbUser, + role: 'ADMIN', // stale cache + memberships: [{ organizationId: 'org-1', role: 'VIEWER' }], + }); + const result = await strategy.validate(dashboard); + expect(result.role).toBe('VIEWER'); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'user-cuid-1' }, + data: { role: 'VIEWER' }, + }); + }); + + // A deactivated member must not keep a working session for that workspace. + it('repoints to another ACTIVE membership when the cached one is deactivated', async () => { + prisma.user.findUnique.mockResolvedValue({ + ...dbUser, + organizationId: 'org-1', + // org-1 is absent from the active list: deactivated there + memberships: [{ organizationId: 'org-2', role: 'VIEWER' }], + }); + const result = await strategy.validate(dashboard); + expect(result).toMatchObject({ organizationId: 'org-2', role: 'VIEWER' }); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'user-cuid-1' }, + data: { organizationId: 'org-2', role: 'VIEWER' }, + }); + }); + + it('fails closed when no active membership remains', async () => { + prisma.user.findUnique.mockResolvedValue({ ...dbUser, memberships: [] }); + await expect(strategy.validate(dashboard)).rejects.toThrow('No active workspace'); + }); + + it('only queries ACTIVE memberships', async () => { + await strategy.validate(dashboard); + const arg = prisma.user.findUnique.mock.calls[0][0]; + expect(arg.select.memberships.where).toEqual({ deactivatedAt: null }); + }); + }); }); diff --git a/packages/backend/src/auth/jwt.strategy.ts b/packages/backend/src/auth/jwt.strategy.ts index d5c2bb38..fe4e803a 100644 --- a/packages/backend/src/auth/jwt.strategy.ts +++ b/packages/backend/src/auth/jwt.strategy.ts @@ -31,37 +31,63 @@ export class JwtStrategy extends PassportStrategy(Strategy) { throw new UnauthorizedException('Token is not valid for this API'); } + // One query: the user row plus their ACTIVE memberships. The membership + // is the authoritative role and the only proof the user may still act in + // the organization; `users.role` / `users.organizationId` are caches. const user = await this.prisma.user.findUnique({ where: { id: payload.sub }, + select: { + id: true, + email: true, + role: true, + organizationId: true, + sessionsValidFrom: true, + memberships: { + where: { deactivatedAt: null }, + orderBy: { joinedAt: 'asc' }, + select: { organizationId: true, role: true }, + }, + }, }); if (!user) { throw new UnauthorizedException('User no longer exists'); } // Revocation: refuse tokens minted before the user's cutover instant (set - // on password change, demotion, deprovisioning or an SSO config change). - // Free to check — the user row is already loaded. + // on password change, demotion and deactivation). Free to check — the + // user row is already loaded. if (isTokenRevoked(payload, user.sessionsValidFrom)) { throw new UnauthorizedException('Session has been revoked'); } - // Self-heal a NULL active org by snapping to the oldest remaining membership. - // This happens when an organization the user was active in was deleted while - // they were a member of others — schema's onDelete:SetNull leaves them dangling. - if (user.organizationId === null) { - const fallback = await this.prisma.organizationMember.findFirst({ - where: { userId: user.id }, - orderBy: { joinedAt: 'asc' }, - }); - if (fallback) { + const active = user.organizationId + ? user.memberships.find((m) => m.organizationId === user.organizationId) + : undefined; + + if (active) { + // Repair cache drift in place. Rare: only after a role sync wrote the + // membership, or a pre-fix role change wrote the cache alone. + if (user.role !== active.role) { await this.prisma.user.update({ where: { id: user.id }, - data: { organizationId: fallback.organizationId, role: fallback.role }, + data: { role: active.role }, }); - return { sub: user.id, email: user.email, role: fallback.role, organizationId: fallback.organizationId }; } + return { sub: user.id, email: user.email, role: active.role, organizationId: active.organizationId }; } - return { sub: user.id, email: user.email, role: user.role, organizationId: user.organizationId }; + // The cached org is null, deleted, or the membership in it was deactivated: + // snap to the oldest remaining ACTIVE membership. Nothing left means the + // user may not act anywhere — fail closed rather than serve a workspace + // they were removed from. + const fallback = user.memberships[0]; + if (!fallback) { + throw new UnauthorizedException('No active workspace'); + } + await this.prisma.user.update({ + where: { id: user.id }, + data: { organizationId: fallback.organizationId, role: fallback.role }, + }); + return { sub: user.id, email: user.email, role: fallback.role, organizationId: fallback.organizationId }; } } diff --git a/packages/backend/src/auth/sso-enforcement.service.ts b/packages/backend/src/auth/sso-enforcement.service.ts index e4081ab8..d29c5a5a 100644 --- a/packages/backend/src/auth/sso-enforcement.service.ts +++ b/packages/backend/src/auth/sso-enforcement.service.ts @@ -25,8 +25,10 @@ export class SsoEnforcementService { // password sign-in in the deployment. if (this.deployment.isCloud()) return []; + // Deactivated memberships do not count: an inert membership should + // neither grant access nor constrain how the user signs in elsewhere. const memberships = await this.prisma.organizationMember.findMany({ - where: { userId }, + where: { userId, deactivatedAt: null }, select: { organizationId: true }, }); if (memberships.length === 0) return []; diff --git a/packages/backend/src/ee/cloud/onboarding-cron.service.ts b/packages/backend/src/ee/cloud/onboarding-cron.service.ts index fa901a4d..dbb9c43d 100644 --- a/packages/backend/src/ee/cloud/onboarding-cron.service.ts +++ b/packages/backend/src/ee/cloud/onboarding-cron.service.ts @@ -213,7 +213,7 @@ export class OnboardingCronService { // Recipients: org admins (authoritative membership). const admins = await this.prisma.organizationMember.findMany({ - where: { organizationId, role: 'ADMIN' }, + where: { organizationId, role: 'ADMIN', deactivatedAt: null }, select: { user: { select: { email: true, name: true } } }, }); if (admins.length === 0) { diff --git a/packages/backend/src/identity-providers/sso.service.ts b/packages/backend/src/identity-providers/sso.service.ts index 51768018..e554562c 100644 --- a/packages/backend/src/identity-providers/sso.service.ts +++ b/packages/backend/src/identity-providers/sso.service.ts @@ -729,7 +729,7 @@ export class SsoService { /** Fails closed: a non-member is treated exactly like an unknown provider. */ private async assertMember(userId: string, organizationId: string) { const member = await this.prisma.organizationMember.findFirst({ - where: { userId, organizationId }, + where: { userId, organizationId, deactivatedAt: null }, select: { id: true }, }); if (!member) throw new SsoError('not_a_member'); diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index 553e1c42..7e64c162 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -166,7 +166,14 @@ export class McpEndpointController { // explicitly enabled anonymous mode. Both are operator credentials on a // single-tenant self-hosted box, so the pre-existing "everything" answer // is the correct one and narrowing it here would break those deployments. - if (!user?.sub || !user.organizationId) return null; + if (!user?.sub) return null; + + // An identified user with NO organization is a deactivated one whose + // active org was cleared — not an operator. They see nothing. + if (!user.organizationId) { + user.roles = Array.isArray(user.roles) ? user.roles : []; + return new Set(); + } const orgTools = this.toolRegistry .getAllTools() diff --git a/packages/backend/src/mcp-servers/mcp-servers.service.ts b/packages/backend/src/mcp-servers/mcp-servers.service.ts index 9ee29f2c..3665e60d 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.service.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.service.ts @@ -53,7 +53,7 @@ export class McpServersService { ): Promise { if (!userId || !organizationId) return false; const count = await this.prisma.organizationMember.count({ - where: { userId, organizationId }, + where: { userId, organizationId, deactivatedAt: null }, }); return count > 0; } diff --git a/packages/backend/src/organizations/last-admin.exception.ts b/packages/backend/src/organizations/last-admin.exception.ts new file mode 100644 index 00000000..fbb22894 --- /dev/null +++ b/packages/backend/src/organizations/last-admin.exception.ts @@ -0,0 +1,20 @@ +import { ConflictException } from '@nestjs/common'; + +/** + * Thrown when an action would leave an organization with no ACTIVE admin. + * + * Lives in its own file so that `UsersModule` and `IdentityProvidersModule` + * can both import it without pulling in `OrganizationsService` and creating a + * module cycle. The `code` lets non-JSON callers (SCIM) map it to their own + * error shape. + */ +export class LastAdminConflictException extends ConflictException { + constructor(organizationId: string) { + super({ + code: 'LAST_ADMIN', + error: + 'Cannot remove the only administrator of this organization. Promote another member first.', + organizationId, + }); + } +} diff --git a/packages/backend/src/organizations/organizations.service.spec.ts b/packages/backend/src/organizations/organizations.service.spec.ts new file mode 100644 index 00000000..c36f8ee4 --- /dev/null +++ b/packages/backend/src/organizations/organizations.service.spec.ts @@ -0,0 +1,167 @@ +import { ForbiddenException } from '@nestjs/common'; +import { OrganizationsService } from './organizations.service'; +import { LastAdminConflictException } from './last-admin.exception'; +import { SecurityEventService } from '../audit/security-event.service'; +import { PrismaService } from '../common/prisma.service'; + +const ORG = 'org-1'; + +describe('OrganizationsService', () => { + let prisma: any; + let events: any[]; + let service: OrganizationsService; + const ctx = { actorUserId: 'admin-1', ip: '127.0.0.1', userAgent: 'jest' }; + + beforeEach(() => { + events = []; + prisma = { + organizationMember: { + findUnique: jest.fn(), + findMany: jest.fn(async () => []), + findFirst: jest.fn(async () => null), + count: jest.fn(async () => 0), + update: jest.fn(async () => ({})), + delete: jest.fn(async () => ({})), + }, + user: { + findUnique: jest.fn(async () => ({ organizationId: ORG })), + update: jest.fn(async () => ({})), + }, + securityEvent: { + create: jest.fn(async (args: any) => { + events.push(args.data); + return args.data; + }), + }, + $transaction: jest.fn((fn: any) => fn(prisma)), + }; + service = new OrganizationsService( + prisma, + new SecurityEventService(prisma as unknown as PrismaService), + ); + }); + + describe('assertNotLastAdmin', () => { + it('is a no-op for a non-admin or a deactivated admin', async () => { + prisma.organizationMember.findUnique.mockResolvedValueOnce({ role: 'EDITOR', deactivatedAt: null }); + await expect(service.assertNotLastAdmin(ORG, 'u1')).resolves.toBeUndefined(); + prisma.organizationMember.findUnique.mockResolvedValueOnce({ role: 'ADMIN', deactivatedAt: new Date() }); + await expect(service.assertNotLastAdmin(ORG, 'u1')).resolves.toBeUndefined(); + expect(prisma.organizationMember.count).not.toHaveBeenCalled(); + }); + + it('throws when no other ACTIVE admin exists', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: null }); + prisma.organizationMember.count.mockResolvedValue(0); + await expect(service.assertNotLastAdmin(ORG, 'u1')).rejects.toBeInstanceOf(LastAdminConflictException); + // A deactivated co-admin must not count as "another admin". + expect(prisma.organizationMember.count).toHaveBeenCalledWith({ + where: { organizationId: ORG, role: 'ADMIN', deactivatedAt: null, userId: { not: 'u1' } }, + }); + }); + + it('passes when another active admin exists, and honours a tx client', async () => { + const tx = { + organizationMember: { + findUnique: jest.fn(async () => ({ role: 'ADMIN', deactivatedAt: null })), + count: jest.fn(async () => 1), + }, + }; + await expect(service.assertNotLastAdmin(ORG, 'u1', tx as any)).resolves.toBeUndefined(); + expect(prisma.organizationMember.findUnique).not.toHaveBeenCalled(); + }); + }); + + describe('updateMemberRole', () => { + it('returns null for a non-member', async () => { + prisma.organizationMember.findUnique.mockResolvedValue(null); + expect(await service.updateMemberRole('u1', ORG, 'VIEWER', ctx)).toBeNull(); + expect(prisma.$transaction).not.toHaveBeenCalled(); + }); + + it('writes nothing when the role is unchanged', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: null }); + expect(await service.updateMemberRole('u1', ORG, 'EDITOR', ctx)).toEqual({ + from: 'EDITOR', + to: 'EDITOR', + sessionsRevoked: false, + }); + expect(prisma.$transaction).not.toHaveBeenCalled(); + expect(events).toHaveLength(0); + }); + + // The defect this fixes: only the users.role cache was written, so a + // demoted admin kept membership.role = ADMIN and unrestricted MCP tools. + it('a demotion writes the MEMBERSHIP, refreshes the cache and revokes sessions', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: null }); + prisma.organizationMember.count.mockResolvedValue(1); // another admin exists + const result = await service.updateMemberRole('u1', ORG, 'VIEWER', ctx); + expect(result).toEqual({ from: 'ADMIN', to: 'VIEWER', sessionsRevoked: true }); + expect(prisma.organizationMember.update).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: 'u1', organizationId: ORG } }, + data: { role: 'VIEWER' }, + }); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'u1' }, + data: { role: 'VIEWER', sessionsValidFrom: expect.any(Date) }, + }); + expect(events.map((e) => e.event)).toEqual(['ROLE_CHANGED', 'SESSIONS_REVOKED']); + expect(events[0].metadata).toEqual({ from: 'ADMIN', to: 'VIEWER', via: 'admin' }); + }); + + it('a promotion revokes nothing', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'VIEWER', deactivatedAt: null }); + const result = await service.updateMemberRole('u1', ORG, 'ADMIN', ctx); + expect(result?.sessionsRevoked).toBe(false); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'u1' }, + data: { role: 'ADMIN' }, + }); + expect(events.map((e) => e.event)).toEqual(['ROLE_CHANGED']); + }); + + it('refreshes the cache only when this is the active organization', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'VIEWER', deactivatedAt: null }); + prisma.user.findUnique.mockResolvedValue({ organizationId: 'org-other' }); + await service.updateMemberRole('u1', ORG, 'EDITOR', ctx); + expect(prisma.user.update).toHaveBeenCalledWith({ where: { id: 'u1' }, data: {} }); + }); + + it('refuses to demote the last active admin', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: null }); + prisma.organizationMember.count.mockResolvedValue(0); + await expect(service.updateMemberRole('u1', ORG, 'EDITOR', ctx)).rejects.toBeInstanceOf( + LastAdminConflictException, + ); + expect(prisma.organizationMember.update).not.toHaveBeenCalled(); + }); + }); + + describe('switchOrg / listUserOrgs', () => { + it('refuses to switch into a deactivated membership', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'VIEWER', deactivatedAt: new Date() }); + await expect(service.switchOrg('u1', ORG)).rejects.toBeInstanceOf(ForbiddenException); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it('lists only active memberships', async () => { + await service.listUserOrgs('u1'); + expect(prisma.organizationMember.findMany).toHaveBeenCalledWith( + expect.objectContaining({ where: { userId: 'u1', deactivatedAt: null } }), + ); + }); + }); + + describe('removeMember', () => { + it('points the cache at nothing when no active membership remains', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'VIEWER', deactivatedAt: null }); + prisma.user.findUnique.mockResolvedValue({ organizationId: ORG }); + prisma.organizationMember.findFirst.mockResolvedValue(null); + await service.removeMember('u1', ORG); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: 'u1' }, + data: { organizationId: null }, + }); + }); + }); +}); diff --git a/packages/backend/src/organizations/organizations.service.ts b/packages/backend/src/organizations/organizations.service.ts index bb2e50c6..d3f38a36 100644 --- a/packages/backend/src/organizations/organizations.service.ts +++ b/packages/backend/src/organizations/organizations.service.ts @@ -1,10 +1,24 @@ import { Injectable, ForbiddenException, NotFoundException, BadRequestException } from '@nestjs/common'; import { PrismaService } from '../common/prisma.service'; -import { UserRole } from '../generated/prisma/client'; +import { Prisma, UserRole } from '../generated/prisma/client'; +import { SecurityEventService, SecurityEvents } from '../audit/security-event.service'; +import { LastAdminConflictException } from './last-admin.exception'; + +/** Ordered least- to most-privileged; used to tell a demotion from a promotion. */ +const ORG_ROLE_RANK: Record = { VIEWER: 1, EDITOR: 2, ADMIN: 3 }; + +export interface MemberActionContext { + actorUserId: string; + ip?: string | null; + userAgent?: string | null; +} @Injectable() export class OrganizationsService { - constructor(private readonly prisma: PrismaService) {} + constructor( + private readonly prisma: PrismaService, + private readonly securityEvents: SecurityEventService, + ) {} async create(name: string) { return this.prisma.organization.create({ @@ -21,8 +35,11 @@ export class OrganizationsService { } async listUserOrgs(userId: string) { + // Deactivated memberships are not offered as switch targets: the switch + // would be refused anyway, and listing them would advertise a workspace + // the user can no longer enter. const memberships = await this.prisma.organizationMember.findMany({ - where: { userId }, + where: { userId, deactivatedAt: null }, include: { organization: { select: { id: true, name: true, createdAt: true } }, }, @@ -45,7 +62,7 @@ export class OrganizationsService { async switchOrg(userId: string, organizationId: string) { const membership = await this.getMembership(userId, organizationId); - if (!membership) { + if (!membership || membership.deactivatedAt) { throw new ForbiddenException('Not a member of this organization'); } @@ -62,40 +79,137 @@ export class OrganizationsService { }); } + /** + * Removes ONE membership. Refuses to remove the last active admin. + * + * Callers wanting "the user loses access" should prefer + * `UserLifecycleService.deactivateInOrganization`, which also revokes + * sessions and MCP keys; this is the destructive follow-up. + */ async removeMember(userId: string, organizationId: string) { + await this.assertNotLastAdmin(organizationId, userId); await this.prisma.organizationMember.delete({ where: { userId_organizationId: { userId, organizationId } }, }); - // If this was the user's active org, switch to another + // If this was the user's active org, point the cache at another ACTIVE + // membership — or at nothing. Leaving it at the removed org would keep a + // fully working dashboard session for a workspace the user is no longer in, + // because the JWT strategy reads the active org from this column. const user = await this.prisma.user.findUnique({ where: { id: userId } }); if (user?.organizationId === organizationId) { const remaining = await this.prisma.organizationMember.findFirst({ - where: { userId }, + where: { userId, deactivatedAt: null }, orderBy: { joinedAt: 'asc' }, }); - if (remaining) { - await this.switchOrg(userId, remaining.organizationId); - } + await this.prisma.user.update({ + where: { id: userId }, + data: remaining + ? { organizationId: remaining.organizationId, role: remaining.role } + : { organizationId: null }, + }); } } - async updateMemberRole(userId: string, organizationId: string, role: UserRole) { - const membership = await this.prisma.organizationMember.update({ + /** + * Throws `LastAdminConflictException` if `userId` is currently an ACTIVE + * admin of the organization and no other active admin exists. A no-op for + * anyone who is not an active admin. Accepts a transaction client so callers + * can hold the row inside their own transaction. + * + * Deactivated admins do not count: a workspace whose only other admin has + * been deactivated has, for every practical purpose, no other admin. + */ + async assertNotLastAdmin( + organizationId: string, + userId: string, + db: Prisma.TransactionClient | PrismaService = this.prisma, + ): Promise { + const target = await db.organizationMember.findUnique({ where: { userId_organizationId: { userId, organizationId } }, - data: { role }, + select: { role: true, deactivatedAt: true }, }); + if (!target || target.role !== 'ADMIN' || target.deactivatedAt) return; - // Sync cache if this is the user's active org - const user = await this.prisma.user.findUnique({ where: { id: userId } }); - if (user?.organizationId === organizationId) { - await this.prisma.user.update({ - where: { id: userId }, + const others = await db.organizationMember.count({ + where: { organizationId, role: 'ADMIN', deactivatedAt: null, userId: { not: userId } }, + }); + if (others === 0) throw new LastAdminConflictException(organizationId); + } + + /** + * Changes a member's role in ONE organization. + * + * Writes the membership row — the authoritative role — and refreshes the + * `users.role` cache only when this is the user's active organization. The + * previous implementation of the admin endpoint wrote the cache alone, which + * left a demoted admin with membership.role = ADMIN and therefore with + * unrestricted MCP tool access: `getAllowedToolIds` reads the membership. + * + * A DEMOTION also revokes every existing session (`sessionsValidFrom`): a + * token minted while the user was an admin must not keep admin power until + * it expires. A promotion only widens, so it revokes nothing. + * + * Returns null when the user is not a member (the controller's 404). + */ + async updateMemberRole( + userId: string, + organizationId: string, + role: UserRole, + ctx: MemberActionContext, + ): Promise<{ from: UserRole; to: UserRole; sessionsRevoked: boolean } | null> { + const membership = await this.getMembership(userId, organizationId); + if (!membership) return null; + const from = membership.role; + if (from === role) return { from, to: role, sessionsRevoked: false }; + + const demotion = ORG_ROLE_RANK[from] > ORG_ROLE_RANK[role]; + + await this.prisma.$transaction(async (tx) => { + if (from === 'ADMIN' && demotion) { + await this.assertNotLastAdmin(organizationId, userId, tx); + } + await tx.organizationMember.update({ + where: { userId_organizationId: { userId, organizationId } }, data: { role }, }); + const user = await tx.user.findUnique({ + where: { id: userId }, + select: { organizationId: true }, + }); + await tx.user.update({ + where: { id: userId }, + data: { + ...(user?.organizationId === organizationId ? { role } : {}), + ...(demotion ? { sessionsValidFrom: new Date() } : {}), + }, + }); + }); + + await this.securityEvents.log({ + event: SecurityEvents.ROLE_CHANGED, + actorType: 'USER', + organizationId, + actorUserId: ctx.actorUserId, + targetUserId: userId, + metadata: { from, to: role, via: 'admin' }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); + if (demotion) { + await this.securityEvents.log({ + event: SecurityEvents.SESSIONS_REVOKED, + actorType: 'USER', + organizationId, + actorUserId: ctx.actorUserId, + targetUserId: userId, + metadata: { reason: 'role_demotion', organizationId }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); } - return membership; + return { from, to: role, sessionsRevoked: demotion }; } async deleteOrganization( @@ -115,7 +229,7 @@ export class OrganizationsService { } const membership = await this.getMembership(userId, organizationId); - if (!membership || membership.role !== 'ADMIN') { + if (!membership || membership.role !== 'ADMIN' || membership.deactivatedAt) { throw new ForbiddenException('Only org admins can delete the organization'); } @@ -129,14 +243,14 @@ export class OrganizationsService { const nextByOrphan = new Map(); for (const o of orphans) { const m = await this.prisma.organizationMember.findFirst({ - where: { userId: o.id, organizationId: { not: organizationId } }, + where: { userId: o.id, organizationId: { not: organizationId }, deactivatedAt: null }, orderBy: { joinedAt: 'asc' }, }); nextByOrphan.set(o.id, m ? { organizationId: m.organizationId, role: m.role } : null); } const selfNext = await this.prisma.organizationMember.findFirst({ - where: { userId, organizationId: { not: organizationId } }, + where: { userId, organizationId: { not: organizationId }, deactivatedAt: null }, orderBy: { joinedAt: 'asc' }, }); diff --git a/packages/backend/src/roles/mcp-api-keys.service.spec.ts b/packages/backend/src/roles/mcp-api-keys.service.spec.ts index 6aef8646..c569916d 100644 --- a/packages/backend/src/roles/mcp-api-keys.service.spec.ts +++ b/packages/backend/src/roles/mcp-api-keys.service.spec.ts @@ -14,6 +14,9 @@ describe('McpApiKeysService', () => { deleteMany: jest.fn(), update: jest.fn(), }, + organizationMember: { + findUnique: jest.fn().mockResolvedValue({ deactivatedAt: null }), + }, }; service = new McpApiKeysService(mockPrisma); }); @@ -146,4 +149,35 @@ describe('McpApiKeysService', () => { expect(result!.id).toBe('u1'); }); }); + + describe('resolveUserByKey — membership guard', () => { + const record = { + id: 'k1', key: 'mcp_x', isActive: true, userId: 'u1', organizationId: 'org-1', name: 'n', mcpServerId: null, + user: { id: 'u1', email: 'a@x', role: 'VIEWER', organizationId: 'org-1', mcpRoleId: null }, + }; + + it('resolves a key whose owner is an active member', async () => { + mockPrisma.mcpApiKey.findUnique.mockResolvedValue(record); + mockPrisma.mcpApiKey.update.mockResolvedValue({}); + expect(await service.resolveUserByKey('mcp_x')).toMatchObject({ id: 'u1' }); + expect(mockPrisma.organizationMember.findUnique).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: 'u1', organizationId: 'org-1' } }, + select: { deactivatedAt: true }, + }); + }); + + // Defence in depth: even a key row that escaped deactivation must not + // authenticate a member who was deactivated in the key's organization. + it('refuses a key whose owner is deactivated in that organization', async () => { + mockPrisma.mcpApiKey.findUnique.mockResolvedValue(record); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ deactivatedAt: new Date() }); + expect(await service.resolveUserByKey('mcp_x')).toBeNull(); + }); + + it('refuses a key whose owner is no longer a member', async () => { + mockPrisma.mcpApiKey.findUnique.mockResolvedValue(record); + mockPrisma.organizationMember.findUnique.mockResolvedValue(null); + expect(await service.resolveUserByKey('mcp_x')).toBeNull(); + }); + }); }); diff --git a/packages/backend/src/roles/mcp-api-keys.service.ts b/packages/backend/src/roles/mcp-api-keys.service.ts index 9bb97d29..c772415b 100644 --- a/packages/backend/src/roles/mcp-api-keys.service.ts +++ b/packages/backend/src/roles/mcp-api-keys.service.ts @@ -73,6 +73,17 @@ export class McpApiKeysService { if (!record || !record.isActive) return null; + // Defence in depth: a key is bound to one organization, and it must not + // authenticate a user who has been deactivated there — even if the key row + // itself somehow escaped deactivation. + const membership = await this.prisma.organizationMember.findUnique({ + where: { + userId_organizationId: { userId: record.userId, organizationId: record.organizationId }, + }, + select: { deactivatedAt: true }, + }); + if (!membership || membership.deactivatedAt) return null; + // Update last used timestamp await this.prisma.mcpApiKey.update({ where: { id: record.id }, diff --git a/packages/backend/src/roles/roles.service.spec.ts b/packages/backend/src/roles/roles.service.spec.ts index f1656c88..ca1e4512 100644 --- a/packages/backend/src/roles/roles.service.spec.ts +++ b/packages/backend/src/roles/roles.service.spec.ts @@ -206,16 +206,24 @@ describe('RolesService', () => { }); describe('getAllowedToolIds', () => { - it('should return null for ADMIN users (unrestricted)', async () => { - mockPrisma.user.findUnique.mockResolvedValue({ role: 'ADMIN', mcpRoleId: null }); - const result = await service.getAllowedToolIds('user-1'); - expect(result).toBeNull(); + it('returns null (unrestricted) for an ADMIN of the organization', async () => { + mockPrisma.user.findUnique.mockResolvedValue({ role: 'ADMIN', organizationId: 'org-1' }); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: null }); + expect(await service.getAllowedToolIds('user-1')).toBeNull(); }); - it('should return null when user has no mcpRoleId (backward compat)', async () => { - mockPrisma.user.findUnique.mockResolvedValue({ role: 'USER', mcpRoleId: null }); - const result = await service.getAllowedToolIds('user-1'); - expect(result).toBeNull(); + // The old `users.role` fallback is exactly what a deactivated user is left + // with once their active org is cleared — and it read ADMIN as everything. + it('returns [] for an identified principal with no organization', async () => { + mockPrisma.user.findUnique.mockResolvedValue({ role: 'ADMIN', organizationId: null }); + expect(await service.getAllowedToolIds('user-1')).toEqual([]); + expect(mockPrisma.organizationMember.findUnique).not.toHaveBeenCalled(); + }); + + it('returns [] for a DEACTIVATED membership, even an admin one', async () => { + mockPrisma.user.findUnique.mockResolvedValue({ role: 'ADMIN', organizationId: 'org-1' }); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: new Date() }); + expect(await service.getAllowedToolIds('user-1', 'org-1')).toEqual([]); }); it('returns the UNION of every assigned role, deduplicated', async () => { @@ -224,7 +232,7 @@ describe('RolesService', () => { role: 'EDITOR', organizationId: 'org-1', }); - mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR' }); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: null }); mockPrisma.userRoleAssignment.findMany.mockResolvedValue([ { roleId: 'sales' }, { roleId: 'support' }, @@ -250,7 +258,7 @@ describe('RolesService', () => { role: 'EDITOR', organizationId: 'org-1', }); - mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR' }); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: null }); mockPrisma.userRoleAssignment.findMany.mockResolvedValue([ { roleId: 'empty' }, { roleId: 'sales' }, @@ -266,7 +274,7 @@ describe('RolesService', () => { role: 'EDITOR', organizationId: 'org-1', }); - mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR' }); + mockPrisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: null }); mockPrisma.userRoleAssignment.findMany.mockResolvedValue([{ roleId: 'sys' }]); mockPrisma.toolRoleAccess.findMany.mockResolvedValue([{ toolId: 't9' }]); @@ -308,7 +316,7 @@ describe('RolesService', () => { organizationId: 'org-corporate', }, }, - select: { role: true }, + select: { role: true, deactivatedAt: true }, }); // Restricted to the role's whitelist — NOT null/unrestricted. expect(result).toEqual(['t1']); @@ -337,14 +345,12 @@ describe('RolesService', () => { expect(await service.getAllowedToolIds('user-1', 'org-other')).toEqual([]); }); - it('falls back to the cached role when no org can be resolved', async () => { - // Self-host / instance-level path: no org context anywhere. - mockPrisma.user.findUnique.mockResolvedValue({ - role: 'ADMIN', - organizationId: null, - }); - - expect(await service.getAllowedToolIds('user-1')).toBeNull(); + it('fails closed when no org can be resolved (the cache fallback is gone)', async () => { + // Previously the `users.role` cache decided here, and ADMIN meant + // everything. A deactivated user whose active org was cleared lands + // exactly in this state, so it must yield nothing. + mockPrisma.user.findUnique.mockResolvedValue({ role: 'ADMIN', organizationId: null }); + expect(await service.getAllowedToolIds('user-1')).toEqual([]); expect(mockPrisma.organizationMember.findUnique).not.toHaveBeenCalled(); }); diff --git a/packages/backend/src/roles/roles.service.ts b/packages/backend/src/roles/roles.service.ts index 0877e050..4c44557e 100644 --- a/packages/backend/src/roles/roles.service.ts +++ b/packages/backend/src/roles/roles.service.ts @@ -208,19 +208,23 @@ export class RolesService { // The authoritative per-org role lives in organization_members, so resolve // against the organization actually being acted on. const orgId = organizationId ?? user.organizationId ?? null; - let effectiveRole: string = user.role; - if (orgId) { - const membership = await this.prisma.organizationMember.findUnique({ - where: { - userId_organizationId: { userId, organizationId: orgId }, - }, - select: { role: true }, - }); - // Not a member of this org — fail closed. The endpoint-level tenant check - // denies this case first; this is defense in depth, not the only gate. - if (!membership) return []; - effectiveRole = membership.role; - } + // An identified principal with no organization context gets no tools. The + // old fallback to the `users.role` cache is exactly what a deactivated + // user is left with once their active org is cleared, and it read ADMIN + // as "everything". + if (!orgId) return []; + + const membership = await this.prisma.organizationMember.findUnique({ + where: { + userId_organizationId: { userId, organizationId: orgId }, + }, + select: { role: true, deactivatedAt: true }, + }); + // Not a member, or deactivated — fail closed. The endpoint-level tenant + // check denies the first case first; this is defense in depth, not the + // only gate. Deactivation is caught HERE for every MCP path at once. + if (!membership || membership.deactivatedAt) return []; + const effectiveRole: string = membership.role; // ADMIN of THIS organization always has full access if (effectiveRole === 'ADMIN') return null; diff --git a/packages/backend/src/users/user-lifecycle.service.spec.ts b/packages/backend/src/users/user-lifecycle.service.spec.ts new file mode 100644 index 00000000..5eb68207 --- /dev/null +++ b/packages/backend/src/users/user-lifecycle.service.spec.ts @@ -0,0 +1,212 @@ +import { UserLifecycleService } from './user-lifecycle.service'; +import { LastAdminConflictException } from '../organizations/last-admin.exception'; +import { SecurityEventService } from '../audit/security-event.service'; +import { PrismaService } from '../common/prisma.service'; + +const ORG = 'org-1'; +const USER = 'u1'; + +/** + * Deactivation is the one action that must close every access path at once. + * These tests pin each write the primitive makes, and — through a REAL + * SecurityEventService over a fake `securityEvent.create` — that the audit + * rows say what happened without the redactor blanking the key ids. + */ +describe('UserLifecycleService', () => { + let prisma: any; + let organizations: any; + let events: any[]; + let service: UserLifecycleService; + + const admin = { reason: 'admin' as const, actor: { type: 'USER' as const, userId: 'admin-1' } }; + const scim = { reason: 'scim' as const, actor: { type: 'SYSTEM' as const }, providerId: 'idp-1' }; + + beforeEach(() => { + events = []; + prisma = { + organizationMember: { + findUnique: jest.fn(async () => ({ role: 'VIEWER', deactivatedAt: null })), + update: jest.fn(async () => ({})), + findFirst: jest.fn(async () => null), + }, + mcpApiKey: { + findMany: jest.fn(async () => [{ id: 'k1' }, { id: 'k2' }]), + updateMany: jest.fn(async () => ({ count: 2 })), + }, + user: { + findUnique: jest.fn(async () => ({ organizationId: ORG })), + update: jest.fn(async () => ({})), + }, + securityEvent: { + create: jest.fn(async (args: any) => { + events.push(args.data); + return args.data; + }), + }, + $transaction: jest.fn((fn: any) => fn(prisma)), + }; + organizations = { assertNotLastAdmin: jest.fn(async () => undefined) }; + service = new UserLifecycleService( + prisma, + organizations, + new SecurityEventService(prisma as unknown as PrismaService), + ); + }); + + const eventNames = () => events.map((e) => e.event); + + it('returns not_a_member and writes nothing', async () => { + prisma.organizationMember.findUnique.mockResolvedValue(null); + expect(await service.deactivateInOrganization(USER, ORG, admin)).toEqual({ status: 'not_a_member' }); + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(events).toHaveLength(0); + }); + + it('is idempotent for an already inactive member', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'VIEWER', deactivatedAt: new Date() }); + expect(await service.deactivateInOrganization(USER, ORG, admin)).toEqual({ status: 'already_inactive' }); + expect(prisma.mcpApiKey.updateMany).not.toHaveBeenCalled(); + expect(events).toHaveLength(0); + }); + + it('closes every path: membership, org-scoped keys, sessions, active-org cache', async () => { + const result = await service.deactivateInOrganization(USER, ORG, admin); + + expect(result).toMatchObject({ status: 'deactivated', keysDeactivated: 2, previousRole: 'VIEWER' }); + expect(prisma.organizationMember.update).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: USER, organizationId: ORG } }, + data: { deactivatedAt: expect.any(Date) }, + }); + // Keys are scoped to THIS organization. + expect(prisma.mcpApiKey.findMany).toHaveBeenCalledWith({ + where: { userId: USER, organizationId: ORG, isActive: true }, + select: { id: true }, + }); + expect(prisma.mcpApiKey.updateMany).toHaveBeenCalledWith({ + where: { id: { in: ['k1', 'k2'] } }, + data: { isActive: false }, + }); + // Sessions revoked and, since this was the active org, the cache cleared. + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: USER }, + data: { sessionsValidFrom: expect.any(Date), organizationId: null }, + }); + expect(eventNames()).toEqual(['USER_DEACTIVATED', 'SESSIONS_REVOKED', 'API_KEY_DEACTIVATED']); + }); + + it('repoints the active org to the oldest other ACTIVE membership', async () => { + prisma.organizationMember.findFirst.mockResolvedValue({ organizationId: 'org-2', role: 'EDITOR' }); + const result = await service.deactivateInOrganization(USER, ORG, admin); + expect(result).toMatchObject({ activeOrgRepointedTo: 'org-2' }); + expect(prisma.organizationMember.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: { userId: USER, deactivatedAt: null, organizationId: { not: ORG } }, + }), + ); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: USER }, + data: { sessionsValidFrom: expect.any(Date), organizationId: 'org-2', role: 'EDITOR' }, + }); + }); + + it('leaves the active org alone when it is a different workspace', async () => { + prisma.user.findUnique.mockResolvedValue({ organizationId: 'org-9' }); + await service.deactivateInOrganization(USER, ORG, admin); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: USER }, + data: { sessionsValidFrom: expect.any(Date) }, + }); + }); + + // The redactor blanks any metadata key that looks like "api_key". The key + // ids are the one fact an investigation wants, so they must survive. + it('audits the deactivated key ids without redaction', async () => { + await service.deactivateInOrganization(USER, ORG, admin); + const keyEvent = events.find((e) => e.event === 'API_KEY_DEACTIVATED'); + expect(keyEvent.metadata).toEqual({ reason: 'deactivation', keyCount: 2, keyIds: ['k1', 'k2'] }); + expect(JSON.stringify(events)).not.toContain('[REDACTED]'); + }); + + it('skips the key event when there were no active keys', async () => { + prisma.mcpApiKey.findMany.mockResolvedValue([]); + await service.deactivateInOrganization(USER, ORG, admin); + expect(prisma.mcpApiKey.updateMany).not.toHaveBeenCalled(); + expect(eventNames()).toEqual(['USER_DEACTIVATED', 'SESSIONS_REVOKED']); + }); + + describe('last admin', () => { + beforeEach(() => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'ADMIN', deactivatedAt: null }); + organizations.assertNotLastAdmin.mockRejectedValue(new LastAdminConflictException(ORG)); + }); + + it('refuses an admin-initiated deactivation before any write', async () => { + await expect(service.deactivateInOrganization(USER, ORG, admin)).rejects.toBeInstanceOf( + LastAdminConflictException, + ); + expect(prisma.$transaction).not.toHaveBeenCalled(); + expect(events).toHaveLength(0); + }); + + // A directory push cannot be argued with: everything that grants ACCESS + // goes, but the membership — the workspace's one way back in — stays. + it('for SCIM revokes sessions and keys but keeps the membership active', async () => { + const result = await service.deactivateInOrganization(USER, ORG, scim); + expect(result).toEqual({ status: 'last_admin_retained', keysDeactivated: 2 }); + expect(prisma.organizationMember.update).not.toHaveBeenCalled(); + expect(prisma.mcpApiKey.updateMany).toHaveBeenCalled(); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: USER }, + data: { sessionsValidFrom: expect.any(Date) }, + }); + expect(eventNames()).toEqual([ + 'LAST_ADMIN_PROTECTION_TRIGGERED', + 'SESSIONS_REVOKED', + 'API_KEY_DEACTIVATED', + ]); + expect(events[0].metadata).toEqual({ reason: 'scim', providerId: 'idp-1', action: 'deactivate' }); + }); + + it('rethrows anything that is not the last-admin conflict', async () => { + organizations.assertNotLastAdmin.mockRejectedValue(new Error('db down')); + await expect(service.deactivateInOrganization(USER, ORG, scim)).rejects.toThrow('db down'); + }); + }); + + describe('reactivateInOrganization', () => { + it('returns not_a_member / already_active without writes', async () => { + prisma.organizationMember.findUnique.mockResolvedValueOnce(null); + expect(await service.reactivateInOrganization(USER, ORG, admin)).toEqual({ status: 'not_a_member' }); + prisma.organizationMember.findUnique.mockResolvedValueOnce({ role: 'VIEWER', deactivatedAt: null }); + expect(await service.reactivateInOrganization(USER, ORG, admin)).toEqual({ status: 'already_active' }); + expect(prisma.organizationMember.update).not.toHaveBeenCalled(); + }); + + // What was cut on the way out is not silently re-armed on the way back. + it('clears the flag, repoints a null active org, and restores nothing else', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: new Date() }); + prisma.user.findUnique.mockResolvedValue({ organizationId: null }); + const result = await service.reactivateInOrganization(USER, ORG, scim); + expect(result).toEqual({ status: 'reactivated', role: 'EDITOR' }); + expect(prisma.organizationMember.update).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: USER, organizationId: ORG } }, + data: { deactivatedAt: null }, + }); + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: USER }, + data: { organizationId: ORG, role: 'EDITOR' }, + }); + expect(prisma.mcpApiKey.updateMany).not.toHaveBeenCalled(); + const updates = prisma.user.update.mock.calls.map((c: any[]) => c[0].data); + expect(updates.some((d: any) => 'sessionsValidFrom' in d)).toBe(false); + expect(eventNames()).toEqual(['USER_REACTIVATED']); + }); + + it('does not touch the active org when the user already has one', async () => { + prisma.organizationMember.findUnique.mockResolvedValue({ role: 'EDITOR', deactivatedAt: new Date() }); + prisma.user.findUnique.mockResolvedValue({ organizationId: 'org-2' }); + await service.reactivateInOrganization(USER, ORG, admin); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/backend/src/users/user-lifecycle.service.ts b/packages/backend/src/users/user-lifecycle.service.ts new file mode 100644 index 00000000..9b15cbe5 --- /dev/null +++ b/packages/backend/src/users/user-lifecycle.service.ts @@ -0,0 +1,251 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { PrismaService } from '../common/prisma.service'; +import { UserRole } from '../generated/prisma/client'; +import { + SecurityEventService, + SecurityEvents, +} from '../audit/security-event.service'; +import { OrganizationsService } from '../organizations/organizations.service'; +import { LastAdminConflictException } from '../organizations/last-admin.exception'; + +export type LifecycleReason = 'admin' | 'scim' | 'system'; + +export interface LifecycleContext { + reason: LifecycleReason; + actor: { type: 'USER' | 'SYSTEM'; userId?: string | null }; + /** Set when a directory push (SCIM) is the origin. */ + providerId?: string | null; + ip?: string | null; + userAgent?: string | null; +} + +export type DeactivateResult = + | { + status: 'deactivated'; + keysDeactivated: number; + previousRole: UserRole; + activeOrgRepointedTo: string | null; + } + /** + * The directory asked to deactivate the only active admin. Sessions and + * keys were revoked — the leaver has lost MCP and dashboard access — but + * the membership stays active so the workspace keeps one way back in. + */ + | { status: 'last_admin_retained'; keysDeactivated: number } + | { status: 'already_inactive' } + | { status: 'not_a_member' }; + +export type ReactivateResult = + | { status: 'reactivated'; role: UserRole } + | { status: 'already_active' } + | { status: 'not_a_member' }; + +/** + * The one place that removes a person's access to a workspace. + * + * "Deactivate" has to close every path at once — dashboard tokens, MCP OAuth + * tokens, MCP API keys, and the membership itself — because each is checked + * by different code and a leaver who keeps any one of them has not left. The + * admin's Deactivate button and SCIM's `active: false` both land here so the + * two can never drift apart. + */ +@Injectable() +export class UserLifecycleService { + private readonly logger = new Logger(UserLifecycleService.name); + + constructor( + private readonly prisma: PrismaService, + private readonly organizations: OrganizationsService, + private readonly securityEvents: SecurityEventService, + ) {} + + async deactivateInOrganization( + userId: string, + organizationId: string, + ctx: LifecycleContext, + ): Promise { + const membership = await this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { role: true, deactivatedAt: true }, + }); + if (!membership) return { status: 'not_a_member' }; + if (membership.deactivatedAt) return { status: 'already_inactive' }; + + // Last-admin protection. An admin gets a refusal and promotes someone + // first. A directory push cannot be argued with, so for it we still + // revoke everything that grants ACCESS — sessions and keys — and keep + // only the membership, which is what lets someone recover the workspace. + let lastAdmin = false; + try { + await this.organizations.assertNotLastAdmin(organizationId, userId); + } catch (e) { + if (!(e instanceof LastAdminConflictException)) throw e; + if (ctx.reason === 'admin') throw e; + lastAdmin = true; + } + + const now = new Date(); + const result = await this.prisma.$transaction(async (tx) => { + if (!lastAdmin) { + await tx.organizationMember.update({ + where: { userId_organizationId: { userId, organizationId } }, + data: { deactivatedAt: now }, + }); + } + + // Keys are org-scoped: a multi-workspace user keeps the keys they use + // elsewhere. + const keys = await tx.mcpApiKey.findMany({ + where: { userId, organizationId, isActive: true }, + select: { id: true }, + }); + if (keys.length > 0) { + await tx.mcpApiKey.updateMany({ + where: { id: { in: keys.map((k) => k.id) } }, + data: { isActive: false }, + }); + } + + // `sessionsValidFrom` is deliberately global. Neither token family is + // org-scoped for revocation — both resolve the organization from the + // user row — so a per-membership cutover would need a second column and + // a check in every token path, to spare a multi-org user one re-login. + const user = await tx.user.findUnique({ + where: { id: userId }, + select: { organizationId: true }, + }); + let repointedTo: string | null = null; + const data: { sessionsValidFrom: Date; organizationId?: string | null; role?: UserRole } = { + sessionsValidFrom: now, + }; + if (!lastAdmin && user?.organizationId === organizationId) { + const next = await tx.organizationMember.findFirst({ + where: { userId, deactivatedAt: null, organizationId: { not: organizationId } }, + orderBy: { joinedAt: 'asc' }, + select: { organizationId: true, role: true }, + }); + data.organizationId = next?.organizationId ?? null; + if (next) data.role = next.role; + repointedTo = next?.organizationId ?? null; + } + await tx.user.update({ where: { id: userId }, data }); + + return { keyIds: keys.map((k) => k.id), repointedTo }; + }); + + const base = { + organizationId, + actorType: ctx.actor.type, + actorUserId: ctx.actor.userId ?? null, + targetUserId: userId, + ip: ctx.ip, + userAgent: ctx.userAgent, + } as const; + + if (lastAdmin) { + this.logger.warn( + `Refused to deactivate the last admin of org ${organizationId} (reason ${ctx.reason}); sessions and keys revoked, membership kept`, + ); + await this.securityEvents.log({ + ...base, + event: SecurityEvents.LAST_ADMIN_PROTECTION_TRIGGERED, + metadata: { reason: ctx.reason, providerId: ctx.providerId ?? null, action: 'deactivate' }, + }); + } else { + await this.securityEvents.log({ + ...base, + event: SecurityEvents.USER_DEACTIVATED, + metadata: { + reason: ctx.reason, + providerId: ctx.providerId ?? null, + previousRole: membership.role, + keysDeactivated: result.keyIds.length, + activeOrgRepointedTo: result.repointedTo, + }, + }); + } + await this.securityEvents.log({ + ...base, + event: SecurityEvents.SESSIONS_REVOKED, + metadata: { reason: 'deactivation', organizationId }, + }); + if (result.keyIds.length > 0) { + // `keyIds`, not `apiKeyIds`: the redactor blanks any metadata key that + // looks like "api_key", and an audit row saying "[REDACTED]" here would + // hide the one fact an investigation wants. + await this.securityEvents.log({ + ...base, + event: SecurityEvents.API_KEY_DEACTIVATED, + metadata: { reason: 'deactivation', keyCount: result.keyIds.length, keyIds: result.keyIds }, + }); + } + + if (lastAdmin) { + return { status: 'last_admin_retained', keysDeactivated: result.keyIds.length }; + } + return { + status: 'deactivated', + keysDeactivated: result.keyIds.length, + previousRole: membership.role, + activeOrgRepointedTo: result.repointedTo, + }; + } + + /** + * Restores the membership. Deliberately restores NOTHING else: revoked keys + * stay revoked (the user mints a new one) and old sessions stay dead. What + * was cut on the way out is not silently re-armed on the way back. + */ + async reactivateInOrganization( + userId: string, + organizationId: string, + ctx: LifecycleContext, + ): Promise { + const membership = await this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { role: true, deactivatedAt: true }, + }); + if (!membership) return { status: 'not_a_member' }; + if (!membership.deactivatedAt) return { status: 'already_active' }; + + await this.prisma.$transaction(async (tx) => { + await tx.organizationMember.update({ + where: { userId_organizationId: { userId, organizationId } }, + data: { deactivatedAt: null }, + }); + // A user left with no active org is pointed back at this one. + const user = await tx.user.findUnique({ + where: { id: userId }, + select: { organizationId: true }, + }); + if (user && user.organizationId === null) { + await tx.user.update({ + where: { id: userId }, + data: { organizationId, role: membership.role }, + }); + } + }); + + await this.securityEvents.log({ + event: SecurityEvents.USER_REACTIVATED, + actorType: ctx.actor.type, + actorUserId: ctx.actor.userId ?? null, + organizationId, + targetUserId: userId, + metadata: { reason: ctx.reason, providerId: ctx.providerId ?? null, role: membership.role }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); + return { status: 'reactivated', role: membership.role }; + } + + /** For callers that render an "active" flag (SCIM). null = not a member. */ + async getMembershipState(userId: string, organizationId: string) { + const m = await this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { role: true, deactivatedAt: true }, + }); + if (!m) return null; + return { active: m.deactivatedAt === null, role: m.role, deactivatedAt: m.deactivatedAt }; + } +} diff --git a/packages/backend/src/users/users.controller.ts b/packages/backend/src/users/users.controller.ts index 0a3e82a1..ac372fe9 100644 --- a/packages/backend/src/users/users.controller.ts +++ b/packages/backend/src/users/users.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, + Post, Put, Patch, Delete, @@ -8,7 +9,11 @@ import { Param, Req, UseGuards, + HttpCode, + HttpStatus, UnauthorizedException, + BadRequestException, + NotFoundException, } from '@nestjs/common'; import { ApiTags, ApiBearerAuth, ApiOperation, ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { AuthGuard } from '@nestjs/passport'; @@ -17,6 +22,8 @@ import { UserRole } from '../generated/prisma/client'; import { UsersService } from './users.service'; import { AuthService } from '../auth/auth.service'; import { Roles, RolesGuard } from '../auth/roles.guard'; +import { OrganizationsService } from '../organizations/organizations.service'; +import { UserLifecycleService, LifecycleContext } from './user-lifecycle.service'; class UpdateProfileDto { @ApiPropertyOptional({ description: 'Display name.' }) @@ -94,6 +101,8 @@ export class UsersController { constructor( private readonly usersService: UsersService, private readonly authService: AuthService, + private readonly organizations: OrganizationsService, + private readonly lifecycle: UserLifecycleService, ) {} @Get('me') @@ -238,29 +247,112 @@ export class UsersController { @Body() dto: UpdateUserRoleDto, ) { if (id === req.user.sub) { - return { error: 'Cannot change your own role' }; + throw new BadRequestException('Cannot change your own role'); } - const updated = await this.usersService.updateInOrg( + // Writes the MEMBERSHIP role (what authorization reads), refreshes the + // cache, and revokes sessions on a demotion. The previous implementation + // wrote only `users.role`, so a demoted admin kept unrestricted MCP + // tools. + const result = await this.organizations.updateMemberRole( id, req.user.organizationId, - { role: dto.role }, + dto.role, + this.actionContext(req), ); - if (!updated) return { error: 'User not found' }; - return { message: `User role updated to ${dto.role}` }; + if (!result) throw new NotFoundException('User not found'); + return { + message: `User role updated to ${dto.role}`, + sessionsRevoked: result.sessionsRevoked, + }; + } + + @Post(':id/deactivate') + @HttpCode(HttpStatus.OK) + @UseGuards(RolesGuard) + @Roles('ADMIN') + @ApiOperation({ + summary: + 'Deactivate a member (ADMIN only): ends their sessions, revokes their MCP keys and removes their access to this workspace. Reversible.', + }) + async deactivateUser(@Req() req: any, @Param('id') id: string) { + if (id === req.user.sub) { + throw new BadRequestException('Cannot deactivate your own account'); + } + const result = await this.lifecycle.deactivateInOrganization( + id, + req.user.organizationId, + { reason: 'admin', ...this.lifecycleContext(req) }, + ); + switch (result.status) { + case 'not_a_member': + throw new NotFoundException('User not found'); + case 'already_inactive': + return { message: 'User is already deactivated' }; + case 'last_admin_retained': + // Unreachable for reason 'admin' (the service throws instead), kept + // so the switch is exhaustive if that policy ever changes. + return { message: 'Sessions and keys revoked; the last administrator was kept active' }; + default: + return { + message: 'User deactivated', + keysDeactivated: result.keysDeactivated, + }; + } + } + + @Post(':id/reactivate') + @HttpCode(HttpStatus.OK) + @UseGuards(RolesGuard) + @Roles('ADMIN') + @ApiOperation({ + summary: + 'Reactivate a member (ADMIN only). Restores the membership only — revoked MCP keys stay revoked.', + }) + async reactivateUser(@Req() req: any, @Param('id') id: string) { + const result = await this.lifecycle.reactivateInOrganization( + id, + req.user.organizationId, + { reason: 'admin', ...this.lifecycleContext(req) }, + ); + if (result.status === 'not_a_member') throw new NotFoundException('User not found'); + if (result.status === 'already_active') return { message: 'User is already active' }; + return { message: 'User reactivated' }; } @Delete(':id') @UseGuards(RolesGuard) @Roles('ADMIN') - @ApiOperation({ summary: 'Delete a user (ADMIN only)' }) + @ApiOperation({ + summary: + 'Remove a user from this workspace (ADMIN only). Deletes the account only when this was their sole workspace.', + }) async deleteUser(@Req() req: any, @Param('id') id: string) { if (id === req.user.sub) { - return { error: 'Cannot delete your own account' }; + throw new BadRequestException('Cannot delete your own account'); } - const ok = await this.usersService.deleteInOrg(id, req.user.organizationId); - if (!ok) return { error: 'User not found' }; - return { message: 'User deleted' }; + const ok = await this.usersService.deleteInOrg(id, req.user.organizationId, { + reason: 'admin', + ...this.lifecycleContext(req), + }); + if (!ok) throw new NotFoundException('User not found'); + return { message: 'User removed' }; + } + + private actionContext(req: any) { + return { + actorUserId: req.user.sub as string, + ip: req.ip as string | undefined, + userAgent: req.headers?.['user-agent'] as string | undefined, + }; + } + + private lifecycleContext(req: any): Omit { + return { + actor: { type: 'USER', userId: req.user.sub }, + ip: req.ip, + userAgent: req.headers?.['user-agent'], + }; } } diff --git a/packages/backend/src/users/users.module.ts b/packages/backend/src/users/users.module.ts index 276e9b63..dab6a7f4 100644 --- a/packages/backend/src/users/users.module.ts +++ b/packages/backend/src/users/users.module.ts @@ -1,10 +1,13 @@ import { Module } from '@nestjs/common'; import { UsersService } from './users.service'; import { UsersController } from './users.controller'; +import { UserLifecycleService } from './user-lifecycle.service'; +import { OrganizationsModule } from '../organizations/organizations.module'; @Module({ + imports: [OrganizationsModule], controllers: [UsersController], - providers: [UsersService], - exports: [UsersService], + providers: [UsersService, UserLifecycleService], + exports: [UsersService, UserLifecycleService], }) export class UsersModule {} diff --git a/packages/backend/src/users/users.service.spec.ts b/packages/backend/src/users/users.service.spec.ts index b7326a4e..373db4fb 100644 --- a/packages/backend/src/users/users.service.spec.ts +++ b/packages/backend/src/users/users.service.spec.ts @@ -3,6 +3,9 @@ import { UsersService } from './users.service'; describe('UsersService', () => { let service: UsersService; let mockPrisma: any; + let organizations: any; + let lifecycle: any; + let securityEvents: any; const mockUser = { id: 'user-1', @@ -25,8 +28,20 @@ describe('UsersService', () => { update: jest.fn(), delete: jest.fn(), }, + organizationMember: { + findMany: jest.fn().mockResolvedValue([]), + findUnique: jest.fn(), + count: jest.fn(), + delete: jest.fn(), + }, + userRoleAssignment: { deleteMany: jest.fn() }, + mcpApiKey: { deleteMany: jest.fn() }, + $transaction: jest.fn(async (ops: any) => (Array.isArray(ops) ? Promise.all(ops) : ops(mockPrisma))), }; - service = new UsersService(mockPrisma); + organizations = { assertNotLastAdmin: jest.fn() }; + lifecycle = { deactivateInOrganization: jest.fn(async () => ({ status: 'deactivated' })) }; + securityEvents = { log: jest.fn() }; + service = new UsersService(mockPrisma, organizations, lifecycle, securityEvents); }); describe('findByEmail', () => { @@ -81,31 +96,73 @@ describe('UsersService', () => { }); describe('findAll', () => { - it('should call findMany with correct select fields and no where when no org passed', async () => { - mockPrisma.user.findMany.mockResolvedValue([]); - await service.findAll(); - expect(mockPrisma.user.findMany).toHaveBeenCalledWith({ - where: undefined, - select: { - id: true, - email: true, - name: true, - role: true, - organizationId: true, - mcpRoleId: true, - mcpRole: { select: { id: true, name: true } }, - createdAt: true, - updatedAt: true, + // The member list is the MEMBERSHIP table, not the users.organization_id + // cache: a multi-workspace user must be visible to every workspace's + // admins, and the role shown must be the one authorization uses. + it('lists memberships of the organization with the membership role', async () => { + mockPrisma.organizationMember.findMany.mockResolvedValue([ + { + role: 'VIEWER', + joinedAt: new Date('2026-01-01'), + deactivatedAt: null, + user: { id: 'u1', email: 'a@x', name: 'A', mcpRoleId: null, mcpRole: null, createdAt: new Date(), updatedAt: new Date() }, }, - }); - }); - - it('should scope by organizationId when provided', async () => { - mockPrisma.user.findMany.mockResolvedValue([]); - await service.findAll('org-1'); - expect(mockPrisma.user.findMany).toHaveBeenCalledWith( + { + role: 'ADMIN', + joinedAt: new Date('2026-01-02'), + deactivatedAt: new Date('2026-02-01'), + user: { id: 'u2', email: 'b@x', name: 'B', mcpRoleId: null, mcpRole: null, createdAt: new Date(), updatedAt: new Date() }, + }, + ]); + const rows = await service.findAll('org-1'); + expect(mockPrisma.organizationMember.findMany).toHaveBeenCalledWith( expect.objectContaining({ where: { organizationId: 'org-1' } }), ); + expect(rows.map((r) => [r.id, r.role, r.active, r.organizationId])).toEqual([ + ['u1', 'VIEWER', true, 'org-1'], + ['u2', 'ADMIN', false, 'org-1'], + ]); + expect(rows[1].deactivatedAt).toEqual(new Date('2026-02-01')); + }); + }); + + describe('deleteInOrg', () => { + const ctx = { reason: 'admin' as const, actor: { type: 'USER' as const, userId: 'admin' } }; + + it('returns false for a non-member and touches nothing', async () => { + mockPrisma.organizationMember.findUnique.mockResolvedValue(null); + expect(await service.deleteInOrg('u1', 'org-1', ctx)).toBe(false); + expect(mockPrisma.user.delete).not.toHaveBeenCalled(); + }); + + it('refuses to remove the last active admin', async () => { + mockPrisma.organizationMember.findUnique.mockResolvedValue({ id: 'm1' }); + organizations.assertNotLastAdmin.mockRejectedValue(new Error('LAST_ADMIN')); + await expect(service.deleteInOrg('u1', 'org-1', ctx)).rejects.toThrow('LAST_ADMIN'); + expect(mockPrisma.user.delete).not.toHaveBeenCalled(); + }); + + it('deletes the account only when this was the sole workspace', async () => { + mockPrisma.organizationMember.findUnique.mockResolvedValue({ id: 'm1' }); + mockPrisma.organizationMember.count.mockResolvedValue(0); + expect(await service.deleteInOrg('u1', 'org-1', ctx)).toBe(true); + expect(mockPrisma.user.delete).toHaveBeenCalledWith({ where: { id: 'u1' } }); + expect(lifecycle.deactivateInOrganization).not.toHaveBeenCalled(); + }); + + // The admin of one workspace must not be able to destroy a person's access + // to every other workspace they belong to. + it('removes only this membership when the user belongs to other workspaces', async () => { + mockPrisma.organizationMember.findUnique.mockResolvedValue({ id: 'm1' }); + mockPrisma.organizationMember.count.mockResolvedValue(2); + expect(await service.deleteInOrg('u1', 'org-1', ctx)).toBe(true); + expect(mockPrisma.user.delete).not.toHaveBeenCalled(); + expect(lifecycle.deactivateInOrganization).toHaveBeenCalledWith('u1', 'org-1', ctx); + expect(mockPrisma.organizationMember.delete).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: 'u1', organizationId: 'org-1' } }, + }); + expect(mockPrisma.mcpApiKey.deleteMany).toHaveBeenCalledWith({ where: { userId: 'u1', organizationId: 'org-1' } }); + expect(securityEvents.log).toHaveBeenCalledWith(expect.objectContaining({ event: 'MEMBERSHIP_REMOVED' })); }); }); diff --git a/packages/backend/src/users/users.service.ts b/packages/backend/src/users/users.service.ts index d33ab1d8..ba439e1c 100644 --- a/packages/backend/src/users/users.service.ts +++ b/packages/backend/src/users/users.service.ts @@ -1,12 +1,20 @@ import { ConflictException, Injectable, Logger, NotFoundException } from '@nestjs/common'; import { PrismaService } from '../common/prisma.service'; import { User, UserRole } from '../generated/prisma/client'; +import { OrganizationsService } from '../organizations/organizations.service'; +import { UserLifecycleService, LifecycleContext } from './user-lifecycle.service'; +import { SecurityEventService, SecurityEvents } from '../audit/security-event.service'; @Injectable() export class UsersService { private readonly logger = new Logger(UsersService.name); - constructor(private readonly prisma: PrismaService) {} + constructor( + private readonly prisma: PrismaService, + private readonly organizations: OrganizationsService, + private readonly lifecycle: UserLifecycleService, + private readonly securityEvents: SecurityEventService, + ) {} async findByEmail(email: string): Promise { return this.prisma.user.findUnique({ where: { email } }); @@ -30,21 +38,46 @@ export class UsersService { return this.prisma.user.count(); } - async findAll(organizationId?: string) { - return this.prisma.user.findMany({ - where: organizationId ? { organizationId } : undefined, - select: { - id: true, - email: true, - name: true, - role: true, - organizationId: true, - mcpRoleId: true, - mcpRole: { select: { id: true, name: true } }, - createdAt: true, - updatedAt: true, + /** + * Members of an organization, from `organization_members`. + * + * Previously read `users.organization_id`, which is only the ACTIVE-org + * cache: a person who belongs to several workspaces was invisible to the + * admins of every workspace but the one they last switched to. `role` is + * the membership role — the one authorization actually uses. + */ + async findAll(organizationId: string) { + const rows = await this.prisma.organizationMember.findMany({ + where: { organizationId }, + include: { + user: { + select: { + id: true, + email: true, + name: true, + mcpRoleId: true, + mcpRole: { select: { id: true, name: true } }, + createdAt: true, + updatedAt: true, + }, + }, }, + orderBy: { joinedAt: 'asc' }, }); + return rows.map((m) => ({ + id: m.user.id, + email: m.user.email, + name: m.user.name, + role: m.role, + organizationId, + mcpRoleId: m.user.mcpRoleId, + mcpRole: m.user.mcpRole, + createdAt: m.user.createdAt, + updatedAt: m.user.updatedAt, + joinedAt: m.joinedAt, + deactivatedAt: m.deactivatedAt, + active: m.deactivatedAt === null, + })); } async update(userId: string, data: Partial): Promise { @@ -127,13 +160,61 @@ export class UsersService { await this.prisma.user.delete({ where: { id: userId } }); } - async deleteInOrg(userId: string, organizationId: string): Promise { - const target = await this.prisma.user.findFirst({ - where: { id: userId, organizationId }, + /** + * Removes a user from an organization, as that organization's admin. + * + * Keyed on the membership, not on the `users.organization_id` cache, and + * scoped to what this admin actually owns: if the user also belongs to + * other workspaces, only THIS membership goes (plus its keys and roles) and + * the account survives. Deleting the global user row from here let the admin + * of one workspace destroy someone's access to every other one. + * + * Returns false when the user is not a member (the controller's 404). + * Throws LastAdminConflictException for the only active admin. + */ + async deleteInOrg( + userId: string, + organizationId: string, + ctx: LifecycleContext, + ): Promise { + const membership = await this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, select: { id: true }, }); - if (!target) return false; - await this.prisma.user.delete({ where: { id: userId } }); + if (!membership) return false; + + await this.organizations.assertNotLastAdmin(organizationId, userId); + + const otherMemberships = await this.prisma.organizationMember.count({ + where: { userId, organizationId: { not: organizationId } }, + }); + + if (otherMemberships === 0) { + // Sole workspace: the account has nowhere else to live. Cascade cleans + // memberships, roles, keys and identities. + await this.prisma.user.delete({ where: { id: userId } }); + return true; + } + + // Cut access first (sessions, keys, cache repoint), then remove the rows. + await this.lifecycle.deactivateInOrganization(userId, organizationId, ctx); + await this.prisma.$transaction([ + this.prisma.userRoleAssignment.deleteMany({ where: { userId, organizationId } }), + this.prisma.mcpApiKey.deleteMany({ where: { userId, organizationId } }), + this.prisma.organizationMember.delete({ + where: { userId_organizationId: { userId, organizationId } }, + }), + ]); + await this.securityEvents.log({ + event: SecurityEvents.MEMBERSHIP_REMOVED, + actorType: ctx.actor.type, + actorUserId: ctx.actor.userId ?? null, + organizationId, + targetUserId: userId, + metadata: { reason: ctx.reason, remainingMemberships: otherMemberships }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); return true; } @@ -142,7 +223,7 @@ export class UsersService { if (!user) throw new NotFoundException('User not found'); const adminMemberships = await this.prisma.organizationMember.findMany({ - where: { userId, role: 'ADMIN' }, + where: { userId, role: 'ADMIN', deactivatedAt: null }, include: { organization: { select: { @@ -164,7 +245,7 @@ export class UsersService { continue; } const otherAdmins = await this.prisma.organizationMember.count({ - where: { organizationId: m.organizationId, role: 'ADMIN', userId: { not: userId } }, + where: { organizationId: m.organizationId, role: 'ADMIN', deactivatedAt: null, userId: { not: userId } }, }); if (otherAdmins === 0) { blocking.push({ id: m.organization.id, name: m.organization.name }); diff --git a/packages/frontend/src/app/admin/users/page.tsx b/packages/frontend/src/app/admin/users/page.tsx index d1b0e1a6..e0db88b4 100644 --- a/packages/frontend/src/app/admin/users/page.tsx +++ b/packages/frontend/src/app/admin/users/page.tsx @@ -60,8 +60,40 @@ export default function AdminUsersPage() { } }; + const handleDeactivate = async (userId: string, email: string) => { + if ( + !token || + !confirm( + `Deactivate ${email}?\n\nTheir sessions end now, their MCP keys stop working and they lose access to this workspace. You can reactivate them later.`, + ) + ) + return; + try { + await users.deactivate(userId, token); + setUserList((prev) => + prev.map((u) => (u.id === userId ? { ...u, active: false, deactivatedAt: new Date().toISOString() } : u)), + ); + setMsg('User deactivated'); + } catch (err: any) { + setMsg(`Error: ${err.message}`); + } + }; + + const handleReactivate = async (userId: string) => { + if (!token) return; + try { + await users.reactivate(userId, token); + setUserList((prev) => + prev.map((u) => (u.id === userId ? { ...u, active: true, deactivatedAt: null } : u)), + ); + setMsg('User reactivated. Revoked MCP keys stay revoked — they can create a new one.'); + } catch (err: any) { + setMsg(`Error: ${err.message}`); + } + }; + const handleDelete = async (userId: string, email: string) => { - if (!token || !confirm(`Delete user ${email}? This cannot be undone.`)) return; + if (!token || !confirm(`Remove ${email} from this workspace? If this is their only workspace the account is deleted. This cannot be undone.`)) return; try { await users.delete(userId, token); setUserList((prev) => prev.filter((u) => u.id !== userId)); @@ -205,7 +237,7 @@ export default function AdminUsersPage() { {loading ? (

Loading...

) : ( - + @@ -225,6 +257,9 @@ export default function AdminUsersPage() { {u.id === currentUser?.id && ( you )} + {u.deactivatedAt && ( + Deactivated + )} - diff --git a/packages/frontend/src/app/settings/users/page.tsx b/packages/frontend/src/app/settings/users/page.tsx index 06eb2b87..9cc57a35 100644 --- a/packages/frontend/src/app/settings/users/page.tsx +++ b/packages/frontend/src/app/settings/users/page.tsx @@ -67,8 +67,40 @@ export default function SettingsUsersPage() { } }; + const handleDeactivate = async (userId: string, email: string) => { + if ( + !token || + !confirm( + `Deactivate ${email}?\n\nTheir sessions end now, their MCP keys stop working and they lose access to this workspace. You can reactivate them later.`, + ) + ) + return; + try { + await users.deactivate(userId, token); + setUserList((prev) => + prev.map((u) => (u.id === userId ? { ...u, active: false, deactivatedAt: new Date().toISOString() } : u)), + ); + setMsg('User deactivated'); + } catch (err: any) { + setMsg(`Error: ${err.message}`); + } + }; + + const handleReactivate = async (userId: string) => { + if (!token) return; + try { + await users.reactivate(userId, token); + setUserList((prev) => + prev.map((u) => (u.id === userId ? { ...u, active: true, deactivatedAt: null } : u)), + ); + setMsg('User reactivated. Revoked MCP keys stay revoked — they can create a new one.'); + } catch (err: any) { + setMsg(`Error: ${err.message}`); + } + }; + const handleDelete = async (userId: string, email: string) => { - if (!token || !confirm(`Delete user ${email}? This cannot be undone.`)) return; + if (!token || !confirm(`Remove ${email} from this workspace? If this is their only workspace the account is deleted. This cannot be undone.`)) return; try { await users.delete(userId, token); setUserList((prev) => prev.filter((u) => u.id !== userId)); @@ -225,7 +257,7 @@ export default function SettingsUsersPage() { {loading ? (

Loading...

) : ( - +
{u.name || '—'} @@ -233,6 +268,7 @@ export default function AdminUsersPage() { ) : ( handleRoleChange(u.id, v)} className="h-8 rounded-[9px] border border-[var(--border)] bg-[var(--surface)] px-2 text-xs text-[var(--text)]" options={ROLES.map((r) => ({ value: r, label: r }))} @@ -251,15 +287,26 @@ export default function AdminUsersPage() { {new Date(u.createdAt).toLocaleDateString()} + {u.id !== currentUser?.id && ( - +
+ {u.deactivatedAt ? ( + + ) : ( + + )} + +
)}
@@ -249,7 +281,11 @@ export default function SettingsUsersPage() { - diff --git a/packages/frontend/src/lib/api.ts b/packages/frontend/src/lib/api.ts index 14cda477..dc6e3f8a 100644 --- a/packages/frontend/src/lib/api.ts +++ b/packages/frontend/src/lib/api.ts @@ -138,6 +138,12 @@ export const users = { request(`/api/users/${id}/role`, { method: 'PUT', body: { role }, token }), delete: (id: string, token: string) => request(`/api/users/${id}`, { method: 'DELETE', token }), + /** Ends sessions, revokes MCP keys and removes access to this workspace. Reversible. */ + deactivate: (id: string, token: string) => + request<{ message: string; keysDeactivated?: number }>(`/api/users/${id}/deactivate`, { method: 'POST', token }), + /** Restores the membership only — revoked keys stay revoked. */ + reactivate: (id: string, token: string) => + request<{ message: string }>(`/api/users/${id}/reactivate`, { method: 'POST', token }), deleteSelf: (data: { password: string; confirm: 'DELETE' }, token: string) => request<{ message: string }>('/api/users/me', { method: 'DELETE',
{u.name || '—'} - Active + {u.deactivatedAt ? ( + Deactivated + ) : ( + Active + )} {u.id === currentUser?.id ? ( @@ -257,6 +293,7 @@ export default function SettingsUsersPage() { ) : ( handleRoleChange(u.id, v)} className="h-8 rounded-[9px] border border-[var(--border)] px-2 text-xs bg-[var(--surface)] text-[var(--text)]" options={ROLES.map((r) => ({ value: r, label: r }))} @@ -275,11 +312,22 @@ export default function SettingsUsersPage() { {new Date(u.createdAt).toLocaleDateString()} + {u.id !== currentUser?.id && ( - +
+ {u.deactivatedAt ? ( + + ) : ( + + )} + +
)}