diff --git a/packages/core/content-type-builder/server/src/controllers/validation/__tests__/component.test.ts b/packages/core/content-type-builder/server/src/controllers/validation/__tests__/component.test.ts index 078102b4b4..73ac46b742 100644 --- a/packages/core/content-type-builder/server/src/controllers/validation/__tests__/component.test.ts +++ b/packages/core/content-type-builder/server/src/controllers/validation/__tests__/component.test.ts @@ -1,3 +1,4 @@ +import * as builder from '../../../services/builder'; import { validateComponentInput, validateUpdateComponentInput } from '../component'; const componentValidation = { @@ -11,14 +12,7 @@ describe('Component validator', () => { plugins: { 'content-type-builder': { services: { - builder: { - getReservedNames() { - return { - models: [], - attributes: ['thisIsReserved'], - }; - }, - }, + builder, }, }, }, diff --git a/packages/core/content-type-builder/server/src/controllers/validation/__tests__/content-type.test.ts b/packages/core/content-type-builder/server/src/controllers/validation/__tests__/content-type.test.ts index 4478e612e4..e716a12fe6 100644 --- a/packages/core/content-type-builder/server/src/controllers/validation/__tests__/content-type.test.ts +++ b/packages/core/content-type-builder/server/src/controllers/validation/__tests__/content-type.test.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ +import * as builder from '../../../services/builder'; import { validateKind, validateUpdateContentTypeInput, @@ -12,14 +13,7 @@ describe('Content type validator', () => { plugins: { 'content-type-builder': { services: { - builder: { - getReservedNames() { - return { - models: ['reserved-name'], - attributes: ['thisIsReserved'], - }; - }, - }, + builder, }, }, }, @@ -49,7 +43,7 @@ describe('Content type validator', () => { pluralName: 'tests', displayName: 'Test', attributes: { - thisIsReserved: { + entryId: { type: 'string', default: '', }, @@ -62,13 +56,14 @@ describe('Content type validator', () => { await validateUpdateContentTypeInput(data).catch((err) => { expect(err).toMatchObject({ name: 'ValidationError', - message: 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', + message: + 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*', details: { errors: [ { - path: ['contentType', 'attributes', 'thisIsReserved'], + path: ['contentType', 'attributes', 'entryId'], message: - 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', + 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*', name: 'ValidationError', }, ], @@ -84,7 +79,7 @@ describe('Content type validator', () => { pluralName: 'tests', displayName: 'Test', attributes: { - THIS_IS_RESERVED: { + ENTRY_ID: { type: 'string', default: '', }, @@ -97,13 +92,14 @@ describe('Content type validator', () => { await validateUpdateContentTypeInput(data).catch((err) => { expect(err).toMatchObject({ name: 'ValidationError', - message: 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', + message: + 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*', details: { errors: [ { - path: ['contentType', 'attributes', 'THIS_IS_RESERVED'], + path: ['contentType', 'attributes', 'ENTRY_ID'], message: - 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', + 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*', name: 'ValidationError', }, ], @@ -119,8 +115,8 @@ describe('Content type validator', () => { test.each(reservedNames)('Throws when reserved model names are used in %s', async (name) => { const data = { contentType: { - singularName: name === 'singularName' ? 'reserved-name' : 'not-reserved-single', - pluralName: name === 'pluralName' ? 'reserved-name' : 'not-reserved-plural', + singularName: name === 'singularName' ? 'date-time' : 'not-reserved-single', + pluralName: name === 'pluralName' ? 'date-time' : 'not-reserved-plural', displayName: 'Test', attributes: { notReserved: { @@ -136,12 +132,12 @@ describe('Content type validator', () => { await validateUpdateContentTypeInput(data).catch((err) => { expect(err).toMatchObject({ name: 'ValidationError', - message: `Content Type name cannot be one of reserved-name`, + message: `Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*`, details: { errors: [ { path: ['contentType', name], - message: `Content Type name cannot be one of reserved-name`, + message: `Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*`, name: 'ValidationError', }, ], diff --git a/packages/core/content-type-builder/server/src/controllers/validation/content-type.ts b/packages/core/content-type-builder/server/src/controllers/validation/content-type.ts index 6fd4b8f006..cffb02e568 100644 --- a/packages/core/content-type-builder/server/src/controllers/validation/content-type.ts +++ b/packages/core/content-type-builder/server/src/controllers/validation/content-type.ts @@ -146,8 +146,8 @@ const forbiddenContentTypeNameValidator = () => { if (typeof value !== 'string') { return true; } - // compare snake case to check the actual column names that will be used in the database - return reservedNames.every((reservedName) => snakeCase(reservedName) !== snakeCase(value)); + + return !getService('builder').isReservedModelName(value); }, }; }; diff --git a/packages/core/content-type-builder/server/src/controllers/validation/model-schema.ts b/packages/core/content-type-builder/server/src/controllers/validation/model-schema.ts index 69cbe07484..4400753630 100644 --- a/packages/core/content-type-builder/server/src/controllers/validation/model-schema.ts +++ b/packages/core/content-type-builder/server/src/controllers/validation/model-schema.ts @@ -1,7 +1,7 @@ import { yup } from '@strapi/utils'; import _ from 'lodash'; import { snakeCase } from 'lodash/fp'; -import { modelTypes, FORBIDDEN_ATTRIBUTE_NAMES, typeKinds } from '../../services/constants'; +import { modelTypes, typeKinds } from '../../services/constants'; import { getService } from '../../utils'; import { isValidKey, isValidCollectionName } from './common'; import { getTypeValidator } from './types'; @@ -78,22 +78,11 @@ const isConflictingKey = (key: string, attributes: Record) => { }; const isForbiddenKey = (key: string) => { - const snakeCaseKey = snakeCase(key); - const reservedNames = [ - ...FORBIDDEN_ATTRIBUTE_NAMES, - ...getService('builder').getReservedNames().attributes, - ]; - - return reservedNames.some((reserved) => { - return snakeCase(reserved) === snakeCaseKey; - }); + return getService('builder').isReservedAttributeName(key); }; const forbiddenValidator = () => { - const reservedNames = [ - ...FORBIDDEN_ATTRIBUTE_NAMES, - ...getService('builder').getReservedNames().attributes, - ]; + const reservedNames = [...getService('builder').getReservedNames().attributes]; return yup.mixed().test({ name: 'forbiddenKeys', diff --git a/packages/core/content-type-builder/server/src/services/builder.ts b/packages/core/content-type-builder/server/src/services/builder.ts index d297b224e3..249853ce6f 100644 --- a/packages/core/content-type-builder/server/src/services/builder.ts +++ b/packages/core/content-type-builder/server/src/services/builder.ts @@ -1,44 +1,95 @@ +import { snakeCase } from 'lodash/fp'; + +// use snake_case +export const reservedAttributes = [ + // TODO: these need to come from a centralized place so we don't break things accidentally in the future and can share them outside the CTB, for example on Strapi bootstrap prior to schema db sync + + // ID fields + 'id', + 'document_id', + + // Creator fields + 'created_at', + 'updated_at', + 'published_at', + 'created_by_id', + 'updated_by_id', + // does not actually conflict because the fields are called *_by_id but we'll leave it to avoid confusion + 'created_by', + 'updated_by', + + // Used for Strapi functionality + 'entry_id', + 'status', + 'localizations', + 'meta', + 'locale', + '__component', + '__contentType', + + // We support ending with * to denote prefixes + 'strapi*', + '_strapi*', + '__strapi*', +]; + +// use snake_case +export const reservedModels = [ + 'boolean', + 'date', + 'date_time', + 'time', + 'upload', + 'document', + 'then', // no longer an issue but still restricting for being a javascript keyword + + // We support ending with * to denote prefixes + 'strapi*', + '_strapi*', + '__strapi*', +]; + export const getReservedNames = () => { return { - // use kebab case everywhere since singularName and pluralName are validated that way - models: [ - 'boolean', - 'date', - 'date-time', - 'time', - 'upload', - 'document', - 'then', // no longer an issue but still restricting for being a javascript keyword - ], - // attributes are compared with snake_case(name), so only snake_case is needed here and camelCase + UPPER_CASE matches will still be caught - attributes: [ - // TODO: these need to come from a centralized place so we don't break things accidentally in the future and can share them outside the CTB, for example on Strapi bootstrap prior to schema db sync - - // ID fields - 'id', - 'document_id', - - // Creator fields - 'created_at', - 'updated_at', - 'published_at', - 'created_by_id', - 'updated_by_id', - // does not actually conflict because the fields are called *_by_id but we'll leave it to avoid confusion - 'created_by', - 'updated_by', - - // Used for Strapi functionality - 'entry_id', - 'status', - 'localizations', - 'meta', - 'locale', - // TODO: remove these in favor of restricting the strapi_ prefix - 'strapi', - 'strapi_stage', - 'strapi_assignee', - ], + models: reservedModels, + attributes: reservedAttributes, }; - // strapi.db.getReservedNames(); +}; + +// compare snake case to check the actual column names that will be used in the database +export const isReservedModelName = (name: string) => { + const snakeCaseName = snakeCase(name); + if (reservedModels.includes(snakeCaseName)) { + return true; + } + + if ( + reservedModels + .filter((key) => key.endsWith('*')) + .map((key) => key.slice(0, -1)) + .some((prefix) => snakeCaseName.startsWith(prefix)) + ) { + return true; + } + + return false; +}; + +// compare snake case to check the actual column names that will be used in the database +export const isReservedAttributeName = (name: string) => { + const snakeCaseName = snakeCase(name); + if (reservedAttributes.includes(snakeCaseName)) { + return true; + } + + if ( + reservedAttributes + .filter((key) => key.endsWith('*')) + .map((key) => key.slice(0, -1)) + .some((prefix) => snakeCaseName.startsWith(prefix)) + ) { + return true; + } + + return false; }; diff --git a/packages/core/content-type-builder/server/src/services/constants.ts b/packages/core/content-type-builder/server/src/services/constants.ts index 4e6ba4ded5..6b05716b1e 100644 --- a/packages/core/content-type-builder/server/src/services/constants.ts +++ b/packages/core/content-type-builder/server/src/services/constants.ts @@ -36,8 +36,6 @@ export const DEFAULT_TYPES = [ export const VALID_UID_TARGETS = ['string', 'text'] as const; -export const FORBIDDEN_ATTRIBUTE_NAMES = ['__component', '__contentType'] as const; - export const coreUids = { STRAPI_USER: 'admin::user', PREFIX: 'strapi::', diff --git a/tests/api/core/content-type-builder/collection-type.test.api.js b/tests/api/core/content-type-builder/collection-type.test.api.js index 50058f6d96..fee27a64c0 100644 --- a/tests/api/core/content-type-builder/collection-type.test.api.js +++ b/tests/api/core/content-type-builder/collection-type.test.api.js @@ -8,6 +8,7 @@ const { createStrapiInstance } = require('api-tests/strapi'); const { createAuthRequest } = require('api-tests/request'); const modelsUtils = require('api-tests/models'); const { createTestBuilder } = require('api-tests/builder'); +const { kebabCase } = require('lodash/fp'); let strapi; let rq; @@ -48,6 +49,7 @@ describe('Content Type Builder - Content types', () => { await restart(); }); + // TODO FIXME: this depends on all tests to run or else it throws an error afterAll(async () => { const modelsUIDs = [ 'api::test-collection-type.test-collection-type', @@ -200,6 +202,58 @@ describe('Content Type Builder - Content types', () => { }); }); + test.each(['strapi', '_strapi', '__strapi'])( + 'Cannot use %s prefix for content type name', + async (prefix) => { + const res = await rq({ + method: 'POST', + url: '/content-type-builder/content-types', + body: { + contentType: { + displayName: 'unique string', + singularName: `${kebabCase(prefix)}-singular`, + pluralName: `${kebabCase(prefix)}-plural`, + attributes: { + [prefix]: { + type: 'string', + }, + }, + }, + }, + }); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ + error: { + details: { + errors: [ + { + message: + 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*', + name: 'ValidationError', + path: ['contentType', 'attributes', prefix], + }, + { + message: + 'Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*', + name: 'ValidationError', + path: ['contentType', 'singularName'], + }, + { + message: + 'Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*', + name: 'ValidationError', + path: ['contentType', 'pluralName'], + }, + ], + }, + message: '3 errors occurred', + name: 'ValidationError', + }, + }); + } + ); + test('Cannot use same string for singularName and pluralName', async () => { const res = await rq({ method: 'POST',