Skip to content

Commit

Permalink
Make Reset Password Optional (#1576)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hanmac authored Jul 21, 2024
1 parent c5e4bcd commit 8713dce
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 63 deletions.
10 changes: 6 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
{
"name": "Thomas Rabaix",
"email": "[email protected]",
"homepage": "http://sonata-project.org"
"homepage": "https://sonata-project.org"
},
{
"name": "Sonata Community",
Expand All @@ -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",
Expand Down Expand Up @@ -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"
},
Expand All @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions src/Action/LoginAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
}

Expand Down Expand Up @@ -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,
]));
}

Expand Down
5 changes: 2 additions & 3 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ private function addResettingSection(ArrayNodeDefinition $node): void
$node
->children()
->arrayNode('resetting')
->isRequired()
->addDefaultsIfNotSet()
->children()
->integerNode('retry_ttl')->defaultValue(7200)->end()
Expand All @@ -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()
Expand Down
15 changes: 13 additions & 2 deletions src/DependencyInjection/SonataUserExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
43 changes: 1 addition & 42 deletions src/Resources/config/actions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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)
Expand Down
61 changes: 61 additions & 0 deletions src/Resources/config/actions_resetting.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* 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'),
]);
};
2 changes: 2 additions & 0 deletions src/Resources/views/Admin/Security/login.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ file that was distributed with this source code.
</div>
</form>

{% if reset_route|default %}
<a href="{{ reset_route }}">{{ 'forgotten_password'|trans({}, 'SonataUserBundle') }}</a>
{% endif %}
{% endblock %}
</div>
</div>
Expand Down
22 changes: 13 additions & 9 deletions tests/Action/LoginActionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 = [
Expand All @@ -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);
Expand All @@ -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');
Expand All @@ -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());
Expand All @@ -206,16 +207,18 @@ public function testUnauthenticated(string $lastUsername, ?AuthenticationExcepti
/**
* @return iterable<mixed[]>
*
* @phpstan-return iterable<array{string, AuthenticationException|null}>
* @phpstan-return iterable<array{string, AuthenticationException|null, boolean}>
*/
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,
Expand All @@ -225,7 +228,8 @@ private function getAction(): LoginAction
$this->templateRegistry,
$this->tokenStorage,
$this->translator,
$this->csrfTokenManager
$this->csrfTokenManager,
$resetting,
);
}
}
2 changes: 1 addition & 1 deletion tests/DependencyInjection/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ final class ConfigurationTest extends TestCase

public function testMinimalConfigurationRequired(): void
{
$this->assertConfigurationIsInvalid([]);
$this->assertConfigurationIsValid([]);
$this->assertConfigurationIsValid([
'sonata_user' => [
'resetting' => [
Expand Down
53 changes: 53 additions & 0 deletions tests/DependencyInjection/SonataUserExtensionNoResettingTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* 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 <[email protected]>
*/
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(),
];
}
}

0 comments on commit 8713dce

Please sign in to comment.