V3 - allow all symbols by default, set the default min length to 12 instead of 10, fix license filename (fix #78) #81
Conversation
…case, uppercase letters and 10 digits)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Jshint (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
dist/index.mjs
Outdated
| regex: '[0-9]', | ||
| message: 'number' | ||
| key: "symbol", | ||
| regex: restrictSymbolsTo ? `[${restrictSymbolsTo}]` : "[^a-zA-Z0-9]", |
There was a problem hiding this comment.
it's not documented that restrictSymbolsTo needs to be a valid string for inside a character class in a regular expression. i'd recommend using escape-string-regexp here (then cleaning up owaspSymbols to get rid of the extra \s)
There was a problem hiding this comment.
for example, passing $][& for restrictSymbolsTo would require that the password match [$][&], so the strings x$x, x&x, and x&$x would not match, but x$&x would.
There was a problem hiding this comment.
additionally, ,-. in the owaspSymbols string will technically get interpreted as "any character from , to ." because of the -. in reality, the only character between , and . in the ascii table is -, so it's fine, but this feels like a potential bug if the symbols in that string ever get rearranged. (but escaping the string like i laid out above would solve this problem)
There was a problem hiding this comment.
You are right. I added the library which is extremely tiny. I added the necessary information in the doc.
There was a problem hiding this comment.
Hi @Ennoriel,
Amazing changes! 🥳 🎆 I also tested it on my local and everything seems fine
🏗️ updated code analysis from v1 to v4, 🦯 added node.yml, ❌ remove .travis.yml
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
2 similar comments
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
@deanilvincent could you please review this PR that allows all symbols by default (fix #78)? I also increased the default minlength since it's quite common to see a minlength of 12 nowadays.
I did not increase the version in the readme. Since it is a breaking change, it should go to version 3.0.0.
I also cleaned and simplified the README.