8000 [Security/Http] call auth listeners/guards eagerly when they "support… · symfony/symfony@82c27c1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 82c27c1

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

32 files changed

+473
-251
lines changed

‎UPGRADE-4.3.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ Security
170170
Role hierarchies must implement the `getReachableRoleNames()` method instead and return roles as strings.
171171
* The `getRoles()` method of the `TokenInterface` is deprecated. Tokens must implement the `getRoleNames()`
172172
method instead and return roles as strings.
173-
* The `ListenerInterface` is deprecated, turn your listeners into callables instead.
173+
* The `ListenerInterface` is deprecated, extend `AbstractListener` instead.
174174
* The `Firewall::handleRequest()` method is deprecated, use `Firewall::callListeners()` instead.
175175
* The `AbstractToken::serialize()`, `AbstractToken::unserialize()`,
176176
`AuthenticationException::serialize()` and `AuthenticationException::unserialize()`

‎UPGRADE-5.0.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ Security
434434
* `SimpleAuthenticatorInterface`, `SimpleFormAuthenticatorInterface`, `SimplePreAuthenticatorInterface`,
435435
`SimpleAuthenticationProvider`, `SimpleAuthenticationHandler`, `SimpleFormAuthenticationListener` and
436436
`SimplePreAuthenticationListener` have been removed. Use Guard instead.
437-
* The `ListenerInterface` has been removed, turn your listeners into callables instead.
437+
* The `ListenerInterface` has been removed, extend `AbstractListener` instead.
438438
* The `Firewall::handleRequest()` method has been removed, use `Firewall::callListeners()` instead.
439439
* `\Serializable` interface has been removed from `AbstractToken` and `AuthenticationException`,
440440
thus `serialize()` and `unserialize()` aren't available.

‎src/Symfony/Bundle/SecurityBundle/Debug/WrappedListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function __invoke(RequestEvent $event)
5050
if (\is_callable($this->listener)) {
5151
($this->listener)($event);
5252
} else {
53-
@trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($this->listener)), E_USER_DEPRECATED);
53+
@trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, extend "%s" instead.', \get_class($this->listener), AbstractListener::class), E_USER_DEPRECATED);
5454
$this->listener->handle($event);
5555
}
5656
$this->time = microtime(true) - $startTime;

‎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

Lines changed: 33 additions & 25 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

@@ -28,17 +25,13 @@
2825
*/
2926
class LazyFirewallContext extends FirewallContext
3027
{
31-
private $accessListener;
3228
private $tokenStorage;
33-
private $map;
3429

35-
public function __construct(iterable $listeners, ?ExceptionListener $exceptionListener, ?LogoutListener $logoutListener, ?FirewallConfig $config, AccessListener $accessListener, TokenStorage $tokenStorage, AccessMapInterface $map)
30+
public function __construct(iterable $listeners, ?ExceptionListener $exceptionListener, ?LogoutListener $logoutListener, ?FirewallConfig $config, TokenStorage $tokenStorage)
3631
{
3732
parent::__construct($listeners, $exceptionListener, $logoutListener, $config);
3833

39-
$this->accessListener = $accessListener;
4034
$this->tokenStorage = $tokenStorage;
41-
$this->map = $map;
4235
}
4336

4437
public function getListeners(): iterable
@@ -48,26 +41,41 @@ public function getListeners(): iterable
4841

4942
public function __invoke(RequestEvent $event)
5043
{
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()" method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($listener)), E_USER_DEPRECATED);
58-
$listener->handle($event);
59-
}
44+
$listeners = [];
45+
$request = $event->getRequest();
46+
$lazy = $request->isMethodCacheable();
47+
48+
foreach (parent::getListeners() as $listener) {
49+
if (!\is_callable($listener)) {
50+
@trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, extend "%s" instead.', \get_class($listener), AbstractListener::class), E_USER_DEPRECATED);
51+
$listeners[] = [$listener, 'handle'];
52+
$lazy = false;
53+
} elseif (!$lazy || !$listener instanceof AbstractListener) {
54+
$listeners[] = $listener;
55+
$lazy = $lazy && $listener instanceof AbstractListener;
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/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ CHANGELOG
2929
* Made the `serialize()` and `unserialize()` methods of `AbstractToken` and
3030
`AuthenticationException` final, use `__serialize()`/`__unserialize()` instead
3131
* `AuthenticationException` doesn't implement `Serializable` anymore
32-
* Deprecated the `ListenerInterface`, turn your listeners into callables instead
32+
* Deprecated the `ListenerInterface`, extend `AbstractListener` instead
3333
* Deprecated `Firewall::handleRequest()`, use `Firewall::callListeners()` instead
3434
* Dispatch `AuthenticationSuccessEvent` on `security.authentication.success`
3535
* Dispatch `AuthenticationFailureEvent` on `security.authentication.failure`

‎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.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ protected function handleRequest(GetResponseEvent $event, $listeners)
138138
if (\is_callable($listener)) {
139139
$listener($event);
140140
} else {
141-
@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);
141+
@trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, extend "%s" instead.', \get_class($listener), AbstractListener::class), E_USER_DEPRECATED);
142142
$listener->handle($event);
143143
}
144144

0 commit comments

Comments
 (0)
0