diff --git a/packages/core/upload/server/controllers/admin-file.js b/packages/core/upload/server/controllers/admin-file.js index 50df0f892f..78e137a7f3 100644 --- a/packages/core/upload/server/controllers/admin-file.js +++ b/packages/core/upload/server/controllers/admin-file.js @@ -1,7 +1,7 @@ 'use strict'; const { merge } = require('lodash/fp'); -const { mapAsync } = require('@strapi/utils'); +const { mapAsync, pipeAsync } = require('@strapi/utils'); const { getService } = require('../utils'); const { ACTIONS, FILE_MODEL_UID } = require('../constants'); const { findEntityAndCheckPermissions } = require('./utils/find-entity-and-check-permissions'); @@ -24,10 +24,17 @@ module.exports = { return ctx.forbidden(); } - const pmQuery = pm.addPermissionsQueryTo(merge(defaultQuery, ctx.query)); + // validate the incoming user query params + await pm.validateQuery(ctx.query); - await pm.validateQuery(pmQuery); - const query = await pm.sanitizeQuery(pmQuery); + const query = await pipeAsync( + // Start by sanitizing the incoming query + (q) => pm.sanitizeQuery(q), + // Add the default query which should not be validated or sanitized + (q) => merge(defaultQuery, q), + // Add the dynamic filters based on permissions' conditions + (q) => pm.addPermissionsQueryTo(q) + )(ctx.query); const { results: files, pagination } = await getService('upload').findPage(query); diff --git a/tests/api/core/upload/admin/file.test.api.js b/tests/api/core/upload/admin/file.test.api.js index 5e1aaff92c..d6d52faf72 100644 --- a/tests/api/core/upload/admin/file.test.api.js +++ b/tests/api/core/upload/admin/file.test.api.js @@ -6,10 +6,12 @@ const path = require('path'); const { createTestBuilder } = require('api-tests/builder'); const { createStrapiInstance } = require('api-tests/strapi'); const { createAuthRequest } = require('api-tests/request'); +const { createUtils } = require('api-tests/utils'); const builder = createTestBuilder(); let strapi; let rq; +let utils; const dogModel = { displayName: 'Dog', @@ -28,6 +30,7 @@ describe('Upload', () => { await builder.addContentType(dogModel).build(); strapi = await createStrapiInstance(); rq = await createAuthRequest({ strapi }); + utils = createUtils(strapi); }); afterAll(async () => { @@ -53,6 +56,81 @@ describe('Upload', () => { }); describe('Read', () => { + let uploadReaderRole; + + let u1Req; + let u2Req; + + const users = { u1: null, u2: null }; + + beforeAll(async () => { + uploadReaderRole = await utils.createRole({ + name: 'UploadReader', + description: 'Can only see files created by same role as creator', + }); + + // Add permissions to the role with conditions + // This is important in order to dynamically add filters with sensitive fields to the final query + await utils.assignPermissionsToRole(uploadReaderRole.id, [ + { + action: 'plugin::upload.read', + subject: null, + conditions: ['admin::has-same-role-as-creator'], + properties: {}, + }, + { + action: 'plugin::upload.assets.create', + subject: null, + conditions: ['admin::has-same-role-as-creator'], + properties: {}, + }, + { + action: 'plugin::upload.assets.update', + subject: null, + conditions: ['admin::has-same-role-as-creator'], + properties: {}, + }, + ]); + + // TODO: We create 2 users in order to be able to test the condition itself (same role as creator) + + users.u1 = await utils.createUser({ + firstname: 'reader1', + lastname: 'reader1', + email: 'reader1@strapi.io', + password: 'Reader1', + isActive: true, + roles: [uploadReaderRole.id], + }); + + users.u2 = await utils.createUser({ + firstname: 'reader2', + lastname: 'reader2', + email: 'reader2@strapi.io', + password: 'Reader2', + isActive: true, + roles: [uploadReaderRole.id], + }); + + // Users' requests + + u1Req = await createAuthRequest({ + strapi, + userInfo: { email: 'reader1@strapi.io', password: 'Reader1' }, + }); + + u2Req = await createAuthRequest({ + strapi, + userInfo: { email: 'reader2@strapi.io', password: 'Reader2' }, + }); + }); + + // Cleanup test fixtures + afterAll(async () => { + await utils.deleteUsersById([users.u1.id, users.u2.id]); + await utils.deleteRolesById([uploadReaderRole.id]); + }); + test('GET /upload/files => Find files', async () => { const res = await rq({ method: 'GET', url: '/upload/files' }); @@ -73,5 +151,23 @@ describe('Upload', () => { }); res.body.results.forEach((file) => expect(file.folder).toBeDefined()); }); + + test(`Using custom conditions don't trigger validation errors for dynamically added fields`, async () => { + const res = await u1Req({ method: 'GET', url: '/upload/files' }); + + // The request succeed, no validation error + expect(res.statusCode).toBe(200); + + // No data is returned, the condition is successfully applied (u1 did not upload any file) + expect(res.body).toEqual({ + results: [], + pagination: { + page: expect.any(Number), + pageSize: expect.any(Number), + pageCount: expect.any(Number), + total: expect.any(Number), + }, + }); + }); }); });