User email verifications: Add rate limiting (Z#23232834)

Also, refactor rate limiting into a central utility

src/pretix/base/forms/auth.py

@@ -33,8 +33,6 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations under the License.
 
-import hashlib
-import ipaddress
 import logging
 
 from django import forms
@@ -42,13 +40,12 @@ from django.conf import settings
 from django.contrib.auth.password_validation import (
     password_validators_help_texts, validate_password,
 )
-from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
 from pretix.base.metrics import pretix_failed_logins
 from pretix.base.models import User
 from pretix.helpers.dicts import move_to_end
-from pretix.helpers.http import get_client_ip
+from pretix.helpers.ratelimit import rate_limit, rate_limit_reset
 
 logger = logging.getLogger(__name__)
 
@@ -85,38 +82,14 @@ class LoginForm(forms.Form):
         else:
             move_to_end(self.fields, 'keep_logged_in')
 
-    @cached_property
-    def ratelimit_key(self):
-        if not settings.HAS_REDIS:
-            return None
-        client_ip = get_client_ip(self.request)
-        if not client_ip:
-            return None
-        try:
-            client_ip = ipaddress.ip_address(client_ip)
-        except ValueError:
-            # Web server not set up correctly
-            return None
-        if client_ip.is_private:
-            # This is the private IP of the server, web server not set up correctly
-            return None
-        return 'pretix_login_{}'.format(hashlib.sha1(str(client_ip).encode()).hexdigest())
-
     def clean(self):
         if all(k in self.cleaned_data for k, f in self.fields.items() if f.required):
-            if self.ratelimit_key:
+            if rate_limit("login", include_ip_from_request=self.request, max_num=10, expire_time=300):
-                from django_redis import get_redis_connection
+                pretix_failed_logins.inc(1, reason="ratelimit")
-                rc = get_redis_connection("redis")
+                logger.info("Backend login rejected due to rate limit.")
-                cnt = rc.get(self.ratelimit_key)
+                raise forms.ValidationError(self.error_messages['rate_limit'], code='rate_limit')
-                if cnt and int(cnt) > 10:
-                    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')
             self.user_cache = self.backend.form_authenticate(self.request, self.cleaned_data)
             if self.user_cache is None:
-                if self.ratelimit_key:
-                    rc.incr(self.ratelimit_key)
-                    rc.expire(self.ratelimit_key, 300)
                 logger.info("Backend login invalid.")
                 pretix_failed_logins.inc(1, reason="invalid")
                 raise forms.ValidationError(
@@ -124,6 +97,7 @@ class LoginForm(forms.Form):
                     code='invalid_login'
                 )
             else:
+                rate_limit_reset("login", include_ip_from_request=self.request)
                 self.confirm_login_allowed(self.user_cache)
 
         return self.cleaned_data

src/pretix/base/forms/user.py

@@ -33,7 +33,6 @@
 # License for the specific language governing permissions and limitations under the License.
 
 from django import forms
-from django.conf import settings
 from django.contrib.auth.hashers import check_password
 from django.contrib.auth.password_validation import (
     password_validators_help_texts, validate_password,
@@ -46,6 +45,7 @@ from pytz import common_timezones
 from pretix.base.models import User
 from pretix.control.forms import SingleLanguageWidget
 from pretix.helpers.format import format_map
+from pretix.helpers.ratelimit import rate_limit
 
 
 class UserSettingsForm(forms.ModelForm):
@@ -128,16 +128,11 @@ class UserPasswordChangeForm(forms.Form):
     def clean_old_pw(self):
         old_pw = self.cleaned_data.get('old_pw')
 
-        if settings.HAS_REDIS:
+        if rate_limit("pwchange", self.user.pk, max_num=10, expire_time=300):
-            from django_redis import get_redis_connection
+            raise forms.ValidationError(
-            rc = get_redis_connection("redis")
+                self.error_messages['rate_limit'],
-            cnt = rc.incr('pretix_pwchange_%s' % self.user.pk)
+                code='rate_limit',
-            rc.expire('pretix_pwchange_%s' % self.user.pk, 300)
+            )
-            if cnt > 10:
-                raise forms.ValidationError(
-                    self.error_messages['rate_limit'],
-                    code='rate_limit',
-                )
 
         if not check_password(old_pw, self.user.password):
             raise forms.ValidationError(
@@ -175,6 +170,7 @@ class UserEmailChangeForm(forms.Form):
     error_messages = {
         'duplicate_identifier': _("There already is an account associated with this email address. "
                                   "Please choose a different one."),
+        'rate_limit': _("For security reasons, please wait 5 minutes before you try again."),
     }
     old_email = forms.EmailField(label=_('Old email address'), disabled=True)
     new_email = forms.EmailField(label=_('New email address'))
@@ -190,4 +186,10 @@ class UserEmailChangeForm(forms.Form):
                 self.error_messages['duplicate_identifier'],
                 code='duplicate_identifier',
             )
+
+        if rate_limit("emailchange", self.user.pk, max_num=1, expire_time=300):
+            raise forms.ValidationError(
+                self.error_messages['rate_limit'],
+                code='rate_limit',
+            )
         return email

src/pretix/control/views/auth.py

@@ -65,6 +65,7 @@ from pretix.base.forms.auth import (
 from pretix.base.metrics import pretix_failed_logins, pretix_successful_logins
 from pretix.base.models import TeamInvite, U2FDevice, User, WebAuthnDevice
 from pretix.helpers.http import get_client_ip, redirect_to_url
+from pretix.helpers.ratelimit import rate_limit, rate_limit_reset
 from pretix.helpers.security import handle_login_source, session_login
 
 logger = logging.getLogger(__name__)
@@ -318,19 +319,12 @@ class Forgot(TemplateView):
         if self.form.is_valid():
             email = self.form.cleaned_data['email']
 
-            has_redis = settings.HAS_REDIS
-
             try:
                 user = User.objects.get(is_active=True, auth_backend='native', email__iexact=email)
 
-                if has_redis:
+                if rate_limit("pwreset", user.pk, max_num=1, expire_time=3600 * 24):
-                    from django_redis import get_redis_connection
+                    user.log_action('pretix.control.auth.user.forgot_password.denied.repeated')
-                    rc = get_redis_connection("redis")
+                    raise RepeatedResetDenied()
-                    if rc.exists('pretix_pwreset_%s' % (user.id)):
-                        user.log_action('pretix.control.auth.user.forgot_password.denied.repeated')
-                        raise RepeatedResetDenied()
-                    else:
-                        rc.setex('pretix_pwreset_%s' % (user.id), 3600 * 24, '1')
 
             except User.DoesNotExist:
                 logger.warning('Backend password reset for unregistered e-mail \"' + email + '\" requested.')
@@ -343,6 +337,7 @@ class Forgot(TemplateView):
                 user.log_action('pretix.control.auth.user.forgot_password.mail_sent')
 
             finally:
+                has_redis = settings.HAS_REDIS
                 if has_redis:
                     messages.info(request, _('If the address is registered to valid account, then we have sent you an email containing further instructions. '
                                              'Please note that we will send at most one email every 24 hours.'))
@@ -411,11 +406,7 @@ class Recover(TemplateView):
             messages.success(request, _('You can now login using your new password.'))
             user.log_action('pretix.control.auth.user.forgot_password.recovered')
 
-            has_redis = settings.HAS_REDIS
+            rate_limit_reset("pwreset", user.pk)
-            if has_redis:
-                from django_redis import get_redis_connection
-                rc = get_redis_connection("redis")
-                rc.delete('pretix_pwreset_%s' % user.id)
             return redirect('control:auth.login')
         else:
             return self.get(request, *args, **kwargs)

src/pretix/control/views/user.py

@@ -80,6 +80,7 @@ from pretix.control.permissions import (
 )
 from pretix.control.views.auth import get_u2f_appid, get_webauthn_rp_id
 from pretix.helpers.http import redirect_to_url
+from pretix.helpers.ratelimit import rate_limit
 from pretix.helpers.security import session_reauth
 from pretix.helpers.u2f import websafe_encode
 
@@ -908,6 +909,10 @@ class UserEmailVerifyView(View):
             messages.success(self.request, _('Your email address was already verified.'))
             return redirect(reverse('control:user.settings', kwargs={}))
 
+        if rate_limit("emailverify", self.request.user.pk, max_num=2, expire_time=300):
+            messages.error(self.request, _("For security reasons, please wait 5 minutes before you try again."))
+            return redirect(reverse('control:user.settings', kwargs={}))
+
         self.request.user.send_confirmation_code(
             session=self.request.session,
             reason='email_verify',

src/pretix/helpers/ratelimit.py

@@ -0,0 +1,112 @@
+#
+# This file is part of pretix (Community Edition).
+#
+# Copyright (C) 2014-2020  Raphael Michel and contributors
+# Copyright (C) 2020-today pretix GmbH and contributors
+#
+# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General
+# Public License as published by the Free Software Foundation in version 3 of the License.
+#
+# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are
+# applicable granting you additional permissions and placing additional restrictions on your usage of this software.
+# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive
+# this file, see <https://pretix.eu/about/en/license>.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along with this program.  If not, see
+# <https://www.gnu.org/licenses/>.
+#
+import hashlib
+import ipaddress
+import logging
+
+from django.conf import settings
+from django.http import HttpRequest
+
+from pretix.helpers.http import get_client_ip
+
+logger = logging.getLogger(__name__)
+
+
+def _get_key(key, parameters):
+    return f'pretix:ratelimit:{key}:' + hashlib.sha256(','.join(str(p) for p in parameters).encode()).hexdigest()
+
+
+def _get_ip(request):
+    if not settings.HAS_REDIS:
+        return None
+    client_ip = get_client_ip(request)
+    if not client_ip:
+        return None
+    try:
+        client_ip = ipaddress.ip_address(client_ip)
+    except ValueError:
+        # Web server not set up correctly
+        return None
+    if client_ip.is_private:
+        # This is the private IP of the server, web server not set up correctly
+        return None
+    return str(client_ip)
+
+
+def rate_limit(key: str, *parameters, include_ip_from_request: HttpRequest=None, max_num: int, expire_time: int):
+    """
+    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
+
+    :param key: The key refering to the feature like "pwreset"
+    :param parameters: Any number of things to be hashed as the bucket key
+    :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
+    :return:
+    """
+    if not settings.HAS_REDIS:
+        # No rate limiting
+        return False
+
+    from django_redis import get_redis_connection
+    rc = get_redis_connection("redis")
+
+    if include_ip_from_request:
+        ip = _get_ip(include_ip_from_request)
+        if not ip:
+            # IP not discovered, can't rate limit
+            return False
+        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 new_counter > max_num:
+        return True
+
+    return False
+
+
+def rate_limit_reset(key: str, *parameters, include_ip_from_request: HttpRequest=None):
+    """
+    Reset a rate limit bucket.
+    """
+    if not settings.HAS_REDIS:
+        # No rate limiting
+        return
+
+    from django_redis import get_redis_connection
+    rc = get_redis_connection("redis")
+
+    if include_ip_from_request:
+        ip = _get_ip(include_ip_from_request)
+        if not ip:
+            # IP not discovered, can't rate limit
+            return False
+        parameters = (*parameters, ip)
+
+    redis_key = _get_key(key, parameters)
+    rc.delete(redis_key)

src/pretix/presale/forms/customer.py

@@ -20,8 +20,6 @@
 # <https://www.gnu.org/licenses/>.
 #
 import functools
-import hashlib
-import ipaddress
 import random
 
 from django import forms
@@ -32,7 +30,6 @@ from django.contrib.auth.password_validation import (
 )
 from django.contrib.auth.tokens import PasswordResetTokenGenerator
 from django.core import signing
-from django.utils.functional import cached_property
 from django.utils.html import escape
 from django.utils.translation import gettext_lazy as _
 from phonenumber_field.formfields import PhoneNumberField
@@ -44,6 +41,7 @@ from pretix.base.forms.questions import (
 from pretix.base.i18n import get_language_without_region
 from pretix.base.models import Customer
 from pretix.helpers.http import get_client_ip
+from pretix.helpers.ratelimit import rate_limit
 from pretix.multidomain.urlreverse import build_absolute_uri
 
 
@@ -205,23 +203,6 @@ class RegistrationForm(forms.Form):
                 min_value=0,
             )
 
-    @cached_property
-    def ratelimit_key(self):
-        if not settings.HAS_REDIS:
-            return None
-        client_ip = get_client_ip(self.request)
-        if not client_ip:
-            return None
-        try:
-            client_ip = ipaddress.ip_address(client_ip)
-        except ValueError:
-            # Web server not set up correctly
-            return None
-        if client_ip.is_private:
-            # This is the private IP of the server, web server not set up correctly
-            return None
-        return 'pretix_customer_registration_{}'.format(hashlib.sha1(str(client_ip).encode()).hexdigest())
-
     def clean(self):
         email = self.cleaned_data.get('email')
 
@@ -255,17 +236,11 @@ class RegistrationForm(forms.Form):
                 code='incomplete'
             )
         else:
-            if self.ratelimit_key:
+            if rate_limit("customer_signup", include_ip_from_request=self.request, max_num=10, expire_time=600):
-                from django_redis import get_redis_connection
+                raise forms.ValidationError(
-
+                    self.error_messages['rate_limit'],
-                rc = get_redis_connection("redis")
+                    code='rate_limit',
-                cnt = rc.incr(self.ratelimit_key)
+                )
-                rc.expire(self.ratelimit_key, 600)
-                if cnt > 10:
-                    raise forms.ValidationError(
-                        self.error_messages['rate_limit'],
-                        code='rate_limit',
-                    )
         return self.cleaned_data
 
     def create(self):
@@ -370,13 +345,8 @@ class ResetPasswordForm(forms.Form):
 
     def clean(self):
         d = super().clean()
-        if d.get('email') and settings.HAS_REDIS:
+        if d.get('email'):
-            from django_redis import get_redis_connection
+            if rate_limit("customer_pwreset", self.customer.pk, max_num=2, expire_time=600):
-
-            rc = get_redis_connection("redis")
-            cnt = rc.incr('pretix_pwreset_customer_%s' % self.customer.pk)
-            rc.expire('pretix_pwreset_customer_%s' % self.customer.pk, 600)
-            if cnt > 2:
                 raise forms.ValidationError(
                     self.error_messages['rate_limit'],
                     code='rate_limit',
@@ -445,13 +415,8 @@ class ChangePasswordForm(forms.Form):
     def clean_password_current(self):
         old_pw = self.cleaned_data.get('password_current')
 
-        if old_pw and settings.HAS_REDIS:
+        if old_pw:
-            from django_redis import get_redis_connection
+            if rate_limit("customer_pwchange", self.customer.pk, max_num=10, expire_time=300):
-
-            rc = get_redis_connection("redis")
-            cnt = rc.incr('pretix_pwchange_customer_%s' % self.customer.pk)
-            rc.expire('pretix_pwchange_customer_%s' % self.customer.pk, 300)
-            if cnt > 10:
                 raise forms.ValidationError(
                     self.error_messages['rate_limit'],
                     code='rate_limit',
@@ -521,17 +486,11 @@ class ChangeInfoForm(forms.ModelForm):
         old_pw = self.cleaned_data.get('password_current')
 
         if old_pw:
-            if settings.HAS_REDIS:
+            if rate_limit("customer_pwchange", self.instance.pk, max_num=10, expire_time=300):
-                from django_redis import get_redis_connection
+                raise forms.ValidationError(
-
+                    self.error_messages['rate_limit'],
-                rc = get_redis_connection("redis")
+                    code='rate_limit',
-                cnt = rc.incr('pretix_pwchange_customer_%s' % self.instance.pk)
+                )
-                rc.expire('pretix_pwchange_customer_%s' % self.instance.pk, 300)
-                if cnt > 10:
-                    raise forms.ValidationError(
-                        self.error_messages['rate_limit'],
-                        code='rate_limit',
-                    )
 
             if not check_password(old_pw, self.instance.password):
                 raise forms.ValidationError(

src/pretix/presale/views/user.py

@@ -32,7 +32,6 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations under the License.
 
-from django.conf import settings
 from django.contrib import messages
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
@@ -42,6 +41,7 @@ from django.views.generic import TemplateView
 from pretix.base.email import get_email_context
 from pretix.base.services.mail import INVALID_ADDRESS, mail
 from pretix.helpers.http import redirect_to_url
+from pretix.helpers.ratelimit import rate_limit
 from pretix.multidomain.urlreverse import eventreverse
 from pretix.presale.forms.user import ResendLinkForm
 from pretix.presale.views import EventViewMixin
@@ -61,17 +61,12 @@ class ResendLinkView(EventViewMixin, TemplateView):
 
         user = self.link_form.cleaned_data.get('email')
 
-        if settings.HAS_REDIS:
+        if rate_limit("order_resend", self.request.event.pk, user, max_num=1, expire_time=3600 * 24):
-            from django_redis import get_redis_connection
+            messages.error(request, _('If the email address you entered is valid and associated with a ticket, we have '
-            rc = get_redis_connection("redis")
+                                      'already sent you an email with a link to your ticket in the past {number} hours. '
-            if rc.exists('pretix_resend_{}_{}'.format(request.event.pk, user)):
+                                      'If the email did not arrive, please check your spam folder and also double check '
-                messages.error(request, _('If the email address you entered is valid and associated with a ticket, we have '
+                                      'that you used the correct email address.').format(number=24))
-                                          'already sent you an email with a link to your ticket in the past {number} hours. '
+            return redirect_to_url(eventreverse(self.request.event, 'presale:event.resend_link'))
-                                          'If the email did not arrive, please check your spam folder and also double check '
-                                          'that you used the correct email address.').format(number=24))
-                return redirect_to_url(eventreverse(self.request.event, 'presale:event.resend_link'))
-            else:
-                rc.setex('pretix_resend_{}_{}'.format(request.event.pk, user), 3600 * 24, '1')
 
         orders = self.request.event.orders.filter(email__iexact=user)
 

src/tests/conftest.py

@@ -120,6 +120,7 @@ def fakeredis_client(monkeypatch):
         redis = get_redis_connection("default", True)
         redis.flushall()
         monkeypatch.setattr('django_redis.get_redis_connection', get_redis_connection, raising=False)
+        monkeypatch.setattr('pretix.base.metrics.redis', redis, raising=False)
         yield redis
 
 

src/tests/control/test_auth.py

@@ -504,33 +504,8 @@ class Login2FAFormTest(TestCase):
         assert "recovery code" in djmail.outbox[0].body
 
 
-class FakeRedis(object):
-    def get_redis_connection(self, connection_string):
-        return self
-
-    def __init__(self):
-        self.storage = {}
-
-    def pipeline(self):
-        return self
-
-    def hincrbyfloat(self, rkey, key, amount):
-        return self
-
-    def commit(self):
-        return self
-
-    def exists(self, rkey):
-        return rkey in self.storage
-
-    def setex(self, rkey, value, expiration):
-        self.storage[rkey] = value
-
-    def execute(self):
-        pass
-
-
 @pytest.mark.usefixtures("class_monkeypatch")
+@pytest.mark.usefixtures("fakeredis_client")
 class PasswordRecoveryFormTest(TestCase):
     def setUp(self):
         super().setUp()
@@ -560,11 +535,6 @@ class PasswordRecoveryFormTest(TestCase):
 
     @override_settings(HAS_REDIS=True)
     def test_email_reset_twice_redis(self):
-        fake_redis = FakeRedis()
-        m = self.monkeypatch
-        m.setattr('django_redis.get_redis_connection', fake_redis.get_redis_connection, raising=False)
-        m.setattr('pretix.base.metrics.redis', fake_redis, raising=False)
-
         djmail.outbox = []
 
         response = self.client.post('/control/forgot', {