From 28999621a8cb342778911beb6f8e7f1ea9589961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kopacz?= Date: Wed, 7 Jan 2015 19:53:44 +0100 Subject: [PATCH 1/6] add oauth2 response format --- config/module.config.php | 2 + src/Controller/AuthController.php | 114 +++++++++++++-------- src/Factory/AuthControllerFactory.php | 9 +- test/Controller/AuthControllerTest.php | 77 +++++++++++++- test/Factory/AuthControllerFactoryTest.php | 2 + 5 files changed, 158 insertions(+), 46 deletions(-) diff --git a/config/module.config.php b/config/module.config.php index 82ad12c..3f4c085 100644 --- a/config/module.config.php +++ b/config/module.config.php @@ -78,6 +78,8 @@ * // see https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/Storage/Pdo.php#L57-L66 * ] */ + 'api_problem_error_response' => true, // if true, client errors is returned in application/problem+json content type, + // otherwise in format from oauth2 specification (default: true) ), 'zf-content-negotiation' => array( 'ZF\OAuth2\Controller\Auth' => array( diff --git a/src/Controller/AuthController.php b/src/Controller/AuthController.php index cfe2e90..e62051f 100644 --- a/src/Controller/AuthController.php +++ b/src/Controller/AuthController.php @@ -23,6 +23,11 @@ class AuthController extends AbstractActionController */ protected $server; + /** + * @var boolean + */ + protected $apiProblemErrorResponse = true; + /** * Constructor * @@ -33,6 +38,22 @@ public function __construct(OAuth2Server $server) $this->server = $server; } + /** + * @return boolean + */ + public function isApiProblemErrorResponse() + { + return $this->apiProblemErrorResponse; + } + + /** + * @param boolean $apiProblemErrorResponse + */ + public function setApiProblemErrorResponse($apiProblemErrorResponse) + { + $this->apiProblemErrorResponse = $apiProblemErrorResponse; + } + /** * Token Action (/oauth) */ @@ -52,21 +73,11 @@ public function tokenAction() $oauth2request = $this->getOAuth2Request(); $response = $this->server->handleTokenRequest($oauth2request); + if ($response->isClientError()) { - $parameters = $response->getParameters(); - $errorUri = isset($parameters['error_uri']) ? $parameters['error_uri'] : null; - $error = isset($parameters['error']) ? $parameters['error'] : null; - $errorDescription = isset($parameters['error_description']) ? $parameters['error_description'] : null; - - return new ApiProblemResponse( - new ApiProblem( - $response->getStatusCode(), - $errorDescription, - $errorUri, - $error - ) - ); + return $this->getErrorResponse($response); } + return $this->setHttpResponse($response); } @@ -78,17 +89,9 @@ public function resourceAction() // Handle a request for an OAuth2.0 Access Token and send the response to the client if (!$this->server->verifyResourceRequest($this->getOAuth2Request())) { $response = $this->server->getResponse(); - $parameters = $response->getParameters(); - $errorUri = isset($parameters['error_uri']) ? $parameters['error_uri'] : null; - return new ApiProblemResponse( - new ApiProblem( - $response->getStatusCode(), - $parameters['error_description'], - $errorUri, - $parameters['error'] - ) - ); + return $this->getApiProblemResponse($response); } + $httpResponse = $this->getResponse(); $httpResponse->setStatusCode(200); $httpResponse->getHeaders()->addHeaders(array('Content-type' => 'application/json')); @@ -107,17 +110,10 @@ public function authorizeAction() $response = new OAuth2Response(); // validate the authorize request - if (!$this->server->validateAuthorizeRequest($request, $response)) { - $parameters = $response->getParameters(); - $errorUri = isset($parameters['error_uri']) ? $parameters['error_uri'] : null; - return new ApiProblemResponse( - new ApiProblem( - $response->getStatusCode(), - $parameters['error_description'], - $errorUri, - $parameters['error'] - ) - ); + $isValid = $this->server->validateAuthorizeRequest($request, $response); + + if (!$isValid) { + return $this->getErrorResponse($response); } $authorized = $request->request('authorized', false); @@ -141,16 +137,7 @@ public function authorizeAction() return $this->redirect()->toUrl($redirect); } - $parameters = $response->getParameters(); - $errorUri = isset($parameters['error_uri']) ? $parameters['error_uri'] : null; - return new ApiProblemResponse( - new ApiProblem( - $response->getStatusCode(), - $parameters['error_description'], - $errorUri, - $parameters['error'] - ) - ); + return $this->getErrorResponse($response); } /** @@ -166,6 +153,42 @@ public function receiveCodeAction() return $view; } + /** + * @param OAuth2Response $response + * @return \ZF\ApiProblem\ApiProblemResponse|\Zend\Stdlib\ResponseInterface + */ + protected function getErrorResponse(OAuth2Response $response) + { + if ($this->isApiProblemErrorResponse()) { + return $this->getApiProblemResponse($response); + } else { + return $this->setHttpResponse($response); + } + } + + /** + * Map OAuth2Response to ApiProblemResponse + * + * @param OAuth2Response $response + * @return ApiProblemResponse + */ + protected function getApiProblemResponse(OAuth2Response $response) + { + $parameters = $response->getParameters(); + $errorUri = isset($parameters['error_uri']) ? $parameters['error_uri'] : null; + $error = isset($parameters['error']) ? $parameters['error'] : null; + $errorDescription = isset($parameters['error_description']) ? $parameters['error_description'] : null; + + return new ApiProblemResponse( + new ApiProblem( + $response->getStatusCode(), + $errorDescription, + $errorUri, + $error + ) + ); + } + /** * Create an OAuth2 request based on the ZF2 request object * @@ -210,6 +233,9 @@ protected function getOAuth2Request() if (isset($server['PHP_AUTH_PW'])) { $headers['PHP_AUTH_PW'] = $server['PHP_AUTH_PW']; } + if (isset($server['HTTP_AUTHORIZATION'])) { + $headers['AUTHORIZATION'] = $server['HTTP_AUTHORIZATION']; + } // Ensure the bodyParams are passed as an array $bodyParams = $this->bodyParams() ?: array(); diff --git a/src/Factory/AuthControllerFactory.php b/src/Factory/AuthControllerFactory.php index a8baaa6..1c86c3f 100644 --- a/src/Factory/AuthControllerFactory.php +++ b/src/Factory/AuthControllerFactory.php @@ -19,6 +19,13 @@ class AuthControllerFactory implements FactoryInterface public function createService(ServiceLocatorInterface $controllers) { $services = $controllers->getServiceLocator()->get('ServiceManager'); - return new AuthController($services->get('ZF\OAuth2\Service\OAuth2Server')); + $authController = new AuthController($services->get('ZF\OAuth2\Service\OAuth2Server')); + + $config = $services->get('Config'); + $apiProblemErrorResponse = isset($config['zf-oauth2']['api_problem_error_response']) && $config['zf-oauth2']['api_problem_error_response'] === true; + + $authController->setApiProblemErrorResponse($apiProblemErrorResponse); + + return $authController; } } diff --git a/test/Controller/AuthControllerTest.php b/test/Controller/AuthControllerTest.php index dc7f1a6..ff826f6 100644 --- a/test/Controller/AuthControllerTest.php +++ b/test/Controller/AuthControllerTest.php @@ -45,6 +45,51 @@ public function testToken() $this->assertTrue(!empty($response['token_type'])); } + public function testTokenErrorIsApiProblem() + { + $request = $this->getRequest(); + $request->getPost()->set('grant_type', 'fake_grant_type'); + $request->getServer()->set('PHP_AUTH_USER', 'testclient'); + $request->getServer()->set('PHP_AUTH_PW', 'testpass'); + $request->setMethod('POST'); + + $this->dispatch('/oauth'); + $this->assertControllerName('ZF\OAuth2\Controller\Auth'); + $this->assertActionName('token'); + $this->assertResponseStatusCode(400); + + $headers = $this->getResponse()->getHeaders(); + $this->assertEquals('application/problem+json', $headers->get('content-type')->getFieldValue()); + + $response = json_decode($this->getResponse()->getContent(), true); + $this->assertEquals('unsupported_grant_type', $response['title']); + $this->assertEquals('Grant type "fake_grant_type" not supported', $response['detail']); + $this->assertEquals('400', $response['status']); + } + + public function testTokenErrorIsOAuth2Format() + { + $request = $this->getRequest(); + $request->getPost()->set('grant_type', 'fake_grant_type'); + $request->getServer()->set('PHP_AUTH_USER', 'testclient'); + $request->getServer()->set('PHP_AUTH_PW', 'testpass'); + $request->setMethod('POST'); + + $this->setIsOAuth2FormatResponse(); + + $this->dispatch('/oauth'); + $this->assertControllerName('ZF\OAuth2\Controller\Auth'); + $this->assertActionName('token'); + $this->assertResponseStatusCode(400); + + $headers = $this->getResponse()->getHeaders(); + $this->assertEquals('application/json', $headers->get('content-type')->getFieldValue()); + + $response = json_decode($this->getResponse()->getContent(), true); + $this->assertEquals('unsupported_grant_type', $response['error']); + $this->assertEquals('Grant type "fake_grant_type" not supported', $response['error_description']); + } + public function testAuthorizeForm() { $_GET['response_type'] = 'code'; @@ -59,7 +104,7 @@ public function testAuthorizeForm() $this->assertXpathQuery('//form/input[@name="authorized" and @value="no"]'); } - public function testAuthorizeErrorParam() + public function testAuthorizeParamErrorIsApiProblem() { $this->dispatch('/oauth/authorize'); @@ -76,6 +121,24 @@ public function testAuthorizeErrorParam() $this->assertEquals('400', $response['status']); } + public function testAuthorizeParamErrorIsOAuth2Format() + { + $this->setIsOAuth2FormatResponse(); + + $this->dispatch('/oauth/authorize'); + + $this->assertControllerName('ZF\OAuth2\Controller\Auth'); + $this->assertActionName('authorize'); + $this->assertResponseStatusCode(400); + + $headers = $this->getResponse()->getHeaders(); + $this->assertEquals('application/json', $headers->get('content-type')->getFieldValue()); + + $response = json_decode($this->getResponse()->getContent(), true); + $this->assertEquals('invalid_client', $response['error']); + $this->assertEquals('No client id supplied', $response['error_description']); + } + public function testAuthorizeCode() { $_GET['response_type'] = 'code'; @@ -174,6 +237,7 @@ public function testResource() $this->assertResponseStatusCode(200); $response = json_decode($this->getResponse()->getContent(), true); + $this->assertTrue($response['success']); $this->assertEquals('You accessed my APIs!', $response['message']); @@ -191,4 +255,15 @@ public function testResource() $this->assertTrue($response['success']); $this->assertEquals('You accessed my APIs!', $response['message']); } + + protected function setIsOAuth2FormatResponse() + { + $serviceManager = $this->getApplication()->getServiceManager(); + + $config = $serviceManager->get('Config'); + $config['zf-oauth2']['api_problem_error_response'] = false; + + $serviceManager->setAllowOverride(true); + $serviceManager->setService('Config', $config); + } } diff --git a/test/Factory/AuthControllerFactoryTest.php b/test/Factory/AuthControllerFactoryTest.php index 5ad5013..61dc84e 100644 --- a/test/Factory/AuthControllerFactoryTest.php +++ b/test/Factory/AuthControllerFactoryTest.php @@ -48,6 +48,8 @@ protected function setUp() $this->services = $services = new ServiceManager(); + $this->services->setService('Config', array()); + $this->controllers = $controllers = new ControllerManager(); $controllers->setServiceLocator(new ServiceManager()); $controllers->getServiceLocator()->setService('ServiceManager', $services); From 4f709ebd57d29ec5a271963adfac65eb695b6be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kopacz?= Date: Wed, 7 Jan 2015 20:10:52 +0100 Subject: [PATCH 2/6] add api_problem_error_response config for test --- src/Factory/AuthControllerFactory.php | 3 ++- test/Factory/AuthControllerFactoryTest.php | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Factory/AuthControllerFactory.php b/src/Factory/AuthControllerFactory.php index 1c86c3f..5c03731 100644 --- a/src/Factory/AuthControllerFactory.php +++ b/src/Factory/AuthControllerFactory.php @@ -22,7 +22,8 @@ public function createService(ServiceLocatorInterface $controllers) $authController = new AuthController($services->get('ZF\OAuth2\Service\OAuth2Server')); $config = $services->get('Config'); - $apiProblemErrorResponse = isset($config['zf-oauth2']['api_problem_error_response']) && $config['zf-oauth2']['api_problem_error_response'] === true; + $apiProblemErrorResponse = isset($config['zf-oauth2']['api_problem_error_response']) + && $config['zf-oauth2']['api_problem_error_response'] === true; $authController->setApiProblemErrorResponse($apiProblemErrorResponse); diff --git a/test/Factory/AuthControllerFactoryTest.php b/test/Factory/AuthControllerFactoryTest.php index 61dc84e..2f6f81d 100644 --- a/test/Factory/AuthControllerFactoryTest.php +++ b/test/Factory/AuthControllerFactoryTest.php @@ -48,7 +48,11 @@ protected function setUp() $this->services = $services = new ServiceManager(); - $this->services->setService('Config', array()); + $this->services->setService('Config', array( + 'zf-oauth2' => array( + 'api_problem_error_response' => true, + ), + )); $this->controllers = $controllers = new ControllerManager(); $controllers->setServiceLocator(new ServiceManager()); From 548bff1f107c605ce72f3ee88143d706a21a098a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kopacz?= Date: Wed, 7 Jan 2015 20:11:15 +0100 Subject: [PATCH 3/6] support for windows --- bin/phpcs.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/phpcs.sh b/bin/phpcs.sh index 9ca9164..074f346 100755 --- a/bin/phpcs.sh +++ b/bin/phpcs.sh @@ -1,7 +1,6 @@ #!/bin/sh -SCRIPT_PATH=$(readlink -f $0) -WORKDIR=$(dirname $SCRIPT_PATH) -WORKDIR=$(dirname $WORKDIR) +SCRIPT_PATH=$(php -r "echo readlink('${0}');") +WORKDIR=$(php -r "echo dirname(dirname('${SCRIPT_PATH}'));") PHP_VERSION=$(php -v | grep '^PHP [[:digit:]].[[:digit:]]' | cut -d ' ' -f2) IS_PHP_5_3=$(php -r "echo version_compare('${PHP_VERSION}', '5.4.0');") From 0172f2fe69bb96d56322f06e8fc7cc11c9fb26f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kopacz?= Date: Wed, 7 Jan 2015 20:29:04 +0100 Subject: [PATCH 4/6] undo support for windows --- bin/phpcs.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/phpcs.sh b/bin/phpcs.sh index 074f346..9ca9164 100755 --- a/bin/phpcs.sh +++ b/bin/phpcs.sh @@ -1,6 +1,7 @@ #!/bin/sh -SCRIPT_PATH=$(php -r "echo readlink('${0}');") -WORKDIR=$(php -r "echo dirname(dirname('${SCRIPT_PATH}'));") +SCRIPT_PATH=$(readlink -f $0) +WORKDIR=$(dirname $SCRIPT_PATH) +WORKDIR=$(dirname $WORKDIR) PHP_VERSION=$(php -v | grep '^PHP [[:digit:]].[[:digit:]]' | cut -d ' ' -f2) IS_PHP_5_3=$(php -r "echo version_compare('${PHP_VERSION}', '5.4.0');") From ef3105e2eae37e36378935d22be4bb38090c21d7 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 30 Mar 2015 09:59:00 -0500 Subject: [PATCH 5/6] Remove requirement for intermediary variable --- src/Controller/AuthController.php | 23 ++++++++++++++--------- src/Factory/AuthControllerFactory.php | 8 +++----- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/Controller/AuthController.php b/src/Controller/AuthController.php index e62051f..3c51e28 100644 --- a/src/Controller/AuthController.php +++ b/src/Controller/AuthController.php @@ -39,7 +39,9 @@ public function __construct(OAuth2Server $server) } /** - * @return boolean + * Should the controller return ApiProblemResponse? + * + * @return bool */ public function isApiProblemErrorResponse() { @@ -47,11 +49,17 @@ public function isApiProblemErrorResponse() } /** - * @param boolean $apiProblemErrorResponse + * Indicate whether ApiProblemResponse or oauth2 errors should be returned. + * + * Boolean true indicates ApiProblemResponse should be returned (the + * default), while false indicates oauth2 errors (per the oauth2 spec) + * should be returned. + * + * @param bool $apiProblemErrorResponse */ public function setApiProblemErrorResponse($apiProblemErrorResponse) { - $this->apiProblemErrorResponse = $apiProblemErrorResponse; + $this->apiProblemErrorResponse = (bool) $apiProblemErrorResponse; } /** @@ -155,15 +163,15 @@ public function receiveCodeAction() /** * @param OAuth2Response $response - * @return \ZF\ApiProblem\ApiProblemResponse|\Zend\Stdlib\ResponseInterface + * @return ApiProblemResponse|\Zend\Stdlib\ResponseInterface */ protected function getErrorResponse(OAuth2Response $response) { if ($this->isApiProblemErrorResponse()) { return $this->getApiProblemResponse($response); - } else { - return $this->setHttpResponse($response); } + + return $this->setHttpResponse($response); } /** @@ -233,9 +241,6 @@ protected function getOAuth2Request() if (isset($server['PHP_AUTH_PW'])) { $headers['PHP_AUTH_PW'] = $server['PHP_AUTH_PW']; } - if (isset($server['HTTP_AUTHORIZATION'])) { - $headers['AUTHORIZATION'] = $server['HTTP_AUTHORIZATION']; - } // Ensure the bodyParams are passed as an array $bodyParams = $this->bodyParams() ?: array(); diff --git a/src/Factory/AuthControllerFactory.php b/src/Factory/AuthControllerFactory.php index 5c03731..6781444 100644 --- a/src/Factory/AuthControllerFactory.php +++ b/src/Factory/AuthControllerFactory.php @@ -21,11 +21,9 @@ public function createService(ServiceLocatorInterface $controllers) $services = $controllers->getServiceLocator()->get('ServiceManager'); $authController = new AuthController($services->get('ZF\OAuth2\Service\OAuth2Server')); - $config = $services->get('Config'); - $apiProblemErrorResponse = isset($config['zf-oauth2']['api_problem_error_response']) - && $config['zf-oauth2']['api_problem_error_response'] === true; - - $authController->setApiProblemErrorResponse($apiProblemErrorResponse); + $config = $services->get('Config'); + $authController->setApiProblemErrorResponse((isset($config['zf-oauth2']['api_problem_error_response']) + && $config['zf-oauth2']['api_problem_error_response'] === true)); return $authController; } From 5c04f7aad839de35a741bff1b30c358aa1ef7c32 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 30 Mar 2015 10:03:14 -0500 Subject: [PATCH 6/6] Fix test setup Instead of using `$_SERVER` to set the authorization header, set the authorization header directly on the request instance. --- test/Controller/AuthControllerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Controller/AuthControllerTest.php b/test/Controller/AuthControllerTest.php index ff826f6..81d0368 100644 --- a/test/Controller/AuthControllerTest.php +++ b/test/Controller/AuthControllerTest.php @@ -242,7 +242,8 @@ public function testResource() $this->assertEquals('You accessed my APIs!', $response['message']); // test resource through token by Bearer header - $server->set('HTTP_AUTHORIZATION', "Bearer $token"); + $request->getHeaders() + ->addHeaderLine('Authorization', 'Bearer ' . $token); unset($post['access_token']); $request->setMethod('GET');