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. <jens@beryju.org>
Signed-off-by: sdimovv <36302090+sdimovv@users.noreply.github.com>

* Added test case

* fix black formatting

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>

Signed-off-by: sdimovv <36302090+sdimovv@users.noreply.github.com>
Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
Co-authored-by: Jens L. <jens@beryju.org>
Co-authored-by: Jens Langhammer <jens.langhammer@beryju.org>
This commit is contained in:
sdimovv 2022-11-30 07:58:16 +00:00 committed by GitHub
parent 1690812936
commit 5156aeee0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 1 deletions

View File

@ -150,6 +150,8 @@ class PasswordPolicy(Policy):
results = zxcvbn(password[:100], user_inputs) results = zxcvbn(password[:100], user_inputs)
LOGGER.debug("password failed", check="zxcvbn", score=results["score"]) LOGGER.debug("password failed", check="zxcvbn", score=results["score"])
result = PolicyResult(results["score"] > self.zxcvbn_score_threshold) 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): if isinstance(results["feedback"]["warning"], list):
result.messages += tuple(results["feedback"]["warning"]) result.messages += tuple(results["feedback"]["warning"])
if isinstance(results["feedback"]["suggestions"], list): if isinstance(results["feedback"]["suggestions"], list):

View File

@ -28,13 +28,21 @@ class TestPasswordPolicyZxcvbn(TestCase):
policy = PasswordPolicy.objects.create( policy = PasswordPolicy.objects.create(
check_zxcvbn=True, check_zxcvbn=True,
check_static_rules=False, check_static_rules=False,
zxcvbn_score_threshold=3,
name="test_false", name="test_false",
) )
request = PolicyRequest(get_anonymous_user()) request = PolicyRequest(get_anonymous_user())
request.context[PLAN_CONTEXT_PROMPT] = {"password": "password"} # nosec request.context[PLAN_CONTEXT_PROMPT] = {"password": "password"} # nosec
result: PolicyResult = policy.passes(request) result: PolicyResult = policy.passes(request)
self.assertFalse(result.passing, result.messages) 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): def test_true(self):
"""Positive password case""" """Positive password case"""