Change generatePassword() logic #15894
Conversation
|
@Mark-H I would like your opinion on this change. |
| if ($options['alphabet']) { | ||
| $alphabet = array_merge(range('a', 'z'), range('A', 'Z')); | ||
| shuffle($alphabet); | ||
| return substr(implode($alphabet),0,$length); |
There was a problem hiding this comment.
| return substr(implode($alphabet),0,$length); | |
| return substr(implode($alphabet), 0, $length); |
| } | ||
|
|
||
| return $pass; | ||
| return substr(bin2hex(random_bytes($length)),$length); |
There was a problem hiding this comment.
| return substr(bin2hex(random_bytes($length)),$length); | |
| return substr(bin2hex(random_bytes($length)), $length); |
|
Other than the phpcs issues I don't have a strong opinion on this - the use of The |
|
@crystaldaking would it be possible for you to update the Unit Tests to match your implementation? |
| for ($i = 0; $i < $length; $i++) { | ||
| $pass .= $options['allowable_characters'][mt_rand(0, $ps_len - 1)]; | ||
|
|
||
| if ($options['alphabet']) { |
There was a problem hiding this comment.
This may also require a check as I don't see the alphabet key being required elsewhere. So something like if (!empty($options['alphabet'])) { to avoid php warnings.
|
@crystaldaking ping... |
|
As this PR has gone stale, I recreated it at #16521. |
What does it do?
Updated the password generation function using a truly random string
Why is it needed?
I am deeply convinced that the more a password looks like a hash, the less it attracts the attention of hackers. Moreover, this implementation uses a truly random string in contrast to the stock implementation
The
optionsparameter as part of password generation is a rather useless parameter. It is not safe to specify salt, any other parameters simply do not occur to me. As an option, I left the implementation of the classicalphabetpassword that can be invoked using optionsHow to test
Standard tests work, no logic is affected.
You can also test password generation in the user control panel
Related issue(s)/PR(s)
#15740