-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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.
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
feat(isStrongPassword): add @
as valid symbol
#1566
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1566 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 100 100
Lines 1796 1796
=========================================
Hits 1796 1796
Continue to review full report at Codecov.
|
@
as valid symbol
Looking forward to seeing this merged in. Spent an hour debugging my password validation to realize the @ symbol wasn't part of the regex lmao |
Same, is there any update on this? It's a dead-simple PR, ready to be merged. |
Probably gonna have to ping an owner or something. |
@@ -4,7 +4,7 @@ import assertString from './util/assertString'; | |||
const upperCaseRegex = /^[A-Z]$/; | |||
const lowerCaseRegex = /^[a-z]$/; | |||
const numberRegex = /^[0-9]$/; | |||
const symbolRegex = /^[-#!$%^&*()_+|~=`{}\[\]:";'<>?,.\/ ]$/; | |||
const symbolRegex = /^[-#!$@%^&*()_+|~=`{}\[\]:";'<>?,.\/ ]$/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do /^[\W_ ]$/
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to hardcode specific characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's the case then we should include in the documentation that this function is restricted to ASCII characters only and symbols such as º
or §
will not work, which will be quite a surprise for some locales that have these symbols in their keyboards.
It would work with /^[\W_ ]$/
though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's true. I don't know if people use these "weird" characters in their password? Personally I've never seen someone do that but I dunno. Maybe it is better to do that other regex
@profnandaa @ezkemboi @tux-tn any update? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
Thank you for your contribution @stingalleman
adds @ as a valid symbol, also fixes #1563
Checklist