Skip to content

Commit 66af8a6

Browse files
MatTheCatMatTheCat
committed
[Security] Add ability for authenticators to explain why they didn't support a request
1 parent d92477a commit 66af8a6

22 files changed

+233
-30
lines changed

‎UPGRADE-7.4.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
UPGRADE FROM 7.3 to 7.4
2+
=======================
3+
4+
Symfony 7.4 is a minor release. According to the Symfony release process, there should be no significant
5+
backward compatibility breaks. Minor backward compatibility breaks are prefixed in this document with
6+
`[BC BREAK]`, make sure your code is compatible with these entries before upgrading.
7+
Read more about this in the [Symfony documentation](https://symfony.com/doc/7.4/setup/upgrade_minor.html).
8+
9+
If you're upgrading from a version below 7.3, follow the [7.3 upgrade guide](UPGRADE-7.3.md) first.
10+
11+
Security
12+
--------
13+
14+
* Add argument `$requestSupport` to `AuthenticatorInterface::supports()`;
15+
it should be used to report the reason a request isn't supported. E.g:
16+
17+
```php
18+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
19+
{
20+
$requestSupport?->addReason('This authenticator does not support any request.');
21+
22+
return false;
23+
}
24+
```

‎src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,9 @@
423423
<div id="authenticator-{{ i }}" class="font-normal">
424424
{% if authenticator.supports is same as(false) %}
425425
<div class="empty">
426-
<p>This authenticator did not support the request.</p>
426+
{% for reason in (authenticator.supportReasons ?? []) is empty ? ['This authenticator did not support the request.'] : authenticator.supportReasons %}
427+
<p>{{ reason }}</p>
428+
{% endfor %}
427429
</div>
428430
{% elseif authenticator.authenticated is null %}
429431
<div class="empty">

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/RegisterEntryPointsPassTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2929
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
3030
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
31+
use Symfony\Component\Security\Http\RequestSupport;
3132

3233
class RegisterEntryPointsPassTest extends TestCase
3334
{
@@ -71,7 +72,7 @@ public function testProcessResolvesChildDefinitionsClass()
7172

7273
class CustomAuthenticator extends AbstractAuthenticator implements AuthenticationEntryPointInterface
7374
{
74-
public function supports(Request $request): ?bool
75+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
7576
{
7677
return false;
7778
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
3939
use Symfony\Component\Security\Http\Authenticator\HttpBasicAuthenticator;
4040
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
41+
use Symfony\Component\Security\Http\RequestSupport;
4142

4243
class SecurityExtensionTest extends TestCase
4344
{
@@ -959,7 +960,7 @@ protected function getContainer()
959960

960961
class TestAuthenticator implements AuthenticatorInterface
961962
{
962-
public function supports(Request $request): ?bool
963+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
963964
{
964965
}
965966

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/AuthenticatorBundle/ApiAuthenticator.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2323
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2424
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
25+
use Symfony\Component\Security\Http\RequestSupport;
2526

2627
class ApiAuthenticator extends AbstractAuthenticator
2728
{
@@ -32,7 +33,7 @@ public function __construct(bool $selfLoadingUser = false)
3233
$this->selfLoadingUser = $selfLoadingUser;
3334
}
3435

35-
public function supports(Request $request): ?bool
36+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
3637
{
3738
return $request->headers->has('X-USER-EMAIL');
3839
}

‎src/Symfony/Component/Ldap/Security/LdapAuthenticator.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2121
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
2222
use Symfony\Component\Security\Http\EntryPoint\Exception\NotAnEntryPointException;
23+
use Symfony\Component\Security\Http\RequestSupport;
2324

2425
/**
2526
* This class decorates internal authenticators to add the LDAP integration.
@@ -44,9 +45,9 @@ public function __construct(
4445
) {
4546
}
4647

47-
public function supports(Request $request): ?bool
48+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
4849
{
49-
return $this->authenticator->supports($request);
50+
return $this->authenticator->supports($request, $requestSupport);
5051
}
5152

5253
public function authenticate(Request $request): Passport

‎src/Symfony/Component/Ldap/Tests/Security/CheckLdapCredentialsListenerTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
3434
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
3535
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
36+
use Symfony\Component\Security\Http\RequestSupport;
3637
use Symfony\Contracts\Service\ServiceLocatorTrait;
3738

3839
class CheckLdapCredentialsListenerTest extends TestCase
@@ -206,7 +207,7 @@ private function createListener()
206207
if (interface_exists(AuthenticatorInterface::class)) {
207208
class TestAuthenticator implements AuthenticatorInterface
208209
{
209-
public function supports(Request $request): ?bool
210+
public function supports(Request $request, ?RequestSupport $requestSupport = null): ?bool
210211
{
211212
}
212213

‎src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpFoundation\Response;
1717
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1818
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
19+
use Symfony\Component\Security\Http\RequestSupport;
1920
use Symfony\Component\Security\Http\SecurityRequestAttributes;
2021

2122
/**
@@ -36,10 +37,26 @@ abstract protected function getLoginUrl(Request $request): string;
3637
*
3738
* This default implementation handles all POST requests to the
3839
* login path (@see getLoginUrl()).
40+
*
41+
* @param RequestSupport|null $requestSupport
3942
*/
40-
public function supports(Request $request): bool
43+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): bool
4144
{
42-
return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getBaseUrl().$request->getPathInfo();
45+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : null;
46+
47+
if (!$request->isMethod('POST')) {
48+
$requestSupport?->addReason('Request is not a POST.');
49+
50+
return false;
51+
}
52+
53+
if ($this->getLoginUrl($request) !== $request->getBaseUrl().$request->getPathInfo()) {
54+
$requestSupport?->addReason('Request does not match the login URL.');
55+
56+
return false;
57+
}
58+
59+
return true;
4360
}
4461

4562
/**

‎src/Symfony/Component/Security/Http/Authenticator/AbstractPreAuthenticatedAuthenticator.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2525
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2626
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
27+
use Symfony\Component\Security\Http\RequestSupport;
2728

2829
/**
2930
* The base authenticator for authenticators to use pre-authenticated
@@ -52,20 +53,27 @@ public function __construct(
5253
*/
5354
abstract protected function extractUsername(Request $request): ?string;
5455

55-
public function supports(Request $request): ?bool
56+
/**
57+
* @param RequestSupport|null $requestSupport
58+
*/
59+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): ?bool
5660
{
61+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : null;
62+
5763
try {
5864
$username = $this->extractUsername($request);
5965
} catch (BadCredentialsException $e) {
6066
$this->clearToken($e);
6167

6268
$this->logger?->debug('Skipping pre-authenticated authenticator as a BadCredentialsException is thrown.', ['exception' => $e, 'authenticator' => static::class]);
69+
$requestSupport?->addReason('Could not find the username in the request.');
6370

6471
return false;
6572
}
6673

6774
if (null === $username) {
6875
$this->logger?->debug('Skipping pre-authenticated authenticator no username could be extracted.', ['authenticator' => static::class]);
76+
$requestSupport?->addReason('Username found in the request was empty.');
6977

7078
return false;
7179
}
@@ -75,6 +83,7 @@ public function supports(Request $request): ?bool
7583

7684
if ($token instanceof PreAuthenticatedToken && $this->firewallName === $token->getFirewallName() && $token->getUserIdentifier() === $username) {
7785
$this->logger?->debug('Skipping pre-authenticated authenticator as the user already has an existing session.', ['authenticator' => static::class]);
86+
$requestSupport?->addReason('A token already exists for the user in the session.');
7887

7988
return false;
8089
}

‎src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2525
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
2626
use Symfony\Component\Security\Http\Authenticator\Token\PostAuthenticationToken;
27+
use Symfony\Component\Security\Http\RequestSupport;
2728
use Symfony\Contracts\Translation\TranslatorInterface;
2829

2930
/**
@@ -46,9 +47,20 @@ public function __construct(
4647
) {
4748
}
4849

49-
public function supports(Request $request): ?bool
50+
/**
51+
* @param RequestSupport|null $requestSupport
52+
*/
53+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): ?bool
5054
{
51-
return null === $this->accessTokenExtractor->extractAccessToken($request) ? false : null;
55+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : null;
56+
57+
if (null === $this->accessTokenExtractor->extractAccessToken($request)) {
58+
$requestSupport?->addReason(sprintf('No token was found in the request by the %s.', $this->accessTokenExtractor::class));
59+
60+
return false;
61+
}
62+
63+
return null;
5264
}
5365

5466
public function authenticate(Request $request): Passport

0 commit comments

Comments
 (0)