-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Security][DX] Added SecurityPasswordType #35654
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,5 +111,15 @@ | |
<argument type="service" id="translator"/> | ||
<argument type="string">%validator.translation_domain%</argument> | ||
</service> | ||
|
||
<!-- SecurityExtension --> | ||
<service id="form.extension.security" class="Symfony\Component\Form\Extension\Security\SecurityExtension"> | ||
<argument type="service" id="security.user_password_encoder" on-invalid="ignore"/> | ||
<tag name="form.type_extension" /> | ||
</service> | ||
<service id="form.extension.security.type.security_password" class="Symfony\Component\Form\Extension\Security\Type\SecurityPasswordType"> | ||
<argument type="service" id="security.user_password_encoder" on-invalid="ignore"/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class would be pointless without the required password encoder, I'd suggest make the argument mandatory and remove the whole service definition if "security" isn't enabled. See |
||
<tag name="form.type" alias="security_password" /> | ||
</service> | ||
</services> | ||
</container> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Form\Extension\Security; | ||
|
||
use Symfony\Component\Form\AbstractExtension; | ||
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; | ||
|
||
/** | ||
* Integrates the Security (Core) component with the Form library. | ||
* | ||
* @author Loïck Piera <pyrech@gmail.com> | ||
*/ | ||
class SecurityExtension extends AbstractExtension | ||
{ | ||
private $passwordEncoder; | ||
|
||
public function __construct(UserPasswordEncoderInterface $passwordEncoder = null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, remove the nullable definition. |
||
{ | ||
$this->passwordEncoder = $passwordEncoder; | ||
} | ||
|
||
protected function loadTypes() | ||
{ | ||
if (!$this->passwordEncoder) { | ||
return []; | ||
} | ||
|
||
return [ | ||
new Type\SecurityPasswordType($this->passwordEncoder), | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Form\Extension\Security\Type; | ||
|
||
use Symfony\Component\Form\AbstractType; | ||
use Symfony\Component\Form\Event\SubmitEvent; | ||
use Symfony\Component\Form\Exception\InvalidConfigurationException; | ||
use Symfony\Component\Form\Extension\Core\Type\PasswordType; | ||
use Symfony\Component\Form\FormBuilderInterface; | ||
use Symfony\Component\Form\FormEvents; | ||
use Symfony\Component\OptionsResolver\OptionsResolver; | ||
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; | ||
use Symfony\Component\Security\Core\User\UserInterface; | ||
|
||
/** | ||
* @author Loïck Piera <pyrech@gmail.com> | ||
*/ | ||
class SecurityPasswordType extends AbstractType | ||
{ | ||
private $passwordEncoder; | ||
|
||
public function __construct(UserPasswordEncoderInterface $passwordEncoder) | ||
{ | ||
$this->passwordEncoder = $passwordEncoder; | ||
} | ||
|
||
public function buildForm(FormBuilderInterface $builder, array $options) | ||
{ | ||
$builder->addEventListener(FormEvents::SUBMIT, function (SubmitEvent $event) { | ||
$securityUser = $event->getForm()->getConfig()->getOption('security_user'); | ||
|
||
if (!$securityUser) { | ||
$parentData = $event->getForm()->getParent()->getData(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the parent form cannot be null here, should we check? |
||
|
||
if (!$parentData instanceof UserInterface) { | ||
throw new InvalidConfigurationException(sprintf('You should either use "%s" inside a parent form where data is an instance of "%s" or specify the user in "security_user" option', self::class, UserInterface::class)); | ||
} | ||
|
||
$securityUser = $parentData; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the aux variable can be avoided here. i.e. |
||
} | ||
|
||
$plainPassword = $event->getData(); | ||
|
||
$event->setData($plainPassword ? $this->passwordEncoder->encodePassword($securityUser, $plainPassword) : $securityUser->getPassword()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The submitted data cannot be changed this way before validating the plain password. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In effect, if the validation step fails, the encoded password will be visible to the end user. |
||
}); | ||
} | ||
|
||
public function configureOptions(OptionsResolver $resolver) | ||
{ | ||
$resolver->setDefault('security_user', null); | ||
$resolver->setAllowedTypes('security_user', [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the statement should be in one line |
||
'null', | ||
UserInterface::class, | ||
]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we've recently added a fluent interface to define options, let's use it. |
||
} | ||
|
||
public function getParent() | ||
{ | ||
return PasswordType::class; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Form\Tests\Extension\Security; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Symfony\Component\Form\AbstractType; | ||
use Symfony\Component\Form\Extension\Core\Type\FormType; | ||
use Symfony\Component\Form\Extension\Security\SecurityExtension; | ||
use Symfony\Component\Form\Extension\Security\Type\SecurityPasswordType; | ||
use Symfony\Component\Form\FormBuilderInterface; | ||
use Symfony\Component\Form\FormFactoryBuilder; | ||
use Symfony\Component\OptionsResolver\OptionsResolver; | ||
use Symfony\Component\Security\Core\User\User; | ||
|
||
class SecurityExtensionTest extends TestCase | ||
{ | ||
public function testSecurityPasswordTypeWorks() | ||
{ | ||
$user = new User('email@example.com', 'previous_password'); | ||
|
||
$this->assertSame('previous_password', $user->getPassword()); | ||
|
||
$form = $this->getFormFactory() | ||
->createBuilder(FormType::class, $user) | ||
->add('password', SecurityPasswordType::class, [ | ||
'security_user' => $user, | ||
]) | ||
->getForm() | ||
; | ||
|
||
$form->submit(['password' => 'new_password']); | ||
|
||
$this->assertTrue($form->isSubmitted()); | ||
$this->assertTrue($form->isValid()); | ||
$this->assertSame('encoded_password', $user->getPassword()); | ||
} | ||
|
||
public function testSecurityPasswordTypeDetectsUserObject() | ||
{ | ||
$user = new User('email@example.com', 'previous_password'); | ||
|
||
$this->assertSame('previous_password', $user->getPassword()); | ||
|
||
$form = $this->getFormFactory()->create(PasswordResetType::class, $user); | ||
$form->submit(['password' => 'new_password']); | ||
|
||
$this->assertTrue($form->isSubmitted()); | ||
$this->assertTrue($form->isValid()); | ||
$this->assertSame('encoded_password', $user->getPassword()); | ||
} | ||
|
||
public function testSecurityPasswordTypeDoesNotUpdatePasswordWhenEmpty() | ||
{ | ||
$user = new User('email@example.com', 'previous_password'); | ||
|
||
$this->assertSame('previous_password', $user->getPassword()); | ||
|
||
$form = $this->getFormFactory()->create(PasswordResetType::class, $user); | ||
$form->submit([]); | ||
|
||
$this->assertTrue($form->isSubmitted()); | ||
$this->assertTrue($form->isValid()); | ||
$this->assertSame('previous_password', $user->getPassword()); | ||
} | ||
|
||
private function getFormFactory() | ||
{ | ||
$userPasswordEncoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface')->getMock(); | ||
|
||
$userPasswordEncoder | ||
->method('encodePassword') | ||
->willReturn('encoded_password'); | ||
|
||
$formFactoryBuilder = new FormFactoryBuilder(); | ||
$formFactoryBuilder->addExtension(new SecurityExtension($userPasswordEncoder)); | ||
|
||
return $formFactoryBuilder->getFormFactory(); | ||
} | ||
} | ||
|
||
class PasswordResetType extends AbstractType | ||
{ | ||
public function buildForm(FormBuilderInterface $builder, array $options) | ||
{ | ||
$builder | ||
->add('password', SecurityPasswordType::class) | ||
; | ||
} | ||
|
||
public function configureOptions(OptionsResolver $resolver) | ||
{ | ||
$resolver->setDefault('data_class', User::class); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target class isn't a form type extension, but a "form extension" which doesn't need to be registered as service.