From 76b30753693b2c68edec615d89cbf7f54565fd39 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 19 Jul 2024 11:53:00 -0400 Subject: [PATCH] feat: update reserved fields name check to be more comprehensive and only check top level fields (#7235) Continuation of https://github.com/payloadcms/payload/pull/7179 --- .../config/reservedFieldNames.spec.ts | 172 ++++++++++++++++++ .../collections/config/reservedFieldNames.ts | 150 +++++++++++++++ .../src/collections/config/sanitize.ts | 8 + .../src/fields/config/sanitize.spec.ts | 24 +-- .../payload/src/fields/config/sanitize.ts | 21 +-- 5 files changed, 340 insertions(+), 35 deletions(-) create mode 100644 packages/payload/src/collections/config/reservedFieldNames.spec.ts create mode 100644 packages/payload/src/collections/config/reservedFieldNames.ts diff --git a/packages/payload/src/collections/config/reservedFieldNames.spec.ts b/packages/payload/src/collections/config/reservedFieldNames.spec.ts new file mode 100644 index 0000000000..4049aa9ff1 --- /dev/null +++ b/packages/payload/src/collections/config/reservedFieldNames.spec.ts @@ -0,0 +1,172 @@ +import type { Config } from '../../config/types.js' +import type { CollectionConfig, Field } from '../../index.js' + +import { ReservedFieldName } from '../../errors/index.js' +import { sanitizeCollection } from './sanitize.js' + +describe('reservedFieldNames - collections -', () => { + const config = { + collections: [], + globals: [], + } as Partial + + describe('uploads -', () => { + const collectionWithUploads: CollectionConfig = { + slug: 'collection-with-uploads', + fields: [], + upload: true, + } + + it('should throw on file', async () => { + const fields: Field[] = [ + { + name: 'file', + type: 'text', + label: 'some-collection', + }, + ] + await expect(async () => { + await sanitizeCollection( + // @ts-expect-error + { + ...config, + collections: [ + { + ...collectionWithUploads, + fields, + }, + ], + }, + { + ...collectionWithUploads, + fields, + }, + ) + }).rejects.toThrow(ReservedFieldName) + }) + + it('should not throw on a custom field', async () => { + const fields: Field[] = [ + { + name: 'customField', + type: 'text', + label: 'some-collection', + }, + ] + await expect(async () => { + await sanitizeCollection( + // @ts-expect-error + { + ...config, + collections: [ + { + ...collectionWithUploads, + fields, + }, + ], + }, + { + ...collectionWithUploads, + fields, + }, + ) + }).not.toThrow() + }) + }) + + describe('auth -', () => { + const collectionWithAuth: CollectionConfig = { + slug: 'collection-with-auth', + fields: [], + auth: { + verify: true, + useAPIKey: true, + loginWithUsername: true, + }, + } + + it('should throw on hash', async () => { + const fields: Field[] = [ + { + name: 'hash', + type: 'text', + label: 'some-collection', + }, + ] + await expect(async () => { + await sanitizeCollection( + // @ts-expect-error + { + ...config, + collections: [ + { + ...collectionWithAuth, + fields, + }, + ], + }, + { + ...collectionWithAuth, + fields, + }, + ) + }).rejects.toThrow(ReservedFieldName) + }) + + it('should throw on salt', async () => { + const fields: Field[] = [ + { + name: 'salt', + type: 'text', + label: 'some-collection', + }, + ] + await expect(async () => { + await sanitizeCollection( + // @ts-expect-error + { + ...config, + collections: [ + { + ...collectionWithAuth, + fields, + }, + ], + }, + { + ...collectionWithAuth, + fields, + }, + ) + }).rejects.toThrow(ReservedFieldName) + }) + + it('should not throw on a custom field', async () => { + const fields: Field[] = [ + { + name: 'customField', + type: 'text', + label: 'some-collection', + }, + ] + await expect(async () => { + await sanitizeCollection( + // @ts-expect-error + { + ...config, + collections: [ + { + ...collectionWithAuth, + fields, + }, + ], + }, + { + ...collectionWithAuth, + fields, + }, + ) + }).not.toThrow() + }) + }) +}) diff --git a/packages/payload/src/collections/config/reservedFieldNames.ts b/packages/payload/src/collections/config/reservedFieldNames.ts new file mode 100644 index 0000000000..41e3fe3dd9 --- /dev/null +++ b/packages/payload/src/collections/config/reservedFieldNames.ts @@ -0,0 +1,150 @@ +import type { Field } from '../../fields/config/types.js' +import type { CollectionConfig } from '../../index.js' + +import { ReservedFieldName } from '../../errors/ReservedFieldName.js' +import { fieldAffectsData } from '../../fields/config/types.js' + +// Note for future reference: We've slimmed down the reserved field names but left them in here for reference in case it's needed in the future. + +/** + * Reserved field names for collections with auth config enabled + */ +const reservedBaseAuthFieldNames = [ + /* 'email', + 'resetPasswordToken', + 'resetPasswordExpiration', */ + 'salt', + 'hash', +] +/** + * Reserved field names for auth collections with verify: true + */ +const reservedVerifyFieldNames = [ + /* '_verified', '_verificationToken' */ +] +/** + * Reserved field names for auth collections with useApiKey: true + */ +const reservedAPIKeyFieldNames = [ + /* 'enableAPIKey', 'apiKeyIndex', 'apiKey' */ +] + +/** + * Reserved field names for collections with upload config enabled + */ +const reservedBaseUploadFieldNames = [ + 'file', + /* 'mimeType', + 'thumbnailURL', + 'width', + 'height', + 'filesize', + 'filename', + 'url', + 'focalX', + 'focalY', + 'sizes', */ +] + +/** + * Reserved field names for collections with versions enabled + */ +const reservedVersionsFieldNames = [ + /* '__v', '_status' */ +] + +/** + * Sanitize fields for collections with auth config enabled. + * + * Should run on top level fields only. + */ +export const sanitizeAuthFields = (fields: Field[], config: CollectionConfig) => { + for (let i = 0; i < fields.length; i++) { + const field = fields[i] + + if (fieldAffectsData(field) && field.name) { + if (config.auth && typeof config.auth === 'object' && !config.auth.disableLocalStrategy) { + const auth = config.auth + + if (reservedBaseAuthFieldNames.includes(field.name)) { + throw new ReservedFieldName(field, field.name) + } + + if (auth.verify) { + if (reservedAPIKeyFieldNames.includes(field.name)) { + throw new ReservedFieldName(field, field.name) + } + } + + /* if (auth.maxLoginAttempts) { + if (field.name === 'loginAttempts' || field.name === 'lockUntil') { + throw new ReservedFieldName(field, field.name) + } + } */ + + /* if (auth.loginWithUsername) { + if (field.name === 'username') { + throw new ReservedFieldName(field, field.name) + } + } */ + + if (auth.verify) { + if (reservedVerifyFieldNames.includes(field.name)) { + throw new ReservedFieldName(field, field.name) + } + } + } + } + + // Handle tabs without a name + if (field.type === 'tabs') { + for (let j = 0; j < field.tabs.length; j++) { + const tab = field.tabs[j] + + if (!('name' in tab)) { + sanitizeAuthFields(tab.fields, config) + } + } + } + + // Handle presentational fields like rows and collapsibles + if (!fieldAffectsData(field) && 'fields' in field && field.fields) { + sanitizeAuthFields(field.fields, config) + } + } +} + +/** + * Sanitize fields for collections with upload config enabled. + * + * Should run on top level fields only. + */ +export const sanitizeUploadFields = (fields: Field[], config: CollectionConfig) => { + if (config.upload && typeof config.upload === 'object') { + for (let i = 0; i < fields.length; i++) { + const field = fields[i] + + if (fieldAffectsData(field) && field.name) { + if (reservedBaseUploadFieldNames.includes(field.name)) { + throw new ReservedFieldName(field, field.name) + } + } + + // Handle tabs without a name + if (field.type === 'tabs') { + for (let j = 0; j < field.tabs.length; j++) { + const tab = field.tabs[j] + + if (!('name' in tab)) { + sanitizeUploadFields(tab.fields, config) + } + } + } + + // Handle presentational fields like rows and collapsibles + if (!fieldAffectsData(field) && 'fields' in field && field.fields) { + sanitizeUploadFields(field.fields, config) + } + } + } +} diff --git a/packages/payload/src/collections/config/sanitize.ts b/packages/payload/src/collections/config/sanitize.ts index 91829c4987..a973a13cbd 100644 --- a/packages/payload/src/collections/config/sanitize.ts +++ b/packages/payload/src/collections/config/sanitize.ts @@ -14,6 +14,7 @@ import { isPlainObject } from '../../utilities/isPlainObject.js' import baseVersionFields from '../../versions/baseFields.js' import { versionDefaults } from '../../versions/defaults.js' import { authDefaults, defaults, loginWithUsernameDefaults } from './defaults.js' +import { sanitizeAuthFields, sanitizeUploadFields } from './reservedFieldNames.js' export const sanitizeCollection = async ( config: Config, @@ -38,6 +39,7 @@ export const sanitizeCollection = async ( const validRelationships = config.collections.map((c) => c.slug) || [] sanitized.fields = await sanitizeFields({ + collectionConfig: sanitized, config, fields: sanitized.fields, richTextSanitizationPromises, @@ -115,6 +117,9 @@ export const sanitizeCollection = async ( if (sanitized.upload) { if (sanitized.upload === true) sanitized.upload = {} + // sanitize fields for reserved names + sanitizeUploadFields(sanitized.fields, sanitized) + // disable duplicate for uploads by default sanitized.disableDuplicate = sanitized.disableDuplicate || true @@ -133,6 +138,9 @@ export const sanitizeCollection = async ( } if (sanitized.auth) { + // sanitize fields for reserved names + sanitizeAuthFields(sanitized.fields, sanitized) + sanitized.auth = merge(authDefaults, typeof sanitized.auth === 'object' ? sanitized.auth : {}, { isMergeableObject: isPlainObject, }) diff --git a/packages/payload/src/fields/config/sanitize.spec.ts b/packages/payload/src/fields/config/sanitize.spec.ts index 75ec4cce45..623aabbe87 100644 --- a/packages/payload/src/fields/config/sanitize.spec.ts +++ b/packages/payload/src/fields/config/sanitize.spec.ts @@ -9,12 +9,7 @@ import type { TextField, } from './types.js' -import { - InvalidFieldName, - InvalidFieldRelationship, - MissingFieldType, - ReservedFieldName, -} from '../../errors/index.js' +import { InvalidFieldName, InvalidFieldRelationship, MissingFieldType } from '../../errors/index.js' import { sanitizeFields } from './sanitize.js' describe('sanitizeFields', () => { @@ -52,23 +47,6 @@ describe('sanitizeFields', () => { }).rejects.toThrow(InvalidFieldName) }) - it('should throw on a reserved field name', async () => { - const fields: Field[] = [ - { - name: 'hash', - type: 'text', - label: 'hash', - }, - ] - await expect(async () => { - await sanitizeFields({ - config, - fields, - validRelationships: [], - }) - }).rejects.toThrow(ReservedFieldName) - }) - describe('auto-labeling', () => { it('should populate label if missing', async () => { const fields: Field[] = [ diff --git a/packages/payload/src/fields/config/sanitize.ts b/packages/payload/src/fields/config/sanitize.ts index 03a2e06621..09dfd749bf 100644 --- a/packages/payload/src/fields/config/sanitize.ts +++ b/packages/payload/src/fields/config/sanitize.ts @@ -1,3 +1,4 @@ +import type { CollectionConfig } from '../../collections/config/types.js' import type { Config, SanitizedConfig } from '../../config/types.js' import type { Field } from './types.js' @@ -7,7 +8,6 @@ import { InvalidFieldName, InvalidFieldRelationship, MissingFieldType, - ReservedFieldName, } from '../../errors/index.js' import { deepMerge } from '../../utilities/deepMerge.js' import { formatLabels, toWords } from '../../utilities/formatLabels.js' @@ -18,6 +18,7 @@ import validations from '../validations.js' import { fieldAffectsData, tabHasName } from './types.js' type Args = { + collectionConfig?: CollectionConfig config: Config existingFieldNames?: Set fields: Field[] @@ -40,9 +41,8 @@ type Args = { validRelationships: null | string[] } -export const reservedFieldNames = ['__v', 'salt', 'hash', 'file'] - export const sanitizeFields = async ({ + collectionConfig, config, existingFieldNames = new Set(), fields, @@ -62,11 +62,6 @@ export const sanitizeFields = async ({ throw new InvalidFieldName(field, field.name) } - // assert that field names are not one of reserved names - if (fieldAffectsData(field) && reservedFieldNames.includes(field.name)) { - throw new ReservedFieldName(field, field.name) - } - // Auto-label if ( 'name' in field && @@ -116,10 +111,12 @@ export const sanitizeFields = async ({ } if (field.type === 'blocks' && field.blocks) { - field.blocks = field.blocks.map((block) => ({ - ...block, - fields: block.fields.concat(baseBlockFields), - })) + field.blocks = field.blocks.map((block) => { + return { + ...block, + fields: block.fields.concat(baseBlockFields), + } + }) } if (field.type === 'array' && field.fields) {