diff --git a/packages/payload/src/fields/hooks/beforeValidate/getFallbackValue.ts b/packages/payload/src/fields/hooks/beforeValidate/getFallbackValue.ts new file mode 100644 index 000000000..7c83848f9 --- /dev/null +++ b/packages/payload/src/fields/hooks/beforeValidate/getFallbackValue.ts @@ -0,0 +1,31 @@ +import type { JsonObject, JsonValue, PayloadRequest } from '../../../types/index.js' +import type { FieldAffectingData } from '../../config/types.js' + +import { getDefaultValue } from '../../getDefaultValue.js' +import { cloneDataFromOriginalDoc } from '../beforeChange/cloneDataFromOriginalDoc.js' + +export async function getFallbackValue({ + field, + req, + siblingDoc, +}: { + field: FieldAffectingData + req: PayloadRequest + siblingDoc: JsonObject +}): Promise { + let fallbackValue = undefined + if ('name' in field && field.name) { + if (typeof siblingDoc[field.name] !== 'undefined') { + fallbackValue = cloneDataFromOriginalDoc(siblingDoc[field.name]) + } else if ('defaultValue' in field && typeof field.defaultValue !== 'undefined') { + fallbackValue = await getDefaultValue({ + defaultValue: field.defaultValue, + locale: req.locale || '', + req, + user: req.user, + }) + } + } + + return fallbackValue +} diff --git a/packages/payload/src/fields/hooks/beforeValidate/promise.ts b/packages/payload/src/fields/hooks/beforeValidate/promise.ts index e1ab380ea..2cda4e9b3 100644 --- a/packages/payload/src/fields/hooks/beforeValidate/promise.ts +++ b/packages/payload/src/fields/hooks/beforeValidate/promise.ts @@ -8,10 +8,9 @@ import type { Block, Field, TabAsField } from '../../config/types.js' import { MissingEditorProp } from '../../../errors/index.js' import { fieldAffectsData, tabHasName, valueIsValueWithRelation } from '../../config/types.js' -import { getDefaultValue } from '../../getDefaultValue.js' import { getFieldPathsModified as getFieldPaths } from '../../getFieldPaths.js' -import { cloneDataFromOriginalDoc } from '../beforeChange/cloneDataFromOriginalDoc.js' import { getExistingRowDoc } from '../beforeChange/getExistingRowDoc.js' +import { getFallbackValue } from './getFallbackValue.js' import { traverseFields } from './traverseFields.js' type Args = { @@ -274,21 +273,15 @@ export const promise = async ({ } } + // ensure the fallback value is only computed one time + // either here or when access control returns false + const fallbackResult = { + executed: false, + value: undefined, + } if (typeof siblingData[field.name] === 'undefined') { - // If no incoming data, but existing document data is found, merge it in - if (typeof siblingDoc[field.name] !== 'undefined') { - siblingData[field.name] = cloneDataFromOriginalDoc(siblingDoc[field.name]) - - // Otherwise compute default value - } else if (typeof field.defaultValue !== 'undefined') { - siblingData[field.name] = await getDefaultValue({ - defaultValue: field.defaultValue, - locale: req.locale, - req, - user: req.user, - value: siblingData[field.name], - }) - } + fallbackResult.value = await getFallbackValue({ field, req, siblingDoc }) + fallbackResult.executed = true } // Execute hooks @@ -312,7 +305,10 @@ export const promise = async ({ schemaPath: schemaPathSegments, siblingData, siblingFields, - value: siblingData[field.name], + value: + typeof siblingData[field.name] === 'undefined' + ? fallbackResult.value + : siblingData[field.name], }) if (hookedValue !== undefined) { @@ -331,6 +327,12 @@ export const promise = async ({ delete siblingData[field.name] } } + + if (typeof siblingData[field.name] === 'undefined') { + siblingData[field.name] = !fallbackResult.executed + ? await getFallbackValue({ field, req, siblingDoc }) + : fallbackResult.value + } } // Traverse subfields diff --git a/test/access-control/collections/hooks/index.ts b/test/access-control/collections/hooks/index.ts new file mode 100644 index 000000000..960889ebe --- /dev/null +++ b/test/access-control/collections/hooks/index.ts @@ -0,0 +1,44 @@ +import type { CollectionConfig } from 'payload' + +import { hooksSlug } from '../../shared.js' + +export const Hooks: CollectionConfig = { + slug: hooksSlug, + access: { + update: () => true, + }, + fields: [ + { + name: 'cannotMutateRequired', + type: 'text', + access: { + update: () => false, + }, + required: true, + }, + { + name: 'cannotMutateNotRequired', + type: 'text', + access: { + update: () => false, + }, + hooks: { + beforeChange: [ + ({ value }) => { + if (!value) { + return 'no value found' + } + return value + }, + ], + }, + }, + { + name: 'canMutate', + type: 'text', + access: { + update: () => true, + }, + }, + ], +} diff --git a/test/access-control/config.ts b/test/access-control/config.ts index 17a3a271f..182a834b5 100644 --- a/test/access-control/config.ts +++ b/test/access-control/config.ts @@ -10,6 +10,7 @@ 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 { Hooks } from './collections/hooks/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' @@ -567,6 +568,7 @@ export default buildConfigWithDefaults( RichText, Regression1, Regression2, + Hooks, ], globals: [ { diff --git a/test/access-control/int.spec.ts b/test/access-control/int.spec.ts index df3055ffc..a91d17fee 100644 --- a/test/access-control/int.spec.ts +++ b/test/access-control/int.spec.ts @@ -8,7 +8,7 @@ import type { } from 'payload' import path from 'path' -import { Forbidden } from 'payload' +import { Forbidden, ValidationError } from 'payload' import { fileURLToPath } from 'url' import type { FullyRestricted, Post } from './payload-types.js' @@ -21,6 +21,7 @@ import { hiddenAccessCountSlug, hiddenAccessSlug, hiddenFieldsSlug, + hooksSlug, relyOnRequestHeadersSlug, restrictedVersionsSlug, secondArrayText, @@ -58,115 +59,181 @@ describe('Access Control', () => { } }) - it('should not affect hidden fields when patching data', async () => { - const doc = await payload.create({ - collection: hiddenFieldsSlug, - data: { - partiallyHiddenArray: [ - { + describe('Fields', () => { + it('should not affect hidden fields when patching data', async () => { + const doc = await payload.create({ + collection: hiddenFieldsSlug, + data: { + partiallyHiddenArray: [ + { + name: 'public_name', + value: 'private_value', + }, + ], + partiallyHiddenGroup: { name: 'public_name', value: 'private_value', }, - ], - partiallyHiddenGroup: { - name: 'public_name', - value: 'private_value', }, - }, + }) + + await payload.update({ + id: doc.id, + collection: hiddenFieldsSlug, + data: { + title: 'Doc Title', + }, + }) + + const updatedDoc = await payload.findByID({ + id: doc.id, + collection: hiddenFieldsSlug, + showHiddenFields: true, + }) + + expect(updatedDoc.partiallyHiddenGroup.value).toStrictEqual('private_value') + expect(updatedDoc.partiallyHiddenArray[0].value).toStrictEqual('private_value') }) - await payload.update({ - id: doc.id, - collection: hiddenFieldsSlug, - data: { - title: 'Doc Title', - }, - }) - - const updatedDoc = await payload.findByID({ - id: doc.id, - collection: hiddenFieldsSlug, - showHiddenFields: true, - }) - - expect(updatedDoc.partiallyHiddenGroup.value).toStrictEqual('private_value') - expect(updatedDoc.partiallyHiddenArray[0].value).toStrictEqual('private_value') - }) - - it('should not affect hidden fields when patching data - update many', async () => { - const docsMany = await payload.create({ - collection: hiddenFieldsSlug, - data: { - partiallyHiddenArray: [ - { + it('should not affect hidden fields when patching data - update many', async () => { + const docsMany = await payload.create({ + collection: hiddenFieldsSlug, + data: { + partiallyHiddenArray: [ + { + name: 'public_name', + value: 'private_value', + }, + ], + partiallyHiddenGroup: { name: 'public_name', value: 'private_value', }, - ], - partiallyHiddenGroup: { - name: 'public_name', - value: 'private_value', }, - }, + }) + + await payload.update({ + collection: hiddenFieldsSlug, + data: { + title: 'Doc Title', + }, + where: { + id: { equals: docsMany.id }, + }, + }) + + const updatedMany = await payload.findByID({ + id: docsMany.id, + collection: hiddenFieldsSlug, + showHiddenFields: true, + }) + + expect(updatedMany.partiallyHiddenGroup.value).toStrictEqual('private_value') + expect(updatedMany.partiallyHiddenArray[0].value).toStrictEqual('private_value') }) - await payload.update({ - collection: hiddenFieldsSlug, - data: { - title: 'Doc Title', - }, - where: { - id: { equals: docsMany.id }, - }, + it('should be able to restrict access based upon siblingData', async () => { + const { id } = await payload.create({ + collection: siblingDataSlug, + data: { + array: [ + { + allowPublicReadability: true, + text: firstArrayText, + }, + { + allowPublicReadability: false, + text: secondArrayText, + }, + ], + }, + }) + + const doc = await payload.findByID({ + id, + collection: siblingDataSlug, + overrideAccess: false, + }) + + expect(doc.array?.[0].text).toBe(firstArrayText) + // Should respect PublicReadabilityAccess function and not be sent + expect(doc.array?.[1].text).toBeUndefined() + + // Retrieve with default of overriding access + const docOverride = await payload.findByID({ + id, + collection: siblingDataSlug, + }) + + expect(docOverride.array?.[0].text).toBe(firstArrayText) + expect(docOverride.array?.[1].text).toBe(secondArrayText) }) - const updatedMany = await payload.findByID({ - id: docsMany.id, - collection: hiddenFieldsSlug, - showHiddenFields: true, + it('should use fallback value when trying to update a field without permission', async () => { + const doc = await payload.create({ + collection: hooksSlug, + data: { + cannotMutateRequired: 'original', + }, + }) + + const updatedDoc = await payload.update({ + id: doc.id, + collection: hooksSlug, + overrideAccess: false, + data: { + cannotMutateRequired: 'new', + canMutate: 'canMutate', + }, + }) + + expect(updatedDoc.cannotMutateRequired).toBe('original') }) - expect(updatedMany.partiallyHiddenGroup.value).toStrictEqual('private_value') - expect(updatedMany.partiallyHiddenArray[0].value).toStrictEqual('private_value') + it('should use fallback value when required data is missing', async () => { + const doc = await payload.create({ + collection: hooksSlug, + data: { + cannotMutateRequired: 'original', + }, + }) + + const updatedDoc = await payload.update({ + id: doc.id, + collection: hooksSlug, + overrideAccess: false, + data: { + canMutate: 'canMutate', + }, + }) + + // should fallback to original data and not throw validation error + expect(updatedDoc.cannotMutateRequired).toBe('original') + }) + + it('should pass fallback value through to beforeChange hook when access returns false', async () => { + const doc = await payload.create({ + collection: hooksSlug, + data: { + cannotMutateRequired: 'cannotMutateRequired', + cannotMutateNotRequired: 'cannotMutateNotRequired', + }, + }) + + const updatedDoc = await payload.update({ + id: doc.id, + collection: hooksSlug, + overrideAccess: false, + data: { + cannotMutateNotRequired: 'updated', + }, + }) + + // should fallback to original data and not throw validation error + expect(updatedDoc.cannotMutateRequired).toBe('cannotMutateRequired') + expect(updatedDoc.cannotMutateNotRequired).toBe('cannotMutateNotRequired') + }) }) - - it('should be able to restrict access based upon siblingData', async () => { - const { id } = await payload.create({ - collection: siblingDataSlug, - data: { - array: [ - { - allowPublicReadability: true, - text: firstArrayText, - }, - { - allowPublicReadability: false, - text: secondArrayText, - }, - ], - }, - }) - - const doc = await payload.findByID({ - id, - collection: siblingDataSlug, - overrideAccess: false, - }) - - expect(doc.array?.[0].text).toBe(firstArrayText) - // Should respect PublicReadabilityAccess function and not be sent - expect(doc.array?.[1].text).toBeUndefined() - - // Retrieve with default of overriding access - const docOverride = await payload.findByID({ - id, - collection: siblingDataSlug, - }) - - expect(docOverride.array?.[0].text).toBe(firstArrayText) - expect(docOverride.array?.[1].text).toBe(secondArrayText) - }) - describe('Collections', () => { describe('restricted collection', () => { it('field without read access should not show', async () => { diff --git a/test/access-control/payload-types.ts b/test/access-control/payload-types.ts index 8be2a00c1..eda5877a6 100644 --- a/test/access-control/payload-types.ts +++ b/test/access-control/payload-types.ts @@ -89,6 +89,7 @@ export interface Config { 'rich-text': RichText; regression1: Regression1; regression2: Regression2; + hooks: Hook; 'payload-locked-documents': PayloadLockedDocument; 'payload-preferences': PayloadPreference; 'payload-migrations': PayloadMigration; @@ -117,6 +118,7 @@ export interface Config { 'rich-text': RichTextSelect | RichTextSelect; regression1: Regression1Select | Regression1Select; regression2: Regression2Select | Regression2Select; + hooks: HooksSelect | HooksSelect; 'payload-locked-documents': PayloadLockedDocumentsSelect | PayloadLockedDocumentsSelect; 'payload-preferences': PayloadPreferencesSelect | PayloadPreferencesSelect; 'payload-migrations': PayloadMigrationsSelect | PayloadMigrationsSelect; @@ -680,6 +682,18 @@ export interface Regression2 { updatedAt: string; createdAt: string; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "hooks". + */ +export interface Hook { + id: string; + cannotMutateRequired: string; + cannotMutateNotRequired?: string | null; + canMutate?: string | null; + updatedAt: string; + createdAt: string; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "payload-locked-documents". @@ -774,6 +788,10 @@ export interface PayloadLockedDocument { | ({ relationTo: 'regression2'; value: string | Regression2; + } | null) + | ({ + relationTo: 'hooks'; + value: string | Hook; } | null); globalSlug?: string | null; user: @@ -1168,6 +1186,17 @@ export interface Regression2Select { updatedAt?: T; createdAt?: T; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "hooks_select". + */ +export interface HooksSelect { + cannotMutateRequired?: T; + cannotMutateNotRequired?: T; + canMutate?: T; + updatedAt?: T; + createdAt?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "payload-locked-documents_select". diff --git a/test/access-control/shared.ts b/test/access-control/shared.ts index 0de87257e..7e88f6bda 100644 --- a/test/access-control/shared.ts +++ b/test/access-control/shared.ts @@ -5,6 +5,7 @@ export const slug = 'posts' export const unrestrictedSlug = 'unrestricted' export const readOnlySlug = 'read-only-collection' export const readOnlyGlobalSlug = 'read-only-global' +export const hooksSlug = 'hooks' export const userRestrictedCollectionSlug = 'user-restricted-collection' export const fullyRestrictedSlug = 'fully-restricted' diff --git a/test/hooks/payload-types.ts b/test/hooks/payload-types.ts index 87ad25904..073a259c0 100644 --- a/test/hooks/payload-types.ts +++ b/test/hooks/payload-types.ts @@ -156,6 +156,8 @@ export interface BeforeValidate { id: string; title?: string | null; selection?: ('a' | 'b') | null; + cannotMutate?: string | null; + canMutate?: string | null; updatedAt: string; createdAt: string; } @@ -741,6 +743,8 @@ export interface BeforeChangeHooksSelect { export interface BeforeValidateSelect { title?: T; selection?: T; + cannotMutate?: T; + canMutate?: T; updatedAt?: T; createdAt?: T; }