Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OGUI-1580] add middleware for detector matching user role #2683

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Control/lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const {minimumRoleMiddleware} = require('./middleware/minimumRole.middleware.js'
const {addDetectorIdMiddleware} = require('./middleware/addDetectorId.middleware.js');
const {DetectorId} = require('./common/detectorId.enum.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');
Expand Down Expand Up @@ -200,6 +200,7 @@ module.exports.setup = (http, ws) => {

http.put('/locks/:action/:detectorId',
minimumRoleMiddleware(Role.DETECTOR),
detectorOwnershipMiddleware,
lockController.actionLockHandler.bind(lockController)
);
http.put('/locks/force/:action/:detectorId',
Expand Down
16 changes: 16 additions & 0 deletions Control/lib/common/StringToArray.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Convert string to list
* @param {String|Array} data
* @returns {Array} list
*/
const stringToArray = (data) => {

let list = [];
if (typeof data === 'string' && data.length > 0) {
list = data.split(',');
} else if (Array.isArray(data)) {
list = data;
}
return list;
}
exports.stringToArray = stringToArray;
13 changes: 13 additions & 0 deletions Control/lib/dtos/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@ class User {
return Boolean(accessList.includes('admin'));
}

/**
* Checks if the given user can access the given detector
* @param {string} detectorId
* @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}`));
}

/**
* Check if provided details of a user are the same as the current instance one;
* @param {User} user - to compare to
Expand Down
43 changes: 43 additions & 0 deletions Control/lib/middleware/detectorOwnership.middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
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.
* 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 { name, username, personid, access } = req.session || {};

if (!detectorId || !access) {
return updateExpressResponseFromNativeError(res,
new UnauthorizedAccessError('Invalid request: missing information'));
}

try {
// 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)
})) {
next();
}
const user = new User(username, name, personid, access);
if (!user.belongsToDetector(detectorId)) {
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
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) {
return updateExpressResponseFromNativeError(res, error);
}
};

exports.detectorOwnershipMiddleware = detectorOwnershipMiddleware;
9 changes: 2 additions & 7 deletions Control/lib/middleware/minimumRole.middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions Control/test/lib/common/mocha-stringToArray.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
const assert = require('assert');
const sinon = require('sinon');
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(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: ['GLOBAL'] }};
const res = { status: sinon.stub().returnsThis(), json: sinon.stub() };
const next = sinon.stub();

detectorOwnershipMiddleware(req, res, next);

assert.ok(next.calledOnce);

});

});
Loading