From f00c4b77384967641b7abf7a91ccf06d87a6cf55 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 14 Apr 2026 15:23:04 +0530 Subject: [PATCH 1/8] fix: enhance password reset flow to prevent username enumeration --- frappe/core/doctype/user/user.py | 30 ++++++++++++------------ frappe/templates/includes/login/login.js | 14 +++-------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 5f4b1d3fae..d1616b3831 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1127,24 +1127,24 @@ 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: + # 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" + # Do not reveal whether the account exists — fall through to generic response + + return frappe.msgprint( + msg=_("If an account with this email exists, password reset instructions have been sent."), + title=_("Password Reset"), + ) @frappe.whitelist() diff --git a/frappe/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index acaafa1199..c039c2351a 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'); From a0f4526c58ac5fcac2a3227a4839753d4390b4c8 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 14 Apr 2026 17:44:31 +0530 Subject: [PATCH 2/8] fix: update password reset tests for improved accuracy and messaging --- frappe/core/doctype/user/test_user.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 62cba4c54c..0884c010b0 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,15 @@ 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 an account with this email exists, password reset instructions have been sent.", ) 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") + self.assertEqual(reset_password(user="Administrator"), None) + self.assertEqual(reset_password(user="random"), None) def test_user_onload_modules(self): from frappe.desk.form.load import getdoc From 9f92e7bf0d9a7ebd4f10da113dcb7867228f4c03 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 15 Apr 2026 12:02:20 +0530 Subject: [PATCH 3/8] fix: enhance error handling in password reset process --- frappe/core/doctype/user/user.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index f7d8e19cf1..797894f99d 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1167,7 +1167,12 @@ def reset_password(user: str) -> str: # For Administrator or disabled users: silently skip — same response below except frappe.DoesNotExistError: frappe.clear_messages() - # Do not reveal whether the account exists — fall through to generic response + 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()) return frappe.msgprint( msg=_("If an account with this email exists, password reset instructions have been sent."), From fe1edb3c01f36fc8fb0abc29cd4b5ac98cab533a Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 15 Apr 2026 12:04:53 +0530 Subject: [PATCH 4/8] fix: change return type to None --- frappe/core/doctype/user/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 797894f99d..6c6c4375a2 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1154,7 +1154,7 @@ 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). @@ -1174,7 +1174,7 @@ def reset_password(user: str) -> str: frappe.clear_messages() frappe.log_error(title="Password reset failed unexpectedly", message=frappe.get_traceback()) - return frappe.msgprint( + frappe.msgprint( msg=_("If an account with this email exists, password reset instructions have been sent."), title=_("Password Reset"), ) From 33077e0a2c98377dc6112473618a98ee8fca4cd1 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 15 Apr 2026 12:13:19 +0530 Subject: [PATCH 5/8] test: ensure consistent response and messaging in password reset functionality --- frappe/core/doctype/user/test_user.py | 19 ++++++++++++++++--- frappe/templates/includes/login/login.js | 1 - 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 0884c010b0..fc1a905f59 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -437,9 +437,22 @@ class TestUser(IntegrationTestCase): 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"), None) - self.assertEqual(reset_password(user="random"), None) + # 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 an account with this email exists, password reset instructions have been sent." + + 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/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index c039c2351a..98ab1acc33 100644 --- a/frappe/templates/includes/login/login.js +++ b/frappe/templates/includes/login/login.js @@ -277,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 }}), }; From 74a5b3c8a375e28a4273bdce92405f25217553a1 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 15 Apr 2026 02:39:17 -0700 Subject: [PATCH 6/8] fix: updated message --- frappe/core/doctype/user/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 6c6c4375a2..367a2ba5a1 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1175,7 +1175,7 @@ def reset_password(user: str) -> None: frappe.log_error(title="Password reset failed unexpectedly", message=frappe.get_traceback()) frappe.msgprint( - msg=_("If an account with this email exists, password reset instructions have been sent."), + msg=_("If this email is registered with us, we’ve sent password reset instructions to it. Please check your inbox."), title=_("Password Reset"), ) From 667787cb47d71b7f2c9a788a8e70b3be2fd25631 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 15 Apr 2026 02:53:07 -0700 Subject: [PATCH 7/8] fix: updated message --- frappe/core/doctype/user/test_user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index fc1a905f59..c80ca7b616 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -431,7 +431,7 @@ class TestUser(IntegrationTestCase): update_password(old_password, old_password=new_password) self.assertEqual( frappe.message_log[0].get("message"), - "If an account with this email exists, password reset instructions have been sent.", + "If this email is registered with us, we’ve sent password reset instructions to it. Please check your inbox.", ) sendmail.assert_called_once() @@ -440,7 +440,7 @@ class TestUser(IntegrationTestCase): # 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 an account with this email exists, password reset instructions have been sent." + _GENERIC_MSG = "If this email is registered with us, we’ve sent password reset instructions to it. Please check your inbox." frappe.clear_messages() self.assertIsNone(reset_password(user="test2@example.com")) From 71613d6fc86521129e37be48faf694c60dcb1fb3 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 15 Apr 2026 15:32:03 +0530 Subject: [PATCH 8/8] fix: correct wording in password reset message for consistency --- frappe/core/doctype/user/test_user.py | 4 ++-- frappe/core/doctype/user/user.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index c80ca7b616..a091eeff2e 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -431,7 +431,7 @@ class TestUser(IntegrationTestCase): update_password(old_password, old_password=new_password) self.assertEqual( frappe.message_log[0].get("message"), - "If this email is registered with us, we’ve sent password reset instructions to it. Please check your inbox.", + "If this email is registered with us, we have sent password reset instructions to it. Please check your inbox.", ) sendmail.assert_called_once() @@ -440,7 +440,7 @@ class TestUser(IntegrationTestCase): # 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’ve sent password reset instructions to it. Please check your inbox." + _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")) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 367a2ba5a1..50bb43e198 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1175,7 +1175,9 @@ def reset_password(user: str) -> None: frappe.log_error(title="Password reset failed unexpectedly", message=frappe.get_traceback()) frappe.msgprint( - msg=_("If this email is registered with us, we’ve sent password reset instructions to it. Please check your inbox."), + msg=_( + "If this email is registered with us, we have sent password reset instructions to it. Please check your inbox." + ), title=_("Password Reset"), )