From c15d7e32027473eb2a21759f1445e082aadff7d1 Mon Sep 17 00:00:00 2001 From: Dinesh Date: Wed, 24 Jun 2020 18:58:47 +0530 Subject: [PATCH] requirements: Update social-auth-core to latest version. Uses git release as this version 3.4.0 is not released to pypi. This is required for removing some overriden functions of apple auth backend class AppleAuthBackend. With the update we also make following changes: * Fix full name being populated as "None None". c5c74f27dd that's included in update assigns first_name and last_name to None when no name is provided by apple. Due to this our code is filling return_data['full_name'] to 'None None'. This commit fixes it by making first and last name strings empty. * Remove decode_id_token override. Python social auth merged the PR we sent including the changes we made to decode_id_token function. So, now there is no necessity for the override. * Add _AUDIENCE setting in computed_settings.py. `decode_id_token` is dependent on this setting. --- requirements/common.in | 2 +- requirements/dev.txt | 6 ++--- requirements/prod.txt | 6 ++--- version.py | 2 +- zerver/tests/test_auth_backends.py | 3 ++- zproject/backends.py | 40 +++--------------------------- zproject/computed_settings.py | 3 +++ zproject/test_extra_settings.py | 1 + 8 files changed, 15 insertions(+), 48 deletions(-) diff --git a/requirements/common.in b/requirements/common.in index dc29c13151..f747d08b4c 100644 --- a/requirements/common.in +++ b/requirements/common.in @@ -141,7 +141,7 @@ py3dns # Install Python Social Auth social-auth-app-django -social-auth-core[azuread,saml] +https://github.com/python-social-auth/social-core/archive/3.4.0.zip/#egg=social-auth-core[azuread,saml]==3.4.0 # For encrypting a login token to the desktop app cryptography diff --git a/requirements/dev.txt b/requirements/dev.txt index 9c21fcefdc..649ddf6c62 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -943,10 +943,8 @@ social-auth-app-django==4.0.0 \ --hash=sha256:567ad0e028311541d7dfed51d3bf2c60440a6fd236d5d4d06c5a618b3d6c57c5 \ --hash=sha256:df5212370bd250108987c4748419a1a1d0cec750878856c2644c36aaa0fd3e58 \ # via -r requirements/common.in -social-auth-core[azuread,saml]==3.3.3 \ - --hash=sha256:21c0639c56befd33ec162c2210d583bb1de8e1136d53b21bafb96afaf2f86c91 \ - --hash=sha256:2f6ce1af8ec2b2cc37b86d647f7d4e4292f091ee556941db34b1e0e2dee77fc0 \ - --hash=sha256:4a3cdf69c449b235cdabd54a1be7ba3722611297e69fded52e3584b1a990af25 \ +https://github.com/python-social-auth/social-core/archive/3.4.0.zip/#egg=social-auth-core[azuread,saml]==3.4.0 \ + --hash=sha256:75c6819cdbd8755253ea9d87624c732596356966939ea60822296669e1974b42 \ # via -r requirements/common.in, social-auth-app-django soupsieve==2.0.1 \ --hash=sha256:1634eea42ab371d3d346309b93df7870a88610f0725d47528be902a0d95ecc55 \ diff --git a/requirements/prod.txt b/requirements/prod.txt index 6226bc83cc..6135a717cf 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -671,10 +671,8 @@ social-auth-app-django==4.0.0 \ --hash=sha256:567ad0e028311541d7dfed51d3bf2c60440a6fd236d5d4d06c5a618b3d6c57c5 \ --hash=sha256:df5212370bd250108987c4748419a1a1d0cec750878856c2644c36aaa0fd3e58 \ # via -r requirements/common.in -social-auth-core[azuread,saml]==3.3.3 \ - --hash=sha256:21c0639c56befd33ec162c2210d583bb1de8e1136d53b21bafb96afaf2f86c91 \ - --hash=sha256:2f6ce1af8ec2b2cc37b86d647f7d4e4292f091ee556941db34b1e0e2dee77fc0 \ - --hash=sha256:4a3cdf69c449b235cdabd54a1be7ba3722611297e69fded52e3584b1a990af25 \ +https://github.com/python-social-auth/social-core/archive/3.4.0.zip/#egg=social-auth-core[azuread,saml]==3.4.0 \ + --hash=sha256:75c6819cdbd8755253ea9d87624c732596356966939ea60822296669e1974b42 \ # via -r requirements/common.in, social-auth-app-django soupsieve==2.0.1 \ --hash=sha256:1634eea42ab371d3d346309b93df7870a88610f0725d47528be902a0d95ecc55 \ diff --git a/version.py b/version.py index 873f67af98..785dded4ed 100644 --- a/version.py +++ b/version.py @@ -44,4 +44,4 @@ API_FEATURE_LEVEL = 27 # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = '92.0' +PROVISION_VERSION = '93.0' diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 20b194efb4..696370105b 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -25,6 +25,7 @@ from django.test.client import RequestFactory from django.urls import reverse from django.utils.timezone import now as timezone_now from django_auth_ldap.backend import LDAPSearch, _LDAPUser +from jwt.exceptions import PyJWTError from onelogin.saml2.auth import OneLogin_Saml2_Auth from onelogin.saml2.response import OneLogin_Saml2_Response from social_core.exceptions import AuthFailed, AuthStateForbidden @@ -2168,7 +2169,7 @@ class AppleIdAuthBackendTest(AppleAuthMixin, SocialAuthBase): def test_id_token_verification_failure(self) -> None: account_data_dict = self.get_account_data_dict(email=self.email, name=self.name) with self.assertLogs(self.logger_string, level='INFO') as m: - with mock.patch("jwt.decode", side_effect=jwt.exceptions.PyJWTError): + with mock.patch("jwt.decode", side_effect=PyJWTError): result = self.social_auth_test(account_data_dict, expect_choose_email_screen=True, subdomain='zulip', is_signup=True) diff --git a/zproject/backends.py b/zproject/backends.py index 8b66c2d445..0d614f76a1 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -18,7 +18,6 @@ import logging from abc import ABC, abstractmethod from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type, TypeVar, Union, cast -import jwt import magic import ujson from decorator import decorator @@ -33,8 +32,6 @@ from django.shortcuts import render from django.urls import reverse from django.utils.translation import ugettext as _ from django_auth_ldap.backend import LDAPBackend, LDAPReverseEmailSearch, _LDAPUser, ldap_error -from jwt.algorithms import RSAAlgorithm -from jwt.exceptions import PyJWTError from lxml.etree import XMLSyntaxError from onelogin.saml2.errors import OneLogin_Saml2_Error from onelogin.saml2.response import OneLogin_Saml2_Response @@ -1203,10 +1200,10 @@ def social_associate_user_helper(backend: BaseAuth, return_data: Dict[str, Any], if full_name: return_data["full_name"] = full_name else: - # In SAML authentication, the IdP may support only sending - # the first and last name as separate attributes - in that case + # Some authentications methods like Apple and SAML send + # first name and last name as seperate attributes. In that case # we construct the full name from them. - return_data["full_name"] = f"{first_name} {last_name}".strip() # strip removes the unnecessary ' ' + return_data["full_name"] = f"{first_name or ''} {last_name or ''}".strip() # strip removes the unnecessary ' ' return user_profile @@ -1621,37 +1618,6 @@ class AppleAuthBackend(SocialAuthMixin, AppleIdAuth): self.strategy.session_set(param, value) return request_state - def decode_id_token(self, id_token: str) -> Dict[str, Any]: - '''Decode and validate JWT token from Apple and return payload including user data. - - We override this method from upstream python-social-auth, for two reasons: - * To improve error handling (correctly raising AuthFailed; see comment below). - * To facilitate this to support the native flow, where - the Apple-generated id_token is signed for "Bundle ID" - audience instead of "Services ID". - - It is likely that small upstream tweaks could make it possible - to make this function a thin wrapper around the upstream - method; we may want to submit a PR to achieve that. - ''' - if self.is_native_flow(): - audience = self.setting("BUNDLE_ID") - else: - audience = self.setting("SERVICES_ID") - - try: - kid = jwt.get_unverified_header(id_token).get('kid') - public_key = RSAAlgorithm.from_jwk(self.get_apple_jwk(kid)) - decoded = jwt.decode(id_token, key=public_key, - audience=audience, algorithm="RS256") - except PyJWTError: - # Changed from upstream python-social-auth to raise - # AuthFailed, which is more appropriate than upstream's - # AuthCanceled, for this case. - raise AuthFailed(self, "Token validation failed") - - return decoded - def auth_complete(self, *args: Any, **kwargs: Any) -> Optional[HttpResponse]: if not self.is_native_flow(): # The default implementation in python-social-auth is the browser flow. diff --git a/zproject/computed_settings.py b/zproject/computed_settings.py index 040bb7ec27..0edf3afcd9 100644 --- a/zproject/computed_settings.py +++ b/zproject/computed_settings.py @@ -47,6 +47,7 @@ from .configured_settings import ( REMOTE_POSTGRES_SSLMODE, SENDFILE_BACKEND, SENTRY_DSN, + SOCIAL_AUTH_APPLE_BUNDLE_ID, SOCIAL_AUTH_APPLE_SERVICES_ID, SOCIAL_AUTH_GITHUB_KEY, SOCIAL_AUTH_GITHUB_ORG_NAME, @@ -1028,6 +1029,8 @@ SOCIAL_AUTH_LOGIN_ERROR_URL = '/login/' # SERVICES_ID to make things more readable in the configuration # and our own custom backend code. SOCIAL_AUTH_APPLE_CLIENT = SOCIAL_AUTH_APPLE_SERVICES_ID +SOCIAL_AUTH_APPLE_AUDIENCE = [id for id in [SOCIAL_AUTH_APPLE_CLIENT, SOCIAL_AUTH_APPLE_BUNDLE_ID] if id is not None] + if PRODUCTION: SOCIAL_AUTH_APPLE_SECRET = get_from_file_if_exists("/etc/zulip/apple/zulip-private-key.key") else: diff --git a/zproject/test_extra_settings.py b/zproject/test_extra_settings.py index 951d225fdf..942b17fb50 100644 --- a/zproject/test_extra_settings.py +++ b/zproject/test_extra_settings.py @@ -180,6 +180,7 @@ SOCIAL_AUTH_SUBDOMAIN = 'auth' SOCIAL_AUTH_APPLE_SERVICES_ID = 'com.zulip.chat' SOCIAL_AUTH_APPLE_BUNDLE_ID = 'com.zulip.bundle.id' SOCIAL_AUTH_APPLE_CLIENT = 'com.zulip.chat' +SOCIAL_AUTH_APPLE_AUDIENCE = [SOCIAL_AUTH_APPLE_BUNDLE_ID, SOCIAL_AUTH_APPLE_SERVICES_ID] SOCIAL_AUTH_APPLE_KEY = 'KEYISKEY' SOCIAL_AUTH_APPLE_TEAM = 'TEAMSTRING' SOCIAL_AUTH_APPLE_SECRET = get_from_file_if_exists("zerver/tests/fixtures/apple/private_key.pem")