diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 62cba4c54c..a091eeff2e 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -292,7 +292,7 @@ class TestUser(IntegrationTestCase): c = FrappeClient(url) res1 = c.session.post(url, data=data, verify=c.verify, headers=c.headers) res2 = c.session.post(url, data=data, verify=c.verify, headers=c.headers) - self.assertEqual(res1.status_code, 404) + self.assertEqual(res1.status_code, 200) self.assertEqual(res2.status_code, 429) def test_user_rename(self): @@ -431,15 +431,28 @@ class TestUser(IntegrationTestCase): update_password(old_password, old_password=new_password) self.assertEqual( frappe.message_log[0].get("message"), - f"Password reset instructions have been sent to {test_user.full_name}'s email", + "If this email is registered with us, we have sent password reset instructions to it. Please check your inbox.", ) sendmail.assert_called_once() self.assertEqual(sendmail.call_args[1]["recipients"], "test2@example.com") - self.assertEqual(reset_password(user="test2@example.com"), None) - self.assertEqual(reset_password(user="Administrator"), "not allowed") - self.assertEqual(reset_password(user="random"), "not found") + # Constant-response guarantee: every path — existing user, Administrator, + # and non-existent user — must return None AND enqueue the same generic + # message, so callers cannot distinguish between them. + _GENERIC_MSG = "If this email is registered with us, we have sent password reset instructions to it. Please check your inbox." + + frappe.clear_messages() + self.assertIsNone(reset_password(user="test2@example.com")) + self.assertEqual(frappe.message_log[0].get("message"), _GENERIC_MSG) + + frappe.clear_messages() + self.assertIsNone(reset_password(user="Administrator")) + self.assertEqual(frappe.message_log[0].get("message"), _GENERIC_MSG) + + frappe.clear_messages() + self.assertIsNone(reset_password(user="random")) + self.assertEqual(frappe.message_log[0].get("message"), _GENERIC_MSG) def test_user_onload_modules(self): from frappe.desk.form.load import getdoc diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index cfb86e4c8e..02a7604ea3 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1152,25 +1152,32 @@ def sign_up(email: str, full_name: str, redirect_to: str) -> tuple[int, str]: @frappe.whitelist(allow_guest=True, methods=["POST"]) @rate_limit(limit=get_password_reset_limit, seconds=60 * 60) -def reset_password(user: str) -> str: +def reset_password(user: str) -> None: + # Always return the same generic response regardless of whether the user + # exists, is disabled, or is restricted. This prevents username enumeration + # via different messages or HTTP status codes (CWE-204). + try: - user: User = frappe.get_doc("User", user) - if user.name == "Administrator": - return "not allowed" - if not user.enabled: - return "disabled" - - user.validate_reset_password() - user._reset_password(send_email=True) - - return frappe.msgprint( - msg=_("Password reset instructions have been sent to {}'s email").format(user.full_name), - title=_("Password Email Sent"), - ) + user_doc: User = frappe.get_doc("User", user) + if user_doc.name != "Administrator" and user_doc.enabled: + user_doc.validate_reset_password() + user_doc._reset_password(send_email=True) + # For Administrator or disabled users: silently skip — same response below except frappe.DoesNotExistError: - frappe.local.response["http_status_code"] = 404 frappe.clear_messages() - return "not found" + except frappe.OutgoingEmailError: + frappe.clear_messages() + frappe.log_error(title="Password reset email could not be sent", message=frappe.get_traceback()) + except Exception: + frappe.clear_messages() + frappe.log_error(title="Password reset failed unexpectedly", message=frappe.get_traceback()) + + frappe.msgprint( + msg=_( + "If this email is registered with us, we have sent password reset instructions to it. Please check your inbox." + ), + title=_("Password Reset"), + ) @frappe.whitelist() diff --git a/frappe/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index acaafa1199..98ab1acc33 100644 --- a/frappe/templates/includes/login/login.js +++ b/frappe/templates/includes/login/login.js @@ -247,17 +247,9 @@ login.login_handlers = (function () { window.location.href = data.home_page; } } else if (window.location.hash === '#forgot') { - if (data.message === 'not found') { - login.set_status({{ _("Not a valid user") | tojson }}, 'red'); - } else if (data.message == 'not allowed') { - login.set_status({{ _("Not Allowed") | tojson }}, 'red'); - } else if (data.message == 'disabled') { - login.set_status({{ _("Not Allowed: Disabled User") | tojson }}, 'red'); - } else { - login.set_status({{ _("Instructions Emailed") | tojson }}, 'green'); - } - - + // Always show the same message regardless of whether the account + // exists or not, to prevent username enumeration (CWE-204). + login.set_status({{ _("Instructions Emailed") | tojson }}, 'green'); } else if (window.location.hash === '#signup') { if (cint(data.message[0]) == 0) { login.set_status(data.message[1], 'red'); @@ -285,7 +277,6 @@ login.login_handlers = (function () { }, 401: get_error_handler({{ _("Invalid Login. Try again.") | tojson }}), 417: get_error_handler({{ _("Oops! Something went wrong.") | tojson }}), - 404: get_error_handler({{ _("User does not exist.") | tojson }}), 429: get_error_handler({{ _("Too many requests. Please try again later.") | tojson }}), 500: get_error_handler({{ _("Something went wrong.") | tojson }}), };