From 0302284311a8aeffbf24ab7ee970c68135f7f325 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 12:54:11 +0200 Subject: [PATCH 1/6] PM-809 - check for permissions when listing artifacts --- src/common/helper.js | 70 +++++++++++++++++++++++++++ src/constants/index.js | 18 +++++++ src/controllers/ArtifactController.js | 2 +- src/services/ArtifactService.js | 24 +++++++-- src/services/HelperService.js | 4 ++ 5 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 src/constants/index.js diff --git a/src/common/helper.js b/src/common/helper.js index 6391d79d..030598f5 100755 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -14,6 +14,7 @@ const errors = require('common-errors') const { validate: uuidValidate } = require('uuid') const NodeCache = require('node-cache') const { axiosInstance } = require('./axiosInstance') +const { UserRoles, ProjectRoles } = require('../constants') AWS.config.region = config.get('aws.AWS_REGION') const s3 = new AWS.S3() @@ -312,6 +313,44 @@ function setPaginationHeaders (req, res, data) { res.json(data.rows) } +/** + * Get challenge resources + * @param {String} challengeId the challenge id + * @param {String} userId specific userId for which to check roles + */ +const getChallengeResources = async (challengeId, userId) => { + let resourcesResponse + + // Get map of role id to role name + const resourceRolesMap = await getRoleIdToRoleNameMap() + + // Check if role id to role name mapping is available. If not user's role cannot be determined. + if (resourceRolesMap == null || _.size(resourceRolesMap) === 0) { + throw new errors.HttpStatusError(503, `Could not determine the user's role in the challenge with id ${challengeId}`) + } + + const resourcesUrl = `${config.RESOURCEAPI_V5_BASE_URL}/resources?challengeId=${challengeId}${userId ? `&memberId=${userId}` : ''}` + try { + resourcesResponse = _.get(await axiosInstance.get(resourcesUrl), 'data', []) + } catch (ex) { + logger.error(`Error while accessing ${resourcesUrl}`) + throw new errors.HttpStatusError(503, `Could not determine the user's role in the challenge with id ${challengeId}`) + } + + const resources = {} + _.each((resourcesResponse || []), (resource) => { + if (!resources[resource.memberId]) { + resources[resource.memberId] = { + memberId: resource.memberId, + memberHandle: resource.memberHandle, + roles: [] + } + } + resources[resource.memberId].roles.push(resourceRolesMap[resource.roleId]) + }) + return resources +} + /** * Function to get challenge by id * @param {String} challengeId Challenge id @@ -506,6 +545,36 @@ async function checkCreateAccess (authUser, memberId, challengeDetails) { } } +/** + * Check the user's access to a challenge + * @param {Object} authUser the user + * @param {Array} resources the challenge resources + */ +const getChallengeAccessLevel = async (authUser, challengeId) => { + const resources = await getChallengeResources(challengeId, authUser.userId) + + // Case Insensitive Role checks + const hasFullAccess = authUser.roles.findIndex(item => UserRoles.Admin.toLowerCase() === item.toLowerCase()) > -1 || _.intersectionWith(_.get(resources[authUser.userId], 'roles', []), [ + ProjectRoles.Manager, + ProjectRoles.Copilot, + ProjectRoles.Observer, + ProjectRoles.Client_Manager + ], (act, exp) => act.toLowerCase() === exp.toLowerCase()).length > 0 + + const isReviewer = !hasFullAccess && _.intersectionWith(_.get(resources[authUser.userId], 'roles', []), [ + ProjectRoles.Reviewer, + ProjectRoles.Iterative_Reviewer + ], (act, exp) => act.toLowerCase() === exp.toLowerCase()).length > 0 + + const isSubmitter = !hasFullAccess && !isReviewer && _.intersectionWith(_.get(resources[authUser.userId], 'roles', []), [ + ProjectRoles.Submitter + ], (act, exp) => act.toLowerCase() === exp.toLowerCase()).length > 0 + + const hasNoAccess = !hasFullAccess && !isReviewer && !isSubmitter + + return { hasFullAccess, isReviewer, isSubmitter, hasNoAccess } +} + /** * Function to check user access to get a submission * @param authUser Authenticated user @@ -922,6 +991,7 @@ module.exports = { setPaginationHeaders, getSubmissionPhaseId, checkCreateAccess, + getChallengeAccessLevel, checkGetAccess, checkReviewGetAccess, createS3ReadStream, diff --git a/src/constants/index.js b/src/constants/index.js new file mode 100644 index 00000000..44c35d80 --- /dev/null +++ b/src/constants/index.js @@ -0,0 +1,18 @@ +const UserRoles = { + Admin: 'Administrator' +} + +const ProjectRoles = { + Manager: 'Manager', + Copilot: 'Copilot', + Observer: 'Observer', + Reviewer: 'Reviewer', + Submitter: 'Submitter', + Client_Manager: 'Client Manager', + Iterative_Reviewer: 'Iterative Reviewer' +} + +module.exports = { + UserRoles, + ProjectRoles +} diff --git a/src/controllers/ArtifactController.js b/src/controllers/ArtifactController.js index e16680a3..3073332f 100644 --- a/src/controllers/ArtifactController.js +++ b/src/controllers/ArtifactController.js @@ -21,7 +21,7 @@ async function downloadArtifact (req, res) { * @param res the http response */ async function listArtifacts (req, res) { - res.json(await ArtifactService.listArtifacts(req.params.submissionId)) + res.json(await ArtifactService.listArtifacts(req.authUser, req.params.submissionId)) } /** diff --git a/src/services/ArtifactService.js b/src/services/ArtifactService.js index bba4961d..b306610f 100644 --- a/src/services/ArtifactService.js +++ b/src/services/ArtifactService.js @@ -12,6 +12,7 @@ const _ = require('lodash') const s3 = new AWS.S3() const logger = require('../common/logger') const HelperService = require('./HelperService') +const commonHelper = require('../common/helper') /* * Function to upload file to S3 @@ -68,14 +69,31 @@ downloadArtifact.schema = joi.object({ * @param {String} submissionId Submission ID * @return {Promise} List of files present in S3 bucket under submissionId directory */ -async function listArtifacts (submissionId) { +async function listArtifacts (authUser, submissionId) { // Check the validness of Submission ID - await HelperService._checkRef({ submissionId }) + const submission = await HelperService._checkRef({ submissionId }) + + let challenge + try { + challenge = await commonHelper.getChallenge(submission.challengeId) + } catch (e) { + throw new errors.NotFoundError(`Could not load challenge: ${submission.challengeId}.\n Details: ${_.get(e, 'message')}`) + } + + const { hasFullAccess, isSubmitter, hasNoAccess } = await commonHelper.getChallengeAccessLevel(authUser, submission.challengeId) + + challenge.isMM = true + if (hasNoAccess || (isSubmitter && challenge.isMM && submission.memberId.toString() !== authUser.userId.toString())) { + throw new errors.HttpStatusError(403, 'You are not allowed to access this submission artifact.') + } + const artifacts = await s3.listObjects({ Bucket: config.aws.ARTIFACT_BUCKET, Prefix: submissionId }).promise() - return { artifacts: _.map(artifacts.Contents, (at) => path.parse(at.Key).name) } + const artifactsContents = _.map(artifacts.Contents, (at) => path.parse(at.Key).name) + return hasFullAccess ? artifactsContents : _.filter(artifactsContents, artifactName => !artifactName.includes('internal')) } listArtifacts.schema = joi.object({ + authUser: joi.object().required(), submissionId: joi.string().uuid().required() }).required() diff --git a/src/services/HelperService.js b/src/services/HelperService.js index 7e949964..edad7beb 100644 --- a/src/services/HelperService.js +++ b/src/services/HelperService.js @@ -19,6 +19,8 @@ async function _checkRef (entity) { if (!existReviewType) { throw new errors.HttpStatusError(400, `Review type with ID = ${entity.typeId} does not exist`) } + + return existReviewType } if (entity.submissionId) { @@ -27,6 +29,8 @@ async function _checkRef (entity) { if (!existSubmission) { throw new errors.HttpStatusError(400, `Submission with ID = ${entity.submissionId} does not exist`) } + + return existSubmission } } From fa47fa9b34620daed511466f2c27e53546c0d99c Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 13:43:51 +0200 Subject: [PATCH 2/6] PM-809 - restrict access for artifcat download --- src/controllers/ArtifactController.js | 2 +- src/services/ArtifactService.js | 24 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/controllers/ArtifactController.js b/src/controllers/ArtifactController.js index 3073332f..56976f8b 100644 --- a/src/controllers/ArtifactController.js +++ b/src/controllers/ArtifactController.js @@ -10,7 +10,7 @@ const ArtifactService = require('../services/ArtifactService') * @param res the http response */ async function downloadArtifact (req, res) { - const result = await ArtifactService.downloadArtifact(req.params.submissionId, req.params.file) + const result = await ArtifactService.downloadArtifact(req.authUser, req.params.submissionId, req.params.file) res.attachment(result.fileName) res.send(result.file) } diff --git a/src/services/ArtifactService.js b/src/services/ArtifactService.js index b306610f..3f5dde0f 100644 --- a/src/services/ArtifactService.js +++ b/src/services/ArtifactService.js @@ -40,9 +40,27 @@ async function _uploadToS3 (file, name) { * @param {String} fileName File name which need to be downloaded from S3 * @return {Promise} File downloaded from S3 */ -async function downloadArtifact (submissionId, fileName) { +async function downloadArtifact (authUser, submissionId, fileName) { // Check the validness of Submission ID - await HelperService._checkRef({ submissionId }) + const submission = await HelperService._checkRef({ submissionId }) + + let challenge + try { + challenge = await commonHelper.getChallenge(submission.challengeId) + } catch (e) { + throw new errors.NotFoundError(`Could not load challenge: ${submission.challengeId}.\n Details: ${_.get(e, 'message')}`) + } + + const { hasFullAccess, isSubmitter, hasNoAccess } = await commonHelper.getChallengeAccessLevel(authUser, submission.challengeId) + + if (hasNoAccess || (isSubmitter && challenge.isMM && submission.memberId.toString() !== authUser.userId.toString())) { + throw new errors.HttpStatusError(403, 'You are not allowed to download this submission artifact.') + } + + if (fileName.includes('internal') && !hasFullAccess) { + throw new errors.HttpStatusError(403, 'Could not access artifact.') + } + const artifacts = await s3.listObjects({ Bucket: config.aws.ARTIFACT_BUCKET, Prefix: `${submissionId}/${fileName}` }).promise() if (artifacts.Contents.length === 0) { throw new errors.HttpStatusError(400, `Artifact ${fileName} doesn't exist for ${submissionId}`) @@ -60,6 +78,7 @@ async function downloadArtifact (submissionId, fileName) { } downloadArtifact.schema = joi.object({ + authUser: joi.object().required(), submissionId: joi.string().uuid().required(), fileName: joi.string().trim().required() }).required() @@ -82,7 +101,6 @@ async function listArtifacts (authUser, submissionId) { const { hasFullAccess, isSubmitter, hasNoAccess } = await commonHelper.getChallengeAccessLevel(authUser, submission.challengeId) - challenge.isMM = true if (hasNoAccess || (isSubmitter && challenge.isMM && submission.memberId.toString() !== authUser.userId.toString())) { throw new errors.HttpStatusError(403, 'You are not allowed to access this submission artifact.') } From a688e795182694bea871191365cbb894ccc9cf45 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 14:13:41 +0200 Subject: [PATCH 3/6] PM-809 - fix atifact download --- src/services/ArtifactService.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/services/ArtifactService.js b/src/services/ArtifactService.js index 3f5dde0f..8ca7012b 100644 --- a/src/services/ArtifactService.js +++ b/src/services/ArtifactService.js @@ -61,13 +61,15 @@ async function downloadArtifact (authUser, submissionId, fileName) { throw new errors.HttpStatusError(403, 'Could not access artifact.') } - const artifacts = await s3.listObjects({ Bucket: config.aws.ARTIFACT_BUCKET, Prefix: `${submissionId}/${fileName}` }).promise() + const prefix = submissionId + '/' + fileName + const artifacts = await s3.listObjects({ Bucket: config.aws.ARTIFACT_BUCKET, Prefix: prefix }).promise() + if (artifacts.Contents.length === 0) { throw new errors.HttpStatusError(400, `Artifact ${fileName} doesn't exist for ${submissionId}`) } - const key = submissionId + '/' + fileName + '.zip' - if (!_.includes(_.map(artifacts.Contents, 'Key'), key)) { + const key = _.get(_.find(artifacts.Contents, { Key: `${prefix}.zip` }) || (artifacts.Contents.length === 1 ? artifacts.Contents[0] : {}), 'Key', null) + if (!key) { throw new errors.HttpStatusError(400, `Artifact ${fileName} doesn't exist for ${submissionId}`) } @@ -130,7 +132,7 @@ async function createArtifact (files, submissionId, entity) { logger.info('Creating a new Artifact') if (files && files.artifact) { const uFileType = (await FileType.fromBuffer(files.artifact.data)).ext // File type of uploaded file - fileName = `${submissionId}/${files.artifact.name}.${uFileType}` + fileName = `${submissionId}/${files.artifact.name.split('.').slice(0, -1)}.${uFileType}` // Upload the artifact to S3 await _uploadToS3(files.artifact, fileName) @@ -163,11 +165,6 @@ async function deleteArtifact (submissionId, fileName) { logger.info(`deleteArtifact: deleted artifact ${fileName} of Submission ID: ${submissionId}`) } -downloadArtifact.schema = joi.object({ - submissionId: joi.string().uuid().required(), - fileName: joi.string().trim().required() -}).required() - module.exports = { downloadArtifact, listArtifacts, From 08829cd7baacc3d76cab86d3b1e05af94afd03d4 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 14:15:36 +0200 Subject: [PATCH 4/6] deploy --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0f2510af..b4f8d839 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,7 +69,7 @@ workflows: context: org-global filters: branches: - only: ["develop", "PLAT-3383"] + only: ["develop", "PM-809_artifact-endpoint-update"] - "build-prod": context: org-global filters: From 36f19c3df3a2aa07484742251fe3d296afade5a9 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 15:19:38 +0200 Subject: [PATCH 5/6] allow artifact access for m2m tokens --- src/common/helper.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common/helper.js b/src/common/helper.js index 030598f5..26e48fab 100755 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -551,6 +551,10 @@ async function checkCreateAccess (authUser, memberId, challengeDetails) { * @param {Array} resources the challenge resources */ const getChallengeAccessLevel = async (authUser, challengeId) => { + if (authUser.isMachine) { + return { hasFullAccess: true } + } + const resources = await getChallengeResources(challengeId, authUser.userId) // Case Insensitive Role checks From f2d631abf2a08a53951330e926e25d26e5560539 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Tue, 25 Feb 2025 15:58:40 +0200 Subject: [PATCH 6/6] Keep same response format as before --- src/services/ArtifactService.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/ArtifactService.js b/src/services/ArtifactService.js index 8ca7012b..dec97133 100644 --- a/src/services/ArtifactService.js +++ b/src/services/ArtifactService.js @@ -109,7 +109,7 @@ async function listArtifacts (authUser, submissionId) { const artifacts = await s3.listObjects({ Bucket: config.aws.ARTIFACT_BUCKET, Prefix: submissionId }).promise() const artifactsContents = _.map(artifacts.Contents, (at) => path.parse(at.Key).name) - return hasFullAccess ? artifactsContents : _.filter(artifactsContents, artifactName => !artifactName.includes('internal')) + return { artifacts: hasFullAccess ? artifactsContents : _.filter(artifactsContents, artifactName => !artifactName.includes('internal')) } } listArtifacts.schema = joi.object({