Skip to content

Commit 75465ac

Browse files
sean-brydonvolneieunjae-lee
authored
feat: PBAC on team members page + org members page to use specific permissi… (#23004)
* PBAC on team members page + org members page to use specific permissions with fallbacks * Implement checks on edit sheet fetch to check for permisisons * WIP permission fixes * fix tests * WIP * fix edit mode perms * fix remove and list view permissions * fix add * feat: add pbac permissions for impersonation (#23035) * registery + migration + i18n * UI for can impersonate * impersonation backend * remove permission from db query * add missing prisma import * add resend invitation and edit mode checks for delete/changeRole * fix canChangeRole * canChangeRole gate * fix permission logic * fix depends on * push migration * fix migration * fix scoped advanced permissions * use listMembers instead of invite proxy * remove isOrgAdmin check to rely on pbac * wait for session to load before navigating * use event-types testId to ensure session loaded * use profile page instead of eventTypes * use correct roles * fix wait for session * correct orgs private isFallBackRoles * set a timeout on waiting for session * remove adminOrOwner check since its in fallpack --------- Co-authored-by: Volnei Munhoz <[email protected]> Co-authored-by: Eunjae Lee <[email protected]>
1 parent 2dbd0b3 commit 75465ac

File tree

28 files changed

+831
-215
lines changed

28 files changed

+831
-215
lines changed

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,18 @@ const organizationAdminKeys = [
181181
"delegation_credential",
182182
];
183183

184+
export interface SettingsPermissions {
185+
canViewRoles?: boolean;
186+
}
187+
184188
const useTabs = ({
185189
isDelegationCredentialEnabled,
186190
isPbacEnabled,
187-
canViewRoles,
191+
permissions,
188192
}: {
189193
isDelegationCredentialEnabled: boolean;
190194
isPbacEnabled: boolean;
191-
canViewRoles?: boolean;
195+
permissions?: SettingsPermissions;
192196
}) => {
193197
const session = useSession();
194198
const { data: user } = trpc.viewer.me.get.useQuery({ includePasswordAdded: true });
@@ -227,7 +231,7 @@ const useTabs = ({
227231

228232
// Add pbac menu item only if feature flag is enabled AND user has permission to view roles
229233
// This prevents showing the menu item when user has no organization permissions
230-
if (isPbacEnabled && canViewRoles) {
234+
if (isPbacEnabled && permissions?.canViewRoles) {
231235
newArray.push({
232236
name: "roles_and_permissions",
233237
href: "/settings/organizations/roles",
@@ -294,7 +298,7 @@ interface SettingsSidebarContainerProps {
294298
navigationIsOpenedOnMobile?: boolean;
295299
bannersHeight?: number;
296300
teamFeatures?: Record<number, TeamFeatures>;
297-
canViewRoles?: boolean;
301+
permissions?: SettingsPermissions;
298302
}
299303

300304
const TeamRolesNavItem = ({
@@ -487,7 +491,7 @@ const SettingsSidebarContainer = ({
487491
navigationIsOpenedOnMobile,
488492
bannersHeight,
489493
teamFeatures,
490-
canViewRoles,
494+
permissions,
491495
}: SettingsSidebarContainerProps) => {
492496
const searchParams = useCompatSearchParams();
493497
const orgBranding = useOrgBranding();
@@ -517,7 +521,7 @@ const SettingsSidebarContainer = ({
517521
const tabsWithPermissions = useTabs({
518522
isDelegationCredentialEnabled,
519523
isPbacEnabled,
520-
canViewRoles,
524+
permissions,
521525
});
522526

523527
const { data: otherTeams } = trpc.viewer.organizations.listOtherTeams.useQuery(undefined, {
@@ -792,13 +796,13 @@ export type SettingsLayoutProps = {
792796
children: React.ReactNode;
793797
containerClassName?: string;
794798
teamFeatures?: Record<number, TeamFeatures>;
795-
canViewRoles?: boolean;
799+
permissions?: SettingsPermissions;
796800
} & ComponentProps<typeof Shell>;
797801

798802
export default function SettingsLayoutAppDirClient({
799803
children,
800804
teamFeatures,
801-
canViewRoles,
805+
permissions,
802806
...rest
803807
}: SettingsLayoutProps) {
804808
const pathname = usePathname();
@@ -833,7 +837,7 @@ export default function SettingsLayoutAppDirClient({
833837
sideContainerOpen={sideContainerOpen}
834838
setSideContainerOpen={setSideContainerOpen}
835839
teamFeatures={teamFeatures}
836-
canViewRoles={canViewRoles}
840+
permissions={permissions}
837841
/>
838842
}
839843
drawerState={state}
@@ -856,15 +860,15 @@ type SidebarContainerElementProps = {
856860
bannersHeight?: number;
857861
setSideContainerOpen: React.Dispatch<React.SetStateAction<boolean>>;
858862
teamFeatures?: Record<number, TeamFeatures>;
859-
canViewRoles?: boolean;
863+
permissions?: SettingsPermissions;
860864
};
861865

862866
const SidebarContainerElement = ({
863867
sideContainerOpen,
864868
bannersHeight,
865869
setSideContainerOpen,
866870
teamFeatures,
867-
canViewRoles,
871+
permissions,
868872
}: SidebarContainerElementProps) => {
869873
const { t } = useLocale();
870874
return (
@@ -881,7 +885,7 @@ const SidebarContainerElement = ({
881885
navigationIsOpenedOnMobile={sideContainerOpen}
882886
bannersHeight={bannersHeight}
883887
teamFeatures={teamFeatures}
884-
canViewRoles={canViewRoles}
888+
permissions={permissions}
885889
/>
886890
</>
887891
);

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ export default async function SettingsLayoutAppDir(props: SettingsLayoutProps) {
6767

6868
return (
6969
<>
70-
<SettingsLayoutAppDirClient {...props} teamFeatures={teamFeatures ?? {}} canViewRoles={canViewRoles} />
70+
<SettingsLayoutAppDirClient
71+
{...props}
72+
teamFeatures={teamFeatures ?? {}}
73+
permissions={{ canViewRoles }}
74+
/>
7175
</>
7276
);
7377
}

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/AdvancedPermissionGroup.tsx‎

Lines changed: 69 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
import { useState } from "react";
44

55
import type { Resource } from "@calcom/features/pbac/domain/types/permission-registry";
6-
import { PERMISSION_REGISTRY, CrudAction } from "@calcom/features/pbac/domain/types/permission-registry";
6+
import {
7+
Scope,
8+
CrudAction,
9+
getPermissionsForScope,
10+
} from "@calcom/features/pbac/domain/types/permission-registry";
711
import { useLocale } from "@calcom/lib/hooks/useLocale";
812
import classNames from "@calcom/ui/classNames";
913
import { Checkbox, Label } from "@calcom/ui/components/form";
@@ -17,6 +21,7 @@ interface AdvancedPermissionGroupProps {
1721
selectedPermissions: string[];
1822
onChange: (permissions: string[]) => void;
1923
disabled?: boolean;
24+
scope?: Scope;
2025
}
2126

2227
const INTERNAL_DATAACCESS_KEY = "_resource";
@@ -26,21 +31,31 @@ export function AdvancedPermissionGroup({
2631
selectedPermissions,
2732
onChange,
2833
disabled,
34+
scope = Scope.Organization,
2935
}: AdvancedPermissionGroupProps) {
3036
const { t } = useLocale();
31-
const { toggleSinglePermission, toggleResourcePermissionLevel } = usePermissions();
32-
const resourceConfig = PERMISSION_REGISTRY[resource];
37+
const { toggleSinglePermission, toggleResourcePermissionLevel } = usePermissions(scope);
38+
const scopedRegistry = getPermissionsForScope(scope);
39+
const resourceConfig = scopedRegistry[resource];
3340
const [isExpanded, setIsExpanded] = useState(false);
3441

3542
const isAllResources = resource === "*";
43+
44+
// Early return if resource is not in the scoped registry (and not the special "*" resource)
45+
if (!isAllResources && !resourceConfig) {
46+
return null;
47+
}
48+
3649
const allResourcesSelected = selectedPermissions.includes("*.*");
3750

3851
// Get all possible permissions for this resource
3952
const allPermissions = isAllResources
4053
? ["*.*"]
41-
: Object.entries(resourceConfig)
54+
: resourceConfig
55+
? Object.entries(resourceConfig)
4256
.filter(([action]) => action !== INTERNAL_DATAACCESS_KEY)
43-
.map(([action]) => `${resource}.${action}`);
57+
.map(([action]) => `${resource}.${action}`)
58+
: [];
4459

4560
// Check if all permissions for this resource are selected
4661
const isAllSelected = isAllResources
@@ -99,7 +114,7 @@ export function AdvancedPermissionGroup({
99114
<span
100115
className="text-default cursor-pointer text-sm font-medium leading-none"
101116
onClick={() => setIsExpanded(!isExpanded)}>
102-
{t(resourceConfig._resource?.i18nKey || "")}
117+
{t(resourceConfig?._resource?.i18nKey || "")}
103118
</span>
104119
<span
105120
className="text-muted cursor-pointer text-sm font-medium leading-none"
@@ -113,56 +128,57 @@ export function AdvancedPermissionGroup({
113128
className="bg-default border-muted m-1 flex flex-col gap-2.5 rounded-xl border p-3"
114129
onClick={(e) => e.stopPropagation()} // Stop clicks in the permission list from affecting parent
115130
>
116-
{Object.entries(resourceConfig).map(([action, actionConfig]) => {
117-
const permission = `${resource}.${action}`;
118-
119-
if (action === INTERNAL_DATAACCESS_KEY) {
120-
return null;
121-
}
122-
123-
const isChecked = selectedPermissions.includes(permission);
124-
const isReadPermission = action === CrudAction.Read;
125-
const isAutoEnabled = isReadAutoEnabled(action);
126-
127-
return (
128-
<div key={action} className="flex items-center">
129-
<Checkbox
130-
id={permission}
131-
checked={isChecked}
132-
className="mr-2"
133-
onCheckedChange={(checked) => {
134-
if (!disabled) {
135-
onChange(toggleSinglePermission(permission, !!checked, selectedPermissions));
136-
}
137-
}}
138-
onClick={(e) => e.stopPropagation()} // Stop checkbox clicks from affecting parent
139-
disabled={disabled}
140-
/>
141-
<div
142-
className="flex items-center gap-2"
143-
onClick={(e) => e.stopPropagation()} // Stop label clicks from affecting parent
144-
>
145-
<Label htmlFor={permission} className="mb-0">
146-
<span className={classNames(isAutoEnabled && "text-muted-foreground")}>
147-
{t(actionConfig?.i18nKey || "")}
131+
{resourceConfig &&
132+
Object.entries(resourceConfig).map(([action, actionConfig]) => {
133+
const permission = `${resource}.${action}`;
134+
135+
if (action === INTERNAL_DATAACCESS_KEY) {
136+
return null;
137+
}
138+
139+
const isChecked = selectedPermissions.includes(permission);
140+
const isReadPermission = action === CrudAction.Read;
141+
const isAutoEnabled = isReadAutoEnabled(action);
142+
143+
return (
144+
<div key={action} className="flex items-center">
145+
<Checkbox
146+
id={permission}
147+
checked={isChecked}
148+
className="mr-2"
149+
onCheckedChange={(checked) => {
150+
if (!disabled) {
151+
onChange(toggleSinglePermission(permission, !!checked, selectedPermissions));
152+
}
153+
}}
154+
onClick={(e) => e.stopPropagation()} // Stop checkbox clicks from affecting parent
155+
disabled={disabled}
156+
/>
157+
<div
158+
className="flex items-center gap-2"
159+
onClick={(e) => e.stopPropagation()} // Stop label clicks from affecting parent
160+
>
161+
<Label htmlFor={permission} className="mb-0">
162+
<span className={classNames(isAutoEnabled && "text-muted-foreground")}>
163+
{t(actionConfig?.i18nKey || "")}
164+
</span>
165+
</Label>
166+
<span className="text-sm text-gray-500">
167+
{t(
168+
actionConfig && "descriptionI18nKey" in actionConfig
169+
? actionConfig.descriptionI18nKey
170+
: ""
171+
)}
148172
</span>
149-
</Label>
150-
<span className="text-sm text-gray-500">
151-
{t(
152-
actionConfig && "descriptionI18nKey" in actionConfig
153-
? actionConfig.descriptionI18nKey
154-
: ""
173+
{isAutoEnabled && (
174+
<Tooltip content={t("read_permission_auto_enabled_tooltip")}>
175+
<Icon name="info" className="text-muted-foreground h-3 w-3" />
176+
</Tooltip>
155177
)}
156-
</span>
157-
{isAutoEnabled && (
158-
<Tooltip content={t("read_permission_auto_enabled_tooltip")}>
159-
<Icon name="info" className="text-muted-foreground h-3 w-3" />
160-
</Tooltip>
161-
)}
178+
</div>
162179
</div>
163-
</div>
164-
);
165-
})}{" "}
180+
);
181+
})}{" "}
166182
</div>
167183
)}
168184
</div>

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RoleSheet.tsx‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,6 @@ export function RoleSheet({ role, open, onOpenChange, teamId, scope = Scope.Orga
145145
});
146146

147147
const onSubmit = (values: FormValues) => {
148-
// Store the color in localStorage
149-
const roleKey = isEditing && role ? role.id : `new_role_${values.name}`;
150-
localStorage.setItem(`role_color_${roleKey}`, values.color);
151-
152148
if (isEditing && role) {
153149
updateMutation.mutate({
154150
teamId,
@@ -224,6 +220,7 @@ export function RoleSheet({ role, open, onOpenChange, teamId, scope = Scope.Orga
224220
selectedPermissions={permissions}
225221
onChange={(newPermissions) => form.setValue("permissions", newPermissions)}
226222
disabled={isSystemRole}
223+
scope={scope}
227224
/>
228225
))}
229226
</div>
@@ -247,6 +244,7 @@ export function RoleSheet({ role, open, onOpenChange, teamId, scope = Scope.Orga
247244
permissions={permissions}
248245
onChange={(newPermissions) => form.setValue("permissions", newPermissions)}
249246
disabled={isSystemRole}
247+
scope={scope}
250248
/>
251249
))}
252250
</div>

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/SimplePermissionItem.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use client";
22

3-
import type { Resource } from "@calcom/features/pbac/domain/types/permission-registry";
3+
import type { Resource, Scope } from "@calcom/features/pbac/domain/types/permission-registry";
44
import { PERMISSION_REGISTRY } from "@calcom/features/pbac/domain/types/permission-registry";
55
import { useLocale } from "@calcom/lib/hooks/useLocale";
66
import { ToggleGroup } from "@calcom/ui/components/form";
@@ -13,16 +13,18 @@ interface SimplePermissionItemProps {
1313
permissions: string[];
1414
onChange: (permissions: string[]) => void;
1515
disabled?: boolean;
16+
scope?: Scope;
1617
}
1718

1819
export function SimplePermissionItem({
1920
resource,
2021
permissions,
2122
onChange,
2223
disabled,
24+
scope,
2325
}: SimplePermissionItemProps) {
2426
const { t } = useLocale();
25-
const { getResourcePermissionLevel, toggleResourcePermissionLevel } = usePermissions();
27+
const { getResourcePermissionLevel, toggleResourcePermissionLevel } = usePermissions(scope);
2628

2729
const isAllResources = resource === "*";
2830
const options = isAllResources

‎apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import { CrudAction } from "@calcom/features/pbac/domain/types/permission-registry";
2-
import { PERMISSION_REGISTRY } from "@calcom/features/pbac/domain/types/permission-registry";
1+
import { CrudAction, Scope } from "@calcom/features/pbac/domain/types/permission-registry";
2+
import {
3+
PERMISSION_REGISTRY,
4+
getPermissionsForScope,
5+
} from "@calcom/features/pbac/domain/types/permission-registry";
36
import {
47
getTransitiveDependencies,
58
getTransitiveDependents,
@@ -18,10 +21,11 @@ interface UsePermissionsReturn {
1821
toggleSinglePermission: (permission: string, enabled: boolean, currentPermissions: string[]) => string[];
1922
}
2023

21-
export function usePermissions(): UsePermissionsReturn {
24+
export function usePermissions(scope: Scope = Scope.Organization): UsePermissionsReturn {
2225
const getAllPossiblePermissions = () => {
2326
const permissions: string[] = [];
24-
Object.entries(PERMISSION_REGISTRY).forEach(([resource, config]) => {
27+
const scopedRegistry = getPermissionsForScope(scope);
28+
Object.entries(scopedRegistry).forEach(([resource, config]) => {
2529
if (resource !== "*") {
2630
Object.keys(config)
2731
.filter((action) => !action.startsWith("_"))
@@ -34,7 +38,8 @@ export function usePermissions(): UsePermissionsReturn {
3438
};
3539

3640
const hasAllPermissions = (permissions: string[]) => {
37-
return Object.entries(PERMISSION_REGISTRY).every(([resource, config]) => {
41+
const scopedRegistry = getPermissionsForScope(scope);
42+
return Object.entries(scopedRegistry).every(([resource, config]) => {
3843
if (resource === "*") return true;
3944
return Object.keys(config)
4045
.filter((action) => !action.startsWith("_"))
@@ -85,7 +90,8 @@ export function usePermissions(): UsePermissionsReturn {
8590
} else {
8691
// Filter out current resource permissions
8792
newPermissions = newPermissions.filter((p) => !p.startsWith(`${resource}.`));
88-
const resourceConfig = PERMISSION_REGISTRY[resource as keyof typeof PERMISSION_REGISTRY];
93+
const scopedRegistry = getPermissionsForScope(scope);
94+
const resourceConfig = scopedRegistry[resource as keyof typeof scopedRegistry];
8995

9096
if (!resourceConfig) return currentPermissions;
9197

0 commit comments

Comments
 (0)