Remove Content-Type header check in Files POST requests (#16176)

* remove request header check in files POST

* basic test

* it's filename_download instead of file_download

* unused vars begone

* make type required when creating file via JSON

* fix blackbox test

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

api/src/controllers/files.ts

@@ -5,7 +5,7 @@ import express, { RequestHandler } from 'express';
 import Joi from 'joi';
 import path from 'path';
 import env from '../env';
-import { ForbiddenException, InvalidPayloadException, UnsupportedMediaTypeException } from '../exceptions';
+import { ForbiddenException, InvalidPayloadException } from '../exceptions';
 import { respond } from '../middleware/respond';
 import useCollection from '../middleware/use-collection';
 import { validateBatch } from '../middleware/validate-batch';
@@ -121,10 +121,6 @@ router.post(
 	'/',
 	asyncHandler(multipartHandler),
 	asyncHandler(async (req, res, next) => {
-		if (req.is('multipart/form-data') === false) {
-			throw new UnsupportedMediaTypeException(`Unsupported Content-Type header`);
-		}
-
 		const service = new FilesService({
 			accountability: req.accountability,
 			schema: req.schema,

api/src/services/files.test.ts

@@ -1,7 +1,8 @@
 import exifr from 'exifr';
 import knex, { Knex } from 'knex';
 import { MockClient, Tracker, getTracker } from 'knex-mock-client';
-import { FilesService } from '.';
+import { FilesService, ItemsService } from '.';
+import { InvalidPayloadException } from '../exceptions';
 
 jest.mock('exifr');
 jest.mock('../../src/database/index', () => {
@@ -20,9 +21,49 @@ describe('Integration Tests', () => {
 
 	afterEach(() => {
 		tracker.reset();
+		jest.clearAllMocks();
 	});
 
 	describe('Services / Files', () => {
+		describe('createOne', () => {
+			let service: FilesService;
+			let superCreateOne: jest.SpyInstance;
+
+			beforeEach(() => {
+				service = new FilesService({
+					knex: db,
+					schema: { collections: {}, relations: [] },
+				});
+				superCreateOne = jest.spyOn(ItemsService.prototype, 'createOne').mockImplementation(jest.fn());
+			});
+
+			it('throws InvalidPayloadException when "type" is not provided', async () => {
+				try {
+					await service.createOne({
+						title: 'Test File',
+						storage: 'local',
+						filename_download: 'test_file',
+					});
+				} catch (err: any) {
+					expect(err).toBeInstanceOf(InvalidPayloadException);
+					expect(err.message).toBe('"type" is required');
+				}
+
+				expect(superCreateOne).not.toHaveBeenCalled();
+			});
+
+			it('creates a file entry when "type" is provided', async () => {
+				await service.createOne({
+					title: 'Test File',
+					storage: 'local',
+					filename_download: 'test_file',
+					type: 'application/octet-stream',
+				});
+
+				expect(superCreateOne).toHaveBeenCalled();
+			});
+		});
+
 		describe('getMetadata', () => {
 			let service: FilesService;
 			let exifrParseSpy: jest.SpyInstance<any>;

api/src/services/files.ts

@@ -10,7 +10,7 @@ import { promisify } from 'util';
 import { lookup } from 'dns';
 import emitter from '../emitter';
 import env from '../env';
-import { ForbiddenException, ServiceUnavailableException } from '../exceptions';
+import { ForbiddenException, InvalidPayloadException, ServiceUnavailableException } from '../exceptions';
 import logger from '../logger';
 import storage from '../storage';
 import { AbstractServiceOptions, File, PrimaryKey, MutationOptions, Metadata } from '../types';
@@ -268,6 +268,19 @@ export class FilesService extends ItemsService {
 		return await this.uploadOne(fileResponse.data, payload);
 	}
 
+	/**
+	 * Create a file (only applicable when it is not a multipart/data POST request)
+	 * Useful for associating metadata with existing file in storage
+	 */
+	async createOne(data: Partial<File>, opts?: MutationOptions): Promise<PrimaryKey> {
+		if (!data.type) {
+			throw new InvalidPayloadException(`"type" is required`);
+		}
+
+		const key = await super.createOne(data, opts);
+		return key;
+	}
+
 	/**
 	 * Delete a file
 	 */

tests-blackbox/routes/collections/files.test.ts

@@ -0,0 +1,63 @@
+import { getUrl } from '@common/config';
+import vendors from '@common/get-dbs-to-test';
+import * as common from '@common/index';
+import request from 'supertest';
+
+describe('/files', () => {
+	describe('POST /', () => {
+		describe('when only request body is provided without any multipart files', () => {
+			describe('returns created file when required properties are included', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const payload = {
+						title: 'Test File',
+						storage: 'local',
+						filename_download: 'test_file',
+						type: 'application/octet-stream',
+					};
+
+					// Action
+					const response = await request(getUrl(vendor))
+						.post(`/files`)
+						.send(payload)
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					// Assert
+					expect(response.statusCode).toBe(200);
+					expect(response.body).toMatchObject({
+						data: {
+							title: payload.title,
+							storage: payload.storage,
+							filename_download: payload.filename_download,
+						},
+					});
+				});
+			});
+			describe('returns code: FAILED_VALIDATION when required property "storage" is not included', () => {
+				it.each(vendors)('%s', async (vendor) => {
+					// Setup
+					const payload = { title: 'Test File', filename_download: 'test_file', type: 'application/octet-stream' };
+
+					// Action
+					const response = await request(getUrl(vendor))
+						.post(`/files`)
+						.send(payload)
+						.set('Authorization', `Bearer ${common.USER.ADMIN.TOKEN}`);
+
+					// Assert
+					expect(response.statusCode).toBe(400);
+					expect(response.body).toMatchObject({
+						errors: [
+							{
+								message: '"storage" is required',
+								extensions: {
+									code: 'FAILED_VALIDATION',
+								},
+							},
+						],
+					});
+				});
+			});
+		});
+	});
+});