From c23aaa17855eecc970ee7ff7c9dd70c674dd755b Mon Sep 17 00:00:00 2001 From: Umair Khan Date: Fri, 7 Oct 2016 14:10:21 +0500 Subject: [PATCH] GitHub: Show error on login page for wrong subdomain. While logging in through GitHub, if the user tries to login to the wrong subdomain then show an appropriate message. --- templates/zerver/login.html | 6 +++++ zerver/tests/test_auth_backends.py | 35 +++++++++++++++++++++++------- zerver/views/__init__.py | 25 +++++++++++++++++---- zproject/backends.py | 10 +++++++-- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/templates/zerver/login.html b/templates/zerver/login.html index 47fcdbf327..a6457aaf61 100644 --- a/templates/zerver/login.html +++ b/templates/zerver/login.html @@ -76,6 +76,12 @@ autofocus('#id_username');
{{ _("You've already registered with this email address. Please log in below") }}.
+ {% endif %} + + {% if subdomain %} +
+ {{ wrong_subdomain_error }}. +
{% endif %} {% if password_auth_enabled or desktop_sso_dispatch %} diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 11a673613b..9190b2e24e 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -181,19 +181,38 @@ class GitHubAuthBackendTest(ZulipTestCase): self.user_profile = get_user_profile_by_email(self.email) self.user_profile.backend = self.backend - def test_github_backend_do_auth(self): + rf = RequestFactory() + request = rf.get('/complete') + request.session = {} + request.get_host = lambda: 'acme.testserver' + request.user = self.user_profile + self.backend.strategy.request = request + + def test_github_backend_do_auth_without_subdomains(self): # type: () -> None def do_auth(*args, **kwargs): # type: (*Any, **Any) -> UserProfile - return self.user_profile + return self.backend.authenticate(*args, **kwargs) - with mock.patch('zerver.views.login_or_register_remote_user') as result, \ - mock.patch('social.backends.github.GithubOAuth2.do_auth', - side_effect=do_auth): + with mock.patch('social.backends.github.GithubOAuth2.do_auth', + side_effect=do_auth), \ + mock.patch('zerver.views.login'): response=dict(email=self.email, name=self.name) - self.backend.do_auth(response=response) - result.assert_called_with(None, self.email, self.user_profile, - self.name) + result = self.backend.do_auth(response=response) + self.assertNotIn('subdomain=1', result.url) + + def test_github_backend_do_auth_with_subdomains(self): + # type: () -> None + def do_auth(*args, **kwargs): + # type: (*Any, **Any) -> UserProfile + return self.backend.authenticate(*args, **kwargs) + + with mock.patch('social.backends.github.GithubOAuth2.do_auth', + side_effect=do_auth): + with self.settings(REALMS_HAVE_SUBDOMAINS=True): + response=dict(email=self.email, name=self.name) + result = self.backend.do_auth(response=response) + self.assertIn('subdomain=1', result.url) def test_github_backend_do_auth_for_default(self): # type: () -> None diff --git a/zerver/views/__init__.py b/zerver/views/__init__.py index 5a343244f3..44d0c0df55 100644 --- a/zerver/views/__init__.py +++ b/zerver/views/__init__.py @@ -36,7 +36,7 @@ from zerver.lib.actions import do_change_password, do_change_full_name, do_chang do_update_pointer, realm_user_count from zerver.lib.push_notifications import num_push_devices_for_user from zerver.forms import RegistrationForm, HomepageForm, RealmCreationForm, ToSForm, \ - CreateUserForm, OurAuthenticationForm + CreateUserForm, OurAuthenticationForm, WRONG_SUBDOMAIN_ERROR from zerver.lib.actions import is_inactive from django.views.decorators.csrf import csrf_exempt from django_auth_ldap.backend import LDAPBackend, _LDAPUser @@ -380,9 +380,14 @@ def maybe_send_to_registration(request, email, full_name=''): {'form': form, 'current_url': lambda: url}, request=request) -def login_or_register_remote_user(request, remote_username, user_profile, full_name=''): - # type: (HttpRequest, text_type, UserProfile, text_type) -> HttpResponse - if user_profile is None or user_profile.is_mirror_dummy: +def login_or_register_remote_user(request, remote_username, user_profile, full_name='', + invalid_subdomain=False): + # type: (HttpRequest, text_type, UserProfile, text_type, Optional[bool]) -> HttpResponse + if invalid_subdomain: + # Show login page with an error message + return redirect_to_subdomain_login_url() + + elif user_profile is None or user_profile.is_mirror_dummy: # Since execution has reached here, the client specified a remote user # but no associated user account exists. Send them over to the # PreregistrationUser flow. @@ -572,6 +577,12 @@ def login_page(request, **kwargs): except KeyError: pass + try: + template_response.context_data['subdomain'] = request.GET['subdomain'] + template_response.context_data['wrong_subdomain_error'] = WRONG_SUBDOMAIN_ERROR + except KeyError: + pass + return template_response def dev_direct_login(request, **kwargs): @@ -723,6 +734,12 @@ def redirect_to_email_login_url(email): redirect_url = login_url + '?email=' + urllib.parse.quote_plus(email) return HttpResponseRedirect(redirect_url) +def redirect_to_subdomain_login_url(): + # type: () -> HttpResponseRedirect + login_url = reverse('django.contrib.auth.views.login') + redirect_url = login_url + '?subdomain=1' + return HttpResponseRedirect(redirect_url) + """ When settings.OPEN_REALM_CREATION is enabled public users can create new realm. For creating the realm the user should not be the member of any current realm. The realm is created with domain same as the that of the user's email. diff --git a/zproject/backends.py b/zproject/backends.py index 853354df2f..e46a1ed872 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -22,7 +22,7 @@ from social.backends.github import GithubOAuth2, GithubOrganizationOAuth2, \ GithubTeamOAuth2 from social.exceptions import AuthFailed from django.contrib.auth import authenticate -from zerver.lib.utils import check_subdomain +from zerver.lib.utils import check_subdomain, get_subdomain def password_auth_enabled(realm): # type: (Realm) -> bool @@ -133,6 +133,7 @@ class SocialAuthMixin(ZulipAuthMixin): inactive_user = return_data.get('inactive_user') inactive_realm = return_data.get('inactive_realm') + invalid_subdomain = return_data.get('invalid_subdomain') if inactive_user or inactive_realm: return None @@ -142,7 +143,8 @@ class SocialAuthMixin(ZulipAuthMixin): full_name = self.get_full_name(*args, **kwargs) return login_or_register_remote_user(request, email_address, - user_profile, full_name) + user_profile, full_name, + bool(invalid_subdomain)) class ZulipDummyBackend(ZulipAuthMixin): """ @@ -347,6 +349,10 @@ class GitHubAuthBackend(SocialAuthMixin, GithubOAuth2): def do_auth(self, *args, **kwargs): # type: (*Any, **Any) -> Optional[UserProfile] kwargs['return_data'] = {} + + request = self.strategy.request # type: ignore # This comes from Python Social Auth. + kwargs['realm_subdomain'] = get_subdomain(request) + user_profile = None team_id = settings.SOCIAL_AUTH_GITHUB_TEAM_ID