Skip to content

Navigation Menu

Sign in
Appearance settings

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 fd0dc96

Browse filesBrowse files
wouterjwouterj
committed
bug #42596 [Security] Fix wrong cache directive when using the new PUBLIC_ACCESS attribute (wouterj)
This PR was squashed before being merged into the 5.3 branch. Discussion ---------- [Security] Fix wrong cache directive when using the new PUBLIC_ACCESS attribute | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Ref #41613 (comment) | License | MIT | Doc PR | - `PUBLIC_ACCESS` is the new `IS_AUTHENTICATED_ANONYMOUSLY` since 5.2, but we didn't correctly check for this causing a private cache directive for a stateless page. This PR also includes 2 changes from #42595 that could be backported to 5.3 Commits ------- ca80ee3 [Security] Fix wrong cache directive when using the new PUBLIC_ACCESS attribute
2 parents c5565a8 + ca80ee3 commit fd0dc96
Copy full SHA for fd0dc96

File tree

5 files changed

+48
-4
lines changed
Filter options

5 files changed

+48
-4
lines changed

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/RememberMeBundle/Security/UserChangingUserProvider.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/RememberMeBundle/Security/UserChangingUserProvider.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\RememberMeBundle\Security;
1313

14+
use Symfony\Component\Security\Core\User\InMemoryUser;
1415
use Symfony\Component\Security\Core\User\InMemoryUserProvider;
1516
use Symfony\Component\Security\Core\User\User;
1617
use Symfony\Component\Security\Core\User\UserInterface;
@@ -39,7 +40,7 @@ public function refreshUser(UserInterface $user)
3940
{
4041
$user = $this->inner->refreshUser($user);
4142

42-
$alterUser = \Closure::bind(function (User $user) { $user->password = 'foo'; }, null, User::class);
43+
$alterUser = \Closure::bind(function (InMemoryUser $user) { $user->password = 'foo'; }, null, class_exists(User::class) ? User::class : InMemoryUser::class);
4344
$alterUser($user);
4445

4546
return $user;

‎src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function hashPassword($user, string $plainPassword): string
5050
} elseif ($user instanceof UserInterface) {
5151
$salt = $user->getSalt();
5252

53-
if (null !== $salt) {
53+
if ($salt) {
5454
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
5555
}
5656
}

‎src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public function testNeedsRehash()
158158

159159
$passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory);
160160

161-
\Closure::bind(function () use ($passwordHasher) { $this->password = $passwordHasher->hashPassword($this, 'foo', 'salt'); }, $user, User::class)();
161+
\Closure::bind(function () use ($passwordHasher) { $this->password = $passwordHasher->hashPassword($this, 'foo', 'salt'); }, $user, class_exists(User::class) ? User::class : InMemoryUser::class)();
162162
$this->assertFalse($passwordHasher->needsRehash($user));
163163
$this->assertTrue($passwordHasher->needsRehash($user));
164164
$this->assertFalse($passwordHasher->needsRehash($user));

‎src/Symfony/Component/Security/Http/Firewall/AccessListener.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Firewall/AccessListener.php
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,13 @@ public function authenticate(RequestEvent $event)
7575
$attributes = $request->attributes->get('_access_control_attributes');
7676
$request->attributes->remove('_access_control_attributes');
7777

78-
if (!$attributes || ([AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] === $attributes && $event instanceof LazyResponseEvent)) {
78+
if (
79+
!$attributes
80+
|| (
81+
([AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] === $attributes || [AuthenticatedVoter::PUBLIC_ACCESS] === $attributes)
82+
&& $event instanceof LazyResponseEvent
83+
)
84+
) {
7985
return;
8086
}
8187

‎src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,4 +360,41 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
360360

361361
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST));
362362
}
363+
364+
public function testLazyPublicPagesShouldNotAccessTokenStorage()
365+
{
366+
$tokenStorage = $this->createMock(TokenStorageInterface::class);
367+
$tokenStorage->expects($this->never())->method('getToken');
368+
369+
$request = new Request();
370+
$accessMap = $this->createMock(AccessMapInterface::class);
371+
$accessMap->expects($this->any())
372+
->method('getPatterns')
373+
->with($this->equalTo($request))
374+
->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null])
375+
;
376+
377+
$listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, $this->createMock(AuthenticationManagerInterface::class), false);
378+
$listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)));
379+
}
380+
381+
/**
382+
* @group legacy
383+
*/
384+
public function testLegacyLazyPublicPagesShouldNotAccessTokenStorage()
385+
{
386+
$tokenStorage = $this->createMock(TokenStorageInterface::class);
387+
$tokenStorage->expects($this->never())->method('getToken');
388+
389+
$request = new Request();
390+
$accessMap = $this->createMock(AccessMapInterface::class);
391+
$accessMap->expects($this->any())
392+
->method('getPatterns')
393+
->with($this->equalTo($request))
394+
->willReturn([[AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY], null])
395+
;
396+
397+
$listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, $this->createMock(AuthenticationManagerInterface::class), false);
398+
$listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)));
399+
}
363400
}

0 commit comments

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