Password Strength Policy Enhancement (#3194)

* Add or_filters filter for export_fixtures, to filter by doctype or fieldnames

Eg:
fixtures = [
        {
        "doctype": "Custom Field",
        "or_filters": {
            "dt": ["in", [
                "Process Payroll",
                "Journal Entry Account"
            ]],
            "name": ["in", [
                "Print Settings-compact_item_print",
                "Account-account_id",
                "Account-some_bank_name"
            ]]
        }
    }
]

* Passsword Policy Enhancement

In Security Settings, the Password Plociy can be enabled and the passcord strength score can be set.

User will see helpful password validation messages when they change password.

* Enhance Update Password to handle Password Policy

* remove function argument

* update test cases to use strong password

* Add test cases, add default value for minimum password score, make error messages translatable

* make message translatable

* Update update-password.html

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");
+    }
+});

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

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(":")

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

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)

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"]);
 				}
 			}

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("<br/>")
+	frappe.throw(_('Invalid Password: ' + ' '.join([warning, suggestions])))

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)
 

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 👍'));
 			}
 		}