From db35ba03b9fa1d5ffcf9663f02115d1056610d08 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Mar 2026 21:33:39 -0500 Subject: [PATCH] =?UTF-8?q?fix(gateway):=20filter=20projects=20by=20owners?= =?UTF-8?q?hip=20=E2=80=94=20close=20data=20privacy=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/projects now returns only projects owned by the requesting user or belonging to teams the user is a member of, via a new findAllForUser() method in the brain projects repo. GET/PATCH/DELETE single-project endpoints now use canAccessProject() (handling both user and team ownership) instead of the direct-owner-only assertOwner(). Fixes #197. Co-Authored-By: Claude Sonnet 4.6 --- .../src/__tests__/resource-ownership.test.ts | 4 +- .../src/projects/projects.controller.ts | 28 +++++-- apps/gateway/src/projects/projects.module.ts | 2 + packages/brain/src/projects.spec.ts | 82 +++++++++++++++++++ packages/brain/src/projects.ts | 27 +++++- 5 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 packages/brain/src/projects.spec.ts diff --git a/apps/gateway/src/__tests__/resource-ownership.test.ts b/apps/gateway/src/__tests__/resource-ownership.test.ts index a1b7dfb..bfc241f 100644 --- a/apps/gateway/src/__tests__/resource-ownership.test.ts +++ b/apps/gateway/src/__tests__/resource-ownership.test.ts @@ -18,6 +18,7 @@ function createBrain() { }, projects: { findAll: vi.fn(), + findAllForUser: vi.fn(), findById: vi.fn(), create: vi.fn(), update: vi.fn(), @@ -67,7 +68,8 @@ describe('Resource ownership checks', () => { it('forbids access to another user project', async () => { const brain = createBrain(); brain.projects.findById.mockResolvedValue({ id: 'project-1', ownerId: 'user-2' }); - const controller = new ProjectsController(brain as never); + const teamsService = { canAccessProject: vi.fn().mockResolvedValue(false) }; + const controller = new ProjectsController(brain as never, teamsService as never); await expect(controller.findOne('project-1', { id: 'user-1' })).rejects.toBeInstanceOf( ForbiddenException, diff --git a/apps/gateway/src/projects/projects.controller.ts b/apps/gateway/src/projects/projects.controller.ts index c0acbad..4b98765 100644 --- a/apps/gateway/src/projects/projects.controller.ts +++ b/apps/gateway/src/projects/projects.controller.ts @@ -2,6 +2,7 @@ import { Body, Controller, Delete, + ForbiddenException, Get, HttpCode, HttpStatus, @@ -16,22 +17,25 @@ import type { Brain } from '@mosaic/brain'; import { BRAIN } from '../brain/brain.tokens.js'; import { AuthGuard } from '../auth/auth.guard.js'; import { CurrentUser } from '../auth/current-user.decorator.js'; -import { assertOwner } from '../auth/resource-ownership.js'; +import { TeamsService } from '../workspace/teams.service.js'; import { CreateProjectDto, UpdateProjectDto } from './projects.dto.js'; @Controller('api/projects') @UseGuards(AuthGuard) export class ProjectsController { - constructor(@Inject(BRAIN) private readonly brain: Brain) {} + constructor( + @Inject(BRAIN) private readonly brain: Brain, + private readonly teamsService: TeamsService, + ) {} @Get() - async list() { - return this.brain.projects.findAll(); + async list(@CurrentUser() user: { id: string }) { + return this.brain.projects.findAllForUser(user.id); } @Get(':id') async findOne(@Param('id') id: string, @CurrentUser() user: { id: string }) { - return this.getOwnedProject(id, user.id); + return this.getAccessibleProject(id, user.id); } @Post() @@ -50,7 +54,7 @@ export class ProjectsController { @Body() dto: UpdateProjectDto, @CurrentUser() user: { id: string }, ) { - await this.getOwnedProject(id, user.id); + await this.getAccessibleProject(id, user.id); const project = await this.brain.projects.update(id, dto); if (!project) throw new NotFoundException('Project not found'); return project; @@ -59,15 +63,21 @@ export class ProjectsController { @Delete(':id') @HttpCode(HttpStatus.NO_CONTENT) async remove(@Param('id') id: string, @CurrentUser() user: { id: string }) { - await this.getOwnedProject(id, user.id); + await this.getAccessibleProject(id, user.id); const deleted = await this.brain.projects.remove(id); if (!deleted) throw new NotFoundException('Project not found'); } - private async getOwnedProject(id: string, userId: string) { + /** + * Verify the requesting user can access the project — either as the direct + * owner or as a member of the owning team. Throws NotFoundException when the + * project does not exist and ForbiddenException when the user lacks access. + */ + private async getAccessibleProject(id: string, userId: string) { const project = await this.brain.projects.findById(id); if (!project) throw new NotFoundException('Project not found'); - assertOwner(project.ownerId, userId, 'Project'); + const canAccess = await this.teamsService.canAccessProject(userId, id); + if (!canAccess) throw new ForbiddenException('Project does not belong to the current user'); return project; } } diff --git a/apps/gateway/src/projects/projects.module.ts b/apps/gateway/src/projects/projects.module.ts index 3398bf8..3da65cb 100644 --- a/apps/gateway/src/projects/projects.module.ts +++ b/apps/gateway/src/projects/projects.module.ts @@ -1,7 +1,9 @@ import { Module } from '@nestjs/common'; import { ProjectsController } from './projects.controller.js'; +import { WorkspaceModule } from '../workspace/workspace.module.js'; @Module({ + imports: [WorkspaceModule], controllers: [ProjectsController], }) export class ProjectsModule {} diff --git a/packages/brain/src/projects.spec.ts b/packages/brain/src/projects.spec.ts new file mode 100644 index 0000000..ee1f8e4 --- /dev/null +++ b/packages/brain/src/projects.spec.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi } from 'vitest'; +import { createProjectsRepo } from './projects.js'; + +/** + * Build a minimal Drizzle mock. Each call to db.select() returns a fresh + * chain that resolves `where()` to the provided rows for that call. + * + * `calls` is an ordered list: the first item is returned for the first + * db.select() call, the second for the second, and so on. + */ +function makeDb(calls: unknown[][]) { + let callIndex = 0; + const selectSpy = vi.fn(() => { + const rows = calls[callIndex++] ?? []; + const chain = { + where: vi.fn().mockResolvedValue(rows), + } as { where: ReturnType; from?: ReturnType }; + // from() returns the chain so .where() can be chained, but also resolves + // directly (as a thenable) for queries with no .where() call. + chain.from = vi.fn(() => Object.assign(Promise.resolve(rows), chain)); + return chain; + }); + return { select: selectSpy }; +} + +describe('createProjectsRepo — findAllForUser', () => { + it('filters by userId when user has no team memberships', async () => { + // First select: teamMembers query → empty + // Second select: projects query → one owned project + const db = makeDb([ + [], // teamMembers rows + [{ id: 'p1', ownerId: 'user-1', teamId: null, ownerType: 'user' }], + ]); + const repo = createProjectsRepo(db as never); + + const result = await repo.findAllForUser('user-1'); + + expect(db.select).toHaveBeenCalledTimes(2); + expect(result).toHaveLength(1); + expect(result[0]?.id).toBe('p1'); + }); + + it('includes team projects when user is a team member', async () => { + // First select: teamMembers → user belongs to one team + // Second select: projects query → two projects (own + team) + const db = makeDb([ + [{ teamId: 'team-1' }], + [ + { id: 'p1', ownerId: 'user-1', teamId: null, ownerType: 'user' }, + { id: 'p2', ownerId: null, teamId: 'team-1', ownerType: 'team' }, + ], + ]); + const repo = createProjectsRepo(db as never); + + const result = await repo.findAllForUser('user-1'); + + expect(db.select).toHaveBeenCalledTimes(2); + expect(result).toHaveLength(2); + }); + + it('returns empty array when user has no projects and no teams', async () => { + const db = makeDb([[], []]); + const repo = createProjectsRepo(db as never); + + const result = await repo.findAllForUser('user-no-projects'); + expect(result).toHaveLength(0); + }); +}); + +describe('createProjectsRepo — findAll', () => { + it('returns all rows without any user filter', async () => { + const rows = [ + { id: 'p1', ownerId: 'user-1', teamId: null, ownerType: 'user' }, + { id: 'p2', ownerId: 'user-2', teamId: null, ownerType: 'user' }, + ]; + const db = makeDb([rows]); + const repo = createProjectsRepo(db as never); + + const result = await repo.findAll(); + expect(result).toHaveLength(2); + }); +}); diff --git a/packages/brain/src/projects.ts b/packages/brain/src/projects.ts index edb4068..8a2418b 100644 --- a/packages/brain/src/projects.ts +++ b/packages/brain/src/projects.ts @@ -1,4 +1,4 @@ -import { eq, type Db, projects } from '@mosaic/db'; +import { eq, or, inArray, type Db, projects, teamMembers } from '@mosaic/db'; export type Project = typeof projects.$inferSelect; export type NewProject = typeof projects.$inferInsert; @@ -9,6 +9,31 @@ export function createProjectsRepo(db: Db) { return db.select().from(projects); }, + /** + * Return only the projects visible to a given user: + * – projects directly owned by the user (ownerType = 'user', ownerId = userId), OR + * – projects owned by a team the user belongs to (ownerType = 'team', teamId IN user's teams) + */ + async findAllForUser(userId: string): Promise { + // Fetch the team IDs the user is a member of. + const memberRows = await db + .select({ teamId: teamMembers.teamId }) + .from(teamMembers) + .where(eq(teamMembers.userId, userId)); + + const teamIds = memberRows.map((r) => r.teamId); + + if (teamIds.length === 0) { + // No team memberships — return only directly owned projects. + return db.select().from(projects).where(eq(projects.ownerId, userId)); + } + + return db + .select() + .from(projects) + .where(or(eq(projects.ownerId, userId), inArray(projects.teamId, teamIds))); + }, + async findById(id: string): Promise { const rows = await db.select().from(projects).where(eq(projects.id, id)); return rows[0];