diff --git a/confirmation/models.py b/confirmation/models.py index 9080aa0b41..0f966ea03e 100644 --- a/confirmation/models.py +++ b/confirmation/models.py @@ -47,13 +47,14 @@ def generate_key(): # 24 characters * 5 bits of entropy/character = 120 bits of entropy return ''.join(generator.choice(string.ascii_lowercase + string.digits) for _ in range(24)) -def get_object_from_key(confirmation_key): - # type: (str) -> Union[MultiuseInvite, PreregistrationUser, EmailChangeStatus] +def get_object_from_key(confirmation_key, confirmation_type): + # type: (str, int) -> Union[MultiuseInvite, 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) + confirmation = Confirmation.objects.get(confirmation_key=confirmation_key, + type=confirmation_type) except Confirmation.DoesNotExist: raise ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST) diff --git a/confirmation/views.py b/confirmation/views.py index 7bd91ee18c..6671013def 100644 --- a/confirmation/views.py +++ b/confirmation/views.py @@ -21,9 +21,12 @@ from typing import Any, Dict def confirm(request, confirmation_key): # type: (HttpRequest, str) -> HttpResponse try: - get_object_from_key(confirmation_key) - except ConfirmationKeyException as exception: - return render_confirmation_key_error(request, exception) + get_object_from_key(confirmation_key, Confirmation.USER_REGISTRATION) + except ConfirmationKeyException: + try: + get_object_from_key(confirmation_key, Confirmation.INVITATION) + except ConfirmationKeyException as exception: + return render_confirmation_key_error(request, exception) return render(request, 'confirmation/confirm_preregistrationuser.html', context={ diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index b7903515cc..8555fa6baa 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -11,7 +11,7 @@ from mock import patch, MagicMock from zerver.lib.test_helpers import MockLDAP from confirmation.models import Confirmation, create_confirmation_link, MultiuseInvite, \ - generate_key, confirmation_url + generate_key, confirmation_url, get_object_from_key, ConfirmationKeyException from confirmation import settings as confirmation_settings from zerver.forms import HomepageForm, WRONG_SUBDOMAIN_ERROR @@ -856,6 +856,17 @@ so we didn't send them an invitation. We did send invitations to everyone else!" scheduled_timestamp__lte=timezone_now(), type=ScheduledEmail.INVITATION_REMINDER) self.assertEqual(len(email_jobs_to_deliver), 0) + # make sure users can't take a valid confirmation key from another + # pathway and use it with the invitation url route + # Mainly a test of get_object_from_key, rather than of the invitation pathway + def test_confirmation_key_of_wrong_type(self): + # type: () -> None + user = self.example_user('hamlet') + registration_key = create_confirmation_link(user, 'host', Confirmation.USER_REGISTRATION).split('/')[-1] + with self.assertRaises(ConfirmationKeyException) as exception: + get_object_from_key(registration_key, Confirmation.INVITATION) + self.assertEqual(exception.error_type, ConfirmationKeyException.DOES_NOT_EXIST) + class InvitationsTestCase(InviteUserBase): def test_successful_get_open_invitations(self): # type: () -> None diff --git a/zerver/views/registration.py b/zerver/views/registration.py index f22bfe503d..d4ba3c47c7 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -385,7 +385,7 @@ def accounts_home_from_multiuse_invite(request, confirmation_key): # type: (HttpRequest, str) -> HttpResponse multiuse_object = None try: - multiuse_object = get_object_from_key(confirmation_key) + multiuse_object = get_object_from_key(confirmation_key, Confirmation.MULTIUSE_INVITE) # Required for oAuth2 request.session["multiuse_object_key"] = confirmation_key except ConfirmationKeyException as exception: diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 816da98071..da3e105031 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -26,7 +26,7 @@ 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, render_confirmation_key_error, \ - ConfirmationKeyException + ConfirmationKeyException, Confirmation @zulip_login_required def confirm_email_change(request, confirmation_key): @@ -37,7 +37,7 @@ def confirm_email_change(request, confirmation_key): confirmation_key = confirmation_key.lower() try: - obj = get_object_from_key(confirmation_key) + obj = get_object_from_key(confirmation_key, Confirmation.EMAIL_CHANGE) except ConfirmationKeyException as exception: return render_confirmation_key_error(request, exception)