From fa5277803772fd4011a4109cca9067c7921482bb Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 17:40:01 -0400 Subject: [PATCH 1/4] fix(rbac): gate Auditor View tab on audit:read instead of role string (CS-189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Auditor View tab was gated on roles.includes(Role.auditor) — a literal string match against the built-in "auditor" role. That made the tab invisible to any custom organization role (e.g. a "Comp AI" custom role a customer set up for our support team) even when that role was granted audit:read, and also hid it from owners/admins who implicitly have the permission. The route layout already enforces audit:read via requireRoutePermission, so the literal-role gate was both redundant and wrong. Switched to canAccessRoute(permissions, 'auditor') in: - AppSidebar nav item (design-system sidebar) - MainMenu item (legacy sidebar + mobile menu) — reads permissions via usePermissions() so callers no longer need to pass hasAuditorRole - app-shell-search-groups command palette entry Also removed the redundant roles.includes(Role.auditor) page-level check in auditor/(overview)/page.tsx; auditor/layout.tsx's existing requireRoutePermission('auditor', orgId) still enforces audit:read server-side. Aligns with CLAUDE.md RBAC guidance: "No manual role string parsing — always use permission checks." Co-Authored-By: Claude Opus 4.7 (1M context) --- .../(app)/[orgId]/auditor/(overview)/page.tsx | 12 ++++------ .../(app)/[orgId]/components/AppSidebar.tsx | 7 +++++- .../components/app-shell-search-groups.tsx | 3 ++- apps/app/src/components/main-menu.tsx | 12 +++++++--- apps/app/src/components/sidebar.tsx | 1 - apps/app/src/lib/permissions.test.ts | 24 +++++++++++++++++++ 6 files changed, 46 insertions(+), 13 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/auditor/(overview)/page.tsx b/apps/app/src/app/(app)/[orgId]/auditor/(overview)/page.tsx index 892a0b72e4..4803d19419 100644 --- a/apps/app/src/app/(app)/[orgId]/auditor/(overview)/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/auditor/(overview)/page.tsx @@ -1,9 +1,7 @@ import PageWithBreadcrumb from '@/components/pages/PageWithBreadcrumb'; import { serverApi } from '@/lib/api-server'; -import { parseRolesString } from '@/lib/permissions'; -import { Role } from '@db'; import type { Metadata } from 'next'; -import { notFound, redirect } from 'next/navigation'; +import { redirect } from 'next/navigation'; import { AuditorView } from './components/AuditorView'; interface PeopleMember { @@ -74,10 +72,10 @@ export default async function AuditorPage({ redirect('/auth/unauthorized'); } - const roles = parseRolesString(currentMember.role); - if (!roles.includes(Role.auditor)) { - notFound(); - } + // CS-189: auditor/layout.tsx already calls requireRoutePermission('auditor', + // orgId) which enforces audit:read. The prior literal-role check + // (roles.includes(Role.auditor)) was redundant AND wrong — it would 404 for + // owners/admins/custom roles that legitimately have audit:read. const organizationName = orgRes.data?.name ?? 'Organization'; const logoUrl = orgRes.data?.logoUrl ?? null; diff --git a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx index 830ca29e41..d09fadc940 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx @@ -47,7 +47,12 @@ export function AppSidebar({ id: 'auditor', path: `/${organization.id}/auditor`, name: 'Auditor View', - hidden: !hasAuditorRole || !canAccessRoute(permissions, 'auditor'), + // CS-189: gate on permission only — the old role-string check + // (`!hasAuditorRole`) hid the tab from owners/admins and from any + // custom org role (e.g. a "Comp AI" role) even when they had + // audit:read. Route protection at apps/app/.../auditor/layout.tsx + // still enforces audit:read server-side. + hidden: !canAccessRoute(permissions, 'auditor'), }, { id: 'policies', diff --git a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx index 11e4359961..bee19b5ac0 100644 --- a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx @@ -96,7 +96,8 @@ export const getAppShellSearchGroups = ({ }), ] : []), - ...(hasAuditorRole && can('auditor') + // CS-189: permission-only gate — see AppSidebar for rationale. + ...(can('auditor') ? [ createNavItem({ id: 'auditor', diff --git a/apps/app/src/components/main-menu.tsx b/apps/app/src/components/main-menu.tsx index b02c2ec677..e534ba1306 100644 --- a/apps/app/src/components/main-menu.tsx +++ b/apps/app/src/components/main-menu.tsx @@ -1,5 +1,7 @@ 'use client'; +import { canAccessRoute } from '@/lib/permissions'; +import { usePermissions } from '@/hooks/use-permissions'; import { Badge } from '@trycompai/ui/badge'; import { Button } from '@trycompai/ui/button'; import { cn } from '@trycompai/ui/cn'; @@ -54,7 +56,6 @@ export type Props = { onItemClick?: () => void; isQuestionnaireEnabled?: boolean; isTrustNdaEnabled?: boolean; - hasAuditorRole?: boolean; isOnlyAuditor?: boolean; }; @@ -65,12 +66,17 @@ export function MainMenu({ onItemClick, isQuestionnaireEnabled = false, isTrustNdaEnabled = false, - hasAuditorRole = false, isOnlyAuditor = false, }: Props) { const pathname = usePathname(); const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' }); const itemRefs = useRef<(HTMLDivElement | null)[]>([]); + // CS-189: gate the Auditor View tab on audit:read permission rather than + // a literal role-string match. That way any custom org role (e.g. "Comp AI") + // granted the permission — and owners/admins who implicitly have it — can + // access the tab, not just the built-in "auditor" role. + const { permissions } = usePermissions(); + const canAccessAuditor = canAccessRoute(permissions, 'auditor'); const items: MenuItem[] = [ { @@ -96,7 +102,7 @@ export function MainMenu({ disabled: false, icon: ClipboardCheck, protected: false, - hidden: !hasAuditorRole, + hidden: !canAccessAuditor, }, { id: 'controls', diff --git a/apps/app/src/components/sidebar.tsx b/apps/app/src/components/sidebar.tsx index 88b29f7d91..494760d7eb 100644 --- a/apps/app/src/components/sidebar.tsx +++ b/apps/app/src/components/sidebar.tsx @@ -99,7 +99,6 @@ export async function Sidebar({ isCollapsed={isCollapsed} isQuestionnaireEnabled={isQuestionnaireEnabled} isTrustNdaEnabled={isTrustNdaEnabled} - hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} /> diff --git a/apps/app/src/lib/permissions.test.ts b/apps/app/src/lib/permissions.test.ts index 17f6210eae..cc005e8e25 100644 --- a/apps/app/src/lib/permissions.test.ts +++ b/apps/app/src/lib/permissions.test.ts @@ -62,6 +62,30 @@ describe('canAccessRoute', () => { const permissions: UserPermissions = {}; expect(canAccessRoute(permissions, 'nonexistent-route')).toBe(true); }); + + // CS-189: Auditor View tab visibility is now gated solely on audit:read. + // The tab must be visible for ANY role that has audit:read (owner, admin, + // auditor, or a custom org role like "Comp AI" granted the permission) — + // not just the built-in 'auditor' role string. + describe('auditor route (CS-189)', () => { + it('grants access to any role with audit:read', () => { + expect(canAccessRoute({ audit: ['read'] }, 'auditor')).toBe(true); + }); + + it('grants access when audit:read is among multiple permissions (custom role)', () => { + const customCompAi: UserPermissions = { + audit: ['read'], + evidence: ['read', 'update'], + policy: ['read'], + }; + expect(canAccessRoute(customCompAi, 'auditor')).toBe(true); + }); + + it('denies access when audit:read is missing', () => { + expect(canAccessRoute({ evidence: ['read'] }, 'auditor')).toBe(false); + expect(canAccessRoute({}, 'auditor')).toBe(false); + }); + }); }); describe('getDefaultRoute', () => { From d7c39362178d5a18c8d7d8ed342b5387773a1b32 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 16 Apr 2026 18:50:52 -0400 Subject: [PATCH 2/4] fix(rbac): scope Auditor View to explicit auditor roles (CS-189 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up after CTO review: owner/admin should NOT see the Auditor View tab or access the page, even though their implicit all-permissions include audit:read. The tab is scoped to audit work; it should only appear for users whose role explicitly grants audit access. New rule: Show Auditor View iff - user has the built-in `auditor` role, OR - user has a custom org role that explicitly grants `audit:read`. Implementation: - New pure helper `canAccessAuditorView(roleString, customRolePermissions)` in lib/permissions.ts. Separates "permission came from built-in role" (owner/admin/everyone) from "permission came from custom role". - Server-side helpers in lib/permissions.server.ts: - `resolveCustomRolePermissions` fetches only the user's custom-role permissions (built-in roles excluded). - `resolveAuditorViewAccess` runs the check for the current user. - `requireAuditorViewAccess` is the route guard — replaces `requireRoutePermission('auditor', orgId)` on the auditor layout. - usePermissions() now returns `canAccessAuditorView` + `customPermissions` so client components don't have to replicate the rule. - Org layout computes the flag server-side and forwards it through AppShellWrapper → AppSidebar + getAppShellSearchGroups. - main-menu reads the flag directly from usePermissions(). Behavior matrix: - auditor → visible ✅ - owner,auditor / admin,auditor → visible ✅ - CompAI custom role with audit:read → visible ✅ - owner alone / admin alone → hidden (implicit audit:read doesn't count) - owner,ReadOnlyCustomRole (no audit) → hidden - employee / contractor → hidden ✅ Adds 9 unit tests locking in every row of that matrix. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/app/(app)/[orgId]/auditor/layout.tsx | 8 +- .../[orgId]/components/AppShellWrapper.tsx | 5 ++ .../(app)/[orgId]/components/AppSidebar.tsx | 19 +++-- .../components/app-shell-search-groups.tsx | 8 +- apps/app/src/app/(app)/[orgId]/layout.tsx | 25 +++++- apps/app/src/components/main-menu.tsx | 14 ++-- apps/app/src/hooks/use-permissions.ts | 8 ++ apps/app/src/lib/permissions.server.ts | 80 ++++++++++++++++++ apps/app/src/lib/permissions.test.ts | 82 +++++++++++++------ apps/app/src/lib/permissions.ts | 25 ++++++ 10 files changed, 231 insertions(+), 43 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx index 842432ee70..76cc7dd1b9 100644 --- a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx @@ -1,4 +1,4 @@ -import { requireRoutePermission } from '@/lib/permissions.server'; +import { requireAuditorViewAccess } from '@/lib/permissions.server'; export default async function Layout({ children, @@ -8,6 +8,10 @@ export default async function Layout({ params: Promise<{ orgId: string }>; }) { const { orgId } = await params; - await requireRoutePermission('auditor', orgId); + // CS-189: stricter than `requireRoutePermission('auditor', orgId)` — the + // plain check let owner/admin through via their implicit audit:read. + // requireAuditorViewAccess enforces "built-in auditor OR custom role with + // explicit audit:read" to match the sidebar tab visibility. + await requireAuditorViewAccess(orgId); return <>{children}; } diff --git a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx index c336b4bc88..e6a234a9b5 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx @@ -68,6 +68,8 @@ interface AppShellWrapperProps { isSecurityEnabled: boolean; hasAuditorRole: boolean; isOnlyAuditor: boolean; + /** CS-189: server-resolved Auditor View visibility — see resolveAuditorViewAccess. */ + canAccessAuditorView: boolean; permissions: UserPermissions; user: { name: string | null; @@ -99,6 +101,7 @@ function AppShellWrapperContent({ isSecurityEnabled, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, permissions, user, isAdmin, @@ -144,6 +147,7 @@ function AppShellWrapperContent({ permissions, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -319,6 +323,7 @@ function AppShellWrapperContent({ hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} permissions={permissions} + canAccessAuditorView={canAccessAuditorView} /> )} diff --git a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx index d09fadc940..004be27c00 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx @@ -19,6 +19,13 @@ interface AppSidebarProps { hasAuditorRole: boolean; isOnlyAuditor: boolean; permissions: UserPermissions; + /** + * CS-189: Whether this user should see the Auditor View tab. Computed + * server-side (see resolveAuditorViewAccess) because it needs to + * distinguish owner/admin's implicit audit:read from a custom role's + * explicit audit:read. + */ + canAccessAuditorView: boolean; } export function AppSidebar({ @@ -27,6 +34,7 @@ export function AppSidebar({ hasAuditorRole, isOnlyAuditor, permissions, + canAccessAuditorView, }: AppSidebarProps) { const pathname = usePathname() ?? ''; @@ -47,12 +55,11 @@ export function AppSidebar({ id: 'auditor', path: `/${organization.id}/auditor`, name: 'Auditor View', - // CS-189: gate on permission only — the old role-string check - // (`!hasAuditorRole`) hid the tab from owners/admins and from any - // custom org role (e.g. a "Comp AI" role) even when they had - // audit:read. Route protection at apps/app/.../auditor/layout.tsx - // still enforces audit:read server-side. - hidden: !canAccessRoute(permissions, 'auditor'), + // CS-189: visibility is scoped to built-in `auditor` role or a custom + // org role that explicitly grants audit:read. Owner/admin's implicit + // permissions alone are not enough — see `canAccessAuditorView` in + // lib/permissions.ts for the full rule. + hidden: !canAccessAuditorView, }, { id: 'policies', diff --git a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx index bee19b5ac0..1423888be9 100644 --- a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx @@ -26,6 +26,8 @@ interface AppShellSearchGroupsParams { permissions: UserPermissions; hasAuditorRole: boolean; isOnlyAuditor: boolean; + /** CS-189: resolved server-side — see AppShellWrapper. */ + canAccessAuditorView: boolean; isQuestionnaireEnabled: boolean; isTrustNdaEnabled: boolean; isSecurityEnabled: boolean; @@ -64,6 +66,7 @@ export const getAppShellSearchGroups = ({ permissions, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -96,8 +99,9 @@ export const getAppShellSearchGroups = ({ }), ] : []), - // CS-189: permission-only gate — see AppSidebar for rationale. - ...(can('auditor') + // CS-189: gate on the server-resolved canAccessAuditorView flag so + // owner/admin are hidden unless they explicitly opt in via a custom role. + ...(canAccessAuditorView ? [ createNavItem({ id: 'auditor', diff --git a/apps/app/src/app/(app)/[orgId]/layout.tsx b/apps/app/src/app/(app)/[orgId]/layout.tsx index 869c58e6e4..24fae1573e 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.tsx @@ -2,8 +2,15 @@ import { getFeatureFlags } from '@/app/posthog'; import { APP_AWS_ORG_ASSETS_BUCKET, s3Client } from '@/app/s3'; import { TriggerTokenProvider } from '@/components/trigger-token-provider'; import { serverApi } from '@/lib/api-server'; -import { canAccessApp, parseRolesString } from '@/lib/permissions'; -import { resolveUserPermissions } from '@/lib/permissions.server'; +import { + canAccessApp, + canAccessAuditorView, + parseRolesString, +} from '@/lib/permissions'; +import { + resolveCustomRolePermissions, + resolveUserPermissions, +} from '@/lib/permissions.server'; import type { OrganizationFromMe } from '@/types'; import { auth } from '@/utils/auth'; import { GetObjectCommand } from '@aws-sdk/client-s3'; @@ -156,6 +163,19 @@ export default async function Layout({ const hasAuditorRole = roles.includes(Role.auditor); const isOnlyAuditor = hasAuditorRole && roles.length === 1; + // CS-189: the Auditor View tab follows a stricter rule than bare + // 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, + ); + // User data for navbar const user = { name: session.user.name, @@ -180,6 +200,7 @@ export default async function Layout({ isSecurityEnabled={isSecurityEnabled} hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} + canAccessAuditorView={auditorViewVisible} permissions={permissions} user={user} isAdmin={isUserAdmin} diff --git a/apps/app/src/components/main-menu.tsx b/apps/app/src/components/main-menu.tsx index e534ba1306..25117f3408 100644 --- a/apps/app/src/components/main-menu.tsx +++ b/apps/app/src/components/main-menu.tsx @@ -1,6 +1,5 @@ 'use client'; -import { canAccessRoute } from '@/lib/permissions'; import { usePermissions } from '@/hooks/use-permissions'; import { Badge } from '@trycompai/ui/badge'; import { Button } from '@trycompai/ui/button'; @@ -71,12 +70,11 @@ export function MainMenu({ const pathname = usePathname(); const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' }); const itemRefs = useRef<(HTMLDivElement | null)[]>([]); - // CS-189: gate the Auditor View tab on audit:read permission rather than - // a literal role-string match. That way any custom org role (e.g. "Comp AI") - // granted the permission — and owners/admins who implicitly have it — can - // access the tab, not just the built-in "auditor" role. - const { permissions } = usePermissions(); - const canAccessAuditor = canAccessRoute(permissions, 'auditor'); + // CS-189: Auditor View visibility is scoped to the built-in `auditor` + // role or a custom org role that explicitly grants audit:read — NOT to + // owner/admin's implicit all-permissions. See `canAccessAuditorView` in + // lib/permissions.ts for the full rule. + const { canAccessAuditorView } = usePermissions(); const items: MenuItem[] = [ { @@ -102,7 +100,7 @@ export function MainMenu({ disabled: false, icon: ClipboardCheck, protected: false, - hidden: !canAccessAuditor, + hidden: !canAccessAuditorView, }, { id: 'controls', diff --git a/apps/app/src/hooks/use-permissions.ts b/apps/app/src/hooks/use-permissions.ts index 3a9110d9eb..11c0228bea 100644 --- a/apps/app/src/hooks/use-permissions.ts +++ b/apps/app/src/hooks/use-permissions.ts @@ -10,6 +10,7 @@ import { hasPermission, parseRolesString, isBuiltInRole, + canAccessAuditorView, type UserPermissions, } from '@/lib/permissions'; @@ -75,10 +76,17 @@ export function usePermissions() { } } + // CS-189: separate "did a custom role grant this permission" from the + // merged permissions, so the Auditor View visibility check can distinguish + // an owner's implicit audit:read from a custom role's explicit audit:read. + const customPermissions = customData?.permissions ?? {}; + return { permissions, + customPermissions, obligations, hasPermission: (resource: string, action: string) => hasPermission(permissions, resource, action), + canAccessAuditorView: canAccessAuditorView(roleString, customPermissions), }; } diff --git a/apps/app/src/lib/permissions.server.ts b/apps/app/src/lib/permissions.server.ts index 10beb4c323..8412341ca5 100644 --- a/apps/app/src/lib/permissions.server.ts +++ b/apps/app/src/lib/permissions.server.ts @@ -6,6 +6,7 @@ import { headers } from 'next/headers'; import { redirect } from 'next/navigation'; import { type UserPermissions, + canAccessAuditorView, canAccessRoute, getDefaultRoute, mergePermissions, @@ -92,3 +93,82 @@ export async function requireRoutePermission( redirect(defaultRoute ?? '/no-access'); } } + +/** + * CS-189: Resolve only the permissions granted by the user's CUSTOM org + * roles (i.e. not from built-in roles). Needed for the Auditor View + * visibility rule, which wants to know whether a custom role explicitly + * grants `audit:read` — owner/admin's implicit all-permissions don't count. + */ +export async function resolveCustomRolePermissions( + roleString: string | null | undefined, + orgId: string, +): Promise { + const { customRoleNames } = resolveBuiltInPermissions(roleString); + const result: UserPermissions = {}; + if (customRoleNames.length === 0) return result; + + const customRoles = await db.organizationRole.findMany({ + where: { organizationId: orgId, name: { in: customRoleNames } }, + select: { permissions: true }, + }); + + for (const role of customRoles) { + if (!role.permissions) continue; + const parsed = + typeof role.permissions === 'string' + ? JSON.parse(role.permissions) + : role.permissions; + if (parsed && typeof parsed === 'object') { + mergePermissions(result, parsed as Record); + } + } + return result; +} + +/** + * Server-side Auditor View access check. Mirrors the client-side + * `canAccessAuditorView` but pulls the custom-role permissions from the + * DB for the current user. Returns null if the user isn't in the org. + */ +export async function resolveAuditorViewAccess( + orgId: string, +): Promise<{ canAccess: boolean; roleString: string | null } | null> { + const session = await auth.api.getSession({ + headers: await headers(), + }); + if (!session?.user?.id) return null; + + const member = await db.member.findFirst({ + where: { + userId: session.user.id, + organizationId: orgId, + deactivated: false, + }, + select: { role: true }, + }); + if (!member) return null; + + const customPerms = await resolveCustomRolePermissions(member.role, orgId); + return { + canAccess: canAccessAuditorView(member.role, customPerms), + roleString: member.role, + }; +} + +/** + * Route guard for the Auditor View page. Replaces `requireRoutePermission( + * 'auditor', orgId)` — the plain permission check let owner/admin through + * via their implicit `audit:read`. This helper enforces the stricter + * "built-in auditor OR custom role with audit:read" rule. + */ +export async function requireAuditorViewAccess(orgId: string): Promise { + const result = await resolveAuditorViewAccess(orgId); + if (result?.canAccess) return; + + const permissions = await resolveCurrentUserPermissions(orgId); + const defaultRoute = permissions + ? getDefaultRoute(permissions, orgId) + : null; + redirect(defaultRoute ?? '/no-access'); +} diff --git a/apps/app/src/lib/permissions.test.ts b/apps/app/src/lib/permissions.test.ts index cc005e8e25..180db8bf85 100644 --- a/apps/app/src/lib/permissions.test.ts +++ b/apps/app/src/lib/permissions.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { canAccessApp, + canAccessAuditorView, canAccessCompliance, canAccessRoute, getDefaultRoute, @@ -63,29 +64,6 @@ describe('canAccessRoute', () => { expect(canAccessRoute(permissions, 'nonexistent-route')).toBe(true); }); - // CS-189: Auditor View tab visibility is now gated solely on audit:read. - // The tab must be visible for ANY role that has audit:read (owner, admin, - // auditor, or a custom org role like "Comp AI" granted the permission) — - // not just the built-in 'auditor' role string. - describe('auditor route (CS-189)', () => { - it('grants access to any role with audit:read', () => { - expect(canAccessRoute({ audit: ['read'] }, 'auditor')).toBe(true); - }); - - it('grants access when audit:read is among multiple permissions (custom role)', () => { - const customCompAi: UserPermissions = { - audit: ['read'], - evidence: ['read', 'update'], - policy: ['read'], - }; - expect(canAccessRoute(customCompAi, 'auditor')).toBe(true); - }); - - it('denies access when audit:read is missing', () => { - expect(canAccessRoute({ evidence: ['read'] }, 'auditor')).toBe(false); - expect(canAccessRoute({}, 'auditor')).toBe(false); - }); - }); }); describe('getDefaultRoute', () => { @@ -155,3 +133,61 @@ describe('hasPermission', () => { expect(hasPermission(permissions, 'pentest', 'create')).toBe(false); }); }); + +// CS-189: Auditor View visibility is intentionally stricter than bare +// `audit:read` — owner and admin implicitly have every permission, but the +// CTO's product decision is that the tab only appears for users whose role +// explicitly scopes them to audit work (built-in auditor OR a custom role +// that specifically grants audit:read). +describe('canAccessAuditorView', () => { + const noCustom: UserPermissions = {}; + + it('shows for the built-in auditor role', () => { + expect(canAccessAuditorView('auditor', noCustom)).toBe(true); + }); + + it('shows when auditor is one of several roles (e.g. owner,auditor)', () => { + expect(canAccessAuditorView('owner,auditor', noCustom)).toBe(true); + expect(canAccessAuditorView('admin, auditor', noCustom)).toBe(true); + }); + + it('hides for owner alone (implicit audit:read does NOT count)', () => { + expect(canAccessAuditorView('owner', noCustom)).toBe(false); + }); + + it('hides for admin alone (implicit audit:read does NOT count)', () => { + expect(canAccessAuditorView('admin', noCustom)).toBe(false); + }); + + it('hides for employee / contractor', () => { + expect(canAccessAuditorView('employee', noCustom)).toBe(false); + expect(canAccessAuditorView('contractor', noCustom)).toBe(false); + }); + + it('shows when a custom role explicitly grants audit:read', () => { + const customRolePerms: UserPermissions = { audit: ['read'] }; + expect(canAccessAuditorView('CompAI', customRolePerms)).toBe(true); + }); + + it('shows when owner is combined with a custom role that grants audit:read', () => { + const customRolePerms: UserPermissions = { audit: ['read'] }; + expect(canAccessAuditorView('owner,CompAI', customRolePerms)).toBe(true); + }); + + it('hides when owner has a custom role that does NOT grant audit:read', () => { + // Owner's implicit audit:read is what `permissions` would carry, but + // `canAccessAuditorView` only looks at custom-role permissions — so if + // the custom role is something like "ReadOnlyViewer" without audit, the + // tab stays hidden even though the merged permissions would pass. + const customRolePerms: UserPermissions = { evidence: ['read'] }; + expect(canAccessAuditorView('owner,ReadOnlyViewer', customRolePerms)).toBe( + false, + ); + }); + + it('hides when role string is empty / null / undefined', () => { + expect(canAccessAuditorView('', noCustom)).toBe(false); + expect(canAccessAuditorView(null, noCustom)).toBe(false); + expect(canAccessAuditorView(undefined, noCustom)).toBe(false); + }); +}); diff --git a/apps/app/src/lib/permissions.ts b/apps/app/src/lib/permissions.ts index 348368e196..79c96e4db3 100644 --- a/apps/app/src/lib/permissions.ts +++ b/apps/app/src/lib/permissions.ts @@ -96,6 +96,31 @@ export function canAccessRoute(permissions: UserPermissions, routeSegment: strin return hasAnyPermission(permissions, required); } +/** + * CS-189: Auditor View visibility rule (product decision). + * + * "Auditor View" is scoped to audit work — it should only appear for users + * whose role explicitly grants audit access, not for owners/admins whose + * implicit all-permissions include `audit:read`. + * + * Show the tab iff: + * - user has the built-in `auditor` role, OR + * - user has a custom org role that explicitly grants `audit:read`. + * + * Owners/admins who want this tab can opt in by adding the auditor role + * to their membership or by creating a custom role that includes + * `audit:read`. Multi-role users are handled because `auditor` can be + * one of several roles on a membership. + */ +export function canAccessAuditorView( + roleString: string | null | undefined, + customRolePermissions: UserPermissions, +): boolean { + const roles = parseRolesString(roleString); + if (roles.includes('auditor')) return true; + return hasPermission(customRolePermissions, 'audit', 'read'); +} + /** * Ordered list of main navigation routes used to find a user's default landing page. * Order matches sidebar priority — the first accessible route becomes the default. From c4a4da5a5cfff0e8e6e102589b78c695fd1ca5c8 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 20 Apr 2026 14:05:54 -0400 Subject: [PATCH 3/4] refactor(rbac): drop audit:read from owner/admin, simplify Auditor View gating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per CS-189 product decision: owners/admins should not see the Auditor View tab unless they explicitly opt in via a custom role granting audit:read. Removing audit:read from the built-in owner/admin roles lets the standard canAccessRoute('auditor') check do the work — the merged permissions naturally cover owner,auditor / admin,auditor and custom roles with audit:read, while hiding owner/admin alone. audit:read is not referenced anywhere in the codebase outside ROUTE_PERMISSIONS.auditor, so dropping it is safe. Owner/admin still retain audit:create and audit:update for SOA management. Deletes ~100 lines of special-case code that was previously needed to distinguish implicit vs explicit audit:read: - canAccessAuditorView - resolveCustomRolePermissions - resolveAuditorViewAccess - requireAuditorViewAccess - customPermissions plumbing through AppShellWrapper / AppSidebar / search groups / MainMenu / usePermissions Tests updated to exercise the full resolution path (resolveBuiltInPermissions + canAccessRoute). --- .../training/permissions-regression.spec.ts | 9 ++- .../src/app/(app)/[orgId]/auditor/layout.tsx | 8 +- .../[orgId]/components/AppShellWrapper.tsx | 5 -- .../(app)/[orgId]/components/AppSidebar.tsx | 14 +--- .../components/app-shell-search-groups.tsx | 7 +- apps/app/src/app/(app)/[orgId]/layout.tsx | 25 +----- apps/app/src/components/main-menu.tsx | 9 +-- apps/app/src/hooks/use-permissions.ts | 8 -- apps/app/src/lib/permissions.server.ts | 80 ------------------- apps/app/src/lib/permissions.test.ts | 76 ++++++++++-------- apps/app/src/lib/permissions.ts | 25 ------ packages/auth/src/permissions.ts | 10 ++- 12 files changed, 63 insertions(+), 213 deletions(-) diff --git a/apps/api/src/training/permissions-regression.spec.ts b/apps/api/src/training/permissions-regression.spec.ts index 1ec83db5f2..7429cea309 100644 --- a/apps/api/src/training/permissions-regression.spec.ts +++ b/apps/api/src/training/permissions-regression.spec.ts @@ -74,10 +74,11 @@ describe('Built-in role permissions — regression', () => { } }); - it('should have audit create/read/update (no delete)', () => { - expect(perms.audit).toEqual( - expect.arrayContaining(['create', 'read', 'update']), - ); + it('should have audit create/update only (no read, no delete)', () => { + // CS-189: audit:read is reserved for auditor-facing views; owner can + // still manage audit scope via create/update. + expect(perms.audit).toEqual(expect.arrayContaining(['create', 'update'])); + expect(perms.audit).not.toContain('read'); expect(perms.audit).not.toContain('delete'); }); diff --git a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx index 76cc7dd1b9..842432ee70 100644 --- a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx @@ -1,4 +1,4 @@ -import { requireAuditorViewAccess } from '@/lib/permissions.server'; +import { requireRoutePermission } from '@/lib/permissions.server'; export default async function Layout({ children, @@ -8,10 +8,6 @@ export default async function Layout({ params: Promise<{ orgId: string }>; }) { const { orgId } = await params; - // CS-189: stricter than `requireRoutePermission('auditor', orgId)` — the - // plain check let owner/admin through via their implicit audit:read. - // requireAuditorViewAccess enforces "built-in auditor OR custom role with - // explicit audit:read" to match the sidebar tab visibility. - await requireAuditorViewAccess(orgId); + await requireRoutePermission('auditor', orgId); return <>{children}; } diff --git a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx index e6a234a9b5..c336b4bc88 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx @@ -68,8 +68,6 @@ interface AppShellWrapperProps { isSecurityEnabled: boolean; hasAuditorRole: boolean; isOnlyAuditor: boolean; - /** CS-189: server-resolved Auditor View visibility — see resolveAuditorViewAccess. */ - canAccessAuditorView: boolean; permissions: UserPermissions; user: { name: string | null; @@ -101,7 +99,6 @@ function AppShellWrapperContent({ isSecurityEnabled, hasAuditorRole, isOnlyAuditor, - canAccessAuditorView, permissions, user, isAdmin, @@ -147,7 +144,6 @@ function AppShellWrapperContent({ permissions, hasAuditorRole, isOnlyAuditor, - canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -323,7 +319,6 @@ function AppShellWrapperContent({ hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} permissions={permissions} - canAccessAuditorView={canAccessAuditorView} /> )} diff --git a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx index 004be27c00..967e3bc129 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx @@ -19,13 +19,6 @@ interface AppSidebarProps { hasAuditorRole: boolean; isOnlyAuditor: boolean; permissions: UserPermissions; - /** - * CS-189: Whether this user should see the Auditor View tab. Computed - * server-side (see resolveAuditorViewAccess) because it needs to - * distinguish owner/admin's implicit audit:read from a custom role's - * explicit audit:read. - */ - canAccessAuditorView: boolean; } export function AppSidebar({ @@ -34,7 +27,6 @@ export function AppSidebar({ hasAuditorRole, isOnlyAuditor, permissions, - canAccessAuditorView, }: AppSidebarProps) { const pathname = usePathname() ?? ''; @@ -55,11 +47,7 @@ export function AppSidebar({ id: 'auditor', path: `/${organization.id}/auditor`, name: 'Auditor View', - // CS-189: visibility is scoped to built-in `auditor` role or a custom - // org role that explicitly grants audit:read. Owner/admin's implicit - // permissions alone are not enough — see `canAccessAuditorView` in - // lib/permissions.ts for the full rule. - hidden: !canAccessAuditorView, + hidden: !canAccessRoute(permissions, 'auditor'), }, { id: 'policies', diff --git a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx index 1423888be9..5625e05e9e 100644 --- a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx @@ -26,8 +26,6 @@ interface AppShellSearchGroupsParams { permissions: UserPermissions; hasAuditorRole: boolean; isOnlyAuditor: boolean; - /** CS-189: resolved server-side — see AppShellWrapper. */ - canAccessAuditorView: boolean; isQuestionnaireEnabled: boolean; isTrustNdaEnabled: boolean; isSecurityEnabled: boolean; @@ -66,7 +64,6 @@ export const getAppShellSearchGroups = ({ permissions, hasAuditorRole, isOnlyAuditor, - canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -99,9 +96,7 @@ export const getAppShellSearchGroups = ({ }), ] : []), - // CS-189: gate on the server-resolved canAccessAuditorView flag so - // owner/admin are hidden unless they explicitly opt in via a custom role. - ...(canAccessAuditorView + ...(can('auditor') ? [ createNavItem({ id: 'auditor', diff --git a/apps/app/src/app/(app)/[orgId]/layout.tsx b/apps/app/src/app/(app)/[orgId]/layout.tsx index 24fae1573e..869c58e6e4 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.tsx @@ -2,15 +2,8 @@ import { getFeatureFlags } from '@/app/posthog'; import { APP_AWS_ORG_ASSETS_BUCKET, s3Client } from '@/app/s3'; 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, parseRolesString } from '@/lib/permissions'; +import { resolveUserPermissions } from '@/lib/permissions.server'; import type { OrganizationFromMe } from '@/types'; import { auth } from '@/utils/auth'; import { GetObjectCommand } from '@aws-sdk/client-s3'; @@ -163,19 +156,6 @@ export default async function Layout({ const hasAuditorRole = roles.includes(Role.auditor); const isOnlyAuditor = hasAuditorRole && roles.length === 1; - // CS-189: the Auditor View tab follows a stricter rule than bare - // 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, - ); - // User data for navbar const user = { name: session.user.name, @@ -200,7 +180,6 @@ export default async function Layout({ isSecurityEnabled={isSecurityEnabled} hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} - canAccessAuditorView={auditorViewVisible} permissions={permissions} user={user} isAdmin={isUserAdmin} diff --git a/apps/app/src/components/main-menu.tsx b/apps/app/src/components/main-menu.tsx index 25117f3408..404a6e2cfe 100644 --- a/apps/app/src/components/main-menu.tsx +++ b/apps/app/src/components/main-menu.tsx @@ -1,6 +1,7 @@ 'use client'; import { usePermissions } from '@/hooks/use-permissions'; +import { canAccessRoute } from '@/lib/permissions'; import { Badge } from '@trycompai/ui/badge'; import { Button } from '@trycompai/ui/button'; import { cn } from '@trycompai/ui/cn'; @@ -70,11 +71,7 @@ export function MainMenu({ const pathname = usePathname(); const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' }); const itemRefs = useRef<(HTMLDivElement | null)[]>([]); - // CS-189: Auditor View visibility is scoped to the built-in `auditor` - // role or a custom org role that explicitly grants audit:read — NOT to - // owner/admin's implicit all-permissions. See `canAccessAuditorView` in - // lib/permissions.ts for the full rule. - const { canAccessAuditorView } = usePermissions(); + const { permissions } = usePermissions(); const items: MenuItem[] = [ { @@ -100,7 +97,7 @@ export function MainMenu({ disabled: false, icon: ClipboardCheck, protected: false, - hidden: !canAccessAuditorView, + hidden: !canAccessRoute(permissions, 'auditor'), }, { id: 'controls', diff --git a/apps/app/src/hooks/use-permissions.ts b/apps/app/src/hooks/use-permissions.ts index 95feb67493..bbbd82af6b 100644 --- a/apps/app/src/hooks/use-permissions.ts +++ b/apps/app/src/hooks/use-permissions.ts @@ -10,7 +10,6 @@ import { hasPermission, parseRolesString, isBuiltInRole, - canAccessAuditorView, type UserPermissions, } from '@/lib/permissions'; @@ -76,18 +75,11 @@ export function usePermissions() { } } - // CS-189: separate "did a custom role grant this permission" from the - // merged permissions, so the Auditor View visibility check can distinguish - // an owner's implicit audit:read from a custom role's explicit audit:read. - const customPermissions = customData?.permissions ?? {}; - return { permissions, - customPermissions, obligations, roles: roleNames, hasPermission: (resource: string, action: string) => hasPermission(permissions, resource, action), - canAccessAuditorView: canAccessAuditorView(roleString, customPermissions), }; } diff --git a/apps/app/src/lib/permissions.server.ts b/apps/app/src/lib/permissions.server.ts index 8412341ca5..10beb4c323 100644 --- a/apps/app/src/lib/permissions.server.ts +++ b/apps/app/src/lib/permissions.server.ts @@ -6,7 +6,6 @@ import { headers } from 'next/headers'; import { redirect } from 'next/navigation'; import { type UserPermissions, - canAccessAuditorView, canAccessRoute, getDefaultRoute, mergePermissions, @@ -93,82 +92,3 @@ export async function requireRoutePermission( redirect(defaultRoute ?? '/no-access'); } } - -/** - * CS-189: Resolve only the permissions granted by the user's CUSTOM org - * roles (i.e. not from built-in roles). Needed for the Auditor View - * visibility rule, which wants to know whether a custom role explicitly - * grants `audit:read` — owner/admin's implicit all-permissions don't count. - */ -export async function resolveCustomRolePermissions( - roleString: string | null | undefined, - orgId: string, -): Promise { - const { customRoleNames } = resolveBuiltInPermissions(roleString); - const result: UserPermissions = {}; - if (customRoleNames.length === 0) return result; - - const customRoles = await db.organizationRole.findMany({ - where: { organizationId: orgId, name: { in: customRoleNames } }, - select: { permissions: true }, - }); - - for (const role of customRoles) { - if (!role.permissions) continue; - const parsed = - typeof role.permissions === 'string' - ? JSON.parse(role.permissions) - : role.permissions; - if (parsed && typeof parsed === 'object') { - mergePermissions(result, parsed as Record); - } - } - return result; -} - -/** - * Server-side Auditor View access check. Mirrors the client-side - * `canAccessAuditorView` but pulls the custom-role permissions from the - * DB for the current user. Returns null if the user isn't in the org. - */ -export async function resolveAuditorViewAccess( - orgId: string, -): Promise<{ canAccess: boolean; roleString: string | null } | null> { - const session = await auth.api.getSession({ - headers: await headers(), - }); - if (!session?.user?.id) return null; - - const member = await db.member.findFirst({ - where: { - userId: session.user.id, - organizationId: orgId, - deactivated: false, - }, - select: { role: true }, - }); - if (!member) return null; - - const customPerms = await resolveCustomRolePermissions(member.role, orgId); - return { - canAccess: canAccessAuditorView(member.role, customPerms), - roleString: member.role, - }; -} - -/** - * Route guard for the Auditor View page. Replaces `requireRoutePermission( - * 'auditor', orgId)` — the plain permission check let owner/admin through - * via their implicit `audit:read`. This helper enforces the stricter - * "built-in auditor OR custom role with audit:read" rule. - */ -export async function requireAuditorViewAccess(orgId: string): Promise { - const result = await resolveAuditorViewAccess(orgId); - if (result?.canAccess) return; - - const permissions = await resolveCurrentUserPermissions(orgId); - const defaultRoute = permissions - ? getDefaultRoute(permissions, orgId) - : null; - redirect(defaultRoute ?? '/no-access'); -} diff --git a/apps/app/src/lib/permissions.test.ts b/apps/app/src/lib/permissions.test.ts index 180db8bf85..e5727a6780 100644 --- a/apps/app/src/lib/permissions.test.ts +++ b/apps/app/src/lib/permissions.test.ts @@ -1,11 +1,12 @@ import { describe, expect, it } from 'vitest'; import { canAccessApp, - canAccessAuditorView, canAccessCompliance, canAccessRoute, getDefaultRoute, hasPermission, + mergePermissions, + resolveBuiltInPermissions, type UserPermissions, } from './permissions'; @@ -134,60 +135,65 @@ describe('hasPermission', () => { }); }); -// CS-189: Auditor View visibility is intentionally stricter than bare -// `audit:read` — owner and admin implicitly have every permission, but the -// CTO's product decision is that the tab only appears for users whose role -// explicitly scopes them to audit work (built-in auditor OR a custom role -// that specifically grants audit:read). -describe('canAccessAuditorView', () => { - const noCustom: UserPermissions = {}; +// CS-189: Auditor View is gated by `audit:read` (see ROUTE_PERMISSIONS.auditor). +// Owners/admins intentionally do NOT have audit:read in their built-in role +// definitions (packages/auth/src/permissions.ts) — that permission is reserved +// for the built-in `auditor` role or for custom org roles that explicitly +// opt in. This test exercises the full resolution path the app uses. +describe('Auditor View visibility (audit:read gating)', () => { + const resolve = ( + roleString: string, + customRolePerms: UserPermissions = {}, + ): UserPermissions => { + const { permissions } = resolveBuiltInPermissions(roleString); + mergePermissions(permissions, customRolePerms); + return permissions; + }; it('shows for the built-in auditor role', () => { - expect(canAccessAuditorView('auditor', noCustom)).toBe(true); + expect(canAccessRoute(resolve('auditor'), 'auditor')).toBe(true); }); it('shows when auditor is one of several roles (e.g. owner,auditor)', () => { - expect(canAccessAuditorView('owner,auditor', noCustom)).toBe(true); - expect(canAccessAuditorView('admin, auditor', noCustom)).toBe(true); + expect(canAccessRoute(resolve('owner,auditor'), 'auditor')).toBe(true); + expect(canAccessRoute(resolve('admin, auditor'), 'auditor')).toBe(true); }); - it('hides for owner alone (implicit audit:read does NOT count)', () => { - expect(canAccessAuditorView('owner', noCustom)).toBe(false); + it('hides for owner alone (no audit:read on owner role)', () => { + expect(canAccessRoute(resolve('owner'), 'auditor')).toBe(false); }); - it('hides for admin alone (implicit audit:read does NOT count)', () => { - expect(canAccessAuditorView('admin', noCustom)).toBe(false); + it('hides for admin alone (no audit:read on admin role)', () => { + expect(canAccessRoute(resolve('admin'), 'auditor')).toBe(false); }); it('hides for employee / contractor', () => { - expect(canAccessAuditorView('employee', noCustom)).toBe(false); - expect(canAccessAuditorView('contractor', noCustom)).toBe(false); + expect(canAccessRoute(resolve('employee'), 'auditor')).toBe(false); + expect(canAccessRoute(resolve('contractor'), 'auditor')).toBe(false); }); it('shows when a custom role explicitly grants audit:read', () => { - const customRolePerms: UserPermissions = { audit: ['read'] }; - expect(canAccessAuditorView('CompAI', customRolePerms)).toBe(true); + expect( + canAccessRoute(resolve('CompAI', { audit: ['read'] }), 'auditor'), + ).toBe(true); }); it('shows when owner is combined with a custom role that grants audit:read', () => { - const customRolePerms: UserPermissions = { audit: ['read'] }; - expect(canAccessAuditorView('owner,CompAI', customRolePerms)).toBe(true); + expect( + canAccessRoute(resolve('owner,CompAI', { audit: ['read'] }), 'auditor'), + ).toBe(true); }); it('hides when owner has a custom role that does NOT grant audit:read', () => { - // Owner's implicit audit:read is what `permissions` would carry, but - // `canAccessAuditorView` only looks at custom-role permissions — so if - // the custom role is something like "ReadOnlyViewer" without audit, the - // tab stays hidden even though the merged permissions would pass. - const customRolePerms: UserPermissions = { evidence: ['read'] }; - expect(canAccessAuditorView('owner,ReadOnlyViewer', customRolePerms)).toBe( - false, - ); - }); - - it('hides when role string is empty / null / undefined', () => { - expect(canAccessAuditorView('', noCustom)).toBe(false); - expect(canAccessAuditorView(null, noCustom)).toBe(false); - expect(canAccessAuditorView(undefined, noCustom)).toBe(false); + expect( + canAccessRoute( + resolve('owner,ReadOnlyViewer', { evidence: ['read'] }), + 'auditor', + ), + ).toBe(false); + }); + + it('hides when role string is empty', () => { + expect(canAccessRoute(resolve(''), 'auditor')).toBe(false); }); }); diff --git a/apps/app/src/lib/permissions.ts b/apps/app/src/lib/permissions.ts index 79c96e4db3..348368e196 100644 --- a/apps/app/src/lib/permissions.ts +++ b/apps/app/src/lib/permissions.ts @@ -96,31 +96,6 @@ export function canAccessRoute(permissions: UserPermissions, routeSegment: strin return hasAnyPermission(permissions, required); } -/** - * CS-189: Auditor View visibility rule (product decision). - * - * "Auditor View" is scoped to audit work — it should only appear for users - * whose role explicitly grants audit access, not for owners/admins whose - * implicit all-permissions include `audit:read`. - * - * Show the tab iff: - * - user has the built-in `auditor` role, OR - * - user has a custom org role that explicitly grants `audit:read`. - * - * Owners/admins who want this tab can opt in by adding the auditor role - * to their membership or by creating a custom role that includes - * `audit:read`. Multi-role users are handled because `auditor` can be - * one of several roles on a membership. - */ -export function canAccessAuditorView( - roleString: string | null | undefined, - customRolePermissions: UserPermissions, -): boolean { - const roles = parseRolesString(roleString); - if (roles.includes('auditor')) return true; - return hasPermission(customRolePermissions, 'audit', 'read'); -} - /** * Ordered list of main navigation routes used to find a user's default landing page. * Order matches sidebar priority — the first accessible route becomes the default. diff --git a/packages/auth/src/permissions.ts b/packages/auth/src/permissions.ts index 1943ced0b5..8e80b5dfb4 100644 --- a/packages/auth/src/permissions.ts +++ b/packages/auth/src/permissions.ts @@ -70,7 +70,11 @@ export const owner = ac.newRole({ vendor: ['create', 'read', 'update', 'delete'], task: ['create', 'read', 'update', 'delete'], framework: ['create', 'read', 'update', 'delete'], - audit: ['create', 'read', 'update'], + // CS-189: `audit:read` is reserved for the auditor-facing view. Owners can + // still manage audit scope (create/update) — but by omitting `read` here, + // the Auditor View tab/route is hidden from owners unless they opt in via + // a custom role granting `audit:read`. + audit: ['create', 'update'], // Findings are raised by auditors only; owners/admins can view & transition status via update finding: ['read', 'update'], questionnaire: ['create', 'read', 'update', 'delete'], @@ -106,7 +110,9 @@ export const admin = ac.newRole({ vendor: ['create', 'read', 'update', 'delete'], task: ['create', 'read', 'update', 'delete'], framework: ['create', 'read', 'update', 'delete'], - audit: ['create', 'read', 'update'], + // CS-189: see owner role above — `audit:read` is auditor-facing, admins + // can still create/update audit scope. + audit: ['create', 'update'], // Findings are raised by auditors only; owners/admins can view & transition status via update finding: ['read', 'update'], questionnaire: ['create', 'read', 'update', 'delete'], From 4e8073720ed72ec95522177507f7250cd64d99a8 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 20 Apr 2026 15:37:58 -0400 Subject: [PATCH 4/4] Revert "refactor(rbac): drop audit:read from owner/admin, simplify Auditor View gating" This reverts commit c4a4da5a5cfff0e8e6e102589b78c695fd1ca5c8. --- .../training/permissions-regression.spec.ts | 9 +-- .../src/app/(app)/[orgId]/auditor/layout.tsx | 8 +- .../[orgId]/components/AppShellWrapper.tsx | 5 ++ .../(app)/[orgId]/components/AppSidebar.tsx | 14 +++- .../components/app-shell-search-groups.tsx | 7 +- apps/app/src/app/(app)/[orgId]/layout.tsx | 25 +++++- apps/app/src/components/main-menu.tsx | 9 ++- apps/app/src/hooks/use-permissions.ts | 8 ++ apps/app/src/lib/permissions.server.ts | 80 +++++++++++++++++++ apps/app/src/lib/permissions.test.ts | 76 ++++++++---------- apps/app/src/lib/permissions.ts | 25 ++++++ packages/auth/src/permissions.ts | 10 +-- 12 files changed, 213 insertions(+), 63 deletions(-) diff --git a/apps/api/src/training/permissions-regression.spec.ts b/apps/api/src/training/permissions-regression.spec.ts index 7429cea309..1ec83db5f2 100644 --- a/apps/api/src/training/permissions-regression.spec.ts +++ b/apps/api/src/training/permissions-regression.spec.ts @@ -74,11 +74,10 @@ describe('Built-in role permissions — regression', () => { } }); - it('should have audit create/update only (no read, no delete)', () => { - // CS-189: audit:read is reserved for auditor-facing views; owner can - // still manage audit scope via create/update. - expect(perms.audit).toEqual(expect.arrayContaining(['create', 'update'])); - expect(perms.audit).not.toContain('read'); + it('should have audit create/read/update (no delete)', () => { + expect(perms.audit).toEqual( + expect.arrayContaining(['create', 'read', 'update']), + ); expect(perms.audit).not.toContain('delete'); }); diff --git a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx index 842432ee70..76cc7dd1b9 100644 --- a/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/auditor/layout.tsx @@ -1,4 +1,4 @@ -import { requireRoutePermission } from '@/lib/permissions.server'; +import { requireAuditorViewAccess } from '@/lib/permissions.server'; export default async function Layout({ children, @@ -8,6 +8,10 @@ export default async function Layout({ params: Promise<{ orgId: string }>; }) { const { orgId } = await params; - await requireRoutePermission('auditor', orgId); + // CS-189: stricter than `requireRoutePermission('auditor', orgId)` — the + // plain check let owner/admin through via their implicit audit:read. + // requireAuditorViewAccess enforces "built-in auditor OR custom role with + // explicit audit:read" to match the sidebar tab visibility. + await requireAuditorViewAccess(orgId); return <>{children}; } diff --git a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx index c336b4bc88..e6a234a9b5 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx @@ -68,6 +68,8 @@ interface AppShellWrapperProps { isSecurityEnabled: boolean; hasAuditorRole: boolean; isOnlyAuditor: boolean; + /** CS-189: server-resolved Auditor View visibility — see resolveAuditorViewAccess. */ + canAccessAuditorView: boolean; permissions: UserPermissions; user: { name: string | null; @@ -99,6 +101,7 @@ function AppShellWrapperContent({ isSecurityEnabled, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, permissions, user, isAdmin, @@ -144,6 +147,7 @@ function AppShellWrapperContent({ permissions, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -319,6 +323,7 @@ function AppShellWrapperContent({ hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} permissions={permissions} + canAccessAuditorView={canAccessAuditorView} /> )} diff --git a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx index 967e3bc129..004be27c00 100644 --- a/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx @@ -19,6 +19,13 @@ interface AppSidebarProps { hasAuditorRole: boolean; isOnlyAuditor: boolean; permissions: UserPermissions; + /** + * CS-189: Whether this user should see the Auditor View tab. Computed + * server-side (see resolveAuditorViewAccess) because it needs to + * distinguish owner/admin's implicit audit:read from a custom role's + * explicit audit:read. + */ + canAccessAuditorView: boolean; } export function AppSidebar({ @@ -27,6 +34,7 @@ export function AppSidebar({ hasAuditorRole, isOnlyAuditor, permissions, + canAccessAuditorView, }: AppSidebarProps) { const pathname = usePathname() ?? ''; @@ -47,7 +55,11 @@ export function AppSidebar({ id: 'auditor', path: `/${organization.id}/auditor`, name: 'Auditor View', - hidden: !canAccessRoute(permissions, 'auditor'), + // CS-189: visibility is scoped to built-in `auditor` role or a custom + // org role that explicitly grants audit:read. Owner/admin's implicit + // permissions alone are not enough — see `canAccessAuditorView` in + // lib/permissions.ts for the full rule. + hidden: !canAccessAuditorView, }, { id: 'policies', diff --git a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx index 5625e05e9e..1423888be9 100644 --- a/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx +++ b/apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx @@ -26,6 +26,8 @@ interface AppShellSearchGroupsParams { permissions: UserPermissions; hasAuditorRole: boolean; isOnlyAuditor: boolean; + /** CS-189: resolved server-side — see AppShellWrapper. */ + canAccessAuditorView: boolean; isQuestionnaireEnabled: boolean; isTrustNdaEnabled: boolean; isSecurityEnabled: boolean; @@ -64,6 +66,7 @@ export const getAppShellSearchGroups = ({ permissions, hasAuditorRole, isOnlyAuditor, + canAccessAuditorView, isQuestionnaireEnabled, isTrustNdaEnabled, isSecurityEnabled, @@ -96,7 +99,9 @@ export const getAppShellSearchGroups = ({ }), ] : []), - ...(can('auditor') + // CS-189: gate on the server-resolved canAccessAuditorView flag so + // owner/admin are hidden unless they explicitly opt in via a custom role. + ...(canAccessAuditorView ? [ createNavItem({ id: 'auditor', diff --git a/apps/app/src/app/(app)/[orgId]/layout.tsx b/apps/app/src/app/(app)/[orgId]/layout.tsx index 869c58e6e4..24fae1573e 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.tsx @@ -2,8 +2,15 @@ import { getFeatureFlags } from '@/app/posthog'; import { APP_AWS_ORG_ASSETS_BUCKET, s3Client } from '@/app/s3'; import { TriggerTokenProvider } from '@/components/trigger-token-provider'; import { serverApi } from '@/lib/api-server'; -import { canAccessApp, parseRolesString } from '@/lib/permissions'; -import { resolveUserPermissions } from '@/lib/permissions.server'; +import { + canAccessApp, + canAccessAuditorView, + parseRolesString, +} from '@/lib/permissions'; +import { + resolveCustomRolePermissions, + resolveUserPermissions, +} from '@/lib/permissions.server'; import type { OrganizationFromMe } from '@/types'; import { auth } from '@/utils/auth'; import { GetObjectCommand } from '@aws-sdk/client-s3'; @@ -156,6 +163,19 @@ export default async function Layout({ const hasAuditorRole = roles.includes(Role.auditor); const isOnlyAuditor = hasAuditorRole && roles.length === 1; + // CS-189: the Auditor View tab follows a stricter rule than bare + // 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, + ); + // User data for navbar const user = { name: session.user.name, @@ -180,6 +200,7 @@ export default async function Layout({ isSecurityEnabled={isSecurityEnabled} hasAuditorRole={hasAuditorRole} isOnlyAuditor={isOnlyAuditor} + canAccessAuditorView={auditorViewVisible} permissions={permissions} user={user} isAdmin={isUserAdmin} diff --git a/apps/app/src/components/main-menu.tsx b/apps/app/src/components/main-menu.tsx index 404a6e2cfe..25117f3408 100644 --- a/apps/app/src/components/main-menu.tsx +++ b/apps/app/src/components/main-menu.tsx @@ -1,7 +1,6 @@ 'use client'; import { usePermissions } from '@/hooks/use-permissions'; -import { canAccessRoute } from '@/lib/permissions'; import { Badge } from '@trycompai/ui/badge'; import { Button } from '@trycompai/ui/button'; import { cn } from '@trycompai/ui/cn'; @@ -71,7 +70,11 @@ export function MainMenu({ const pathname = usePathname(); const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' }); const itemRefs = useRef<(HTMLDivElement | null)[]>([]); - const { permissions } = usePermissions(); + // CS-189: Auditor View visibility is scoped to the built-in `auditor` + // role or a custom org role that explicitly grants audit:read — NOT to + // owner/admin's implicit all-permissions. See `canAccessAuditorView` in + // lib/permissions.ts for the full rule. + const { canAccessAuditorView } = usePermissions(); const items: MenuItem[] = [ { @@ -97,7 +100,7 @@ export function MainMenu({ disabled: false, icon: ClipboardCheck, protected: false, - hidden: !canAccessRoute(permissions, 'auditor'), + hidden: !canAccessAuditorView, }, { id: 'controls', diff --git a/apps/app/src/hooks/use-permissions.ts b/apps/app/src/hooks/use-permissions.ts index bbbd82af6b..95feb67493 100644 --- a/apps/app/src/hooks/use-permissions.ts +++ b/apps/app/src/hooks/use-permissions.ts @@ -10,6 +10,7 @@ import { hasPermission, parseRolesString, isBuiltInRole, + canAccessAuditorView, type UserPermissions, } from '@/lib/permissions'; @@ -75,11 +76,18 @@ export function usePermissions() { } } + // CS-189: separate "did a custom role grant this permission" from the + // merged permissions, so the Auditor View visibility check can distinguish + // an owner's implicit audit:read from a custom role's explicit audit:read. + const customPermissions = customData?.permissions ?? {}; + return { permissions, + customPermissions, obligations, roles: roleNames, hasPermission: (resource: string, action: string) => hasPermission(permissions, resource, action), + canAccessAuditorView: canAccessAuditorView(roleString, customPermissions), }; } diff --git a/apps/app/src/lib/permissions.server.ts b/apps/app/src/lib/permissions.server.ts index 10beb4c323..8412341ca5 100644 --- a/apps/app/src/lib/permissions.server.ts +++ b/apps/app/src/lib/permissions.server.ts @@ -6,6 +6,7 @@ import { headers } from 'next/headers'; import { redirect } from 'next/navigation'; import { type UserPermissions, + canAccessAuditorView, canAccessRoute, getDefaultRoute, mergePermissions, @@ -92,3 +93,82 @@ export async function requireRoutePermission( redirect(defaultRoute ?? '/no-access'); } } + +/** + * CS-189: Resolve only the permissions granted by the user's CUSTOM org + * roles (i.e. not from built-in roles). Needed for the Auditor View + * visibility rule, which wants to know whether a custom role explicitly + * grants `audit:read` — owner/admin's implicit all-permissions don't count. + */ +export async function resolveCustomRolePermissions( + roleString: string | null | undefined, + orgId: string, +): Promise { + const { customRoleNames } = resolveBuiltInPermissions(roleString); + const result: UserPermissions = {}; + if (customRoleNames.length === 0) return result; + + const customRoles = await db.organizationRole.findMany({ + where: { organizationId: orgId, name: { in: customRoleNames } }, + select: { permissions: true }, + }); + + for (const role of customRoles) { + if (!role.permissions) continue; + const parsed = + typeof role.permissions === 'string' + ? JSON.parse(role.permissions) + : role.permissions; + if (parsed && typeof parsed === 'object') { + mergePermissions(result, parsed as Record); + } + } + return result; +} + +/** + * Server-side Auditor View access check. Mirrors the client-side + * `canAccessAuditorView` but pulls the custom-role permissions from the + * DB for the current user. Returns null if the user isn't in the org. + */ +export async function resolveAuditorViewAccess( + orgId: string, +): Promise<{ canAccess: boolean; roleString: string | null } | null> { + const session = await auth.api.getSession({ + headers: await headers(), + }); + if (!session?.user?.id) return null; + + const member = await db.member.findFirst({ + where: { + userId: session.user.id, + organizationId: orgId, + deactivated: false, + }, + select: { role: true }, + }); + if (!member) return null; + + const customPerms = await resolveCustomRolePermissions(member.role, orgId); + return { + canAccess: canAccessAuditorView(member.role, customPerms), + roleString: member.role, + }; +} + +/** + * Route guard for the Auditor View page. Replaces `requireRoutePermission( + * 'auditor', orgId)` — the plain permission check let owner/admin through + * via their implicit `audit:read`. This helper enforces the stricter + * "built-in auditor OR custom role with audit:read" rule. + */ +export async function requireAuditorViewAccess(orgId: string): Promise { + const result = await resolveAuditorViewAccess(orgId); + if (result?.canAccess) return; + + const permissions = await resolveCurrentUserPermissions(orgId); + const defaultRoute = permissions + ? getDefaultRoute(permissions, orgId) + : null; + redirect(defaultRoute ?? '/no-access'); +} diff --git a/apps/app/src/lib/permissions.test.ts b/apps/app/src/lib/permissions.test.ts index e5727a6780..180db8bf85 100644 --- a/apps/app/src/lib/permissions.test.ts +++ b/apps/app/src/lib/permissions.test.ts @@ -1,12 +1,11 @@ import { describe, expect, it } from 'vitest'; import { canAccessApp, + canAccessAuditorView, canAccessCompliance, canAccessRoute, getDefaultRoute, hasPermission, - mergePermissions, - resolveBuiltInPermissions, type UserPermissions, } from './permissions'; @@ -135,65 +134,60 @@ describe('hasPermission', () => { }); }); -// CS-189: Auditor View is gated by `audit:read` (see ROUTE_PERMISSIONS.auditor). -// Owners/admins intentionally do NOT have audit:read in their built-in role -// definitions (packages/auth/src/permissions.ts) — that permission is reserved -// for the built-in `auditor` role or for custom org roles that explicitly -// opt in. This test exercises the full resolution path the app uses. -describe('Auditor View visibility (audit:read gating)', () => { - const resolve = ( - roleString: string, - customRolePerms: UserPermissions = {}, - ): UserPermissions => { - const { permissions } = resolveBuiltInPermissions(roleString); - mergePermissions(permissions, customRolePerms); - return permissions; - }; +// CS-189: Auditor View visibility is intentionally stricter than bare +// `audit:read` — owner and admin implicitly have every permission, but the +// CTO's product decision is that the tab only appears for users whose role +// explicitly scopes them to audit work (built-in auditor OR a custom role +// that specifically grants audit:read). +describe('canAccessAuditorView', () => { + const noCustom: UserPermissions = {}; it('shows for the built-in auditor role', () => { - expect(canAccessRoute(resolve('auditor'), 'auditor')).toBe(true); + expect(canAccessAuditorView('auditor', noCustom)).toBe(true); }); it('shows when auditor is one of several roles (e.g. owner,auditor)', () => { - expect(canAccessRoute(resolve('owner,auditor'), 'auditor')).toBe(true); - expect(canAccessRoute(resolve('admin, auditor'), 'auditor')).toBe(true); + expect(canAccessAuditorView('owner,auditor', noCustom)).toBe(true); + expect(canAccessAuditorView('admin, auditor', noCustom)).toBe(true); }); - it('hides for owner alone (no audit:read on owner role)', () => { - expect(canAccessRoute(resolve('owner'), 'auditor')).toBe(false); + it('hides for owner alone (implicit audit:read does NOT count)', () => { + expect(canAccessAuditorView('owner', noCustom)).toBe(false); }); - it('hides for admin alone (no audit:read on admin role)', () => { - expect(canAccessRoute(resolve('admin'), 'auditor')).toBe(false); + it('hides for admin alone (implicit audit:read does NOT count)', () => { + expect(canAccessAuditorView('admin', noCustom)).toBe(false); }); it('hides for employee / contractor', () => { - expect(canAccessRoute(resolve('employee'), 'auditor')).toBe(false); - expect(canAccessRoute(resolve('contractor'), 'auditor')).toBe(false); + expect(canAccessAuditorView('employee', noCustom)).toBe(false); + expect(canAccessAuditorView('contractor', noCustom)).toBe(false); }); it('shows when a custom role explicitly grants audit:read', () => { - expect( - canAccessRoute(resolve('CompAI', { audit: ['read'] }), 'auditor'), - ).toBe(true); + const customRolePerms: UserPermissions = { audit: ['read'] }; + expect(canAccessAuditorView('CompAI', customRolePerms)).toBe(true); }); it('shows when owner is combined with a custom role that grants audit:read', () => { - expect( - canAccessRoute(resolve('owner,CompAI', { audit: ['read'] }), 'auditor'), - ).toBe(true); + const customRolePerms: UserPermissions = { audit: ['read'] }; + expect(canAccessAuditorView('owner,CompAI', customRolePerms)).toBe(true); }); it('hides when owner has a custom role that does NOT grant audit:read', () => { - expect( - canAccessRoute( - resolve('owner,ReadOnlyViewer', { evidence: ['read'] }), - 'auditor', - ), - ).toBe(false); - }); - - it('hides when role string is empty', () => { - expect(canAccessRoute(resolve(''), 'auditor')).toBe(false); + // Owner's implicit audit:read is what `permissions` would carry, but + // `canAccessAuditorView` only looks at custom-role permissions — so if + // the custom role is something like "ReadOnlyViewer" without audit, the + // tab stays hidden even though the merged permissions would pass. + const customRolePerms: UserPermissions = { evidence: ['read'] }; + expect(canAccessAuditorView('owner,ReadOnlyViewer', customRolePerms)).toBe( + false, + ); + }); + + it('hides when role string is empty / null / undefined', () => { + expect(canAccessAuditorView('', noCustom)).toBe(false); + expect(canAccessAuditorView(null, noCustom)).toBe(false); + expect(canAccessAuditorView(undefined, noCustom)).toBe(false); }); }); diff --git a/apps/app/src/lib/permissions.ts b/apps/app/src/lib/permissions.ts index 348368e196..79c96e4db3 100644 --- a/apps/app/src/lib/permissions.ts +++ b/apps/app/src/lib/permissions.ts @@ -96,6 +96,31 @@ export function canAccessRoute(permissions: UserPermissions, routeSegment: strin return hasAnyPermission(permissions, required); } +/** + * CS-189: Auditor View visibility rule (product decision). + * + * "Auditor View" is scoped to audit work — it should only appear for users + * whose role explicitly grants audit access, not for owners/admins whose + * implicit all-permissions include `audit:read`. + * + * Show the tab iff: + * - user has the built-in `auditor` role, OR + * - user has a custom org role that explicitly grants `audit:read`. + * + * Owners/admins who want this tab can opt in by adding the auditor role + * to their membership or by creating a custom role that includes + * `audit:read`. Multi-role users are handled because `auditor` can be + * one of several roles on a membership. + */ +export function canAccessAuditorView( + roleString: string | null | undefined, + customRolePermissions: UserPermissions, +): boolean { + const roles = parseRolesString(roleString); + if (roles.includes('auditor')) return true; + return hasPermission(customRolePermissions, 'audit', 'read'); +} + /** * Ordered list of main navigation routes used to find a user's default landing page. * Order matches sidebar priority — the first accessible route becomes the default. diff --git a/packages/auth/src/permissions.ts b/packages/auth/src/permissions.ts index 8e80b5dfb4..1943ced0b5 100644 --- a/packages/auth/src/permissions.ts +++ b/packages/auth/src/permissions.ts @@ -70,11 +70,7 @@ export const owner = ac.newRole({ vendor: ['create', 'read', 'update', 'delete'], task: ['create', 'read', 'update', 'delete'], framework: ['create', 'read', 'update', 'delete'], - // CS-189: `audit:read` is reserved for the auditor-facing view. Owners can - // still manage audit scope (create/update) — but by omitting `read` here, - // the Auditor View tab/route is hidden from owners unless they opt in via - // a custom role granting `audit:read`. - audit: ['create', 'update'], + audit: ['create', 'read', 'update'], // Findings are raised by auditors only; owners/admins can view & transition status via update finding: ['read', 'update'], questionnaire: ['create', 'read', 'update', 'delete'], @@ -110,9 +106,7 @@ export const admin = ac.newRole({ vendor: ['create', 'read', 'update', 'delete'], task: ['create', 'read', 'update', 'delete'], framework: ['create', 'read', 'update', 'delete'], - // CS-189: see owner role above — `audit:read` is auditor-facing, admins - // can still create/update audit scope. - audit: ['create', 'update'], + audit: ['create', 'read', 'update'], // Findings are raised by auditors only; owners/admins can view & transition status via update finding: ['read', 'update'], questionnaire: ['create', 'read', 'update', 'delete'],