Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

Commit

Permalink
fix: adding validation for enhanced study security (#211)
Browse files Browse the repository at this point in the history 
* fix: adding validation for enhanced study security

* feat: adding unit tests for study service
  • Loading branch information
@SanketD92
SanketD92 authored Nov 18, 2020
1 parent 8d869ec commit 6e57e51
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 9 deletions.
5 changes: 5 additions & 0 deletions addons/addon-base-raas/packages/base-raas-services/lib/helpers/is-role.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,15 @@ function isAdmin(requestContext) {
return isRole(requestContext, 'admin');
}

function isSystem(requestContext) {
return _.get(requestContext, 'principalIdentifier.uid') === '_system_';
}

module.exports = {
isInternalResearcher,
isExternalResearcher,
isInternalGuest,
isExternalGuest,
isAdmin,
isSystem,
};
94 changes: 88 additions & 6 deletions addons/addon-base-raas/packages/base-raas-services/lib/study/__tests__/study-service.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,96 @@ describe('studyService', () => {
});
// OPERATE
try {
await service.create({ principal: { userRole: 'admin' } }, dataIpt);
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, dataIpt);
expect.hasAssertions();
} catch (err) {
// CHECK
expect(err.message).toEqual('study with id "doppelganger" already exists');
}
});

it('should fail if non-system user is trying to create Open Data study', async () => {
// BUILD
const dataIpt = {
id: 'newOpenStudy',
category: 'Open Data',
};

// OPERATE
try {
await service.create(
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: 'someRandomUserUid' } },
dataIpt,
);
expect.hasAssertions();
} catch (err) {
// CHECK
expect(err.message).toEqual('Only the system can create Open Data studies.');
}
});

it('should pass if system is trying to create Open Data study', async () => {
// BUILD
const dataIpt = {
id: 'newOpenStudy',
category: 'Open Data',
};
service.audit = jest.fn();

// OPERATE
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, dataIpt);

// CHECK
expect(dbService.table.update).toHaveBeenCalled();
expect(service.audit).toHaveBeenCalledWith(
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } },
{ action: 'create-study', body: undefined },
);
});

it('should fail if non-Open Data study type has non-empty resources list', async () => {
// BUILD
const dataIpt = {
id: 'newOpenStudy',
category: 'Organization',
projectId: 'existingProjId',
resources: [{ arn: 'arn:aws:s3:::someRandomStudyArn' }],
};
projectService.verifyUserProjectAssociation.mockImplementationOnce(() => true);

// OPERATE
try {
await service.create(
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: 'someRandomUserUid' } },
dataIpt,
);
expect.hasAssertions();
} catch (err) {
// CHECK
expect(err.message).toEqual('Resources can only be assigned to Open Data study category');
}
});

it('should pass if Open Data study type has non-empty resources list', async () => {
// BUILD
const dataIpt = {
id: 'newOpenStudy',
category: 'Open Data',
resources: [{ arn: 'arn:aws:s3:::someRandomStudyArn' }],
};
service.audit = jest.fn();

// OPERATE
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, dataIpt);

// CHECK
expect(dbService.table.update).toHaveBeenCalled();
expect(service.audit).toHaveBeenCalledWith(
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } },
{ action: 'create-study', body: undefined },
);
});

it('should try to create the study successfully', async () => {
// BUILD
const dataIpt = {
Expand All @@ -167,12 +249,12 @@ describe('studyService', () => {
service.audit = jest.fn();

// OPERATE
await service.create({ principal: { userRole: 'admin' } }, dataIpt);
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, dataIpt);

// CHECK
expect(dbService.table.update).toHaveBeenCalled();
expect(service.audit).toHaveBeenCalledWith(
{ principal: { userRole: 'admin' } },
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } },
{ action: 'create-study', body: undefined },
);
});
Expand All @@ -188,12 +270,12 @@ describe('studyService', () => {
service.audit = jest.fn();

// OPERATE
await service.create({ principal: { userRole: 'admin' } }, dataIpt);
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, dataIpt);

// CHECK
expect(dbService.table.update).toHaveBeenCalled();
expect(service.audit).toHaveBeenCalledWith(
{ principal: { userRole: 'admin' } },
{ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } },
{ action: 'create-study', body: undefined },
);
});
Expand Down Expand Up @@ -298,7 +380,7 @@ describe('studyService', () => {

// OPERATE
try {
await service.create({ principal: { userRole: 'admin' } }, ipt);
await service.create({ principal: { userRole: 'admin' }, principalIdentifier: { uid: '_system_' } }, ipt);
expect.hasAssertions();
} catch (err) {
// CHECK
Expand Down
13 changes: 10 additions & 3 deletions addons/addon-base-raas/packages/base-raas-services/lib/study/study-service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const Service = require('@aws-ee/base-services-container/lib/service');
const { runAndCatch } = require('@aws-ee/base-services/lib/helpers/utils');

const { buildTaggingXml } = require('../helpers/aws-tags');
const { isInternalResearcher, isAdmin } = require('../helpers/is-role');
const { isInternalResearcher, isAdmin, isSystem } = require('../helpers/is-role');
const createSchema = require('../schema/create-study');
const updateSchema = require('../schema/update-study');

Expand Down Expand Up @@ -89,6 +89,9 @@ class StudyService extends Service {
if (!(isInternalResearcher(requestContext) || isAdmin(requestContext))) {
throw this.boom.forbidden('Only admin and internal researcher are authorized to create studies. ');
}
if (rawData.category === 'Open Data' && !isSystem(requestContext)) {
throw this.boom.badRequest('Only the system can create Open Data studies.', true);
}
const [validationService, projectService] = await this.service(['jsonSchemaValidationService', 'projectService']);

// Validate input
Expand All @@ -105,13 +108,17 @@ class StudyService extends Service {
if (rawData.category !== 'Open Data') {
const projectId = rawData.projectId;
if (!projectId) {
throw this.boom.badRequest('Missing required projectId');
throw this.boom.badRequest('Missing required projectId', true);
}
// Verify user has access to the project the new study will be associated with
if (!(await projectService.verifyUserProjectAssociation(by, projectId))) {
throw this.boom.forbidden(`Not authorized to add study related to project "${projectId}"`);
throw this.boom.forbidden(`Not authorized to add study related to project "${projectId}"`, true);
}
await projectService.mustFind(requestContext, { id: rawData.projectId });
// Verify user is not trying to create resources for non-Open data studies
if (!_.isEmpty(rawData.resources)) {
throw this.boom.badRequest('Resources can only be assigned to Open Data study category', true);
}
}

const id = rawData.id;
Expand Down

0 comments on commit 6e57e51

Please sign in to comment.