diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index d95fd2232e..7dc2136578 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -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(): diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 3feff222dc..09d1c24894 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -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