diff --git a/confirmation/models.py b/confirmation/models.py index 2f04b1fd52..87d91b125f 100644 --- a/confirmation/models.py +++ b/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) diff --git a/confirmation/views.py b/confirmation/views.py index d0aa65b622..7bd91ee18c 100644 --- a/confirmation/views.py +++ b/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)}) diff --git a/templates/confirmation/confirm.html b/templates/confirmation/confirm.html deleted file mode 100644 index 1ca44f3026..0000000000 --- a/templates/confirmation/confirm.html +++ /dev/null @@ -1,27 +0,0 @@ -{% extends "zerver/portico.html" %} - -{% block portico_content %} - -
-
- {% if confirmed %} -

You're confirmed. We're not exactly sure what we confirmed you for, but whatever it is you're totally good.

- {% else %} - -

- Whoops, something's not right. We couldn't find your confirmation ID, - or maybe the link expired. -

- -

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.

- -

Anyway, shoot us a line at {{ support_email }} and we'll get - this resolved shortly. -

- - {% endif %} -
- -{% endblock %} diff --git a/templates/confirmation/confirm_email_change.html b/templates/confirmation/confirm_email_change.html index b901bb6698..edc7bde2ed 100644 --- a/templates/confirmation/confirm_email_change.html +++ b/templates/confirmation/confirm_email_change.html @@ -4,31 +4,10 @@

- {% if confirmed %} -

- This confirms that the email address for your Zulip account has changed - from {{old_email}} to {{ new_email }}. -

- {% else %} -

Whoops, something's not right. We couldn't find your confirmation ID!

- - {% if verbose_support_offers %} -

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.

- -

Anyway, shoot us a line at - {{ support_email }} - and we'll get this resolved shortly. -

- {% else %} -

Make sure you copied the link correctly in to your browser.

- -

If you're still having problems, please contact your Zulip administrator at - {{ support_email }}. -

- {% endif %} - - {% endif %} +

+ This confirms that the email address for your Zulip account has changed + from {{old_email}} to {{ new_email }}. +

{% endblock %} diff --git a/zerver/tests/test_email_change.py b/zerver/tests/test_email_change.py index 8bb5b58194..92174b8465 100644 --- a/zerver/tests/test_email_change.py +++ b/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 diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 4a9bdab542..5cca585768 100644 --- a/zerver/views/user_settings.py +++ b/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, }