Skip to content

Conversation

@MoonISO
Copy link

@MoonISO MoonISO commented Jul 28, 2020

Password policy validation fail for new users #22014

  1. Create a new GenerateSecurePasswordEvent
  2. Dispatch the event
  3. If the passwordEvent is null use fallback

Other option for fallback:
/apps/provisioning_api/lib/Controller/UsersController.php (around line324)

...
$passwordEvent = new GenerateSecurePasswordEvent();
$this->eventDispatcher->dispatchTyped($passwordEvent);
$password .= $passwordEvent->getPassword() ?? $this->secureRandom->generate(21);
...

…tch the event; if even is null use fallback

Signed-off-by: Murielle Lerch <[email protected]>
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

$passwordEvent->getPassword();
if(isset($passwordEvent)){
$password .= $passwordEvent->getPassword();
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($password === null) {
  // fallback logic for password
}

If $password is null the password_policy (or any other listener to GenerateSecurePasswordEvent) is not enabled hence we can use a simpler fallback but using a strong password by default is probably not wrong.

@MoonISO
Copy link
Author

MoonISO commented Jul 28, 2020

I added your suggested modifications.

Unfortunately, I am a newbie and tried to fix the "php-cs check" failure.
However, this became a terrible commit where the entire file got modified due to tabs and spaces, thus I reverted it.
Thank you very much for your help.

@MoonISO MoonISO force-pushed the new_branch branch 2 times, most recently from 310f1d2 to 94e54e5 Compare July 28, 2020 12:01
…word is null for fallback

Signed-off-by: Murielle Lerch <[email protected]>
@kesselb
Copy link
Contributor

kesselb commented Aug 9, 2020

Hi @MoonISO,

thanks again for your pull request 👍 I'm not able to push changes to your branch so I redid the changes and updated the tests: #22145 #22159.

@kesselb kesselb closed this Aug 9, 2020
@MoonISO MoonISO deleted the new_branch branch August 10, 2020 06:10
@MorrisJobke
Copy link
Member

thanks again for your pull request 👍 I'm not able to push changes to your branch so I redid the changes and updated the tests: #22145.

I guess it should be #22159

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.

3 participants