fix: harden auth against brute force, add auth audit logging, fix cookie/TLS gaps

Security review findings (HIGH-1/2/3, MEDIUM):

- Rate limiting via @nestjs/throttler: global 100 req/min default, stricter
  10 req/min on login and refresh. Also closes the MEDIUM finding (public
  batch-lookup enumeration) since it now sits behind the global limit too.
  Trust the K8s ingress's single proxy hop so throttling keys off the real
  client IP, not the ingress pod's IP.
- Auth events (login success/failure, logout, token refresh) are now
  audit-logged via new AuditAction values, closing the gap where the
  project's own audit-log principle never actually covered the Auth module.
- Refresh-token cookie now defaults to Secure=true (opt out only via
  COOKIE_SECURE=false for local HTTP dev), instead of keying off NODE_ENV
  which was never set in the K8s deploy templates. Added a TLS block +
  cert-manager annotation to both the backend and frontend ingress
  templates, which previously had no HTTPS termination configured at all.
This commit is contained in:
Nick Szaunig committed 2026-08-25 10:49:25 +02:00
1 parent 1a7331b6b2
commit f1298eff08
16 files changed
+224 -15

No files matched your search

+5
View File
@@ -10,6 +10,11 @@ JWT_REFRESH_EXPIRES_IN="7d"
PORT=3001 PORT=3001
FRONTEND_ORIGIN="http://localhost:3000" FRONTEND_ORIGIN="http://localhost:3000"
# Refresh-token cookie requires HTTPS by default. Set to "false" only for
# local HTTP development - every real deployment must leave this unset (or
# "true").
COOKIE_SECURE=false
# Lokales Verzeichnis fuer hochgeladene Dokumente/Bilder (Modul 29). # Lokales Verzeichnis fuer hochgeladene Dokumente/Bilder (Modul 29).
# Bei horizontaler Skalierung (>1 Replica) muss dies auf ein geteiltes # Bei horizontaler Skalierung (>1 Replica) muss dies auf ein geteiltes
# Volume (z.B. ReadWriteMany-PVC) zeigen, siehe ARCHITECTURE.md Annahme 7. # Volume (z.B. ReadWriteMany-PVC) zeigen, siehe ARCHITECTURE.md Annahme 7.
+12
View File
@@ -16,6 +16,7 @@
"@nestjs/passport": "^11.0.5", "@nestjs/passport": "^11.0.5",
"@nestjs/platform-express": "^11.0.1", "@nestjs/platform-express": "^11.0.1",
"@nestjs/swagger": "^11.4.7", "@nestjs/swagger": "^11.4.7",
"@nestjs/throttler": "^6.5.0",
"@prisma/adapter-pg": "^7.9.1", "@prisma/adapter-pg": "^7.9.1",
"@prisma/client": "^7.9.1", "@prisma/client": "^7.9.1",
"bcryptjs": "^3.0.3", "bcryptjs": "^3.0.3",
@@ -2664,6 +2665,17 @@
} }
} }
}, },
"node_modules/@nestjs/throttler": {
"version": "6.5.0",
"resolved": "https://registry.npmjs.org/@nestjs/throttler/-/throttler-6.5.0.tgz",
"integrity": "sha512-9j0ZRfH0QE1qyrj9JjIRDz5gQLPqq9yVC2nHsrosDVAfI5HHw08/aUAWx9DZLSdQf4HDkmhTTEGLrRFHENvchQ==",
"license": "MIT",
"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"
}
},
"node_modules/@noble/ciphers": { "node_modules/@noble/ciphers": {
"version": "1.3.0", "version": "1.3.0",
"resolved": "https://registry.npmjs.org/@noble/ciphers/-/ciphers-1.3.0.tgz", "resolved": "https://registry.npmjs.org/@noble/ciphers/-/ciphers-1.3.0.tgz",
+1
View File
@@ -28,6 +28,7 @@
"@nestjs/passport": "^11.0.5", "@nestjs/passport": "^11.0.5",
"@nestjs/platform-express": "^11.0.1", "@nestjs/platform-express": "^11.0.1",
"@nestjs/swagger": "^11.4.7", "@nestjs/swagger": "^11.4.7",
"@nestjs/throttler": "^6.5.0",
"@prisma/adapter-pg": "^7.9.1", "@prisma/adapter-pg": "^7.9.1",
"@prisma/client": "^7.9.1", "@prisma/client": "^7.9.1",
"bcryptjs": "^3.0.3", "bcryptjs": "^3.0.3",
@@ -0,0 +1,12 @@
-- AlterEnum
-- This migration adds more than one value to an enum.
-- With PostgreSQL versions 11 and earlier, this is not possible
-- in a single migration. This can be worked around by creating
-- multiple migrations, each migration adding only one value to
-- the enum.
ALTER TYPE "AuditAction" ADD VALUE 'LOGIN';
ALTER TYPE "AuditAction" ADD VALUE 'LOGIN_FAILED';
ALTER TYPE "AuditAction" ADD VALUE 'LOGOUT';
ALTER TYPE "AuditAction" ADD VALUE 'TOKEN_REFRESHED';
+4
View File
@@ -79,6 +79,10 @@ enum AuditAction {
UPDATE UPDATE
DEACTIVATE DEACTIVATE
DELETE DELETE
LOGIN
LOGIN_FAILED
LOGOUT
TOKEN_REFRESHED
} }
model AuditLog { model AuditLog {
+7 -7
View File
@@ -1,13 +1,13 @@
import { import { RequestMethod, ValidationPipe, VersioningType } from '@nestjs/common';
INestApplication, import { NestExpressApplication } from '@nestjs/platform-express';
RequestMethod,
ValidationPipe,
VersioningType,
} from '@nestjs/common';
import helmet from 'helmet'; import helmet from 'helmet';
import cookieParser from 'cookie-parser'; import cookieParser from 'cookie-parser';
export function configureApp(app: INestApplication): void { export function configureApp(app: NestExpressApplication): void {
// Trust the single reverse-proxy hop (K8s ingress) so req.ip reflects the
// real client IP from X-Forwarded-For — required for per-client throttling
// to work correctly rather than rate-limiting all traffic together.
app.set('trust proxy', 1);
app.use(helmet()); app.use(helmet());
app.use(cookieParser()); app.use(cookieParser());
+3
View File
@@ -1,6 +1,7 @@
import { Module } from '@nestjs/common'; import { Module } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config'; import { ConfigModule } from '@nestjs/config';
import { APP_GUARD } from '@nestjs/core'; import { APP_GUARD } from '@nestjs/core';
import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
import { PrismaModule } from './prisma/prisma.module'; import { PrismaModule } from './prisma/prisma.module';
import { HealthModule } from './health/health.module'; import { HealthModule } from './health/health.module';
import { AuditModule } from './audit/audit.module'; import { AuditModule } from './audit/audit.module';
@@ -41,6 +42,7 @@ import { RolesGuard } from './auth/guards/roles.guard';
@Module({ @Module({
imports: [ imports: [
ConfigModule.forRoot({ isGlobal: true }), ConfigModule.forRoot({ isGlobal: true }),
ThrottlerModule.forRoot([{ name: 'default', ttl: 60000, limit: 100 }]),
PrismaModule, PrismaModule,
AuditModule, AuditModule,
HealthModule, HealthModule,
@@ -80,6 +82,7 @@ import { RolesGuard } from './auth/guards/roles.guard';
providers: [ providers: [
{ provide: APP_GUARD, useClass: JwtAuthGuard }, { provide: APP_GUARD, useClass: JwtAuthGuard },
{ provide: APP_GUARD, useClass: RolesGuard }, { provide: APP_GUARD, useClass: RolesGuard },
{ provide: APP_GUARD, useClass: ThrottlerGuard },
], ],
}) })
export class AppModule {} export class AppModule {}
+10 -1
View File
@@ -9,6 +9,7 @@ import {
UnauthorizedException, UnauthorizedException,
} from '@nestjs/common'; } from '@nestjs/common';
import { ApiTags } from '@nestjs/swagger'; import { ApiTags } from '@nestjs/swagger';
import { Throttle } from '@nestjs/throttler';
import type { Request, Response } from 'express'; import type { Request, Response } from 'express';
import { Public } from './decorators/public.decorator'; import { Public } from './decorators/public.decorator';
import { LoginDto } from './dto/login.dto'; import { LoginDto } from './dto/login.dto';
@@ -18,6 +19,9 @@ import { parseDurationToMs } from './util/duration';
const REFRESH_TOKEN_COOKIE = 'refreshToken'; const REFRESH_TOKEN_COOKIE = 'refreshToken';
const REFRESH_TOKEN_COOKIE_PATH = '/api/v1/auth'; const REFRESH_TOKEN_COOKIE_PATH = '/api/v1/auth';
// Stricter than the global default: bounds brute-force/credential-stuffing
// attempts against login and refresh-token guessing specifically.
const AUTH_THROTTLE = { default: { limit: 10, ttl: 60000 } };
@ApiTags('auth') @ApiTags('auth')
@Controller('auth') @Controller('auth')
@@ -27,7 +31,10 @@ export class AuthController {
private setRefreshTokenCookie(res: Response, refreshToken: string): void { private setRefreshTokenCookie(res: Response, refreshToken: string): void {
res.cookie(REFRESH_TOKEN_COOKIE, refreshToken, { res.cookie(REFRESH_TOKEN_COOKIE, refreshToken, {
httpOnly: true, httpOnly: true,
secure: process.env.NODE_ENV === 'production', // Explicit opt-out for local HTTP dev only; every real deployment must
// set COOKIE_SECURE=true so the refresh-token cookie is never sent
// over plaintext HTTP.
secure: process.env.COOKIE_SECURE !== 'false',
sameSite: 'lax', sameSite: 'lax',
path: REFRESH_TOKEN_COOKIE_PATH, path: REFRESH_TOKEN_COOKIE_PATH,
maxAge: parseDurationToMs(process.env.JWT_REFRESH_EXPIRES_IN ?? '7d'), maxAge: parseDurationToMs(process.env.JWT_REFRESH_EXPIRES_IN ?? '7d'),
@@ -49,6 +56,7 @@ export class AuthController {
} }
@Public() @Public()
@Throttle(AUTH_THROTTLE)
@Post('login') @Post('login')
@HttpCode(HttpStatus.OK) @HttpCode(HttpStatus.OK)
async login( async login(
@@ -64,6 +72,7 @@ export class AuthController {
} }
@Public() @Public()
@Throttle(AUTH_THROTTLE)
@Post('refresh') @Post('refresh')
@HttpCode(HttpStatus.OK) @HttpCode(HttpStatus.OK)
async refresh( async refresh(
+62 -3
View File
@@ -3,6 +3,7 @@ import * as bcrypt from 'bcryptjs';
import { AuthService } from './auth.service'; import { AuthService } from './auth.service';
import { PrismaService } from '../prisma/prisma.service'; import { PrismaService } from '../prisma/prisma.service';
import { UsersService } from '../users/users.service'; import { UsersService } from '../users/users.service';
import { AuditLogService } from '../audit/audit-log.service';
describe('AuthService', () => { describe('AuthService', () => {
let authService: AuthService; let authService: AuthService;
@@ -16,6 +17,7 @@ describe('AuthService', () => {
}; };
}; };
let jwtService: { sign: jest.Mock }; let jwtService: { sign: jest.Mock };
let auditLog: { record: jest.Mock };
const activeUser = { const activeUser = {
id: 'user-1', id: 'user-1',
@@ -37,11 +39,13 @@ describe('AuthService', () => {
}, },
}; };
jwtService = { sign: jest.fn().mockReturnValue('signed.jwt.token') }; jwtService = { sign: jest.fn().mockReturnValue('signed.jwt.token') };
auditLog = { record: jest.fn().mockResolvedValue(undefined) };
authService = new AuthService( authService = new AuthService(
usersService as unknown as UsersService, usersService as unknown as UsersService,
prisma as unknown as PrismaService, prisma as unknown as PrismaService,
jwtService as never, jwtService as never,
auditLog as unknown as AuditLogService,
); );
}); });
@@ -85,6 +89,36 @@ describe('AuthService', () => {
[{ data: { userId: string } }], [{ data: { userId: string } }],
]; ];
expect(createArg.data.userId).toBe(activeUser.id); expect(createArg.data.userId).toBe(activeUser.id);
expect(auditLog.record).toHaveBeenCalledWith(
expect.objectContaining({ action: 'LOGIN', userId: activeUser.id }),
);
});
it('records a LOGIN_FAILED audit entry on wrong password', async () => {
usersService.findByEmail.mockResolvedValue(activeUser);
await expect(
authService.login(activeUser.email, 'wrong-password'),
).rejects.toBeInstanceOf(UnauthorizedException);
expect(auditLog.record).toHaveBeenCalledWith(
expect.objectContaining({
action: 'LOGIN_FAILED',
userId: activeUser.id,
}),
);
});
it('records a LOGIN_FAILED audit entry keyed by email when the user is unknown', async () => {
usersService.findByEmail.mockResolvedValue(null);
await expect(
authService.login('[email protected]', 'x'),
).rejects.toBeInstanceOf(UnauthorizedException);
expect(auditLog.record).toHaveBeenCalledWith(
expect.objectContaining({
action: 'LOGIN_FAILED',
entityId: '[email protected]',
userId: undefined,
}),
);
}); });
}); });
@@ -137,14 +171,39 @@ describe('AuthService', () => {
expect(prisma.refreshToken.update).toHaveBeenCalledWith( expect(prisma.refreshToken.update).toHaveBeenCalledWith(
expect.objectContaining({ where: { id: 'rt-1' } }), expect.objectContaining({ where: { id: 'rt-1' } }),
); );
expect(auditLog.record).toHaveBeenCalledWith(
expect.objectContaining({
action: 'TOKEN_REFRESHED',
userId: activeUser.id,
}),
);
}); });
}); });
describe('logout', () => { describe('logout', () => {
it('revokes the matching refresh token', async () => { it('revokes the matching refresh token and records a LOGOUT audit entry', async () => {
prisma.refreshToken.updateMany.mockResolvedValue({ count: 1 }); prisma.refreshToken.findUnique.mockResolvedValue({
id: 'rt-1',
userId: activeUser.id,
revokedAt: null,
});
prisma.refreshToken.update.mockResolvedValue({});
await authService.logout('some-token'); await authService.logout('some-token');
expect(prisma.refreshToken.updateMany).toHaveBeenCalled();
expect(prisma.refreshToken.update).toHaveBeenCalledWith(
expect.objectContaining({ where: { id: 'rt-1' } }),
);
expect(auditLog.record).toHaveBeenCalledWith(
expect.objectContaining({ action: 'LOGOUT', userId: activeUser.id }),
);
});
it('does nothing when the token is unknown or already revoked', async () => {
prisma.refreshToken.findUnique.mockResolvedValue(null);
await authService.logout('unknown-token');
expect(prisma.refreshToken.update).not.toHaveBeenCalled();
expect(auditLog.record).not.toHaveBeenCalled();
}); });
}); });
}); });
+40 -2
View File
@@ -4,6 +4,7 @@ import * as bcrypt from 'bcryptjs';
import * as crypto from 'node:crypto'; import * as crypto from 'node:crypto';
import { PrismaService } from '../prisma/prisma.service'; import { PrismaService } from '../prisma/prisma.service';
import { UsersService } from '../users/users.service'; import { UsersService } from '../users/users.service';
import { AuditLogService } from '../audit/audit-log.service';
import { AuthenticatedUser } from './types/authenticated-user'; import { AuthenticatedUser } from './types/authenticated-user';
import { parseDurationToMs } from './util/duration'; import { parseDurationToMs } from './util/duration';
@@ -20,6 +21,7 @@ export class AuthService {
private readonly usersService: UsersService, private readonly usersService: UsersService,
private readonly prisma: PrismaService, private readonly prisma: PrismaService,
private readonly jwtService: JwtService, private readonly jwtService: JwtService,
private readonly auditLog: AuditLogService,
) {} ) {}
private hashRefreshToken(token: string): string { private hashRefreshToken(token: string): string {
@@ -54,10 +56,22 @@ export class AuthService {
async login(email: string, password: string): Promise<AuthTokens> { async login(email: string, password: string): Promise<AuthTokens> {
const user = await this.usersService.findByEmail(email); const user = await this.usersService.findByEmail(email);
if (!user || !user.isActive) { if (!user || !user.isActive) {
await this.auditLog.record({
entityType: 'AuthSession',
entityId: user?.id ?? email,
action: 'LOGIN_FAILED',
userId: user?.id,
});
throw new UnauthorizedException('Invalid credentials'); throw new UnauthorizedException('Invalid credentials');
} }
const passwordMatches = await bcrypt.compare(password, user.passwordHash); const passwordMatches = await bcrypt.compare(password, user.passwordHash);
if (!passwordMatches) { if (!passwordMatches) {
await this.auditLog.record({
entityType: 'AuthSession',
entityId: user.id,
action: 'LOGIN_FAILED',
userId: user.id,
});
throw new UnauthorizedException('Invalid credentials'); throw new UnauthorizedException('Invalid credentials');
} }
const authenticatedUser: AuthenticatedUser = { const authenticatedUser: AuthenticatedUser = {
@@ -67,6 +81,12 @@ export class AuthService {
}; };
const accessToken = this.signAccessToken(authenticatedUser); const accessToken = this.signAccessToken(authenticatedUser);
const refreshToken = await this.issueRefreshToken(user.id); const refreshToken = await this.issueRefreshToken(user.id);
await this.auditLog.record({
entityType: 'AuthSession',
entityId: user.id,
action: 'LOGIN',
userId: user.id,
});
return { accessToken, refreshToken }; return { accessToken, refreshToken };
} }
@@ -93,14 +113,32 @@ export class AuthService {
}; };
const accessToken = this.signAccessToken(authenticatedUser); const accessToken = this.signAccessToken(authenticatedUser);
const newRefreshToken = await this.issueRefreshToken(user.id); const newRefreshToken = await this.issueRefreshToken(user.id);
await this.auditLog.record({
entityType: 'AuthSession',
entityId: user.id,
action: 'TOKEN_REFRESHED',
userId: user.id,
});
return { accessToken, refreshToken: newRefreshToken }; return { accessToken, refreshToken: newRefreshToken };
} }
async logout(refreshToken: string): Promise<void> { async logout(refreshToken: string): Promise<void> {
const tokenHash = this.hashRefreshToken(refreshToken); const tokenHash = this.hashRefreshToken(refreshToken);
await this.prisma.refreshToken.updateMany({ const stored = await this.prisma.refreshToken.findUnique({
where: { tokenHash, revokedAt: null }, where: { tokenHash },
});
if (!stored || stored.revokedAt) {
return;
}
await this.prisma.refreshToken.update({
where: { id: stored.id },
data: { revokedAt: new Date() }, data: { revokedAt: new Date() },
}); });
await this.auditLog.record({
entityType: 'AuthSession',
entityId: stored.userId,
action: 'LOGOUT',
userId: stored.userId,
});
} }
} }
+2 -1
View File
@@ -1,10 +1,11 @@
import { NestFactory } from '@nestjs/core'; import { NestFactory } from '@nestjs/core';
import { NestExpressApplication } from '@nestjs/platform-express';
import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger';
import { AppModule } from './app.module'; import { AppModule } from './app.module';
import { configureApp } from './app.config'; import { configureApp } from './app.config';
async function bootstrap() { async function bootstrap() {
const app = await NestFactory.create(AppModule); const app = await NestFactory.create<NestExpressApplication>(AppModule);
configureApp(app); configureApp(app);
@@ -0,0 +1,48 @@
import { INestApplication } from '@nestjs/common';
import * as bcrypt from 'bcryptjs';
import request from 'supertest';
import { App } from 'supertest/types';
import { PrismaService } from '../src/prisma/prisma.service';
import { createTestApp } from './utils/create-test-app';
const LOGIN_THROTTLE_LIMIT = 10;
describe('Auth login throttling (e2e)', () => {
let app: INestApplication<App>;
let prisma: PrismaService;
const email = `auth-throttle-e2e-${Date.now()}@example.com`;
const password = 'SuperSecret123!';
let userId: string;
beforeAll(async () => {
app = await createTestApp();
prisma = app.get(PrismaService);
const passwordHash = await bcrypt.hash(password, 12);
const user = await prisma.user.create({
data: { email, passwordHash, name: 'Throttle E2E User', role: 'STAFF' },
});
userId = user.id;
});
afterAll(async () => {
await prisma.refreshToken.deleteMany({ where: { userId } });
await prisma.auditLog.deleteMany({ where: { userId } });
await prisma.user.delete({ where: { id: userId } });
await app.close();
});
it('rejects login attempts beyond the per-minute limit with 429', async () => {
for (let attempt = 0; attempt < LOGIN_THROTTLE_LIMIT; attempt += 1) {
await request(app.getHttpServer())
.post('/api/v1/auth/login')
.send({ email, password: 'wrong-password' })
.expect(401);
}
await request(app.getHttpServer())
.post('/api/v1/auth/login')
.send({ email, password: 'wrong-password' })
.expect(429);
});
});
@@ -1,5 +1,6 @@
import { Test, TestingModule } from '@nestjs/testing'; import { Test, TestingModule } from '@nestjs/testing';
import { INestApplication } from '@nestjs/common'; import { INestApplication } from '@nestjs/common';
import { NestExpressApplication } from '@nestjs/platform-express';
import { App } from 'supertest/types'; import { App } from 'supertest/types';
import { AppModule } from '../../src/app.module'; import { AppModule } from '../../src/app.module';
import { configureApp } from '../../src/app.config'; import { configureApp } from '../../src/app.config';
@@ -9,7 +10,7 @@ export async function createTestApp(): Promise<INestApplication<App>> {
imports: [AppModule], imports: [AppModule],
}).compile(); }).compile();
const app = moduleFixture.createNestApplication<INestApplication<App>>(); const app = moduleFixture.createNestApplication<NestExpressApplication>();
configureApp(app); configureApp(app);
await app.init(); await app.init();
return app; return app;
@@ -8,3 +8,4 @@ data:
JWT_REFRESH_EXPIRES_IN: "7d" JWT_REFRESH_EXPIRES_IN: "7d"
FRONTEND_ORIGIN: "https://apicore.example.com" FRONTEND_ORIGIN: "https://apicore.example.com"
DOCUMENTS_STORAGE_PATH: "/data/documents" DOCUMENTS_STORAGE_PATH: "/data/documents"
COOKIE_SECURE: "true"
+7
View File
@@ -4,8 +4,15 @@ metadata:
name: apicore-backend name: apicore-backend
annotations: annotations:
nginx.ingress.kubernetes.io/rewrite-target: /$2 nginx.ingress.kubernetes.io/rewrite-target: /$2
# Requires cert-manager with a ClusterIssuer named "letsencrypt-prod"
# (or swap for whatever issuer/cert mechanism the cluster already uses).
cert-manager.io/cluster-issuer: letsencrypt-prod
spec: spec:
ingressClassName: nginx ingressClassName: nginx
tls:
- hosts:
- api.apicore.example.com
secretName: apicore-backend-tls
rules: rules:
- host: api.apicore.example.com - host: api.apicore.example.com
http: http:
+8
View File
@@ -2,8 +2,16 @@ apiVersion: networking.k8s.io/v1
kind: Ingress kind: Ingress
metadata: metadata:
name: apicore-frontend name: apicore-frontend
annotations:
# Requires cert-manager with a ClusterIssuer named "letsencrypt-prod"
# (or swap for whatever issuer/cert mechanism the cluster already uses).
cert-manager.io/cluster-issuer: letsencrypt-prod
spec: spec:
ingressClassName: nginx ingressClassName: nginx
tls:
- hosts:
- apicore.example.com
secretName: apicore-frontend-tls
rules: rules:
- host: apicore.example.com - host: apicore.example.com
http: http: