Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Password similarity policy is too strict #581

Closed
aeneasr opened this issue Jul 16, 2020 · 2 comments · Fixed by #585
Closed

Password similarity policy is too strict #581

aeneasr opened this issue Jul 16, 2020 · 2 comments · Fixed by #585
Assignees
@zepatrik
Labels
feat New feature or request.
Projects
Maintainer's Board
Milestone
v0.5.0-alpha.1

Comments

@aeneasr
Copy link
Member

aeneasr commented Jul 16, 2020

Describe the bug

In #577 we observe flaky tests because both the user id and the password are generated using UUID v4 and the password policy deemed them too similar.

Given that the entropy is really high, the password policy definitely returned a false positive here and should subsequently be improved (also with tests) to ensure that the false positive rate is reduced.

Reproducing the bug

Check the password policy against a combination of UUIDs
@aeneasr aeneasr added the feat New feature or request. label Jul 16, 2020
@aeneasr aeneasr added this to the v0.5.0-alpha.1 milestone Jul 16, 2020
@aeneasr aeneasr added this to To do in Maintainer's Board via automation Jul 16, 2020
aeneasr added a commit that referenced this issue Jul 16, 2020
@aeneasr
fix: resolve flaky passwort setting tests
a06b3b0 
Resolves an issue where the password was too similar to the user ID as both were generated using UUIDs (see #581). Closes #577
@aeneasr aeneasr mentioned this issue Jul 16, 2020
Merged
aeneasr added a commit that referenced this issue Jul 16, 2020
@aeneasr
fix: resolve flaky passwort setting tests (#582)
c42d936 
Resolves an issue where the password was too similar to the user ID as both were generated using UUIDs (see #581). Closes #577
@zepatrik
Copy link
Member

zepatrik commented Jul 17, 2020 •
edited

Some examples of uuids being too similar:

a b
d4f6090b-5a84-4d4d-af5e-edfb9bb4ee8b 31f7cb9f-2184-4404-8d1b-8da3eb00ebbe
6b0e8fc2-6123-430f-8f10-5796f645b1aa 81cbaa10-5a47-4499-88ec-9fb920d71bc1
a3d7df25-0**5fb-**4808-bac3-69c57ec26b25 b8e8f28c-3ca0-4**5fb-**b653-16cf0846bbee

After debugging those a bit I found out the longest common substring check is causing these to fail, not the levenshtein distance.

@zepatrik
Copy link
Member

I will fix this by requiring the lcs to be propotionally less than 20% of the password length. Or are there any arguments for/against a specific threshold?

@zepatrik zepatrik mentioned this issue Jul 17, 2020
Merged
Maintainer's Board automation moved this from To do to Done Jul 17, 2020
aeneasr pushed a commit that referenced this issue Jul 17, 2020
@zepatrik
fix: use relative threshold to judge longest common substring in pass…
3e9f8cc 
…word policy (#585)

Closes #581
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zepatrik zepatrik

Labels
feat New feature or request.
Projects
Maintainer's Board
  
Done
Milestone
v0.5.0-alpha.1
Development

Successfully merging a pull request may close this issue.

fix: password validator uses relative threshold to judge longest common substring ory/kratos
2 participants
@aeneasr @zepatrik