Skip to content

Commit 541085e

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

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

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1717
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1818
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
19+
use Symfony\Component\Security\Http\RequestSupport;
1920

2021
/**
2122
* The interface for all authenticators.
@@ -32,8 +33,10 @@ interface AuthenticatorInterface
3233
* If this returns true, authenticate() will be called. If false, the authenticator will be skipped.
3334
*
3435
* Returning null means authenticate() can be called lazily when accessing the token storage.
36+
*
37+
* @param RequestSupport|null $requestSupport
3538
*/
36-
public function supports(Request $request): ?bool;
39+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): ?bool;
3740

3841
/**
3942
* Create a passport for the current request.

‎src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticator.php

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2222
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
2323
use Symfony\Component\Security\Http\EntryPoint\Exception\NotAnEntryPointException;
24+
use Symfony\Component\Security\Http\RequestSupport;
2425
use Symfony\Component\VarDumper\Caster\ClassStub;
2526

2627
/**
@@ -31,6 +32,7 @@
3132
final class TraceableAuthenticator implements AuthenticatorInterface, InteractiveAuthenticatorInterface, AuthenticationEntryPointInterface
3233
{
3334
private ?bool $supports = false;
35+
private array $supportReasons = [];
3436
private ?Passport $passport = null;
3537
private ?float $duration = null;
3638
private ClassStub|string $stub;
@@ -45,6 +47,7 @@ public function getInfo(): array
4547
{
4648
return [
4749
'supports' => $this->supports,
50+
'supportReasons' => $this->supportReasons,
4851
'passport' => $this->passport,
4952
'duration' => $this->duration,
5053
'stub' => $this->stub ??= class_exists(ClassStub::class) ? new ClassStub($this->authenticator::class) : $this->authenticator::class,
@@ -62,9 +65,24 @@ static function (BadgeInterface $badge): array {
6265
];
6366
}
6467

65-
public function supports(Request $request): ?bool
68+
/**
69+
* @param RequestSupport|null $requestSupport
70+
*/
71+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): ?bool
6672
{
67-
return $this->supports = $this->authenticator->supports($request);
73+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : new RequestSupport();
74+
75+
$this->supports = $this->authenticator->supports($request, $requestSupport);
76+
$this->supportReasons = $requestSupport->reasons;
77+
78+
if ($this->supports === false) {
79+
$requestSupport->result = false;
80+
} else {
81+
$requestSupport->result = true;
82+
$requestSupport->lazy = null === $this->supports;
83+
}
84+
85+
return $this->supports;
6886
}
6987

7088
public function authenticate(Request $request): Passport

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
3232
use Symfony\Component\Security\Http\HttpUtils;
3333
use Symfony\Component\Security\Http\ParameterBagUtils;
34+
use Symfony\Component\Security\Http\RequestSupport;
3435
use Symfony\Component\Security\Http\SecurityRequestAttributes;
3536

3637
/**
@@ -68,11 +69,32 @@ protected function getLoginUrl(Request $request): string
6869
return $this->httpUtils->generateUri($request, $this->options['login_path']);
6970
}
7071

71-
public function supports(Request $request): bool
72+
/**
73+
* @param RequestSupport|null $requestSupport
74+
*/
75+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): bool
7276
{
73-
return ($this->options['post_only'] ? $request->isMethod('POST') : true)
74-
&& $this->httpUtils->checkRequestPath($request, $this->options['check_path'])
75-
&& ($this->options['form_only'] ? 'form' === $request->getContentTypeFormat() : true);
77+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : null;
78+
79+
if ($this->options['post_only'] && !$request->isMethod('POST')) {
80+
$requestSupport?->addReason('Request is not a POST while "post_only" is set.');
81+
82+
return false;
83+
}
84+
85+
if (!$this->httpUtils->checkRequestPath($request, $this->options['check_path'])) {
86+
$requestSupport?->addReason('Request does not match the "check_path".');
87+
88+
return false;
89+
}
90+
91+
if ($this->options['form_only'] && 'form' !== $request->getContentTypeFormat()) {
92+
$requestSupport?->addReason('Request is not a form submission while "form_only" is set.');
93+
94+
return false;
95+
}
96+
97+
return true;
7698
}
7799

78100
public function authenticate(Request $request): Passport

‎src/Symfony/Component/Security/Http/Authenticator/HttpBasicAuthenticator.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\Credentials\PasswordCredentials;
2525
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
2626
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
27+
use Symfony\Component\Security\Http\RequestSupport;
2728

2829
/**
2930
* @author Wouter de Jong <wouter@wouterj.nl>
@@ -49,9 +50,20 @@ public function start(Request $request, ?AuthenticationException $authException
4950
return $response;
5051
}
5152

52-
public function supports(Request $request): ?bool
53+
/**
54+
* @param RequestSupport|null $requestSupport
55+
*/
56+
public function supports(Request $request, /* ?RequestSupport $requestSupport = null */): ?bool
5357
{
54-
return $request->headers->has('PHP_AUTH_USER');
58+
$requestSupport = 2 <= \func_num_args() ? func_get_arg(1) : null;
59+
60+
if (!$request->headers->has('PHP_AUTH_USER')) {
61+
$requestSupport?->addReason('Request has no basic authentication header.');
62+
63+
return false;
64+
}
65+
66+
return true;
5567
}
5668

5769
public function authenticate(Request $request): Passport

0 commit comments

Comments
 (0)