Allow admins to upload files on any report

src/middleware/userAdminAccessMiddleware.js

@@ -16,9 +16,7 @@ import handleErrors from '../lib/apiErrorHandler';
 export default async function userAdminAccessMiddleware(req, res, next) {
   try {
     const { userId } = req.session;
-    if ((await validateUserAuthForAdmin(userId))) {
-      auditLogger.info(`User ${userId} successfully checked ADMIN access`);
-    } else {
+    if (!(await validateUserAuthForAdmin(userId))) {
       auditLogger.error(`User ${userId} attempted to access an ADMIN route without permission`);
       // consider sending a 404 rather than a 403 (Forbidden) to avoid confirming route
       return res.sendStatus(httpCodes.FORBIDDEN);

src/routes/files/handlers.js

@@ -9,6 +9,7 @@ import createFileMetaData, {
 import ActivityReportPolicy from '../../policies/activityReport';
 import { activityReportById } from '../../services/activityReports';
 import { userById } from '../../services/users';
+import { validateUserAuthForAdmin } from '../../services/accessValidation';
 import { auditLogger } from '../../logger';
 import { FILE_STATUSES } from '../../constants';
 
@@ -53,10 +54,10 @@ export const deleteHandler = async (req, res) => {
 
 export default async function uploadHandler(req, res) {
   const form = new multiparty.Form();
-  form.parse(req, async (error, fields, files) => {
+  await form.parse(req, async (error, fields, files) => {
     const { reportId } = fields;
     if (error) {
-      res.status(500).send(error);
+      return res.status(500).send(error);
     }
     let buffer;
     let metadata;
@@ -67,29 +68,25 @@ export default async function uploadHandler(req, res) {
     const report = await activityReportById(reportId);
     const authorization = new ActivityReportPolicy(user, report);
 
-    if (!authorization.canUpdate()) {
-      res.sendStatus(403);
-      return;
+    if (!(authorization.canUpdate() || (await validateUserAuthForAdmin(req.session.userId)))) {
+      return res.sendStatus(403);
     }
 
     try {
       if (!files.file) {
-        res.status(400).send({ error: 'file required' });
-        return;
+        return res.status(400).send({ error: 'file required' });
       }
       const { path, originalFilename, size } = files.file[0];
       if (!size) {
-        res.status(400).send({ error: 'fileSize required' });
+        return res.status(400).send({ error: 'fileSize required' });
       }
       if (!reportId) {
-        res.status(400).send({ error: 'reportId required' });
-        return;
+        return res.status(400).send({ error: 'reportId required' });
       }
       buffer = fs.readFileSync(path);
       type = await fileType.fromFile(path);
       if (!type) {
-        res.status(400).send('Could not determine file type');
-        return;
+        return res.status(400).send('Could not determine file type');
       }
       fileName = `${uuidv4()}.${type.ext}`;
       metadata = await createFileMetaData(

src/routes/files/handlers.test.js

@@ -3,7 +3,6 @@ import db, {
   File,
   ActivityReport,
   User,
-  Permission,
 } from '../../models';
 import app from '../../app';
 import { uploadFile, deleteFileFromS3, getPresignedURL } from '../../lib/s3';
@@ -12,8 +11,13 @@ import SCOPES from '../../middleware/scopeConstants';
 import { REPORT_STATUSES, FILE_STATUSES } from '../../constants';
 import ActivityReportPolicy from '../../policies/activityReport';
 import * as Files from '../../services/files';
+import { validateUserAuthForAdmin, validateUserAuthForAccess } from '../../services/accessValidation';
 
 jest.mock('../../policies/activityReport');
+jest.mock('../../services/accessValidation', () => ({
+  validateUserAuthForAdmin: jest.fn().mockResolvedValue(false),
+  validateUserAuthForAccess: jest.fn().mockResolvedValue(true),
+}));
 
 const request = require('supertest');
 
@@ -26,23 +30,6 @@ const mockUser = {
   hsesUserId: '2046',
   hsesUsername: '2046',
   homeRegionId: 1,
-  permissions: [
-    {
-      userId: 2046,
-      regionId: 5,
-      scopeId: SCOPES.READ_WRITE_REPORTS,
-    },
-    {
-      userId: 2046,
-      regionId: 6,
-      scopeId: SCOPES.READ_WRITE_REPORTS,
-    },
-    {
-      userId: 2046,
-      regionId: 14,
-      scopeId: SCOPES.SITE_ACCESS,
-    },
-  ],
 };
 
 const mockSession = jest.fn();
@@ -63,7 +50,7 @@ describe('File Upload', () => {
   let report;
   let fileId;
   beforeAll(async () => {
-    user = await User.create(mockUser, { include: [{ model: Permission, as: 'permissions' }] });
+    user = await User.create(mockUser);
     report = await ActivityReport.create(reportObject);
     process.env.NODE_ENV = 'test';
     process.env.BYPASS_AUTH = 'true';
@@ -76,7 +63,7 @@ describe('File Upload', () => {
     process.env = ORIGINAL_ENV; // restore original env
     await db.sequelize.close();
   });
-  afterEach(() => {
+  beforeEach(() => {
     jest.clearAllMocks();
   });
 
@@ -90,15 +77,35 @@ describe('File Upload', () => {
         canUpdate: () => true,
       }));
       uploadFile.mockResolvedValue({ key: 'key' });
-      await request(app)
+      const response = await request(app)
         .post('/api/files')
         .field('reportId', report.dataValues.id)
         .attach('file', `${__dirname}/testfiles/testfile.pdf`)
-        .expect(200)
-        .then((res) => {
-          fileId = res.body.id;
-          expect(uploadFile).toHaveBeenCalled();
-        });
+        .expect(200);
+      fileId = response.body.id;
+      expect(uploadFile).toHaveBeenCalled();
+      expect(mockAddToScanQueue).toHaveBeenCalled();
+      const file = await File.findOne({ where: { id: fileId } });
+      const uuid = file.dataValues.key.slice(0, -4);
+      expect(file.dataValues.id).toBe(fileId);
+      expect(file.dataValues.status).not.toBe(null);
+      expect(file.dataValues.originalFileName).toBe('testfile.pdf');
+      expect(file.dataValues.activityReportId).toBe(report.dataValues.id);
+      expect(validate(uuid)).toBe(true);
+    });
+    it('allows an admin to upload a file', async () => {
+      ActivityReportPolicy.mockImplementation(() => ({
+        canUpdate: () => false,
+      }));
+      validateUserAuthForAdmin.mockResolvedValue(true);
+      uploadFile.mockResolvedValue({ key: 'key' });
+      const response = await request(app)
+        .post('/api/files')
+        .field('reportId', report.dataValues.id)
+        .attach('file', `${__dirname}/testfiles/testfile.pdf`)
+        .expect(200);
+      fileId = response.body.id;
+      expect(uploadFile).toHaveBeenCalled();
       expect(mockAddToScanQueue).toHaveBeenCalled();
       const file = await File.findOne({ where: { id: fileId } });
       const uuid = file.dataValues.key.slice(0, -4);
@@ -155,9 +162,7 @@ describe('File Upload', () => {
         .post('/api/files')
         .attach('file', `${__dirname}/testfiles/testfile.pdf`)
         .expect(400, { error: 'reportId required' })
-        .then(() => {
-          expect(uploadFile).not.toHaveBeenCalled();
-        });
+      await expect(uploadFile).not.toHaveBeenCalled();
     });
     it('tests a file upload without a file', async () => {
       ActivityReportPolicy.mockImplementation(() => ({
@@ -172,7 +177,7 @@ describe('File Upload', () => {
         });
     });
     it('tests an unauthorized upload', async () => {
-      jest.clearAllMocks();
+      validateUserAuthForAdmin.mockResolvedValue(false);
       ActivityReportPolicy.mockImplementation(() => ({
         canUpdate: () => false,
       }));

src/services/accessValidation.js

@@ -65,7 +65,12 @@ export async function validateUserAuthForAdmin(userId) {
         scopeId: ADMIN,
       },
     });
-    return userPermission !== null;
+    if (userPermission !== null) {
+      logger.info(`User ${userId} successfully checked ADMIN access`);
+      return true;
+    }
+    logger.warn(`User ${userId} unsuccessfully checked ADMIN access`);
+    return false;
   } catch (error) {
     logger.error(`${JSON.stringify({ ...logContext })} - ADMIN Access error - ${error}`);
     throw error;