From cc443d7b454c3e457c52fff6ca437a7c946bc93b Mon Sep 17 00:00:00 2001 From: Paul Marillonnet Date: Wed, 21 Dec 2022 10:41:29 +0100 Subject: [PATCH 1/5] models: sms code adjustments for password reset (#69890) --- .../migrations/0045_auto_20230117_1513.py | 30 +++++++++++++++++++ src/authentic2/models.py | 16 ++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/authentic2/migrations/0045_auto_20230117_1513.py diff --git a/src/authentic2/migrations/0045_auto_20230117_1513.py b/src/authentic2/migrations/0045_auto_20230117_1513.py new file mode 100644 index 000000000..89e45716e --- /dev/null +++ b/src/authentic2/migrations/0045_auto_20230117_1513.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.26 on 2023-01-17 14:13 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('authentic2', '0047_initialize_services_runtime_settings'), + ] + + operations = [ + migrations.AddField( + model_name='smscode', + name='fake', + field=models.BooleanField(default=False, verbose_name='Is a fake code'), + ), + migrations.AddField( + model_name='smscode', + name='user', + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + verbose_name='user', + ), + ), + ] diff --git a/src/authentic2/models.py b/src/authentic2/models.py index 60e16252b..99c2b07ca 100644 --- a/src/authentic2/models.py +++ b/src/authentic2/models.py @@ -814,7 +814,11 @@ class APIClient(models.Model): class SMSCode(models.Model): CODE_DURATION = 120 KIND_REGISTRATION = 'registration' - KIND_PASSWORD_LOST = 'password-lost' + KIND_PASSWORD_LOST = 'password-reset' + CODE_TO_TOKEN_KINDS = { + KIND_REGISTRATION: 'registration', + KIND_PASSWORD_LOST: 'pw-reset', + } value = models.CharField( verbose_name=_('Identifier'), default=create_sms_code, editable=False, max_length=32 ) @@ -822,6 +826,9 @@ class SMSCode(models.Model): phone = models.CharField( _('phone number'), null=True, blank=True, max_length=64, validators=[PhoneNumberValidator] ) + user = models.ForeignKey( + settings.AUTH_USER_MODEL, verbose_name=_('user'), on_delete=models.CASCADE, null=True + ) url_token = models.UUIDField( verbose_name=_('URL token'), default=uuid.uuid4, @@ -830,19 +837,22 @@ class SMSCode(models.Model): expires = models.DateTimeField(verbose_name=_('Expires')) sent = models.BooleanField(default=False, verbose_name=_('SMS code sent')) + # fake codes to avoid disclosing account existence info on unjustified password reset attempts + fake = models.BooleanField(default=False, verbose_name=_('Is a fake code')) + @classmethod def cleanup(cls, now=None): now = now or timezone.now() cls.objects.filter(expires__lte=now).delete() @classmethod - def create(cls, phone, kind=None, expires=None, duration=None): + def create(cls, phone, user=None, kind=None, expires=None, fake=False, duration=None): if not kind: kind = cls.KIND_REGISTRATION if not duration: duration = cls.CODE_DURATION expires = expires or (timezone.now() + datetime.timedelta(seconds=duration)) - return cls.objects.create(kind=kind, phone=phone, expires=expires) + return cls.objects.create(kind=kind, user=user, phone=phone, expires=expires, fake=fake) class Setting(models.Model): -- 2.39.2 From 45bc870c064a50433329629b3df84f4a3163f34f Mon Sep 17 00:00:00 2001 From: Paul Marillonnet Date: Thu, 15 Dec 2022 10:16:13 +0100 Subject: [PATCH 2/5] utils/sms: add password lost sms code recovery utils (#69890) --- .../password_lost/sms_code_password_lost.txt | 1 + src/authentic2/utils/sms.py | 44 +++++++++++++++---- src/authentic2/views.py | 2 +- tests/test_registration.py | 2 +- 4 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 src/authentic2/templates/password_lost/sms_code_password_lost.txt diff --git a/src/authentic2/templates/password_lost/sms_code_password_lost.txt b/src/authentic2/templates/password_lost/sms_code_password_lost.txt new file mode 100644 index 000000000..f16b234ae --- /dev/null +++ b/src/authentic2/templates/password_lost/sms_code_password_lost.txt @@ -0,0 +1 @@ +{% load i18n %}{% blocktrans trimmed with value=code.value %}Your code is {{ value }}{% endblocktrans %} \ No newline at end of file diff --git a/src/authentic2/utils/sms.py b/src/authentic2/utils/sms.py index 0e9cf35a1..b81850d6e 100644 --- a/src/authentic2/utils/sms.py +++ b/src/authentic2/utils/sms.py @@ -48,12 +48,14 @@ def create_sms_code(): ) -def generate_code(phone_number): +def generate_code(phone_number, user=None, kind=None, fake=False): from authentic2.models import SMSCode return SMSCode.create( phone_number, - kind='registration', + user=user, + kind=kind or SMSCode.KIND_REGISTRATION, + fake=fake or kind is SMSCode.KIND_PASSWORD_LOST and user is None, ) @@ -61,7 +63,7 @@ class SMSError(Exception): pass -def send_registration_sms(request, phone_number, ou, template_names=None, context=None, **kwargs): +def send_sms(phone_number, ou, user=None, template_names=None, context=None, kind=None, **kwargs): """Sends a registration code sms to a user, the latter inputs the received code in a dedicated form to validate their account creation. """ @@ -79,12 +81,12 @@ def send_registration_sms(request, phone_number, ou, template_names=None, contex logger.error('settings.SMS_URL is not set') raise SMSError('SMS improperly configured') - if not template_names: - template_names = ['registration/sms_code_registration.txt'] if not isinstance(context, dict): context = {} - code = generate_code(phone_number) + code = generate_code(phone_number, user=user, kind=kind) + if code.fake is True: + return code context.update({'code': code}) message = render_plain_text_template_to_string(template_names, context) @@ -99,9 +101,35 @@ def send_registration_sms(request, phone_number, ou, template_names=None, contex with transaction.atomic(): response = requests.post(url, json=payload, timeout=10) response.raise_for_status() - code.sent = True code.save() except RequestException as e: - logger.warning('sms registration to %s using %s failed: %s', phone_number, url, e) + logger.warning('sms code to %s using %s failed: %s', phone_number, url, e) raise SMSError(f'Error while contacting SMS service: {e}') return code + + +def send_registration_sms(phone_number, ou, template_names=None, context=None, **kwargs): + from authentic2.models import SMSCode + + return send_sms( + phone_number, + ou, + template_names=template_names or ['registration/sms_code_registration.txt'], + context=context, + kind=SMSCode.KIND_REGISTRATION, + **kwargs, + ) + + +def send_password_reset_sms(phone_number, ou, user=None, template_names=None, context=None, **kwargs): + from authentic2.models import SMSCode + + return send_sms( + phone_number, + ou, + user=user, + template_names=template_names or ['password_lost/sms_code_password_lost.txt'], + context=context, + kind=SMSCode.KIND_PASSWORD_LOST, + **kwargs, + ) diff --git a/src/authentic2/views.py b/src/authentic2/views.py index 7863ffac3..5d25a08bd 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -1152,7 +1152,7 @@ class BaseRegistrationView(HomeURLMixin, FormView): ) return self.form_invalid(form) try: - code = send_registration_sms(self.request, phone, ou=self.ou, **self.token) + code = send_registration_sms(phone, ou=self.ou, **self.token) except SMSError: messages.warning( self.request, diff --git a/tests/test_registration.py b/tests/test_registration.py index aa460bdd2..1561bf660 100644 --- a/tests/test_registration.py +++ b/tests/test_registration.py @@ -1052,7 +1052,7 @@ def test_phone_registration_connection_error(app, db, settings, freezer, caplog) ) assert ( caplog.records[0].message - == 'sms registration to +33612345678 using https://foo.whatever.none/ failed: unreachable' + == 'sms code to +33612345678 using https://foo.whatever.none/ failed: unreachable' ) -- 2.39.2 From 9de50d40efbd16aca45c055d4d54e0befb4b1991 Mon Sep 17 00:00:00 2001 From: Paul Marillonnet Date: Thu, 15 Dec 2022 10:16:55 +0100 Subject: [PATCH 3/5] forms/passwords: add phone field (#69890) --- src/authentic2/forms/passwords.py | 98 ++++++++++++++++++++++++++----- 1 file changed, 83 insertions(+), 15 deletions(-) diff --git a/src/authentic2/forms/passwords.py b/src/authentic2/forms/passwords.py index 2512c8b68..01a0aabd5 100644 --- a/src/authentic2/forms/passwords.py +++ b/src/authentic2/forms/passwords.py @@ -20,6 +20,7 @@ from collections import OrderedDict from django import forms from django.conf import settings from django.contrib.auth import forms as auth_forms +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.forms import Form from django.utils.translation import gettext_lazy as _ @@ -32,7 +33,8 @@ from .. import app_settings, models, validators from ..backends import get_user_queryset from ..utils import hooks from ..utils import misc as utils_misc -from .fields import CheckPasswordField, NewPasswordField, PasswordField, ValidatedEmailField +from ..utils import sms as utils_sms +from .fields import CheckPasswordField, NewPasswordField, PasswordField, PhoneField, ValidatedEmailField from .honeypot import HoneypotForm from .utils import NextUrlFormMixin @@ -42,14 +44,29 @@ logger = logging.getLogger(__name__) class PasswordResetForm(HoneypotForm): next_url = forms.CharField(widget=forms.HiddenInput, required=False) - email = ValidatedEmailField(label=_("Email")) + email = ValidatedEmailField(label=_("Email"), required=False) + + phone = PhoneField( + label=_('Phone number'), + help_text=_('Your mobile phone number.'), + required=False, + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.users = [] if app_settings.A2_USER_CAN_RESET_PASSWORD_BY_USERNAME: del self.fields['email'] - self.fields['email_or_username'] = forms.CharField(label=_('Email or username'), max_length=254) + self.fields['email_or_username'] = forms.CharField( + label=_('Email or username'), max_length=254, required=False + ) + + if not app_settings.A2_ACCEPT_PHONE_AUTHENTICATION or not get_user_model()._meta.get_field('phone'): + del self.fields['phone'] + if 'email' in self.fields: + self.fields['email'].required = True + else: + self.fields['email_or_username'].required = True def clean_email(self): email = self.cleaned_data.get('email') @@ -71,25 +88,44 @@ class PasswordResetForm(HoneypotForm): self.cleaned_data['email'] = email_or_username return email_or_username + def clean_phone(self): + phone = self.cleaned_data.get('phone') + if phone: + self.users = get_user_queryset().filter(phone=phone) + return phone + def clean(self): - if self.users and not any(user.email for user in self.users): + if app_settings.A2_ACCEPT_PHONE_AUTHENTICATION and get_user_model()._meta.get_field('phone'): + if ( + not self.cleaned_data['email'] + and not self.cleaned_data.get('email_or_username') + and not self.cleaned_data['phone'] + ): + raise ValidationError(_('Please provide an email address or a mobile phone number.')) + elif self.users and not any(user.email for user in self.users): raise ValidationError(_('Your account has no email, you cannot ask for a password reset.')) return self.cleaned_data def save(self): """ - Generates a one-use only link for resetting password and sends to the - user. + Generates either: + · a one-use only link for resetting password and sends to the user. + · a code sent by SMS which the user needs to input in order to confirm password reset. """ email = self.cleaned_data.get('email') email_or_username = self.cleaned_data.get('email_or_username') + phone = self.cleaned_data.get('phone') active_users = self.users.filter(is_active=True) email_sent = False + sms_sent = False for user in active_users: - if not user.email: - logger.info('password reset failed for account "%r": account has no email', user) + if not user.email and not user.phone: + logger.info( + 'password reset failed for account "%r": account has no email nor mobile phone number', + user, + ) continue if user.userexternalid_set.exists(): @@ -116,15 +152,34 @@ class PasswordResetForm(HoneypotForm): # we don't set the password to a random string, as some users should not have # a password set_random_password = user.has_usable_password() and app_settings.A2_SET_RANDOM_PASSWORD_ON_RESET - email_sent = True - utils_misc.send_password_reset_mail( - user, set_random_password=set_random_password, next_url=self.cleaned_data.get('next_url') - ) journal.record('user.password.reset.request', email=user.email, user=user) + if email or email_or_username: + email_sent = True + utils_misc.send_password_reset_mail( + user, set_random_password=set_random_password, next_url=self.cleaned_data.get('next_url') + ) + elif phone: + try: + sms_sent = True + code = utils_sms.send_password_reset_sms( + phone, + user.ou, + user=user, + ) + except utils_sms.SMSError: + pass + else: + # all user info sending logic contained here, however the view needs to know + # which code was sent: + return code + for user in self.users.filter(is_active=False): logger.info('password reset failed for user "%r": account is disabled', user) - email_sent = True - utils_misc.send_templated_mail(user, ['authentic2/password_reset_refused']) + if email or email_or_username: + email_sent = True + code = utils_misc.send_templated_mail(user, ['authentic2/password_reset_refused']) + elif phone: + sms_sent = True if not email_sent and email: logger.info('password reset request for "%s", no user found', email) if getattr(settings, 'REGISTRATION_OPEN', True): @@ -139,7 +194,20 @@ class PasswordResetForm(HoneypotForm): else: ctx = {} utils_misc.send_templated_mail(email, ['authentic2/password_reset_no_account'], context=ctx) - hooks.call_hooks('event', name='password-reset', email=email or email_or_username, users=active_users) + hooks.call_hooks( + 'event', name='password-reset', email=email or email_or_username, users=active_users + ) + elif not email_sent and not sms_sent and phone: + try: + code = utils_sms.send_password_reset_sms( + phone, + ou=None, + user=None, + ) + except utils_sms.SMSError: + pass + else: + return code class PasswordResetMixin(Form): -- 2.39.2 From af8adcef7181d08ec70c5252d9c9770ba35faf47 Mon Sep 17 00:00:00 2001 From: Paul Marillonnet Date: Tue, 20 Dec 2022 11:31:38 +0100 Subject: [PATCH 4/5] views: handle phone input on pw reset view (#69890) --- src/authentic2/views.py | 123 ++++++++++++++++++++++++++++------------ 1 file changed, 87 insertions(+), 36 deletions(-) diff --git a/src/authentic2/views.py b/src/authentic2/views.py index 5d25a08bd..bea50475c 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -848,9 +848,13 @@ class PasswordResetView(FormView): form_class = passwords_forms.PasswordResetForm title = _('Password Reset') + code = None def get_success_url(self): - return reverse('password_reset_instructions') + if not app_settings.A2_ACCEPT_PHONE_AUTHENTICATION or not self.code: # user input is email + return reverse('password_reset_instructions') + else: # user input is phone number + return reverse('input_registration_code', kwargs={'token': self.code.url_token}) def get_template_names(self): return [ @@ -882,6 +886,7 @@ class PasswordResetView(FormView): ) email_field = 'email_or_username' if app_settings.A2_USER_CAN_RESET_PASSWORD_BY_USERNAME else 'email' email = form.cleaned_data.get(email_field) + phone = form.cleaned_data.get('phone') # if an email has already been sent, warn once before allowing resend token = models.Token.objects.filter( @@ -901,41 +906,87 @@ class PasswordResetView(FormView): return self.form_invalid(form) self.request.session[resend_key] = False - if is_ratelimited( - self.request, - key='post:email', - group='pw-reset-email', - rate=app_settings.A2_EMAILS_ADDRESS_RATELIMIT, - increment=True, - ): - self.request.journal.record('user.password.reset.failure', email=email) - form.add_error( - email_field, - _( - 'Multiple emails have already been sent to this address. Further attempts are blocked,' - ' please check your spam folder or try again later.' - ), - ) - return self.form_invalid(form) - if is_ratelimited( - self.request, - key='ip', - group='pw-reset-email', - rate=app_settings.A2_EMAILS_IP_RATELIMIT, - increment=True, - ): - self.request.journal.record('user.password.reset.failure', email=email) - form.add_error( - email_field, - _( - 'Multiple password reset attempts have already been made from this IP address. No further' - ' email will be sent, please check your spam folder or try again later.' - ), - ) - return self.form_invalid(form) + if email: + if is_ratelimited( + self.request, + key='post:email', + group='pw-reset-email', + rate=app_settings.A2_EMAILS_ADDRESS_RATELIMIT, + increment=True, + ): + self.request.journal.record('user.password.reset.failure', email=email) + form.add_error( + email_field, + _( + 'Multiple emails have already been sent to this address. Further attempts are blocked,' + ' please check your spam folder or try again later.' + ), + ) + return self.form_invalid(form) + if is_ratelimited( + self.request, + key='ip', + group='pw-reset-email', + rate=app_settings.A2_EMAILS_IP_RATELIMIT, + increment=True, + ): + self.request.journal.record('user.password.reset.failure', email=email) + form.add_error( + email_field, + _( + 'Multiple password reset attempts have already been made from this IP address. No further' + ' email will be sent, please check your spam folder or try again later.' + ), + ) + return self.form_invalid(form) + form.save() - form.save() - self.request.session['reset_email'] = email + elif phone: + if is_ratelimited( + self.request, + key=sms_ratelimit_key, + group='pw-reset-sms', + rate=app_settings.A2_SMS_NUMBER_RATELIMIT, + increment=True, + ): + form.add_error( + 'phone', + _( + 'Multiple SMSs have already been sent to this number. Further attempts are blocked,' + ' try again later.' + ), + ) + return self.form_invalid(form) + if is_ratelimited( + self.request, + key='ip', + group='pw-reset-sms', + rate=app_settings.A2_SMS_IP_RATELIMIT, + increment=True, + ): + form.add_error( + 'email', + _( + 'Multiple registration attempts have already been made from this IP address. No further' + ' SMS will be sent for now, try again later.' + ), + ) + return self.form_invalid(form) + + self.code = form.save() + if not self.code: + messages.error( + self.request, + _( + 'Something went wrong while trying to send the SMS code to you. ' + 'Please contact your administrator and try again later.' + ), + ) + return utils_misc.redirect(self.request, reverse('auth_homepage')) + if email: + self.request.session['reset_email'] = email + elif phone: + self.request.session['reset_phone'] = phone return super().form_valid(form) @@ -1092,7 +1143,7 @@ class BaseRegistrationView(HomeURLMixin, FormView): if email: return self.perform_email_registration(form, email) - if settings.A2_ACCEPT_PHONE_AUTHENTICATION: + if app_settings.A2_ACCEPT_PHONE_AUTHENTICATION: phone = form.cleaned_data.pop('phone') return self.perform_phone_registration(form, phone) -- 2.39.2 From ba9550511e0b42c794b42fc7e4e97935df17aa7d Mon Sep 17 00:00:00 2001 From: Paul Marillonnet Date: Wed, 21 Dec 2022 10:42:16 +0100 Subject: [PATCH 5/5] provide generic input code logic (#69890) Suited to both registration & password-change actions. --- src/authentic2/forms/registration.py | 8 +- ...stration_code.html => sms_input_code.html} | 2 +- src/authentic2/urls.py | 4 +- src/authentic2/views.py | 48 ++++--- tests/test_password_reset.py | 124 ++++++++++++++++++ tests/test_registration.py | 16 +-- 6 files changed, 166 insertions(+), 36 deletions(-) rename src/authentic2/templates/registration/{sms_input_registration_code.html => sms_input_code.html} (87%) diff --git a/src/authentic2/forms/registration.py b/src/authentic2/forms/registration.py index 0e71cf2d5..eaf2438a2 100644 --- a/src/authentic2/forms/registration.py +++ b/src/authentic2/forms/registration.py @@ -195,9 +195,9 @@ class RegistrationCompletionForm(RegistrationCompletionFormNoPassword): return self.cleaned_data -class InputRegistrationCodeForm(Form): - registration_code = CharField( - label=_('Registration code'), - help_text=_('The registration code you received by SMS.'), +class InputSMSCodeForm(Form): + sms_code = CharField( + label=_('SMS code'), + help_text=_('The code you received by SMS.'), max_length=settings.SMS_CODE_LENGTH, ) diff --git a/src/authentic2/templates/registration/sms_input_registration_code.html b/src/authentic2/templates/registration/sms_input_code.html similarity index 87% rename from src/authentic2/templates/registration/sms_input_registration_code.html rename to src/authentic2/templates/registration/sms_input_code.html index 55b12e157..eee90004c 100644 --- a/src/authentic2/templates/registration/sms_input_registration_code.html +++ b/src/authentic2/templates/registration/sms_input_code.html @@ -7,7 +7,7 @@ {% block content %}
-

{% blocktrans trimmed %}Input your account activation code.{% endblocktrans %}

+

{% blocktrans trimmed %}Input the code you received by SMS.{% endblocktrans %}

{% blocktrans count counter=duration %} Your code is valid for the next minute. diff --git a/src/authentic2/urls.py b/src/authentic2/urls.py index 8baa2669d..66c32071a 100644 --- a/src/authentic2/urls.py +++ b/src/authentic2/urls.py @@ -116,8 +116,8 @@ urlpatterns = [ ), re_path( '^register/input_code/(?P[A-Za-z0-9_ -]+)/$', - views.input_registration_code, - name='input_registration_code', + views.input_sms_code, + name='input_sms_code', ), # Password reset re_path( diff --git a/src/authentic2/views.py b/src/authentic2/views.py index bea50475c..ceb008a3a 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -854,7 +854,7 @@ class PasswordResetView(FormView): if not app_settings.A2_ACCEPT_PHONE_AUTHENTICATION or not self.code: # user input is email return reverse('password_reset_instructions') else: # user input is phone number - return reverse('input_registration_code', kwargs={'token': self.code.url_token}) + return reverse('input_sms_code', kwargs={'token': self.code.url_token}) def get_template_names(self): return [ @@ -1208,7 +1208,7 @@ class BaseRegistrationView(HomeURLMixin, FormView): messages.warning( self.request, _( - 'Something went wrong while trying to send the SMS registration code to you.' + 'Something went wrong while trying to send the SMS code to you.' ' Please contact your administrator and try again later.' ), ) @@ -1217,7 +1217,7 @@ class BaseRegistrationView(HomeURLMixin, FormView): self.request.session['registered_phone'] = phone return utils_misc.redirect( self.request, - reverse('input_registration_code', kwargs={'token': code.url_token}), + reverse('input_sms_code', kwargs={'token': code.url_token}), params={REDIRECT_FIELD_NAME: self.next_url, 'token': code.url_token}, ) @@ -1296,20 +1296,18 @@ class BaseRegistrationView(HomeURLMixin, FormView): return context -class InputRegistrationCodeView(cbv.ValidateCSRFMixin, FormView): - template_name = 'registration/sms_input_registration_code.html' - form_class = registration_forms.InputRegistrationCodeForm +class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView): + template_name = 'registration/sms_input_code.html' + form_class = registration_forms.InputSMSCodeForm success_url = '/accounts/' - title = _('Account activation') + title = _('SMS code validation') def dispatch(self, request, *args, **kwargs): token = kwargs.get('token') try: self.code = models.SMSCode.objects.get(url_token=token) except models.SMSCode.DoesNotExist: - return HttpResponseBadRequest(_('Invalid account activation request')) - if not self.code.sent: - return HttpResponseBadRequest(_('Invalid account activation code')) + return HttpResponseBadRequest(_('Invalid request')) return super().dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): @@ -1326,35 +1324,43 @@ class InputRegistrationCodeView(cbv.ValidateCSRFMixin, FormView): @atomic(savepoint=False) def form_valid(self, form): super().form_valid(form) - registration_code = form.cleaned_data.pop('registration_code') - if self.code.value != registration_code: + sms_code = form.cleaned_data.pop('sms_code') + if self.code.value != sms_code or self.code.fake: # TODO ratelimit on erroneous code inputs(?) # (code expires after 120 seconds) - form.add_error('registration_code', _('Wrong registration code.')) + form.add_error('sms_code', _('Wrong SMS code.')) return self.form_invalid(form) if self.code.expires < timezone.now(): - form.add_error('registration_code', _('The code has expired.')) + form.add_error('sms_code', _('The code has expired.')) return self.form_invalid(form) Lock.lock_identifier(self.code.phone) content = { # TODO missing ou registration management 'authentication_method': 'phone', 'phone': self.code.phone, + 'user': self.code.user.pk if self.code.user else None, } # create token to process final account activation and user-defined attributes token = models.Token.create( - kind='registration', + kind=self.code.CODE_TO_TOKEN_KINDS[self.code.kind], content=content, duration=120, ) - return utils_misc.redirect( - # TODO next_url management throughout account creation process - self.request, - reverse('registration_activate', kwargs={'registration_token': token.uuid}), - ) + + # TODO next_url management throughout account creation process + if self.code.kind == models.SMSCode.KIND_REGISTRATION: + return utils_misc.redirect( + self.request, + reverse('registration_activate', kwargs={'registration_token': token.uuid}), + ) + elif self.code.kind == models.SMSCode.KIND_PASSWORD_LOST: + return utils_misc.redirect( + self.request, + reverse('password_reset_confirm', kwargs={'token': token.uuid}), + ) -input_registration_code = InputRegistrationCodeView.as_view() +input_sms_code = InputSMSCodeView.as_view() class RegistrationView(cbv.ValidateCSRFMixin, BaseRegistrationView): diff --git a/tests/test_password_reset.py b/tests/test_password_reset.py index 05ef9c7c5..dedcad084 100644 --- a/tests/test_password_reset.py +++ b/tests/test_password_reset.py @@ -13,15 +13,33 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . + +import json + import pytest +from django.contrib.auth import authenticate from django.test.utils import override_settings from django.urls import reverse +from httmock import HTTMock, remember_called, urlmatch +from authentic2.models import SMSCode, Token from authentic2.utils.misc import send_password_reset_mail from . import utils +@urlmatch(netloc='foo.whatever.none') +@remember_called +def sms_service_mock(url, request): + return { + 'content': {}, + 'headers': { + 'content-type': 'application/json', + }, + 'status_code': 200, + } + + def test_send_password_reset_email(app, simple_user, mailoutbox): assert len(mailoutbox) == 0 with utils.run_on_commit_hooks(): @@ -41,6 +59,112 @@ def test_send_password_reset_email(app, simple_user, mailoutbox): utils.assert_event('user.password.reset', user=simple_user, session=app.session) +def test_send_password_reset_by_sms_code_improperly_configured(app, nomail_user, settings): + settings.A2_ACCEPT_PHONE_AUTHENTICATION = True + settings.SMS_URL = 'https://foo.whatever.none/' + + assert not SMSCode.objects.count() + assert not Token.objects.count() + + url = reverse('password_reset') + resp = app.get(url, status=200) + resp.form.set('phone_1', '0123456789') + resp = resp.form.submit().follow().maybe_follow() + assert 'Something went wrong while trying to send' in resp.pyquery('li.error').text() + + +def test_send_password_reset_by_sms_code(app, nomail_user, settings): + settings.A2_ACCEPT_PHONE_AUTHENTICATION = True + settings.SMS_URL = 'https://foo.whatever.none/' + + code_length = settings.SMS_CODE_LENGTH + assert not SMSCode.objects.count() + assert not Token.objects.count() + + url = reverse('password_reset') + resp = app.get(url, status=200) + resp.form.set('phone_1', '0123456789') + with HTTMock(sms_service_mock): + resp = resp.form.submit().follow().maybe_follow() + body = json.loads(sms_service_mock.call['requests'][0].body) + assert body['message'].startswith('Your code is') + code = SMSCode.objects.get() + assert body['message'][-code_length:] == code.value + assert ("Your code is valid for the next %s minute" % (SMSCode.CODE_DURATION // 60)) in resp.text + assert "The code you received by SMS." in resp.text + resp.form.set('sms_code', code.value) + resp = resp.form.submit().follow() + assert Token.objects.count() == 1 + + assert authenticate(username='user', password='1234==aA') is None + resp.form.set('new_password1', '1234==aA') + resp.form.set('new_password2', '1234==aA') + resp.form.submit() + # verify user is logged + assert str(app.session['_auth_user_id']) == str(nomail_user.pk) + user = authenticate(username='user', password='1234==aA') + assert user == nomail_user + + with override_settings(A2_USER_CAN_RESET_PASSWORD=False): + url = reverse('password_reset') + app.get(url, status=404) + + +def test_password_reset_empty_form(app, db, settings): + settings.A2_ACCEPT_PHONE_AUTHENTICATION = True + settings.SMS_URL = 'https://foo.whatever.none/' + + url = reverse('password_reset') + resp = app.get(url, status=200) + resp = resp.form.submit() + assert 'There were errors processing your form.' in resp.pyquery('div.errornotice').text() + assert ( + 'Please provide an email address or a mobile phone number.' in resp.pyquery('div.errornotice').text() + ) + + +def test_password_reset_both_fields_filled_email_precedence(app, simple_user, settings, mailoutbox): + settings.A2_ACCEPT_PHONE_AUTHENTICATION = True + settings.SMS_URL = 'https://foo.whatever.none/' + + url = reverse('password_reset') + resp = app.get(url, status=200) + resp.form.set('email', simple_user.email) + resp.form.set('phone_1', '0123456789') + resp = resp.form.submit() + utils.assert_event('user.password.reset.request', user=simple_user, email=simple_user.email) + assert resp['Location'].endswith('/instructions/') + resp = resp.follow() + assert len(mailoutbox) == 1 + assert not SMSCode.objects.count() + + +def test_send_password_reset_by_sms_code_erroneous_phone_number(app, nomail_user, settings): + settings.A2_ACCEPT_PHONE_AUTHENTICATION = True + settings.SMS_URL = 'https://foo.whatever.none/' + + assert not SMSCode.objects.count() + assert not Token.objects.count() + + url = reverse('password_reset') + resp = app.get(url, status=200) + resp.form.set('phone_1', '0111111111') + resp = resp.form.submit().follow().maybe_follow() + assert 'Something went wrong while trying to send' not in resp.text + assert 'error' not in resp.text + assert resp.pyquery('title').text() == 'Authentic2 - testserver - SMS code validation' + code = SMSCode.objects.get() + assert code.fake + resp.form.set('sms_code', 'whatever') + resp = resp.form.submit() + assert resp.pyquery('ul.errorlist').text() == 'Wrong SMS code.' + # even if the correct value is guessed, the code is still fake & not valid whatsoever + resp.form.set('sms_code', code.value) + resp = resp.form.submit() + assert resp.pyquery('ul.errorlist').text() == 'Wrong SMS code.' + assert not Token.objects.count() + + def test_reset_by_email(app, simple_user, mailoutbox, settings): url = reverse('password_reset') resp = app.get(url, status=200) diff --git a/tests/test_registration.py b/tests/test_registration.py index 1561bf660..eb3eee9ad 100644 --- a/tests/test_registration.py +++ b/tests/test_registration.py @@ -978,10 +978,10 @@ def test_phone_registration_wrong_code(app, db, settings): resp.form.set('phone_1', '612345678') with HTTMock(sms_service_mock): resp = resp.form.submit().follow() - resp.form.set('registration_code', 'abc') + resp.form.set('sms_code', 'abc') resp = resp.form.submit() assert not Token.objects.count() - assert resp.pyquery('li')[0].text_content() == 'Wrong registration code.' + assert resp.pyquery('li')[0].text_content() == 'Wrong SMS code.' def test_phone_registration_expired_code(app, db, settings, freezer): @@ -993,7 +993,7 @@ def test_phone_registration_expired_code(app, db, settings, freezer): with HTTMock(sms_service_mock): resp = resp.form.submit().follow() code = SMSCode.objects.get() - resp.form.set('registration_code', code.value) + resp.form.set('sms_code', code.value) freezer.move_to(timedelta(hours=1)) resp = resp.form.submit() assert not Token.objects.count() @@ -1009,7 +1009,7 @@ def test_phone_registration_cancel(app, db, settings, freezer): with HTTMock(sms_service_mock): resp = resp.form.submit().follow() code = SMSCode.objects.get() - resp.form.set('registration_code', code.value) + resp.form.set('sms_code', code.value) resp.form.submit('cancel').follow() assert not Token.objects.count() assert not SMSCode.objects.count() @@ -1026,7 +1026,7 @@ def test_phone_registration_improperly_configured(app, db, settings, freezer, ca assert not Token.objects.count() assert not SMSCode.objects.count() assert ( - "Something went wrong while trying to send the SMS registration code to you" + "Something went wrong while trying to send the SMS code to you" in resp.pyquery('li.warning')[0].text_content() ) assert caplog.records[0].message == 'settings.SMS_URL is not set' @@ -1047,7 +1047,7 @@ def test_phone_registration_connection_error(app, db, settings, freezer, caplog) mock_send.return_value = mock_response resp = resp.form.submit().follow().maybe_follow() assert ( - "Something went wrong while trying to send the SMS registration code to you" + "Something went wrong while trying to send the SMS code to you" in resp.pyquery('li.warning')[0].text_content() ) assert ( @@ -1072,8 +1072,8 @@ def test_phone_registration(app, db, settings): code = SMSCode.objects.get() assert body['message'][-code_length:] == code.value assert ("Your code is valid for the next %s minute" % (SMSCode.CODE_DURATION // 60)) in resp.text - assert "The registration code you received by SMS." in resp.text - resp.form.set('registration_code', code.value) + assert "The code you received by SMS." in resp.text + resp.form.set('sms_code', code.value) resp = resp.form.submit().follow() assert Token.objects.count() == 1 -- 2.39.2