Compare commits

..
Author SHA1 Message Date
fred dcb9da14d5 fix(gateway): scope /api/teams endpoints to team membership (#1428)
ci/woodpecker/pr/ci Pipeline was successful
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.
2026-08-26 17:16:07 -05:00
5 changed files with 253 additions and 163 deletions
+65 -82
View File
@@ -8,7 +8,7 @@ import {
Post,
} from '@nestjs/common';
import { randomBytes, createHash } from 'node:crypto';
import { count, eq, sql, type Db, users as usersTable, adminTokens } from '@mosaicstack/db';
import { count, eq, type Db, users as usersTable, adminTokens } from '@mosaicstack/db';
import type { Auth } from '@mosaicstack/auth';
import { v4 as uuid } from 'uuid';
import { AUTH } from '../auth/auth.tokens.js';
@@ -16,12 +16,6 @@ import { DB } from '../database/database.module.js';
import { BootstrapSetupDto } from './bootstrap.dto.js';
import type { BootstrapStatusDto, BootstrapResultDto } from './bootstrap.dto.js';
/**
* Advisory lock key serializing bootstrap setup. Arbitrary constant; must only
* be unique among advisory lock keys used against this database.
*/
export const BOOTSTRAP_SETUP_LOCK_KEY = 0x626f6f74; // 'boot'
@Controller('api/bootstrap')
export class BootstrapController {
constructor(
@@ -37,83 +31,72 @@ export class BootstrapController {
@Post('setup')
async setup(@Body() dto: BootstrapSetupDto): Promise<BootstrapResultDto> {
// #1430: the zero-user check and the admin creation must be one critical
// section, or two concurrent setup calls can each create an admin. The
// transaction-scoped advisory lock serializes setup across all gateway
// instances sharing this database; the second caller blocks on the lock,
// then re-reads the count and gets 403.
return this.db.transaction(async (tx) => {
await tx.execute(sql`select pg_advisory_xact_lock(${BOOTSTRAP_SETUP_LOCK_KEY})`);
// Only allow setup when zero users exist
const [result] = await this.db.select({ total: count() }).from(usersTable);
if ((result?.total ?? 0) > 0) {
throw new ForbiddenException('Setup already completed — users exist');
}
// Only allow setup when zero users exist
const [result] = await tx.select({ total: count() }).from(usersTable);
if ((result?.total ?? 0) > 0) {
throw new ForbiddenException('Setup already completed — users exist');
}
// Create admin user via BetterAuth API
const authApi = this.auth.api as unknown as {
createUser: (opts: {
body: { name: string; email: string; password: string; role?: string };
}) => Promise<{
user: { id: string; name: string; email: string };
}>;
};
// Create admin user via BetterAuth API. BetterAuth writes on its own
// connection and commits independently of this transaction; the reads
// below still see the committed row (READ COMMITTED statement snapshot).
const authApi = this.auth.api as unknown as {
createUser: (opts: {
body: { name: string; email: string; password: string; role?: string };
}) => Promise<{
user: { id: string; name: string; email: string };
}>;
};
const created = await authApi.createUser({
body: {
name: dto.name,
email: dto.email,
password: dto.password,
role: 'admin',
},
});
// Verify user was created
const [user] = await tx
.select()
.from(usersTable)
.where(eq(usersTable.id, created.user.id))
.limit(1);
if (!user) throw new InternalServerErrorException('User created but not found');
// Ensure role is admin (createUser may not set it via BetterAuth)
if (user.role !== 'admin') {
await tx.update(usersTable).set({ role: 'admin' }).where(eq(usersTable.id, user.id));
}
// Generate admin API token
const plaintext = randomBytes(32).toString('hex');
const tokenHash = createHash('sha256').update(plaintext).digest('hex');
const tokenId = uuid();
const [token] = await tx
.insert(adminTokens)
.values({
id: tokenId,
userId: user.id,
tokenHash,
label: 'Initial setup token',
scope: 'admin',
})
.returning();
return {
user: {
id: user.id,
name: user.name,
email: user.email,
role: 'admin',
},
token: {
id: token!.id,
plaintext,
label: token!.label,
},
};
const created = await authApi.createUser({
body: {
name: dto.name,
email: dto.email,
password: dto.password,
role: 'admin',
},
});
// Verify user was created
const [user] = await this.db
.select()
.from(usersTable)
.where(eq(usersTable.id, created.user.id))
.limit(1);
if (!user) throw new InternalServerErrorException('User created but not found');
// Ensure role is admin (createUser may not set it via BetterAuth)
if (user.role !== 'admin') {
await this.db.update(usersTable).set({ role: 'admin' }).where(eq(usersTable.id, user.id));
}
// Generate admin API token
const plaintext = randomBytes(32).toString('hex');
const tokenHash = createHash('sha256').update(plaintext).digest('hex');
const tokenId = uuid();
const [token] = await this.db
.insert(adminTokens)
.values({
id: tokenId,
userId: user.id,
tokenHash,
label: 'Initial setup token',
scope: 'admin',
})
.returning();
return {
user: {
id: user.id,
name: user.name,
email: user.email,
role: 'admin',
},
token: {
id: token!.id,
plaintext,
label: token!.label,
},
};
}
}
+4 -73
View File
@@ -20,7 +20,7 @@
*/
import 'reflect-metadata';
import { describe, it, expect, afterAll, beforeAll, vi } from 'vitest';
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
import { Test } from '@nestjs/testing';
import { ValidationPipe, type INestApplication } from '@nestjs/common';
import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify';
@@ -52,22 +52,13 @@ const mockAuth = {
},
};
// The controller runs setup inside db.transaction(tx) and first takes the
// #1430 advisory lock via tx.execute(). Inside the transaction it calls
// select().from() twice:
// Override db.select() so the second query (verify user exists) returns a user.
// The bootstrap controller calls select().from() twice:
// 1. count() to check zero users → returns [{total: 0}]
// 2. select().where().limit() → returns [the created user]
// callLog records the tx call order so the lock-before-check invariant is
// testable.
let selectCallCount = 0;
const callLog: string[] = [];
const mockTx = {
execute: () => {
callLog.push('execute');
return Promise.resolve([]);
},
const mockDbWithUser = {
select: () => {
callLog.push('select');
selectCallCount++;
return {
from: () => {
@@ -109,9 +100,6 @@ const mockTx = {
}),
}),
};
const mockDbWithUser = {
transaction: <T>(cb: (tx: typeof mockTx) => Promise<T>) => cb(mockTx),
};
// ─── Test suite ───────────────────────────────────────────────────────────────
@@ -165,11 +153,6 @@ describe('POST /api/bootstrap/setup — ValidationPipe DTO binding', () => {
expect(body.user.email).toBe('[email protected]');
expect(body.token).toBeDefined();
expect(body.token.plaintext).toBeDefined();
// #1430: the advisory lock must be taken before the zero-user count, or
// two concurrent setups can both pass the check.
expect(callLog[0]).toBe('execute');
expect(callLog[1]).toBe('select');
});
it('returns 400 when extra forbidden properties are sent', async () => {
@@ -205,55 +188,3 @@ describe('POST /api/bootstrap/setup — ValidationPipe DTO binding', () => {
expect(res.status).toBe(400);
});
});
// ─── #1430 regression: users-exist check runs inside the locked transaction ──
describe('POST /api/bootstrap/setup — setup already completed', () => {
let app: INestApplication;
const createUserSpy = vi.fn();
const lockedTx = {
execute: () => Promise.resolve([]),
select: () => ({
// count() sees an existing user — the locked re-check must reject.
from: () => Promise.resolve([{ total: 1 }]),
}),
};
const mockDbUsersExist = {
transaction: <T>(cb: (tx: typeof lockedTx) => Promise<T>) => cb(lockedTx),
};
beforeAll(async () => {
const moduleRef = await Test.createTestingModule({
controllers: [BootstrapController],
providers: [
{ provide: AUTH, useValue: { api: { createUser: createUserSpy } } },
{ provide: DB, useValue: mockDbUsersExist },
],
}).compile();
app = moduleRef.createNestApplication<NestFastifyApplication>(new FastifyAdapter());
app.useGlobalPipes(
new ValidationPipe({
whitelist: true,
forbidNonWhitelisted: true,
transform: true,
}),
);
await app.init();
await app.getHttpAdapter().getInstance().ready();
});
afterAll(async () => {
await app.close();
});
it('returns 403 and never calls createUser when users already exist', async () => {
const res = await request(app.getHttpServer())
.post('/api/bootstrap/setup')
.send({ name: 'Admin', email: '[email protected]', password: 'password123' })
.set('Content-Type', 'application/json');
expect(res.status).toBe(403);
expect(createUserSpy).not.toHaveBeenCalled();
});
});
@@ -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<NestFastifyApplication>(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 });
});
});
+45 -7
View File
@@ -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;
}
}
+16 -1
View File
@@ -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.
*/