Skip to content

Commit e16ac30

Browse files
RobertMenicolas-grekas
RobertMe
authored and
nicolas-grekas
committed
[SecurityBundle] Fix using same handler for multiple authenticators
Using a reference for custom success/failure handler breaks using the same service for multiple authenticators. This as the options will be overriden by any later authenticators. So use a ChildDefinition instead so every authenticator gets a unique instance. Fixes: #48923
1 parent 8203ec9 commit e16ac30

File tree

8 files changed

+171
-5
lines changed

8 files changed

+171
-5
lines changed

‎DependencyInjection/Security/Factory/AbstractFactory.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ protected function createAuthenticationSuccessHandler(ContainerBuilder $containe
164164

165165
if (isset($config['success_handler'])) {
166166
$successHandler = $container->setDefinition($successHandlerId, new ChildDefinition('security.authentication.custom_success_handler'));
167-
$successHandler->replaceArgument(0, new Reference($config['success_handler']));
167+
$successHandler->replaceArgument(0, new ChildDefinition($config['success_handler']));
168168
$successHandler->replaceArgument(1, $options);
169169
$successHandler->replaceArgument(2, $id);
170170
} else {
@@ -183,7 +183,7 @@ protected function createAuthenticationFailureHandler(ContainerBuilder $containe
183183

184184
if (isset($config['failure_handler'])) {
185185
$failureHandler = $container->setDefinition($id, new ChildDefinition('security.authentication.custom_failure_handler'));
186-
$failureHandler->replaceArgument(0, new Reference($config['failure_handler']));
186+
$failureHandler->replaceArgument(0, new ChildDefinition($config['failure_handler']));
187187
$failureHandler->replaceArgument(1, $options);
188188
} else {
189189
$failureHandler = $container->setDefinition($id, new ChildDefinition('security.authentication.failure_handler'));

‎Tests/DependencyInjection/Security/Factory/AbstractFactoryTest.php‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory;
16+
use Symfony\Component\DependencyInjection\ChildDefinition;
1617
use Symfony\Component\DependencyInjection\ContainerBuilder;
1718
use Symfony\Component\DependencyInjection\Reference;
1819

@@ -69,12 +70,19 @@ public function testDefaultFailureHandler($serviceId, $defaultHandlerInjection)
6970
$this->assertEquals(new Reference('security.authentication.failure_handler.foo.abstract_factory'), $arguments['index_6']);
7071
$failureHandler = $container->findDefinition((string) $arguments['index_6']);
7172

73+
$expectedFailureHandlerOptions = ['login_path' => '/bar'];
7274
$methodCalls = $failureHandler->getMethodCalls();
7375
if ($defaultHandlerInjection) {
7476
$this->assertEquals('setOptions', $methodCalls[0][0]);
75-
$this->assertEquals(['login_path' => '/bar'], $methodCalls[0][1][0]);
77+
$this->assertEquals($expectedFailureHandlerOptions, $methodCalls[0][1][0]);
7678
} else {
7779
$this->assertCount(0, $methodCalls);
80+
$this->assertInstanceOf(ChildDefinition::class, $failureHandler);
81+
$this->assertEquals('security.authentication.custom_failure_handler', $failureHandler->getParent());
82+
$failureHandlerArguments = $failureHandler->getArguments();
83+
$this->assertInstanceOf(ChildDefinition::class, $failureHandlerArguments['index_0']);
84+
$this->assertEquals($serviceId, $failureHandlerArguments['index_0']->getParent());
85+
$this->assertEquals($expectedFailureHandlerOptions, $failureHandlerArguments['index_1']);
7886
}
7987
}
8088

@@ -108,13 +116,22 @@ public function testDefaultSuccessHandler($serviceId, $defaultHandlerInjection)
108116
$successHandler = $container->findDefinition((string) $arguments['index_5']);
109117
$methodCalls = $successHandler->getMethodCalls();
110118

119+
$expectedSuccessHandlerOptions = ['default_target_path' => '/bar'];
120+
$expectedFirewallName = 'foo';
111121
if ($defaultHandlerInjection) {
112122
$this->assertEquals('setOptions', $methodCalls[0][0]);
113-
$this->assertEquals(['default_target_path' => '/bar'], $methodCalls[0][1][0]);
123+
$this->assertEquals($expectedSuccessHandlerOptions, $methodCalls[0][1][0]);
114124
$this->assertEquals('setFirewallName', $methodCalls[1][0]);
115-
$this->assertEquals(['foo'], $methodCalls[1][1]);
125+
$this->assertEquals($expectedFirewallName, $methodCalls[1][1][0]);
116126
} else {
117127
$this->assertCount(0, $methodCalls);
128+
$this->assertInstanceOf(ChildDefinition::class, $successHandler);
129+
$this->assertEquals('security.authentication.custom_success_handler', $successHandler->getParent());
130+
$successHandlerArguments = $successHandler->getArguments();
131+
$this->assertInstanceOf(ChildDefinition::class, $successHandlerArguments['index_0']);
132+
$this->assertEquals($serviceId, $successHandlerArguments['index_0']->getParent());
133+
$this->assertEquals($expectedSuccessHandlerOptions, $successHandlerArguments['index_1']);
134+
$this->assertEquals($expectedFirewallName, $successHandlerArguments['index_2']);
118135
}
119136
}
120137

‎Tests/Functional/AuthenticatorTest.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,38 @@ public function testMultipleFirewalls()
102102
$client->request('GET', '/firewall2/profile');
103103
$this->assertResponseRedirects('http://localhost/login');
104104
}
105+
106+
public function testCustomSuccessHandler()
107+
{
108+
$client = $this->createClient(['test_case' => 'Authenticator', 'root_config' => 'custom_handlers.yml']);
109+
110+
$client->request('POST', '/firewall1/login', [
111+
'_username' => 'jane@example.org',
112+
'_password' => 'test',
113+
]);
114+
$this->assertResponseRedirects('http://localhost/firewall1/test');
115+
116+
$client->request('POST', '/firewall1/dummy_login', [
117+
'_username' => 'jane@example.org',
118+
'_password' => 'test',
119+
]);
120+
$this->assertResponseRedirects('http://localhost/firewall1/dummy');
121+
}
122+
123+
public function testCustomFailureHandler()
124+
{
125+
$client = $this->createClient(['test_case' => 'Authenticator', 'root_config' => 'custom_handlers.yml']);
126+
127+
$client->request('POST', '/firewall1/login', [
128+
'_username' => 'jane@example.org',
129+
'_password' => '',
130+
]);
131+
$this->assertResponseRedirects('http://localhost/firewall1/login');
132+
133+
$client->request('POST', '/firewall1/dummy_login', [
134+
'_username' => 'jane@example.org',
135+
'_password' => '',
136+
]);
137+
$this->assertResponseRedirects('http://localhost/firewall1/dummy_login');
138+
}
105139
}

‎Tests/Functional/Bundle/AuthenticatorBundle/AuthenticatorBundle.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AuthenticatorBundle;
13+
14+
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\HttpKernel\Bundle\Bundle;
17+
18+
class AuthenticatorBundle extends Bundle
19+
{
20+
public function build(ContainerBuilder $container)
21+
{
22+
parent::build($container);
23+
24+
$this->configureSecurityExtension($container);
25+
}
26+
27+
private function configureSecurityExtension(ContainerBuilder $container): void
28+
{
29+
/** @var SecurityExtension $extension */
30+
$extension = $container->getExtension('security');
31+
32+
$extension->addAuthenticatorFactory(new DummyFormLoginFactory());
33+
}
34+
}

‎Tests/Functional/Bundle/AuthenticatorBundle/DummyFormLoginFactory.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AuthenticatorBundle;
13+
14+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
15+
use Symfony\Component\DependencyInjection\ChildDefinition;
16+
use Symfony\Component\DependencyInjection\ContainerBuilder;
17+
use Symfony\Component\DependencyInjection\Reference;
18+
19+
class DummyFormLoginFactory extends FormLoginFactory
20+
{
21+
public function getKey(): string
22+
{
23+
return 'dummy_form_login';
24+
}
25+
26+
public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId): string
27+
{
28+
$authenticatorId = 'security.authenticator.dummy_form_login.'.$firewallName;
29+
$options = array_intersect_key($config, $this->options);
30+
$authenticator = $container
31+
->setDefinition($authenticatorId, new ChildDefinition('security.authenticator.form_login'))
32+
->replaceArgument(1, new Reference($userProviderId))
33+
->replaceArgument(2, new Reference($this->createAuthenticationSuccessHandler($container, $firewallName, $config)))
34+
->replaceArgument(3, new Reference($this->createAuthenticationFailureHandler($container, $firewallName, $config)))
35+
->replaceArgument(4, $options);
36+
37+
if ($options['use_forward'] ?? false) {
38+
$authenticator->addMethodCall('setHttpKernel', [new Reference('http_kernel')]);
39+
}
40+
41+
return $authenticatorId;
42+
}
43+
}

‎Tests/Functional/app/Authenticator/bundles.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@
1212
return [
1313
new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
1414
new Symfony\Bundle\SecurityBundle\SecurityBundle(),
15+
new Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AuthenticatorBundle\AuthenticatorBundle(),
1516
];

‎Tests/Functional/app/Authenticator/custom_handlers.yml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
imports:
2+
- { resource: ./config.yml }
3+
- { resource: ./security.yml }
4+
5+
security:
6+
enable_authenticator_manager: true
7+
firewalls:
8+
firewall1:
9+
pattern: /firewall1
10+
provider: in_memory
11+
entry_point: form_login
12+
form_login:
13+
check_path: /firewall1/login
14+
success_handler: success_handler
15+
failure_handler: failure_handler
16+
default_target_path: /firewall1/test
17+
login_path: /firewall1/login
18+
dummy_form_login:
19+
check_path: /firewall1/dummy_login
20+
success_handler: success_handler
21+
failure_handler: failure_handler
22+
default_target_path: /firewall1/dummy
23+
login_path: /firewall1/dummy_login
24+
25+
services:
26+
success_handler:
27+
class: Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler
28+
arguments:
29+
- '@security.http_utils'
30+
failure_handler:
31+
class: Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler
32+
arguments:
33+
- '@http_kernel'
34+
- '@security.http_utils'

‎Tests/Functional/app/Authenticator/routing.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ security_custom_profile:
2222
firewall1_login:
2323
path: /firewall1/login
2424

25+
firewall_dummy_login:
26+
path: /firewall1/dummy_login
27+
2528
firewall2_profile:
2629
path: /firewall2/profile
2730
defaults:

0 commit comments

Comments
 (0)