Merge pull request #24857 from ankush/refactor_password_reset

refactor: Reset password flow
This commit is contained in:
Ankush Menat 2024-02-11 20:56:13 +05:30 committed by GitHub
commit d0de5da21e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 82 additions and 268 deletions

View file

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

View file

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

View file

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

View file

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

View file

@ -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<br>\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": ""
}

View file

@ -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<br><br>\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": "<h5 style=\"line-height:1.5;\">Click to visit the Workspace</h5>",
"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": ""
}

View file

@ -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<br>\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"
}

View file

@ -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

View file

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

View file

@ -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."):

View file

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

View file

@ -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);