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

0 commit comments

Comments
 (0)