8000 bug #36661 [SecurityBundle] Fixed entry point service ID resolving an… · symfony/symfony@28bb74c · GitHub
[go: up one dir, main page]
Skip to content

Commit 28bb74c

Browse files
fabpotfabpot
committed
bug #36661 [SecurityBundle] Fixed entry point service ID resolving and multiple guard entry points (wouterj)
This PR was squashed before being merged into the 5.1-dev branch. Discussion ---------- [SecurityBundle] Fixed entry point service ID resolving and multiple guard entry points | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | n/a @fabpot I am not able to reproduce [the error you reported](#36575 (comment)) in any of my demo applications or in the tests introduced in this PR. The error indicates that no entry point is configured in your application, can you maybe try out this patch (given it now makes a hard error when more than one guard is used)? If it still doesn't work, can you maybe share your firewall configuration? --- _build failures are unrelated_ Commits ------- c756593 Do not make AbstractFactory internal and revert method rename 6870a18 Fixed entry point resolving and guard entry point configuration
2 parents 1308dd5 + c756593 commit 28bb74c

12 files changed

+156
-21
lines changed

‎UPGRADE-5.1.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ Routing
112112
SecurityBundle
113113
--------------
114114

115-
* Marked the `AbstractFactory`, `AnonymousFactory`, `FormLoginFactory`, `FormLoginLdapFactory`, `GuardAuthenticationFactory`,
115+
* Marked the `AnonymousFactory`, `FormLoginFactory`, `FormLoginLdapFactory`, `GuardAuthenticationFactory`,
116116
`HttpBasicFactory`, `HttpBasicLdapFactory`, `JsonLoginFactory`, `JsonLoginLdapFactory`, `RememberMeFactory`, `RemoteUserFactory`
117117
and `X509Factory` as `@internal`. Instead of extending these classes, create your own implementation based on
118118
`SecurityFactoryInterface`.

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ CHANGELOG
66

77
* Added XSD for configuration
88
* Added security configuration for priority-based access decision strategy
9-
* Marked the `AbstractFactory`, `AnonymousFactory`, `FormLoginFactory`, `FormLoginLdapFactory`, `GuardAuthenticationFactory`, `HttpBasicFactory`, `HttpBasicLdapFactory`, `JsonLoginFactory`, `JsonLoginLdapFactory`, `RememberMeFactory`, `RemoteUserFactory` and `X509Factory` as `@internal`
9+
* Marked the `AnonymousFactory`, `FormLoginFactory`, `FormLoginLdapFactory`, `GuardAuthenticationFactory`, `HttpBasicFactory`, `HttpBasicLdapFactory`, `JsonLoginFactory`, `JsonLoginLdapFactory`, `RememberMeFactory`, `RemoteUserFactory` and `X509Factory` as `@internal`
1010
* Renamed method `AbstractFactory#createEntryPoint()` to `AbstractFactory#createDefaultEntryPoint()`
1111

1212
5.0.0

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
* @author Fabien Potencier <fabien@symfony.com>
2424
* @author Lukas Kahwe Smith <smith@pooteeweet.org>
2525
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
26-
*
27-
* @internal
2826
*/
2927
abstract class AbstractFactory implements SecurityFactoryInterface
3028
{
@@ -67,7 +65,7 @@ public function create(ContainerBuilder $container, string $id, array $config, s
6765
}
6866

6967
// create entry point if applicable (optional)
70-
$entryPointId = $this->createDefaultEntryPoint($container, $id, $config, $defaultEntryPointId);
68+
$entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPointId);
7169

7270
return [$authProviderId, $listenerId, $entryPointId];
7371
}
@@ -128,7 +126,7 @@ abstract protected function getListenerId();
128126
*
129127
* @return string|null the entry point id
130128
*/
131-
protected function createDefaultEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId)
129+
protected function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId)
132130
{
133131
return $defaultEntryPointId;
134132
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/EntryPointFactoryInterface.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
interface EntryPointFactoryInterface
2222
{
2323
/**
24-
* Creates the entry point and returns the service ID.
24+
* Register the entry point on the container and returns the service ID.
25+
*
26+
* This does not mean that the entry point is also used. This is managed
27+
* by the "entry_point" firewall setting.
2528
*/
26-
public function createEntryPoint(ContainerBuilder $container, string $id, array $config): ?string;
29+
public function registerEntryPoint(ContainerBuilder $container, string $id, array $config): ?string;
2730
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginFactory.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ protected function createListener(ContainerBuilder $container, string $id, array
9292
return $listenerId;
9393
}
9494

95-
protected function createDefaultEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId)
95+
protected function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId)
9696
{
97-
return $this->createEntryPoint($container, $id, $config);
97+
return $this->registerEntryPoint($container, $id, $config);
9898
}
9999

100-
public function createEntryPoint(ContainerBuilder $container, string $id, array $config): string
100+
public function registerEntryPoint(ContainerBuilder $container, string $id, array $config): string
101101
{
102102
$entryPointId = 'security.authentication.form_entry_point.'.$id;
103103
$container

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory;
1313

1414
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
15+
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
1516
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
1617
use Symfony\Component\DependencyInjection\ChildDefinition;
1718
use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -113,15 +114,13 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
113114
return $authenticatorIds;
114115
}
115116

116-
public function createEntryPoint(ContainerBuilder $container, string $id, array $config): ?string
117+
public function registerEntryPoint(ContainerBuilder $container, string $id, array $config): ?string
117118
{
118119
try {
119120
return $this->determineEntryPoint(null, $config);
120121
} catch (\LogicException $e) {
121-
// ignore the exception, the new system prefers setting "entry_point" over "guard.entry_point"
122+
throw new InvalidConfigurationException(sprintf('Because you have multiple guard authenticators, you need to set the "entry_point" key to one of your authenticators (%s).', implode(', ', $config['authenticators'])));
122123
}
123-
124-
return null;
125124
}
126125

127126
private function determineEntryPoint(?string $defaultEntryPointId, array $config): string

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function create(ContainerBuilder $container, string $id, array $config, s
3838
// entry point
3939
$entryPointId = $defaultEntryPoint;
4040
if (null === $entryPointId) {
41-
$entryPointId = $this->createEntryPoint($container, $id, $config);
41+
$entryPointId = $this->registerEntryPoint($container, $id, $config);
4242
}
4343

4444
// listener
@@ -82,7 +82,7 @@ public function addConfiguration(NodeDefinition $node)
8282
;
8383
}
8484

85-
public function createEntryPoint(ContainerBuilder $container, string $id, array $config): string
85+
public function registerEntryPoint(ContainerBuilder $container, string $id, array $config): string
8686
{
8787
$entryPointId = 'security.authentication.basic_entry_point.'.$id;
8888
$container

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php

Lines changed: 1 addition & 1 deletion
< 2851 div data-testid="deletion diffstat" class="DiffSquares-module__diffSquare--h5kjy DiffSquares-module__deletion--hKV3q">
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function create(ContainerBuilder $container, string $id, array $config, s
4343
;
4444

4545
// entry point
46-
$entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPoint);
46+
$entryPointId = $this->registerEntryPoint($container, $id, $config, $defaultEntryPoint);
4747

4848
if (!empty($config['query_string'])) {
4949
if ('' === $config['search_dn'] || '' === $config['search_password']) {

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use Symfony\Component\DependencyInjection\Reference;
3434
use Symfony\Component\EventDispatcher\EventDispatcher;
3535
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
36+
use Symfony\Component\Ldap\Entry;
3637
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
3738
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
3839
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
@@ -443,6 +444,9 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
443444
if (!$this->authenticatorManagerEnabled) {
444445
$authenticationProviders = array_merge($authenticationProviders, $firewallAuthenticationProviders);
445446
} else {
447+
// $configuredEntryPoint is resolved into a service ID and stored in $defaultEntryPoint
448+
$configuredEntryPoint = $defaultEntryPoint;
449+
446450
// authenticator manager
447451
$authenticators = array_map(function ($id) {
448452
return new Reference($id);
@@ -543,7 +547,7 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri
543547
$authenticationProviders[] = $authenticators;
544548
}
545549

546-
if ($factory instanceof EntryPointFactoryInterface && ($entryPoint = $factory->createEntryPoint($container, $id, $firewall[$key], null))) {
550+
if ($factory instanceof EntryPointFactoryInterface && ($entryPoint = $factory->registerEntryPoint($container, $id, $firewall[$key]))) {
547551
$entryPoints[$key] = $entryPoint;
548552
}
549553
} else {

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Security/Factory/GuardAuthenticationFactoryTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public function testAuthenticatorSystemCreate()
178178
$authenticators = $factory->createAuthenticator($container, $firewallName, $config, $userProviderId);
179179
$this->assertEquals('security.authenticator.guard.my_firewall.0', $authenticators[0]);
180180

181-
$entryPointId = $factory->createEntryPoint($container, $firewallName, $config, null);
181+
$entryPointId = $factory->registerEntryPoint($container, $firewallName, $config, null);
182182
$this->assertEquals('authenticator123', $entryPointId);
183183

184184
$authenticatorDefinition = $container->getDefinition('security.authenticator.guard.my_firewall.0');

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,19 @@
1616
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
1717
use Symfony\Bundle\SecurityBundle\SecurityBundle;
1818
use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider\DummyProvider;
19+
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FirewallEntryPointBundle\Security\EntryPointStub;
20+
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\GuardedBundle\AppCustomAuthenticator;
21+
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
1922
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
2023
use Symfony\Component\DependencyInjection\ContainerBuilder;
2124
use Symfony\Component\DependencyInjection\Reference;
2225
use Symfony\Component\ExpressionLanguage\Expression;
26+
use Symfony\Component\HttpFoundation\Request;
27+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
28+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
29+
use Symfony\Component\Security\Core\User\UserInterface;
2330
use Symfony\Component\Security\Core\User\UserProviderInterface;
31+
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2432

2533
class SecurityExtensionTest extends TestCase
2634
{
@@ -413,6 +421,90 @@ public function testSwitchUserWithSeveralDefinedProvidersButNoFirewallRootProvid
413421
$this->assertEquals(new Reference('security.user.provider.concrete.second'), $container->getDefinition('security.authentication.switchuser_listener.foobar')->getArgument(1));
414422
}
415423

424+
/**
425+
* @dataProvider provideEntryPointFirewalls
426+
*/
427+
public function testAuthenticatorManagerEnabledEntryPoint(array $firewall, $entryPointId)
428+
{
429+
$container = $this->getRawContainer();
430+
$container->loadFromExtension('security', [
431+
'enable_authenticator_manager' => true,
432+
'providers' => [
433+
'first' => ['id' => 'users'],
434+
],
435+
436+
'firewalls' => [
437+
'main' => $firewall,
438+
],
439+
]);
440+
441+
$container->compile();
442+
443+
$this->assertEquals($entryPointId, (string) $container->getDefinition('security.firewall.map.config.main')->getArgument(7));
444+
$this->assertEquals($entryPointId, (string) $container->getDefinition('security.exception_listener.main')->getArgument(4));
445+
}
446+
447+
public function provideEntryPointFirewalls()
448+
{
449+
// only one entry point available
450+
yield [['http_basic' => true], 'security.authentication.basic_entry_point.main'];
451+
// explicitly configured by authenticator key
452+
yield [['form_login' => true, 'http_basic' => true, 'entry_point' => 'form_login'], 'security.authentication.form_entry_point.main'];
453+
// explicitly configured another service
454+
yield [['form_login' => true, 'entry_point' => EntryPointStub::class], EntryPointStub::class];
455+
// no entry point required
456+
yield [['json_login' => true], null];
457+
458+
// only one guard authenticator entry point available
459+
yield [[
460+
'guard' => ['authenticators' => [AppCustomAuthenticator::class]],
461+
], AppCustomAuthenticator::class];
462+
// explicitly configured guard authenticator entry point
463+
yield [[
464+
'guard' => [
465+
'authenticators' => [AppCustomAuthenticator::class, NullAuthenticator::class],
466+
'entry_point' => NullAuthenticator::class,
467+
],
468+
], NullAuthenticator::class];
469+
}
470+
471+
/**
472+
* @dataProvider provideEntryPointRequiredData
473+
*/
474+
public function testEntryPointRequired(array $firewall, $messageRegex)
475+
{
476+
$this->expectException(InvalidConfigurationException::class);
477+
$this->expectExceptionMessageMatches($messageRegex);
478+
479+
$container = $this->getRawContainer();
480+
$container->loadFromExtension('security', [
481+
'enable_authenticator_manager' => true,
482+
'providers' => [
483+
'first' => ['id' => 'users'],
484+
],
485+
486+
'firewalls' => [
487+
'main' => $firewall,
488+
],
489+
]);
490+
491+
$container->compile();
492+
}
493+
494+
public function provideEntryPointRequiredData()
495+
{
496+
// more than one entry point available and not explicitly set
497+
yield [
498+
['http_basic' => true, 'form_login' => true],
499+
'/^Because you have multiple authenticators in firewall "main", you need to set the "entry_point" key to one of your authenticators/',
500+
];
501+
// more than one guard entry point available and not explicitly set
502+
yield [
503+
['guard' => ['authenticators' => [AppCustomAuthenticator::class, NullAuthenticator::class]]],
504+
'/^Because you have multiple guard authenticators, you need to set the "entry_point" key to one of your authenticators/',
505+
];
506+
}
507+
416508
protected function getRawContainer()
417509
{
418510
$container = new ContainerBuilder();
@@ -439,3 +531,42 @@ protected function getContainer()
439531
return $container;
440532
}
441533
}
534+
535+
class NullAuthenticator implements AuthenticatorInterface
536+
{
537+
public function start(Request $request, AuthenticationException $authException = null)
538+
{
539+
}
540+
541+
public function supports(Request $request)
542+
{
543+
}
544+
545+
public function getCredentials(Request $request)
546+
{
547+
}
548+
549+
public function getUser($credentials, UserProviderInterface $userProvider)
550+
{
551+
}
552+
553+
public function checkCredentials($credentials, UserInterface $user)
554+
{
555+
}
556+
557+
public function createAuthenticatedToken(UserInterface $user, string $providerKey)
558+
{
559+
}
560+
561+
public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
562+
{
563+
}
564+
565+
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $providerKey)
566+
{
567+
}
568+
569+
public function supportsRememberMe()
570+
{
571+
}
572+
}

‎src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"php": "^7.2.5",
2020
"ext-xml": "*",
2121
"symfony/config": "^4.4|^5.0",
22-
"symfony/dependency-injection": "^4.4|^5.0",
22+
"symfony/dependency-injection": "^5.1",
2323
"symfony/event-dispatcher": "^5.1",
2424
"symfony/http-kernel": "^5.0",
2525
"symfony/polyfill-php80": "^1.15",

0 commit comments

Comments
 (0)
0