From 5156aeee0fb268b59bc571e1d4ea3d714f8c0e2a Mon Sep 17 00:00:00 2001 From: sdimovv <36302090+sdimovv@users.noreply.github.com> Date: Wed, 30 Nov 2022 07:58:16 +0000 Subject: [PATCH] policies/password: Always add generic message to failing zxcvbn check (#4100) * Always add generic message to failing zxcvbn password policy Depending on the settings, sometimes a password policy that checks a password with the zxcvbn tool can fail without any message. For example: ``` $ echo 'Awdccdw1234' | zxcvbn | jq | grep "feedback" -A 5 -B 1 Password: "score": 3, "feedback": { "warning": "", "suggestions": [] } } ``` As seen above the tool does not produce any warnings or suggestions for the given password, but if the password policy is set to have a zxcvbn threshold of 3, the policy will silently fail without communicating the reason to the user. There are two ways to handle this: 1. Always add a generic "password is too weak" message when the policy fails. 2. Check if there are any suggestions or warnings from the zxcvbn tool and only add the generic message if not. I personally prefer 1. This way the generic message will be shown whenever the policy fails, and will get combined with extra "tips" whenever zxcvbn has some. Signed-off-by: sdimovv <36302090+sdimovv@users.noreply.github.com> * Update authentik/policies/password/models.py Co-authored-by: Jens L. Signed-off-by: sdimovv <36302090+sdimovv@users.noreply.github.com> * Added test case * fix black formatting Signed-off-by: Jens Langhammer Signed-off-by: sdimovv <36302090+sdimovv@users.noreply.github.com> Signed-off-by: Jens Langhammer Co-authored-by: Jens L. Co-authored-by: Jens Langhammer --- authentik/policies/password/models.py | 2 ++ authentik/policies/password/tests/test_zxcvbn.py | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/authentik/policies/password/models.py b/authentik/policies/password/models.py index fed63464f..97697136f 100644 --- a/authentik/policies/password/models.py +++ b/authentik/policies/password/models.py @@ -150,6 +150,8 @@ class PasswordPolicy(Policy): results = zxcvbn(password[:100], user_inputs) LOGGER.debug("password failed", check="zxcvbn", score=results["score"]) result = PolicyResult(results["score"] > self.zxcvbn_score_threshold) + if not result.passing: + result.messages += tuple((_("Password is too weak."),)) if isinstance(results["feedback"]["warning"], list): result.messages += tuple(results["feedback"]["warning"]) if isinstance(results["feedback"]["suggestions"], list): diff --git a/authentik/policies/password/tests/test_zxcvbn.py b/authentik/policies/password/tests/test_zxcvbn.py index 7d03ba841..dc2f47c5c 100644 --- a/authentik/policies/password/tests/test_zxcvbn.py +++ b/authentik/policies/password/tests/test_zxcvbn.py @@ -28,13 +28,21 @@ class TestPasswordPolicyZxcvbn(TestCase): policy = PasswordPolicy.objects.create( check_zxcvbn=True, check_static_rules=False, + zxcvbn_score_threshold=3, name="test_false", ) request = PolicyRequest(get_anonymous_user()) request.context[PLAN_CONTEXT_PROMPT] = {"password": "password"} # nosec result: PolicyResult = policy.passes(request) self.assertFalse(result.passing, result.messages) - self.assertEqual(result.messages[0], "Add another word or two. Uncommon words are better.") + self.assertEqual(result.messages[0], "Password is too weak.") + self.assertEqual(result.messages[1], "Add another word or two. Uncommon words are better.") + + request.context[PLAN_CONTEXT_PROMPT] = {"password": "Awdccdw1234"} # nosec + result: PolicyResult = policy.passes(request) + self.assertFalse(result.passing, result.messages) + self.assertEqual(result.messages[0], "Password is too weak.") + self.assertEqual(len(result.messages), 1) def test_true(self): """Positive password case"""