diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbfa00c..a95d67f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,3 +66,6 @@ jobs: - name: Build run: pnpm build + + - name: Test + run: pnpm test diff --git a/apps/api/package.json b/apps/api/package.json index 4f07590..acd1535 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -18,11 +18,13 @@ "@nestjs/core": "^10.4.15", "@nestjs/jwt": "^11.0.2", "@nestjs/platform-express": "^10.4.15", + "@nestjs/throttler": "^6.4.0", "@prisma/client": "^5.22.0", "@sentry/node": "^10.53.1", "@statify/db": "workspace:*", "@statify/shared": "workspace:*", "argon2": "^0.44.0", + "helmet": "^8.0.0", "nestjs-pino": "^4.1.0", "pino": "^9.5.0", "pino-http": "^10.3.0", diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts index 6c8dbd2..be7c2a5 100644 --- a/apps/api/src/app.module.ts +++ b/apps/api/src/app.module.ts @@ -1,4 +1,6 @@ import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common'; +import { APP_GUARD } from '@nestjs/core'; +import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler'; import { LoggerModule } from './common/logger/logger.module'; import { RequestIdMiddleware } from './common/logger/request-id.middleware'; import { ConfigModule } from './config/config.module'; @@ -17,6 +19,8 @@ import { UserPlaylistsModule } from './modules/user-playlists/user-playlists.mod imports: [ ConfigModule, LoggerModule, + // Per-IP rate limiting (global default; stricter limits on auth routes). + ThrottlerModule.forRoot([{ ttl: 60_000, limit: 100 }]), PrismaModule, ItunesModule, AuthModule, @@ -28,6 +32,7 @@ import { UserPlaylistsModule } from './modules/user-playlists/user-playlists.mod AnalyticsModule, AdminModule, ], + providers: [{ provide: APP_GUARD, useClass: ThrottlerGuard }], }) export class AppModule implements NestModule { configure(consumer: MiddlewareConsumer): void { diff --git a/apps/api/src/main.ts b/apps/api/src/main.ts index 60c0a5d..618b632 100644 --- a/apps/api/src/main.ts +++ b/apps/api/src/main.ts @@ -2,6 +2,7 @@ import 'reflect-metadata'; import { HEADERS } from '@statify/shared'; import { NestFactory } from '@nestjs/core'; import * as Sentry from '@sentry/node'; +import helmet from 'helmet'; import { Logger } from 'nestjs-pino'; import { AppModule } from './app.module'; import { AllExceptionsFilter } from './common/filters/all-exceptions.filter'; @@ -21,6 +22,7 @@ async function bootstrap(): Promise { const app = await NestFactory.create(AppModule, { bufferLogs: true }); + app.use(helmet()); app.useLogger(app.get(Logger)); app.useGlobalFilters(new AllExceptionsFilter()); diff --git a/apps/api/src/modules/auth/auth.controller.ts b/apps/api/src/modules/auth/auth.controller.ts index 952aa58..09071c1 100644 --- a/apps/api/src/modules/auth/auth.controller.ts +++ b/apps/api/src/modules/auth/auth.controller.ts @@ -10,6 +10,7 @@ import { Res, UseGuards, } from '@nestjs/common'; +import { Throttle } from '@nestjs/throttler'; import { AccountDeleteRequest, AccountDeleteRequestSchema, @@ -42,6 +43,7 @@ export class AuthController { private readonly cookieService: AuthCookieService, ) {} + @Throttle({ default: { limit: 5, ttl: 60_000 } }) @Post('register') async register( @Body(new ZodValidationPipe(RegisterRequestSchema)) body: RegisterRequest, @@ -55,6 +57,7 @@ export class AuthController { return { user: session.user }; } + @Throttle({ default: { limit: 10, ttl: 60_000 } }) @Post('login') async login( @Body(new ZodValidationPipe(LoginRequestSchema)) body: LoginRequest, @@ -68,6 +71,7 @@ export class AuthController { return { user: session.user }; } + @Throttle({ default: { limit: 30, ttl: 60_000 } }) @Post('refresh') @UseGuards(CsrfGuard) async refresh( diff --git a/apps/api/src/modules/auth/auth.service.ts b/apps/api/src/modules/auth/auth.service.ts index ecfb126..41fe49a 100644 --- a/apps/api/src/modules/auth/auth.service.ts +++ b/apps/api/src/modules/auth/auth.service.ts @@ -1,4 +1,5 @@ import { HttpStatus, Injectable } from '@nestjs/common'; +import { Prisma, type User } from '@prisma/client'; import { AppError, ErrorCode, @@ -33,11 +34,21 @@ export class AuthService { }); } - const user = await this.repository.createUser({ - email: input.email, - displayName: input.displayName, - passwordHash: await this.passwordService.hash(input.password), - }); + // findUserByEmail excludes soft-deleted/banned rows, so a soft-deleted + // account can still own this email; catch the unique violation as a 409. + let user: User; + try { + user = await this.repository.createUser({ + email: input.email, + displayName: input.displayName, + passwordHash: await this.passwordService.hash(input.password), + }); + } catch (error) { + if (isUniqueEmailViolation(error)) { + throw emailTakenError(); + } + throw error; + } await this.auditLog.record({ actorUserId: user.id, @@ -201,10 +212,18 @@ export class AuthService { } } - const updated = await this.repository.updateProfile(userId, { - displayName: input.displayName, - email: input.email, - }); + let updated: User; + try { + updated = await this.repository.updateProfile(userId, { + displayName: input.displayName, + email: input.email, + }); + } catch (error) { + if (isUniqueEmailViolation(error)) { + throw emailTakenError(); + } + throw error; + } await this.auditLog.record({ actorUserId: userId, @@ -320,6 +339,10 @@ function isUsableRefreshToken(token: RefreshTokenWithUser | null): token is Refr return token !== null && token.revokedAt === null && token.expiresAt > new Date(); } +function isUniqueEmailViolation(error: unknown): boolean { + return error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002'; +} + function clientMetadata(context: AuthRequestContext): Record { const meta: Record = {}; if (context.ipAddr !== undefined && context.ipAddr.length > 0) meta.ip = context.ipAddr; diff --git a/apps/api/src/modules/auth/guards/jwt-auth.guard.spec.ts b/apps/api/src/modules/auth/guards/jwt-auth.guard.spec.ts index b2b00a0..6d31643 100644 --- a/apps/api/src/modules/auth/guards/jwt-auth.guard.spec.ts +++ b/apps/api/src/modules/auth/guards/jwt-auth.guard.spec.ts @@ -1,26 +1,37 @@ import { ErrorCode } from '@statify/shared'; import type { ExecutionContext } from '@nestjs/common'; +import type { User } from '@prisma/client'; import type { Request } from 'express'; import { describe, expect, it, vi } from 'vitest'; +import type { AuthRepository } from '../auth.repository'; import type { AuthTokenService } from '../auth-token.service'; import { JwtAuthGuard } from './jwt-auth.guard'; describe('JwtAuthGuard', () => { - it('attaches the authenticated user from a valid access token', async () => { + it('attaches the authenticated user from the current account record', async () => { const tokenService = { verifyAccessToken: vi.fn().mockResolvedValue({ sub: 1, + email: 'stale@example.com', + displayName: 'Stale Name', + role: 'user', + }), + } as unknown as AuthTokenService; + const repository = { + findUserById: vi.fn().mockResolvedValue({ + id: 1, email: 'user@example.com', displayName: 'Admin User', role: 'admin', - }), - } as unknown as AuthTokenService; - const guard = new JwtAuthGuard(tokenService); + } as User), + } as unknown as AuthRepository; + const guard = new JwtAuthGuard(tokenService, repository); const request = { headers: { cookie: 'sf_access=access-token' }, } as Request & { user?: unknown }; await expect(guard.canActivate(createContext(request))).resolves.toBe(true); + // Identity comes from the DB row, not the (possibly stale) token payload. expect(request.user).toEqual({ id: 1, email: 'user@example.com', @@ -29,10 +40,26 @@ describe('JwtAuthGuard', () => { }); }); + it('rejects a token whose account no longer exists (banned or deleted)', async () => { + const tokenService = { + verifyAccessToken: vi.fn().mockResolvedValue({ sub: 1, role: 'user' }), + } as unknown as AuthTokenService; + const repository = { + findUserById: vi.fn().mockResolvedValue(null), + } as unknown as AuthRepository; + const guard = new JwtAuthGuard(tokenService, repository); + const request = { headers: { cookie: 'sf_access=access-token' } } as Request; + + await expect(guard.canActivate(createContext(request))).rejects.toMatchObject({ + code: ErrorCode.UNAUTHENTICATED, + }); + }); + it('rejects requests without an access token', async () => { - const guard = new JwtAuthGuard({ - verifyAccessToken: vi.fn(), - } as unknown as AuthTokenService); + const guard = new JwtAuthGuard( + { verifyAccessToken: vi.fn() } as unknown as AuthTokenService, + { findUserById: vi.fn() } as unknown as AuthRepository, + ); await expect( guard.canActivate(createContext({ headers: {} } as Request)), diff --git a/apps/api/src/modules/auth/guards/jwt-auth.guard.ts b/apps/api/src/modules/auth/guards/jwt-auth.guard.ts index 57167a5..418dc22 100644 --- a/apps/api/src/modules/auth/guards/jwt-auth.guard.ts +++ b/apps/api/src/modules/auth/guards/jwt-auth.guard.ts @@ -1,13 +1,18 @@ import { CanActivate, ExecutionContext, HttpStatus, Injectable } from '@nestjs/common'; import { AppError, COOKIE_NAMES, ErrorCode } from '@statify/shared'; import type { Request } from 'express'; +import { toAuthUser } from '../auth.mapper'; +import { AuthRepository } from '../auth.repository'; import { AuthTokenService } from '../auth-token.service'; import type { RequestWithUser } from '../auth.types'; import { getCookie } from '../cookie.utils'; @Injectable() export class JwtAuthGuard implements CanActivate { - constructor(private readonly tokenService: AuthTokenService) {} + constructor( + private readonly tokenService: AuthTokenService, + private readonly repository: AuthRepository, + ) {} async canActivate(context: ExecutionContext): Promise { const request = context.switchToHttp().getRequest(); @@ -17,20 +22,25 @@ export class JwtAuthGuard implements CanActivate { throw unauthenticatedError(); } + let userId: number; try { const payload = await this.tokenService.verifyAccessToken(accessToken); - - request.user = { - id: payload.sub, - email: payload.email, - displayName: payload.displayName, - role: payload.role, - }; - - return true; + userId = payload.sub; } catch { throw unauthenticatedError(); } + + // Re-check the account on every request so a banned, soft-deleted, or + // role-changed user cannot keep acting on a still-valid access token + // (findUserById filters out deletedAt/bannedAt rows). + const user = await this.repository.findUserById(userId); + if (user === null) { + throw unauthenticatedError(); + } + + request.user = toAuthUser(user); + + return true; } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8651558..10e9849 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -69,6 +69,9 @@ importers: '@nestjs/platform-express': specifier: ^10.4.15 version: 10.4.22(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.22) + '@nestjs/throttler': + specifier: ^6.4.0 + version: 6.5.0(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.22)(reflect-metadata@0.2.2) '@prisma/client': specifier: ^5.22.0 version: 5.22.0(prisma@5.22.0) @@ -84,6 +87,9 @@ importers: argon2: specifier: ^0.44.0 version: 0.44.0 + helmet: + specifier: ^8.0.0 + version: 8.2.0 nestjs-pino: specifier: ^4.1.0 version: 4.6.1(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(pino-http@10.5.0)(pino@9.14.0)(rxjs@7.8.2) @@ -972,6 +978,13 @@ packages: '@nestjs/platform-express': optional: true + '@nestjs/throttler@6.5.0': + resolution: {integrity: sha512-9j0ZRfH0QE1qyrj9JjIRDz5gQLPqq9yVC2nHsrosDVAfI5HHw08/aUAWx9DZLSdQf4HDkmhTTEGLrRFHENvchQ==} + peerDependencies: + '@nestjs/common': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + '@nestjs/core': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + reflect-metadata: ^0.1.13 || ^0.2.0 + '@next/env@15.5.18': resolution: {integrity: sha512-hAV85Ckd9QR6RvH04MEKwsfLTksvFpO47j9xwtoIuvuPnlwecpSi+uZTtm8HirVbtlI2Fnz//xpcSTjFdyJk+g==} @@ -2647,6 +2660,10 @@ packages: resolution: {integrity: sha512-T2UbfbBEF32wiepXIsMlTW9+dDYC6wMh/t/vYA4tuOMKqWz/n3vr1NFSxQiyP+zk2mXsoMA/i/7qV6LKut1t1A==} engines: {node: '>= 0.4'} + helmet@8.2.0: + resolution: {integrity: sha512-DRgTIUgnWcJ62KyarxxziuqYxKGnR6Rgg19BlbucN/dpmJbl1XOit6qvoOX0ZT+HhWe5OUVhU/a1zpGyc1xA0Q==} + engines: {node: '>=18.0.0'} + help-me@5.0.0: resolution: {integrity: sha512-7xgomUX6ADmcYzFik0HzAxh/73YlKR9bmFzf51CZwR+b6YtzU2m0u49hQCqV6SvlqIqsaxovfwdvbnsw3b/zpg==} @@ -5028,6 +5045,12 @@ snapshots: optionalDependencies: '@nestjs/platform-express': 10.4.22(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.22) + '@nestjs/throttler@6.5.0(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.22)(reflect-metadata@0.2.2)': + dependencies: + '@nestjs/common': 10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/core': 10.4.22(@nestjs/common@10.4.22(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@10.4.22)(reflect-metadata@0.2.2)(rxjs@7.8.2) + reflect-metadata: 0.2.2 + '@next/env@15.5.18': {} '@next/swc-darwin-arm64@15.5.18': @@ -6824,6 +6847,8 @@ snapshots: dependencies: function-bind: 1.1.2 + helmet@8.2.0: {} + help-me@5.0.0: {} http-errors@2.0.1: