mirror of https://github.com/zulip/zulip.git
auth: Restore a minimal SocialAuthMixin.
We need to do a small monkey-patching of python-social-auth to ensure that it doesn't 500 the request when a user does something funny in their browser (e.g. using the back button in the auth flow) that is fundamentally a user error, not a server error. This was present in the pre-rewrite version of our Social auth codebase, without clear documentation; I've fixed the explanation part here. It's perhaps worth investigating with the core social auth team whether there's a better way to do this.
This commit is contained in:
parent
c9b0c0add4
commit
5a99118b3e
|
@ -774,6 +774,21 @@ class GitHubAuthBackendTest(ZulipTestCase):
|
|||
'in one of the domains that are allowed to register '
|
||||
'for accounts in this organization.'.format(email), result)
|
||||
|
||||
def test_github_complete(self) -> None:
|
||||
with mock.patch('social_core.backends.oauth.BaseOAuth2.process_error',
|
||||
side_effect=AuthFailed('Not found')):
|
||||
result = self.client_get(reverse('social:complete', args=['github']))
|
||||
self.assertEqual(result.status_code, 302)
|
||||
self.assertIn('login', result.url)
|
||||
|
||||
def test_github_complete_when_base_exc_is_raised(self) -> None:
|
||||
with mock.patch('social_core.backends.oauth.BaseOAuth2.auth_complete',
|
||||
side_effect=AuthStateForbidden('State forbidden')), \
|
||||
mock.patch('zproject.backends.logging.warning'):
|
||||
result = self.client_get(reverse('social:complete', args=['github']))
|
||||
self.assertEqual(result.status_code, 302)
|
||||
self.assertIn('login', result.url)
|
||||
|
||||
def test_github_auth_enabled(self) -> None:
|
||||
with self.settings(AUTHENTICATION_BACKENDS=('zproject.backends.GitHubAuthBackend',)):
|
||||
self.assertTrue(github_auth_enabled())
|
||||
|
|
|
@ -499,7 +499,31 @@ def social_auth_finish(backend: Any,
|
|||
is_signup=is_signup,
|
||||
redirect_to=redirect_to)
|
||||
|
||||
class GitHubAuthBackend(GithubOAuth2):
|
||||
class SocialAuthMixin(ZulipAuthMixin):
|
||||
def auth_complete(self, *args: Any, **kwargs: Any) -> Optional[HttpResponse]:
|
||||
"""This is a small wrapper around the core `auth_complete` method of
|
||||
python-social-auth, designed primarily to prevent 500s for
|
||||
exceptions in the social auth code from situations that are
|
||||
really user errors. Returning `None` from this function will
|
||||
redirect the browser to the login page.
|
||||
"""
|
||||
try:
|
||||
# Call the auth_complete method of social_core.backends.oauth.BaseOAuth2
|
||||
return super().auth_complete(*args, **kwargs) # type: ignore # monkey-patching
|
||||
except AuthFailed as e:
|
||||
# When a user's social authentication fails (e.g. because
|
||||
# they did something funny with reloading in the middle of
|
||||
# the flow), don't throw a 500, just send them back to the
|
||||
# login page and record the event at the info log level.
|
||||
logging.info(str(e))
|
||||
return None
|
||||
except SocialAuthBaseException as e:
|
||||
# Other python-social-auth exceptions are likely
|
||||
# interesting enough that we should log a warning.
|
||||
logging.warning(str(e))
|
||||
return None
|
||||
|
||||
class GitHubAuthBackend(SocialAuthMixin, GithubOAuth2):
|
||||
auth_backend_name = "GitHub"
|
||||
|
||||
def get_verified_emails(self, *args: Any, **kwargs: Any) -> List[str]:
|
||||
|
|
Loading…
Reference in New Issue