confirmation: Add confirmation_type to get_object_from_key.

This change:

* Prevents weird potential attacks like taking a valid confirmation link
  (say an unsubscribe link), and putting it into the URL of a multiuse
  invite link. I don't know of any such attacks one could do right now, but
  reasoning about it is complicated.

* Makes the code easier to read, and in the case of confirmation/views.py,
  exposes something that needed refactoring anyway (USER_REGISTRATION and
  INVITATION should have different endpoints, and both of those endpoints
  should be in zerver/views/registration, not this file).

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)
 

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={

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

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:

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)