diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index a85a8ebe38..06c14042f5 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -965,7 +965,9 @@ class SocialAuthBase(ZulipTestCase): def stage_two_of_registration(self, result: HttpResponse, realm: Realm, subdomain: str, email: str, name: str, expected_final_name: str, - skip_registration_form: bool) -> None: + skip_registration_form: bool, + mobile_flow_otp: Optional[str]=None, + desktop_flow_otp: Optional[str]=None) -> None: data = load_subdomain_token(result) self.assertEqual(data['email'], email) self.assertEqual(data['name'], name) @@ -1003,6 +1005,35 @@ class SocialAuthBase(ZulipTestCase): 'key': confirmation_key, 'terms': True}) + # Mobile and desktop flow have additional steps: + if mobile_flow_otp: + self.assertEqual(result.status_code, 302) + redirect_url = result['Location'] + parsed_url = urllib.parse.urlparse(redirect_url) + query_params = urllib.parse.parse_qs(parsed_url.query) + self.assertEqual(parsed_url.scheme, 'zulip') + self.assertEqual(query_params["realm"], ['http://zulip.testserver']) + self.assertEqual(query_params["email"], [email]) + encrypted_api_key = query_params["otp_encrypted_api_key"][0] + user_api_keys = get_all_api_keys(get_user(email, realm)) + self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), user_api_keys) + return + elif desktop_flow_otp: + self.assertEqual(result.status_code, 302) + redirect_url = result['Location'] + parsed_url = urllib.parse.urlparse(redirect_url) + query_params = urllib.parse.parse_qs(parsed_url.query) + self.assertEqual(parsed_url.scheme, 'zulip') + self.assertEqual(query_params["realm"], ['http://zulip.testserver']) + self.assertEqual(query_params["email"], [email]) + + encrypted_key = query_params["otp_encrypted_login_key"][0] + decrypted_key = otp_decrypt_api_key(encrypted_key, desktop_flow_otp) + auth_url = 'http://zulip.testserver/accounts/login/subdomain/{}'.format(decrypted_key) + + result = self.client_get(auth_url) + # Now the desktop app is logged in, continue with the logged in check: + self.assertEqual(result.status_code, 302) user_profile = get_user(email, realm) self.assert_logged_in_user_id(user_profile.id) @@ -1024,6 +1055,40 @@ class SocialAuthBase(ZulipTestCase): self.stage_two_of_registration(result, realm, subdomain, email, name, name, self.BACKEND_CLASS.full_name_validated) + @override_settings(TERMS_OF_SERVICE=None) + def test_social_auth_mobile_registration(self) -> None: + email = "newuser@zulip.com" + name = 'Full Name' + subdomain = 'zulip' + realm = get_realm("zulip") + mobile_flow_otp = '1234abcd' * 8 + account_data_dict = self.get_account_data_dict(email=email, name=name) + + result = self.social_auth_test(account_data_dict, subdomain='zulip', + expect_choose_email_screen=True, + is_signup='1', + mobile_flow_otp=mobile_flow_otp) + self.stage_two_of_registration(result, realm, subdomain, email, name, name, + self.BACKEND_CLASS.full_name_validated, + mobile_flow_otp=mobile_flow_otp) + + @override_settings(TERMS_OF_SERVICE=None) + def test_social_auth_desktop_registration(self) -> None: + email = "newuser@zulip.com" + name = 'Full Name' + subdomain = 'zulip' + realm = get_realm("zulip") + desktop_flow_otp = '1234abcd' * 8 + account_data_dict = self.get_account_data_dict(email=email, name=name) + + result = self.social_auth_test(account_data_dict, subdomain='zulip', + expect_choose_email_screen=True, + is_signup='1', + desktop_flow_otp=desktop_flow_otp) + self.stage_two_of_registration(result, realm, subdomain, email, name, name, + self.BACKEND_CLASS.full_name_validated, + desktop_flow_otp=desktop_flow_otp) + @override_settings(TERMS_OF_SERVICE=None) def test_social_auth_registration_invitation_exists(self) -> None: """ diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 71a9de2b18..8bd26e7a01 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -86,6 +86,8 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase): 'full_name_validated': False, 'subdomain': realm.subdomain, 'is_signup': False, + 'desktop_flow_otp': None, + 'mobile_flow_otp': None, 'multiuse_object_key': ''}) response = redirect_and_log_into_subdomain(realm, name, email, @@ -97,6 +99,8 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase): 'full_name_validated': False, 'subdomain': realm.subdomain, 'is_signup': True, + 'desktop_flow_otp': None, + 'mobile_flow_otp': None, 'multiuse_object_key': 'key' }) @@ -110,6 +114,8 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase): 'full_name_validated': True, 'subdomain': realm.subdomain, 'is_signup': True, + 'desktop_flow_otp': None, + 'mobile_flow_otp': None, 'multiuse_object_key': 'key' }) diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 81ce7c57fd..f5289b9d2d 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -29,6 +29,7 @@ from zerver.lib.push_notifications import push_notifications_enabled from zerver.lib.redis_utils import get_redis_client, get_dict_from_redis, put_dict_in_redis from zerver.lib.request import REQ, has_request_variables, JsonableError from zerver.lib.response import json_success, json_error +from zerver.lib.sessions import set_expirable_session_var from zerver.lib.subdomains import get_subdomain, is_subdomain_root_or_alias from zerver.lib.user_agent import parse_user_agent from zerver.lib.users import get_api_key @@ -82,6 +83,8 @@ def create_preregistration_user(email: str, request: HttpRequest, realm_creation ) def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str='', + mobile_flow_otp: Optional[str]=None, + desktop_flow_otp: Optional[str]=None, is_signup: bool=False, password_required: bool=True, multiuse_object_key: str='', full_name_validated: bool=False) -> HttpResponse: @@ -92,6 +95,31 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str= depending on is_signup, whether the email address can join the organization (checked in HomepageForm), and similar details. """ + + # In the desktop and mobile registration flows, the sign up + # happens in the browser so the user can use their + # already-logged-in social accounts. Then at the end, with the + # user account created, we pass the appropriate data to the app + # via e.g. a `zulip://` redirect. We store the OTP keys for the + # mobile/desktop flow in the session with 1-hour expiry, because + # we want this configuration of having a successful authentication + # result in being logged into the app to persist if the user makes + # mistakes while trying to authenticate (E.g. clicks the wrong + # Google account, hits back, etc.) during a given browser session, + # rather than just logging into the webapp in the target browser. + # + # We can't use our usual pre-account-creation state storage + # approach of putting something in PreregistrationUser, because + # that would apply to future registration attempts on other + # devices, e.g. just creating an account on the web on their laptop. + assert not (mobile_flow_otp and desktop_flow_otp) + if mobile_flow_otp: + set_expirable_session_var(request.session, 'registration_mobile_flow_otp', mobile_flow_otp, + expiry_seconds=3600) + elif desktop_flow_otp: + set_expirable_session_var(request.session, 'registration_desktop_flow_otp', desktop_flow_otp, + expiry_seconds=3600) + if multiuse_object_key: from_multiuse_invite = True multiuse_obj = Confirmation.objects.get(confirmation_key=multiuse_object_key).content_object @@ -172,7 +200,9 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str= context = login_context(request) extra_context = {'form': form, 'current_url': lambda: url, 'from_multiuse_invite': from_multiuse_invite, - 'multiuse_object_key': multiuse_object_key} # type: Mapping[str, Any] + 'multiuse_object_key': multiuse_object_key, + 'mobile_flow_otp': mobile_flow_otp, + 'desktop_flow_otp': desktop_flow_otp} # type: Mapping[str, Any] context.update(extra_context) return render(request, 'zerver/accounts_home.html', context=context) @@ -183,6 +213,8 @@ def redirect_to_subdomain_login_url() -> HttpResponseRedirect: def register_remote_user(request: HttpRequest, remote_username: str, full_name: str='', + mobile_flow_otp: Optional[str]=None, + desktop_flow_otp: Optional[str]=None, is_signup: bool=False, multiuse_object_key: str='', full_name_validated: bool=False) -> HttpResponse: @@ -191,6 +223,8 @@ def register_remote_user(request: HttpRequest, remote_username: str, # 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, + mobile_flow_otp=mobile_flow_otp, + desktop_flow_otp=desktop_flow_otp, is_signup=is_signup, multiuse_object_key=multiuse_object_key, full_name_validated=full_name_validated) @@ -222,7 +256,10 @@ def login_or_register_remote_user(request: HttpRequest, remote_username: str, """ if user_profile is None or user_profile.is_mirror_dummy: return register_remote_user(request, remote_username, full_name, - is_signup=is_signup, multiuse_object_key=multiuse_object_key, + is_signup=is_signup, + mobile_flow_otp=mobile_flow_otp, + desktop_flow_otp=desktop_flow_otp, + multiuse_object_key=multiuse_object_key, full_name_validated=full_name_validated) # Otherwise, the user has successfully authenticated to an @@ -526,6 +563,8 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse: full_name = data.get('name', '') is_signup = data.get('is_signup', False) redirect_to = data.get('next', '') + mobile_flow_otp = data.get('mobile_flow_otp') + desktop_flow_otp = data.get('desktop_flow_otp') full_name_validated = data.get('full_name_validated', False) multiuse_object_key = data.get('multiuse_object_key', '') @@ -561,6 +600,8 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse: return login_or_register_remote_user(request, email_address, user_profile, full_name, is_signup=is_signup, redirect_to=redirect_to, + mobile_flow_otp=mobile_flow_otp, + desktop_flow_otp=desktop_flow_otp, multiuse_object_key=multiuse_object_key, full_name_validated=full_name_validated) @@ -580,10 +621,14 @@ def get_login_data(token: str, should_delete: bool=True) -> Optional[Dict[str, A def redirect_and_log_into_subdomain(realm: Realm, full_name: str, email_address: str, is_signup: bool=False, redirect_to: str='', + mobile_flow_otp: Optional[str]=None, + desktop_flow_otp: Optional[str]=None, multiuse_object_key: str='', - full_name_validated: bool=False) -> HttpResponse: + full_name_validated: bool=False,) -> HttpResponse: data = {'name': full_name, 'email': email_address, 'subdomain': realm.subdomain, 'is_signup': is_signup, 'next': redirect_to, + 'mobile_flow_otp': mobile_flow_otp, + 'desktop_flow_otp': desktop_flow_otp, 'multiuse_object_key': multiuse_object_key, 'full_name_validated': full_name_validated} token = store_login_data(data) diff --git a/zerver/views/registration.py b/zerver/views/registration.py index 429a046459..b76ef68728 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -26,12 +26,13 @@ from django_auth_ldap.backend import LDAPBackend, _LDAPUser from zerver.decorator import require_post, \ do_login from zerver.lib.onboarding import send_initial_realm_messages, setup_realm_internal_bots +from zerver.lib.sessions import get_expirable_session_var from zerver.lib.subdomains import get_subdomain, is_root_domain_available from zerver.lib.timezone import get_all_timezones from zerver.lib.users import get_accounts_for_email from zerver.lib.zephyr import compute_mit_user_fullname from zerver.views.auth import create_preregistration_user, redirect_and_log_into_subdomain, \ - redirect_to_deactivation_notice, get_safe_redirect_to + redirect_to_deactivation_notice, get_safe_redirect_to, finish_desktop_flow, finish_mobile_flow from zproject.backends import ldap_auth_enabled, password_auth_enabled, \ ZulipLDAPExceptionNoMatchingLDAPUser, email_auth_enabled, ZulipLDAPAuthBackend, \ @@ -380,6 +381,15 @@ def accounts_register(request: HttpRequest) -> HttpResponse: ) def login_and_go_to_home(request: HttpRequest, user_profile: UserProfile) -> HttpResponse: + mobile_flow_otp = get_expirable_session_var(request.session, 'registration_mobile_flow_otp', + delete=True) + desktop_flow_otp = get_expirable_session_var(request.session, 'registration_desktop_flow_otp', + delete=True) + if mobile_flow_otp is not None: + return finish_mobile_flow(request, user_profile, mobile_flow_otp) + elif desktop_flow_otp is not None: + return finish_desktop_flow(request, user_profile, user_profile.realm, desktop_flow_otp) + do_login(request, user_profile) return HttpResponseRedirect(user_profile.realm.uri + reverse('zerver.views.home.home')) diff --git a/zproject/backends.py b/zproject/backends.py index c50131a408..771a21f980 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -1172,20 +1172,25 @@ def social_auth_finish(backend: Any, extra_kwargs["desktop_flow_otp"] = desktop_flow_otp extra_kwargs["realm"] = realm - # For mobile and desktop app authentication, login_or_register_remote_user - # will redirect to a special zulip:// URL that is handled by - # the app after a successful authentication; so we can - # redirect directly from here, saving a round trip over what - # we need to do to create session cookies on the right domain - # in the web login flow (below). - return login_or_register_remote_user( - strategy.request, email_address, - user_profile, full_name, - is_signup=is_signup, - redirect_to=redirect_to, - full_name_validated=full_name_validated, - **extra_kwargs - ) + if user_profile is not None and not user_profile.is_mirror_dummy: + # For mobile and desktop app authentication, login_or_register_remote_user + # will redirect to a special zulip:// URL that is handled by + # the app after a successful authentication; so we can + # redirect directly from here, saving a round trip over what + # we need to do to create session cookies on the right domain + # in the web login flow (below). + return login_or_register_remote_user( + strategy.request, email_address, + user_profile, full_name, + is_signup=is_signup, + redirect_to=redirect_to, + full_name_validated=full_name_validated, + **extra_kwargs + ) + else: + # The user needs to register, so we need to go the realm's + # subdomain for that. + pass # If this authentication code were executing on # subdomain.zulip.example.com, we would just call @@ -1205,7 +1210,9 @@ def social_auth_finish(backend: Any, is_signup=is_signup, redirect_to=redirect_to, multiuse_object_key=multiuse_object_key, - full_name_validated=full_name_validated + full_name_validated=full_name_validated, + mobile_flow_otp=mobile_flow_otp, + desktop_flow_otp=desktop_flow_otp ) class SocialAuthMixin(ZulipAuthMixin, ExternalAuthMethod):