fix(#338): Add rate limiting and logging to auth catch-all route
- Apply restrictive rate limits (10 req/min) to prevent brute-force attacks - Log requests with path and client IP for monitoring and debugging - Extract client IP handling for proxy setups (X-Forwarded-For) - Add comprehensive tests for rate limiting and logging behavior Refs #338 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
|||||||
import { Controller, All, Req, Get, UseGuards, Request } from "@nestjs/common";
|
import { Controller, All, Req, Get, UseGuards, Request, Logger } from "@nestjs/common";
|
||||||
|
import { Throttle } from "@nestjs/throttler";
|
||||||
import type { AuthUser, AuthSession } from "@mosaic/shared";
|
import type { AuthUser, AuthSession } from "@mosaic/shared";
|
||||||
import { AuthService } from "./auth.service";
|
import { AuthService } from "./auth.service";
|
||||||
import { AuthGuard } from "./guards/auth.guard";
|
import { AuthGuard } from "./guards/auth.guard";
|
||||||
@@ -16,6 +17,8 @@ interface RequestWithSession {
|
|||||||
|
|
||||||
@Controller("auth")
|
@Controller("auth")
|
||||||
export class AuthController {
|
export class AuthController {
|
||||||
|
private readonly logger = new Logger(AuthController.name);
|
||||||
|
|
||||||
constructor(private readonly authService: AuthService) {}
|
constructor(private readonly authService: AuthService) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -76,10 +79,46 @@ export class AuthController {
|
|||||||
/**
|
/**
|
||||||
* Handle all other auth routes (sign-in, sign-up, sign-out, etc.)
|
* Handle all other auth routes (sign-in, sign-up, sign-out, etc.)
|
||||||
* Delegates to BetterAuth
|
* Delegates to BetterAuth
|
||||||
|
*
|
||||||
|
* Rate limit: "strict" tier (10 req/min) - More restrictive than normal routes
|
||||||
|
* to prevent brute-force attacks on auth endpoints
|
||||||
|
*
|
||||||
|
* Security note: This catch-all route bypasses standard guards that other routes have.
|
||||||
|
* Rate limiting and logging are applied to mitigate abuse (SEC-API-10).
|
||||||
*/
|
*/
|
||||||
@All("*")
|
@All("*")
|
||||||
|
@Throttle({ strict: { limit: 10, ttl: 60000 } })
|
||||||
async handleAuth(@Req() req: Request): Promise<unknown> {
|
async handleAuth(@Req() req: Request): Promise<unknown> {
|
||||||
|
// Extract client IP for logging
|
||||||
|
const clientIp = this.getClientIp(req);
|
||||||
|
const requestPath = (req as unknown as { url?: string }).url ?? "unknown";
|
||||||
|
const method = (req as unknown as { method?: string }).method ?? "UNKNOWN";
|
||||||
|
|
||||||
|
// Log auth catch-all hits for monitoring and debugging
|
||||||
|
this.logger.debug(`Auth catch-all: ${method} ${requestPath} from ${clientIp}`);
|
||||||
|
|
||||||
const auth = this.authService.getAuth();
|
const auth = this.authService.getAuth();
|
||||||
return auth.handler(req);
|
return auth.handler(req);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Extract client IP from request, handling proxies
|
||||||
|
*/
|
||||||
|
private getClientIp(req: Request): string {
|
||||||
|
const reqWithHeaders = req as unknown as {
|
||||||
|
headers?: Record<string, string | string[] | undefined>;
|
||||||
|
ip?: string;
|
||||||
|
socket?: { remoteAddress?: string };
|
||||||
|
};
|
||||||
|
|
||||||
|
// Check X-Forwarded-For header (for reverse proxy setups)
|
||||||
|
const forwardedFor = reqWithHeaders.headers?.["x-forwarded-for"];
|
||||||
|
if (forwardedFor) {
|
||||||
|
const ips = Array.isArray(forwardedFor) ? forwardedFor[0] : forwardedFor;
|
||||||
|
return ips?.split(",")[0]?.trim() ?? "unknown";
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fall back to direct IP
|
||||||
|
return reqWithHeaders.ip ?? reqWithHeaders.socket?.remoteAddress ?? "unknown";
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
206
apps/api/src/auth/auth.rate-limit.spec.ts
Normal file
206
apps/api/src/auth/auth.rate-limit.spec.ts
Normal file
@@ -0,0 +1,206 @@
|
|||||||
|
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||||
|
import { Test, TestingModule } from "@nestjs/testing";
|
||||||
|
import { INestApplication, HttpStatus, Logger } from "@nestjs/common";
|
||||||
|
import request from "supertest";
|
||||||
|
import { AuthController } from "./auth.controller";
|
||||||
|
import { AuthService } from "./auth.service";
|
||||||
|
import { ThrottlerModule } from "@nestjs/throttler";
|
||||||
|
import { APP_GUARD } from "@nestjs/core";
|
||||||
|
import { ThrottlerApiKeyGuard } from "../common/throttler";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Rate Limiting Tests for Auth Controller Catch-All Route
|
||||||
|
*
|
||||||
|
* These tests verify that rate limiting is properly enforced on the auth
|
||||||
|
* catch-all route to prevent brute-force attacks (SEC-API-10).
|
||||||
|
*
|
||||||
|
* Test Coverage:
|
||||||
|
* - Rate limit enforcement (429 status after 10 requests in 1 minute)
|
||||||
|
* - Retry-After header inclusion
|
||||||
|
* - Logging occurs for auth catch-all hits
|
||||||
|
*/
|
||||||
|
describe("AuthController - Rate Limiting", () => {
|
||||||
|
let app: INestApplication;
|
||||||
|
let loggerSpy: ReturnType<typeof vi.spyOn>;
|
||||||
|
|
||||||
|
const mockAuthService = {
|
||||||
|
getAuth: vi.fn().mockReturnValue({
|
||||||
|
handler: vi.fn().mockResolvedValue({ status: 200, body: {} }),
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
// Spy on Logger.prototype.debug to verify logging
|
||||||
|
loggerSpy = vi.spyOn(Logger.prototype, "debug").mockImplementation(() => {});
|
||||||
|
|
||||||
|
const moduleFixture: TestingModule = await Test.createTestingModule({
|
||||||
|
imports: [
|
||||||
|
ThrottlerModule.forRoot([
|
||||||
|
{
|
||||||
|
ttl: 60000, // 1 minute
|
||||||
|
limit: 10, // Match the "strict" tier limit
|
||||||
|
},
|
||||||
|
]),
|
||||||
|
],
|
||||||
|
controllers: [AuthController],
|
||||||
|
providers: [
|
||||||
|
{ provide: AuthService, useValue: mockAuthService },
|
||||||
|
{
|
||||||
|
provide: APP_GUARD,
|
||||||
|
useClass: ThrottlerApiKeyGuard,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}).compile();
|
||||||
|
|
||||||
|
app = moduleFixture.createNestApplication();
|
||||||
|
await app.init();
|
||||||
|
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await app.close();
|
||||||
|
loggerSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Auth Catch-All Route - Rate Limiting", () => {
|
||||||
|
it("should allow requests within rate limit", async () => {
|
||||||
|
// Make 3 requests (within limit of 10)
|
||||||
|
for (let i = 0; i < 3; i++) {
|
||||||
|
const response = await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Should not be rate limited
|
||||||
|
expect(response.status).not.toBe(HttpStatus.TOO_MANY_REQUESTS);
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(mockAuthService.getAuth).toHaveBeenCalledTimes(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should return 429 when rate limit is exceeded", async () => {
|
||||||
|
// Exhaust rate limit (10 requests)
|
||||||
|
for (let i = 0; i < 10; i++) {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// The 11th request should be rate limited
|
||||||
|
const response = await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(response.status).toBe(HttpStatus.TOO_MANY_REQUESTS);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should include Retry-After header in 429 response", async () => {
|
||||||
|
// Exhaust rate limit (10 requests)
|
||||||
|
for (let i = 0; i < 10; i++) {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get rate limited response
|
||||||
|
const response = await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(response.status).toBe(HttpStatus.TOO_MANY_REQUESTS);
|
||||||
|
expect(response.headers).toHaveProperty("retry-after");
|
||||||
|
expect(parseInt(response.headers["retry-after"])).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should rate limit different auth endpoints under the same limit", async () => {
|
||||||
|
// Make 5 sign-in requests
|
||||||
|
for (let i = 0; i < 5; i++) {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Make 5 sign-up requests (total now 10)
|
||||||
|
for (let i = 0; i < 5; i++) {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-up").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
name: "Test User",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// The 11th request (any auth endpoint) should be rate limited
|
||||||
|
const response = await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(response.status).toBe(HttpStatus.TOO_MANY_REQUESTS);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Auth Catch-All Route - Logging", () => {
|
||||||
|
it("should log auth catch-all hits with request details", async () => {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Verify logging was called
|
||||||
|
expect(loggerSpy).toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Find the log call that contains our expected message
|
||||||
|
const logCalls = loggerSpy.mock.calls;
|
||||||
|
const authLogCall = logCalls.find(
|
||||||
|
(call) => typeof call[0] === "string" && call[0].includes("Auth catch-all:")
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(authLogCall).toBeDefined();
|
||||||
|
expect(authLogCall?.[0]).toMatch(/Auth catch-all: POST/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should log different HTTP methods correctly", async () => {
|
||||||
|
// Test GET request
|
||||||
|
await request(app.getHttpServer()).get("/auth/callback");
|
||||||
|
|
||||||
|
const logCalls = loggerSpy.mock.calls;
|
||||||
|
const getLogCall = logCalls.find(
|
||||||
|
(call) =>
|
||||||
|
typeof call[0] === "string" &&
|
||||||
|
call[0].includes("Auth catch-all:") &&
|
||||||
|
call[0].includes("GET")
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(getLogCall).toBeDefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Per-IP Rate Limiting", () => {
|
||||||
|
it("should track rate limits per IP independently", async () => {
|
||||||
|
// Note: In a real scenario, different IPs would have different limits
|
||||||
|
// This test verifies the rate limit tracking behavior
|
||||||
|
|
||||||
|
// Exhaust rate limit with requests
|
||||||
|
for (let i = 0; i < 10; i++) {
|
||||||
|
await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should be rate limited now
|
||||||
|
const response = await request(app.getHttpServer()).post("/auth/sign-in").send({
|
||||||
|
email: "test@example.com",
|
||||||
|
password: "password",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(response.status).toBe(HttpStatus.TOO_MANY_REQUESTS);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user