diff --git a/frappe/core/doctype/user/patches/__init__.py b/frappe/core/doctype/user/patches/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/user/patches/hash_reset_password_tokens.py b/frappe/core/doctype/user/patches/hash_reset_password_tokens.py new file mode 100644 index 0000000000..85a7f5ec80 --- /dev/null +++ b/frappe/core/doctype/user/patches/hash_reset_password_tokens.py @@ -0,0 +1,16 @@ +import frappe +from frappe.utils.data import sha256_hash + + +def execute(): + """hash reset password tokens""" + + users = frappe.get_all("User", {"reset_password_key": ("is", "set")}, ["name", "reset_password_key"]) + for user in users: + frappe.db.set_value( + "User", + user.name, + "reset_password_key", + sha256_hash(user.reset_password_key), + update_modified=False, + ) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 8ab6b9500d..910844a8b7 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -3,6 +3,7 @@ import json import time from unittest.mock import patch +from urllib.parse import parse_qs, urlparse import frappe import frappe.exceptions @@ -17,7 +18,7 @@ from frappe.core.doctype.user.user import ( from frappe.desk.notifications import extract_mentions from frappe.frappeclient import FrappeClient from frappe.model.delete_doc import delete_doc -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import get_url user_module = frappe.core.doctype.user.user @@ -32,10 +33,14 @@ class TestUser(FrappeTestCase): frappe.db.set_single_value("System Settings", "password_reset_limit", 3) frappe.set_user("Administrator") + @staticmethod + def reset_password(user) -> str: + link = user.reset_password() + return parse_qs(urlparse(link).query)["key"][0] + def test_user_type(self): - new_user = frappe.get_doc( - doctype="User", email="test-for-type@example.com", first_name="Tester" - ).insert(ignore_if_duplicate=True) + user_id = frappe.generate_hash() + "@example.com" + new_user = frappe.get_doc(doctype="User", email=user_id, first_name="Tester").insert() self.assertEqual(new_user.user_type, "Website User") # social login userid for frappe @@ -269,11 +274,8 @@ class TestUser(FrappeTestCase): """ self.assertListEqual(extract_mentions(comment), ["test@example.com", "test1@example.com"]) + @change_settings("System Settings", commit=True, password_reset_limit=1) def test_rate_limiting_for_reset_password(self): - # Allow only one reset request for a day - frappe.db.set_single_value("System Settings", "password_reset_limit", 1) - frappe.db.commit() - url = get_url() data = {"cmd": "frappe.core.doctype.user.user.reset_password", "user": "test@test.com"} @@ -351,6 +353,7 @@ class TestUser(FrappeTestCase): "/signup", ) + @change_settings("System Settings", password_reset_limit=6) def test_reset_password(self): from frappe.auth import CookieManager, LoginManager from frappe.utils import set_request @@ -363,12 +366,11 @@ class TestUser(FrappeTestCase): frappe.local.login_manager = LoginManager() # used by rate limiter when calling reset_password frappe.local.request_ip = "127.0.0.69" - frappe.db.set_single_value("System Settings", "password_reset_limit", 6) frappe.set_user("testpassword@example.com") test_user = frappe.get_doc("User", "testpassword@example.com") - test_user.reset_password() - self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "/app") + key = self.reset_password(test_user) + self.assertEqual(update_password(new_password, key=key), "/app") self.assertEqual( update_password(new_password, key="wrong_key"), "The reset password link has either been used before or is invalid", @@ -409,7 +411,9 @@ class TestUser(FrappeTestCase): test_user = frappe.get_doc("User", "test2@example.com") self.assertEqual(reset_password(user="test2@example.com"), None) test_user.reload() - self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "/") + link = sendmail.call_args_list[0].kwargs["args"]["link"] + key = parse_qs(urlparse(link).query)["key"][0] + self.assertEqual(update_password(new_password, key=key), "/") update_password(old_password, old_password=new_password) self.assertEqual( frappe.message_log[0].get("message"), @@ -435,16 +439,16 @@ class TestUser(FrappeTestCase): sorted(m.get("module_name") for m in get_modules_from_all_apps()), ) + @change_settings("System Settings", reset_password_link_expiry_duration=1) def test_reset_password_link_expiry(self): new_password = "new_password" - # set the reset password expiry to 1 second - frappe.db.set_single_value("System Settings", "reset_password_link_expiry_duration", 1) frappe.set_user("testpassword@example.com") test_user = frappe.get_doc("User", "testpassword@example.com") - test_user.reset_password() - time.sleep(1) # sleep for 1 sec to expire the reset link + key = self.reset_password(test_user) + time.sleep(1) + self.assertEqual( - update_password(new_password, key=test_user.reset_password_key), + update_password(new_password, key=key), "The reset password link has been expired", ) diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index 889984a95e..894216a3e0 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -329,6 +329,7 @@ "hidden": 1, "label": "Reset Password Key", "no_copy": 1, + "permlevel": 1, "print_hide": 1, "read_only": 1 }, @@ -627,6 +628,7 @@ "fieldtype": "Datetime", "hidden": 1, "label": "Last Reset Password Key Generated On", + "permlevel": 1, "read_only": 1 }, { @@ -771,7 +773,7 @@ "link_fieldname": "user" } ], - "modified": "2024-02-09 19:34:51.081079", + "modified": "2024-02-11 13:16:29.574666", "modified_by": "Administrator", "module": "Core", "name": "User", diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 97c5a503c8..4f5009b9c8 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -31,6 +31,7 @@ from frappe.utils import ( now_datetime, today, ) +from frappe.utils.data import sha256_hash from frappe.utils.deprecations import deprecated, deprecation_warning from frappe.utils.password import check_password, get_password_reset_limit from frappe.utils.password import update_password as _update_password @@ -404,10 +405,11 @@ class User(Document): pass def reset_password(self, send_email=False, password_expired=False): - from frappe.utils import get_url, random_string + from frappe.utils import get_url - key = random_string(32) - self.db_set("reset_password_key", key) + key = frappe.generate_hash() + hashed_key = sha256_hash(key) + self.db_set("reset_password_key", hashed_key) self.db_set("last_reset_password_key_generated_on", now_datetime()) url = "/update-password?key=" + key @@ -948,8 +950,9 @@ def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() if key: + hashed_key = sha256_hash(key) user = frappe.db.get_value( - "User", {"reset_password_key": key}, ["name", "last_reset_password_key_generated_on"] + "User", {"reset_password_key": hashed_key}, ["name", "last_reset_password_key_generated_on"] ) result.user, last_reset_password_key_generated_on = user or (None, None) if result.user: @@ -1041,11 +1044,10 @@ def sign_up(email: str, full_name: str, redirect_to: str) -> tuple[int, str]: @frappe.whitelist(allow_guest=True) @rate_limit(limit=get_password_reset_limit, seconds=60 * 60) def reset_password(user: str) -> str: - if user == "Administrator": - return "not allowed" - try: user: User = frappe.get_doc("User", user) + if user.name == "Administrator": + return "not allowed" if not user.enabled: return "disabled" diff --git a/frappe/core/form_tour/user_list_tour/user_list_tour.json b/frappe/core/form_tour/user_list_tour/user_list_tour.json deleted file mode 100644 index 83ae481d25..0000000000 --- a/frappe/core/form_tour/user_list_tour/user_list_tour.json +++ /dev/null @@ -1,95 +0,0 @@ -{ - "creation": "2023-05-24 12:53:02.844582", - "dashboard_name": "", - "docstatus": 0, - "doctype": "Form Tour", - "first_document": 0, - "idx": 0, - "include_name_field": 0, - "is_standard": 1, - "list_name": "List", - "modified": "2023-05-24 13:21:29.552864", - "modified_by": "Administrator", - "module": "Core", - "name": "User List Tour", - "new_document_form": 0, - "owner": "Administrator", - "page_name": "", - "page_route": "[\"List\",\"User\",\"List\"]", - "reference_doctype": "User", - "report_name": "", - "save_on_complete": 0, - "steps": [ - { - "description": "List view shows all the documents for a particular DocType. Here you can see all the current enabled users in the system. ", - "element_selector": ".frappe-list", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Top Center", - "title": "Users List", - "ui_tour": 1 - }, - { - "description": "These are filters. You can use them to narrow down list of records.", - "element_selector": ".standard-filter-section.flex", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 1, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Bottom", - "title": "Filters", - "ui_tour": 1 - }, - { - "description": "When standard filters are not enough you can use advance filters. ", - "element_selector": ".filter-selector > .btn-group", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "ondemand_description": "Advance filters are applied on fields with different operators. \n
\nClick on \"Apply Filters\" to continue.", - "popover_element": 0, - "position": "Left", - "title": "Advanced Filters", - "ui_tour": 1 - }, - { - "description": "Let's create a new user.", - "element_selector": ".btn-primary.primary-action", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 1, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 1, - "offset_x": 0, - "offset_y": 0, - "parent_element_selector": "", - "popover_element": 0, - "position": "Bottom", - "title": "New User", - "ui_tour": 1 - } - ], - "title": "User List Tour", - "track_steps": 1, - "ui_tour": 1, - "view_name": "List", - "workspace_name": "" -} \ No newline at end of file diff --git a/frappe/desk/form_tour/main_workspace_tour/main_workspace_tour.json b/frappe/desk/form_tour/main_workspace_tour/main_workspace_tour.json deleted file mode 100644 index afd0583cfb..0000000000 --- a/frappe/desk/form_tour/main_workspace_tour/main_workspace_tour.json +++ /dev/null @@ -1,79 +0,0 @@ -{ - "creation": "2023-05-18 12:08:23.196462", - "dashboard_name": "", - "docstatus": 0, - "doctype": "Form Tour", - "first_document": 0, - "idx": 0, - "include_name_field": 0, - "is_standard": 1, - "list_name": "", - "modified": "2023-05-24 12:43:43.741781", - "modified_by": "Administrator", - "module": "Desk", - "name": "Main Workspace Tour", - "new_document_form": 0, - "owner": "Administrator", - "page_name": "", - "page_route": "[\"Workspaces\",\"*\"]", - "reference_doctype": "", - "report_name": "", - "save_on_complete": 0, - "steps": [ - { - "description": "This is Awesomebar, it helps you to navigate anywhere in the system, find documents, reports, settings, create new records and many more things.", - "element_selector": "#navbar-search", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "parent_element_selector": ".input-group.search-bar", - "popover_element": 0, - "position": "Left", - "title": "Awesomebar", - "ui_tour": 1 - }, - { - "description": "These are workspaces. Each module workspace provides insightful information and shortcuts on one page. \n\n

\n\nTip: You can build custom workspaces for your needs.", - "element_selector": ".col-lg-2.layout-side-section", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Right", - "title": "Workspace List", - "ui_tour": 1 - }, - { - "description": "
Click to visit the Workspace
", - "element_selector": ".desk-sidebar-item.standard-sidebar-item > [title=\"Users\"]", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 1, - "is_table_field": 0, - "modal_trigger": 0, - "next_form_tour": "New Tools Tour", - "next_on_click": 1, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Right", - "title": "Users Workspace", - "ui_tour": 1 - } - ], - "title": "Main Workspace Tour", - "track_steps": 1, - "ui_tour": 1, - "view_name": "Workspaces", - "workspace_name": "" -} \ No newline at end of file diff --git a/frappe/desk/form_tour/users_workspace_tour/users_workspace_tour.json b/frappe/desk/form_tour/users_workspace_tour/users_workspace_tour.json deleted file mode 100644 index 97159ba6e3..0000000000 --- a/frappe/desk/form_tour/users_workspace_tour/users_workspace_tour.json +++ /dev/null @@ -1,62 +0,0 @@ -{ - "creation": "2023-05-24 12:50:23.740052", - "dashboard_name": "", - "docstatus": 0, - "doctype": "Form Tour", - "first_document": 0, - "idx": 0, - "include_name_field": 0, - "is_standard": 1, - "list_name": "", - "modified": "2023-05-24 13:01:56.539128", - "modified_by": "Administrator", - "module": "Desk", - "name": "Users Workspace Tour", - "new_document_form": 0, - "owner": "Administrator", - "page_name": "", - "page_route": "[\"Workspaces\",\"Users\"]", - "reference_doctype": "", - "report_name": "", - "save_on_complete": 0, - "steps": [ - { - "description": "This is Users Workspace. You'll find all shortcuts for user, roles and permission management here.", - "element_selector": ".codex-editor", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 0, - "is_table_field": 0, - "modal_trigger": 0, - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Left", - "title": "Workspace", - "ui_tour": 1 - }, - { - "description": "This is a shortcut to User DocType. \n
\n\nLet's Click on the User shortcut to explore all users in System.", - "element_selector": "[shortcut_name=\"User\"]", - "fieldtype": "0", - "has_next_condition": 0, - "hide_buttons": 1, - "is_table_field": 0, - "modal_trigger": 0, - "next_form_tour": "User List Tour", - "next_on_click": 0, - "offset_x": 0, - "offset_y": 0, - "popover_element": 0, - "position": "Right", - "title": "Users Shortcut", - "ui_tour": 1 - } - ], - "title": "Users Workspace Tour", - "track_steps": 1, - "ui_tour": 1, - "view_name": "Workspaces", - "workspace_name": "Users" -} \ No newline at end of file diff --git a/frappe/patches.txt b/frappe/patches.txt index e2a0f3e849..a09008d921 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -233,4 +233,5 @@ frappe.patches.v15_0.set_file_type frappe.core.doctype.data_import.patches.remove_stale_docfields_from_legacy_version frappe.patches.v15_0.validate_newsletter_recipients frappe.patches.v15_0.sanitize_workspace_titles -frappe.patches.v15_0.migrate_role_profile_to_table_multi_select \ No newline at end of file +frappe.patches.v15_0.migrate_role_profile_to_table_multi_select +frappe.core.doctype.user.patches.hash_reset_password_tokens diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 6ab87f23bf..312a9f3228 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -70,6 +70,7 @@ from frappe.utils.data import ( nowtime, pretty_date, rounded, + sha256_hash, to_timedelta, validate_python_code, ) @@ -1323,3 +1324,12 @@ class TestChangeLog(FrappeTestCase): self.assertIsNone(repo) self.assertRaises(ValueError, parse_github_url, remote_url=None) + + +class TestCrypto(FrappeTestCase): + def test_hashing(self): + self.assertEqual(sha256_hash(""), "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + self.assertEqual( + sha256_hash(b"The quick brown fox jumps over the lazy dog"), + "d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592", + ) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index a1f0c29941..fcc4f75de7 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -222,7 +222,7 @@ def _restore_thread_locals(flags): @contextmanager -def change_settings(doctype, settings_dict=None, /, **settings): +def change_settings(doctype, settings_dict=None, /, commit=False, **settings): """A context manager to ensure that settings are changed before running function and restored after running it regardless of exceptions occurred. This is useful in tests where you want to make changes in a function but @@ -255,6 +255,8 @@ def change_settings(doctype, settings_dict=None, /, **settings): settings.save(ignore_permissions=True) # singles are cached by default, clear to avoid flake frappe.db.value_cache[settings] = {} + if commit: + frappe.db.commit() yield # yield control to calling function finally: @@ -263,6 +265,8 @@ def change_settings(doctype, settings_dict=None, /, **settings): for key, value in previous_settings.items(): setattr(settings, key, value) settings.save(ignore_permissions=True) + if commit: + frappe.db.commit() def timeout(seconds=30, error_message="Test timed out."): diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 2ba5ea2d5f..fc07b4ad8f 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -3,6 +3,7 @@ import base64 import datetime +import hashlib import json import math import operator @@ -2248,6 +2249,13 @@ def generate_hash(*args, **kwargs) -> str: return frappe.generate_hash(*args, **kwargs) +def sha256_hash(input: str | bytes) -> str: + """Return hash of the string using sha256 algorithm.""" + if isinstance(input, str): + input = input.encode() + return hashlib.sha256(input).hexdigest() + + def dict_with_keys(dict, keys): """Return a new dict with a subset of keys.""" out = {} diff --git a/frappe/www/update-password.html b/frappe/www/update-password.html index 43a94c012d..96c24c0b07 100644 --- a/frappe/www/update-password.html +++ b/frappe/www/update-password.html @@ -186,9 +186,8 @@ frappe.ready(function() { window.timout_password_strength = setTimeout(window.test_password_strength, 200); }); - $("#old_password, #new_password, #confirm_password").on("keyup", frappe.utils.debounce(function () { - let common_conditions = new_password.val() && confirm_password.val() && new_password.val() === confirm_password.val() && - password_strength_message.text() === password_strength_message_success + $("#old_password, #new_password, #confirm_password").on("keyup paste", frappe.utils.debounce(function () { + let common_conditions = new_password.val() && confirm_password.val() && new_password.val() === confirm_password.val() if (new_password.val() && old_password.val() === new_password.val()) { password_mismatch_message.text(password_not_same_as_old_password) @@ -211,7 +210,7 @@ frappe.ready(function() { .removeClass("hidden text-muted").addClass("text-danger"); password_strength_message.addClass("hidden"); } - if ((key || (!key && old_password.val() && password_mismatch_message.text() !== password_not_same_as_old_password )) && common_conditions ) { + if ((key || (!key && old_password.val() )) && common_conditions ) { update_button.prop("disabled", false).css("cursor", "pointer"); } else { @@ -250,9 +249,13 @@ frappe.ready(function() { var score = r.message.score, feedback = r.message.feedback; + if (!feedback) { + return; + } + feedback.score = score; - if(feedback.password_policy_validation_passed){ + if (feedback.password_policy_validation_passed) { set_strength_indicator('green', feedback); }else{ set_strength_indicator('red', feedback);