From 522c7478c70f82398efff9b9dbe85807bf3e2cb2 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Wed, 6 Aug 2014 09:53:47 +0100 Subject: [PATCH] Fix #169 --- src/Exception/InvalidRequestException.php | 4 +- src/Exception/InvalidScopeException.php | 4 +- src/Exception/OAuthException.php | 15 +++ .../UnsupportedResponseTypeException.php | 1 + src/Grant/AbstractGrant.php | 2 +- src/Grant/AuthCodeGrant.php | 30 ++--- tests/unit/Grant/AuthCodeGrantTest.php | 121 ++++++++++-------- 7 files changed, 109 insertions(+), 68 deletions(-) diff --git a/src/Exception/InvalidRequestException.php b/src/Exception/InvalidRequestException.php index 56dd75df..cafa7df3 100644 --- a/src/Exception/InvalidRequestException.php +++ b/src/Exception/InvalidRequestException.php @@ -30,7 +30,7 @@ class InvalidRequestException extends OAuthException * {@inheritdoc} */ - public function __construct($parameter) + public function __construct($parameter, $shouldRedirect = false) { parent::__construct( sprintf( @@ -38,5 +38,7 @@ class InvalidRequestException extends OAuthException $parameter ) ); + + $this->serverShouldRedirect = $shouldRedirect; } } diff --git a/src/Exception/InvalidScopeException.php b/src/Exception/InvalidScopeException.php index eb74213d..0120ca23 100644 --- a/src/Exception/InvalidScopeException.php +++ b/src/Exception/InvalidScopeException.php @@ -30,7 +30,7 @@ class InvalidScopeException extends OAuthException * {@inheritdoc} */ - public function __construct($parameter) + public function __construct($parameter, $shouldRedirect = false) { parent::__construct( sprintf( @@ -38,5 +38,7 @@ class InvalidScopeException extends OAuthException $parameter ) ); + + $this->serverShouldRedirect = $shouldRedirect; } } diff --git a/src/Exception/OAuthException.php b/src/Exception/OAuthException.php index 5cbb0b3a..c4e8d157 100644 --- a/src/Exception/OAuthException.php +++ b/src/Exception/OAuthException.php @@ -23,6 +23,12 @@ class OAuthException extends \Exception */ public $httpStatusCode = 400; + /** + * If true the server should redirect back to the client + * @var boolean + */ + public $serverShouldRedirect = false; + /** * The exception type */ @@ -36,6 +42,15 @@ class OAuthException extends \Exception parent::__construct($msg); } + /** + * Should the server redirect back to the client? + * @return bool + */ + public function shouldRedirect() + { + return $this->serverShouldRedirect; + } + /** * Get all headers that have to be send with the error response * @return array Array with header values diff --git a/src/Exception/UnsupportedResponseTypeException.php b/src/Exception/UnsupportedResponseTypeException.php index 05677686..cfd74c1a 100644 --- a/src/Exception/UnsupportedResponseTypeException.php +++ b/src/Exception/UnsupportedResponseTypeException.php @@ -32,5 +32,6 @@ class UnsupportedResponseTypeException extends OAuthException public function __construct($parameter) { parent::__construct('The authorization server does not support obtaining an access token using this method.'); + $this->serverShouldRedirect = true; } } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 10195c79..93e45020 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -144,7 +144,7 @@ abstract class AbstractGrant implements GrantTypeInterface ); if (($scope instanceof ScopeEntity) === false) { - throw new Exception\InvalidScopeException($scopeItem); + throw new Exception\InvalidScopeException($scopeItem, true); } $scopes[$scope->getId()] = $scope; diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index b7fee22c..04ee914d 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -83,21 +83,6 @@ class AuthCodeGrant extends AbstractGrant throw new Exception\InvalidRequestException('redirect_uri'); } - $state = $this->server->getRequest()->query->get('state', null); - if ($this->server->stateParamRequired() === true && is_null($state)) { - throw new Exception\InvalidRequestException('state'); - } - - $responseType = $this->server->getRequest()->query->get('response_type', null); - if (is_null($responseType)) { - throw new Exception\InvalidRequestException('response_type'); - } - - // Ensure response type is one that is recognised - if (!in_array($responseType, $this->server->getResponseTypes())) { - throw new Exception\UnsupportedResponseTypeException($responseType); - } - // Validate client ID and redirect URI $client = $this->server->getStorage('client')->get( $clientId, @@ -110,6 +95,21 @@ class AuthCodeGrant extends AbstractGrant throw new Exception\InvalidClientException(); } + $state = $this->server->getRequest()->query->get('state', null); + if ($this->server->stateParamRequired() === true && is_null($state)) { + throw new Exception\InvalidRequestException('state', true); + } + + $responseType = $this->server->getRequest()->query->get('response_type', null); + if (is_null($responseType)) { + throw new Exception\InvalidRequestException('response_type', true); + } + + // Ensure response type is one that is recognised + if (!in_array($responseType, $this->server->getResponseTypes())) { + throw new Exception\UnsupportedResponseTypeException($responseType); + } + // Validate any scopes that are in the request $scopeParam = $this->server->getRequest()->query->get('scope', ''); $scopes = $this->validateScopes($scopeParam); diff --git a/tests/unit/Grant/AuthCodeGrantTest.php b/tests/unit/Grant/AuthCodeGrantTest.php index c0641438..4cb8f353 100644 --- a/tests/unit/Grant/AuthCodeGrantTest.php +++ b/tests/unit/Grant/AuthCodeGrantTest.php @@ -54,56 +54,6 @@ class AuthCodeGrantTest extends \PHPUnit_Framework_TestCase $grant->checkAuthorizeParams(); } - public function testCheckAuthoriseParamsMissingStateParam() - { - $this->setExpectedException('League\OAuth2\Server\Exception\InvalidRequestException'); - - $_GET = [ - 'client_id' => 'testapp', - 'redirect_uri' => 'http://foo/bar' - ]; - $server = new AuthorizationServer; - - $grant = new AuthCodeGrant; - $server->requireStateParam(true); - - $server->addGrantType($grant); - $grant->checkAuthorizeParams(); - } - - public function testCheckAuthoriseParamsMissingResponseType() - { - $this->setExpectedException('League\OAuth2\Server\Exception\InvalidRequestException'); - - $_GET = [ - 'client_id' => 'testapp', - 'redirect_uri' => 'http://foo/bar' - ]; - $server = new AuthorizationServer; - - $grant = new AuthCodeGrant; - - $server->addGrantType($grant); - $grant->checkAuthorizeParams(); - } - - public function testCheckAuthoriseParamsInvalidResponseType() - { - $this->setExpectedException('League\OAuth2\Server\Exception\UnsupportedResponseTypeException'); - - $_GET = [ - 'client_id' => 'testapp', - 'redirect_uri' => 'http://foo/bar', - 'response_type' => 'foobar' - ]; - $server = new AuthorizationServer; - - $grant = new AuthCodeGrant; - - $server->addGrantType($grant); - $grant->checkAuthorizeParams(); - } - public function testCheckAuthoriseParamsInvalidClient() { $this->setExpectedException('League\OAuth2\Server\Exception\InvalidClientException'); @@ -127,6 +77,77 @@ class AuthCodeGrantTest extends \PHPUnit_Framework_TestCase $grant->checkAuthorizeParams(); } + public function testCheckAuthoriseParamsMissingStateParam() + { + $this->setExpectedException('League\OAuth2\Server\Exception\InvalidRequestException'); + + $_GET = [ + 'client_id' => 'testapp', + 'redirect_uri' => 'http://foo/bar' + ]; + $server = new AuthorizationServer; + + $clientStorage = M::mock('League\OAuth2\Server\Storage\ClientInterface'); + $clientStorage->shouldReceive('setServer'); + $clientStorage->shouldReceive('get')->andReturn( + (new ClientEntity($server))->hydrate(['id' => 'testapp']) + ); + $server->setClientStorage($clientStorage); + + $grant = new AuthCodeGrant; + $server->requireStateParam(true); + + $server->addGrantType($grant); + $grant->checkAuthorizeParams(); + } + + public function testCheckAuthoriseParamsMissingResponseType() + { + $this->setExpectedException('League\OAuth2\Server\Exception\InvalidRequestException'); + + $_GET = [ + 'client_id' => 'testapp', + 'redirect_uri' => 'http://foo/bar' + ]; + $server = new AuthorizationServer; + + $clientStorage = M::mock('League\OAuth2\Server\Storage\ClientInterface'); + $clientStorage->shouldReceive('setServer'); + $clientStorage->shouldReceive('get')->andReturn( + (new ClientEntity($server))->hydrate(['id' => 'testapp']) + ); + $server->setClientStorage($clientStorage); + + $grant = new AuthCodeGrant; + + $server->addGrantType($grant); + $grant->checkAuthorizeParams(); + } + + public function testCheckAuthoriseParamsInvalidResponseType() + { + $this->setExpectedException('League\OAuth2\Server\Exception\UnsupportedResponseTypeException'); + + $_GET = [ + 'client_id' => 'testapp', + 'redirect_uri' => 'http://foo/bar', + 'response_type' => 'foobar' + ]; + $server = new AuthorizationServer; + + $clientStorage = M::mock('League\OAuth2\Server\Storage\ClientInterface'); + $clientStorage->shouldReceive('setServer'); + $clientStorage->shouldReceive('get')->andReturn( + (new ClientEntity($server))->hydrate(['id' => 'testapp']) + ); + $server->setClientStorage($clientStorage); + + $grant = new AuthCodeGrant; + + $server->addGrantType($grant); + $grant->checkAuthorizeParams(); + } + public function testCheckAuthoriseParamsInvalidScope() { $this->setExpectedException('League\OAuth2\Server\Exception\InvalidScopeException');