mirror of https://github.com/zulip/zulip.git
auth: Expect name in request params in Apple auth.
The name used to be included in the id_token, but this seems to have been changed by Apple and now it's sent in the `user` request param. https://github.com/python-social-auth/social-core/pull/483 is the upstream PR for this - but upstream is currently unmaintained, so we have to monkey patch. We also alter the tests to reflect this situation. Tests no longer put the name in the id_token, but rather in the `user` request param in the browser flow, just like it happens in reality. An adaptation has to be made in the native flow - since the name won't be included by Apple in the id_token anymore, the app, when POSTing to the /complete/apple/ endpoint, can (and should for better user experience) add the `user` param formatted as json of {"email": "hamlet@zulip.com", "name": {"firstName": "Full", "lastName": "Name"}} dict. This is also reflected by the change in the native flow tests.
This commit is contained in:
parent
0ea20bd7d8
commit
48f80fcb0a
|
@ -2064,7 +2064,7 @@ class AppleAuthMixin:
|
|||
CONFIG_ERROR_URL = "/config-error/apple"
|
||||
|
||||
def generate_id_token(self, account_data_dict: Dict[str, str], audience: Optional[str]=None) -> str:
|
||||
payload = account_data_dict
|
||||
payload = dict(email=account_data_dict['email'])
|
||||
|
||||
# This setup is important because python-social-auth decodes `id_token`
|
||||
# with `SOCIAL_AUTH_APPLE_CLIENT` as the `audience`
|
||||
|
@ -2107,9 +2107,10 @@ class AppleIdAuthBackendTest(AppleAuthMixin, SocialAuthBase):
|
|||
**extra_data: Any) -> HttpResponse:
|
||||
parsed_url = urllib.parse.urlparse(result.url)
|
||||
state = urllib.parse.parse_qs(parsed_url.query)['state']
|
||||
user_param = json.dumps(account_data_dict)
|
||||
self.client.session.flush()
|
||||
result = self.client_post(self.AUTH_FINISH_URL,
|
||||
dict(state=state), **headers)
|
||||
dict(state=state, user=user_param), **headers)
|
||||
return result
|
||||
|
||||
def register_extra_endpoints(self, requests_mock: responses.RequestsMock,
|
||||
|
@ -2205,6 +2206,7 @@ class AppleAuthBackendNativeFlowTest(AppleAuthMixin, SocialAuthBase):
|
|||
multiuse_object_key: str='',
|
||||
alternative_start_url: Optional[str]=None,
|
||||
id_token: Optional[str]=None,
|
||||
account_data_dict: Dict[str, str]={},
|
||||
*,
|
||||
user_agent: Optional[str]=None,
|
||||
) -> Tuple[str, Dict[str, Any]]:
|
||||
|
@ -2225,6 +2227,8 @@ class AppleAuthBackendNativeFlowTest(AppleAuthMixin, SocialAuthBase):
|
|||
if subdomain:
|
||||
params['subdomain'] = subdomain
|
||||
|
||||
params['user'] = json.dumps(account_data_dict)
|
||||
|
||||
url += f"&{urllib.parse.urlencode(params)}"
|
||||
return url, headers
|
||||
|
||||
|
@ -2259,7 +2263,7 @@ class AppleAuthBackendNativeFlowTest(AppleAuthMixin, SocialAuthBase):
|
|||
url, headers = self.prepare_login_url_and_headers(
|
||||
subdomain, mobile_flow_otp, desktop_flow_otp, is_signup, next,
|
||||
multiuse_object_key, alternative_start_url=self.AUTH_FINISH_URL,
|
||||
user_agent=user_agent, id_token=id_token,
|
||||
user_agent=user_agent, id_token=id_token, account_data_dict=account_data_dict,
|
||||
)
|
||||
|
||||
with self.apple_jwk_url_mock():
|
||||
|
@ -2291,11 +2295,13 @@ class AppleAuthBackendNativeFlowTest(AppleAuthMixin, SocialAuthBase):
|
|||
|
||||
def test_social_auth_session_fields_cleared_correctly(self) -> None:
|
||||
mobile_flow_otp = '1234abcd' * 8
|
||||
account_data_dict = self.get_account_data_dict(email=self.email, name=self.name)
|
||||
|
||||
def initiate_auth(mobile_flow_otp: Optional[str]=None) -> None:
|
||||
url, headers = self.prepare_login_url_and_headers(subdomain='zulip',
|
||||
id_token='invalid',
|
||||
mobile_flow_otp=mobile_flow_otp)
|
||||
mobile_flow_otp=mobile_flow_otp,
|
||||
account_data_dict=account_data_dict)
|
||||
result = self.client_get(url, **headers)
|
||||
self.assertEqual(result.status_code, 302)
|
||||
|
||||
|
@ -2324,6 +2330,7 @@ class AppleAuthBackendNativeFlowTest(AppleAuthMixin, SocialAuthBase):
|
|||
url, headers = self.prepare_login_url_and_headers(
|
||||
subdomain='zulip', alternative_start_url=self.AUTH_FINISH_URL,
|
||||
id_token=self.generate_id_token(account_data_dict, audience='com.different.app'),
|
||||
account_data_dict=account_data_dict,
|
||||
)
|
||||
|
||||
with self.apple_jwk_url_mock(), self.assertLogs(self.logger_string, level='INFO') as m:
|
||||
|
|
|
@ -1643,6 +1643,33 @@ class AppleAuthBackend(SocialAuthMixin, AppleIdAuth):
|
|||
self.strategy.session_set(param, value)
|
||||
return request_state
|
||||
|
||||
def get_user_details(self, response: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""
|
||||
Overriden to correctly grab the user's name from the request params,
|
||||
as current upstream code expects it in the id_token and Apple changed
|
||||
the API.
|
||||
Taken from https://github.com/python-social-auth/social-core/pull/483
|
||||
TODO: Remove this when the PR is merged.
|
||||
"""
|
||||
name = response.get('name') or {}
|
||||
name = json.loads(self.data.get('user', '{}')).get('name', {})
|
||||
fullname, first_name, last_name = self.get_user_names(
|
||||
fullname='',
|
||||
first_name=name.get('firstName', ''),
|
||||
last_name=name.get('lastName', '')
|
||||
)
|
||||
email = response.get('email', '')
|
||||
# prevent updating User with empty strings
|
||||
user_details = {
|
||||
'fullname': fullname or None,
|
||||
'first_name': first_name or None,
|
||||
'last_name': last_name or None,
|
||||
'email': email,
|
||||
}
|
||||
user_details['username'] = email
|
||||
|
||||
return user_details
|
||||
|
||||
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.
|
||||
|
|
Loading…
Reference in New Issue