From 0ec2a9d25991e03bb562e487bce00c97e75caeb7 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Mon, 27 Nov 2017 21:26:41 -0800
Subject: [PATCH] auth: Try switching to register even if user exists on
 another realm.

For example, this means that if a user already has an account on one
realm and they try to make an account on another by hitting "Sign in
with Google" (rather than following the little "Register" link to a
"Sign up with Google" button instead), they'll get to make an account
instead of getting an error.

Until very recently, if the user existed on another realm, any attempt
to register with that email address had to fail in the end, so this
logic gave the user a useful error message early.  We introduced it in
c23aaa178 "GitHub: Show error on login page for wrong subdomain"
back in 2016-10 for that purpose.  No longer!  We now support reusing
an email on multiple realms, so we let the user proceed instead.

This function's interface is kind of confusing, but I believe when its
callers use it properly, `invalid_subdomain` should only ever be true
when `user_profile` is None -- in which case the revised
`invalid_subdomain` condition in this commit can never actually fire,
and the `invalid_subdomain` parameter no longer has any effect.  (At
least some unit tests call this function improperly in that respect.)
I've kept this commit to a minimal change, but it would be a good
followup to go through the call sites, verify that, eliminate the use
of `invalid_subdomain`, then remove it from the function entirely.
---
 zerver/tests/test_auth_backends.py | 11 +++--------
 zerver/tests/test_signup.py        |  5 ++---
 zerver/views/auth.py               |  7 ++++---
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py
index e41ec23480..4abb6f9df6 100644
--- a/zerver/tests/test_auth_backends.py
+++ b/zerver/tests/test_auth_backends.py
@@ -807,14 +807,12 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
         user_profile = self.example_user('hamlet')
         self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
 
-        # If authenticate_remote_user detects a subdomain mismatch, then
-        # the result should redirect to the login page.
         with mock.patch(
                 'zerver.views.auth.authenticate_remote_user',
                 return_value=(None, {'invalid_subdomain': True})):
             result = self.get_log_into_subdomain(data)
-            self.assertEqual(result.status_code, 302)
-            self.assertTrue(result['Location'].endswith, '?subdomain=1')
+            self.assert_in_success_response(['Would you like to register instead?'],
+                                            result)
 
     def test_log_into_subdomain_when_signature_is_bad(self) -> None:
         data = {'name': 'Full Name',
@@ -965,10 +963,7 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
         self.assertTrue(result.url.startswith("http://zephyr.testserver/accounts/login/subdomain/"))
         result = self.client_get(result.url.replace('http://zephyr.testserver', ''),
                                  subdomain="zephyr")
-        self.assertEqual(result.status_code, 302)
-        result = self.client_get('/accounts/login/?subdomain=1', subdomain="zephyr")
-        self.assert_in_success_response(["Your Zulip account is not a member of the organization associated with this subdomain."],
-                                        result)
+        self.assert_in_success_response(['Would you like to register instead?'], result)
 
     def test_user_cannot_log_into_wrong_subdomain_with_cookie(self) -> None:
         data = {'name': 'Full Name',
diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py
index 0705f59f40..34bd3e8d26 100644
--- a/zerver/tests/test_signup.py
+++ b/zerver/tests/test_signup.py
@@ -2447,14 +2447,13 @@ class LoginOrAskForRegistrationTestCase(ZulipTestCase):
         user_profile = None  # type: Optional[UserProfile]
         full_name = 'New User'
         invalid_subdomain = True
-        response = login_or_register_remote_user(
+        result = login_or_register_remote_user(
             request,
             email,
             user_profile,
             full_name=full_name,
             invalid_subdomain=invalid_subdomain)
-        self.assertEqual(response.status_code, 302)
-        self.assertIn('/accounts/login/?subdomain=1', response.url)
+        self.assert_in_success_response(['Would you like to register instead?'], result)
 
     def test_invalid_email(self) -> None:
         request = HostRequestMock()
diff --git a/zerver/views/auth.py b/zerver/views/auth.py
index 0a9413656e..b14808532a 100644
--- a/zerver/views/auth.py
+++ b/zerver/views/auth.py
@@ -120,9 +120,6 @@ def login_or_register_remote_user(request, remote_username, user_profile, full_n
                                   invalid_subdomain=False, mobile_flow_otp=None,
                                   is_signup=False):
     # type: (HttpRequest, Optional[Text], Optional[UserProfile], Text, bool, Optional[str], bool) -> HttpResponse
-    if invalid_subdomain:
-        # Show login page with an error message
-        return redirect_to_subdomain_login_url()
 
     if user_profile is None or user_profile.is_mirror_dummy:
         # Since execution has reached here, we have verified the user
@@ -149,6 +146,10 @@ def login_or_register_remote_user(request, remote_username, user_profile, full_n
                       'zerver/confirm_continue_registration.html',
                       context=context)
 
+    if invalid_subdomain:
+        # Show login page with an error message
+        return redirect_to_subdomain_login_url()
+
     if mobile_flow_otp is not None:
         # For the mobile Oauth flow, we send the API key and other
         # necessary details in a redirect to a zulip:// URI scheme.