From e70c329fb555c2de291e16f4403db14f5f0525e9 Mon Sep 17 00:00:00 2001 From: pdik Date: Wed, 20 Nov 2024 19:40:28 +0100 Subject: [PATCH 01/10] Added Locks Manage All Detectors endpoint Added Endpoint to manage all detectors, enforce Global role. --- Control/lib/api.js | 6 ++++++ Control/test/api/lock/api-put-locks.test.js | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Control/lib/api.js b/Control/lib/api.js index df72eca5f..2ed612004 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -184,6 +184,12 @@ module.exports.setup = (http, ws) => { // Lock Service http.get('/locks', lockController.getLocksStateHandler.bind(lockController)); + + http.put('/locks/:action/ALL', + minimumRoleMiddleware(Role.GLOBAL), + lockController.actionLockHandler.bind(lockController) + ); + http.put('/locks/:action/:detectorId', minimumRoleMiddleware(Role.DETECTOR), lockController.actionLockHandler.bind(lockController) diff --git a/Control/test/api/lock/api-put-locks.test.js b/Control/test/api/lock/api-put-locks.test.js index 75928354b..5641b714e 100644 --- a/Control/test/api/lock/api-put-locks.test.js +++ b/Control/test/api/lock/api-put-locks.test.js @@ -154,7 +154,7 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { MID: { name: 'MID', state: 'FREE' }, DCS: { name: 'DCS', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, ODC: { name: 'ODC', state: 'TAKEN', owner: { username: 'admin', fullName: 'Admin User', personid: 0 } }, - }); + }); await request(`${TEST_URL}/api/locks`) .put(`/force/${DetectorLockAction.RELEASE}/ALL?token=${GLOBAL_TEST_TOKEN}`) @@ -164,4 +164,16 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { ODC: { name: 'ODC', state: 'FREE' }, }); }); + + it('should return 403 error for Guest user on locks ALL', async () => { + await request(`${TEST_URL}/api/locks`) + .put(`/${DetectorLockAction.TAKE}/ALL?token=${GUEST_TEST_TOKEN}`) + .expect(403, { message: 'Not enough permissions for this operation' }); + }); + + it('should allow access for Global user on locks ALL', async () => { + await request(`${TEST_URL}/api/locks`) + .put(`/${DetectorLockAction.TAKE}/ALL?token=${GLOBAL_TEST_TOKEN}`) + .expect(200); + }); }); From cbe65d11008f8ec6279efcdb4e69723b4e68db0a Mon Sep 17 00:00:00 2001 From: pdik Date: Wed, 20 Nov 2024 20:03:15 +0100 Subject: [PATCH 02/10] Add Expected Results to test should allow access for Global user on locks ALL. Expect Taken states --- Control/test/api/lock/api-put-locks.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Control/test/api/lock/api-put-locks.test.js b/Control/test/api/lock/api-put-locks.test.js index 5641b714e..7cdf22457 100644 --- a/Control/test/api/lock/api-put-locks.test.js +++ b/Control/test/api/lock/api-put-locks.test.js @@ -174,6 +174,11 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { it('should allow access for Global user on locks ALL', async () => { await request(`${TEST_URL}/api/locks`) .put(`/${DetectorLockAction.TAKE}/ALL?token=${GLOBAL_TEST_TOKEN}`) - .expect(200); + .expect(200,{ + MID: { name: 'MID', state: 'TAKEN',owner: { username: 'global', fullName: 'Global User', personid: 1 }}, + DCS: { name: 'DCS', state: 'TAKEN',owner: { username: 'global', fullName: 'Global User', personid: 1 } }, + ODC: { name: 'ODC', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, + }); }); }); + From bcfb562cfb28947fc280a0fc427cd55c9e5c1a2e Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 25 Nov 2024 10:18:21 +0100 Subject: [PATCH 03/10] Added Tests: Locks/All needs Global role --- Control/test/api/lock/api-put-locks.test.js | 36 +++++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/Control/test/api/lock/api-put-locks.test.js b/Control/test/api/lock/api-put-locks.test.js index 7cdf22457..b2a0e0591 100644 --- a/Control/test/api/lock/api-put-locks.test.js +++ b/Control/test/api/lock/api-put-locks.test.js @@ -165,20 +165,36 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { }); }); - it('should return 403 error for Guest user on locks ALL', async () => { - await request(`${TEST_URL}/api/locks`) - .put(`/${DetectorLockAction.TAKE}/ALL?token=${GUEST_TEST_TOKEN}`) - .expect(403, { message: 'Not enough permissions for this operation' }); - }); - - it('should allow access for Global user on locks ALL', async () => { + it('should successfully Take ALL locks when >=global ', async () => { + // first we retake a lock to ensure we have a lock to release from different types of users await request(`${TEST_URL}/api/locks`) .put(`/${DetectorLockAction.TAKE}/ALL?token=${GLOBAL_TEST_TOKEN}`) - .expect(200,{ - MID: { name: 'MID', state: 'TAKEN',owner: { username: 'global', fullName: 'Global User', personid: 1 }}, - DCS: { name: 'DCS', state: 'TAKEN',owner: { username: 'global', fullName: 'Global User', personid: 1 } }, + .expect(200, { + + DCS: { name: 'DCS', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, + MID: { name: 'MID', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, ODC: { name: 'ODC', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, + }); + + await request(`${TEST_URL}/api/locks`) + .put(`/${DetectorLockAction.RELEASE}/ALL?token=${GLOBAL_TEST_TOKEN}`) + .expect(200, { + MID: { name: 'MID', state: 'FREE' }, + DCS: { name: 'DCS', state: 'FREE' }, + ODC: { name: 'ODC', state: 'FREE' }, + }); + }); + + it('should fail taking ALL locks when <=global ', async () => { + // first we retake a lock to ensure we have a lock to release from different types of users + await request(`${TEST_URL}/api/locks`) + .put(`/${DetectorLockAction.TAKE}/ALL?token=${GUEST_TEST_TOKEN}`) + .expect(403); + + await request(`${TEST_URL}/api/locks`) + .put(`/${DetectorLockAction.RELEASE}/ALL?token=${GUEST_TEST_TOKEN}`) + .expect(403); }); }); From 47b890f9d6ba4523c61984dc61766a2023937e5f Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 25 Nov 2024 11:08:05 +0100 Subject: [PATCH 04/10] Added DetectorId Middleware This middleware will add the detectorid object to the requests --- Control/lib/api.js | 3 + .../middleware/addDetectorId.middleware.js | 11 ++++ Control/test/api/lock/api-put-locks.test.js | 2 - .../mocha-addDetectorId.middleware.test.js | 57 +++++++++++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 Control/lib/middleware/addDetectorId.middleware.js create mode 100644 Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js diff --git a/Control/lib/api.js b/Control/lib/api.js index cae42e427..2e05480b4 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -19,6 +19,8 @@ const config = require('./config/configProvider.js'); // middleware const {minimumRoleMiddleware} = require('./middleware/minimumRole.middleware.js'); +const {addDetectorIdMiddleware} = require('./middleware/addDetectorId.middleware.js'); + const {lockOwnershipMiddleware} = require('./middleware/lockOwnership.middleware.js'); // controllers @@ -192,6 +194,7 @@ module.exports.setup = (http, ws) => { http.put('/locks/:action/ALL', minimumRoleMiddleware(Role.GLOBAL), + addDetectorIdMiddleware('ALL'), lockController.actionLockHandler.bind(lockController) ); diff --git a/Control/lib/middleware/addDetectorId.middleware.js b/Control/lib/middleware/addDetectorId.middleware.js new file mode 100644 index 000000000..b3386ad59 --- /dev/null +++ b/Control/lib/middleware/addDetectorId.middleware.js @@ -0,0 +1,11 @@ +/** + * Middleware function to add detectorID to the request object + * @return {function(req, res, next): void} - middleware function + */ +const addDetectorIdMiddleware = (detectorId) => { + return async (req, res, next) => { + req.params.detectorId = detectorId; + next(); + }; +} +exports.addDetectorIdMiddleware = addDetectorIdMiddleware \ No newline at end of file diff --git a/Control/test/api/lock/api-put-locks.test.js b/Control/test/api/lock/api-put-locks.test.js index b2a0e0591..5d7c1c0e0 100644 --- a/Control/test/api/lock/api-put-locks.test.js +++ b/Control/test/api/lock/api-put-locks.test.js @@ -170,11 +170,9 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { await request(`${TEST_URL}/api/locks`) .put(`/${DetectorLockAction.TAKE}/ALL?token=${GLOBAL_TEST_TOKEN}`) .expect(200, { - DCS: { name: 'DCS', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, MID: { name: 'MID', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, ODC: { name: 'ODC', state: 'TAKEN', owner: { username: 'global', fullName: 'Global User', personid: 1 } }, - }); await request(`${TEST_URL}/api/locks`) diff --git a/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js new file mode 100644 index 000000000..213ace4a0 --- /dev/null +++ b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js @@ -0,0 +1,57 @@ +const assert = require('assert'); +const sinon = require('sinon'); +const express = require('express'); +const request = require('supertest'); + +// Import the middleware +const { addDetectorIdMiddleware } = require('../../../lib/middleware/addDetectorId.middleware'); + +describe('`addDetectorIdMiddleware` test suite', () => { + it('should add the specified detectorId to req.params and call next()', () => { + const detectorId = '1234'; + const req = { params: {} }; // Mock req object + const res = {}; // Mock res object + const next = sinon.stub(); // Mock next function + + addDetectorIdMiddleware(detectorId)(req, res, next); + + // Validate the detectorId is added + assert.strictEqual(req.params.detectorId, detectorId); + // Ensure next() is called + assert.ok(next.calledOnce); + }); + + it('should overwrite existing detectorId in req.params', () => { + const detectorId = '5678'; + const req = { params: { detectorId: 'original' } }; + const res = {}; + const next = sinon.stub(); + + addDetectorIdMiddleware(detectorId)(req, res, next); + + // Validate the detectorId is overwritten + assert.strictEqual(req.params.detectorId, detectorId); + assert.ok(next.calledOnce); + }); + + it('should integrate with an Express route and add detectorId', async () => { + const app = express(); + + // Use the middleware with a specific detectorId + app.get( + '/:id', + addDetectorIdMiddleware('testDetector'), + (req, res) => { + res.json(req.params); + } + ); + + // Test the route with Supertest + const response = await request(app).get('/456'); + assert.strictEqual(response.status, 200); + assert.deepStrictEqual(response.body, { + id: '456', // Route parameter + detectorId: 'testDetector', // Added by middleware + }); + }); +}); From 9e8953f15ac8338388d9c722042faeffc83cb495 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 26 Nov 2024 18:15:21 +0100 Subject: [PATCH 05/10] Resolved Issues --- Control/lib/api.js | 6 ++--- .../lib/common/lock/detectorIdState.enum.js | 22 +++++++++++++++++++ .../middleware/addDetectorId.middleware.js | 20 ++++++++++++++++- 3 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 Control/lib/common/lock/detectorIdState.enum.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 2e05480b4..5d2c79e30 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -20,7 +20,7 @@ const config = require('./config/configProvider.js'); // middleware const {minimumRoleMiddleware} = require('./middleware/minimumRole.middleware.js'); const {addDetectorIdMiddleware} = require('./middleware/addDetectorId.middleware.js'); - +const {DetectorIdState} = require('./common/lock/detectorIdState.enum.js'); const {lockOwnershipMiddleware} = require('./middleware/lockOwnership.middleware.js'); // controllers @@ -192,9 +192,9 @@ module.exports.setup = (http, ws) => { // Lock Service http.get('/locks', lockController.getLocksStateHandler.bind(lockController)); - http.put('/locks/:action/ALL', + http.put(`/locks/:action/${DetectorIdState.ALL}`, minimumRoleMiddleware(Role.GLOBAL), - addDetectorIdMiddleware('ALL'), + addDetectorIdMiddleware(DetectorIdState.ALL), lockController.actionLockHandler.bind(lockController) ); diff --git a/Control/lib/common/lock/detectorIdState.enum.js b/Control/lib/common/lock/detectorIdState.enum.js new file mode 100644 index 000000000..2c437c1a3 --- /dev/null +++ b/Control/lib/common/lock/detectorIdState.enum.js @@ -0,0 +1,22 @@ +/** + * @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. +*/ + +/** + * Available lock states + */ +const DetectorIdState = Object.freeze({ + ALL: 'ALL', +}); + +exports.DetectorIdState = DetectorIdState; diff --git a/Control/lib/middleware/addDetectorId.middleware.js b/Control/lib/middleware/addDetectorId.middleware.js index b3386ad59..8826d5884 100644 --- a/Control/lib/middleware/addDetectorId.middleware.js +++ b/Control/lib/middleware/addDetectorId.middleware.js @@ -1,6 +1,23 @@ +/** + * @license + * Copyright 2019-2024 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. +*/ + /** * Middleware function to add detectorID to the request object - * @return {function(req, res, next): void} - middleware function + * @param {Request} req - HTTP Request object + * @param {Next} next - HTTP Next object to use if checks pass + * @param {Response} res - HTTP Response object + * @return {void} */ const addDetectorIdMiddleware = (detectorId) => { return async (req, res, next) => { @@ -8,4 +25,5 @@ const addDetectorIdMiddleware = (detectorId) => { next(); }; } + exports.addDetectorIdMiddleware = addDetectorIdMiddleware \ No newline at end of file From da25fdc9970ed50981c20cd126a4c2667dbbdfa2 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 2 Dec 2024 09:42:28 +0100 Subject: [PATCH 06/10] Resolve DetectorIDState Naming --- Control/lib/api.js | 6 +++--- .../{lock/detectorIdState.enum.js => detectorId.enum.js} | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) rename Control/lib/common/{lock/detectorIdState.enum.js => detectorId.enum.js} (75%) diff --git a/Control/lib/api.js b/Control/lib/api.js index 5d2c79e30..a3af0458c 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -20,7 +20,7 @@ const config = require('./config/configProvider.js'); // middleware const {minimumRoleMiddleware} = require('./middleware/minimumRole.middleware.js'); const {addDetectorIdMiddleware} = require('./middleware/addDetectorId.middleware.js'); -const {DetectorIdState} = require('./common/lock/detectorIdState.enum.js'); +const {DetectorId} = require('./common/detectorId.enum.js'); const {lockOwnershipMiddleware} = require('./middleware/lockOwnership.middleware.js'); // controllers @@ -192,9 +192,9 @@ module.exports.setup = (http, ws) => { // Lock Service http.get('/locks', lockController.getLocksStateHandler.bind(lockController)); - http.put(`/locks/:action/${DetectorIdState.ALL}`, + http.put(`/locks/:action/${DetectorId.ALL}`, minimumRoleMiddleware(Role.GLOBAL), - addDetectorIdMiddleware(DetectorIdState.ALL), + addDetectorIdMiddleware(DetectorId.ALL), lockController.actionLockHandler.bind(lockController) ); diff --git a/Control/lib/common/lock/detectorIdState.enum.js b/Control/lib/common/detectorId.enum.js similarity index 75% rename from Control/lib/common/lock/detectorIdState.enum.js rename to Control/lib/common/detectorId.enum.js index 2c437c1a3..f8541d847 100644 --- a/Control/lib/common/lock/detectorIdState.enum.js +++ b/Control/lib/common/detectorId.enum.js @@ -1,6 +1,6 @@ /** * @license - * Copyright 2019-2020 CERN and copyright holders of ALICE O2. + * Copyright 2019-2024 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. * @@ -13,10 +13,10 @@ */ /** - * Available lock states + * Enum for detector IDs. */ -const DetectorIdState = Object.freeze({ +const DetectorId = Object.freeze({ ALL: 'ALL', }); -exports.DetectorIdState = DetectorIdState; +exports.DetectorId = DetectorId; From 33a40681ce9d39cc2bb11707168fe5103c9b53fd Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 2 Dec 2024 09:49:41 +0100 Subject: [PATCH 07/10] Resolved New Line end of file --- Control/lib/middleware/addDetectorId.middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Control/lib/middleware/addDetectorId.middleware.js b/Control/lib/middleware/addDetectorId.middleware.js index 8826d5884..9de7d3d68 100644 --- a/Control/lib/middleware/addDetectorId.middleware.js +++ b/Control/lib/middleware/addDetectorId.middleware.js @@ -26,4 +26,4 @@ const addDetectorIdMiddleware = (detectorId) => { }; } -exports.addDetectorIdMiddleware = addDetectorIdMiddleware \ No newline at end of file +exports.addDetectorIdMiddleware = addDetectorIdMiddleware From 083903f8ef73139ec0624e953a6045f379093e6d Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 09:01:30 +0100 Subject: [PATCH 08/10] Removed Should integrate with a express route --- .../mocha-addDetectorId.middleware.test.js | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js index 213ace4a0..849fa6e56 100644 --- a/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js +++ b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js @@ -34,24 +34,4 @@ describe('`addDetectorIdMiddleware` test suite', () => { assert.ok(next.calledOnce); }); - it('should integrate with an Express route and add detectorId', async () => { - const app = express(); - - // Use the middleware with a specific detectorId - app.get( - '/:id', - addDetectorIdMiddleware('testDetector'), - (req, res) => { - res.json(req.params); - } - ); - - // Test the route with Supertest - const response = await request(app).get('/456'); - assert.strictEqual(response.status, 200); - assert.deepStrictEqual(response.body, { - id: '456', // Route parameter - detectorId: 'testDetector', // Added by middleware - }); - }); }); From 3e01c3afedebe924e7cde7d49d7d16c04d42c024 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 19:15:06 +0100 Subject: [PATCH 09/10] Fixed CodeScanning --- .../test/lib/middleware/mocha-addDetectorId.middleware.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js index 849fa6e56..e1d570980 100644 --- a/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js +++ b/Control/test/lib/middleware/mocha-addDetectorId.middleware.test.js @@ -1,7 +1,5 @@ const assert = require('assert'); const sinon = require('sinon'); -const express = require('express'); -const request = require('supertest'); // Import the middleware const { addDetectorIdMiddleware } = require('../../../lib/middleware/addDetectorId.middleware'); From 7512a91ae2953853a87ffb41223106c898b993eb Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 19:16:11 +0100 Subject: [PATCH 10/10] Removed unrelated comments --- Control/test/api/lock/api-put-locks.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Control/test/api/lock/api-put-locks.test.js b/Control/test/api/lock/api-put-locks.test.js index 5d7c1c0e0..9f5767e7b 100644 --- a/Control/test/api/lock/api-put-locks.test.js +++ b/Control/test/api/lock/api-put-locks.test.js @@ -166,7 +166,6 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { }); it('should successfully Take ALL locks when >=global ', async () => { - // first we retake a lock to ensure we have a lock to release from different types of users await request(`${TEST_URL}/api/locks`) .put(`/${DetectorLockAction.TAKE}/ALL?token=${GLOBAL_TEST_TOKEN}`) .expect(200, { @@ -185,7 +184,7 @@ describe(`'API - PUT - /locks/:action/:detectorId' test suite`, () => { }); it('should fail taking ALL locks when <=global ', async () => { - // first we retake a lock to ensure we have a lock to release from different types of users + await request(`${TEST_URL}/api/locks`) .put(`/${DetectorLockAction.TAKE}/ALL?token=${GUEST_TEST_TOKEN}`) .expect(403);