From e179c5823328b29c2ee52cd49c53f2ef690469fb Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 8 Jul 2026 19:43:26 -0400 Subject: [PATCH 1/9] feat(org): treat platform admins as members in internal organizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `Organization.isInternal` so platform-operated organizations can run compliance with their platform-admin staff as real participants — assignable, counted toward compliance progress, and included in notifications and device rollups. Behavior is unchanged for every other organization (flag defaults to false; admin-only toggle on the org admin screen). Centralizes the rule behind a single `isOrgParticipant` predicate (`@trycompai/auth/participation`, mirrored for the Trigger.dev bundle), replacing the scattered inline `user.role === 'admin'` participation checks across assignment/approval, compliance counting, notifications, reminders, and device queries. Platform-admin access guards are intentionally untouched. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb --- .../admin-organizations.service.ts | 1 + .../dto/update-admin-organization.dto.ts | 8 ++ .../comment-mention-notifier.service.ts | 10 +-- apps/api/src/people/people-fleet.helper.ts | 7 +- apps/api/src/policies/policies.service.ts | 57 ++++++------ apps/api/src/risks/risks.service.ts | 6 +- apps/api/src/soa/soa.service.spec.ts | 12 ++- apps/api/src/soa/soa.service.ts | 9 +- .../task-item-assignment-notifier.service.ts | 11 ++- .../task-item-mention-notifier.service.ts | 10 +-- .../task-management.service.ts | 23 ++++- apps/api/src/tasks/task-notifier.service.ts | 13 ++- apps/api/src/tasks/tasks.service.ts | 24 +++-- .../run-browser-automation.ts | 16 ++-- apps/api/src/utils/compliance-filters.ts | 9 +- apps/api/src/utils/org-participation.spec.ts | 73 +++++++++++++++ apps/api/src/utils/org-participation.ts | 54 ++++++++++++ apps/api/src/vendors/vendors.service.ts | 6 +- .../accept-requested-policy-changes.ts | 4 +- .../[adminOrgId]/components/AdminOrgTabs.tsx | 1 + .../components/OrganizationDetail.tsx | 88 ++++++++++++------- apps/app/src/app/(app)/[orgId]/layout.tsx | 66 ++++++-------- .../src/app/api/people/agent-devices/route.ts | 13 ++- .../src/app/api/people/fleet-hosts/route.ts | 87 +++++++++--------- apps/app/src/components/SelectAssignee.tsx | 19 ++-- .../src/components/org-internal-context.tsx | 28 ++++++ apps/app/src/lib/compliance.ts | 19 ++-- .../src/lib/org-participation-rule.test.ts | 28 ++++++ apps/app/src/lib/org-participation-rule.ts | 23 +++++ apps/app/src/lib/org-participation.ts | 20 +++++ .../onboarding/generate-vendor-mitigation.ts | 21 +++-- .../policy-acknowledgment-digest-helpers.ts | 11 +-- .../task/policy-acknowledgment-digest.ts | 20 ++--- .../src/trigger/tasks/task/policy-schedule.ts | 49 +++++++---- .../src/trigger/tasks/task/task-schedule.ts | 56 ++++++++---- .../tasks/task/weekly-task-reminder.ts | 27 ++++-- packages/auth/package.json | 4 + packages/auth/src/index.ts | 31 ++++--- packages/auth/src/participation.ts | 55 ++++++++++++ .../migration.sql | 6 ++ packages/db/prisma/schema/organization.prisma | 4 + 41 files changed, 732 insertions(+), 297 deletions(-) create mode 100644 apps/api/src/utils/org-participation.spec.ts create mode 100644 apps/api/src/utils/org-participation.ts create mode 100644 apps/app/src/components/org-internal-context.tsx create mode 100644 apps/app/src/lib/org-participation-rule.test.ts create mode 100644 apps/app/src/lib/org-participation-rule.ts create mode 100644 apps/app/src/lib/org-participation.ts create mode 100644 packages/auth/src/participation.ts create mode 100644 packages/db/prisma/migrations/20260708120000_add_organization_is_internal/migration.sql diff --git a/apps/api/src/admin-organizations/admin-organizations.service.ts b/apps/api/src/admin-organizations/admin-organizations.service.ts index a1a5c33f5c..fc4ca96c50 100644 --- a/apps/api/src/admin-organizations/admin-organizations.service.ts +++ b/apps/api/src/admin-organizations/admin-organizations.service.ts @@ -240,6 +240,7 @@ export class AdminOrganizationsService { onboardingCompleted: true, website: true, backgroundCheckStepEnabled: true, + isInternal: true, members: { where: { isActive: true, deactivated: false }, select: { diff --git a/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts b/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts index 2026fee16f..beaaa75e58 100644 --- a/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts +++ b/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts @@ -17,4 +17,12 @@ export class UpdateAdminOrganizationDto { @IsOptional() @IsBoolean() backgroundCheckStepEnabled?: boolean; + + @ApiPropertyOptional({ + description: + "When true, the organization is platform-operated (e.g. Comp AI's own org). Platform admins are then treated as real participants — assignable, counted in compliance, and notified. Leave false for all customer orgs.", + }) + @IsOptional() + @IsBoolean() + isInternal?: boolean; } diff --git a/apps/api/src/comments/comment-mention-notifier.service.ts b/apps/api/src/comments/comment-mention-notifier.service.ts index f15b641ab6..69d6a47922 100644 --- a/apps/api/src/comments/comment-mention-notifier.service.ts +++ b/apps/api/src/comments/comment-mention-notifier.service.ts @@ -1,5 +1,6 @@ import { Injectable, Logger } from '@nestjs/common'; import { db } from '@db'; +import { orgParticipantMemberWhere } from '../utils/org-participation'; import { isUserUnsubscribed } from '@trycompai/email'; import { triggerEmail } from '../email/trigger-email'; import { CommentMentionedEmail } from '../email/templates/comment-mentioned'; @@ -261,16 +262,15 @@ export class CommentMentionNotifierService { return; } - // Get mentioned users: exclude platform admins unless they are an owner of this org + // Get mentioned users: exclude platform admins unless they are an owner of + // this org (or the org is internal, where platform admins are real members) + const participantWhere = await orgParticipantMemberWhere(organizationId); const mentionedMembers = await db.member.findMany({ where: { organizationId, deactivated: false, user: { id: { in: mentionedUserIds } }, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + ...participantWhere, }, include: { user: true }, }); diff --git a/apps/api/src/people/people-fleet.helper.ts b/apps/api/src/people/people-fleet.helper.ts index 784d26cc89..be4ed40c93 100644 --- a/apps/api/src/people/people-fleet.helper.ts +++ b/apps/api/src/people/people-fleet.helper.ts @@ -1,5 +1,6 @@ import { Logger } from '@nestjs/common'; import { db } from '@db'; +import { orgParticipantMemberWhere } from '../utils/org-participation'; import { FleetService } from '../lib/fleet.service'; const MDM_POLICY_ID = -9999; @@ -115,14 +116,12 @@ export async function getAllEmployeeDevices( organizationId: string, ) { try { + const participantWhere = await orgParticipantMemberWhere(organizationId); const employees = await db.member.findMany({ where: { organizationId, deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + ...participantWhere, }, include: { user: true }, }); diff --git a/apps/api/src/policies/policies.service.ts b/apps/api/src/policies/policies.service.ts index f420dc3ce1..1c264efaec 100644 --- a/apps/api/src/policies/policies.service.ts +++ b/apps/api/src/policies/policies.service.ts @@ -5,14 +5,12 @@ import { NotFoundException, } from '@nestjs/common'; import { db, Frequency, PolicyStatus, Prisma } from '@db'; -import { - HeadObjectCommand, - PutObjectCommand, -} from '@aws-sdk/client-s3'; +import { HeadObjectCommand, PutObjectCommand } from '@aws-sdk/client-s3'; import { PDFDocument, rgb, StandardFonts } from 'pdf-lib'; import { AttachmentsService } from '../attachments/attachments.service'; import { PolicyPdfRendererService } from '../trust-portal/policy-pdf-renderer.service'; import { filterComplianceMembers } from '../utils/compliance-filters'; +import { isMemberOrgParticipant } from '../utils/org-participation'; import { BUCKET_NAME, getSignedUrl, s3Client } from '../app/s3'; import type { CreatePolicyDto } from './dto/create-policy.dto'; import type { UpdatePolicyDto } from './dto/update-policy.dto'; @@ -99,9 +97,7 @@ export class PoliciesService { name: true, description: true, status: true, - ...(excludeContent - ? {} - : { content: true, draftContent: true }), + ...(excludeContent ? {} : { content: true, draftContent: true }), frequency: true, department: true, isRequiredToSign: true, @@ -215,7 +211,10 @@ export class PoliciesService { organizationId, timelinesService: this.timelinesService, }).catch((err) => { - this.logger.warn('timeline auto-complete check failed after publish-all', err); + this.logger.warn( + 'timeline auto-complete check failed after publish-all', + err, + ); }); return { @@ -301,7 +300,10 @@ export class PoliciesService { where: { id: createData.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (assignee?.user.role === 'admin') { + if ( + assignee && + !(await isMemberOrgParticipant(assignee.user.role, organizationId)) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); @@ -757,9 +759,10 @@ export class PoliciesService { sourceVersion = requestedVersion; } - const contentForVersion = (sourceVersion - ? (sourceVersion.content as Prisma.InputJsonValue[]) - : (policy.content as Prisma.InputJsonValue[])) ?? []; + const contentForVersion = + (sourceVersion + ? (sourceVersion.content as Prisma.InputJsonValue[]) + : (policy.content as Prisma.InputJsonValue[])) ?? []; const sourcePdfUrl = sourceVersion?.pdfUrl ?? policy.pdfUrl; // S3 copy is done AFTER the transaction to prevent orphaned files on retry @@ -1116,8 +1119,8 @@ export class PoliciesService { organizationId, timelinesService: this.timelinesService, }).catch((err) => { - this.logger.warn('timeline auto-complete check failed', err); - }); + this.logger.warn('timeline auto-complete check failed', err); + }); return result; } catch (error) { @@ -1180,8 +1183,8 @@ export class PoliciesService { organizationId, timelinesService: this.timelinesService, }).catch((err) => { - this.logger.warn('timeline auto-complete check failed', err); - }); + this.logger.warn('timeline auto-complete check failed', err); + }); return { versionId: version.id, @@ -1236,12 +1239,12 @@ export class PoliciesService { ); } - // Cannot assign a platform admin as approver + // Cannot assign a platform admin as approver (unless this is an internal org) const approverUser = await db.user.findUnique({ where: { id: approver.userId }, select: { role: true }, }); - if (approverUser?.role === 'admin') { + if (!(await isMemberOrgParticipant(approverUser?.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as approver', ); @@ -1340,8 +1343,8 @@ export class PoliciesService { organizationId, timelinesService: this.timelinesService, }).catch((err) => { - this.logger.warn('timeline auto-complete check failed', err); - }); + this.logger.warn('timeline auto-complete check failed', err); + }); // Publishing cleared signedBy[] above, so everyone with the compliance // obligation must (re-)acknowledge the new version. Surface that audience so @@ -1491,10 +1494,7 @@ export class PoliciesService { /** * Download all published policies as a single PDF bundle (no watermark) */ - async downloadAllPoliciesPdf( - organizationId: string, - policyIds?: string[], - ) { + async downloadAllPoliciesPdf(organizationId: string, policyIds?: string[]) { // Get organization info const organization = await db.organization.findUnique({ where: { id: organizationId }, @@ -1511,9 +1511,7 @@ export class PoliciesService { organizationId, isArchived: false, archivedAt: null, - ...(policyIds && policyIds.length > 0 - ? { id: { in: policyIds } } - : {}), + ...(policyIds && policyIds.length > 0 ? { id: { in: policyIds } } : {}), }, select: { id: true, @@ -1831,10 +1829,7 @@ export class PoliciesService { select: { id: true, version: true }, }); if (!version) throw new NotFoundException('Version not found'); - if ( - version.id === policy.currentVersionId && - policy.status !== 'draft' - ) { + if (version.id === policy.currentVersionId && policy.status !== 'draft') { throw new BadRequestException( 'Cannot upload PDF to the published version', ); diff --git a/apps/api/src/risks/risks.service.ts b/apps/api/src/risks/risks.service.ts index e4d29cff92..5c5fe3c9bb 100644 --- a/apps/api/src/risks/risks.service.ts +++ b/apps/api/src/risks/risks.service.ts @@ -5,6 +5,7 @@ import { Logger, } from '@nestjs/common'; import { db, Prisma } from '@db'; +import { isMemberOrgParticipant } from '../utils/org-participation'; import { CreateRiskDto } from './dto/create-risk.dto'; import { GetRisksQueryDto } from './dto/get-risks-query.dto'; import { UpdateRiskDto } from './dto/update-risk.dto'; @@ -39,7 +40,10 @@ export class RisksService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (member?.user.role === 'admin') { + if ( + member && + !(await isMemberOrgParticipant(member.user.role, organizationId)) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); diff --git a/apps/api/src/soa/soa.service.spec.ts b/apps/api/src/soa/soa.service.spec.ts index 8a9b69ff3b..ae0a199ad1 100644 --- a/apps/api/src/soa/soa.service.spec.ts +++ b/apps/api/src/soa/soa.service.spec.ts @@ -26,6 +26,7 @@ jest.mock('@db', () => ({ }, member: { findFirst: jest.fn() }, user: { findUnique: jest.fn() }, + organization: { findUnique: jest.fn() }, sOAAnswer: { findFirst: jest.fn(), create: jest.fn(), update: jest.fn() }, }, })); @@ -366,9 +367,10 @@ describe('SOAService', () => { status: 'completed', }), }); - expect((mockDb.sOADocument.update as jest.Mock).mock.calls[0][0].data.declinedAt).toBeInstanceOf( - Date, - ); + expect( + (mockDb.sOADocument.update as jest.Mock).mock.calls[0][0].data + .declinedAt, + ).toBeInstanceOf(Date); }); }); @@ -480,7 +482,9 @@ describe('SOAService', () => { it('throws NotFoundException when document not found', async () => { (mockDb.sOADocument.findFirst as jest.Mock).mockResolvedValue(null); - await expect(service.exportDocument(dto)).rejects.toThrow(NotFoundException); + await expect(service.exportDocument(dto)).rejects.toThrow( + NotFoundException, + ); }); it('maps document data and delegates to generateSOAExportFile', async () => { diff --git a/apps/api/src/soa/soa.service.ts b/apps/api/src/soa/soa.service.ts index d0c4c1f838..016e433b91 100644 --- a/apps/api/src/soa/soa.service.ts +++ b/apps/api/src/soa/soa.service.ts @@ -7,6 +7,7 @@ import { InternalServerErrorException, } from '@nestjs/common'; import { db } from '@db'; +import { isMemberOrgParticipant } from '../utils/org-participation'; import { SaveSOAAnswerDto } from './dto/save-soa-answer.dto'; import { CreateSOADocumentDto } from './dto/create-soa-document.dto'; import { EnsureSOASetupDto } from './dto/ensure-soa-setup.dto'; @@ -447,7 +448,9 @@ export class SOAService { where: { id: approverMember.userId }, select: { role: true }, }); - if (approverUser?.role === 'admin') { + if ( + !(await isMemberOrgParticipant(approverUser?.role, dto.organizationId)) + ) { throw new BadRequestException( 'Cannot assign a platform admin as approver', ); @@ -554,9 +557,7 @@ export class SOAService { declinedAt: (document as { declinedAt?: Date | null }).declinedAt ?? null, status: document.status, approverName: - document.approver?.user?.name || - document.approver?.user?.email || - null, + document.approver?.user?.name || document.approver?.user?.email || null, }; return generateSOAExportFile( diff --git a/apps/api/src/task-management/task-item-assignment-notifier.service.ts b/apps/api/src/task-management/task-item-assignment-notifier.service.ts index 8abc54e0b4..ac8cb04a5a 100644 --- a/apps/api/src/task-management/task-item-assignment-notifier.service.ts +++ b/apps/api/src/task-management/task-item-assignment-notifier.service.ts @@ -1,4 +1,5 @@ import { db } from '@db'; +import { isOrgParticipant } from '@trycompai/auth/participation'; import { Injectable, Logger } from '@nestjs/common'; import { isUserUnsubscribed } from '@trycompai/email'; import { triggerEmail } from '../email/trigger-email'; @@ -55,7 +56,7 @@ export class TaskItemAssignmentNotifierService { const [organization, assigneeMember, assignedByUser] = await Promise.all([ db.organization.findUnique({ where: { id: organizationId }, - select: { name: true }, + select: { name: true, isInternal: true }, }), db.member.findUnique({ where: { id: assigneeMemberId }, @@ -93,11 +94,17 @@ export class TaskItemAssignmentNotifierService { } // Skip notifications for platform admin members unless they are an owner + // (or this is an internal org, where platform admins are real members) const isOwner = assigneeMember.role ?.split(',') .map((r: string) => r.trim()) .includes('owner'); - if (assigneeUser.role === 'admin' && !isOwner) { + if ( + !isOrgParticipant(assigneeUser.role, { + orgIsInternal: organization?.isInternal ?? false, + }) && + !isOwner + ) { this.logger.log( `Skipping assignment notification: assignee ${assigneeUser.email} is a platform admin (non-owner)`, ); diff --git a/apps/api/src/task-management/task-item-mention-notifier.service.ts b/apps/api/src/task-management/task-item-mention-notifier.service.ts index b45c6605f2..95b30d8cce 100644 --- a/apps/api/src/task-management/task-item-mention-notifier.service.ts +++ b/apps/api/src/task-management/task-item-mention-notifier.service.ts @@ -1,5 +1,6 @@ import { Injectable, Logger } from '@nestjs/common'; import { db } from '@db'; +import { orgParticipantMemberWhere } from '../utils/org-participation'; import { isUserUnsubscribed } from '@trycompai/email'; import { triggerEmail } from '../email/trigger-email'; import { TaskItemMentionedEmail } from '../email/templates/task-item-mentioned'; @@ -50,16 +51,15 @@ export class TaskItemMentionNotifierService { return; } - // Get mentioned users: exclude platform admins unless they are an owner of this org + // Get mentioned users: exclude platform admins unless they are an owner of + // this org (or the org is internal, where platform admins are real members) + const participantWhere = await orgParticipantMemberWhere(organizationId); const mentionedMembers = await db.member.findMany({ where: { organizationId, deactivated: false, user: { id: { in: mentionedUserIds } }, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + ...participantWhere, }, include: { user: true }, }); diff --git a/apps/api/src/task-management/task-management.service.ts b/apps/api/src/task-management/task-management.service.ts index 9960c1504d..e981e48986 100644 --- a/apps/api/src/task-management/task-management.service.ts +++ b/apps/api/src/task-management/task-management.service.ts @@ -1,5 +1,6 @@ import { db } from '@db'; import { TaskItemEntityType, TaskItemStatus, TaskItemPriority } from '@db'; +import { isMemberOrgParticipant } from '../utils/org-participation'; import { BadRequestException, @@ -54,7 +55,11 @@ export class TaskManagementService { orderBy: { createdAt: 'asc' }, }); if (member?.userId) { - return { memberId: member.id, userId: member.userId, viaApiKey: true }; + return { + memberId: member.id, + userId: member.userId, + viaApiKey: true, + }; } } throw new BadRequestException( @@ -304,7 +309,13 @@ export class TaskManagementService { where: { id: createTaskItemDto.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (assigneeMember?.user.role === 'admin') { + if ( + assigneeMember && + !(await isMemberOrgParticipant( + assigneeMember.user.role, + organizationId, + )) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); @@ -513,7 +524,13 @@ export class TaskManagementService { where: { id: updateTaskItemDto.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (assigneeMember?.user.role === 'admin') { + if ( + assigneeMember && + !(await isMemberOrgParticipant( + assigneeMember.user.role, + organizationId, + )) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); diff --git a/apps/api/src/tasks/task-notifier.service.ts b/apps/api/src/tasks/task-notifier.service.ts index 2ace0f781c..721e83bf91 100644 --- a/apps/api/src/tasks/task-notifier.service.ts +++ b/apps/api/src/tasks/task-notifier.service.ts @@ -1,4 +1,5 @@ import { db } from '@db'; +import { orgParticipantMemberWhere } from '../utils/org-participation'; import { Injectable, Logger } from '@nestjs/common'; import { TaskStatus } from '@db'; import { isUserUnsubscribed } from '@trycompai/email'; @@ -1085,6 +1086,7 @@ export class TaskNotifierService { } = params; try { + const participantWhere = await orgParticipantMemberWhere(organizationId); const [organization, task, allMembers] = await Promise.all([ db.organization.findUnique({ where: { id: organizationId }, @@ -1110,10 +1112,7 @@ export class TaskNotifierService { where: { organizationId, deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + ...participantWhere, }, select: { id: true, @@ -1289,6 +1288,7 @@ export class TaskNotifierService { try { const taskIds = failedTasks.map((t) => t.taskId); + const participantWhere = await orgParticipantMemberWhere(organizationId); const [organization, tasks, allMembers] = await Promise.all([ db.organization.findUnique({ @@ -1319,10 +1319,7 @@ export class TaskNotifierService { where: { organizationId, deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + ...participantWhere, }, select: { id: true, diff --git a/apps/api/src/tasks/tasks.service.ts b/apps/api/src/tasks/tasks.service.ts index f6cfa76ba1..771d624d3d 100644 --- a/apps/api/src/tasks/tasks.service.ts +++ b/apps/api/src/tasks/tasks.service.ts @@ -12,6 +12,7 @@ import { TaskResponseDto } from './dto/task-responses.dto'; import { TaskNotifierService } from './task-notifier.service'; import { checkAutoCompletePhases } from '../frameworks/frameworks-timeline.helper'; import { TimelinesService } from '../timelines/timelines.service'; +import { isMemberOrgParticipant } from '../utils/org-participation'; function computeNextTaskReviewDate( frequency: TaskFrequency | null | undefined, @@ -472,7 +473,13 @@ export class TasksService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (assigneeMember?.user.role === 'admin') { + if ( + assigneeMember && + !(await isMemberOrgParticipant( + assigneeMember.user.role, + organizationId, + )) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); @@ -675,7 +682,13 @@ export class TasksService { where: { id: updateData.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (assigneeMember?.user.role === 'admin') { + if ( + assigneeMember && + !(await isMemberOrgParticipant( + assigneeMember.user.role, + organizationId, + )) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); @@ -761,8 +774,7 @@ export class TasksService { newValue: updateData.status, ...(updateData.status === TaskStatus.not_relevant && updateData.notRelevantJustification && { - notRelevantJustification: - updateData.notRelevantJustification, + notRelevantJustification: updateData.notRelevantJustification, }), }, }, @@ -1056,7 +1068,7 @@ export class TasksService { throw new BadRequestException('Approver not found or is deactivated'); } - if (approver.user.role === 'admin') { + if (!(await isMemberOrgParticipant(approver.user.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as approver', ); @@ -1135,7 +1147,7 @@ export class TasksService { throw new BadRequestException('Approver not found or is deactivated'); } - if (approver.user.role === 'admin') { + if (!(await isMemberOrgParticipant(approver.user.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as approver', ); diff --git a/apps/api/src/trigger/browser-automation/run-browser-automation.ts b/apps/api/src/trigger/browser-automation/run-browser-automation.ts index eb649e6c6e..b079a2279d 100644 --- a/apps/api/src/trigger/browser-automation/run-browser-automation.ts +++ b/apps/api/src/trigger/browser-automation/run-browser-automation.ts @@ -40,11 +40,15 @@ async function sendTaskStatusChangeEmails(params: { try { // Get organization, task assignee, and org owners - const [organization, task, allMembers] = await Promise.all([ - db.organization.findUnique({ - where: { id: organizationId }, - select: { name: true }, - }), + // Internal (platform-operated) orgs treat platform admins as real members, + // so they should also receive task notifications. Resolve before building + // the member query since the where-clause depends on it. + const organization = await db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true, isInternal: true }, + }); + const orgIsInternal = organization?.isInternal ?? false; + const [task, allMembers] = await Promise.all([ db.task.findUnique({ where: { id: taskId }, select: { @@ -65,7 +69,7 @@ async function sendTaskStatusChangeEmails(params: { where: { organizationId, deactivated: false, - user: { role: { not: 'admin' } }, + ...(orgIsInternal ? {} : { user: { role: { not: 'admin' } } }), }, select: { role: true, diff --git a/apps/api/src/utils/compliance-filters.ts b/apps/api/src/utils/compliance-filters.ts index dd64b3c948..c352aebe01 100644 --- a/apps/api/src/utils/compliance-filters.ts +++ b/apps/api/src/utils/compliance-filters.ts @@ -2,8 +2,10 @@ import { BUILT_IN_ROLE_OBLIGATIONS, type RoleObligations, allRoles, + isOrgParticipant, } from '@trycompai/auth'; import { db } from '@db'; +import { getOrgIsInternal } from './org-participation'; /** * Check if any of the given role names have the compliance obligation, @@ -42,6 +44,8 @@ export async function filterComplianceMembers( ): Promise { if (members.length === 0) return []; + const orgIsInternal = await getOrgIsInternal(organizationId); + // Collect all custom role names const allCustomRoleNames = new Set(); const builtInRoleNames = new Set(Object.keys(allRoles)); @@ -75,8 +79,9 @@ export async function filterComplianceMembers( return memberRoles .filter(({ member, roleNames }) => { - // Platform admins are excluded — they join customer orgs to debug - if (member.user?.role === 'admin') return false; + // Platform admins are excluded — they join customer orgs to debug — unless + // this is an internal (platform-operated) org where they are real members. + if (!isOrgParticipant(member.user?.role, { orgIsInternal })) return false; return hasComplianceObligation(roleNames, customObligationMap); }) .map(({ member }) => member); diff --git a/apps/api/src/utils/org-participation.spec.ts b/apps/api/src/utils/org-participation.spec.ts new file mode 100644 index 0000000000..32d11d3d4f --- /dev/null +++ b/apps/api/src/utils/org-participation.spec.ts @@ -0,0 +1,73 @@ +jest.mock('@db', () => ({ + db: { organization: { findUnique: jest.fn() } }, + Prisma: {}, +})); + +import { db } from '@db'; +import { + getOrgIsInternal, + isMemberOrgParticipant, + orgParticipantMemberWhere, +} from './org-participation'; + +const orgFindUnique = db.organization.findUnique as jest.Mock; + +describe('org-participation', () => { + beforeEach(() => jest.clearAllMocks()); + + describe('getOrgIsInternal', () => { + it('returns true when the org is internal', async () => { + orgFindUnique.mockResolvedValue({ isInternal: true }); + await expect(getOrgIsInternal('org_1')).resolves.toBe(true); + }); + + it('returns false when the org is not internal', async () => { + orgFindUnique.mockResolvedValue({ isInternal: false }); + await expect(getOrgIsInternal('org_1')).resolves.toBe(false); + }); + + it('returns false when the org is not found', async () => { + orgFindUnique.mockResolvedValue(null); + await expect(getOrgIsInternal('missing')).resolves.toBe(false); + }); + }); + + describe('isMemberOrgParticipant', () => { + it('excludes a platform admin in a customer org', async () => { + orgFindUnique.mockResolvedValue({ isInternal: false }); + await expect(isMemberOrgParticipant('admin', 'org_1')).resolves.toBe( + false, + ); + }); + + it('includes a platform admin in an internal org', async () => { + orgFindUnique.mockResolvedValue({ isInternal: true }); + await expect(isMemberOrgParticipant('admin', 'org_1')).resolves.toBe( + true, + ); + }); + + it('includes non-admins regardless of org type', async () => { + orgFindUnique.mockResolvedValue({ isInternal: false }); + await expect(isMemberOrgParticipant('user', 'org_1')).resolves.toBe(true); + await expect(isMemberOrgParticipant(null, 'org_1')).resolves.toBe(true); + }); + }); + + describe('orgParticipantMemberWhere', () => { + it('returns an empty fragment for internal orgs (no exclusion)', async () => { + orgFindUnique.mockResolvedValue({ isInternal: true }); + await expect(orgParticipantMemberWhere('org_1')).resolves.toEqual({}); + }); + + it('excludes platform admins but keeps owner-admins for customer orgs', async () => { + orgFindUnique.mockResolvedValue({ isInternal: false }); + await expect(orgParticipantMemberWhere('org_1')).resolves.toEqual({ + OR: [ + { user: { role: { not: 'admin' } } }, + { role: { contains: 'owner' } }, + ], + }); + }); + }); +}); diff --git a/apps/api/src/utils/org-participation.ts b/apps/api/src/utils/org-participation.ts new file mode 100644 index 0000000000..d8183a514f --- /dev/null +++ b/apps/api/src/utils/org-participation.ts @@ -0,0 +1,54 @@ +import { db, Prisma } from '@db'; +// Import from the dedicated subpath (not the package index) so this stays free +// of better-auth — keeps the util light and Jest-loadable without ESM interop. +import { + PLATFORM_ADMIN_ROLE, + isOrgParticipant, +} from '@trycompai/auth/participation'; + +/** + * Resolve whether an organization is platform-operated ("internal", e.g. Comp + * AI's own org). Internal orgs treat platform admins as real participants. + */ +export async function getOrgIsInternal( + organizationId: string, +): Promise { + const org = await db.organization.findUnique({ + where: { id: organizationId }, + select: { isInternal: true }, + }); + return org?.isInternal ?? false; +} + +/** + * Whether a member with the given global `User.role` may participate in the org + * (be an assignee/approver, count toward compliance, receive notifications). + * Fetches the org's internal flag, then delegates to the shared predicate. + */ +export async function isMemberOrgParticipant( + userRole: string | null | undefined, + organizationId: string, +): Promise { + return isOrgParticipant(userRole, { + orgIsInternal: await getOrgIsInternal(organizationId), + }); +} + +/** + * A Prisma `Member` where-fragment that keeps only org participants — i.e. + * everyone except platform admins, but still including platform admins who are + * org owners (they opted into this org). Spread it into a member query's + * `where`. Returns an empty fragment for internal orgs, so platform admins are + * included there. Use for recipient/participant lists (notifications, devices). + */ +export async function orgParticipantMemberWhere( + organizationId: string, +): Promise { + if (await getOrgIsInternal(organizationId)) return {}; + return { + OR: [ + { user: { role: { not: PLATFORM_ADMIN_ROLE } } }, + { role: { contains: 'owner' } }, + ], + }; +} diff --git a/apps/api/src/vendors/vendors.service.ts b/apps/api/src/vendors/vendors.service.ts index 349a2cff9a..0b29477425 100644 --- a/apps/api/src/vendors/vendors.service.ts +++ b/apps/api/src/vendors/vendors.service.ts @@ -12,6 +12,7 @@ import { Prisma } from '@db'; import type { TriggerVendorRiskAssessmentVendorDto } from './dto/trigger-vendor-risk-assessment.dto'; import { resolveTaskCreatorAndAssignee } from '../trigger/vendor/vendor-risk-assessment/assignee'; import { resolveStrategyDescriptionUpdate } from '../risks/strategy-descriptions'; +import { isMemberOrgParticipant } from '../utils/org-participation'; const normalizeWebsite = ( website: string | null | undefined, @@ -225,7 +226,10 @@ export class VendorsService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if (member?.user.role === 'admin') { + if ( + member && + !(await isMemberOrgParticipant(member.user.role, organizationId)) + ) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); diff --git a/apps/app/src/actions/policies/accept-requested-policy-changes.ts b/apps/app/src/actions/policies/accept-requested-policy-changes.ts index 1d7a7c4604..24814a8db7 100644 --- a/apps/app/src/actions/policies/accept-requested-policy-changes.ts +++ b/apps/app/src/actions/policies/accept-requested-policy-changes.ts @@ -1,5 +1,6 @@ 'use server'; +import { getOrgIsInternal } from '@/lib/org-participation'; import { sendNewPolicyEmail } from '@/trigger/tasks/email/new-policy-email'; import { db, PolicyStatus, type Prisma } from '@db/server'; import { tasks } from '@trigger.dev/sdk'; @@ -102,12 +103,13 @@ export const acceptRequestedPolicyChangesAction = authActionClient // Get all active members — the downstream isUserUnsubscribed check // handles role-based notification filtering via the org's notification matrix. + const orgIsInternal = await getOrgIsInternal(session.activeOrganizationId); const members = await db.member.findMany({ where: { organizationId: session.activeOrganizationId, isActive: true, deactivated: false, - user: { role: { not: 'admin' } }, + ...(orgIsInternal ? {} : { user: { role: { not: 'admin' } } }), }, include: { user: true, diff --git a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.tsx b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.tsx index 31b90ec8fd..44dee32377 100644 --- a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.tsx +++ b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.tsx @@ -55,6 +55,7 @@ export interface AdminOrgDetail { onboardingCompleted: boolean; website: string | null; backgroundCheckStepEnabled: boolean; + isInternal: boolean; members: OrgMember[]; } diff --git a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx index 0d9b21b3dc..ab39dce8bf 100644 --- a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx +++ b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx @@ -1,18 +1,12 @@ 'use client'; +import { RecentAuditLogs } from '@/components/RecentAuditLogs'; +import type { AuditLogWithRelations } from '@/hooks/use-audit-logs'; +import { apiClient } from '@/lib/api-client'; +import { Badge, Section, Stack, Switch, Text } from '@trycompai/design-system'; import { useState } from 'react'; import { toast } from 'sonner'; import useSWR from 'swr'; -import { apiClient } from '@/lib/api-client'; -import { RecentAuditLogs } from '@/components/RecentAuditLogs'; -import type { AuditLogWithRelations } from '@/hooks/use-audit-logs'; -import { - Badge, - Section, - Stack, - Switch, - Text, -} from '@trycompai/design-system'; interface AdminOrgDetail { id: string; @@ -22,6 +16,7 @@ interface AdminOrgDetail { onboardingCompleted: boolean; members: { id: string }[]; backgroundCheckStepEnabled: boolean; + isInternal: boolean; } export function OrganizationDetail({ @@ -32,20 +27,19 @@ export function OrganizationDetail({ currentOrgId: string; hasAccess: boolean; }) { - const [bgCheckEnabled, setBgCheckEnabled] = useState( - org.backgroundCheckStepEnabled, - ); + const [bgCheckEnabled, setBgCheckEnabled] = useState(org.backgroundCheckStepEnabled); const [savingBgCheck, setSavingBgCheck] = useState(false); + const [isInternal, setIsInternal] = useState(org.isInternal); + const [savingInternal, setSavingInternal] = useState(false); const handleToggleBgCheck = async (next: boolean) => { const previous = bgCheckEnabled; setBgCheckEnabled(next); setSavingBgCheck(true); - const res = await apiClient.patch( - `/v1/admin/organizations/${org.id}`, - { backgroundCheckStepEnabled: next }, - ); + const res = await apiClient.patch(`/v1/admin/organizations/${org.id}`, { + backgroundCheckStepEnabled: next, + }); setSavingBgCheck(false); @@ -55,10 +49,32 @@ export function OrganizationDetail({ return; } + toast.success( + next ? 'Background checks now required' : 'Background checks bypassed for this organization', + ); + }; + + const handleToggleInternal = async (next: boolean) => { + const previous = isInternal; + setIsInternal(next); + setSavingInternal(true); + + const res = await apiClient.patch(`/v1/admin/organizations/${org.id}`, { + isInternal: next, + }); + + setSavingInternal(false); + + if (res.error) { + setIsInternal(previous); + toast.error('Failed to update internal-organization setting'); + return; + } + toast.success( next - ? 'Background checks now required' - : 'Background checks bypassed for this organization', + ? 'Marked as internal — platform admins can now participate here' + : 'Unmarked as internal — platform admins are excluded again', ); }; @@ -81,14 +97,8 @@ export function OrganizationDetail({ variant={hasAccess ? 'default' : 'destructive'} /> - - + +
@@ -96,9 +106,8 @@ export function OrganizationDetail({
Require background checks - When off, this org's members do not need to pass a background - check to count toward people completion. Existing requests stay - accessible. + When off, this org's members do not need to pass a background check to count + toward people completion. Existing requests stay accessible.
+
+
+
+ Internal organization + + For Comp AI-operated orgs only. When on, platform admins are treated as real members + here — assignable, counted in compliance, and notified. Leave off for every customer + organization. + +
+ +
+
+ {isLoading ? (
diff --git a/apps/app/src/app/(app)/[orgId]/layout.tsx b/apps/app/src/app/(app)/[orgId]/layout.tsx index b284e75bea..ea58ead245 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.tsx @@ -1,22 +1,16 @@ import { getFeatureFlags } from '@/app/posthog'; import { APP_AWS_ORG_ASSETS_BUCKET, s3Client } from '@/app/s3'; +import { OrgInternalProvider } from '@/components/org-internal-context'; import { TriggerTokenProvider } from '@/components/trigger-token-provider'; import { serverApi } from '@/lib/api-server'; -import { - canAccessApp, - canAccessAuditorView, - parseRolesString, -} from '@/lib/permissions'; -import { - resolveCustomRolePermissions, - resolveUserPermissions, -} from '@/lib/permissions.server'; +import { canAccessApp, canAccessAuditorView, parseRolesString } from '@/lib/permissions'; +import { resolveCustomRolePermissions, resolveUserPermissions } from '@/lib/permissions.server'; +import { getSignedUrl } from '@/lib/s3-presigner'; import type { OrganizationFromMe } from '@/types'; import { auth } from '@/utils/auth'; import { GetObjectCommand } from '@aws-sdk/client-s3'; -import { getSignedUrl } from '@/lib/s3-presigner'; -import { OrganizationIdentifier, ServerFeatureFlagsProvider } from '@trycompai/analytics'; import { db, Role } from '@db/server'; +import { OrganizationIdentifier, ServerFeatureFlagsProvider } from '@trycompai/analytics'; import dynamic from 'next/dynamic'; import { cookies, headers } from 'next/headers'; import { redirect } from 'next/navigation'; @@ -170,14 +164,8 @@ export default async function Layout({ // audit:read — built-in `auditor` role OR a custom role with explicit // audit:read. Resolve the custom-role permissions once so we don't // second-guess the owner/admin's implicit all-permissions in the UI. - const customRolePermissions = await resolveCustomRolePermissions( - member.role, - requestedOrgId, - ); - const auditorViewVisible = canAccessAuditorView( - member.role, - customRolePermissions, - ); + const customRolePermissions = await resolveCustomRolePermissions(member.role, requestedOrgId); + const auditorViewVisible = canAccessAuditorView(member.role, customRolePermissions); // User data for navbar const user = { @@ -193,25 +181,27 @@ export default async function Layout({ > - - {children} - + + + {children} + + diff --git a/apps/app/src/app/api/people/agent-devices/route.ts b/apps/app/src/app/api/people/agent-devices/route.ts index c44fa85ae4..9bdc95741a 100644 --- a/apps/app/src/app/api/people/agent-devices/route.ts +++ b/apps/app/src/app/api/people/agent-devices/route.ts @@ -1,11 +1,9 @@ +import type { CheckDetails, DeviceWithChecks } from '@/app/(app)/[orgId]/people/devices/types'; +import { getOrgIsInternal } from '@/lib/org-participation'; +import { requireApiPermission } from '@/lib/permissions.server'; import { db } from '@db/server'; +import { daysSinceCheckIn, getDeviceComplianceStatus } from '@trycompai/utils/devices'; import { NextResponse } from 'next/server'; -import { requireApiPermission } from '@/lib/permissions.server'; -import { - daysSinceCheckIn, - getDeviceComplianceStatus, -} from '@trycompai/utils/devices'; -import type { CheckDetails, DeviceWithChecks } from '@/app/(app)/[orgId]/people/devices/types'; /** Maps the DB `DeviceSource` enum to the frontend source discriminant. */ function mapSource(source: string): DeviceWithChecks['source'] { @@ -23,12 +21,13 @@ export async function GET(req: Request) { if (ctx instanceof NextResponse) return ctx; const { organizationId } = ctx; + const orgIsInternal = await getOrgIsInternal(organizationId); const devices = await db.device.findMany({ where: { organizationId, member: { deactivated: false, - NOT: { user: { role: 'admin' } }, + ...(orgIsInternal ? {} : { NOT: { user: { role: 'admin' } } }), }, }, include: { diff --git a/apps/app/src/app/api/people/fleet-hosts/route.ts b/apps/app/src/app/api/people/fleet-hosts/route.ts index 4046f585bb..abe6e9ac2e 100644 --- a/apps/app/src/app/api/people/fleet-hosts/route.ts +++ b/apps/app/src/app/api/people/fleet-hosts/route.ts @@ -1,8 +1,9 @@ +import type { Host } from '@/app/(app)/[orgId]/people/devices/types'; import { getFleetInstance } from '@/lib/fleet'; +import { getOrgIsInternal } from '@/lib/org-participation'; +import { requireApiPermission } from '@/lib/permissions.server'; import { db } from '@db/server'; import { NextResponse } from 'next/server'; -import { requireApiPermission } from '@/lib/permissions.server'; -import type { Host } from '@/app/(app)/[orgId]/people/devices/types'; const MDM_POLICY_ID = -9999; @@ -15,11 +16,12 @@ export async function GET(req: Request) { const fleet = await getFleetInstance(); + const orgIsInternal = await getOrgIsInternal(organizationId); const employees = await db.member.findMany({ where: { organizationId, deactivated: false, - NOT: { user: { role: 'admin' } }, + ...(orgIsInternal ? {} : { NOT: { user: { role: 'admin' } } }), }, include: { user: true }, }); @@ -70,48 +72,43 @@ export async function GET(req: Request) { } } - const data: Host[] = devices.map( - (device: { data: { host: Host } }, index: number) => { - const host = device.data.host; - const platform = host.platform?.toLowerCase(); - const osVersion = host.os_version?.toLowerCase(); - const isMacOS = - platform === 'darwin' || - platform === 'macos' || - platform === 'osx' || - osVersion?.includes('mac'); - return { - ...host, - user_name: userNames[index], - member_id: memberIds[index], - policies: [ - ...(host.policies || []), - ...(isMacOS - ? [ - { - id: MDM_POLICY_ID, - name: 'MDM Enabled', - response: host.mdm?.connected_to_fleet ? 'pass' : 'fail', - }, - ] - : []), - ].map((policy) => { - const policyResult = resultIndex.get( - `${userIds[index]}:${policy.id}`, - ); - return { - ...policy, - response: - policy.response === 'pass' || - policyResult?.fleetPolicyResponse === 'pass' - ? 'pass' - : 'fail', - attachments: policyResult?.attachments || [], - }; - }), - }; - }, - ); + const data: Host[] = devices.map((device: { data: { host: Host } }, index: number) => { + const host = device.data.host; + const platform = host.platform?.toLowerCase(); + const osVersion = host.os_version?.toLowerCase(); + const isMacOS = + platform === 'darwin' || + platform === 'macos' || + platform === 'osx' || + osVersion?.includes('mac'); + return { + ...host, + user_name: userNames[index], + member_id: memberIds[index], + policies: [ + ...(host.policies || []), + ...(isMacOS + ? [ + { + id: MDM_POLICY_ID, + name: 'MDM Enabled', + response: host.mdm?.connected_to_fleet ? 'pass' : 'fail', + }, + ] + : []), + ].map((policy) => { + const policyResult = resultIndex.get(`${userIds[index]}:${policy.id}`); + return { + ...policy, + response: + policy.response === 'pass' || policyResult?.fleetPolicyResponse === 'pass' + ? 'pass' + : 'fail', + attachments: policyResult?.attachments || [], + }; + }), + }; + }); return NextResponse.json({ data }); } diff --git a/apps/app/src/components/SelectAssignee.tsx b/apps/app/src/components/SelectAssignee.tsx index 0dcece0260..1c1f09f43e 100644 --- a/apps/app/src/components/SelectAssignee.tsx +++ b/apps/app/src/components/SelectAssignee.tsx @@ -1,7 +1,8 @@ +import { useOrgIsInternal } from '@/components/org-internal-context'; import { authClient } from '@/utils/auth-client'; +import { Member, User } from '@db'; import { Avatar, AvatarFallback, AvatarImage } from '@trycompai/ui/avatar'; import { Select, SelectContent, SelectItem, SelectTrigger } from '@trycompai/ui/select'; -import { Member, User } from '@db'; import { UserIcon } from 'lucide-react'; import { useEffect, useState } from 'react'; @@ -21,13 +22,13 @@ export const SelectAssignee = ({ withTitle = true, }: SelectAssigneeProps) => { const { data: activeMember } = authClient.useActiveMember(); - // Exclude platform admins from assignee selection + const orgIsInternal = useOrgIsInternal(); + // Exclude platform admins from assignee selection — except in internal + // (platform-operated) orgs, where they are real members. const assignees = rawAssignees - .filter((a) => a.user.role !== 'admin') + .filter((a) => orgIsInternal || a.user.role !== 'admin') .sort((a, b) => - (a.user.name || a.user.email || '').localeCompare( - b.user.name || b.user.email || '', - ), + (a.user.name || a.user.email || '').localeCompare(b.user.name || b.user.email || ''), ); const [selectedAssignee, setSelectedAssignee] = useState<(Member & { user: User }) | null>(null); @@ -124,11 +125,7 @@ export const SelectAssignee = ({
{assignees.map((assignee) => ( - +
(false); + +export function OrgInternalProvider({ + isInternal, + children, +}: { + isInternal: boolean; + children: ReactNode; +}) { + return ( + {children} + ); +} + +export function useOrgIsInternal(): boolean { + return useContext(OrgIsInternalContext); +} diff --git a/apps/app/src/lib/compliance.ts b/apps/app/src/lib/compliance.ts index d916451fb8..6ce5efc984 100644 --- a/apps/app/src/lib/compliance.ts +++ b/apps/app/src/lib/compliance.ts @@ -1,7 +1,8 @@ import 'server-only'; -import { BUILT_IN_ROLE_OBLIGATIONS } from '@trycompai/auth'; import { db } from '@db/server'; +import { BUILT_IN_ROLE_OBLIGATIONS } from '@trycompai/auth'; +import { getOrgIsInternal, isOrgParticipant } from './org-participation'; import { type UserPermissions, canAccessApp, @@ -51,9 +52,7 @@ async function filterMembersByPermission( .filter((r) => r.permissions) .map((r) => { const parsed = - typeof r.permissions === 'string' - ? JSON.parse(r.permissions) - : r.permissions; + typeof r.permissions === 'string' ? JSON.parse(r.permissions) : r.permissions; return [r.name, parsed as Record]; }), ); @@ -81,6 +80,8 @@ export async function filterComplianceMembers( ): Promise { if (members.length === 0) return []; + const orgIsInternal = await getOrgIsInternal(organizationId); + const builtInRoleNames = new Set(Object.keys(BUILT_IN_ROLE_OBLIGATIONS)); const allRoleNames = new Set(); @@ -101,9 +102,8 @@ export async function filterComplianceMembers( }); obligationMap = Object.fromEntries( dbRoles.map((r) => { - const obligations = typeof r.obligations === 'string' - ? JSON.parse(r.obligations) - : (r.obligations || {}); + const obligations = + typeof r.obligations === 'string' ? JSON.parse(r.obligations) : r.obligations || {}; return [r.name, obligations as Record]; }), ); @@ -111,8 +111,9 @@ export async function filterComplianceMembers( return memberRoles .filter(({ member, roleNames }) => { - // Platform admins are excluded — they join customer orgs to debug - if (member.user?.role === 'admin') return false; + // Platform admins are excluded — they join customer orgs to debug — unless + // this is an internal (platform-operated) org where they are real members. + if (!isOrgParticipant(member.user?.role, { orgIsInternal })) return false; for (const name of roleNames) { // DB override wins, but only if `compliance` is explicitly set — // otherwise fall back to the hardcoded built-in default. diff --git a/apps/app/src/lib/org-participation-rule.test.ts b/apps/app/src/lib/org-participation-rule.test.ts new file mode 100644 index 0000000000..945c429c65 --- /dev/null +++ b/apps/app/src/lib/org-participation-rule.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { isOrgParticipant, PLATFORM_ADMIN_ROLE } from './org-participation-rule'; + +describe('isOrgParticipant', () => { + it('excludes platform admins in a customer org', () => { + expect(isOrgParticipant(PLATFORM_ADMIN_ROLE, { orgIsInternal: false })).toBe(false); + }); + + it('includes platform admins in an internal org', () => { + expect(isOrgParticipant(PLATFORM_ADMIN_ROLE, { orgIsInternal: true })).toBe(true); + }); + + it('includes non-admin roles in any org', () => { + expect(isOrgParticipant('user', { orgIsInternal: false })).toBe(true); + expect(isOrgParticipant('owner', { orgIsInternal: false })).toBe(true); + }); + + it('treats null/undefined roles as participants (not platform admins)', () => { + expect(isOrgParticipant(null, { orgIsInternal: false })).toBe(true); + expect(isOrgParticipant(undefined, { orgIsInternal: false })).toBe(true); + }); + + it('includes everyone in an internal org', () => { + expect(isOrgParticipant('admin', { orgIsInternal: true })).toBe(true); + expect(isOrgParticipant('user', { orgIsInternal: true })).toBe(true); + expect(isOrgParticipant(null, { orgIsInternal: true })).toBe(true); + }); +}); diff --git a/apps/app/src/lib/org-participation-rule.ts b/apps/app/src/lib/org-participation-rule.ts new file mode 100644 index 0000000000..e3cebdf8d9 --- /dev/null +++ b/apps/app/src/lib/org-participation-rule.ts @@ -0,0 +1,23 @@ +/** + * Pure, dependency-free org-participation rule. + * + * This is a deliberate mirror of `packages/auth/src/participation.ts`. The app + * cannot import `@trycompai/auth` from files that end up in the Trigger.dev + * bundle (that package pulls in better-auth, which the deploy pipeline can't + * bundle — see policy-acknowledgment-digest-helpers.ts for the same pattern). + * Keeping this rule free of imports lets both RSC/server code and Trigger.dev + * tasks share one implementation. KEEP IN SYNC with the auth package version. + * + * Platform admins (`User.role === 'admin'`) are Comp AI staff embedded in + * customer orgs for support; they are excluded from an org's business logic + * UNLESS the org is internal (platform-operated, e.g. Comp AI's own org). + */ +export const PLATFORM_ADMIN_ROLE = 'admin'; + +export function isOrgParticipant( + userRole: string | null | undefined, + { orgIsInternal }: { orgIsInternal: boolean }, +): boolean { + if (orgIsInternal) return true; + return userRole !== PLATFORM_ADMIN_ROLE; +} diff --git a/apps/app/src/lib/org-participation.ts b/apps/app/src/lib/org-participation.ts new file mode 100644 index 0000000000..7f97696533 --- /dev/null +++ b/apps/app/src/lib/org-participation.ts @@ -0,0 +1,20 @@ +import { db } from '@db/server'; +import { isOrgParticipant } from './org-participation-rule'; + +/** + * App-side mirror of the API's org-participation helpers. See + * `packages/auth/src/participation.ts` for the shared rule: platform admins are + * Comp AI staff embedded in customer orgs and are excluded from an org's + * business logic, UNLESS the org is internal (platform-operated, e.g. Comp AI's + * own org) where they are real members. + */ +export { isOrgParticipant }; + +/** Whether an org is platform-operated ("internal", e.g. Comp AI's own org). */ +export async function getOrgIsInternal(organizationId: string): Promise { + const org = await db.organization.findUnique({ + where: { id: organizationId }, + select: { isInternal: true }, + }); + return org?.isInternal ?? false; +} diff --git a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts index f7f2662fa9..8ee45042ed 100644 --- a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts +++ b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts @@ -1,3 +1,4 @@ +import { isOrgParticipant } from '@/lib/org-participation-rule'; import { VendorStatus, db } from '@db/server'; import { logger, metadata, queue, tags, task, tasks } from '@trigger.dev/sdk'; import axios from 'axios'; @@ -49,12 +50,22 @@ export const generateVendorMitigation = task({ // platform admins are hidden from the assignee UI, so skip them too. let assigneeUpdate: { assigneeId: string | null } | Record = {}; if (authorId) { - const author = await db.member.findFirst({ - where: { id: authorId, organizationId }, - include: { user: { select: { role: true } } }, - }); + const [author, org] = await Promise.all([ + db.member.findFirst({ + where: { id: authorId, organizationId }, + include: { user: { select: { role: true } } }, + }), + db.organization.findUnique({ + where: { id: organizationId }, + select: { isInternal: true }, + }), + ]); assigneeUpdate = { - assigneeId: author?.user.role === 'admin' ? null : authorId, + assigneeId: isOrgParticipant(author?.user.role, { + orgIsInternal: org?.isInternal ?? false, + }) + ? authorId + : null, }; } diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts index da26088017..8cea31d1de 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts @@ -2,6 +2,7 @@ * Helper types and pure filter function for the policy acknowledgment digest. * Extracted from the scheduled task for testability. */ +import { isOrgParticipant } from '@/lib/org-participation-rule'; import type { PolicyVisibility } from '@db'; // Inlined from @trycompai/auth to avoid pulling that package into the Trigger.dev bundle. @@ -34,6 +35,7 @@ export async function filterDigestMembersByCompliance( db: ComplianceFilterDb, members: T[], organizationId: string, + orgIsInternal = false, ): Promise { if (members.length === 0) return []; @@ -58,9 +60,7 @@ export async function filterDigestMembersByCompliance( obligationMap = Object.fromEntries( dbRoles.map((r) => { const obligations = - typeof r.obligations === 'string' - ? JSON.parse(r.obligations) - : r.obligations || {}; + typeof r.obligations === 'string' ? JSON.parse(r.obligations) : r.obligations || {}; return [r.name, obligations as { compliance?: boolean }]; }), ); @@ -68,8 +68,9 @@ export async function filterDigestMembersByCompliance( return parsed .filter(({ member, roleNames }) => { - // Platform admins are excluded — matches the @/lib/compliance behavior. - if (member.user?.role === 'admin') return false; + // Platform admins are excluded — matches the @/lib/compliance behavior — + // unless this is an internal (platform-operated) org. + if (!isOrgParticipant(member.user?.role, { orgIsInternal })) return false; for (const name of roleNames) { // DB override wins, but only if `compliance` is explicitly set — // otherwise fall back to the hardcoded built-in default. diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts index 3d8c293a4b..b22e1868f6 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts @@ -18,10 +18,7 @@ import { } from './policy-acknowledgment-digest-helpers'; const getPortalBase = () => - (process.env.NEXT_PUBLIC_PORTAL_URL ?? 'https://portal.trycomp.ai').replace( - /\/+$/, - '', - ); + (process.env.NEXT_PUBLIC_PORTAL_URL ?? 'https://portal.trycomp.ai').replace(/\/+$/, ''); // Skip orgs that look abandoned — same threshold weekly-task-reminder uses so // we don't keep hitting dead addresses and burning domain reputation. @@ -43,9 +40,7 @@ export const policyAcknowledgmentDigest = schedules.task({ maxDuration: 1000 * 60 * 15, // 15 minutes run: async () => { const inactivityCutoff = new Date(); - inactivityCutoff.setDate( - inactivityCutoff.getDate() - ORG_INACTIVITY_DAYS, - ); + inactivityCutoff.setDate(inactivityCutoff.getDate() - ORG_INACTIVITY_DAYS); const organizations = await db.organization.findMany({ where: { @@ -72,6 +67,7 @@ export const policyAcknowledgmentDigest = schedules.task({ select: { id: true, name: true, + isInternal: true, policy: { where: { status: 'published', @@ -124,6 +120,7 @@ export const policyAcknowledgmentDigest = schedules.task({ db as unknown as ComplianceFilterDb, org.members, org.id, + org.isInternal, ); if (complianceMembers.length === 0) continue; @@ -150,9 +147,7 @@ export const policyAcknowledgmentDigest = schedules.task({ if (pendingByMember.length === 0) continue; // One unsubscribe query per org, batched across members. - const emailsWithPending = pendingByMember.map( - (p) => p.member.user.email, - ); + const emailsWithPending = pendingByMember.map((p) => p.member.user.email); const unsubscribedEmails = await getUnsubscribedEmails( db, emailsWithPending, @@ -188,10 +183,7 @@ export const policyAcknowledgmentDigest = schedules.task({ // Render all emails and group by primaryOrgId for batch sending. let emailsSent = 0; let emailsFailed = 0; - const emailsByOrg = new Map< - string, - Array<{ to: string; subject: string; html: string }> - >(); + const emailsByOrg = new Map>(); for (const entry of rollup.values()) { const subject = computePolicyAcknowledgmentDigestSubject(entry.orgs); diff --git a/apps/app/src/trigger/tasks/task/policy-schedule.ts b/apps/app/src/trigger/tasks/task/policy-schedule.ts index 444df004e8..17a838040c 100644 --- a/apps/app/src/trigger/tasks/task/policy-schedule.ts +++ b/apps/app/src/trigger/tasks/task/policy-schedule.ts @@ -1,3 +1,4 @@ +import { isOrgParticipant } from '@/lib/org-participation-rule'; import { db } from '@db/server'; import { Novu } from '@novu/api'; import { logger, schedules } from '@trigger.dev/sdk'; @@ -30,20 +31,19 @@ export const policySchedule = schedules.task({ select: { id: true, name: true, + isInternal: true, members: { where: { deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], }, select: { + role: true, user: { select: { id: true, name: true, email: true, + role: true, }, }, }, @@ -127,21 +127,38 @@ export const policySchedule = schedules.task({ } >(); const addRecipients = ( - users: Array<{ user: { id: string; email: string; name?: string } }>, + members: Array<{ + role: string; + user: { + id: string; + email: string; + name?: string; + role?: string | null; + }; + }>, policy: (typeof overduePolicies)[number], ) => { - for (const entry of users) { + // Exclude platform admins (Comp AI staff) unless they are an org owner + // or the org is internal — same rule as isOrgParticipant elsewhere. + const orgIsInternal = policy.organization?.isInternal ?? false; + for (const entry of members) { const user = entry.user; - if (user && user.email && user.id) { - const key = `${user.id}-${policy.id}`; - if (!recipientsMap.has(key)) { - recipientsMap.set(key, { - email: user.email, - userId: user.id, - name: user.name ?? '', - policy, - }); - } + if (!user?.email || !user.id) continue; + const isOwner = entry.role + ?.split(',') + .map((r) => r.trim()) + .includes('owner'); + if (!isOrgParticipant(user.role, { orgIsInternal }) && !isOwner) { + continue; + } + const key = `${user.id}-${policy.id}`; + if (!recipientsMap.has(key)) { + recipientsMap.set(key, { + email: user.email, + userId: user.id, + name: user.name ?? '', + policy, + }); } } }; diff --git a/apps/app/src/trigger/tasks/task/task-schedule.ts b/apps/app/src/trigger/tasks/task/task-schedule.ts index 872b3cbd28..a80ac832bc 100644 --- a/apps/app/src/trigger/tasks/task/task-schedule.ts +++ b/apps/app/src/trigger/tasks/task/task-schedule.ts @@ -1,3 +1,4 @@ +import { isOrgParticipant } from '@/lib/org-participation-rule'; import { db } from '@db/server'; import { Novu } from '@novu/api'; import { logger, schedules } from '@trigger.dev/sdk'; @@ -33,20 +34,19 @@ export const taskSchedule = schedules.task({ select: { id: true, name: true, + isInternal: true, members: { where: { deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], }, select: { + role: true, user: { select: { id: true, name: true, email: true, + role: true, }, }, }, @@ -196,21 +196,38 @@ export const taskSchedule = schedules.task({ } >(); const addRecipients = ( - users: Array<{ user: { id: string; email: string; name?: string } }>, + members: Array<{ + role: string; + user: { + id: string; + email: string; + name?: string; + role?: string | null; + }; + }>, task: (typeof allUpdatedTasks)[number], ) => { - for (const entry of users) { + // Exclude platform admins (Comp AI staff) unless they are an org owner + // or the org is internal — same rule as isOrgParticipant elsewhere. + const orgIsInternal = task.organization?.isInternal ?? false; + for (const entry of members) { const user = entry.user; - if (user && user.email && user.id) { - const key = `${user.id}-${task.id}`; - if (!recipientsMap.has(key)) { - recipientsMap.set(key, { - email: user.email, - userId: user.id, - name: user.name ?? '', - task, - }); - } + if (!user?.email || !user.id) continue; + const isOwner = entry.role + ?.split(',') + .map((r) => r.trim()) + .includes('owner'); + if (!isOrgParticipant(user.role, { orgIsInternal }) && !isOwner) { + continue; + } + const key = `${user.id}-${task.id}`; + if (!recipientsMap.has(key)) { + recipientsMap.set(key, { + email: user.email, + userId: user.id, + name: user.name ?? '', + task, + }); } } }; @@ -238,7 +255,12 @@ export const taskSchedule = schedules.task({ : ('todo' as const); // Check if user is unsubscribed - const isUnsubscribed = await isUserUnsubscribed(db, recipient.email, 'taskReminders', recipient.task.organizationId); + const isUnsubscribed = await isUserUnsubscribed( + db, + recipient.email, + 'taskReminders', + recipient.task.organizationId, + ); if (isUnsubscribed) { logger.info( diff --git a/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts b/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts index f1466eac6a..397b983fff 100644 --- a/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts +++ b/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts @@ -1,3 +1,4 @@ +import { isOrgParticipant } from '@/lib/org-participation-rule'; import { db } from '@db/server'; import { logger, schedules } from '@trigger.dev/sdk'; import { sendWeeklyTaskDigestEmailTask } from '../email/weekly-task-digest-email'; @@ -36,21 +37,20 @@ export const weeklyTaskReminder = schedules.task({ select: { id: true, name: true, + isInternal: true, members: { where: { deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], }, select: { id: true, + role: true, user: { select: { id: true, name: true, email: true, + role: true, }, }, }, @@ -58,7 +58,9 @@ export const weeklyTaskReminder = schedules.task({ }, }); - logger.info(`Found ${organizations.length} active organizations to process (skipped orgs with no sessions in ${ORG_INACTIVITY_DAYS} days)`); + logger.info( + `Found ${organizations.length} active organizations to process (skipped orgs with no sessions in ${ORG_INACTIVITY_DAYS} days)`, + ); // Build email payloads for all members with TODO tasks const emailPayloads = []; @@ -97,6 +99,21 @@ export const weeklyTaskReminder = schedules.task({ continue; } + // Exclude platform admins (Comp AI staff) unless they are an org owner + // or the org is internal — same rule as isOrgParticipant elsewhere. + const isOwner = member.role + .split(',') + .map((r) => r.trim()) + .includes('owner'); + if ( + !isOrgParticipant(member.user.role, { + orgIsInternal: org.isInternal, + }) && + !isOwner + ) { + continue; + } + emailPayloads.push({ payload: { email: member.user.email, diff --git a/packages/auth/package.json b/packages/auth/package.json index f561f7c08b..cbe88d748a 100644 --- a/packages/auth/package.json +++ b/packages/auth/package.json @@ -13,6 +13,10 @@ "types": "./dist/permissions.d.ts", "default": "./dist/permissions.js" }, + "./participation": { + "types": "./dist/participation.d.ts", + "default": "./dist/participation.js" + }, "./server": { "types": "./dist/server.d.ts", "default": "./dist/server.js" diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index 489deab2ae..465fbba06d 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -1,23 +1,30 @@ export { + BUILT_IN_ROLE_OBLIGATIONS, + BUILT_IN_ROLE_PERMISSIONS, + PRIVILEGED_ROLES, + RESTRICTED_ROLES, + ROLE_HIERARCHY, ac, - statement, - owner, admin, + allRoles, auditor, - employee, contractor, - allRoles, - ROLE_HIERARCHY, - RESTRICTED_ROLES, + employee, isRestrictedRole, - PRIVILEGED_ROLES, - BUILT_IN_ROLE_PERMISSIONS, - BUILT_IN_ROLE_OBLIGATIONS, + owner, + parseRoleObligations, + parseRolePermissions, + statement, type RoleName, type RoleObligations, type RolePermissions, - parseRolePermissions, - parseRoleObligations, } from './permissions'; -export { createAuthServer, type CreateAuthServerOptions, type AuthServer } from './server'; +export { createAuthServer, type AuthServer, type CreateAuthServerOptions } from './server'; + +export { + PLATFORM_ADMIN_ROLE, + isExcludedFromOrgParticipation, + isOrgParticipant, + type OrgParticipationContext, +} from './participation'; diff --git a/packages/auth/src/participation.ts b/packages/auth/src/participation.ts new file mode 100644 index 0000000000..ae48fd624b --- /dev/null +++ b/packages/auth/src/participation.ts @@ -0,0 +1,55 @@ +/** + * Organization participation rules. + * + * A *platform admin* is a global Comp AI staff account (`User.role === 'admin'`) + * that can enter any organization for support/debugging. By default such an + * account is NOT a genuine participant of the customer orgs it enters, so it is + * excluded from that org's business logic: it cannot be an assignee/approver, is + * not counted toward compliance progress, and does not receive org-scoped + * notifications. + * + * The single exception is an *internal* organization — one operated by the + * platform itself (e.g. Comp AI's own org). There, the platform admins ARE the + * real members and must be able to run compliance like any other org. + * + * `isOrgParticipant` is the single source of truth for that rule. Do NOT + * reintroduce inline `user.role === 'admin'` participation checks anywhere — + * call this instead so the internal-org exception stays consistent. + * + * SCOPE: this governs *participation* only. It must never be used to decide + * platform-admin access or privileges — those stay with PlatformAdminGuard / + * `isPlatformAdmin`, which are unaffected by an org being internal. + */ +export const PLATFORM_ADMIN_ROLE = 'admin'; + +export interface OrgParticipationContext { + /** Whether the organization is platform-operated (e.g. Comp AI's own org). */ + orgIsInternal: boolean; +} + +/** + * Returns true when `userRole` should be treated as a real participant of an + * organization with the given context. + * + * @param userRole The global `User.role` value (e.g. `'admin'` for platform + * admins, `'user'`/null otherwise). This is NOT the org-scoped member role. + */ +export function isOrgParticipant( + userRole: string | null | undefined, + { orgIsInternal }: OrgParticipationContext, +): boolean { + if (orgIsInternal) return true; + return userRole !== PLATFORM_ADMIN_ROLE; +} + +/** + * Inverse of {@link isOrgParticipant} — true when the user must be excluded from + * the org's participation-level business logic. Convenience for the many call + * sites whose existing shape is "if (isPlatformAdmin) exclude". + */ +export function isExcludedFromOrgParticipation( + userRole: string | null | undefined, + ctx: OrgParticipationContext, +): boolean { + return !isOrgParticipant(userRole, ctx); +} diff --git a/packages/db/prisma/migrations/20260708120000_add_organization_is_internal/migration.sql b/packages/db/prisma/migrations/20260708120000_add_organization_is_internal/migration.sql new file mode 100644 index 0000000000..ad838a5888 --- /dev/null +++ b/packages/db/prisma/migrations/20260708120000_add_organization_is_internal/migration.sql @@ -0,0 +1,6 @@ +-- Mark platform-operated ("internal") organizations, e.g. Comp AI's own org. +-- When true, platform admins are treated as real participants of the org +-- (assignable, counted in compliance, notified). Defaults to false so every +-- existing and future customer org keeps excluding platform-admin staff. +ALTER TABLE "Organization" + ADD COLUMN "isInternal" BOOLEAN NOT NULL DEFAULT false; diff --git a/packages/db/prisma/schema/organization.prisma b/packages/db/prisma/schema/organization.prisma index b1cf46f429..64d51d49d1 100644 --- a/packages/db/prisma/schema/organization.prisma +++ b/packages/db/prisma/schema/organization.prisma @@ -9,6 +9,10 @@ model Organization { website String? onboardingCompleted Boolean @default(false) hasAccess Boolean @default(false) + // Internal (platform-operated) organization — e.g. Comp AI's own org. + // When true, platform admins are treated as real participants of this org + // (assignable, counted in compliance, notified). Off for all customer orgs. + isInternal Boolean @default(false) advancedModeEnabled Boolean @default(false) evidenceApprovalEnabled Boolean @default(false) deviceAgentStepEnabled Boolean @default(true) From 415ee40786aa104638ed34f27d00b94af3485f09 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 8 Jul 2026 20:33:37 -0400 Subject: [PATCH 2/9] =?UTF-8?q?fix(org):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?align=20browser-automation,=20tighten=20null=20validation,=20ke?= =?UTF-8?q?ep=20dep=20boundary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - run-browser-automation: use the shared `orgParticipantMemberWhere` helper instead of an inline copy, so it matches the other task notifiers (uses the role constant + owner exception) and still includes everyone in internal orgs. - update-admin-organization DTO: reject an explicit `null` isInternal at validation (400) instead of letting it reach the non-null column (500). - org-participation (app): stop re-exporting `isOrgParticipant` from the module that imports `@db/server`; consumers import it from the dependency-free `org-participation-rule` so server-only code can't leak into client/Trigger bundles. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb --- .../dto/update-admin-organization.dto.ts | 6 ++++-- .../browser-automation/run-browser-automation.ts | 13 +++++++------ apps/app/src/lib/compliance.ts | 3 ++- apps/app/src/lib/org-participation.ts | 13 ++++++------- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts b/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts index beaaa75e58..7398270f3b 100644 --- a/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts +++ b/apps/api/src/admin-organizations/dto/update-admin-organization.dto.ts @@ -1,5 +1,5 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsBoolean, IsOptional } from 'class-validator'; +import { IsBoolean, IsOptional, ValidateIf } from 'class-validator'; export class UpdateAdminOrganizationDto { @ApiPropertyOptional({ @@ -22,7 +22,9 @@ export class UpdateAdminOrganizationDto { description: "When true, the organization is platform-operated (e.g. Comp AI's own org). Platform admins are then treated as real participants — assignable, counted in compliance, and notified. Leave false for all customer orgs.", }) - @IsOptional() + // Optional, but reject an explicit `null` at validation (400) rather than + // letting it hit the non-null DB column and 500. + @ValidateIf((_, value) => value !== undefined) @IsBoolean() isInternal?: boolean; } diff --git a/apps/api/src/trigger/browser-automation/run-browser-automation.ts b/apps/api/src/trigger/browser-automation/run-browser-automation.ts index b079a2279d..a6140a81f0 100644 --- a/apps/api/src/trigger/browser-automation/run-browser-automation.ts +++ b/apps/api/src/trigger/browser-automation/run-browser-automation.ts @@ -1,4 +1,5 @@ import { db } from '@db'; +import { orgParticipantMemberWhere } from '../../utils/org-participation'; import { logger, tags, task } from '@trigger.dev/sdk'; import { BrowserbaseService } from '../../browserbase/browserbase.service'; import { triggerEmail } from '../../email/trigger-email'; @@ -40,14 +41,14 @@ async function sendTaskStatusChangeEmails(params: { try { // Get organization, task assignee, and org owners - // Internal (platform-operated) orgs treat platform admins as real members, - // so they should also receive task notifications. Resolve before building - // the member query since the where-clause depends on it. + // Use the shared participation rule so this path stays aligned with the + // other task notifiers: internal (platform-operated) orgs include platform + // admins; other orgs exclude them (except platform admins who are owners). const organization = await db.organization.findUnique({ where: { id: organizationId }, - select: { name: true, isInternal: true }, + select: { name: true }, }); - const orgIsInternal = organization?.isInternal ?? false; + const participantWhere = await orgParticipantMemberWhere(organizationId); const [task, allMembers] = await Promise.all([ db.task.findUnique({ where: { id: taskId }, @@ -69,7 +70,7 @@ async function sendTaskStatusChangeEmails(params: { where: { organizationId, deactivated: false, - ...(orgIsInternal ? {} : { user: { role: { not: 'admin' } } }), + ...participantWhere, }, select: { role: true, diff --git a/apps/app/src/lib/compliance.ts b/apps/app/src/lib/compliance.ts index 6ce5efc984..068b1eb2a6 100644 --- a/apps/app/src/lib/compliance.ts +++ b/apps/app/src/lib/compliance.ts @@ -2,7 +2,8 @@ import 'server-only'; import { db } from '@db/server'; import { BUILT_IN_ROLE_OBLIGATIONS } from '@trycompai/auth'; -import { getOrgIsInternal, isOrgParticipant } from './org-participation'; +import { getOrgIsInternal } from './org-participation'; +import { isOrgParticipant } from './org-participation-rule'; import { type UserPermissions, canAccessApp, diff --git a/apps/app/src/lib/org-participation.ts b/apps/app/src/lib/org-participation.ts index 7f97696533..cb731b0634 100644 --- a/apps/app/src/lib/org-participation.ts +++ b/apps/app/src/lib/org-participation.ts @@ -1,14 +1,13 @@ import { db } from '@db/server'; -import { isOrgParticipant } from './org-participation-rule'; /** - * App-side mirror of the API's org-participation helpers. See - * `packages/auth/src/participation.ts` for the shared rule: platform admins are - * Comp AI staff embedded in customer orgs and are excluded from an org's - * business logic, UNLESS the org is internal (platform-operated, e.g. Comp AI's - * own org) where they are real members. + * App-side server helper for the org-participation rule. This module imports + * `@db/server`, so it must only be used from server code. The pure predicate + * lives in `./org-participation-rule` (dependency-free) — import + * `isOrgParticipant` from there directly so client/Trigger.dev bundles never + * pull `@db/server` in transitively. See `packages/auth/src/participation.ts` + * for the canonical rule. */ -export { isOrgParticipant }; /** Whether an org is platform-operated ("internal", e.g. Comp AI's own org). */ export async function getOrgIsInternal(organizationId: string): Promise { From ef1a53fe51a814d2c7947cc8198b0d5fb2f2f802 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 8 Jul 2026 20:55:14 -0400 Subject: [PATCH 3/9] =?UTF-8?q?fix(org):=20address=202nd=20review=20pass?= =?UTF-8?q?=20=E2=80=94=20harden=20vendor=20assignee,=20unify=20UI=20rule,?= =?UTF-8?q?=20guard=20boundaries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - generate-vendor-mitigation: fail closed — only assign the author when the member actually exists in this org (add `author &&` guard) so a missed lookup can never write an unknown/cross-org member id. - SelectAssignee: use the shared `isOrgParticipant` predicate instead of an inline copy, so the picker can't drift from the backend rule. - lib/org-participation.ts: add `import 'server-only'` (matches compliance.ts / permissions.server.ts) so this DB-backed helper can't be pulled into a client bundle. - Add a drift guard test asserting the app-local participation rule stays identical to `@trycompai/auth/participation`. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb --- apps/app/src/components/SelectAssignee.tsx | 6 ++-- .../src/lib/org-participation-rule.test.ts | 30 +++++++++++++++++++ apps/app/src/lib/org-participation.ts | 2 ++ .../onboarding/generate-vendor-mitigation.ts | 15 ++++++---- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/apps/app/src/components/SelectAssignee.tsx b/apps/app/src/components/SelectAssignee.tsx index 1c1f09f43e..83206c72a9 100644 --- a/apps/app/src/components/SelectAssignee.tsx +++ b/apps/app/src/components/SelectAssignee.tsx @@ -1,4 +1,5 @@ import { useOrgIsInternal } from '@/components/org-internal-context'; +import { isOrgParticipant } from '@/lib/org-participation-rule'; import { authClient } from '@/utils/auth-client'; import { Member, User } from '@db'; import { Avatar, AvatarFallback, AvatarImage } from '@trycompai/ui/avatar'; @@ -24,9 +25,10 @@ export const SelectAssignee = ({ const { data: activeMember } = authClient.useActiveMember(); const orgIsInternal = useOrgIsInternal(); // Exclude platform admins from assignee selection — except in internal - // (platform-operated) orgs, where they are real members. + // (platform-operated) orgs, where they are real members. Uses the shared + // participation rule so the UI stays in sync with the backend. const assignees = rawAssignees - .filter((a) => orgIsInternal || a.user.role !== 'admin') + .filter((a) => isOrgParticipant(a.user.role, { orgIsInternal })) .sort((a, b) => (a.user.name || a.user.email || '').localeCompare(b.user.name || b.user.email || ''), ); diff --git a/apps/app/src/lib/org-participation-rule.test.ts b/apps/app/src/lib/org-participation-rule.test.ts index 945c429c65..1bcb35d80b 100644 --- a/apps/app/src/lib/org-participation-rule.test.ts +++ b/apps/app/src/lib/org-participation-rule.test.ts @@ -1,3 +1,7 @@ +import { + PLATFORM_ADMIN_ROLE as AUTH_PLATFORM_ADMIN_ROLE, + isOrgParticipant as authIsOrgParticipant, +} from '@trycompai/auth/participation'; import { describe, expect, it } from 'vitest'; import { isOrgParticipant, PLATFORM_ADMIN_ROLE } from './org-participation-rule'; @@ -26,3 +30,29 @@ describe('isOrgParticipant', () => { expect(isOrgParticipant(null, { orgIsInternal: true })).toBe(true); }); }); + +// Drift guard: this app-local rule is a deliberate dependency-free mirror of +// `@trycompai/auth/participation` (the app can't import the auth index from +// Trigger.dev-bundled files). Fail CI if the two ever diverge. +describe('org-participation rule stays in sync with @trycompai/auth', () => { + const roles: Array = [ + 'admin', + 'owner', + 'user', + 'auditor', + '', + null, + undefined, + ]; + + it('matches the canonical predicate for every role × internal flag', () => { + expect(PLATFORM_ADMIN_ROLE).toBe(AUTH_PLATFORM_ADMIN_ROLE); + for (const role of roles) { + for (const orgIsInternal of [true, false]) { + expect(isOrgParticipant(role, { orgIsInternal })).toBe( + authIsOrgParticipant(role, { orgIsInternal }), + ); + } + } + }); +}); diff --git a/apps/app/src/lib/org-participation.ts b/apps/app/src/lib/org-participation.ts index cb731b0634..f205b12b6f 100644 --- a/apps/app/src/lib/org-participation.ts +++ b/apps/app/src/lib/org-participation.ts @@ -1,3 +1,5 @@ +import 'server-only'; + import { db } from '@db/server'; /** diff --git a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts index 8ee45042ed..ad700b0b03 100644 --- a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts +++ b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts @@ -60,12 +60,17 @@ export const generateVendorMitigation = task({ select: { isInternal: true }, }), ]); + // Only assign when the author is a real member of THIS org and is a + // participant (fail closed if the lookup missed — never write an unknown + // or cross-org member id). assigneeUpdate = { - assigneeId: isOrgParticipant(author?.user.role, { - orgIsInternal: org?.isInternal ?? false, - }) - ? authorId - : null, + assigneeId: + author && + isOrgParticipant(author.user.role, { + orgIsInternal: org?.isInternal ?? false, + }) + ? authorId + : null, }; } From 77df89cdb61b038bcbc0f615f5f8a6df58657134 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 8 Jul 2026 22:03:14 -0400 Subject: [PATCH 4/9] fix(org): unify participation where-fragment, guard deactivated author, skip needless query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - orgParticipantMemberWhere: make it a faithful Prisma translation of isOrgParticipant — for non-internal orgs exclude only platform admins (include null roles; drop the owner carve-out, which the isInternal flag now supersedes). Removes the split source of truth between the predicate and the query fragment. - generate-vendor-mitigation: add `deactivated: false` to the author lookup so a deactivated member can't be auto-assigned (matches the other trigger tasks). - filterComplianceMembers (app + api): skip the `getOrgIsInternal` query when the member list has no platform admins — the flag can't change the result there. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb --- apps/api/src/utils/compliance-filters.ts | 10 +++++++++- apps/api/src/utils/org-participation.spec.ts | 7 ++----- apps/api/src/utils/org-participation.ts | 17 ++++++++--------- apps/app/src/lib/compliance.ts | 7 +++++-- .../onboarding/generate-vendor-mitigation.ts | 2 +- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/apps/api/src/utils/compliance-filters.ts b/apps/api/src/utils/compliance-filters.ts index c352aebe01..7c864f40d7 100644 --- a/apps/api/src/utils/compliance-filters.ts +++ b/apps/api/src/utils/compliance-filters.ts @@ -1,5 +1,6 @@ import { BUILT_IN_ROLE_OBLIGATIONS, + PLATFORM_ADMIN_ROLE, type RoleObligations, allRoles, isOrgParticipant, @@ -44,7 +45,14 @@ export async function filterComplianceMembers( ): Promise { if (members.length === 0) return []; - const orgIsInternal = await getOrgIsInternal(organizationId); + // The internal flag only changes a platform admin's participation, so skip the + // extra query entirely when the list has no platform admins (the common case). + const hasPlatformAdmin = members.some( + (m) => m.user?.role === PLATFORM_ADMIN_ROLE, + ); + const orgIsInternal = hasPlatformAdmin + ? await getOrgIsInternal(organizationId) + : false; // Collect all custom role names const allCustomRoleNames = new Set(); diff --git a/apps/api/src/utils/org-participation.spec.ts b/apps/api/src/utils/org-participation.spec.ts index 32d11d3d4f..a1a6d4cce5 100644 --- a/apps/api/src/utils/org-participation.spec.ts +++ b/apps/api/src/utils/org-participation.spec.ts @@ -60,13 +60,10 @@ describe('org-participation', () => { await expect(orgParticipantMemberWhere('org_1')).resolves.toEqual({}); }); - it('excludes platform admins but keeps owner-admins for customer orgs', async () => { + it('excludes only platform admins (incl. null roles) for customer orgs', async () => { orgFindUnique.mockResolvedValue({ isInternal: false }); await expect(orgParticipantMemberWhere('org_1')).resolves.toEqual({ - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], + user: { OR: [{ role: { not: 'admin' } }, { role: null }] }, }); }); }); diff --git a/apps/api/src/utils/org-participation.ts b/apps/api/src/utils/org-participation.ts index d8183a514f..cc8de545eb 100644 --- a/apps/api/src/utils/org-participation.ts +++ b/apps/api/src/utils/org-participation.ts @@ -35,20 +35,19 @@ export async function isMemberOrgParticipant( } /** - * A Prisma `Member` where-fragment that keeps only org participants — i.e. - * everyone except platform admins, but still including platform admins who are - * org owners (they opted into this org). Spread it into a member query's - * `where`. Returns an empty fragment for internal orgs, so platform admins are - * included there. Use for recipient/participant lists (notifications, devices). + * A Prisma `Member` where-fragment that keeps only org participants — the SQL + * translation of {@link isOrgParticipant}. Spread it into a member query's + * `where`. Returns an empty fragment for internal orgs (everyone participates). + * For other orgs it excludes only platform admins; `role: { not }` skips NULL in + * SQL, so null roles are included explicitly to match the predicate (a null + * global role is a normal member, not a platform admin). Use for + * recipient/participant lists (notifications, mentions, devices). */ export async function orgParticipantMemberWhere( organizationId: string, ): Promise { if (await getOrgIsInternal(organizationId)) return {}; return { - OR: [ - { user: { role: { not: PLATFORM_ADMIN_ROLE } } }, - { role: { contains: 'owner' } }, - ], + user: { OR: [{ role: { not: PLATFORM_ADMIN_ROLE } }, { role: null }] }, }; } diff --git a/apps/app/src/lib/compliance.ts b/apps/app/src/lib/compliance.ts index 068b1eb2a6..ddc41a4226 100644 --- a/apps/app/src/lib/compliance.ts +++ b/apps/app/src/lib/compliance.ts @@ -3,7 +3,7 @@ import 'server-only'; import { db } from '@db/server'; import { BUILT_IN_ROLE_OBLIGATIONS } from '@trycompai/auth'; import { getOrgIsInternal } from './org-participation'; -import { isOrgParticipant } from './org-participation-rule'; +import { PLATFORM_ADMIN_ROLE, isOrgParticipant } from './org-participation-rule'; import { type UserPermissions, canAccessApp, @@ -81,7 +81,10 @@ export async function filterComplianceMembers( ): Promise { if (members.length === 0) return []; - const orgIsInternal = await getOrgIsInternal(organizationId); + // The internal flag only changes a platform admin's participation, so skip the + // extra query entirely when the list has no platform admins (the common case). + const hasPlatformAdmin = members.some((m) => m.user?.role === PLATFORM_ADMIN_ROLE); + const orgIsInternal = hasPlatformAdmin ? await getOrgIsInternal(organizationId) : false; const builtInRoleNames = new Set(Object.keys(BUILT_IN_ROLE_OBLIGATIONS)); const allRoleNames = new Set(); diff --git a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts index ad700b0b03..6e8d60af12 100644 --- a/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts +++ b/apps/app/src/trigger/tasks/onboarding/generate-vendor-mitigation.ts @@ -52,7 +52,7 @@ export const generateVendorMitigation = task({ if (authorId) { const [author, org] = await Promise.all([ db.member.findFirst({ - where: { id: authorId, organizationId }, + where: { id: authorId, organizationId, deactivated: false }, include: { user: { select: { role: true } } }, }), db.organization.findUnique({ From 26cecbe4c15f0dcf0e5b5e0b24f631408a475980 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 9 Jul 2026 11:27:27 -0400 Subject: [PATCH 5/9] =?UTF-8?q?fix(org):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?spread-safe=20participant=20filter,=20confirm=20internal=20togg?= =?UTF-8?q?le?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - org-participation: return the participant where-fragment wrapped in `AND` (via new `orgParticipantMemberWhereForFlag`) so spreading it no longer overwrites a caller's existing `user` filter. The two mention notifiers constrain `user: { id: { in } }`, which the bare `user` key clobbered, so notifications reached every non-admin member instead of the mentioned ones. Adds a spread-safety unit test. - OrganizationDetail: refresh the server layout (router.refresh) after toggling isInternal for the current org so OrgInternalProvider isn't stale, and require a confirmation dialog before changing this org-wide setting. - run-browser-automation: select `isInternal` in the existing org query and use the pure where-builder, dropping the redundant getOrgIsInternal query. - Normalize participation onto isOrgParticipant everywhere: drop the per-member owner carve-out from weekly-task-reminder, task-schedule, policy-schedule and the task-item assignment notifier (matches the API where-fragment and the predicate); remove now-dead role selections and stale comments. - tests: add a PointerEvent shim to the vitest setup (jsdom lacks it, base-ui Switch needs it), fix admin-org test mocks missing the new isInternal field, and cover the internal-toggle confirmation flow. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XzEVwhhoFghng6SDMGecMq --- .../comment-mention-notifier.service.ts | 6 +- .../task-item-assignment-notifier.service.ts | 14 ++-- .../task-item-mention-notifier.service.ts | 6 +- .../run-browser-automation.ts | 11 +-- apps/api/src/utils/org-participation.spec.ts | 30 +++++++- apps/api/src/utils/org-participation.ts | 41 ++++++++--- .../components/AdminOrgTabs.test.tsx | 1 + .../components/OrganizationDetail.test.tsx | 62 +++++++++++++++++ .../components/OrganizationDetail.tsx | 68 ++++++++++++++++++- apps/app/src/test-utils/setup.ts | 18 +++++ .../src/trigger/tasks/task/policy-schedule.ts | 12 +--- .../src/trigger/tasks/task/task-schedule.ts | 12 +--- .../tasks/task/weekly-task-reminder.ts | 12 +--- 13 files changed, 233 insertions(+), 60 deletions(-) diff --git a/apps/api/src/comments/comment-mention-notifier.service.ts b/apps/api/src/comments/comment-mention-notifier.service.ts index 69d6a47922..8f432f6622 100644 --- a/apps/api/src/comments/comment-mention-notifier.service.ts +++ b/apps/api/src/comments/comment-mention-notifier.service.ts @@ -262,8 +262,10 @@ export class CommentMentionNotifierService { return; } - // Get mentioned users: exclude platform admins unless they are an owner of - // this org (or the org is internal, where platform admins are real members) + // Get mentioned users: exclude platform admins unless the org is internal + // (where platform admins are real members). `orgParticipantMemberWhere` + // returns an AND-wrapped fragment so it intersects with the mentioned-user + // filter above rather than overwriting it. const participantWhere = await orgParticipantMemberWhere(organizationId); const mentionedMembers = await db.member.findMany({ where: { diff --git a/apps/api/src/task-management/task-item-assignment-notifier.service.ts b/apps/api/src/task-management/task-item-assignment-notifier.service.ts index ac8cb04a5a..5fe78653b9 100644 --- a/apps/api/src/task-management/task-item-assignment-notifier.service.ts +++ b/apps/api/src/task-management/task-item-assignment-notifier.service.ts @@ -62,7 +62,6 @@ export class TaskItemAssignmentNotifierService { where: { id: assigneeMemberId }, select: { id: true, - role: true, user: { select: { id: true, @@ -93,20 +92,15 @@ export class TaskItemAssignmentNotifierService { return; } - // Skip notifications for platform admin members unless they are an owner - // (or this is an internal org, where platform admins are real members) - const isOwner = assigneeMember.role - ?.split(',') - .map((r: string) => r.trim()) - .includes('owner'); + // Skip notifications for platform admins unless this is an internal org + // (where platform admins are real members) — the single participation rule. if ( !isOrgParticipant(assigneeUser.role, { orgIsInternal: organization?.isInternal ?? false, - }) && - !isOwner + }) ) { this.logger.log( - `Skipping assignment notification: assignee ${assigneeUser.email} is a platform admin (non-owner)`, + `Skipping assignment notification: assignee ${assigneeUser.email} is a platform admin`, ); return; } diff --git a/apps/api/src/task-management/task-item-mention-notifier.service.ts b/apps/api/src/task-management/task-item-mention-notifier.service.ts index 95b30d8cce..9dced1ae24 100644 --- a/apps/api/src/task-management/task-item-mention-notifier.service.ts +++ b/apps/api/src/task-management/task-item-mention-notifier.service.ts @@ -51,8 +51,10 @@ export class TaskItemMentionNotifierService { return; } - // Get mentioned users: exclude platform admins unless they are an owner of - // this org (or the org is internal, where platform admins are real members) + // Get mentioned users: exclude platform admins unless the org is internal + // (where platform admins are real members). `orgParticipantMemberWhere` + // returns an AND-wrapped fragment so it intersects with the mentioned-user + // filter above rather than overwriting it. const participantWhere = await orgParticipantMemberWhere(organizationId); const mentionedMembers = await db.member.findMany({ where: { diff --git a/apps/api/src/trigger/browser-automation/run-browser-automation.ts b/apps/api/src/trigger/browser-automation/run-browser-automation.ts index a6140a81f0..35373e040e 100644 --- a/apps/api/src/trigger/browser-automation/run-browser-automation.ts +++ b/apps/api/src/trigger/browser-automation/run-browser-automation.ts @@ -1,5 +1,5 @@ import { db } from '@db'; -import { orgParticipantMemberWhere } from '../../utils/org-participation'; +import { orgParticipantMemberWhereForFlag } from '../../utils/org-participation'; import { logger, tags, task } from '@trigger.dev/sdk'; import { BrowserbaseService } from '../../browserbase/browserbase.service'; import { triggerEmail } from '../../email/trigger-email'; @@ -40,15 +40,16 @@ async function sendTaskStatusChangeEmails(params: { const { organizationId, taskId, taskTitle, oldStatus, newStatus } = params; try { - // Get organization, task assignee, and org owners // Use the shared participation rule so this path stays aligned with the // other task notifiers: internal (platform-operated) orgs include platform - // admins; other orgs exclude them (except platform admins who are owners). + // admins; other orgs exclude them. const organization = await db.organization.findUnique({ where: { id: organizationId }, - select: { name: true }, + select: { name: true, isInternal: true }, }); - const participantWhere = await orgParticipantMemberWhere(organizationId); + const participantWhere = orgParticipantMemberWhereForFlag( + organization?.isInternal ?? false, + ); const [task, allMembers] = await Promise.all([ db.task.findUnique({ where: { id: taskId }, diff --git a/apps/api/src/utils/org-participation.spec.ts b/apps/api/src/utils/org-participation.spec.ts index a1a6d4cce5..aa66b9b4a0 100644 --- a/apps/api/src/utils/org-participation.spec.ts +++ b/apps/api/src/utils/org-participation.spec.ts @@ -8,6 +8,7 @@ import { getOrgIsInternal, isMemberOrgParticipant, orgParticipantMemberWhere, + orgParticipantMemberWhereForFlag, } from './org-participation'; const orgFindUnique = db.organization.findUnique as jest.Mock; @@ -54,6 +55,33 @@ describe('org-participation', () => { }); }); + describe('orgParticipantMemberWhereForFlag', () => { + it('returns an empty fragment for internal orgs (no exclusion)', () => { + expect(orgParticipantMemberWhereForFlag(true)).toEqual({}); + }); + + it('excludes only platform admins (incl. null roles) for customer orgs', () => { + expect(orgParticipantMemberWhereForFlag(false)).toEqual({ + AND: [{ user: { OR: [{ role: { not: 'admin' } }, { role: null }] } }], + }); + }); + + it('is spread-safe: does not clobber a caller-supplied user filter', () => { + // Regression: the fragment used to return a bare `user` key, which + // overwrote the mention notifiers' `user: { id: { in } }` filter and + // broadcast to the whole org. `AND` must keep both conditions. + const where = { + organizationId: 'org_1', + user: { id: { in: ['u1', 'u2'] } }, + ...orgParticipantMemberWhereForFlag(false), + }; + expect(where.user).toEqual({ id: { in: ['u1', 'u2'] } }); + expect(where.AND).toEqual([ + { user: { OR: [{ role: { not: 'admin' } }, { role: null }] } }, + ]); + }); + }); + describe('orgParticipantMemberWhere', () => { it('returns an empty fragment for internal orgs (no exclusion)', async () => { orgFindUnique.mockResolvedValue({ isInternal: true }); @@ -63,7 +91,7 @@ describe('org-participation', () => { it('excludes only platform admins (incl. null roles) for customer orgs', async () => { orgFindUnique.mockResolvedValue({ isInternal: false }); await expect(orgParticipantMemberWhere('org_1')).resolves.toEqual({ - user: { OR: [{ role: { not: 'admin' } }, { role: null }] }, + AND: [{ user: { OR: [{ role: { not: 'admin' } }, { role: null }] } }], }); }); }); diff --git a/apps/api/src/utils/org-participation.ts b/apps/api/src/utils/org-participation.ts index cc8de545eb..f8413eac87 100644 --- a/apps/api/src/utils/org-participation.ts +++ b/apps/api/src/utils/org-participation.ts @@ -36,18 +36,39 @@ export async function isMemberOrgParticipant( /** * A Prisma `Member` where-fragment that keeps only org participants — the SQL - * translation of {@link isOrgParticipant}. Spread it into a member query's - * `where`. Returns an empty fragment for internal orgs (everyone participates). - * For other orgs it excludes only platform admins; `role: { not }` skips NULL in - * SQL, so null roles are included explicitly to match the predicate (a null - * global role is a normal member, not a platform admin). Use for - * recipient/participant lists (notifications, mentions, devices). + * translation of {@link isOrgParticipant}, built from an already-resolved + * internal flag. Returns an empty fragment for internal orgs (everyone + * participates). For other orgs it excludes only platform admins; `role: { not }` + * skips NULL in SQL, so null roles are included explicitly to match the + * predicate (a null global role is a normal member, not a platform admin). + * + * The exclusion is wrapped in `AND` rather than a bare `user` key so this + * fragment is safe to spread into a query that already constrains `user` (e.g. + * the mention notifiers filter `user: { id: { in } }`) — a bare `user` key would + * silently overwrite the caller's, broadening the recipient set. `AND` makes + * Prisma intersect the two conditions instead. Use for recipient/participant + * lists (notifications, mentions, devices). + */ +export function orgParticipantMemberWhereForFlag( + orgIsInternal: boolean, +): Prisma.MemberWhereInput { + if (orgIsInternal) return {}; + return { + AND: [ + { user: { OR: [{ role: { not: PLATFORM_ADMIN_ROLE } }, { role: null }] } }, + ], + }; +} + +/** + * Async convenience over {@link orgParticipantMemberWhereForFlag} that resolves + * the org's internal flag first. Prefer the `-ForFlag` variant when the caller + * has already loaded the organization, to avoid a redundant query. */ export async function orgParticipantMemberWhere( organizationId: string, ): Promise { - if (await getOrgIsInternal(organizationId)) return {}; - return { - user: { OR: [{ role: { not: PLATFORM_ADMIN_ROLE } }, { role: null }] }, - }; + return orgParticipantMemberWhereForFlag( + await getOrgIsInternal(organizationId), + ); } diff --git a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.test.tsx b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.test.tsx index e0cf395d69..085951d713 100644 --- a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.test.tsx @@ -39,6 +39,7 @@ const mockOrg: AdminOrgDetail = { onboardingCompleted: true, website: 'https://test.com', backgroundCheckStepEnabled: true, + isInternal: false, members: [ { id: 'mem_1', diff --git a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.test.tsx b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.test.tsx index dbd1ade802..19b71c135d 100644 --- a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.test.tsx @@ -21,6 +21,7 @@ const baseOrg = { onboardingCompleted: true, members: [], backgroundCheckStepEnabled: true, + isInternal: false, }; describe('OrganizationDetail — background-check toggle', () => { @@ -91,3 +92,64 @@ describe('OrganizationDetail — background-check toggle', () => { }); }); }); + +describe('OrganizationDetail — internal-organization toggle', () => { + beforeEach(() => { + patchMock.mockReset(); + patchMock.mockResolvedValue({ data: { success: true } }); + }); + + it('asks for confirmation before saving (no PATCH on the toggle click)', async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click( + screen.getByRole('switch', { name: /internal organization/i }), + ); + + expect(await screen.findByRole('alertdialog')).toBeInTheDocument(); + expect(patchMock).not.toHaveBeenCalled(); + }); + + it('PATCHes isInternal only after the change is confirmed', async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click( + screen.getByRole('switch', { name: /internal organization/i }), + ); + await user.click( + await screen.findByRole('button', { name: /mark as internal/i }), + ); + + await waitFor(() => { + expect(patchMock).toHaveBeenCalledWith('/v1/admin/organizations/org_1', { + isInternal: true, + }); + }); + }); + + it('does not PATCH when the confirmation is canceled', async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click( + screen.getByRole('switch', { name: /internal organization/i }), + ); + await user.click(await screen.findByRole('button', { name: /cancel/i })); + + await waitFor(() => { + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + expect(patchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx index ab39dce8bf..df67801335 100644 --- a/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx +++ b/apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx @@ -3,7 +3,22 @@ import { RecentAuditLogs } from '@/components/RecentAuditLogs'; import type { AuditLogWithRelations } from '@/hooks/use-audit-logs'; import { apiClient } from '@/lib/api-client'; -import { Badge, Section, Stack, Switch, Text } from '@trycompai/design-system'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + Badge, + Section, + Stack, + Switch, + Text, +} from '@trycompai/design-system'; +import { useRouter } from 'next/navigation'; import { useState } from 'react'; import { toast } from 'sonner'; import useSWR from 'swr'; @@ -21,16 +36,19 @@ interface AdminOrgDetail { export function OrganizationDetail({ org, + currentOrgId, hasAccess, }: { org: AdminOrgDetail; currentOrgId: string; hasAccess: boolean; }) { + const router = useRouter(); const [bgCheckEnabled, setBgCheckEnabled] = useState(org.backgroundCheckStepEnabled); const [savingBgCheck, setSavingBgCheck] = useState(false); const [isInternal, setIsInternal] = useState(org.isInternal); const [savingInternal, setSavingInternal] = useState(false); + const [pendingInternal, setPendingInternal] = useState(null); const handleToggleBgCheck = async (next: boolean) => { const previous = bgCheckEnabled; @@ -54,8 +72,17 @@ export function OrganizationDetail({ ); }; - const handleToggleInternal = async (next: boolean) => { + // Toggling `isInternal` changes org-wide membership semantics, so confirm + // first (the switch flips only after the admin confirms). + const handleRequestToggleInternal = (next: boolean) => { + setPendingInternal(next); + }; + + const handleConfirmToggleInternal = async () => { + if (pendingInternal === null) return; + const next = pendingInternal; const previous = isInternal; + setPendingInternal(null); setIsInternal(next); setSavingInternal(true); @@ -71,6 +98,13 @@ export function OrganizationDetail({ return; } + // If this is the org the admin is currently browsing, refresh the server + // layout so OrgInternalProvider (and consumers like the assignee picker) + // reflect the new flag without a full page reload. + if (org.id === currentOrgId) { + router.refresh(); + } + toast.success( next ? 'Marked as internal — platform admins can now participate here' @@ -132,7 +166,7 @@ export function OrganizationDetail({
@@ -152,6 +186,34 @@ export function OrganizationDetail({ ) : ( )} + + { + if (!open) setPendingInternal(null); + }} + > + + + + {pendingInternal + ? 'Mark as internal organization?' + : 'Remove internal organization?'} + + + {pendingInternal + ? 'Platform admins will be treated as real members here — assignable, counted in compliance, and notified. Only enable this for Comp AI-operated orgs, never a customer organization.' + : 'Platform admins will be excluded from this organization again — removed from assignments, compliance counts, and notifications.'} + + + + Cancel + + {pendingInternal ? 'Mark as internal' : 'Remove internal'} + + + + ); } diff --git a/apps/app/src/test-utils/setup.ts b/apps/app/src/test-utils/setup.ts index db80fa28af..db4803924d 100644 --- a/apps/app/src/test-utils/setup.ts +++ b/apps/app/src/test-utils/setup.ts @@ -2,6 +2,24 @@ import '@testing-library/jest-dom/vitest'; import { afterAll, afterEach, beforeAll, vi } from 'vitest'; +// jsdom does not implement PointerEvent, which base-ui components (e.g. Switch) +// dispatch on interaction. Provide a minimal shim so pointer-driven tests work +// instead of throwing "PointerEvent is not defined". +if (typeof globalThis.PointerEvent === 'undefined') { + class PointerEventShim extends MouseEvent { + readonly pointerId: number; + readonly pointerType: string; + + constructor(type: string, params: PointerEventInit = {}) { + super(type, params); + this.pointerId = params.pointerId ?? 0; + this.pointerType = params.pointerType ?? ''; + } + } + + globalThis.PointerEvent = PointerEventShim as unknown as typeof PointerEvent; +} + // Stub `server-only` so modules that use the marker can still be imported // in jsdom test runs (the package throws on client environments by design). vi.mock('server-only', () => ({})); diff --git a/apps/app/src/trigger/tasks/task/policy-schedule.ts b/apps/app/src/trigger/tasks/task/policy-schedule.ts index 17a838040c..8856f4a1ce 100644 --- a/apps/app/src/trigger/tasks/task/policy-schedule.ts +++ b/apps/app/src/trigger/tasks/task/policy-schedule.ts @@ -37,7 +37,6 @@ export const policySchedule = schedules.task({ deactivated: false, }, select: { - role: true, user: { select: { id: true, @@ -128,7 +127,6 @@ export const policySchedule = schedules.task({ >(); const addRecipients = ( members: Array<{ - role: string; user: { id: string; email: string; @@ -138,17 +136,13 @@ export const policySchedule = schedules.task({ }>, policy: (typeof overduePolicies)[number], ) => { - // Exclude platform admins (Comp AI staff) unless they are an org owner - // or the org is internal — same rule as isOrgParticipant elsewhere. + // Exclude platform admins (Comp AI staff) unless the org is internal — + // the single participation rule (no per-member owner carve-out). const orgIsInternal = policy.organization?.isInternal ?? false; for (const entry of members) { const user = entry.user; if (!user?.email || !user.id) continue; - const isOwner = entry.role - ?.split(',') - .map((r) => r.trim()) - .includes('owner'); - if (!isOrgParticipant(user.role, { orgIsInternal }) && !isOwner) { + if (!isOrgParticipant(user.role, { orgIsInternal })) { continue; } const key = `${user.id}-${policy.id}`; diff --git a/apps/app/src/trigger/tasks/task/task-schedule.ts b/apps/app/src/trigger/tasks/task/task-schedule.ts index a80ac832bc..84bf0a551a 100644 --- a/apps/app/src/trigger/tasks/task/task-schedule.ts +++ b/apps/app/src/trigger/tasks/task/task-schedule.ts @@ -40,7 +40,6 @@ export const taskSchedule = schedules.task({ deactivated: false, }, select: { - role: true, user: { select: { id: true, @@ -197,7 +196,6 @@ export const taskSchedule = schedules.task({ >(); const addRecipients = ( members: Array<{ - role: string; user: { id: string; email: string; @@ -207,17 +205,13 @@ export const taskSchedule = schedules.task({ }>, task: (typeof allUpdatedTasks)[number], ) => { - // Exclude platform admins (Comp AI staff) unless they are an org owner - // or the org is internal — same rule as isOrgParticipant elsewhere. + // Exclude platform admins (Comp AI staff) unless the org is internal — + // the single participation rule (no per-member owner carve-out). const orgIsInternal = task.organization?.isInternal ?? false; for (const entry of members) { const user = entry.user; if (!user?.email || !user.id) continue; - const isOwner = entry.role - ?.split(',') - .map((r) => r.trim()) - .includes('owner'); - if (!isOrgParticipant(user.role, { orgIsInternal }) && !isOwner) { + if (!isOrgParticipant(user.role, { orgIsInternal })) { continue; } const key = `${user.id}-${task.id}`; diff --git a/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts b/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts index 397b983fff..6e49fb534a 100644 --- a/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts +++ b/apps/app/src/trigger/tasks/task/weekly-task-reminder.ts @@ -44,7 +44,6 @@ export const weeklyTaskReminder = schedules.task({ }, select: { id: true, - role: true, user: { select: { id: true, @@ -99,17 +98,12 @@ export const weeklyTaskReminder = schedules.task({ continue; } - // Exclude platform admins (Comp AI staff) unless they are an org owner - // or the org is internal — same rule as isOrgParticipant elsewhere. - const isOwner = member.role - .split(',') - .map((r) => r.trim()) - .includes('owner'); + // Exclude platform admins (Comp AI staff) unless the org is internal — + // the single participation rule (no per-member owner carve-out). if ( !isOrgParticipant(member.user.role, { orgIsInternal: org.isInternal, - }) && - !isOwner + }) ) { continue; } From dd071b65f74ee59edb47a78660200f3771a00c71 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 10 Jul 2026 13:30:35 -0400 Subject: [PATCH 6/9] fix(api): reject assignees that aren't members of the organization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assignee guards only threw when a member was found in the org and failed the participant check. When the org-scoped `member.findFirst` returned null — the id belongs to another org or doesn't exist — the guard was skipped and the id was persisted, since assignee columns don't enforce same-org membership. This gap is pre-existing (the prior `member?.user.role === 'admin'` check had the same null hole); hardened here while the participation refactor already touches these guards. Every assignee guard now rejects a non-member with a clear 400. Covers tasks (bulk + single update), task items (create + update), policies (create), vendors, and risks. Approver guards already rejected null members. Also consolidate task-notifier's automation-failure paths, which fetched the org twice (once for isInternal via orgParticipantMemberWhere, once for name), into a single `{ name, isInternal }` lookup + orgParticipantMemberWhereForFlag. Adds a regression test for the cross-org assignee rejection. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XzEVwhhoFghng6SDMGecMq --- apps/api/src/policies/policies.service.ts | 10 ++++--- apps/api/src/risks/risks.service.ts | 10 ++++--- .../task-management.service.ts | 12 ++++++-- apps/api/src/tasks/task-notifier.service.ts | 30 +++++++++++-------- apps/api/src/tasks/tasks.service.spec.ts | 13 ++++++++ apps/api/src/tasks/tasks.service.ts | 12 ++++++-- apps/api/src/vendors/vendors.service.ts | 10 ++++--- 7 files changed, 68 insertions(+), 29 deletions(-) diff --git a/apps/api/src/policies/policies.service.ts b/apps/api/src/policies/policies.service.ts index 1c264efaec..eb0fdefdbe 100644 --- a/apps/api/src/policies/policies.service.ts +++ b/apps/api/src/policies/policies.service.ts @@ -300,10 +300,12 @@ export class PoliciesService { where: { id: createData.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if ( - assignee && - !(await isMemberOrgParticipant(assignee.user.role, organizationId)) - ) { + if (!assignee) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } + if (!(await isMemberOrgParticipant(assignee.user.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); diff --git a/apps/api/src/risks/risks.service.ts b/apps/api/src/risks/risks.service.ts index 5c5fe3c9bb..b237e95c0e 100644 --- a/apps/api/src/risks/risks.service.ts +++ b/apps/api/src/risks/risks.service.ts @@ -40,10 +40,12 @@ export class RisksService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if ( - member && - !(await isMemberOrgParticipant(member.user.role, organizationId)) - ) { + if (!member) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } + if (!(await isMemberOrgParticipant(member.user.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); diff --git a/apps/api/src/task-management/task-management.service.ts b/apps/api/src/task-management/task-management.service.ts index e981e48986..f39b96d6e0 100644 --- a/apps/api/src/task-management/task-management.service.ts +++ b/apps/api/src/task-management/task-management.service.ts @@ -309,8 +309,12 @@ export class TaskManagementService { where: { id: createTaskItemDto.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); + if (!assigneeMember) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } if ( - assigneeMember && !(await isMemberOrgParticipant( assigneeMember.user.role, organizationId, @@ -524,8 +528,12 @@ export class TaskManagementService { where: { id: updateTaskItemDto.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); + if (!assigneeMember) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } if ( - assigneeMember && !(await isMemberOrgParticipant( assigneeMember.user.role, organizationId, diff --git a/apps/api/src/tasks/task-notifier.service.ts b/apps/api/src/tasks/task-notifier.service.ts index 721e83bf91..b981148e54 100644 --- a/apps/api/src/tasks/task-notifier.service.ts +++ b/apps/api/src/tasks/task-notifier.service.ts @@ -1,5 +1,5 @@ import { db } from '@db'; -import { orgParticipantMemberWhere } from '../utils/org-participation'; +import { orgParticipantMemberWhereForFlag } from '../utils/org-participation'; import { Injectable, Logger } from '@nestjs/common'; import { TaskStatus } from '@db'; import { isUserUnsubscribed } from '@trycompai/email'; @@ -1086,12 +1086,14 @@ export class TaskNotifierService { } = params; try { - const participantWhere = await orgParticipantMemberWhere(organizationId); - const [organization, task, allMembers] = await Promise.all([ - db.organization.findUnique({ - where: { id: organizationId }, - select: { name: true }, - }), + const organization = await db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true, isInternal: true }, + }); + const participantWhere = orgParticipantMemberWhereForFlag( + organization?.isInternal ?? false, + ); + const [task, allMembers] = await Promise.all([ db.task.findUnique({ where: { id: taskId }, select: { @@ -1288,13 +1290,15 @@ export class TaskNotifierService { try { const taskIds = failedTasks.map((t) => t.taskId); - const participantWhere = await orgParticipantMemberWhere(organizationId); + const organization = await db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true, isInternal: true }, + }); + const participantWhere = orgParticipantMemberWhereForFlag( + organization?.isInternal ?? false, + ); - const [organization, tasks, allMembers] = await Promise.all([ - db.organization.findUnique({ - where: { id: organizationId }, - select: { name: true }, - }), + const [tasks, allMembers] = await Promise.all([ db.task.findMany({ where: { id: { in: taskIds }, diff --git a/apps/api/src/tasks/tasks.service.spec.ts b/apps/api/src/tasks/tasks.service.spec.ts index 29c3b802a9..4a9ec6a1e5 100644 --- a/apps/api/src/tasks/tasks.service.spec.ts +++ b/apps/api/src/tasks/tasks.service.spec.ts @@ -217,4 +217,17 @@ describe('TasksService approval gating', () => { expect(where.status).toEqual({ not: 'in_review' }); }); }); + + describe('updateTasksAssignee', () => { + it('SECURITY: rejects an assignee that is not a member of the organization', async () => { + // A cross-org or non-existent member id makes the org-scoped lookup return + // null; the guard must reject it rather than persist a foreign assignee. + memberFindFirst.mockResolvedValueOnce(null); + + await expect( + service.updateTasksAssignee(ORG_ID, [TASK_ID], 'mem_other_org', USER_ID), + ).rejects.toThrow('Assignee is not a member of this organization'); + expect(taskUpdateMany).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/tasks/tasks.service.ts b/apps/api/src/tasks/tasks.service.ts index 771d624d3d..5188d8938d 100644 --- a/apps/api/src/tasks/tasks.service.ts +++ b/apps/api/src/tasks/tasks.service.ts @@ -473,8 +473,12 @@ export class TasksService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); + if (!assigneeMember) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } if ( - assigneeMember && !(await isMemberOrgParticipant( assigneeMember.user.role, organizationId, @@ -682,8 +686,12 @@ export class TasksService { where: { id: updateData.assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); + if (!assigneeMember) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } if ( - assigneeMember && !(await isMemberOrgParticipant( assigneeMember.user.role, organizationId, diff --git a/apps/api/src/vendors/vendors.service.ts b/apps/api/src/vendors/vendors.service.ts index 0b29477425..cdf39e5cab 100644 --- a/apps/api/src/vendors/vendors.service.ts +++ b/apps/api/src/vendors/vendors.service.ts @@ -226,10 +226,12 @@ export class VendorsService { where: { id: assigneeId, organizationId }, include: { user: { select: { role: true } } }, }); - if ( - member && - !(await isMemberOrgParticipant(member.user.role, organizationId)) - ) { + if (!member) { + throw new BadRequestException( + 'Assignee is not a member of this organization', + ); + } + if (!(await isMemberOrgParticipant(member.user.role, organizationId))) { throw new BadRequestException( 'Cannot assign a platform admin as assignee', ); From c01351518a61f104f1f39555f872760974dae587 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:24:29 -0400 Subject: [PATCH 7/9] feat(isms): add Roles, Responsibilities & Authorities document (CS-698) (#3391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(isms): add Roles, Responsibilities & Authorities document (CS-698) Add a new ISMS > Roles sub-page (ISO 27001 Clause 5.3): four seeded governance roles (Top Management, SPO, Deputy SPO, Internal Auditor) with pre-filled text, member assignments with per-member competence evidence (Clause 7.2), an Internal Auditor route picker with an independence soft-warning, 1-3 / 4+ team-size bands, and generate-time validation. The Clause 5.3 document renders to branded PDF + DOCX through the existing ISMS export pipeline, including the two auto-generated governance rows and an operational-responsibilities summary read from per-artifact owners. Mirrors the existing ISMS register pattern; reuses the evidence permission resource and the is-isms-enabled flag gate. Role seeding is idempotent (never destructive) so member assignments survive a regenerate. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015bx4urA8n2E6zzBAjEuxwR * fix(isms): address CS-698 review — validation, concurrency, UI hardening - Enforce Clause 5.3 completeness server-side on submit-for-approval (not just the client), so an incomplete Roles doc can't be published by calling the API directly. Validation now iterates the required seeded roles, catching an entirely-missing row, not only present-but-unassigned ones (client + server). - Add a unique (documentId, roleKey) index + skipDuplicates so concurrent first-load provisioning can't double-seed governance roles. - Make the role-assignment idempotency check run inside the per-document lock, so concurrent duplicate adds return the existing row instead of hitting the unique index. - Extract a shared parseOptionalDate util (was duplicated in both role services). - Neutralise the small-team note so it no longer asserts an external audit route that can contradict the customer's chosen route. - Audit-route member pickers: drop the free-text fallback (a member id is required); disable the select when no members exist. - Swallow already-handled mutation rejections in the add-member and delete-role handlers to avoid unhandled promise rejections. - Fix stale "all six ISMS documents" JSDoc. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015bx4urA8n2E6zzBAjEuxwR * fix(isms): harden roles submit gate + strict date parsing (CS-698 review 2) - Server-side clause-5.3 gate now counts only assignments that resolve to an active member, so a required role assigned solely to a deactivated/removed member no longer passes submit-for-approval. - parseOptionalDate now validates a strict YYYY-MM-DD format and round-trips the parsed value, rejecting ambiguous/rolled-over input (e.g. 2026-02-30) instead of relying on permissive Date parsing. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015bx4urA8n2E6zzBAjEuxwR * fix(isms): close CS-698 review round 3 — submit race, active-member + route validation, doc-type scoping - Serialize submit-for-approval: validate completeness and flip status inside one transaction under the per-document lock, and take that same lock on role/ assignment update+remove, so an edit can't invalidate the doc between the check and the needs_review transition (TOCTOU). - Enforce active (non-deactivated) members everywhere: role audit-route member, assignment member, and the completeness gate count only active-member assignments; the client validation mirrors it via the active-member set so the Submit button and the server agree. - Route-specific Internal Auditor validation (client + server): external requires firm/person + evidence reference; in-house requires a member; training-planned requires member + course + due date. - Scope role + assignment register lookups to roles_and_responsibilities documents so role rows can't attach to unrelated ISMS documents. - Generated governance table renders only the four seeded roles + two auto rows (custom roles are managed on the page but not part of the standardized 5.3 table). - AuditRoutePicker: swallow the already-handled save rejection (no unhandled promise rejection). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015bx4urA8n2E6zzBAjEuxwR * fix(isms): audit-route validation counts active members + trims required text (CS-698 review 4) - The Internal Auditor's in-house/training-planned member must now resolve to an ACTIVE member (same active-member set used for assignments), on both the client gate and the server completeness check — a deactivated auditor no longer passes. - Required audit text is trimmed before the check: external firm/person + evidence and the training-planned course can't be satisfied by whitespace via the API. - Unify the active-member logic in roleValidationMessages (assignments + audit member) so client and server agree. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015bx4urA8n2E6zzBAjEuxwR --------- Co-authored-by: Claude Opus 4.8 (1M context) --- apps/api/src/isms/documents/generate.ts | 7 + apps/api/src/isms/documents/registry.ts | 2 + apps/api/src/isms/documents/roles-defaults.ts | 106 +++++ .../src/isms/documents/roles-export-data.ts | 137 ++++++ .../src/isms/documents/roles-export.spec.ts | 100 +++++ apps/api/src/isms/documents/roles.spec.ts | 340 +++++++++++++++ apps/api/src/isms/documents/roles.ts | 391 ++++++++++++++++++ apps/api/src/isms/documents/snapshot.ts | 4 + apps/api/src/isms/documents/types.ts | 48 +++ .../isms/isms-registers.controller.spec.ts | 20 + .../api/src/isms/isms-registers.controller.ts | 32 ++ .../isms/isms-role-assignment.service.spec.ts | 127 ++++++ .../src/isms/isms-role-assignment.service.ts | 210 ++++++++++ apps/api/src/isms/isms-role.service.spec.ts | 139 +++++++ apps/api/src/isms/isms-role.service.ts | 199 +++++++++ .../api/src/isms/isms-version.service.spec.ts | 1 + apps/api/src/isms/isms-version.service.ts | 4 +- apps/api/src/isms/isms.module.ts | 6 + ...isms.service.ensure-setup-fallback.spec.ts | 4 +- .../src/isms/isms.service.lifecycle.spec.ts | 87 +++- apps/api/src/isms/isms.service.ts | 104 ++++- .../src/isms/registers/register-registry.ts | 99 +++++ .../api/src/isms/utils/document-types.spec.ts | 5 +- apps/api/src/isms/utils/document-types.ts | 7 + apps/api/src/isms/utils/export-payload.ts | 62 ++- .../isms/utils/parse-optional-date.spec.ts | 31 ++ .../api/src/isms/utils/parse-optional-date.ts | 33 ++ .../src/isms/wizard/isms-profile.service.ts | 5 +- .../[orgId]/documents/isms/[type]/page.tsx | 11 + .../isms/components/AuditRoutePicker.test.tsx | 101 +++++ .../isms/components/AuditRoutePicker.tsx | 318 ++++++++++++++ .../ContextOfOrganizationClient.test.tsx | 1 + .../InterestedPartiesClient.test.tsx | 1 + .../components/IsmsApprovalSection.test.tsx | 1 + .../isms/components/IsmsApprovalSection.tsx | 20 +- .../components/IsmsControlMappings.test.tsx | 1 + .../isms/components/IsmsDocumentShell.tsx | 9 + .../isms/components/LeadershipClient.test.tsx | 1 + .../isms/components/ObjectivesClient.test.tsx | 1 + .../components/RequirementsClient.test.tsx | 1 + .../components/RoleAssignmentRow.test.tsx | 85 ++++ .../isms/components/RoleAssignmentRow.tsx | 277 +++++++++++++ .../isms/components/RoleAssignments.tsx | 106 +++++ .../documents/isms/components/RoleFields.tsx | 94 +++++ .../documents/isms/components/RolesClient.tsx | 136 ++++++ .../documents/isms/components/RolesForm.tsx | 73 ++++ .../documents/isms/components/RolesRow.tsx | 244 +++++++++++ .../documents/isms/components/RolesTable.tsx | 90 ++++ .../isms/components/ScopeClient.test.tsx | 1 + .../documents/isms/components/role-schema.ts | 18 + .../isms/components/roles-constants.test.ts | 147 +++++++ .../isms/components/roles-constants.ts | 130 ++++++ .../documents/isms/hooks/useIsmsDocument.ts | 4 +- .../[orgId]/documents/isms/isms-types.ts | 53 +++ .../20260710142804_isms_roles/migration.sql | 70 ++++ .../migration.sql | 4 + packages/db/prisma/schema/isms.prisma | 88 ++++ packages/db/prisma/seed/seed.ts | 7 + 58 files changed, 4376 insertions(+), 27 deletions(-) create mode 100644 apps/api/src/isms/documents/roles-defaults.ts create mode 100644 apps/api/src/isms/documents/roles-export-data.ts create mode 100644 apps/api/src/isms/documents/roles-export.spec.ts create mode 100644 apps/api/src/isms/documents/roles.spec.ts create mode 100644 apps/api/src/isms/documents/roles.ts create mode 100644 apps/api/src/isms/isms-role-assignment.service.spec.ts create mode 100644 apps/api/src/isms/isms-role-assignment.service.ts create mode 100644 apps/api/src/isms/isms-role.service.spec.ts create mode 100644 apps/api/src/isms/isms-role.service.ts create mode 100644 apps/api/src/isms/utils/parse-optional-date.spec.ts create mode 100644 apps/api/src/isms/utils/parse-optional-date.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignments.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleFields.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RolesClient.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RolesForm.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RolesRow.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/RolesTable.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/role-schema.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/roles-constants.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/roles-constants.ts create mode 100644 packages/db/prisma/migrations/20260710142804_isms_roles/migration.sql create mode 100644 packages/db/prisma/migrations/20260710150000_isms_role_unique_key/migration.sql diff --git a/apps/api/src/isms/documents/generate.ts b/apps/api/src/isms/documents/generate.ts index 4497aa66f1..14d71f8354 100644 --- a/apps/api/src/isms/documents/generate.ts +++ b/apps/api/src/isms/documents/generate.ts @@ -4,6 +4,7 @@ import { deriveContextOfOrganization } from './context'; import { deriveInterestedParties } from './interested-parties'; import { deriveRequirements } from './requirements'; import { deriveObjectives } from './objectives'; +import { seedRolesIfMissing } from './roles'; import { deriveNarrativeForType, isNarrativeType } from './registry'; import type { IsmsPlatformData } from './types'; @@ -248,6 +249,12 @@ export async function runDerivation({ await generateObjectives({ tx, documentId, data }); return; } + if (type === 'roles_and_responsibilities') { + // Idempotent seed only — never a destructive replace, so member assignments + // (IsmsRoleAssignment) and customer edits survive every regenerate. + await seedRolesIfMissing({ tx, documentId, memberCount: data.memberCount }); + return; + } if (isNarrativeType(type)) { await generateNarrative({ tx, documentId, type, data }); return; diff --git a/apps/api/src/isms/documents/registry.ts b/apps/api/src/isms/documents/registry.ts index 84a8ee8d24..d18b64c8b3 100644 --- a/apps/api/src/isms/documents/registry.ts +++ b/apps/api/src/isms/documents/registry.ts @@ -5,6 +5,7 @@ import { buildContextSections } from './context'; import { buildInterestedPartiesSections } from './interested-parties'; import { buildRequirementsSections } from './requirements'; import { buildObjectivesSections } from './objectives'; +import { buildRolesSections } from './roles'; import { buildScopeSections, deriveScopeNarrative, @@ -31,6 +32,7 @@ const EXPORT_SECTION_BUILDERS: Record< interested_parties_register: buildInterestedPartiesSections, interested_parties_requirements: buildRequirementsSections, objectives_plan: buildObjectivesSections, + roles_and_responsibilities: buildRolesSections, isms_scope: buildScopeSections, leadership_commitment: buildLeadershipSections, }; diff --git a/apps/api/src/isms/documents/roles-defaults.ts b/apps/api/src/isms/documents/roles-defaults.ts new file mode 100644 index 0000000000..1987398a31 --- /dev/null +++ b/apps/api/src/isms/documents/roles-defaults.ts @@ -0,0 +1,106 @@ +import type { SeedRoleDefinition } from './types'; + +/** + * The four seeded ISMS governance roles (clause 5.3) and their pre-filled + * default text. Drafted from the reference document + * ("05a - ISMS Roles, Responsibilities & Authorities") so a customer with no ISO + * expertise gets a workable baseline; every field is editable in the app. + * + * Order is the render/seed order. `roleKey` is the idempotency key for seeding. + */ +export const SEED_ROLE_DEFINITIONS: SeedRoleDefinition[] = [ + { + roleKey: 'top_management', + name: 'Top Management', + description: + 'The executive leadership accountable for the ISMS. Top management sets the direction of the information security programme and provides the mandate and resources for it to operate.', + responsibilities: + 'Approve the ISMS scope, information security policy, and objectives; provide the resources the ISMS needs; direct and support the people contributing to the ISMS; promote continual improvement; and accept or escalate residual risk.', + authorities: + 'Approve the ISMS and accept residual risk on behalf of the organisation.', + authorityGrantedBy: 'The executive office and the Board of Directors', + requiredCompetence: + 'Executive understanding of the organisation, its risk appetite, and its legal and regulatory obligations, sufficient to direct the ISMS and be accountable for it.', + }, + { + roleKey: 'spo', + name: 'Security & Privacy Owner (SPO)', + description: + 'The person who owns and operates the ISMS day to day and is the focal point for information security and privacy across the organisation.', + responsibilities: + 'Operate the ISMS day to day — scope, Statement of Applicability, risk, vendor, policy, training, incident, and audit programmes; ensure the ISMS conforms to ISO/IEC 27001; report ISMS performance to top management; and assign control, risk, and policy owners.', + authorities: + 'Direct the security and privacy programme; assign control, risk, and policy owners; and approve exceptions within the organisation’s risk appetite.', + authorityGrantedBy: 'Top Management', + requiredCompetence: + 'Working knowledge of ISO/IEC 27001 and the organisation’s control environment, with the experience to operate an ISMS and make risk-based decisions within the agreed risk appetite.', + }, + { + roleKey: 'deputy_spo', + name: 'Deputy Security & Privacy Owner', + description: + 'The documented backup for the Security & Privacy Owner, ensuring the ISMS remains covered during the SPO’s absence.', + responsibilities: + 'Provide documented backup for the SPO; act with the SPO’s authority during the SPO’s absence; and participate in incident response and management reviews as required.', + authorities: 'Act with the SPO’s authority during the SPO’s absence.', + authorityGrantedBy: 'Top Management', + requiredCompetence: + 'Sufficient familiarity with the ISMS and the organisation’s controls to stand in for the SPO and make interim risk-based decisions.', + }, + { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + description: + 'The person or party responsible for independently auditing the ISMS to confirm it conforms to ISO/IEC 27001 and is effectively implemented and maintained.', + responsibilities: + 'Plan and conduct internal audits of the ISMS at the intervals defined in the audit programme; report findings and nonconformities; and maintain independence and impartiality from the areas audited.', + authorities: + 'Access the information, systems, and personnel needed to conduct the audit, and report findings directly to top management.', + authorityGrantedBy: 'Top Management', + requiredCompetence: + 'Knowledge of ISO/IEC 27001 auditing (e.g. ISO 19011 principles) and the independence to audit the ISMS objectively; where in-house audit competence is not held, it is engaged externally.', + }, +]; + +/** roleKey values of the seeded roles, for validation and lookups. */ +export const SEED_ROLE_KEYS: string[] = SEED_ROLE_DEFINITIONS.map( + (role) => role.roleKey, +); + +/** + * The two governance rows that appear only in the generated document (never as + * editable role cards): the per-artifact operational owners and the whole + * workforce. Holders/text are fixed. + */ +export const AUTO_DOC_ROLE_ROWS = [ + { + name: 'Control / asset / risk / policy owners', + holders: 'Identified in the platform per policy, control, risk, task, and vendor.', + responsibilities: + 'Implement, operate, and evidence the specific controls, policies, risks, and evidence tasks assigned to them. Operational ownership is assigned at the artifact level in Comp AI.', + authority: 'Make operational decisions for their assigned item. Authority granted by the SPO.', + }, + { + name: 'All personnel and contractors', + holders: 'All workforce members.', + responsibilities: + 'Comply with the policy set; complete required training; protect company data on personal devices; and report incidents and near-misses.', + authority: 'Use approved tools and identity flows. Granted on engagement.', + }, +] as const; + +/** + * Non-removable note reproduced verbatim in the generated document (§2). Clarifies + * that Comp AI application-access levels are not ISMS governance roles. + */ +export const APPLICATION_ACCESS_NOTE = + 'Comp AI application-access levels (Owner / Admin / Auditor / Employee / Contractor) are not ISMS governance roles. The ISMS governance roles are defined below.'; + +/** The application-access levels and what they grant (generated document §2). */ +export const APPLICATION_ACCESS_LEVELS: string[] = [ + 'Owner — the account creator.', + 'Admin — administrative access to the GRC platform.', + 'Auditor — read access provided to external auditors.', + 'Employee — access to the employee portal.', + 'Contractor — limited access for engaged contractors.', +]; diff --git a/apps/api/src/isms/documents/roles-export-data.ts b/apps/api/src/isms/documents/roles-export-data.ts new file mode 100644 index 0000000000..4f4b9887b3 --- /dev/null +++ b/apps/api/src/isms/documents/roles-export-data.ts @@ -0,0 +1,137 @@ +import { db } from '@db'; +import type { Prisma } from '@db'; +import type { IsmsTeamSizeBand, OperationalOwnershipRow } from './types'; +import { teamSizeBand } from './roles'; + +/** + * Extra data the Roles document (5.3) needs at export time but that isn't on the + * document's own rows: display names for assigned members (assignments store a + * plain memberId, no FK), the live per-artifact operational owners, and the + * team-size band. Resolved once and frozen into the version snapshot so a + * historical export re-renders byte-faithfully. + */ +export interface RolesExtras { + /** memberId → display name (name, else email, else a placeholder). */ + memberNames: Record; + operationalOwnership: OperationalOwnershipRow[]; + band: IsmsTeamSizeBand; +} + +type Client = Prisma.TransactionClient | typeof db; + +const OWNER_DISPLAY_CAP = 12; + +type NamedAssignee = { + assignee: { user: { name: string | null; email: string | null } | null } | null; +}; + +function memberDisplayName(user: { name: string | null; email: string | null } | null): string { + return user?.name?.trim() || user?.email?.trim() || 'Unknown member'; +} + +function dedupeOwners(rows: NamedAssignee[]): string[] { + const seen = new Set(); + const names: string[] = []; + for (const row of rows) { + if (!row.assignee?.user) continue; + const name = memberDisplayName(row.assignee.user); + if (seen.has(name)) continue; + seen.add(name); + names.push(name); + } + names.sort((a, b) => a.localeCompare(b)); + if (names.length <= OWNER_DISPLAY_CAP) return names; + return [ + ...names.slice(0, OWNER_DISPLAY_CAP), + `and ${names.length - OWNER_DISPLAY_CAP} more`, + ]; +} + +/** Load the Roles document's export extras for an organization. */ +export async function loadRolesExtras({ + organizationId, + client, +}: { + organizationId: string; + client?: Client; +}): Promise { + const prisma = client ?? db; + const assigneeSelect = { + assignee: { select: { user: { select: { name: true, email: true } } } }, + } as const; + + const [members, memberCount, policies, risks, tasks, vendors] = + await Promise.all([ + prisma.member.findMany({ + where: { organizationId }, + select: { id: true, user: { select: { name: true, email: true } } }, + }), + prisma.member.count({ where: { organizationId, deactivated: false } }), + prisma.policy.findMany({ + where: { organizationId, assigneeId: { not: null } }, + select: assigneeSelect, + }), + prisma.risk.findMany({ + where: { organizationId, assigneeId: { not: null } }, + select: assigneeSelect, + }), + prisma.task.findMany({ + where: { organizationId, assigneeId: { not: null } }, + select: assigneeSelect, + }), + prisma.vendor.findMany({ + where: { organizationId, assigneeId: { not: null } }, + select: assigneeSelect, + }), + ]); + + const memberNames: Record = {}; + for (const member of members) { + memberNames[member.id] = memberDisplayName(member.user); + } + + // Controls have no owner field in the platform; their ownership is descriptive + // (assigned per control via their linked tasks). Kept in the matrix per the + // reference document, without enumerating names. + const operationalOwnership: OperationalOwnershipRow[] = [ + { + artifact: 'Policies', + assignedWhere: 'Policy assignee / approver in Comp AI', + ownerResponsibility: + 'Keep the policy current and accurate; ensure required acknowledgement.', + owners: dedupeOwners(policies), + }, + { + artifact: 'Controls', + assignedWhere: 'Control owner in Comp AI', + ownerResponsibility: 'Implement, operate, and evidence the control.', + owners: [], + }, + { + artifact: 'Risks', + assignedWhere: 'Risk owner in Comp AI', + ownerResponsibility: + 'Assess, treat, and monitor the risk within the risk appetite.', + owners: dedupeOwners(risks), + }, + { + artifact: 'Evidence tasks', + assignedWhere: 'Task assignee in Comp AI', + ownerResponsibility: 'Complete and evidence the task by its due date.', + owners: dedupeOwners(tasks), + }, + { + artifact: 'Vendors / sub-processors', + assignedWhere: 'Vendor owner in Comp AI', + ownerResponsibility: + 'Perform due diligence and ongoing security review; maintain the DPA.', + owners: dedupeOwners(vendors), + }, + ]; + + return { + memberNames, + operationalOwnership, + band: teamSizeBand(memberCount), + }; +} diff --git a/apps/api/src/isms/documents/roles-export.spec.ts b/apps/api/src/isms/documents/roles-export.spec.ts new file mode 100644 index 0000000000..10658900b9 --- /dev/null +++ b/apps/api/src/isms/documents/roles-export.spec.ts @@ -0,0 +1,100 @@ +import { buildExportSections } from './registry'; +import { generateIsmsExportFile } from '../utils/export-generator'; +import { buildExportMetadata } from '../utils/export-metadata'; +import type { DocumentExportInput, RoleExportRow } from './types'; + +/** + * End-to-end render check for the Roles document (5.3): the section builder + + * both real renderers (jsPDF, docx) must produce non-empty files. Guards the + * whole export pipeline for the new type without needing a live org. + */ +function role(overrides: Partial): RoleExportRow { + return { + roleKey: 'spo', + name: 'Security & Privacy Owner (SPO)', + description: 'Owns the ISMS.', + responsibilities: 'Operate the ISMS.', + authorities: 'Direct the programme.', + authorityGrantedBy: 'Top Management', + requiredCompetence: 'ISO 27001 knowledge.', + holders: ['Alex Petrisor'], + auditRoute: null, + auditRouteHolderName: null, + auditFirmName: null, + auditEvidenceRef: null, + auditCourse: null, + auditDueDate: null, + ...overrides, + }; +} + +const INPUT: DocumentExportInput = { + contextIssues: [], + interestedParties: [], + requirements: [], + objectives: [], + narrative: null, + roles: [ + role({ roleKey: 'top_management', name: 'Top Management', holders: ['Raoul'] }), + role({}), + role({ + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: 'Acme Audit LLP', + holders: ['External auditor'], + }), + ], + operationalOwnership: [ + { + artifact: 'Policies', + assignedWhere: 'Policy assignee in Comp AI', + ownerResponsibility: 'Keep it current.', + owners: ['Alice'], + }, + ], + band: 'standard', +}; + +function metadata() { + return buildExportMetadata({ + type: 'roles_and_responsibilities', + title: 'Roles, Responsibilities and Authorities', + frameworkName: 'ISO 27001', + version: 1, + status: 'approved', + preparedBy: 'Comp AI', + owner: null, + approverName: 'Raoul Plickat', + approvedAt: new Date('2026-05-26T00:00:00.000Z'), + declinedAt: null, + organizationName: 'Pressmaster AI Inc.', + primaryColor: '#004D3D', + }); +} + +describe('Roles document export', () => { + const sections = buildExportSections({ + type: 'roles_and_responsibilities', + input: INPUT, + }); + + it('renders a non-empty PDF', async () => { + const result = await generateIsmsExportFile({ + sections, + metadata: metadata(), + format: 'pdf', + }); + expect(result.fileBuffer.length).toBeGreaterThan(0); + expect(result.mimeType).toBe('application/pdf'); + }); + + it('renders a non-empty DOCX', async () => { + const result = await generateIsmsExportFile({ + sections, + metadata: metadata(), + format: 'docx', + }); + expect(result.fileBuffer.length).toBeGreaterThan(0); + }); +}); diff --git a/apps/api/src/isms/documents/roles.spec.ts b/apps/api/src/isms/documents/roles.spec.ts new file mode 100644 index 0000000000..786c6e8242 --- /dev/null +++ b/apps/api/src/isms/documents/roles.spec.ts @@ -0,0 +1,340 @@ +import { + buildRolesSections, + roleValidationMessages, + seedRolesIfMissing, + teamSizeBand, + type RoleValidationRow, +} from './roles'; +import type { + DocumentExportInput, + RoleExportRow, + OperationalOwnershipRow, +} from './types'; +import type { IsmsExportSection } from '../utils/export-shared'; + +function role(overrides: Partial): RoleExportRow { + return { + roleKey: null, + name: 'Custom role', + description: 'desc', + responsibilities: 'resp', + authorities: 'auth', + authorityGrantedBy: 'Top Management', + requiredCompetence: 'comp', + holders: [], + auditRoute: null, + auditRouteHolderName: null, + auditFirmName: null, + auditEvidenceRef: null, + auditCourse: null, + auditDueDate: null, + ...overrides, + }; +} + +const OWNERSHIP: OperationalOwnershipRow[] = [ + { + artifact: 'Policies', + assignedWhere: 'Policy assignee / approver in Comp AI', + ownerResponsibility: 'Keep the policy current.', + owners: ['Alice'], + }, + { + artifact: 'Controls', + assignedWhere: 'Control owner in Comp AI', + ownerResponsibility: 'Operate the control.', + owners: [], + }, +]; + +function input(overrides: Partial): DocumentExportInput { + return { + contextIssues: [], + interestedParties: [], + requirements: [], + objectives: [], + narrative: null, + roles: [], + operationalOwnership: OWNERSHIP, + band: 'standard', + ...overrides, + }; +} + +function findSection( + sections: IsmsExportSection[], + heading: string, +): IsmsExportSection | undefined { + return sections.find((section) => section.heading === heading); +} + +describe('teamSizeBand', () => { + it.each([ + [0, 'small'], + [1, 'small'], + [3, 'small'], + [4, 'standard'], + [50, 'standard'], + ])('maps %i members to %s', (count, band) => { + expect(teamSizeBand(count)).toBe(band); + }); +}); + +describe('buildRolesSections', () => { + it('renders a 6-row governance table (roles + 2 auto rows) with holders', () => { + const sections = buildRolesSections( + input({ + roles: [ + role({ roleKey: 'top_management', name: 'Top Management', holders: ['Raoul'] }), + role({ roleKey: 'spo', name: 'SPO', holders: ['Alex'] }), + ], + }), + ); + const table = findSection(sections, 'ISMS governance roles')?.table; + expect(table?.headers).toHaveLength(4); + // 2 provided roles + 2 auto-generated rows + expect(table?.rows).toHaveLength(4); + expect(table?.rows[0][1]).toBe('Raoul'); // holder column + expect(table?.rows.some((r) => r[0] === 'Control / asset / risk / policy owners')).toBe(true); + expect(table?.rows.some((r) => r[0] === 'All personnel and contractors')).toBe(true); + }); + + it('shows [To be named] when a role has no holders', () => { + const sections = buildRolesSections( + input({ roles: [role({ roleKey: 'spo', name: 'SPO', holders: [] })] }), + ); + const table = findSection(sections, 'ISMS governance roles')?.table; + expect(table?.rows[0][1]).toBe('[To be named]'); + }); + + it('assigns 5.3(a) and (b) to the SPO holder in prose', () => { + const sections = buildRolesSections( + input({ roles: [role({ roleKey: 'spo', name: 'SPO', holders: ['Alex Petrisor'] })] }), + ); + const assignments = findSection(sections, 'Specific assignments required by Clause 5.3'); + const paragraphs = assignments?.paragraphs ?? []; + expect(paragraphs.find((p) => p.text.startsWith('(a)'))?.text).toContain('Alex Petrisor'); + expect(paragraphs.find((p) => p.text.startsWith('(b)'))?.text).toContain('Alex Petrisor'); + }); + + it('describes the chosen internal audit route (external)', () => { + const sections = buildRolesSections( + input({ + roles: [ + role({ + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: 'Acme Audit LLP', + }), + ], + }), + ); + const text = findSection(sections, 'Internal audit route')?.paragraphs?.[0].text ?? ''; + expect(text).toContain('external independent auditor'); + expect(text).toContain('Acme Audit LLP'); + }); + + it('renders the team-size note and a summary ONLY for the small band', () => { + const small = buildRolesSections(input({ band: 'small' })); + expect(findSection(small, 'Note on team size')).toBeDefined(); + // Operational responsibilities are a bulleted summary in the small band. + const smallOps = findSection(small, 'Operational responsibilities'); + expect(smallOps?.bullets).toBeDefined(); + expect(smallOps?.table).toBeUndefined(); + + const standard = buildRolesSections(input({ band: 'standard' })); + expect(findSection(standard, 'Note on team size')).toBeUndefined(); + // ...and a 4-column table in the standard band, surfacing live owners. + const stdOps = findSection(standard, 'Operational responsibilities'); + expect(stdOps?.table?.headers).toHaveLength(4); + expect(stdOps?.table?.rows[0]).toContain('Alice'); + }); +}); + +describe('roleValidationMessages (server gate)', () => { + const ACTIVE = new Set(['m1']); + const assigned = { memberId: 'm1' }; + const check = (roles: RoleValidationRow[], memberCount = 20) => + roleValidationMessages({ roles, memberCount, activeMemberIds: ACTIVE }); + const externalAuditor: RoleValidationRow = { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: 'Acme Audit LLP', + auditEvidenceRef: 'LA cert on file', + assignments: [assigned], + }; + const complete = (): RoleValidationRow[] => [ + { roleKey: 'top_management', name: 'Top Management', auditRoute: null, assignments: [assigned] }, + { roleKey: 'spo', name: 'SPO', auditRoute: null, assignments: [assigned] }, + { roleKey: 'deputy_spo', name: 'Deputy SPO', auditRoute: null, assignments: [assigned] }, + { ...externalAuditor }, + ]; + + it('passes when every seeded role is present + assigned + routed', () => { + expect(check(complete())).toEqual([]); + }); + + it('does not count an assignment to a non-active member', () => { + const roles = complete(); + roles[0] = { + roleKey: 'top_management', + name: 'Top Management', + auditRoute: null, + assignments: [{ memberId: 'deactivated' }], + }; + expect(check(roles)).toContain('Top Management needs at least one assigned member.'); + }); + + it('requires firm + evidence for the external audit route (whitespace does not count)', () => { + const roles = complete(); + roles[3] = { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: ' ', + auditEvidenceRef: '', + assignments: [assigned], + }; + expect(check(roles)).toContain( + 'The external Internal Auditor needs a firm/person name and an evidence reference.', + ); + }); + + it('requires an ACTIVE member for the in-house audit route', () => { + const roles = complete(); + roles[3] = { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'in_house', + auditRouteMemberId: 'deactivated', // set, but not in the active set + assignments: [assigned], + }; + expect(check(roles)).toContain( + 'The in-house Internal Auditor needs an active member selected.', + ); + }); + + it('requires active member + course + due date for the training-planned route', () => { + const roles = complete(); + roles[3] = { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'training_planned', + auditRouteMemberId: 'm1', // active + auditCourse: ' ', // whitespace → missing + auditDueDate: '2026-05-26', + assignments: [assigned], + }; + expect(check(roles)).toContain( + 'The training-planned Internal Auditor needs an active member, a course, and a due date.', + ); + }); + + it('flags an entirely-missing required seeded role', () => { + const roles = complete().filter((r) => r.roleKey !== 'top_management'); + expect(check(roles)).toContain('Top Management is missing from the document.'); + }); + + it('flags a present-but-unassigned seeded role', () => { + const roles = complete(); + roles[1] = { roleKey: 'spo', name: 'SPO', auditRoute: null, assignments: [] }; + expect(check(roles)).toContain('SPO needs at least one assigned member.'); + }); + + it('treats Deputy SPO as optional only in the small band', () => { + const roles = complete().filter((r) => r.roleKey !== 'deputy_spo'); + expect(check(roles, 2)).toEqual([]); + expect(check(roles, 10)).toContain( + 'Deputy Security & Privacy Owner is missing from the document.', + ); + }); + + it('requires the Internal Auditor route', () => { + const roles = complete(); + roles[3] = { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: null, + assignments: [assigned], + }; + expect(check(roles)).toContain('The Internal Auditor needs an audit route selected.'); + }); +}); + +describe('seedRolesIfMissing', () => { + function makeTx(existingRoleKeys: Array) { + return { + ismsRole: { + findMany: jest + .fn() + .mockResolvedValue( + existingRoleKeys.map((roleKey, i) => ({ roleKey, position: i })), + ), + createMany: jest.fn().mockResolvedValue({ count: 0 }), + }, + }; + } + + it('creates all four seeded roles on an empty document', async () => { + const tx = makeTx([]); + await seedRolesIfMissing({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + tx: tx as any, + documentId: 'doc_1', + memberCount: 10, + }); + const created = tx.ismsRole.createMany.mock.calls[0][0].data; + expect(created).toHaveLength(4); + expect(created.map((r: { roleKey: string }) => r.roleKey)).toEqual([ + 'top_management', + 'spo', + 'deputy_spo', + 'internal_auditor', + ]); + }); + + it('is idempotent: creates nothing when all seeded roles exist', async () => { + const tx = makeTx(['top_management', 'spo', 'deputy_spo', 'internal_auditor']); + await seedRolesIfMissing({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + tx: tx as any, + documentId: 'doc_1', + memberCount: 10, + }); + expect(tx.ismsRole.createMany).not.toHaveBeenCalled(); + }); + + it('defaults the Internal Auditor route to external for a small team', async () => { + const tx = makeTx([]); + await seedRolesIfMissing({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + tx: tx as any, + documentId: 'doc_1', + memberCount: 2, + }); + const created = tx.ismsRole.createMany.mock.calls[0][0].data as Array<{ + roleKey: string; + auditRoute: string | null; + }>; + const auditor = created.find((r) => r.roleKey === 'internal_auditor'); + expect(auditor?.auditRoute).toBe('external'); + }); + + it('leaves the Internal Auditor route unset for a standard team', async () => { + const tx = makeTx([]); + await seedRolesIfMissing({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + tx: tx as any, + documentId: 'doc_1', + memberCount: 20, + }); + const created = tx.ismsRole.createMany.mock.calls[0][0].data as Array<{ + roleKey: string; + auditRoute: string | null; + }>; + const auditor = created.find((r) => r.roleKey === 'internal_auditor'); + expect(auditor?.auditRoute).toBeNull(); + }); +}); diff --git a/apps/api/src/isms/documents/roles.ts b/apps/api/src/isms/documents/roles.ts new file mode 100644 index 0000000000..bdc1490d04 --- /dev/null +++ b/apps/api/src/isms/documents/roles.ts @@ -0,0 +1,391 @@ +import type { Prisma } from '@db'; +import type { IsmsExportSection } from '../utils/export-shared'; +import type { + DocumentExportInput, + IsmsTeamSizeBand, + OperationalOwnershipRow, + RoleExportRow, +} from './types'; +import { + APPLICATION_ACCESS_LEVELS, + APPLICATION_ACCESS_NOTE, + AUTO_DOC_ROLE_ROWS, + SEED_ROLE_DEFINITIONS, + SEED_ROLE_KEYS, +} from './roles-defaults'; + +type Tx = Prisma.TransactionClient; + +/** 1-3 people → 'small', 4+ → 'standard'. Drives the Roles document's copy/defaults. */ +export function teamSizeBand(memberCount: number): IsmsTeamSizeBand { + return memberCount <= 3 ? 'small' : 'standard'; +} + +/** Seeded roles that must be present + assigned before the 5.3 doc can be published. */ +const REQUIRED_SEED_ROLE_KEYS = SEED_ROLE_DEFINITIONS.map((role) => role.roleKey); + +/** The subset of role fields the completeness check needs (server + client share it). */ +export interface RoleValidationRow { + roleKey: string | null; + name: string; + auditRoute: string | null; + auditRouteMemberId?: string | null; + auditFirmName?: string | null; + auditEvidenceRef?: string | null; + auditCourse?: string | null; + auditDueDate?: Date | string | null; + assignments: Array<{ memberId: string }>; +} + +/** True when the audit-route member is set AND is an active organization member. */ +function hasActiveAuditMember( + role: RoleValidationRow, + activeMemberIds: Set, +): boolean { + return ( + !!role.auditRouteMemberId && activeMemberIds.has(role.auditRouteMemberId) + ); +} + +/** + * Route-specific required fields for the Internal Auditor (CS-698). The in-house + * and training-planned routes need an ACTIVE selected member; external needs a + * firm/person and evidence reference. Text is trimmed so whitespace can't satisfy + * a required field via the API. + */ +function auditRouteMessages( + role: RoleValidationRow, + activeMemberIds: Set, +): string[] { + const route = role.auditRoute; + if (!route) return ['The Internal Auditor needs an audit route selected.']; + if (route === 'in_house') { + return hasActiveAuditMember(role, activeMemberIds) + ? [] + : ['The in-house Internal Auditor needs an active member selected.']; + } + if (route === 'external') { + return role.auditFirmName?.trim() && role.auditEvidenceRef?.trim() + ? [] + : [ + 'The external Internal Auditor needs a firm/person name and an evidence reference.', + ]; + } + // training_planned + const complete = + hasActiveAuditMember(role, activeMemberIds) && + !!role.auditCourse?.trim() && + !!role.auditDueDate; + return complete + ? [] + : [ + 'The training-planned Internal Auditor needs an active member, a course, and a due date.', + ]; +} + +/** + * Clause-5.3 completeness check, shared by the submit-for-approval server gate. + * Every seeded role must exist and have at least one assigned member (except the + * Deputy SPO in the 1-3 band); the Internal Auditor must have a route AND its + * route-specific fields. Only assignments (and audit-route members) that resolve + * to an ACTIVE member count. Missing rows are reported too. Returns the unmet + * requirements; empty = ready. + */ +export function roleValidationMessages({ + roles, + memberCount, + activeMemberIds, +}: { + roles: RoleValidationRow[]; + memberCount: number; + activeMemberIds: Set; +}): string[] { + const band = teamSizeBand(memberCount); + const byKey = new Map( + roles + .filter((role) => role.roleKey) + .map((role) => [role.roleKey as string, role]), + ); + const messages: string[] = []; + for (const key of REQUIRED_SEED_ROLE_KEYS) { + const role = byKey.get(key); + const name = + role?.name ?? + SEED_ROLE_DEFINITIONS.find((seed) => seed.roleKey === key)?.name ?? + key; + const optional = key === 'deputy_spo' && band === 'small'; + if (!role) { + if (!optional) messages.push(`${name} is missing from the document.`); + continue; + } + const activeAssignments = role.assignments.filter((assignment) => + activeMemberIds.has(assignment.memberId), + ); + if (!optional && activeAssignments.length === 0) { + messages.push(`${name} needs at least one assigned member.`); + } + if (key === 'internal_auditor') { + messages.push(...auditRouteMessages(role, activeMemberIds)); + } + } + return messages; +} + +/** + * Seed the four governance roles for a Roles document, idempotently by `roleKey`. + * Only creates seed roles that are missing — it NEVER deletes or overwrites, so a + * regenerate can never clobber the customer's edits or member assignments (unlike + * the derived-row replace used by the other registers, which would cascade-delete + * IsmsRoleAssignment rows). Safe to call at document creation and on every generate. + * + * The Internal Auditor route defaults to `external` for small teams (1-3), the + * standard route at that size; larger teams choose their own route. + */ +export async function seedRolesIfMissing({ + tx, + documentId, + memberCount, +}: { + tx: Tx; + documentId: string; + memberCount: number; +}): Promise { + const existing = await tx.ismsRole.findMany({ + where: { documentId }, + select: { roleKey: true, position: true }, + }); + const existingKeys = new Set( + existing.map((role) => role.roleKey).filter((key): key is string => !!key), + ); + const missing = SEED_ROLE_DEFINITIONS.filter( + (role) => !existingKeys.has(role.roleKey), + ); + if (missing.length === 0) return; + + const maxPosition = existing.reduce( + (max, role) => Math.max(max, role.position), + -1, + ); + const band = teamSizeBand(memberCount); + + await tx.ismsRole.createMany({ + data: missing.map((role, index) => ({ + documentId, + roleKey: role.roleKey, + name: role.name, + description: role.description, + responsibilities: role.responsibilities, + authorities: role.authorities, + authorityGrantedBy: role.authorityGrantedBy, + requiredCompetence: role.requiredCompetence, + auditRoute: + role.roleKey === 'internal_auditor' && band === 'small' + ? 'external' + : null, + source: 'derived', + derivedFrom: `seed:${role.roleKey}`, + position: maxPosition + 1 + index, + })), + // Belt-and-braces with the @@unique([documentId, roleKey]) constraint: a + // concurrent provision/generate that races this seed is absorbed silently + // instead of throwing, so parallel first-loads can't double-seed. + skipDuplicates: true, + }); +} + +// ---- Export section builder ------------------------------------------------- + +function holderText(holders: string[]): string { + return holders.length > 0 ? holders.join(', ') : '[To be named]'; +} + +/** Combine the authority text and its source into the reference doc's column. */ +function authorityText(role: RoleExportRow): string { + const granted = role.authorityGrantedBy.trim(); + if (!granted) return role.authorities; + return `${role.authorities} Authority granted by ${granted}.`; +} + +function internalAuditParagraph(role: RoleExportRow | undefined): string { + if (!role || !role.auditRoute) { + return 'The internal audit route has not yet been selected.'; + } + if (role.auditRoute === 'in_house') { + const who = role.auditRouteHolderName + ? `, performed by ${role.auditRouteHolderName}` + : ''; + return `The organisation conducts its internal ISMS audit in-house${who}. The internal auditor maintains independence and impartiality from the areas they audit.`; + } + if (role.auditRoute === 'external') { + const firm = role.auditFirmName ? `: ${role.auditFirmName}` : ''; + const evidence = role.auditEvidenceRef + ? ` Supporting evidence: ${role.auditEvidenceRef}.` + : ''; + return `The organisation engages an external independent auditor to conduct its internal ISMS audit${firm}.${evidence}`; + } + // training_planned + const who = role.auditRouteHolderName ? ` by ${role.auditRouteHolderName}` : ''; + const course = role.auditCourse ? ` through ${role.auditCourse}` : ''; + const due = role.auditDueDate ? `, due ${role.auditDueDate}` : ''; + return `Internal audit competence is being developed${who}${course}${due}. Until it is in place, an external independent auditor is the recommended interim route.`; +} + +function buildRoleTable(roles: RoleExportRow[]): IsmsExportSection['table'] { + // Per CS-698 the governance table is exactly the four seeded roles + the two + // auto-generated rows. Custom roles are managed on the page but are not part of + // this standardized Clause 5.3 table, so they're excluded here. + const seededRoles = roles.filter( + (role) => role.roleKey !== null && SEED_ROLE_KEYS.includes(role.roleKey), + ); + return { + headers: ['Role', 'Holder', 'Responsibility', 'Authority — and granted by'], + rows: [ + ...seededRoles.map((role) => [ + role.name, + holderText(role.holders), + role.responsibilities, + authorityText(role), + ]), + ...AUTO_DOC_ROLE_ROWS.map((row) => [ + row.name, + row.holders, + row.responsibilities, + row.authority, + ]), + ], + }; +} + +function buildOperationalSection( + ownership: OperationalOwnershipRow[], + band: IsmsTeamSizeBand, +): IsmsExportSection { + const intro = + 'Operational responsibility for the ISMS is assigned at the level of individual artifacts. Comp AI records a named owner or assignee for every policy, control, risk, evidence task, and vendor; this constitutes the live, auditable responsibilities matrix.'; + + if (band === 'small') { + return { + heading: 'Operational responsibilities', + intro, + bullets: ownership.map((row) => { + const owners = + row.owners.length > 0 + ? row.owners.join(', ') + : 'assigned per item in Comp AI'; + return `${row.artifact}: ${owners}.`; + }), + }; + } + + return { + heading: 'Operational responsibilities', + intro, + table: { + headers: [ + 'ISMS artifact', + 'Where responsibility is assigned', + 'Owner’s responsibility', + 'Current owner(s)', + ], + rows: ownership.map((row) => [ + row.artifact, + row.assignedWhere, + row.ownerResponsibility, + row.owners.length > 0 ? row.owners.join(', ') : '—', + ]), + }, + }; +} + +/** + * Build the ISMS Roles, Responsibilities & Authorities document (clause 5.3). + * Structure follows the reference document merged with the ticket's ordered + * contents. `roles`, `operationalOwnership` and `band` are populated by + * loadRolesExtras at export-input assembly (see roles-export-data.ts). + */ +export function buildRolesSections( + input: DocumentExportInput, +): IsmsExportSection[] { + const roles = input.roles ?? []; + const ownership = input.operationalOwnership ?? []; + const band: IsmsTeamSizeBand = input.band ?? 'standard'; + const spo = roles.find((role) => role.roleKey === 'spo'); + const spoHolders = spo && spo.holders.length > 0 ? ` (${spo.holders.join(', ')})` : ''; + const auditor = roles.find((role) => role.roleKey === 'internal_auditor'); + + const sections: IsmsExportSection[] = [ + { + heading: 'Purpose', + paragraphs: [ + { + text: 'This document defines the responsibilities and authorities for the roles relevant to information security in the organisation, and the authority from which each is derived, in accordance with ISO/IEC 27001:2022, Clause 5.3. It also supports the authorities-and-responsibilities matrix referenced in Clause 5.2.', + }, + ], + }, + { + heading: 'Relationship to Comp AI application-access roles', + intro: + 'The Comp AI platform assigns application-access levels that determine access to the software only:', + bullets: APPLICATION_ACCESS_LEVELS, + paragraphs: [{ text: APPLICATION_ACCESS_NOTE }], + }, + { + heading: 'ISMS governance roles', + intro: + 'The following named roles carry the accountability and decision authority for the ISMS.', + table: buildRoleTable(roles), + }, + { + heading: 'Specific assignments required by Clause 5.3', + intro: 'Top management has assigned the responsibility and authority for:', + paragraphs: [ + { + text: `(a) Ensuring that the ISMS conforms to the requirements of ISO/IEC 27001 — assigned to the Security & Privacy Owner${spoHolders}.`, + }, + { + text: `(b) Reporting on the performance of the ISMS to top management — assigned to the Security & Privacy Owner${spoHolders}, delivered at each management review and on material incidents or risk changes.`, + }, + { + text: 'The authority to assign tasks and make decisions within the ISMS rests with top management, who assign it to the Security & Privacy Owner; the SPO in turn assigns operational ownership of individual controls, policies, risks, and tasks through Comp AI.', + }, + ], + }, + { + heading: 'Internal audit route', + paragraphs: [{ text: internalAuditParagraph(auditor) }], + }, + ]; + + if (band === 'small') { + sections.push({ + heading: 'Note on team size', + paragraphs: [ + { + text: 'The organisation currently operates with a small team (three or fewer people). Some ISMS roles are necessarily held by the same individuals, and the Deputy Security & Privacy Owner may be unfilled. This is a pragmatic and accepted arrangement at this size; the internal audit route selected for this ISMS is stated in the "Internal audit route" section above. Be prepared to explain this structure at audit.', + }, + ], + }); + } + + sections.push(buildOperationalSection(ownership, band)); + + sections.push({ + heading: 'Communication of roles', + paragraphs: [ + { + text: 'Responsibilities and authorities are made known throughout the organisation through policy acknowledgement at onboarding and annually, the per-item owner assignments recorded in Comp AI, and the leadership communications captured in management reviews.', + }, + ], + }); + + sections.push({ + heading: 'Review', + paragraphs: [ + { + text: 'This document is owned by the Security & Privacy Owner and is reviewed at least annually and on material change to governance, organisational structure, or key personnel. The approver and approval date are recorded in the document control table above.', + }, + ], + }); + + return sections; +} diff --git a/apps/api/src/isms/documents/snapshot.ts b/apps/api/src/isms/documents/snapshot.ts index d37dfec93c..eb04c6d8a1 100644 --- a/apps/api/src/isms/documents/snapshot.ts +++ b/apps/api/src/isms/documents/snapshot.ts @@ -86,6 +86,10 @@ const TYPE_DRIFT_SOURCES: Record> = { 'members', 'wizardAnswers', ], + // Roles text is static defaults; the only platform input the 5.3 document + // consumes is headcount, which sets the team-size band (small vs standard) and + // so changes the team-size note + operational-responsibilities rendering. + roles_and_responsibilities: ['members'], isms_scope: [ 'frameworks', 'vendors', diff --git a/apps/api/src/isms/documents/types.ts b/apps/api/src/isms/documents/types.ts index a902387a16..f5c58add49 100644 --- a/apps/api/src/isms/documents/types.ts +++ b/apps/api/src/isms/documents/types.ts @@ -77,6 +77,48 @@ export interface DerivedObjective extends DerivedRegisterRow { measurementMethod: string | null; } +/** Team-size band that drives the Roles document's copy and defaults (5.3). */ +export type IsmsTeamSizeBand = 'small' | 'standard'; // small = 1-3 people, standard = 4+ + +/** The four seeded ISMS governance roles and their pre-filled default text. */ +export interface SeedRoleDefinition { + roleKey: 'top_management' | 'spo' | 'deputy_spo' | 'internal_auditor'; + name: string; + description: string; + responsibilities: string; + authorities: string; + authorityGrantedBy: string; + requiredCompetence: string; +} + +/** A role, resolved for export: fields + named holders + internal-audit route. */ +export interface RoleExportRow { + roleKey: string | null; + name: string; + description: string; + responsibilities: string; + authorities: string; + authorityGrantedBy: string; + requiredCompetence: string; + /** Display names of the assigned members (resolved + frozen at build time). */ + holders: string[]; + auditRoute: string | null; + auditRouteHolderName: string | null; + auditFirmName: string | null; + auditEvidenceRef: string | null; + auditCourse: string | null; + auditDueDate: string | null; +} + +/** One row of the operational-responsibilities summary (5.3 §5). */ +export interface OperationalOwnershipRow { + artifact: string; + assignedWhere: string; + ownerResponsibility: string; + /** Distinct owner display names read from the platform (may be empty). */ + owners: string[]; +} + /** * The organization profile that fills the narrative parts of the Context of the * Organization document (clause 4.1) — overview table, mission, intended @@ -120,4 +162,10 @@ export interface DocumentExportInput { narrative: unknown; /** Org overview/mission/outcomes — only populated for the Context document. */ orgProfile?: IsmsOrgProfile; + /** Governance roles with resolved holders — only populated for the Roles document (5.3). */ + roles?: RoleExportRow[]; + /** Operational per-artifact ownership — only populated for the Roles document (5.3). */ + operationalOwnership?: OperationalOwnershipRow[]; + /** Team-size band — only populated for the Roles document (5.3). */ + band?: IsmsTeamSizeBand; } diff --git a/apps/api/src/isms/isms-registers.controller.spec.ts b/apps/api/src/isms/isms-registers.controller.spec.ts index fe99b08ae0..548b2eb604 100644 --- a/apps/api/src/isms/isms-registers.controller.spec.ts +++ b/apps/api/src/isms/isms-registers.controller.spec.ts @@ -10,6 +10,8 @@ import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; import { IsmsRequirementService } from './isms-requirement.service'; import { IsmsObjectiveService } from './isms-objective.service'; +import { IsmsRoleService } from './isms-role.service'; +import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsNarrativeService } from './isms-narrative.service'; jest.mock('../auth/auth.server', () => ({ @@ -38,6 +40,12 @@ jest.mock('./isms-requirement.service', () => ({ jest.mock('./isms-objective.service', () => ({ IsmsObjectiveService: class {}, })); +jest.mock('./isms-role.service', () => ({ + IsmsRoleService: class {}, +})); +jest.mock('./isms-role-assignment.service', () => ({ + IsmsRoleAssignmentService: class {}, +})); jest.mock('./isms-narrative.service', () => ({ IsmsNarrativeService: class {}, })); @@ -68,6 +76,16 @@ describe('IsmsRegistersController', () => { update: jest.fn(), remove: jest.fn(), }; + const roleService = { + create: jest.fn(), + update: jest.fn(), + remove: jest.fn(), + }; + const roleAssignmentService = { + create: jest.fn(), + update: jest.fn(), + remove: jest.fn(), + }; const narrativeService = { save: jest.fn() }; const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; @@ -83,6 +101,8 @@ describe('IsmsRegistersController', () => { }, { provide: IsmsRequirementService, useValue: requirementService }, { provide: IsmsObjectiveService, useValue: objectiveService }, + { provide: IsmsRoleService, useValue: roleService }, + { provide: IsmsRoleAssignmentService, useValue: roleAssignmentService }, { provide: IsmsNarrativeService, useValue: narrativeService }, ], }) diff --git a/apps/api/src/isms/isms-registers.controller.ts b/apps/api/src/isms/isms-registers.controller.ts index 6528372300..957e6da398 100644 --- a/apps/api/src/isms/isms-registers.controller.ts +++ b/apps/api/src/isms/isms-registers.controller.ts @@ -27,6 +27,8 @@ import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; import { IsmsRequirementService } from './isms-requirement.service'; import { IsmsObjectiveService } from './isms-objective.service'; +import { IsmsRoleService } from './isms-role.service'; +import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsNarrativeService } from './isms-narrative.service'; import { createRegisterRegistry, @@ -67,6 +69,32 @@ const REGISTER_ROW_BODY = { type: 'string', enum: ['not_started', 'on_track', 'at_risk', 'met'], }, + // Roles register (5.3) + role assignments (7.2 competence) + responsibilities: { type: 'string' }, + authorities: { type: 'string' }, + authorityGrantedBy: { type: 'string' }, + requiredCompetence: { type: 'string' }, + auditRoute: { + type: 'string', + enum: ['in_house', 'external', 'training_planned'], + nullable: true, + }, + auditRouteMemberId: { type: 'string', nullable: true }, + auditFirmName: { type: 'string', nullable: true }, + auditEvidenceRef: { type: 'string', nullable: true }, + auditCourse: { type: 'string', nullable: true }, + auditDueDate: { type: 'string', nullable: true }, + roleId: { type: 'string' }, + memberId: { type: 'string' }, + basisOfCompetence: { + type: 'string', + enum: ['education', 'training', 'experience', 'combination'], + nullable: true, + }, + evidenceRetained: { type: 'string', nullable: true }, + gap: { type: 'string', nullable: true }, + remediationAction: { type: 'string', nullable: true }, + remediationDueDate: { type: 'string', nullable: true }, position: { type: 'integer', minimum: 0 }, }, }, @@ -106,6 +134,8 @@ export class IsmsRegistersController { interestedPartyService: IsmsInterestedPartyService, requirementService: IsmsRequirementService, objectiveService: IsmsObjectiveService, + roleService: IsmsRoleService, + roleAssignmentService: IsmsRoleAssignmentService, private readonly narrativeService: IsmsNarrativeService, ) { this.registry = createRegisterRegistry({ @@ -113,6 +143,8 @@ export class IsmsRegistersController { interestedParties: interestedPartyService, requirements: requirementService, objectives: objectiveService, + roles: roleService, + roleAssignments: roleAssignmentService, }); } diff --git a/apps/api/src/isms/isms-role-assignment.service.spec.ts b/apps/api/src/isms/isms-role-assignment.service.spec.ts new file mode 100644 index 0000000000..18419b70f5 --- /dev/null +++ b/apps/api/src/isms/isms-role-assignment.service.spec.ts @@ -0,0 +1,127 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; + +jest.mock('@db', () => { + const db = { + ismsDocument: { findFirst: jest.fn(), findUnique: jest.fn(), update: jest.fn() }, + ismsRole: { findFirst: jest.fn() }, + member: { findFirst: jest.fn() }, + ismsRoleAssignment: { + findFirst: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); + +const mockDb = jest.mocked(db); + +describe('IsmsRoleAssignmentService', () => { + let service: IsmsRoleAssignmentService; + + beforeEach(() => { + jest.clearAllMocks(); + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ status: 'draft' }); + service = new IsmsRoleAssignmentService(); + }); + + const createArgs = { + documentId: 'doc_1', + organizationId: 'org_1', + dto: { roleId: 'role_1', memberId: 'mem_1' }, + }; + + function stubCreatePreconditions() { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1' }); + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ id: 'role_1' }); + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + } + + it('rejects a role that is not in the document', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1' }); + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.create(createArgs)).rejects.toThrow(NotFoundException); + }); + + it('rejects a member who is not in the org', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1' }); + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ id: 'role_1' }); + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.create(createArgs)).rejects.toThrow(NotFoundException); + }); + + it('creates an assignment carrying the denormalized documentId', async () => { + stubCreatePreconditions(); + (mockDb.ismsRoleAssignment.findFirst as jest.Mock) + .mockResolvedValueOnce(null) // no existing (roleId, memberId) + .mockResolvedValueOnce({ position: 0 }); // nextPosition + (mockDb.ismsRoleAssignment.create as jest.Mock).mockResolvedValue({ id: 'ra_1' }); + + await service.create(createArgs); + + expect(mockDb.ismsRoleAssignment.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + roleId: 'role_1', + memberId: 'mem_1', + documentId: 'doc_1', + position: 1, + }), + }); + }); + + it('is idempotent: returns the existing assignment instead of duplicating', async () => { + stubCreatePreconditions(); + (mockDb.ismsRoleAssignment.findFirst as jest.Mock).mockResolvedValue({ + id: 'ra_existing', + }); + + const result = await service.create(createArgs); + + expect(result).toEqual({ id: 'ra_existing' }); + expect(mockDb.ismsRoleAssignment.create).not.toHaveBeenCalled(); + }); + + it('updates competence fields', async () => { + (mockDb.ismsRoleAssignment.findFirst as jest.Mock).mockResolvedValue({ + id: 'ra_1', + documentId: 'doc_1', + }); + (mockDb.ismsRoleAssignment.update as jest.Mock).mockResolvedValue({}); + + await service.update({ + assignmentId: 'ra_1', + organizationId: 'org_1', + dto: { basisOfCompetence: 'training', gap: 'Needs ISO course' }, + }); + + expect(mockDb.ismsRoleAssignment.update).toHaveBeenCalledWith({ + where: { id: 'ra_1' }, + data: expect.objectContaining({ + basisOfCompetence: 'training', + gap: 'Needs ISO course', + }), + }); + }); + + it('throws when updating an assignment outside the org', async () => { + (mockDb.ismsRoleAssignment.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.update({ assignmentId: 'ra_x', organizationId: 'org_1', dto: {} }), + ).rejects.toThrow(NotFoundException); + }); + + it('removes an assignment', async () => { + (mockDb.ismsRoleAssignment.findFirst as jest.Mock).mockResolvedValue({ + id: 'ra_1', + documentId: 'doc_1', + }); + (mockDb.ismsRoleAssignment.delete as jest.Mock).mockResolvedValue({}); + const result = await service.remove({ assignmentId: 'ra_1', organizationId: 'org_1' }); + expect(result).toEqual({ success: true }); + }); +}); diff --git a/apps/api/src/isms/isms-role-assignment.service.ts b/apps/api/src/isms/isms-role-assignment.service.ts new file mode 100644 index 0000000000..1ecc08414c --- /dev/null +++ b/apps/api/src/isms/isms-role-assignment.service.ts @@ -0,0 +1,210 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { lockDocument } from './utils/document-lock'; +import { parseOptionalDate } from './utils/parse-optional-date'; +import type { + CreateRoleAssignmentInput, + UpdateRoleAssignmentInput, +} from './registers/register-registry'; + +/** + * CRUD for role member assignments + their competence evidence (clauses 5.3/7.2). + * Each row is one member assigned to one role. Creating is idempotent per + * (role, member) so re-adding a member in the picker never duplicates. Assignment + * changes revert an approved document to draft, like every other register edit. + */ +@Injectable() +export class IsmsRoleAssignmentService { + async create({ + documentId, + organizationId, + dto, + }: { + documentId: string; + organizationId: string; + dto: CreateRoleAssignmentInput; + }) { + await this.requireDocument({ documentId, organizationId }); + await this.requireRoleInDocument({ roleId: dto.roleId, documentId }); + await this.requireMember({ memberId: dto.memberId, organizationId }); + + return db.$transaction(async (tx) => { + await lockDocument(tx, documentId); + // Idempotency inside the per-document lock: concurrent duplicate adds for + // the same (role, member) serialize here, so the second caller returns the + // first's row instead of hitting the (roleId, memberId) unique index. + const existing = await tx.ismsRoleAssignment.findFirst({ + where: { roleId: dto.roleId, memberId: dto.memberId }, + }); + if (existing) return existing; + + const position = + dto.position ?? (await this.nextPosition({ tx, roleId: dto.roleId })); + await invalidateApprovalIfNeeded({ tx, documentId }); + return tx.ismsRoleAssignment.create({ + data: { + roleId: dto.roleId, + documentId, + memberId: dto.memberId, + basisOfCompetence: dto.basisOfCompetence ?? null, + evidenceRetained: dto.evidenceRetained ?? null, + gap: dto.gap ?? null, + remediationAction: dto.remediationAction ?? null, + remediationDueDate: parseOptionalDate(dto.remediationDueDate) ?? null, + position, + }, + }); + }); + } + + async update({ + assignmentId, + organizationId, + dto, + }: { + assignmentId: string; + organizationId: string; + dto: UpdateRoleAssignmentInput; + }) { + const assignment = await this.requireAssignment({ + assignmentId, + organizationId, + }); + + return db.$transaction(async (tx) => { + await lockDocument(tx, assignment.documentId); + await invalidateApprovalIfNeeded({ + tx, + documentId: assignment.documentId, + }); + return tx.ismsRoleAssignment.update({ + where: { id: assignmentId }, + data: { + basisOfCompetence: + dto.basisOfCompetence === undefined + ? undefined + : dto.basisOfCompetence, + evidenceRetained: + dto.evidenceRetained === undefined + ? undefined + : dto.evidenceRetained, + gap: dto.gap === undefined ? undefined : dto.gap, + remediationAction: + dto.remediationAction === undefined + ? undefined + : dto.remediationAction, + remediationDueDate: parseOptionalDate(dto.remediationDueDate), + position: dto.position ?? undefined, + }, + }); + }); + } + + async remove({ + assignmentId, + organizationId, + }: { + assignmentId: string; + organizationId: string; + }) { + const assignment = await this.requireAssignment({ + assignmentId, + organizationId, + }); + await db.$transaction(async (tx) => { + await lockDocument(tx, assignment.documentId); + await invalidateApprovalIfNeeded({ + tx, + documentId: assignment.documentId, + }); + await tx.ismsRoleAssignment.delete({ where: { id: assignmentId } }); + }); + return { success: true }; + } + + private async nextPosition({ + tx, + roleId, + }: { + tx: Prisma.TransactionClient; + roleId: string; + }) { + const last = await tx.ismsRoleAssignment.findFirst({ + where: { roleId }, + orderBy: { position: 'desc' }, + select: { position: true }, + }); + return (last?.position ?? -1) + 1; + } + + private async requireDocument({ + documentId, + organizationId, + }: { + documentId: string; + organizationId: string; + }) { + const document = await db.ismsDocument.findFirst({ + where: { + id: documentId, + organizationId, + type: 'roles_and_responsibilities', + }, + }); + if (!document) { + throw new NotFoundException('ISMS roles document not found'); + } + return document; + } + + private async requireRoleInDocument({ + roleId, + documentId, + }: { + roleId: string; + documentId: string; + }) { + const role = await db.ismsRole.findFirst({ + where: { id: roleId, documentId }, + }); + if (!role) { + throw new NotFoundException('Role not found in document'); + } + return role; + } + + private async requireMember({ + memberId, + organizationId, + }: { + memberId: string; + organizationId: string; + }) { + // Assignments must reference active People members (CS-698). + const member = await db.member.findFirst({ + where: { id: memberId, organizationId, deactivated: false }, + }); + if (!member) { + throw new NotFoundException('Active member not found in organization'); + } + return member; + } + + private async requireAssignment({ + assignmentId, + organizationId, + }: { + assignmentId: string; + organizationId: string; + }) { + const assignment = await db.ismsRoleAssignment.findFirst({ + where: { id: assignmentId, role: { document: { organizationId } } }, + }); + if (!assignment) { + throw new NotFoundException('Role assignment not found'); + } + return assignment; + } +} diff --git a/apps/api/src/isms/isms-role.service.spec.ts b/apps/api/src/isms/isms-role.service.spec.ts new file mode 100644 index 0000000000..dc0cc7514e --- /dev/null +++ b/apps/api/src/isms/isms-role.service.spec.ts @@ -0,0 +1,139 @@ +import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { IsmsRoleService } from './isms-role.service'; + +jest.mock('@db', () => { + const db = { + ismsDocument: { findFirst: jest.fn(), findUnique: jest.fn(), update: jest.fn() }, + member: { findFirst: jest.fn() }, + ismsRole: { + findFirst: jest.fn(), + count: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); + +const mockDb = jest.mocked(db); + +describe('IsmsRoleService', () => { + let service: IsmsRoleService; + + beforeEach(() => { + jest.clearAllMocks(); + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ status: 'draft' }); + service = new IsmsRoleService(); + }); + + describe('create', () => { + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + dto: { name: 'Data Protection Officer' }, + }; + + it('throws NotFoundException when the document is missing', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.create(args)).rejects.toThrow(NotFoundException); + }); + + it('creates a custom (manual, roleKey null) role', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1' }); + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ position: 1 }); + (mockDb.ismsRole.create as jest.Mock).mockResolvedValue({ id: 'role_1' }); + + await service.create(args); + + expect(mockDb.ismsRole.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + roleKey: null, + source: 'manual', + position: 2, + name: 'Data Protection Officer', + authorityGrantedBy: 'Top Management', + }), + }); + }); + }); + + describe('update', () => { + it('throws NotFoundException when the role is not in the org', async () => { + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.update({ roleId: 'role_1', organizationId: 'org_1', dto: {} }), + ).rejects.toThrow(NotFoundException); + }); + + it('saves the audit route and flips source to manual', async () => { + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ + id: 'role_1', + documentId: 'doc_1', + source: 'derived', + }); + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsRole.update as jest.Mock).mockResolvedValue({}); + + await service.update({ + roleId: 'role_1', + organizationId: 'org_1', + dto: { auditRoute: 'in_house', auditRouteMemberId: 'mem_1' }, + }); + + expect(mockDb.ismsRole.update).toHaveBeenCalledWith({ + where: { id: 'role_1' }, + data: expect.objectContaining({ + auditRoute: 'in_house', + auditRouteMemberId: 'mem_1', + source: 'manual', + }), + }); + }); + + it('rejects an audit-route member who is not in the org', async () => { + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ + id: 'role_1', + documentId: 'doc_1', + }); + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.update({ + roleId: 'role_1', + organizationId: 'org_1', + dto: { auditRouteMemberId: 'mem_other' }, + }), + ).rejects.toThrow(NotFoundException); + expect(mockDb.ismsRole.update).not.toHaveBeenCalled(); + }); + }); + + describe('remove', () => { + it('blocks deleting a seeded role', async () => { + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ + id: 'role_1', + roleKey: 'spo', + documentId: 'doc_1', + }); + await expect( + service.remove({ roleId: 'role_1', organizationId: 'org_1' }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.ismsRole.delete).not.toHaveBeenCalled(); + }); + + it('deletes a custom role', async () => { + (mockDb.ismsRole.findFirst as jest.Mock).mockResolvedValue({ + id: 'role_1', + roleKey: null, + documentId: 'doc_1', + }); + (mockDb.ismsRole.delete as jest.Mock).mockResolvedValue({}); + const result = await service.remove({ roleId: 'role_1', organizationId: 'org_1' }); + expect(result).toEqual({ success: true }); + expect(mockDb.ismsRole.delete).toHaveBeenCalledWith({ where: { id: 'role_1' } }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-role.service.ts b/apps/api/src/isms/isms-role.service.ts new file mode 100644 index 0000000000..028222299a --- /dev/null +++ b/apps/api/src/isms/isms-role.service.ts @@ -0,0 +1,199 @@ +import { + BadRequestException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { lockDocument } from './utils/document-lock'; +import { parseOptionalDate } from './utils/parse-optional-date'; +import type { + CreateRoleInput, + UpdateRoleInput, +} from './registers/register-registry'; + +/** + * CRUD for the ISMS Governance Roles register (clause 5.3). The four seeded roles + * are created idempotently by seedRolesIfMissing; this service handles custom-role + * creation, edits to any role's text, and the Internal Auditor route selection. + * Editing a row flips its source to 'manual' (records the override); seeded roles + * cannot be removed. + */ +@Injectable() +export class IsmsRoleService { + async create({ + documentId, + organizationId, + dto, + }: { + documentId: string; + organizationId: string; + dto: CreateRoleInput; + }) { + await this.requireDocument({ documentId, organizationId }); + + return db.$transaction(async (tx) => { + await lockDocument(tx, documentId); + const position = + dto.position ?? (await this.nextPosition({ tx, documentId })); + await invalidateApprovalIfNeeded({ tx, documentId }); + return tx.ismsRole.create({ + data: { + documentId, + roleKey: null, // customer-added custom role + name: dto.name, + description: dto.description ?? '', + responsibilities: dto.responsibilities ?? '', + authorities: dto.authorities ?? '', + authorityGrantedBy: dto.authorityGrantedBy ?? 'Top Management', + requiredCompetence: dto.requiredCompetence ?? '', + source: 'manual', + position, + }, + }); + }); + } + + async update({ + roleId, + organizationId, + dto, + }: { + roleId: string; + organizationId: string; + dto: UpdateRoleInput; + }) { + const role = await this.requireRole({ roleId, organizationId }); + const auditRouteMemberId = await this.resolveMember({ + memberId: dto.auditRouteMemberId, + organizationId, + }); + + return db.$transaction(async (tx) => { + // Same per-document lock submit/approve take, so an edit can't race the + // submission's completeness check. + await lockDocument(tx, role.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: role.documentId }); + return tx.ismsRole.update({ + where: { id: roleId }, + data: { + name: dto.name ?? undefined, + description: dto.description ?? undefined, + responsibilities: dto.responsibilities ?? undefined, + authorities: dto.authorities ?? undefined, + authorityGrantedBy: dto.authorityGrantedBy ?? undefined, + requiredCompetence: dto.requiredCompetence ?? undefined, + auditRoute: dto.auditRoute === undefined ? undefined : dto.auditRoute, + auditRouteMemberId: + dto.auditRouteMemberId === undefined ? undefined : auditRouteMemberId, + auditFirmName: + dto.auditFirmName === undefined ? undefined : dto.auditFirmName, + auditEvidenceRef: + dto.auditEvidenceRef === undefined + ? undefined + : dto.auditEvidenceRef, + auditCourse: + dto.auditCourse === undefined ? undefined : dto.auditCourse, + auditDueDate: parseOptionalDate(dto.auditDueDate), + position: dto.position ?? undefined, + source: 'manual', + }, + }); + }); + } + + async remove({ + roleId, + organizationId, + }: { + roleId: string; + organizationId: string; + }) { + const role = await this.requireRole({ roleId, organizationId }); + if (role.roleKey) { + throw new BadRequestException( + 'Seeded governance roles cannot be removed; edit their text instead.', + ); + } + await db.$transaction(async (tx) => { + await lockDocument(tx, role.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: role.documentId }); + await tx.ismsRole.delete({ where: { id: roleId } }); + }); + return { success: true }; + } + + private async resolveMember({ + memberId, + organizationId, + }: { + memberId: string | null | undefined; + organizationId: string; + }): Promise { + if (memberId === undefined) return undefined; + const trimmed = memberId?.trim(); + if (!trimmed) return null; + // Role holders (incl. the audit-route member) must be active People members. + const member = await db.member.findFirst({ + where: { id: trimmed, organizationId, deactivated: false }, + }); + if (!member) { + throw new NotFoundException('Active member not found in organization'); + } + return trimmed; + } + + private async nextPosition({ + tx, + documentId, + }: { + tx: Prisma.TransactionClient; + documentId: string; + }) { + const last = await tx.ismsRole.findFirst({ + where: { documentId }, + orderBy: { position: 'desc' }, + select: { position: true }, + }); + return (last?.position ?? -1) + 1; + } + + private async requireDocument({ + documentId, + organizationId, + }: { + documentId: string; + organizationId: string; + }) { + // Scope to the Roles document type: role rows must never attach to another + // ISMS document (they'd be invisible to the Clause 5.3 export). + const document = await db.ismsDocument.findFirst({ + where: { + id: documentId, + organizationId, + type: 'roles_and_responsibilities', + }, + }); + if (!document) { + throw new NotFoundException('ISMS roles document not found'); + } + return document; + } + + private async requireRole({ + roleId, + organizationId, + }: { + roleId: string; + organizationId: string; + }) { + const role = await db.ismsRole.findFirst({ + where: { id: roleId, document: { organizationId } }, + }); + if (!role) { + throw new NotFoundException('Role not found'); + } + return role; + } +} diff --git a/apps/api/src/isms/isms-version.service.spec.ts b/apps/api/src/isms/isms-version.service.spec.ts index ceff4d5764..8a29625c43 100644 --- a/apps/api/src/isms/isms-version.service.spec.ts +++ b/apps/api/src/isms/isms-version.service.spec.ts @@ -24,6 +24,7 @@ jest.mock('@db', () => ({ jest.mock('./utils/export-payload', () => ({ buildExportInput: jest.fn(() => ({ rows: [] })), resolveOrgProfile: jest.fn(), + resolveRolesExtras: jest.fn(), parseExportSnapshot: jest.fn(() => null), })); jest.mock('./utils/export-metadata', () => ({ diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts index d54a0a9926..d22055f92f 100644 --- a/apps/api/src/isms/isms-version.service.ts +++ b/apps/api/src/isms/isms-version.service.ts @@ -16,6 +16,7 @@ import { parseExportSnapshot, renderSnapshot, resolveOrgProfile, + resolveRolesExtras, type IsmsExportSnapshot, type LoadedExportDocument, } from './utils/export-payload'; @@ -69,7 +70,8 @@ export class IsmsVersionService { // Read the org profile through the same transaction so the snapshot's profile // and the register rows written in this transaction share one point in time. const orgProfile = await resolveOrgProfile(document, tx); - const input = buildExportInput({ document, orgProfile }); + const rolesExtras = await resolveRolesExtras(document, tx); + const input = buildExportInput({ document, orgProfile, rolesExtras }); const metadata = buildExportMetadata({ type: document.type, title: document.title, diff --git a/apps/api/src/isms/isms.module.ts b/apps/api/src/isms/isms.module.ts index 9d894b737a..68cb5e841f 100644 --- a/apps/api/src/isms/isms.module.ts +++ b/apps/api/src/isms/isms.module.ts @@ -9,6 +9,8 @@ import { IsmsDocumentControlService } from './isms-document-control.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; import { IsmsRequirementService } from './isms-requirement.service'; import { IsmsObjectiveService } from './isms-objective.service'; +import { IsmsRoleService } from './isms-role.service'; +import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsNarrativeService } from './isms-narrative.service'; import { IsmsProfileController } from './wizard/isms-profile.controller'; import { IsmsProfileService } from './wizard/isms-profile.service'; @@ -32,6 +34,8 @@ import { AttachmentsModule } from '../attachments/attachments.module'; IsmsInterestedPartyService, IsmsRequirementService, IsmsObjectiveService, + IsmsRoleService, + IsmsRoleAssignmentService, IsmsNarrativeService, IsmsProfileService, ], @@ -44,6 +48,8 @@ import { AttachmentsModule } from '../attachments/attachments.module'; IsmsInterestedPartyService, IsmsRequirementService, IsmsObjectiveService, + IsmsRoleService, + IsmsRoleAssignmentService, IsmsNarrativeService, IsmsProfileService, ], diff --git a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts index 318f0adfe4..8326f12b8c 100644 --- a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts +++ b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts @@ -72,7 +72,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template const result = await service.ensureSetup(dto); expect(mockDb.ismsDocument.createMany).toHaveBeenCalledTimes(1); - expect(createManyData()).toHaveLength(5); + expect(createManyData()).toHaveLength(6); // Definition-derived docs carry no templateId. expect(createManyData()[0].templateId).toBeNull(); expect(result.success).toBe(true); @@ -96,7 +96,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template await service.ensureSetup(dto); - expect(createManyData()).toHaveLength(6); + expect(createManyData()).toHaveLength(7); expect(createManyData()[0].requirementId).toBeNull(); }); }); diff --git a/apps/api/src/isms/isms.service.lifecycle.spec.ts b/apps/api/src/isms/isms.service.lifecycle.spec.ts index 2defb6ba8f..5184ab0772 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -9,17 +9,21 @@ import type { IsmsVersionService } from './isms-version.service'; // approve() (the CS-701 freeze/version flow) is covered in // isms.service.approve.spec.ts. -jest.mock('@db', () => ({ - db: { +jest.mock('@db', () => { + const db = { ismsDocument: { findFirst: jest.fn(), update: jest.fn(), updateMany: jest.fn(), }, - member: { findFirst: jest.fn() }, - $transaction: jest.fn(), - }, -})); + member: { findFirst: jest.fn(), findMany: jest.fn() }, + ismsRole: { findMany: jest.fn() }, + // lockDocument runs $executeRaw; submitForApproval wraps validate+update in a tx. + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); const mockDb = jest.mocked(db); @@ -118,6 +122,77 @@ describe('IsmsService document lifecycle', () => { }), }); }); + + it('blocks a Roles doc when a required role is only assigned to a deactivated member', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + type: 'roles_and_responsibilities', + }); + // Active members exclude 'mem_dead'. + (mockDb.member.findMany as jest.Mock).mockResolvedValue([ + { id: 'mem_active' }, + { id: 'b' }, + { id: 'c' }, + { id: 'd' }, + ]); + (mockDb.ismsRole.findMany as jest.Mock).mockResolvedValue([ + { + roleKey: 'top_management', + name: 'Top Management', + auditRoute: null, + assignments: [{ memberId: 'mem_dead' }], // only a deactivated member + }, + { roleKey: 'spo', name: 'SPO', auditRoute: null, assignments: [{ memberId: 'b' }] }, + { roleKey: 'deputy_spo', name: 'Deputy SPO', auditRoute: null, assignments: [{ memberId: 'c' }] }, + { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: 'Acme', + auditEvidenceRef: 'cert', + assignments: [{ memberId: 'd' }], + }, + ]); + + await expect(service.submitForApproval(args)).rejects.toThrow(BadRequestException); + expect(mockDb.ismsDocument.update).not.toHaveBeenCalled(); + }); + + it('allows a Roles doc when every seeded role has an active assignment + route', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + type: 'roles_and_responsibilities', + }); + (mockDb.member.findMany as jest.Mock).mockResolvedValue([ + { id: 'a' }, + { id: 'b' }, + { id: 'c' }, + { id: 'd' }, + { id: 'e' }, + ]); + (mockDb.ismsRole.findMany as jest.Mock).mockResolvedValue([ + { roleKey: 'top_management', name: 'Top Management', auditRoute: null, assignments: [{ memberId: 'a' }] }, + { roleKey: 'spo', name: 'SPO', auditRoute: null, assignments: [{ memberId: 'b' }] }, + { roleKey: 'deputy_spo', name: 'Deputy SPO', auditRoute: null, assignments: [{ memberId: 'c' }] }, + { + roleKey: 'internal_auditor', + name: 'Internal Auditor', + auditRoute: 'external', + auditFirmName: 'Acme Audit LLP', + auditEvidenceRef: 'LA cert on file', + assignments: [{ memberId: 'd' }], + }, + ]); + (mockDb.ismsDocument.update as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + }); + + await service.submitForApproval(args); + expect(mockDb.ismsDocument.update).toHaveBeenCalled(); + }); }); describe('decline', () => { diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index 39777c115c..c1b2110ada 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -5,10 +5,12 @@ import { NotFoundException, } from '@nestjs/common'; import { db } from '@db'; +import type { Prisma } from '@db'; import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; import { deriveControlLinks, resolveDocumentPlans } from './utils/ensure-setup-plan'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; +import { roleValidationMessages, seedRolesIfMissing } from './documents/roles'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; import { lockDocument } from './utils/document-lock'; @@ -135,6 +137,21 @@ export class IsmsService { controlTemplateIds: controlTemplatesByType.get(doc.type) ?? [], }); } + + // Seed the four governance roles for a newly-created Roles document so they + // are visible with default text on first load, without a Generate click. + // Idempotent by roleKey, so concurrent provisioning calls can't duplicate. + const rolesDoc = created.find( + (doc) => doc.type === 'roles_and_responsibilities', + ); + if (rolesDoc) { + const memberCount = await db.member.count({ + where: { organizationId, deactivated: false }, + }); + await db.$transaction((tx) => + seedRolesIfMissing({ tx, documentId: rolesDoc.id, memberCount }), + ); + } } async getDocument({ @@ -156,6 +173,10 @@ export class IsmsService { interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, objectives: { orderBy: { position: 'asc' } }, + roles: { + orderBy: { position: 'asc' }, + include: { assignments: { orderBy: { position: 'asc' } } }, + }, controlLinks: { select: { id: true, @@ -189,16 +210,30 @@ export class IsmsService { throw new NotFoundException('Approver not found in organization'); } - await this.requireDocument({ documentId, organizationId }); + const document = await this.requireDocument({ documentId, organizationId }); - return db.ismsDocument.update({ - where: { id: documentId }, - data: { - approverId: dto.approverId, - status: 'needs_review', - approvedAt: null, - declinedAt: null, - }, + return db.$transaction(async (tx) => { + // Serialize with concurrent register edits (which take the same per-document + // lock) so the completeness check and the status flip are atomic — an edit + // can't invalidate the document between validation and the transition to + // needs_review (TOCTOU). Validate inside the lock, then transition. + await lockDocument(tx, documentId); + + // Clause 5.3 generate-time validation, enforced server-side so it can't be + // bypassed by calling the API directly (the client disables Submit too). + if (document.type === 'roles_and_responsibilities') { + await this.assertRolesComplete({ tx, documentId, organizationId }); + } + + return tx.ismsDocument.update({ + where: { id: documentId }, + data: { + approverId: dto.approverId, + status: 'needs_review', + approvedAt: null, + declinedAt: null, + }, + }); }); } @@ -343,6 +378,57 @@ export class IsmsService { } } + /** + * Enforce clause-5.3 completeness before the Roles document can be submitted for + * approval (each seeded role assigned — except Deputy SPO in the 1-3 band — and + * the Internal Auditor route chosen). Mirrors the client-side gate. + */ + private async assertRolesComplete({ + tx, + documentId, + organizationId, + }: { + tx: Prisma.TransactionClient; + documentId: string; + organizationId: string; + }) { + const [roles, activeMembers] = await Promise.all([ + tx.ismsRole.findMany({ + where: { documentId }, + select: { + roleKey: true, + name: true, + auditRoute: true, + auditRouteMemberId: true, + auditFirmName: true, + auditEvidenceRef: true, + auditCourse: true, + auditDueDate: true, + assignments: { select: { memberId: true } }, + }, + }), + tx.member.findMany({ + where: { organizationId, deactivated: false }, + select: { id: true }, + }), + ]); + // A role "assigned" only to a deactivated/removed member is not really + // covered — count only assignments that resolve to an active member. + // roleValidationMessages counts only assignments (and audit-route members) + // that resolve to an active member, so pass the raw rows + the active set. + const activeMemberIds = new Set(activeMembers.map((member) => member.id)); + const messages = roleValidationMessages({ + roles, + memberCount: activeMemberIds.size, + activeMemberIds, + }); + if (messages.length > 0) { + throw new BadRequestException( + `This Clause 5.3 document is not ready to submit. ${messages.join(' ')}`, + ); + } + } + private async requireDocument({ documentId, organizationId, diff --git a/apps/api/src/isms/registers/register-registry.ts b/apps/api/src/isms/registers/register-registry.ts index d331ef4786..67b0c711bb 100644 --- a/apps/api/src/isms/registers/register-registry.ts +++ b/apps/api/src/isms/registers/register-registry.ts @@ -4,6 +4,8 @@ import type { IsmsContextIssueService } from '../isms-context-issue.service'; import type { IsmsInterestedPartyService } from '../isms-interested-party.service'; import type { IsmsObjectiveService } from '../isms-objective.service'; import type { IsmsRequirementService } from '../isms-requirement.service'; +import type { IsmsRoleService } from '../isms-role.service'; +import type { IsmsRoleAssignmentService } from '../isms-role-assignment.service'; /** * One generic dispatch for every ISMS register row (context issues, interested @@ -16,6 +18,13 @@ import type { IsmsRequirementService } from '../isms-requirement.service'; const position = z.number().int().min(0).optional(); const OBJECTIVE_STATUS = ['not_started', 'on_track', 'at_risk', 'met'] as const; +const AUDIT_ROUTE = ['in_house', 'external', 'training_planned'] as const; +const COMPETENCE_BASIS = [ + 'education', + 'training', + 'experience', + 'combination', +] as const; const schemas = { contextIssueCreate: z.object({ @@ -82,6 +91,49 @@ const schemas = { status: z.enum(OBJECTIVE_STATUS).optional(), position, }), + roleCreate: z.object({ + name: z.string().min(1), + description: z.string().optional(), + responsibilities: z.string().optional(), + authorities: z.string().optional(), + authorityGrantedBy: z.string().optional(), + requiredCompetence: z.string().optional(), + position, + }), + roleUpdate: z.object({ + name: z.string().min(1).optional(), + description: z.string().optional(), + responsibilities: z.string().optional(), + authorities: z.string().optional(), + authorityGrantedBy: z.string().optional(), + requiredCompetence: z.string().optional(), + // Internal Auditor route. Nullish: undefined = leave as-is, null = clear. + auditRoute: z.enum(AUDIT_ROUTE).nullish(), + auditRouteMemberId: z.string().nullish(), + auditFirmName: z.string().nullish(), + auditEvidenceRef: z.string().nullish(), + auditCourse: z.string().nullish(), + auditDueDate: z.string().nullish(), // ISO date string + position, + }), + roleAssignmentCreate: z.object({ + roleId: z.string(), + memberId: z.string(), + basisOfCompetence: z.enum(COMPETENCE_BASIS).nullish(), + evidenceRetained: z.string().nullish(), + gap: z.string().nullish(), + remediationAction: z.string().nullish(), + remediationDueDate: z.string().nullish(), + position, + }), + roleAssignmentUpdate: z.object({ + basisOfCompetence: z.enum(COMPETENCE_BASIS).nullish(), + evidenceRetained: z.string().nullish(), + gap: z.string().nullish(), + remediationAction: z.string().nullish(), + remediationDueDate: z.string().nullish(), + position, + }), } as const; // Inferred input types — the single source of truth for register row shapes. @@ -99,12 +151,22 @@ export type CreateRequirementInput = z.infer; export type UpdateRequirementInput = z.infer; export type CreateObjectiveInput = z.infer; export type UpdateObjectiveInput = z.infer; +export type CreateRoleInput = z.infer; +export type UpdateRoleInput = z.infer; +export type CreateRoleAssignmentInput = z.infer< + typeof schemas.roleAssignmentCreate +>; +export type UpdateRoleAssignmentInput = z.infer< + typeof schemas.roleAssignmentUpdate +>; export const ISMS_REGISTER_KEYS = [ 'context-issues', 'interested-parties', 'requirements', 'objectives', + 'roles', + 'role-assignments', ] as const; export type IsmsRegisterKey = (typeof ISMS_REGISTER_KEYS)[number]; @@ -139,6 +201,8 @@ export interface RegisterServices { interestedParties: IsmsInterestedPartyService; requirements: IsmsRequirementService; objectives: IsmsObjectiveService; + roles: IsmsRoleService; + roleAssignments: IsmsRoleAssignmentService; } /** Build the register → handler map from the injected per-register services. */ @@ -210,5 +274,40 @@ export function createRegisterRegistry( remove: ({ rowId, organizationId }) => services.objectives.remove({ objectiveId: rowId, organizationId }), }, + roles: { + create: ({ documentId, organizationId, data }) => + services.roles.create({ + documentId, + organizationId, + dto: parse(schemas.roleCreate, data), + }), + update: ({ rowId, organizationId, data }) => + services.roles.update({ + roleId: rowId, + organizationId, + dto: parse(schemas.roleUpdate, data), + }), + remove: ({ rowId, organizationId }) => + services.roles.remove({ roleId: rowId, organizationId }), + }, + 'role-assignments': { + create: ({ documentId, organizationId, data }) => + services.roleAssignments.create({ + documentId, + organizationId, + dto: parse(schemas.roleAssignmentCreate, data), + }), + update: ({ rowId, organizationId, data }) => + services.roleAssignments.update({ + assignmentId: rowId, + organizationId, + dto: parse(schemas.roleAssignmentUpdate, data), + }), + remove: ({ rowId, organizationId }) => + services.roleAssignments.remove({ + assignmentId: rowId, + organizationId, + }), + }, }; } diff --git a/apps/api/src/isms/utils/document-types.spec.ts b/apps/api/src/isms/utils/document-types.spec.ts index dd479691ec..2e7aff6e9d 100644 --- a/apps/api/src/isms/utils/document-types.spec.ts +++ b/apps/api/src/isms/utils/document-types.spec.ts @@ -1,8 +1,8 @@ import { ISMS_TYPE_DEFINITIONS, matchRequirementId } from './document-types'; describe('ISMS_TYPE_DEFINITIONS', () => { - it('defines all six foundational document types with clauses', () => { - expect(ISMS_TYPE_DEFINITIONS).toHaveLength(6); + it('defines all seven foundational document types with clauses', () => { + expect(ISMS_TYPE_DEFINITIONS).toHaveLength(7); const types = ISMS_TYPE_DEFINITIONS.map((d) => d.type); expect(types).toEqual( expect.arrayContaining([ @@ -11,6 +11,7 @@ describe('ISMS_TYPE_DEFINITIONS', () => { 'interested_parties_requirements', 'isms_scope', 'leadership_commitment', + 'roles_and_responsibilities', 'objectives_plan', ]), ); diff --git a/apps/api/src/isms/utils/document-types.ts b/apps/api/src/isms/utils/document-types.ts index a4179bdbd5..7fe6c94643 100644 --- a/apps/api/src/isms/utils/document-types.ts +++ b/apps/api/src/isms/utils/document-types.ts @@ -52,6 +52,13 @@ export const ISMS_TYPE_DEFINITIONS: IsmsTypeDefinition[] = [ description: 'Evidence of top management leadership and commitment to the ISMS (ISO 27001 clause 5.1).', }, + { + type: 'roles_and_responsibilities', + clause: '5.3', + title: 'Roles, Responsibilities and Authorities', + description: + 'The ISMS governance roles, their responsibilities and authorities, and the members who hold them (ISO 27001 clause 5.3).', + }, { type: 'objectives_plan', clause: '6.2', diff --git a/apps/api/src/isms/utils/export-payload.ts b/apps/api/src/isms/utils/export-payload.ts index efd9346c33..e534ec8a5a 100644 --- a/apps/api/src/isms/utils/export-payload.ts +++ b/apps/api/src/isms/utils/export-payload.ts @@ -3,7 +3,15 @@ import { db } from '@db'; import type { IsmsDocumentType, Prisma } from '@db'; import { buildExportSections } from '../documents/registry'; import { loadOrgProfile } from '../documents/org-profile'; -import type { DocumentExportInput, IsmsOrgProfile } from '../documents/types'; +import { + loadRolesExtras, + type RolesExtras, +} from '../documents/roles-export-data'; +import type { + DocumentExportInput, + IsmsOrgProfile, + RoleExportRow, +} from '../documents/types'; import { buildExportMetadata } from './export-metadata'; import { generateIsmsExportFile, @@ -34,6 +42,10 @@ export const EXPORT_DOCUMENT_INCLUDE = { interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, objectives: { orderBy: { position: 'asc' } }, + roles: { + orderBy: { position: 'asc' }, + include: { assignments: { orderBy: { position: 'asc' } } }, + }, } satisfies Prisma.IsmsDocumentInclude; export type LoadedExportDocument = Prisma.IsmsDocumentGetPayload<{ @@ -64,13 +76,55 @@ export async function resolveOrgProfile( }); } +/** The Roles document (5.3) resolves member names + ownership; other types don't. */ +export async function resolveRolesExtras( + document: LoadedExportDocument, + client?: Prisma.TransactionClient, +): Promise { + if (document.type !== 'roles_and_responsibilities') return undefined; + return loadRolesExtras({ organizationId: document.organizationId, client }); +} + +function formatDateYmd(date: Date | null): string | null { + return date ? date.toISOString().slice(0, 10) : null; +} + +/** Map role rows + assignments into export rows, resolving holder names. */ +function mapRoles( + document: LoadedExportDocument, + extras: RolesExtras, +): RoleExportRow[] { + return document.roles.map((role) => ({ + roleKey: role.roleKey, + name: role.name, + description: role.description, + responsibilities: role.responsibilities, + authorities: role.authorities, + authorityGrantedBy: role.authorityGrantedBy, + requiredCompetence: role.requiredCompetence, + holders: role.assignments + .map((assignment) => extras.memberNames[assignment.memberId]) + .filter((name): name is string => !!name), + auditRoute: role.auditRoute, + auditRouteHolderName: role.auditRouteMemberId + ? (extras.memberNames[role.auditRouteMemberId] ?? null) + : null, + auditFirmName: role.auditFirmName, + auditEvidenceRef: role.auditEvidenceRef, + auditCourse: role.auditCourse, + auditDueDate: formatDateYmd(role.auditDueDate), + })); +} + /** Map the loaded document's live rows + draft narrative into export input. */ export function buildExportInput({ document, orgProfile, + rolesExtras, }: { document: LoadedExportDocument; orgProfile?: IsmsOrgProfile; + rolesExtras?: RolesExtras; }): DocumentExportInput { return { contextIssues: document.contextIssues.map((issue) => ({ @@ -99,6 +153,9 @@ export function buildExportInput({ })), narrative: document.draftNarrative ?? null, orgProfile, + roles: rolesExtras ? mapRoles(document, rolesExtras) : undefined, + operationalOwnership: rolesExtras?.operationalOwnership, + band: rolesExtras?.band, }; } @@ -118,7 +175,8 @@ export async function buildDraftSnapshot( document: LoadedExportDocument, ): Promise { const orgProfile = await resolveOrgProfile(document); - const input = buildExportInput({ document, orgProfile }); + const rolesExtras = await resolveRolesExtras(document); + const input = buildExportInput({ document, orgProfile, rolesExtras }); const metadata = buildExportMetadata({ type: document.type, title: document.title, diff --git a/apps/api/src/isms/utils/parse-optional-date.spec.ts b/apps/api/src/isms/utils/parse-optional-date.spec.ts new file mode 100644 index 0000000000..1d681a5f42 --- /dev/null +++ b/apps/api/src/isms/utils/parse-optional-date.spec.ts @@ -0,0 +1,31 @@ +import { BadRequestException } from '@nestjs/common'; +import { parseOptionalDate } from './parse-optional-date'; + +describe('parseOptionalDate', () => { + it('passes undefined through (leave column as-is)', () => { + expect(parseOptionalDate(undefined)).toBeUndefined(); + }); + + it('clears on null / empty / whitespace', () => { + expect(parseOptionalDate(null)).toBeNull(); + expect(parseOptionalDate('')).toBeNull(); + expect(parseOptionalDate(' ')).toBeNull(); + }); + + it('parses a valid YYYY-MM-DD to UTC midnight', () => { + const date = parseOptionalDate('2026-05-26'); + expect(date).toBeInstanceOf(Date); + expect((date as Date).toISOString()).toBe('2026-05-26T00:00:00.000Z'); + }); + + it('rejects non-ISO / ambiguous formats', () => { + for (const bad of ['26-05-2026', '05/26/2026', '2026-5-6', '2026', 'garbage']) { + expect(() => parseOptionalDate(bad)).toThrow(BadRequestException); + } + }); + + it('rejects a well-formatted but non-existent calendar date (no silent rollover)', () => { + expect(() => parseOptionalDate('2026-02-30')).toThrow(BadRequestException); + expect(() => parseOptionalDate('2026-13-01')).toThrow(BadRequestException); + }); +}); diff --git a/apps/api/src/isms/utils/parse-optional-date.ts b/apps/api/src/isms/utils/parse-optional-date.ts new file mode 100644 index 0000000000..3ad3531939 --- /dev/null +++ b/apps/api/src/isms/utils/parse-optional-date.ts @@ -0,0 +1,33 @@ +import { BadRequestException } from '@nestjs/common'; + +/** Date-only inputs from the UI's — strict YYYY-MM-DD. */ +const DATE_ONLY = /^\d{4}-\d{2}-\d{2}$/; + +/** + * Parse an optional date-only string (YYYY-MM-DD) from a register DTO into a + * Prisma-friendly value, using the three-state convention shared by the ISMS + * services: + * undefined → leave the column as-is (field omitted) + * null / empty / whitespace → clear the column + * a valid YYYY-MM-DD string → the parsed Date (UTC midnight) + * + * Validation is strict: the format must be YYYY-MM-DD AND the parsed value must + * round-trip to the same string, so `new Date`'s permissive parsing can't accept + * ambiguous input (e.g. "2026-02-30" rolling to March, or partial strings). + * Throws BadRequestException otherwise. + */ +export function parseOptionalDate( + value: string | null | undefined, +): Date | null | undefined { + if (value === undefined) return undefined; + const trimmed = value?.trim(); + if (!trimmed) return null; + if (!DATE_ONLY.test(trimmed)) { + throw new BadRequestException('Invalid date; expected format YYYY-MM-DD'); + } + const date = new Date(`${trimmed}T00:00:00.000Z`); + if (Number.isNaN(date.getTime()) || date.toISOString().slice(0, 10) !== trimmed) { + throw new BadRequestException('Invalid date; expected a real calendar date'); + } + return date; +} diff --git a/apps/api/src/isms/wizard/isms-profile.service.ts b/apps/api/src/isms/wizard/isms-profile.service.ts index e296572061..c50d1ffb3c 100644 --- a/apps/api/src/isms/wizard/isms-profile.service.ts +++ b/apps/api/src/isms/wizard/isms-profile.service.ts @@ -31,7 +31,8 @@ const GENERATION_ORDER: Record = { interested_parties_requirements: 2, isms_scope: 3, leadership_commitment: 4, - objectives_plan: 5, + roles_and_responsibilities: 5, + objectives_plan: 6, }; const GENERATION_ORDER_DEFAULT = Object.keys(GENERATION_ORDER).length; @@ -125,7 +126,7 @@ export class IsmsProfileService { } /** - * Ensure all six ISMS documents exist, then regenerate each from the latest + * Ensure every ISMS document exists, then regenerate each from the latest * profile + platform data. Called by the wizard on completion so every document * reflects the answers just saved. Returns the regenerated documents. */ diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx index cfa51758d0..4c11a6be30 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx @@ -12,6 +12,7 @@ import type { ApproverOption } from '../components/IsmsApprovalSection'; import { LeadershipClient } from '../components/LeadershipClient'; import { ObjectivesClient } from '../components/ObjectivesClient'; import { RequirementsClient } from '../components/RequirementsClient'; +import { RolesClient } from '../components/RolesClient'; import { ScopeClient } from '../components/ScopeClient'; import { ISMS_SLUG_TO_TYPE, @@ -29,6 +30,8 @@ interface IsmsDetailClientProps { fallbackData: IsmsDocumentData | null; currentMemberId: string | null; approverOptions: ApproverOption[]; + /** All active members (for the Roles member pickers); superset of approvers. */ + memberOptions: ApproverOption[]; } const ISMS_DETAIL_CLIENTS: Record< @@ -39,6 +42,7 @@ const ISMS_DETAIL_CLIENTS: Record< interested_parties_register: InterestedPartiesClient, interested_parties_requirements: RequirementsClient, objectives_plan: ObjectivesClient, + roles_and_responsibilities: RolesClient, isms_scope: ScopeClient, leadership_commitment: LeadershipClient, }; @@ -151,6 +155,12 @@ export default async function IsmsDocumentPage({ .map((p) => ({ id: p.id, name: p.user?.name ?? p.user?.email ?? 'Unknown' })) .sort((a, b) => a.name.localeCompare(b.name)); + // All active members — the Roles document assigns to the whole workforce, not + // just approvers, so it needs the full list plus the headcount for the band. + const memberOptions: ApproverOption[] = activeMembers + .map((p) => ({ id: p.id, name: p.user?.name ?? p.user?.email ?? 'Unknown' })) + .sort((a, b) => a.name.localeCompare(b.name)); + const DetailClient = ISMS_DETAIL_CLIENTS[documentType]; return ( @@ -162,6 +172,7 @@ export default async function IsmsDocumentPage({ fallbackData={fallbackData} currentMemberId={currentMember?.id ?? null} approverOptions={approverOptions} + memberOptions={memberOptions} /> ); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.test.tsx new file mode 100644 index 0000000000..2c2c4f97b8 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.test.tsx @@ -0,0 +1,101 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { IsmsRole } from '../isms-types'; +import type { ApproverOption } from './IsmsApprovalSection'; +import { ismsDesignSystemMock, ismsIconsMock, ismsSharedMock } from './__test-helpers__/dsMocks'; + +vi.mock('@trycompai/design-system', () => ismsDesignSystemMock()); +vi.mock('@trycompai/design-system/icons', () => ismsIconsMock()); +vi.mock('./shared', () => ismsSharedMock()); + +import { AuditRoutePicker } from './AuditRoutePicker'; + +const MEMBERS: ApproverOption[] = [ + { id: 'mem_1', name: 'Alex Petrisor' }, + { id: 'mem_2', name: 'Jordan Lee' }, +]; + +function auditorRole(overrides: Partial = {}): IsmsRole { + return { + id: 'role_ia', + roleKey: 'internal_auditor', + name: 'Internal Auditor', + description: '', + responsibilities: '', + authorities: '', + authorityGrantedBy: 'Top Management', + requiredCompetence: '', + auditRoute: null, + auditRouteMemberId: null, + auditFirmName: null, + auditEvidenceRef: null, + auditCourse: null, + auditDueDate: null, + source: 'derived', + derivedFrom: null, + position: 3, + assignments: [], + ...overrides, + }; +} + +describe('AuditRoutePicker', () => { + beforeEach(() => vi.clearAllMocks()); + + it('warns when the in-house auditor is also the SPO', () => { + render( + , + ); + expect(screen.getByText(/is also assigned as the Security/)).toBeInTheDocument(); + }); + + it('does not warn when the in-house auditor is not the SPO', () => { + render( + , + ); + expect(screen.queryByText(/is also assigned as the Security/)).not.toBeInTheDocument(); + }); + + it('saves an external route, clearing in-house/training fields', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + render( + , + ); + + // Make the form dirty so Save enables, then submit. + fireEvent.change(screen.getByLabelText('External auditor evidence reference'), { + target: { value: 'LA cert #123' }, + }); + fireEvent.click(screen.getByText('Save audit route')); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave).toHaveBeenCalledWith( + expect.objectContaining({ + auditRoute: 'external', + auditFirmName: 'Acme', + auditEvidenceRef: 'LA cert #123', + auditRouteMemberId: null, + auditCourse: null, + auditDueDate: null, + }), + ); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.tsx new file mode 100644 index 0000000000..29b01006cd --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditRoutePicker.tsx @@ -0,0 +1,318 @@ +'use client'; + +import { + Alert, + AlertDescription, + AlertTitle, + Button, + Field, + Grid, + HStack, + Input, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + Stack, +} from '@trycompai/design-system'; +import { WarningAlt } from '@trycompai/design-system/icons'; +import { useEffect } from 'react'; +import { Controller, useForm, useWatch } from 'react-hook-form'; +import type { IsmsRole } from '../isms-types'; +import type { ApproverOption } from './IsmsApprovalSection'; +import { AUDIT_ROUTE_OPTIONS } from './roles-constants'; +import { IsmsFieldLabel } from './shared'; + +/** The audit-route payload sent to the role update endpoint. */ +export interface AuditRouteUpdate { + auditRoute: string | null; + auditRouteMemberId: string | null; + auditFirmName: string | null; + auditEvidenceRef: string | null; + auditCourse: string | null; + auditDueDate: string | null; +} + +interface AuditRouteFormValues { + auditRoute: string; + auditRouteMemberId: string; + auditFirmName: string; + auditEvidenceRef: string; + auditCourse: string; + auditDueDate: string; +} + +interface AuditRoutePickerProps { + role: IsmsRole; + canEdit: boolean; + memberOptions: ApproverOption[]; + /** Member ids assigned to the SPO role, for the conflict-of-interest warning. */ + spoMemberIds: string[]; + onSave: (update: AuditRouteUpdate) => Promise; +} + +function toFormValues(role: IsmsRole): AuditRouteFormValues { + return { + auditRoute: role.auditRoute ?? '', + auditRouteMemberId: role.auditRouteMemberId ?? '', + auditFirmName: role.auditFirmName ?? '', + auditEvidenceRef: role.auditEvidenceRef ?? '', + auditCourse: role.auditCourse ?? '', + auditDueDate: role.auditDueDate?.slice(0, 10) ?? '', + }; +} + +function emptyToNull(value: string): string | null { + const trimmed = value.trim(); + return trimmed ? trimmed : null; +} + +export function AuditRoutePicker({ + role, + canEdit, + memberOptions, + spoMemberIds, + onSave, +}: AuditRoutePickerProps) { + const { + control, + handleSubmit, + reset, + formState: { isDirty, isSubmitting }, + } = useForm({ defaultValues: toFormValues(role) }); + + useEffect(() => { + reset(toFormValues(role)); + }, [role, reset]); + + const route = useWatch({ control, name: 'auditRoute' }); + const selectedMemberId = useWatch({ control, name: 'auditRouteMemberId' }); + + const conflictMember = + route === 'in_house' && selectedMemberId && spoMemberIds.includes(selectedMemberId) + ? (memberOptions.find((option) => option.id === selectedMemberId)?.name ?? + 'This member') + : null; + + const handleSave = handleSubmit(async (values) => { + const inHouseOrTraining = + values.auditRoute === 'in_house' || values.auditRoute === 'training_planned'; + try { + await onSave({ + auditRoute: values.auditRoute || null, + auditRouteMemberId: inHouseOrTraining + ? emptyToNull(values.auditRouteMemberId) + : null, + auditFirmName: + values.auditRoute === 'external' ? emptyToNull(values.auditFirmName) : null, + auditEvidenceRef: + values.auditRoute === 'external' + ? emptyToNull(values.auditEvidenceRef) + : null, + auditCourse: + values.auditRoute === 'training_planned' + ? emptyToNull(values.auditCourse) + : null, + auditDueDate: + values.auditRoute === 'training_planned' + ? emptyToNull(values.auditDueDate) + : null, + }); + } catch { + // The caller surfaces the failure via toast and re-throws; swallow here so a + // failed save keeps the form dirty for retry without an unhandled rejection. + } + }); + + const hasMemberOptions = memberOptions.length > 0; + + return ( + + + ( + + )} + /> + + + {route === 'in_house' ? ( + + ( + // A member id is required (it must resolve to a real person), so we + // never offer editable free text — the select disables when there + // are no members to pick. + + )} + /> + + ) : null} + + {route === 'external' ? ( + + + + ( + + )} + /> + + + + + ( + + )} + /> + + + + ) : null} + + {route === 'training_planned' ? ( + + + ( + + )} + /> + + + + ( + + )} + /> + + + + + ( + + )} + /> + + + + ) : null} + + {conflictMember ? ( + }> + Independence conflict + + {conflictMember} is also assigned as the Security & Privacy Owner. ISO 27001 + requires the internal auditor to be objective and impartial — a person auditing an + ISMS they also run creates a conflict. For teams of your size, outsourcing the + internal audit to an external auditor is the standard route and usually the most + cost-effective way to satisfy the independence requirement. If you keep this as-is, be + prepared to justify the arrangement at Stage 2. + + + ) : null} + + {canEdit ? ( + + + + ) : null} + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx index c4dd38c70e..83402dd4d9 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx @@ -109,6 +109,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: [], objectives: [], + roles: [], controlLinks: [], draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx index 7618d876e2..245a2d6738 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx @@ -132,6 +132,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: PARTIES, interestedPartyRequirements: [], objectives: [], + roles: [], controlLinks: [], draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx index 9e1cccdbe3..addc53c6ad 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx @@ -104,6 +104,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: [], objectives: [], + roles: [], controlLinks: [], draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx index 37ba6d724a..b013b955ec 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx @@ -33,6 +33,11 @@ interface IsmsApprovalSectionProps { canManage: boolean; currentMemberId: string | null; approverOptions: ApproverOption[]; + /** + * When set, "Submit for approval" is disabled and this reason is shown — used + * by documents with generate-time validation (e.g. Roles, clause 5.3). + */ + submitBlockedReason?: string | null; onSubmitForApproval: (approverId: string) => Promise; onApprove: () => Promise; onDecline: () => Promise; @@ -55,6 +60,7 @@ export function IsmsApprovalSection({ canManage, currentMemberId, approverOptions, + submitBlockedReason, onSubmitForApproval, onApprove, onDecline, @@ -174,10 +180,20 @@ export function IsmsApprovalSection({ )} {(showSubmitButton || showResubmitButton) && ( -
- + {submitBlockedReason ? ( + + {submitBlockedReason} + + ) : null}
)} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx index 269a667b86..c4483f018f 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx @@ -191,6 +191,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: [], objectives: [], + roles: [], controlLinks: LINKS, draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx index 6e50a82601..f1c4494ae4 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx @@ -55,6 +55,13 @@ export interface IsmsDocumentShellProps { sectionDescription: string; /** Toast shown after a successful generate, e.g. "Generated issues from platform data". */ generateSuccessMessage: string; + /** + * Optional per-document validation gate. When it returns a non-null reason for + * the current document, "Submit for approval" is disabled and the reason is + * shown (used by the Roles document to enforce clause-5.3 completeness). Other + * documents omit it and are never gated. + */ + getSubmitBlockedReason?: (document: IsmsDocumentData) => string | null; /** Renders the register-specific body once a document is loaded. */ children: (args: IsmsDocumentBodyArgs) => ReactNode; } @@ -78,6 +85,7 @@ export function IsmsDocumentShell({ sectionTitle, sectionDescription, generateSuccessMessage, + getSubmitBlockedReason, children, }: IsmsDocumentShellProps) { const { hasPermission } = usePermissions(); @@ -203,6 +211,7 @@ export function IsmsDocumentShell({ canManage={canManage} currentMemberId={currentMemberId} approverOptions={approverOptions} + submitBlockedReason={getSubmitBlockedReason?.(document) ?? null} onSubmitForApproval={handleSubmit} onApprove={handleApprove} onDecline={handleDecline} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx index e0dd17d495..e55d19fba5 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx @@ -172,6 +172,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: [], objectives: [], + roles: [], controlLinks: [], draftNarrative: NARRATIVE, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx index 424590c6f0..81325a7e8e 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx @@ -115,6 +115,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: [], objectives: OBJECTIVES, + roles: [], controlLinks: [], draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx index 258f1c7920..a6a95596f5 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx @@ -113,6 +113,7 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedParties: [], interestedPartyRequirements: REQUIREMENTS, objectives: [], + roles: [], controlLinks: [], draftNarrative: null, currentVersionId: null, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.test.tsx new file mode 100644 index 0000000000..7102e6d07f --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.test.tsx @@ -0,0 +1,85 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { IsmsRoleAssignment } from '../isms-types'; +import { ismsDesignSystemMock, ismsIconsMock, ismsSharedMock } from './__test-helpers__/dsMocks'; + +vi.mock('@trycompai/design-system', () => ismsDesignSystemMock()); +vi.mock('@trycompai/design-system/icons', () => ismsIconsMock()); +vi.mock('./shared', () => ismsSharedMock()); + +import { RoleAssignmentRow } from './RoleAssignmentRow'; + +function makeAssignment(overrides: Partial = {}): IsmsRoleAssignment { + return { + id: 'ra_1', + roleId: 'role_1', + memberId: 'mem_1', + basisOfCompetence: 'training', + evidenceRetained: 'Training record', + gap: null, + remediationAction: null, + remediationDueDate: null, + position: 0, + ...overrides, + }; +} + +describe('RoleAssignmentRow', () => { + beforeEach(() => vi.clearAllMocks()); + + it('shows the member name and competence basis in read view', () => { + render( + , + ); + expect(screen.getByText('Alex Petrisor')).toBeInTheDocument(); + expect(screen.getByText('Training')).toBeInTheDocument(); + }); + + it('flags a gap and reveals remediation fields when a gap is present', () => { + render( + , + ); + // Read view surfaces the gap text + remediation ('Gap' appears as both a + // badge and a field label, so assert on the unique values instead). + expect(screen.getByText('Needs ISO 27001 course')).toBeInTheDocument(); + expect(screen.getByText('Enrol')).toBeInTheDocument(); + }); + + it('saves edited competence via onUpdate', async () => { + const onUpdate = vi.fn().mockResolvedValue(undefined); + render( + , + ); + + fireEvent.click(screen.getByLabelText('Edit competence for Alex')); + // Remediation fields are visible because a gap is set. + expect(screen.getByLabelText('Remediation action')).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText('Evidence retained'), { + target: { value: 'Updated evidence' }, + }); + fireEvent.click(screen.getByText('Save')); + + await waitFor(() => expect(onUpdate).toHaveBeenCalled()); + expect(onUpdate).toHaveBeenCalledWith( + expect.objectContaining({ evidenceRetained: 'Updated evidence', gap: 'Needs course' }), + ); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.tsx new file mode 100644 index 0000000000..518ad77942 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RoleAssignmentRow.tsx @@ -0,0 +1,277 @@ +'use client'; + +import { + Badge, + Field, + Grid, + HStack, + Input, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + Stack, + Text, + Textarea, +} from '@trycompai/design-system'; +import { useEffect, useState } from 'react'; +import { Controller, useForm, useWatch } from 'react-hook-form'; +import type { IsmsRoleAssignment } from '../isms-types'; +import { + COMPETENCE_BASIS_LABELS, + COMPETENCE_BASIS_OPTIONS, +} from './roles-constants'; +import { IsmsCardActions, IsmsFieldLabel, IsmsRegisterField } from './shared'; + +/** The competence payload sent to the assignment update endpoint. */ +export interface AssignmentCompetenceUpdate { + basisOfCompetence: string | null; + evidenceRetained: string | null; + gap: string | null; + remediationAction: string | null; + remediationDueDate: string | null; +} + +interface CompetenceFormValues { + basisOfCompetence: string; + evidenceRetained: string; + gap: string; + remediationAction: string; + remediationDueDate: string; +} + +interface RoleAssignmentRowProps { + assignment: IsmsRoleAssignment; + memberName: string; + canEdit: boolean; + onUpdate: (update: AssignmentCompetenceUpdate) => Promise; + onRemove: () => Promise; +} + +function toFormValues(assignment: IsmsRoleAssignment): CompetenceFormValues { + return { + basisOfCompetence: assignment.basisOfCompetence ?? '', + evidenceRetained: assignment.evidenceRetained ?? '', + gap: assignment.gap ?? '', + remediationAction: assignment.remediationAction ?? '', + remediationDueDate: assignment.remediationDueDate?.slice(0, 10) ?? '', + }; +} + +function emptyToNull(value: string): string | null { + const trimmed = value.trim(); + return trimmed ? trimmed : null; +} + +export function RoleAssignmentRow({ + assignment, + memberName, + canEdit, + onUpdate, + onRemove, +}: RoleAssignmentRowProps) { + const [isEditing, setIsEditing] = useState(false); + const [isDeleting, setIsDeleting] = useState(false); + + const { + control, + handleSubmit, + reset, + formState: { isDirty, isSubmitting }, + } = useForm({ + defaultValues: toFormValues(assignment), + }); + + useEffect(() => { + if (!isEditing) reset(toFormValues(assignment)); + }, [assignment, isEditing, reset]); + + const gapValue = useWatch({ control, name: 'gap' }); + const hasGap = !!gapValue?.trim(); + + const handleSave = handleSubmit(async (values) => { + try { + await onUpdate({ + basisOfCompetence: values.basisOfCompetence || null, + evidenceRetained: emptyToNull(values.evidenceRetained), + gap: emptyToNull(values.gap), + // Remediation only applies when a gap is recorded. + remediationAction: values.gap.trim() + ? emptyToNull(values.remediationAction) + : null, + remediationDueDate: values.gap.trim() + ? emptyToNull(values.remediationDueDate) + : null, + }); + } catch { + return; + } + setIsEditing(false); + }); + + const handleDelete = async () => { + setIsDeleting(true); + try { + await onRemove(); + } finally { + setIsDeleting(false); + } + }; + + const actions = canEdit ? ( + { + reset(toFormValues(assignment)); + setIsEditing(true); + }} + onSave={handleSave} + onCancel={() => { + reset(toFormValues(assignment)); + setIsEditing(false); + }} + onDelete={handleDelete} + isDirty={isDirty} + isSaving={isSubmitting} + isDeleting={isDeleting} + editLabel={`Edit competence for ${memberName}`} + deleteLabel={`Remove ${memberName}`} + /> + ) : undefined; + + const header = ( + + + {memberName} + + {assignment.gap ? Gap : null} + + ); + + return ( +
+ + {header} + {actions ?
{actions}
: null} + + + {isEditing ? ( + + + + ( + + )} + /> + + + + ( + + )} + /> + + + + + + ( +