bug #37031 [Security] Fixed PUBLIC_ACCESS in authenticated sessions (wouterj)

This PR was merged into the 5.1 branch.

Discussion
----------

[Security] Fixed PUBLIC_ACCESS in authenticated sessions

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Found while testing https://github.com/scheb/2fa/pull/8, sorry for not spotting it before the stable release 😞

Currently, authenticated users are denied access for pages that have `PUBLIC_ACCESS` set, as this attribute is only checked when no token was set. It should be checked for both cases.

Commits
-------

0ac530f460 Also check PUBLIC_ACCESS for authenticated tokens

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

@@ -95,11 +95,13 @@ class AccessListener extends AbstractListener
                 return;
             }
 
-            if ([self::PUBLIC_ACCESS] === $attributes) {
-                return;
+            if ([self::PUBLIC_ACCESS] !== $attributes) {
+                throw $this->createAccessDeniedException($request, $attributes);
             }
+        }
 
-            throw $this->createAccessDeniedException($request, $attributes);
+        if ([self::PUBLIC_ACCESS] === $attributes) {
+            return;
         }
 
         if (!$token->isAuthenticated()) {

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

@@ -18,8 +18,10 @@ use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
+use Symfony\Component\Security\Core\User\User;
 use Symfony\Component\Security\Http\AccessMapInterface;
 use Symfony\Component\Security\Http\Event\LazyResponseEvent;
 use Symfony\Component\Security\Http\Firewall\AccessListener;
@@ -279,6 +281,33 @@ class AccessListenerTest extends TestCase
         $this->expectNotToPerformAssertions();
     }
 
+    public function testHandleWhenPublicAccessWhileAuthenticated()
+    {
+        $token = new UsernamePasswordToken(new User('Wouter', null, ['ROLE_USER']), null, 'main', ['ROLE_USER']);
+        $tokenStorage = new TokenStorage();
+        $tokenStorage->setToken($token);
+        $request = new Request();
+
+        $accessMap = $this->createMock(AccessMapInterface::class);
+        $accessMap->expects($this->any())
+            ->method('getPatterns')
+            ->with($this->equalTo($request))
+            ->willReturn([[AccessListener::PUBLIC_ACCESS], null])
+        ;
+
+        $listener = new AccessListener(
+            $tokenStorage,
+            $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock(),
+            $accessMap,
+            $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
+            false
+        );
+
+        $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
+
+        $this->expectNotToPerformAssertions();
+    }
+
     public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
     {
         $request = new Request();