confirmation: Use ConfirmationKeyException in get_object_from_key.

Fixes #5739.

confirmation/models.py

@@ -52,14 +52,17 @@ def generate_key():
 
 def get_object_from_key(confirmation_key):
     # type: (str) -> Union[bool, PreregistrationUser, EmailChangeStatus]
+    # Confirmation keys used to be 40 characters
+    if len(confirmation_key) not in (24, 40):
+        raise ConfirmationKeyException(ConfirmationKeyException.WRONG_LENGTH)
     try:
         confirmation = Confirmation.objects.get(confirmation_key=confirmation_key)
     except Confirmation.DoesNotExist:
-        return False
+        raise ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST)
 
     time_elapsed = timezone_now() - confirmation.date_sent
     if time_elapsed.total_seconds() > _properties[confirmation.type].validity_in_days * 24 * 3600:
-        return False
+        raise ConfirmationKeyException(ConfirmationKeyException.EXPIRED)
 
     obj = confirmation.content_object
     obj.status = getattr(settings, 'STATUS_ACTIVE', 1)

confirmation/views.py

@@ -10,7 +10,8 @@ from django.template import RequestContext
 from django.conf import settings
 from django.http import HttpRequest, HttpResponse
 
-from confirmation.models import Confirmation, get_object_from_key
+from confirmation.models import Confirmation, get_object_from_key, ConfirmationKeyException, \
+    render_confirmation_key_error
 from zerver.models import PreregistrationUser
 
 from typing import Any, Dict
@@ -19,11 +20,12 @@ from typing import Any, Dict
 # Do not add other confirmation paths here.
 def confirm(request, confirmation_key):
     # type: (HttpRequest, str) -> HttpResponse
-    obj = get_object_from_key(confirmation_key)
-    if obj:
-        return render(request, 'confirmation/confirm_preregistrationuser.html',
-                      context={
-                          'key': confirmation_key,
-                          'full_name': request.GET.get("full_name", None)})
-    else:
-        return render(request, 'confirmation/confirm.html', context = {'confirmed': False})
+    try:
+        get_object_from_key(confirmation_key)
+    except ConfirmationKeyException as exception:
+        return render_confirmation_key_error(request, exception)
+
+    return render(request, 'confirmation/confirm_preregistrationuser.html',
+                  context={
+                      'key': confirmation_key,
+                      'full_name': request.GET.get("full_name", None)})

templates/confirmation/confirm.html

@@ -1,27 +0,0 @@
-{% extends "zerver/portico.html" %}
-
-{% block portico_content %}
-
-<div class="pitch">
-    <hr/>
-    {% if confirmed %}
-    <p>You're confirmed. We're not exactly sure what we confirmed you for, but whatever it is you're totally good.</p>
-    {% else %}
-
-    <p class="lead">
-        Whoops, something's not right. We couldn't find your confirmation ID,
-        or maybe the link expired.
-    </p>
-
-    <p>Make sure you copied the link correctly in to your browser. If you're
-    still encountering this page, and you got this link recently, it's probably our fault. We're sorry.</p>
-
-    <p>Anyway, shoot us a line at <a
-        href="mailto:{{ support_email }}">{{ support_email }}</a> and we'll get
-        this resolved shortly.
-    </p>
-
-    {% endif %}
-</div>
-
-{% endblock %}

templates/confirmation/confirm_email_change.html

@@ -4,31 +4,10 @@
 
 <div class="pitch">
     <hr/>
-    {% if confirmed %}
-        <p>
-            This confirms that the email address for your Zulip account has changed
-            from {{old_email}} to {{ new_email }}.
-        </p>
-    {% else %}
-        <p class="lead">Whoops, something's not right. We couldn't find your confirmation ID!</p>
-
-        {% if verbose_support_offers %}
-        <p>Make sure you copied the link correctly in to your browser. If you're
-        still encountering this page, its probably our fault. We're sorry.</p>
-
-        <p>Anyway, shoot us a line at
-            <a href="mailto:{{ support_email }}">{{ support_email }}</a>
-            and we'll get this resolved shortly.
-        </p>
-        {% else %}
-        <p>Make sure you copied the link correctly in to your browser.</p>
-
-        <p>If you're still having problems, please contact your Zulip administrator at
-            <a href="mailto:{{ support_email }}">{{ support_email }}</a>.
-        </p>
-        {% endif %}
-
-    {% endif %}
+    <p>
+        This confirms that the email address for your Zulip account has changed
+        from {{old_email}} to {{ new_email }}.
+    </p>
 </div>
 
 {% endblock %}

zerver/tests/test_email_change.py

@@ -28,7 +28,7 @@ class EmailChangeTestCase(ZulipTestCase):
         key = generate_key()
         url = confirmation_url(key, 'testserver', Confirmation.EMAIL_CHANGE)
         response = self.client_get(url)
-        self.assert_in_success_response(["Whoops"], response)
+        self.assert_in_success_response(["Whoops. We couldn't find your confirmation link in the system."], response)
 
     def test_confirm_email_change_with_invalid_key(self):
         # type: () -> None
@@ -36,7 +36,7 @@ class EmailChangeTestCase(ZulipTestCase):
         key = 'invalid key'
         url = confirmation_url(key, 'testserver', Confirmation.EMAIL_CHANGE)
         response = self.client_get(url)
-        self.assert_in_success_response(["Whoops"], response)
+        self.assert_in_success_response(["Whoops. The confirmation link is malformed."], response)
 
     def test_email_change_when_not_logging_in(self):
         # type: () -> None
@@ -63,7 +63,7 @@ class EmailChangeTestCase(ZulipTestCase):
                                     type=Confirmation.EMAIL_CHANGE)
         url = confirmation_url(key, user_profile.realm.host, Confirmation.EMAIL_CHANGE)
         response = self.client_get(url)
-        self.assert_in_success_response(["Whoops"], response)
+        self.assert_in_success_response(["Whoops. The confirmation link has expired."], response)
 
     def test_confirm_email_change(self):
         # type: () -> None

zerver/views/user_settings.py

@@ -26,7 +26,8 @@ from zerver.lib.users import check_change_full_name
 from zerver.lib.timezone import get_all_timezones
 from zerver.models import UserProfile, Realm, name_changes_disabled, \
     EmailChangeStatus
-from confirmation.models import get_object_from_key
+from confirmation.models import get_object_from_key, render_confirmation_key_error, \
+    ConfirmationKeyException
 
 @zulip_login_required
 def confirm_email_change(request, confirmation_key):
@@ -36,26 +37,23 @@ def confirm_email_change(request, confirmation_key):
         raise JsonableError(_("Email address changes are disabled in this organization."))
 
     confirmation_key = confirmation_key.lower()
-    obj = get_object_from_key(confirmation_key)
-    confirmed = False
-    new_email = old_email = None  # type: Text
-    if obj:
-        confirmed = True
-        assert isinstance(obj, EmailChangeStatus)
-        new_email = obj.new_email
-        old_email = obj.old_email
+    try:
+        obj = get_object_from_key(confirmation_key)
+    except ConfirmationKeyException as exception:
+        return render_confirmation_key_error(request, exception)
 
-        do_change_user_email(obj.user_profile, obj.new_email)
+    assert isinstance(obj, EmailChangeStatus)
+    new_email = obj.new_email
+    old_email = obj.old_email
 
-        context = {'realm': obj.realm,
-                   'new_email': new_email,
-                   }
-        send_email('zerver/emails/notify_change_in_email', to_email=old_email,
-                   from_name="Zulip Account Security", from_address=FromAddress.SUPPORT,
-                   context=context)
+    do_change_user_email(obj.user_profile, obj.new_email)
+
+    context = {'realm': obj.realm, 'new_email': new_email}
+    send_email('zerver/emails/notify_change_in_email', to_email=old_email,
+               from_name="Zulip Account Security", from_address=FromAddress.SUPPORT,
+               context=context)
 
     ctx = {
-        'confirmed': confirmed,
         'new_email': new_email,
         'old_email': old_email,
     }