mastodon icon indicating copy to clipboard operation
mastodon copied to clipboard

[RFC] [WIP] Allow IP addresses to be added to the allowlist which skips the rate limit checks

Open Kami opened this issue 3 years ago • 2 comments

Background, Context

I'm self hosting my own Mastodon instance and I encounter "Too many requests" / "Rate limit reached" errors very often even when just very lightly and casually browsing my timeline as an authenticated user using a web browser.

Possibly related issue - https://github.com/mastodon/mastodon/issues/13098

Proposed Changes

In this PR, I introduce new RATE_LIMIT_ALLOWLIST_IPS environment variable with which instance administrator can specify which IP addresses should be allowlisted / excluded from the rate limits.

This change / approach works fine for me since I always access my instance over VPN using internal IP address.

Keep in mind that this PR is not final, it's just a quick draft to get the discussion going on the possible solutions and to see if the project is opening to accepting such change.

TODO (once agreed on the approach, etc.)

  • [ ] Any code changes / polishing
  • [ ] Tests
  • [ ] Docs

Kami avatar Nov 26 '22 20:11 Kami

This is very useful for instance admins / moderators. Thank you. I have a little suggestion regarding this, not sure if it can be done, but let me try to ask....

May this be done in a role way? This way we can easily add a custom permission to a role like "Allow bypass rate limit check", which can be assigned to a group of people such as Owner, Admin or Moderators? This way it can be more useful than hardcoding the IP into the env, hence useful for people whose IP address is dynamic.

trankten avatar Nov 27 '22 10:11 trankten

@trankten Thanks for the review / response.

Role / permission based approach does indeed some more flexible. I have some questions / comments about it:

  1. How will it impact the performance (since this check needs to happen on every request)? Will it add additional overhead or authenticated User model is already available somewhere in thread local request context (or similar) since it's already needed for other purposes so this additional check shouldn't add much overhead (but I guess it may still include overhead in terms of querying roles and permissions - not sure how that happens today and if any of that is already cached either in memory, Redis or similar)?
  2. I'm not really familiar with the code base so it will take me a while to dig in and add such change (if it's possible).
  3. Would the project be open to accepting this simpler change first (of course after I finish it - add tests, docs, examples, etc.) and in the future accepting this more flexible role / permission based change in the future? I still see some value in this simpler change - in addition to my scenario (static IP or access over VPN), I think it could maybe also come handy in other scenarios (e.g. when using a proxy which doesn't propagate x-forwarded-for header and user wants to allowlist the internal proxy ip or similar).

Kami avatar Nov 27 '22 11:11 Kami

Closing this primarily due to age and lack of embrace by the core team -- but also, this seems like something where we should solve the usability issue of hitting these rate limits through normal usage.

I know there has been some improvement on that front since this PR ... if this is still an issue for you, can you either comment here, or on an existing issue which documents this bug?

mjankowski avatar May 24 '24 18:05 mjankowski