diff --git a/apps/server/src/shared-helpers/asset-public-access.ts b/apps/server/src/shared-helpers/asset-public-access.ts index 516868b9a..8cea67818 100644 --- a/apps/server/src/shared-helpers/asset-public-access.ts +++ b/apps/server/src/shared-helpers/asset-public-access.ts @@ -79,7 +79,7 @@ export const checkIfAssetIsEditable = async ({ assetType: AssetType; organizationId: string; workspaceSharing: WorkspaceSharing; - requiredRole?: AssetPermissionRole | AssetPermissionRole[]; + requiredRole?: AssetPermissionRole; }) => { const assetPermissionResult = await checkPermission({ userId: user.id, diff --git a/packages/access-controls/src/assets/cache.ts b/packages/access-controls/src/assets/cache.ts index ba069231f..7164f5938 100644 --- a/packages/access-controls/src/assets/cache.ts +++ b/packages/access-controls/src/assets/cache.ts @@ -33,11 +33,9 @@ export function getPermissionCacheKey( userId: string, assetId: string, assetType: AssetType, - requiredRole: AssetPermissionRole | AssetPermissionRole[] + requiredRole: AssetPermissionRole ): CacheKey { - // Normalize and sort roles for consistent cache keys - const roles = Array.isArray(requiredRole) ? [...requiredRole].sort() : [requiredRole]; - return `${userId}:${assetId}:${assetType}:${roles.join(',')}`; + return `${userId}:${assetId}:${assetType}:${requiredRole}`; } /** @@ -47,7 +45,7 @@ export function getCachedPermission( userId: string, assetId: string, assetType: AssetType, - requiredRole: AssetPermissionRole | AssetPermissionRole[] + requiredRole: AssetPermissionRole ): AssetPermissionResult | undefined { const key = getPermissionCacheKey(userId, assetId, assetType, requiredRole); const cached = permissionCache.get(key); @@ -68,7 +66,7 @@ export function setCachedPermission( userId: string, assetId: string, assetType: AssetType, - requiredRole: AssetPermissionRole | AssetPermissionRole[], + requiredRole: AssetPermissionRole, result: AssetPermissionResult ): void { const key = getPermissionCacheKey(userId, assetId, assetType, requiredRole); @@ -198,13 +196,18 @@ export function invalidateUser(userId: string) { * Invalidate all cached entries for a user-asset combination */ export function invalidateUserAsset(userId: string, assetId: string, assetType: AssetType) { - // Invalidate all permission cache entries for this user-asset combination - // Since we now support arrays, we need to invalidate all possible combinations - // The simplest approach is to invalidate all entries containing the user-asset-type pattern - for (const key of Array.from(permissionCache.keys())) { - if (key.startsWith(`${userId}:${assetId}:${assetType}:`)) { - permissionCache.delete(key); - } + // Invalidate all permission levels for this user-asset combination + const permissionRoles: AssetPermissionRole[] = [ + 'owner', + 'full_access', + 'can_edit', + 'can_filter', + 'can_view', + ]; + + for (const role of permissionRoles) { + const key = getPermissionCacheKey(userId, assetId, assetType, role); + permissionCache.delete(key); } // Invalidate cascading cache diff --git a/packages/access-controls/src/assets/checks.test.ts b/packages/access-controls/src/assets/checks.test.ts index 703d2d656..abe407f8f 100644 --- a/packages/access-controls/src/assets/checks.test.ts +++ b/packages/access-controls/src/assets/checks.test.ts @@ -205,7 +205,7 @@ describe('Asset Permission Checks', () => { 'user123', 'asset123', 'dashboard_file', - ['can_view'], + 'can_view', { hasAccess: false } ); }); diff --git a/packages/access-controls/src/assets/checks.ts b/packages/access-controls/src/assets/checks.ts index e368dc146..94a7147f9 100644 --- a/packages/access-controls/src/assets/checks.ts +++ b/packages/access-controls/src/assets/checks.ts @@ -6,7 +6,7 @@ import { import type { User } from '@buster/database/queries'; import type { AssetType } from '@buster/database/schema-types'; import type { AssetPermissionRole, OrganizationMembership, WorkspaceSharing } from '../types'; -import { getHighestPermission, isPermissionSufficientForAny } from '../types/asset-permissions'; +import { getHighestPermission, isPermissionSufficient } from '../types/asset-permissions'; import { getCachedPermission, setCachedPermission } from './cache'; import { checkCascadingPermissions } from './cascading-permissions'; @@ -14,7 +14,7 @@ export interface AssetPermissionCheck { userId: string; assetId: string; assetType: AssetType; - requiredRole: AssetPermissionRole | AssetPermissionRole[]; + requiredRole: AssetPermissionRole; organizationId?: string; workspaceSharing?: WorkspaceSharing; } @@ -31,12 +31,8 @@ export interface AssetPermissionResult { export async function checkPermission(check: AssetPermissionCheck): Promise { const { userId, assetId, assetType, requiredRole, organizationId, workspaceSharing } = check; - // Normalize requiredRole to an array for consistent handling - const requiredRoles = Array.isArray(requiredRole) ? requiredRole : [requiredRole]; - - // Check cache first (using serialized array as key) - const cached = getCachedPermission(userId, assetId, assetType, requiredRoles); - + // Check cache first + const cached = getCachedPermission(userId, assetId, assetType, requiredRole); if (cached !== undefined) { return cached; } @@ -60,8 +56,8 @@ export async function checkPermission(check: AssetPermissionCheck): Promise = { id: userId }; const hasCascadingAccess = await checkCascadingPermissions(assetId, assetType, user as User); @@ -104,13 +99,13 @@ export async function checkPermission(check: AssetPermissionCheck): Promise {