[#83] Return a 401 for invalid OAuth2 credentials

`OAuth2\Server` returns boolean false for each of:

- no token present
- invalid and/or expired token present
- token invalid for current scope

The first case, is expected; it's what will happen in a public API when no
credentials are provided, and should result in marshaling a `GuestIdentity`.

The second two cases, however, should result in returning the appropriate status
code and headers. For this to happen, we must introspect the response composed
in the server:

- First, to see if we have a 401 or 403 status, and
- second, to see if an error has been reported to the response.

If both conditions occur, we merge the `OAuth2\Response` status and headers to
the MVC response and return it immediately.

Otherwise, we only merge any headers present (typically, the `WWW-Authenticate`
header, which will be a prompt for clients to know that authentication is
possible), and return a `GuestIdentity`.

composer.json

@@ -32,7 +32,7 @@
         "zfcampus/zf-oauth2": "^1.1.1"
     },
     "require-dev": {
-        "phpunit/PHPUnit": "3.7.*",
+        "phpunit/phpunit": "~4.7",
         "squizlabs/php_codesniffer": "^2.3",
         "zendframework/zend-loader": ">=2.3,<2.5",
         "zendframework/zend-session": ">=2.3,<2.5"

src/Authentication/OAuth2Adapter.php

@@ -7,6 +7,7 @@
 namespace ZF\MvcAuth\Authentication;
 
 use OAuth2\Request as OAuth2Request;
+use OAuth2\Response as OAuth2Response;
 use OAuth2\Server as OAuth2Server;
 use Zend\Http\Request;
 use Zend\Http\Response;
@@ -133,25 +134,72 @@ public function preAuth(Request $request, Response $response)
      */
     public function authenticate(Request $request, Response $response, MvcAuthEvent $mvcAuthEvent)
     {
-        $content       = $request->getContent();
         $oauth2request = new OAuth2Request(
-            $_GET,
-            $_POST,
+            $request->getQuery()->toArray(),
+            $request->getPost()->toArray(),
             array(),
-            $_COOKIE,
-            $_FILES,
-            $_SERVER,
-            $content,
+            ($request->getCookie() ? $request->getCookie()->getArrayCopy() : array()),
+            ($request->getFiles() ? $request->getFiles()->toArray() : array()),
+            (method_exists($request, 'getServer') ? $request->getServer()->toArray() : $_SERVER),
+            $request->getContent(),
             $request->getHeaders()->toArray()
         );
 
+        // Failure to validate
         if (! $this->oauth2Server->verifyResourceRequest($oauth2request)) {
-            return false;
+            $oauth2Response = $this->oauth2Server->getResponse();
+            $status = $oauth2Response->getStatusCode();
+
+            // 401 or 403 mean invalid credentials or unauthorized scopes; report those.
+            if (in_array($status, array(401, 403), true) && null !== $oauth2Response->getParameter('error')) {
+                return $this->mergeOAuth2Response($status, $response, $oauth2Response);
+            }
+
+            // Merge in any headers; typically sets a WWW-Authenticate header.
+            $this->mergeOAuth2ResponseHeaders($response, $oauth2Response->getHttpHeaders());
+
+            // Otherwise, no credentials were present at all, so we just return a guest identity.
+            return new Identity\GuestIdentity();
         }
 
         $token    = $this->oauth2Server->getAccessTokenData($oauth2request);
         $identity = new Identity\AuthenticatedIdentity($token);
         $identity->setName($token['user_id']);
         return $identity;
     }
+
+    /**
+     * Merge the OAuth2\Response instance's status and headers into the current Zend\Http\Response.
+     *
+     * @param int $status
+     * @param Response $response
+     * @param OAuth2Response $oauth2Response
+     * @return Response
+     */
+    private function mergeOAuth2Response($status, Response $response, OAuth2Response $oauth2Response)
+    {
+        $response->setStatusCode($status);
+        return $this->mergeOAuth2ResponseHeaders($response, $oauth2Response->getHttpHeaders());
+    }
+
+    /**
+     * Merge the OAuth2\Response headers into the current Zend\Http\Response.
+     *
+     * @param Response $response
+     * @param array $oauth2Headers
+     * @return Response
+     */
+    private function mergeOAuth2ResponseHeaders(Response $response, array $oauth2Headers)
+    {
+        if (empty($oauth2Headers)) {
+            return $response;
+        }
+
+        $headers = $response->getHeaders();
+        foreach ($oauth2Headers as $header => $value) {
+            $headers->addHeaderLine($header, $value);
+        }
+
+        return $response;
+    }
 }

test/Authentication/DefaultAuthenticationListenerTest.php

@@ -798,7 +798,13 @@ public function testOauth2RequestIncludesHeaders()
             ->method('verifyResourceRequest')
             ->with($this->callback(function (OAuth2Request $request) {
                 return $request->headers('Authorization') === 'Bearer TOKEN';
-            }));
+            }))
+            ->willReturn(true);
+
+        $server->expects($this->atLeastOnce())
+            ->method('getAccessTokenData')
+            ->with($this->anything())
+            ->willReturn(array('user_id' => 'TOKEN'));
 
         $this->listener->attach(new OAuth2Adapter($server));
         $this->listener->__invoke($this->mvcAuthEvent);

test/Authentication/OAuth2AdapterTest.php

@@ -0,0 +1,217 @@
+<?php
+/**
+ * @license   http://opensource.org/licenses/BSD-3-Clause BSD-3-Clause
+ * @copyright Copyright (c) 2014 Zend Technologies USA Inc. (http://www.zend.com)
+ */
+
+namespace ZFTest\MvcAuth\Authentication;
+
+use ArrayIterator;
+use OAuth2\Request as OAuth2Request;
+use PHPUnit_Framework_TestCase as TestCase;
+use Zend\Http\PhpEnvironment\Request as HttpRequest;
+use Zend\Http\Response as HttpResponse;
+use ZF\MvcAuth\Authentication\OAuth2Adapter;
+
+class OAuth2AdapterTest extends TestCase
+{
+    public function setUp()
+    {
+        $this->oauthServer = $this->getMock('OAuth2\Server');
+        $this->adapter = new OAuth2Adapter($this->oauthServer);
+    }
+
+    /**
+     * @group 83
+     */
+    public function testReturns401ResponseWhenErrorOccursDuringValidation()
+    {
+        $oauth2Response = $this->getMockBuilder('OAuth2\Response')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getStatusCode')
+            ->willReturn(401);
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getParameter')
+            ->with($this->equalTo('error'))
+            ->willReturn('invalid');
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getHttpHeaders')
+            ->willReturn(array());
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('verifyResourceRequest')
+            ->with($this->callback(function ($subject) {
+                return ($subject instanceof OAuth2Request);
+            }))
+            ->willReturn(false);
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('getResponse')
+            ->willReturn($oauth2Response);
+
+        $mvcAuthEvent = $this->getMockBuilder('ZF\MvcAuth\MvcAuthEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $result = $this->adapter->authenticate(new HttpRequest, new HttpResponse, $mvcAuthEvent);
+        $this->assertInstanceOf('Zend\Http\Response', $result);
+        $this->assertEquals(401, $result->getStatusCode());
+    }
+
+    /**
+     * @group 83
+     */
+    public function testReturns403ResponseWhenInvalidScopeDetected()
+    {
+        $oauth2Response = $this->getMockBuilder('OAuth2\Response')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getStatusCode')
+            ->willReturn(403);
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getParameter')
+            ->with($this->equalTo('error'))
+            ->willReturn('invalid');
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getHttpHeaders')
+            ->willReturn(array());
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('verifyResourceRequest')
+            ->with($this->callback(function ($subject) {
+                return ($subject instanceof OAuth2Request);
+            }))
+            ->willReturn(false);
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('getResponse')
+            ->willReturn($oauth2Response);
+
+        $mvcAuthEvent = $this->getMockBuilder('ZF\MvcAuth\MvcAuthEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $result = $this->adapter->authenticate(new HttpRequest, new HttpResponse, $mvcAuthEvent);
+        $this->assertInstanceOf('Zend\Http\Response', $result);
+        $this->assertEquals(403, $result->getStatusCode());
+    }
+
+    /**
+     * @group 83
+     */
+    public function testReturnsGuestIdentityIfOAuth2ResponseIsNotAnError()
+    {
+        $oauth2Response = $this->getMockBuilder('OAuth2\Response')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getStatusCode')
+            ->willReturn(200);
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getHttpHeaders')
+            ->willReturn(array());
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('verifyResourceRequest')
+            ->with($this->callback(function ($subject) {
+                return ($subject instanceof OAuth2Request);
+            }))
+            ->willReturn(false);
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('getResponse')
+            ->willReturn($oauth2Response);
+
+        $mvcAuthEvent = $this->getMockBuilder('ZF\MvcAuth\MvcAuthEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $result = $this->adapter->authenticate(new HttpRequest, new HttpResponse, $mvcAuthEvent);
+        $this->assertInstanceOf('ZF\MvcAuth\Identity\GuestIdentity', $result);
+    }
+
+    /**
+     * @group 83
+     */
+    public function testErrorResponseIncludesOAuth2ResponseHeaders()
+    {
+        $expectedHeaders = array(
+            'WWW-Authenticate' => 'Bearer realm="example.com", '
+            . 'scope="user", '
+            . 'error="unauthorized", '
+            . 'error_description="User has insufficient privileges"',
+        );
+        $oauth2Response = $this->getMockBuilder('OAuth2\Response')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getStatusCode')
+            ->willReturn(401);
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getParameter')
+            ->with($this->equalTo('error'))
+            ->willReturn('invalid');
+        $oauth2Response
+            ->expects($this->once())
+            ->method('getHttpHeaders')
+            ->willReturn($expectedHeaders);
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('verifyResourceRequest')
+            ->with($this->callback(function ($subject) {
+                return ($subject instanceof OAuth2Request);
+            }))
+            ->willReturn(false);
+
+        $this->oauthServer
+            ->expects($this->once())
+            ->method('getResponse')
+            ->willReturn($oauth2Response);
+
+        $mvcAuthEvent = $this->getMockBuilder('ZF\MvcAuth\MvcAuthEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $result = $this->adapter->authenticate(new HttpRequest, new HttpResponse, $mvcAuthEvent);
+        $this->assertInstanceOf('Zend\Http\Response', $result);
+
+        $headers = $result->getHeaders();
+        foreach ($expectedHeaders as $name => $value) {
+            $this->assertTrue($headers->has($name));
+            $header = $headers->get($name);
+            if ($header instanceof ArrayIterator) {
+                $found = false;
+                foreach ($header as $instance) {
+                    if ($instance->getFieldValue() == $value) {
+                        $found = true;
+                        break;
+                    }
+                }
+                $this->assertTrue($found, 'Expected header value not found');
+                continue;
+            }
+
+            $this->assertEquals($value, $header->getFieldValue());
+        }
+    }
+}