diff --git a/Caddyfile b/Caddyfile index 0f95af4..5705a44 100644 --- a/Caddyfile +++ b/Caddyfile @@ -1,11 +1,4 @@ lilastudy.com { - header { - X-Content-Type-Options "nosniff" - X-Frame-Options "DENY" - Referrer-Policy "strict-origin-when-cross-origin" - Strict-Transport-Security "max-age=31536000; includeSubDomains" - Content-Security-Policy "default-src 'self'; connect-src 'self' https://api.lilastudy.com wss://api.lilastudy.com; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'" - } reverse_proxy web:80 } diff --git a/apps/api/package.json b/apps/api/package.json index 870a77d..bfd2878 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -15,8 +15,6 @@ "better-auth": "^1.6.2", "cors": "^2.8.6", "express": "^5.2.1", - "express-rate-limit": "^8.4.0", - "helmet": "^8.1.0", "ws": "^8.20.0" }, "devDependencies": { diff --git a/apps/api/src/app.test.ts b/apps/api/src/app.test.ts deleted file mode 100644 index f41d3f0..0000000 --- a/apps/api/src/app.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import request from "supertest"; -import { describe, it, expect } from "vitest"; -import { createApp } from "./app.js"; - -const app = createApp(); - -describe("security headers (helmet)", () => { - it("sets X-Content-Type-Options to nosniff", async () => { - const res = await request(app).get("/api/v1/health"); - expect(res.headers["x-content-type-options"]).toBe("nosniff"); - }); - - it("sets X-Frame-Options to SAMEORIGIN", async () => { - const res = await request(app).get("/api/v1/health"); - expect(res.headers["x-frame-options"]).toBe("SAMEORIGIN"); - }); - - it("removes X-Powered-By header", async () => { - const res = await request(app).get("/api/v1/health"); - expect(res.headers).not.toHaveProperty("x-powered-by"); - }); - - it("sets Content-Security-Policy", async () => { - const res = await request(app).get("/api/v1/health"); - expect(res.headers).toHaveProperty("content-security-policy"); - }); -}); - -describe("auth rate limiting", () => { - it("returns 429 after exceeding the auth limit", async () => { - const testApp = createApp(); - const limit = 20; - for (let i = 0; i < limit; i++) { - await request(testApp).post("/api/auth/sign-in"); - } - const res = await request(testApp).post("/api/auth/sign-in"); - expect(res.status).toBe(429); - }); -}); diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index 635a92a..93d1864 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -1,26 +1,20 @@ import express from "express"; import type { Express } from "express"; import { toNodeHandler } from "better-auth/node"; -import cors from "cors"; -import helmet from "helmet"; import { auth } from "./lib/auth.js"; import { apiRouter } from "./routes/apiRouter.js"; import { errorHandler } from "./middleware/errorHandler.js"; -import { authLimiter } from "./middleware/rateLimiters.js"; +import cors from "cors"; export function createApp() { const app: Express = express(); - app.set("trust proxy", 1); - app.use(helmet()); - app.use( cors({ origin: process.env["CORS_ORIGIN"] || "http://localhost:5173", credentials: true, }), ); - app.use("/api/auth", authLimiter); app.all("/api/auth/*splat", toNodeHandler(auth)); app.use(express.json()); app.use("/api/v1", apiRouter); diff --git a/apps/api/src/middleware/rateLimiters.test.ts b/apps/api/src/middleware/rateLimiters.test.ts deleted file mode 100644 index 29a219f..0000000 --- a/apps/api/src/middleware/rateLimiters.test.ts +++ /dev/null @@ -1,179 +0,0 @@ -import express from "express"; -import request from "supertest"; -import { describe, it, expect, beforeEach } from "vitest"; -import { authLimiter, gameLimiter, lobbyLimiter } from "./rateLimiters.js"; - -import type { Session, User } from "better-auth"; - -// Minimal app to test the limiter in isolation -function createTestApp() { - const app = express(); - app.set("trust proxy", 1); - app.use("/api/auth", authLimiter); - app.all("/api/auth/*splat", (_req, res) => { - res.status(200).json({ success: true }); - }); - return app; -} - -describe("authLimiter", () => { - let app: ReturnType; - - beforeEach(() => { - // Fresh app = fresh in-memory store = counters reset between tests - app = createTestApp(); - }); - - it("allows requests under the limit through", async () => { - const res = await request(app).post("/api/auth/sign-in"); - expect(res.status).toBe(200); - }); - - it("returns 429 after exceeding the limit", async () => { - const limit = 20; - for (let i = 0; i < limit; i++) { - await request(app).post("/api/auth/sign-in"); - } - const res = await request(app).post("/api/auth/sign-in"); - expect(res.status).toBe(429); - expect(res.body).toEqual({ - success: false, - error: "Too many requests, please try again later.", - }); - }); - - it("sets RateLimit headers on responses", async () => { - const res = await request(app).post("/api/auth/sign-in"); - expect(res.headers).toHaveProperty("ratelimit"); - }); -}); - -function fakeAuth(userId: string) { - return ( - req: express.Request, - _res: express.Response, - next: express.NextFunction, - ) => { - req.session = { session: {} as Session, user: { id: userId } as User }; - next(); - }; -} - -function createGameTestApp(userId = "user-1") { - const app = express(); - app.set("trust proxy", 1); - app.use(fakeAuth(userId)); - app.use(gameLimiter); - app.post("/game/start", (_req, res) => - res.status(200).json({ success: true }), - ); - app.post("/game/answer", (_req, res) => - res.status(200).json({ success: true }), - ); - return app; -} - -describe("gameLimiter", () => { - it("allows requests under the limit through", async () => { - const app = createGameTestApp(); - const res = await request(app).post("/game/start"); - expect(res.status).toBe(200); - }); - - it("returns 429 after exceeding the limit", async () => { - const app = createGameTestApp(); - const limit = 150; - for (let i = 0; i < limit; i++) { - await request(app).post("/game/answer"); - } - const res = await request(app).post("/game/answer"); - expect(res.status).toBe(429); - expect(res.body).toEqual({ - success: false, - error: "Too many requests, please try again later.", - }); - }); - - it("tracks limits per user, not per IP", async () => { - const app = express(); - app.set("trust proxy", 1); - - // Two routes, same limiter, different users - app.use("/user1", fakeAuth("user-1"), gameLimiter, (_req, res) => - res.status(200).json({ success: true }), - ); - app.use("/user2", fakeAuth("user-2"), gameLimiter, (_req, res) => - res.status(200).json({ success: true }), - ); - - const limit = 150; - for (let i = 0; i < limit; i++) { - await request(app).post("/user1"); - } - - // user-1 is exhausted - const blocked = await request(app).post("/user1"); - expect(blocked.status).toBe(429); - - // user-2 is unaffected - const allowed = await request(app).post("/user2"); - expect(allowed.status).toBe(200); - }); -}); - -function createLobbyTestApp(userId = "user-1") { - const app = express(); - app.set("trust proxy", 1); - app.use(fakeAuth(userId)); - app.use(lobbyLimiter); - app.post("/lobbies", (_req, res) => res.status(200).json({ success: true })); - app.post("/lobbies/:code/join", (_req, res) => - res.status(200).json({ success: true }), - ); - return app; -} - -describe("lobbyLimiter", () => { - it("allows requests under the limit through", async () => { - const app = createLobbyTestApp(); - const res = await request(app).post("/lobbies"); - expect(res.status).toBe(200); - }); - - it("returns 429 after exceeding the limit", async () => { - const app = createLobbyTestApp(); - const limit = 20; - for (let i = 0; i < limit; i++) { - await request(app).post("/lobbies"); - } - const res = await request(app).post("/lobbies"); - expect(res.status).toBe(429); - expect(res.body).toEqual({ - success: false, - error: "Too many requests, please try again later.", - }); - }); - - it("tracks limits per user, not per IP", async () => { - const app = express(); - app.set("trust proxy", 1); - - app.use("/user1", fakeAuth("user-1"), lobbyLimiter, (_req, res) => - res.status(200).json({ success: true }), - ); - app.use("/user2", fakeAuth("user-2"), lobbyLimiter, (_req, res) => - res.status(200).json({ success: true }), - ); - - const limit = 20; - for (let i = 0; i < limit; i++) { - await request(app).post("/user1"); - } - - const blocked = await request(app).post("/user1"); - expect(blocked.status).toBe(429); - - const allowed = await request(app).post("/user2"); - expect(allowed.status).toBe(200); - }); -}); diff --git a/apps/api/src/middleware/rateLimiters.ts b/apps/api/src/middleware/rateLimiters.ts deleted file mode 100644 index 479be03..0000000 --- a/apps/api/src/middleware/rateLimiters.ts +++ /dev/null @@ -1,44 +0,0 @@ -import rateLimit from "express-rate-limit"; -import type { Request } from "express"; - -// TODO: When Valkey is wired up, swap the default in-memory store for -// rate-limit-redis to persist limits across restarts: -// -// import { RedisStore } from "rate-limit-redis"; -// import { valkey } from "../lib/valkey.js"; -// Then add to each limiter: store: new RedisStore({ sendCommand: (...args) => valkey.call(...args) }) - -export const authLimiter = rateLimit({ - windowMs: 15 * 60 * 1000, - limit: 20, - standardHeaders: "draft-8", - legacyHeaders: false, - message: { - success: false, - error: "Too many requests, please try again later.", - }, -}); - -export const gameLimiter = rateLimit({ - windowMs: 15 * 60 * 1000, - limit: 150, - standardHeaders: "draft-8", - legacyHeaders: false, - keyGenerator: (req: Request) => req.session!.user.id, - message: { - success: false, - error: "Too many requests, please try again later.", - }, -}); - -export const lobbyLimiter = rateLimit({ - windowMs: 15 * 60 * 1000, - limit: 20, - standardHeaders: "draft-8", - legacyHeaders: false, - keyGenerator: (req: Request) => req.session!.user.id, - message: { - success: false, - error: "Too many requests, please try again later.", - }, -}); diff --git a/apps/api/src/routes/gameRouter.ts b/apps/api/src/routes/gameRouter.ts index 850a146..f65bfb6 100644 --- a/apps/api/src/routes/gameRouter.ts +++ b/apps/api/src/routes/gameRouter.ts @@ -2,12 +2,9 @@ import express from "express"; import type { Router } from "express"; import { createGame, submitAnswer } from "../controllers/gameController.js"; import { requireAuth } from "../middleware/authMiddleware.js"; -import { gameLimiter } from "../middleware/rateLimiters.js"; export const gameRouter: Router = express.Router(); gameRouter.use(requireAuth); -gameRouter.use(gameLimiter); - gameRouter.post("/start", createGame); gameRouter.post("/answer", submitAnswer); diff --git a/apps/api/src/routes/lobbyRouter.ts b/apps/api/src/routes/lobbyRouter.ts index 5cc24c9..5bd82dd 100644 --- a/apps/api/src/routes/lobbyRouter.ts +++ b/apps/api/src/routes/lobbyRouter.ts @@ -5,12 +5,10 @@ import { joinLobbyHandler, } from "../controllers/lobbyController.js"; import { requireAuth } from "../middleware/authMiddleware.js"; -import { lobbyLimiter } from "../middleware/rateLimiters.js"; export const lobbyRouter: Router = express.Router(); lobbyRouter.use(requireAuth); -lobbyRouter.use(lobbyLimiter); lobbyRouter.post("/", createLobbyHandler); lobbyRouter.post("/:code/join", joinLobbyHandler); diff --git a/documentation/backlog.md b/documentation/backlog.md index 557ef1f..c1cd276 100644 --- a/documentation/backlog.md +++ b/documentation/backlog.md @@ -32,9 +32,6 @@ Things that are actively in progress or should be picked up immediately. Mostly - **Security headers with helmet** `[security]` Add helmet middleware to set secure HTTP response headers. One-liner: app.use(helmet()). Covers headers like X-Content-Type-Options, X-Frame-Options, and Content-Security-Policy. -- **Conditionally register OAuth providers** `[debt]` - Better Auth logs warnings when social providers are registered without credentials (`Social provider google is missing clientId or clientSecret`). Instead of registering all providers unconditionally, only add a provider to the config when its credentials are present in the environment. Keeps local dev clean for contributors who don't have OAuth apps set up. - --- ## next @@ -108,9 +105,6 @@ Directionally right, timing is unclear. Revisit when the next/now work is done. - **OpenAPI documentation for REST endpoints** `[feature]` Document the API surface using OpenAPI/Swagger. Covers all REST endpoints with request/response shapes. Useful groundwork for the admin dashboard and any future contributors. -- **Frontend tests** `[debt]` - component tests for QuestionCard, OptionButton, ScoreScreen; consider Playwright or Vitest browser mode for e2e - --- ## changelog diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 15acc4d..ef2ffc4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -62,12 +62,6 @@ importers: express: specifier: ^5.2.1 version: 5.2.1 - express-rate-limit: - specifier: ^8.4.0 - version: 8.4.0(express@5.2.1) - helmet: - specifier: ^8.1.0 - version: 8.1.0 ws: specifier: ^8.20.0 version: 8.20.0 @@ -2051,12 +2045,6 @@ packages: resolution: {integrity: sha512-knvyeauYhqjOYvQ66MznSMs83wmHrCycNEN6Ao+2AeYEfxUIkuiVxdEa1qlGEPK+We3n0THiDciYSsCcgW/DoA==} engines: {node: '>=12.0.0'} - express-rate-limit@8.4.0: - resolution: {integrity: sha512-gDK8yiqKxrGta+3WtON59arrrw6GLmadA1qoFgYXzdcch8fmKDID2XqO8itsi3f1wufXYPT51387dN6cvVBS3Q==} - engines: {node: '>= 16'} - peerDependencies: - express: '>= 4.11' - express@5.2.1: resolution: {integrity: sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==} engines: {node: '>= 18'} @@ -2197,10 +2185,6 @@ packages: resolution: {integrity: sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==} engines: {node: '>= 0.4'} - helmet@8.1.0: - resolution: {integrity: sha512-jOiHyAZsmnr8LqoPGmCjYAaiuWwjAPLgY8ZX2XrmHawt99/u1y6RgrZMTeoPfpUbV96HOalYgz1qzkRbw54Pmg==} - engines: {node: '>=18.0.0'} - hermes-estree@0.25.1: resolution: {integrity: sha512-0wUoCcLp+5Ev5pDW2OriHC2MJCbwLwuRx+gAqMTOkGKJJiBCLjtrvy4PWUGn6MIVefecRpzoOZ/UV6iGdOr+Cw==} @@ -2243,10 +2227,6 @@ packages: ini@1.3.8: resolution: {integrity: sha512-JV/yugV2uzW5iMRSiZAyDtQd+nxtUnjeLt0acNdw98kKLrvuRVyB80tsREOE7yvGVgalhZ6RNXCmEHkUKBKxew==} - ip-address@10.1.0: - resolution: {integrity: sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==} - engines: {node: '>= 12'} - ipaddr.js@1.9.1: resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==} engines: {node: '>= 0.10'} @@ -4841,11 +4821,6 @@ snapshots: expect-type@1.3.0: {} - express-rate-limit@8.4.0(express@5.2.1): - dependencies: - express: 5.2.1 - ip-address: 10.1.0 - express@5.2.1: dependencies: accepts: 2.0.0 @@ -5007,8 +4982,6 @@ snapshots: dependencies: function-bind: 1.1.2 - helmet@8.1.0: {} - hermes-estree@0.25.1: {} hermes-parser@0.25.1: @@ -5047,8 +5020,6 @@ snapshots: ini@1.3.8: {} - ip-address@10.1.0: {} - ipaddr.js@1.9.1: {} is-binary-path@2.1.0: