vynte-infra/calendar
2
0
Fork 0
Code Issues Pull Requests Actions 2.2k Packages Projects Releases Wiki Activity

fix: issues found in PBAC creation (#22659)

Browse Source
This commit is contained in:
sean-brydon
2025-07-22 10:01:41 +01:00
committed by GitHub
parent cb78692347
commit 4e1048b221
5 changed files with 98 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx
+19 -3
View File
@@ -184,9 +184,11 @@ const organizationAdminKeys = [
const useTabs = ({
isDelegationCredentialEnabled,
isPbacEnabled,
canViewRoles,
}: {
isDelegationCredentialEnabled: boolean;
isPbacEnabled: boolean;
canViewRoles?: boolean;
}) => {
const session = useSession();
const { data: user } = trpc.viewer.me.get.useQuery({ includePasswordAdded: true });
@@ -223,8 +225,9 @@ const useTabs = ({
});
}
// Add pbac menu item only if feature flag is enabled
if (isPbacEnabled) {
// Add pbac menu item only if feature flag is enabled AND user has permission to view roles
// This prevents showing the menu item when user has no organization permissions
if (isPbacEnabled && canViewRoles) {
newArray.push({
name: "roles_and_permissions",
href: "/settings/organizations/roles",
@@ -291,6 +294,7 @@ interface SettingsSidebarContainerProps {
navigationIsOpenedOnMobile?: boolean;
bannersHeight?: number;
teamFeatures?: Record<number, TeamFeatures>;
canViewRoles?: boolean;
}
const TeamRolesNavItem = ({
@@ -483,6 +487,7 @@ const SettingsSidebarContainer = ({
navigationIsOpenedOnMobile,
bannersHeight,
teamFeatures,
canViewRoles,
}: SettingsSidebarContainerProps) => {
const searchParams = useCompatSearchParams();
const orgBranding = useOrgBranding();
@@ -512,6 +517,7 @@ const SettingsSidebarContainer = ({
const tabsWithPermissions = useTabs({
isDelegationCredentialEnabled,
isPbacEnabled,
canViewRoles,
});
const { data: otherTeams } = trpc.viewer.organizations.listOtherTeams.useQuery(undefined, {
@@ -786,9 +792,15 @@ export type SettingsLayoutProps = {
children: React.ReactNode;
containerClassName?: string;
teamFeatures?: Record<number, TeamFeatures>;
canViewRoles?: boolean;
} & ComponentProps<typeof Shell>;
export default function SettingsLayoutAppDirClient({ children, teamFeatures, ...rest }: SettingsLayoutProps) {
export default function SettingsLayoutAppDirClient({
children,
teamFeatures,
canViewRoles,
...rest
}: SettingsLayoutProps) {
const pathname = usePathname();
const state = useState(false);
const [sideContainerOpen, setSideContainerOpen] = state;
@@ -821,6 +833,7 @@ export default function SettingsLayoutAppDirClient({ children, teamFeatures, ...
sideContainerOpen={sideContainerOpen}
setSideContainerOpen={setSideContainerOpen}
teamFeatures={teamFeatures}
canViewRoles={canViewRoles}
/>
}
drawerState={state}
@@ -843,6 +856,7 @@ type SidebarContainerElementProps = {
bannersHeight?: number;
setSideContainerOpen: React.Dispatch<React.SetStateAction<boolean>>;
teamFeatures?: Record<number, TeamFeatures>;
canViewRoles?: boolean;
};
const SidebarContainerElement = ({
@@ -850,6 +864,7 @@ const SidebarContainerElement = ({
bannersHeight,
setSideContainerOpen,
teamFeatures,
canViewRoles,
}: SidebarContainerElementProps) => {
const { t } = useLocale();
return (
@@ -866,6 +881,7 @@ const SidebarContainerElement = ({
navigationIsOpenedOnMobile={sideContainerOpen}
bannersHeight={bannersHeight}
teamFeatures={teamFeatures}
canViewRoles={canViewRoles}
/>
</>
);
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx
+24 -2
View File
@@ -6,6 +6,9 @@ import React from "react";
import { getServerSession } from "@calcom/features/auth/lib/getServerSession";
import type { TeamFeatures } from "@calcom/features/flags/config";
import { FeaturesRepository } from "@calcom/features/flags/features.repository";
import { PermissionMapper } from "@calcom/features/pbac/domain/mappers/PermissionMapper";
import { Resource, CrudAction } from "@calcom/features/pbac/domain/types/permission-registry";
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import { buildLegacyRequest } from "@lib/buildLegacyCtx";
@@ -23,6 +26,15 @@ const getTeamFeatures = unstable_cache(
}
);
const getCachedResourcePermissions = unstable_cache(
async (userId: number, teamId: number, resource: Resource) => {
const permissionService = new PermissionCheckService();
return permissionService.getResourcePermissions({ userId, teamId, resource });
},
["resource-permissions"],
{ revalidate: 120 }
);
export default async function SettingsLayoutAppDir(props: SettingsLayoutProps) {
const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) });
const userId = session?.user?.id;
@@ -31,20 +43,30 @@ export default async function SettingsLayoutAppDir(props: SettingsLayoutProps) {
}
let teamFeatures: Record<number, TeamFeatures> | null = null;
let canViewRoles = false;
const orgId = session?.user?.profile?.organizationId ?? session?.user.org?.id;
// For now we only grab organization features but it would be nice to fetch these on the server side for specific team feature flags
if (orgId) {
const features = await getTeamFeatures(orgId);
const [features, rolePermissions] = await Promise.all([
getTeamFeatures(orgId),
getCachedResourcePermissions(userId, orgId, Resource.Role),
]);
if (features) {
teamFeatures = {
[orgId]: features,
};
// Check if user has permission to read roles
const roleActions = PermissionMapper.toActionMap(rolePermissions, Resource.Role);
canViewRoles = roleActions[CrudAction.Read] ?? false;
}
}
return (
<>
<SettingsLayoutAppDirClient {...props} teamFeatures={teamFeatures ?? {}} />
<SettingsLayoutAppDirClient {...props} teamFeatures={teamFeatures ?? {}} canViewRoles={canViewRoles} />
</>
);
}
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/AdvancedPermissionGroup.tsx
+30 -11
View File
@@ -54,6 +54,12 @@ export function AdvancedPermissionGroup({
}
};
const handleCheckedChange = (checked: boolean | string) => {
if (!disabled) {
onChange(toggleResourcePermissionLevel(resource, checked ? "all" : "none", selectedPermissions));
}
};
// Helper function to check if read permission is auto-enabled
const isReadAutoEnabled = (action: string) => {
if (action === CrudAction.Read) return false;
@@ -68,25 +74,38 @@ export function AdvancedPermissionGroup({
<button
type="button"
className="flex cursor-pointer items-center justify-between gap-1.5 p-4"
onClick={() => setIsExpanded(!isExpanded)}>
<Icon
name={isAllResources ? "chevron-right" : "chevron-down"}
className={classNames(
"h-4 w-4 transition-transform",
isExpanded && !isAllResources ? "rotate-180" : ""
)}
/>
onClick={(e) => {
// Only toggle expansion if clicking on the button itself, not child elements
if (e.target === e.currentTarget) {
setIsExpanded(!isExpanded);
}
}}>
<div className="flex items-center gap-1.5" onClick={() => setIsExpanded(!isExpanded)}>
<Icon
name="chevron-right"
className={classNames(
"h-4 w-4 transition-transform",
isExpanded && !isAllResources ? "rotate-90" : ""
)}
/>
</div>
<div className="flex items-center gap-2">
<Checkbox
checked={isAllSelected}
onCheckedChange={() => handleToggleAll}
onCheckedChange={handleCheckedChange}
onClick={handleToggleAll}
disabled={disabled}
/>
<span className="text-default text-sm font-medium leading-none">
<span
className="text-default cursor-pointer text-sm font-medium leading-none"
onClick={() => setIsExpanded(!isExpanded)}>
{t(resourceConfig._resource?.i18nKey || "")}
</span>
<span className="text-muted text-sm font-medium leading-none">{t("all_permissions")}</span>
<span
className="text-muted cursor-pointer text-sm font-medium leading-none"
onClick={() => setIsExpanded(!isExpanded)}>
{t("all_permissions")}
</span>
</div>
</button>
{isExpanded && !isAllResources && (
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts
+19 -7
View File
@@ -19,9 +19,11 @@ export function usePermissions(): UsePermissionsReturn {
const permissions: string[] = [];
Object.entries(PERMISSION_REGISTRY).forEach(([resource, config]) => {
if (resource !== "*") {
Object.keys(config).forEach((action) => {
permissions.push(`${resource}.${action}`);
});
Object.keys(config)
.filter((action) => !action.startsWith("_"))
.forEach((action) => {
permissions.push(`${resource}.${action}`);
});
}
});
return permissions;
@@ -30,7 +32,9 @@ export function usePermissions(): UsePermissionsReturn {
const hasAllPermissions = (permissions: string[]) => {
return Object.entries(PERMISSION_REGISTRY).every(([resource, config]) => {
if (resource === "*") return true;
return Object.keys(config).every((action) => permissions.includes(`${resource}.${action}`));
return Object.keys(config)
.filter((action) => !action.startsWith("_"))
.every((action) => permissions.includes(`${resource}.${action}`));
});
};
@@ -42,7 +46,10 @@ export function usePermissions(): UsePermissionsReturn {
const resourceConfig = PERMISSION_REGISTRY[resource as keyof typeof PERMISSION_REGISTRY];
if (!resourceConfig) return "none";
const allResourcePerms = Object.keys(resourceConfig).map((action) => `${resource}.${action}`);
// Filter out internal keys like _resource when checking permissions
const allResourcePerms = Object.keys(resourceConfig)
.filter((action) => !action.startsWith("_"))
.map((action) => `${resource}.${action}`);
const hasAllPerms = allResourcePerms.every((p) => permissions.includes(p));
const hasReadPerm = permissions.includes(`${resource}.${CrudAction.Read}`);
@@ -73,6 +80,9 @@ export function usePermissions(): UsePermissionsReturn {
if (!resourceConfig) return currentPermissions;
// Declare variable before switch to avoid scope issues
let allResourcePerms: string[];
switch (level) {
case "none":
// No permissions to add, just keep other permissions
@@ -82,8 +92,10 @@ export function usePermissions(): UsePermissionsReturn {
newPermissions.push(`${resource}.${CrudAction.Read}`);
break;
case "all":
// Add all permissions for this resource
const allResourcePerms = Object.keys(resourceConfig).map((action) => `${resource}.${action}`);
// Add all permissions for this resource (excluding internal keys)
allResourcePerms = Object.keys(resourceConfig)
.filter((action) => !action.startsWith("_"))
.map((action) => `${resource}.${action}`);
newPermissions.push(...allResourcePerms);
break;
}
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/actions.ts
+6 -1
View File
@@ -9,7 +9,12 @@ export async function revalidateTeamRoles(teamId: number) {
// Revalidate team roles paths (dynamic routes)
revalidatePath("/settings/teams/[id]/roles", "page");
// Invalidate team-specific cache tags
// Invalidate cache tags that match the unstable_cache keys
revalidateTag("team-roles");
revalidateTag("resource-permissions");
revalidateTag("team-feature");
// Also invalidate team-specific cache tags for completeness
revalidateTag(`team-roles-${teamId}`);
revalidateTag(`resource-permissions-${teamId}`);
revalidateTag(`team-members-${teamId}`);