diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index dab5ce374..a27c955b9 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -21,6 +21,7 @@ from ietf.person.models import Person, Email from ietf.mailinglists.models import Allowlisted from ietf.utils.text import isascii + class RegistrationForm(forms.Form): email = forms.EmailField(label="Your email (lowercase)") @@ -193,10 +194,18 @@ class ResetPasswordForm(forms.Form): username = forms.EmailField(label="Your email (lowercase)") def clean_username(self): - import ietf.ietfauth.views + """Verify that the username is valid + + In addition to EmailField's checks, verifies that a User matching the username exists. + """ username = self.cleaned_data["username"] if not User.objects.filter(username=username).exists(): - raise forms.ValidationError(mark_safe("Didn't find a matching account. If you don't have an account yet, you can create one.".format(urlreverse(ietf.ietfauth.views.create_account)))) + raise forms.ValidationError(mark_safe( + "Didn't find a matching account. " + "If you don't have an account yet, you can create one.".format( + urlreverse('ietf.ietfauth.views.create_account') + ) + )) return username diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 9fd75d06d..4821bdb36 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -414,12 +414,10 @@ class IetfAuthTests(TestCase): email = 'someone@example.com' password = 'foobar' - user = User.objects.create(username=email, email=email) + user = PersonFactory(user__email=email).user user.set_password(password) user.save() - p = Person.objects.create(name="Some One", ascii="Some One", user=user) - Email.objects.create(address=user.username, person=p, origin=user.username) - + # get r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -497,6 +495,39 @@ class IetfAuthTests(TestCase): r = self.client.get(confirm_url) self.assertEqual(r.status_code, 404) + def test_reset_password_without_person(self): + """No password reset for account without a person""" + url = urlreverse('ietf.ietfauth.views.password_reset') + user = UserFactory() + user.set_password('some password') + user.save() + empty_outbox() + r = self.client.post(url, { 'username': user.username}) + self.assertContains(r, 'No known active email addresses', status_code=200) + q = PyQuery(r.content) + self.assertTrue(len(q("form .is-invalid")) > 0) + self.assertEqual(len(outbox), 0) + + def test_reset_password_address_handling(self): + """Reset password links are only sent to known, active addresses""" + url = urlreverse('ietf.ietfauth.views.password_reset') + person = PersonFactory() + person.email_set.update(active=False) + empty_outbox() + r = self.client.post(url, { 'username': person.user.username}) + self.assertContains(r, 'No known active email addresses', status_code=200) + q = PyQuery(r.content) + self.assertTrue(len(q("form .is-invalid")) > 0) + self.assertEqual(len(outbox), 0) + + active_address = EmailFactory(person=person).address + r = self.client.post(url, {'username': person.user.username}) + self.assertNotContains(r, 'No known active email addresses', status_code=200) + self.assertEqual(len(outbox), 1) + to = outbox[0].get('To') + self.assertIn(active_address, to) + self.assertNotIn(person.user.username, to) + def test_review_overview(self): review_req = ReviewRequestFactory() assignment = ReviewAssignmentFactory(review_request=review_req,reviewer=EmailFactory(person__user__username='reviewer')) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index f89bc0149..01a43672d 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -413,32 +413,39 @@ def password_reset(request): if request.method == 'POST': form = ResetPasswordForm(request.POST) if form.is_valid(): - username = form.cleaned_data['username'] + submitted_username = form.cleaned_data['username'] + # The form validation checks that a matching User exists. Add the person__isnull check + # because the OneToOne field does not gracefully handle checks for user.person is Null. + # If we don't get a User here, we know it's because there's no related Person. + user = User.objects.filter(username=submitted_username, person__isnull=False).first() + if not (user and user.person.email_set.filter(active=True).exists()): + form.add_error( + 'username', + 'No known active email addresses are associated with this account. ' + 'Please contact the secretariat for assistance.', + ) + else: + data = { + 'username': user.username, + 'password': user.password and user.password[-4:], + 'last_login': user.last_login.timestamp() if user.last_login else None, + } + auth = django.core.signing.dumps(data, salt="password_reset") - data = { 'username': username } - if User.objects.filter(username=username).exists(): - user = User.objects.get(username=username) - data['password'] = user.password and user.password[-4:] - if user.last_login: - data['last_login'] = user.last_login.timestamp() - else: - data['last_login'] = None - - auth = django.core.signing.dumps(data, salt="password_reset") - - domain = Site.objects.get_current().domain - subject = 'Confirm password reset at %s' % domain - from_email = settings.DEFAULT_FROM_EMAIL - to_email = username # form validation makes sure that this is an email address - - send_mail(request, to_email, from_email, subject, 'registration/password_reset_email.txt', { - 'domain': domain, - 'auth': auth, - 'username': username, - 'expire': settings.MINUTES_TO_EXPIRE_RESET_PASSWORD_LINK, - }) - - success = True + domain = Site.objects.get_current().domain + subject = 'Confirm password reset at %s' % domain + from_email = settings.DEFAULT_FROM_EMAIL + # Send email to addresses from the database, NOT to the address from the form. + # This prevents unicode spoofing tricks (https://nvd.nist.gov/vuln/detail/CVE-2019-19844). + to_emails = list(set(email.address for email in user.person.email_set.filter(active=True))) + to_emails.sort() + send_mail(request, to_emails, from_email, subject, 'registration/password_reset_email.txt', { + 'domain': domain, + 'auth': auth, + 'username': submitted_username, + 'expire': settings.MINUTES_TO_EXPIRE_RESET_PASSWORD_LINK, + }) + success = True else: form = ResetPasswordForm() return render(request, 'registration/password_reset.html', {