Review notes

src/pretix/base/forms/auth.py

@@ -84,7 +84,9 @@ class LoginForm(forms.Form):
 
     def clean(self):
         if all(k in self.cleaned_data for k, f in self.fields.items() if f.required):
-            if rate_limit("login", include_ip_from_request=self.request, max_num=10, expire_time=300):
+            rate_limit_kwargs = dict(include_ip_from_request=self.request, max_num=10, expire_time=300)
+            if rate_limit("login", **rate_limit_kwargs, increase=False):
+                # Check rate limit without counting up, we increase below only on failed logins
                 pretix_failed_logins.inc(1, reason="ratelimit")
                 logger.info("Backend login rejected due to rate limit.")
                 raise forms.ValidationError(self.error_messages['rate_limit'], code='rate_limit')
@@ -92,6 +94,8 @@ class LoginForm(forms.Form):
             if self.user_cache is None:
                 logger.info("Backend login invalid.")
                 pretix_failed_logins.inc(1, reason="invalid")
+                # Count towards rate limit (result is ignored, we are checking above)
+                rate_limit("login", **rate_limit_kwargs)
                 raise forms.ValidationError(
                     self.error_messages['invalid_login'],
                     code='invalid_login'

src/pretix/base/forms/user.py

@@ -177,10 +177,19 @@ class UserEmailChangeForm(forms.Form):
 
     def __init__(self, *args, **kwargs):
         self.user = kwargs.pop('user')
+        self.request = kwargs.pop('request')
         super().__init__(*args, **kwargs)
 
     def clean_new_email(self):
         email = self.cleaned_data['new_email']
+
+        if rate_limit("emailchange_attempt", include_ip_from_request=self.request, max_num=5, expire_time=300):
+            # Rate limit lookup for conflicting email addresses to make enumeration harder
+            raise forms.ValidationError(
+                self.error_messages['rate_limit'],
+                code='rate_limit',
+            )
+
         if User.objects.filter(Q(email__iexact=email) & ~Q(pk=self.user.pk)).exists():
             raise forms.ValidationError(
                 self.error_messages['duplicate_identifier'],

src/pretix/control/views/user.py

@@ -880,6 +880,7 @@ class UserEmailChangeView(RecentAuthenticationRequiredMixin, FormView):
 
         return {
             **super().get_form_kwargs(),
+            "request": self.request,
             "user": self.request.user,
         }
 

src/pretix/helpers/ratelimit.py

@@ -50,16 +50,18 @@ def _get_ip(request):
     return str(client_ip)
 
 
-def rate_limit(key: str, *parameters, include_ip_from_request: HttpRequest=None, max_num: int, expire_time: int):
+def rate_limit(key: str, *parameters, include_ip_from_request: HttpRequest=None, max_num: int, expire_time: int, increase: bool = True):
     """
     This is a shared utility to implement simple rate limiting in operations like
-    password resets. This is by far no perfect implementation of rate limiting, as
-    it the window is prolonge
+    password resets.
 
-    :param key: The key refering to the feature like "pwreset"
+    :param key: The key referring to the feature like "pwreset"
     :param parameters: Any number of things to be hashed as the bucket key
+    :param include_ip_from_request: Add IP address from request to the bucket key. If IP address cannot be determined,
+                                    rate limit is not applied.
     :param max_num: The maximum number of actions to performed within expire_time of the first action
     :param expire_time: The length of the time window in seconds
+    :param increase: Whether to count the call as an event counted towards the rate, or just check
     :return:
     """
     if not settings.HAS_REDIS:
@@ -77,10 +79,14 @@ def rate_limit(key: str, *parameters, include_ip_from_request: HttpRequest=None,
         parameters = (*parameters, ip)
 
     redis_key = _get_key(key, parameters)
-    p = rc.pipeline()
-    p.set(redis_key, 0, nx=True, ex=expire_time)  # Start a rate limit window if none is running
-    p.incr(redis_key)
-    new_counter = p.execute()[1]
+
+    if increase:
+        p = rc.pipeline()
+        p.set(redis_key, 0, nx=True, ex=expire_time)  # Start a rate limit window if none is running
+        p.incr(redis_key)
+        new_counter = p.execute()[1]
+    else:
+        new_counter = int(rc.get(redis_key) or 0)
 
     if new_counter > max_num:
         return True