auth: Extend the template for "choose email" in GitHub auth flow.

This commit extends the template for "choose email" to mention for
users who have unverified emails that they need to verify them before
using them for Zulip authentication.

Also modified `social_auth_test_finish` to assert if all emails
are present in "choose email" screen as we need unverified emails
to be shown to user and verified emails to login/signup.

Fixes #12638 as this was the last task for that issue.
This commit is contained in:
Dinesh
2020-03-28 15:29:06 +05:30
committed by Tim Abbott
parent 4a07a6def7
commit 5c1fe776c3
3 changed files with 72 additions and 3 deletions

View File

@@ -57,8 +57,21 @@
</form> </form>
{% endfor %} {% endfor %}
</div> </div>
<p class="bottom-text"> {% if unverified_emails %}
Only verified GitHub email addresses are listed. <div class="bottom-text">
<p>
{% trans %}
Your GitHub account also has unverified email addresses
associated with it.
{% endtrans %}
</p>
<p>
{% trans %}
To use one of these to login to Zulip, you must first
<a href="https://github.com/settings/emails">verify it with GitHub.</a>
{% endtrans %}
</p> </p>
</div> </div>
{% endif %}
</div>
{% endblock %} {% endblock %}

View File

@@ -1674,6 +1674,7 @@ class GitHubAuthBackendTest(SocialAuthBase):
USER_INFO_URL = "https://api.github.com/user" USER_INFO_URL = "https://api.github.com/user"
AUTH_FINISH_URL = "/complete/github/" AUTH_FINISH_URL = "/complete/github/"
CONFIG_ERROR_URL = "/config-error/github" CONFIG_ERROR_URL = "/config-error/github"
email_data: List[Dict[str, Any]] = []
def social_auth_test_finish(self, result: HttpResponse, def social_auth_test_finish(self, result: HttpResponse,
account_data_dict: Dict[str, str], account_data_dict: Dict[str, str],
@@ -1694,6 +1695,25 @@ class GitHubAuthBackendTest(SocialAuthBase):
# authentication backends when a new authentacation backend # authentication backends when a new authentacation backend
# that requires "choose email" screen; # that requires "choose email" screen;
self.assert_in_success_response(["Select account"], result) self.assert_in_success_response(["Select account"], result)
# Verify that all the emails returned by GitHub auth
# are in the "choose email" screen.
all_emails_verified = True
for email_data_dict in self.email_data:
email = email_data_dict["email"]
if email.endswith("noreply.github.com"):
self.assert_not_in_success_response([email], result)
elif email_data_dict.get('verified'):
self.assert_in_success_response([email], result)
else:
# We may change this if we provide a way to see
# the list of emails the user had.
self.assert_not_in_success_response([email], result)
all_emails_verified = False
if all_emails_verified:
self.assert_not_in_success_response(["also has unverified email"], result)
else:
self.assert_in_success_response(["also has unverified email"], result)
result = self.client_get(self.AUTH_FINISH_URL, result = self.client_get(self.AUTH_FINISH_URL,
dict(state=csrf_state, email=account_data_dict['email']), **headers) dict(state=csrf_state, email=account_data_dict['email']), **headers)
@@ -2058,6 +2078,30 @@ class GitHubAuthBackendTest(SocialAuthBase):
"GitHub", "GitHub",
) )
def test_github_unverified_email_with_existing_account(self) -> None:
# check if a user is denied to login if the user manages to
# send an unverified email that has an existing account in
# organisation through `email` GET parameter.
account_data_dict = dict(email='hamlet@zulip.com', name=self.name)
email_data = [
dict(email='iago@zulip.com',
verified=True,),
dict(email='hamlet@zulip.com',
verified=False),
dict(email="aaron@zulip.com",
verified=True,
primary=True)
]
with mock.patch('logging.warning') as mock_warning:
result = self.social_auth_test(account_data_dict,
subdomain='zulip',
expect_choose_email_screen=True,
email_data=email_data)
self.assertEqual(result.status_code, 302)
self.assertEqual(result.url, "/login/")
mock_warning.assert_called_once_with("Social auth (%s) failed because user has no verified"
" emails associated with the account", "GitHub")
class GitLabAuthBackendTest(SocialAuthBase): class GitLabAuthBackendTest(SocialAuthBase):
__unittest_skip__ = False __unittest_skip__ = False

View File

@@ -1093,10 +1093,15 @@ def social_associate_user_helper(backend: BaseAuth, return_data: Dict[str, Any],
if existing_account is not None: if existing_account is not None:
existing_account_emails.append(email) existing_account_emails.append(email)
avatars[email] = avatar_url(existing_account) avatars[email] = avatar_url(existing_account)
if (len(existing_account_emails) != 1 or backend.strategy.session_get('is_signup') == '1'): if (len(existing_account_emails) != 1 or backend.strategy.session_get('is_signup') == '1'):
unverified_emails = []
if hasattr(backend, 'get_unverified_emails'):
unverified_emails = backend.get_unverified_emails(*args, **kwargs)
return render(backend.strategy.request, 'zerver/social_auth_select_email.html', context = { return render(backend.strategy.request, 'zerver/social_auth_select_email.html', context = {
'primary_email': verified_emails[0], 'primary_email': verified_emails[0],
'verified_non_primary_emails': verified_emails[1:], 'verified_non_primary_emails': verified_emails[1:],
'unverified_emails': unverified_emails,
'backend': 'github', 'backend': 'github',
'avatar_urls': avatars, 'avatar_urls': avatars,
}) })
@@ -1374,6 +1379,13 @@ class GitHubAuthBackend(SocialAuthMixin, GithubOAuth2):
emails = [] emails = []
return emails return emails
def get_unverified_emails(self, *args: Any, **kwargs: Any) -> List[str]:
emails = self.get_all_associated_email_objects(*args, **kwargs)
return [
email_obj['email'] for email_obj in emails
if not email_obj.get('verified') and not email_obj["email"].endswith("noreply.github.com")
]
def get_verified_emails(self, *args: Any, **kwargs: Any) -> List[str]: def get_verified_emails(self, *args: Any, **kwargs: Any) -> List[str]:
emails = self.get_all_associated_email_objects(*args, **kwargs) emails = self.get_all_associated_email_objects(*args, **kwargs)
verified_emails: List[str] = [] verified_emails: List[str] = []