Merge pull request #20458 from strapi/chore/merge-develop-to-v5-main

chore: merge develop into v5/main

docs/docs/docs/01-core/admin/01-ee/00-intro.md

@@ -34,6 +34,7 @@ Everytime a new EE feature is added in Strapi, in the settings menu, you should
             },
             to: '/settings/purchase-new-ee-feature',
             id: 'new-ee-feature',
+            // TODO: to replace with another name in v5
             lockIcon: true,
           },
         ]

packages/core/admin/admin/src/constants.ts

@@ -125,7 +125,7 @@ export const HOOKS = {
 
 export interface SettingsMenuLink
   extends Omit<StrapiAppSettingLink, 'Component' | 'permissions' | 'lockIcon'> {
-  lockIcon?: boolean;
+  lockIcon?: boolean; // TODO: to replace with another name in v5
 }
 
 export type SettingsMenu = {
@@ -164,7 +164,7 @@ export const SETTINGS_LINKS_CE = (): SettingsMenu => ({
             intlLabel: { id: 'Settings.sso.title', defaultMessage: 'Single Sign-On' },
             to: '/settings/purchase-single-sign-on',
             id: 'sso-purchase-page',
-            lockIcon: true,
+            lockIcon: true, // TODO: to replace with another name in v5
           },
         ]
       : []),
@@ -189,7 +189,7 @@ export const SETTINGS_LINKS_CE = (): SettingsMenu => ({
             intlLabel: { id: 'global.auditLogs', defaultMessage: 'Audit Logs' },
             to: '/settings/purchase-audit-logs',
             id: 'auditLogs-purchase-page',
-            lockIcon: true,
+            lockIcon: true, // TODO: to replace with another name in v5
           },
         ]
       : []),

packages/core/admin/admin/src/hooks/useSettingsMenu.ts

@@ -30,7 +30,7 @@ interface SettingsMenuLinkWithPermissions extends SettingsMenuLink {
 }
 
 interface StrapiAppSettingsLink extends IStrapiAppSettingLink {
-  lockIcon?: never;
+  lockIcon?: never; // TODO: to replace with another name in v5
   hasNotification?: never;
 }
 

packages/core/core/src/middlewares/__tests__/security.test.ts

@@ -0,0 +1,67 @@
+import Koa from 'koa';
+import request from 'supertest';
+import { security } from '../security';
+
+const parseCspHeader = (csp: string) =>
+  Object.fromEntries(
+    csp
+      .split(';')
+      .map((directive) => directive.split(' '))
+      .map(([k, ...v]) => [k, v])
+  );
+
+describe('Security middleware', () => {
+  describe('Content security policy', () => {
+    // GIVEN
+    const app = new Koa();
+    const securityMiddleware = security(
+      {
+        contentSecurityPolicy: {
+          useDefaults: true,
+          directives: {
+            'script-src': ["'self'", 'https://cdn.custom.com'],
+            upgradeInsecureRequests: null,
+          },
+        },
+      },
+      {
+        strapi: {
+          plugin: () => null,
+        } as any,
+      }
+    )!;
+
+    // WHEN
+    app.use(securityMiddleware);
+    const agent = request.agent(app.callback());
+
+    // THEN
+    it.each(['/', '/admin', '/api'])(
+      'includes user custom CSP directives in GET %s response',
+      async (path) => {
+        await agent.get(path).expect((req) => {
+          const csp = parseCspHeader(req.header['content-security-policy']);
+          expect(csp['script-src']).toContain('https://cdn.custom.com');
+        });
+      }
+    );
+
+    it('includes required default CSP directives in GET /admin response', async () => {
+      await agent.get('/admin').expect((req) => {
+        const csp = parseCspHeader(req.header['content-security-policy']);
+        expect(csp['script-src']).toContain("'unsafe-inline'");
+        expect(csp['connect-src']).toContain('ws:');
+      });
+    });
+
+    it('includes required default CSP directives in GET /documentation response', async () => {
+      await agent.get('/documentation').expect((req) => {
+        const csp = parseCspHeader(req.header['content-security-policy']);
+        expect(csp['script-src']).toContain("'unsafe-inline'");
+        expect(csp['script-src']).toContain('cdn.jsdelivr.net');
+        expect(csp['img-src']).toContain('strapi.io');
+        expect(csp['img-src']).toContain('cdn.jsdelivr.net');
+      });
+    });
+  });
+});

packages/core/core/src/middlewares/security.ts

@@ -1,4 +1,4 @@
-import { defaultsDeep, merge } from 'lodash/fp';
+import { defaultsDeep, mergeWith } from 'lodash/fp';
 import helmet, { KoaHelmet } from 'koa-helmet';
 
 import type { Core } from '@strapi/types';
@@ -29,6 +29,14 @@ const defaults: Config = {
   },
 };
 
+const mergeConfig = (existingConfig: Config, newConfig: Config) => {
+  return mergeWith(
+    (obj, src) => (Array.isArray(obj) && Array.isArray(src) ? obj.concat(src) : undefined),
+    existingConfig,
+    newConfig
+  );
+};
+
 export const security: Core.MiddlewareFactory<Config> =
   (config, { strapi }) =>
   (ctx, next) => {
@@ -63,7 +71,7 @@ export const security: Core.MiddlewareFactory<Config> =
 
     // TODO: we shouldn't combine playground exceptions with documentation for all routes, we should first check the path and then return exceptions specific to that
     if (ctx.method === 'GET' && specialPaths.some((str) => ctx.path.startsWith(str))) {
-      helmetConfig = merge(helmetConfig, {
+      helmetConfig = mergeConfig(helmetConfig, {
         crossOriginEmbedderPolicy: false, // TODO: only use this for graphql playground
         contentSecurityPolicy: {
           directives,
@@ -80,11 +88,11 @@ export const security: Core.MiddlewareFactory<Config> =
      * that are part of the admin route.
      */
     if (
-      process.env.NODE_ENV === 'development' &&
+      ['development', 'test'].includes(process.env.NODE_ENV ?? '') &&
       ctx.method === 'GET' &&
       ['/admin'].some((str) => ctx.path.startsWith(str))
     ) {
-      helmetConfig = merge(helmetConfig, {
+      helmetConfig = mergeConfig(helmetConfig, {
         contentSecurityPolicy: {
           directives: {
             'script-src': ["'self'", "'unsafe-inline'"],

packages/core/upload/server/src/controllers/admin-file.ts

@@ -25,10 +25,17 @@ export default {
       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 async.pipe(
-    const query = await pm.sanitizeQuery(pmQuery);
+      // 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);
 

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),
+        },
+      });
+    });
   });
 });

tests/e2e/tests/content-type-builder/collection-type/edit-collection-type.spec.ts

@@ -5,7 +5,7 @@ import { waitForRestart } from '../../../utils/restart';
 import { resetFiles } from '../../../utils/file-reset';
 import { createCollectionType, navToHeader, skipCtbTour } from '../../../utils/shared';
 
-test.describe('Edit collection type', () => {
+test.skip('Edit collection type', () => {
   // use a name with a capital and a space to ensure we also test the kebab-casing conversion for api ids
   const ctName = 'Secret Document';
 

tests/e2e/tests/content-type-builder/single-type/edit-single-type.spec.ts

@@ -5,7 +5,7 @@ import { waitForRestart } from '../../../utils/restart';
 import { resetFiles } from '../../../utils/file-reset';
 import { createSingleType, navToHeader, skipCtbTour } from '../../../utils/shared';
 
-test.describe('Edit single type', () => {
+test.skip('Edit single type', () => {
   // use a name with a capital and a space to ensure we also test the kebab-casing conversion for api ids
   const ctName = 'Secret Document';
 
@@ -36,7 +36,7 @@ test.describe('Edit single type', () => {
   });
 
   test('Can toggle internationalization', async ({ page }) => {
-    await page.getByRole('button', { name: 'Edit' }).click();
+    await page.getByRole('button', { name: 'Edit', exact: true }).click();
     await page.getByRole('tab', { name: 'Advanced settings' }).click();
     await page.getByText('Internationalization').click();
     await page.getByRole('button', { name: 'Finish' }).click();
@@ -47,7 +47,7 @@ test.describe('Edit single type', () => {
   });
 
   test('Can toggle draft&publish', async ({ page }) => {
-    await page.getByRole('button', { name: 'Edit' }).click();
+    await page.getByRole('button', { name: 'Edit', exact: true }).click();
     await page.getByRole('tab', { name: 'Advanced settings' }).click();
     await page.getByText('Draft & publish').click();
     await page.getByRole('button', { name: 'Yes, disable' }).click();