Skip to content

Commit

Permalink
auth: Expect name in request params in Apple auth.
Browse files Browse the repository at this point in the history
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.

python-social-auth/social-core#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 {'firstName': 'John',
'lastName': 'Doe'} dict. This is also reflected by the change in the
native flow tests.
  • Loading branch information
mateuszmandera committed Oct 22, 2020
1 parent b18a17f commit 247c014
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 4 deletions.
15 changes: 11 additions & 4 deletions zerver/tests/test_auth_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -2107,9 +2107,10 @@ def social_auth_test_finish(self, result: HttpResponse,
**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,
Expand Down Expand Up @@ -2205,6 +2206,7 @@ def prepare_login_url_and_headers(
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]]:
Expand All @@ -2225,6 +2227,8 @@ def prepare_login_url_and_headers(
if subdomain:
params['subdomain'] = subdomain

params['user'] = json.dumps(account_data_dict)

url += f"&{urllib.parse.urlencode(params)}"
return url, headers

Expand Down Expand Up @@ -2259,7 +2263,7 @@ class for details.
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():
Expand Down Expand Up @@ -2291,11 +2295,13 @@ def test_no_id_token_sent(self) -> None:

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)

Expand Down Expand Up @@ -2324,6 +2330,7 @@ def test_id_token_with_invalid_aud_sent(self) -> None:
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:
Expand Down
27 changes: 27 additions & 0 deletions zproject/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,33 @@ def validate_state(self) -> Optional[str]:
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.
Expand Down

0 comments on commit 247c014

Please sign in to comment.