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.

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

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

stingalleman
Copy link
Contributor

adds @ as a valid symbol, also fixes #1563

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov
Copy link

codecov bot commented Jan 1, 2021

Codecov Report

Merging #1566 (adab6e4) into master (8831db3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1566   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          100       100           
  Lines         1796      1796           
=========================================
  Hits          1796      1796           
Impacted Files Coverage Δ
src/lib/isStrongPassword.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8831db3...adab6e4. Read the comment docs.

@stingalleman stingalleman changed the title feat(isGoodPassword): add @ as valid symbol feat(isStrongPassword): add @ as valid symbol Jan 1, 2021
@thedevelinmycode
Copy link

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

@stingalleman
Copy link
Contributor Author

Same, is there any update on this? It's a dead-simple PR, ready to be merged.

@thedevelinmycode
Copy link

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 = /^[-#!$@%^&*()_+|~=`{}\[\]:";'<>?,.\/ ]$/;
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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

@stingalleman
Copy link
Contributor Author

@profnandaa @ezkemboi @tux-tn any update?

Copy link
Member

@tux-tn tux-tn left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@ is not considered as A Special Character in "isStrongPassword" function
6 participants