Skip to content

Commit

Permalink
[OGUI-1436] Adds lock check before deploying new environments (#2191)
Browse files Browse the repository at this point in the history
* updates the lock service to naming conventions
* makes use of the service to check that the user has the lock taken before deploying environment
* displays short message of error on request failure
  • Loading branch information
graduta authored Nov 3, 2023
1 parent 1d536c9 commit a768881
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 28 deletions.
16 changes: 8 additions & 8 deletions Control/lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ const {BroadcastService} = require('./services/Broadcast.service.js');
const {CacheService} = require('./services/Cache.service.js');
const {EnvironmentService} = require('./services/Environment.service.js');
const {Intervals} = require('./services/Intervals.service.js');
const {LockService} = require('./services/Lock.service.js');
const {RunService} = require('./services/Run.service.js');
const {StatusService} = require('./services/Status.service.js');
const {WorkflowTemplateService} = require('./services/WorkflowTemplate.service.js');
const Lock = require('./services/Lock.js');

// web-ui services
const {NotificationService, ConsulService} = require('@aliceo2/web-ui');
Expand All @@ -59,8 +59,8 @@ if (!config.grafana) {
}

module.exports.setup = (http, ws) => {
const lock = new Lock();
lock.setWs(ws);
const lockService = new LockService();
lockService.setWs(ws);

let consulService;
if (config.consul) {
Expand All @@ -82,7 +82,7 @@ module.exports.setup = (http, ws) => {
const envService = new EnvironmentService(ctrlProxy, apricotService, cacheService, broadcastService);
const workflowService = new WorkflowTemplateService(ctrlProxy, apricotService);

const envCtrl = new EnvironmentController(envService, workflowService);
const envCtrl = new EnvironmentController(envService, workflowService, lockService);
const workflowController = new WorkflowTemplateController(workflowService);

const aliecsReqHandler = new AliecsRequestHandler(ctrlService);
Expand Down Expand Up @@ -145,10 +145,10 @@ module.exports.setup = (http, ws) => {
http.post('/execute/o2-roc-config', coreMiddleware, (req, res) => ctrlService.createAutoEnvironment(req, res));

// Lock Service
http.post('/lockState', (req, res) => res.json(lock.state(req.body.name)));
http.post('/lock', (req, res) => lock.lockDetector(req, res));
http.post('/unlock', (req, res) => lock.unlockDetector(req, res));
http.post('/forceUnlock', (req, res) => lock.forceUnlock(req, res));
http.post('/lockState', (req, res) => res.json(lockService.state(req.body.name)));
http.post('/lock', (req, res) => lockService.lockDetector(req, res));
http.post('/unlock', (req, res) => lockService.unlockDetector(req, res));
http.post('/forceUnlock', (req, res) => lockService.forceUnlock(req, res));

// Status Service
http.get('/status/consul', statusController.getConsulStatus.bind(statusController));
Expand Down
18 changes: 15 additions & 3 deletions Control/lib/controllers/Environment.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
*/
const {Log} = require('@aliceo2/web-ui');
const {updateExpressResponseFromNativeError} = require('./../errors/updateExpressResponseFromNativeError.js');
const {InvalidInputError} = require('../errors/InvalidInputError.js');
const {InvalidInputError} = require('./../errors/InvalidInputError.js');
const {UnauthorizedAccessError} = require('./../errors/UnauthorizedAccessError.js');

/**
* Controller for dealing with all API requests on environments from AliECS:
Expand All @@ -23,8 +24,9 @@ class EnvironmentController {
* Constructor for initializing controller of environments
* @param {EnvironmentService} envService - service to use to query AliECS with regards to environments
* @param {WorkflowTemplateService} workflowService - service to use to query Apricot for workflow details
* @param {LockService} lockService - service to use to check lock is taken
*/
constructor(envService, workflowService) {
constructor(envService, workflowService, lockService) {
this._logger = new Log(`${process.env.npm_config_log_label ?? 'cog'}/env-ctrl`);

/**
Expand All @@ -36,6 +38,11 @@ class EnvironmentController {
* @type {WorkflowTemplateService}
*/
this._workflowService = workflowService;

/**
* @type {LockService}
*/
this._lockService = lockService;
}

/**
Expand Down Expand Up @@ -65,11 +72,16 @@ class EnvironmentController {
* @returns {void}
*/
async newAutoEnvironmentHandler(req, res) {
// TODO LOCK TAKEN
const {personid, name} = req.session;
let workflowTemplatePath;
let variables;
const {detector, runType, configurationName} = req.body;

if (!this._lockService.isLockTakenByUser(detector, personid, name)) {
updateExpressResponseFromNativeError(res, new UnauthorizedAccessError('Lock not taken'));
return;
}

if (!configurationName) {
updateExpressResponseFromNativeError(res, new InvalidInputError('Missing Configuration Name for deployment'));
return;
Expand Down
19 changes: 19 additions & 0 deletions Control/lib/errors/UnauthorizedAccessError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* @license
* Copyright CERN and copyright holders of ALICE O2. This software is
* distributed under the terms of the GNU General Public License v3 (GPL
* Version 3), copied verbatim in the file "COPYING".
*
* See http://alice-o2.web.cern.ch/license for full licensing information.
*
* 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.
*/

/**
* Specific error to throw when an user does not have permissions for the action
*/
class UnauthorizedAccessError extends Error {}

exports.UnauthorizedAccessError = UnauthorizedAccessError;
4 changes: 4 additions & 0 deletions Control/lib/errors/updateExpressResponseFromNativeError.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* or submit itself to any jurisdiction.
*/

const {UnauthorizedAccessError} = require('./UnauthorizedAccessError.js');
const {InvalidInputError} = require('./InvalidInputError.js');
const {NotFoundError} = require('./NotFoundError.js');
const {TimeoutError} = require('./TimeoutError.js');
Expand All @@ -30,6 +31,9 @@ const updateExpressResponseFromNativeError = (response, error) => {
case InvalidInputError:
status = 400;
break;
case UnauthorizedAccessError:
status = 403;
break;
case NotFoundError:
status = 404;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,20 @@
* or submit itself to any jurisdiction.
*/
const {WebSocketMessage, Log} = require('@aliceo2/web-ui');
const log = new Log(`${process.env.npm_config_log_label ?? 'cog'}/lockservice`);
const {errorHandler} = require('./../utils.js');
const {errorHandler} = require('../utils.js');

/**
* Model representing the lock of the UI, one owner at a time
*/
class Lock {
class LockService {
/**
* Initialize lock as free / unlocked.
*/
constructor() {
this.lockedBy = {};
this.lockedByName = {};

this._logger = new Log(`${process.env.npm_config_log_label ?? 'cog'}/lockservice`);
}

/**
Expand All @@ -51,7 +52,7 @@ class Lock {
*/
broadcastLockState() {
this.webSocket.broadcast(new WebSocketMessage().setCommand('padlock-update').setPayload(this.state()));
}
}

/**
* Method to try to acquire lock with given name
Expand All @@ -76,10 +77,10 @@ class Lock {
return
}
throw new Error(`Lock ${entity} is already hold by ${this.lockedByName[entity]} (id ${this.lockedBy[entity]})`);
}
}
this.lockedBy[entity] = req.session.personid;
this.lockedByName[entity] = req.session.name;
log.info(`Lock ${entity} taken by ${req.session.name}`);
this._logger.info(`Lock ${entity} taken by ${req.session.name}`);
this.broadcastLockState();
res.status(201).json({
lockedBy: this.lockedBy,
Expand All @@ -106,21 +107,21 @@ class Lock {
lockedBy: this.lockedBy,
lockedByName: this.lockedByName
});
}
}
if (!req.session.access.includes('admin')) {
throw new Error(`Insufficient permission`);
}
}
delete this.lockedBy[entity];
delete this.lockedByName[entity];
log.info(`Lock ${entity} forced by ${req.session.name}`);
this._logger.info(`Lock ${entity} forced by ${req.session.name}`);
this.broadcastLockState();
res.status(200).json({
lockedBy: this.lockedBy,
lockedByName: this.lockedByName
});
} catch (error) {
errorHandler(`Unable to force lock by ${req.session.name}: ${error}`, res, 403, 'lockservice');
}
}
}

/**
Expand All @@ -146,16 +147,26 @@ class Lock {
}
delete this.lockedBy[entity];
delete this.lockedByName[entity];
log.info(`Lock ${entity} released by ${req.session.name}`);
this._logger.info(`Lock ${entity} released by ${req.session.name}`);
this.broadcastLockState();
res.status(200).json({
lockedBy: this.lockedBy,
lockedByName: this.lockedByName
});
} catch (error) {
errorHandler(`Unable to give away lock to ${req.session.name}: ${error}`, res, 403, 'lockservice');
}
}
}

/**
* Method to check if lock is taken by specific user
* @param {String} detector - detector for which check should be done
* @param {Number} userId - id of the user that should be checked against
* @param {String} userName - username of the user that should be checked against
*/
isLockTakenByUser(detector, userId, userName) {
return this.lockedBy[detector] === userId && this.lockedByName[detector] === userName;
}
}

module.exports = Lock;
module.exports = {LockService};
4 changes: 2 additions & 2 deletions Control/public/pages/CalibrationRuns/CalibrationRuns.model.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class CalibrationRunsModel extends Observable {
this._calibrationRuns = RemoteData.loading();
this.notify();

const {result, ok} = await this._model.loader.get('/api/runs/calibration');
const {result, ok} = await this._model.loader.get('/api/runs/calibration', true);
this._calibrationRuns = ok ? RemoteData.success(result) : RemoteData.failure(result.message);

this.notify();
Expand All @@ -74,7 +74,7 @@ export class CalibrationRunsModel extends Observable {
const payload = {
detector, runType, configurationName
};
const {result, ok} = await this._model.loader.post('/api/environment/auto', payload);
const {result, ok} = await this._model.loader.post('/api/environment/auto', payload, true);

this._calibrationRuns.payload[detector][runType].ongoingCalibrationRun =
ok ? RemoteData.success(result) : RemoteData.failure(result.message);
Expand Down
4 changes: 2 additions & 2 deletions Control/test/lib/mocha-lock.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

const assert = require('assert');
const sinon = require('sinon');
const Lock = require('../../lib/services/Lock.js');
const {LockService} = require('../../lib/services/Lock.service.js');

describe('Lock test suite', () => {
let req, res, fakeWs, lock;
Expand All @@ -37,7 +37,7 @@ describe('Lock test suite', () => {
json: sinon.spy(),
send: sinon.spy(),
};
lock = new Lock();
lock = new LockService();
lock.setWs(fakeWs);
});

Expand Down

0 comments on commit a768881

Please sign in to comment.