From 803722375c05f123ccc4621c4d2eb1085ddc11a4 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 25 Sep 2026 15:20:03 +1000 Subject: [PATCH 1/2] fix(PM-6415): grant talent managers non-internal project access --- .env.example | 3 + README.md | 6 ++ src/api/project/project.service.spec.ts | 53 +++++++++---- src/api/project/project.service.ts | 21 ++++- .../projectContext.interceptor.spec.ts | 29 +++++++ .../projectContext.interceptor.ts | 29 ++++++- .../utils/internal-project.utils.spec.ts | 79 +++++++++++++++++++ src/shared/utils/internal-project.utils.ts | 42 ++++++++++ src/shared/utils/project.utils.ts | 19 ++++- 9 files changed, 261 insertions(+), 20 deletions(-) create mode 100644 src/shared/utils/internal-project.utils.spec.ts create mode 100644 src/shared/utils/internal-project.utils.ts diff --git a/.env.example b/.env.example index e43eee7..4826994 100644 --- a/.env.example +++ b/.env.example @@ -94,3 +94,6 @@ CHALLENGES_DB_URL="" MEMBERS_DB_URL="" RESOURCES_DB_URL="" SKILLS_DB_URL="" + +# Comma-separated internal billing account IDs, excluded from Talent Manager access. +INTERNAL_BILLING_ACCOUNT_IDS="" diff --git a/README.md b/README.md index fd3c354..6bc60a7 100644 --- a/README.md +++ b/README.md @@ -521,3 +521,9 @@ Migration runbook (phased rollout, rollback, monitoring): `docs/MIGRATION_RUNBOO | `docs/MIGRATION_RUNBOOK.md` | Phased rollout, rollback procedures, monitoring alerts | | `docs/DEPENDENCIES.md` | Dependency security audit, outdated packages, overrides | | `docs/timeline-milestone-migration.md` | Guidance for future timeline/milestone migration | + +### Talent Manager project visibility + +`Talent Manager` and `Topcoder Talent Manager` users can list and open all non-internal projects without membership. Set `INTERNAL_BILLING_ACCOUNT_IDS` to the comma-separated positive billing account IDs used for internal projects before deploying this policy (for example, `123,456`); an empty value excludes no accounts, and invalid values reject requests rather than applying a partial list. Projects without a billing account remain visible. + +Internal projects are excluded from list results and totals and rejected on direct project and nested project routes, even when the Talent Manager has membership or a pending invite. Administrators and machine tokens keep their existing authorization policies. `memberOnly=true` narrows the visible projects to existing membership/pending-invite associations and still excludes internal projects; Work exposes this as **My Projects**. diff --git a/src/api/project/project.service.spec.ts b/src/api/project/project.service.spec.ts index 9a824dc..e0611a7 100644 --- a/src/api/project/project.service.spec.ts +++ b/src/api/project/project.service.spec.ts @@ -270,10 +270,26 @@ describe('ProjectService', () => { ); }); - it.each([ - ['project manager', UserRole.PROJECT_MANAGER], - ['talent manager', UserRole.TALENT_MANAGER], - ])( + it.each([UserRole.TALENT_MANAGER, UserRole.TOPCODER_TALENT_MANAGER])( + 'lists non-member projects for %s', + async (role) => { + permissionServiceMock.hasIntersection.mockImplementation( + (roles: string[], allowed: string[]) => + roles.some((value) => allowed.includes(value)), + ); + prismaMock.project.count.mockResolvedValue(0); + prismaMock.project.findMany.mockResolvedValue([]); + await service.listProjects( + { page: 1, perPage: 20 }, + { isMachine: false, userId: '999', roles: [role] }, + ); + expect(prismaMock.project.count).toHaveBeenCalledWith({ + where: { deletedAt: null }, + }); + }, + ); + + it.each([['project manager', UserRole.PROJECT_MANAGER]])( 'scopes %s project listings to project membership', async (_label: string, role: UserRole) => { permissionServiceMock.hasNamedPermission.mockImplementation( @@ -522,11 +538,12 @@ describe('ProjectService', () => { }); it.each([ - ['project manager', UserRole.PROJECT_MANAGER], - ['talent manager', UserRole.TALENT_MANAGER], + ['project manager', UserRole.PROJECT_MANAGER, false], + ['Talent Manager', UserRole.TALENT_MANAGER, true], + ['Topcoder Talent Manager', UserRole.TOPCODER_TALENT_MANAGER, true], ])( - 'rejects direct project access for %s callers who are not on the project', - async (_label: string, role: UserRole) => { + 'checks non-member direct access for %s', + async (_label: string, role: UserRole, canView: boolean) => { const now = new Date(); prismaMock.project.findFirst.mockResolvedValue({ @@ -570,15 +587,19 @@ describe('ProjectService', () => { permission === Permission.VIEW_PROJECT || permission === Permission.READ_PROJECT_ANY, ); - permissionServiceMock.hasIntersection.mockReturnValue(false); + permissionServiceMock.hasIntersection.mockImplementation( + (roles: string[], allowed: string[]) => + roles.some((value) => allowed.includes(value)), + ); - await expect( - service.getProject('1001', undefined, { - userId: '999', - roles: [role], - isMachine: false, - }), - ).rejects.toBeInstanceOf(ForbiddenException); + const result = service.getProject('1001', undefined, { + userId: '999', + roles: [role], + isMachine: false, + }); + if (canView) + await expect(result).resolves.toMatchObject({ name: 'Demo' }); + else await expect(result).rejects.toBeInstanceOf(ForbiddenException); }, ); diff --git a/src/api/project/project.service.ts b/src/api/project/project.service.ts index 4d6098a..cbcb714 100644 --- a/src/api/project/project.service.ts +++ b/src/api/project/project.service.ts @@ -1,3 +1,7 @@ +import { + internalBillingAccountIds, + isRestrictedTalentManager, +} from '../../shared/utils/internal-project.utils'; import { normalizeShowcaseProjectMetadata } from 'src/shared/utils/showcase-metadata.utils'; import { BadRequestException, @@ -135,7 +139,7 @@ export class ProjectService { * Returns a paginated project list for the caller. * * Builds query clauses from shared utilities, scopes non-admin callers to - * their memberships, enriches member/invite handles, and hydrates billing + * their memberships, grants Talent Managers non-internal projects, enriches handles, and hydrates billing * account names. * * @param criteria List filters, paging, sort, and field selection. @@ -256,6 +260,16 @@ export class ProjectService { ); } + if ( + isRestrictedTalentManager(user) && + project.billingAccountId !== null && + internalBillingAccountIds().includes(project.billingAccountId) + ) { + throw new ForbiddenException( + 'Talent Managers cannot access internal projects', + ); + } + const [projectWithMemberHandles] = await this.enrichProjectsWithMemberHandles([project]); const projectWithRelations = projectWithMemberHandles || project; @@ -1466,7 +1480,8 @@ export class ProjectService { /** * Returns whether the caller may bypass project membership visibility checks. * - * Human callers only retain global access for admin or legacy manager roles. + * Human callers retain global access for admins, legacy managers, and Talent Managers. + * Talent Manager internal-account exclusions are enforced separately. * Machine principals continue to rely on the named permission so scoped * service tokens can read any project when authorized. * @@ -1490,6 +1505,8 @@ export class ProjectService { ...ADMIN_ROLES, UserRole.MANAGER, UserRole.TOPCODER_MANAGER, + UserRole.TALENT_MANAGER, + UserRole.TOPCODER_TALENT_MANAGER, ]); } diff --git a/src/shared/interceptors/projectContext.interceptor.spec.ts b/src/shared/interceptors/projectContext.interceptor.spec.ts index 87ae2d7..dbd78e8 100644 --- a/src/shared/interceptors/projectContext.interceptor.spec.ts +++ b/src/shared/interceptors/projectContext.interceptor.spec.ts @@ -2,12 +2,14 @@ import { BadRequestException, CallHandler, ExecutionContext, + ForbiddenException, } from '@nestjs/common'; import { of } from 'rxjs'; import { ProjectContextInterceptor } from './projectContext.interceptor'; describe('ProjectContextInterceptor', () => { const prismaServiceMock = { + project: { findFirst: jest.fn() }, projectMember: { findMany: jest.fn(), }, @@ -31,6 +33,33 @@ describe('ProjectContextInterceptor', () => { }), }) as ExecutionContext; + it('rejects internal project routes even when membership was already cached', async () => { + const previous = process.env.INTERNAL_BILLING_ACCOUNT_IDS; + process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123'; + try { + prismaServiceMock.project.findFirst.mockResolvedValue({ id: 1001n }); + const request = { + params: { projectId: '1001' }, + user: { userId: '42', roles: ['Talent Manager'] }, + projectContext: { + projectId: '1001', + projectMembers: [{ userId: '42', role: 'manager' }], + }, + }; + await expect( + interceptor.intercept(createExecutionContext(request), next), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(prismaServiceMock.project.findFirst).toHaveBeenCalledWith({ + where: { id: 1001n, deletedAt: null, billingAccountId: { in: [123n] } }, + select: { id: true }, + }); + } finally { + if (previous === undefined) + delete process.env.INTERNAL_BILLING_ACCOUNT_IDS; + else process.env.INTERNAL_BILLING_ACCOUNT_IDS = previous; + } + }); + it('loads project members when projectId exists', async () => { const request: any = { params: { diff --git a/src/shared/interceptors/projectContext.interceptor.ts b/src/shared/interceptors/projectContext.interceptor.ts index 2699dde..1f12cad 100644 --- a/src/shared/interceptors/projectContext.interceptor.ts +++ b/src/shared/interceptors/projectContext.interceptor.ts @@ -7,17 +7,22 @@ import { CallHandler, ExecutionContext, + ForbiddenException, Injectable, NestInterceptor, } from '@nestjs/common'; import { Observable } from 'rxjs'; +import { + internalBillingAccountIds, + isRestrictedTalentManager, +} from '../utils/internal-project.utils'; import { AuthenticatedRequest } from '../interfaces/request.interface'; import { LoggerService } from '../modules/global/logger.service'; import { PrismaService } from '../modules/global/prisma.service'; import { parseNumericStringId } from '../utils/service.utils'; /** - * Interceptor that preloads and caches project membership context per request. + * Interceptor that enforces internal-project exclusions and caches membership context. */ @Injectable() export class ProjectContextInterceptor implements NestInterceptor { @@ -36,6 +41,7 @@ export class ProjectContextInterceptor implements NestInterceptor { * `projectId` route param is available. * * Behavior: + * - Rejects Talent Manager access to configured internal projects before cache hits. * - Initializes `request.projectContext` if absent. * - Short-circuits when no project id is present. * - Short-circuits on cache hits where project id already matches. @@ -43,6 +49,8 @@ export class ProjectContextInterceptor implements NestInterceptor { * - Queries active project members and maps `role` to plain strings. * - On query error, logs a warning and stores `projectMembers = []`. * + * @throws ForbiddenException for internal projects accessed by a Talent Manager. + * @throws Database/configuration errors during internal-project checks propagate. * @todo Member query + mapping logic is duplicated in multiple guards. * Introduce a shared `ProjectContextService` to centralize loading behavior. */ @@ -66,6 +74,25 @@ export class ProjectContextInterceptor implements NestInterceptor { const parsedProjectId = parseNumericStringId(projectId, 'Project id'); + if (isRestrictedTalentManager(request.user)) { + const internalIds = internalBillingAccountIds(); + if (internalIds.length) { + const internalProject = await this.prisma.project.findFirst({ + where: { + id: parsedProjectId, + deletedAt: null, + billingAccountId: { in: internalIds }, + }, + select: { id: true }, + }); + if (internalProject) { + throw new ForbiddenException( + 'Talent Managers cannot access internal projects', + ); + } + } + } + if ( request.projectContext.projectId === projectId && Array.isArray(request.projectContext.projectMembers) diff --git a/src/shared/utils/internal-project.utils.spec.ts b/src/shared/utils/internal-project.utils.spec.ts new file mode 100644 index 0000000..5ca85dc --- /dev/null +++ b/src/shared/utils/internal-project.utils.spec.ts @@ -0,0 +1,79 @@ +import { + internalBillingAccountIds, + isRestrictedTalentManager, +} from './internal-project.utils'; +import { buildProjectWhereClause } from './project.utils'; + +describe('Talent Manager internal project visibility', () => { + const originalIds = process.env.INTERNAL_BILLING_ACCOUNT_IDS; + afterEach(() => { + if (originalIds === undefined) + delete process.env.INTERNAL_BILLING_ACCOUNT_IDS; + else process.env.INTERNAL_BILLING_ACCOUNT_IDS = originalIds; + }); + + it.each(['Talent Manager', ' Topcoder Talent Manager '])( + 'restricts %s', + (role) => { + expect( + isRestrictedTalentManager({ + isMachine: false, + userId: '42', + roles: [role], + }), + ).toBe(true); + }, + ); + + it('retains administrator and machine policies', () => { + expect( + isRestrictedTalentManager({ + isMachine: false, + roles: ['Talent Manager', 'administrator'], + }), + ).toBe(false); + expect( + isRestrictedTalentManager({ roles: ['Talent Manager'], isMachine: true }), + ).toBe(false); + expect( + isRestrictedTalentManager({ + isMachine: false, + roles: ['Project Manager'], + }), + ).toBe(false); + }); + + it('parses large IDs without precision loss and rejects invalid lists', () => { + expect(internalBillingAccountIds('123, 9007199254740993,123')).toEqual([ + 123n, + 9007199254740993n, + ]); + expect(() => internalBillingAccountIds('123,broken')).toThrow( + 'INTERNAL_BILLING_ACCOUNT_IDS', + ); + expect(internalBillingAccountIds('')).toEqual([]); + }); + + it.each([false, true])( + 'excludes internal projects with memberOnly=%s', + (memberOnly) => { + process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123,456'; + const where = buildProjectWhereClause( + { memberOnly }, + { isMachine: false, userId: '42', roles: ['Talent Manager'] }, + true, + ); + expect(where.AND).toEqual( + expect.arrayContaining([ + { + OR: [ + { billingAccountId: null }, + { billingAccountId: { notIn: [123n, 456n] } }, + ], + }, + ]), + ); + expect((where.AND as unknown[]).length).toBe(memberOnly ? 2 : 1); + }, + ); +}); diff --git a/src/shared/utils/internal-project.utils.ts b/src/shared/utils/internal-project.utils.ts new file mode 100644 index 0000000..d285c42 --- /dev/null +++ b/src/shared/utils/internal-project.utils.ts @@ -0,0 +1,42 @@ +import { ADMIN_ROLES, UserRole } from '../enums/userRole.enum'; +import { JwtUser } from '../modules/global/jwt.service'; + +/** + * Identifies human Talent Managers subject to internal-project restrictions. + * Used by project queries and the global project-context interceptor. + * @param user Authenticated caller; administrators and machine tokens retain their policies. + * @returns Whether the caller must be excluded from internal projects. + * @throws Does not throw. + */ +export function isRestrictedTalentManager(user?: JwtUser): boolean { + if (!user || user.isMachine) return false; + const roles = (user.roles || []).map((role) => role.trim().toLowerCase()); + return ( + !ADMIN_ROLES.some((role) => roles.includes(role.toLowerCase())) && + [UserRole.TALENT_MANAGER, UserRole.TOPCODER_TALENT_MANAGER].some((role) => + roles.includes(role.toLowerCase()), + ) + ); +} + +/** + * Reads the configured internal billing account IDs for list and direct access checks. + * @param value Comma-separated positive IDs, defaulting to INTERNAL_BILLING_ACCOUNT_IDS. + * @returns Deduplicated bigint IDs; an empty configuration has no internal accounts. + * @throws Error if a configured ID is invalid, preventing a partial exclusion list. + */ +export function internalBillingAccountIds( + value: string = process.env.INTERNAL_BILLING_ACCOUNT_IDS || '', +): bigint[] { + if (!value.trim()) return []; + const ids = value.split(',').map((entry) => { + const id = entry.trim(); + if (!/^\d+$/.test(id) || BigInt(id) <= 0n) { + throw new Error( + 'INTERNAL_BILLING_ACCOUNT_IDS must contain positive numeric IDs', + ); + } + return BigInt(id); + }); + return [...new Set(ids)]; +} diff --git a/src/shared/utils/project.utils.ts b/src/shared/utils/project.utils.ts index 0268baf..fd4f1ee 100644 --- a/src/shared/utils/project.utils.ts +++ b/src/shared/utils/project.utils.ts @@ -5,6 +5,10 @@ * filter nested resources based on caller permissions. */ import { Prisma, ProjectStatus } from '@prisma/client'; +import { + internalBillingAccountIds, + isRestrictedTalentManager, +} from './internal-project.utils'; import { ProjectListQueryDto } from 'src/api/project/dto/project-list-query.dto'; import { PROJECT_MEMBER_MANAGER_ROLES } from 'src/shared/enums/projectMemberRole.enum'; import { JwtUser } from 'src/shared/modules/global/jwt.service'; @@ -300,7 +304,8 @@ export function parseFieldsParameter(fields?: string): ParsedProjectFields { * full-text terms plus case-insensitive contains matching. * - `code`: case-insensitive contains on name. * - `customer` / `manager`: member-subquery constraints. - * - non-admin or `memberOnly=true`: restrict to membership/invite ownership. + * - callers without global access or `memberOnly=true`: restrict to membership/invite ownership. + * - Talent Managers: exclude configured internal billing accounts, including for memberOnly. * When the caller has no resolvable membership identity (no parseable userId * and no email), applies an impossible `id = -1` guard to return zero rows. */ @@ -313,6 +318,18 @@ export function buildProjectWhereClause( deletedAt: null, }; + if (isRestrictedTalentManager(user)) { + const internalIds = internalBillingAccountIds(); + if (internalIds.length) { + appendAndCondition(where, { + OR: [ + { billingAccountId: null }, + { billingAccountId: { notIn: internalIds } }, + ], + }); + } + } + const idFilter = parseFilterValue(criteria.id); if (idFilter.length > 0) { const idList = toBigIntList(idFilter); From 10f66f2844fb20cc3bc55a3af6e9b78ef31713f8 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 28 Sep 2026 08:45:32 +1000 Subject: [PATCH 2/2] fix: allow talent managers to access internal projects they belong to --- .env.example | 2 +- README.md | 2 +- src/api/project/project.service.spec.ts | 82 +++++++++++++++++++ src/api/project/project.service.ts | 24 ++++-- .../projectContext.interceptor.spec.ts | 60 +++++++++++++- .../projectContext.interceptor.ts | 17 ++-- .../utils/internal-project.utils.spec.ts | 28 ++++++- src/shared/utils/internal-project.utils.ts | 20 ++++- src/shared/utils/project.utils.ts | 4 +- 9 files changed, 218 insertions(+), 21 deletions(-) diff --git a/.env.example b/.env.example index 4826994..6791d6a 100644 --- a/.env.example +++ b/.env.example @@ -95,5 +95,5 @@ MEMBERS_DB_URL="" RESOURCES_DB_URL="" SKILLS_DB_URL="" -# Comma-separated internal billing account IDs, excluded from Talent Manager access. +# Comma-separated internal billing account IDs, require active project membership for Talent Manager access. INTERNAL_BILLING_ACCOUNT_IDS="" diff --git a/README.md b/README.md index 6bc60a7..478bded 100644 --- a/README.md +++ b/README.md @@ -526,4 +526,4 @@ Migration runbook (phased rollout, rollback, monitoring): `docs/MIGRATION_RUNBOO `Talent Manager` and `Topcoder Talent Manager` users can list and open all non-internal projects without membership. Set `INTERNAL_BILLING_ACCOUNT_IDS` to the comma-separated positive billing account IDs used for internal projects before deploying this policy (for example, `123,456`); an empty value excludes no accounts, and invalid values reject requests rather than applying a partial list. Projects without a billing account remain visible. -Internal projects are excluded from list results and totals and rejected on direct project and nested project routes, even when the Talent Manager has membership or a pending invite. Administrators and machine tokens keep their existing authorization policies. `memberOnly=true` narrows the visible projects to existing membership/pending-invite associations and still excludes internal projects; Work exposes this as **My Projects**. +Internal projects are excluded from list results and totals and rejected on direct project and nested project routes unless the Talent Manager is an active project member. Explicit membership in any project role overrides the internal billing account exclusion; deleted memberships and pending invites alone do not. Administrators and machine tokens keep their existing authorization policies. `memberOnly=true` narrows the visible projects to existing membership/pending-invite associations, including internal projects with active membership; Work exposes this as **My Projects**. diff --git a/src/api/project/project.service.spec.ts b/src/api/project/project.service.spec.ts index e0611a7..a07b4c4 100644 --- a/src/api/project/project.service.spec.ts +++ b/src/api/project/project.service.spec.ts @@ -603,6 +603,88 @@ describe('ProjectService', () => { }, ); + describe('Talent Manager internal project membership override', () => { + const originalIds = process.env.INTERNAL_BILLING_ACCOUNT_IDS; + + beforeEach(() => { + process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123'; + billingAccountServiceMock.getBillingAccountsByIds.mockResolvedValue({}); + permissionServiceMock.hasNamedPermission.mockReturnValue(true); + }); + + afterEach(() => { + if (originalIds === undefined) + delete process.env.INTERNAL_BILLING_ACCOUNT_IDS; + else process.env.INTERNAL_BILLING_ACCOUNT_IDS = originalIds; + }); + + it.each([ + [UserRole.TALENT_MANAGER, 'manager', null, '42', true], + [UserRole.TOPCODER_TALENT_MANAGER, 'read', null, '42', true], + [UserRole.TALENT_MANAGER, 'customer', null, '42', true], + [UserRole.TALENT_MANAGER, 'manager', new Date(), '42', false], + [UserRole.TALENT_MANAGER, 'manager', null, '99', false], + ])( + 'checks %s with project role %s, deletedAt %s, and user %s', + async (role, memberRole, deletedAt, userId, allowed) => { + prismaMock.project.findFirst.mockResolvedValue({ + id: 1001n, + name: 'Internal', + billingAccountId: 123n, + members: [{ userId: 42n, role: memberRole, deletedAt }], + invites: [ + { userId: BigInt(userId), status: 'pending', deletedAt: null }, + ], + }); + const result = service.getProject('1001', 'id,name', { + userId, + roles: [role], + isMachine: false, + }); + if (allowed) + await expect(result).resolves.toMatchObject({ name: 'Internal' }); + else await expect(result).rejects.toBeInstanceOf(ForbiddenException); + }, + ); + + it.each([false, true])( + 'uses the membership override for both list results and totals with memberOnly=%s', + async (memberOnly) => { + prismaMock.project.count.mockResolvedValue(1); + prismaMock.project.findMany.mockResolvedValue([ + { + id: 1001n, + name: 'Internal', + billingAccountId: 123n, + members: [{ userId: 42n, role: 'read', deletedAt: null }], + invites: [], + }, + ]); + const result = await service.listProjects( + { memberOnly }, + { + userId: '42', + roles: [UserRole.TALENT_MANAGER], + isMachine: false, + }, + ); + expect(result.total).toBe(1); + expect(result.data).toEqual([ + expect.objectContaining({ name: 'Internal' }), + ]); + const where = prismaMock.project.findMany.mock.calls[0][0].where; + expect(prismaMock.project.count).toHaveBeenCalledWith({ where }); + expect(where.AND).toContainEqual({ + OR: [ + { billingAccountId: null }, + { billingAccountId: { notIn: [123n] } }, + { members: { some: { userId: 42n, deletedAt: null } } }, + ], + }); + }, + ); + }); + it('lists billing accounts for project id', async () => { billingAccountServiceMock.getBillingAccountsForProject.mockResolvedValue([ { diff --git a/src/api/project/project.service.ts b/src/api/project/project.service.ts index cbcb714..06cfbab 100644 --- a/src/api/project/project.service.ts +++ b/src/api/project/project.service.ts @@ -1,4 +1,5 @@ import { + activeProjectMembershipWhere, internalBillingAccountIds, isRestrictedTalentManager, } from '../../shared/utils/internal-project.utils'; @@ -139,8 +140,8 @@ export class ProjectService { * Returns a paginated project list for the caller. * * Builds query clauses from shared utilities, scopes non-admin callers to - * their memberships, grants Talent Managers non-internal projects, enriches handles, and hydrates billing - * account names. + * their memberships, grants Talent Managers non-internal projects and internal + * projects with active membership, enriches handles, and hydrates billing account names. * * @param criteria List filters, paging, sort, and field selection. * @param user Authenticated caller context. @@ -221,9 +222,11 @@ export class ProjectService { * * Members and invites are always loaded for permission evaluation regardless * of requested `fields`, then relation visibility is filtered by caller - * permissions before response serialization. Human PM/TM-style callers must - * still be a project member or pending invitee; only admins, legacy manager - * roles, and authorized machine principals bypass membership scoping. + * permissions before response serialization. Talent Managers can read non-internal + * projects without membership, but internal projects require active membership. + * Other human callers need membership or a pending invite unless they have an + * administrator or legacy manager role; authorized machine principals also bypass + * membership scoping. * * @param projectId Project id path parameter. * @param fieldsParam Optional CSV list of relation fields. @@ -263,10 +266,15 @@ export class ProjectService { if ( isRestrictedTalentManager(user) && project.billingAccountId !== null && - internalBillingAccountIds().includes(project.billingAccountId) + internalBillingAccountIds().includes(project.billingAccountId) && + !(project.members || []).some( + (member) => + member.userId === activeProjectMembershipWhere(user).userId && + member.deletedAt === null, + ) ) { throw new ForbiddenException( - 'Talent Managers cannot access internal projects', + 'Talent Managers must be active members to access internal projects', ); } @@ -1481,7 +1489,7 @@ export class ProjectService { * Returns whether the caller may bypass project membership visibility checks. * * Human callers retain global access for admins, legacy managers, and Talent Managers. - * Talent Manager internal-account exclusions are enforced separately. + * Talent Manager internal-account exclusions and active-membership overrides are enforced separately. * Machine principals continue to rely on the named permission so scoped * service tokens can read any project when authorized. * diff --git a/src/shared/interceptors/projectContext.interceptor.spec.ts b/src/shared/interceptors/projectContext.interceptor.spec.ts index dbd78e8..d759003 100644 --- a/src/shared/interceptors/projectContext.interceptor.spec.ts +++ b/src/shared/interceptors/projectContext.interceptor.spec.ts @@ -33,11 +33,14 @@ describe('ProjectContextInterceptor', () => { }), }) as ExecutionContext; - it('rejects internal project routes even when membership was already cached', async () => { + it('rejects internal project routes when cached membership is no longer active', async () => { const previous = process.env.INTERNAL_BILLING_ACCOUNT_IDS; process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123'; try { - prismaServiceMock.project.findFirst.mockResolvedValue({ id: 1001n }); + prismaServiceMock.project.findFirst.mockResolvedValue({ + id: 1001n, + members: [], + }); const request = { params: { projectId: '1001' }, user: { userId: '42', roles: ['Talent Manager'] }, @@ -51,7 +54,13 @@ describe('ProjectContextInterceptor', () => { ).rejects.toBeInstanceOf(ForbiddenException); expect(prismaServiceMock.project.findFirst).toHaveBeenCalledWith({ where: { id: 1001n, deletedAt: null, billingAccountId: { in: [123n] } }, - select: { id: true }, + select: { + id: true, + members: { + where: { userId: 42n, deletedAt: null }, + select: { id: true }, + }, + }, }); } finally { if (previous === undefined) @@ -60,6 +69,51 @@ describe('ProjectContextInterceptor', () => { } }); + it.each(['Talent Manager', 'Topcoder Talent Manager'])( + 'allows %s internal project routes with active membership', + async (role) => { + const previous = process.env.INTERNAL_BILLING_ACCOUNT_IDS; + process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123'; + try { + prismaServiceMock.project.findFirst.mockResolvedValue({ + id: 1001n, + members: [{ id: 1n }], + }); + prismaServiceMock.projectMember.findMany.mockResolvedValue([ + { id: 1n, userId: 42n, role: 'read', deletedAt: null }, + ]); + const request: any = { + params: { projectId: '1001' }, + user: { userId: '42', roles: [role] }, + }; + await expect( + interceptor.intercept(createExecutionContext(request), next), + ).resolves.toBeDefined(); + expect(request.projectContext.projectMembers).toEqual([ + expect.objectContaining({ userId: 42n, role: 'read' }), + ]); + expect(prismaServiceMock.project.findFirst).toHaveBeenCalledWith({ + where: { + id: 1001n, + deletedAt: null, + billingAccountId: { in: [123n] }, + }, + select: { + id: true, + members: { + where: { userId: 42n, deletedAt: null }, + select: { id: true }, + }, + }, + }); + } finally { + if (previous === undefined) + delete process.env.INTERNAL_BILLING_ACCOUNT_IDS; + else process.env.INTERNAL_BILLING_ACCOUNT_IDS = previous; + } + }, + ); + it('loads project members when projectId exists', async () => { const request: any = { params: { diff --git a/src/shared/interceptors/projectContext.interceptor.ts b/src/shared/interceptors/projectContext.interceptor.ts index 1f12cad..737e0f5 100644 --- a/src/shared/interceptors/projectContext.interceptor.ts +++ b/src/shared/interceptors/projectContext.interceptor.ts @@ -13,6 +13,7 @@ import { } from '@nestjs/common'; import { Observable } from 'rxjs'; import { + activeProjectMembershipWhere, internalBillingAccountIds, isRestrictedTalentManager, } from '../utils/internal-project.utils'; @@ -41,7 +42,7 @@ export class ProjectContextInterceptor implements NestInterceptor { * `projectId` route param is available. * * Behavior: - * - Rejects Talent Manager access to configured internal projects before cache hits. + * - Rejects Talent Manager access to configured internal projects without active membership, before cache hits. * - Initializes `request.projectContext` if absent. * - Short-circuits when no project id is present. * - Short-circuits on cache hits where project id already matches. @@ -49,7 +50,7 @@ export class ProjectContextInterceptor implements NestInterceptor { * - Queries active project members and maps `role` to plain strings. * - On query error, logs a warning and stores `projectMembers = []`. * - * @throws ForbiddenException for internal projects accessed by a Talent Manager. + * @throws ForbiddenException for internal projects accessed by a Talent Manager without active membership. * @throws Database/configuration errors during internal-project checks propagate. * @todo Member query + mapping logic is duplicated in multiple guards. * Introduce a shared `ProjectContextService` to centralize loading behavior. @@ -83,11 +84,17 @@ export class ProjectContextInterceptor implements NestInterceptor { deletedAt: null, billingAccountId: { in: internalIds }, }, - select: { id: true }, + select: { + id: true, + members: { + where: activeProjectMembershipWhere(request.user), + select: { id: true }, + }, + }, }); - if (internalProject) { + if (internalProject && !internalProject.members.length) { throw new ForbiddenException( - 'Talent Managers cannot access internal projects', + 'Talent Managers must be active members to access internal projects', ); } } diff --git a/src/shared/utils/internal-project.utils.spec.ts b/src/shared/utils/internal-project.utils.spec.ts index 5ca85dc..635028b 100644 --- a/src/shared/utils/internal-project.utils.spec.ts +++ b/src/shared/utils/internal-project.utils.spec.ts @@ -1,4 +1,5 @@ import { + activeProjectMembershipWhere, internalBillingAccountIds, isRestrictedTalentManager, } from './internal-project.utils'; @@ -25,6 +26,30 @@ describe('Talent Manager internal project visibility', () => { }, ); + it.each([undefined, '', 'handle', '42broken'])( + 'does not grant membership for an unresolved identity %s', + (userId) => { + expect( + activeProjectMembershipWhere({ userId, isMachine: false }), + ).toEqual({ + userId: -1n, + deletedAt: null, + }); + }, + ); + + it('normalizes numeric membership identities without precision loss', () => { + expect( + activeProjectMembershipWhere({ + userId: ' 9007199254740993 ', + isMachine: false, + }), + ).toEqual({ + userId: 9007199254740993n, + deletedAt: null, + }); + }); + it('retains administrator and machine policies', () => { expect( isRestrictedTalentManager({ @@ -55,7 +80,7 @@ describe('Talent Manager internal project visibility', () => { }); it.each([false, true])( - 'excludes internal projects with memberOnly=%s', + 'allows active membership to override internal exclusions with memberOnly=%s', (memberOnly) => { process.env.INTERNAL_BILLING_ACCOUNT_IDS = '123,456'; const where = buildProjectWhereClause( @@ -69,6 +94,7 @@ describe('Talent Manager internal project visibility', () => { OR: [ { billingAccountId: null }, { billingAccountId: { notIn: [123n, 456n] } }, + { members: { some: { userId: 42n, deletedAt: null } } }, ], }, ]), diff --git a/src/shared/utils/internal-project.utils.ts b/src/shared/utils/internal-project.utils.ts index d285c42..2bdc585 100644 --- a/src/shared/utils/internal-project.utils.ts +++ b/src/shared/utils/internal-project.utils.ts @@ -5,7 +5,7 @@ import { JwtUser } from '../modules/global/jwt.service'; * Identifies human Talent Managers subject to internal-project restrictions. * Used by project queries and the global project-context interceptor. * @param user Authenticated caller; administrators and machine tokens retain their policies. - * @returns Whether the caller must be excluded from internal projects. + * @returns Whether the caller needs active membership to access internal projects. * @throws Does not throw. */ export function isRestrictedTalentManager(user?: JwtUser): boolean { @@ -40,3 +40,21 @@ export function internalBillingAccountIds( }); return [...new Set(ids)]; } + +/** + * Builds the active membership filter used to override internal-project exclusions. + * Used by project list queries, direct reads, and the project-context interceptor. + * @param user Authenticated caller whose numeric user ID identifies membership. + * @returns Member criteria; missing or nonnumeric IDs use an impossible user ID. + * @throws Does not throw. + */ +export function activeProjectMembershipWhere(user?: JwtUser): { + userId: bigint; + deletedAt: null; +} { + const userId = String(user?.userId ?? '').trim(); + return { + userId: /^\d+$/.test(userId) ? BigInt(userId) : -1n, + deletedAt: null, + }; +} diff --git a/src/shared/utils/project.utils.ts b/src/shared/utils/project.utils.ts index fd4f1ee..692731e 100644 --- a/src/shared/utils/project.utils.ts +++ b/src/shared/utils/project.utils.ts @@ -6,6 +6,7 @@ */ import { Prisma, ProjectStatus } from '@prisma/client'; import { + activeProjectMembershipWhere, internalBillingAccountIds, isRestrictedTalentManager, } from './internal-project.utils'; @@ -305,7 +306,7 @@ export function parseFieldsParameter(fields?: string): ParsedProjectFields { * - `code`: case-insensitive contains on name. * - `customer` / `manager`: member-subquery constraints. * - callers without global access or `memberOnly=true`: restrict to membership/invite ownership. - * - Talent Managers: exclude configured internal billing accounts, including for memberOnly. + * - Talent Managers: exclude configured internal billing accounts unless actively a member, including for memberOnly. * When the caller has no resolvable membership identity (no parseable userId * and no email), applies an impossible `id = -1` guard to return zero rows. */ @@ -325,6 +326,7 @@ export function buildProjectWhereClause( OR: [ { billingAccountId: null }, { billingAccountId: { notIn: internalIds } }, + { members: { some: activeProjectMembershipWhere(user) } }, ], }); }