Skip to content

Commit 88cb687

Browse files
authored
fix: blocks access control not respecting update access whether on collection or on a per field basis (#14226)
Fixes #14221 Fixes #13569 Fixes a problem where blocks would have their fields disappear entirely if update permissions were false on the collection or on the blocks field itself. This fixes that and makes sure that fields are always visible and that the blocks fields are correctly being set to readOnly mode if you have permissions to read but not update
1 parent 8136a84 commit 88cb687

File tree

9 files changed

+626
-22
lines changed

9 files changed

+626
-22
lines changed

packages/payload/src/utilities/getEntityPolicies.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,12 @@ const executeFieldPolicies = async ({
247247

248248
await Promise.all(
249249
(field.blockReferences ?? field.blocks).map(async (_block) => {
250-
const block = typeof _block === 'string' ? payload.blocks[_block]! : _block
250+
const block = typeof _block === 'string' ? payload.blocks[_block] : _block
251+
252+
// Skip if block doesn't exist (invalid block reference)
253+
if (!block) {
254+
return
255+
}
251256

252257
if (typeof _block === 'string') {
253258
if (blockPolicies[_block]) {
@@ -264,22 +269,32 @@ const executeFieldPolicies = async ({
264269
// so that any parallel calls will just await this same promise
265270
// instead of re-running executeFieldPolicies.
266271
blockPolicies[_block] = (async () => {
267-
// If the block doesnt exist yet in our mutablePolicies, initialize it
272+
// If the block doesn't exist yet in our mutablePolicies, initialize it
268273
if (!mutablePolicies[field.name].blocks?.[block.slug]) {
274+
// Use field-level permission instead of entityPermission for blocks
275+
// This ensures that if the field has access control, it applies to all blocks in the field
276+
const fieldPermission =
277+
mutablePolicies[field.name][operation]?.permission ?? entityPermission
278+
269279
mutablePolicies[field.name].blocks[block.slug] = {
270280
fields: {},
271-
[operation]: { permission: entityPermission },
281+
[operation]: { permission: fieldPermission },
272282
}
273283
} else if (!mutablePolicies[field.name].blocks[block.slug][operation]) {
284+
// Use field-level permission for consistency
285+
const fieldPermission =
286+
mutablePolicies[field.name][operation]?.permission ?? entityPermission
287+
274288
mutablePolicies[field.name].blocks[block.slug][operation] = {
275-
permission: entityPermission,
289+
permission: fieldPermission,
276290
}
277291
}
278292

279293
await executeFieldPolicies({
280294
blockPolicies,
281295
createAccessPromise,
282-
entityPermission,
296+
entityPermission:
297+
mutablePolicies[field.name][operation]?.permission ?? entityPermission,
283298
fields: block.fields,
284299
operation,
285300
payload,
@@ -296,20 +311,29 @@ const executeFieldPolicies = async ({
296311
}
297312

298313
if (!mutablePolicies[field.name].blocks?.[block.slug]) {
314+
// Use field-level permission instead of entityPermission for blocks
315+
const fieldPermission =
316+
mutablePolicies[field.name][operation]?.permission ?? entityPermission
317+
299318
mutablePolicies[field.name].blocks[block.slug] = {
300319
fields: {},
301-
[operation]: { permission: entityPermission },
320+
[operation]: { permission: fieldPermission },
302321
}
303322
} else if (!mutablePolicies[field.name].blocks[block.slug][operation]) {
323+
// Use field-level permission for consistency
324+
const fieldPermission =
325+
mutablePolicies[field.name][operation]?.permission ?? entityPermission
326+
304327
mutablePolicies[field.name].blocks[block.slug][operation] = {
305-
permission: entityPermission,
328+
permission: fieldPermission,
306329
}
307330
}
308331

309332
await executeFieldPolicies({
310333
blockPolicies,
311334
createAccessPromise,
312-
entityPermission,
335+
entityPermission:
336+
mutablePolicies[field.name][operation]?.permission ?? entityPermission,
313337
fields: block.fields,
314338
operation,
315339
payload,
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { SanitizedDocumentPermissions } from '../auth/types.js'
2+
3+
import { getFieldPermissions } from './getFieldPermissions.js'
4+
5+
describe('getFieldPermissions with collection fallback', () => {
6+
const mockField = {
7+
name: 'testField',
8+
type: 'text' as const,
9+
}
10+
11+
describe('fallback to collection permissions', () => {
12+
it('should enable read-only mode when field permissions are missing but collection has read access', () => {
13+
const fieldPermissions = {} // Empty/sanitized field permissions
14+
const collectionPermissions: SanitizedDocumentPermissions = {
15+
read: true,
16+
fields: {},
17+
}
18+
19+
const result = getFieldPermissions({
20+
field: mockField,
21+
operation: 'update',
22+
parentName: '',
23+
permissions: fieldPermissions,
24+
collectionPermissions,
25+
})
26+
27+
expect(result.read).toBe(true)
28+
expect(result.operation).toBe(false) // Should be read-only
29+
expect(result.permissions).toEqual({ read: true })
30+
})
31+
32+
it('should respect existing field permissions when they exist', () => {
33+
const fieldPermissions = true // All permissions are true
34+
const collectionPermissions: SanitizedDocumentPermissions = {
35+
read: true,
36+
fields: {},
37+
}
38+
39+
const result = getFieldPermissions({
40+
field: mockField,
41+
operation: 'update',
42+
parentName: '',
43+
permissions: fieldPermissions,
44+
})
45+
46+
expect(result.read).toBe(true)
47+
expect(result.operation).toBe(true) // Should have operation permission
48+
expect(result.permissions).toBe(true)
49+
})
50+
51+
it('should not provide access when neither field nor collection has read permission', () => {
52+
const fieldPermissions = {}
53+
const collectionPermissions: SanitizedDocumentPermissions = {
54+
// No read permission at collection level
55+
fields: {},
56+
}
57+
58+
const result = getFieldPermissions({
59+
field: mockField,
60+
operation: 'update',
61+
parentName: '',
62+
permissions: fieldPermissions,
63+
collectionPermissions,
64+
})
65+
66+
expect(result.read).toBe(false)
67+
expect(result.operation).toBe(false)
68+
})
69+
70+
it('should work without collection permissions (backward compatibility)', () => {
71+
const fieldPermissions = true // All permissions
72+
73+
const result = getFieldPermissions({
74+
field: mockField,
75+
operation: 'update',
76+
parentName: '',
77+
permissions: fieldPermissions,
78+
// No collectionPermissions provided
79+
})
80+
81+
expect(result.read).toBe(true)
82+
expect(result.operation).toBe(true)
83+
expect(result.permissions).toBe(true)
84+
})
85+
})
86+
})

packages/payload/src/utilities/getFieldPermissions.ts

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
2-
import type { SanitizedFieldPermissions, SanitizedFieldsPermissions } from '../auth/types.js'
2+
import type {
3+
SanitizedDocumentPermissions,
4+
SanitizedFieldPermissions,
5+
SanitizedFieldsPermissions,
6+
} from '../auth/types.js'
37
import type { ClientField, Field } from '../fields/config/types.js'
48
import type { Operation } from '../types/index.js'
59

@@ -11,11 +15,13 @@ import type { Operation } from '../types/index.js'
1115
* - `read`: Whether the user has permission to read the field.
1216
*/
1317
export const getFieldPermissions = ({
18+
collectionPermissions,
1419
field,
1520
operation,
1621
parentName,
1722
permissions,
1823
}: {
24+
readonly collectionPermissions?: SanitizedDocumentPermissions
1925
readonly field: ClientField | Field
2026
readonly operation: Operation
2127
readonly parentName: string
@@ -28,8 +34,9 @@ export const getFieldPermissions = ({
2834
*/
2935
permissions: SanitizedFieldPermissions | SanitizedFieldsPermissions
3036
read: boolean
31-
} => ({
32-
operation:
37+
} => {
38+
// First, calculate permissions using the existing logic
39+
const fieldOperation =
3340
permissions === true ||
3441
permissions?.[operation as keyof typeof permissions] === true ||
3542
permissions?.[parentName as keyof typeof permissions] === true ||
@@ -38,21 +45,53 @@ export const getFieldPermissions = ({
3845
permissions?.[field.name as keyof typeof permissions] &&
3946
(permissions[field.name as keyof typeof permissions] === true ||
4047
(operation in (permissions as any)[field.name] &&
41-
(permissions as any)[field.name][operation]))),
48+
(permissions as any)[field.name][operation])))
4249

43-
permissions:
50+
const fieldPermissions =
4451
permissions === undefined || permissions === null || permissions === true
4552
? true
4653
: 'name' in field
4754
? (permissions as any)[field.name]
48-
: permissions,
49-
read:
55+
: permissions
56+
57+
const fieldRead =
5058
permissions === true ||
5159
permissions?.read === true ||
5260
permissions?.[parentName as keyof typeof permissions] === true ||
5361
('name' in field &&
5462
typeof permissions === 'object' &&
5563
permissions?.[field.name as keyof typeof permissions] &&
5664
((permissions as any)[field.name] === true ||
57-
('read' in (permissions as any)[field.name] && (permissions as any)[field.name].read))),
58-
})
65+
('read' in (permissions as any)[field.name] && (permissions as any)[field.name].read)))
66+
67+
// Check if field permissions are effectively empty/missing
68+
const hasFieldPermissions =
69+
permissions === true ||
70+
(typeof permissions === 'object' && permissions !== null && Object.keys(permissions).length > 0)
71+
72+
// If no field permissions are defined, fallback to collection permissions
73+
if (!hasFieldPermissions && collectionPermissions) {
74+
const collectionRead = Boolean(collectionPermissions.read)
75+
let collectionOperation = false
76+
77+
// Check operation-specific permission on collection
78+
if (operation === 'create' && 'create' in collectionPermissions) {
79+
collectionOperation = Boolean(collectionPermissions.create)
80+
} else if (operation === 'update') {
81+
collectionOperation = Boolean(collectionPermissions.update)
82+
}
83+
84+
return {
85+
operation: collectionOperation,
86+
permissions: { read: collectionRead } as SanitizedFieldPermissions,
87+
read: collectionRead,
88+
}
89+
}
90+
91+
// Return the calculated permissions
92+
return {
93+
operation: Boolean(fieldOperation),
94+
permissions: fieldPermissions,
95+
read: Boolean(fieldRead),
96+
}
97+
}

packages/ui/src/fields/Blocks/BlockRow.tsx

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,39 @@ export const BlockRow: React.FC<BlocksFieldProps> = ({
9494
.filter(Boolean)
9595
.join(' ')
9696

97-
let blockPermissions: RenderFieldsProps['permissions'] = undefined
97+
let blockPermissions: RenderFieldsProps['permissions'] = true
9898

9999
if (permissions === true) {
100100
blockPermissions = true
101101
} else {
102-
const permissionsBlockSpecific = permissions?.blocks?.[block.slug]
102+
const permissionsBlockSpecific = permissions?.blocks?.[block.slug] || permissions?.blocks
103103
if (permissionsBlockSpecific === true) {
104104
blockPermissions = true
105+
} else if (permissionsBlockSpecific?.fields) {
106+
blockPermissions = permissionsBlockSpecific.fields
105107
} else {
106-
blockPermissions = permissionsBlockSpecific?.fields
108+
// Check if we should fall back to read-only mode based on permission structure
109+
// This handles cases where field-level access control exists but block permissions were sanitized
110+
if (typeof permissions === 'object' && permissions && !permissionsBlockSpecific) {
111+
// If permissions object exists but has no block-specific permissions,
112+
// check if it has any restrictive characteristics
113+
const hasReadPermission = permissions.read === true
114+
const missingCreateOrUpdate = !permissions.create || !permissions.update
115+
const hasRestrictiveStructure =
116+
hasReadPermission &&
117+
(missingCreateOrUpdate ||
118+
(typeof permissions === 'object' &&
119+
Object.keys(permissions).length === 1 &&
120+
permissions.read))
121+
122+
if (hasRestrictiveStructure) {
123+
blockPermissions = { read: true }
124+
} else {
125+
blockPermissions = permissionsBlockSpecific?.fields
126+
}
127+
} else {
128+
blockPermissions = permissionsBlockSpecific?.fields
129+
}
107130
}
108131
}
109132

0 commit comments

Comments
 (0)