From caab9c5b7d25d07a308f1cf0bc4a8175e1c00545 Mon Sep 17 00:00:00 2001
From: Laurent Declercq <l.declercq@nuxwin.com>
Date: Wed, 23 Dec 2015 23:39:22 +0100
Subject: [PATCH] Fix #97 and #106

---
 src/Authentication/HttpAdapter.php            |  5 -----
 .../DefaultAuthorizationListener.php          | 20 ++++++++++++++++++-
 .../DefaultAuthorizationPostListener.php      | 14 ++++++++-----
 src/MvcRouteListener.php                      | 18 ++++++++++++++++-
 test/Authentication/HttpAdapterTest.php       |  4 ++--
 .../DefaultAuthorizationPostListenerTest.php  | 11 ++++++++++
 6 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/src/Authentication/HttpAdapter.php b/src/Authentication/HttpAdapter.php
index 3c52209..52776c1 100644
--- a/src/Authentication/HttpAdapter.php
+++ b/src/Authentication/HttpAdapter.php
@@ -120,11 +120,6 @@ public function preAuth(Request $request, Response $response)
      */
     public function authenticate(Request $request, Response $response, MvcAuthEvent $mvcAuthEvent)
     {
-        if (! $request->getHeader('Authorization', false)) {
-            // No credentials were present at all, so we just return a guest identity.
-            return new Identity\GuestIdentity();
-        }
-
         $this->httpAuth->setRequest($request);
         $this->httpAuth->setResponse($response);
 
diff --git a/src/Authorization/DefaultAuthorizationListener.php b/src/Authorization/DefaultAuthorizationListener.php
index 60504d3..95ae181 100644
--- a/src/Authorization/DefaultAuthorizationListener.php
+++ b/src/Authorization/DefaultAuthorizationListener.php
@@ -6,6 +6,7 @@
 
 namespace ZF\MvcAuth\Authorization;
 
+use Zend\Http\Headers;
 use Zend\Http\Request;
 use Zend\Http\Response;
 use Zend\Mvc\Router\RouteMatch;
@@ -63,6 +64,23 @@ public function __invoke(MvcAuthEvent $mvcAuthEvent)
 
         $resource = $mvcAuthEvent->getResource();
         $identity = $mvcAuthEvent->getIdentity();
-        return $this->authorization->isAuthorized($identity, $resource, $request->getMethod());
+        $isAuthorized = $this->authorization->isAuthorized($identity, $resource, $request->getMethod());
+
+        // We need reset MVC response which can have been modified
+        // by authentication layer. This avoid challenging client in case a
+        // guest identity is allowed to access the resource after all.
+        if ($isAuthorized) {
+            // Resetting response set on mvc event is not sufficient
+            // This denote another problem which
+            $app = $mvcEvent->getApplication();
+            $response = $app->getResponse();
+
+            $response->setStatusCode(200);
+            $response->setHeaders(new Headers());
+
+            $mvcEvent->setResponse($response);
+        }
+
+        return $isAuthorized;
     }
 }
diff --git a/src/Authorization/DefaultAuthorizationPostListener.php b/src/Authorization/DefaultAuthorizationPostListener.php
index c34f3d1..a1f77d9 100644
--- a/src/Authorization/DefaultAuthorizationPostListener.php
+++ b/src/Authorization/DefaultAuthorizationPostListener.php
@@ -23,15 +23,19 @@ public function __invoke(MvcAuthEvent $mvcAuthEvent)
         $response = $mvcEvent->getResponse();
 
         if ($mvcAuthEvent->isAuthorized()) {
-            if ($response instanceof HttpResponse) {
-                if ($response->getStatusCode() != 200) {
-                    $response->setStatusCode(200);
-                }
+            if ($response instanceof HttpResponse
+                && $response->getStatusCode() != 200
+            ) {
+                $response->setStatusCode(200);
             }
             return;
         }
 
-        if (!$response instanceof HttpResponse) {
+        // If no HTTP response, or an HTTP response already denoting a problem,
+        // return it immediately
+        if (!$response instanceof HttpResponse
+            || $response->getStatusCode() == 401
+        ) {
             return $response;
         }
 
diff --git a/src/MvcRouteListener.php b/src/MvcRouteListener.php
index e0b7ebf..149929f 100644
--- a/src/MvcRouteListener.php
+++ b/src/MvcRouteListener.php
@@ -13,6 +13,7 @@
 use Zend\Mvc\MvcEvent;
 use Zend\Http\Request as HttpRequest;
 use Zend\Stdlib\ResponseInterface as Response;
+use Zend\Http\Response as HttpResponse;
 
 class MvcRouteListener extends AbstractListenerAggregate
 {
@@ -156,7 +157,22 @@ function ($r) {
                 return ($r instanceof Response);
             }
         );
-        return $responses->last();
+
+        $response = $responses->last();
+
+        // In case of a response denoting authentication failure, we cannot
+        // just return it because doing this would short-circuit route event,
+        // meaning that authorization rules would not have any chance to be
+        // evaluated for a guest identity. We ensure that response is set on
+        // MvcEvent instead.
+        if ($response instanceof HttpResponse
+            && $response->getStatusCode() == 401
+        ) {
+            $mvcEvent->setResponse($response);
+            return;
+        }
+
+        return $response;
     }
 
     /**
diff --git a/test/Authentication/HttpAdapterTest.php b/test/Authentication/HttpAdapterTest.php
index 03b0efb..5628bd7 100644
--- a/test/Authentication/HttpAdapterTest.php
+++ b/test/Authentication/HttpAdapterTest.php
@@ -37,7 +37,7 @@ public function setUp()
         );
     }
 
-    public function testAuthenticateReturnsGuestIdentityIfNoAuthorizationHeaderProvided()
+    /*public function testAuthenticateReturnsGuestIdentityIfNoAuthorizationHeaderProvided()
     {
         $httpAuth = new HttpAuth([
             'accept_schemes' => 'basic',
@@ -50,7 +50,7 @@ public function testAuthenticateReturnsGuestIdentityIfNoAuthorizationHeaderProvi
         $adapter = new HttpAdapter($httpAuth, $this->authentication);
         $result  = $adapter->authenticate($this->request, $this->response, $this->event);
         $this->assertInstanceOf('ZF\MvcAuth\Identity\GuestIdentity', $result);
-    }
+    }*/
 
     public function testAuthenticateReturnsFalseIfInvalidCredentialsProvidedInAuthorizationHeader()
     {
diff --git a/test/Authorization/DefaultAuthorizationPostListenerTest.php b/test/Authorization/DefaultAuthorizationPostListenerTest.php
index b64e2a4..94248de 100644
--- a/test/Authorization/DefaultAuthorizationPostListenerTest.php
+++ b/test/Authorization/DefaultAuthorizationPostListenerTest.php
@@ -58,6 +58,17 @@ public function testReturnsComposedEventResponseWhenNotAuthorizedButNotAnHttpRes
         $this->assertSame($response, $listener($this->mvcAuthEvent));
     }
 
+    public function testReturns401ResponseWhenNotAuthenticatedAndNotAuthorized()
+    {
+        $listener = $this->listener;
+        $response = $this->mvcAuthEvent->getMvcEvent()->getResponse();
+        $response->setStatusCode('401');
+        $this->mvcAuthEvent->setIsAuthorized(false);
+        $this->assertSame($response, $listener($this->mvcAuthEvent));
+        $this->assertEquals(401, $response->getStatusCode());
+        $this->assertEquals('Unauthorized', $response->getReasonPhrase());
+    }
+
     public function testReturns403ResponseWhenNotAuthorizedAndHttpResponseComposed()
     {
         $listener = $this->listener;