From 9dc4b04e52416624ecf7578ae2dfc150638c1ad0 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Tue, 29 Jan 2019 12:04:53 -0800 Subject: [PATCH] auth: Migrate Google authentication off deprecated name API. As part of Google+ being removed, they've eliminated support for the /plus/v1/people/me endpoint. Replace it with the very similar /oauth2/v3/userinfo endpoint. --- zerver/tests/test_auth_backends.py | 64 ++++++++++-------------------- zerver/views/auth.py | 20 +++------- 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 7e5aebfa35..0361aaad6c 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -881,9 +881,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): def test_google_oauth2_success(self) -> None: token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[dict(type="account", - value=self.example_email("hamlet"))]) + account_data = dict(name="Full Name", + email_verified=True, + email=self.example_email("hamlet")) account_response = ResponseMock(200, account_data) result = self.google_oauth2_test(token_response, account_response, subdomain='zulip', next='/user_uploads/image') @@ -899,24 +899,15 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): parsed_url.path) self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/')) - def test_google_oauth2_no_fullname(self) -> None: + def test_user_cannot_log_without_verified_email(self) -> None: token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(givenName="Test", familyName="User"), - emails=[dict(type="account", - value=self.example_email("hamlet"))]) + account_data = dict(name="Full Name", + email_verified=False, + email=self.example_email("hamlet")) account_response = ResponseMock(200, account_data) - result = self.google_oauth2_test(token_response, account_response, subdomain='zulip') - - data = load_subdomain_token(result) - self.assertEqual(data['email'], self.example_email("hamlet")) - self.assertEqual(data['name'], 'Test User') - self.assertEqual(data['subdomain'], 'zulip') - self.assertEqual(data['next'], '') - 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.google_oauth2_test(token_response, account_response, + subdomain='zulip') + self.assertEqual(result.status_code, 400) def test_google_oauth2_mobile_success(self) -> None: self.user_profile = self.example_user('hamlet') @@ -924,9 +915,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): self.user_profile.save() mobile_flow_otp = '1234abcd' * 8 token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[dict(type="account", - value=self.user_profile.email)]) + account_data = dict(name="Full Name", + email_verified=True, + email=self.user_profile.email) account_response = ResponseMock(200, account_data) self.assertEqual(len(mail.outbox), 0) @@ -1144,9 +1135,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): def test_user_cannot_log_into_nonexisting_realm(self) -> None: token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[dict(type="account", - value=self.example_email("hamlet"))]) + account_data = dict(name="Full Name", + email_verified=True, + email=self.example_email("hamlet")) account_response = ResponseMock(200, account_data) result = self.google_oauth2_test(token_response, account_response, subdomain='nonexistent') @@ -1155,9 +1146,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): def test_user_cannot_log_into_wrong_subdomain(self) -> None: token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[dict(type="account", - value=self.example_email("hamlet"))]) + account_data = dict(name="Full Name", + email_verified=True, + email=self.example_email("hamlet")) account_response = ResponseMock(200, account_data) result = self.google_oauth2_test(token_response, account_response, subdomain='zephyr') @@ -1182,9 +1173,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest): email = "newuser@zulip.com" realm = get_realm("zulip") token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[dict(type="account", - value=email)]) + 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') @@ -1274,17 +1265,6 @@ class GoogleLoginTest(GoogleOAuthTest): self.assertEqual(m.call_args_list[0][0][0], "Google login failed making API call: Response text") - def test_google_oauth2_account_response_no_email(self) -> None: - token_response = ResponseMock(200, {'access_token': "unique_token"}) - account_data = dict(name=dict(formatted="Full Name"), - emails=[]) - account_response = ResponseMock(200, account_data) - with mock.patch("logging.error") as m: - result = self.google_oauth2_test(token_response, account_response, - subdomain="zulip") - self.assertEqual(result.status_code, 400) - self.assertIn("Google oauth2 account email not found:", m.call_args_list[0][0][0]) - def test_google_oauth2_error_access_denied(self) -> None: result = self.client_get("/accounts/login/google/done/?error=access_denied") self.assertEqual(result.status_code, 302) diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 3be29d0635..f4cbde504d 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -397,7 +397,7 @@ def finish_google_oauth2(request: HttpRequest) -> HttpResponse: access_token = resp.json()['access_token'] resp = requests.get( - 'https://www.googleapis.com/plus/v1/people/me', + 'https://www.googleapis.com/oauth2/v3/userinfo', params={'access_token': access_token} ) if resp.status_code == 400: @@ -408,21 +408,13 @@ def finish_google_oauth2(request: HttpRequest) -> HttpResponse: return HttpResponse(status=400) body = resp.json() - try: - full_name = body['name']['formatted'] - except KeyError: - # Only google+ users have a formatted name. I am ignoring i18n here. - full_name = '{} {}'.format( - body['name']['givenName'], body['name']['familyName'] - ) - for email in body['emails']: - if email['type'] == 'account': - break - else: - logging.error('Google oauth2 account email not found: %s' % (body,)) + if not body['email_verified']: + logging.error('Google oauth2 account email not verified.') return HttpResponse(status=400) - email_address = email['value'] + # Extract the user info from the Google response + full_name = body['name'] + email_address = body['email'] try: realm = Realm.objects.get(string_id=subdomain)