fix: sign file URLs in upload service and in graphql (#23834)

* fix: add URL signing support for media files in upload service and GraphQL resolvers

* fix: graphql api test

* fix: graphql association

* chore: remove unnecessary tests

* fix: sign when querying media files directly

* chore: removed signedData

* fix: backend ci

* fix: pr feedback

* fix: signed images not loading in media input

* fix: make gql unit test pass ci

* chore: move rawData

packages/core/upload/admin/src/components/MediaLibraryInput/Carousel/CarouselAsset.tsx

@@ -57,7 +57,10 @@ export const CarouselAsset = ({ asset }: { asset: FileAsset }) => {
     if (!assetUrl) return null;
 
     // Adding a param to the url to bust the cache and force the refresh of the image when replaced
-    const cacheBustedUrl = `${assetUrl}${assetUrl.includes('?') ? '&' : '?'}updatedAt=${asset.updatedAt}`;
+    // Only add updatedAt parameter if the URL is not signed to prevent signature invalidation
+    const cacheBustedUrl = asset.isUrlSigned
+      ? assetUrl
+      : `${assetUrl}${assetUrl.includes('?') ? '&' : '?'}updatedAt=${asset.updatedAt}`;
 
     return (
       <Box

packages/core/upload/server/src/services/__tests__/upload/formatFileInfo.test.ts

@@ -1,130 +1,128 @@
 import createUploadService from '../../upload';
 import imageManipulation from '../../image-manipulation';
 
-const uploadService = createUploadService({} as any);
-
-describe('Upload service', () => {
-  beforeAll(() => {
-    global.strapi = {
-      plugins: {
-        upload: {
-          services: {
-            'image-manipulation': imageManipulation,
-            file: {
-              getFolderPath: () => '/a-folder-path',
-            },
-          },
+// Set up mock before service creation
+global.strapi = {
+  plugins: {
+    upload: {
+      services: {
+        'image-manipulation': imageManipulation,
+        file: {
+          getFolderPath: () => '/a-folder-path',
         },
       },
-    } as any;
+    },
+  },
+  plugin: (name: string) => global.strapi.plugins[name],
+} as any;
+
+const uploadService = createUploadService({} as any);
+
+describe('formatFileInfo', () => {
+  test('Generates hash', async () => {
+    const fileData = {
+      filename: 'File Name.png',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
+
+    expect(await uploadService.formatFileInfo(fileData)).toMatchObject({
+      name: 'File Name.png',
+      hash: expect.stringContaining('File_Name'),
+      ext: '.png',
+      mime: 'image/png',
+      size: 1000,
+    });
   });
 
-  describe('formatFileInfo', () => {
-    test('Generates hash', async () => {
-      const fileData = {
-        filename: 'File Name.png',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
+  test('Replaces reserved and unsafe characters for URLs and files in hash', async () => {
+    const fileData = {
+      filename: 'File%&Näme.png',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
 
-      expect(await uploadService.formatFileInfo(fileData)).toMatchObject({
-        name: 'File Name.png',
-        hash: expect.stringContaining('File_Name'),
-        ext: '.png',
-        mime: 'image/png',
-        size: 1000,
-      });
+    expect(await uploadService.formatFileInfo(fileData)).toMatchObject({
+      name: 'File%&Näme.png',
+      hash: expect.stringContaining('File_and_Naeme'),
+      ext: '.png',
+      mime: 'image/png',
+      size: 1000,
     });
+  });
 
-    test('Replaces reserved and unsafe characters for URLs and files in hash', async () => {
-      const fileData = {
-        filename: 'File%&Näme.png',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
+  test('Prevents invalid characters in file name', async () => {
+    const fileData = {
+      filename: 'filename.png\u0000',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
 
-      expect(await uploadService.formatFileInfo(fileData)).toMatchObject({
-        name: 'File%&Näme.png',
-        hash: expect.stringContaining('File_and_Naeme'),
-        ext: '.png',
-        mime: 'image/png',
-        size: 1000,
-      });
+    expect(uploadService.formatFileInfo(fileData)).rejects.toThrowError(
+      'File name contains invalid characters'
+    );
+  });
+
+  test('Overrides name with fileInfo', async () => {
+    const fileData = {
+      filename: 'File Name.png',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
+
+    const fileInfo = {
+      name: 'Custom File Name.png',
+    };
+
+    expect(await uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
+      name: fileInfo.name,
+      hash: expect.stringContaining('Custom_File_Name'),
+      ext: '.png',
+      mime: 'image/png',
+      size: 1000,
     });
+  });
 
-    test('Prevents invalid characters in file name', async () => {
-      const fileData = {
-        filename: 'filename.png\u0000',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
+  test('Sets alternativeText and caption', async () => {
+    const fileData = {
+      filename: 'File Name.png',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
 
-      expect(uploadService.formatFileInfo(fileData)).rejects.toThrowError(
-        'File name contains invalid characters'
-      );
+    const fileInfo = {
+      alternativeText: 'some text',
+      caption: 'caption this',
+    };
+
+    expect(await uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
+      name: 'File Name.png',
+      caption: fileInfo.caption,
+      alternativeText: fileInfo.alternativeText,
+      hash: expect.stringContaining('File_Name'),
+      ext: '.png',
+      mime: 'image/png',
+      size: 1000,
     });
+  });
 
-    test('Overrides name with fileInfo', async () => {
-      const fileData = {
-        filename: 'File Name.png',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
+  test('Set a path folder', async () => {
+    const fileData = {
+      filename: 'File Name.png',
+      type: 'image/png',
+      size: 1000 * 1000,
+    };
 
-      const fileInfo = {
-        name: 'Custom File Name.png',
-      };
+    const fileMetas = {
+      path: 'folder',
+    };
 
-      expect(await uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
-        name: fileInfo.name,
-        hash: expect.stringContaining('Custom_File_Name'),
-        ext: '.png',
-        mime: 'image/png',
-        size: 1000,
-      });
-    });
-
-    test('Sets alternativeText and caption', async () => {
-      const fileData = {
-        filename: 'File Name.png',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
-
-      const fileInfo = {
-        alternativeText: 'some text',
-        caption: 'caption this',
-      };
-
-      expect(await uploadService.formatFileInfo(fileData, fileInfo)).toMatchObject({
-        name: 'File Name.png',
-        caption: fileInfo.caption,
-        alternativeText: fileInfo.alternativeText,
-        hash: expect.stringContaining('File_Name'),
-        ext: '.png',
-        mime: 'image/png',
-        size: 1000,
-      });
-    });
-
-    test('Set a path folder', async () => {
-      const fileData = {
-        filename: 'File Name.png',
-        type: 'image/png',
-        size: 1000 * 1000,
-      };
-
-      const fileMetas = {
-        path: 'folder',
-      };
-
-      expect(await uploadService.formatFileInfo(fileData, {}, fileMetas)).toMatchObject({
-        name: 'File Name.png',
-        ext: '.png',
-        mime: 'image/png',
-        size: 1000,
-        path: expect.stringContaining('folder'),
-      });
+    expect(await uploadService.formatFileInfo(fileData, {}, fileMetas)).toMatchObject({
+      name: 'File Name.png',
+      ext: '.png',
+      mime: 'image/png',
+      size: 1000,
+      path: expect.stringContaining('folder'),
     });
   });
 });

packages/core/upload/server/src/services/__tests__/upload/uploadImage.test.ts

@@ -5,42 +5,47 @@ import _ from 'lodash';
 import createUploadService from '../../upload';
 import imageManipulation from '../../image-manipulation';
 
+const defaultConfig = {
+  'plugin::upload': {
+    breakpoints: {
+      large: 1000,
+      medium: 750,
+    },
+  },
+};
+
+// Set up initial mock before service creation
+global.strapi = {
+  config: {
+    get: (path: any, defaultValue: any) => _.get(defaultConfig, path, defaultValue),
+  },
+  plugins: {
+    upload: {
+      services: {
+        provider: {
+          upload: jest.fn(),
+        },
+        upload: {
+          getSettings: () => ({ responsiveDimensions: false }),
+        },
+        'image-manipulation': imageManipulation,
+      },
+    },
+  },
+  plugin: (name: string) => global.strapi.plugins[name],
+} as any;
+
 const uploadService = createUploadService({} as any);
 
 const imageFilePath = path.join(__dirname, './image.png');
-
 const tmpWorkingDirectory = path.join(__dirname, './tmp');
 
 function mockUploadProvider(uploadFunc: any, props?: any) {
   const { responsiveDimensions = false } = props || {};
 
-  const defaultConfig = {
-    'plugin::upload': {
-      breakpoints: {
-        large: 1000,
-        medium: 750,
-      },
-    },
-  };
-
-  global.strapi = {
-    config: {
-      get: (path: any, defaultValue: any) => _.get(defaultConfig, path, defaultValue),
-    },
-    plugins: {
-      upload: {
-        services: {
-          provider: {
-            upload: uploadFunc,
-          },
-          upload: {
-            getSettings: () => ({ responsiveDimensions }),
-          },
-          'image-manipulation': imageManipulation,
-        },
-      },
-    },
-  } as any;
+  // Only mutate the parts that depend on the parameters
+  global.strapi.plugins.upload.services.provider.upload = uploadFunc;
+  global.strapi.plugins.upload.services.upload.getSettings = () => ({ responsiveDimensions });
 }
 
 const getFileData = (filePath: string) => ({

packages/core/upload/server/src/services/extensions/utils.ts

@@ -3,6 +3,7 @@ import { async, traverseEntity } from '@strapi/utils';
 import type { Schema, UID } from '@strapi/types';
 
 import { getService } from '../../utils';
+import { FILE_MODEL_UID } from '../../constants';
 
 import type { File } from '../../types';
 
@@ -67,6 +68,17 @@ const signEntityMediaVisitor: SignEntityMediaVisitor = async (
  * @returns
  */
 const signEntityMedia = async (entity: any, uid: UID.Schema) => {
+  if (!entity) {
+    return entity;
+  }
+
+  // If the entity itself is a file, sign it directly
+  if (uid === FILE_MODEL_UID) {
+    const { signFileUrls } = getService('file');
+    return signFileUrls(entity);
+  }
+
+  // If the entity is a regular content type, look for media attributes
   const model = strapi.getModel(uid);
   return traverseEntity(
     // @ts-expect-error - FIXME: fix traverseEntity using wrong types

packages/core/upload/server/src/services/upload.ts

@@ -5,6 +5,7 @@ import fse from 'fs-extra';
 import _ from 'lodash';
 import { extension } from 'mime-types';
 import {
+  async,
   sanitize,
   contentTypes as contentTypesUtils,
   errors,
@@ -45,6 +46,8 @@ const { ApplicationError, NotFoundError } = errors;
 const { bytesToKbytes } = fileUtils;
 
 export default ({ strapi }: { strapi: Core.Strapi }) => {
+  const fileService = getService('file');
+
   const sendMediaMetrics = (data: Pick<File, 'caption' | 'alternativeText'>) => {
     if (_.has(data, 'caption') && !_.isEmpty(data.caption)) {
       strapi.telemetry.send('didSaveMediaWithCaption');
@@ -462,27 +465,45 @@ export default ({ strapi }: { strapi: Core.Strapi }) => {
     return res;
   }
 
-  function findOne(id: ID, populate = {}) {
+  async function findOne(id: ID, populate = {}) {
     const query = strapi.get('query-params').transform(FILE_MODEL_UID, {
       populate,
     });
 
-    return strapi.db.query(FILE_MODEL_UID).findOne({
+    const file = await strapi.db.query(FILE_MODEL_UID).findOne({
       where: { id },
       ...query,
     });
+
+    if (!file) return file;
+
+    // Sign file URLs if using private provider
+    return fileService.signFileUrls(file);
   }
 
-  function findMany(query: any = {}): Promise<File[]> {
-    return strapi.db
+  async function findMany(query: any = {}): Promise<File[]> {
+    const files = await strapi.db
       .query(FILE_MODEL_UID)
       .findMany(strapi.get('query-params').transform(FILE_MODEL_UID, query));
+
+    // Sign file URLs if using private provider
+    return async.map(files, (file: File) => fileService.signFileUrls(file));
   }
 
-  function findPage(query: any = {}) {
-    return strapi.db
+  async function findPage(query: any = {}) {
+    const result = await strapi.db
       .query(FILE_MODEL_UID)
       .findPage(strapi.get('query-params').transform(FILE_MODEL_UID, query));
+
+    // Sign file URLs if using private provider
+    const signedResults = await async.map(result.results, (file: File) =>
+      fileService.signFileUrls(file)
+    );
+
+    return {
+      ...result,
+      results: signedResults,
+    };
   }
 
   async function remove(file: File) {

packages/plugins/graphql/jest.config.js

@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = {
+  preset: '../../../jest-preset.unit.js',
+  displayName: 'Plugin GraphQL',
+  // passWithNoTests: true,
+};

packages/plugins/graphql/package.json

@@ -49,6 +49,8 @@
     "build:types:admin": "run -T tsc -p admin/tsconfig.build.json --emitDeclarationOnly",
     "clean": "run -T rimraf ./dist",
     "lint": "run -T eslint .",
+    "test:unit": "run -T jest --passWithNoTests",
+    "test:unit:watch": "run -T jest --watch",
     "watch": "run -T rollup -c -w"
   },
   "dependencies": {

packages/plugins/graphql/server/src/services/builders/resolvers/association.ts

@@ -76,7 +76,26 @@ export default ({ strapi }: Context) => {
           : {};
 
         const dbQuery = merge(defaultFilters, transformedQuery);
-        const data = await strapi.db?.query(contentTypeUID).load(parent, attributeName, dbQuery);
+
+        // Sign media URLs if upload plugin is available and using private provider
+        const data = await (async () => {
+          const rawData = await strapi.db
+            .query(contentTypeUID)
+            .load(parent, attributeName, dbQuery);
+          if (isMediaAttribute && strapi.plugin('upload')) {
+            const { signFileUrls } = strapi.plugin('upload').service('file');
+
+            if (Array.isArray(rawData)) {
+              return async.map(rawData, (item: any) => signFileUrls(item));
+            }
+
+            if (rawData) {
+              return signFileUrls(rawData);
+            }
+          }
+
+          return rawData;
+        })();
 
         const info = {
           args: sanitizedQuery,