Rate limit /token endpoint

[vbrown608]

- Summary

This PR applies a rate limit to the /token endpoint. See internal issue https://github.com/netlify/bitballoon/issues/6099

I don't want this to be a breaking change for other deploys, especially those that may be behind a proxy, so the rate limit is only applied if GOTRUE_RATE_LIMIT_IP_LOOKUPS is set.

- Test plan Added a test.

Will also test in staging from multiple IPs before rollout to verify the env var is loaded correctly and we are accurately rate limiting by IP.

- Description for the changelog Rate limit /token endpoint.

- A picture of a cute animal (not mandatory but encouraged)

[vbrown608]

Welp I did some testing and SetIPLookups doesn't actually let you look for the IP in an arbitrary header. Instead it can only contain some of the default values: https://github.com/didip/tollbooth/blob/c5a3bbb8c0a37a430ac34d3db6abe2572933189b/libstring/libstring.go#L21

~For that we need to use lmt.SetHeader, add the header values we want to rate limit as they come in, and set a TTL for them.~ I'm gonna update to do that.

Edit: Nope, we need to use tollbooth.LimitByKeys. lmt.SetHeader can only limit on (IP, header) pairs.

[vbrown608]

I have one more question about this which is, should we be concerned about memory usage? Tollbooth uses an in-memory rate limiter rather than something like Redis. I'm not really sure how to estimate the memory usage of this:

• There will be one "token bucket" per unique IP. I can use Humio to figure out the rate of unique IPs but it's not very useful without knowing the size of a bucket.
• The token buckets are GC'd after an hour, we can change that value. I'm a bit unclear on exactly how expiration works or how to find the right value. Is it per token? Per bucket?

[vbrown608]

Huh apparently gotrue doesn't have the org-wide setting to dismiss review when new commits are pushed.

[keiko713]

I have one more question about this which is, should we be concerned about memory usage?

good point! I think we want to be mindful, in the way that the default setup is not memory-heavy, as well as possibly it's configurable so that folks could adjust the settings per their infrastructure setup. actually that reminds me, maybe good to mention what the rate limit is? could go with the comment even. e.g. what 30.0/(60*5) means with the human-friendly description?

With our infra, with the current our load, my "guess" (which is not even educated guess, so more of the hope) is that it'll do fine with 1 hour TTL.

[vbrown608]

maybe good to mention what the rate limit is? could go with the comment even. e.g. what 30.0/(60*5) means with the human-friendly description?

Ahh yeah I used to have one but the "token bucket" algorithm this uses makes it a little hard to explain concisely.

I'll explain it in the complicated way here. What it means is:

• Imagine that you have a bucket that can fit 30 tokens. It starts out full.
• Whenever a request is made, remove one token from the bucket. If there are no tokens left, no request can be made.
• Every second, if there's enough room, add .1 token back into the bucket (so, it can be completely refilled in 5 minutes).

The "it can fit 30 tokens" comes from SetBurst.

It used to say "Allow up to 30 requests per 5 minutes" but that's not exactly true because the bucket is getting refilled during those 5 minutes, so you can actually make 60 requests if you start with a full bucket. During the next 5 minutes you can only make 30 because the bucket starts out empty.

Having typed that out I think we can say, "Allow requests at a rate of 30 per 5 minutes", and that will be true :+1:

[keiko713]

Having typed that out I think we can say, "Allow requests at a rate of 30 per 5 minutes", and that will be true 👍

thanks for explaining, sounds good to me 👍

[vbrown608]

possibly it's configurable so that folks could adjust the settings per their infrastructure setup.

I think right now, it's not great on configurability overall. I didn't wind up being super happy with tollbooth's support for using a custom header for the IP address.

Normally, tollbooth parses certain default IP headers in a smart way (https://github.com/didip/tollbooth/blob/c5a3bbb8c0a37a430ac34d3db6abe2572933189b/libstring/libstring.go#L21). Using at least one of those headers (X-Real-IP, X-Forwarded-For, or RemoteAddr) is enforced by the built-in tollbooth middleware, which is why I had to write a custom one. We could add smarter parsing of those headers to the custom middleware but I think we should wait and see if it's desired.

There's no rate limiting by default (and therefore no memory usage) so I think maybe we should keep it this way unless we get a specific request from someone else who is self-hosting it.

[mheffner]

If there isn't a shared token bucket across instances, do you set each instance's limit to 1/Nth the total and assume an even request distribution?