Strengthen password policy#868
Conversation
|
Note: as per this slack conversation, this PR leaves the demo site password hard coded in three places:
Removing password login from demo mode is one way to address this, but is out of scope for this PR. It also deserves some thought given demo site is publicly deployed 🤔 |
plouka13
left a comment
plouka13
left a comment
There was a problem hiding this comment.
This is a much needed PR @ignatiusm!
Overall looks great. A few things I think need actioning:
- studyId: There's a few places that could be updated to use the
TestStudies.*object to replace hardcoded values. (I'm aware that some of this might not be necessary because it's creating new studies) - Would be good to pull out the common base words into a list that we can easily edit in the future, or, could we use an api here to grab a list of common base words in passwords to check against?
Otherwise looks great! Very happy to have cleaned up the test data importing!
| if (password.length < 14) { | ||
| fields.Length = { message: 'Password must be at least 14 characters' } | ||
| } | ||
| if (/password|welcome|changeme/i.test(password)) { |
There was a problem hiding this comment.
Would be good to have a const list of base words to allow easier adding in the future. Also should we make this list more exhaustive? However I'm mindful that adding more words to this list could be a hinderance to users.
There was a problem hiding this comment.
Good spotting. I've created the constant and applied it in the appropriate places in abf8ee2
I haven't added to the list though. Did you have other words in mind to add?
There was a problem hiding this comment.
Top of my mind I think:
["abc123", "qwerty"]
Otherwise happy to leave as is for now?
There was a problem hiding this comment.
Might move that to another issue, I found a reference list of common passwords: https://en.wikipedia.org/wiki/List_of_the_most_common_passwords
2 participants
Resolves #867
Updated checkPasswordStrength function to:
This causes backend auth tests to fail (therefore verifying those changes are good):
