fix(gateway): filter projects by ownership — close data privacy leak (#202)
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Some checks failed
ci/woodpecker/push/ci Pipeline failed
Co-authored-by: Jason Woltje <jason@diversecanvas.com> Co-committed-by: Jason Woltje <jason@diversecanvas.com>
This commit was merged in pull request #202.
This commit is contained in:
@@ -18,6 +18,7 @@ function createBrain() {
|
|||||||
},
|
},
|
||||||
projects: {
|
projects: {
|
||||||
findAll: vi.fn(),
|
findAll: vi.fn(),
|
||||||
|
findAllForUser: vi.fn(),
|
||||||
findById: vi.fn(),
|
findById: vi.fn(),
|
||||||
create: vi.fn(),
|
create: vi.fn(),
|
||||||
update: vi.fn(),
|
update: vi.fn(),
|
||||||
@@ -67,7 +68,8 @@ describe('Resource ownership checks', () => {
|
|||||||
it('forbids access to another user project', async () => {
|
it('forbids access to another user project', async () => {
|
||||||
const brain = createBrain();
|
const brain = createBrain();
|
||||||
brain.projects.findById.mockResolvedValue({ id: 'project-1', ownerId: 'user-2' });
|
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(
|
await expect(controller.findOne('project-1', { id: 'user-1' })).rejects.toBeInstanceOf(
|
||||||
ForbiddenException,
|
ForbiddenException,
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import {
|
|||||||
Body,
|
Body,
|
||||||
Controller,
|
Controller,
|
||||||
Delete,
|
Delete,
|
||||||
|
ForbiddenException,
|
||||||
Get,
|
Get,
|
||||||
HttpCode,
|
HttpCode,
|
||||||
HttpStatus,
|
HttpStatus,
|
||||||
@@ -16,22 +17,25 @@ import type { Brain } from '@mosaic/brain';
|
|||||||
import { BRAIN } from '../brain/brain.tokens.js';
|
import { BRAIN } from '../brain/brain.tokens.js';
|
||||||
import { AuthGuard } from '../auth/auth.guard.js';
|
import { AuthGuard } from '../auth/auth.guard.js';
|
||||||
import { CurrentUser } from '../auth/current-user.decorator.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';
|
import { CreateProjectDto, UpdateProjectDto } from './projects.dto.js';
|
||||||
|
|
||||||
@Controller('api/projects')
|
@Controller('api/projects')
|
||||||
@UseGuards(AuthGuard)
|
@UseGuards(AuthGuard)
|
||||||
export class ProjectsController {
|
export class ProjectsController {
|
||||||
constructor(@Inject(BRAIN) private readonly brain: Brain) {}
|
constructor(
|
||||||
|
@Inject(BRAIN) private readonly brain: Brain,
|
||||||
|
private readonly teamsService: TeamsService,
|
||||||
|
) {}
|
||||||
|
|
||||||
@Get()
|
@Get()
|
||||||
async list() {
|
async list(@CurrentUser() user: { id: string }) {
|
||||||
return this.brain.projects.findAll();
|
return this.brain.projects.findAllForUser(user.id);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Get(':id')
|
@Get(':id')
|
||||||
async findOne(@Param('id') id: string, @CurrentUser() user: { id: string }) {
|
async findOne(@Param('id') id: string, @CurrentUser() user: { id: string }) {
|
||||||
return this.getOwnedProject(id, user.id);
|
return this.getAccessibleProject(id, user.id);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Post()
|
@Post()
|
||||||
@@ -50,7 +54,7 @@ export class ProjectsController {
|
|||||||
@Body() dto: UpdateProjectDto,
|
@Body() dto: UpdateProjectDto,
|
||||||
@CurrentUser() user: { id: string },
|
@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);
|
const project = await this.brain.projects.update(id, dto);
|
||||||
if (!project) throw new NotFoundException('Project not found');
|
if (!project) throw new NotFoundException('Project not found');
|
||||||
return project;
|
return project;
|
||||||
@@ -59,15 +63,21 @@ export class ProjectsController {
|
|||||||
@Delete(':id')
|
@Delete(':id')
|
||||||
@HttpCode(HttpStatus.NO_CONTENT)
|
@HttpCode(HttpStatus.NO_CONTENT)
|
||||||
async remove(@Param('id') id: string, @CurrentUser() user: { id: string }) {
|
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);
|
const deleted = await this.brain.projects.remove(id);
|
||||||
if (!deleted) throw new NotFoundException('Project not found');
|
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);
|
const project = await this.brain.projects.findById(id);
|
||||||
if (!project) throw new NotFoundException('Project not found');
|
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;
|
return project;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,7 +1,9 @@
|
|||||||
import { Module } from '@nestjs/common';
|
import { Module } from '@nestjs/common';
|
||||||
import { ProjectsController } from './projects.controller.js';
|
import { ProjectsController } from './projects.controller.js';
|
||||||
|
import { WorkspaceModule } from '../workspace/workspace.module.js';
|
||||||
|
|
||||||
@Module({
|
@Module({
|
||||||
|
imports: [WorkspaceModule],
|
||||||
controllers: [ProjectsController],
|
controllers: [ProjectsController],
|
||||||
})
|
})
|
||||||
export class ProjectsModule {}
|
export class ProjectsModule {}
|
||||||
|
|||||||
82
packages/brain/src/projects.spec.ts
Normal file
82
packages/brain/src/projects.spec.ts
Normal file
@@ -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<typeof vi.fn>; from?: ReturnType<typeof vi.fn> };
|
||||||
|
// 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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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 Project = typeof projects.$inferSelect;
|
||||||
export type NewProject = typeof projects.$inferInsert;
|
export type NewProject = typeof projects.$inferInsert;
|
||||||
@@ -9,6 +9,31 @@ export function createProjectsRepo(db: Db) {
|
|||||||
return db.select().from(projects);
|
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<Project[]> {
|
||||||
|
// 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<Project | undefined> {
|
async findById(id: string): Promise<Project | undefined> {
|
||||||
const rows = await db.select().from(projects).where(eq(projects.id, id));
|
const rows = await db.select().from(projects).where(eq(projects.id, id));
|
||||||
return rows[0];
|
return rows[0];
|
||||||
|
|||||||
Reference in New Issue
Block a user