From 90a86f595c31de9a89f4255318bfb0cccb30ceed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Herbaux?= Date: Fri, 9 Feb 2024 15:05:32 +0100 Subject: [PATCH] Fix input payload validation --- .../core/strapi/api/validate-body.test.api.js | 89 +++++++++++++++++++ .../permissions-manager/sanitize.ts | 7 +- .../permissions-manager/validate.ts | 9 +- packages/core/utils/src/sanitize/index.ts | 6 +- packages/core/utils/src/validate/index.ts | 10 ++- 5 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 api-tests/core/strapi/api/validate-body.test.api.js diff --git a/api-tests/core/strapi/api/validate-body.test.api.js b/api-tests/core/strapi/api/validate-body.test.api.js new file mode 100644 index 0000000000..9b481c367d --- /dev/null +++ b/api-tests/core/strapi/api/validate-body.test.api.js @@ -0,0 +1,89 @@ +'use strict'; + +const { createStrapiInstance } = require('api-tests/strapi'); +const { createTestBuilder } = require('api-tests/builder'); +const { createContentAPIRequest } = require('api-tests/request'); + +const builder = createTestBuilder(); +let strapi; +let rq; +let data; + +const productFixtures = [ + { + name: 'foo', + description: 'first product', + }, + { + name: 'bar', + description: 'second product', + }, +]; + +const product = { + attributes: { + name: { type: 'string' }, + description: { type: 'text' }, + }, + displayName: 'product', + singularName: 'product', + pluralName: 'products', + description: '', + collectionName: '', +}; + +describe('Validate Body', () => { + beforeAll(async () => { + await builder + .addContentType(product) + .addFixtures(product.singularName, productFixtures) + .build(); + + data = builder.fixturesFor(product.singularName); + + strapi = await createStrapiInstance(); + rq = await createContentAPIRequest({ strapi }); + }); + + afterAll(async () => { + await strapi.destroy(); + await builder.cleanup(); + }); + + describe('Create', () => { + test('Cannot specify the ID during entity creation', async () => { + const createPayload = { data: { id: -1, name: 'baz', description: 'third product' } }; + + const response = await rq.post('/products', { body: createPayload }); + + expect(response.statusCode).toBe(200); + + const { id, attributes } = response.body.data; + + expect(id).not.toBe(createPayload.data.id); + + expect(attributes).toHaveProperty('name', createPayload.data.name); + expect(attributes).toHaveProperty('description', createPayload.data.description); + }); + }); + + describe('Update', () => { + test('ID cannot be updated, but allowed fields can', async () => { + const target = data[0]; + + const updatePayload = { data: { id: -1, name: 'baz' } }; + + const response = await rq.put(`/products/${target.id}`, { + body: updatePayload, + }); + + expect(response.statusCode).toBe(200); + + const { id, attributes } = response.body.data; + + expect(id).toBe(target.id); + expect(attributes).toHaveProperty('name', updatePayload.data.name); + expect(attributes).toHaveProperty('description', target.description); + }); + }); +}); diff --git a/packages/core/admin/server/src/services/permission/permissions-manager/sanitize.ts b/packages/core/admin/server/src/services/permission/permissions-manager/sanitize.ts index 83cfae4257..2023f55f3f 100644 --- a/packages/core/admin/server/src/services/permission/permissions-manager/sanitize.ts +++ b/packages/core/admin/server/src/services/permission/permissions-manager/sanitize.ts @@ -241,12 +241,7 @@ export default ({ action, ability, model }: any) => { const nonVisibleWritableAttributes = intersection(nonVisibleAttributes, writableAttributes); - return uniq([ - ...fields, - ...STATIC_FIELDS, - ...COMPONENT_FIELDS, - ...nonVisibleWritableAttributes, - ]); + return uniq([...fields, ...COMPONENT_FIELDS, ...nonVisibleWritableAttributes]); }; const getOutputFields = (fields = []) => { diff --git a/packages/core/admin/server/src/services/permission/permissions-manager/validate.ts b/packages/core/admin/server/src/services/permission/permissions-manager/validate.ts index 64c2d45d30..8d0c990a44 100644 --- a/packages/core/admin/server/src/services/permission/permissions-manager/validate.ts +++ b/packages/core/admin/server/src/services/permission/permissions-manager/validate.ts @@ -118,7 +118,7 @@ export default ({ action, ability, model }: any) => { const wrapValidate = (createValidateFunction: any) => { // TODO // @ts-expect-error define the correct return type - const wrappedValidate = async (data, options = {}) => { + const wrappedValidate = async (data, options = {}): Promise => { if (isArray(data)) { return Promise.all(data.map((entity: unknown) => wrappedValidate(entity, options))); } @@ -187,12 +187,7 @@ export default ({ action, ability, model }: any) => { const nonVisibleWritableAttributes = intersection(nonVisibleAttributes, writableAttributes); - return uniq([ - ...fields, - ...STATIC_FIELDS, - ...COMPONENT_FIELDS, - ...nonVisibleWritableAttributes, - ]); + return uniq([...fields, ...COMPONENT_FIELDS, ...nonVisibleWritableAttributes]); }; const getQueryFields = (fields = []) => { diff --git a/packages/core/utils/src/sanitize/index.ts b/packages/core/utils/src/sanitize/index.ts index 1676259376..e2fd87815c 100644 --- a/packages/core/utils/src/sanitize/index.ts +++ b/packages/core/utils/src/sanitize/index.ts @@ -1,5 +1,5 @@ import { CurriedFunction1 } from 'lodash'; -import { isArray, cloneDeep } from 'lodash/fp'; +import { isArray, cloneDeep, omit } from 'lodash/fp'; import { getNonWritableAttributes } from '../content-types'; import { pipeAsync } from '../async'; @@ -34,7 +34,9 @@ const createContentAPISanitizers = () => { const nonWritableAttributes = getNonWritableAttributes(schema); const transforms = [ - // Remove non writable attributes + // Remove first level ID in inputs + omit('id'), + // Remove non-writable attributes traverseEntity(visitors.removeRestrictedFields(nonWritableAttributes), { schema }), ]; diff --git a/packages/core/utils/src/validate/index.ts b/packages/core/utils/src/validate/index.ts index 0b32d17429..5181cc2b6e 100644 --- a/packages/core/utils/src/validate/index.ts +++ b/packages/core/utils/src/validate/index.ts @@ -1,8 +1,9 @@ import { CurriedFunction1 } from 'lodash'; -import { isArray } from 'lodash/fp'; +import { isArray, isObject } from 'lodash/fp'; import { getNonWritableAttributes } from '../content-types'; import { pipeAsync } from '../async'; +import { throwInvalidParam } from './utils'; import * as visitors from './visitors'; import * as validators from './validators'; @@ -37,7 +38,12 @@ const createContentAPIValidators = () => { const nonWritableAttributes = getNonWritableAttributes(schema); const transforms = [ - // non writable attributes + (data: unknown) => { + if (isObject(data) && 'id' in data) { + throwInvalidParam({ key: 'id' }); + } + }, + // non-writable attributes traverseEntity(visitors.throwRestrictedFields(nonWritableAttributes), { schema }), ];