Validate before accessing email parts.

For somereason earlier validation is not ensuring that we have a valid e-email.
In this case, break out of the flow since we don't have a domain that's in our
list and log the user's id so that we can learn more about when this happens.

By a reading of the code flow, it doesn't seem like it should be possible except
with a handful of users that have invalid e-mail addresses in the database but it
seems to be happening pretty regularly.
This commit is contained in:
Feanil Patel
2020-07-08 12:04:30 -04:00
parent e8a7529243
commit f2ac18049b
2 changed files with 33 additions and 2 deletions

View File

@@ -326,7 +326,15 @@ def _check_user_auth_flow(site, user):
"""
if user and ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.is_enabled():
allowed_domain = site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_DOMAIN', '').lower()
user_domain = user.email.split('@')[1].strip().lower()
email_parts = user.email.split('@')
if len(email_parts) != 2:
# User has a nonstandard email so we record their id.
# we don't record their e-mail in case there is sensitive info accidentally
# in there.
set_custom_metric('login_tpa_domain_shortcircuit_user_id', user.id)
log.warn("User %s has nonstandard e-mail. Shortcircuiting THIRD_PART_AUTH_ONLY_DOMAIN check.", user.id)
return
user_domain = email_parts[1].strip().lower()
# If user belongs to allowed domain and not whitelisted then user must login through allowed domain SSO
if user_domain == allowed_domain and not AllowedAuthUser.objects.filter(site=site, email=user.email).exists():

View File

@@ -30,7 +30,8 @@ from openedx.core.djangoapps.user_api.accounts import EMAIL_MIN_LENGTH, EMAIL_MA
from openedx.core.djangoapps.user_authn.cookies import jwt_cookies
from openedx.core.djangoapps.user_authn.views.login import (
AllowedAuthUser,
ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY
ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY,
_check_user_auth_flow
)
from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
@@ -785,6 +786,28 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
)
self.assertFalse(mock_check_user_auth_flow.called)
def test_check_user_auth_flow_bad_email(self):
"""Regression Exception was thrown on missing @ char in TPA."""
provider = 'Google'
provider_tpa_hint = 'saml-test'
username = 'batman'
invalid_email_user = self._create_user(username, username)
allowed_domain = 'edx.org'
default_site_configuration_values = {
'SITE_NAME': allowed_domain,
'THIRD_PARTY_AUTH_ONLY_DOMAIN': allowed_domain,
'THIRD_PARTY_AUTH_ONLY_PROVIDER': provider,
'THIRD_PARTY_AUTH_ONLY_HINT': provider_tpa_hint,
}
with ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.override(True):
site = self.set_up_site(allowed_domain, default_site_configuration_values)
with self.assertLogs(level='WARN') as log:
_check_user_auth_flow(site, invalid_email_user)
assert len(log.output) == 1
assert "Shortcircuiting THIRD_PART_AUTH_ONLY_DOMAIN check." in log.output[0]
@ddt.ddt
@skip_unless_lms