altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

fix: Don't expose existing emails via reset password and account creation forms (#5288)

Browse source 
* Rebase to feat/postgres

* Attempt to address further review comments
This commit is contained in:
Lars Eggert 2023-03-16 00:27:29 +02:00 committed by GitHub
parent 4b4e876305
commit 0faa2e40e7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 139 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
ietf/ietfauth/forms.py
View file

@ -10,8 +10,6 @@ from django.conf import settings
from django.core.exceptions import ValidationError
from django.db import models
from django.contrib.auth.models import User
from django.utils.html import mark_safe # type:ignore
from django.urls import reverse as urlreverse
from django_password_strength.widgets import PasswordStrengthInput, PasswordConfirmationInput
@ -31,8 +29,6 @@ class RegistrationForm(forms.Form):
return email
if email.lower() != email:
raise forms.ValidationError('The supplied address contained uppercase letters. Please use a lowercase email address.')
if User.objects.filter(username__iexact=email).exists():
raise forms.ValidationError('An account with the email address you provided already exists.')
return email
@ -164,11 +160,6 @@ class NewEmailForm(forms.Form):
def clean_new_email(self):
email = self.cleaned_data.get("new_email", "")
if email:
existing = Email.objects.filter(address=email).first()
if existing:
raise forms.ValidationError("Email address '%s' is already assigned to account '%s' (%s)" % (existing, existing.person and existing.person.user, existing.person))
for pat in settings.EXCLUDED_PERSONAL_EMAIL_REGEX_PATTERNS:
if re.search(pat, email):
raise ValidationError("This email address is not valid in a datatracker account")
@ -193,21 +184,6 @@ class RoleEmailForm(forms.Form):
class ResetPasswordForm(forms.Form):
username = forms.EmailField(label="Your email (lowercase)")
def clean_username(self):
"""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__iexact=username).exists():
raise forms.ValidationError(mark_safe(
"Didn't find a matching account. "
"If you don't have an account yet, you can <a href=\"{}\">create one</a>.".format(
urlreverse('ietf.ietfauth.views.create_account')
)
))
return username
class TestEmailForm(forms.Form):
email = forms.EmailField(required=False)

46
ietf/ietfauth/tests.py
View file

@ -165,7 +165,7 @@ class IetfAuthTests(TestCase):
r = render_to_string('registration/manual.html', { 'account_request_email': settings.ACCOUNT_REQUEST_EMAIL })
self.assertTrue("Additional Assistance Required" in r)
def register_and_verify(self, email):
def register(self, email):
url = urlreverse(ietf.ietfauth.views.create_account)
# register email
@ -175,6 +175,9 @@ class IetfAuthTests(TestCase):
self.assertContains(r, "Account request received")
self.assertEqual(len(outbox), 1)
def register_and_verify(self, email):
self.register(email)
# go to confirm page
confirm_url = self.extract_confirm_url(outbox[-1])
r = self.client.get(confirm_url)
@ -229,6 +232,20 @@ class IetfAuthTests(TestCase):
self.register_and_verify(email)
settings.LIST_ACCOUNT_DELAY = saved_delay
def test_create_existing_account(self):
# create account once
email = "new-account@example.com"
self.register_and_verify(email)
# create account again
self.register(email)
# check notification
note = get_payload_text(outbox[-1])
self.assertIn(email, note)
self.assertIn("A datatracker account for that email already exists", note)
self.assertIn(urlreverse(ietf.ietfauth.views.password_reset), note)
def test_ietfauth_profile(self):
EmailFactory(person__user__username='plain')
GroupFactory(acronym='mars')
@ -317,11 +334,14 @@ class IetfAuthTests(TestCase):
self.assertEqual(r.status_code, 200)
self.assertEqual(Email.objects.filter(address=new_email_address, person__user__username=username, active=1).count(), 1)
# check that we can't re-add it - that would give a duplicate
r = self.client.get(confirm_url)
# try and add it again
empty_outbox()
r = self.client.post(url, with_new_email_address)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('[name="action"][value="confirm"]')), 0)
self.assertEqual(len(outbox), 1)
note = get_payload_text(outbox[-1])
self.assertIn(new_email_address, note)
self.assertIn("already associated with your account", note)
pronoundish = base_data.copy()
pronoundish["pronouns_freetext"] = "baz/boom"
@ -395,7 +415,7 @@ class IetfAuthTests(TestCase):
"new_email": "testaddress@example.net",
}
r = self.client.post(url, data)
self.assertContains(r, "Email address &#39;TestAddress@example.net&#39; is already assigned", status_code=200)
self.assertContains(r, "A confirmation email has been sent to", status_code=200)
def test_nomcom_dressing_on_profile(self):
url = urlreverse('ietf.ietfauth.views.profile')
@ -437,11 +457,11 @@ class IetfAuthTests(TestCase):
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
# ask for reset, wrong username
# ask for reset, wrong username (form should not fail)
r = self.client.post(url, { 'username': "nobody@example.com" })
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertTrue(len(q("form .is-invalid")) > 0)
self.assertTrue(len(q("form .is-invalid")) == 0)
# ask for reset
empty_outbox()
@ -518,9 +538,9 @@ class IetfAuthTests(TestCase):
user.save()
empty_outbox()
r = self.client.post(url, { 'username': user.username})
self.assertContains(r, 'No known active email addresses', status_code=200)
self.assertContains(r, 'We have sent you an email with instructions', status_code=200)
q = PyQuery(r.content)
self.assertTrue(len(q("form .is-invalid")) > 0)
self.assertTrue(len(q("form .is-invalid")) == 0)
self.assertEqual(len(outbox), 0)
def test_reset_password_address_handling(self):
@ -530,14 +550,14 @@ class IetfAuthTests(TestCase):
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)
self.assertContains(r, 'We have sent you an email with instructions', status_code=200)
q = PyQuery(r.content)
self.assertTrue(len(q("form .is-invalid")) > 0)
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.assertContains(r, 'We have sent you an email with instructions', status_code=200)
self.assertEqual(len(outbox), 1)
to = outbox[0].get('To')
self.assertIn(active_address, to)

111
ietf/ietfauth/views.py
View file

@ -112,33 +112,51 @@ def index(request):
# redirect_to = settings.LOGIN_REDIRECT_URL
# return HttpResponseRedirect(redirect_to)
def create_account(request):
to_email = None
if request.method == 'POST':
def create_account(request):
new_account_email = None
if request.method == "POST":
form = RegistrationForm(request.POST)
if form.is_valid():
to_email = form.cleaned_data['email'] # This will be lowercase if form.is_valid()
new_account_email = form.cleaned_data[
"email"
] # This will be lowercase if form.is_valid()
# For the IETF 113 Registration period (at least) we are lowering the barriers for account creation
# to the simple email round-trip check
send_account_creation_email(request, to_email)
user = User.objects.filter(username__iexact=new_account_email)
email = Email.objects.filter(address__iexact=new_account_email)
if user.exists() or email.exists():
person_to_contact = user.first().person if user else email.first().person
to_email = person_to_contact.email_address()
if to_email:
send_account_creation_exists_email(request, new_account_email, to_email)
else:
raise ValidationError(f"Account for {{new_account_email}} exists, but cannot email it")
else:
# For the IETF 113 Registration period (at least) we are lowering the
# barriers for account creation to the simple email round-trip check
send_account_creation_email(request, new_account_email)
# The following is what to revert to should that lowered barrier prove problematic
# existing = Subscribed.objects.filter(email__iexact=to_email).first()
# ok_to_create = ( Allowlisted.objects.filter(email__iexact=to_email).exists()
# or existing and (existing.time + TimeDelta(seconds=settings.LIST_ACCOUNT_DELAY)) < DateTime.now() )
# if ok_to_create:
# send_account_creation_email(request, to_email)
# else:
# return render(request, 'registration/manual.html', { 'account_request_email': settings.ACCOUNT_REQUEST_EMAIL })
# The following is what to revert to should that lowered barrier prove problematic
# existing = Subscribed.objects.filter(email__iexact=new_account_email).first()
# ok_to_create = ( Allowlisted.objects.filter(email__iexact=new_account_email).exists()
# or existing and (existing.time + TimeDelta(seconds=settings.LIST_ACCOUNT_DELAY)) < DateTime.now() )
# if ok_to_create:
# send_account_creation_email(request, new_account_email)
# else:
# return render(request, 'registration/manual.html', { 'account_request_email': settings.ACCOUNT_REQUEST_EMAIL })
else:
form = RegistrationForm()
return render(request, 'registration/create.html', {
'form': form,
'to_email': to_email,
})
return render(
request,
"registration/create.html",
{
"form": form,
"to_email": new_account_email,
},
)
def send_account_creation_email(request, to_email):
auth = django.core.signing.dumps(to_email, salt="create_account")
@ -153,6 +171,23 @@ def send_account_creation_email(request, to_email):
})
def send_account_creation_exists_email(request, new_account_email, to_email):
domain = Site.objects.get_current().domain
subject = "Attempted account creation at %s" % domain
from_email = settings.DEFAULT_FROM_EMAIL
send_mail(
request,
to_email,
from_email,
subject,
"registration/creation_exists_email.txt",
{
"domain": domain,
"username": new_account_email,
},
)
def confirm_account(request, auth):
try:
email = django.core.signing.loads(auth, salt="create_account", max_age=settings.DAYS_TO_EXPIRE_REGISTRATION_LINK * 24 * 60 * 60)
@ -255,17 +290,25 @@ def profile(request):
auth = django.core.signing.dumps([person.user.username, to_email], salt="add_email")
domain = Site.objects.get_current().domain
subject = 'Confirm email address for %s' % person.name
from_email = settings.DEFAULT_FROM_EMAIL
send_mail(request, to_email, from_email, subject, 'registration/add_email_email.txt', {
'domain': domain,
'auth': auth,
'email': to_email,
'person': person,
'expire': settings.DAYS_TO_EXPIRE_REGISTRATION_LINK,
})
existing = Email.objects.filter(address=to_email).first()
if existing:
subject = 'Attempt to add your email address by %s' % person.name
send_mail(request, to_email, from_email, subject, 'registration/add_email_exists_email.txt', {
'domain': domain,
'email': to_email,
'person': person,
})
else:
subject = 'Confirm email address for %s' % person.name
send_mail(request, to_email, from_email, subject, 'registration/add_email_email.txt', {
'domain': domain,
'auth': auth,
'email': to_email,
'person': person,
'expire': settings.DAYS_TO_EXPIRE_REGISTRATION_LINK,
})
for r in roles:
e = r.email_form.cleaned_data["email"]
@ -417,14 +460,10 @@ def password_reset(request):
# 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.
# We still report that the action succeeded, so we're not leaking the existence of user
# email addresses.
user = User.objects.filter(username__iexact=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:
if user and user.person.email_set.filter(active=True).exists():
data = {
'username': user.username,
'password': user.password and user.password[-4:],
@ -445,7 +484,7 @@ def password_reset(request):
'username': submitted_username,
'expire': settings.MINUTES_TO_EXPIRE_RESET_PASSWORD_LINK,
})
success = True
success = True
else:
form = ResetPasswordForm()
return render(request, 'registration/password_reset.html', {

14
ietf/templates/registration/add_email_exists_email.txt Normal file
View file

@ -0,0 +1,14 @@
{% autoescape off %}{% load ietf_filters %}
Hello,
{% filter wordwrap:78 %}We have received a request to add the email address {{ email }} to the user account '{{ person.user }}' at {{ domain }}.
This email address {{ email }} is already associated with your account at {{ domain }} and cannot be associated with two accounts.{% endfilter %}
If you did not request this change, you may safely ignore this email,
as no actions have been taken.
Best regards,
The datatracker login manager service
(for the IETF Secretariat)
{% endautoescape %}

17
ietf/templates/registration/creation_exists_email.txt Normal file
View file

@ -0,0 +1,17 @@
{% autoescape off %}{% load ietf_filters %}
Hello,
{% filter wordwrap:78 %}We have received an account creation request for {{ username }} at {{ domain }}.{% endfilter %}
{% filter wordwrap:78 %}A datatracker account for that email already exists. If you have forgotten the password for the {{ username }} account, please go to the following link and follow the instructions there:{% endfilter %}
https://{{ domain }}{% url "ietf.ietfauth.views.password_reset" %}
If you have not requested the account creation you can ignore this email, your
credentials have been left untouched.
Best regards,
The datatracker login manager service
(for the IETF Secretariat)
{% endautoescape %}