fix: beforeValidate deleting value when access returns false (#11549)

### What?
Regression caused by https://github.com/payloadcms/payload/pull/11433 

If a beforeChange hook was checking for a missing or undefined `value`
in order to change the value before inserting into the database, data
could be lost.

### Why?
In #11433 the logic for setting the fallback field value was moved above
the logic that cleared the value when access control returned false.

### How?
This change ensures that the fallback value is passed into the
beforeValidate function _and_ still available with the fallback value on
siblingData if access control returns false.

Fixes https://github.com/payloadcms/payload/issues/11543

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<JsonValue> {
+  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
+}

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<T> = {
@@ -274,21 +273,15 @@ export const promise = async <T>({
       }
     }
 
+    // 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
+      fallbackResult.value = await getFallbackValue({ field, req, siblingDoc })
-      if (typeof siblingDoc[field.name] !== 'undefined') {
+      fallbackResult.executed = true
-        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],
-        })
-      }
     }
 
     // Execute hooks
@@ -312,7 +305,10 @@ export const promise = async <T>({
           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 <T>({
         delete siblingData[field.name]
       }
     }
+
+    if (typeof siblingData[field.name] === 'undefined') {
+      siblingData[field.name] = !fallbackResult.executed
+        ? await getFallbackValue({ field, req, siblingDoc })
+        : fallbackResult.value
+    }
   }
 
   // Traverse subfields

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,
+      },
+    },
+  ],
+}

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: [
       {

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 () => {
+  describe('Fields', () => {
-    const doc = await payload.create({
+    it('should not affect hidden fields when patching data', async () => {
-      collection: hiddenFieldsSlug,
+      const doc = await payload.create({
-      data: {
+        collection: hiddenFieldsSlug,
-        partiallyHiddenArray: [
+        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({
+    it('should not affect hidden fields when patching data - update many', async () => {
-      id: doc.id,
+      const docsMany = await payload.create({
-      collection: hiddenFieldsSlug,
+        collection: hiddenFieldsSlug,
-      data: {
+        data: {
-        title: 'Doc Title',
+          partiallyHiddenArray: [
-      },
+            {
-    })
+              name: 'public_name',
-
+              value: 'private_value',
-    const updatedDoc = await payload.findByID({
+            },
-      id: doc.id,
+          ],
-      collection: hiddenFieldsSlug,
+          partiallyHiddenGroup: {
-      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: [
-          {
             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({
+    it('should be able to restrict access based upon siblingData', async () => {
-      collection: hiddenFieldsSlug,
+      const { id } = await payload.create({
-      data: {
+        collection: siblingDataSlug,
-        title: 'Doc Title',
+        data: {
-      },
+          array: [
-      where: {
+            {
-        id: { equals: docsMany.id },
+              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({
+    it('should use fallback value when trying to update a field without permission', async () => {
-      id: docsMany.id,
+      const doc = await payload.create({
-      collection: hiddenFieldsSlug,
+        collection: hooksSlug,
-      showHiddenFields: true,
+        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')
+    it('should use fallback value when required data is missing', async () => {
-    expect(updatedMany.partiallyHiddenArray[0].value).toStrictEqual('private_value')
+      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 () => {

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<false> | RichTextSelect<true>;
     regression1: Regression1Select<false> | Regression1Select<true>;
     regression2: Regression2Select<false> | Regression2Select<true>;
+    hooks: HooksSelect<false> | HooksSelect<true>;
     'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
     'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
     'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
@@ -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<T extends boolean = true> {
   updatedAt?: T;
   createdAt?: T;
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "hooks_select".
+ */
+export interface HooksSelect<T extends boolean = true> {
+  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".

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'

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<T extends boolean = true> {
 export interface BeforeValidateSelect<T extends boolean = true> {
   title?: T;
   selection?: T;
+  cannotMutate?: T;
+  canMutate?: T;
   updatedAt?: T;
   createdAt?: T;
 }