Skip to content

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

Closed
ben29 opened this issue Nov 2, 2018 · 8 comments · Fixed by #46224
Closed

Auto Password Encoder #29066

ben29 opened this issue Nov 2, 2018 · 8 comments · Fixed by #46224

Comments

@ben29
Copy link
Contributor

ben29 commented Nov 2, 2018

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:

security:
  encoders:
    App\Entity\User:
      algorithm: bcrypt
      cost: 12

so he can know how the password will be encoded.

Thanks!

@ro0NL
Copy link
Contributor

ro0NL commented Nov 2, 2018

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. $form = $factory->create(PasswordType::class, null, ['hash_for' => MyUser::class])

@ben29
Copy link
Contributor Author

ben29 commented Nov 3, 2018

@ro0NL this why I ask for Symfony members, to add this =]

@ro0NL
Copy link
Contributor

ro0NL commented Nov 3, 2018

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.

@sstok
Copy link
Contributor

sstok commented Nov 4, 2018

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.

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

Friendly ping? Should this still be open? I will close if I don't hear anything.

@Seb33300
Copy link
Contributor

Seb33300 commented Sep 1, 2021

I created a pull request for this: #42807

fabpot added a commit that referenced this issue Oct 22, 2022
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment