views: fix sms-registration phone-number ratelimit key (#72597)

src/authentic2/app_settings.py

@@ -301,8 +301,8 @@ default_settings = dict(
     A2_EMAILS_ADDRESS_RATELIMIT=Setting(
         default='3/d', definition='Maximum rate of emails sent to the same email address.'
     ),
-    A2_SMS_RATELIMIT=Setting(
-        default='1/h', definition='Maximum rate of SMSs sent to the same email address.'
+    A2_SMS_NUMBER_RATELIMIT=Setting(
+        default='10/h', definition='Maximum rate of SMSs sent to the same phone number.'
     ),
     A2_USER_DELETED_KEEP_DATA=Setting(
         default=['email', 'uuid', 'phone'], definition='User data to keep after deletion'

src/authentic2/utils/sms.py

@@ -29,6 +29,16 @@ except ImportError:  # fallback on python requests, no Publik signature
     from requests.sessions import Session as Requests  # # pylint: disable=ungrouped-imports
 
 
+def sms_ratelimit_key(group, request):
+    if 'phone' in request.session:
+        phone = request.session['phone']
+        return f'{group}:{phone}'
+    else:
+        prefix = request.POST['phone_0'][0]
+        number = request.POST['phone_1'][0]
+        return f'{group}:{prefix}:{number}'
+
+
 def create_sms_code():
     return ''.join(
         choices(

src/authentic2/views.py

@@ -70,7 +70,7 @@ from .utils import misc as utils_misc
 from .utils import switch_user as utils_switch_user
 from .utils.evaluate import make_condition_context
 from .utils.service import get_service, set_home_url
-from .utils.sms import SMSError, send_registration_sms
+from .utils.sms import SMSError, send_registration_sms, sms_ratelimit_key
 from .utils.view_decorators import enable_view_restriction
 from .utils.views import csrf_token_check
 
@@ -1110,12 +1110,13 @@ class BaseRegistrationView(HomeURLMixin, FormView):
             )
             return self.form_invalid(form)
         self.request.session[resend_key] = False
+        self.request.session['phone'] = phone
 
         if is_ratelimited(
             self.request,
-            key='post:sms',
+            key=sms_ratelimit_key,
             group='registration-sms',
-            rate=app_settings.A2_SMS_RATELIMIT,
+            rate=app_settings.A2_SMS_NUMBER_RATELIMIT,
             increment=True,
         ):
             form.add_error(

tests/test_views.py

@@ -16,12 +16,16 @@
 # authentic2
 
 import datetime
+from random import randint
 from unittest import mock
 from urllib.parse import urlparse
 
 import pytest
 from django.urls import reverse
 from django.utils.html import escape
+from httmock import HTTMock
+from httmock import response as httmock_response
+from httmock import urlmatch
 
 from authentic2.custom_user.models import DeletedUser, User
 from authentic2.forms.passwords import PasswordChangeForm, SetPasswordForm
@@ -315,6 +319,80 @@ def test_custom_account(settings, app, simple_user):
     assert response['Location'] == settings.A2_ACCOUNTS_URL
 
 
+@pytest.mark.parametrize('view_name', ['registration_register'])  # password_lost to be added with #69890
+def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name):
+    freezer.move_to('2020-01-01')
+    settings.A2_SMS_IP_RATELIMIT = '10/h'
+    settings.A2_SMS_NUMBER_RATELIMIT = '3/d'
+    settings.A2_ACCEPT_PHONE_AUTHENTICATION = True
+    settings.SMS_SENDER = 'EO'
+    settings.SMS_URL = 'https://www.example.com/send'
+
+    @urlmatch(scheme='https', netloc='www.example.com', path='/send')
+    def sms_endpoint_response(url, request):
+        return httmock_response(200, {})
+
+    with HTTMock(sms_endpoint_response):
+        # reach email limit
+        response = app.get(reverse(view_name))
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', '0612345678')
+        response = response.form.submit()
+        assert 'try again later' not in response.text
+        for _ in range(2):
+            response = app.get(reverse(view_name))
+            response.form.set('phone_0', '33')
+            response.form.set('phone_1', '0612345678')
+            response = response.form.submit()
+            response = response.form.submit()  # validate warning message "sms already sent"
+            assert 'try again later' not in response.text
+
+        response = app.get(reverse(view_name))
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', '0612345678')
+        response = response.form.submit().form.submit()
+        assert 'try again later' in response.text
+
+        # reach ip limit
+        for _ in range(7):
+            response = app.get(reverse(view_name))
+            random_suffix = randint(0, 9999)
+            response.form.set('phone_0', '33')
+            response.form.set('phone_1', f'061234{random_suffix:04d}')
+            response = response.form.submit()
+            assert 'try again later' not in response.text
+
+        response = app.get(reverse(view_name))
+        random_suffix = randint(0, 9999)
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', f'061234{random_suffix:04d}')
+        response = response.form.submit()
+        assert 'try again later' in response.text
+
+        # ip ratelimits are lifted after an hour
+        freezer.tick(datetime.timedelta(hours=1))
+        response = app.get(reverse(view_name))
+        random_suffix = randint(0, 9999)
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', f'061234{random_suffix:04d}')
+        response = response.form.submit()
+        assert 'try again later' not in response.text
+
+        # email ratelimits are lifted after a day
+        response = app.get(reverse(view_name))
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', '0612345678')
+        response = response.form.submit().form.submit()
+        assert 'try again later' in response.text
+
+        freezer.tick(datetime.timedelta(days=1))
+        response = app.get(reverse(view_name))
+        response.form.set('phone_0', '33')
+        response.form.set('phone_1', '0612345678')
+        response = response.form.submit()
+        assert 'try again later' not in response.text
+
+
 @pytest.mark.parametrize('view_name', ['registration_register', 'password_reset'])
 def test_views_email_ratelimit(app, db, simple_user, settings, mailoutbox, freezer, view_name):
     freezer.move_to('2020-01-01')