Skip to content

Commit c4a4da5

Browse files
committed
refactor(rbac): drop audit:read from owner/admin, simplify Auditor View gating
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).
1 parent 13ac985 commit c4a4da5

12 files changed

Lines changed: 63 additions & 213 deletions

File tree

apps/api/src/training/permissions-regression.spec.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,11 @@ describe('Built-in role permissions — regression', () => {
7474
}
7575
});
7676

77-
it('should have audit create/read/update (no delete)', () => {
78-
expect(perms.audit).toEqual(
79-
expect.arrayContaining(['create', 'read', 'update']),
80-
);
77+
it('should have audit create/update only (no read, no delete)', () => {
78+
// CS-189: audit:read is reserved for auditor-facing views; owner can
79+
// still manage audit scope via create/update.
80+
expect(perms.audit).toEqual(expect.arrayContaining(['create', 'update']));
81+
expect(perms.audit).not.toContain('read');
8182
expect(perms.audit).not.toContain('delete');
8283
});
8384

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { requireAuditorViewAccess } from '@/lib/permissions.server';
1+
import { requireRoutePermission } from '@/lib/permissions.server';
22

33
export default async function Layout({
44
children,
@@ -8,10 +8,6 @@ export default async function Layout({
88
params: Promise<{ orgId: string }>;
99
}) {
1010
const { orgId } = await params;
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);
11+
await requireRoutePermission('auditor', orgId);
1612
return <>{children}</>;
1713
}

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,6 @@ interface AppShellWrapperProps {
6868
isSecurityEnabled: boolean;
6969
hasAuditorRole: boolean;
7070
isOnlyAuditor: boolean;
71-
/** CS-189: server-resolved Auditor View visibility — see resolveAuditorViewAccess. */
72-
canAccessAuditorView: boolean;
7371
permissions: UserPermissions;
7472
user: {
7573
name: string | null;
@@ -101,7 +99,6 @@ function AppShellWrapperContent({
10199
isSecurityEnabled,
102100
hasAuditorRole,
103101
isOnlyAuditor,
104-
canAccessAuditorView,
105102
permissions,
106103
user,
107104
isAdmin,
@@ -147,7 +144,6 @@ function AppShellWrapperContent({
147144
permissions,
148145
hasAuditorRole,
149146
isOnlyAuditor,
150-
canAccessAuditorView,
151147
isQuestionnaireEnabled,
152148
isTrustNdaEnabled,
153149
isSecurityEnabled,
@@ -323,7 +319,6 @@ function AppShellWrapperContent({
323319
hasAuditorRole={hasAuditorRole}
324320
isOnlyAuditor={isOnlyAuditor}
325321
permissions={permissions}
326-
canAccessAuditorView={canAccessAuditorView}
327322
/>
328323
)}
329324
</AppShellSidebar>

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,6 @@ 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;
2922
}
3023

3124
export function AppSidebar({
@@ -34,7 +27,6 @@ export function AppSidebar({
3427
hasAuditorRole,
3528
isOnlyAuditor,
3629
permissions,
37-
canAccessAuditorView,
3830
}: AppSidebarProps) {
3931
const pathname = usePathname() ?? '';
4032

@@ -55,11 +47,7 @@ export function AppSidebar({
5547
id: 'auditor',
5648
path: `/${organization.id}/auditor`,
5749
name: 'Auditor View',
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,
50+
hidden: !canAccessRoute(permissions, 'auditor'),
6351
},
6452
{
6553
id: 'policies',

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ interface AppShellSearchGroupsParams {
2626
permissions: UserPermissions;
2727
hasAuditorRole: boolean;
2828
isOnlyAuditor: boolean;
29-
/** CS-189: resolved server-side — see AppShellWrapper. */
30-
canAccessAuditorView: boolean;
3129
isQuestionnaireEnabled: boolean;
3230
isTrustNdaEnabled: boolean;
3331
isSecurityEnabled: boolean;
@@ -66,7 +64,6 @@ export const getAppShellSearchGroups = ({
6664
permissions,
6765
hasAuditorRole,
6866
isOnlyAuditor,
69-
canAccessAuditorView,
7067
isQuestionnaireEnabled,
7168
isTrustNdaEnabled,
7269
isSecurityEnabled,
@@ -99,9 +96,7 @@ export const getAppShellSearchGroups = ({
9996
}),
10097
]
10198
: []),
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
99+
...(can('auditor')
105100
? [
106101
createNavItem({
107102
id: 'auditor',

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

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,8 @@ 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 {
6-
canAccessApp,
7-
canAccessAuditorView,
8-
parseRolesString,
9-
} from '@/lib/permissions';
10-
import {
11-
resolveCustomRolePermissions,
12-
resolveUserPermissions,
13-
} from '@/lib/permissions.server';
5+
import { canAccessApp, parseRolesString } from '@/lib/permissions';
6+
import { resolveUserPermissions } from '@/lib/permissions.server';
147
import type { OrganizationFromMe } from '@/types';
158
import { auth } from '@/utils/auth';
169
import { GetObjectCommand } from '@aws-sdk/client-s3';
@@ -163,19 +156,6 @@ export default async function Layout({
163156
const hasAuditorRole = roles.includes(Role.auditor);
164157
const isOnlyAuditor = hasAuditorRole && roles.length === 1;
165158

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-
179159
// User data for navbar
180160
const user = {
181161
name: session.user.name,
@@ -200,7 +180,6 @@ export default async function Layout({
200180
isSecurityEnabled={isSecurityEnabled}
201181
hasAuditorRole={hasAuditorRole}
202182
isOnlyAuditor={isOnlyAuditor}
203-
canAccessAuditorView={auditorViewVisible}
204183
permissions={permissions}
205184
user={user}
206185
isAdmin={isUserAdmin}

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

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

33
import { usePermissions } from '@/hooks/use-permissions';
4+
import { canAccessRoute } from '@/lib/permissions';
45
import { Badge } from '@trycompai/ui/badge';
56
import { Button } from '@trycompai/ui/button';
67
import { cn } from '@trycompai/ui/cn';
@@ -70,11 +71,7 @@ export function MainMenu({
7071
const pathname = usePathname();
7172
const [activeStyle, setActiveStyle] = useState({ top: '0px', height: '0px' });
7273
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();
74+
const { permissions } = usePermissions();
7875

7976
const items: MenuItem[] = [
8077
{
@@ -100,7 +97,7 @@ export function MainMenu({
10097
disabled: false,
10198
icon: ClipboardCheck,
10299
protected: false,
103-
hidden: !canAccessAuditorView,
100+
hidden: !canAccessRoute(permissions, 'auditor'),
104101
},
105102
{
106103
id: 'controls',

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

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

@@ -76,18 +75,11 @@ export function usePermissions() {
7675
}
7776
}
7877

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-
8478
return {
8579
permissions,
86-
customPermissions,
8780
obligations,
8881
roles: roleNames,
8982
hasPermission: (resource: string, action: string) =>
9083
hasPermission(permissions, resource, action),
91-
canAccessAuditorView: canAccessAuditorView(roleString, customPermissions),
9284
};
9385
}

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

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { headers } from 'next/headers';
66
import { redirect } from 'next/navigation';
77
import {
88
type UserPermissions,
9-
canAccessAuditorView,
109
canAccessRoute,
1110
getDefaultRoute,
1211
mergePermissions,
@@ -93,82 +92,3 @@ export async function requireRoutePermission(
9392
redirect(defaultRoute ?? '/no-access');
9493
}
9594
}
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)