From 101feec0ec963726f7735c9daa23ba2904504d3b Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 7 Jul 2015 11:35:36 -0500 Subject: [PATCH] [#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 | 2 +- src/Authentication/OAuth2Adapter.php | 64 +++++- .../DefaultAuthenticationListenerTest.php | 8 +- test/Authentication/OAuth2AdapterTest.php | 217 ++++++++++++++++++ 4 files changed, 281 insertions(+), 10 deletions(-) create mode 100644 test/Authentication/OAuth2AdapterTest.php diff --git a/composer.json b/composer.json index e33db0c..53c8095 100644 --- a/composer.json +++ b/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" diff --git a/src/Authentication/OAuth2Adapter.php b/src/Authentication/OAuth2Adapter.php index 3a3ea7d..1cff79a 100644 --- a/src/Authentication/OAuth2Adapter.php +++ b/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,20 +134,32 @@ 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); @@ -154,4 +167,39 @@ public function authenticate(Request $request, Response $response, MvcAuthEvent $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; + } } diff --git a/test/Authentication/DefaultAuthenticationListenerTest.php b/test/Authentication/DefaultAuthenticationListenerTest.php index f694b08..dcf3f4a 100644 --- a/test/Authentication/DefaultAuthenticationListenerTest.php +++ b/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); diff --git a/test/Authentication/OAuth2AdapterTest.php b/test/Authentication/OAuth2AdapterTest.php new file mode 100644 index 0000000..d63bbf9 --- /dev/null +++ b/test/Authentication/OAuth2AdapterTest.php @@ -0,0 +1,217 @@ +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()); + } + } +}