Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0fc8f7a

Browse filesBrowse files
fabpotfabpot
committed
bug #45331 [Security]  Fix wrong authenticator class in debug logs (chalasr)
This PR was merged into the 5.4 branch. Discussion ---------- [Security]  Fix wrong authenticator class in debug logs | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Before: <img width="1034" alt="Screenshot 2022-02-07 at 02 00 25" src="https://user-images.githubusercontent.com/7502063/152709794-4bc64ae0-caa9-4e28-b60e-6acff8070248.png"> After: <img width="1033" alt="Screenshot 2022-02-07 at 02 01 08" src="https://user-images.githubusercontent.com/7502063/152709801-2c84ed0c-3bce-4bb0-98ee-bad309f29c96.png"> Commits ------- 49352e0 [Security] Fix wrong authenticator class in debug logs
2 parents 2e32f2f + 49352e0 commit 0fc8f7a
Copy full SHA for 0fc8f7a

File tree

3 files changed

+58
-8
lines changed
Filter options

3 files changed

+58
-8
lines changed

‎src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php
+7-6Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
2626
use Symfony\Component\Security\Core\User\UserInterface;
2727
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
28+
use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
2829
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
2930
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\BadgeInterface;
3031
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
@@ -151,7 +152,7 @@ private function executeAuthenticators(array $authenticators, Request $request):
151152
// lazily (after initialization).
152153
if (false === $authenticator->supports($request)) {
153154
if (null !== $this->logger) {
154-
$this->logger->debug('Skipping the "{authenticator}" authenticator as it did not support the request.', ['authenticator' => \get_class($authenticator)]);
155+
$this->logger->debug('Skipping the "{authenticator}" authenticator as it did not support the request.', ['authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
155156
}
156157

157158
continue;
@@ -160,7 +161,7 @@ private function executeAuthenticators(array $authenticators, Request $request):
160161
$response = $this->executeAuthenticator($authenticator, $request);
161162
if (null !== $response) {
162163
if (null !== $this->logger) {
163-
$this->logger->debug('The "{authenticator}" authenticator set the response. Any later authenticator will not be called', ['authenticator' => \get_class($authenticator)]);
164+
$this->logger->debug('The "{authenticator}" authenticator set the response. Any later authenticator will not be called', ['authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
164165
}
165166

166167
return $response;
@@ -210,7 +211,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
210211
$this->eventDispatcher->dispatch(new AuthenticationSuccessEvent($authenticatedToken), AuthenticationEvents::AUTHENTICATION_SUCCESS);
211212

212213
if (null !== $this->logger) {
213-
$this->logger->info('Authenticator successful!', ['token' => $authenticatedToken, 'authenticator' => \get_class($authenticator)]);
214+
$this->logger->info('Authenticator successful!', ['token' => $authenticatedToken, 'authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
214215
}
215216
} catch (AuthenticationException $e) {
216217
// oh no! Authentication failed!
@@ -229,7 +230,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
229230
}
230231

231232
if (null !== $this->logger) {
232-
$this->logger->debug('Authenticator set no success response: request continues.', ['authenticator' => \get_class($authenticator)]);
233+
$this->logger->debug('Authenticator set no success response: request continues.', ['authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
233234
}
234235

235236
return null;
@@ -262,7 +263,7 @@ private function handleAuthenticationSuccess(TokenInterface $authenticatedToken,
262263
private function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, AuthenticatorInterface $authenticator, ?PassportInterface $passport): ?Response
263264
{
264265
if (null !== $this->logger) {
265-
$this->logger->info('Authenticator failed.', ['exception' => $authenticationException, 'authenticator' => \get_class($authenticator)]);
266+
$this->logger->info('Authenticator failed.', ['exception' => $authenticationException, 'authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
266267
}
267268

268269
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
@@ -273,7 +274,7 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
273274

274275
$response = $authenticator->onAuthenticationFailure($request, $authenticationException);
275276
if (null !== $response && null !== $this->logger) {
276-
$this->logger->debug('The "{authenticator}" authenticator set the failure response.', ['authenticator' => \get_class($authenticator)]);
277+
$this->logger->debug('The "{authenticator}" authenticator set the failure response.', ['authenticator' => \get_class($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)]);
277278
}
278279

279280
$this->eventDispatcher->dispatch($loginFailureEvent = new LoginFailureEvent($authenticationException, $authenticator, $request, $response, $this->firewallName, $passport));

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticator.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,14 @@ public function isInteractive(): bool
100100
return $this->authenticator instanceof InteractiveAuthenticatorInterface && $this->authenticator->isInteractive();
101101
}
102102

103+
/**
104+
* @internal
105+
*/
106+
public function getAuthenticator(): AuthenticatorInterface
107+
{
108+
return $this->authenticator;
109+
}
110+
103111
public function __call($method, $args)
104112
{
105113
return $this->authenticator->{$method}(...$args);

‎src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php
+43-2Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\Security\Http\Tests\Authentication;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Psr\Log\AbstractLogger;
16+
use Psr\Log\LoggerInterface;
1517
use Symfony\Component\EventDispatcher\EventDispatcher;
1618
use Symfony\Component\HttpFoundation\Request;
1719
use Symfony\Component\HttpFoundation\Response;
@@ -22,6 +24,7 @@
2224
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
2325
use Symfony\Component\Security\Core\User\InMemoryUser;
2426
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
27+
use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
2528
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
2629
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge;
2730
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
@@ -310,6 +313,44 @@ public function testAuthenticateRequestHidesInvalidUserExceptions()
310313
$this->assertSame($this->response, $response);
311314
}
312315

316+
public function testLogsUseTheDecoratedAuthenticatorWhenItIsTraceable()
317+
{
318+
$authenticator = $this->createMock(TestInteractiveAuthenticator::class);
319+
$authenticator->expects($this->any())->method('isInteractive')->willReturn(true);
320+
$this->request->attributes->set('_security_authenticators', [new TraceableAuthenticator($authenticator)]);
321+
322+
$authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', function () { return $this->user; })));
323+
$authenticator->expects($this->any())->method('createToken')->willReturn($this->token);
324+
325+
$this->tokenStorage->expects($this->once())->method('setToken')->with($this->token);
326+
327+
$authenticator->expects($this->any())
328+
->method('onAuthenticationSuccess')
329+
->with($this->anything(), $this->token, 'main')
330+
->willReturn($this->response);
331+
332+
$authenticator->expects($this->any())
333+
->method('onAuthenticationSuccess')
334+
->with($this->anything(), $this->token, 'main')
335+
->willReturn($this->response);
336+
337+
$logger = new class() extends AbstractLogger {
338+
public $logContexts = [];
339+
340+
public function log($level, $message, array $context = []): void
341+
{
342+
if ($context['authenticator'] ?? false) {
343+
$this->logContexts[] = $context;
344+
}
345+
}
346+
};
347+
348+
$manager = $this->createManager([$authenticator], 'main', true, [], $logger);
349+
$response = $manager->authenticateRequest($this->request);
350+
$this->assertSame($this->response, $response);
351+
$this->assertStringContainsString('Mock_TestInteractiveAuthenticator', $logger->logContexts[0]['authenticator']);
352+
}
353+
313354
private function createAuthenticator($supports = true)
314355
{
315356
$authenticator = $this->createMock(TestInteractiveAuthenticator::class);
@@ -318,9 +359,9 @@ private function createAuthenticator($supports = true)
318359
return $authenticator;
319360
}
320361

321-
private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [])
362+
private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], LoggerInterface $logger = null)
322363
{
323-
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, null, $eraseCredentials, true, $requiredBadges);
364+
return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, true, $requiredBadges);
324365
}
325366
}
326367

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.