diff --git a/frappe/core/doctype/system_settings/system_settings.js b/frappe/core/doctype/system_settings/system_settings.js index aa3f4a766b..f8ac1734f9 100644 --- a/frappe/core/doctype/system_settings/system_settings.js +++ b/frappe/core/doctype/system_settings/system_settings.js @@ -13,3 +13,10 @@ frappe.ui.form.on("System Settings", "refresh", function(frm) { }); }); +frappe.ui.form.on("System Settings", "enable_password_policy", function(frm) { + if(frm.doc.enable_password_policy == 0){ + frm.set_value("minimum_password_score", ""); + }else{ + frm.set_value("minimum_password_score", "2"); + } +}); \ No newline at end of file diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 6f68cf3842..d742681469 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -614,6 +614,71 @@ "set_only_once": 0, "unique": 0 }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "0", + "description": "If enabled, the password strength will be enforced based on the Minimum Password Score value. A value of 2 being medium strong and 4 being very strong.", + "fieldname": "enable_password_policy", + "fieldtype": "Check", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Enable Password Policy", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "2", + "depends_on": "eval:doc.enable_password_policy==1", + "fieldname": "minimum_password_score", + "fieldtype": "Select", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Minimum Password Score", + "length": 0, + "no_copy": 0, + "options": "2\n4", + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, { "allow_bulk_edit": 0, "allow_on_submit": 0, @@ -837,7 +902,7 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2017-04-25 12:00:12.846542", + "modified": "2017-05-01 15:27:11.079447", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index 2805151a25..8f64feda9f 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -12,6 +12,13 @@ from frappe.utils.momentjs import get_all_timezones class SystemSettings(Document): def validate(self): + enable_password_policy = cint(self.enable_password_policy) and True or False + minimum_password_score = cint(self.minimum_password_score) or 0 + if enable_password_policy and minimum_password_score <= 0: + frappe.throw(_("Please select Minimum Password Score")) + elif not enable_password_policy: + self.minimum_password_score = "" + for key in ("session_expiry", "session_expiry_mobile"): if self.get(key): parts = self.get(key).split(":") diff --git a/frappe/core/doctype/user/test_records.json b/frappe/core/doctype/user/test_records.json index a2dbb0b689..3f5dd87e55 100644 --- a/frappe/core/doctype/user/test_records.json +++ b/frappe/core/doctype/user/test_records.json @@ -4,7 +4,7 @@ "email": "test@example.com", "enabled": 1, "first_name": "_Test", - "new_password": "testpassword", + "new_password": "Eastern_43A1W", "roles": [ { "doctype": "Has Role", @@ -22,20 +22,20 @@ "doctype": "User", "email": "test1@example.com", "first_name": "_Test1", - "new_password": "testpassword" + "new_password": "Eastern_43A1W" }, { "doctype": "User", "email": "test2@example.com", "first_name": "_Test2", - "new_password": "testpassword", + "new_password": "Eastern_43A1W", "enabled": 1 }, { "doctype": "User", "email": "testperm@example.com", "first_name": "_Test Perm", - "new_password": "testpassword", + "new_password": "Eastern_43A1W", "enabled": 1 }, { @@ -43,7 +43,7 @@ "email": "testdelete@example.com", "enabled": 1, "first_name": "_Test", - "new_password": "testpassword", + "new_password": "Eastern_43A1W", "roles": [ { "doctype": "Has Role", diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 31a1b58320..0b7bcb7ff8 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -11,7 +11,7 @@ from frappe import _dict from frappe.limits import update_limits, clear_limit from frappe.utils import get_url from frappe.core.doctype.user.user import get_total_users -from frappe.core.doctype.user.user import MaxUsersReachedError +from frappe.core.doctype.user.user import MaxUsersReachedError, test_password_strength test_records = frappe.get_test_records('User') @@ -147,7 +147,7 @@ class TestUser(unittest.TestCase): # # allow one session # user = frappe.get_doc('User', 'test@example.com') # user.simultaneous_sessions = 1 - # user.new_password = 'testpassword' + # user.new_password = 'Eastern_43A1W' # user.save() # # def test_request(conn): @@ -158,18 +158,18 @@ class TestUser(unittest.TestCase): # update_site_config('deny_multiple_sessions', 0) # # print 'conn1' - # conn1 = FrappeClient(get_url(), "test@example.com", "testpassword", verify=False) + # conn1 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False) # test_request(conn1) # # print 'conn2' - # conn2 = FrappeClient(get_url(), "test@example.com", "testpassword", verify=False) + # conn2 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False) # test_request(conn2) # # update_site_config('deny_multiple_sessions', 1) # # print 'conn3' # - # conn3 = FrappeClient(get_url(), "test@example.com", "testpassword", verify=False) + # conn3 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False) # test_request(conn3) # # # first connection should fail @@ -178,7 +178,7 @@ class TestUser(unittest.TestCase): def test_site_expiry(self): user = frappe.get_doc('User', 'test@example.com') user.enabled = 1 - user.new_password = 'testpassword' + user.new_password = 'Eastern_43A1W' user.save() update_limits({'expiry': add_to_date(today(), days=-1), 'support_email': 'support@example.com'}) @@ -187,7 +187,7 @@ class TestUser(unittest.TestCase): frappe.db.commit() res = requests.post(get_url(), params={'cmd': 'login', 'usr': - 'test@example.com', 'pwd': 'testpassword', 'device': 'desktop'}) + 'test@example.com', 'pwd': 'Eastern_43A1W', 'device': 'desktop'}) # While site is expired status code returned is 417 Failed Expectation self.assertEqual(res.status_code, 417) @@ -214,3 +214,23 @@ class TestUser(unittest.TestCase): # Clear the user limit clear_limit('users') + def test_password_strength(self): + #Test Password without Password Strenth Policy + frappe.db.set_value("System Settings", "System Settings", "enable_password_policy", 0) + frappe.db.set_value("System Settings", "System Settings", "minimum_password_score", "") + + # Should pass password strength test + result = test_password_strength("test_password") + self.assertEqual(result['feedback']['password_policy_validation_passed'], True) + + # Test Password with Password Strenth Policy Set + frappe.db.set_value("System Settings", "System Settings", "enable_password_policy", 1) + frappe.db.set_value("System Settings", "System Settings", "minimum_password_score", 2) + + #Should fail password strength test + result = test_password_strength("test_password") + self.assertEqual(result['feedback']['password_policy_validation_passed'], False) + + # Should pass password strength test + result = test_password_strength("Eastern_43A1W") + self.assertEqual(result['feedback']['password_policy_validation_passed'], True) \ No newline at end of file diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index cbd839d425..b540601da1 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -64,7 +64,7 @@ frappe.ui.form.on('User', { frm.toggle_display(['sb1', 'sb3', 'modules_access'], true); } - + frm.add_custom_button(__("Reset Password"), function() { frappe.call({ method: "frappe.core.doctype.user.user.reset_password", @@ -125,7 +125,6 @@ frappe.ui.form.on('User', { frm.toggle_enable('email', doc.__islocal); } }, - create_user_email:function(frm) { frappe.call({ method: 'frappe.core.doctype.user.user.has_email_account', @@ -146,7 +145,7 @@ frappe.ui.form.on('User', { frappe.set_route("Form", "Email Account", doc.name); }) } else { - frappe.route_flags.create_user_account = frm.doc.name; + frappe.route_flags.create_user_account = frm.doc.name; frappe.set_route("Form", "Email Account", r.message[0]["name"]); } } diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 1f58d3ab50..ed84cdf8c6 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -49,6 +49,7 @@ class User(Document): self.__new_password = self.new_password self.new_password = "" + self.password_strength_test() if self.name not in STANDARD_USERS: self.validate_email_type(self.email) self.validate_email_type(self.name) @@ -407,6 +408,14 @@ class User(Document): frappe.msgprint(_("Username should not contain any special characters other than letters, numbers and underscore")) self.username = "" + def password_strength_test(self): + if self.__new_password: + user_data = (self.first_name, self.middle_name, self.last_name, self.email, self.birth_date) + result = test_password_strength(self.__new_password, '', None, user_data) + + if not result['feedback']['password_policy_validation_passed']: + handle_password_test_fail(result) + def suggest_username(self): def _check_suggestion(suggestion): if self.username != suggestion and not self.username_exists(suggestion): @@ -494,6 +503,11 @@ def get_perm_info(role): @frappe.whitelist(allow_guest=True) def update_password(new_password, key=None, old_password=None): + result = test_password_strength(new_password, key, old_password) + + if not result['feedback']['password_policy_validation_passed']: + handle_password_test_fail(result) + res = _get_user_for_update_password(key, old_password) if res.get('message'): return res['message'] @@ -519,13 +533,25 @@ def update_password(new_password, key=None, old_password=None): return redirect_url if redirect_url else "/" @frappe.whitelist(allow_guest=True) -def test_password_strength(new_password, key=None, old_password=None): +def test_password_strength(new_password, key=None, old_password=None, user_data=[]): from frappe.utils.password_strength import test_password_strength as _test_password_strength - user_data = frappe.db.get_value('User', frappe.session.user, ['first_name', 'middle_name', 'last_name', 'email', 'birth_date']) + if not user_data: + user_data = frappe.db.get_value('User', frappe.session.user, ['first_name', 'middle_name', 'last_name', 'email', 'birth_date']) if new_password: - return _test_password_strength(new_password, user_inputs=user_data) + result = _test_password_strength(new_password, user_inputs=user_data) + + enable_password_policy = cint(frappe.db.get_single_value("System Settings", "enable_password_policy")) and True or False + minimum_password_score = cint(frappe.db.get_single_value("System Settings", "minimum_password_score")) or 0 + + password_policy_validation_passed = True + if enable_password_policy and result['score'] < minimum_password_score: + password_policy_validation_passed = False + + result['feedback']['password_policy_validation_passed'] = password_policy_validation_passed + + return result #for login @frappe.whitelist() @@ -837,3 +863,10 @@ def extract_mentions(txt): """Find all instances of @username in the string. The mentions will be separated by non-word characters or may appear at the start of the string""" return re.findall(r'(?:[^\w]|^)@([\w]*)', txt) + + +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 += _("{0} Hint : Include Underscores, Numbers and Capital Letters in the password {0} Example : Eastern_43A1W").format("
") + frappe.throw(_('Invalid Password: ' + ' '.join([warning, suggestions]))) \ No newline at end of file diff --git a/frappe/tests/test_password.py b/frappe/tests/test_password.py index acaca6a290..f35f778ab9 100644 --- a/frappe/tests/test_password.py +++ b/frappe/tests/test_password.py @@ -47,8 +47,8 @@ class TestPassword(unittest.TestCase): return frappe.get_doc('Email Account', name) def test_hashed_password(self, user='test@example.com'): - old_password = 'testpassword' - new_password = 'testpassword-new' + old_password = 'Eastern_43A1W' + new_password = 'Eastern_43A1W-new' update_password(user, new_password) diff --git a/frappe/www/update-password.html b/frappe/www/update-password.html index cffcdde37a..632c4e0c9f 100644 --- a/frappe/www/update-password.html +++ b/frappe/www/update-password.html @@ -132,12 +132,10 @@ frappe.ready(function() { feedback.crack_time_display = r.message.crack_time_display; feedback.score = score; - if (score < 2) { - set_strength_indicator('red', feedback); - } else if (score < 4) { - set_strength_indicator('yellow', feedback); - } else { + if(feedback.password_policy_validation_passed){ set_strength_indicator('green', feedback); + }else{ + set_strength_indicator('red', feedback); } } } @@ -148,18 +146,25 @@ frappe.ready(function() { window.set_strength_indicator = function(color, feedback) { var message = []; + feedback.help_msg = ""; + if(!feedback.password_policy_validation_passed){ + feedback.help_msg = __("Hint: Include symbols, numbers and capital letters in the password"); + } if (feedback) { - if (feedback.suggestions && feedback.suggestions.length) { - message = message.concat(feedback.suggestions); - } else if (feedback.warning) { - message.push(feedback.warning); - } + if(!feedback.password_policy_validation_passed){ + if (feedback.suggestions && feedback.suggestions.length) { + feedback.suggestions = feedback.suggestions + ' ' + feedback.help_msg; + message = message.concat(feedback.suggestions); + } else if (feedback.warning) { + feedback.warning = feedback.warning + ' ' + feedback.help_msg; + message.push(feedback.warning); + } - if (!message.length && feedback.crack_time_display) { - message.push(__('This password will take {0} to crack', [feedback.crack_time_display])); - if (feedback.score > 3) { - message.push('👍'); + if (!message.length) { + message.push(feedback.help_msg); } + }else{ + message.push(__('Success! You are good to go 👍')); } }