Skip to content

Commit fa16795

Browse files
authored
Merge pull request #2581 from trycompai/tofik/cs-189-auditor-view-permission-gating
fix(rbac): gate Auditor View on audit:read instead of role string (CS-189)
2 parents 3e9d55a + 2573c20 commit fa16795

12 files changed

Lines changed: 238 additions & 15 deletions

File tree

apps/app/src/app/(app)/[orgId]/auditor/(overview)/page.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { parseRolesString } from '@/lib/permissions';
33
import { PageHeader, PageLayout } from '@trycompai/design-system';
44
import { Role } from '@db';
55
import type { Metadata } from 'next';
6-
import { notFound, redirect } from 'next/navigation';
6+
import { redirect } from 'next/navigation';
77
import { AuditorView } from './components/AuditorView';
88
import { ExportEvidenceButton } from './components/ExportEvidenceButton';
99

@@ -75,10 +75,10 @@ export default async function AuditorPage({
7575
redirect('/auth/unauthorized');
7676
}
7777

78-
const roles = parseRolesString(currentMember.role);
79-
if (!roles.includes(Role.auditor)) {
80-
notFound();
81-
}
78+
// CS-189: auditor/layout.tsx already calls requireRoutePermission('auditor',
79+
// orgId) which enforces audit:read. The prior literal-role check
80+
// (roles.includes(Role.auditor)) was redundant AND wrong — it would 404 for
81+
// owners/admins/custom roles that legitimately have audit:read.
8282

8383
const organizationName = orgRes.data?.name ?? 'Organization';
8484
const logoUrl = orgRes.data?.logoUrl ?? null;
Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { requireRoutePermission } from '@/lib/permissions.server';
1+
import { requireAuditorViewAccess } from '@/lib/permissions.server';
22

33
export default async function Layout({
44
children,
@@ -8,6 +8,10 @@ export default async function Layout({
88
params: Promise<{ orgId: string }>;
99
}) {
1010
const { orgId } = await params;
11-
await requireRoutePermission('auditor', orgId);
11+
// CS-189: stricter than `requireRoutePermission('auditor', orgId)` — the
12+
// plain check let owner/admin through via their implicit audit:read.
13+
// requireAuditorViewAccess enforces "built-in auditor OR custom role with
14+
// explicit audit:read" to match the sidebar tab visibility.
15+
await requireAuditorViewAccess(orgId);
1216
return <>{children}</>;
1317
}

apps/app/src/app/(app)/[orgId]/components/AppShellWrapper.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ interface AppShellWrapperProps {
6868
isSecurityEnabled: boolean;
6969
hasAuditorRole: boolean;
7070
isOnlyAuditor: boolean;
71+
/** CS-189: server-resolved Auditor View visibility — see resolveAuditorViewAccess. */
72+
canAccessAuditorView: boolean;
7173
permissions: UserPermissions;
7274
user: {
7375
name: string | null;
@@ -99,6 +101,7 @@ function AppShellWrapperContent({
99101
isSecurityEnabled,
100102
hasAuditorRole,
101103
isOnlyAuditor,
104+
canAccessAuditorView,
102105
permissions,
103106
user,
104107
isAdmin,
@@ -144,6 +147,7 @@ function AppShellWrapperContent({
144147
permissions,
145148
hasAuditorRole,
146149
isOnlyAuditor,
150+
canAccessAuditorView,
147151
isQuestionnaireEnabled,
148152
isTrustNdaEnabled,
149153
isSecurityEnabled,
@@ -319,6 +323,7 @@ function AppShellWrapperContent({
319323
hasAuditorRole={hasAuditorRole}
320324
isOnlyAuditor={isOnlyAuditor}
321325
permissions={permissions}
326+
canAccessAuditorView={canAccessAuditorView}
322327
/>
323328
)}
324329
</AppShellSidebar>

apps/app/src/app/(app)/[orgId]/components/AppSidebar.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ interface AppSidebarProps {
1919
hasAuditorRole: boolean;
2020
isOnlyAuditor: boolean;
2121
permissions: UserPermissions;
22+
/**
23+
* CS-189: Whether this user should see the Auditor View tab. Computed
24+
* server-side (see resolveAuditorViewAccess) because it needs to
25+
* distinguish owner/admin's implicit audit:read from a custom role's
26+
* explicit audit:read.
27+
*/
28+
canAccessAuditorView: boolean;
2229
}
2330

2431
export function AppSidebar({
@@ -27,6 +34,7 @@ export function AppSidebar({
2734
hasAuditorRole,
2835
isOnlyAuditor,
2936
permissions,
37+
canAccessAuditorView,
3038
}: AppSidebarProps) {
3139
const pathname = usePathname() ?? '';
3240

@@ -47,7 +55,11 @@ export function AppSidebar({
4755
id: 'auditor',
4856
path: `/${organization.id}/auditor`,
4957
name: 'Auditor View',
50-
hidden: !hasAuditorRole || !canAccessRoute(permissions, 'auditor'),
58+
// CS-189: visibility is scoped to built-in `auditor` role or a custom
59+
// org role that explicitly grants audit:read. Owner/admin's implicit
60+
// permissions alone are not enough — see `canAccessAuditorView` in
61+
// lib/permissions.ts for the full rule.
62+
hidden: !canAccessAuditorView,
5163
},
5264
{
5365
id: 'policies',

apps/app/src/app/(app)/[orgId]/components/app-shell-search-groups.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ interface AppShellSearchGroupsParams {
2626
permissions: UserPermissions;
2727
hasAuditorRole: boolean;
2828
isOnlyAuditor: boolean;
29+
/** CS-189: resolved server-side — see AppShellWrapper. */
30+
canAccessAuditorView: boolean;
2931
isQuestionnaireEnabled: boolean;
3032
isTrustNdaEnabled: boolean;
3133
isSecurityEnabled: boolean;
@@ -64,6 +66,7 @@ export const getAppShellSearchGroups = ({
6466
permissions,
6567
hasAuditorRole,
6668
isOnlyAuditor,
69+
canAccessAuditorView,
6770
isQuestionnaireEnabled,
6871
isTrustNdaEnabled,
6972
isSecurityEnabled,
@@ -96,7 +99,9 @@ export const getAppShellSearchGroups = ({
9699
}),
97100
]
98101
: []),
99-
...(hasAuditorRole && can('auditor')
102+
// CS-189: gate on the server-resolved canAccessAuditorView flag so
103+
// owner/admin are hidden unless they explicitly opt in via a custom role.
104+
...(canAccessAuditorView
100105
? [
101106
createNavItem({
102107
id: 'auditor',

apps/app/src/app/(app)/[orgId]/layout.tsx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@ import { getFeatureFlags } from '@/app/posthog';
22
import { APP_AWS_ORG_ASSETS_BUCKET, s3Client } from '@/app/s3';
33
import { TriggerTokenProvider } from '@/components/trigger-token-provider';
44
import { serverApi } from '@/lib/api-server';
5-
import { canAccessApp, parseRolesString } from '@/lib/permissions';
6-
import { resolveUserPermissions } from '@/lib/permissions.server';
5+
import {
6+
canAccessApp,
7+
canAccessAuditorView,
8+
parseRolesString,
9+
} from '@/lib/permissions';
10+
import {
11+
resolveCustomRolePermissions,
12+
resolveUserPermissions,
13+
} from '@/lib/permissions.server';
714
import type { OrganizationFromMe } from '@/types';
815
import { auth } from '@/utils/auth';
916
import { GetObjectCommand } from '@aws-sdk/client-s3';
@@ -156,6 +163,19 @@ export default async function Layout({
156163
const hasAuditorRole = roles.includes(Role.auditor);
157164
const isOnlyAuditor = hasAuditorRole && roles.length === 1;
158165

166+
// CS-189: the Auditor View tab follows a stricter rule than bare
167+
// audit:read — built-in `auditor` role OR a custom role with explicit
168+
// audit:read. Resolve the custom-role permissions once so we don't
169+
// second-guess the owner/admin's implicit all-permissions in the UI.
170+
const customRolePermissions = await resolveCustomRolePermissions(
171+
member.role,
172+
requestedOrgId,
173+
);
174+
const auditorViewVisible = canAccessAuditorView(
175+
member.role,
176+
customRolePermissions,
177+
);
178+
159179
// User data for navbar
160180
const user = {
161181
name: session.user.name,
@@ -180,6 +200,7 @@ export default async function Layout({
180200
isSecurityEnabled={isSecurityEnabled}
181201
hasAuditorRole={hasAuditorRole}
182202
isOnlyAuditor={isOnlyAuditor}
203+
canAccessAuditorView={auditorViewVisible}
183204
permissions={permissions}
184205
user={user}
185206
isAdmin={isUserAdmin}

apps/app/src/components/main-menu.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client';
22

3+
import { usePermissions } from '@/hooks/use-permissions';
34
import { Badge } from '@trycompai/ui/badge';
45
import { Button } from '@trycompai/ui/button';
56
import { cn } from '@trycompai/ui/cn';
@@ -54,7 +55,6 @@ export type Props = {
5455
onItemClick?: () => void;
5556
isQuestionnaireEnabled?: boolean;
5657
isTrustNdaEnabled?: boolean;
57-
hasAuditorRole?: boolean;
5858
isOnlyAuditor?: boolean;
5959
};
6060

@@ -65,12 +65,16 @@ export function MainMenu({
6565
onItemClick,
6666
isQuestionnaireEnabled = false,
6767
isTrustNdaEnabled = false,
68-
hasAuditorRole = false,
6968
isOnlyAuditor = false,
7069
}: Props) {
7170
const pathname = usePathname();
7271
const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' });
7372
const itemRefs = useRef<(HTMLDivElement | null)[]>([]);
73+
// CS-189: Auditor View visibility is scoped to the built-in `auditor`
74+
// role or a custom org role that explicitly grants audit:read — NOT to
75+
// owner/admin's implicit all-permissions. See `canAccessAuditorView` in
76+
// lib/permissions.ts for the full rule.
77+
const { canAccessAuditorView } = usePermissions();
7478

7579
const items: MenuItem[] = [
7680
{
@@ -96,7 +100,7 @@ export function MainMenu({
96100
disabled: false,
97101
icon: ClipboardCheck,
98102
protected: false,
99-
hidden: !hasAuditorRole,
103+
hidden: !canAccessAuditorView,
100104
},
101105
{
102106
id: 'controls',

apps/app/src/components/sidebar.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ export async function Sidebar({
9999
isCollapsed={isCollapsed}
100100
isQuestionnaireEnabled={isQuestionnaireEnabled}
101101
isTrustNdaEnabled={isTrustNdaEnabled}
102-
hasAuditorRole={hasAuditorRole}
103102
isOnlyAuditor={isOnlyAuditor}
104103
/>
105104
</div>

apps/app/src/hooks/use-permissions.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
hasPermission,
1111
parseRolesString,
1212
isBuiltInRole,
13+
canAccessAuditorView,
1314
type UserPermissions,
1415
} from '@/lib/permissions';
1516

@@ -75,11 +76,18 @@ export function usePermissions() {
7576
}
7677
}
7778

79+
// CS-189: separate "did a custom role grant this permission" from the
80+
// merged permissions, so the Auditor View visibility check can distinguish
81+
// an owner's implicit audit:read from a custom role's explicit audit:read.
82+
const customPermissions = customData?.permissions ?? {};
83+
7884
return {
7985
permissions,
86+
customPermissions,
8087
obligations,
8188
roles: roleNames,
8289
hasPermission: (resource: string, action: string) =>
8390
hasPermission(permissions, resource, action),
91+
canAccessAuditorView: canAccessAuditorView(roleString, customPermissions),
8492
};
8593
}

apps/app/src/lib/permissions.server.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { headers } from 'next/headers';
66
import { redirect } from 'next/navigation';
77
import {
88
type UserPermissions,
9+
canAccessAuditorView,
910
canAccessRoute,
1011
getDefaultRoute,
1112
mergePermissions,
@@ -92,3 +93,82 @@ export async function requireRoutePermission(
9293
redirect(defaultRoute ?? '/no-access');
9394
}
9495
}
96+
97+
/**
98+
* CS-189: Resolve only the permissions granted by the user's CUSTOM org
99+
* roles (i.e. not from built-in roles). Needed for the Auditor View
100+
* visibility rule, which wants to know whether a custom role explicitly
101+
* grants `audit:read` — owner/admin's implicit all-permissions don't count.
102+
*/
103+
export async function resolveCustomRolePermissions(
104+
roleString: string | null | undefined,
105+
orgId: string,
106+
): Promise<UserPermissions> {
107+
const { customRoleNames } = resolveBuiltInPermissions(roleString);
108+
const result: UserPermissions = {};
109+
if (customRoleNames.length === 0) return result;
110+
111+
const customRoles = await db.organizationRole.findMany({
112+
where: { organizationId: orgId, name: { in: customRoleNames } },
113+
select: { permissions: true },
114+
});
115+
116+
for (const role of customRoles) {
117+
if (!role.permissions) continue;
118+
const parsed =
119+
typeof role.permissions === 'string'
120+
? JSON.parse(role.permissions)
121+
: role.permissions;
122+
if (parsed && typeof parsed === 'object') {
123+
mergePermissions(result, parsed as Record<string, string[]>);
124+
}
125+
}
126+
return result;
127+
}
128+
129+
/**
130+
* Server-side Auditor View access check. Mirrors the client-side
131+
* `canAccessAuditorView` but pulls the custom-role permissions from the
132+
* DB for the current user. Returns null if the user isn't in the org.
133+
*/
134+
export async function resolveAuditorViewAccess(
135+
orgId: string,
136+
): Promise<{ canAccess: boolean; roleString: string | null } | null> {
137+
const session = await auth.api.getSession({
138+
headers: await headers(),
139+
});
140+
if (!session?.user?.id) return null;
141+
142+
const member = await db.member.findFirst({
143+
where: {
144+
userId: session.user.id,
145+
organizationId: orgId,
146+
deactivated: false,
147+
},
148+
select: { role: true },
149+
});
150+
if (!member) return null;
151+
152+
const customPerms = await resolveCustomRolePermissions(member.role, orgId);
153+
return {
154+
canAccess: canAccessAuditorView(member.role, customPerms),
155+
roleString: member.role,
156+
};
157+
}
158+
159+
/**
160+
* Route guard for the Auditor View page. Replaces `requireRoutePermission(
161+
* 'auditor', orgId)` — the plain permission check let owner/admin through
162+
* via their implicit `audit:read`. This helper enforces the stricter
163+
* "built-in auditor OR custom role with audit:read" rule.
164+
*/
165+
export async function requireAuditorViewAccess(orgId: string): Promise<void> {
166+
const result = await resolveAuditorViewAccess(orgId);
167+
if (result?.canAccess) return;
168+
169+
const permissions = await resolveCurrentUserPermissions(orgId);
170+
const defaultRoute = permissions
171+
? getDefaultRoute(permissions, orgId)
172+
: null;
173+
redirect(defaultRoute ?? '/no-access');
174+
}

0 commit comments

Comments
 (0)