From a84951ace28df352bae0fb50c76725530e6a9b4c Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Thu, 16 Oct 2025 21:35:33 +0530 Subject: [PATCH 1/7] Replaced handle with memberId for challenge createdBy comparison --- src/services/ResourceService.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index fae387f..e611a35 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -307,7 +307,7 @@ async function init (currentUser, challengeId, resource, isCreated) { `User ${handle} is not allowed to register.` ) } - if (!_.get(challenge, 'task.isTask', false) && (_.toLower(challenge.createdBy) === _.toLower(handle) || + if (!_.get(challenge, 'task.isTask', false) && (_.toLower(challenge.createdBy) === _.toLower(memberId) || _.some(userResources, r => r.roleId === config.REVIEWER_RESOURCE_ROLE_ID || r.roleId === config.ITERATIVE_REVIEWER_RESOURCE_ROLE_ID))) { throw new errors.BadRequestError( `User ${handle} is not allowed to register.` From c699a78b51441e001f5c0433f224537a7cae08a1 Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Wed, 29 Oct 2025 19:44:05 +0530 Subject: [PATCH 2/7] prevent submitter and restricted roles from being combined --- src/services/ResourceService.js | 63 +++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index e611a35..bd30c8c 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -17,6 +17,7 @@ const prisma = require('../common/prisma').getClient() const payloadFields = ['id', 'challengeId', 'memberId', 'memberHandle', 'roleId', 'phaseChangeNotifications', 'created', 'createdBy', 'updated', 'updatedBy'] let copilotResourceRoleIdsCache +let restrictedRoleIdsCache async function getCopilotResourceRoleIds () { if (copilotResourceRoleIdsCache) { @@ -34,6 +35,41 @@ async function getCopilotResourceRoleIds () { return copilotResourceRoleIdsCache } +/** + * Get resource role IDs that are restricted from being combined with submitter role. + * These include: Manager, Copilot, Reviewer, Iterative Reviewer, Screener, + * Checkpoint Screener, Checkpoint Reviewer, and Approver. + * @returns {Promise>} Array of restricted role IDs + */ +async function getRestrictedRoleIds () { + if (restrictedRoleIdsCache) { + return restrictedRoleIdsCache + } + const restrictedRoleNames = [ + 'manager', + 'copilot', + 'reviewer', + 'iterative reviewer', + 'screener', + 'checkpoint screener', + 'checkpoint reviewer', + 'approver' + ] + const roles = await prisma.resourceRole.findMany({ + where: { + nameLower: { + in: restrictedRoleNames + } + }, + select: { + id: true, + nameLower: true + } + }) + restrictedRoleIdsCache = roles.map(role => role.id) + return restrictedRoleIdsCache +} + /** * Check whether the user can access resources * @param {Array} resources resources of current user for specified challenge id @@ -307,12 +343,22 @@ async function init (currentUser, challengeId, resource, isCreated) { `User ${handle} is not allowed to register.` ) } - if (!_.get(challenge, 'task.isTask', false) && (_.toLower(challenge.createdBy) === _.toLower(memberId) || - _.some(userResources, r => r.roleId === config.REVIEWER_RESOURCE_ROLE_ID || r.roleId === config.ITERATIVE_REVIEWER_RESOURCE_ROLE_ID))) { + // Prevent challenge creator from registering as submitter (for non-tasks) + if (!_.get(challenge, 'task.isTask', false) && _.toLower(challenge.createdBy) === _.toLower(memberId)) { throw new errors.BadRequestError( `User ${handle} is not allowed to register.` ) } + // Check if user already has a restricted role (Manager, Copilot, Reviewer, etc.) + const restrictedRoleIds = await getRestrictedRoleIds() + const existingRestrictedRole = _.find(userResources, r => restrictedRoleIds.includes(r.roleId)) + if (existingRestrictedRole) { + // Get the role name for better error message + const existingRole = await getResourceRole(existingRestrictedRole.roleId, false) + throw new errors.BadRequestError( + `User ${handle} is already assigned a ${existingRole.name} role and cannot be registered as a submitter.` + ) + } } // Prevent from creating more than 1 submitter resources on tasks @@ -344,6 +390,19 @@ async function init (currentUser, challengeId, resource, isCreated) { // ensure resource role existed const resourceRole = await getResourceRole(resource.roleId, isCreated) + // Check if user is trying to assign a restricted role and already has submitter role + if (isCreated) { + const restrictedRoleIds = await getRestrictedRoleIds() + if (restrictedRoleIds.includes(resource.roleId)) { + const existingSubmitterRole = _.find(userResources, r => r.roleId === config.SUBMITTER_RESOURCE_ROLE_ID) + if (existingSubmitterRole) { + throw new errors.BadRequestError( + `User ${handle} is already registered as a submitter and cannot be assigned a ${resourceRole.name} role.` + ) + } + } + } + // Verify the member has agreed to the challenge terms if (isCreated) { await helper.checkAgreedTerms(memberId, _.filter(_.get(challenge, 'terms', []), t => t.roleId === resourceRole.id)) From 1a332b86049b6a15857f53e5f1a4c486393a07b9 Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Wed, 29 Oct 2025 21:46:33 +0530 Subject: [PATCH 3/7] Removed DB call to get existingRole for error message --- src/services/ResourceService.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index bd30c8c..7cb85ac 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -353,10 +353,8 @@ async function init (currentUser, challengeId, resource, isCreated) { const restrictedRoleIds = await getRestrictedRoleIds() const existingRestrictedRole = _.find(userResources, r => restrictedRoleIds.includes(r.roleId)) if (existingRestrictedRole) { - // Get the role name for better error message - const existingRole = await getResourceRole(existingRestrictedRole.roleId, false) throw new errors.BadRequestError( - `User ${handle} is already assigned a ${existingRole.name} role and cannot be registered as a submitter.` + `User ${handle} is already assigned a restricted role (Manager, Copilot, Reviewer, Iterative Reviewer, Screener, Checkpoint Screener, Checkpoint Reviewer, or Approver) and cannot be registered as a submitter.` ) } } From fb67833c0e8d0dc10f650b5767c09ed5693cb72c Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Wed, 29 Oct 2025 21:52:51 +0530 Subject: [PATCH 4/7] Added constant for restricted roles --- src/services/ResourceService.js | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index 7cb85ac..217f39e 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -16,6 +16,18 @@ const prisma = require('../common/prisma').getClient() const payloadFields = ['id', 'challengeId', 'memberId', 'memberHandle', 'roleId', 'phaseChangeNotifications', 'created', 'createdBy', 'updated', 'updatedBy'] +// Restricted roles that cannot be combined with submitter role +const RESTRICTED_ROLE_NAMES = [ + 'manager', + 'copilot', + 'reviewer', + 'iterative reviewer', + 'screener', + 'checkpoint screener', + 'checkpoint reviewer', + 'approver' +] + let copilotResourceRoleIdsCache let restrictedRoleIdsCache @@ -45,20 +57,10 @@ async function getRestrictedRoleIds () { if (restrictedRoleIdsCache) { return restrictedRoleIdsCache } - const restrictedRoleNames = [ - 'manager', - 'copilot', - 'reviewer', - 'iterative reviewer', - 'screener', - 'checkpoint screener', - 'checkpoint reviewer', - 'approver' - ] const roles = await prisma.resourceRole.findMany({ where: { nameLower: { - in: restrictedRoleNames + in: RESTRICTED_ROLE_NAMES } }, select: { @@ -353,8 +355,9 @@ async function init (currentUser, challengeId, resource, isCreated) { const restrictedRoleIds = await getRestrictedRoleIds() const existingRestrictedRole = _.find(userResources, r => restrictedRoleIds.includes(r.roleId)) if (existingRestrictedRole) { + const roleNamesList = RESTRICTED_ROLE_NAMES.slice(0, -1).join(', ') + ', or ' + RESTRICTED_ROLE_NAMES.slice(-1) throw new errors.BadRequestError( - `User ${handle} is already assigned a restricted role (Manager, Copilot, Reviewer, Iterative Reviewer, Screener, Checkpoint Screener, Checkpoint Reviewer, or Approver) and cannot be registered as a submitter.` + `User ${handle} is already assigned a restricted role (${roleNamesList}) and cannot be registered as a submitter.` ) } } From 8fc15a506b8df3b55e958fb58d7c71e8c50d9b07 Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Wed, 29 Oct 2025 21:58:24 +0530 Subject: [PATCH 5/7] Cached restrictedRoleIds locally --- src/services/ResourceService.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index 217f39e..9346b94 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -329,6 +329,12 @@ async function init (currentUser, challengeId, resource, isCreated) { const { memberId, email } = memberInfoFromDb handle = memberInfoFromDb.handle const userResources = allResources.filter((r) => _.toLower(r.memberHandle) === _.toLower(handle)) + + let restrictedRoleIds + if (isCreated) { + restrictedRoleIds = await getRestrictedRoleIds() + } + // Retrieve the constraint - Allowed Registrants if (isCreated && resource.roleId === config.SUBMITTER_RESOURCE_ROLE_ID) { const allowedRegistrants = _.get(challenge, 'constraints.allowedRegistrants') @@ -352,7 +358,6 @@ async function init (currentUser, challengeId, resource, isCreated) { ) } // Check if user already has a restricted role (Manager, Copilot, Reviewer, etc.) - const restrictedRoleIds = await getRestrictedRoleIds() const existingRestrictedRole = _.find(userResources, r => restrictedRoleIds.includes(r.roleId)) if (existingRestrictedRole) { const roleNamesList = RESTRICTED_ROLE_NAMES.slice(0, -1).join(', ') + ', or ' + RESTRICTED_ROLE_NAMES.slice(-1) @@ -393,7 +398,6 @@ async function init (currentUser, challengeId, resource, isCreated) { // Check if user is trying to assign a restricted role and already has submitter role if (isCreated) { - const restrictedRoleIds = await getRestrictedRoleIds() if (restrictedRoleIds.includes(resource.roleId)) { const existingSubmitterRole = _.find(userResources, r => r.roleId === config.SUBMITTER_RESOURCE_ROLE_ID) if (existingSubmitterRole) { From 9ea5fde8f3d85d9ee635cf7b0289df1b4a36cd9a Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Thu, 30 Oct 2025 17:42:21 +0530 Subject: [PATCH 6/7] Fixed lint error --- src/services/ResourceService.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index 9631eb1..f641f4e 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -329,12 +329,12 @@ async function init (currentUser, challengeId, resource, isCreated) { const { memberId, email } = memberInfoFromDb handle = memberInfoFromDb.handle const userResources = allResources.filter((r) => _.toLower(r.memberHandle) === _.toLower(handle)) - + let restrictedRoleIds if (isCreated) { restrictedRoleIds = await getRestrictedRoleIds() } - + // Retrieve the constraint - Allowed Registrants if (isCreated && resource.roleId === config.SUBMITTER_RESOURCE_ROLE_ID) { const allowedRegistrants = _.get(challenge, 'constraints.allowedRegistrants') From 41cfbfee0e85a211750cb174ef9e1bdd2c72a86b Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 31 Oct 2025 06:51:31 +1100 Subject: [PATCH 7/7] Consider a copilot to have "full access" to a challenge (PM-2654) --- src/services/ResourceService.js | 6 ++++++ test/unit/createResource.test.js | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/services/ResourceService.js b/src/services/ResourceService.js index f641f4e..e226deb 100644 --- a/src/services/ResourceService.js +++ b/src/services/ResourceService.js @@ -77,6 +77,12 @@ async function getRestrictedRoleIds () { * @param {Array} resources resources of current user for specified challenge id */ async function checkAccess (currentUserResources) { + const copilotRoleIds = await getCopilotResourceRoleIds() + const hasCopilotRole = _.some(currentUserResources, r => copilotRoleIds.includes(r.roleId)) + if (hasCopilotRole) { + return + } + const list = await prisma.resourceRole.findMany({}) const fullAccessRoles = [] _.each(list, e => { diff --git a/test/unit/createResource.test.js b/test/unit/createResource.test.js index 4b41ef5..68416e2 100644 --- a/test/unit/createResource.test.js +++ b/test/unit/createResource.test.js @@ -9,6 +9,7 @@ const { v4: uuid } = require('uuid') const service = require('../../src/services/ResourceService') const ResourceRolePhaseDependencyService = require('../../src/services/ResourceRolePhaseDependencyService') const prisma = require('../../src/common/prisma').getClient() +const helper = require('../../src/common/helper') const ResourceRoleService = require('../../src/services/ResourceRoleService') const { requestBody, user } = require('../common/testData') const { assertValidationError, assertError, assertResource, getRoleIds, clearDependencies } = require('../common/testHelper') @@ -264,6 +265,39 @@ module.exports = describe('Create resource', () => { await assertResource(ret.id, ret) }) + it('copilot can manage resources without full access flags', async () => { + const originalRole = await helper.getById('ResourceRole', copilotRoleId) + await ResourceRoleService.updateResourceRole(user.admin, copilotRoleId, { + name: originalRole.name, + fullReadAccess: false, + fullWriteAccess: false, + isActive: originalRole.isActive, + selfObtainable: originalRole.selfObtainable + }) + + const entity = resources.createBody('diazz', reviewerRoleId, challengeId2) + let createdResource + try { + createdResource = await service.createResource(user.phead, entity) + should.equal(createdResource.roleId, entity.roleId) + should.equal(createdResource.memberHandle.toLowerCase(), entity.memberHandle.toLowerCase()) + await assertResource(createdResource.id, createdResource) + } finally { + if (createdResource && createdResource.id) { + await prisma.resource.deleteMany({ + where: { id: createdResource.id } + }) + } + await ResourceRoleService.updateResourceRole(user.admin, copilotRoleId, { + name: originalRole.name, + fullReadAccess: originalRole.fullReadAccess, + fullWriteAccess: originalRole.fullWriteAccess, + isActive: originalRole.isActive, + selfObtainable: originalRole.selfObtainable + }) + } + }) + it('create resource for user ghostar 1', async () => { const entity = resources.createBody('ghostar', reviewerRoleId, challengeId2) const ret = await service.createResource(user.m2m, entity)