diff --git a/apps/api/src/roles/dto/update-built-in-obligations.dto.ts b/apps/api/src/roles/dto/update-built-in-obligations.dto.ts new file mode 100644 index 0000000000..f509e74de3 --- /dev/null +++ b/apps/api/src/roles/dto/update-built-in-obligations.dto.ts @@ -0,0 +1,27 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsBoolean, IsObject, IsOptional, ValidateNested } from 'class-validator'; +import { Type } from 'class-transformer'; + +export class BuiltInObligationsBody { + @ApiProperty({ + description: 'Whether the role must complete compliance tasks.', + example: false, + required: false, + }) + @IsBoolean() + @IsOptional() + compliance?: boolean; +} + +export class UpdateBuiltInObligationsDto { + @ApiProperty({ + description: 'Obligations override for the built-in role.', + example: { compliance: false }, + required: true, + type: BuiltInObligationsBody, + }) + @IsObject() + @ValidateNested() + @Type(() => BuiltInObligationsBody) + obligations!: BuiltInObligationsBody; +} diff --git a/apps/api/src/roles/roles.controller.ts b/apps/api/src/roles/roles.controller.ts index 6515a9e0ca..d28ccd3242 100644 --- a/apps/api/src/roles/roles.controller.ts +++ b/apps/api/src/roles/roles.controller.ts @@ -23,6 +23,7 @@ import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; import type { AuthContext as AuthContextType } from '../auth/types'; import { CreateRoleDto } from './dto/create-role.dto'; +import { UpdateBuiltInObligationsDto } from './dto/update-built-in-obligations.dto'; import { UpdateRoleDto } from './dto/update-role.dto'; import { RolesService } from './roles.service'; @@ -172,6 +173,75 @@ export class RolesController { return { permissions, obligations }; } + @Get('built-in/:name/obligations') + @RequirePermission('ac', 'read') + @ApiOperation({ + summary: 'Get obligations for a built-in role', + description: + 'Returns the effective obligations for a built-in role (owner, admin, auditor, employee, contractor) — DB override if present, else the hardcoded default.', + }) + @ApiParam({ name: 'name', description: 'Built-in role name', example: 'owner' }) + @ApiResponse({ + status: 200, + description: 'Effective obligations for the built-in role', + schema: { + type: 'object', + properties: { + name: { type: 'string' }, + obligations: { + type: 'object', + properties: { compliance: { type: 'boolean' } }, + }, + }, + }, + }) + async getBuiltInObligations( + @OrganizationId() organizationId: string, + @Param('name') name: string, + ) { + const obligations = await this.rolesService.getBuiltInObligations( + organizationId, + name, + ); + return { name, obligations }; + } + + @Patch('built-in/:name/obligations') + @RequirePermission('ac', 'update') + @ApiOperation({ + summary: 'Update obligations for a built-in role', + description: + 'Override the obligations for a built-in role (e.g., turn off the compliance obligation for owners). Permissions stay sourced from the hardcoded defaults.', + }) + @ApiParam({ name: 'name', description: 'Built-in role name', example: 'owner' }) + @ApiBody({ type: UpdateBuiltInObligationsDto }) + @ApiResponse({ + status: 200, + description: 'Obligations updated', + schema: { + type: 'object', + properties: { + name: { type: 'string' }, + obligations: { + type: 'object', + properties: { compliance: { type: 'boolean' } }, + }, + }, + }, + }) + @ApiResponse({ status: 400, description: 'Not a built-in role' }) + async updateBuiltInObligations( + @OrganizationId() organizationId: string, + @Param('name') name: string, + @Body() dto: UpdateBuiltInObligationsDto, + ) { + return this.rolesService.updateBuiltInObligations( + organizationId, + name, + dto.obligations, + ); + } + @Get(':roleId') @RequirePermission('ac', 'read') @ApiOperation({ diff --git a/apps/api/src/roles/roles.service.spec.ts b/apps/api/src/roles/roles.service.spec.ts index b5677628a2..ee709e6f5f 100644 --- a/apps/api/src/roles/roles.service.spec.ts +++ b/apps/api/src/roles/roles.service.spec.ts @@ -74,7 +74,7 @@ jest.mock('@trycompai/auth', () => { const BUILT_IN_ROLE_OBLIGATIONS: Record> = { owner: { compliance: true }, - admin: { compliance: true }, + admin: {}, auditor: {}, employee: { compliance: true }, contractor: { compliance: true }, @@ -97,6 +97,7 @@ jest.mock('@db', () => ({ count: jest.fn(), create: jest.fn(), update: jest.fn(), + upsert: jest.fn(), delete: jest.fn(), }, member: { @@ -229,6 +230,32 @@ describe('RolesService', () => { ).rejects.toThrow('Maximum of 20 custom roles per organization'); }); + it('excludes built-in obligation override rows from the 20-role limit', async () => { + // The count query should filter out override rows so they don't steal + // slots from the customer's custom-role budget. + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.organizationRole.count as jest.Mock).mockResolvedValue(0); + (mockDb.organizationRole.create as jest.Mock).mockResolvedValue({ + id: 'rol_xyz', + name: validDto.name, + permissions: JSON.stringify(validDto.permissions), + obligations: '{}', + organizationId, + createdAt: new Date(), + updatedAt: new Date(), + }); + + await service.createRole(organizationId, validDto, ['owner']); + expect(mockDb.organizationRole.count).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + organizationId, + name: { notIn: expect.arrayContaining(['owner', 'admin']) }, + }), + }), + ); + }); + it('should prevent privilege escalation - cannot grant permissions you do not have', async () => { // Employee trying to grant admin-level permissions const dto = { @@ -713,4 +740,219 @@ describe('RolesService', () => { expect(result.map((m) => m.id)).toEqual(['m1']); }); }); + + describe('getBuiltInObligations', () => { + const organizationId = 'org_1'; + + it('returns the hardcoded default when no override row exists', async () => { + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + const result = await service.getBuiltInObligations(organizationId, 'owner'); + expect(result).toEqual({ compliance: true }); + }); + + it('returns the DB override when one exists (string JSON)', async () => { + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ + obligations: JSON.stringify({ compliance: false }), + }); + const result = await service.getBuiltInObligations(organizationId, 'owner'); + expect(result).toEqual({ compliance: false }); + }); + + it('returns the DB override when one exists (object JSON)', async () => { + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ + obligations: { compliance: false }, + }); + const result = await service.getBuiltInObligations(organizationId, 'owner'); + expect(result).toEqual({ compliance: false }); + }); + + it('returns empty for admin (no default compliance, no override)', async () => { + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + const result = await service.getBuiltInObligations(organizationId, 'admin'); + expect(result).toEqual({}); + }); + + it('rejects unknown role names', async () => { + await expect( + service.getBuiltInObligations(organizationId, 'not-a-built-in'), + ).rejects.toThrow(BadRequestException); + }); + + it('falls back to built-in default when override row exists but has no compliance key', async () => { + // Override row with `{}` should be treated as "no override on compliance" + // — UI must show the built-in default to match what enforcement does. + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ + obligations: '{}', + }); + const result = await service.getBuiltInObligations(organizationId, 'owner'); + expect(result).toEqual({ compliance: true }); + }); + }); + + describe('updateBuiltInObligations', () => { + const organizationId = 'org_1'; + + it('upserts an organization_role row with the built-in name', async () => { + (mockDb.organizationRole.upsert as jest.Mock).mockResolvedValue({ + name: 'owner', + obligations: JSON.stringify({ compliance: false }), + }); + + const result = await service.updateBuiltInObligations( + organizationId, + 'owner', + { compliance: false }, + ); + + expect(result).toEqual({ name: 'owner', obligations: { compliance: false } }); + expect(mockDb.organizationRole.upsert).toHaveBeenCalledWith({ + where: { organizationId_name: { organizationId, name: 'owner' } }, + create: expect.objectContaining({ + organizationId, + name: 'owner', + obligations: JSON.stringify({ compliance: false }), + }), + update: { obligations: JSON.stringify({ compliance: false }) }, + }); + }); + + it('rejects unknown role names', async () => { + await expect( + service.updateBuiltInObligations(organizationId, 'not-a-built-in', { + compliance: true, + }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.organizationRole.upsert).not.toHaveBeenCalled(); + }); + + it('rejects built-in roles whose obligations are not user-editable', async () => { + for (const locked of ['auditor', 'employee', 'contractor']) { + await expect( + service.updateBuiltInObligations(organizationId, locked, { + compliance: false, + }), + ).rejects.toThrow(BadRequestException); + } + expect(mockDb.organizationRole.upsert).not.toHaveBeenCalled(); + }); + + it('accepts owner and admin', async () => { + (mockDb.organizationRole.upsert as jest.Mock).mockResolvedValue({ + name: 'admin', + obligations: JSON.stringify({ compliance: true }), + }); + await expect( + service.updateBuiltInObligations(organizationId, 'admin', { + compliance: true, + }), + ).resolves.toEqual({ name: 'admin', obligations: { compliance: true } }); + }); + }); + + describe('getObligationsForRoles with built-in overrides', () => { + const organizationId = 'org_1'; + + it('returns hardcoded defaults when no DB rows match', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([]); + const result = await service.getObligationsForRoles(organizationId, [ + 'owner', + 'admin', + ]); + // owner has compliance:true by default, admin has none — union is true + expect(result).toEqual({ compliance: true }); + }); + + it('DB override beats the hardcoded default (owner override → no compliance)', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { name: 'owner', obligations: JSON.stringify({ compliance: false }) }, + ]); + const result = await service.getObligationsForRoles(organizationId, [ + 'owner', + ]); + expect(result).toEqual({}); + }); + + it('admin override can opt INTO compliance even though default is none', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { name: 'admin', obligations: JSON.stringify({ compliance: true }) }, + ]); + const result = await service.getObligationsForRoles(organizationId, [ + 'admin', + ]); + expect(result).toEqual({ compliance: true }); + }); + + it('falls back to built-in default when override JSON has no compliance key', async () => { + // A row with `{}` should not silently disable the owner default. + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { name: 'owner', obligations: '{}' }, + ]); + const result = await service.getObligationsForRoles(organizationId, [ + 'owner', + ]); + expect(result).toEqual({ compliance: true }); + }); + }); + + describe('listRoles built-in overrides', () => { + const organizationId = 'org_1'; + + it('hides override rows from customRoles and reflects them on builtInRoles', async () => { + const ownerOverride = { + id: 'rol_owner_override', + name: 'owner', + permissions: '{}', + obligations: JSON.stringify({ compliance: false }), + organizationId, + createdAt: new Date(), + updatedAt: new Date(), + }; + const customRole = { + id: 'rol_custom_1', + name: 'compliance-lead', + permissions: JSON.stringify({ control: ['read'] }), + obligations: '{}', + organizationId, + createdAt: new Date(), + updatedAt: new Date(), + }; + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + ownerOverride, + customRole, + ]); + (mockDb.member.count as jest.Mock).mockResolvedValue(0); + + const result = await service.listRoles(organizationId); + // Override row must not appear as a custom role + expect(result.customRoles.map((r) => r.name)).toEqual(['compliance-lead']); + // Built-in entries carry effective obligations — owner reflects the override + const ownerEntry = result.builtInRoles.find((r) => r.name === 'owner'); + expect(ownerEntry?.obligations).toEqual({ compliance: false }); + // Other built-ins still show their hardcoded defaults + const adminEntry = result.builtInRoles.find((r) => r.name === 'admin'); + expect(adminEntry?.obligations).toEqual({}); + const employeeEntry = result.builtInRoles.find((r) => r.name === 'employee'); + expect(employeeEntry?.obligations).toEqual({ compliance: true }); + }); + + it('shows the built-in default when override row exists with no compliance key', async () => { + // Override row with `{}` should not silently flip the built-in entry off. + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { + id: 'rol_owner_override', + name: 'owner', + permissions: '{}', + obligations: '{}', + organizationId, + createdAt: new Date(), + updatedAt: new Date(), + }, + ]); + (mockDb.member.count as jest.Mock).mockResolvedValue(0); + + const result = await service.listRoles(organizationId); + const ownerEntry = result.builtInRoles.find((r) => r.name === 'owner'); + expect(ownerEntry?.obligations).toEqual({ compliance: true }); + }); + }); }); diff --git a/apps/api/src/roles/roles.service.ts b/apps/api/src/roles/roles.service.ts index 42cdccf5ff..12bb421e6f 100644 --- a/apps/api/src/roles/roles.service.ts +++ b/apps/api/src/roles/roles.service.ts @@ -23,6 +23,40 @@ const VALID_RESOURCES: Record = Object.fromEntries( // Built-in roles that cannot be modified or deleted const BUILT_IN_ROLES = Object.keys(allRoles); +// Subset of built-in roles whose obligations the customer can override. Today +// only owner and admin — the others keep their hardcoded defaults to avoid +// changing behavior for roles the request didn't cover. +const EDITABLE_BUILT_IN_OBLIGATION_ROLES = new Set(['owner', 'admin']); + +function parseObligationsField(value: unknown): RoleObligations { + if (typeof value === 'string') { + try { + return JSON.parse(value) as RoleObligations; + } catch { + return {}; + } + } + return (value as RoleObligations) || {}; +} + +/** + * Resolve the effective obligations for a single role, given an optional DB + * override row. Centralized so every read path applies the same fallback + * rule: a DB row only wins when it explicitly sets `compliance`, otherwise + * we fall back to the hardcoded built-in default. Prevents the UI and the + * enforcement layer from diverging when a row exists with `{}` obligations. + */ +function resolveEffectiveObligations( + roleName: string, + override: RoleObligations | null, +): RoleObligations { + if (override && 'compliance' in override) return override; + if (BUILT_IN_ROLES.includes(roleName)) { + return BUILT_IN_ROLE_OBLIGATIONS[roleName] ?? {}; + } + return override ?? {}; +} + @Injectable() export class RolesService { /** @@ -194,9 +228,11 @@ export class RolesService { throw new BadRequestException(`Role '${dto.name}' already exists`); } - // Check max roles limit + // Check max roles limit — exclude rows that exist solely as obligation + // overrides for built-in roles, since those don't count against the + // customer's 20-custom-role budget. const roleCount = await db.organizationRole.count({ - where: { organizationId }, + where: { organizationId, name: { notIn: BUILT_IN_ROLES } }, }); if (roleCount >= 20) { @@ -226,11 +262,20 @@ export class RolesService { * List all roles for an organization (built-in + custom) */ async listRoles(organizationId: string) { - // Get custom roles - const customRoles = await db.organizationRole.findMany({ + // Get all organization_role rows; rows named after a built-in role are + // obligation overrides, not custom roles. + const allRows = await db.organizationRole.findMany({ where: { organizationId }, orderBy: { createdAt: 'desc' }, }); + const overrideByName = new Map(); + const customRoles = allRows.filter((r) => { + if (BUILT_IN_ROLES.includes(r.name)) { + overrideByName.set(r.name, r); + return false; + } + return true; + }); // Get member counts for custom roles const memberCounts = await Promise.all( @@ -243,12 +288,17 @@ export class RolesService { ); const countMap = new Map(memberCounts.map((mc) => [mc.roleId, mc.count])); - // Include built-in roles info - const builtInRoles = BUILT_IN_ROLES.map((name) => ({ - name, - isBuiltIn: true, - description: this.getBuiltInRoleDescription(name), - })); + // Include built-in roles info (with effective obligations: override or default) + const builtInRoles = BUILT_IN_ROLES.map((name) => { + const override = overrideByName.get(name); + const parsed = override ? parseObligationsField(override.obligations) : null; + return { + name, + isBuiltIn: true, + description: this.getBuiltInRoleDescription(name), + obligations: resolveEffectiveObligations(name, parsed), + }; + }); return { builtInRoles, @@ -533,8 +583,11 @@ export class RolesService { } /** - * Get merged obligations for a list of custom role names. - * Used by the frontend to resolve effective obligations for custom roles. + * Get merged obligations for a list of role names (custom + built-in). + * + * Built-in roles default to `BUILT_IN_ROLE_OBLIGATIONS[name]`, but an + * organization can override by storing an `organization_role` row with the + * built-in name — the DB row wins when present. */ async getObligationsForRoles( organizationId: string, @@ -542,23 +595,87 @@ export class RolesService { ): Promise { if (roleNames.length === 0) return {}; - const customRoles = await db.organizationRole.findMany({ + const dbRoles = await db.organizationRole.findMany({ where: { organizationId, name: { in: roleNames } }, select: { name: true, obligations: true }, }); + const overrideByName = new Map(); + for (const role of dbRoles) { + overrideByName.set(role.name, parseObligationsField(role.obligations)); + } const combined: RoleObligations = {}; - for (const role of customRoles) { - const obligations = - typeof role.obligations === 'string' - ? JSON.parse(role.obligations) - : role.obligations || {}; - if (obligations.compliance) combined.compliance = true; + for (const name of roleNames) { + const fromDb = overrideByName.get(name) ?? null; + const effective = resolveEffectiveObligations(name, fromDb); + if (effective.compliance) combined.compliance = true; } return combined; } + /** + * Upsert an obligation override for a built-in role. The `organization_role` + * row stores only the obligations JSON; permissions stay sourced from the + * hardcoded `BUILT_IN_ROLE_PERMISSIONS` map. + * + * Only owner and admin obligations are user-overridable today (matches the + * customer request); the other built-in roles keep their hardcoded defaults. + */ + async updateBuiltInObligations( + organizationId: string, + roleName: string, + obligations: RoleObligations, + ) { + if (!BUILT_IN_ROLES.includes(roleName)) { + throw new BadRequestException(`Not a built-in role: ${roleName}`); + } + if (!EDITABLE_BUILT_IN_OBLIGATION_ROLES.has(roleName)) { + throw new BadRequestException( + `Obligations are not editable for role: ${roleName}`, + ); + } + + const permissions = JSON.stringify(BUILT_IN_ROLE_PERMISSIONS[roleName] ?? {}); + const obligationsJson = JSON.stringify(obligations); + + const role = await db.organizationRole.upsert({ + where: { organizationId_name: { organizationId, name: roleName } }, + create: { + organizationId, + name: roleName, + permissions, + obligations: obligationsJson, + }, + update: { obligations: obligationsJson }, + }); + + return { + name: role.name, + obligations: JSON.parse(role.obligations) as RoleObligations, + }; + } + + /** + * Read the obligations for a single built-in role for this organization — + * DB override if present, else the hardcoded default. + */ + async getBuiltInObligations( + organizationId: string, + roleName: string, + ): Promise { + if (!BUILT_IN_ROLES.includes(roleName)) { + throw new BadRequestException(`Not a built-in role: ${roleName}`); + } + + const override = await db.organizationRole.findFirst({ + where: { organizationId, name: roleName }, + select: { obligations: true }, + }); + const parsed = override ? parseObligationsField(override.obligations) : null; + return resolveEffectiveObligations(roleName, parsed); + } + /** * Get description for built-in roles */ diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx index b499140ee8..d9fac52325 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx @@ -104,7 +104,15 @@ interface PermissionMatrixProps { onChange: (permissions: Record) => void; obligations?: Record; onObligationsChange?: (obligations: Record) => void; + /** Disables the whole matrix. */ disabled?: boolean; + /** + * When true, keeps the permission matrix and access toggles disabled but + * leaves the obligation toggles editable. Used on built-in role pages so + * customers can opt in/out of the compliance obligation without being able + * to edit the role's permissions. + */ + obligationsEditable?: boolean; } /** @@ -215,7 +223,8 @@ function AccessToggle({ ); } -export function PermissionMatrix({ value, onChange, obligations, onObligationsChange, disabled = false }: PermissionMatrixProps) { +export function PermissionMatrix({ value, onChange, obligations, onObligationsChange, disabled = false, obligationsEditable = false }: PermissionMatrixProps) { + const obligationsDisabled = disabled && !obligationsEditable; const handleObligationChange = (key: string, enabled: boolean) => { if (!onObligationsChange) return; const newObligations = { ...obligations }; @@ -306,7 +315,7 @@ export function PermissionMatrix({ value, onChange, obligations, onObligationsCh toggle={toggle} enabled={Boolean(obligations?.[toggle.key])} onToggle={(enabled) => handleObligationChange(toggle.key, enabled)} - disabled={disabled} + disabled={obligationsDisabled} /> ))} diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/page.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/page.tsx index a32fa3b991..f606c1d6fa 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/page.tsx @@ -3,9 +3,15 @@ import type { Metadata } from 'next'; import Link from 'next/link'; import { notFound } from 'next/navigation'; import { BUILT_IN_ROLE_OBLIGATIONS } from '@trycompai/auth'; +import { serverApi } from '@/lib/api-server'; import { SYSTEM_ROLES, SYSTEM_ROLE_PERMISSIONS } from '../../constants/system-roles'; import { SystemRoleDetail } from './system-role-detail'; +// Customers can opt admins/owners in or out of the Employee Compliance +// obligation; the other built-in roles keep their obligation locked because +// the request only covered admin + owner. +const EDITABLE_OBLIGATION_ROLES = new Set(['owner', 'admin']); + export default async function SystemRolePage({ params, }: { @@ -20,6 +26,16 @@ export default async function SystemRolePage({ notFound(); } + // Resolve the effective obligations for this org — falls back to the + // hardcoded default if the API call fails or no override is set. + const overrideRes = await serverApi.get<{ + name: string; + obligations: Record; + }>(`/v1/roles/built-in/${encodeURIComponent(roleName)}/obligations`); + const effectiveObligations = + overrideRes.data?.obligations ?? + ((BUILT_IN_ROLE_OBLIGATIONS[roleName] || {}) as Record); + return ( - } description={role.description} /> + ); } diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/system-role-detail.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/system-role-detail.tsx index 7f146c9e69..dcbf782107 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/system-role-detail.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/system/[roleName]/system-role-detail.tsx @@ -1,16 +1,54 @@ 'use client'; +import { useEffect, useState } from 'react'; import { Card, CardContent } from '@trycompai/ui/card'; import { Stack, Text } from '@trycompai/design-system'; +import { toast } from 'sonner'; +import { apiClient } from '@/lib/api-client'; import { PermissionMatrix } from '../../components/PermissionMatrix'; interface SystemRoleDetailProps { + roleName: string; permissions: Record; obligations: Record; description: string; + /** When false, the obligation toggle stays locked along with the rest of the matrix. */ + obligationsEditable?: boolean; } -export function SystemRoleDetail({ permissions, obligations, description }: SystemRoleDetailProps) { +export function SystemRoleDetail({ roleName, permissions, obligations, description, obligationsEditable = false }: SystemRoleDetailProps) { + const [currentObligations, setCurrentObligations] = + useState>(obligations); + const [isSaving, setIsSaving] = useState(false); + + // Resync when the parent passes new props — either because the user + // navigated to a different built-in role, or because the page re-fetched + // the same role's obligations. + useEffect(() => { + setCurrentObligations(obligations); + }, [roleName, obligations]); + + const handleObligationsChange = async (next: Record) => { + if (isSaving || !obligationsEditable) return; + const previous = currentObligations; + setCurrentObligations(next); + setIsSaving(true); + try { + const res = await apiClient.patch( + `/v1/roles/built-in/${encodeURIComponent(roleName)}/obligations`, + { obligations: next }, + ); + if (res.error) throw new Error(res.error); + } catch (err) { + setCurrentObligations(previous); + toast.error(err instanceof Error ? err.message : 'Failed to update obligation'); + return; + } finally { + setIsSaving(false); + } + toast.success('Obligation updated'); + }; + return ( @@ -21,9 +59,10 @@ export function SystemRoleDetail({ permissions, obligations, description }: Syst {}} - obligations={obligations} - onObligationsChange={() => {}} + obligations={currentObligations} + onObligationsChange={handleObligationsChange} disabled + obligationsEditable={obligationsEditable} /> diff --git a/apps/app/src/hooks/use-permissions.ts b/apps/app/src/hooks/use-permissions.ts index 95feb67493..254f0feef1 100644 --- a/apps/app/src/hooks/use-permissions.ts +++ b/apps/app/src/hooks/use-permissions.ts @@ -24,10 +24,11 @@ export function usePermissions() { const roleString = activeMember?.role ?? null; // Resolve built-in roles synchronously - const { permissions: builtInPerms, customRoleNames } = - resolveBuiltInPermissions(roleString); + const { permissions: builtInPerms } = resolveBuiltInPermissions(roleString); - // Resolve built-in obligations synchronously + // Resolve built-in obligations synchronously — used as the initial value + // until the server returns the effective obligations (which include any + // per-organization overrides on the built-in roles). const roleNames = parseRolesString(roleString); const builtInObligations: Record = {}; for (const name of roleNames) { @@ -41,14 +42,13 @@ export function usePermissions() { } } - // Fetch custom role permissions (and obligations) if needed (SWR-cached) + // Query the resolver for all role names — custom roles contribute + // permissions, and built-in roles may carry a per-org obligation override. const { data: customData } = useSWR( - customRoleNames.length > 0 - ? ['/v1/roles/permissions', ...customRoleNames] - : null, + roleNames.length > 0 ? ['/v1/roles/permissions', ...roleNames] : null, async () => { const res = await apiClient.get( - `/v1/roles/permissions?roles=${customRoleNames.join(',')}`, + `/v1/roles/permissions?roles=${roleNames.join(',')}`, ); return { permissions: res.data?.permissions ?? {}, @@ -68,13 +68,15 @@ export function usePermissions() { mergePermissions(permissions, customData.permissions); } - // Merge built-in + custom obligations - const obligations: Record = { ...builtInObligations }; - if (customData?.obligations) { - for (const [key, val] of Object.entries(customData.obligations)) { - if (val) obligations[key] = true; - } - } + // Once the server response arrives it represents the effective obligations + // (defaults merged with any overrides). Before it arrives, fall back to the + // synchronous built-in defaults so initial render is correct for the common + // case (no overrides). + const obligations: Record = customData?.obligations + ? Object.fromEntries( + Object.entries(customData.obligations).filter(([, val]) => val), + ) + : { ...builtInObligations }; // CS-189: separate "did a custom role grant this permission" from the // merged permissions, so the Auditor View visibility check can distinguish diff --git a/apps/app/src/lib/compliance.ts b/apps/app/src/lib/compliance.ts index c047b0882b..d916451fb8 100644 --- a/apps/app/src/lib/compliance.ts +++ b/apps/app/src/lib/compliance.ts @@ -82,24 +82,25 @@ export async function filterComplianceMembers( if (members.length === 0) return []; const builtInRoleNames = new Set(Object.keys(BUILT_IN_ROLE_OBLIGATIONS)); - const allCustomRoleNames = new Set(); + const allRoleNames = new Set(); const memberRoles = members.map((member) => { const roleNames = parseRolesString(member.role); - const customNames = roleNames.filter((n) => !builtInRoleNames.has(n)); - for (const name of customNames) allCustomRoleNames.add(name); + for (const name of roleNames) allRoleNames.add(name); return { member, roleNames }; }); - // Single DB query for custom role obligations - let customObligationMap: Record> = {}; - if (allCustomRoleNames.size > 0) { - const customRoles = await db.organizationRole.findMany({ - where: { organizationId, name: { in: [...allCustomRoleNames] } }, + // Single DB query for role obligations (custom + built-in overrides). + // A row in organization_role named after a built-in role overrides the + // hardcoded default for that organization. + let obligationMap: Record> = {}; + if (allRoleNames.size > 0) { + const dbRoles = await db.organizationRole.findMany({ + where: { organizationId, name: { in: [...allRoleNames] } }, select: { name: true, obligations: true }, }); - customObligationMap = Object.fromEntries( - customRoles.map((r) => { + obligationMap = Object.fromEntries( + dbRoles.map((r) => { const obligations = typeof r.obligations === 'string' ? JSON.parse(r.obligations) : (r.obligations || {}); @@ -113,10 +114,17 @@ export async function filterComplianceMembers( // Platform admins are excluded — they join customer orgs to debug if (member.user?.role === 'admin') return false; for (const name of roleNames) { - const builtIn = BUILT_IN_ROLE_OBLIGATIONS[name]; - if (builtIn?.compliance) return true; - const custom = customObligationMap[name]; - if (custom?.compliance) return true; + // DB override wins, but only if `compliance` is explicitly set — + // otherwise fall back to the hardcoded built-in default. + const override = obligationMap[name]; + if (override && 'compliance' in override) { + if (override.compliance) return true; + continue; + } + if (builtInRoleNames.has(name)) { + const builtIn = BUILT_IN_ROLE_OBLIGATIONS[name]; + if (builtIn?.compliance) return true; + } } return false; }) diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts index f9a9026e04..17838c03fc 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts @@ -132,4 +132,44 @@ describe('filterDigestMembersByCompliance', () => { ); expect(result.map((m) => m.id)).toEqual(['m4']); }); + + it('respects per-org override that turns OFF compliance on a built-in role', async () => { + // employee defaults to compliance:true — an organization can opt out by + // storing an obligation override row for the built-in role name. + mockDb.organizationRole.findMany.mockResolvedValueOnce([ + { name: 'employee', obligations: { compliance: false } }, + ]); + const member: DigestMember = { + id: 'm6', role: 'employee', department: 'it', + user: { id: 'u6', name: 'F', email: 'f@x', role: null }, + }; + const result = await filterDigestMembersByCompliance(mockDb, [member], 'org_1'); + expect(result).toEqual([]); + }); + + it('respects per-org override that turns ON compliance on a built-in role', async () => { + // admin defaults to no compliance — an org can opt in via override. + mockDb.organizationRole.findMany.mockResolvedValueOnce([ + { name: 'admin', obligations: { compliance: true } }, + ]); + const member: DigestMember = { + id: 'm7', role: 'admin', department: 'it', + user: { id: 'u7', name: 'G', email: 'g@x', role: null }, + }; + const result = await filterDigestMembersByCompliance(mockDb, [member], 'org_1'); + expect(result).toEqual([member]); + }); + + it('falls back to built-in default when override row has no compliance key', async () => { + // A row with `{}` should NOT silently disable the owner default. + mockDb.organizationRole.findMany.mockResolvedValueOnce([ + { name: 'owner', obligations: {} }, + ]); + const member: DigestMember = { + id: 'm8', role: 'owner', department: 'it', + user: { id: 'u8', name: 'H', email: 'h@x', role: null }, + }; + const result = await filterDigestMembersByCompliance(mockDb, [member], 'org_1'); + expect(result).toEqual([member]); + }); }); 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 f35f81604c..bc738efc26 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 @@ -8,7 +8,7 @@ import type { Departments, PolicyVisibility } from '@db'; // Keep in sync with packages/auth/src/permissions.ts BUILT_IN_ROLE_OBLIGATIONS. const BUILT_IN_ROLE_OBLIGATIONS: Record = { owner: { compliance: true }, - admin: { compliance: true }, + admin: {}, auditor: {}, employee: { compliance: true }, contractor: { compliance: true }, @@ -37,26 +37,26 @@ export async function filterDigestMembersByCompliance( ): Promise { if (members.length === 0) return []; - const customRoleNames = new Set(); + const allRoleNames = new Set(); const parsed = members.map((member) => { const roleNames = member.role .split(',') .map((r) => r.trim()) .filter(Boolean); - for (const name of roleNames) { - if (!BUILT_IN_ROLE_NAMES.has(name)) customRoleNames.add(name); - } + for (const name of roleNames) allRoleNames.add(name); return { member, roleNames }; }); - let customObligationMap: Record = {}; - if (customRoleNames.size > 0) { - const customRoles = await db.organizationRole.findMany({ - where: { organizationId, name: { in: [...customRoleNames] } }, + // DB query covers both custom roles AND obligation overrides on built-in + // roles (rows in organization_role named after a built-in role). + let obligationMap: Record = {}; + if (allRoleNames.size > 0) { + const dbRoles = await db.organizationRole.findMany({ + where: { organizationId, name: { in: [...allRoleNames] } }, select: { name: true, obligations: true }, }); - customObligationMap = Object.fromEntries( - customRoles.map((r) => { + obligationMap = Object.fromEntries( + dbRoles.map((r) => { const obligations = typeof r.obligations === 'string' ? JSON.parse(r.obligations) @@ -71,10 +71,17 @@ export async function filterDigestMembersByCompliance( // Platform admins are excluded — matches the @/lib/compliance behavior. if (member.user?.role === 'admin') return false; for (const name of roleNames) { - const builtIn = BUILT_IN_ROLE_OBLIGATIONS[name]; - if (builtIn?.compliance) return true; - const custom = customObligationMap[name]; - if (custom?.compliance) return true; + // DB override wins, but only if `compliance` is explicitly set — + // otherwise fall back to the hardcoded built-in default. + const override = obligationMap[name]; + if (override && 'compliance' in override) { + if (override.compliance) return true; + continue; + } + if (BUILT_IN_ROLE_NAMES.has(name)) { + const builtIn = BUILT_IN_ROLE_OBLIGATIONS[name]; + if (builtIn?.compliance) return true; + } } return false; }) diff --git a/apps/portal/src/utils/portal-access.ts b/apps/portal/src/utils/portal-access.ts index 14016f0de7..1ceab0b028 100644 --- a/apps/portal/src/utils/portal-access.ts +++ b/apps/portal/src/utils/portal-access.ts @@ -16,30 +16,43 @@ export async function hasPortalAccess({ const roles = roleString.split(',').map((r) => r.trim()).filter(Boolean); const permissions: Record = {}; let hasComplianceObligation = false; - const customRoleNames: string[] = []; + // Built-in role permissions come from the hardcoded map. Obligations can be + // overridden per-org by a row in organization_role with the built-in name. for (const role of roles) { const builtInPerms = BUILT_IN_ROLE_PERMISSIONS[role]; - if (builtInPerms) { - mergePermissions(permissions, builtInPerms); - if (BUILT_IN_ROLE_OBLIGATIONS[role]?.compliance) { - hasComplianceObligation = true; - } - } else { - customRoleNames.push(role); - } + if (builtInPerms) mergePermissions(permissions, builtInPerms); } - if (customRoleNames.length > 0) { - const customRoles = await db.organizationRole.findMany({ - where: { organizationId, name: { in: customRoleNames } }, - select: { permissions: true, obligations: true }, + if (roles.length > 0) { + const dbRoles = await db.organizationRole.findMany({ + where: { organizationId, name: { in: roles } }, + select: { name: true, permissions: true, obligations: true }, }); + type DbRoleRow = (typeof dbRoles)[number]; + const overrideByName = new Map( + dbRoles.map((r: DbRoleRow) => [r.name, r] as const), + ); - for (const role of customRoles) { - const perms = parseRolePermissions(role.permissions); - if (perms) mergePermissions(permissions, perms); - if (parseRoleObligations(role.obligations).compliance) { + for (const role of roles) { + const dbRow = overrideByName.get(role); + const isBuiltIn = Boolean(BUILT_IN_ROLE_PERMISSIONS[role]); + + // For custom roles, the DB row is the source of permissions. + // For built-in roles, we only consult it for the obligations override. + if (dbRow && !isBuiltIn) { + const perms = parseRolePermissions(dbRow.permissions); + if (perms) mergePermissions(permissions, perms); + } + + // Obligations: use the override only when `compliance` is explicitly + // set in it; otherwise fall back to the built-in default. + const overrideObligations = dbRow ? parseRoleObligations(dbRow.obligations) : null; + if (overrideObligations && 'compliance' in overrideObligations) { + if (overrideObligations.compliance) hasComplianceObligation = true; + continue; + } + if (isBuiltIn && BUILT_IN_ROLE_OBLIGATIONS[role]?.compliance) { hasComplianceObligation = true; } }