From 9fa4cf492b4e81c4ca2bb4d7c0abae0506ce01b2 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 26 Nov 2024 18:05:26 +0100 Subject: [PATCH 01/14] detectorLockMiddleware --- .../detectorOwnership.middleware.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Control/lib/middleware/detectorOwnership.middleware.js diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js new file mode 100644 index 000000000..3690c9648 --- /dev/null +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -0,0 +1,33 @@ +//const {User} = require('../dtos/User'); +/** + * Middleware function to check detector ownership. + * + * @param {Request} req - Express Request object. + * @param {Response} res - Express Response object. + * @param {Function} next - Next middleware to call. + */ +const detectorLockMiddleware = (req, res, next) => { + const { detectorId } = req.params; + const { access } = req.session || {}; + + if (!detectorId) { + return res.status(400).json({ message: 'Invalid request: missing user or detector information' }); + } + + try { + + // if (!hasLock) { + // return res + // .status(403) + // .json({ message: `User ${name} does not have ownership of the lock for detector ${detectorId}` }); + // } + + next(); // Proceed if lock ownership is verified + } catch (error) { + console.error(`Error checking locks:`, error); + res.status(500).json({ message: 'Internal server error' }); + } +}; + +module.exports = { detectorLockMiddleware }; + \ No newline at end of file From 05250eca3a312c02427b1b80b6012ef17c372f75 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 26 Nov 2024 18:42:44 +0100 Subject: [PATCH 02/14] Adding the detectorOwnershipMiddleware --- Control/lib/api.js | 3 ++- Control/lib/middleware/detectorOwnership.middleware.js | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Control/lib/api.js b/Control/lib/api.js index ae11d3399..f39a9ec53 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 {lockOwnershipMiddleware} = require('./middleware/lockOwnership.middleware.js'); - +const { detectorOwnershipMiddleware } = require('./middleware/detectorOwnership.middleware.js'); // controllers const {ConsulController} = require('./controllers/Consul.controller.js'); const {EnvironmentController} = require('./controllers/Environment.controller.js'); @@ -191,6 +191,7 @@ module.exports.setup = (http, ws) => { http.get('/locks', lockController.getLocksStateHandler.bind(lockController)); http.put('/locks/:action/:detectorId', minimumRoleMiddleware(Role.DETECTOR), + detectorOwnershipMiddleware, lockController.actionLockHandler.bind(lockController) ); http.put('/locks/force/:action/:detectorId', diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index 3690c9648..0267faeaf 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -6,16 +6,17 @@ * @param {Response} res - Express Response object. * @param {Function} next - Next middleware to call. */ -const detectorLockMiddleware = (req, res, next) => { +const detectorOwnershipMiddleware = (req, res, next) => { const { detectorId } = req.params; const { access } = req.session || {}; if (!detectorId) { - return res.status(400).json({ message: 'Invalid request: missing user or detector information' }); + return res.status(400).json({ message: 'Invalid request: missing detector information' }); } try { - + console.log(`Checking locks for detector ${detectorId}`); + console.log(access); // if (!hasLock) { // return res // .status(403) @@ -29,5 +30,5 @@ const detectorLockMiddleware = (req, res, next) => { } }; -module.exports = { detectorLockMiddleware }; +exports.detectorOwnershipMiddleware = detectorOwnershipMiddleware; \ No newline at end of file From 150c60856774c063b1f4a69ca05e501faa774b70 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 2 Dec 2024 10:30:26 +0100 Subject: [PATCH 03/14] * Added User belongsToDetector * DetectorOwnership middleware * DetectorOwnershipMiddleware tests --- Control/lib/dtos/User.js | 9 ++ .../detectorOwnership.middleware.js | 25 +++-- ...mocha-detectorOwnership.middleware.test.js | 99 +++++++++++++++++++ 3 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js diff --git a/Control/lib/dtos/User.js b/Control/lib/dtos/User.js index c80b2de0b..723841218 100644 --- a/Control/lib/dtos/User.js +++ b/Control/lib/dtos/User.js @@ -66,6 +66,15 @@ class User { return Boolean(accessList.includes('admin')); } + /** + * Checks if the given user can access the given detector + * @param {string} detectorId + * @returns {Boolean} + */ + belongsToDetector(detectorId) { + return Boolean(this._accessList.includes(`det-${detectorId}`)); + } + /** * Check if provided details of a user are the same as the current instance one; * @param {User} user - to compare to diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index 0267faeaf..d3e737eed 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -1,27 +1,27 @@ -//const {User} = require('../dtos/User'); +const {User} = require('../dtos/User'); /** * Middleware function to check detector ownership. - * + * Based on the session object, it checks if the user has ownership of the detector lock. * @param {Request} req - Express Request object. * @param {Response} res - Express Response object. * @param {Function} next - Next middleware to call. */ const detectorOwnershipMiddleware = (req, res, next) => { const { detectorId } = req.params; - const { access } = req.session || {}; - - if (!detectorId) { - return res.status(400).json({ message: 'Invalid request: missing detector information' }); + const { name, username, personid, access } = req.session || {}; + + if (!detectorId || !access) { + return res.status(400).json({ message: 'Invalid request: missing information' }); } try { + const u = new User(username, name, personid, access); console.log(`Checking locks for detector ${detectorId}`); - console.log(access); - // if (!hasLock) { - // return res - // .status(403) - // .json({ message: `User ${name} does not have ownership of the lock for detector ${detectorId}` }); - // } + if (!u.belongsToDetector(detectorId)) { + return res + .status(403) + .json({ message: `User ${name} does not have ownership of the lock for detector ${detectorId}` }); + } next(); // Proceed if lock ownership is verified } catch (error) { @@ -31,4 +31,3 @@ const detectorOwnershipMiddleware = (req, res, next) => { }; exports.detectorOwnershipMiddleware = detectorOwnershipMiddleware; - \ No newline at end of file diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js new file mode 100644 index 000000000..abe234219 --- /dev/null +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -0,0 +1,99 @@ +const assert = require('assert'); +const sinon = require('sinon'); +const express = require('express'); +const request = require('supertest'); +const { User } = require('../../../lib/dtos/User'); +const { detectorOwnershipMiddleware } = require('../../../lib/middleware/detectorOwnership.middleware'); + +describe('`DetectorOwnerShipmiddleware` test suite', () => { + let userStub; + + beforeEach(() => { + userStub = sinon.stub(User.prototype, 'belongsToDetector'); + }); + + afterEach(() => { + userStub.restore(); + }); + + it('should call next() if user has ownership of the detector', () => { + const detectorId = 'det-its'; + const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: ['det-its'] } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + const next = sinon.stub(); + + userStub.returns(true); + + detectorOwnershipMiddleware(req, res, next); + + assert.ok(next.calledOnce); + }); + + it('should return 403 if user does not have ownership of the detector', () => { + const detectorId = 'det-its'; + const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: [] } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + const next = sinon.stub(); + + userStub.returns(false); + + detectorOwnershipMiddleware(req, res, next); + + assert.ok(res.status.calledWith(403)); + assert.ok(res.json.calledWith({ message: `User testUser does not have ownership of the lock for detector ${detectorId}` })); + assert.ok(next.notCalled); + }); + + it('should call belongsToDetector method of User', () => { + const detectorId = 'det-its'; + const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: ['det-its'] } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + const next = sinon.stub(); + + userStub.returns(true); + + detectorOwnershipMiddleware(req, res, next); + + assert.ok(userStub.calledOnceWith(detectorId)); + }); + + it('should handle empty session object gracefully', () => { + const detectorId = '1234'; + const req = { params: { detectorId }, session: {} }; // Empty session object + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + const next = sinon.stub(); + + detectorOwnershipMiddleware(req, res, next); + + assert.ok(res.status.calledWith(400)); + assert.ok(res.json.calledWith({ message: 'Invalid request: missing information' })); + assert.ok(next.notCalled); + }); + + it('should integrate with an Express route and add detectorId', async () => { + const app = express(); + app.use(express.json()); + + app.get( + '/:id', + (req, res, next) => { + req.session = { personid: 0, name: 'testUser', access: ['det-its'] }; + req.params.detectorId = 'det-its'; + next(); + }, + detectorOwnershipMiddleware, + (req, res) => { + res.json(req.params); + } + ); + + userStub.returns(true); // Ensure the user has ownership + + const response = await request(app).get('/456'); + assert.strictEqual(response.status, 200); + assert.deepStrictEqual(response.body, { + id: '456', + detectorId: 'det-its', + }); + }); +}); \ No newline at end of file From d42b4572f3c3b7b9b73fa2073c180b44568bfa67 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 2 Dec 2024 10:34:34 +0100 Subject: [PATCH 04/14] Fixed Eslint --- .../lib/middleware/mocha-detectorOwnership.middleware.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index abe234219..2fdab3253 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -40,7 +40,8 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { detectorOwnershipMiddleware(req, res, next); assert.ok(res.status.calledWith(403)); - assert.ok(res.json.calledWith({ message: `User testUser does not have ownership of the lock for detector ${detectorId}` })); + assert.ok(res. + json.calledWith({ message: `User testUser does not have ownership of the lock for detector ${detectorId}` })); assert.ok(next.notCalled); }); From f07dae4aac387e3316a2fee790d6fc7cdd99ce66 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 2 Dec 2024 10:35:00 +0100 Subject: [PATCH 05/14] End of line enter --- .../lib/middleware/mocha-detectorOwnership.middleware.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index 2fdab3253..b371ce45a 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -97,4 +97,4 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { detectorId: 'det-its', }); }); -}); \ No newline at end of file +}); From 264b866be72c1a7dc08bcfee7e4dec3f0f8078c1 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 09:15:28 +0100 Subject: [PATCH 06/14] Changed requested changes --- .../detectorOwnership.middleware.js | 18 ++++++------- ...mocha-detectorOwnership.middleware.test.js | 27 ------------------- 2 files changed, 9 insertions(+), 36 deletions(-) diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index d3e737eed..b0f72e7de 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -1,4 +1,7 @@ const {User} = require('../dtos/User'); + +const {UnauthorizedAccessError} = require('./../errors/UnauthorizedAccessError.js'); +const {updateExpressResponseFromNativeError} = require('./../errors/updateExpressResponseFromNativeError.js'); /** * Middleware function to check detector ownership. * Based on the session object, it checks if the user has ownership of the detector lock. @@ -11,22 +14,19 @@ const detectorOwnershipMiddleware = (req, res, next) => { const { name, username, personid, access } = req.session || {}; if (!detectorId || !access) { - return res.status(400).json({ message: 'Invalid request: missing information' }); + updateExpressResponseFromNativeError(res, new UnauthorizedAccessError('Invalid request: missing information')); } try { - const u = new User(username, name, personid, access); - console.log(`Checking locks for detector ${detectorId}`); - if (!u.belongsToDetector(detectorId)) { - return res - .status(403) - .json({ message: `User ${name} does not have ownership of the lock for detector ${detectorId}` }); + const user = new User(username, name, personid, access); + if (!user.belongsToDetector(detectorId)) { + updateExpressResponseFromNativeError(res, + new UnauthorizedAccessError(`User ${name} does not have ownership of the lock for detector ${detectorId}`)); } next(); // Proceed if lock ownership is verified } catch (error) { - console.error(`Error checking locks:`, error); - res.status(500).json({ message: 'Internal server error' }); + updateExpressResponseFromNativeError(res, error); } }; diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index b371ce45a..4f3c38563 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -70,31 +70,4 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { assert.ok(res.json.calledWith({ message: 'Invalid request: missing information' })); assert.ok(next.notCalled); }); - - it('should integrate with an Express route and add detectorId', async () => { - const app = express(); - app.use(express.json()); - - app.get( - '/:id', - (req, res, next) => { - req.session = { personid: 0, name: 'testUser', access: ['det-its'] }; - req.params.detectorId = 'det-its'; - next(); - }, - detectorOwnershipMiddleware, - (req, res) => { - res.json(req.params); - } - ); - - userStub.returns(true); // Ensure the user has ownership - - const response = await request(app).get('/456'); - assert.strictEqual(response.status, 200); - assert.deepStrictEqual(response.body, { - id: '456', - detectorId: 'det-its', - }); - }); }); From 7e5981bc2bd7715433e43c6a46bef3f0c9859b33 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 09:24:32 +0100 Subject: [PATCH 07/14] Code scanning, Removed unused imports --- .../lib/middleware/mocha-detectorOwnership.middleware.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index 4f3c38563..7d9e4d928 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -1,7 +1,5 @@ const assert = require('assert'); const sinon = require('sinon'); -const express = require('express'); -const request = require('supertest'); const { User } = require('../../../lib/dtos/User'); const { detectorOwnershipMiddleware } = require('../../../lib/middleware/detectorOwnership.middleware'); From 70ba36cd71885a388be82c369fa6fb7b750f2ae8 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 19:25:51 +0100 Subject: [PATCH 08/14] DetectorOwnerShip Check Role Check isRoleSufficient --- .../lib/middleware/detectorOwnership.middleware.js | 7 ++++++- .../mocha-detectorOwnership.middleware.test.js | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index b0f72e7de..163123e8c 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -1,5 +1,5 @@ const {User} = require('../dtos/User'); - +const {isRoleSufficient,Role} = require('../common/role.enum.js'); const {UnauthorizedAccessError} = require('./../errors/UnauthorizedAccessError.js'); const {updateExpressResponseFromNativeError} = require('./../errors/updateExpressResponseFromNativeError.js'); /** @@ -18,6 +18,11 @@ const detectorOwnershipMiddleware = (req, res, next) => { } try { + + // Check if the user's role is sufficient to bypass the ownership check + if (access.some(role => isRoleSufficient(role, Role.GLOBAL))) { + return next(); + } const user = new User(username, name, personid, access); if (!user.belongsToDetector(detectorId)) { updateExpressResponseFromNativeError(res, diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index 7d9e4d928..c1dbc7d8e 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const { User } = require('../../../lib/dtos/User'); const { detectorOwnershipMiddleware } = require('../../../lib/middleware/detectorOwnership.middleware'); +const {Role} = require('../../lib/common/role.enum.js'); describe('`DetectorOwnerShipmiddleware` test suite', () => { let userStub; @@ -43,6 +44,7 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { assert.ok(next.notCalled); }); + it('should call belongsToDetector method of User', () => { const detectorId = 'det-its'; const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: ['det-its'] } }; @@ -68,4 +70,16 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { assert.ok(res.json.calledWith({ message: 'Invalid request: missing information' })); assert.ok(next.notCalled); }); + + it('should call next() if user has a role higher than DETECTOR', () => { + const detectorId = 'det-its'; + const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: [Role.GLOBAL] } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + const next = sinon.stub(); + + detectorOwnershipMiddleware(req, res, next); + + assert.ok(next.calledOnce); + }); + }); From 5c50174569031a79cf8fb92f3e194ce4c28905f9 Mon Sep 17 00:00:00 2001 From: pdik Date: Tue, 3 Dec 2024 20:03:22 +0100 Subject: [PATCH 09/14] Added Tests for RoleChecks Detector ownership can have higher role than it's detector. Test this behavior --- .../detectorOwnership.middleware.js | 21 ++++++++++++++----- ...mocha-detectorOwnership.middleware.test.js | 20 +++++++++++------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index 163123e8c..121e9e2a9 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -1,6 +1,7 @@ const {User} = require('../dtos/User'); const {isRoleSufficient,Role} = require('../common/role.enum.js'); const {UnauthorizedAccessError} = require('./../errors/UnauthorizedAccessError.js'); + const {updateExpressResponseFromNativeError} = require('./../errors/updateExpressResponseFromNativeError.js'); /** * Middleware function to check detector ownership. @@ -14,24 +15,34 @@ const detectorOwnershipMiddleware = (req, res, next) => { const { name, username, personid, access } = req.session || {}; if (!detectorId || !access) { - updateExpressResponseFromNativeError(res, new UnauthorizedAccessError('Invalid request: missing information')); + return updateExpressResponseFromNativeError(res, + new UnauthorizedAccessError('Invalid request: missing information')); } try { + + let accessList = []; + if (typeof access === 'string') { + accessList = access.split(','); + } else if (Array.isArray(access)) { + accessList = access; + } // Check if the user's role is sufficient to bypass the ownership check - if (access.some(role => isRoleSufficient(role, Role.GLOBAL))) { - return next(); + if (accessList?.some((role) => { + return isRoleSufficient(role, Role.GLOBAL) + })) { + next(); } const user = new User(username, name, personid, access); if (!user.belongsToDetector(detectorId)) { - updateExpressResponseFromNativeError(res, + return updateExpressResponseFromNativeError(res, new UnauthorizedAccessError(`User ${name} does not have ownership of the lock for detector ${detectorId}`)); } next(); // Proceed if lock ownership is verified } catch (error) { - updateExpressResponseFromNativeError(res, error); + return updateExpressResponseFromNativeError(res, error); } }; diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index c1dbc7d8e..d8f7f98d5 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -3,7 +3,7 @@ const sinon = require('sinon'); const { User } = require('../../../lib/dtos/User'); const { detectorOwnershipMiddleware } = require('../../../lib/middleware/detectorOwnership.middleware'); -const {Role} = require('../../lib/common/role.enum.js'); +const {Role} = require('../../../lib/common/role.enum.js'); describe('`DetectorOwnerShipmiddleware` test suite', () => { let userStub; @@ -30,17 +30,19 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { it('should return 403 if user does not have ownership of the detector', () => { const detectorId = 'det-its'; - const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: [] } }; + const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: [ + + ] }}; const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; const next = sinon.stub(); userStub.returns(false); detectorOwnershipMiddleware(req, res, next); - + assert.ok(res.status.calledWith(403)); - assert.ok(res. - json.calledWith({ message: `User testUser does not have ownership of the lock for detector ${detectorId}` })); + // assert.ok(res. + // json.calledWith({ message: `User testUser does not have ownership of the lock for detector ${detectorId}` })); assert.ok(next.notCalled); }); @@ -66,20 +68,22 @@ describe('`DetectorOwnerShipmiddleware` test suite', () => { detectorOwnershipMiddleware(req, res, next); - assert.ok(res.status.calledWith(400)); + assert.ok(res.status.calledWith(403)); assert.ok(res.json.calledWith({ message: 'Invalid request: missing information' })); assert.ok(next.notCalled); }); it('should call next() if user has a role higher than DETECTOR', () => { const detectorId = 'det-its'; - const req = { params: { detectorId }, session: { personid: 0, name: 'testUser', access: [Role.GLOBAL] } }; + const req = { params: { detectorId }, session: + { personid: 0, name: 'testUser', access: ['GLOBAL'] }}; const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; const next = sinon.stub(); detectorOwnershipMiddleware(req, res, next); - + assert.ok(next.calledOnce); + }); }); From 560ff11ffc3301c36afd823cf35a2e4930d9ed63 Mon Sep 17 00:00:00 2001 From: pdik Date: Wed, 4 Dec 2024 09:31:40 +0100 Subject: [PATCH 10/14] Added ConvertStringToArray Converts string into Array, with basic checks --- Control/lib/common/StringToArray.js | 16 ++++++++++++++++ .../middleware/detectorOwnership.middleware.js | 12 +++--------- Control/lib/middleware/minimumRole.middleware.js | 9 ++------- 3 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 Control/lib/common/StringToArray.js diff --git a/Control/lib/common/StringToArray.js b/Control/lib/common/StringToArray.js new file mode 100644 index 000000000..7c48f7780 --- /dev/null +++ b/Control/lib/common/StringToArray.js @@ -0,0 +1,16 @@ +/** + * Convert string to list + * @param {String|Array} data + * @returns {Array} list + */ +const stringToArray = (data) => { + + let list = []; + if (typeof data === 'string') { + list = data.split(','); + } else if (Array.isArray(data)) { + list = data; + } + return list; +} +exports.stringToArray = stringToArray; \ No newline at end of file diff --git a/Control/lib/middleware/detectorOwnership.middleware.js b/Control/lib/middleware/detectorOwnership.middleware.js index 121e9e2a9..412904750 100644 --- a/Control/lib/middleware/detectorOwnership.middleware.js +++ b/Control/lib/middleware/detectorOwnership.middleware.js @@ -1,7 +1,7 @@ const {User} = require('../dtos/User'); const {isRoleSufficient,Role} = require('../common/role.enum.js'); const {UnauthorizedAccessError} = require('./../errors/UnauthorizedAccessError.js'); - +const {stringToArray} = require('../common/StringToArray.js'); const {updateExpressResponseFromNativeError} = require('./../errors/updateExpressResponseFromNativeError.js'); /** * Middleware function to check detector ownership. @@ -20,14 +20,8 @@ const detectorOwnershipMiddleware = (req, res, next) => { } try { - - - let accessList = []; - if (typeof access === 'string') { - accessList = access.split(','); - } else if (Array.isArray(access)) { - accessList = access; - } + // Convert access string to Array + let accessList = stringToArray(access); // Check if the user's role is sufficient to bypass the ownership check if (accessList?.some((role) => { return isRoleSufficient(role, Role.GLOBAL) diff --git a/Control/lib/middleware/minimumRole.middleware.js b/Control/lib/middleware/minimumRole.middleware.js index bee85d2d7..b0212add1 100644 --- a/Control/lib/middleware/minimumRole.middleware.js +++ b/Control/lib/middleware/minimumRole.middleware.js @@ -15,7 +15,7 @@ const {isRoleSufficient} = require('../common/role.enum.js'); const {UnauthorizedAccessError} = require('../errors/UnauthorizedAccessError.js'); const {updateExpressResponseFromNativeError} = require('../errors/updateExpressResponseFromNativeError.js'); - +const {stringToArray} = require('../common/StringToArray.js'); /** * Method to receive a minimum role that needs to be met by owner of request and to return a middleware function * @param {Role} minimumRole - minimum role that should be fulfilled by the requestor @@ -33,12 +33,7 @@ const minimumRoleMiddleware = (minimumRole) => { try { const { access } = req?.session ?? ''; - let accessList = []; - if (typeof access === 'string') { - accessList = access.split(','); - } else if (Array.isArray(access)) { - accessList = access; - } + let accessList = stringToArray(access); const isAllowed = accessList.some((role) => isRoleSufficient(role, minimumRole)); if (!isAllowed) { updateExpressResponseFromNativeError(res, From 9d6ddd98b8aba15e8f1b2deefd9cf0387be890e1 Mon Sep 17 00:00:00 2001 From: pdik Date: Wed, 4 Dec 2024 09:33:10 +0100 Subject: [PATCH 11/14] Removed unsused Import --- .../lib/middleware/mocha-detectorOwnership.middleware.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js index d8f7f98d5..8238a4476 100644 --- a/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js +++ b/Control/test/lib/middleware/mocha-detectorOwnership.middleware.test.js @@ -3,7 +3,6 @@ const sinon = require('sinon'); const { User } = require('../../../lib/dtos/User'); const { detectorOwnershipMiddleware } = require('../../../lib/middleware/detectorOwnership.middleware'); -const {Role} = require('../../../lib/common/role.enum.js'); describe('`DetectorOwnerShipmiddleware` test suite', () => { let userStub; From ae41c5f29038fd352b38dd515d9e79d4cb63c2dc Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 9 Dec 2024 09:14:37 +0100 Subject: [PATCH 12/14] Added stringToArray mocha tests --- .../lib/common/mocha-stringToArray.test.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Control/test/lib/common/mocha-stringToArray.test.js diff --git a/Control/test/lib/common/mocha-stringToArray.test.js b/Control/test/lib/common/mocha-stringToArray.test.js new file mode 100644 index 000000000..ab1223675 --- /dev/null +++ b/Control/test/lib/common/mocha-stringToArray.test.js @@ -0,0 +1,28 @@ +const { stringToArray } = require('../../../lib/common/StringToArray'); +const assert = require('assert'); + +describe('stringToArray', () => { + it('converts comma-separated string to array', () => { + const input = 'a,b,c'; + const expectedOutput = ['a', 'b', 'c']; + assert.deepStrictEqual(stringToArray(input), expectedOutput); + }); + + it('returns array as is', () => { + const input = ['a', 'b', 'c']; + const expectedOutput = ['a', 'b', 'c']; + assert.deepStrictEqual(stringToArray(input), expectedOutput); + }); + + it('returns empty array for empty string', () => { + const input = ''; + const expectedOutput = []; + assert.deepStrictEqual(stringToArray(input), expectedOutput); + }); + + it('returns empty array for empty array', () => { + const input = []; + const expectedOutput = []; + assert.deepStrictEqual(stringToArray(input), expectedOutput); + }); +}); \ No newline at end of file From 560a9fe00b50802fdc8c732ed404fe2219f83932 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 9 Dec 2024 09:18:42 +0100 Subject: [PATCH 13/14] Fixed Mocha stringToArray Test --- Control/lib/common/StringToArray.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Control/lib/common/StringToArray.js b/Control/lib/common/StringToArray.js index 7c48f7780..775ea116c 100644 --- a/Control/lib/common/StringToArray.js +++ b/Control/lib/common/StringToArray.js @@ -6,7 +6,7 @@ const stringToArray = (data) => { let list = []; - if (typeof data === 'string') { + if (typeof data === 'string' && data.length > 0) { list = data.split(','); } else if (Array.isArray(data)) { list = data; From 0aa46e189587caa6a415443a87759108318a5703 Mon Sep 17 00:00:00 2001 From: pdik Date: Mon, 9 Dec 2024 11:46:01 +0100 Subject: [PATCH 14/14] Fixed Api-lock tests in URL detectorid is uppercase while in accesslist not so convert them to lower --- Control/lib/dtos/User.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Control/lib/dtos/User.js b/Control/lib/dtos/User.js index 723841218..2384f65db 100644 --- a/Control/lib/dtos/User.js +++ b/Control/lib/dtos/User.js @@ -72,6 +72,10 @@ class User { * @returns {Boolean} */ belongsToDetector(detectorId) { + //Convert detectorId to lowercase if it is uppercase + if (detectorId === detectorId.toUpperCase()) { + detectorId = detectorId.toLowerCase(); + } return Boolean(this._accessList.includes(`det-${detectorId}`)); }