From 34b62f7516b04293ac6e09ab8ecf896f8d750d47 Mon Sep 17 00:00:00 2001 From: Eunjae Lee Date: Mon, 15 Sep 2025 10:55:51 +0200 Subject: [PATCH] feat: implement generalized navigation permission system with PBAC (#23706) * feat: implement generalized navigation permission system with PBAC - Create NavigationPermissionsProvider context for server-side permission data - Add checkNavigationPermissions utility using PermissionCheckService - Update main navigation layout to check permissions server-side - Enhance useShouldDisplayNavigationItem for generalized permission filtering - Support permission mapping for insights, workflows, routing, teams, members - Use unstable_cache for performance optimization - Replace insights-specific logic with scalable PBAC-based system Co-Authored-By: eunjae@cal.com * fix: correct flag check logic in useShouldDisplayNavigationItem - Only return false when flag is explicitly false - Allow navigation permission checks to run when flags are truthy - Fixes bug where truthy flags would short-circuit permission checks Co-Authored-By: eunjae@cal.com * fix: resolve linting errors in embed-core files - Replace forbidden non-null assertion with null check in embed.ts - Replace 'any' type with 'unknown' in embed.test.ts - Replace non-null assertion with proper null check in EmbedElement.test.ts - Remove unused variable declaration These are pre-existing linting issues unrelated to navigation permissions feature. Co-Authored-By: eunjae@cal.com * fix: resolve server-client boundary issue in NavigationPermissionsProvider - Add 'use client' directive to NavigationPermissionsProvider.ts - Replace JSX syntax with React.createElement to fix TypeScript compilation - Create NavigationPermissionsWrapper.tsx as client component bridge - Update layout.tsx to use wrapper instead of provider directly - Ensure proper separation between server-side permission checking and client-side context consumption This fixes the architectural issue where useContext was being used in server components. Co-Authored-By: eunjae@cal.com * Revert "fix: resolve linting errors in embed-core files" This reverts commit 4f481bae576376c9f9177215f7872c8bf7132894. * refactor: simplify NavigationPermissionsProvider approach - Remove NavigationPermissionsWrapper.tsx (revert to simpler approach) - Rename NavigationPermissionsProvider.ts to .tsx for JSX support - Update layout.tsx to use NavigationPermissionsProvider directly - Keep 'use client' directive for proper server-client boundary handling - Maintain all navigation permission functionality with cleaner architecture Co-Authored-By: eunjae@cal.com * clean up * return true for no team situation * fix: prevent hidden navigation items from reappearing via stored expansion state - Modified usePersistedExpansionState hook to accept shouldDisplay parameter - Clear stored expansion state for items that shouldn't be displayed - Prevent state persistence for items without permissions - Updated both NavigationItem and MobileNavigationMoreItem components - Use safe sessionStorage import from @calcom/lib/webstorage Fixes bug where clicking 'Workflows' would restore 'Insights' menu visibility even when user lacks insights permissions due to sessionStorage state restoration. Co-Authored-By: eunjae@cal.com * fix: add sessionStorage to webstorage module and export NavigationItemName type - Added sessionStorage wrapper to @calcom/lib/webstorage with safe error handling - Export NavigationItemName type from NavigationPermissionsProvider for proper type access - Resolves TypeScript compilation errors in navigation permission system These changes enable the collapsible menu state bug fix to compile properly. Co-Authored-By: eunjae@cal.com * refactor: improve separation of concerns in usePersistedExpansionState hook - Remove shouldDisplay parameter from usePersistedExpansionState hook - Move state clearing logic for hidden items to NavigationItem component level - Add setIsExpanded to useEffect dependency arrays to fix ESLint warnings - Maintain bug fix functionality while improving code design - Better separation between expansion state management and display permissions This addresses user feedback about inappropriate coupling between concerns. Co-Authored-By: eunjae@cal.com * fix provider * fix unstable_cache * refactor: move navigation permission checks to client-side with tRPC - Replace server-side unstable_cache permission checks with client-side tRPC query - Add getNavigationPermissions query to PBAC router - Update NavigationPermissionsProvider to use tRPC with loading states - Remove server-side checkNavigationPermissions function - Improve initial page load performance by deferring permission checks Co-Authored-By: eunjae@cal.com * refactor: centralize navigation item definitions to eliminate repetition - Create NAVIGATION_ITEMS_CONFIG as single source of truth for navigation items - Auto-generate NAVIGATION_PERMISSION_MAP from centralized config - Update Navigation.tsx to use centralized definitions for teams, routing, workflows, insights - Fix import issues in NavigationPermissionsProvider.tsx - Eliminate hardcoded repetition of navigation item properties across codebase Co-Authored-By: eunjae@cal.com * clean up * clean up permissions --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- apps/web/app/(use-page-wrapper)/layout.tsx | 6 +- .../shell/navigation/NavigationItem.tsx | 1 + .../useShouldDisplayNavigationItem.ts | 19 +++++- .../NavigationPermissionsProvider.tsx | 65 +++++++++++++++++++ packages/features/shell/permissions/types.ts | 19 ++++++ packages/lib/server/repository/membership.ts | 2 +- packages/lib/webstorage.ts | 32 +++++++++ .../server/routers/viewer/pbac/_router.tsx | 30 +++++++++ 8 files changed, 170 insertions(+), 4 deletions(-) create mode 100644 packages/features/shell/permissions/NavigationPermissionsProvider.tsx create mode 100644 packages/features/shell/permissions/types.ts diff --git a/apps/web/app/(use-page-wrapper)/layout.tsx b/apps/web/app/(use-page-wrapper)/layout.tsx index 878635c237..cecdb5b371 100644 --- a/apps/web/app/(use-page-wrapper)/layout.tsx +++ b/apps/web/app/(use-page-wrapper)/layout.tsx @@ -1,6 +1,8 @@ import { headers } from "next/headers"; import Script from "next/script"; +import { NavigationPermissionsProvider } from "@calcom/features/shell/permissions/NavigationPermissionsProvider"; + import PageWrapper from "@components/PageWrapperAppDir"; export default async function PageWrapperLayout({ children }: { children: React.ReactNode }) { @@ -21,7 +23,7 @@ export default async function PageWrapperLayout({ children }: { children: React. ].filter((script): script is { id: string; script: string } => !!script.script); return ( - <> + {children} {scripts.map((script) => ( @@ -36,6 +38,6 @@ export default async function PageWrapperLayout({ children }: { children: React. /> ))} - + ); } diff --git a/packages/features/shell/navigation/NavigationItem.tsx b/packages/features/shell/navigation/NavigationItem.tsx index 7b59c7ed0f..72affdd75b 100644 --- a/packages/features/shell/navigation/NavigationItem.tsx +++ b/packages/features/shell/navigation/NavigationItem.tsx @@ -4,6 +4,7 @@ import React, { Fragment, useState, useEffect } from "react"; import { useLocale } from "@calcom/lib/hooks/useLocale"; import useMediaQuery from "@calcom/lib/hooks/useMediaQuery"; +import { sessionStorage } from "@calcom/lib/webstorage"; import classNames from "@calcom/ui/classNames"; import { Badge } from "@calcom/ui/components/badge"; import { Icon } from "@calcom/ui/components/icon"; diff --git a/packages/features/shell/navigation/useShouldDisplayNavigationItem.ts b/packages/features/shell/navigation/useShouldDisplayNavigationItem.ts index ffba4b37c8..75ed5ee181 100644 --- a/packages/features/shell/navigation/useShouldDisplayNavigationItem.ts +++ b/packages/features/shell/navigation/useShouldDisplayNavigationItem.ts @@ -1,10 +1,27 @@ import { useFlagMap } from "@calcom/features/flags/context/provider"; import { isKeyInObject } from "@calcom/lib/isKeyInObject"; +import { + useNavigationPermissions, + type NavigationItemName, +} from "../permissions/NavigationPermissionsProvider"; import type { NavigationItemType } from "./NavigationItem"; export function useShouldDisplayNavigationItem(item: NavigationItemType) { const flags = useFlagMap(); - if (isKeyInObject(item.name, flags)) return flags[item.name]; + const { permissions: navigationPermissions, isLoading } = useNavigationPermissions(); + + if (isKeyInObject(item.name, flags) && flags[item.name] === false) { + return false; + } + + if (isLoading) { + return true; + } + + if (item.name in navigationPermissions) { + return navigationPermissions[item.name as NavigationItemName]; + } + return true; } diff --git a/packages/features/shell/permissions/NavigationPermissionsProvider.tsx b/packages/features/shell/permissions/NavigationPermissionsProvider.tsx new file mode 100644 index 0000000000..d452fc8fcf --- /dev/null +++ b/packages/features/shell/permissions/NavigationPermissionsProvider.tsx @@ -0,0 +1,65 @@ +"use client"; + +import React, { createContext, useContext } from "react"; + +import { trpc } from "@calcom/trpc/react"; + +import type { NavigationItemName, NavigationPermissions } from "./types"; +import { DEFAULT_PERMISSIONS } from "./types"; + +export type { NavigationItemName, NavigationPermissions }; + +/** + * Context for navigation permissions + */ +const NavigationPermissionsContext = createContext<{ + permissions: NavigationPermissions; + isLoading: boolean; +} | null>(null); + +/** + * Hook to access navigation permissions from context + * @returns NavigationPermissions object with boolean flags for each navigation item + */ +export function useNavigationPermissions(): { permissions: NavigationPermissions; isLoading: boolean } { + const context = useContext(NavigationPermissionsContext); + if (context === null) { + return { + permissions: DEFAULT_PERMISSIONS, + isLoading: false, + }; + } + return context; +} + +/** + * Hook to check if a specific navigation item should be displayed + * @param itemName The navigation item name to check + * @returns boolean indicating if the item should be displayed + */ +export function useNavigationPermission(itemName: NavigationItemName): boolean { + const { permissions } = useNavigationPermissions(); + return permissions[itemName]; +} + +/** + * Provider component for navigation permissions + * Fetches permissions client-side using tRPC + */ +export function NavigationPermissionsProvider({ children }: { children: React.ReactNode }) { + const { data: permissions, isLoading } = trpc.viewer.pbac.getNavigationPermissions.useQuery(undefined, { + staleTime: 5 * 60 * 1000, + refetchOnWindowFocus: false, + }); + + const contextValue = { + permissions: permissions || DEFAULT_PERMISSIONS, + isLoading, + }; + + return ( + + {children} + + ); +} diff --git a/packages/features/shell/permissions/types.ts b/packages/features/shell/permissions/types.ts new file mode 100644 index 0000000000..a24ea8dec8 --- /dev/null +++ b/packages/features/shell/permissions/types.ts @@ -0,0 +1,19 @@ +/** + * Navigation permission mapping for menu items + */ +export const NAVIGATION_PERMISSION_MAP = { + members: "organization.listMembers", + teams: "team.read", + insights: "insights.read", +} as const; + +export const DEFAULT_PERMISSIONS = Object.fromEntries( + Object.keys(NAVIGATION_PERMISSION_MAP).map((key) => [key, true]) +) as NavigationPermissions; + +export type NavigationItemName = keyof typeof NAVIGATION_PERMISSION_MAP; + +/** + * Navigation permissions object containing boolean flags for each navigation item + */ +export type NavigationPermissions = Record; diff --git a/packages/lib/server/repository/membership.ts b/packages/lib/server/repository/membership.ts index 6063f14136..9850ddccb1 100644 --- a/packages/lib/server/repository/membership.ts +++ b/packages/lib/server/repository/membership.ts @@ -1,7 +1,7 @@ import { availabilityUserSelect, prisma, type PrismaTransaction } from "@calcom/prisma"; -import { MembershipRole } from "@calcom/prisma/enums"; import type { Prisma, Membership, PrismaClient } from "@calcom/prisma/client"; +import { MembershipRole } from "@calcom/prisma/enums"; import { credentialForCalendarServiceSelect } from "@calcom/prisma/selects/credential"; import logger from "../../logger"; diff --git a/packages/lib/webstorage.ts b/packages/lib/webstorage.ts index c703bfda05..9522799179 100644 --- a/packages/lib/webstorage.ts +++ b/packages/lib/webstorage.ts @@ -34,3 +34,35 @@ export const localStorage = { } }, }; + +export const sessionStorage = { + getItem(key: string) { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + return window.sessionStorage.getItem(key); + } catch (e) { + // In case storage is restricted. Possible reasons + // 1. Third Party Context in Chrome Incognito mode. + return null; + } + }, + setItem(key: string, value: string) { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + window.sessionStorage.setItem(key, value); + } catch (e) { + // In case storage is restricted. Possible reasons + // 1. Third Party Context in Chrome Incognito mode. + // 2. Storage limit reached + return; + } + }, + removeItem: (key: string) => { + try { + // eslint-disable-next-line @calcom/eslint/avoid-web-storage + window.sessionStorage.removeItem(key); + } catch (e) { + return; + } + }, +}; diff --git a/packages/trpc/server/routers/viewer/pbac/_router.tsx b/packages/trpc/server/routers/viewer/pbac/_router.tsx index c3c07b8e09..b314d39263 100644 --- a/packages/trpc/server/routers/viewer/pbac/_router.tsx +++ b/packages/trpc/server/routers/viewer/pbac/_router.tsx @@ -5,6 +5,12 @@ import { isValidPermissionString } from "@calcom/features/pbac/domain/types/perm import type { PermissionString } from "@calcom/features/pbac/domain/types/permission-registry"; import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service"; import { RoleService } from "@calcom/features/pbac/services/role.service"; +import { + NAVIGATION_PERMISSION_MAP, + DEFAULT_PERMISSIONS, + type NavigationItemName, +} from "@calcom/features/shell/permissions/types"; +import { MembershipRepository } from "@calcom/lib/server/repository/membership"; import prisma from "@calcom/prisma"; import { RoleType, MembershipRole } from "@calcom/prisma/enums"; @@ -33,6 +39,30 @@ export const permissionsRouter = router({ return await permissionCheckService.getUserPermissions(ctx.user.id); }), + getNavigationPermissions: authedProcedure.query(async ({ ctx }) => { + if (!ctx.user?.id) { + return DEFAULT_PERMISSIONS; + } + + const teamIds = await MembershipRepository.findUserTeamIds({ userId: ctx.user.id }); + if (teamIds.length === 0) { + return DEFAULT_PERMISSIONS; + } + + const permissionService = new PermissionCheckService(); + const navigationItems = Object.keys(NAVIGATION_PERMISSION_MAP) as Array; + + const permissionChecks = await Promise.all( + navigationItems.map((itemName) => + permissionService.getTeamIdsWithPermission(ctx.user.id, NAVIGATION_PERMISSION_MAP[itemName]) + ) + ); + + return Object.fromEntries( + navigationItems.map((itemName, index) => [itemName, permissionChecks[index].length > 0]) + ) as Record; + }), + checkPermission: authedProcedure .input( z.object({