'xcvbn_dictionary' test fails on any pattern match, not just by username or password

[GregPeden]

Referencing this block of code in the 'xcvbn_dictionary' test: https://github.com/olssonm/laravel-zxcvbn/blob/7032f908204d7e524a251e7e5f324c479b81acbe/src/ZxcvbnServiceProvider.php#L53-L58

This will fail on ANY sequence match. So suppose the password includes pattern similar to one of the dictionary words, this property being checked for will be present. But it is not necessarily with regards to the username or email address. It could just be any other. This test should not fail in such a condition, but it will.

If you do want it to fail on any dictionary match, then the readme description needs to be updated.

This is a bit more interesting. zxcvbn_dictionary allows you to input both the users username and/or email, and their password. The validator checks that the password doesn't exist in the username, or that they are too similar.

I think it would make sense to extend "zxcvbn_min" to optionally accept the username and password as additional arguments, since that's where the target score can be set as well, and that's really what most users of this package are going to care about. I am going to go ahead and extend that to submit as a PR but let me know if you would not be willing to accept this.

[olssonm]

Yes, you seem to be 100% correct. This seems to come from that this package previously ran on a fork of https://github.com/bjeavons/zxcvbn-php, which hade additional methods which are not currently available.

We could either:

1. Fix the base issue, look for ways to only check against the user-input dictionary matches
2. Or just deprecate the xcvbn_dictionary-test and as you have suggested, work in the user input with the main password score in zxcvbn_min – and if xcvbn_dictionary is removed, a single zxcvbn-method can be available.

PRs are welcome! Will dig into this soon as well and refresh my memory (haven't done much work to this package the past 6 years... 😕 )

[GregPeden]

No worries. I wrote a fix which I think works, I'm just running on my project for a bit to make sure it's good before I submit it for a PR.

I decided to just extend the "min" validator to optionally accept username and email and leave the dictionary one as it is, then change the documentation to clarify the function of the dictionary validator (which I don't think is especially useful but that's okay).

If you want to remove the dictionary validator you can but that'd be a breaking change. I tried to make the modification non-breaking so that package updates are safe.

Also I cannot get the tests to run for some reasons on my system, probably my own problem but I might ask you to run the new tests and make sure they pass.

[olssonm]

@GregPeden Finally got around to look into this and fix the issue. My guess is that it isn't relevant anymore, but here it is 😄 https://github.com/olssonm/laravel-zxcvbn/releases/tag/v5.0