refactor: Reset password flow

- Hash one time reset tokens instead of storing them as is
- Up the perm level
- Use better source of randomness for generating token
- minor code cleanup here and there

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,
+		)

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",

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"
 

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
+frappe.patches.v15_0.migrate_role_profile_to_table_multi_select
+frappe.core.doctype.user.patches.hash_reset_password_tokens

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",
+		)

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 = {}