Skip to content

Commit 0afb37c

Browse files
committed
bug #51350 [Security] Prevent creating session in stateless firewalls (Seb33300)
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Security] Prevent creating session in stateless firewalls | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix symfony/symfony#51319 | License | MIT | Doc PR | <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> Please check related issue for details. Same as symfony/symfony#51320 with `@chalasr` suggestion: symfony/symfony#51320 (comment) Commits ------- 4efd50e34c [Security] Prevent creating session in stateless firewalls
2 parents 16f232d + e2af19c commit 0afb37c

File tree

4 files changed

+35
-2
lines changed

4 files changed

+35
-2
lines changed

Authentication/DefaultAuthenticationFailureHandler.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio
9191

9292
$this->logger?->debug('Authentication failure, redirect triggered.', ['failure_path' => $options['failure_path']]);
9393

94-
$request->getSession()->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, $exception);
94+
if (!$request->attributes->getBoolean('_stateless')) {
95+
$request->getSession()->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, $exception);
96+
}
9597

9698
return $this->httpUtils->createRedirectResponse($request, $options['failure_path']);
9799
}

Authentication/DefaultAuthenticationSuccessHandler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ protected function determineTargetUrl(Request $request): string
103103
}
104104

105105
$firewallName = $this->getFirewallName();
106-
if (null !== $firewallName && $targetUrl = $this->getTargetPath($request->getSession(), $firewallName)) {
106+
if (null !== $firewallName && !$request->attributes->getBoolean('_stateless') && $targetUrl = $this->getTargetPath($request->getSession(), $firewallName)) {
107107
$this->removeTargetPath($request->getSession(), $firewallName);
108108

109109
return $targetUrl;

Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ protected function setUp(): void
4646

4747
$this->session = $this->createMock(SessionInterface::class);
4848
$this->request = $this->createMock(Request::class);
49+
$this->request->attributes = new ParameterBag(['_stateless' => false]);
4950
$this->request->expects($this->any())->method('getSession')->willReturn($this->session);
5051
$this->exception = $this->getMockBuilder(AuthenticationException::class)->onlyMethods(['getMessage'])->getMock();
5152
}
@@ -89,6 +90,17 @@ public function testExceptionIsPersistedInSession()
8990
$handler->onAuthenticationFailure($this->request, $this->exception);
9091
}
9192

93+
public function testExceptionIsNotPersistedInSessionOnStatelessRequest()
94+
{
95+
$this->request->attributes = new ParameterBag(['_stateless' => true]);
96+
97+
$this->session->expects($this->never())
98+
->method('set')->with(SecurityRequestAttributes::AUTHENTICATION_ERROR, $this->exception);
99+
100+
$handler = new DefaultAuthenticationFailureHandler($this->httpKernel, $this->httpUtils, [], $this->logger);
101+
$handler->onAuthenticationFailure($this->request, $this->exception);
102+
}
103+
92104
public function testExceptionIsPassedInRequestOnForward()
93105
{
94106
$options = ['failure_forward' => true];

Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,25 @@ public function testRequestRedirectionsWithTargetPathInSessions()
5656
$this->assertSame('http://localhost/admin/dashboard', $handler->onAuthenticationSuccess($requestWithSession, $token)->getTargetUrl());
5757
}
5858

59+
public function testStatelessRequestRedirections()
60+
{
61+
$session = $this->createMock(SessionInterface::class);
62+
$session->expects($this->never())->method('get')->with('_security.admin.target_path');
63+
$session->expects($this->never())->method('remove')->with('_security.admin.target_path');
64+
$statelessRequest = Request::create('/');
65+
$statelessRequest->setSession($session);
66+
$statelessRequest->attributes->set('_stateless', true);
67+
68+
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
69+
$urlGenerator->expects($this->any())->method('generate')->willReturn('http://localhost/login');
70+
$httpUtils = new HttpUtils($urlGenerator);
71+
$token = $this->createMock(TokenInterface::class);
72+
$handler = new DefaultAuthenticationSuccessHandler($httpUtils);
73+
$handler->setFirewallName('admin');
74+
75+
$this->assertSame('http://localhost/', $handler->onAuthenticationSuccess($statelessRequest, $token)->getTargetUrl());
76+
}
77+
5978
public static function getRequestRedirections()
6079
{
6180
return [

0 commit comments

Comments
 (0)