diff --git a/apps/api/src/people/people.controller.spec.ts b/apps/api/src/people/people.controller.spec.ts index b1751ad93..7e471dddf 100644 --- a/apps/api/src/people/people.controller.spec.ts +++ b/apps/api/src/people/people.controller.spec.ts @@ -305,6 +305,24 @@ describe('PeopleController', () => { 'mem_1', 'org_123', 'usr_123', + { skipOffboarding: false }, + ); + }); + + it('should pass skipOffboarding=true when query param is "true"', async () => { + const deleteResult = { + success: true, + deletedMember: { id: 'mem_1', name: 'Alice', email: 'alice@test.com' }, + }; + mockPeopleService.deleteById.mockResolvedValue(deleteResult); + + await controller.deleteMember('mem_1', 'org_123', mockAuthContext, 'true'); + + expect(peopleService.deleteById).toHaveBeenCalledWith( + 'mem_1', + 'org_123', + 'usr_123', + { skipOffboarding: true }, ); }); }); diff --git a/apps/api/src/people/people.controller.ts b/apps/api/src/people/people.controller.ts index 6e6fa8edc..781993a74 100644 --- a/apps/api/src/people/people.controller.ts +++ b/apps/api/src/people/people.controller.ts @@ -526,11 +526,13 @@ export class PeopleController { @Param('id') memberId: string, @OrganizationId() organizationId: string, @AuthContext() authContext: AuthContextType, + @Query('skipOffboarding') skipOffboarding?: string, ) { const result = await this.peopleService.deleteById( memberId, organizationId, authContext.userId, + { skipOffboarding: skipOffboarding === 'true' }, ); return { diff --git a/apps/api/src/people/people.service.spec.ts b/apps/api/src/people/people.service.spec.ts index a3003b5f5..259cfc867 100644 --- a/apps/api/src/people/people.service.spec.ts +++ b/apps/api/src/people/people.service.spec.ts @@ -48,6 +48,13 @@ jest.mock('@db', () => ({ update: jest.fn(), }, }, + BackgroundCheckStatus: { + pending: 'pending', + in_progress: 'in_progress', + completed: 'completed', + completed_with_flags: 'completed_with_flags', + cancelled: 'cancelled', + }, FindingType: { soc2: 'soc2', iso27001: 'iso27001' }, FindingStatus: { open: 'open', closed: 'closed' }, PhaseCompletionType: { manual: 'manual', auto: 'auto' }, @@ -556,10 +563,14 @@ describe('PeopleService', () => { expect(result.success).toBe(true); expect(result.deletedMember.id).toBe('mem_1'); - expect(db.member.update).toHaveBeenCalledWith({ - where: { id: 'mem_1', organizationId: 'org_123' }, - data: { deactivated: true, isActive: false }, + const updateCall = (db.member.update as jest.Mock).mock.calls[0]?.[0]; + expect(updateCall.where).toEqual({ + id: 'mem_1', + organizationId: 'org_123', }); + expect(updateCall.data.deactivated).toBe(true); + expect(updateCall.data.isActive).toBe(false); + expect(updateCall.data.offboardDate).toBeInstanceOf(Date); expect(db.session.deleteMany).toHaveBeenCalledWith({ where: { userId: 'usr_1' }, }); @@ -633,6 +644,46 @@ describe('PeopleService', () => { expect(fleetService.removeHostsByLabel).toHaveBeenCalledWith(42); }); + + describe('when skipOffboarding is true', () => { + it('should not set offboardDate', async () => { + await service.deleteById('mem_1', 'org_123', 'usr_actor', { + skipOffboarding: true, + }); + + const updateCall = (db.member.update as jest.Mock).mock.calls[0]?.[0]; + expect(updateCall.data).toEqual({ deactivated: true, isActive: false }); + expect(updateCall.data).not.toHaveProperty('offboardDate'); + }); + + it('should not collect assigned items or notify the owner', async () => { + await service.deleteById('mem_1', 'org_123', 'usr_actor', { + skipOffboarding: true, + }); + + // collectAssignedItems is skipped — no findMany on tasks/policies/risks/vendors + expect(db.task.findMany).not.toHaveBeenCalled(); + expect(db.policy.findMany).not.toHaveBeenCalled(); + expect(db.risk.findMany).not.toHaveBeenCalled(); + expect(db.vendor.findMany).not.toHaveBeenCalled(); + // notifyOwnerOfUnassignedItems is skipped — no owner lookup + expect(db.organization.findUnique).not.toHaveBeenCalled(); + }); + + it('should still clear assignments and delete sessions', async () => { + await service.deleteById('mem_1', 'org_123', 'usr_actor', { + skipOffboarding: true, + }); + + expect(db.task.updateMany).toHaveBeenCalledWith({ + where: { assigneeId: 'mem_1', organizationId: 'org_123' }, + data: { assigneeId: null }, + }); + expect(db.session.deleteMany).toHaveBeenCalledWith({ + where: { userId: 'usr_1' }, + }); + }); + }); }); describe('unlinkDevice', () => { diff --git a/apps/api/src/people/people.service.ts b/apps/api/src/people/people.service.ts index 3be40dd44..601348b25 100644 --- a/apps/api/src/people/people.service.ts +++ b/apps/api/src/people/people.service.ts @@ -363,6 +363,7 @@ export class PeopleService { memberId: string, organizationId: string, callerUserId?: string, + options?: { skipOffboarding?: boolean }, ): Promise<{ success: boolean; deletedMember: { id: string; name: string; email: string }; @@ -392,10 +393,11 @@ export class PeopleService { throw new ForbiddenException('Cannot remove a platform admin'); } - const unassignedItems = await collectAssignedItems({ - memberId, - organizationId, - }); + const skipOffboarding = options?.skipOffboarding === true; + + const unassignedItems = skipOffboarding + ? [] + : await collectAssignedItems({ memberId, organizationId }); await clearAssignments({ memberId, organizationId }); await removeMemberFromOrgChart({ organizationId, memberId }); @@ -405,7 +407,9 @@ export class PeopleService { data: { deactivated: true, isActive: false, - offboardDate: member.offboardDate ?? new Date(), + ...(skipOffboarding + ? {} + : { offboardDate: member.offboardDate ?? new Date() }), }, }); @@ -422,11 +426,13 @@ export class PeopleService { } } - await notifyOwnerOfUnassignedItems({ - organizationId, - removedMemberName: member.user.name || member.user.email || 'Member', - unassignedItems, - }); + if (!skipOffboarding) { + await notifyOwnerOfUnassignedItems({ + organizationId, + removedMemberName: member.user.name || member.user.email || 'Member', + unassignedItems, + }); + } this.logger.log( `Deactivated member: ${member.user.name} (${memberId}) from organization ${organizationId}`, diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx index 1026a33b7..7a7f06780 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx @@ -49,7 +49,7 @@ import type { BackgroundCheckStatus, MemberWithUser, TaskCompletion } from './Te interface MemberRowProps { member: MemberWithUser; - onRemove: (memberId: string) => void; + onRemove: (memberId: string, options: { skipOffboarding: boolean }) => void; onRemoveDevice: (memberId: string) => void; onUpdateRole: (memberId: string, roles: string[]) => void; onReactivate: (memberId: string) => void; @@ -226,12 +226,12 @@ export function MemberRow({ setIsUpdateRolesOpen(false); }; - const handleRemoveClick = async () => { + const handleRemoveClick = async (options: { skipOffboarding: boolean }) => { if (!canRemove) return; setIsRemoveAlertOpen(false); setIsRemoving(true); try { - await onRemove(memberId); + await onRemove(memberId, options); } finally { setIsRemoving(false); } diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/RemoveMemberAlert.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/RemoveMemberAlert.tsx index 6f75dcc29..b82287550 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/RemoveMemberAlert.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/RemoveMemberAlert.tsx @@ -10,12 +10,14 @@ import { AlertDialogTitle, } from '@trycompai/ui/alert-dialog'; import { Button } from '@trycompai/ui/button'; +import { Checkbox, Label } from '@trycompai/design-system'; +import { useEffect, useState } from 'react'; interface RemoveMemberAlertProps { open: boolean; onOpenChange: (open: boolean) => void; memberName: string; - onRemove: () => void; + onRemove: (options: { skipOffboarding: boolean }) => void; isRemoving: boolean; } @@ -26,6 +28,18 @@ export function RemoveMemberAlert({ onRemove, isRemoving, }: RemoveMemberAlertProps) { + const [skipOffboarding, setSkipOffboarding] = useState(false); + + useEffect(() => { + if (!open) { + setSkipOffboarding(false); + } + }, [open]); + + const handleRemoveClick = () => { + onRemove({ skipOffboarding }); + }; + return ( @@ -36,9 +50,17 @@ export function RemoveMemberAlert({ {'They will no longer have access to this organization.'} +
+ setSkipOffboarding(checked === true)} + /> + +
{'Cancel'} - diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx index f3936b718..c481f278b 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx @@ -205,9 +205,12 @@ export function TeamMembersClient({ } }; - const handleRemoveMember = async (memberId: string) => { + const handleRemoveMember = async ( + memberId: string, + options: { skipOffboarding: boolean }, + ) => { try { - await removeMember(memberId); + await removeMember(memberId, options); toast.success('Member has been removed from the organization'); router.refresh(); } catch (error) { diff --git a/apps/app/src/hooks/use-people-api.ts b/apps/app/src/hooks/use-people-api.ts index 4a8d05d2b..3fdef28a1 100644 --- a/apps/app/src/hooks/use-people-api.ts +++ b/apps/app/src/hooks/use-people-api.ts @@ -47,11 +47,12 @@ export function usePeopleActions() { ); const removeMember = useCallback( - async (memberId: string) => { + async (memberId: string, options?: { skipOffboarding?: boolean }) => { + const query = options?.skipOffboarding ? '?skipOffboarding=true' : ''; const response = await api.delete<{ success: boolean; deletedMember: { id: string; name: string; email: string }; - }>(`/v1/people/${memberId}`); + }>(`/v1/people/${memberId}${query}`); if (response.error) { throw new Error(response.error); } diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index 6da5f196b..dc46790cf 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -99,6 +99,37 @@ describe('Azure storage checks', () => { expect(passed).toHaveLength(1); }); + it("public-access: 'Selected networks' (publicNetworkAccess Enabled + defaultAction Deny) passes", async () => { + const { passed, failed } = await run( + storagePublicAccessCheck, + storageList({ + allowBlobPublicAccess: false, + publicNetworkAccess: 'Enabled', + networkAcls: { + defaultAction: 'Deny', + bypass: 'AzureServices', + ipRules: [{ value: '203.0.113.0/24', action: 'Allow' }], + }, + }), + ); + expect(failed).toHaveLength(0); + expect(passed).toHaveLength(1); + }); + + it('public-access: publicNetworkAccess Enabled with default Allow fails', async () => { + const { passed, failed } = await run( + storagePublicAccessCheck, + storageList({ + allowBlobPublicAccess: false, + publicNetworkAccess: 'Enabled', + networkAcls: { defaultAction: 'Allow' }, + }), + ); + expect(passed).toHaveLength(0); + expect(failed).toHaveLength(1); + expect(failed[0]!.severity).toBe('medium'); + }); + it('encryption fails when a service is disabled, passes when enabled', async () => { const bad = await run( storageEncryptionCheck, diff --git a/packages/integration-platform/src/manifests/azure/checks/storage.ts b/packages/integration-platform/src/manifests/azure/checks/storage.ts index f1058c7df..82c26ac29 100644 --- a/packages/integration-platform/src/manifests/azure/checks/storage.ts +++ b/packages/integration-platform/src/manifests/azure/checks/storage.ts @@ -106,14 +106,15 @@ export const storagePublicAccessCheck: IntegrationCheck = { const p = a.properties ?? {}; const publicBlob = p.allowBlobPublicAccess === true; // publicNetworkAccess 'Disabled' or 'SecuredByPerimeter' (network security - // perimeter) overrides the firewall default action and is not public. - const networkRestricted = + // perimeter) takes the endpoint off the public internet entirely. When it + // is 'Enabled', networkAcls.defaultAction === 'Deny' still restricts + // traffic to the explicit IP/VNet allowlist (Azure's "Selected networks" + // mode) and is not public. + const networkDisabled = p.publicNetworkAccess === 'Disabled' || p.publicNetworkAccess === 'SecuredByPerimeter'; - const publicNetwork = - !networkRestricted && - (p.publicNetworkAccess === 'Enabled' || - p.networkAcls?.defaultAction === 'Allow'); + const firewallEnforced = p.networkAcls?.defaultAction === 'Deny'; + const publicNetwork = !networkDisabled && !firewallEnforced; if (publicBlob || publicNetwork) { ctx.fail({ title: `Public access enabled: ${a.name}`, @@ -122,11 +123,12 @@ export const storagePublicAccessCheck: IntegrationCheck = { resourceId: a.id, severity: publicBlob ? 'high' : 'medium', remediation: - 'Disable "Allow Blob public access" and restrict network access to specific VNets/IPs or private endpoints.', + 'Disable "Allow Blob public access" and restrict network access to specific VNets/IPs or private endpoints (set networkAcls.defaultAction to "Deny").', evidence: { account: a.name, allowBlobPublicAccess: p.allowBlobPublicAccess, publicNetworkAccess: p.publicNetworkAccess ?? null, + networkDefaultAction: p.networkAcls?.defaultAction ?? null, }, }); } else {