auth2: Don't use session for passing multiuse invite key.

For Google auth, the multiuse invite key should be stored in the
csrf_state sent to google along with other values like is_signup,
mobile_flow_otp.

For social auth, the multiuse invite key should be passed as params to
the social-auth backend. The passing of the key is handled by
social_auth pipeline and made available to us when the auth is
completed.

templates/zerver/accounts_home.html

@@ -79,6 +79,7 @@ $(function () {
                         <div class="login-social">
                             <form class="form-inline" action="{{ url('zerver.views.auth.start_google_oauth2') }}" method="get">
                                 <input type='hidden' name='is_signup' value='1' />
+                                <input type='hidden' name='multiuse_object_key' value='{{ multiuse_object_key }}' />
                                 <button class="login-social-button login-google-button full-width">
                                     {{ _('Sign up with %(identity_provider)s', identity_provider="Google") }}
                                 </button>
@@ -89,6 +90,7 @@ $(function () {
                         {% if github_auth_enabled %}
                         <div class="login-social">
                             <form class="form-inline github-wrapper" action="{{ url('signup-social', args=('github',)) }}" method="get">
+                                <input type='hidden' name='multiuse_object_key' value='{{ multiuse_object_key }}' />
                                 <button class="login-social-button full-width">
                                     {{ _('Sign up with %(identity_provider)s', identity_provider="GitHub") }}
                                 </button>

zerver/tests/test_auth_backends.py

@@ -440,6 +440,7 @@ class SocialAuthBase(ZulipTestCase):
                          mobile_flow_otp: Optional[str]=None,
                          is_signup: Optional[str]=None,
                          next: str='',
+                         multiuse_object_key: str='',
                          **extra_data: Any) -> HttpResponse:
         url = self.LOGIN_URL
         params = {}
@@ -452,6 +453,7 @@ class SocialAuthBase(ZulipTestCase):
         if is_signup is not None:
             url = self.SIGNUP_URL
         params['next'] = next
+        params['multiuse_object_key'] = multiuse_object_key
         if len(params) > 0:
             url += "?%s" % (urllib.parse.urlencode(params))
 
@@ -687,6 +689,79 @@ class SocialAuthBase(ZulipTestCase):
         user_profile = get_user(email, realm)
         self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
 
+    def test_social_auth_registration_using_multiuse_invite(self) -> None:
+        """If the user doesn't exist yet, social auth can be used to register an account"""
+        email = "newuser@zulip.com"
+        name = 'Full Name'
+        realm = get_realm("zulip")
+        realm.invite_required = True
+        realm.save()
+
+        stream_names = ["new_stream_1", "new_stream_2"]
+        streams = []
+        for stream_name in set(stream_names):
+            stream = ensure_stream(realm, stream_name)
+            streams.append(stream)
+
+        referrer = self.example_user("hamlet")
+        multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer)
+        multiuse_obj.streams.set(streams)
+        create_confirmation_link(multiuse_obj, realm.host, Confirmation.MULTIUSE_INVITE)
+        multiuse_confirmation = Confirmation.objects.all().last()
+        multiuse_object_key = multiuse_confirmation.confirmation_key
+        account_data_dict = self.get_account_data_dict(email=email, name=name)
+
+        # First, try to signup for closed realm without using an invitation
+        result = self.social_auth_test(account_data_dict,
+                                       subdomain='zulip', is_signup='1')
+        result = self.client_get(result.url)
+        # Verify that we're unable to signup, since this is a closed realm
+        self.assertEqual(result.status_code, 200)
+        self.assert_in_success_response(["Sign up"], result)
+
+        result = self.social_auth_test(account_data_dict, subdomain='zulip', is_signup='1',
+                                       multiuse_object_key=multiuse_object_key)
+
+        data = load_subdomain_token(result)
+        self.assertEqual(data['email'], email)
+        self.assertEqual(data['name'], name)
+        self.assertEqual(data['subdomain'], 'zulip')
+        self.assertEqual(data['multiuse_object_key'], multiuse_object_key)
+        self.assertEqual(result.status_code, 302)
+        parsed_url = urllib.parse.urlparse(result.url)
+        uri = "{}://{}{}".format(parsed_url.scheme, parsed_url.netloc,
+                                 parsed_url.path)
+        self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
+
+        result = self.client_get(result.url)
+
+        self.assertEqual(result.status_code, 302)
+        confirmation = Confirmation.objects.all().last()
+        confirmation_key = confirmation.confirmation_key
+        self.assertIn('do_confirm/' + confirmation_key, result.url)
+        result = self.client_get(result.url)
+        self.assert_in_response('action="/accounts/register/"', result)
+        data = {"from_confirmation": "1",
+                "full_name": name,
+                "key": confirmation_key}
+        result = self.client_post('/accounts/register/', data)
+        self.assert_in_response("We just need you to do one last thing", result)
+
+        # Verify that the user is asked for name but not password
+        self.assert_not_in_success_response(['id_password'], result)
+        self.assert_in_success_response(['id_full_name'], result)
+
+        # Click confirm registration button.
+        result = self.client_post(
+            '/accounts/register/',
+            {'full_name': name,
+             'key': confirmation_key,
+             'terms': True})
+
+        self.assertEqual(result.status_code, 302)
+        user_profile = get_user(email, realm)
+        self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
+
     def test_social_auth_registration_without_is_signup(self) -> None:
         """If `is_signup` is not set then a new account isn't created"""
         email = "newuser@zulip.com"
@@ -863,7 +938,8 @@ class GoogleOAuthTest(ZulipTestCase):
                            *, subdomain: Optional[str]=None,
                            mobile_flow_otp: Optional[str]=None,
                            is_signup: Optional[str]=None,
-                           next: str='') -> HttpResponse:
+                           next: str='',
+                           multiuse_object_key: str='') -> HttpResponse:
         url = "/accounts/login/google/"
         params = {}
         headers = {}
@@ -875,6 +951,7 @@ class GoogleOAuthTest(ZulipTestCase):
         if is_signup is not None:
             params['is_signup'] = is_signup
         params['next'] = next
+        params['multiuse_object_key'] = multiuse_object_key
         if len(params) > 0:
             url += "?%s" % (urllib.parse.urlencode(params))
 
@@ -1117,12 +1194,11 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
         referrer = self.example_user("hamlet")
         multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer)
         multiuse_obj.streams.set(streams)
-        invite_link = create_confirmation_link(multiuse_obj, realm.host,
-                                               Confirmation.MULTIUSE_INVITE)
-
-        result = self.client_get(invite_link, subdomain="zulip")
-        self.assert_in_success_response(['Sign up for Zulip'], result)
+        create_confirmation_link(multiuse_obj, realm.host, Confirmation.MULTIUSE_INVITE)
+        multiuse_confirmation = Confirmation.objects.all().last()
+        multiuse_object_key = multiuse_confirmation.confirmation_key
 
+        data["multiuse_object_key"] = multiuse_object_key
         result = self.get_log_into_subdomain(data)
         self.assertEqual(result.status_code, 302)
 
@@ -1247,6 +1323,85 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
         user_profile = get_user(email, realm)
         self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
 
+    def test_google_oauth2_registration_using_multiuse_invite(self) -> None:
+        """If the user doesn't exist yet, Google auth can be used to register an account"""
+        email = "newuser@zulip.com"
+        realm = get_realm("zulip")
+        realm.invite_required = True
+        realm.save()
+
+        stream_names = ["new_stream_1", "new_stream_2"]
+        streams = []
+        for stream_name in set(stream_names):
+            stream = ensure_stream(realm, stream_name)
+            streams.append(stream)
+
+        referrer = self.example_user("hamlet")
+        multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer)
+        multiuse_obj.streams.set(streams)
+        link = create_confirmation_link(multiuse_obj, realm.host, Confirmation.MULTIUSE_INVITE)
+        multiuse_confirmation = Confirmation.objects.all().last()
+        multiuse_object_key = multiuse_confirmation.confirmation_key
+
+        input_element = "name=\'multiuse_object_key\' value=\'{}\' /".format(multiuse_object_key)
+        response = self.client_get(link)
+        self.assert_in_success_response([input_element], response)
+
+        # First, try to signup for closed realm without using an invitation
+        token_response = ResponseMock(200, {'access_token': "unique_token"})
+        account_data = dict(name="Full Name",
+                            email_verified=True,
+                            email=email)
+        account_response = ResponseMock(200, account_data)
+        result = self.google_oauth2_test(token_response, account_response, subdomain='zulip',
+                                         is_signup='1', multiuse_object_key="")
+        result = self.client_get(result.url)
+        # Verify that we're unable to signup, since this is a closed realm
+        self.assertEqual(result.status_code, 200)
+        self.assert_in_success_response(["Sign up"], result)
+
+        result = self.google_oauth2_test(token_response, account_response, subdomain='zulip',
+                                         is_signup='1', multiuse_object_key=multiuse_object_key)
+        data = load_subdomain_token(result)
+        name = 'Full Name'
+        self.assertEqual(data['name'], name)
+        self.assertEqual(data['subdomain'], 'zulip')
+        self.assertEqual(data['multiuse_object_key'], multiuse_object_key)
+        self.assertEqual(result.status_code, 302)
+        parsed_url = urllib.parse.urlparse(result.url)
+        uri = "{}://{}{}".format(parsed_url.scheme, parsed_url.netloc,
+                                 parsed_url.path)
+        self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
+
+        result = self.client_get(result.url)
+        self.assertEqual(result.status_code, 302)
+        confirmation = Confirmation.objects.all().last()
+        confirmation_key = confirmation.confirmation_key
+        self.assertIn('do_confirm/' + confirmation_key, result.url)
+        result = self.client_get(result.url)
+        self.assert_in_response('action="/accounts/register/"', result)
+        data = {"from_confirmation": "1",
+                "full_name": name,
+                "key": confirmation_key}
+        result = self.client_post('/accounts/register/', data)
+        self.assert_in_response("We just need you to do one last thing", result)
+
+        # Verify that the user is asked for name but not password
+        self.assert_not_in_success_response(['id_password'], result)
+        self.assert_in_success_response(['id_full_name'], result)
+
+        # Click confirm registration button.
+        result = self.client_post(
+            '/accounts/register/',
+            {'full_name': name,
+             'key': confirmation_key,
+             'terms': True})
+
+        self.assertEqual(result.status_code, 302)
+        user_profile = get_user(email, realm)
+        self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
+        self.assertEqual(sorted(self.get_streams(email, realm)), stream_names)
+
 class GoogleLoginTest(GoogleOAuthTest):
     @override_settings(ROOT_DOMAIN_LANDING_PAGE=True)
     def test_google_oauth2_subdomains_homepage(self) -> None:
@@ -1322,7 +1477,7 @@ class GoogleLoginTest(GoogleOAuthTest):
 
     def test_google_oauth2_csrf_badstate(self) -> None:
         with mock.patch("logging.warning") as m:
-            result = self.client_get("/accounts/login/google/done/?state=badstate:otherbadstate:more:::")
+            result = self.client_get("/accounts/login/google/done/?state=badstate:otherbadstate:more::::")
         self.assertEqual(result.status_code, 400)
         self.assertEqual(m.call_args_list[0][0][0],
                          'Google oauth2 CSRF error')

zerver/tests/test_signup.py

@@ -81,15 +81,19 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase):
         self.assertDictEqual(data, {'name': name, 'next': '',
                                     'email': email,
                                     'subdomain': realm.subdomain,
-                                    'is_signup': False})
+                                    'is_signup': False,
+                                    'multiuse_object_key': ''})
 
         response = redirect_and_log_into_subdomain(realm, name, email,
-                                                   is_signup=True)
+                                                   is_signup=True,
+                                                   multiuse_object_key='key')
         data = load_subdomain_token(response)
         self.assertDictEqual(data, {'name': name, 'next': '',
                                     'email': email,
                                     'subdomain': realm.subdomain,
-                                    'is_signup': True})
+                                    'is_signup': True,
+                                    'multiuse_object_key': 'key'
+                                    })
 
 class DeactivationNoticeTestCase(ZulipTestCase):
     def test_redirection_for_deactivated_realm(self) -> None:

zerver/views/auth.py

@@ -69,14 +69,14 @@ def create_preregistration_user(email: str, request: HttpRequest, realm_creation
                                               realm=realm)
 
 def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str='',
-                               is_signup: bool=False, password_required: bool=True) -> HttpResponse:
+                               is_signup: bool=False, password_required: bool=True,
+                               multiuse_object_key: str='') -> HttpResponse:
     realm = get_realm(get_subdomain(request))
     from_multiuse_invite = False
     multiuse_obj = None
     streams_to_subscribe = None
-    multiuse_object_key = request.session.get("multiuse_object_key", None)
     invited_as = PreregistrationUser.INVITE_AS['MEMBER']
-    if multiuse_object_key is not None:
+    if multiuse_object_key:
         from_multiuse_invite = True
         multiuse_obj = Confirmation.objects.get(confirmation_key=multiuse_object_key).content_object
         realm = multiuse_obj.realm
@@ -99,8 +99,7 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str=
             prereg_user = create_preregistration_user(email, request,
                                                       password_required=password_required)
 
-        if multiuse_object_key is not None:
-            del request.session["multiuse_object_key"]
+        if multiuse_object_key:
             request.session.modified = True
             if streams_to_subscribe is not None:
                 prereg_user.streams.set(streams_to_subscribe)
@@ -123,7 +122,8 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str=
         return render(request,
                       'zerver/accounts_home.html',
                       context={'form': form, 'current_url': lambda: url,
-                               'from_multiuse_invite': from_multiuse_invite},
+                               'from_multiuse_invite': from_multiuse_invite,
+                               'multiuse_object_key': multiuse_object_key},
                       )
 
 def redirect_to_subdomain_login_url() -> HttpResponseRedirect:
@@ -137,15 +137,15 @@ def redirect_to_config_error(error_type: str) -> HttpResponseRedirect:
 def login_or_register_remote_user(request: HttpRequest, remote_username: Optional[str],
                                   user_profile: Optional[UserProfile], full_name: str='',
                                   invalid_subdomain: bool=False, mobile_flow_otp: Optional[str]=None,
-                                  is_signup: bool=False,
-                                  redirect_to: str='') -> HttpResponse:
+                                  is_signup: bool=False, redirect_to: str='',
+                                  multiuse_object_key: str='') -> HttpResponse:
     email = remote_user_to_email(remote_username)
     if user_profile is None or user_profile.is_mirror_dummy:
         # We have verified the user controls an email address, but
         # there's no associated Zulip user account.  Consider sending
         # the request to registration.
-        return maybe_send_to_registration(request, email,
-                                          full_name, password_required=False, is_signup=is_signup)
+        return maybe_send_to_registration(request, email, full_name, password_required=False,
+                                          is_signup=is_signup, multiuse_object_key=multiuse_object_key)
 
     # Otherwise, the user has successfully authenticated to an
     # account, and we need to do the right thing depending whether
@@ -288,6 +288,8 @@ def oauth_redirect_to_root(request: HttpRequest, url: str,
         'is_signup': '1' if is_signup else '0',
     }
 
+    params['multiuse_object_key'] = request.GET.get('multiuse_object_key', '')
+
     # mobile_flow_otp is a one-time pad provided by the app that we
     # can use to encrypt the API key when passing back to the app.
     mobile_flow_otp = request.GET.get('mobile_flow_otp')
@@ -329,6 +331,7 @@ def send_oauth_request_to_google(request: HttpRequest) -> HttpResponse:
     is_signup = request.GET.get('is_signup', '')
     next = request.GET.get('next', '')
     mobile_flow_otp = request.GET.get('mobile_flow_otp', '0')
+    multiuse_object_key = request.GET.get('multiuse_object_key', '')
 
     if ((settings.ROOT_DOMAIN_LANDING_PAGE and subdomain == '') or
             not Realm.objects.filter(string_id=subdomain).exists()):
@@ -336,8 +339,8 @@ def send_oauth_request_to_google(request: HttpRequest) -> HttpResponse:
 
     google_uri = 'https://accounts.google.com/o/oauth2/auth?'
     cur_time = str(int(time.time()))
-    csrf_state = '%s:%s:%s:%s:%s' % (cur_time, subdomain, mobile_flow_otp, is_signup, next)
-
+    csrf_state = '%s:%s:%s:%s:%s:%s' % (cur_time, subdomain, mobile_flow_otp, is_signup,
+                                        next, multiuse_object_key)
     # Now compute the CSRF hash with the other parameters as an input
     csrf_state += ":%s" % (google_oauth2_csrf(request, csrf_state),)
 
@@ -361,7 +364,7 @@ def finish_google_oauth2(request: HttpRequest) -> HttpResponse:
         return HttpResponse(status=400)
 
     csrf_state = request.GET.get('state')
-    if csrf_state is None or len(csrf_state.split(':')) != 6:
+    if csrf_state is None or len(csrf_state.split(':')) != 7:
         logging.warning('Missing Google oauth2 CSRF state')
         return HttpResponse(status=400)
 
@@ -369,7 +372,7 @@ def finish_google_oauth2(request: HttpRequest) -> HttpResponse:
     if hmac_value != google_oauth2_csrf(request, csrf_data):
         logging.warning('Google oauth2 CSRF error')
         return HttpResponse(status=400)
-    cur_time, subdomain, mobile_flow_otp, is_signup, next = csrf_data.split(':')
+    cur_time, subdomain, mobile_flow_otp, is_signup, next, multiuse_object_key = csrf_data.split(':')
     if mobile_flow_otp == '0':
         mobile_flow_otp = None
 
@@ -429,7 +432,8 @@ def finish_google_oauth2(request: HttpRequest) -> HttpResponse:
                                              redirect_to=next)
 
     return redirect_and_log_into_subdomain(
-        realm, full_name, email_address, is_signup=is_signup, redirect_to=next)
+        realm, full_name, email_address, is_signup=is_signup,
+        redirect_to=next, multiuse_object_key=multiuse_object_key)
 
 def authenticate_remote_user(realm: Realm, email_address: str) -> Tuple[UserProfile, Dict[str, Any]]:
     return_data = {}  # type: Dict[str, bool]
@@ -470,6 +474,12 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
     full_name = data['name']
     is_signup = data['is_signup']
     redirect_to = data['next']
+
+    if 'multiuse_object_key' in data:
+        multiuse_object_key = data['multiuse_object_key']
+    else:
+        multiuse_object_key = ''
+
     if is_signup:
         # If we are signing up, user_profile should be None. In case
         # email_address already exists, user will get an error message.
@@ -486,12 +496,15 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
     invalid_subdomain = bool(return_data.get('invalid_subdomain'))
     return login_or_register_remote_user(request, email_address, user_profile,
                                          full_name, invalid_subdomain=invalid_subdomain,
-                                         is_signup=is_signup, redirect_to=redirect_to)
+                                         is_signup=is_signup, redirect_to=redirect_to,
+                                         multiuse_object_key=multiuse_object_key)
 
 def redirect_and_log_into_subdomain(realm: Realm, full_name: str, email_address: str,
-                                    is_signup: bool=False, redirect_to: str='') -> HttpResponse:
+                                    is_signup: bool=False, redirect_to: str='',
+                                    multiuse_object_key: str='') -> HttpResponse:
     data = {'name': full_name, 'email': email_address, 'subdomain': realm.subdomain,
-            'is_signup': is_signup, 'next': redirect_to}
+            'is_signup': is_signup, 'next': redirect_to,
+            'multiuse_object_key': multiuse_object_key}
     token = signing.dumps(data, salt=_subdomain_token_salt)
     subdomain_login_uri = (realm.uri
                            + reverse('zerver.views.auth.log_into_subdomain', args=[token]))

zerver/views/registration.py

@@ -417,7 +417,8 @@ def create_realm(request: HttpRequest, creation_key: Optional[str]=None) -> Http
                   context={'form': form, 'current_url': request.get_full_path},
                   )
 
-def accounts_home(request: HttpRequest, multiuse_object: Optional[MultiuseInvite]=None) -> HttpResponse:
+def accounts_home(request: HttpRequest, multiuse_object_key: Optional[str]="",
+                  multiuse_object: Optional[MultiuseInvite]=None) -> HttpResponse:
     realm = get_realm(get_subdomain(request))
 
     if realm is None:
@@ -459,6 +460,7 @@ def accounts_home(request: HttpRequest, multiuse_object: Optional[MultiuseInvite
     return render(request,
                   'zerver/accounts_home.html',
                   context={'form': form, 'current_url': request.get_full_path,
+                           'multiuse_object_key': multiuse_object_key,
                            'from_multiuse_invite': from_multiuse_invite},
                   )
 
@@ -467,12 +469,12 @@ def accounts_home_from_multiuse_invite(request: HttpRequest, confirmation_key: s
     try:
         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:
         realm = get_realm_from_request(request)
         if realm is None or realm.invite_required:
             return render_confirmation_key_error(request, exception)
-    return accounts_home(request, multiuse_object=multiuse_object)
+    return accounts_home(request, multiuse_object_key=confirmation_key,
+                         multiuse_object=multiuse_object)
 
 def generate_204(request: HttpRequest) -> HttpResponse:
     return HttpResponse(content=None, status=204)

zproject/backends.py

@@ -635,7 +635,7 @@ def social_auth_finish(backend: Any,
     is_signup = strategy.session_get('is_signup') == '1'
     redirect_to = strategy.session_get('next')
     realm = Realm.objects.get(id=return_data["realm_id"])
-
+    multiuse_object_key = strategy.session_get('multiuse_object_key', '')
     mobile_flow_otp = strategy.session_get('mobile_flow_otp')
     if mobile_flow_otp is not None:
         return login_or_register_remote_user(strategy.request, email_address,
@@ -646,7 +646,8 @@ def social_auth_finish(backend: Any,
                                              redirect_to=redirect_to)
     return redirect_and_log_into_subdomain(realm, full_name, email_address,
                                            is_signup=is_signup,
-                                           redirect_to=redirect_to)
+                                           redirect_to=redirect_to,
+                                           multiuse_object_key=multiuse_object_key)
 
 class SocialAuthMixin(ZulipAuthMixin):
     auth_backend_name = "undeclared"

zproject/settings.py

@@ -1349,7 +1349,7 @@ if REGISTER_LINK_DISABLED is None:
 # SOCIAL AUTHENTICATION SETTINGS
 ########################################################################
 
-SOCIAL_AUTH_FIELDS_STORED_IN_SESSION = ['subdomain', 'is_signup', 'mobile_flow_otp']
+SOCIAL_AUTH_FIELDS_STORED_IN_SESSION = ['subdomain', 'is_signup', 'mobile_flow_otp', 'multiuse_object_key']
 SOCIAL_AUTH_LOGIN_ERROR_URL = '/login/'
 
 SOCIAL_AUTH_GITHUB_SECRET = get_secret('social_auth_github_secret')