diff --git a/docs/rate-limiting.md b/docs/rate-limiting.md index 6ab1498..9e3ae66 100644 --- a/docs/rate-limiting.md +++ b/docs/rate-limiting.md @@ -9,6 +9,11 @@ window (default 5 requests per minute) to reduce abuse from repeated challenge o verification attempts. The limiter uses the submitted Stellar address when present and falls back to the client IP otherwise. +User-facing routes under `/api/users` (excluding `/api/users/health`) use a +per-user fixed-window limiter (default **60 requests per minute**). Authenticated +callers are keyed by database user id (`users:{id}`); anonymous callers fall back +to IP. See [`docs/users-rate-limiting.md`](./users-rate-limiting.md). + ## Configuration | Variable | Default | Description | diff --git a/docs/users-rate-limiting.md b/docs/users-rate-limiting.md new file mode 100644 index 0000000..ad601e6 --- /dev/null +++ b/docs/users-rate-limiting.md @@ -0,0 +1,54 @@ +# Users API rate limiting + +Authenticated and anonymous requests under `GET /api/users/*` (via `usersRouter`) +use a per-identity fixed-window rate limit powered by +[`createPerUserRateLimiter`](../src/middleware/rateLimit.ts) (IETF draft-7 headers). + +## Policy + +| Setting | Value | +|---------|--------| +| Window | 60 seconds | +| Limit | 60 requests per key | +| Authenticated key | `users:{user.id}` (after `requireAuthForbidden` on `/me`) | +| Anonymous key | `users:ip:{socket.remoteAddress}` (public profile / predictions) | + +`GET /api/users/me` runs auth **before** the limiter so the bucket is per +database user id. Public GETs are IP-keyed (Bearer on those paths does not +change the key unless `req.user` is already populated by upstream middleware). + +`/api/users/health` is mounted on a separate router and is **not** rate-limited. + +Social (`socialRouter`) and portfolio (`userPortfolioRouter`) mounts under +`/api/users` are unchanged by this policy. + +## 429 response + +```json +{ + "error": { + "code": "rate_limit_exceeded", + "message": "Too many requests", + "retryAfter": 12, + "resetAt": "2026-07-24T12:00:00.000Z" + } +} +``` + +Also includes: + +- `Retry-After` header (seconds) +- Draft-7 `RateLimit-Limit` / `RateLimit-Remaining` / `RateLimit-Reset` headers +- Audit event `rate_limit.blocked` with correlation ID + +## Implementation + +- Router: [`src/routes/users.ts`](../src/routes/users.ts) +- Middleware: [`src/middleware/rateLimit.ts`](../src/middleware/rateLimit.ts) +- Tests: [`tests/usersRateLimit.test.ts`](../tests/usersRateLimit.test.ts) + +## Verification + +```bash +npm test -- tests/usersRateLimit.test.ts +``` diff --git a/src/middleware/rateLimit.ts b/src/middleware/rateLimit.ts index d28728b..d54ca13 100644 --- a/src/middleware/rateLimit.ts +++ b/src/middleware/rateLimit.ts @@ -148,6 +148,16 @@ export function createRateLimiter(options: Partial = {}): RateLimitRequ }) as RateLimitRequestHandler; } +/** + * Fixed-window per-identity limiter (default 60 requests / 60 seconds). + * + * Prefer an explicit `keyGenerator` when mounting on a route family so buckets + * stay isolated (e.g. `users:{id}` on `/api/users`, `predictions:{id}` on + * `/api/predictions`). Falls back to `user:{address|sub|id}` then `ip:{ip}`. + * + * Emits IETF draft-7 `RateLimit-*` headers and the standard + * `{ error: { code: "rate_limit_exceeded", ... } }` envelope on 429. + */ export function createPerUserRateLimiter(options: Partial = {}): RateLimitRequestHandler { return createRateLimiter({ windowMs: 60 * 1000, diff --git a/src/routes/users.ts b/src/routes/users.ts index 63336ea..c1ada1b 100644 --- a/src/routes/users.ts +++ b/src/routes/users.ts @@ -21,6 +21,12 @@ * → req.id → new UUID) and stamps it on `res.locals.correlationId`. * - Echoes the correlation ID back via the X-Correlation-Id response header. * - Emits a structured `users_access_log` entry on every response finish. + * + * Rate limiting (issue #411 / users-rl-v7): + * - `createPerUserRateLimiter` (60 req/min, IETF draft-7 headers) on each route. + * - `GET /me` authenticates first, then keys by `users:{user.id}`. + * - Public GETs fall back to `users:ip:{ip}` (no soft auth — preserves 403/401 contracts). + * - `/api/users/health` is mounted separately and is not throttled here. */ import { Router, Request, Response, NextFunction } from "express"; @@ -34,6 +40,7 @@ import { import { requireAuthForbidden } from "../middleware/requireAuth"; import { AuthenticatedRequest } from "../middleware/auth"; import { accessLog } from "../middleware/accessLog"; +import { createPerUserRateLimiter } from "../middleware/rateLimit"; import { logger } from "../config/logger"; import { getRequestId } from "../lib/requestContext"; import { clampLimit, DEFAULT_PAGE_SIZE } from "../utils/cursor"; @@ -48,6 +55,22 @@ const stellarAddressSchema = z .string() .regex(/^G[A-Z2-7]{55}$/, "Invalid Stellar address"); +/** + * Shared /api/users limiter. Authenticated requests (req.user set) use + * `users:{id}`; anonymous traffic uses `users:ip:{ip}`. + */ +const usersRateLimit = createPerUserRateLimiter({ + windowMs: 60 * 1000, + limit: 60, + keyGenerator: (req) => { + const userId = (req as AuthenticatedRequest).user?.id; + if (typeof userId === "string" && userId.trim().length > 0) { + return `users:${userId}`; + } + return `users:ip:${req.socket?.remoteAddress ?? "unknown"}`; + }, +}); + // --------------------------------------------------------------------------- // Access log — must be the first middleware so every handler inherits the // correlation ID via res.locals.correlationId. @@ -67,7 +90,11 @@ usersRouter.use(usersMetricsMiddleware); // --------------------------------------------------------------------------- // GET /api/users/me // --------------------------------------------------------------------------- -usersRouter.get("/me", requireAuthForbidden, async (req: AuthenticatedRequest, res, next) => { +usersRouter.get( + "/me", + requireAuthForbidden, + usersRateLimit, + async (req: AuthenticatedRequest, res, next) => { const correlationId = res.locals.correlationId as string; try { @@ -121,6 +148,7 @@ usersRouter.get("/me", requireAuthForbidden, async (req: AuthenticatedRequest, r */ usersRouter.get( "/:address/predictions", + usersRateLimit, async (req: Request, res: Response, next: NextFunction) => { // Prefer the access-log correlation ID; fall back to ALS for non-route callers. const correlationId = (res.locals.correlationId as string | undefined) ?? getRequestId(); @@ -201,6 +229,7 @@ usersRouter.get( // --------------------------------------------------------------------------- usersRouter.get( "/:stellarAddress/profile", + usersRateLimit, async (req, res, next) => { const correlationId = (res.locals.correlationId as string | undefined) ?? getRequestId(); const reqId = correlationId; diff --git a/tests/usersRateLimit.test.ts b/tests/usersRateLimit.test.ts new file mode 100644 index 0000000..2a26a45 --- /dev/null +++ b/tests/usersRateLimit.test.ts @@ -0,0 +1,130 @@ +/** + * Rate-limit coverage for /api/users (issue #411 / users-rl-v7). + * + * Mirrors the production mount in `src/routes/users.ts`: + * - key `users:{user.id}` when authenticated + * - key `users:ip:{ip}` otherwise + * - standard 429 envelope + draft-7 RateLimit headers + */ + +import request from "supertest"; +import express from "express"; +import type { Request } from "express"; +import { createPerUserRateLimiter } from "../src/middleware/rateLimit"; + +jest.mock("../src/services/auditService", () => ({ + createAuditLog: jest.fn().mockResolvedValue(undefined), +})); + +type TestUser = { id?: string; address?: string }; + +/** Same keying rules as `usersRouter` (low limit for fast tests). */ +function usersRateLimiter(limit = 2) { + return createPerUserRateLimiter({ + windowMs: 60_000, + limit, + keyGenerator: (req: Request) => { + const userId = (req as Request & { user?: TestUser }).user?.id; + if (typeof userId === "string" && userId.trim().length > 0) { + return `users:${userId}`; + } + return `users:ip:${req.socket?.remoteAddress ?? "unknown"}`; + }, + }); +} + +function makeApp(limit = 2) { + const app = express(); + app.use((req, _res, next) => { + const userId = req.headers["x-test-user-id"]; + if (typeof userId === "string" && userId.trim().length > 0) { + (req as Request & { user?: TestUser }).user = { id: userId }; + } + next(); + }); + app.use(usersRateLimiter(limit)); + app.get("/api/users/me", (_req, res) => { + res.json({ data: { ok: true } }); + }); + app.get("/api/users/:address/profile", (_req, res) => { + res.json({ data: { ok: true } }); + }); + return app; +} + +describe("per-user rate limiting for /api/users", () => { + it("enforces the limit independently for each authenticated user id", async () => { + const app = makeApp(); + + expect((await request(app).get("/api/users/me").set("x-test-user-id", "user-a")).status).toBe( + 200, + ); + expect((await request(app).get("/api/users/me").set("x-test-user-id", "user-a")).status).toBe( + 200, + ); + expect((await request(app).get("/api/users/me").set("x-test-user-id", "user-a")).status).toBe( + 429, + ); + + const otherUser = await request(app).get("/api/users/me").set("x-test-user-id", "user-b"); + expect(otherUser.status).toBe(200); + }); + + it("falls back to a per-IP bucket when no user id is present", async () => { + const app = makeApp(); + + expect((await request(app).get("/api/users/GTEST/profile")).status).toBe(200); + expect((await request(app).get("/api/users/GTEST/profile")).status).toBe(200); + expect((await request(app).get("/api/users/GTEST/profile")).status).toBe(429); + }); + + it("does not share the IP bucket with an authenticated user bucket", async () => { + const app = makeApp(); + + // Exhaust anonymous IP bucket + await request(app).get("/api/users/GTEST/profile"); + await request(app).get("/api/users/GTEST/profile"); + expect((await request(app).get("/api/users/GTEST/profile")).status).toBe(429); + + // Authenticated identity still has its own quota + const authenticated = await request(app) + .get("/api/users/me") + .set("x-test-user-id", "user-isolated"); + expect(authenticated.status).toBe(200); + }); + + it("returns the standard rate-limit error envelope", async () => { + const app = makeApp(); + + await request(app).get("/api/users/me").set("x-test-user-id", "user-envelope"); + await request(app).get("/api/users/me").set("x-test-user-id", "user-envelope"); + const response = await request(app) + .get("/api/users/me") + .set("x-test-user-id", "user-envelope"); + + expect(response.status).toBe(429); + expect(response.body.error).toMatchObject({ + code: "rate_limit_exceeded", + message: "Too many requests", + }); + expect(Number(response.headers["retry-after"])).toBeGreaterThanOrEqual(1); + expect(response.body.error.retryAfter).toBe(Number(response.headers["retry-after"])); + expect(typeof response.body.error.resetAt).toBe("string"); + }); + + it("exposes IETF draft-7 RateLimit headers on successful responses", async () => { + const app = makeApp(3); + + const response = await request(app) + .get("/api/users/me") + .set("x-test-user-id", "user-headers"); + + expect(response.status).toBe(200); + // express-rate-limit `standardHeaders: "draft-7"` emits a combined RateLimit header. + const combined = + response.headers["ratelimit"] ?? response.headers["RateLimit"]; + expect(combined).toBeDefined(); + expect(String(combined)).toMatch(/limit=\d+/i); + expect(String(combined)).toMatch(/remaining=\d+/i); + }); +});