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);