bug #30423 [Security] Rework firewall's access denied rule (dimabory)

This PR was merged into the 3.4 branch. Discussion ---------- [Security] Rework firewall's access denied rule | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~~#30099~~, #28229 | License | MIT | Doc PR | Follow tickets provided above to reproduce bugs. (there are also some project examples) ~~In addition, I'm looking for someone who knows an answer to [this](#30099 (comment)) regarding rework in this PR.~~ Commits ------- 5790859 Rework firewall access denied rule
symfony · Apr 10, 2019 · fd1408b · fd1408b
2 parents b93d2bf + 5790859
commit fd1408b
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 10 deletions.
diff --git a/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php b/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php
@@ -131,8 +131,6 @@ private function handleAccessDeniedException(GetResponseForExceptionEvent $event
             } catch (\Exception $e) {
                 $event->setException($e);
             }
-
-            return;
         }
 
         if (null !== $this->logger) {
@@ -150,7 +148,7 @@ private function handleAccessDeniedException(GetResponseForExceptionEvent $event
                 $subRequest = $this->httpUtils->createRequest($event->getRequest(), $this->errorPage);
                 $subRequest->attributes->set(Security::ACCESS_DENIED_ERROR, $exception);
 
-                $event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
+                $event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST));
                 $event->allowCustomResponseCode();
             }
         } catch (\Exception $e) {

diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php
@@ -130,10 +130,8 @@ public function testAccessDeniedExceptionFullFledgedAndWithAccessDeniedHandlerAn
     {
         $event = $this->createEvent($exception);
 
-        $accessDeniedHandler = $this->getMockBuilder('Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface')->getMock();
-        $accessDeniedHandler->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
+        $listener = $this->createExceptionListener(null, $this->createTrustResolver(true), null, null, null, $this->createCustomAccessDeniedHandler(new Response('error')));
 
-        $listener = $this->createExceptionListener(null, $this->createTrustResolver(true), null, null, null, $accessDeniedHandler);
         $listener->onKernelException($event);
 
         $this->assertEquals('error', $event->getResponse()->getContent());
@@ -147,16 +145,51 @@ public function testAccessDeniedExceptionNotFullFledged(\Exception $exception, \
     {
         $event = $this->createEvent($exception);
 
-        $tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
-        $tokenStorage->expects($this->once())->method('getToken')->will($this->returnValue($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
-
-        $listener = $this->createExceptionListener($tokenStorage, $this->createTrustResolver(false), null, $this->createEntryPoint());
+        $listener = $this->createExceptionListener($this->createTokenStorage(), $this->createTrustResolver(false), null, $this->createEntryPoint());
         $listener->onKernelException($event);
 
         $this->assertEquals('OK', $event->getResponse()->getContent());
         $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
     }
 
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionNotFullFledgedAndWithAccessDeniedHandlerAndWithoutErrorPage(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception);
+
+        $listener = $this->createExceptionListener($this->createTokenStorage(), $this->createTrustResolver(false), null, $this->createEntryPoint(), null, $this->createCustomAccessDeniedHandler(new Response('denied', 403)));
+        $listener->onKernelException($event);
+
+        $this->assertEquals('denied', $event->getResponse()->getContent());
+        $this->assertEquals(403, $event->getResponse()->getStatusCode());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionNotFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
+    {
+        $kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock();
+        $kernel->expects($this->once())->method('handle')->will($this->returnValue(new Response('Unauthorized', 401)));
+
+        $event = $this->createEvent($exception, $kernel);
+
+        $httpUtils = $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils')->getMock();
+        $httpUtils->expects($this->once())->method('createRequest')->will($this->returnValue(Request::create('/error')));
+
+        $listener = $this->createExceptionListener($this->createTokenStorage(), $this->createTrustResolver(true), $httpUtils, null, '/error');
+        $listener->onKernelException($event);
+
+        $this->assertTrue($event->isAllowingCustomResponseCode());
+
+        $this->assertEquals('Unauthorized', $event->getResponse()->getContent());
+        $this->assertEquals(401, $event->getResponse()->getStatusCode());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
     public function getAccessDeniedExceptionProvider()
     {
         return [
@@ -168,6 +201,22 @@ public function getAccessDeniedExceptionProvider()
         ];
     }
 
+    private function createTokenStorage()
+    {
+        $tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
+        $tokenStorage->expects($this->once())->method('getToken')->will($this->returnValue($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
+
+        return $tokenStorage;
+    }
+
+    private function createCustomAccessDeniedHandler(Response $response)
+    {
+        $accessDeniedHandler = $this->getMockBuilder('Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface')->getMock();
+        $accessDeniedHandler->expects($this->once())->method('handle')->will($this->returnValue($response));
+
+        return $accessDeniedHandler;
+    }
+
     private function createEntryPoint(Response $response = null)
     {
         $entryPoint = $this->getMockBuilder('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface')->getMock();