Security and Code Quality Remediation (M6-Fixes) #343
@@ -12,6 +12,12 @@ INSTANCE_URL=http://localhost:3000
|
||||
# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))"
|
||||
ENCRYPTION_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
|
||||
|
||||
# CSRF Protection (Required in production)
|
||||
# Secret key for HMAC binding CSRF tokens to user sessions
|
||||
# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))"
|
||||
# In development, a random key is generated if not set
|
||||
CSRF_SECRET=fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210
|
||||
|
||||
# OpenTelemetry Configuration
|
||||
# Enable/disable OpenTelemetry tracing (default: true)
|
||||
OTEL_ENABLED=true
|
||||
|
||||
@@ -4,6 +4,7 @@ import { ThrottlerModule } from "@nestjs/throttler";
|
||||
import { BullModule } from "@nestjs/bullmq";
|
||||
import { ThrottlerValkeyStorageService, ThrottlerApiKeyGuard } from "./common/throttler";
|
||||
import { CsrfGuard } from "./common/guards/csrf.guard";
|
||||
import { CsrfService } from "./common/services/csrf.service";
|
||||
import { AppController } from "./app.controller";
|
||||
import { AppService } from "./app.service";
|
||||
import { CsrfController } from "./common/controllers/csrf.controller";
|
||||
@@ -94,6 +95,7 @@ import { FederationModule } from "./federation/federation.module";
|
||||
controllers: [AppController, CsrfController],
|
||||
providers: [
|
||||
AppService,
|
||||
CsrfService,
|
||||
{
|
||||
provide: APP_INTERCEPTOR,
|
||||
useClass: TelemetryInterceptor,
|
||||
|
||||
@@ -1,37 +1,69 @@
|
||||
/**
|
||||
* CSRF Controller Tests
|
||||
*
|
||||
* Tests CSRF token generation endpoint.
|
||||
* Tests CSRF token generation endpoint with session binding.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { Request, Response } from "express";
|
||||
import { CsrfController } from "./csrf.controller";
|
||||
import { Response } from "express";
|
||||
import { CsrfService } from "../services/csrf.service";
|
||||
import type { AuthenticatedUser } from "../types/user.types";
|
||||
|
||||
interface AuthenticatedRequest extends Request {
|
||||
user?: AuthenticatedUser;
|
||||
}
|
||||
|
||||
describe("CsrfController", () => {
|
||||
let controller: CsrfController;
|
||||
let csrfService: CsrfService;
|
||||
const originalEnv = process.env;
|
||||
|
||||
controller = new CsrfController();
|
||||
beforeEach(() => {
|
||||
process.env = { ...originalEnv };
|
||||
process.env.CSRF_SECRET = "test-secret-0123456789abcdef0123456789abcdef";
|
||||
csrfService = new CsrfService();
|
||||
csrfService.onModuleInit();
|
||||
controller = new CsrfController(csrfService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = originalEnv;
|
||||
});
|
||||
|
||||
const createMockRequest = (userId?: string): AuthenticatedRequest => {
|
||||
return {
|
||||
user: userId ? { id: userId, email: "test@example.com", name: "Test User" } : undefined,
|
||||
} as AuthenticatedRequest;
|
||||
};
|
||||
|
||||
const createMockResponse = (): Response => {
|
||||
return {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
};
|
||||
|
||||
describe("getCsrfToken", () => {
|
||||
it("should generate and return a CSRF token", () => {
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
it("should generate and return a CSRF token with session binding", () => {
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
const result = controller.getCsrfToken(mockResponse);
|
||||
const result = controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(result).toHaveProperty("token");
|
||||
expect(typeof result.token).toBe("string");
|
||||
expect(result.token.length).toBe(64); // 32 bytes as hex = 64 characters
|
||||
// Token format: random:hmac (64 hex chars : 64 hex chars)
|
||||
expect(result.token).toContain(":");
|
||||
const parts = result.token.split(":");
|
||||
expect(parts[0]).toHaveLength(64);
|
||||
expect(parts[1]).toHaveLength(64);
|
||||
});
|
||||
|
||||
it("should set CSRF token in httpOnly cookie", () => {
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
const result = controller.getCsrfToken(mockResponse);
|
||||
const result = controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(mockResponse.cookie).toHaveBeenCalledWith(
|
||||
"csrf-token",
|
||||
@@ -44,14 +76,12 @@ describe("CsrfController", () => {
|
||||
});
|
||||
|
||||
it("should set secure flag in production", () => {
|
||||
const originalEnv = process.env.NODE_ENV;
|
||||
process.env.NODE_ENV = "production";
|
||||
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
controller.getCsrfToken(mockResponse);
|
||||
controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(mockResponse.cookie).toHaveBeenCalledWith(
|
||||
"csrf-token",
|
||||
@@ -60,19 +90,15 @@ describe("CsrfController", () => {
|
||||
secure: true,
|
||||
})
|
||||
);
|
||||
|
||||
process.env.NODE_ENV = originalEnv;
|
||||
});
|
||||
|
||||
it("should not set secure flag in development", () => {
|
||||
const originalEnv = process.env.NODE_ENV;
|
||||
process.env.NODE_ENV = "development";
|
||||
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
controller.getCsrfToken(mockResponse);
|
||||
controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(mockResponse.cookie).toHaveBeenCalledWith(
|
||||
"csrf-token",
|
||||
@@ -81,27 +107,23 @@ describe("CsrfController", () => {
|
||||
secure: false,
|
||||
})
|
||||
);
|
||||
|
||||
process.env.NODE_ENV = originalEnv;
|
||||
});
|
||||
|
||||
it("should generate unique tokens on each call", () => {
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
const result1 = controller.getCsrfToken(mockResponse);
|
||||
const result2 = controller.getCsrfToken(mockResponse);
|
||||
const result1 = controller.getCsrfToken(mockRequest, mockResponse);
|
||||
const result2 = controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(result1.token).not.toBe(result2.token);
|
||||
});
|
||||
|
||||
it("should set cookie with 24 hour expiry", () => {
|
||||
const mockResponse = {
|
||||
cookie: vi.fn(),
|
||||
} as unknown as Response;
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
controller.getCsrfToken(mockResponse);
|
||||
controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
expect(mockResponse.cookie).toHaveBeenCalledWith(
|
||||
"csrf-token",
|
||||
@@ -111,5 +133,45 @@ describe("CsrfController", () => {
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("should throw error when user is not authenticated", () => {
|
||||
const mockRequest = createMockRequest(); // No user ID
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
expect(() => controller.getCsrfToken(mockRequest, mockResponse)).toThrow(
|
||||
"User ID not available after authentication"
|
||||
);
|
||||
});
|
||||
|
||||
it("should generate token bound to specific user session", () => {
|
||||
const mockRequest = createMockRequest("user-123");
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
const result = controller.getCsrfToken(mockRequest, mockResponse);
|
||||
|
||||
// Token should be valid for user-123
|
||||
expect(csrfService.validateToken(result.token, "user-123")).toBe(true);
|
||||
|
||||
// Token should be invalid for different user
|
||||
expect(csrfService.validateToken(result.token, "user-456")).toBe(false);
|
||||
});
|
||||
|
||||
it("should generate different tokens for different users", () => {
|
||||
const mockResponse = createMockResponse();
|
||||
|
||||
const request1 = createMockRequest("user-A");
|
||||
const request2 = createMockRequest("user-B");
|
||||
|
||||
const result1 = controller.getCsrfToken(request1, mockResponse);
|
||||
const result2 = controller.getCsrfToken(request2, mockResponse);
|
||||
|
||||
expect(result1.token).not.toBe(result2.token);
|
||||
|
||||
// Each token only valid for its user
|
||||
expect(csrfService.validateToken(result1.token, "user-A")).toBe(true);
|
||||
expect(csrfService.validateToken(result1.token, "user-B")).toBe(false);
|
||||
expect(csrfService.validateToken(result2.token, "user-B")).toBe(true);
|
||||
expect(csrfService.validateToken(result2.token, "user-A")).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,24 +2,46 @@
|
||||
* CSRF Controller
|
||||
*
|
||||
* Provides CSRF token generation endpoint for client applications.
|
||||
* Tokens are cryptographically bound to the user session via HMAC.
|
||||
*/
|
||||
|
||||
import { Controller, Get, Res } from "@nestjs/common";
|
||||
import { Response } from "express";
|
||||
import * as crypto from "crypto";
|
||||
import { Controller, Get, Res, Req, UseGuards } from "@nestjs/common";
|
||||
import { Response, Request } from "express";
|
||||
import { SkipCsrf } from "../decorators/skip-csrf.decorator";
|
||||
import { CsrfService } from "../services/csrf.service";
|
||||
import { AuthGuard } from "../../auth/guards/auth.guard";
|
||||
import type { AuthenticatedUser } from "../types/user.types";
|
||||
|
||||
interface AuthenticatedRequest extends Request {
|
||||
user?: AuthenticatedUser;
|
||||
}
|
||||
|
||||
@Controller("api/v1/csrf")
|
||||
export class CsrfController {
|
||||
constructor(private readonly csrfService: CsrfService) {}
|
||||
|
||||
/**
|
||||
* Generate and set CSRF token
|
||||
* Generate and set CSRF token bound to user session
|
||||
* Requires authentication to bind token to session
|
||||
* Returns token to client and sets it in httpOnly cookie
|
||||
*/
|
||||
@Get("token")
|
||||
@UseGuards(AuthGuard)
|
||||
@SkipCsrf() // This endpoint itself doesn't need CSRF protection
|
||||
getCsrfToken(@Res({ passthrough: true }) response: Response): { token: string } {
|
||||
// Generate cryptographically secure random token
|
||||
const token = crypto.randomBytes(32).toString("hex");
|
||||
getCsrfToken(
|
||||
@Req() request: AuthenticatedRequest,
|
||||
@Res({ passthrough: true }) response: Response
|
||||
): { token: string } {
|
||||
// Get user ID from authenticated request
|
||||
const userId = request.user?.id;
|
||||
|
||||
if (!userId) {
|
||||
// This should not happen if AuthGuard is working correctly
|
||||
throw new Error("User ID not available after authentication");
|
||||
}
|
||||
|
||||
// Generate session-bound CSRF token
|
||||
const token = this.csrfService.generateToken(userId);
|
||||
|
||||
// Set token in httpOnly cookie
|
||||
response.cookie("csrf-token", token, {
|
||||
|
||||
@@ -1,34 +1,47 @@
|
||||
/**
|
||||
* CSRF Guard Tests
|
||||
*
|
||||
* Tests CSRF protection using double-submit cookie pattern.
|
||||
* Tests CSRF protection using double-submit cookie pattern with session binding.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { ExecutionContext, ForbiddenException } from "@nestjs/common";
|
||||
import { Reflector } from "@nestjs/core";
|
||||
import { CsrfGuard } from "./csrf.guard";
|
||||
import { CsrfService } from "../services/csrf.service";
|
||||
|
||||
describe("CsrfGuard", () => {
|
||||
let guard: CsrfGuard;
|
||||
let reflector: Reflector;
|
||||
let csrfService: CsrfService;
|
||||
const originalEnv = process.env;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env = { ...originalEnv };
|
||||
process.env.CSRF_SECRET = "test-secret-0123456789abcdef0123456789abcdef";
|
||||
reflector = new Reflector();
|
||||
guard = new CsrfGuard(reflector);
|
||||
csrfService = new CsrfService();
|
||||
csrfService.onModuleInit();
|
||||
guard = new CsrfGuard(reflector, csrfService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = originalEnv;
|
||||
});
|
||||
|
||||
const createContext = (
|
||||
method: string,
|
||||
cookies: Record<string, string> = {},
|
||||
headers: Record<string, string> = {},
|
||||
skipCsrf = false
|
||||
skipCsrf = false,
|
||||
userId?: string
|
||||
): ExecutionContext => {
|
||||
const request = {
|
||||
method,
|
||||
cookies,
|
||||
headers,
|
||||
path: "/api/test",
|
||||
user: userId ? { id: userId, email: "test@example.com", name: "Test" } : undefined,
|
||||
};
|
||||
|
||||
return {
|
||||
@@ -41,6 +54,13 @@ describe("CsrfGuard", () => {
|
||||
} as unknown as ExecutionContext;
|
||||
};
|
||||
|
||||
/**
|
||||
* Helper to generate a valid session-bound token
|
||||
*/
|
||||
const generateValidToken = (userId: string): string => {
|
||||
return csrfService.generateToken(userId);
|
||||
};
|
||||
|
||||
describe("Safe HTTP methods", () => {
|
||||
it("should allow GET requests without CSRF token", () => {
|
||||
const context = createContext("GET");
|
||||
@@ -68,73 +88,233 @@ describe("CsrfGuard", () => {
|
||||
|
||||
describe("State-changing methods requiring CSRF", () => {
|
||||
it("should reject POST without CSRF token", () => {
|
||||
const context = createContext("POST");
|
||||
const context = createContext("POST", {}, {}, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token missing");
|
||||
});
|
||||
|
||||
it("should reject PUT without CSRF token", () => {
|
||||
const context = createContext("PUT");
|
||||
const context = createContext("PUT", {}, {}, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
});
|
||||
|
||||
it("should reject PATCH without CSRF token", () => {
|
||||
const context = createContext("PATCH");
|
||||
const context = createContext("PATCH", {}, {}, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
});
|
||||
|
||||
it("should reject DELETE without CSRF token", () => {
|
||||
const context = createContext("DELETE");
|
||||
const context = createContext("DELETE", {}, {}, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
});
|
||||
|
||||
it("should reject when only cookie token is present", () => {
|
||||
const context = createContext("POST", { "csrf-token": "abc123" });
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext("POST", { "csrf-token": token }, {}, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token missing");
|
||||
});
|
||||
|
||||
it("should reject when only header token is present", () => {
|
||||
const context = createContext("POST", {}, { "x-csrf-token": "abc123" });
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext("POST", {}, { "x-csrf-token": token }, false, "user-123");
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token missing");
|
||||
});
|
||||
|
||||
it("should reject when tokens do not match", () => {
|
||||
const token1 = generateValidToken("user-123");
|
||||
const token2 = generateValidToken("user-123");
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": "abc123" },
|
||||
{ "x-csrf-token": "xyz789" }
|
||||
{ "csrf-token": token1 },
|
||||
{ "x-csrf-token": token2 },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token mismatch");
|
||||
});
|
||||
|
||||
it("should allow when tokens match", () => {
|
||||
it("should allow when tokens match and session is valid", () => {
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": "abc123" },
|
||||
{ "x-csrf-token": "abc123" }
|
||||
{ "csrf-token": token },
|
||||
{ "x-csrf-token": token },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
expect(guard.canActivate(context)).toBe(true);
|
||||
});
|
||||
|
||||
it("should allow PATCH when tokens match", () => {
|
||||
it("should allow PATCH when tokens match and session is valid", () => {
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext(
|
||||
"PATCH",
|
||||
{ "csrf-token": "token123" },
|
||||
{ "x-csrf-token": "token123" }
|
||||
{ "csrf-token": token },
|
||||
{ "x-csrf-token": token },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
expect(guard.canActivate(context)).toBe(true);
|
||||
});
|
||||
|
||||
it("should allow DELETE when tokens match", () => {
|
||||
it("should allow DELETE when tokens match and session is valid", () => {
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext(
|
||||
"DELETE",
|
||||
{ "csrf-token": "delete-token" },
|
||||
{ "x-csrf-token": "delete-token" }
|
||||
{ "csrf-token": token },
|
||||
{ "x-csrf-token": token },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
expect(guard.canActivate(context)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Session binding validation", () => {
|
||||
it("should reject when user is not authenticated", () => {
|
||||
const token = generateValidToken("user-123");
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": token },
|
||||
{ "x-csrf-token": token },
|
||||
false
|
||||
// No userId - unauthenticated
|
||||
);
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF validation requires authentication");
|
||||
});
|
||||
|
||||
it("should reject token from different session", () => {
|
||||
// Token generated for user-A
|
||||
const tokenForUserA = generateValidToken("user-A");
|
||||
|
||||
// But request is from user-B
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenForUserA },
|
||||
{ "x-csrf-token": tokenForUserA },
|
||||
false,
|
||||
"user-B" // Different user
|
||||
);
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token not bound to session");
|
||||
});
|
||||
|
||||
it("should reject token with invalid HMAC", () => {
|
||||
// Create a token with tampered HMAC
|
||||
const validToken = generateValidToken("user-123");
|
||||
const parts = validToken.split(":");
|
||||
const tamperedToken = `${parts[0]}:0000000000000000000000000000000000000000000000000000000000000000`;
|
||||
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tamperedToken },
|
||||
{ "x-csrf-token": tamperedToken },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token not bound to session");
|
||||
});
|
||||
|
||||
it("should reject token with invalid format", () => {
|
||||
const invalidToken = "not-a-valid-token";
|
||||
|
||||
const context = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": invalidToken },
|
||||
{ "x-csrf-token": invalidToken },
|
||||
false,
|
||||
"user-123"
|
||||
);
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(ForbiddenException);
|
||||
expect(() => guard.canActivate(context)).toThrow("CSRF token not bound to session");
|
||||
});
|
||||
|
||||
it("should not allow token reuse across sessions", () => {
|
||||
// Generate token for user-A
|
||||
const tokenA = generateValidToken("user-A");
|
||||
|
||||
// Valid for user-A
|
||||
const contextA = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenA },
|
||||
{ "x-csrf-token": tokenA },
|
||||
false,
|
||||
"user-A"
|
||||
);
|
||||
expect(guard.canActivate(contextA)).toBe(true);
|
||||
|
||||
// Invalid for user-B
|
||||
const contextB = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenA },
|
||||
{ "x-csrf-token": tokenA },
|
||||
false,
|
||||
"user-B"
|
||||
);
|
||||
expect(() => guard.canActivate(contextB)).toThrow("CSRF token not bound to session");
|
||||
|
||||
// Invalid for user-C
|
||||
const contextC = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenA },
|
||||
{ "x-csrf-token": tokenA },
|
||||
false,
|
||||
"user-C"
|
||||
);
|
||||
expect(() => guard.canActivate(contextC)).toThrow("CSRF token not bound to session");
|
||||
});
|
||||
|
||||
it("should allow each user to use only their own token", () => {
|
||||
const tokenA = generateValidToken("user-A");
|
||||
const tokenB = generateValidToken("user-B");
|
||||
|
||||
// User A with token A - valid
|
||||
const contextAA = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenA },
|
||||
{ "x-csrf-token": tokenA },
|
||||
false,
|
||||
"user-A"
|
||||
);
|
||||
expect(guard.canActivate(contextAA)).toBe(true);
|
||||
|
||||
// User B with token B - valid
|
||||
const contextBB = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenB },
|
||||
{ "x-csrf-token": tokenB },
|
||||
false,
|
||||
"user-B"
|
||||
);
|
||||
expect(guard.canActivate(contextBB)).toBe(true);
|
||||
|
||||
// User A with token B - invalid (cross-session)
|
||||
const contextAB = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenB },
|
||||
{ "x-csrf-token": tokenB },
|
||||
false,
|
||||
"user-A"
|
||||
);
|
||||
expect(() => guard.canActivate(contextAB)).toThrow("CSRF token not bound to session");
|
||||
|
||||
// User B with token A - invalid (cross-session)
|
||||
const contextBA = createContext(
|
||||
"POST",
|
||||
{ "csrf-token": tokenA },
|
||||
{ "x-csrf-token": tokenA },
|
||||
false,
|
||||
"user-B"
|
||||
);
|
||||
expect(() => guard.canActivate(contextBA)).toThrow("CSRF token not bound to session");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
/**
|
||||
* CSRF Guard
|
||||
*
|
||||
* Implements CSRF protection using double-submit cookie pattern.
|
||||
* Validates that CSRF token in cookie matches token in header.
|
||||
* Implements CSRF protection using double-submit cookie pattern with session binding.
|
||||
* Validates that:
|
||||
* 1. CSRF token in cookie matches token in header
|
||||
* 2. Token HMAC is valid for the current user session
|
||||
*
|
||||
* Usage:
|
||||
* - Apply to controllers handling state-changing operations
|
||||
@@ -19,14 +21,23 @@ import {
|
||||
} from "@nestjs/common";
|
||||
import { Reflector } from "@nestjs/core";
|
||||
import { Request } from "express";
|
||||
import { CsrfService } from "../services/csrf.service";
|
||||
import type { AuthenticatedUser } from "../types/user.types";
|
||||
|
||||
export const SKIP_CSRF_KEY = "skipCsrf";
|
||||
|
||||
interface RequestWithUser extends Request {
|
||||
user?: AuthenticatedUser;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class CsrfGuard implements CanActivate {
|
||||
private readonly logger = new Logger(CsrfGuard.name);
|
||||
|
||||
constructor(private reflector: Reflector) {}
|
||||
constructor(
|
||||
private reflector: Reflector,
|
||||
private csrfService: CsrfService
|
||||
) {}
|
||||
|
||||
canActivate(context: ExecutionContext): boolean {
|
||||
// Check if endpoint is marked to skip CSRF
|
||||
@@ -39,7 +50,7 @@ export class CsrfGuard implements CanActivate {
|
||||
return true;
|
||||
}
|
||||
|
||||
const request = context.switchToHttp().getRequest<Request>();
|
||||
const request = context.switchToHttp().getRequest<RequestWithUser>();
|
||||
|
||||
// Exempt safe HTTP methods (GET, HEAD, OPTIONS)
|
||||
if (["GET", "HEAD", "OPTIONS"].includes(request.method)) {
|
||||
@@ -78,6 +89,32 @@ export class CsrfGuard implements CanActivate {
|
||||
throw new ForbiddenException("CSRF token mismatch");
|
||||
}
|
||||
|
||||
// Validate session binding via HMAC
|
||||
const userId = request.user?.id;
|
||||
if (!userId) {
|
||||
this.logger.warn({
|
||||
event: "CSRF_NO_USER_CONTEXT",
|
||||
method: request.method,
|
||||
path: request.path,
|
||||
securityEvent: true,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
throw new ForbiddenException("CSRF validation requires authentication");
|
||||
}
|
||||
|
||||
if (!this.csrfService.validateToken(cookieToken, userId)) {
|
||||
this.logger.warn({
|
||||
event: "CSRF_SESSION_BINDING_INVALID",
|
||||
method: request.method,
|
||||
path: request.path,
|
||||
securityEvent: true,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
throw new ForbiddenException("CSRF token not bound to session");
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
209
apps/api/src/common/services/csrf.service.spec.ts
Normal file
209
apps/api/src/common/services/csrf.service.spec.ts
Normal file
@@ -0,0 +1,209 @@
|
||||
/**
|
||||
* CSRF Service Tests
|
||||
*
|
||||
* Tests CSRF token generation and validation with session binding.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { CsrfService } from "./csrf.service";
|
||||
|
||||
describe("CsrfService", () => {
|
||||
let service: CsrfService;
|
||||
const originalEnv = process.env;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env = { ...originalEnv };
|
||||
// Set a consistent secret for tests
|
||||
process.env.CSRF_SECRET = "test-secret-key-0123456789abcdef0123456789abcdef";
|
||||
service = new CsrfService();
|
||||
service.onModuleInit();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = originalEnv;
|
||||
});
|
||||
|
||||
describe("onModuleInit", () => {
|
||||
it("should initialize with configured secret", () => {
|
||||
const testService = new CsrfService();
|
||||
process.env.CSRF_SECRET = "configured-secret";
|
||||
expect(() => testService.onModuleInit()).not.toThrow();
|
||||
});
|
||||
|
||||
it("should throw in production without CSRF_SECRET", () => {
|
||||
const testService = new CsrfService();
|
||||
process.env.NODE_ENV = "production";
|
||||
delete process.env.CSRF_SECRET;
|
||||
expect(() => testService.onModuleInit()).toThrow(
|
||||
"CSRF_SECRET environment variable is required in production"
|
||||
);
|
||||
});
|
||||
|
||||
it("should generate random secret in development without CSRF_SECRET", () => {
|
||||
const testService = new CsrfService();
|
||||
process.env.NODE_ENV = "development";
|
||||
delete process.env.CSRF_SECRET;
|
||||
expect(() => testService.onModuleInit()).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("generateToken", () => {
|
||||
it("should generate a token with random:hmac format", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
|
||||
expect(token).toContain(":");
|
||||
const parts = token.split(":");
|
||||
expect(parts).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("should generate 64-char hex random part (32 bytes)", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
const randomPart = token.split(":")[0];
|
||||
|
||||
expect(randomPart).toHaveLength(64);
|
||||
expect(/^[0-9a-f]{64}$/.test(randomPart as string)).toBe(true);
|
||||
});
|
||||
|
||||
it("should generate 64-char hex HMAC (SHA-256)", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
const hmacPart = token.split(":")[1];
|
||||
|
||||
expect(hmacPart).toHaveLength(64);
|
||||
expect(/^[0-9a-f]{64}$/.test(hmacPart as string)).toBe(true);
|
||||
});
|
||||
|
||||
it("should generate unique tokens on each call", () => {
|
||||
const token1 = service.generateToken("user-123");
|
||||
const token2 = service.generateToken("user-123");
|
||||
|
||||
expect(token1).not.toBe(token2);
|
||||
});
|
||||
|
||||
it("should generate different HMACs for different sessions", () => {
|
||||
const token1 = service.generateToken("user-123");
|
||||
const token2 = service.generateToken("user-456");
|
||||
|
||||
const hmac1 = token1.split(":")[1];
|
||||
const hmac2 = token2.split(":")[1];
|
||||
|
||||
// Even with same random part, HMACs would differ due to session binding
|
||||
// But since random parts differ, this just confirms they're different tokens
|
||||
expect(hmac1).not.toBe(hmac2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateToken", () => {
|
||||
it("should validate a token for the correct session", () => {
|
||||
const sessionId = "user-123";
|
||||
const token = service.generateToken(sessionId);
|
||||
|
||||
expect(service.validateToken(token, sessionId)).toBe(true);
|
||||
});
|
||||
|
||||
it("should reject a token for a different session", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
|
||||
expect(service.validateToken(token, "user-456")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject empty token", () => {
|
||||
expect(service.validateToken("", "user-123")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject empty session ID", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
expect(service.validateToken(token, "")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token without colon separator", () => {
|
||||
expect(service.validateToken("invalidtoken", "user-123")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with empty random part", () => {
|
||||
expect(service.validateToken(":somehash", "user-123")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with empty HMAC part", () => {
|
||||
expect(service.validateToken("somerandom:", "user-123")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with invalid hex in random part", () => {
|
||||
expect(
|
||||
service.validateToken(
|
||||
"invalid-hex-here-not-64-chars:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
"user-123"
|
||||
)
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with invalid hex in HMAC part", () => {
|
||||
expect(
|
||||
service.validateToken(
|
||||
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef:not-valid-hex",
|
||||
"user-123"
|
||||
)
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with tampered HMAC", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
const parts = token.split(":");
|
||||
// Tamper with the HMAC
|
||||
const tamperedToken = `${parts[0]}:0000000000000000000000000000000000000000000000000000000000000000`;
|
||||
|
||||
expect(service.validateToken(tamperedToken, "user-123")).toBe(false);
|
||||
});
|
||||
|
||||
it("should reject token with tampered random part", () => {
|
||||
const token = service.generateToken("user-123");
|
||||
const parts = token.split(":");
|
||||
// Tamper with the random part
|
||||
const tamperedToken = `0000000000000000000000000000000000000000000000000000000000000000:${parts[1]}`;
|
||||
|
||||
expect(service.validateToken(tamperedToken, "user-123")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("session binding security", () => {
|
||||
it("should bind token to specific session", () => {
|
||||
const token = service.generateToken("session-A");
|
||||
|
||||
// Token valid for session-A
|
||||
expect(service.validateToken(token, "session-A")).toBe(true);
|
||||
|
||||
// Token invalid for any other session
|
||||
expect(service.validateToken(token, "session-B")).toBe(false);
|
||||
expect(service.validateToken(token, "session-C")).toBe(false);
|
||||
expect(service.validateToken(token, "")).toBe(false);
|
||||
});
|
||||
|
||||
it("should not allow token reuse across sessions", () => {
|
||||
const userAToken = service.generateToken("user-A");
|
||||
const userBToken = service.generateToken("user-B");
|
||||
|
||||
// Each token only valid for its own session
|
||||
expect(service.validateToken(userAToken, "user-A")).toBe(true);
|
||||
expect(service.validateToken(userAToken, "user-B")).toBe(false);
|
||||
|
||||
expect(service.validateToken(userBToken, "user-B")).toBe(true);
|
||||
expect(service.validateToken(userBToken, "user-A")).toBe(false);
|
||||
});
|
||||
|
||||
it("should use different secrets to generate different tokens", () => {
|
||||
// Generate token with current secret
|
||||
const token1 = service.generateToken("user-123");
|
||||
|
||||
// Create new service with different secret
|
||||
process.env.CSRF_SECRET = "different-secret-key-abcdef0123456789";
|
||||
const service2 = new CsrfService();
|
||||
service2.onModuleInit();
|
||||
|
||||
// Token from service1 should not validate with service2
|
||||
expect(service2.validateToken(token1, "user-123")).toBe(false);
|
||||
|
||||
// But service2's own tokens should validate
|
||||
const token2 = service2.generateToken("user-123");
|
||||
expect(service2.validateToken(token2, "user-123")).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
116
apps/api/src/common/services/csrf.service.ts
Normal file
116
apps/api/src/common/services/csrf.service.ts
Normal file
@@ -0,0 +1,116 @@
|
||||
/**
|
||||
* CSRF Service
|
||||
*
|
||||
* Handles CSRF token generation and validation with session binding.
|
||||
* Tokens are cryptographically tied to the user session via HMAC.
|
||||
*
|
||||
* Token format: {random_part}:{hmac(random_part + session_id, secret)}
|
||||
*/
|
||||
|
||||
import { Injectable, Logger, OnModuleInit } from "@nestjs/common";
|
||||
import * as crypto from "crypto";
|
||||
|
||||
@Injectable()
|
||||
export class CsrfService implements OnModuleInit {
|
||||
private readonly logger = new Logger(CsrfService.name);
|
||||
private csrfSecret = "";
|
||||
|
||||
onModuleInit(): void {
|
||||
const secret = process.env.CSRF_SECRET;
|
||||
|
||||
if (process.env.NODE_ENV === "production" && !secret) {
|
||||
throw new Error(
|
||||
"CSRF_SECRET environment variable is required in production. " +
|
||||
"Generate with: node -e \"console.log(require('crypto').randomBytes(32).toString('hex'))\""
|
||||
);
|
||||
}
|
||||
|
||||
// Use provided secret or generate a random one for development
|
||||
if (secret) {
|
||||
this.csrfSecret = secret;
|
||||
this.logger.log("CSRF service initialized with configured secret");
|
||||
} else {
|
||||
this.csrfSecret = crypto.randomBytes(32).toString("hex");
|
||||
this.logger.warn(
|
||||
"CSRF service initialized with random secret (development mode). " +
|
||||
"Set CSRF_SECRET for persistent tokens across restarts."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a CSRF token bound to a session identifier
|
||||
* @param sessionId - User session identifier (e.g., user ID or session token)
|
||||
* @returns Token in format: {random}:{hmac}
|
||||
*/
|
||||
generateToken(sessionId: string): string {
|
||||
// Generate cryptographically secure random part (32 bytes = 64 hex chars)
|
||||
const randomPart = crypto.randomBytes(32).toString("hex");
|
||||
|
||||
// Create HMAC binding the random part to the session
|
||||
const hmac = this.createHmac(randomPart, sessionId);
|
||||
|
||||
return `${randomPart}:${hmac}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a CSRF token against a session identifier
|
||||
* @param token - The full CSRF token (random:hmac format)
|
||||
* @param sessionId - User session identifier to validate against
|
||||
* @returns true if token is valid and bound to the session
|
||||
*/
|
||||
validateToken(token: string, sessionId: string): boolean {
|
||||
if (!token || !sessionId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Parse token parts
|
||||
const colonIndex = token.indexOf(":");
|
||||
if (colonIndex === -1) {
|
||||
this.logger.debug("Invalid token format: missing colon separator");
|
||||
return false;
|
||||
}
|
||||
|
||||
const randomPart = token.substring(0, colonIndex);
|
||||
const providedHmac = token.substring(colonIndex + 1);
|
||||
|
||||
if (!randomPart || !providedHmac) {
|
||||
this.logger.debug("Invalid token format: empty random part or HMAC");
|
||||
return false;
|
||||
}
|
||||
|
||||
// Verify the random part is valid hex (64 characters for 32 bytes)
|
||||
if (!/^[0-9a-fA-F]{64}$/.test(randomPart)) {
|
||||
this.logger.debug("Invalid token format: random part is not valid hex");
|
||||
return false;
|
||||
}
|
||||
|
||||
// Compute expected HMAC
|
||||
const expectedHmac = this.createHmac(randomPart, sessionId);
|
||||
|
||||
// Use timing-safe comparison to prevent timing attacks
|
||||
try {
|
||||
return crypto.timingSafeEqual(
|
||||
Buffer.from(providedHmac, "hex"),
|
||||
Buffer.from(expectedHmac, "hex")
|
||||
);
|
||||
} catch {
|
||||
// Buffer creation fails if providedHmac is not valid hex
|
||||
this.logger.debug("Invalid token format: HMAC is not valid hex");
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create HMAC for token binding
|
||||
* @param randomPart - The random part of the token
|
||||
* @param sessionId - The session identifier
|
||||
* @returns Hex-encoded HMAC
|
||||
*/
|
||||
private createHmac(randomPart: string, sessionId: string): string {
|
||||
return crypto
|
||||
.createHmac("sha256", this.csrfSecret)
|
||||
.update(`${randomPart}:${sessionId}`)
|
||||
.digest("hex");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user