8000 [Security/Http] call auth listeners/guards eagerly when they "support… · symfony/symfony@a2ad5ba · GitHub
[go: up one dir, main page]
Skip to content < 8000 div data-target="react-partial.reactRoot">

Commit a2ad5ba

Browse files
nicolas-grekasnicolas-grekas
committed
[Security/Http] call auth listeners/guards eagerly when they "support" the request
1 parent 59b6cfe commit a2ad5ba

21 files changed

+353
-133
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,7 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
409409
}
410410

411411
// Access listener
412-
if ($firewall['stateless'] || empty($firewall['anonymous']['lazy'])) {
413-
$listeners[] = new Reference('security.access_listener');
414-
}
412+
$listeners[] = new Reference('security.access_listener');
415413

416414
// Exception listener
417415
$exceptionListener = new Reference($this->createExceptionListener($container, $firewall, $id, $configuredEntryPoint ?: $defaultEntryPoint, $firewall['stateless']));

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,7 @@
156156
<argument type="service" id="security.exception_listener" />
157157
<argument /> <!-- LogoutListener -->
158158
<argument /> <!-- FirewallConfig -->
159-
<argument type="service" id="security.access_listener" />
160159
<argument type="service" id="security.untracked_token_storage" />
161-
<argument type="service" id="security.access_map" />
162160
</service>
163161

164162
<service id="security.firewall.config" class="Symfony\Bundle\SecurityBundle\Security\FirewallConfig" abstract="true">

‎src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php

Copy file name to clipboard
Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@
1313

1414
use Symfony\Component\HttpKernel\Event\RequestEvent;
1515
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
16-
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
17-
use Symfony\Component\Security\Core\Exception\LazyResponseException;
18-
use Symfony\Component\Security\Http\AccessMapInterface;
1916
use Symfony\Component\Security\Http\Event\LazyResponseEvent;
20-
use Symfony\Component\Security\Http\Firewall\AccessListener;
17+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
2118
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
2219
use Symfony\Component\Security\Http\Firewall\LogoutListener;
2320

@@ -32,13 +29,11 @@ class LazyFirewallContext extends FirewallContext
3229
private $tokenStorage;
3330
private $map;
3431

35-
public function __construct(iterable $listeners, ?ExceptionListener $exceptionListener, ?LogoutListener $logoutListener, ?FirewallConfig $config, AccessListener $accessListener, TokenStorage $tokenStorage, AccessMapInterface $map)
32+
public function __construct(iterable $listeners, ?ExceptionListener $exceptionListener, ?LogoutListener $logoutListener, ?FirewallConfig $config, TokenStorage $tokenStorage)
3633
{
3734
parent::__construct($listeners, $exceptionListener, $logoutListener, $config);
3835

39-
$this->accessListener = $accessListener;
4036
$this->tokenStorage = $tokenStorage;
41-
$this->map = $map;
4237
}
4338

4439
public function getListeners(): iterable
@@ -48,26 +43,39 @@ public function getListeners(): iterable
4843

4944
public function __invoke(RequestEvent $event)
5045
{
51-
$this->tokenStorage->setInitializer(function () use ($event) {
52-
$event = new LazyResponseEvent($event);
53-
foreach (parent::getListeners() as $listener) {
54-
if (\is_callable($listener)) {
55-
$listener($event);
56-
} else {
57-
@trigger_error(sprintf('Calling the "%s::handle()" 10000 ; method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($listener)), E_USER_DEPRECATED);
58-
$listener->handle($event);
59-
}
46+
$listeners = [];
47+
$request = $event->getRequest();
48+
$lazy = $request->isMethodCacheable();
49+
50+
foreach (parent::getListeners() as $listener) {
51+
if (!\is_callable($listener)) {
52+
@trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($listener)), E_USER_DEPRECATED);
53+
$listeners[] = [$listener, 'handle'];
54+
} elseif (!$lazy || !$listener instanceof AbstractListener) {
55+
$listeners[] = $listener;
56+
} elseif (false !== $supports = $listener->supports($request)) {
57+
$listeners[] = [$listener, 'authenticate'];
58+
$lazy = null === $supports;
6059
}
61-
});
60+
}
6261

63-
try {
64-
[$attributes] = $this->map->getPatterns($event->getRequest());
62+
if (!$lazy) {
63+
foreach ($listeners as $listener) {
64+
$listener($event);
6565

66-
if ($attributes && [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] !== $attributes) {
67-
($this->accessListener)($event);
66+
if ($event->hasResponse()) {
67+
return;
68+
}
6869
}
69-
} catch (LazyResponseException $e) {
70-
$event->setResponse($e->getResponse());
70+
71+
return;
7172
}
73+
74+
$this->tokenStorage->setInitializer(function () use ($event, $listeners) {
75+
$event = new LazyResponseEvent($event);
76+
foreach ($listeners as $listener) {
77+
$listener($event);
78+
}
79+
});
7280
}
7381
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/GuardedBundle/AppCustomAuthenticator.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
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\GuardedBundle;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\Response;
16+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
17+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
18+
use Symfony\Component\Security\Core\User\UserInterface;
19+
use Symfony\Component\Security\Core\User\UserProviderInterface;
20+
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
21+
22+
class AppCustomAuthenticator extends AbstractGuardAuthenticator
23+
{
24+
public function supports(Request $request)
25+
{
26+
return true;
27+
}
28+
29+
public function getCredentials(Request $request)
30+
{
31+
throw new AuthenticationException('This should be hit');
32+
}
33+
34+
public function getUser($credentials, UserProviderInterface $userProvider)
35+
{
36+
}
37+
38+
public function checkCredentials($credentials, UserInterface $user)
39+
{
40+
}
41+
42+
public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
43+
{
44+
return new Response('', 418);
45+
}
46+
47+
public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey)
48+
{
49+
}
50+
51+
public function start(Request $request, AuthenticationException $authException = null)
52+
{
53+
return new Response($authException->getMessage(), Response::HTTP_UNAUTHORIZED);
54+
}
55+
56+
public function supportsRememberMe()
57+
{
58+
}
59+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/GuardedTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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;
13+
14+
class GuardedTest extends AbstractWebTestCase
15+
{
16+
public function testGuarded()
17+
{
18+
$client = $this->createClient(['test_case' => 'Guarded', 'root_config' => 'config.yml']);
19+
20+
$client->request('GET', '/');
21+
22+
$this->assertSame(418, $client->getResponse()->getStatusCode());
23+
}
24+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Guarded/bundles.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
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+
return [
13+
new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
14+
new Symfony\Bundle\SecurityBundle\SecurityBundle(),
15+
];

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Guarded/config.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
framework:
2+
secret: test
3+
router: { resource: "%kernel.project_dir%/%kernel.test_case%/routing.yml" }
4+
test: ~
5+
default_locale: en
6+
profiler: false
7+
session:
8+
storage_id: session.storage.mock_file
9+
10+
services:
11+
logger: { class: Psr\Log\NullLogger }
12+
Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\GuardedBundle\AppCustomAuthenticator: ~
13+
14+
security:
15+
firewalls:
16+
secure:
17+
pattern: ^/
18+
anonymous: lazy
19+
stateless: false
20+
guard:
21+
authenticators:
22+
- Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\GuardedBundle\AppCustomAuthenticator

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Guarded/routing.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
main:
2+
path: /
3+
defaults:
4+
_controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction
5+
path: /app

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"symfony/security-core": "^4.4",
2525
"symfony/security-csrf": "^4.2|^5.0",
2626
"symfony/security-guard": "^4.2|^5.0",
27-
"symfony/security-http": "^4.4"
27+
"symfony/security-http": "^4.4.1"
2828
},
2929
"require-dev": {
3030
"doctrine/doctrine-bundle": "^1.5|^2.0",

‎src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\Security\Guard\AuthenticatorInterface;
2222
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
2323
use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
24+
use Symfony\Component\Security\Http\Firewall\AbstractListener;
2425
use Symfony\Component\Security\Http\Firewall\LegacyListenerTrait;
2526
use Symfony\Component\Security\Http\Firewall\ListenerInterface;
2627
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
@@ -33,7 +34,7 @@
3334
*
3435
* @final since Symfony 4.3
3536
*/
36-
class GuardAuthenticationListener implements ListenerInterface
37+
class GuardAuthenticationListener extends AbstractListener implements ListenerInterface
3738
{
3839
use LegacyListenerTrait;
3940

@@ -62,9 +63,9 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
6263
}
6364

6465
/**
65-
* Iterates over each authenticator to see if each wants to authenticate the request.
66+
* {@inheritdoc}
6667
*/
67-
public function __invoke(RequestEvent $event)
68+
public function supports(Request $request): ?bool
6869
{
6970
if (null !== $this->logger) {
7071
$context = ['firewall_key' => $this->providerKey];
@@ -76,7 +77,39 @@ public function __invoke(RequestEvent $event)
7677
$this->logger->debug('Checking for guard authentication credentials.', $context);
7778
}
7879

80+
$guardAuthenticators = [];
81+
7982
foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
83+
if (null !== $this->logger) {
84+
$this->logger->debug('Checking support on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
85+
}
86+
87+
if ($guardAuthenticator->supports($request)) {
88+
$guardAuthenticators[$key] = $guardAuthenticator;
89+
} elseif (null !== $this->logger) {
90+
$this->logger->debug('Guard authenticator does not support the request.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
91+
}
92+
}
93+
94+
if (!$guardAuthenticators) {
95+
return false;
96+
}
97+
98+
$request->attributes->set('_guard_authenticators', $guardAuthenticators);
99+
100+
return true;
101+
}
102+
103+
/**
104+
* Iterates over each authenticator to see if each wants to authenticate the request.
105+
*/
106+
public function authenticate(RequestEvent $event)
107+
{
108+
$request = $event->getRequest();
109+
$guardAuthenticators = $request->attributes->get('_guard_authenticators');
110+
$request->attributes->remove('_guard_authenticators');
111+
112+
foreach ($guardAuthenticators as $key => $guardAuthenticator) {
80113
// get a key that's unique to *this* guard authenticator
81114
// this MUST be the same as GuardAuthenticationProvider
82115
$uniqueGuardKey = $this->providerKey.'_'.$key;
@@ -97,19 +130,6 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator
97130
{
98131
$request = $event->getRequest();
99132
try {
100-
if (null !== $this->logger) {
101-
$this->logger->debug('Checking support on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
102-
}
103-
104-
// abort the execution of the authenticator if it doesn't support the request
105-
if (!$guardAuthenticator->supports($request)) {
106-
if (null !== $this->logger) {
107-
$this->logger->debug('Guard authenticator does not support the request.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
108-
}
109-
110-
return;
111-
}
112-
113133
if (null !== $this->logger) {
114134
$this->logger->debug('Calling getCredentials() on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
115135
}

‎src/Symfony/Component/Security/Guard/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": "^7.1.3",
2020
"symfony/security-core": "^3.4.22|^4.2.3|^5.0",
21-
"symfony/security-http": "^4.3"
21+
"symfony/security-http": "^4.4.1"
2222
},
2323
"require-dev": {
2424
"psr/log": "~1.0"

‎src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
* @author Fabien Potencier <fabien@symfony.com>
5050
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
5151
*/
52-
abstract class AbstractAuthenticationListener implements ListenerInterface
52+
abstract class AbstractAuthenticationListener extends AbstractListener implements ListenerInterface
5353
{
5454
use LegacyListenerTrait;
5555

@@ -105,20 +105,24 @@ public function setRememberMeServices(RememberMeServicesInterface $rememberMeSer
105105
$this->rememberMeServices = $rememberMeServices;
106106
}
107107

108+
/**
109+
* {@inheritdoc}
110+
*/
111+
public function supports(Request $request): ?bool
112+
{
113+
return $this->requiresAuthentication($request);
114+
}
115+
108116
/**
109117
* Handles form based authentication.
110118
*
111119
* @throws \RuntimeException
112120
* @throws SessionUnavailableException
113121
*/
114-
public function __invoke(RequestEvent $event)
122+
public function authenticate(RequestEvent $event)
115123
{
116124
$request = $event->getRequest();
117125

118-
if (!$this->requiresAuthentication($request)) {
119-
return;
120-
}
121-
122126
if (!$request->hasSession()) {
123127
throw new \RuntimeException('This authentication method requires a session.');
124128
}

0 commit comments

Comments
 (0)
0