Merge pull request #38588 from shariquerik/reset-password-fix
This commit is contained in:
commit
304283c222
3 changed files with 44 additions and 33 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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 }}),
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue