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

Commit

Permalink
feat: added delete permissions, handled add logic (#209)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SanketD92
SanketD92 authored Nov 13, 2020
1 parent 423b5b2 commit c819e28
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 16 deletions.
116 changes: 111 additions & 5 deletions ...s/packages/base-raas-services/lib/environment/__tests__/environment-mount-service.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ describe('EnvironmentMountService', () => {
await service.applyWorkspacePermissions(studyId, updateRequest);

// CHECK
expect(service.addPermissions).toHaveBeenCalledWith(
[{ uid: 'User1-UID', permissionLevel: 'readonly' }],
studyId,
updateRequest,
);
expect(service.addPermissions).toHaveBeenCalledWith([{ uid: 'User1-UID', permissionLevel: 'readonly' }], studyId);
expect(service.removePermissions).not.toHaveBeenCalled();
expect(service.updatePermissions).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -640,6 +636,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`${studyBucket}/${studyPrefix}`],
},
Expand Down Expand Up @@ -707,6 +704,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: ['StudyBucketPath_XYZ'],
},
Expand Down Expand Up @@ -754,6 +752,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: ['StudyBucketPath_XYZ', `${studyBucket}/${studyPrefix}`],
},
Expand Down Expand Up @@ -906,6 +905,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: ['AnotherStudyBucketPath', `${studyBucket}/${studyPrefix}`],
},
Expand Down Expand Up @@ -947,6 +947,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: ['AnotherStudyBucketPath'],
},
Expand Down Expand Up @@ -1058,6 +1059,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`${studyBucket}/${studyPrefix}`],
},
Expand Down Expand Up @@ -1170,6 +1172,108 @@ describe('EnvironmentMountService', () => {
expect(IamUpdateParams.policyDoc).toMatchObject(expectedPolicy);
});

it('should ensure duplicate resources are not created for non-admins after permission addition from a bad state', async () => {
// BUILD
const updateRequest = {
usersToAdd: [{ uid: 'User1-UID', permissionLevel: 'readwrite' }],
};
const studyId = 'StudyA';
const envsForUser = [{ studyIds: ['StudyA'] }];
environmentScService.getActiveEnvsForUser = jest.fn().mockResolvedValue(envsForUser);
iamService.putRolePolicy = jest.fn();
const studyBucket = 'arn:aws:s3:::xxxxxxxx-namespace-studydata';
const studyPrefix = 'studies/Organization/SampleStudy/*';
const inputPolicy = {
Version: '2012-10-17',
Statement: [
{
Sid: 'studyKMSAccess',
Action: ['Permission1', 'Permission2'],
Effect: 'Allow',
Resource: 'arn:aws:kms:region:xxxxxxxx:key/someRandomString',
},
{
Sid: 'studyListS3AccessN',
Effect: 'Allow',
Action: 's3:ListBucket',
Resource: studyBucket,
Condition: {
StringLike: {
's3:prefix': ['AnotherStudyPrefixForThisBucket', studyPrefix],
},
},
},
{
Sid: 'S3StudyReadAccess',
Effect: 'Allow',
Action: ['s3:GetObject'],
Resource: ['AnotherStudyBucketPath', `${studyBucket}/${studyPrefix}`],
},
],
};
const IamUpdateParams = {
iamClient: 'sampleIamClient',
studyPathArn: `${studyBucket}/${studyPrefix}`,
policyDoc: inputPolicy,
roleName: 'sampleRoleName',
studyDataPolicyName: 'sampleStudyDataPolicy',
};
const expectedPolicy = {
Version: '2012-10-17',
Statement: [
{
Sid: 'studyKMSAccess',
Action: ['Permission1', 'Permission2'],
Effect: 'Allow',
Resource: 'arn:aws:kms:region:xxxxxxxx:key/someRandomString',
},
{
Sid: 'studyListS3AccessN',
Effect: 'Allow',
Action: 's3:ListBucket',
Resource: studyBucket,
Condition: {
StringLike: {
's3:prefix': ['AnotherStudyPrefixForThisBucket', studyPrefix],
},
},
},
{
Sid: 'S3StudyReadAccess',
Effect: 'Allow',
Action: ['s3:GetObject'],
Resource: ['AnotherStudyBucketPath'],
},
{
Sid: 'S3StudyReadWriteAccess',
Effect: 'Allow',
Action: [
's3:GetObject',
's3:AbortMultipartUpload',
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`${studyBucket}/${studyPrefix}`],
},
],
};
service._getIamUpdateParams = jest.fn().mockResolvedValue(IamUpdateParams);

// OPERATE
await service.applyWorkspacePermissions(studyId, updateRequest);

// CHECK
expect(iamService.putRolePolicy).toHaveBeenCalledWith(
IamUpdateParams.roleName,
IamUpdateParams.studyDataPolicyName,
JSON.stringify(IamUpdateParams.policyDoc),
IamUpdateParams.iamClient,
);
expect(IamUpdateParams.policyDoc).toMatchObject(expectedPolicy);
});

it('should ensure permission updates for admins are as expected', async () => {
// BUILD
const updateRequest = {
Expand Down Expand Up @@ -1214,6 +1318,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`${studyBucket}/${studyPrefix}`],
},
Expand Down Expand Up @@ -1357,6 +1462,7 @@ describe('EnvironmentMountService', () => {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`${studyBucket}/${studyPrefix}`],
},
Expand Down
32 changes: 21 additions & 11 deletions .../addon-base-raas/packages/base-raas-services/lib/environment/environment-mount-service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,13 @@ class EnvironmentMountService extends Service {
Sid: putSid,
Effect: 'Allow',
Principal: { AWS: [] },
Action: ['s3:AbortMultipartUpload', 's3:ListMultipartUploadParts', 's3:PutObject', 's3:PutObjectAcl'],
Action: [
's3:AbortMultipartUpload',
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
],
Resource: [`arn:aws:s3:::${s3BucketName}/${prefix}*`],
};

Expand Down Expand Up @@ -281,7 +287,7 @@ class EnvironmentMountService extends Service {
}
};

await runAndCaptureErrors(allowedUsers, users => this.addPermissions(users, studyId, updateRequest));
await runAndCaptureErrors(allowedUsers, users => this.addPermissions(users, studyId));
await runAndCaptureErrors(disAllowedUsers, users => this.removePermissions(users, studyId, updateRequest));
await runAndCaptureErrors(permissionChangeUsers, users => this.updatePermissions(users, studyId, updateRequest));

Expand All @@ -304,16 +310,11 @@ class EnvironmentMountService extends Service {
* @param {Object[]} allowedUsers - Users that newly/continue-to have access to given studyId
* @param {String} studyId
*/
async addPermissions(allowedUsers, studyId, updateRequest) {
async addPermissions(allowedUsers, studyId) {
const [iamService, environmentScService] = await this.service(['iamService', 'environmentScService']);
const errors = [];
await Promise.all(
_.map(allowedUsers, async user => {
const existingStudyAdmins = _.filter(updateRequest.usersToAdd, u => u.permissionLevel === 'admin');
const removedStudyAdmins = _.filter(updateRequest.usersToRemove, u => u.permissionLevel === 'admin');
const isStudyAdmin = !_.isEmpty(
_.filter([...existingStudyAdmins, ...removedStudyAdmins], u => u.uid === user.uid),
);
const userOwnedEnvs = await environmentScService.getActiveEnvsForUser(user.uid);
const envsWithStudy = _.filter(userOwnedEnvs, env => _.includes(env.studyIds, studyId));
await Promise.all(
Expand All @@ -328,11 +329,15 @@ class EnvironmentMountService extends Service {
} = await this._getIamUpdateParams(env, studyId);

const statementSidToUse = this._getStatementSidToUse(user.permissionLevel);
let ensureRemovedPermission;
if (statementSidToUse === readOnlyStatementId) {
ensureRemovedPermission = readWriteStatementId;
} else if (statementSidToUse === readWriteStatementId) {
ensureRemovedPermission = readOnlyStatementId;
}
policyDoc.Statement = this._getStatementsAfterAddition(policyDoc, studyPathArn, statementSidToUse);
policyDoc.Statement = this._ensureListAccess(policyDoc, studyPathArn);
if (isStudyAdmin && user.permissionLevel === readWritePermissionLevel) {
policyDoc.Statement = this._getStatementsAfterRemoval(policyDoc, studyPathArn, readOnlyStatementId);
}
policyDoc.Statement = this._getStatementsAfterRemoval(policyDoc, studyPathArn, ensureRemovedPermission);
await iamService.putRolePolicy(roleName, studyDataPolicyName, JSON.stringify(policyDoc), iamClient);
} catch (error) {
const envId = env.id;
Expand Down Expand Up @@ -541,6 +546,7 @@ class EnvironmentMountService extends Service {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
]
: ['s3:GetObject'],
Resource: [studyPathArn],
Expand Down Expand Up @@ -662,6 +668,9 @@ class EnvironmentMountService extends Service {
}

async _getWorkspacePolicy(iamClient, env) {
if (!env.outputs) {
throw new Error('Environment outputs are not ready yet. Please make sure environment is in Completed status');
}
const iamService = await this.service('iamService');
const workspaceRoleArn = _.find(env.outputs, { OutputKey: 'WorkspaceInstanceRoleArn' }).OutputValue;
const roleName = workspaceRoleArn.split('role/')[1];
Expand Down Expand Up @@ -837,6 +846,7 @@ class EnvironmentMountService extends Service {
's3:ListMultipartUploadParts',
's3:PutObject',
's3:PutObjectAcl',
's3:DeleteObject',
];
statements.push({
Sid: readWriteStatementId,
Expand Down

0 comments on commit c819e28

Please sign in to comment.