fix(ui): invalid permissions passed to group and named tab sub-fields (#9366)

Fixes #9363

This fixes the following issues that caused fields to be either hidden,
or incorrectly set to readOnly in certain configurations:
- In some cases, permissions were sanitized incorrectly. This PR
rewrites the sanitizePermissions function and adds new unit tests
- after a document save, the client was receiving unsanitized
permissions. Moving the sanitization logic to the endpoint fixes this
- Various incorrect handling of permissions in our form state endpoints
/ RenderFields

Author: AlessioGr

packages/graphql/src/resolvers/collections/docAccess.ts

@@ -1,4 +1,9 @@
-import type { Collection, CollectionPermission, GlobalPermission, PayloadRequest } from 'payload'
+import type {
+  Collection,
+  PayloadRequest,
+  SanitizedCollectionPermission,
+  SanitizedGlobalPermission,
+} from 'payload'
 
 import { docAccessOperation, isolateObjectProperty } from 'payload'
 
@@ -12,7 +17,7 @@ export type Resolver = (
   context: {
     req: PayloadRequest
   },
-) => Promise<CollectionPermission | GlobalPermission>
+) => Promise<SanitizedCollectionPermission | SanitizedGlobalPermission>
 
 export function docAccessResolver(collection: Collection): Resolver {
   async function resolver(_, args, context: Context) {

packages/graphql/src/resolvers/globals/docAccess.ts

@@ -1,8 +1,8 @@
 import type {
-  CollectionPermission,
-  GlobalPermission,
   PayloadRequest,
+  SanitizedCollectionPermission,
   SanitizedGlobalConfig,
+  SanitizedGlobalPermission,
 } from 'payload'
 
 import { docAccessOperationGlobal, isolateObjectProperty } from 'payload'
@@ -14,7 +14,7 @@ export type Resolver = (
   context: {
     req: PayloadRequest
   },
-) => Promise<CollectionPermission | GlobalPermission>
+) => Promise<SanitizedCollectionPermission | SanitizedGlobalPermission>
 
 export function docAccessResolver(global: SanitizedGlobalConfig): Resolver {
   async function resolver(_, context: Context) {

packages/next/src/views/Document/getDocumentPermissions.tsx

@@ -1,6 +1,5 @@
 import type {
   Data,
-  DocumentPermissions,
   PayloadRequest,
   SanitizedCollectionConfig,
   SanitizedDocumentPermissions,
@@ -11,7 +10,7 @@ import {
   hasSavePermission as getHasSavePermission,
   isEditing as getIsEditing,
 } from '@payloadcms/ui/shared'
-import { docAccessOperation, docAccessOperationGlobal, sanitizePermissions } from 'payload'
+import { docAccessOperation, docAccessOperationGlobal } from 'payload'
 
 export const getDocumentPermissions = async (args: {
   collectionConfig?: SanitizedCollectionConfig
@@ -26,7 +25,7 @@ export const getDocumentPermissions = async (args: {
 }> => {
   const { id, collectionConfig, data = {}, globalConfig, req } = args
 
-  let docPermissions: DocumentPermissions
+  let docPermissions: SanitizedDocumentPermissions
   let hasPublishPermission = false
 
   if (collectionConfig) {
@@ -58,7 +57,7 @@ export const getDocumentPermissions = async (args: {
               _status: 'published',
             },
           },
-        }).then(({ update }) => update?.permission)
+        }).then((permissions) => permissions.update)
       }
     } catch (error) {
       req.payload.logger.error(error)
@@ -85,20 +84,16 @@ export const getDocumentPermissions = async (args: {
               _status: 'published',
             },
           },
-        }).then(({ update }) => update?.permission)
+        }).then((permissions) => permissions.update)
       }
     } catch (error) {
       req.payload.logger.error(error)
     }
   }
 
-  // TODO: do this in a better way. Only doing this bc this is how the fn was written (mutates the original object)
-  const sanitizedDocPermissions = { ...docPermissions } as any as SanitizedDocumentPermissions
-  sanitizePermissions(sanitizedDocPermissions)
-
   const hasSavePermission = getHasSavePermission({
     collectionSlug: collectionConfig?.slug,
-    docPermissions: sanitizedDocPermissions,
+    docPermissions,
     globalSlug: globalConfig?.slug,
     isEditing: getIsEditing({
       id,
@@ -108,7 +103,7 @@ export const getDocumentPermissions = async (args: {
   })
 
   return {
-    docPermissions: sanitizedDocPermissions,
+    docPermissions,
     hasPublishPermission,
     hasSavePermission,
   }

packages/payload/src/auth/types.ts

@@ -11,61 +11,61 @@ export type Permission = {
   where?: Where
 }
 
-export type FieldPermissions = {
-  blocks?: {
-    [blockSlug: string]: {
-      create: {
-        permission: boolean
-      }
-      fields: {
-        [fieldName: string]: FieldPermissions
-      }
-      read: {
-        permission: boolean
-      }
-      update: {
-        permission: boolean
-      }
+export type FieldsPermissions = {
+  [fieldName: string]: FieldPermissions
+}
+
+export type BlockPermissions = {
+  create: Permission
+  fields: FieldsPermissions
+  read: Permission
+  update: Permission
+}
+
+export type SanitizedBlockPermissions =
+  | {
+      fields: SanitizedFieldsPermissions
     }
-  }
-  create: {
-    permission: boolean
-  }
-  fields?: {
-    [fieldName: string]: FieldPermissions
-  }
-  read: {
-    permission: boolean
-  }
-  update: {
-    permission: boolean
-  }
+  | true
+
+export type BlocksPermissions = {
+  [blockSlug: string]: BlockPermissions
+}
+
+export type SanitizedBlocksPermissions =
+  | {
+      [blockSlug: string]: SanitizedBlockPermissions
+    }
+  | true
+
+export type FieldPermissions = {
+  blocks?: BlocksPermissions
+  create: Permission
+  fields?: FieldsPermissions
+  read: Permission
+  update: Permission
 }
 
 export type SanitizedFieldPermissions =
   | {
-      blocks?: {
-        [blockSlug: string]: {
-          fields: {
-            [fieldName: string]: SanitizedFieldPermissions
-          }
-        }
-      }
+      blocks?: SanitizedBlocksPermissions
       create: true
-      fields?: {
-        [fieldName: string]: SanitizedFieldPermissions
-      }
+      fields?: SanitizedFieldsPermissions
       read: true
       update: true
     }
   | true
 
+export type SanitizedFieldsPermissions =
+  | {
+      [fieldName: string]: SanitizedFieldPermissions
+    }
+  | true
+
 export type CollectionPermission = {
   create: Permission
   delete: Permission
-  fields: {
-    [fieldName: string]: FieldPermissions
-  }
+  fields: FieldsPermissions
   read: Permission
   readVersions?: Permission
   update: Permission
@@ -74,31 +74,21 @@ export type CollectionPermission = {
 export type SanitizedCollectionPermission = {
   create?: true
   delete?: true
-  fields:
-    | {
-        [fieldName: string]: SanitizedFieldPermissions
-      }
-    | true
+  fields: SanitizedFieldsPermissions
   read?: true
   readVersions?: true
   update?: true
 }
 
 export type GlobalPermission = {
-  fields: {
-    [fieldName: string]: FieldPermissions
-  }
+  fields: FieldsPermissions
   read: Permission
   readVersions?: Permission
   update: Permission
 }
 
 export type SanitizedGlobalPermission = {
-  fields:
-    | {
-        [fieldName: string]: SanitizedFieldPermissions
-      }
-    | true
+  fields: SanitizedFieldsPermissions
   read?: true
   readVersions?: true
   update?: true
@@ -110,7 +100,7 @@ export type SanitizedDocumentPermissions = SanitizedCollectionPermission | Sanit
 
 export type Permissions = {
   canAccessAdmin: boolean
-  collections: {
+  collections?: {
     [collectionSlug: CollectionSlug]: CollectionPermission
   }
   globals?: {
@@ -121,26 +111,10 @@ export type Permissions = {
 export type SanitizedPermissions = {
   canAccessAdmin?: boolean
   collections?: {
-    [collectionSlug: string]: {
-      create?: true
-      delete?: true
-      fields: {
-        [fieldName: string]: SanitizedFieldPermissions
-      }
-      read?: true
-      readVersions?: true
-      update?: true
-    }
+    [collectionSlug: string]: SanitizedCollectionPermission
   }
   globals?: {
-    [globalSlug: string]: {
-      fields: {
-        [fieldName: string]: SanitizedFieldPermissions
-      }
-      read?: true
-      readVersions?: true
-      update?: true
-    }
+    [globalSlug: string]: SanitizedGlobalPermission
   }
 }
 

packages/payload/src/collections/operations/docAccess.ts

@@ -1,9 +1,10 @@
-import type { CollectionPermission } from '../../auth/index.js'
+import type { CollectionPermission, SanitizedCollectionPermission } from '../../auth/index.js'
 import type { AllOperations, PayloadRequest } from '../../types/index.js'
 import type { Collection } from '../config/types.js'
 
 import { getEntityPolicies } from '../../utilities/getEntityPolicies.js'
 import { killTransaction } from '../../utilities/killTransaction.js'
+import { sanitizePermissions } from '../../utilities/sanitizePermissions.js'
 
 const allOperations: AllOperations[] = ['create', 'read', 'update', 'delete']
 
@@ -13,7 +14,7 @@ type Arguments = {
   req: PayloadRequest
 }
 
-export async function docAccessOperation(args: Arguments): Promise<CollectionPermission> {
+export async function docAccessOperation(args: Arguments): Promise<SanitizedCollectionPermission> {
   const {
     id,
     collection: { config },
@@ -43,7 +44,14 @@ export async function docAccessOperation(args: Arguments): Promise<CollectionPer
       req,
     })
 
-    return result
+    const sanitizedPermissions = sanitizePermissions({
+      canAccessAdmin: true,
+      collections: {
+        [config.slug]: result,
+      },
+    })
+
+    return sanitizedPermissions.collections[config.slug]
   } catch (e: unknown) {
     await killTransaction(req)
     throw e

packages/payload/src/globals/operations/docAccess.ts

@@ -1,18 +1,19 @@
-import type { GlobalPermission } from '../../auth/index.js'
+import type { SanitizedGlobalPermission } from '../../auth/index.js'
 import type { AllOperations, PayloadRequest } from '../../types/index.js'
 import type { SanitizedGlobalConfig } from '../config/types.js'
 
 import { commitTransaction } from '../../utilities/commitTransaction.js'
 import { getEntityPolicies } from '../../utilities/getEntityPolicies.js'
 import { initTransaction } from '../../utilities/initTransaction.js'
 import { killTransaction } from '../../utilities/killTransaction.js'
+import { sanitizePermissions } from '../../utilities/sanitizePermissions.js'
 
 type Arguments = {
   globalConfig: SanitizedGlobalConfig
   req: PayloadRequest
 }
 
-export const docAccessOperation = async (args: Arguments): Promise<GlobalPermission> => {
+export const docAccessOperation = async (args: Arguments): Promise<SanitizedGlobalPermission> => {
   const { globalConfig, req } = args
 
   const globalOperations: AllOperations[] = ['read', 'update']
@@ -32,7 +33,14 @@ export const docAccessOperation = async (args: Arguments): Promise<GlobalPermiss
     if (shouldCommit) {
       await commitTransaction(req)
     }
-    return result
+    const sanitizedPermissions = sanitizePermissions({
+      canAccessAdmin: true,
+      globals: {
+        [globalConfig.slug]: result,
+      },
+    })
+
+    return sanitizedPermissions.globals[globalConfig.slug]
   } catch (e: unknown) {
     await killTransaction(req)
     throw e

packages/payload/src/index.ts

@@ -1341,7 +1341,6 @@ export { isValidID } from './utilities/isValidID.js'
 export { killTransaction } from './utilities/killTransaction.js'
 export { mapAsync } from './utilities/mapAsync.js'
 export { sanitizeFallbackLocale } from './utilities/sanitizeFallbackLocale.js'
-export { recursivelySanitizePermissions as sanitizePermissions } from './utilities/sanitizePermissions.js'
 export { traverseFields } from './utilities/traverseFields.js'
 export type { TraverseFieldsCallback } from './utilities/traverseFields.js'
 export { buildVersionCollectionFields } from './versions/buildCollectionFields.js'