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

Avoid simple passwords (e.g. email as password) #1439

Merged
merged 6 commits into from
Mar 19, 2024

Conversation

pscheit
Copy link
Contributor

@pscheit pscheit commented Mar 15, 2024

PR changes that you can't use your email address or your username as a password anymore.

We are aware that it's hard in general to get people to use really good passwords, but at least this crosses out the really dumb mistakes.

@@ -99,7 +99,7 @@ public function reset(Request $request, UserPasswordHasherInterface $passwordHas
}

// The token is valid; allow the user to change their password.
$form = $this->createForm(ResetPasswordFormType::class, null, ['user' => $user]);
$form = $this->createForm(ResetPasswordFormType::class, $user);
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 was a bit torn here, to keep the option. But in the end the ResetPasswordForm only has two non-mapped properties and so this allowed to reuse the Validator as is and not just add another case

@@ -55,6 +56,7 @@ public function configureOptions(OptionsResolver $resolver): void
{
$resolver->setDefaults([
'data_class' => User::class,
'constraints' => [new NotProhibitedPassword()],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alternative would be to add this as an attribute to the UserClass, but I liked that it's now on the same file where we add the Password constraint (cause can't add it to the Password Compound class)

Comment on lines +65 to +68
#[TestWith(['max.example'])]
#[TestWith(['[email protected]'])]
#[TestWith(['[email protected]'])]
public function testRegisterWithTooSimplePasswords(string $password): void
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 died writing a unit test for the validator. So just blew up the cases in the functional test

@Seldaek Seldaek merged commit 033b430 into composer:main Mar 19, 2024
3 checks passed
@Seldaek
Copy link
Member

Seldaek commented Mar 19, 2024

Thanks

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.

2 participants