Skip to content

Commit

Permalink
[O2B-1216] Change QC flag delete button accessibility to admins only (#…
Browse files Browse the repository at this point in the history 
…1521)

* check for admin

* restrict to admin

* fix test
  • Loading branch information
@xsalonx
xsalonx authored Apr 24, 2024
1 parent 2086a4e commit e789e65
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 102 deletions.
5 changes: 3 additions & 2 deletions lib/public/views/QcFlags/details/forDataPass/QcFlagDetailsForDataPassPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* or submit itself to any jurisdiction.
*/

import { BkpRoles } from '../../../../domain/enums/BkpRoles.js';
import { detailsList } from '../../../../components/Detail/detailsList.js';
import errorAlert from '../../../../components/common/errorAlert.js';
import { deleteButton } from '../../../../components/common/form/deleteButton.js';
Expand All @@ -22,7 +23,7 @@ import { PanelComponent } from '../../../../components/common/panel/PanelCompone
import { tooltip } from '../../../../components/common/popover/tooltip.js';
import spinner from '../../../../components/common/spinner.js';
import { qcFlagDetailsConfiguration } from '../qcFlagDetailsConfiguration.js';
import { h, iconWarning } from '/js/src/index.js';
import { h, iconWarning, sessionService } from '/js/src/index.js';

/**
* Render QC flag details for data pass page
Expand All @@ -41,7 +42,7 @@ export const QcFlagDetailsForDataPassPage = ({ qcFlags: { detailsForDataPassMode
return h('', [
h('.flex-row.justify-between.items-center', [
h('h2', 'QC Flag Details'),
h('.pv3', deleteResult.match({
sessionService.hasAccess(BkpRoles.ADMIN) && h('.pv3', deleteResult.match({
Loading: () => deleteButton(null, 'Processing...'),
Success: () => deleteButton(null, 'Processed!'),
Failure: () => deleteButton(() => detailsForDataPassModel.delete()),
Expand Down
5 changes: 3 additions & 2 deletions lib/public/views/QcFlags/details/forSimulationPass/QcFlagDetailsForSimulationPassPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import { LabelPanelHeaderComponent } from '../../../../components/common/panel/L
import { PanelComponent } from '../../../../components/common/panel/PanelComponent.js';
import { tooltip } from '../../../../components/common/popover/tooltip.js';
import spinner from '../../../../components/common/spinner.js';
import { BkpRoles } from '../../../../domain/enums/BkpRoles.js';
import { qcFlagDetailsConfiguration } from '../qcFlagDetailsConfiguration.js';
import { h, iconWarning } from '/js/src/index.js';
import { h, iconWarning, sessionService } from '/js/src/index.js';

/**
* Render QC flag details for simulation pass page
Expand All @@ -41,7 +42,7 @@ export const QcFlagDetailsForSimulationPassPage = ({ qcFlags: { detailsForSimula
return h('', [
h('.flex-row.justify-between.items-center', [
h('h2', 'QC Flag Details'),
h('.pv3', deleteResult.match({
sessionService.hasAccess(BkpRoles.ADMIN) && h('.pv3', deleteResult.match({
Loading: () => deleteButton(null, 'Processing...'),
Success: () => deleteButton(null, 'Processed!'),
Failure: () => deleteButton(() => detailsForSimulationPassModel.delete()),
Expand Down
7 changes: 1 addition & 6 deletions lib/server/controllers/qcFlag.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,7 @@ const deleteQcFlagByIdHandler = async (req, res) => {
);
if (validatedDTO) {
try {
const userWithRoles = {
externalUserId: validatedDTO.session.externalId,
roles: validatedDTO.session?.access ?? [],
};

const qcFlag = await qcFlagService.delete(validatedDTO.params.id, { userWithRoles });
const qcFlag = await qcFlagService.delete(validatedDTO.params.id);
res.json({ data: qcFlag });
} catch (error) {
updateExpressResponseFromNativeError(res, error);
Expand Down
4 changes: 3 additions & 1 deletion lib/server/routers/qcFlag.router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
* or submit itself to any jurisdiction.
*/

const { BkpRoles } = require('../../domain/enums/BkpRoles.js');
const { QcFlagController } = require('../controllers/qcFlag.controller.js');
const { rbacMiddleware } = require('../middleware/rbac.middleware.js');

exports.qcFlagsRouter = {
path: '/qcFlags',
Expand All @@ -35,7 +37,7 @@ exports.qcFlagsRouter = {
},
{
method: 'delete',
controller: QcFlagController.deleteQcFlagByIdHandler,
controller: [rbacMiddleware(BkpRoles.ADMIN), QcFlagController.deleteQcFlagByIdHandler],
},
{
method: 'post',
Expand Down
12 changes: 1 addition & 11 deletions lib/server/services/qualityControlFlag/QcFlagService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const { BadParameterError } = require('../../errors/BadParameterError.js');
const { NotFoundError } = require('../../errors/NotFoundError.js');
const { getUserOrFail } = require('../user/getUserOrFail.js');
const { AccessDeniedError } = require('../../errors/AccessDeniedError.js');
const { BkpRoles } = require('../../../domain/enums/BkpRoles.js');
const { ConflictError } = require('../../errors/ConflictError.js');

const NON_QC_DETECTORS = new Set(['TST']);
Expand Down Expand Up @@ -284,11 +283,9 @@ class QcFlagService {
/**
* Delete single instance of QC flag
* @param {number} id QC flag id
* @param {object} relations QC Flag entity relations
* @param {UserWithRoles} relations.userWithRoles user with roles
* @return {Promise<QcFlag>} promise
*/
async delete(id, relations) {
async delete(id) {
return dataSource.transaction(async () => {
const qcFlag = await QcFlagRepository.findOne({
where: { id },
Expand All @@ -302,13 +299,6 @@ class QcFlagService {
throw new ConflictError('Cannot delete QC flag which is verified');
}

const { userWithRoles: { userId, externalUserId, roles = [] } } = relations;
const user = await getUserOrFail({ userId, externalUserId });

if (qcFlag.createdById !== user.id && !roles.includes(BkpRoles.ADMIN)) {
throw new AccessDeniedError('You are not allowed to remove this QC flag');
}

await qcFlag.removeDataPasses(qcFlag.dataPasses);
await qcFlag.removeSimulationPasses(qcFlag.simulationPasses);
return QcFlagRepository.removeOne({ where: { id } });
Expand Down
34 changes: 4 additions & 30 deletions test/api/qcFlags.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ module.exports = () => {
describe('DELETE /api/qcFlags/:id', () => {
it('should fail to delete QC flag which is verified', async () => {
const id = 4;
const response = await request(server).delete(`/api/qcFlags/${id}`);
const response = await request(server).delete(`/api/qcFlags/${id}?token=admin`);
expect(response.status).to.be.equal(409);
const { errors } = response.body;
expect(errors).to.be.eql([
Expand All @@ -228,16 +228,15 @@ module.exports = () => {
},
]);
});
it('should fail to delete QC flag when being neither owner nor admin', async () => {
it('should fail to delete QC flag when not being admin', async () => {
const id = 5;
const response = await request(server).delete(`/api/qcFlags/${id}`);
expect(response.status).to.be.equal(403);
const { errors } = response.body;
expect(errors).to.be.eql([
{
status: 403,
title: 'Access Denied',
detail: 'You are not allowed to remove this QC flag',
status: '403',
title: 'Access denied',
},
]);
});
Expand All @@ -248,31 +247,6 @@ module.exports = () => {
expect(response.status).to.be.equal(200);
expect(response.body.data.id).to.be.equal(id);
});
it('should succesfuly delete QC flag as owner', async () => {
const qcFlagCreationParameters = {
from: new Date('2019-08-09 01:29:50').getTime(),
to: new Date('2019-08-09 05:40:00').getTime(),
comment: 'VERY INTERESTING REMARK',
flagTypeId: 2,
runNumber: 106,
dataPassId: 1,
dplDetectorId: 1,
};

const creationReponse = await request(server).post('/api/qcFlags').send(qcFlagCreationParameters);
expect(creationReponse.status).to.be.equal(201);
const { id } = creationReponse.body.data;

let fetchedQcFlag = await QcFlagRepository.findOne({ where: { id } });
expect(fetchedQcFlag.id).to.be.equal(id);

const deletionResponse = await request(server).delete(`/api/qcFlags/${id}`);
expect(deletionResponse.status).to.be.equal(200);
expect(deletionResponse.body.data.id).to.be.equal(id);

fetchedQcFlag = await QcFlagRepository.findOne({ where: { id } });
expect(fetchedQcFlag).to.be.equal(null);
});
});

describe('POST /api/qcFlags/:id/verify', () => {
Expand Down
54 changes: 4 additions & 50 deletions test/lib/server/services/qualityControlFlag/QcFlagService.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const assert = require('assert');
const { BadParameterError } = require('../../../../../lib/server/errors/BadParameterError.js');
const { qcFlagService } = require('../../../../../lib/server/services/qualityControlFlag/QcFlagService.js');
const { AccessDeniedError } = require('../../../../../lib/server/errors/AccessDeniedError.js');
const { BkpRoles } = require('../../../../../lib/domain/enums/BkpRoles');
const { ConflictError } = require('../../../../../lib/server/errors/ConflictError');

const qcFlagWithId1 = {
Expand Down Expand Up @@ -482,58 +481,16 @@ module.exports = () => {
new ConflictError('Cannot delete QC flag which is verified'),
);
});
it('should fail to delete QC flag of dataPass when being neither owner nor admin', async () => {
const id = 1;
const relations = {
userWithRoles: { externalUserId: 456 },
};
await assert.rejects(
() => qcFlagService.delete(id, relations),
new AccessDeniedError('You are not allowed to remove this QC flag'),
);
});
it('should succesfuly delete QC flag of dataPass as admin', async () => {
const id = 2;
const relations = {
userWithRoles: { externalUserId: 456, roles: [BkpRoles.ADMIN] },
};

await qcFlagService.delete(id, relations);
const fetchedQcFlag = await qcFlagService.getById(id);
expect(fetchedQcFlag).to.be.equal(null);
});
it('should succesfuly delete QC flag of dataPass as owner', async () => {
it('should succesfuly delete QC flag of dataPass', async () => {
const id = 1;
const relations = {
userWithRoles: { externalUserId: 1 },
};

await qcFlagService.delete(id, relations);
await qcFlagService.delete(id);
const fetchedQcFlag = await qcFlagService.getById(id);
expect(fetchedQcFlag).to.be.equal(null);
});

it('should fail to delete QC flag of simulationPass when being neither owner nor admin', async () => {
const id = 5;
const relations = {
userWithRoles: { externalUserId: 1 },
};
await assert.rejects(
() => qcFlagService.delete(id, relations),
new AccessDeniedError('You are not allowed to remove this QC flag'),
);
});
it('should succesfuly delete QC flag of simulationPass as admin', async () => {
const id = 5;
const relations = {
userWithRoles: { externalUserId: 456, roles: [BkpRoles.ADMIN] },
};

await qcFlagService.delete(id, relations);
const fetchedQcFlag = await qcFlagService.getById(id);
expect(fetchedQcFlag).to.be.equal(null);
});
it('should succesfuly delete QC flag of simulationPass as owner', async () => {
it('should succesfuly delete QC flag of simulationPass ', async () => {
const creationRelations = {
user: {
externalUserId: 1,
Expand All @@ -545,11 +502,8 @@ module.exports = () => {
};

const { id } = await qcFlagService.createForSimulationPass({}, creationRelations);
const deleteRelations = {
userWithRoles: { externalUserId: 1 },
};

await qcFlagService.delete(id, deleteRelations);
await qcFlagService.delete(id);
const fetchedQcFlag = await qcFlagService.getById(id);
expect(fetchedQcFlag).to.be.equal(null);
});
Expand Down

0 comments on commit e789e65

Please sign in to comment.