From 69335b23ce675f999d00a8007743acab9e3fbadd Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 10 Jan 2022 10:38:31 +0000 Subject: [PATCH 01/14] add reset password link expiry feature --- .../doctype/system_settings/system_settings.json | 12 ++++++++++-- frappe/core/doctype/user/user.json | 9 ++++++++- frappe/core/doctype/user/user.py | 12 ++++++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 61410fb1a8..3fcd69a704 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -42,6 +42,7 @@ "allow_error_traceback", "strip_exif_metadata_from_uploaded_images", "password_settings", + "reset_password_link_expiry_seconds", "logout_on_password_reset", "force_user_to_reset_password", "password_reset_limit", @@ -485,7 +486,14 @@ "fieldtype": "Select", "label": "First Day of the Week", "options": "Sunday\nMonday\nTuesday\nWednesday\nThursday\nFriday\nSaturday" - } + }, + { + "default": "1200", + "fieldname": "reset_password_link_expiry_seconds", + "fieldtype": "Int", + "label": "Reset Password Link Expiry Seconds", + "non_negative": 1 + } ], "icon": "fa fa-cog", "issingle": 1, @@ -509,4 +517,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index a47f539466..a09d9c794d 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -43,6 +43,7 @@ "new_password", "logout_all_sessions", "reset_password_key", + "reset_password_key_datetime", "last_password_reset_date", "redirect_url", "document_follow_notifications_section", @@ -606,7 +607,13 @@ "fieldtype": "Link", "label": "Module Profile", "options": "Module Profile" - } + }, + { + "fieldname": "reset_password_key_datetime", + "fieldtype": "Datetime", + "label": "Reset Password Key Generation Datetime", + "read_only": 1 + } ], "icon": "fa fa-user", "idx": 413, diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index ef7845d3b0..254e15f101 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE from bs4 import BeautifulSoup +from datetime import timedelta import frappe import frappe.share import frappe.defaults @@ -265,6 +266,7 @@ class User(Document): key = random_string(32) self.db_set("reset_password_key", key) + self.db_set("reset_password_key_datetime", now_datetime()) url = "/update-password?key=" + key if password_expired: @@ -728,8 +730,14 @@ def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() if key: - result.user = frappe.db.get_value("User", {"reset_password_key": key}) - if not result.user: + user = frappe.db.get_value("User", {"reset_password_key": key}, ["name", "reset_password_key_datetime"]) + result.user, res_pass_key_datetime = user if user else (None, None) + + if result.user: + res_pass_link_exp_sec = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") + if res_pass_link_exp_sec and now_datetime() > res_pass_key_datetime + timedelta(seconds=res_pass_link_exp_sec): + result.message = _("The Link specified has been expired") + else: result.message = _("The Link specified has either been used before or Invalid") elif old_password: From ceae31f6fe3ad155936e2f8e6493369ba534b37f Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 10 Jan 2022 10:38:31 +0000 Subject: [PATCH 02/14] feat: add reset password link expiry feature --- .../doctype/system_settings/system_settings.json | 12 ++++++++++-- frappe/core/doctype/user/user.json | 9 ++++++++- frappe/core/doctype/user/user.py | 12 ++++++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 61410fb1a8..3fcd69a704 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -42,6 +42,7 @@ "allow_error_traceback", "strip_exif_metadata_from_uploaded_images", "password_settings", + "reset_password_link_expiry_seconds", "logout_on_password_reset", "force_user_to_reset_password", "password_reset_limit", @@ -485,7 +486,14 @@ "fieldtype": "Select", "label": "First Day of the Week", "options": "Sunday\nMonday\nTuesday\nWednesday\nThursday\nFriday\nSaturday" - } + }, + { + "default": "1200", + "fieldname": "reset_password_link_expiry_seconds", + "fieldtype": "Int", + "label": "Reset Password Link Expiry Seconds", + "non_negative": 1 + } ], "icon": "fa fa-cog", "issingle": 1, @@ -509,4 +517,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index a47f539466..a09d9c794d 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -43,6 +43,7 @@ "new_password", "logout_all_sessions", "reset_password_key", + "reset_password_key_datetime", "last_password_reset_date", "redirect_url", "document_follow_notifications_section", @@ -606,7 +607,13 @@ "fieldtype": "Link", "label": "Module Profile", "options": "Module Profile" - } + }, + { + "fieldname": "reset_password_key_datetime", + "fieldtype": "Datetime", + "label": "Reset Password Key Generation Datetime", + "read_only": 1 + } ], "icon": "fa fa-user", "idx": 413, diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index ef7845d3b0..254e15f101 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE from bs4 import BeautifulSoup +from datetime import timedelta import frappe import frappe.share import frappe.defaults @@ -265,6 +266,7 @@ class User(Document): key = random_string(32) self.db_set("reset_password_key", key) + self.db_set("reset_password_key_datetime", now_datetime()) url = "/update-password?key=" + key if password_expired: @@ -728,8 +730,14 @@ def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() if key: - result.user = frappe.db.get_value("User", {"reset_password_key": key}) - if not result.user: + user = frappe.db.get_value("User", {"reset_password_key": key}, ["name", "reset_password_key_datetime"]) + result.user, res_pass_key_datetime = user if user else (None, None) + + if result.user: + res_pass_link_exp_sec = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") + if res_pass_link_exp_sec and now_datetime() > res_pass_key_datetime + timedelta(seconds=res_pass_link_exp_sec): + result.message = _("The Link specified has been expired") + else: result.message = _("The Link specified has either been used before or Invalid") elif old_password: From 4cf462a610e0eeea0b91eb08e79922e8c193876e Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 10 Jan 2022 23:26:22 +0530 Subject: [PATCH 03/14] test: add test case for link expiry --- frappe/core/doctype/user/test_user.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index d1291acfc4..608f79c7b4 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import json +import time import unittest from unittest.mock import patch @@ -371,6 +372,18 @@ class TestUser(unittest.TestCase): doc = frappe.response.docs[0] self.assertListEqual(doc.get("__onload").get('all_modules', []), [m.get("module_name") for m in get_modules_from_all_apps()]) + + def test_reset_password_link_expiry(self): + new_password = "new_password" + + frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 1) + frappe.db.commit() + + 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 + self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "The Link specified has been expired") def delete_contact(user): From f68b623b3bbda118763660b2fb77cbab475cd238 Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 10 Jan 2022 23:51:15 +0530 Subject: [PATCH 04/14] style: add comment --- frappe/core/doctype/user/test_user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 608f79c7b4..fd99f51be9 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -376,6 +376,7 @@ class TestUser(unittest.TestCase): def test_reset_password_link_expiry(self): new_password = "new_password" + # set the reset password expir to 1 second frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 1) frappe.db.commit() From 7276721c185681c35ec8a4f2543bf002c332b8ea Mon Sep 17 00:00:00 2001 From: rajkris Date: Thu, 13 Jan 2022 23:29:48 +0530 Subject: [PATCH 05/14] fix(minor): linting errors --- 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 fd99f51be9..ecf7c2b63b 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -376,9 +376,9 @@ class TestUser(unittest.TestCase): def test_reset_password_link_expiry(self): new_password = "new_password" - # set the reset password expir to 1 second + # set the reset password expiry to 1 second frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 1) - frappe.db.commit() + frappe.db.commit() # nosemgrep: this is to set value for a test case frappe.set_user("testpassword@example.com") test_user = frappe.get_doc("User", "testpassword@example.com") From 56234a836c45d24f3ce34936d84d58ff2a254e0e Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 24 Jan 2022 22:55:01 +0530 Subject: [PATCH 06/14] fix(minor): change var name typo --- 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 da84b8eb10..1362a2472c 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -734,8 +734,8 @@ def _get_user_for_update_password(key, old_password): result.user, res_pass_key_datetime = user if user else (None, None) if result.user: - res_pass_link_exp_sec = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") - if res_pass_link_exp_sec and now_datetime() > res_pass_key_datetime + timedelta(seconds=res_pass_link_exp_sec): + reset_password_link_expiry = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") + if reset_password_link_expiry and now_datetime() > res_pass_key_datetime + timedelta(seconds=reset_password_link_expiry): result.message = _("The Link specified has been expired") else: result.message = _("The Link specified has either been used before or Invalid") From c571421e20dec705d97b5be53f1b07291bbc66b0 Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 24 Jan 2022 22:58:07 +0530 Subject: [PATCH 07/14] fix(minor): change var name typo --- frappe/core/doctype/system_settings/system_settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 3fcd69a704..1f79c37e2b 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -491,7 +491,7 @@ "default": "1200", "fieldname": "reset_password_link_expiry_seconds", "fieldtype": "Int", - "label": "Reset Password Link Expiry Seconds", + "label": "Reset Password Link Expiry (in Seconds)", "non_negative": 1 } ], From 692fde6822c46eb04c5aa733208dbfb66b768b44 Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 24 Jan 2022 23:04:42 +0530 Subject: [PATCH 08/14] fix(minor): add description for field reset_password_key_datetime --- frappe/core/doctype/user/user.json | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index a09d9c794d..378406b34a 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -609,6 +609,7 @@ "options": "Module Profile" }, { + "description": "Stores the datetime when the last reset password key was generated.", "fieldname": "reset_password_key_datetime", "fieldtype": "Datetime", "label": "Reset Password Key Generation Datetime", From e6f4359644842fde07e8309643053734044d357f Mon Sep 17 00:00:00 2001 From: rajkris Date: Thu, 27 Jan 2022 11:42:54 +0530 Subject: [PATCH 09/14] fix(minor): var typo fix --- 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 1362a2472c..7064a3a46f 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -731,11 +731,11 @@ def _get_user_for_update_password(key, old_password): result = frappe._dict() if key: user = frappe.db.get_value("User", {"reset_password_key": key}, ["name", "reset_password_key_datetime"]) - result.user, res_pass_key_datetime = user if user else (None, None) + result.user, reset_password_key_datetime = user if user else (None, None) if result.user: reset_password_link_expiry = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") - if reset_password_link_expiry and now_datetime() > res_pass_key_datetime + timedelta(seconds=reset_password_link_expiry): + if reset_password_link_expiry and now_datetime() > reset_password_key_datetime + timedelta(seconds=reset_password_link_expiry): result.message = _("The Link specified has been expired") else: result.message = _("The Link specified has either been used before or Invalid") From 3aef1321c1cbfdb6faf3667ad5f0b2afe5792a19 Mon Sep 17 00:00:00 2001 From: rajkris Date: Thu, 27 Jan 2022 17:38:20 +0530 Subject: [PATCH 10/14] test: remove db commit in test_reset_password_link_expiry --- frappe/core/doctype/user/test_user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index ecf7c2b63b..568bc8ee23 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -378,7 +378,6 @@ class TestUser(unittest.TestCase): # set the reset password expiry to 1 second frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 1) - frappe.db.commit() # nosemgrep: this is to set value for a test case frappe.set_user("testpassword@example.com") test_user = frappe.get_doc("User", "testpassword@example.com") From 0ad6d24816864589ca0933a1de4f85ca94021490 Mon Sep 17 00:00:00 2001 From: rajkris Date: Tue, 19 Apr 2022 12:38:13 +0530 Subject: [PATCH 11/14] fix: indentation, lints --- frappe/core/doctype/user/test_user.py | 24 ++++++++++++--------- frappe/core/doctype/user/user.py | 31 +++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index cd773eedb0..c59464054e 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -16,6 +16,7 @@ from frappe.utils import get_url user_module = frappe.core.doctype.user.user test_records = frappe.get_test_records('User') + class TestUser(unittest.TestCase): def tearDown(self): # disable password strength test @@ -229,7 +230,8 @@ class TestUser(unittest.TestCase): @Team and - + @Unknown Team please check @@ -286,7 +288,8 @@ class TestUser(unittest.TestCase): self.assertRaisesRegex(frappe.exceptions.ValidationError, "Sign Up is disabled", sign_up, random_user, random_user_name, "/signup") - self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), (1, "Please check your email for verification")) + self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), + (1, "Please check your email for verification")) self.assertEqual(frappe.cache().hget('redirect_after_login', random_user), "/welcome") # re-register @@ -304,7 +307,6 @@ class TestUser(unittest.TestCase): self.assertRaisesRegex(frappe.exceptions.ValidationError, "Throttled", sign_up, frappe.mock('email'), random_user_name, "/signup") - def test_reset_password(self): from frappe.auth import CookieManager, LoginManager from frappe.utils import set_request @@ -319,7 +321,8 @@ class TestUser(unittest.TestCase): 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") - self.assertEqual(update_password(new_password, key="wrong_key"), "The Link specified has either been used before or Invalid") + self.assertEqual(update_password(new_password, key="wrong_key"), + "The Link specified has either been used before or Invalid") # password verification should fail with old password self.assertRaises(frappe.exceptions.AuthenticationError, verify_password, old_password) @@ -328,7 +331,8 @@ class TestUser(unittest.TestCase): # reset password update_password(old_password, old_password=new_password) - self.assertRaisesRegex(frappe.exceptions.ValidationError, "Invalid key type", update_password, "test", 1, ['like', '%']) + self.assertRaisesRegex(frappe.exceptions.ValidationError, "Invalid key type", update_password, "test", 1, + ['like', '%']) password_strength_response = { "feedback": { @@ -339,7 +343,8 @@ class TestUser(unittest.TestCase): # password strength failure test with patch.object(user_module, "test_password_strength", return_value=password_strength_response): - self.assertRaisesRegex(frappe.exceptions.ValidationError, "Fix password", update_password, new_password, 0, test_user.reset_password_key) + self.assertRaisesRegex(frappe.exceptions.ValidationError, "Fix password", update_password, new_password, 0, + test_user.reset_password_key) # test redirect URL for website users @@ -376,18 +381,17 @@ class TestUser(unittest.TestCase): doc = frappe.response.docs[0] self.assertListEqual(doc.get("__onload").get('all_modules', []), [m.get("module_name") for m in get_modules_from_all_apps()]) - + def test_reset_password_link_expiry(self): new_password = "new_password" - # set the reset password expiry to 1 second frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 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 - self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "The Link specified has been expired") + self.assertEqual(update_password(new_password, key=test_user.reset_password_key), + "The Link specified has been expired") def delete_contact(user): diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 97ae0e602f..8b5f073fca 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -22,6 +22,7 @@ from frappe.query_builder import DocType STANDARD_USERS = frappe.STANDARD_USERS + class User(Document): __new_password = None @@ -719,6 +720,7 @@ def get_email_awaiting(user): frappe.qb.update(user_email_table).set(user_email_table.user_email_table, 0).where(user_email_table.parent == user).run() return False + def ask_pass_update(): # update the sys defaults as to awaiting users from frappe.utils import set_default @@ -726,28 +728,28 @@ def ask_pass_update(): password_list = frappe.get_all("User Email", filters={"awaiting_password": True}, pluck="parent", distinct=True) set_default("email_user_password", u','.join(password_list)) + def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() if key: user = frappe.db.get_value("User", {"reset_password_key": key}, ["name", "last_reset_password_key_datetime"]) result.user, last_reset_password_key_datetime = user if user else (None, None) - if result.user: reset_password_link_expiry = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") - if reset_password_link_expiry and now_datetime() > last_reset_password_key_datetime + timedelta(seconds=reset_password_link_expiry): + if reset_password_link_expiry and \ + now_datetime() > last_reset_password_key_datetime + timedelta(seconds=reset_password_link_expiry): result.message = _("The Link specified has been expired") else: result.message = _("The Link specified has either been used before or Invalid") - elif old_password: # verify old password frappe.local.login_manager.check_password(frappe.session.user, old_password) user = frappe.session.user result.user = user - return result + def reset_user_data(user): user_doc = frappe.get_doc("User", user) redirect_url = user_doc.redirect_url @@ -757,10 +759,12 @@ def reset_user_data(user): return user_doc, redirect_url + @frappe.whitelist() def verify_password(password): frappe.local.login_manager.check_password(frappe.session.user, password) + @frappe.whitelist(allow_guest=True) def sign_up(email, full_name, redirect_to): if is_signup_disabled(): @@ -804,6 +808,7 @@ def sign_up(email, full_name, redirect_to): else: return 2, _("Please ask your administrator to verify your sign-up") + @frappe.whitelist(allow_guest=True) @rate_limit(limit=get_password_reset_limit, seconds = 24*60*60, methods=['POST']) def reset_password(user): @@ -827,6 +832,7 @@ def reset_password(user): frappe.clear_messages() return 'not found' + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def user_query(doctype, txt, searchfield, start, page_len, filters): @@ -864,6 +870,7 @@ def user_query(doctype, txt, searchfield, start, page_len, filters): dict(start=start, page_len=page_len, txt=txt) ) + def get_total_users(): """Returns total no. of system users""" return flt(frappe.db.sql('''SELECT SUM(`simultaneous_sessions`) @@ -872,6 +879,7 @@ def get_total_users(): AND `user_type` = 'System User' AND `name` NOT IN ({})'''.format(", ".join(["%s"]*len(STANDARD_USERS))), STANDARD_USERS)[0][0]) + def get_system_users(exclude_users=None, limit=None): if not exclude_users: exclude_users = [] @@ -891,6 +899,7 @@ def get_system_users(exclude_users=None, limit=None): return system_users + def get_active_users(): """Returns No. of system users who logged in, in the last 3 days""" return frappe.db.sql("""select count(*) from `tabUser` @@ -898,16 +907,19 @@ def get_active_users(): and name not in ({}) and hour(timediff(now(), last_active)) < 72""".format(", ".join(["%s"]*len(STANDARD_USERS))), STANDARD_USERS)[0][0] + def get_website_users(): """Returns total no. of website users""" return frappe.db.count("User", filters={"enabled": True, "user_type": "Website User"}) + def get_active_website_users(): """Returns No. of website users who logged in, in the last 3 days""" return frappe.db.sql("""select count(*) from `tabUser` where enabled = 1 and user_type = 'Website User' and hour(timediff(now(), last_active)) < 72""")[0][0] + def get_permission_query_conditions(user): if user=="Administrator": return "" @@ -915,11 +927,13 @@ def get_permission_query_conditions(user): return """(`tabUser`.name not in ({standard_users}))""".format( standard_users = ", ".join(frappe.db.escape(user) for user in STANDARD_USERS)) + def has_permission(doc, user): if (user != "Administrator") and (doc.name in STANDARD_USERS): # dont allow non Administrator user to view / edit Administrator user return False + def notify_admin_access_to_system_manager(login_manager=None): if (login_manager and login_manager.user == "Administrator" @@ -940,6 +954,7 @@ def notify_admin_access_to_system_manager(login_manager=None): header=['Access Notification', 'orange'] ) + def extract_mentions(txt): """Find all instances of @mentions in the html.""" soup = BeautifulSoup(txt, 'html.parser') @@ -957,17 +972,20 @@ def extract_mentions(txt): return emails + def handle_password_test_fail(result): suggestions = result['feedback']['suggestions'][0] if result['feedback']['suggestions'] else '' warning = result['feedback']['warning'] if 'warning' in result['feedback'] else '' suggestions += "
" + _("Hint: Include symbols, numbers and capital letters in the password") + '
' frappe.throw(' '.join([_('Invalid Password:'), warning, suggestions])) + def update_gravatar(name): gravatar = has_gravatar(name) if gravatar: frappe.db.set_value('User', name, 'user_image', gravatar) + def throttle_user_creation(): if frappe.flags.in_import: return @@ -975,16 +993,19 @@ def throttle_user_creation(): if frappe.db.get_creation_count('User', 60) > frappe.local.conf.get("throttle_user_limit", 60): frappe.throw(_('Throttled')) + @frappe.whitelist() def get_role_profile(role_profile): roles = frappe.get_doc('Role Profile', {'role_profile': role_profile}) return roles.roles + @frappe.whitelist() def get_module_profile(module_profile): module_profile = frappe.get_doc('Module Profile', {'module_profile_name': module_profile}) return module_profile.get('block_modules') + def create_contact(user, ignore_links=False, ignore_mandatory=False): from frappe.contacts.doctype.contact.contact import get_contact_name if user.name in ["Administrator", "Guest"]: return @@ -1036,6 +1057,7 @@ def create_contact(user, ignore_links=False, ignore_mandatory=False): contact.save(ignore_permissions=True) + @frappe.whitelist() def generate_keys(user): """ @@ -1061,6 +1083,7 @@ def switch_theme(theme): if theme in ["Dark", "Light", "Automatic"]: frappe.db.set_value("User", frappe.session.user, "desk_theme", theme) + def get_enabled_users(): def _get_enabled_users(): enabled_users = frappe.get_all("User", filters={"enabled": "1"}, pluck="name") From a7a34eaad2f7239c235f683b1ac4f46e87823ef4 Mon Sep 17 00:00:00 2001 From: rajkris Date: Tue, 19 Apr 2022 12:49:10 +0530 Subject: [PATCH 12/14] fix: linter errore --- frappe/core/doctype/user/user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 91ef7587c4..6e7b5162de 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -777,7 +777,6 @@ def ask_pass_update(): set_default("email_user_password", ",".join(password_list)) - def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() From 48947796ce4c4f1dd7642d256b123df278dc5025 Mon Sep 17 00:00:00 2001 From: rajkris Date: Mon, 2 May 2022 21:38:07 +0530 Subject: [PATCH 13/14] fix: fix linter issues --- frappe/core/doctype/user/test_user.py | 25 ++++++++++++++++--------- frappe/core/doctype/user/user.py | 26 ++++++++++++-------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 964603756b..2160c02da1 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -23,7 +23,6 @@ user_module = frappe.core.doctype.user.user test_records = frappe.get_test_records("User") - class TestUser(unittest.TestCase): def tearDown(self): # disable password strength test @@ -320,9 +319,11 @@ class TestUser(unittest.TestCase): "/signup", ) - self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), - (1, "Please check your email for verification")) - self.assertEqual(frappe.cache().hget('redirect_after_login', random_user), "/welcome") + self.assertTupleEqual( + sign_up(random_user, random_user_name, "/welcome"), + (1, "Please check your email for verification"), + ) + self.assertEqual(frappe.cache().hget("redirect_after_login", random_user), "/welcome") # re-register self.assertTupleEqual( @@ -429,19 +430,25 @@ class TestUser(unittest.TestCase): frappe.response.docs = [] getdoc("User", "Administrator") doc = frappe.response.docs[0] - self.assertListEqual(doc.get("__onload").get('all_modules', []), - [m.get("module_name") for m in get_modules_from_all_apps()]) + self.assertListEqual( + doc.get("__onload").get("all_modules", []), + [m.get("module_name") for m in get_modules_from_all_apps()], + ) def test_reset_password_link_expiry(self): new_password = "new_password" # set the reset password expiry to 1 second - frappe.db.set_value("System Settings", "System Settings", "reset_password_link_expiry_seconds", 1) + frappe.db.set_value( + "System Settings", "System Settings", "reset_password_link_expiry_seconds", 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 - self.assertEqual(update_password(new_password, key=test_user.reset_password_key), - "The Link specified has been expired") + self.assertEqual( + update_password(new_password, key=test_user.reset_password_key), + "The Link specified has been expired", + ) def delete_contact(user): diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 6e7b5162de..4725bca7a3 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1,7 +1,9 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -from bs4 import BeautifulSoup from datetime import timedelta + +from bs4 import BeautifulSoup + import frappe import frappe.defaults import frappe.permissions @@ -781,12 +783,17 @@ def _get_user_for_update_password(key, old_password): # verify old password result = frappe._dict() if key: - user = frappe.db.get_value("User", {"reset_password_key": key}, ["name", "last_reset_password_key_datetime"]) + user = frappe.db.get_value( + "User", {"reset_password_key": key}, ["name", "last_reset_password_key_datetime"] + ) result.user, last_reset_password_key_datetime = user if user else (None, None) if result.user: - reset_password_link_expiry = frappe.db.get_single_value("System Settings", "reset_password_link_expiry_seconds") - if reset_password_link_expiry and \ - now_datetime() > last_reset_password_key_datetime + timedelta(seconds=reset_password_link_expiry): + reset_password_link_expiry = frappe.db.get_single_value( + "System Settings", "reset_password_link_expiry_seconds" + ) + if reset_password_link_expiry and now_datetime() > last_reset_password_key_datetime + timedelta( + seconds=reset_password_link_expiry + ): result.message = _("The Link specified has been expired") else: result.message = _("The Link specified has either been used before or Invalid") @@ -888,7 +895,6 @@ def reset_password(user): return "not found" - @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def user_query(doctype, txt, searchfield, start, page_len, filters): @@ -945,7 +951,6 @@ def get_total_users(): ) - def get_system_users(exclude_users=None, limit=None): if not exclude_users: exclude_users = [] @@ -983,7 +988,6 @@ def get_active_users(): )[0][0] - def get_website_users(): """Returns total no. of website users""" return frappe.db.count("User", filters={"enabled": True, "user_type": "Website User"}) @@ -998,7 +1002,6 @@ def get_active_website_users(): )[0][0] - def get_permission_query_conditions(user): if user == "Administrator": return "" @@ -1008,7 +1011,6 @@ def get_permission_query_conditions(user): ) - def has_permission(doc, user): if (user != "Administrator") and (doc.name in STANDARD_USERS): # dont allow non Administrator user to view / edit Administrator user @@ -1066,14 +1068,12 @@ def handle_password_test_fail(result): frappe.throw(" ".join([_("Invalid Password:"), warning, suggestions])) - def update_gravatar(name): gravatar = has_gravatar(name) if gravatar: frappe.db.set_value("User", name, "user_image", gravatar) - def throttle_user_creation(): if frappe.flags.in_import: return @@ -1082,7 +1082,6 @@ def throttle_user_creation(): frappe.throw(_("Throttled")) - @frappe.whitelist() def get_role_profile(role_profile): roles = frappe.get_doc("Role Profile", {"role_profile": role_profile}) @@ -1095,7 +1094,6 @@ def get_module_profile(module_profile): return module_profile.get("block_modules") - def create_contact(user, ignore_links=False, ignore_mandatory=False): from frappe.contacts.doctype.contact.contact import get_contact_name From e9e839af014054825f295f31d87baf6c24f8bbc0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 19 May 2022 10:30:02 +0530 Subject: [PATCH 14/14] fix: Use duration field and update fieldname for reset password link expiry --- .../system_settings/system_settings.json | 20 +++++++++---------- frappe/core/doctype/user/test_user.py | 6 +++--- frappe/core/doctype/user/user.json | 6 +++--- frappe/core/doctype/user/user.py | 20 ++++++++++--------- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 1dc5e55e04..52ff5e38e4 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -41,9 +41,9 @@ "allow_error_traceback", "strip_exif_metadata_from_uploaded_images", "password_settings", - "reset_password_link_expiry_seconds", "logout_on_password_reset", "force_user_to_reset_password", + "reset_password_link_expiry_duration", "password_reset_limit", "column_break_31", "enable_password_policy", @@ -482,13 +482,6 @@ "options": "Sunday\nMonday\nTuesday\nWednesday\nThursday\nFriday\nSaturday" }, { - "default": "1200", - "fieldname": "reset_password_link_expiry_seconds", - "fieldtype": "Int", - "label": "Reset Password Link Expiry (in Seconds)", - "non_negative": 1 - }, - { "fieldname": "column_break_64", "fieldtype": "Column Break" }, @@ -503,12 +496,19 @@ "fieldname": "disable_change_log_notification", "fieldtype": "Check", "label": "Disable Change Log Notification" + }, + { + "default": "1200", + "fieldname": "reset_password_link_expiry_duration", + "fieldtype": "Duration", + "label": "Reset Password Link Expiry Duration", + "non_negative": 1 } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2022-05-09 18:53:35.218721", + "modified": "2022-05-19 00:00:18.095269", "modified_by": "Administrator", "module": "Core", "name": "System Settings", @@ -527,4 +527,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} +} \ No newline at end of file diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 2160c02da1..d5e0a108b5 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -367,7 +367,7 @@ class TestUser(unittest.TestCase): self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "/app") self.assertEqual( update_password(new_password, key="wrong_key"), - "The Link specified has either been used before or Invalid", + "The reset password link has either been used before or is invalid", ) # password verification should fail with old password @@ -439,7 +439,7 @@ class TestUser(unittest.TestCase): new_password = "new_password" # set the reset password expiry to 1 second frappe.db.set_value( - "System Settings", "System Settings", "reset_password_link_expiry_seconds", 1 + "System Settings", "System Settings", "reset_password_link_expiry_duration", 1 ) frappe.set_user("testpassword@example.com") test_user = frappe.get_doc("User", "testpassword@example.com") @@ -447,7 +447,7 @@ class TestUser(unittest.TestCase): time.sleep(1) # sleep for 1 sec to expire the reset link self.assertEqual( update_password(new_password, key=test_user.reset_password_key), - "The Link specified has been expired", + "The reset password link has been expired", ) diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index 9ec605ca7c..42122ebfda 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -43,7 +43,7 @@ "new_password", "logout_all_sessions", "reset_password_key", - "last_reset_password_key_datetime", + "last_reset_password_key_generated_on", "last_password_reset_date", "redirect_url", "document_follow_notifications_section", @@ -616,10 +616,10 @@ }, { "description": "Stores the datetime when the last reset password key was generated.", - "fieldname": "last_reset_password_key_datetime", + "fieldname": "last_reset_password_key_generated_on", "fieldtype": "Datetime", "hidden": 1, - "label": "Last Reset Password Key Generation Datetime", + "label": "Last Reset Password Key Generated On", "read_only": 1 }, { diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index d8bb1068a5..1ff5c98a91 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -278,7 +278,7 @@ class User(Document): key = random_string(32) self.db_set("reset_password_key", key) - self.db_set("last_reset_password_key_datetime", now_datetime()) + self.db_set("last_reset_password_key_generated_on", now_datetime()) url = "/update-password?key=" + key if password_expired: @@ -784,19 +784,21 @@ def _get_user_for_update_password(key, old_password): result = frappe._dict() if key: user = frappe.db.get_value( - "User", {"reset_password_key": key}, ["name", "last_reset_password_key_datetime"] + "User", {"reset_password_key": key}, ["name", "last_reset_password_key_generated_on"] ) - result.user, last_reset_password_key_datetime = user if user else (None, None) + result.user, last_reset_password_key_generated_on = user or (None, None) if result.user: - reset_password_link_expiry = frappe.db.get_single_value( - "System Settings", "reset_password_link_expiry_seconds" + reset_password_link_expiry = cint( + frappe.db.get_single_value("System Settings", "reset_password_link_expiry_duration") ) - if reset_password_link_expiry and now_datetime() > last_reset_password_key_datetime + timedelta( - seconds=reset_password_link_expiry + if ( + reset_password_link_expiry + and now_datetime() + > last_reset_password_key_generated_on + timedelta(seconds=reset_password_link_expiry) ): - result.message = _("The Link specified has been expired") + result.message = _("The reset password link has been expired") else: - result.message = _("The Link specified has either been used before or Invalid") + result.message = _("The reset password link has either been used before or is invalid") elif old_password: # verify old password frappe.local.login_manager.check_password(frappe.session.user, old_password)