From 8e512b399fd6187981a76800b20b5429223cc4cc Mon Sep 17 00:00:00 2001 From: Ramakrushna K Date: Mon, 10 Aug 2026 13:18:13 +0530 Subject: [PATCH 1/3] Harden auth input validation: fail-fast login/registration checks, safe error responses, and centralized registration telemetry validation --- .../core/djangoapps/user_authn/views/login.py | 40 +++++++++++++++--- .../djangoapps/user_authn/views/register.py | 19 ++++++++- .../user_authn/views/tests/test_login.py | 42 ++++++++++++++++++- .../user_authn/views/tests/test_register.py | 42 +++++++++++++++++++ .../core/djangoapps/user_authn/views/utils.py | 28 +++++++++++++ 5 files changed, 163 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 0dd9a4819d5e..4c46aae15632 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -10,6 +10,7 @@ import re import urllib +from django.core.exceptions import ValidationError from django.conf import settings from django.contrib.auth import authenticate, get_user_model from django.contrib.auth import login as django_login @@ -19,6 +20,7 @@ from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.translation import gettext as _ +from django.core.validators import validate_email from django.views.decorators.csrf import csrf_exempt, csrf_protect, ensure_csrf_cookie from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.http import require_http_methods @@ -50,6 +52,7 @@ from openedx.core.djangoapps.user_authn.cookies import get_response_with_refreshed_jwt_cookies, set_logged_in_cookies from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError, VulnerablePasswordError from openedx.core.djangoapps.user_authn.tasks import check_pwned_password_and_send_track_event +from openedx.core.djangoapps.user_authn.views.registration_form import validate_username from openedx.core.djangoapps.user_authn.toggles import ( is_require_third_party_auth_enabled, should_redirect_to_authn_microfrontend, @@ -66,6 +69,8 @@ AUDIT_LOG = logging.getLogger("audit") USER_MODEL = get_user_model() PASSWORD_RESET_INITIATED = "edx.user.passwordreset.initiated" +LOGIN_INFO_ERROR = _("There was an error receiving your login information. Please email us.") +LOGIN_INVALID_INPUT_EMAIL = "[invalid input]" def _do_third_party_auth(request): @@ -125,6 +130,26 @@ def _get_user_by_username(username): return None +def _validate_login_identifier(identifier): + """ + Validate the submitted login identifier before any auth logic runs. + """ + if "@" in identifier: + try: + validate_email(identifier) + except ValidationError as exc: + raise AuthFailedError(LOGIN_INFO_ERROR) from exc + return + + if not accounts.USERNAME_MIN_LENGTH <= len(identifier) <= accounts.USERNAME_MAX_LENGTH: + raise AuthFailedError(LOGIN_INFO_ERROR) + + try: + validate_username(identifier) + except ValidationError as exc: + raise AuthFailedError(LOGIN_INFO_ERROR) + + def _get_user_by_email_or_username(request, api_version): """ Finds a user object in the database based on the given request, ignores all fields except for email and username. @@ -135,7 +160,7 @@ def _get_user_by_email_or_username(request, api_version): login_fields = ["email_or_username", "password"] if any(f not in request.POST.keys() for f in login_fields): - raise AuthFailedError(_("There was an error receiving your login information. Please email us.")) + raise AuthFailedError(LOGIN_INFO_ERROR) email_or_username = request.POST.get("email", None) or request.POST.get("email_or_username", None) user = _get_user_by_email(email_or_username) @@ -557,6 +582,7 @@ def login_user(request, api_version="v1"): # pylint: disable=too-many-statement third_party_auth_requested = third_party_auth.is_enabled() and pipeline.running(request) first_party_auth_requested = any(bool(request.POST.get(p)) for p in ["email", "email_or_username", "password"]) is_user_third_party_authenticated = False + user = None set_custom_attribute("login_user_course_id", request.POST.get("course_id")) @@ -564,8 +590,13 @@ def login_user(request, api_version="v1"): # pylint: disable=too-many-statement return HttpResponseForbidden( "Third party authentication is required to login. Username and password were received instead." ) - possibly_authenticated_user = None try: + login_identifier = request.POST.get("email_or_username") + if login_identifier is None: + login_identifier = request.POST.get("email") + if login_identifier is not None: + _validate_login_identifier(login_identifier) + if third_party_auth_requested and not first_party_auth_requested: # The user has already authenticated via third-party auth and has not # asked to do first party auth by supplying a username or password. We @@ -682,10 +713,7 @@ def login_user(request, api_version="v1"): # pylint: disable=too-many-statement error_code = response_content.get("error_code") if error_code: set_custom_attribute("login_error_code", error_code) - email_or_username_key = "email" if api_version == API_V1 else "email_or_username" - email_or_username = request.POST.get(email_or_username_key, None) - email_or_username = possibly_authenticated_user.email if possibly_authenticated_user else email_or_username - response_content["email"] = email_or_username + response_content["email"] = user.email if user else LOGIN_INVALID_INPUT_EMAIL except VulnerablePasswordError as error: response_content = error.get_response() log.exception(response_content) diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index aff210e7b26d..69d70afbff08 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -63,7 +63,10 @@ RegistrationFormFactory, get_registration_extension_form ) -from openedx.core.djangoapps.user_authn.views.utils import get_auto_generated_username +from openedx.core.djangoapps.user_authn.views.utils import ( + get_auto_generated_username, + get_total_registration_time_validation_error, +) from openedx.core.djangoapps.user_authn.tasks import check_pwned_password_and_send_track_event from openedx.core.djangoapps.user_authn.toggles import ( is_require_third_party_auth_enabled, @@ -587,6 +590,20 @@ def post(self, request): ) data = request.POST.copy() + + validation_error = get_total_registration_time_validation_error(data) + if validation_error: + AUDIT_LOG.warning( + "Registration rejected due to invalid total_registration_time payload digest=%s", + validation_error["value_digest"], + ) + return self._create_response( + request, + {validation_error["field"]: [{"user_message": validation_error["user_message"]}]}, + status_code=400, + error_code=validation_error["error_code"], + ) + self._handle_terms_of_service(data) if is_auto_generated_username_enabled() and 'username' not in data: 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 b00702ee25da..e85a09cb3d5b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -354,6 +354,7 @@ def test_login_fail_no_user_exists(self): self._assert_response( response, success=False, value=self.LOGIN_FAILED_WARNING, status_code=400 ) + self._assert_response(response, success=False, email_value="[invalid input]") self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'Unknown user email', email_hash]) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) @@ -364,6 +365,7 @@ def test_login_fail_no_user_exists_no_pii(self): self.password, ) self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) + self._assert_response(response, success=False, email_value="[invalid input]") self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'Unknown user email']) self._assert_not_in_audit_log(mock_audit_log, 'warning', [nonexistent_email]) @@ -373,6 +375,7 @@ def test_login_fail_wrong_password(self): 'wrong_password', ) self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) + self._assert_response(response, success=False, email_value=self.user_email) self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'password for', str(self.user.id), 'invalid']) @@ -380,6 +383,7 @@ def test_login_fail_wrong_password(self): def test_login_fail_wrong_password_no_pii(self): response, mock_audit_log = self._login_response(self.user_email, 'wrong_password') self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) + self._assert_response(response, success=False, email_value=self.user_email) self._assert_audit_log( mock_audit_log, 'warning', ['Login failed', 'password for', str(self.user.id), 'invalid'] ) @@ -433,6 +437,7 @@ def test_login_not_activated_no_pii(self): self.password ) self._assert_response(response, success=False, error_code="inactive-user") + self._assert_response(response, success=False, email_value=self.user_email) self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'Account not active for user']) self._assert_not_in_audit_log(mock_audit_log, 'warning', ['test']) @@ -820,7 +825,16 @@ def _login_response( result = self.client.post(self.url, post_params, **extra) return result, mock_audit_log - def _assert_response(self, response, success=None, value=None, status_code=None, error_code=None): + def _assert_response( + self, + response, + success=None, + value=None, + status_code=None, + error_code=None, + email_value=None, + absent_keys=None, + ): """ Assert that the response has the expected status code and returned a valid JSON-parseable dict. @@ -851,6 +865,13 @@ def _assert_response(self, response, success=None, value=None, status_code=None, (str(response_dict['value']), str(value))) assert value in response_dict['value'], msg + if email_value is not None: + assert response_dict['email'] == email_value + + if absent_keys is not None: + for key in absent_keys: + assert key not in response_dict + def _assert_redirect_url(self, response, expected_redirect_url): """ Assert that the redirect URL is in the response and has the expected value. @@ -1190,6 +1211,7 @@ def test_invalid_credentials(self): "password": "invalid" }) self.assertHttpBadRequest(response) + self._assert_response(response, success=False, absent_keys=["email"]) # Invalid email address response = self.client.post(self.url, { @@ -1197,6 +1219,24 @@ def test_invalid_credentials(self): "password": self.PASSWORD, }) self.assertHttpBadRequest(response) + self._assert_response(response, success=False, absent_keys=["email"]) + + @ddt.data( + "bad input!", + "ab", + "not-an-email@", + "", + ) + @patch('openedx.core.djangoapps.user_authn.views.login._get_user_by_email_or_username') + def test_login_rejects_invalid_login_identifier(self, invalid_identifier, mock_user_lookup): + response = self.client.post(self.url_v2, { + "email_or_username": invalid_identifier, + "password": self.PASSWORD, + }) + + self.assertHttpBadRequest(response) + self._assert_response(response, success=False, email_value="[invalid input]") + assert not mock_user_lookup.called @ddt.data(True, False) def test_missing_login_params(self, is_api_v1): diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index 54d42efa55c0..14c344db5ebc 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -1,5 +1,6 @@ """Tests for account creation""" +import hashlib import json from datetime import datetime from unittest import mock, skipIf, skipUnless @@ -2092,6 +2093,47 @@ def setUp(self): # pylint: disable=arguments-differ super(RegistrationViewTestV1, self).setUp() # lint-amnesty, pylint: disable=bad-super-call self.url = reverse("user_api_registration_v2") + def test_register_invalid_total_registration_time(self): + invalid_total_registration_time = '57.664" AND "1"="1" --' + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + "total_registration_time": invalid_total_registration_time, + }) + + self.assertHttpBadRequest(response) + response_json = json.loads(response.content.decode('utf-8')) + self.assertDictEqual( + response_json, + { + "total_registration_time": [{ + "user_message": "Enter a valid registration time value.", + }], + "error_code": "invalid-total-registration-time", + } + ) + + expected_digest = hashlib.shake_128(invalid_total_registration_time.encode("utf-8")).hexdigest(16) + with LogCapture() as logger: + self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": "another_user", + "password": self.PASSWORD, + "honor_code": "true", + "total_registration_time": invalid_total_registration_time, + }) + logger.check_present( + ( + 'audit', + 'WARNING', + f'Registration rejected due to invalid total_registration_time payload digest={expected_digest}' + ) + ) + @override_settings( REGISTRATION_EXTRA_FIELDS={ "level_of_education": "optional", diff --git a/openedx/core/djangoapps/user_authn/views/utils.py b/openedx/core/djangoapps/user_authn/views/utils.py index 779c77159947..64f465f31ccc 100644 --- a/openedx/core/djangoapps/user_authn/views/utils.py +++ b/openedx/core/djangoapps/user_authn/views/utils.py @@ -2,6 +2,7 @@ User Auth Views Utils """ import logging +import hashlib import re from typing import Dict @@ -25,6 +26,33 @@ API_V1 = 'v1' UUID4_REGEX = '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}' ENTERPRISE_ENROLLMENT_URL_REGEX = fr'/enterprise/{UUID4_REGEX}/course/{settings.COURSE_KEY_REGEX}/enroll' +TOTAL_REGISTRATION_TIME_RE = re.compile(r'^\d+(?:\.\d+)?$') + + +def get_total_registration_time_validation_error(data): + """ + Validate optional registration timing telemetry with an allowlist pattern. + + Returns: + dict | None: error details when invalid, otherwise None. + """ + total_registration_time = data.get('total_registration_time') + if total_registration_time is None: + total_registration_time = data.get('totalRegistrationTime') + + if total_registration_time in (None, ''): + return None + + normalized_value = str(total_registration_time).strip() + if TOTAL_REGISTRATION_TIME_RE.fullmatch(normalized_value): + return None + + return { + "field": "total_registration_time", + "user_message": _("Enter a valid registration time value."), + "error_code": "invalid-total-registration-time", + "value_digest": hashlib.shake_128(normalized_value.encode("utf-8")).hexdigest(16), + } def third_party_auth_context(request, redirect_to, tpa_hint=None): From 1d4395fd998c02656af172c7ce883f33225a10fe Mon Sep 17 00:00:00 2001 From: Ramakrushna K Date: Mon, 10 Aug 2026 14:08:03 +0530 Subject: [PATCH 2/3] harden login and registration input validation and stop raw input reflection --- openedx/core/djangoapps/user_authn/views/tests/test_login.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e85a09cb3d5b..0dbc7cdb8854 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -1211,7 +1211,7 @@ def test_invalid_credentials(self): "password": "invalid" }) self.assertHttpBadRequest(response) - self._assert_response(response, success=False, absent_keys=["email"]) + self._assert_response(response, success=False, email_value=self.EMAIL) # Invalid email address response = self.client.post(self.url, { @@ -1219,7 +1219,7 @@ def test_invalid_credentials(self): "password": self.PASSWORD, }) self.assertHttpBadRequest(response) - self._assert_response(response, success=False, absent_keys=["email"]) + self._assert_response(response, success=False, email_value="[invalid input]") @ddt.data( "bad input!", From 5a51fb9b866b6eb4a0d3d51065f8cb181348e3fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:25:27 +0000 Subject: [PATCH 3/3] Fix pylint W0707 raise-missing-from in login.py Co-authored-by: kramakrushna <293011905+kramakrushna@users.noreply.github.com> --- openedx/core/djangoapps/user_authn/views/login.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 4c46aae15632..b9f692e84dc1 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -147,7 +147,7 @@ def _validate_login_identifier(identifier): try: validate_username(identifier) except ValidationError as exc: - raise AuthFailedError(LOGIN_INFO_ERROR) + raise AuthFailedError(LOGIN_INFO_ERROR) from exc def _get_user_by_email_or_username(request, api_version):