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

[Internet::Password] Improve mix_case and special_characters support #2308

Merged
merged 1 commit into from Aug 8, 2022

Conversation

meuble
Copy link
Contributor

@meuble meuble commented Apr 26, 2021

Ensure password gets at least 1 letter when mix_case and special_characters are both required and doing so even on really small length.

Issue

When activating both mix_case and special_characters flags, it was possible to result in a password without any letter.

Example :

irb(main):044:0> Faker::Internet.password(min_length: 4, max_length: 6, mix_case: true, special_characters: true)
=> "&&%$9"
irb(main):045:0> Faker::Internet.password(min_length: 48, max_length: 48, mix_case: true, special_characters: true)
=> "^##&!@&%$%**@!\#@#\#@#^!@@&$!!^*!$*&^%!&!#%&*!#%&!"

This is due to the special characters blocs replacing between 1 and min_length characters, which might be bigger than the number of letters in the string (which is a minimum of 2 when mix_case is enabled). With bad luck, all letters could end up being all replaced.

This issue can raise errors when testing the uniqueness of passwords with case sensitivity.

…case and special chars are both required, even on really small lentgh
@meuble meuble changed the title [Internet::Pasword] Improve mix_case and special_characters support [Internet::Password] Improve mix_case and special_characters support Apr 27, 2021
@farhatahmad
Copy link

I ran into this issue today and I'm surprised this isn't a bigger issue. I would expect mix_case to ensure the presence of both a lower case and an uppercase character when generating a password, but that's definitely not the case.

This seems like an improvement over the current logic

@meuble
Copy link
Contributor Author

meuble commented Dec 7, 2021

I forgot about this PR since then, thanks @farhatahmad for the reminder ^_^

Can anyone have a look ? @vbrazo maybe ?

@meuble
Copy link
Contributor Author

meuble commented Mar 7, 2022

Up ?

@antonioeloi
Copy link

This should be added to master as soon as possible as it's a good improvement over the internet:password generation. Thank you @meuble.

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

Successfully merging this pull request may close these issues.

None yet

5 participants