fix: Don't expose existing emails via reset password and account creation forms

client/agenda/AgendaScheduleList.vue

@@ -263,7 +263,7 @@ const meetingEvents = computed(() => {
         key: `sesshd-${item.id}`,
         displayType: 'session-head',
         timeslot: itemTimeSlot,
-        name: `${item.adjustedStart.toFormat('cccc')} ${item.slotName}`,
+        name: `${item.adjustedStart.setZone(agendaStore.meeting.timezone).toFormat('cccc')} ${item.slotName}`,
         cssClasses: 'agenda-table-display-session-head' + (isLive ? ' agenda-table-live' : '')
       })
     }

ietf/doc/utils_search.py

@@ -251,7 +251,6 @@ def num(i):
     if query and hasattr(query, "urlencode"):  # fed a Django QueryDict
         d = query.copy()
         for h in meta['headers']:
-            h["sort_url"] = "?" + d.urlencode()
             if h['key'] == sort_key:
                 h['sorted'] = True
                 if sort_reversed:
@@ -262,5 +261,6 @@ def num(i):
                     d["sort"] = "-" + h["key"]
             else:
                 d["sort"] = h["key"]
+            h["sort_url"] = "?" + d.urlencode()
 
     return (docs, meta)

ietf/ietfauth/forms.py

@@ -10,8 +10,6 @@
 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 @@ def clean_email(self):
             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 @@ def __init__(self, role, *args, **kwargs):
 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)

ietf/ietfauth/tests.py

@@ -165,7 +165,7 @@ def test_create_account_failure_template(self):
         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 @@ def register_and_verify(self, email):
         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 @@ def test_create_subscribed_account(self):
         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 @@ def test_ietfauth_profile(self):
         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"
@@ -437,11 +457,11 @@ def test_reset_password(self):
         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 @@ def test_reset_password_without_person(self):
         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 @@ def test_reset_password_address_handling(self):
         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)

ietf/ietfauth/views.py

@@ -112,33 +112,47 @@ def index(request):
 #         redirect_to = settings.LOGIN_REDIRECT_URL
 #     return HttpResponseRedirect(redirect_to)
 
+
 def create_account(request):
-    to_email = None
+    new_account_email = None
 
-    if request.method == 'POST':
+    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()
-
-            # 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)
-
-            # 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 })
+            new_account_email = form.cleaned_data[
+                "email"
+            ]  # This will be lowercase if form.is_valid()
+
+            user = User.objects.filter(username__iexact=new_account_email)
+            email = Email.objects.filter(address__iexact=new_account_email)
+            if user.exists() or email.exists():
+                email = user.person.email_address() if user else new_account_email
+                send_account_creation_exists_email(request, new_account_email, email)
+            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=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 +167,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 +286,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 +456,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 +480,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', {
@@ -777,7 +812,7 @@ class Meta:
 @person_required
 def apikey_disable(request):
     person = request.user.person
-    choices = [ (k.hash(), str(k)) for k in person.apikeys.all() ]
+    choices = [ (k.hash(), str(k)) for k in person.apikeys.exclude(valid=False) ]
     #
     class KeyDeleteForm(forms.Form):
         hash = forms.ChoiceField(label='Key', choices=choices)

ietf/ipr/tests.py

@@ -591,7 +591,7 @@ def test_notify(self):
             get_payload_text(outbox[len_before + 1]).replace('\n', ' ')
         )
         self.assertIn(f'{settings.IDTRACKER_BASE_URL}{urlreverse("ietf.ipr.views.showlist")}', get_payload_text(outbox[len_before]).replace('\n',' '))
-        self.assertIn(f'{settings.IDTRACKER_BASE_URL}{urlreverse("ietf.ipr.views.history",kwargs=dict(id=ipr.pk))}', get_payload_text(outbox[len_before+1]).replace('\n',' '))
+        self.assertIn(f'{settings.IDTRACKER_BASE_URL}{urlreverse("ietf.ipr.views.show",kwargs=dict(id=ipr.pk))}', get_payload_text(outbox[len_before+1]).replace('\n',' '))
 
     def test_notify_generic(self):
         RoleFactory(name_id='ad',group__acronym='gen')

ietf/meeting/helpers.py

@@ -826,7 +826,7 @@ def get_announcement_initial(meeting, is_change=False):
         desc=desc,
         date=meeting.date,
         change=change)
-    body = render_to_string('meeting/interim_announcement.txt', locals())
+    body = render_to_string('meeting/interim_announcement.txt', locals() | {"settings": settings})
     initial['body'] = body
     return initial
 

ietf/meeting/tests_views.py

@@ -4510,10 +4510,11 @@ def do_interim_send_announcement_test(self, base_session=False, extra_session=Fa
             if sess:
                 timeslot = sess.official_timeslotassignment().timeslot
                 self.assertIn(timeslot.time.strftime('%Y-%m-%d'), announcement_text)
-                self.assertIn(
-                    '(%s to %s UTC)' % (
+                self.assertRegex(
+                    announcement_text,
+                    r'(%s\s+to\s+%s\s+UTC)' % (
                         timeslot.utc_start_time().strftime('%H:%M'),timeslot.utc_end_time().strftime('%H:%M')
-                    ), announcement_text)
+                    ))
         # Count number of sessions listed
         if base_session and extra_session:
             expected_session_matches = 3

ietf/templates/doc/document_draft.html

@@ -13,7 +13,7 @@
           title="Document changes"
           href="/feed/document-changes/{{ name }}/">
     <meta name="description"
-          content="{{ doc.title }} {% if doc.get_state_slug == 'rfc' and not snapshot %}(RFC {{ rfc_number }}{% if published %}, {{ doc.pub_date|date:'F Y' }}{% endif %}{% if obsoleted_by %}; obsoleted by {{ obsoleted_by|join:', ' }}{% endif %}){% else %}(Internet-Draft, {{ doc.time|date:'Y' }}){% endif %}">
+          content="{{ doc.title }} {% if doc.get_state_slug == 'rfc' and not snapshot %}(RFC {{ rfc_number }}{% if published %}, {{ doc.pub_date|date:'F Y' }}{% endif %}{% if obsoleted_by %}; obsoleted by {% for rel in obsoleted_by %}{{ rel.source.canonical_name|prettystdname}}{% if not forloop.last%}, {% endif %}{% endfor %}{% endif %}){% endif %}">
 {% endblock %}
 {% block morecss %}.inline { display: inline; }{% endblock %}
 {% block title %}

ietf/templates/doc/document_html.html

@@ -34,7 +34,7 @@
               href="/feed/document-changes/{{ doc.name }}/">
         <meta name="description"
             {% if not snapshot and doc.get_state_slug == 'rfc' %}
-                content="{{ doc.title }} (RFC {{ doc.rfc_number }}{% if published %}, {{ published.time|date:'F Y' }}{% endif %}{% if obsoleted_by %}; obsoleted by {{ obsoleted_by|join:', ' }}{% endif %}"
+                content="{{ doc.title }} (RFC {{ doc.rfc_number }}{% if published %}, {{ published.time|date:'F Y' }}{% endif %}{% if obsoleted_by %}; obsoleted by {% for rel in obsoleted_by %}{{ rel.source.canonical_name|prettystdname}}{% if not forloop.last%}, {% endif %}{% endfor %}{% endif %})"
             {% else %}
                 content="{{ doc.title }} (Internet-Draft, {{ doc.time|date:'Y' }})"
             {% endif %}>