From dcb9da14d588d9485d106975764fdaeb5de5f913 Mon Sep 17 00:00:00 2001 From: fred Date: Wed, 26 Aug 2026 17:16:07 -0500 Subject: [PATCH] fix(gateway): scope /api/teams endpoints to team membership (#1428) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every teams route sat behind AuthGuard only: any authenticated user could enumerate all teams and their full member records (webUI audit A3 F9.2, upgraded to major in cross-review). - GET /api/teams: members see only their teams (new TeamsService.findAllForUser, same two-step pattern as brain/projects.findAllForUser); admins keep the full list. - GET :teamId, :teamId/members: require admin or membership; 404 for a missing team, 403 for no access — the projects-controller convention. - GET :teamId/members/:userId: self-lookup stays open; looking up another user is team-scoped like the rest. No consumers of /api/teams exist in the monorepo, so no caller changes. New controller spec: 9 tests. Gateway suite 818 passed, typecheck clean. --- .../src/workspace/teams.controller.spec.ts | 123 ++++++++++++++++++ .../gateway/src/workspace/teams.controller.ts | 52 +++++++- apps/gateway/src/workspace/teams.service.ts | 17 ++- 3 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 apps/gateway/src/workspace/teams.controller.spec.ts diff --git a/apps/gateway/src/workspace/teams.controller.spec.ts b/apps/gateway/src/workspace/teams.controller.spec.ts new file mode 100644 index 00000000..c24b4ffd --- /dev/null +++ b/apps/gateway/src/workspace/teams.controller.spec.ts @@ -0,0 +1,123 @@ +import 'reflect-metadata'; +import { type CanActivate, type ExecutionContext, type INestApplication } from '@nestjs/common'; +import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify'; +import { Test } from '@nestjs/testing'; +import request from 'supertest'; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { AuthGuard } from '../auth/auth.guard.js'; +import { TeamsController } from './teams.controller.js'; +import { TeamsService } from './teams.service.js'; + +const teamAlpha = { id: 'team-alpha', name: 'Alpha' }; +const teamBeta = { id: 'team-beta', name: 'Beta' }; + +// user-1 is a member of team-alpha only; admin-1 has role admin. +let currentUser: { id: string; role?: string } = { id: 'user-1' }; + +const teamsServiceMock = { + findAll: vi.fn(() => Promise.resolve([teamAlpha, teamBeta])), + findAllForUser: vi.fn((userId: string) => + Promise.resolve(userId === 'user-1' ? [teamAlpha] : []), + ), + findById: vi.fn((id: string) => Promise.resolve([teamAlpha, teamBeta].find((t) => t.id === id))), + listMembers: vi.fn(() => Promise.resolve([{ teamId: 'team-alpha', userId: 'user-1' }])), + isMember: vi.fn((teamId: string, userId: string) => + Promise.resolve(teamId === 'team-alpha' && userId === 'user-1'), + ), +}; + +const authGuard: CanActivate = { + canActivate(context: ExecutionContext): boolean { + const requestContext = context + .switchToHttp() + .getRequest<{ user?: { id: string; role?: string } }>(); + requestContext.user = currentUser; + return true; + }, +}; + +describe('teams endpoints are scoped to membership', () => { + let app: INestApplication; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + controllers: [TeamsController], + providers: [{ provide: TeamsService, useValue: teamsServiceMock }], + }) + .overrideGuard(AuthGuard) + .useValue(authGuard) + .compile(); + + app = moduleRef.createNestApplication(new FastifyAdapter()); + await app.init(); + await app.getHttpAdapter().getInstance().ready(); + }); + + beforeEach(() => { + currentUser = { id: 'user-1' }; + vi.clearAllMocks(); + }); + + afterAll(async () => { + await app.close(); + }); + + it('GET /api/teams returns only the teams the user belongs to', async () => { + const response = await request(app.getHttpServer()).get('/api/teams'); + expect(response.status).toBe(200); + expect(response.body).toEqual([teamAlpha]); + expect(teamsServiceMock.findAll).not.toHaveBeenCalled(); + }); + + it('GET /api/teams returns every team for an admin', async () => { + currentUser = { id: 'admin-1', role: 'admin' }; + const response = await request(app.getHttpServer()).get('/api/teams'); + expect(response.status).toBe(200); + expect(response.body).toEqual([teamAlpha, teamBeta]); + expect(teamsServiceMock.findAllForUser).not.toHaveBeenCalled(); + }); + + it('GET /api/teams/:teamId returns 403 for a non-member', async () => { + const response = await request(app.getHttpServer()).get('/api/teams/team-beta'); + expect(response.status).toBe(403); + }); + + it('GET /api/teams/:teamId returns 404 for a missing team', async () => { + const response = await request(app.getHttpServer()).get('/api/teams/team-missing'); + expect(response.status).toBe(404); + }); + + it('GET /api/teams/:teamId returns the team for a member', async () => { + const response = await request(app.getHttpServer()).get('/api/teams/team-alpha'); + expect(response.status).toBe(200); + expect(response.body).toEqual(teamAlpha); + }); + + it('GET /api/teams/:teamId/members returns 403 for a non-member and members for a member', async () => { + const denied = await request(app.getHttpServer()).get('/api/teams/team-beta/members'); + expect(denied.status).toBe(403); + expect(teamsServiceMock.listMembers).not.toHaveBeenCalled(); + + const allowed = await request(app.getHttpServer()).get('/api/teams/team-alpha/members'); + expect(allowed.status).toBe(200); + expect(allowed.body).toEqual([{ teamId: 'team-alpha', userId: 'user-1' }]); + }); + + it('GET /api/teams/:teamId/members/:userId allows a self-lookup on any team', async () => { + const response = await request(app.getHttpServer()).get('/api/teams/team-beta/members/user-1'); + expect(response.status).toBe(200); + expect(response.body).toEqual({ isMember: false }); + }); + + it('GET /api/teams/:teamId/members/:userId denies looking up another user on a foreign team', async () => { + const response = await request(app.getHttpServer()).get('/api/teams/team-beta/members/user-2'); + expect(response.status).toBe(403); + }); + + it('an admin can look up any membership', async () => { + currentUser = { id: 'admin-1', role: 'admin' }; + const response = await request(app.getHttpServer()).get('/api/teams/team-alpha/members/user-1'); + expect(response.status).toBe(200); + expect(response.body).toEqual({ isMember: true }); + }); +}); diff --git a/apps/gateway/src/workspace/teams.controller.ts b/apps/gateway/src/workspace/teams.controller.ts index 0046b78c..d997b4ff 100644 --- a/apps/gateway/src/workspace/teams.controller.ts +++ b/apps/gateway/src/workspace/teams.controller.ts @@ -1,30 +1,68 @@ -import { Controller, Get, Param, UseGuards } from '@nestjs/common'; +import { + Controller, + ForbiddenException, + Get, + NotFoundException, + Param, + UseGuards, +} from '@nestjs/common'; import { AuthGuard } from '../auth/auth.guard.js'; +import { CurrentUser } from '../auth/current-user.decorator.js'; import { TeamsService } from './teams.service.js'; +type RequestUser = { id: string; role?: string }; + @Controller('api/teams') @UseGuards(AuthGuard) export class TeamsController { constructor(private readonly teams: TeamsService) {} @Get() - async list() { - return this.teams.findAll(); + async list(@CurrentUser() user: RequestUser) { + if (user.role === 'admin') { + return this.teams.findAll(); + } + return this.teams.findAllForUser(user.id); } @Get(':teamId') - async findOne(@Param('teamId') teamId: string) { - return this.teams.findById(teamId); + async findOne(@Param('teamId') teamId: string, @CurrentUser() user: RequestUser) { + return this.getAccessibleTeam(teamId, user); } @Get(':teamId/members') - async listMembers(@Param('teamId') teamId: string) { + async listMembers(@Param('teamId') teamId: string, @CurrentUser() user: RequestUser) { + await this.getAccessibleTeam(teamId, user); return this.teams.listMembers(teamId); } @Get(':teamId/members/:userId') - async checkMembership(@Param('teamId') teamId: string, @Param('userId') userId: string) { + async checkMembership( + @Param('teamId') teamId: string, + @Param('userId') userId: string, + @CurrentUser() user: RequestUser, + ) { + // A user may always ask about their own membership; anything else is + // team-scoped like the other routes. + if (userId !== user.id) { + await this.getAccessibleTeam(teamId, user); + } const isMember = await this.teams.isMember(teamId, userId); return { isMember }; } + + /** + * Team-scoped access: admins see any team; everyone else only teams they + * are a member of. NotFoundException when the team does not exist and + * ForbiddenException when the user lacks access (same convention as the + * projects controller). + */ + private async getAccessibleTeam(teamId: string, user: RequestUser) { + const team = await this.teams.findById(teamId); + if (!team) throw new NotFoundException('Team not found'); + if (user.role === 'admin') return team; + const isMember = await this.teams.isMember(teamId, user.id); + if (!isMember) throw new ForbiddenException('Not a member of this team'); + return team; + } } diff --git a/apps/gateway/src/workspace/teams.service.ts b/apps/gateway/src/workspace/teams.service.ts index 77d2ccb3..f975cc17 100644 --- a/apps/gateway/src/workspace/teams.service.ts +++ b/apps/gateway/src/workspace/teams.service.ts @@ -1,5 +1,5 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; -import { eq, and, type Db, teams, teamMembers, projects } from '@mosaicstack/db'; +import { eq, and, inArray, type Db, teams, teamMembers, projects } from '@mosaicstack/db'; import { DB } from '../database/database.module.js'; @Injectable() @@ -56,6 +56,21 @@ export class TeamsService { return this.db.select().from(teams); } + /** + * List only the teams the user is a member of. + */ + async findAllForUser(userId: string) { + const memberRows = await this.db + .select({ teamId: teamMembers.teamId }) + .from(teamMembers) + .where(eq(teamMembers.userId, userId)); + + const teamIds = memberRows.map((r) => r.teamId); + if (teamIds.length === 0) return []; + + return this.db.select().from(teams).where(inArray(teams.id, teamIds)); + } + /** * Find a team by ID. */