tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

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

Browse Source 
Fixes https://github.com/payloadcms/payload/issues/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
This commit is contained in:
Alessio Gravili
2024-11-20 13:03:35 -07:00
committed by GitHub
parent 5db7e1e864
commit c67291d538
23 changed files with 2051 additions and 284 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
packages/graphql/src/resolvers/collections/docAccess.ts
View File

@@ -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) {

6
packages/graphql/src/resolvers/globals/docAccess.ts
View File

@@ -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) {

17
packages/next/src/views/Document/getDocumentPermissions.tsx
View File

@@ -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,
}

120
packages/payload/src/auth/types.ts
View File

@@ -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
}
}

14
packages/payload/src/collections/operations/docAccess.ts
View File

@@ -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

14
packages/payload/src/globals/operations/docAccess.ts
View File

@@ -1,4 +1,4 @@
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'
@@ -6,13 +6,14 @@ 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

1
packages/payload/src/index.ts
View File

@@ -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'

1105
packages/payload/src/utilities/sanitizePermissions.spec.ts
View File

File diff suppressed because it is too large Load Diff

245
packages/payload/src/utilities/sanitizePermissions.ts
View File

@@ -1,61 +1,147 @@
import type { Permissions, SanitizedPermissions } from '../auth/types.js'
import type {
CollectionPermission,
FieldPermissions,
FieldsPermissions,
GlobalPermission,
Permissions,
SanitizedBlocksPermissions,
SanitizedFieldPermissions,
SanitizedFieldsPermissions,
SanitizedPermissions,
} from '../auth/types.js'
type PermissionObject = {
[key: string]: any
function checkAndSanitizeFieldsPermssions(data: FieldsPermissions): boolean {
let allFieldPermissionsTrue = true
for (const key in data) {
if (typeof data[key] === 'object') {
if (!checkAndSanitizePermissions(data[key])) {
allFieldPermissionsTrue = false
} else {
;(data[key] as unknown as SanitizedFieldPermissions) = true
}
} else if (data[key] !== true) {
allFieldPermissionsTrue = false
}
}
// If all values are true or it's an empty object, return true
return allFieldPermissionsTrue
}
/**
* Check if all permissions in a FieldPermissions object are true on the condition that no nested blocks or fields are present.
* Check if all permissions in a FieldPermissions, CollectionPermission or GlobalPermission object are true.
* If nested fields or blocks are present, the function will recursively check those as well.
*/
function areAllPermissionsTrue(data: PermissionObject): boolean {
if (data.blocks) {
for (const key in data.blocks) {
if (typeof data.blocks[key] === 'object') {
// If any recursive call returns false, the whole function returns false
if (key === 'fields' && !areAllPermissionsTrue(data.blocks[key])) {
return false
function checkAndSanitizePermissions(
data: CollectionPermission | FieldPermissions | GlobalPermission,
): boolean {
/**
* Check blocks permissions
*/
let blocksPermissions = true
if ('blocks' in data && data.blocks) {
for (const blockSlug in data.blocks) {
if (typeof data.blocks[blockSlug] === 'object') {
for (const key in data.blocks[blockSlug]) {
/**
* Check fields in nested blocks
*/
if (key === 'fields') {
if (data.blocks[blockSlug].fields) {
if (!checkAndSanitizeFieldsPermssions(data.blocks[blockSlug].fields)) {
blocksPermissions = false
} else {
;(data.blocks[blockSlug].fields as unknown as SanitizedFieldsPermissions) = true
}
}
} else {
if (typeof data.blocks[blockSlug][key] === 'object') {
/**
* Check Permissions in nested blocks
*/
if (isPermissionObject(data.blocks[blockSlug][key])) {
if (
data.blocks[blockSlug][key]['permission'] === true &&
!('where' in data.blocks[blockSlug][key])
) {
// If the permission is true and there is no where clause, set the key to true
data.blocks[blockSlug][key] = true
continue
} else if (
data.blocks[blockSlug][key]['permission'] === true &&
'where' in data.blocks[blockSlug][key]
) {
// otherwise do nothing so we can keep the where clause
blocksPermissions = false
} else {
blocksPermissions = false
data.blocks[blockSlug][key] = false
delete data.blocks[blockSlug][key]
continue
}
} else {
throw new Error('Unexpected object in block permissions')
}
}
}
}
if (data.blocks[key].fields && !areAllPermissionsTrue(data.blocks[key].fields)) {
return false
}
} else if (data.blocks[key] !== true) {
} else if (data.blocks[blockSlug] !== true) {
// If any value is not true, return false
return false
blocksPermissions = false
delete data.blocks[blockSlug]
}
}
// If all values are true or it's an empty object, return true
return true
if (blocksPermissions) {
;(data.blocks as unknown as SanitizedBlocksPermissions) = true
}
}
/**
* Check nested Fields permissions
*/
let fieldsPermissions = true
if (data.fields) {
for (const key in data.fields) {
if (typeof data.fields[key] === 'object') {
// If any recursive call returns false, the whole function returns false
if (!areAllPermissionsTrue(data.fields[key])) {
return false
}
} else if (data.fields[key] !== true) {
// If any value is not true, return false
return false
}
if (!checkAndSanitizeFieldsPermssions(data.fields)) {
fieldsPermissions = false
} else {
;(data.fields as unknown as SanitizedFieldsPermissions) = true
}
// If all values are true or it's an empty object, return true
return true
}
/**
* Check other Permissions objects (e.g. read, write)
*/
let otherPermissions = true
for (const key in data) {
if (key === 'fields' || key === 'blocks') {
continue
}
if (typeof data[key] === 'object') {
// If any recursive call returns false, the whole function returns false
if (!areAllPermissionsTrue(data[key])) {
return false
if (isPermissionObject(data[key])) {
if (data[key]['permission'] === true && !('where' in data[key])) {
// If the permission is true and there is no where clause, set the key to true
data[key] = true
continue
} else if (data[key]['permission'] === true && 'where' in data[key]) {
// otherwise do nothing so we can keep the where clause
otherPermissions = false
} else {
otherPermissions = false
data[key] = false
delete data[key]
continue
}
} else {
throw new Error('Unexpected object in fields permissions')
}
} else if (data[key] !== true) {
// If any value is not true, return false
return false
otherPermissions = false
}
}
// If all values are true or it's an empty object, return true
return true
return fieldsPermissions && blocksPermissions && otherPermissions
}
/**
@@ -83,84 +169,27 @@ function cleanEmptyObjects(obj: any): void {
})
}
/**
* Recursively resolve permissions in an object.
*/
export function recursivelySanitizePermissions(obj: PermissionObject): void {
export function recursivelySanitizeCollections(obj: Permissions['collections']): void {
if (typeof obj !== 'object') {
return
}
const entries = Object.entries(obj)
const collectionPermissions = Object.values(obj)
for (let i = 0; i < entries.length; i++) {
const [key, value] = entries[i]
// Check if it's a 'fields' key
if (key === 'fields') {
// Check if fields is empty
if (Object.keys(obj[key]).length === 0) {
delete obj[key]
continue
}
// Otherwise set fields to true if all permissions are true
else if (areAllPermissionsTrue(value)) {
obj[key] = true
continue
}
} else if (key === 'blocks') {
// Check if fields is empty
if (Object.keys(obj[key]).length === 0) {
delete obj[key]
continue
}
// Otherwise set fields to true if all permissions are true
else if (areAllPermissionsTrue(value)) {
obj[key] = true
continue
}
}
for (const collectionPermission of collectionPermissions) {
checkAndSanitizePermissions(collectionPermission)
}
}
// Check if the whole object is a permission object
const isFullPermissionObject = Object.keys(value).every(
(subKey) =>
subKey !== 'blocks' &&
typeof value?.[subKey] === 'object' &&
'permission' in value[subKey] &&
!('where' in value[subKey]) &&
typeof value[subKey]['permission'] === 'boolean',
)
export function recursivelySanitizeGlobals(obj: Permissions['globals']): void {
if (typeof obj !== 'object') {
return
}
if (isFullPermissionObject) {
if (areAllPermissionsTrue(value)) {
obj[key] = true
continue
} else {
for (const subKey in value) {
if (value[subKey]['permission'] === true && !('where' in value[subKey])) {
value[subKey] = true
continue
} else if (value[subKey]['permission'] === true && 'where' in value[subKey]) {
// do nothing
} else {
delete value[subKey]
continue
}
}
}
} else if (isPermissionObject(value)) {
if (value['permission'] === true && !('where' in value)) {
// If the permission is true and there is no where clause, set the key to true
obj[key] = true
continue
} else if (value['permission'] === true && 'where' in value) {
// otherwise do nothing so we can keep the where clause
} else {
delete obj[key]
continue
}
} else {
recursivelySanitizePermissions(value)
}
const globalPermissions = Object.values(obj)
for (const globalPermission of globalPermissions) {
checkAndSanitizePermissions(globalPermission)
}
}
@@ -173,11 +202,11 @@ export function sanitizePermissions(data: Permissions): SanitizedPermissions {
}
if (data.collections) {
recursivelySanitizePermissions(data.collections)
recursivelySanitizeCollections(data.collections)
}
if (data.globals) {
recursivelySanitizePermissions(data.globals)
recursivelySanitizeGlobals(data.globals)
}
// Run clean up of empty objects at the end

27
packages/ui/src/fields/Blocks/BlockRow.tsx
View File

@@ -1,10 +1,18 @@
'use client'
import type { ClientBlock, ClientField, Labels, Row, SanitizedFieldPermissions } from 'payload'
import type {
ClientBlock,
ClientField,
Labels,
Row,
SanitizedFieldPermissions,
SanitizedFieldsPermissions,
} from 'payload'
import { getTranslation } from '@payloadcms/translations'
import React from 'react'
import type { UseDraggableSortableReturn } from '../../elements/DraggableSortable/useDraggableSortable/types.js'
import type { RenderFieldsProps } from '../../forms/RenderFields/types.js'
import { Collapsible } from '../../elements/Collapsible/index.js'
import { ErrorPill } from '../../elements/ErrorPill/index.js'
@@ -80,6 +88,19 @@ export const BlockRow: React.FC<BlocksFieldProps> = ({
.filter(Boolean)
.join(' ')
let blockPermissions: RenderFieldsProps['permissions'] = undefined
if (permissions === true) {
blockPermissions = true
} else {
const permissionsBlockSpecific = permissions?.blocks?.[block.slug]
if (permissionsBlockSpecific === true) {
blockPermissions = true
} else {
blockPermissions = permissionsBlockSpecific?.fields
}
}
return (
<div
id={`${parentPath?.split('.').join('-')}-row-${rowIndex}`}
@@ -147,9 +168,7 @@ export const BlockRow: React.FC<BlocksFieldProps> = ({
parentIndexPath=""
parentPath={path}
parentSchemaPath={schemaPath}
permissions={
permissions === true ? permissions : permissions?.blocks?.[block.slug]?.fields
}
permissions={blockPermissions}
readOnly={readOnly}
/>
</Collapsible>

14
packages/ui/src/forms/RenderFields/index.tsx
View File

@@ -3,7 +3,7 @@
import { getFieldPaths } from 'payload/shared'
import React from 'react'
import type { Props } from './types.js'
import type { RenderFieldsProps } from './types.js'
import { RenderIfInViewport } from '../../elements/RenderIfInViewport/index.js'
import { useOperation } from '../../providers/Operation/index.js'
@@ -12,9 +12,9 @@ import { RenderField } from './RenderField.js'
const baseClass = 'render-fields'
export { Props }
export { RenderFieldsProps as Props }
export const RenderFields: React.FC<Props> = (props) => {
export const RenderFields: React.FC<RenderFieldsProps> = (props) => {
const {
className,
fields,
@@ -49,10 +49,15 @@ export const RenderFields: React.FC<Props> = (props) => {
return null
}
const parentName = parentPath?.includes('.')
? parentPath.split('.')[parentPath.split('.').length - 1]
: parentPath
// If the user cannot read the field, then filter it out
// This is different from `admin.readOnly` which is executed based on `operation`
const hasReadPermission =
permissions === true ||
permissions?.[parentName] === true ||
('name' in field &&
typeof permissions === 'object' &&
permissions?.[field.name] &&
@@ -74,6 +79,7 @@ export const RenderFields: React.FC<Props> = (props) => {
// If the user does not have access control to begin with, force it to be read-only
const hasOperationPermission =
permissions === true ||
permissions?.[parentName] === true ||
('name' in field &&
typeof permissions === 'object' &&
permissions?.[field.name] &&
@@ -102,7 +108,7 @@ export const RenderFields: React.FC<Props> = (props) => {
parentSchemaPath={parentSchemaPath}
path={path}
permissions={
permissions === null || permissions === true
permissions === undefined || permissions === null || permissions === true
? true
: 'name' in field
? permissions?.[field.name]

2
packages/ui/src/forms/RenderFields/types.ts
View File

@@ -1,6 +1,6 @@
import type { ClientField, SanitizedFieldPermissions } from 'payload'
export type Props = {
export type RenderFieldsProps = {
readonly className?: string
readonly fields: ClientField[]
/**

53
packages/ui/src/forms/fieldSchemasToFormState/addFieldStatePromise.ts
View File

@@ -8,10 +8,12 @@ import type {
FormStateWithoutComponents,
PayloadRequest,
SanitizedFieldPermissions,
SanitizedFieldsPermissions,
} from 'payload'
import ObjectIdImport from 'bson-objectid'
import {
deepCopyObjectSimple,
fieldAffectsData,
fieldHasSubFields,
fieldIsSidebar,
@@ -59,14 +61,9 @@ export type AddFieldStatePromiseArgs = {
operation: 'create' | 'update'
parentIndexPath: string
parentPath: string
parentPermissions: SanitizedFieldsPermissions
parentSchemaPath: string
passesCondition: boolean
permissions:
| {
[fieldName: string]: SanitizedFieldPermissions
}
| null
| SanitizedFieldPermissions
preferences: DocumentPreferences
previousFormState: FormState
renderAllFields: boolean
@@ -109,9 +106,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
operation,
parentIndexPath,
parentPath,
parentPermissions,
parentSchemaPath,
passesCondition,
permissions,
preferences,
previousFormState,
renderAllFields,
@@ -135,10 +132,15 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
const isHiddenField = 'hidden' in field && field?.hidden
const disabledFromAdmin = field?.admin && 'disabled' in field.admin && field.admin.disabled
let fieldPermissions: SanitizedFieldPermissions = true
if (fieldAffectsData(field) && !(isHiddenField || disabledFromAdmin)) {
const fieldPermissions = permissions === true ? permissions : permissions?.[field.name]
fieldPermissions =
parentPermissions === true
? parentPermissions
: deepCopyObjectSimple(parentPermissions?.[field.name])
let hasPermission: boolean = fieldPermissions === true || fieldPermissions?.read
let hasPermission: boolean =
fieldPermissions === true || deepCopyObjectSimple(fieldPermissions?.read)
if (typeof field?.access?.read === 'function') {
hasPermission = await field.access.read({ doc: fullData, req, siblingData: data })
@@ -385,7 +387,9 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
permissions:
fieldPermissions === true
? fieldPermissions
: permissions?.[field.name]?.blocks?.[block.slug]?.fields || {},
: parentPermissions?.[field.name]?.blocks?.[block.slug] === true
? true
: parentPermissions?.[field.name]?.blocks?.[block.slug]?.fields || {},
preferences,
previousFormState,
renderAllFields: requiresRender,
@@ -470,7 +474,8 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
parentPassesCondition: passesCondition,
parentPath: path,
parentSchemaPath: schemaPath,
permissions: fieldPermissions ?? permissions?.[field.name]?.fields ?? {},
permissions:
typeof fieldPermissions === 'boolean' ? fieldPermissions : fieldPermissions?.fields,
preferences,
previousFormState,
renderAllFields,
@@ -614,7 +619,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
parentPassesCondition: passesCondition,
parentPath,
parentSchemaPath,
permissions,
permissions: parentPermissions, // TODO: Verify this is correct
preferences,
previousFormState,
renderAllFields,
@@ -643,6 +648,22 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
parentSchemaPath,
})
let childPermissions: SanitizedFieldsPermissions = undefined
if (tabHasName(tab)) {
if (parentPermissions === true) {
childPermissions = true
} else {
const tabPermissions = parentPermissions?.[tab.name]
if (tabPermissions === true) {
childPermissions = true
} else {
childPermissions = tabPermissions?.fields
}
}
} else {
childPermissions = parentPermissions
}
return iterateFields({
id,
addErrorPathToParent: addErrorPathToParentArg,
@@ -661,11 +682,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
parentPassesCondition: passesCondition,
parentPath: tabHasName(tab) ? tabPath : parentPath,
parentSchemaPath: tabHasName(tab) ? tabSchemaPath : parentSchemaPath,
permissions: tabHasName(tab)
? typeof permissions === 'boolean'
? permissions
: permissions?.[tab.name] || {}
: permissions,
permissions: childPermissions,
preferences,
previousFormState,
renderAllFields,
@@ -727,7 +744,7 @@ export const addFieldStatePromise = async (args: AddFieldStatePromiseArgs): Prom
parentPath,
parentSchemaPath,
path,
permissions,
permissions: fieldPermissions,
preferences,
previousFieldState: previousFormState?.[path],
req,

4
packages/ui/src/forms/fieldSchemasToFormState/index.tsx
View File

@@ -6,7 +6,7 @@ import type {
FormState,
FormStateWithoutComponents,
PayloadRequest,
SanitizedDocumentPermissions,
SanitizedFieldsPermissions,
} from 'payload'
import type { RenderFieldMethod } from './types.js'
@@ -26,7 +26,7 @@ type Args = {
fieldSchemaMap: FieldSchemaMap | undefined
id?: number | string
operation?: 'create' | 'update'
permissions: SanitizedDocumentPermissions['fields']
permissions: SanitizedFieldsPermissions
preferences: DocumentPreferences
/**
* Optionally accept the previous form state,

11
packages/ui/src/forms/fieldSchemasToFormState/iterateFields.ts
View File

@@ -6,7 +6,7 @@ import type {
FormState,
FormStateWithoutComponents,
PayloadRequest,
SanitizedFieldPermissions,
SanitizedFieldsPermissions,
} from 'payload'
import type { AddFieldStatePromiseArgs } from './addFieldStatePromise.js'
@@ -47,12 +47,7 @@ type Args = {
parentPassesCondition?: boolean
parentPath: string
parentSchemaPath: string
permissions:
| {
[fieldName: string]: SanitizedFieldPermissions
}
| null
| SanitizedFieldPermissions
permissions: SanitizedFieldsPermissions
preferences?: DocumentPreferences
previousFormState: FormState
renderAllFields: boolean
@@ -130,9 +125,9 @@ export const iterateFields = async ({
operation,
parentIndexPath,
parentPath,
parentPermissions: permissions,
parentSchemaPath,
passesCondition,
permissions,
preferences,
previousFormState,
renderAllFields,

23
packages/ui/src/forms/fieldSchemasToFormState/renderField.tsx
View File

@@ -1,14 +1,7 @@
import type {
ClientComponentProps,
ClientField,
FieldPaths,
SanitizedFieldPermissions,
ServerComponentProps,
} from 'payload'
import type { ClientComponentProps, ClientField, FieldPaths, ServerComponentProps } from 'payload'
import { getTranslation } from '@payloadcms/translations'
import { createClientField, deepCopyObjectSimple, MissingEditorProp } from 'payload'
import { fieldAffectsData } from 'payload/shared'
import type { RenderFieldMethod } from './types.js'
@@ -36,15 +29,12 @@ export const renderField: RenderFieldMethod = ({
parentPath,
parentSchemaPath,
path,
permissions: incomingPermissions,
permissions,
preferences,
req,
schemaPath,
siblingData,
}) => {
// TODO (ALESSIO): why are we passing the fieldConfig twice?
// and especially, why are we deepCopyObject -here- instead of inside the createClientField func,
// so no one screws this up in the future?
const clientField = createClientField({
clientField: deepCopyObjectSimple(fieldConfig) as ClientField,
defaultIDType: req.payload.config.db.defaultIDType,
@@ -53,18 +43,11 @@ export const renderField: RenderFieldMethod = ({
importMap: req.payload.importMap,
})
const permissions =
incomingPermissions === true
? true
: fieldAffectsData(fieldConfig)
? incomingPermissions?.[fieldConfig.name]
: ({} as SanitizedFieldPermissions)
const clientProps: ClientComponentProps & Partial<FieldPaths> = {
customComponents: fieldState?.customComponents || {},
field: clientField,
path,
readOnly: permissions !== true && !permissions?.[operation],
readOnly: typeof permissions === 'boolean' ? !permissions : !permissions?.[operation],
schemaPath,
}

7
packages/ui/src/forms/fieldSchemasToFormState/types.ts
View File

@@ -23,12 +23,7 @@ export type RenderFieldArgs = {
parentPath: string
parentSchemaPath: string
path: string
permissions:
| {
[fieldName: string]: SanitizedFieldPermissions
}
| null
| SanitizedFieldPermissions
permissions: SanitizedFieldPermissions
preferences: DocumentPreferences
previousFieldState: FieldState
req: PayloadRequest

126
test/access-control/collections/Regression-1/index.ts Normal file
View File

@@ -0,0 +1,126 @@
import type { CollectionConfig } from 'payload'
import { lexicalEditor } from '@payloadcms/richtext-lexical'
export const Regression1: CollectionConfig = {
slug: 'regression1',
access: {
create: () => false,
read: () => true,
},
fields: [
{
name: 'group1',
type: 'group',
fields: [
{
name: 'richText1',
type: 'richText',
editor: lexicalEditor(),
},
{
name: 'text',
type: 'text',
},
],
},
{
type: 'tabs',
tabs: [
{
name: 'tab1',
fields: [
{
name: 'richText2',
type: 'richText',
editor: lexicalEditor(),
},
{
name: 'blocks2',
type: 'blocks',
blocks: [
{
slug: 'myBlock',
fields: [
{
name: 'richText3',
type: 'richText',
editor: lexicalEditor(),
},
],
},
],
},
],
},
{
label: 'tab2',
fields: [
{
name: 'richText4',
type: 'richText',
editor: lexicalEditor(),
},
{
name: 'blocks3',
type: 'blocks',
blocks: [
{
slug: 'myBlock2',
fields: [
{
name: 'richText5',
type: 'richText',
editor: lexicalEditor(),
},
],
},
],
},
],
},
],
},
{
name: 'array',
type: 'array',
fields: [
{
name: 'art',
type: 'richText',
editor: lexicalEditor(),
},
],
},
{
name: 'arrayWithAccessFalse',
type: 'array',
access: {
update: () => false,
},
fields: [
{
name: 'richText6',
type: 'richText',
editor: lexicalEditor(),
},
],
},
{
name: 'blocks',
type: 'blocks',
blocks: [
{
slug: 'myBlock3',
fields: [
{
name: 'richText7',
type: 'richText',
editor: lexicalEditor(),
},
],
},
],
},
],
}

38
test/access-control/collections/Regression-2/index.ts Normal file
View File

@@ -0,0 +1,38 @@
import type { CollectionConfig } from 'payload'
import { lexicalEditor } from '@payloadcms/richtext-lexical'
export const Regression2: CollectionConfig = {
slug: 'regression2',
fields: [
{
name: 'group',
type: 'group',
fields: [
{
name: 'richText1',
type: 'richText',
editor: lexicalEditor(),
},
{
name: 'text',
type: 'text',
},
],
},
{
name: 'array',
type: 'array',
access: {
update: () => false,
},
fields: [
{
name: 'richText2',
type: 'richText',
editor: lexicalEditor(),
},
],
},
],
}

57
test/access-control/config.ts
View File

@@ -8,7 +8,10 @@ import type { Config, User } from './payload-types.js'
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
import { devUser } from '../credentials.js'
import { textToLexicalJSON } from '../fields/collections/LexicalLocalized/textToLexicalJSON.js'
import { Disabled } from './collections/Disabled/index.js'
import { Regression1 } from './collections/Regression-1/index.js'
import { Regression2 } from './collections/Regression-2/index.js'
import { RichText } from './collections/RichText/index.js'
import {
createNotUpdateCollectionSlug,
@@ -508,6 +511,8 @@ export default buildConfigWithDefaults({
},
Disabled,
RichText,
Regression1,
Regression2,
],
globals: [
{
@@ -645,6 +650,58 @@ export default buildConfigWithDefaults({
name: 'dev@payloadcms.com',
},
})
await payload.create({
collection: 'regression1',
data: {
richText4: textToLexicalJSON({ text: 'Text1' }),
array: [{ art: textToLexicalJSON({ text: 'Text2' }) }],
arrayWithAccessFalse: [{ richText6: textToLexicalJSON({ text: 'Text3' }) }],
group1: {
text: 'Text4',
richText1: textToLexicalJSON({ text: 'Text5' }),
},
blocks: [
{
blockType: 'myBlock3',
richText7: textToLexicalJSON({ text: 'Text6' }),
blockName: 'My Block 1',
},
],
blocks3: [
{
blockType: 'myBlock2',
richText5: textToLexicalJSON({ text: 'Text7' }),
blockName: 'My Block 2',
},
],
tab1: {
richText2: textToLexicalJSON({ text: 'Text8' }),
blocks2: [
{
blockType: 'myBlock',
richText3: textToLexicalJSON({ text: 'Text9' }),
blockName: 'My Block 3',
},
],
},
},
})
await payload.create({
collection: 'regression2',
data: {
array: [
{
richText2: textToLexicalJSON({ text: 'Text1' }),
},
],
group: {
text: 'Text2',
richText1: textToLexicalJSON({ text: 'Text3' }),
},
},
})
},
typescript: {
outputFile: path.resolve(dirname, 'payload-types.ts'),

126
test/access-control/e2e.spec.ts
View File

@@ -169,6 +169,132 @@ describe('access control', () => {
}),
).toHaveCount(1)
})
const ensureRegression1FieldsHaveCorrectAccess = async () => {
await expect(
page.locator('#field-group1 .rich-text-lexical .ContentEditable__root'),
).toBeVisible()
// Wait until the contenteditable is editable
await expect(
page.locator('#field-group1 .rich-text-lexical .ContentEditable__root'),
).toBeEditable()
await expect(async () => {
const isAttached = page.locator('#field-group1 .rich-text-lexical--read-only')
await expect(isAttached).toBeHidden()
}).toPass({ timeout: 10000, intervals: [100] })
await expect(page.locator('#field-group1 #field-group1__text')).toBeEnabled()
// Click on button with text Tab1
await page.locator('.tabs-field__tab-button').getByText('Tab1').click()
await expect(
page.locator('.tabs-field__tab .rich-text-lexical .ContentEditable__root').first(),
).toBeVisible()
await expect(
page.locator('.tabs-field__tab .rich-text-lexical--read-only').first(),
).not.toBeAttached()
await expect(
page.locator(
'.tabs-field__tab #field-tab1__blocks2 .rich-text-lexical .ContentEditable__root',
),
).toBeVisible()
await expect(
page.locator('.tabs-field__tab #field-tab1__blocks2 .rich-text-lexical--read-only'),
).not.toBeAttached()
await expect(
page.locator('#field-array #array-row-0 .rich-text-lexical .ContentEditable__root'),
).toBeVisible()
await expect(
page.locator('#field-array #array-row-0 .rich-text-lexical--read-only'),
).not.toBeAttached()
await expect(
page.locator(
'#field-arrayWithAccessFalse #arrayWithAccessFalse-row-0 .rich-text-lexical .ContentEditable__root',
),
).toBeVisible()
await expect(
page.locator(
'#field-arrayWithAccessFalse #arrayWithAccessFalse-row-0 .rich-text-lexical--read-only',
),
).toBeVisible()
await expect(
page.locator('#field-blocks .rich-text-lexical .ContentEditable__root'),
).toBeVisible()
await expect(page.locator('#field-blocks.rich-text-lexical--read-only')).not.toBeAttached()
}
/**
* This reproduces a bug where certain fields were incorrectly marked as read-only
*/
// eslint-disable-next-line playwright/expect-expect
test('ensure complex collection config fields show up in correct read-only state', async () => {
const regression1URL = new AdminUrlUtil(serverURL, 'regression1')
await page.goto(regression1URL.list)
// Click on first card
await page.locator('.cell-id a').first().click()
// wait for url
await page.waitForURL(`**/collections/regression1/**`)
await ensureRegression1FieldsHaveCorrectAccess()
// Edit any field
await page.locator('#field-group1__text').fill('test!')
// Save the doc
await saveDocAndAssert(page)
await wait(1000)
// Ensure fields still have the correct readOnly state. When saving the document, permissions are re-evaluated
await ensureRegression1FieldsHaveCorrectAccess()
})
const ensureRegression2FieldsHaveCorrectAccess = async () => {
await expect(
page.locator('#field-group .rich-text-lexical .ContentEditable__root'),
).toBeVisible()
// Wait until the contenteditable is editable
await expect(
page.locator('#field-group .rich-text-lexical .ContentEditable__root'),
).toBeEditable()
await expect(async () => {
const isAttached = page.locator('#field-group .rich-text-lexical--read-only')
await expect(isAttached).toBeHidden()
}).toPass({ timeout: 10000, intervals: [100] })
await expect(page.locator('#field-group #field-group__text')).toBeEnabled()
await expect(
page.locator('#field-array #array-row-0 .rich-text-lexical .ContentEditable__root'),
).toBeVisible()
await expect(
page.locator('#field-array #array-row-0 .rich-text-lexical--read-only'),
).toBeVisible() // => is read-only
}
/**
* This reproduces a bug where certain fields were incorrectly marked as read-only
*/
// eslint-disable-next-line playwright/expect-expect
test('ensure complex collection config fields show up in correct read-only state 2', async () => {
const regression2URL = new AdminUrlUtil(serverURL, 'regression2')
await page.goto(regression2URL.list)
// Click on first card
await page.locator('.cell-id a').first().click()
// wait for url
await page.waitForURL(`**/collections/regression2/**`)
await ensureRegression2FieldsHaveCorrectAccess()
// Edit any field
await page.locator('#field-group__text').fill('test!')
// Save the doc
await saveDocAndAssert(page)
await wait(1000)
// Ensure fields still have the correct readOnly state. When saving the document, permissions are re-evaluated
await ensureRegression2FieldsHaveCorrectAccess()
})
})
describe('collection — fully restricted', () => {

313
test/access-control/payload-types.ts
View File

@@ -30,6 +30,8 @@ export interface Config {
'hidden-access-count': HiddenAccessCount;
disabled: Disabled;
'rich-text': RichText;
regression1: Regression1;
regression2: Regression2;
'payload-locked-documents': PayloadLockedDocument;
'payload-preferences': PayloadPreference;
'payload-migrations': PayloadMigration;
@@ -54,6 +56,8 @@ export interface Config {
'hidden-access-count': HiddenAccessCountSelect<false> | HiddenAccessCountSelect<true>;
disabled: DisabledSelect<false> | DisabledSelect<true>;
'rich-text': RichTextSelect<false> | RichTextSelect<true>;
regression1: Regression1Select<false> | Regression1Select<true>;
regression2: Regression2Select<false> | Regression2Select<true>;
'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
@@ -83,9 +87,9 @@ export interface Config {
| (NonAdminUser & {
collection: 'non-admin-user';
});
jobs?: {
jobs: {
tasks: unknown;
workflows?: unknown;
workflows: unknown;
};
}
export interface UserAuthOperations {
@@ -383,6 +387,218 @@ export interface RichText {
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "regression1".
*/
export interface Regression1 {
id: string;
group1?: {
richText1?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
text?: string | null;
};
tab1?: {
richText2?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
blocks2?:
| {
richText3?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
blockName?: string | null;
blockType: 'myBlock';
}[]
| null;
};
richText4?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
blocks3?:
| {
richText5?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
blockName?: string | null;
blockType: 'myBlock2';
}[]
| null;
array?:
| {
art?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
}[]
| null;
arrayWithAccessFalse?:
| {
richText6?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
}[]
| null;
blocks?:
| {
richText7?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
blockName?: string | null;
blockType: 'myBlock3';
}[]
| null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "regression2".
*/
export interface Regression2 {
id: string;
group?: {
richText1?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
text?: string | null;
};
array?:
| {
richText2?: {
root: {
type: string;
children: {
type: string;
version: number;
[k: string]: unknown;
}[];
direction: ('ltr' | 'rtl') | null;
format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
indent: number;
version: number;
};
[k: string]: unknown;
} | null;
id?: string | null;
}[]
| null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "payload-locked-documents".
@@ -461,6 +677,14 @@ export interface PayloadLockedDocument {
| ({
relationTo: 'rich-text';
value: string | RichText;
} | null)
| ({
relationTo: 'regression1';
value: string | Regression1;
} | null)
| ({
relationTo: 'regression2';
value: string | Regression2;
} | null);
globalSlug?: string | null;
user:
@@ -750,6 +974,91 @@ export interface RichTextSelect<T extends boolean = true> {
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "regression1_select".
*/
export interface Regression1Select<T extends boolean = true> {
group1?:
| T
| {
richText1?: T;
text?: T;
};
tab1?:
| T
| {
richText2?: T;
blocks2?:
| T
| {
myBlock?:
| T
| {
richText3?: T;
id?: T;
blockName?: T;
};
};
};
richText4?: T;
blocks3?:
| T
| {
myBlock2?:
| T
| {
richText5?: T;
id?: T;
blockName?: T;
};
};
array?:
| T
| {
art?: T;
id?: T;
};
arrayWithAccessFalse?:
| T
| {
richText6?: T;
id?: T;
};
blocks?:
| T
| {
myBlock3?:
| T
| {
richText7?: T;
id?: T;
blockName?: T;
};
};
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "regression2_select".
*/
export interface Regression2Select<T extends boolean = true> {
group?:
| T
| {
richText1?: T;
text?: T;
};
array?:
| T
| {
richText2?: T;
id?: T;
};
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "payload-locked-documents_select".

3
test/fields/collections/LexicalLocalized/textToLexicalJSON.ts
View File

@@ -13,7 +13,7 @@ export function textToLexicalJSON({
}: {
lexicalLocalizedRelID?: number | string
text: string
}) {
}): any {
const editorJSON: SerializedEditorState = {
root: {
type: 'root',
@@ -39,6 +39,7 @@ export function textToLexicalJSON({
indent: 0,
textFormat: 0,
type: 'paragraph',
textStyle: '',
version: 1,
} as SerializedParagraphNode,
],