-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Auto Password Encoder #29066
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
Comments
In theory this is possible, i've moved this to the form type as well to be less boring: https://github.com/msgphp/msgphp/blob/master/src/User/Infra/Form/Type/HashedPasswordType.php#L77 For core the approach comes down to using
and a model transformer i.e. |
@ro0NL this why I ask for Symfony members, to add this =] |
But you can create the type yourself also, for core this might needs more thought. Do we patch the existing PasswordType or create a new variant? There's the salt value we might need to account for. To confirm the current password, we still need the plain value (i had some struggle with this, and ended up with a all-in-one solution); a compound form. |
Slightly different version (for anyone interested). use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\CallbackTransformer;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Extension\Core\Type\PasswordType;
use Symfony\Component\Form\Extension\Core\Type\RepeatedType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use function array_merge;
use function gettype;
use function is_string;
use function sodium_memzero;
final class HashedPasswordType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options): void
{
$passwordOptions = $options['password_options'] + ['required' => $options['required']];
$passwordOptions['attr'] = array_merge(
$passwordOptions['attr'] ?? [],
[
'autocomplete' => 'off',
'autocorrect' => 'off',
'autocapitalize' => 'off',
'spellcheck' => 'false',
]
);
if ($options['password_confirm']) {
$builder->add(
'password',
RepeatedType::class,
[
'type' => PasswordType::class,
'invalid_message' => 'password_not_the_same',
'first_options' => ['label' => 'label.password'],
'second_options' => ['label' => 'label.password2'],
] + $passwordOptions
);
} else {
$builder->add('password', PasswordType::class, $passwordOptions);
}
$encoder = $options['algorithm'];
$builder->get('password')->addModelTransformer(
new CallbackTransformer(
// Password is always null (as by convention)
function () {
return null;
},
function ($value) use ($encoder): ?string {
if ($value === null) {
return null;
}
if (! is_string($value)) {
throw new TransformationFailedException('Expected string got "' . gettype($value) . '"');
}
$encodePassword = $encoder($value);
sodium_memzero($value); // Recommended but not required (requires Libsodium v2).
return $encodePassword;
}
)
);
}
public function configureOptions(OptionsResolver $resolver): void
{
$resolver->setRequired(['algorithm']);
$resolver->setDefaults([
'inherit_data' => true,
'password_options' => [],
'password_confirm' => false,
]);
$resolver->setAllowedTypes('algorithm', 'callable');
$resolver->setAllowedTypes('password_options', ['array']);
$resolver->setAllowedTypes('password_confirm', ['bool']);
}
} You can extend this type for a UserEncodedPassword type (like @ro0NL demonstrates) this version is more for password-hashing in general. A password should enter the system be "hashed" and then removed from memory (the original) leaving only the hashed version, as keeping the original value in memory might leak it core dumps and such. But this is more advanced and considered a good practice, not something you must absolutely do 👍 hashing is mandatory, zeroing from memory an addition. |
Thank you for this suggestion. |
Friendly ping? Should this still be open? I will close if I don't hear anything. |
I created a pull request for this: #42807 |
…e` (Seb33300) This PR was merged into the 6.2 branch. Discussion ---------- [Form] Add `hash_property_path` option to `PasswordType` | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #29066 | License | MIT | Doc PR | symfony/symfony-docs#15872 Same as #42883 but using a Form Extension and rebased to 6.1 & tests. This PR adds a new `hash_mapping` option to `PasswordType`. The `hash_mapping` option can be set with a property path where we want to set the hashed password. The `hash_mapping` option can only be used on unmapped fields to minimize plain password leak. Commits ------- 7065dfe [Form] Add hash_mapping option to PasswordType
For password encoder we still use:
https://symfony.com/doc/current/doctrine/registration_form.html#handling-the-form-submission
why not making it auto?
by auto he will get the form password, and will encrypt it.
with using this parameters:
so he can know how the password will be encoded.
Thanks!
The text was updated successfully, but these errors were encountered: