Apply user-based API rate limits

[skyfallwastaken]

Summary of the problem

Rack Attack has been disabled in production since 5720daa. Turning it back on with the old rules would repeat the August outage: integrations such as Lapse make requests for all of their users from one server address, so they shared a single IP allowance and logins started failing with 429 responses.

Behind Cloudflare and the Coolify proxy, req.ip can also resolve to a Cloudflare edge address instead of the real client, which would group unrelated users into the same bucket.

Describe your changes

• Tier API rate limits by authentication. Requests without credentials stay limited by IP. Authenticated requests skip the IP limits and get 600 requests per minute per user, shared across OAuth, API key, WakaTime compatible and public stats APIs. Admin API keys and Admin OAuth get 5,000 per minute.
• Limit rejected credentials on those APIs by IP before authentication runs, so fake credentials cannot be used to escape the IP limits. On public stats, credentials that are ignored count as rejected. require_oauth_scope now sets a class attribute instead of prepending a callback, so scoped OAuth checks run after this guard.
• Limit POST /oauth/token per OAuth application and client IP, with a higher IP ceiling, instead of the general POST limit. Client IDs are public, so the IP stays in the key to stop others using up an application's allowance.
• Add CloudflareClientIp middleware. When the trusted proxy chain shows a Cloudflare edge, it prepends CF-Connecting-IP to X-Forwarded-For so Rack Attack and request.remote_ip see the real client. The original chain is kept so req.cloudflare? still passes, and requests that bypass Cloudflare or forge an edge address keep their own IP.
• Ignore the Forwarded header when resolving client IPs. Rack prefers it over X-Forwarded-For and neither proxy strips it, so clients could previously pick their own IP and pass the Cloudflare check.
• Enable Rack Attack in production only, since cloudflare-rails is only bundled there.
• Document API rate limits and retry behaviour for developers.

Screenshots / Media

Not applicable.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[greptile-apps]

Confidence Score: 2/5

[High risk] Implements user-based API rate limiting across multiple endpoints.

The PR does not yet appear safe to merge because concurrent invalid-credential requests can exceed the authentication guard and the published OpenAPI contract remains stale.

Findings

1.   Rejected requests bypass throttles ▶
2.  Admin OAuth splits allowance ▶
3.  API contract remains stale ▶
4.   Concurrent failures bypass limit ▶
5.   OAuth client buckets are attacker-controlled ▶

Fix with agent prompt

### Issue 1
config/initializers/rack_attack.rb:73-90
When a client repeatedly sends missing or invalid credentials to these API families, the path-based exclusions skip both primary IP throttles and authentication halts before the controller limiter runs, allowing a burst of up to 10,000 failed authentication requests instead of 300 per minute. **How this was verified:** The excluded paths retain only the 10,000-per-hour API throttle, while each controller registers authentication before the new rate-limit callback.

### Issue 2
app/controllers/api/admin/application_controller.rb:18-22
When an administrative user calls both regular and Admin APIs through OAuth, `oauth_user:<id>` and `user:<id>` create independent counters in the shared scope, allowing 600 requests per minute instead of the documented shared 300-request user allowance.

```suggestion
      def authenticated_api_rate_limit_identity
        return "admin_api_key:#{current_admin_api_key.id}" if current_admin_api_key

        "user:#{current_user.id}"
      end
```

### Issue 3
spec/requests/api/hackatime/v1/compatibility_spec.rb:136-143
The Rswag response schema and rate-limit header examples changed without regenerating `swagger/v1/swagger.yaml`, so the published OpenAPI contract continues to expose the previous heartbeat rate-limit response.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 4
app/controllers/concerns/authenticated_api_rate_limiting.rb:29-32
If more than 300 rejected requests from one IP overlap, each request can read the counter before any post-authentication increment occurs, so they all execute authentication and the intended 300-per-minute pre-authentication limit is exceeded.

### Issue 5
config/initializers/rack_attack.rb:82-83
When an unauthenticated caller repeatedly submits a known OAuth application UID as `client_id` or the Basic-auth username, Rack::Attack charges those invalid requests to the application's shared 300-request bucket, causing legitimate token exchanges for that application to receive 429 responses. **How this was verified:** Both invalid and legitimate requests derive the same `client:<id>` discriminator directly from the submitted application UID.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR enables production rate limiting based on authenticated user or administrator identity, adds rejected-credential and OAuth-token throttles, and hardens client-IP resolution behind Cloudflare.

• Adds shared controller-level limits for authenticated API families.
• Adds production Rack::Attack rules and Cloudflare-aware IP middleware.
• Documents rate-limit behavior and adds focused controller and middleware tests.

Diagram

sequenceDiagram
  participant Client
  participant Rack as Rack::Attack
  participant Guard as Rejected-credential guard
  participant Auth as Controller authentication
  participant Cache
  Client->>Rack: Credential-bearing API request
  Rack->>Guard: Credential-limited route bypasses IP throttle
  Guard->>Cache: Read per-IP failure count
  Cache-->>Guard: Count below 300
  Guard->>Auth: Run authentication
  Auth-->>Guard: 401/403 or authenticated identity
  Guard->>Cache: Increment failures after response
  Note over Guard,Cache: Concurrent requests can all read before any increment

Loading

_{Reviews (5) · Last reviewed commit: "Key OAuth token limits by client and IP"}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	gem/​brakeman@​8.0.6 ⏵ 8.1.0	+1

View full report