fix: Password strength check for long passwords #19756

barredterra · 2023-01-24T12:58:28Z

I have a long API Key (>600) characters that fails to save because of the password check.

The zxcvbn version we are using at the moment produces an OverflowError. That was fixed in dwolfhub/zxcvbn-python#35 . -> Bumped from 4.4.24 to 4.4.28. zxcvbn-python is not maintained anymore. It has moved to zxcvbn.

However, checking a password this long still took way too much time. I canceled the check after running for half a minute. -> For long passwords, only check the first 128 characters. This way we can still detect user inputs and repeating patterns in the first part, but it's reasonably quick.

To reproduce the problem, you can try this:

>>> from zxcvbn import zxcvbn
>>> import random
>>> import string
>>> password = ''.join(random.choice(string.printable) for i in range(600))
>>> zxcvbn(password)

With zxcvbn==4.4.24 you should get an OverflowError. With zxcvbn==4.4.28 you should have to wait for a long time.

zxcvbn 4.4.28 no longer crashes on long, random passwords.

In order for the check to pass in a reasonable amount of time.

- remove unused imports - import only the required patterns, not the entire file

codecov · 2023-01-24T14:22:42Z

Codecov Report

Merging #19756 (ad77a74) into develop (72ae670) will decrease coverage by 0.05%.
The diff coverage is 77.77%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #19756      +/-   ##
===========================================
- Coverage    62.82%   62.78%   -0.05%     
===========================================
  Files          754      754              
  Lines        71285    71332      +47     
  Branches      6134     6134              
===========================================
  Hits         44787    44787              
- Misses       23022    23069      +47     
  Partials      3476     3476

Flag	Coverage Δ
server	`68.72% <100.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

barredterra · 2023-01-24T18:28:07Z

Failure of cypress test control_color.js seems to be unrelated.
Failure of Vulnerable Dependency Check is caused by the dropbox sdk, which is unrelated (Fix invalid specifier in stone requirement dropbox/dropbox-sdk-python#456).

…9765) * feat: bump zxcvbn version zxcvbn 4.4.28 no longer crashes on long, random passwords. (cherry picked from commit 8aa8ea0) * fix: trim long passwords before check In order for the check to pass in a reasonable amount of time. (cherry picked from commit d5a72b1) * refactor: imports - remove unused imports - import only the required patterns, not the entire file (cherry picked from commit e61d878) * refactor: assign instead of update (cherry picked from commit 2148dc7) * fix: use new source for zxcvbn (cherry picked from commit 92e684d) * feat: add test case for long passwords (cherry picked from commit a33e345) Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com>

# [14.25.0](v14.24.0...v14.25.0) (2023-01-30) ### Bug Fixes * add freeze message for bulk delete ([2a42036](2a42036)) * assertAlmostEqual with precision ([#19794](#19794)) ([9f7c4e0](9f7c4e0)) * Convert doctype name to string ([#19832](#19832)) ([#19834](#19834)) ([a45f31d](a45f31d)) * correct exit code on missing app failure ([#19676](#19676)) ([#19770](#19770)) ([f6139a4](f6139a4)) * **i18n:** Datepicker Turkish translations ([#19777](#19777)) ([#19831](#19831)) ([3e91fb1](3e91fb1)) * incorrect link when std field has problem (backport [#19744](#19744)) ([#19763](#19763)) ([4593bb9](4593bb9)) * **MariaDBTable:** dont attempt to drop index twice ([#19783](#19783)) ([67f80c6](67f80c6)) * Password strength check for long passwords (backport [#19756](#19756)) ([#19765](#19765)) ([a6315f9](a6315f9)) * respect disable sidebar stats on list view ([#19795](#19795)) ([5f57816](5f57816)) * sanitize traceback for common secrets ([#19805](#19805)) ([#19806](#19806)) ([ae6f2b1](ae6f2b1)) * use count instead of concatenated docnames ([06948d1](06948d1)) ### Features * Audit hooks report (backport [#19780](#19780)) ([#19828](#19828)) ([99bdf34](99bdf34)) * better freeze message ([c03f9e7](c03f9e7))

…9843) * feat: bump zxcvbn version zxcvbn 4.4.28 no longer crashes on long, random passwords. (cherry picked from commit 8aa8ea0) # Conflicts: # pyproject.toml * fix: trim long passwords before check In order for the check to pass in a reasonable amount of time. (cherry picked from commit d5a72b1) * refactor: imports - remove unused imports - import only the required patterns, not the entire file (cherry picked from commit e61d878) # Conflicts: # frappe/utils/password_strength.py * refactor: assign instead of update (cherry picked from commit 2148dc7) * fix: use new source for zxcvbn (cherry picked from commit 92e684d) * feat: add test case for long passwords (cherry picked from commit a33e345) * chore: resolve conflicts --------- Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com>

… (frappe#19765) * feat: bump zxcvbn version zxcvbn 4.4.28 no longer crashes on long, random passwords. (cherry picked from commit 8aa8ea0) * fix: trim long passwords before check In order for the check to pass in a reasonable amount of time. (cherry picked from commit d5a72b1) * refactor: imports - remove unused imports - import only the required patterns, not the entire file (cherry picked from commit e61d878) * refactor: assign instead of update (cherry picked from commit 2148dc7) * fix: use new source for zxcvbn (cherry picked from commit 92e684d) * feat: add test case for long passwords (cherry picked from commit a33e345) Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com>

# [14.25.0](frappe/frappe@v14.24.0...v14.25.0) (2023-01-30) ### Bug Fixes * add freeze message for bulk delete ([2a42036](frappe@2a42036)) * assertAlmostEqual with precision ([frappe#19794](frappe#19794)) ([9f7c4e0](frappe@9f7c4e0)) * Convert doctype name to string ([frappe#19832](frappe#19832)) ([frappe#19834](frappe#19834)) ([a45f31d](frappe@a45f31d)) * correct exit code on missing app failure ([frappe#19676](frappe#19676)) ([frappe#19770](frappe#19770)) ([f6139a4](frappe@f6139a4)) * **i18n:** Datepicker Turkish translations ([frappe#19777](frappe#19777)) ([frappe#19831](frappe#19831)) ([3e91fb1](frappe@3e91fb1)) * incorrect link when std field has problem (backport [frappe#19744](frappe#19744)) ([frappe#19763](frappe#19763)) ([4593bb9](frappe@4593bb9)) * **MariaDBTable:** dont attempt to drop index twice ([frappe#19783](frappe#19783)) ([67f80c6](frappe@67f80c6)) * Password strength check for long passwords (backport [frappe#19756](frappe#19756)) ([frappe#19765](frappe#19765)) ([a6315f9](frappe@a6315f9)) * respect disable sidebar stats on list view ([frappe#19795](frappe#19795)) ([5f57816](frappe@5f57816)) * sanitize traceback for common secrets ([frappe#19805](frappe#19805)) ([frappe#19806](frappe#19806)) ([ae6f2b1](frappe@ae6f2b1)) * use count instead of concatenated docnames ([06948d1](frappe@06948d1)) ### Features * Audit hooks report (backport [frappe#19780](frappe#19780)) ([frappe#19828](frappe#19828)) ([99bdf34](frappe@99bdf34)) * better freeze message ([c03f9e7](frappe@c03f9e7))

## [13.49.2](v13.49.1...v13.49.2) (2023-02-13) ### Bug Fixes * 2fa creation token on ldap (backport [#19753](#19753)) ([#19875](#19875)) ([44257fd](44257fd)) * Apply permissions on Report sidebar ([#20001](#20001)) ([7a7b8fa](7a7b8fa)) * build_response for re.Match ([#19899](#19899)) ([3b9712f](3b9712f)) * can't sign out due to missing roles ([#19905](#19905)) ([#19906](#19906)) ([c6e426b](c6e426b)) * Consider parenttype when renaming ([#19901](#19901)) ([#19902](#19902)) ([3380867](3380867)) * Dont setup socketio events on new doc ([#19864](#19864)) ([#19865](#19865)) ([474df51](474df51)) * event date exception ([#19955](#19955)) ([#20008](#20008)) ([b332cc6](b332cc6)) * ignore permission for marking Note as seen ([#19939](#19939)) ([#20004](#20004)) ([a6def66](a6def66)) * login before check should be inclusive ([#19974](#19974)) ([#19975](#19975)) ([2947a45](2947a45)) * Many migration fixes (copy [#19912](#19912)) ([#19922](#19922)) ([951b71b](951b71b)) * Migrate color fields to color doctype ([#20011](#20011)) ([#20012](#20012)) ([e11de2c](e11de2c)) * misc migration related fixes ([#19874](#19874)) ([#19880](#19880)) ([69eb757](69eb757)) * Password strength check for long passwords (backport [#19756](#19756)) ([#19843](#19843)) ([287d398](287d398)) * PermissionError ([#19856](#19856)) ([#19857](#19857)) ([0ed218d](0ed218d)) * sanitize form dict in error logs ([#19835](#19835)) ([9d91bf9](9d91bf9)) * String formatting index in error message ([#19977](#19977)) ([#19978](#19978)) ([b05c156](b05c156)) * strip whitespace when setting recipients in email queue recipients ([#19915](#19915)) ([ac2ec3d](ac2ec3d)) * translations update in MutliSelectDialog ([88c77ed](88c77ed)) * website theme caching ([#19848](#19848)) ([#19849](#19849)) ([af7747e](af7747e)) ### Reverts * Revert "fix: handle additional width from scrollbar" ([779a7c1](779a7c1))

barredterra added 4 commits January 24, 2023 13:46

feat: bump zxcvbn version

8aa8ea0

zxcvbn 4.4.28 no longer crashes on long, random passwords.

fix: trim long passwords before check

d5a72b1

In order for the check to pass in a reasonable amount of time.

refactor: imports

e61d878

- remove unused imports - import only the required patterns, not the entire file

refactor: assign instead of update

2148dc7

barredterra requested a review from a team as a code owner January 24, 2023 12:58

barredterra requested review from phot0n and removed request for a team January 24, 2023 12:58

github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jan 24, 2023

barredterra added 2 commits January 24, 2023 14:07

fix: use new source for zxcvbn

92e684d

feat: add test case for long passwords

a33e345

barredterra removed the add-test-cases Add test case to validate fix or enhancement label Jan 24, 2023

Merge branch 'develop' into password-strength-for-long-passwords

ad77a74

ankush approved these changes Jan 25, 2023

View reviewed changes

ankush merged commit ca0cb93 into frappe:develop Jan 25, 2023

ankush added the backport version-14-hotfix backport to version 14 label Jan 25, 2023

mergify bot mentioned this pull request Jan 25, 2023

fix: Password strength check for long passwords (backport #19756) #19765

Merged

barredterra deleted the password-strength-for-long-passwords branch January 25, 2023 08:38

barredterra added the backport version-13-hotfix label Jan 30, 2023

mergify bot mentioned this pull request Jan 30, 2023

fix: Password strength check for long passwords (backport #19756) #19843

Merged

github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Password strength check for long passwords #19756

fix: Password strength check for long passwords #19756

barredterra commented Jan 24, 2023 •

edited

codecov bot commented Jan 24, 2023 •

edited

barredterra commented Jan 24, 2023 •

edited

fix: Password strength check for long passwords #19756

fix: Password strength check for long passwords #19756

Conversation

barredterra commented Jan 24, 2023 • edited

codecov bot commented Jan 24, 2023 • edited

Codecov Report

barredterra commented Jan 24, 2023 • edited

barredterra commented Jan 24, 2023 •

edited

codecov bot commented Jan 24, 2023 •

edited

barredterra commented Jan 24, 2023 •

edited