TechDebt: Update Multer middleware and OpenAPI spec for file imports

api/src/app.ts

@@ -17,6 +17,16 @@ import { getLogger } from './utils/logger';
 
 const defaultLog = getLogger('app');
 
+/**
+ * Extending express request type.
+ *
+ */
+declare module 'express-serve-static-core' {
+  interface Request {
+    files: Express.Multer.File[];
+  }
+}
+
 const HOST = process.env.API_HOST;
 const PORT = Number(process.env.API_PORT);
 
@@ -84,11 +94,22 @@ const openAPIFramework = initialize({
           return next(error);
         }
 
-        if (req.files && req.files.length) {
-          // Set original request file field to empty string to satisfy OpenAPI validation
-          // See: https://www.npmjs.com/package/express-openapi#argsconsumesmiddleware
-          (req.files as Express.Multer.File[]).forEach((file) => (req.body[file.fieldname] = ''));
-        }
+        /**
+         * @see https://www.npmjs.com/package/express-openapi#argsconsumesmiddleware
+         *
+         * Multer transforms and moves the incoming files from `req.body.media` --> `req.files`.
+         *
+         * OpenAPI only allows validation on specific parts of the request object (requestBody / parameters...) this excludes the contents of `req.files`.
+         * To get around this we re-assign `req.body.media` to the Multer transformed files stored in `req.files`.
+         *
+         * Files can be accessed via `req.body.media` OR `req.files`.
+         *
+         */
+
+        const multerFiles = req.files ?? [];
+
+        req.files = multerFiles;
+        req.body.media = multerFiles;
 
         return next();
       });

api/src/openapi/schemas/file.ts

@@ -0,0 +1,64 @@
+import { OpenAPIV3 } from 'openapi-types';
+
+/**
+ * Shared file schema for all file formats.
+ *
+ * @see MIME types: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
+ *
+ */
+export const fileSchema: OpenAPIV3.SchemaObject = {
+  type: 'object',
+  description: 'Schema defining the structure of a file object uploaded and transformed via Multer middleware.',
+  required: ['fieldname', 'originalname', 'mimetype', 'buffer'],
+  properties: {
+    fieldname: {
+      description: 'Property name of incomming file upload. Multer expects `media`.',
+      type: 'string',
+      enum: ['media']
+    },
+    originalname: {
+      description: 'Original name of the file with its extension as provided by the uploader.',
+      type: 'string',
+      example: 'import-file.txt'
+    },
+    encoding: {
+      description: 'Encoding of the file content.',
+      type: 'string',
+      example: '7bit'
+    },
+    mimetype: {
+      description: 'MIME type of the file, such as `text/csv` for CSV files.',
+      type: 'string',
+      example: 'text/csv'
+    },
+    buffer: {
+      description: 'The content of the file transformed into a Buffer object for further processing.',
+      type: 'object',
+      format: 'buffer'
+    },
+    size: {
+      description: 'Size of the file in kilobytes (KB). Minimum value is 1 KB.',
+      type: 'integer',
+      minimum: 1
+    }
+  }
+};
+
+/**
+ * CSV file schema.
+ *
+ * Note: This could be extended further to have a regex that validates the `originalname` ends with `.csv`
+ * but I think the enum value for `mimetype` is enough for our needs.
+ *
+ */
+export const csvFileSchema: OpenAPIV3.SchemaObject = {
+  ...fileSchema,
+  properties: {
+    ...fileSchema.properties,
+    mimetype: {
+      description: 'CSV File type.',
+      type: 'string',
+      enum: ['text/csv']
+    }
+  }
+};

api/src/paths/project/{projectId}/attachments/report/upload.test.ts

@@ -38,41 +38,6 @@ describe('uploadMedia', () => {
     }
   } as any;
 
-  it('should throw an error when files are missing', async () => {
-    sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result({ ...mockReq, files: [] }, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Missing upload data');
-    }
-  });
-
-  it('should throw an error when file format incorrect', async () => {
-    sinon.stub(db, 'getDBConnection').returns({
-      ...dbConnectionObj,
-      systemUserId: () => {
-        return 20;
-      }
-    });
-
-    sinon.stub(file_utils, 'scanFileForVirus').resolves(false);
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result(mockReq, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Malicious content detected, upload cancelled');
-    }
-  });
-
   it('should throw an error if failure occurs', async () => {
     sinon.stub(db, 'getDBConnection').returns({
       ...dbConnectionObj,

api/src/paths/project/{projectId}/attachments/report/upload.ts

@@ -3,6 +3,7 @@ import { Operation } from 'express-openapi';
 import { PROJECT_PERMISSION, SYSTEM_ROLE } from '../../../../../constants/roles';
 import { getDBConnection } from '../../../../../database/db';
 import { HTTP400 } from '../../../../../errors/http-error';
+import { fileSchema } from '../../../../../openapi/schemas/file';
 import { authorizeRequestHandler } from '../../../../../request-handlers/security/authorization';
 import { AttachmentService } from '../../../../../services/attachment-service';
 import { scanFileForVirus, uploadFileToS3 } from '../../../../../utils/file-utils';
@@ -57,8 +58,11 @@ POST.apiDoc = {
           required: ['media', 'attachmentMeta'],
           properties: {
             media: {
-              type: 'string',
-              format: 'binary'
+              description: 'Attachment report upload file.',
+              type: 'array',
+              minItems: 1,
+              maxItems: 1,
+              items: fileSchema
             },
             attachmentMeta: {
               type: 'object',
@@ -140,14 +144,7 @@ POST.apiDoc = {
  */
 export function uploadMedia(): RequestHandler {
   return async (req, res) => {
-    const rawMediaArray: Express.Multer.File[] = req.files as Express.Multer.File[];
-
-    if (!rawMediaArray || !rawMediaArray.length) {
-      // no media objects included, skipping media upload step
-      throw new HTTP400('Missing upload data');
-    }
-
-    const rawMediaFile: Express.Multer.File = rawMediaArray[0];
+    const rawMediaFile = req.files[0];
 
     defaultLog.debug({
       label: 'uploadMedia',

api/src/paths/project/{projectId}/attachments/upload.test.ts

@@ -36,41 +36,6 @@ describe('uploadMedia', () => {
     body: {}
   } as any;
 
-  it('should throw an error when files are missing', async () => {
-    sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result({ ...mockReq, files: [] }, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Missing upload data');
-    }
-  });
-
-  it('should throw an error when file format incorrect', async () => {
-    sinon.stub(db, 'getDBConnection').returns({
-      ...dbConnectionObj,
-      systemUserId: () => {
-        return 20;
-      }
-    });
-
-    sinon.stub(file_utils, 'scanFileForVirus').resolves(false);
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result(mockReq, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Malicious content detected, upload cancelled');
-    }
-  });
-
   it('should throw an error if failure occurs', async () => {
     sinon.stub(db, 'getDBConnection').returns({
       ...dbConnectionObj,

api/src/paths/project/{projectId}/attachments/upload.ts

@@ -4,6 +4,7 @@ import { ATTACHMENT_TYPE } from '../../../../constants/attachments';
 import { PROJECT_PERMISSION, SYSTEM_ROLE } from '../../../../constants/roles';
 import { getDBConnection } from '../../../../database/db';
 import { HTTP400 } from '../../../../errors/http-error';
+import { fileSchema } from '../../../../openapi/schemas/file';
 import { authorizeRequestHandler } from '../../../../request-handlers/security/authorization';
 import { AttachmentService } from '../../../../services/attachment-service';
 import { scanFileForVirus, uploadFileToS3 } from '../../../../utils/file-utils';
@@ -58,8 +59,11 @@ POST.apiDoc = {
           required: ['media'],
           properties: {
             media: {
-              type: 'string',
-              format: 'binary'
+              description: 'Attachment import file.',
+              type: 'array',
+              minItems: 1,
+              maxItems: 1,
+              items: fileSchema
             }
           }
         }
@@ -115,14 +119,7 @@ POST.apiDoc = {
  */
 export function uploadMedia(): RequestHandler {
   return async (req, res) => {
-    const rawMediaArray: Express.Multer.File[] = req.files as Express.Multer.File[];
-
-    if (!rawMediaArray || !rawMediaArray.length) {
-      // no media objects included, skipping media upload step
-      throw new HTTP400('Missing upload data');
-    }
-
-    const rawMediaFile: Express.Multer.File = rawMediaArray[0];
+    const rawMediaFile = req.files[0];
 
     defaultLog.debug({
       label: 'uploadMedia',

api/src/paths/project/{projectId}/survey/{surveyId}/attachments/keyx/upload.test.ts

@@ -41,20 +41,6 @@ describe('uploadMedia', () => {
 
   const mockBctwResponse = { totalKeyxFiles: 2, newRecords: 1, existingRecords: 1 };
 
-  it('should throw an error when files are missing', async () => {
-    sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
-
-    try {
-      const result = upload.uploadKeyxMedia();
-
-      await result({ ...mockReq, files: [] }, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Missing upload data');
-    }
-  });
-
   it('should throw an error when file has malicious content', async () => {
     sinon.stub(db, 'getDBConnection').returns({
       ...dbConnectionObj,

api/src/paths/project/{projectId}/survey/{surveyId}/attachments/keyx/upload.ts

@@ -4,6 +4,7 @@ import { ATTACHMENT_TYPE } from '../../../../../../../constants/attachments';
 import { PROJECT_PERMISSION, SYSTEM_ROLE } from '../../../../../../../constants/roles';
 import { getDBConnection } from '../../../../../../../database/db';
 import { HTTP400 } from '../../../../../../../errors/http-error';
+import { fileSchema } from '../../../../../../../openapi/schemas/file';
 import { authorizeRequestHandler } from '../../../../../../../request-handlers/security/authorization';
 import { AttachmentService } from '../../../../../../../services/attachment-service';
 import { BctwService, IBctwUser } from '../../../../../../../services/bctw-service';
@@ -70,8 +71,11 @@ POST.apiDoc = {
           required: ['media'],
           properties: {
             media: {
-              type: 'string',
-              format: 'binary'
+              description: 'Keyx import file.',
+              type: 'array',
+              minItems: 1,
+              maxItems: 1,
+              items: fileSchema
             }
           }
         }
@@ -131,14 +135,7 @@ POST.apiDoc = {
  */
 export function uploadKeyxMedia(): RequestHandler {
   return async (req, res) => {
-    const rawMediaArray: Express.Multer.File[] = req.files as Express.Multer.File[];
-
-    if (!rawMediaArray?.length) {
-      // no media objects included, skipping media upload step
-      throw new HTTP400('Missing upload data');
-    }
-
-    const rawMediaFile: Express.Multer.File = rawMediaArray[0];
+    const rawMediaFile = req.files[0];
 
     defaultLog.debug({
       label: 'uploadKeyxMedia',

api/src/paths/project/{projectId}/survey/{surveyId}/attachments/report/upload.test.ts

@@ -16,70 +16,6 @@ describe('uploadMedia', () => {
     sinon.restore();
   });
 
-  it('should throw an error when files are missing', async () => {
-    const dbConnectionObj = getMockDBConnection();
-    sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
-
-    const mockReq = {
-      keycloak_token: {},
-      params: {
-        projectId: 1,
-        attachmentId: 2
-      },
-      files: [],
-      body: {
-        attachmentType: 'Other'
-      }
-    } as any;
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result(mockReq, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Missing upload data');
-    }
-  });
-
-  it('should throw an error when file format incorrect', async () => {
-    const dbConnectionObj = getMockDBConnection();
-    sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
-
-    const mockReq = {
-      keycloak_token: {},
-      params: {
-        projectId: 1,
-        attachmentId: 2
-      },
-      files: [
-        {
-          fieldname: 'media',
-          originalname: 'test.txt',
-          encoding: '7bit',
-          mimetype: 'text/plain',
-          size: 340
-        }
-      ],
-      body: {
-        attachmentType: 'Other'
-      }
-    } as any;
-
-    sinon.stub(file_utils, 'scanFileForVirus').resolves(false);
-
-    try {
-      const result = upload.uploadMedia();
-
-      await result(mockReq, null as unknown as any, null as unknown as any);
-      expect.fail();
-    } catch (actualError) {
-      expect((actualError as HTTPError).status).to.equal(400);
-      expect((actualError as HTTPError).message).to.equal('Malicious content detected, upload cancelled');
-    }
-  });
-
   it('should throw an error if failure occurs', async () => {
     const dbConnectionObj = getMockDBConnection();
     sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);