[OGUI-1474] Add backend validaiton for layout PUT and DELETE endpoints (

#2274)

* adds back-end validation for layout PUT and DELETE actions
 * fixes a bug on front-end that was not displaying failure of removal of layout

QualityControl/lib/api.js

@@ -38,7 +38,7 @@ export const setup = (http, ws) => {
   http.get('/layout/:id', layoutService.getLayoutHandler.bind(layoutService));
   http.post('/layout', layoutService.postLayoutHandler.bind(layoutService));
   http.put('/layout/:id', layoutService.putLayoutHandler.bind(layoutService));
-  http.delete('/layout/:id', layoutService.deleteLayout.bind(layoutService));
+  http.delete('/layout/:id', layoutService.deleteLayoutHandler.bind(layoutService));
 
   http.get('/status/gui', statusController.getQCGStatus.bind(statusController), { public: true });
   http.get('/status/framework', statusController.getFrameworkInfo.bind(statusController), { public: true });

QualityControl/lib/controllers/LayoutController.js

@@ -18,8 +18,9 @@ import assert from 'assert';
 import { LayoutDto } from './../dtos/LayoutDto.js';
 import {
   updateExpressResponseFromNativeError,
-} from '../errors/updateExpressResponseFromNativeError.js';
-import { InvalidInputError } from '../errors/InvalidInputError.js';
+} from './../errors/updateExpressResponseFromNativeError.js';
+import { InvalidInputError } from './../errors/InvalidInputError.js';
+import { UnauthorizedAccessError } from '../errors/UnauthorizedAccessError.js';
 
 /**
  * Gateway for all HTTP requests with regards to QCG Layouts
@@ -97,10 +98,19 @@ export class LayoutController {
       } else if (!req.body) {
         updateExpressResponseFromNativeError(res, new InvalidInputError('Missing body content to update layout with'));
       } else {
-        const data = await LayoutDto.validateAsync(req.body);
-        // TODO retrieve layout, check user is owner or allowed to edit
-        const layout = await this._dataService.updateLayout(id, data);
-        res.status(201).json(layout);
+        const { personid, name } = req.session;
+        const { owner_name, owner_id } = await this._dataService.readLayout(id);
+
+        if (owner_name !== name || owner_id !== personid) {
+          updateExpressResponseFromNativeError(
+            res,
+            new UnauthorizedAccessError('Only the owner of the layout can update it'),
+          );
+        } else {
+          const data = await LayoutDto.validateAsync(req.body);
+          const layout = await this._dataService.updateLayout(id, data);
+          res.status(201).json(layout);
+        }
       }
     } catch (error) {
       updateExpressResponseFromNativeError(
@@ -122,9 +132,17 @@ export class LayoutController {
       if (!id) {
         updateExpressResponseFromNativeError(res, new InvalidInputError('Missing parameter "id" of layout to delete'));
       } else {
-        // TODO retrieve layout, check user is owner
-        const result = await this._dataService.deleteLayout(id);
-        res.status(200).json(result);
+        const { personid, name } = req.session;
+        const { owner_name, owner_id } = await this._dataService.readLayout(id);
+        if (owner_name !== name || owner_id !== personid) {
+          updateExpressResponseFromNativeError(
+            res,
+            new UnauthorizedAccessError('Only the owner of the layout can delete it'),
+          );
+        } else {
+          const result = await this._dataService.deleteLayout(id);
+          res.status(200).json(result);
+        }
       }
     } catch (error) {
       updateExpressResponseFromNativeError(res, new Error(`Unable to delete layout with id: ${id}`));

QualityControl/public/layout/Layout.js

@@ -260,12 +260,15 @@ export default class Layout extends Observable {
     if (!this.item) {
       throw new Error('no layout to delete');
     }
-    await this.model.services.layout.removeLayoutById(this.item.id);
-
-    this.model.notification.show(`Layout "${this.item.name}" has been deleted.`, 'success', 1500);
-    this.model.router.go('?page=layouts');
-    this.model.services.layout.getLayoutsByUserId(this.model.session.personid);
-    this.editEnabled = false;
+    const { ok } = await this.model.services.layout.removeLayoutById(this.item.id);
+    if (ok) {
+      this.model.notification.show(`Layout "${this.item.name}" has been deleted.`, 'success', 1500);
+      this.model.router.go('?page=layouts');
+      this.model.services.layout.getLayoutsByUserId(this.model.session.personid);
+      this.editEnabled = false;
+    } else {
+      this.model.notification.show('Unauthorized action - delete layout', 'danger', 1500);
+    }
     this.notify();
   }
 

QualityControl/public/services/Layout.service.js

@@ -98,9 +98,8 @@ export default class LayoutService {
    * @returns {RemoteData} - result within a RemoteData object
    */
   async removeLayoutById(layoutId) {
-    const request = fetchClient(`/api/layout/${layoutId}`, { method: 'DELETE' });
-    this.loader.watchPromise(request);
-    await request;
+    const result = await fetchClient(`/api/layout/${layoutId}`, { method: 'DELETE' });
+    return result;
   }
 
   /**

QualityControl/test/demoData/layout/layout.mock.js

@@ -0,0 +1,23 @@
+/**
+ * @license
+ * Copyright 2019-2020 CERN and copyright holders of ALICE O2.
+ * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders.
+ * All rights not expressly granted are reserved.
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * License v3 (GPL Version 3), copied verbatim in the file "COPYING".
+ *
+ * In applying this license CERN does not waive the privileges and immunities
+ * granted to it by virtue of its status as an Intergovernmental Organization
+ * or submit itself to any jurisdiction.
+ */
+
+export const LAYOUT_MOCK_1 = {
+  id: 'mylayout', 
+  name: 'something', 
+  tabs: [
+    {name: 'tab', id: '1'}
+  ], 
+  owner_id: 1, 
+  owner_name: 'one'
+};

QualityControl/test/lib/controllers/layout-controller.test.js

@@ -18,9 +18,10 @@
 import assert from 'assert';
 import { AssertionError } from 'assert';
 import sinon from 'sinon';
+import { LAYOUT_MOCK_1 } from '../../demoData/layout/layout.mock.js';
 
-import { LayoutController } from '../../../lib/controllers/LayoutController.js';
-import { JsonFileService } from '../../../lib/services/JsonFileService.js';
+import { LayoutController } from './../../../lib/controllers/LayoutController.js';
+import { JsonFileService } from './../../../lib/services/JsonFileService.js';
 
 export const layoutControllerTestSuite = async () => {
   describe('Creating a new LayoutController instance', () => {
@@ -154,11 +155,6 @@ export const layoutControllerTestSuite = async () => {
     });
 
     it('should successfully return the id of the updated layout', async () => {
-      const jsonStub = sinon.createStubInstance(JsonFileService, {
-        updateLayout: sinon.stub().resolves([{ id: 'somelayout' }]),
-      });
-      const layoutConnector = new LayoutController(jsonStub);
-      const mockLayout = { id: 'mylayout', name: 'something', tabs: [{ name: 'tab', id: '1' }], owner_id: 1, owner_name: 'one' };
       const expectedMockWithDefaults = {
         id: 'mylayout',
         name: 'something',
@@ -169,20 +165,25 @@ export const layoutControllerTestSuite = async () => {
         displayTimestamp: false,
         autoTabChange: 0,
       };
+      const jsonStub = sinon.createStubInstance(JsonFileService, {
+        updateLayout: sinon.stub().resolves(expectedMockWithDefaults),
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
+      });
+      const layoutConnector = new LayoutController(jsonStub);
 
-      const req = { params: { id: 'mylayout' }, body: mockLayout };
+      const req = { params: { id: 'mylayout' }, session: { personid: 1, name: 'one' }, body: LAYOUT_MOCK_1 };
       await layoutConnector.putLayoutHandler(req, res);
       assert.ok(res.status.calledWith(201), 'Response status was not 200');
-      assert.ok(res.json.calledWith([{ id: 'somelayout' }]), 'A layout id should have been sent back');
+      assert.ok(res.json.calledWith(expectedMockWithDefaults), 'A layout id should have been sent back');
       assert.ok(jsonStub.updateLayout.calledWith('mylayout', expectedMockWithDefaults), 'Layout id was not used in data connector call');
     });
 
     it('should return error if data connector failed to update layout', async () => {
       const jsonStub = sinon.createStubInstance(JsonFileService, {
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
         updateLayout: sinon.stub().rejects(new Error('Could not update layout')),
       });
       const layoutConnector = new LayoutController(jsonStub);
-      const mockLayout = { id: 'mylayout', name: 'something', tabs: [{ name: 'tab', id: '1' }], owner_id: 1, owner_name: 'one' };
       const expectedMockWithDefaults = {
         id: 'mylayout',
         name: 'something',
@@ -193,12 +194,26 @@ export const layoutControllerTestSuite = async () => {
         displayTimestamp: false,
         autoTabChange: 0,
       };
-      const req = { params: { id: 'mylayout' }, body: mockLayout };
+      const req = { params: { id: LAYOUT_MOCK_1.id }, session: { personid: 1, name: 'one' }, body: LAYOUT_MOCK_1 };
       await layoutConnector.putLayoutHandler(req, res);
+
       assert.ok(res.status.calledWith(500), 'Response status was not 500');
       assert.ok(res.json.calledWith({ message: 'Failed to update layout ' }), 'DataConnector error message is incorrect');
       assert.ok(jsonStub.updateLayout.calledWith('mylayout', expectedMockWithDefaults), 'Layout id was not used in data connector call');
     });
+
+    it('should return unauthorized error if user requesting update operation is not the owner', async () => {
+      const jsonStub = sinon.createStubInstance(JsonFileService, {
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
+      });
+      const layoutConnector = new LayoutController(jsonStub);
+      const req = { params: { id: LAYOUT_MOCK_1.id }, session: { personid: 2, name: 'one' }, body: {} };
+      await layoutConnector.putLayoutHandler(req, res);
+
+      assert.ok(res.status.calledWith(403), 'Response status was not 403');
+      assert.ok(res.json.calledWith({ message: 'Only the owner of the layout can update it' }), 'DataConnector error message is incorrect');
+      assert.ok(jsonStub.readLayout.calledWith(LAYOUT_MOCK_1.id), 'Layout id was not used in data connector call');
+    });
   });
 
   describe('`deleteLayoutHandler()` tests', () => {
@@ -220,10 +235,11 @@ export const layoutControllerTestSuite = async () => {
 
     it('should successfully return the id of the deleted layout', async () => {
       const jsonStub = sinon.createStubInstance(JsonFileService, {
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
         deleteLayout: sinon.stub().resolves({ id: 'somelayout' }),
       });
       const layoutConnector = new LayoutController(jsonStub);
-      const req = { params: { id: 'somelayout' } };
+      const req = { params: { id: 'somelayout' }, session: { personid: 1, name: 'one' } };
       await layoutConnector.deleteLayoutHandler(req, res);
       assert.ok(res.status.calledWith(200), 'Response status was not 200');
       assert.ok(res.json.calledWith({ id: 'somelayout' }), 'A layout id should have been sent back');
@@ -232,15 +248,29 @@ export const layoutControllerTestSuite = async () => {
 
     it('should return error if data connector failed to delete', async () => {
       const jsonStub = sinon.createStubInstance(JsonFileService, {
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
         deleteLayout: sinon.stub().rejects(new Error('Could not delete layout')),
       });
       const layoutConnector = new LayoutController(jsonStub);
-      const req = { params: { id: 'mylayout' } };
+      const req = { params: { id: 'mylayout' }, session: { personid: 1, name: 'one' } };
       await layoutConnector.deleteLayoutHandler(req, res);
       assert.ok(res.status.calledWith(500), 'Response status was not 500');
       assert.ok(res.json.calledWith({ message: 'Unable to delete layout with id: mylayout' }), 'DataConnector error message is incorrect');
       assert.ok(jsonStub.deleteLayout.calledWith('mylayout'), 'Layout id was not used in data connector call');
     });
+
+    it('should return unauthorized error if user requesting delete operation is not the owner', async () => {
+      const jsonStub = sinon.createStubInstance(JsonFileService, {
+        readLayout: sinon.stub().resolves(LAYOUT_MOCK_1),
+      });
+      const layoutConnector = new LayoutController(jsonStub);
+      const req = { params: { id: 'mylayout' }, session: { personid: 2, name: 'one' } };
+      await layoutConnector.deleteLayoutHandler(req, res);
+
+      assert.ok(res.status.calledWith(403), 'Response status was not 403');
+      assert.ok(res.json.calledWith({ message: 'Only the owner of the layout can delete it' }), 'DataConnector error message is incorrect');
+      assert.ok(jsonStub.readLayout.calledWith('mylayout'), 'Layout id was not used in data connector call');
+    });
   });
 
   describe('`postLayoutHandler()` tests', () => {