feat: support per-user rate limiting on REST endpoints

Closed
RI
2mo ago
+447-176

Proposed changes (including videos or screenshots)

Adds per: 'user' to rateLimiterOptions, so a REST endpoint counts against the caller instead of the address, and applies it to chat.sendMessage. The limiter also moves into the middleware chain, which is what makes it testable.

The rule was hardcoded as { IPAddr, route } with no way to override it, so everyone behind one NAT shared a single allowance. The DDP limiter counted per user, and sendMessage did so from 2015; CORE-2629 was the first symptom — #41966 restored the number on chat.sendMessage but not the subject it counts against.

rateLimiterOptions: { numRequestsAllowed: 5, intervalTimeInMS: 1000, per: 'user' }

'ip' | 'user', defaulting to 'ip', so every route that already declares a limit keeps counting exactly as it does today — chat.sendMessage is the only one opting in. Only the matcher changes, and the matcher is what the counter key is built from.

Key construction lives in server/api/rateLimiterKey.ts, including the fallback that keeps a user-keyed route limited when the request is unauthenticated — rate-limit skips a rule whose matcher returns a falsy value, so without it the route would fail open.

The limiter is now a middleware. Nothing exercised it, which is how CORE-2629 reached the release candidate: it ran as the first statement of the route action and talked to the whole APIClass, so no test could reach it. Pulling it into the middleware chain is what makes it testable — and fixes where it runs, since the action is the last link:

sequenceDiagram
    autonumber
    participant C as Client
    participant Auth as auth
    participant RL as rateLimiter
    participant Perm as permissions
    participant Lic as license
    participant H as handler

    C->>Auth: POST /v1/chat.sendMessage
    Auth->>Auth: resolve user (Mongo)
    Auth->>RL: next()
    RL->>RL: bucket = route + userId
    alt over the allowance
        RL-->>C: 429 + X-RateLimit-*
    else within it
        RL->>Perm: next()
        Perm->>Lic: next()
        Lic->>H: next()
        H-->>C: 200 + X-RateLimit-*
    end

A throttled request no longer pays the permission read or the license check. It stays after authentication because per-user keying needs a verified user, and api-bypass-rate-limit needs a permission read.

rateLimiterMiddleware receives settings, resolve and canBypass instead of reaching for them — the shape cors and metrics already use in that folder, and the reason a spec can drive it. WebHookAPI kept a bucket per integration, and stayed limited whoever owns it, by overriding shouldVerifyRateLimit and enforceRateLimit; those two are now the two seams. The MCP endpoint had a private copy of the middleware and now passes only its JSON-RPC error body.

One behaviour change: a request both over the limit and missing a permission now answers 429 where it answered 403.

Issue(s)

Steps to test or reproduce

yarn jest --selectProjects server --testPathPatterns 'rateLimiter|mcp/index'

34 tests. rateLimiter.spec.ts mounts the real Router on an express app and drives it with supertest, a real CachedSettings and a real counter — allowance, per-user separation, X-RateLimit-*, the 429 body, bypass, both setting gates.

End to end: the limiter registers no rules under TEST_MODE, so run the server without it (env -u TEST_MODE yarn dev), turn Accounts_TwoFactorAuthentication_By_Email_Enabled off, and create two regular users — admins bypass the limiter. Send one message as each, back to back, within the same second:

POST /v1/chat.sendMessage  as A  →  X-RateLimit-Remaining: 4
POST /v1/chat.sendMessage  as B  →  X-RateLimit-Remaining: 4

On develop the second reads 3 — B is spending A's allowance. Keep going as A and the 6th call returns 429 while B still sends fine.

Further comments

Two alternatives considered. The generic half could live in packages/http-router, but that package only describes routes — it records authRequired for OpenAPI and ships no auth middleware. And MCP could have kept its private copy, but it called enforceRateLimitForRoute, so the choice was migrating it or keeping two implementations of the same policy alive.

After this merges, #41966 and #41984 can be updated to key by userId.

Summary by CodeRabbit

  • New Features

    • Added configurable API rate limiting by IP address or user.
    • Added per-user rate limiting to chat.sendMessage—up to 5 requests per second.
    • Added standard rate-limit response headers and JSON responses when limits are exceeded.
    • Added support for bypassing rate limits where permitted.

  • Bug Fixes

    • Improved rate-limit handling for authenticated and anonymous requests, including shared IP addresses.
    • Standardized rate-limit behavior across API and MCP endpoints.