fix: Unauthenticated api throttling #20993
Conversation
| @@ -190,6 +190,7 @@ class LoginViewSet(NonCreatingViewSetMixin, viewsets.GenericViewSet): | |||
| queryset = User.objects.none() | |||
| serializer_class = LoginSerializer | |||
| permission_classes = (permissions.AllowAny,) | |||
| # NOTE: Throttling is handled by the `axes` package | |||
There was a problem hiding this comment.
Added a note here as I wasn't sure why we had no throttler for logging in
| @@ -434,6 +434,23 @@ def test_cant_reset_more_than_six_times(self): | |||
| # Three emails should be sent, fourth should not | |||
| self.assertEqual(len(mail.outbox), 6) | |||
|
|
|||
| def test_is_rate_limited_on_email_not_ip(self): | |||
There was a problem hiding this comment.
i'm lazily assuming there's a test in here that shows the reverse (if we have the same IP but varying addresses then we don't limit (or we're slower to limit))
| @@ -222,6 +223,26 @@ def get_bucket_key(self, request): | |||
| return ident | |||
|
|
|||
|
|
|||
| class UserOrEmailRateThrottle(SimpleRateThrottle): | |||
There was a problem hiding this comment.
i've only got a vague understanding these throttles...
if i'm a bad actor on a single IP stuffing email addresses into the login form will I get rate limited at some point?
There was a problem hiding this comment.
Nope. Only per email after this change.
There was a problem hiding this comment.
Should we have a fallback rate limit?
It's relatively common for attackers to find sites with poor rate limits and use them to check email/common password combos to get around rate limits on the site they really want to attack...
There was a problem hiding this comment.
(can be a follow up since we want to test this really fixes the issue folk are experiencing sooner rather than later)
There was a problem hiding this comment.
Login is protected differently (axes). This only applies to email validation / password reset requests.
There was a problem hiding this comment.
👍 someone could still stuff emails to find valid accounts but relatively low risk :)
Problem
We were throttling certain requests such as email signup / password reset based on the request which is a bit loose given we almost always have something we can identify the intended user with (email or uuid).
Also we didn't set a scope which means that one throttler could be affecting the other. We have really harsh rate limits on password resets which then in turn affect anything else scoped to that user.
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?