password reset: decide on which phone attribute to use (#78046)

This commit is contained in:
Paul Marillonnet 2023-06-19 16:33:02 +02:00
parent 795f04ea7d
commit 053899cb4f
4 changed files with 92 additions and 6 deletions

View File

@ -20,6 +20,7 @@ from collections import OrderedDict
from django import forms
from django.contrib.auth import forms as auth_forms
from django.contrib.auth import get_user_model
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError
from django.forms import Form
from django.utils.translation import gettext_lazy as _
@ -90,8 +91,14 @@ class PasswordResetForm(HoneypotForm):
def clean_phone(self):
phone = self.cleaned_data.get('phone')
user_ct = ContentType.objects.get_for_model(get_user_model())
if phone:
self.users = get_user_queryset().filter(phone=phone)
user_ids = models.AttributeValue.objects.filter(
attribute=self.authenticator.phone_identifier_field,
content=phone,
content_type=user_ct,
).values_list('object_id', flat=True)
self.users = get_user_queryset().filter(id__in=user_ids)
return phone
def clean(self):
@ -120,8 +127,18 @@ class PasswordResetForm(HoneypotForm):
email_sent = False
sms_sent = False
phone_authn_active = self.authenticator.is_phone_authn_active
user_ct = ContentType.objects.get_for_model(get_user_model())
for user in active_users:
if not user.email and not user.phone:
if not user.email and not (
phone_authn_active
and models.AttributeValue.objects.filter(
attribute=self.authenticator.phone_identifier_field,
object_id=user.id,
content_type=user_ct,
content__isnull=False,
)
):
logger.info(
'password reset failed for account "%r": account has no email nor mobile phone number',
user,

View File

@ -904,6 +904,7 @@ class PasswordResetView(FormView):
phone = form.cleaned_data.get('phone')
# if an email has already been sent, warn once before allowing resend
# TODO handle multiple SMS warning message
token = models.Token.objects.filter(
kind='pw-reset', content__email__iexact=email, expires__gt=timezone.now()
).exists()
@ -920,6 +921,7 @@ class PasswordResetView(FormView):
)
return self.form_invalid(form)
self.request.session[resend_key] = False
self.request.session['phone'] = phone
if email:
if is_ratelimited(

View File

@ -23,7 +23,7 @@ from django.urls import reverse
from httmock import HTTMock, remember_called, urlmatch
from authentic2.apps.authenticators.models import LoginPasswordAuthenticator
from authentic2.models import SMSCode, Token
from authentic2.models import Attribute, SMSCode, Token
from authentic2.utils.misc import send_password_reset_mail
from . import utils
@ -115,6 +115,69 @@ def test_send_password_reset_by_sms_code(app, nomail_user, settings, phone_activ
app.get(url, status=404)
def test_send_password_reset_by_sms_code_nondefault_attribute(app, nomail_user, simple_user, settings):
phone, dummy = Attribute.objects.get_or_create(
name='another_phone',
kind='phone_number',
defaults={'label': 'Another phone'},
)
LoginPasswordAuthenticator.objects.update(
accept_phone_authentication=True,
phone_identifier_field=phone,
)
nomail_user.attributes.another_phone = '+33122446688'
nomail_user.phone = ''
nomail_user.save()
settings.SMS_URL = 'https://foo.whatever.none/'
url = reverse('password_reset')
resp = app.get(url, status=200)
resp.form.set('phone_1', '0122446688')
with HTTMock(sms_service_mock):
resp = resp.form.submit().follow().maybe_follow()
json.loads(sms_service_mock.call['requests'][0].body)
code = SMSCode.objects.get()
resp.form.set('sms_code', code.value)
resp = resp.form.submit().follow()
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
SMSCode.objects.all().delete()
simple_user.attributes.another_phone = '+33122446677'
simple_user.phone = '+33122112211'
simple_user.username = 'simpleuser'
simple_user.save()
url = reverse('password_reset')
resp = app.get(url, status=200)
resp.form.set('phone_1', '0122446677')
with HTTMock(sms_service_mock):
resp = resp.form.submit().follow().maybe_follow()
json.loads(sms_service_mock.call['requests'][0].body)
code = SMSCode.objects.get()
resp.form.set('sms_code', code.value)
resp = resp.form.submit().follow()
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(simple_user.pk)
user = authenticate(username='simpleuser', password='1234==aA')
assert user == simple_user
with override_settings(A2_USER_CAN_RESET_PASSWORD=False):
url = reverse('password_reset')
app.get(url, status=404)
def test_send_password_reset_by_sms_code_next_url(app, nomail_user, settings, phone_activated_authn):
nomail_user.attributes.phone = '+33123456789'
nomail_user.save()

View File

@ -343,7 +343,9 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, 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"
if view_name == 'registration_register': # XXX not supported in password_reset yet
assert 'An SMS code has already been sent' in response.text
response = response.form.submit()
assert 'try again later' not in response.text
response = app.get(reverse(view_name))
@ -377,11 +379,13 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
response = response.form.submit()
assert 'try again later' not in response.text
# email ratelimits are lifted after a day
# identifier 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()
response = response.form.submit()
assert 'Multiple SMSs have already been sent to this number.' in response.text
response = response.form.submit()
assert 'try again later' in response.text
freezer.tick(datetime.timedelta(days=1))