From 8713dce8c788b06cd396e6122459768ecad7022f Mon Sep 17 00:00:00 2001 From: Hans Mackowiak Date: Sun, 21 Jul 2024 17:00:10 +0200 Subject: [PATCH] Make Reset Password Optional (#1576) --- composer.json | 10 +-- src/Action/LoginAction.php | 5 +- src/DependencyInjection/Configuration.php | 5 +- .../SonataUserExtension.php | 15 ++++- src/Resources/config/actions.php | 43 +------------ src/Resources/config/actions_resetting.php | 61 +++++++++++++++++++ .../views/Admin/Security/login.html.twig | 2 + tests/Action/LoginActionTest.php | 22 ++++--- .../DependencyInjection/ConfigurationTest.php | 2 +- .../SonataUserExtensionNoResettingTest.php | 53 ++++++++++++++++ 10 files changed, 155 insertions(+), 63 deletions(-) create mode 100644 src/Resources/config/actions_resetting.php create mode 100644 tests/DependencyInjection/SonataUserExtensionNoResettingTest.php diff --git a/composer.json b/composer.json index 5a8b13bc3..f516f69a1 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ { "name": "Thomas Rabaix", "email": "thomas.rabaix@sonata-project.org", - "homepage": "http://sonata-project.org" + "homepage": "https://sonata-project.org" }, { "name": "Sonata Community", @@ -35,8 +35,6 @@ "symfony/framework-bundle": "^5.4 || ^6.2", "symfony/http-foundation": "^5.4 || ^6.2", "symfony/http-kernel": "^5.4 || ^6.2", - "symfony/mailer": "^5.4 || ^6.2", - "symfony/mime": "^5.4 || ^6.2", "symfony/options-resolver": "^5.4 || ^6.2", "symfony/routing": "^5.4 || ^6.2", "symfony/security-acl": "^3.0", @@ -72,6 +70,8 @@ "symfony/console": "^5.4 || ^6.2", "symfony/filesystem": "^5.4 || ^6.2", "symfony/intl": "^5.4 || ^6.2", + "symfony/mailer": "^5.4 || ^6.2", + "symfony/mime": "^5.4 || ^6.2", "symfony/phpunit-bridge": "^6.2", "vimeo/psalm": "^5.0" }, @@ -82,7 +82,9 @@ }, "suggest": { "sonata-project/admin-bundle": "For admin dashboard to manage users and other entities", - "sonata-project/doctrine-orm-admin-bundle": "For persisting entities" + "sonata-project/doctrine-orm-admin-bundle": "For persisting entities", + "symfony/mailer": "For sending reset emails", + "symfony/mime": "For sending reset emails" }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/src/Action/LoginAction.php b/src/Action/LoginAction.php index 9307aa2c1..532294d36 100644 --- a/src/Action/LoginAction.php +++ b/src/Action/LoginAction.php @@ -36,7 +36,8 @@ public function __construct( private TemplateRegistryInterface $templateRegistry, private TokenStorageInterface $tokenStorage, private TranslatorInterface $translator, - private ?CsrfTokenManagerInterface $csrfTokenManager = null + private ?CsrfTokenManagerInterface $csrfTokenManager = null, + private bool $resetMail = true ) { } @@ -68,7 +69,7 @@ public function __invoke(Request $request): Response 'csrf_token' => $csrfToken, 'error' => $this->authenticationUtils->getLastAuthenticationError(), 'last_username' => $this->authenticationUtils->getLastUsername(), - 'reset_route' => $this->urlGenerator->generate('sonata_user_admin_resetting_request'), + 'reset_route' => $this->resetMail ? $this->urlGenerator->generate('sonata_user_admin_resetting_request') : null, ])); } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index b1c11b3d1..5c53b4051 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -103,7 +103,6 @@ private function addResettingSection(ArrayNodeDefinition $node): void $node ->children() ->arrayNode('resetting') - ->isRequired() ->addDefaultsIfNotSet() ->children() ->integerNode('retry_ttl')->defaultValue(7200)->end() @@ -113,8 +112,8 @@ private function addResettingSection(ArrayNodeDefinition $node): void ->addDefaultsIfNotSet() ->children() ->scalarNode('template')->cannotBeEmpty()->defaultValue('@SonataUser/Admin/Security/Resetting/email.html.twig')->end() - ->scalarNode('address')->isRequired()->cannotBeEmpty()->end() - ->scalarNode('sender_name')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('address')->cannotBeEmpty()->end() + ->scalarNode('sender_name')->cannotBeEmpty()->end() ->end() ->end() ->end() diff --git a/src/DependencyInjection/SonataUserExtension.php b/src/DependencyInjection/SonataUserExtension.php index 8ef12a9ef..525ead004 100644 --- a/src/DependencyInjection/SonataUserExtension.php +++ b/src/DependencyInjection/SonataUserExtension.php @@ -70,12 +70,23 @@ public function load(array $configs, ContainerBuilder $container): void $this->checkManagerTypeToModelTypesMapping($config); + $resetting = false; + $this->configureClass($config, $container); - $this->configureMailer($config, $container); $this->configureDefaultAvatar($config['profile'], $container); if (isset($bundles['SonataAdminBundle'])) { $this->configureAdmin($config['admin'], $container); - $this->configureResetting($config['resetting'], $container); + // check for reset configuration + if (isset($config['resetting']['email']['address'], $config['resetting']['email']['sender_name'])) { + $resetting = true; + $loader->load('actions_resetting.php'); + $loader->load('mailer.php'); + $this->configureMailer($config, $container); + // needs to be done after mailer + $this->configureResetting($config['resetting'], $container); + } + $container->getDefinition('sonata.user.action.login') + ->replaceArgument(8, $resetting); } $this->configureImpersonation($config['impersonating'], $container); diff --git a/src/Resources/config/actions.php b/src/Resources/config/actions.php index 1bc50cf71..c47c75464 100644 --- a/src/Resources/config/actions.php +++ b/src/Resources/config/actions.php @@ -13,55 +13,13 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; -use Sonata\UserBundle\Action\CheckEmailAction; use Sonata\UserBundle\Action\CheckLoginAction; use Sonata\UserBundle\Action\LoginAction; use Sonata\UserBundle\Action\LogoutAction; -use Sonata\UserBundle\Action\RequestAction; -use Sonata\UserBundle\Action\ResetAction; return static function (ContainerConfigurator $containerConfigurator): void { $containerConfigurator->services() - ->set('sonata.user.action.request', RequestAction::class) - ->public() - ->args([ - service('twig'), - service('router'), - service('security.authorization_checker'), - service('sonata.admin.pool'), - service('sonata.admin.global_template_registry'), - service('form.factory'), - service('sonata.user.manager.user'), - service('sonata.user.mailer'), - service('sonata.user.util.token_generator'), - abstract_arg('retry ttl'), - ]) - - ->set('sonata.user.action.check_email', CheckEmailAction::class) - ->public() - ->args([ - service('twig'), - service('router'), - service('sonata.admin.pool'), - service('sonata.admin.global_template_registry'), - abstract_arg('token ttl'), - ]) - - ->set('sonata.user.action.reset', ResetAction::class) - ->public() - ->args([ - service('twig'), - service('router'), - service('security.authorization_checker'), - service('sonata.admin.pool'), - service('sonata.admin.global_template_registry'), - service('form.factory'), - service('sonata.user.manager.user'), - service('translator'), - abstract_arg('token ttl'), - ]) - ->set('sonata.user.action.login', LoginAction::class) ->public() ->args([ @@ -73,6 +31,7 @@ service('security.token_storage'), service('translator'), service('security.csrf.token_manager')->nullOnInvalid(), + true, ]) ->set('sonata.user.action.check_login', CheckLoginAction::class) diff --git a/src/Resources/config/actions_resetting.php b/src/Resources/config/actions_resetting.php new file mode 100644 index 000000000..488ddabac --- /dev/null +++ b/src/Resources/config/actions_resetting.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Loader\Configurator; + +use Sonata\UserBundle\Action\CheckEmailAction; +use Sonata\UserBundle\Action\RequestAction; +use Sonata\UserBundle\Action\ResetAction; + +return static function (ContainerConfigurator $containerConfigurator): void { + // Use "param" function for creating references to parameters when dropping support for Symfony 5.1 + $containerConfigurator->services() + ->set('sonata.user.action.request', RequestAction::class) + ->public() + ->args([ + service('twig'), + service('router'), + service('security.authorization_checker'), + service('sonata.admin.pool'), + service('sonata.admin.global_template_registry'), + service('form.factory'), + service('sonata.user.manager.user'), + service('sonata.user.mailer'), + service('sonata.user.util.token_generator'), + abstract_arg('retry ttl'), + ]) + + ->set('sonata.user.action.check_email', CheckEmailAction::class) + ->public() + ->args([ + service('twig'), + service('router'), + service('sonata.admin.pool'), + service('sonata.admin.global_template_registry'), + abstract_arg('token ttl'), + ]) + + ->set('sonata.user.action.reset', ResetAction::class) + ->public() + ->args([ + service('twig'), + service('router'), + service('security.authorization_checker'), + service('sonata.admin.pool'), + service('sonata.admin.global_template_registry'), + service('form.factory'), + service('sonata.user.manager.user'), + service('translator'), + abstract_arg('token ttl'), + ]); +}; diff --git a/src/Resources/views/Admin/Security/login.html.twig b/src/Resources/views/Admin/Security/login.html.twig index db61883d5..43aeab613 100644 --- a/src/Resources/views/Admin/Security/login.html.twig +++ b/src/Resources/views/Admin/Security/login.html.twig @@ -88,7 +88,9 @@ file that was distributed with this source code. + {% if reset_route|default %} {{ 'forgotten_password'|trans({}, 'SonataUserBundle') }} + {% endif %} {% endblock %} diff --git a/tests/Action/LoginActionTest.php b/tests/Action/LoginActionTest.php index 8c736712f..402791c2d 100644 --- a/tests/Action/LoginActionTest.php +++ b/tests/Action/LoginActionTest.php @@ -126,7 +126,7 @@ public function testAlreadyAuthenticated(): void ->with('sonata_admin_dashboard') ->willReturn('/foo'); - $action = $this->getAction(); + $action = $this->getAction(true); $result = $action($request); static::assertInstanceOf(RedirectResponse::class, $result); @@ -136,7 +136,7 @@ public function testAlreadyAuthenticated(): void /** * @dataProvider provideUnauthenticatedCases */ - public function testUnauthenticated(string $lastUsername, ?AuthenticationException $errorMessage = null): void + public function testUnauthenticated(string $lastUsername, ?AuthenticationException $errorMessage = null, bool $resetting = true): void { $session = $this->createMock(Session::class); $sessionParameters = [ @@ -162,7 +162,7 @@ public function testUnauthenticated(string $lastUsername, ?AuthenticationExcepti 'csrf_token' => 'csrf-token', 'error' => $errorMessage, 'last_username' => $lastUsername, - 'reset_route' => '/foo', + 'reset_route' => $resetting ? '/foo' : null, ]; $csrfToken = $this->createMock(CsrfToken::class); @@ -175,6 +175,7 @@ public function testUnauthenticated(string $lastUsername, ?AuthenticationExcepti ->willReturn(null); $this->urlGenerator + ->expects($resetting ? static::once() : static::never()) ->method('generate') ->with('sonata_user_admin_resetting_request') ->willReturn('/foo'); @@ -197,7 +198,7 @@ public function testUnauthenticated(string $lastUsername, ?AuthenticationExcepti ->with('@SonataUser/Admin/Security/login.html.twig', $parameters) ->willReturn('template content'); - $action = $this->getAction(); + $action = $this->getAction($resetting); $result = $action($request); static::assertSame('template content', $result->getContent()); @@ -206,16 +207,18 @@ public function testUnauthenticated(string $lastUsername, ?AuthenticationExcepti /** * @return iterable * - * @phpstan-return iterable + * @phpstan-return iterable */ public function provideUnauthenticatedCases(): iterable { $error = new AuthenticationException('An error'); - yield ['', null]; - yield ['FooUser', $error]; + yield ['', null, true]; + yield ['FooUser', $error, true]; + yield ['', null, false]; + yield ['FooUser', $error, false]; } - private function getAction(): LoginAction + private function getAction(bool $resetting): LoginAction { return new LoginAction( $this->templating, @@ -225,7 +228,8 @@ private function getAction(): LoginAction $this->templateRegistry, $this->tokenStorage, $this->translator, - $this->csrfTokenManager + $this->csrfTokenManager, + $resetting, ); } } diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 9337fcda9..a341255cb 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -26,7 +26,7 @@ final class ConfigurationTest extends TestCase public function testMinimalConfigurationRequired(): void { - $this->assertConfigurationIsInvalid([]); + $this->assertConfigurationIsValid([]); $this->assertConfigurationIsValid([ 'sonata_user' => [ 'resetting' => [ diff --git a/tests/DependencyInjection/SonataUserExtensionNoResettingTest.php b/tests/DependencyInjection/SonataUserExtensionNoResettingTest.php new file mode 100644 index 000000000..0fa5662fc --- /dev/null +++ b/tests/DependencyInjection/SonataUserExtensionNoResettingTest.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\UserBundle\Tests\DependencyInjection; + +use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use Sonata\UserBundle\DependencyInjection\SonataUserExtension; + +/** + * @author Anton Dyshkant + */ +final class SonataUserExtensionNoResettingTest extends AbstractExtensionTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + $this->setParameter('kernel.bundles', [ + 'SonataDoctrineBundle' => true, + 'SonataAdminBundle' => true, + ]); + } + + public function testMailerConfigParameterIfNotSet(): void + { + $this->load(); + + $this->assertContainerBuilderNotHasService('sonata.user.mailer'); + } + + public function testMailerConfigParameter(): void + { + $this->load(['mailer' => 'custom.mailer.service.id']); + $this->assertContainerBuilderNotHasService('sonata.user.mailer'); + } + + protected function getContainerExtensions(): array + { + return [ + new SonataUserExtension(), + ]; + } +}