From d2760e4ec7e5aa768e61491c8d3cc563788983e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Guti=C3=A9rrez?= Date: Fri, 12 Feb 2016 13:56:14 +0100 Subject: [PATCH 1/4] secure access to body params --- src/Grant/AbstractGrant.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 01917c91..9d22418b 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -249,13 +249,15 @@ abstract class AbstractGrant implements GrantTypeInterface */ protected function getRequestParameter($parameter, ServerRequestInterface $request, $default = null) { - return (isset($request->getParsedBody()[$parameter])) ? $request->getParsedBody()[$parameter] : $default; + return (is_array($request->getParsedBody()) && isset($request->getParsedBody()[$parameter])) + ? $request->getParsedBody()[$parameter] + : $default; } /** * Retrieve server parameter. * - * @param string|array $parameter + * @param string $parameter * @param \Psr\Http\Message\ServerRequestInterface $request * @param mixed $default * @@ -314,7 +316,8 @@ abstract class AbstractGrant implements GrantTypeInterface public function canRespondToRequest(ServerRequestInterface $request) { return ( - isset($request->getParsedBody()['grant_type']) + is_array($request->getParsedBody()) + && isset($request->getParsedBody()['grant_type']) && $request->getParsedBody()['grant_type'] === $this->identifier ); } From 2f914a0aa327a4c553d6941ec303fac02ad2303e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Guti=C3=A9rrez?= Date: Fri, 12 Feb 2016 18:32:09 +0100 Subject: [PATCH 2/4] secure params access on authcode grant --- src/Grant/AuthCodeGrant.php | 59 +++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 560b8ad1..010bc482 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -307,17 +307,44 @@ class AuthCodeGrant extends AbstractGrant return $responseType; } + /** + * @inheritdoc + */ + public function respondToRequest( + ServerRequestInterface $request, + ResponseTypeInterface $responseType, + \DateInterval $accessTokenTTL + ) { + $requestParameters = (array) $request->getParsedBody(); + + if (array_key_exists('response_type', $requestParameters) + && $requestParameters['response_type'] === 'code' + && array_key_exists('client_id', $requestParameters) + ) { + return $this->respondToAuthorizationRequest($request); + } elseif (array_key_exists('grant_type', $requestParameters) + && $requestParameters['grant_type'] === $this->getIdentifier() + ) { + return $this->respondToAccessTokenRequest($request, $responseType, $accessTokenTTL); + } else { + throw OAuthServerException::serverError('respondToRequest() should not have been called'); + } + } + /** * @inheritdoc */ public function canRespondToRequest(ServerRequestInterface $request) { + $requestParameters = (array) $request->getParsedBody(); + return ( ( - isset($request->getQueryParams()['response_type']) - && $request->getQueryParams()['response_type'] === 'code' - && isset($request->getQueryParams()['client_id']) - ) || (parent::canRespondToRequest($request)) + array_key_exists('response_type', $requestParameters) + && $requestParameters['response_type'] === 'code' + && array_key_exists('client_id', $requestParameters) + ) + || parent::canRespondToRequest($request) ); } @@ -330,28 +357,4 @@ class AuthCodeGrant extends AbstractGrant { return 'authorization_code'; } - - /** - * @inheritdoc - */ - public function respondToRequest( - ServerRequestInterface $request, - ResponseTypeInterface $responseType, - \DateInterval $accessTokenTTL - ) { - if ( - isset($request->getQueryParams()['response_type']) - && $request->getQueryParams()['response_type'] === 'code' - && isset($request->getQueryParams()['client_id']) - ) { - return $this->respondToAuthorizationRequest($request); - } elseif ( - isset($request->getParsedBody()['grant_type']) - && $request->getParsedBody()['grant_type'] === 'authorization_code' - ) { - return $this->respondToAccessTokenRequest($request, $responseType, $accessTokenTTL); - } else { - throw OAuthServerException::serverError('respondToRequest() should not have been called'); - } - } } From 1f6bb40952e157d63db4a82ab7930a29992b7875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Guti=C3=A9rrez?= Date: Fri, 12 Feb 2016 18:45:47 +0100 Subject: [PATCH 3/4] correcting param access mistake --- src/Grant/AuthCodeGrant.php | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 010bc482..f9e17ea2 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -315,16 +315,12 @@ class AuthCodeGrant extends AbstractGrant ResponseTypeInterface $responseType, \DateInterval $accessTokenTTL ) { - $requestParameters = (array) $request->getParsedBody(); - - if (array_key_exists('response_type', $requestParameters) - && $requestParameters['response_type'] === 'code' - && array_key_exists('client_id', $requestParameters) + if (array_key_exists('response_type', $request->getQueryParams()) + && $request->getQueryParams()['response_type'] === 'code' + && array_key_exists('client_id', $request->getQueryParams()) ) { return $this->respondToAuthorizationRequest($request); - } elseif (array_key_exists('grant_type', $requestParameters) - && $requestParameters['grant_type'] === $this->getIdentifier() - ) { + } elseif (parent::canRespondToRequest($request)) { return $this->respondToAccessTokenRequest($request, $responseType, $accessTokenTTL); } else { throw OAuthServerException::serverError('respondToRequest() should not have been called'); @@ -336,13 +332,11 @@ class AuthCodeGrant extends AbstractGrant */ public function canRespondToRequest(ServerRequestInterface $request) { - $requestParameters = (array) $request->getParsedBody(); - return ( ( - array_key_exists('response_type', $requestParameters) - && $requestParameters['response_type'] === 'code' - && array_key_exists('client_id', $requestParameters) + array_key_exists('response_type', $request->getQueryParams()) + && $request->getQueryParams()['response_type'] === 'code' + && isset($request->getQueryParams()['client_id']) ) || parent::canRespondToRequest($request) ); From 1bdeb71efb1c1e4e7844706bb5c51cc456bcd409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Guti=C3=A9rrez?= Date: Tue, 8 Mar 2016 21:59:10 +0100 Subject: [PATCH 4/4] make StyleCI happy --- src/Grant/AbstractGrant.php | 1 + src/Grant/AuthCodeGrant.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 1267a6b0..f700c57b 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -277,6 +277,7 @@ abstract class AbstractGrant implements GrantTypeInterface protected function getRequestParameter($parameter, ServerRequestInterface $request, $default = null) { $requestParameters = (array) $request->getParsedBody(); + return isset($requestParameters[$parameter]) ? $requestParameters[$parameter] : $default; } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index a33e8fe9..b8ea461c 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -337,7 +337,7 @@ class AuthCodeGrant extends AbstractGrant } /** - * @inheritdoc + * {@inheritdoc} */ public function canRespondToRequest(ServerRequestInterface $request) {