From dcca81c2524a5e8c93f42e5b3d15a5968296027e Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Fri, 7 Jul 2023 21:32:11 +0200 Subject: [PATCH] New Password Policy listener --- .../DependencyInjection/MainConfiguration.php | 16 +++++ .../DependencyInjection/SecurityExtension.php | 7 ++ .../Resources/config/security.php | 7 ++ .../Tests/Functional/PasswordPolicyTest.php | 45 +++++++++++++ .../BlocklistPasswordPolicy.php | 22 +++++++ .../Functional/app/PasswordPolicy/bundles.php | 16 +++++ .../Functional/app/PasswordPolicy/config.yml | 51 +++++++++++++++ .../Functional/app/PasswordPolicy/routing.yml | 3 + .../Component/Security/Core/CHANGELOG.md | 2 +- .../Exception/PasswordPolicyException.php | 20 ++++++ .../Policy/PasswordConstraintPolicy.php | 38 +++++++++++ .../Policy/PasswordPolicyInterface.php | 22 +++++++ .../Component/Security/Http/CHANGELOG.md | 1 + .../EventListener/PasswordPolicyListener.php | 55 ++++++++++++++++ .../PasswordPolicyListenerTest.php | 64 +++++++++++++++++++ 15 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/PasswordPolicyTest.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/BlocklistPasswordPolicy.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/bundles.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/config.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/routing.yml create mode 100644 src/Symfony/Component/Security/Core/Exception/PasswordPolicyException.php create mode 100644 src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordConstraintPolicy.php create mode 100644 src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordPolicyInterface.php create mode 100644 src/Symfony/Component/Security/Http/EventListener/PasswordPolicyListener.php create mode 100644 src/Symfony/Component/Security/Http/Tests/EventListener/PasswordPolicyListenerTest.php diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index e982fc187194..a9da1ed0ab69 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -98,6 +98,7 @@ public function getConfigTreeBuilder(): TreeBuilder $this->addFirewallsSection($rootNode, $this->factories); $this->addAccessControlSection($rootNode); $this->addRoleHierarchySection($rootNode); + $this->addPasswordPolicySection($rootNode); return $tb; } @@ -461,6 +462,21 @@ private function addPasswordHashersSection(ArrayNodeDefinition $rootNode): void ->end(); } + private function addPasswordPolicySection(ArrayNodeDefinition $rootNode): void + { + $rootNode + ->fixXmlConfig('password_policy') + ->children() + ->arrayNode('password_policies') + ->treatNullLike([]) + ->treatFalseLike([]) + ->treatTrueLike([]) + ->prototype('scalar')->end() + ->end() + ->end() + ; + } + private function getAccessDecisionStrategies(): array { return [ diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 17a869516ef1..7c1282220c7e 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -190,6 +190,13 @@ public function load(array $configs, ContainerBuilder $container) $container->getDefinition('security.access_listener')->setArgument(3, false); $container->getDefinition('security.authorization_checker')->setArgument(2, false); $container->getDefinition('security.authorization_checker')->setArgument(3, false); + + if (!$config['password_policies']) { + $container->removeDefinition('security.password_policy_listener'); + } else { + $policyServices = array_map(static fn (string $id) => new Reference($id), $config['password_policies']); + $container->getDefinition('security.password_policy_listener')->setArgument(0, $policyServices); + } } private function createStrategyDefinition(string $strategy, bool $allowIfAllAbstainDecisions, bool $allowIfEqualGrantedDeniedDecisions): Definition diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php index 27cc0ce51e9c..e1aacb2ee40f 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php @@ -45,6 +45,7 @@ use Symfony\Component\Security\Http\Controller\SecurityTokenValueResolver; use Symfony\Component\Security\Http\Controller\UserValueResolver; use Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener; +use Symfony\Component\Security\Http\EventListener\PasswordPolicyListener; use Symfony\Component\Security\Http\Firewall; use Symfony\Component\Security\Http\FirewallMapInterface; use Symfony\Component\Security\Http\HttpUtils; @@ -298,5 +299,11 @@ ->set('cache.security_is_granted_attribute_expression_language') ->parent('cache.system') ->tag('cache.pool') + + ->set('security.password_policy_listener', PasswordPolicyListener::class) + ->args([ + [], + ]) + ->tag('kernel.event_subscriber') ; }; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/PasswordPolicyTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/PasswordPolicyTest.php new file mode 100644 index 000000000000..46b006ed992c --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/PasswordPolicyTest.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional; + +use Symfony\Component\HttpFoundation\JsonResponse; + +class PasswordPolicyTest extends AbstractWebTestCase +{ + /** + * @dataProvider providePassword + */ + public function testLoginFailBecauseThePasswordIsBlacklisted(string $password, string $expectedMessage) + { + // Given + $client = $this->createClient(['test_case' => 'PasswordPolicy', 'root_config' => 'config.yml']); + + // When + $client->request('POST', '/chk', [], [], ['CONTENT_TYPE' => 'application/json'], '{"user": {"login": "dunglas", "password": "'.$password.'"}}'); + $response = $client->getResponse(); + + // Then + $this->assertInstanceOf(JsonResponse::class, $response); + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame(['error' => $expectedMessage], json_decode($response->getContent(), true)); + } + + public static function providePassword(): iterable + { + yield ['foo', 'The password does not fulfill the password policy.']; + yield ['short?', 'The password does not fulfill the password policy.']; + yield ['Good password?', 'The password does not fulfill the password policy.']; + + // The following password fulfills the password policy, but is not valid. + yield ['Is it a v4l1d pasw0rd?', 'Invalid credentials.']; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/BlocklistPasswordPolicy.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/BlocklistPasswordPolicy.php new file mode 100644 index 000000000000..deb67027b0df --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/BlocklistPasswordPolicy.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\app\PasswordPolicy; + +use Symfony\Component\Security\Http\Authenticator\Passport\Policy\PasswordPolicyInterface; + +final class BlocklistPasswordPolicy implements PasswordPolicyInterface +{ + public function verify(#[\SensitiveParameter] string $plaintextPassword): bool + { + return 'foo' !== $plaintextPassword; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/bundles.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/bundles.php new file mode 100644 index 000000000000..edf6dae14c06 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/bundles.php @@ -0,0 +1,16 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +return [ + new Symfony\Bundle\SecurityBundle\SecurityBundle(), + new Symfony\Bundle\FrameworkBundle\FrameworkBundle(), + new Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\TestBundle(), +]; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/config.yml new file mode 100644 index 000000000000..5d44bbc87fc6 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/config.yml @@ -0,0 +1,51 @@ +imports: + - { resource: ./../config/framework.yml } + +framework: + http_method_override: false + serializer: ~ + +security: + password_policies: + - 'policy.blocklist' + - 'policy.constraint_password' + + password_hashers: + Symfony\Component\Security\Core\User\InMemoryUser: plaintext + + providers: + in_memory: + memory: + users: + dunglas: { password: foo, roles: [ROLE_USER] } + + firewalls: + main: + pattern: ^/ + json_login: + check_path: /chk + username_path: user.login + password_path: user.password + + access_control: + - { path: ^/foo, roles: ROLE_USER } + + +services: + policy.constraint_password.length: + class: Symfony\Component\Validator\Constraints\Length + arguments: + - min: 8 + policy.constraint_password.strength: + class: Symfony\Component\Validator\Constraints\PasswordStrength + arguments: + - minScore: 2 + + policy.constraint_password: + class: Symfony\Component\Security\Http\Authenticator\Passport\Policy\PasswordConstraintPolicy + arguments: + - '@validator' + - ['@policy.constraint_password.length', '@policy.constraint_password.strength'] + + policy.blocklist: + class: Symfony\Bundle\SecurityBundle\Tests\Functional\app\PasswordPolicy\BlocklistPasswordPolicy diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/routing.yml new file mode 100644 index 000000000000..e2cee70dd995 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/PasswordPolicy/routing.yml @@ -0,0 +1,3 @@ +login_check: + path: /chk + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Controller\TestController::loginCheckAction } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index b489556c919b..aca431f3cc2d 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -22,7 +22,7 @@ CHANGELOG * Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead * Remove methods `getPassword()` and `getSalt()` from `UserInterface`, use `PasswordAuthenticatedUserInterface` or `LegacyPasswordAuthenticatedUserInterface` instead -* `AccessDecisionManager` requires the strategy to be passed as in instance of `AccessDecisionStrategyInterface` + * `AccessDecisionManager` requires the strategy to be passed as in instance of `AccessDecisionStrategyInterface` 5.4.21 ------ diff --git a/src/Symfony/Component/Security/Core/Exception/PasswordPolicyException.php b/src/Symfony/Component/Security/Core/Exception/PasswordPolicyException.php new file mode 100644 index 000000000000..74efc5883411 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Exception/PasswordPolicyException.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Exception; + +final class PasswordPolicyException extends AuthenticationException +{ + public function getMessageKey(): string + { + return 'The password does not fulfill the password policy.'; + } +} diff --git a/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordConstraintPolicy.php b/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordConstraintPolicy.php new file mode 100644 index 000000000000..e9684b1e9756 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordConstraintPolicy.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authenticator\Passport\Policy; + +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Validator\ValidatorInterface; + +final class PasswordConstraintPolicy implements PasswordPolicyInterface +{ + /** + * @param array $constraints + */ + public function __construct( + private readonly ValidatorInterface $validator, + private readonly array $constraints = [] + ) { + } + + /** + * @throws AuthenticationException + */ + public function verify(#[\SensitiveParameter] string $plaintextPassword): bool + { + $errors = $this->validator->validate($plaintextPassword, $this->constraints); + + return !$errors->count(); + } +} diff --git a/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordPolicyInterface.php b/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordPolicyInterface.php new file mode 100644 index 000000000000..9d1875c012b4 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authenticator/Passport/Policy/PasswordPolicyInterface.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authenticator\Passport\Policy; + +use Symfony\Component\Security\Core\Exception\AuthenticationException; + +interface PasswordPolicyInterface +{ + /** + * @throws AuthenticationException + */ + public function verify(#[\SensitiveParameter] string $plaintextPassword): bool; +} diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index fde2311d6e11..5a63a7453f3a 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 6.4 --- + * Add Password Policy to allow verifying the password using a custom security policy (e.g. password not compromised, change required by IT service, etc.) * `UserValueResolver` no longer implements `ArgumentValueResolverInterface` 6.3 diff --git a/src/Symfony/Component/Security/Http/EventListener/PasswordPolicyListener.php b/src/Symfony/Component/Security/Http/EventListener/PasswordPolicyListener.php new file mode 100644 index 000000000000..b4e49f84be7a --- /dev/null +++ b/src/Symfony/Component/Security/Http/EventListener/PasswordPolicyListener.php @@ -0,0 +1,55 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\EventListener; + +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Security\Core\Exception\PasswordPolicyException; +use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; +use Symfony\Component\Security\Http\Authenticator\Passport\Policy\PasswordPolicyInterface; +use Symfony\Component\Security\Http\Event\CheckPassportEvent; + +final class PasswordPolicyListener implements EventSubscriberInterface +{ + /** + * @param PasswordPolicyInterface[] $policies + */ + public function __construct(private readonly array $policies) + { + } + + public function checkPassport(CheckPassportEvent $event): void + { + $passport = $event->getPassport(); + if (!$passport->hasBadge(PasswordCredentials::class)) { + return; + } + + $badge = $passport->getBadge(PasswordCredentials::class); + if ($badge->isResolved()) { + return; + } + + $plaintextPassword = $badge->getPassword(); + foreach ($this->policies as $policy) { + if (!$policy->verify($plaintextPassword)) { + throw new PasswordPolicyException(); + } + } + } + + public static function getSubscribedEvents(): array + { + return [ + CheckPassportEvent::class => ['checkPassport', 512], + ]; + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordPolicyListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordPolicyListenerTest.php new file mode 100644 index 000000000000..3705fd44c7d6 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/PasswordPolicyListenerTest.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\EventListener; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Exception\PasswordPolicyException; +use Symfony\Component\Security\Core\User\InMemoryUser; +use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; +use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; +use Symfony\Component\Security\Http\Authenticator\Passport\Policy\PasswordPolicyInterface; +use Symfony\Component\Security\Http\Event\CheckPassportEvent; +use Symfony\Component\Security\Http\EventListener\PasswordPolicyListener; + +class PasswordPolicyListenerTest extends TestCase +{ + /** + * @param array $policies + * + * @dataProvider providePassport + */ + public function testPasswordIsNotAcceptable(Passport $passport, array $policies, string $expectedMessage) + { + // Given + $event = new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport); + $listener = new PasswordPolicyListener($policies); + + try { + // When + $listener->checkPassport($event); + $this->fail('Expected exception to be thrown'); + } catch (PasswordPolicyException $e) { + // Then + $this->assertSame($expectedMessage, $e->getMessageKey()); + } + } + + public static function providePassport(): iterable + { + yield [ + new Passport( + new UserBadge('test', fn () => new InMemoryUser('test', 'qwerty')), + new PasswordCredentials('qwerty') + ), + [new class() implements PasswordPolicyInterface { + public function verify(string $plaintextPassword): bool + { + return false; + } + }], + 'The password does not fulfill the password policy.', + ]; + } +}