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 6 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 @@ -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');
Expand Down Expand Up @@ -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',
Expand Down
9 changes: 9 additions & 0 deletions Control/lib/dtos/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 33 additions & 0 deletions Control/lib/middleware/detectorOwnership.middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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 { 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);
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
console.log(`Checking locks for detector ${detectorId}`);
Fixed Show fixed Hide fixed
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
if (!u.belongsToDetector(detectorId)) {
return res
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
.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);
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
res.status(500).json({ message: 'Internal server error' });
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
}
};

exports.detectorOwnershipMiddleware = detectorOwnershipMiddleware;
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
const assert = require('assert');
const sinon = require('sinon');
const express = require('express');
Fixed Show fixed Hide fixed
const request = require('supertest');
Fixed Show fixed Hide fixed
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 () => {
pepijndik marked this conversation as resolved.
Show resolved Hide resolved
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',
});
});
});
Loading