From 8aa8ea0ee223a2f459f0bd1e72d057c1a9afa9aa Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:46:57 +0100 Subject: [PATCH 1/6] feat: bump zxcvbn version zxcvbn 4.4.28 no longer crashes on long, random passwords. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 484fd13d1b..bd4628b504 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,7 +69,7 @@ dependencies = [ "terminaltables~=3.1.0", "traceback-with-variables~=2.0.4", "xlrd~=2.0.1", - "zxcvbn-python~=4.4.24", + "zxcvbn-python~=4.4.28", "markdownify~=0.11.2", # integration dependencies From d5a72b16ce5720265351780f92c4ac85c9e615de Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:48:07 +0100 Subject: [PATCH 2/6] fix: trim long passwords before check In order for the check to pass in a reasonable amount of time. --- frappe/utils/password_strength.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/utils/password_strength.py b/frappe/utils/password_strength.py index 59c784e5b4..f9d82268e4 100644 --- a/frappe/utils/password_strength.py +++ b/frappe/utils/password_strength.py @@ -12,6 +12,12 @@ from frappe import _ def test_password_strength(password, user_inputs=None): """Wrapper around zxcvbn.password_strength""" + if len(password) > 128: + # zxcvbn takes forever when checking long, random passwords. + # repetion patterns or user inputs in the first 128 characters + # will still be checked. + password = password[:128] + result = zxcvbn(password, user_inputs) result.update({"feedback": get_feedback(result.get("score"), result.get("sequence"))}) return result From e61d87827b728e5fa53128d4e5c6a50191b49049 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:50:17 +0100 Subject: [PATCH 3/6] refactor: imports - remove unused imports - import only the required patterns, not the entire file --- frappe/utils/password_strength.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/frappe/utils/password_strength.py b/frappe/utils/password_strength.py index f9d82268e4..9b32e168bb 100644 --- a/frappe/utils/password_strength.py +++ b/frappe/utils/password_strength.py @@ -1,10 +1,8 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -try: - from zxcvbn import zxcvbn -except Exception: - import zxcvbn +from zxcvbn import zxcvbn +from zxcvbn.scoring import ALL_UPPER, START_UPPER import frappe from frappe import _ @@ -27,13 +25,7 @@ def test_password_strength(password, user_inputs=None): # ------------------------------------------- # feedback functionality code from https://github.com/sans-serif/python-zxcvbn/blob/master/zxcvbn/feedback.py # see license for feedback code at https://github.com/sans-serif/python-zxcvbn/blob/master/LICENSE.txt - -# Used for regex matching capitalization -import re - -# Used to get the regex patterns for capitalization -# (Used the same way in the original zxcvbn) -from zxcvbn import scoring +# ------------------------------------------- # Default feedback value default_feedback = { @@ -183,9 +175,9 @@ def get_dictionary_match_feedback(match, is_sole_match): word = match.get("token") # Variations of the match like UPPERCASES - if scoring.START_UPPER.match(word): + if START_UPPER.match(word): suggestions.append(_("Capitalization doesn't help very much.")) - elif scoring.ALL_UPPER.match(word): + elif ALL_UPPER.match(word): suggestions.append(_("All-uppercase is almost as easy to guess as all-lowercase.")) # Match contains l33t speak substitutions From 2148dc745e4027e290caccdbd84cf544dc4a638c Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:51:47 +0100 Subject: [PATCH 4/6] refactor: assign instead of update --- frappe/utils/password_strength.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/password_strength.py b/frappe/utils/password_strength.py index 9b32e168bb..eada58c638 100644 --- a/frappe/utils/password_strength.py +++ b/frappe/utils/password_strength.py @@ -17,7 +17,7 @@ def test_password_strength(password, user_inputs=None): password = password[:128] result = zxcvbn(password, user_inputs) - result.update({"feedback": get_feedback(result.get("score"), result.get("sequence"))}) + result["feedback"] = get_feedback(result.get("score"), result.get("sequence")) return result From 92e684d4fcbed6aa67b9ba2621275b0f3ec2af79 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 14:07:34 +0100 Subject: [PATCH 5/6] fix: use new source for zxcvbn --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index bd4628b504..965990e028 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,7 +69,7 @@ dependencies = [ "terminaltables~=3.1.0", "traceback-with-variables~=2.0.4", "xlrd~=2.0.1", - "zxcvbn-python~=4.4.28", + "zxcvbn~=4.4.28", "markdownify~=0.11.2", # integration dependencies From a33e34519a5f0f4cde6022f2220d719b544db209 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 Jan 2023 15:04:47 +0100 Subject: [PATCH 6/6] feat: add test case for long passwords --- frappe/tests/test_password_strength.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 frappe/tests/test_password_strength.py diff --git a/frappe/tests/test_password_strength.py b/frappe/tests/test_password_strength.py new file mode 100644 index 0000000000..5dc87d185d --- /dev/null +++ b/frappe/tests/test_password_strength.py @@ -0,0 +1,18 @@ +import random +from string import printable +from time import time +from unittest import TestCase + +from frappe.utils.password_strength import test_password_strength + + +class TestPasswordStrength(TestCase): + def test_long_password(self): + password = "".join(random.choice(printable) for _ in range(600)) + + start_second = time() + result = test_password_strength(password) + end_second = time() + + self.assertLess(end_second - start_second, 10) + self.assertIn("feedback", result)