diff --git a/src/pretix/base/forms/auth.py b/src/pretix/base/forms/auth.py index a47c231a7f..69e9d59d7c 100644 --- a/src/pretix/base/forms/auth.py +++ b/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: - from django_redis import get_redis_connection - rc = get_redis_connection("redis") - cnt = rc.get(self.ratelimit_key) - 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') + if rate_limit("login", include_ip_from_request=self.request, max_num=10, expire_time=300): + 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 diff --git a/src/pretix/base/forms/user.py b/src/pretix/base/forms/user.py index 773f952203..4b413ad516 100644 --- a/src/pretix/base/forms/user.py +++ b/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: - from django_redis import get_redis_connection - rc = get_redis_connection("redis") - cnt = rc.incr('pretix_pwchange_%s' % self.user.pk) - rc.expire('pretix_pwchange_%s' % self.user.pk, 300) - if cnt > 10: - raise forms.ValidationError( - self.error_messages['rate_limit'], - code='rate_limit', - ) + if rate_limit("pwchange", self.user.pk, max_num=10, expire_time=300): + 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 diff --git a/src/pretix/control/views/auth.py b/src/pretix/control/views/auth.py index 143d3cfccb..182aba725a 100644 --- a/src/pretix/control/views/auth.py +++ b/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: - from django_redis import get_redis_connection - rc = get_redis_connection("redis") - 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') + if rate_limit("pwreset", user.pk, max_num=1, expire_time=3600 * 24): + user.log_action('pretix.control.auth.user.forgot_password.denied.repeated') + raise RepeatedResetDenied() 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 - if has_redis: - from django_redis import get_redis_connection - rc = get_redis_connection("redis") - rc.delete('pretix_pwreset_%s' % user.id) + rate_limit_reset("pwreset", user.pk) return redirect('control:auth.login') else: return self.get(request, *args, **kwargs) diff --git a/src/pretix/control/views/user.py b/src/pretix/control/views/user.py index 315b1ef041..b7a28a73e3 100644 --- a/src/pretix/control/views/user.py +++ b/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', diff --git a/src/pretix/helpers/ratelimit.py b/src/pretix/helpers/ratelimit.py new file mode 100644 index 0000000000..6e9e29eae4 --- /dev/null +++ b/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 . +# +# 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 +# . +# +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) diff --git a/src/pretix/presale/forms/customer.py b/src/pretix/presale/forms/customer.py index 79d24cb80b..fc5a964e44 100644 --- a/src/pretix/presale/forms/customer.py +++ b/src/pretix/presale/forms/customer.py @@ -20,8 +20,6 @@ # . # 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: - from django_redis import get_redis_connection - - rc = get_redis_connection("redis") - 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', - ) + if rate_limit("customer_signup", include_ip_from_request=self.request, max_num=10, expire_time=600): + 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: - from django_redis import get_redis_connection - - 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: + if d.get('email'): + if rate_limit("customer_pwreset", self.customer.pk, max_num=2, expire_time=600): 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: - from django_redis import get_redis_connection - - 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: + if old_pw: + if rate_limit("customer_pwchange", self.customer.pk, max_num=10, expire_time=300): 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: - from django_redis import get_redis_connection - - rc = get_redis_connection("redis") - 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 rate_limit("customer_pwchange", self.instance.pk, max_num=10, expire_time=300): + raise forms.ValidationError( + self.error_messages['rate_limit'], + code='rate_limit', + ) if not check_password(old_pw, self.instance.password): raise forms.ValidationError( diff --git a/src/pretix/presale/views/user.py b/src/pretix/presale/views/user.py index b81457a1bf..4ac405a000 100644 --- a/src/pretix/presale/views/user.py +++ b/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: - from django_redis import get_redis_connection - rc = get_redis_connection("redis") - if rc.exists('pretix_resend_{}_{}'.format(request.event.pk, user)): - messages.error(request, _('If the email address you entered is valid and associated with a ticket, we have ' - 'already sent you an email with a link to your ticket in the past {number} hours. ' - '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') + if rate_limit("order_resend", self.request.event.pk, user, max_num=1, expire_time=3600 * 24): + messages.error(request, _('If the email address you entered is valid and associated with a ticket, we have ' + 'already sent you an email with a link to your ticket in the past {number} hours. ' + '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')) orders = self.request.event.orders.filter(email__iexact=user) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index aadca81098..fa10ded31d 100644 --- a/src/tests/conftest.py +++ b/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 diff --git a/src/tests/control/test_auth.py b/src/tests/control/test_auth.py index 3a965c9333..5f0dfa1712 100644 --- a/src/tests/control/test_auth.py +++ b/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', {