Skip to content

Commit

Permalink
Improve validation in the grant types to catch empty inputs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonoradich
jonoradich committed Jan 31, 2017
1 parent 7936198 commit 8e54ee1
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 23 deletions.
16 changes: 8 additions & 8 deletions src/League/OAuth2/Server/Grant/AuthCode.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ public function checkAuthoriseParams($inputParams = array())
// Auth params
$authParams = $this->authServer->getParam(array('client_id', 'redirect_uri', 'response_type', 'scope', 'state'), 'get', $inputParams);

if (is_null($authParams['client_id'])) {
if (empty($authParams['client_id'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_id'), 0);
}

if (is_null($authParams['redirect_uri'])) {
if (empty($authParams['redirect_uri'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'redirect_uri'), 0);
}

if ($this->authServer->stateParamRequired() === true && is_null($authParams['state'])) {
if ($this->authServer->stateParamRequired() === true && empty($authParams['state'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'state'), 0);
}

Expand All @@ -98,7 +98,7 @@ public function checkAuthoriseParams($inputParams = array())

$authParams['client_details'] = $clientDetails;

if (is_null($authParams['response_type'])) {
if (empty($authParams['response_type'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'response_type'), 0);
}

Expand Down Expand Up @@ -185,15 +185,15 @@ public function completeFlow($inputParams = null)
// Get the required params
$authParams = $this->authServer->getParam(array('client_id', 'client_secret', 'redirect_uri', 'code'), 'post', $inputParams);

if (is_null($authParams['client_id'])) {
if (empty($authParams['client_id'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_id'), 0);
}

if (is_null($authParams['client_secret'])) {
if (empty($authParams['client_secret'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_secret'), 0);
}

if (is_null($authParams['redirect_uri'])) {
if (empty($authParams['redirect_uri'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'redirect_uri'), 0);
}

Expand All @@ -207,7 +207,7 @@ public function completeFlow($inputParams = null)
$authParams['client_details'] = $clientDetails;

// Validate the authorization code
if (is_null($authParams['code'])) {
if (empty($authParams['code'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'code'), 0);
}

Expand Down
4 changes: 2 additions & 2 deletions src/League/OAuth2/Server/Grant/ClientCredentials.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ public function completeFlow($inputParams = null)
// Get the required params
$authParams = $this->authServer->getParam(array('client_id', 'client_secret'), 'post', $inputParams);

if (is_null($authParams['client_id'])) {
if (empty($authParams['client_id'])) {
throw new Exception\ClientException(sprintf(Authorization::getExceptionMessage('invalid_request'), 'client_id'), 0);
}

if (is_null($authParams['client_secret'])) {
if (empty($authParams['client_secret'])) {
throw new Exception\ClientException(sprintf(Authorization::getExceptionMessage('invalid_request'), 'client_secret'), 0);
}

Expand Down
8 changes: 4 additions & 4 deletions src/League/OAuth2/Server/Grant/Password.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ public function completeFlow($inputParams = null)
// Get the required params
$authParams = $this->authServer->getParam(array('client_id', 'client_secret', 'username', 'password'), 'post', $inputParams);

if (is_null($authParams['client_id'])) {
if (empty($authParams['client_id'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_id'), 0);
}

if (is_null($authParams['client_secret'])) {
if (empty($authParams['client_secret'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_secret'), 0);
}

Expand All @@ -106,11 +106,11 @@ public function completeFlow($inputParams = null)

$authParams['client_details'] = $clientDetails;

if (is_null($authParams['username'])) {
if (empty($authParams['username'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'username'), 0);
}

if (is_null($authParams['password'])) {
if (empty($authParams['password'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'password'), 0);
}

Expand Down
6 changes: 3 additions & 3 deletions src/League/OAuth2/Server/Grant/RefreshToken.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ public function completeFlow($inputParams = null)
// Get the required params
$authParams = $this->authServer->getParam(array('client_id', 'client_secret', 'refresh_token', 'scope'), 'post', $inputParams);

if (is_null($authParams['client_id'])) {
if (empty($authParams['client_id'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_id'), 0);
}

if (is_null($authParams['client_secret'])) {
if (empty($authParams['client_secret'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'client_secret'), 0);
}

Expand All @@ -119,7 +119,7 @@ public function completeFlow($inputParams = null)

$authParams['client_details'] = $clientDetails;

if (is_null($authParams['refresh_token'])) {
if (empty($authParams['refresh_token'])) {
throw new Exception\ClientException(sprintf($this->authServer->getExceptionMessage('invalid_request'), 'refresh_token'), 0);
}

Expand Down
54 changes: 53 additions & 1 deletion tests/authorization/AuthCodeGrantTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ public function test_checkAuthoriseParams_noClientId()
$g->checkAuthoriseParams();
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_checkAuthoriseParams_emptyClientId()
{
$a = $this->returnDefault();
$g = new League\OAuth2\Server\Grant\AuthCode();
$a->addGrantType($g);
$g->checkAuthoriseParams(array(
'client_id' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
Expand All @@ -76,6 +90,21 @@ public function test_checkAuthoriseParams_noRedirectUri()
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_checkAuthoriseParams_emptyRedirectUri()
{
$a = $this->returnDefault();
$g = new League\OAuth2\Server\Grant\AuthCode();
$a->addGrantType($g);
$g->checkAuthoriseParams(array(
'client_id' => 1234,
'redirect_uri' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
Expand Down Expand Up @@ -132,6 +161,29 @@ public function test_checkAuthoriseParams_missingResponseType()
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_checkAuthoriseParams_emptyResponseType()
{
$this->client->shouldReceive('getClient')->andReturn(array(
'client_id' => 1234,
'client_secret' => 5678,
'redirect_uri' => 'http://foo/redirect',
'name' => 'Example Client'
));

$a = $this->returnDefault();
$g = new League\OAuth2\Server\Grant\AuthCode();
$a->addGrantType($g);
$g->checkAuthoriseParams(array(
'client_id' => 1234,
'redirect_uri' => 'http://foo/redirect',
'response_type' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 3
Expand Down Expand Up @@ -425,4 +477,4 @@ function test_newAuthoriseRequest()
}


}
}
41 changes: 39 additions & 2 deletions tests/authorization/ClientCredentialsGrantTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,25 @@ public function test_issueAccessToken_clientCredentialsGrant_missingClientId()
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_clientCredentialsGrant_missingClientPassword()
public function test_issueAccessToken_clientCredentialsGrant_emptyClientId()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\ClientCredentials());

$request = new League\OAuth2\Server\Util\Request(array(), $_POST);
$a->setRequest($request);

$a->issueAccessToken(array(
'grant_type' => 'client_credentials',
'client_id' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_clientCredentialsGrant_missingClientSecret()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\ClientCredentials());
Expand All @@ -55,6 +73,25 @@ public function test_issueAccessToken_clientCredentialsGrant_missingClientPasswo
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_clientCredentialsGrant_emptyClientSecret()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\ClientCredentials());

$request = new League\OAuth2\Server\Util\Request(array(), $_POST);
$a->setRequest($request);

$a->issueAccessToken(array(
'grant_type' => 'client_credentials',
'client_id' => 1234,
'client_secret' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 8
Expand Down Expand Up @@ -411,4 +448,4 @@ function test_issueAccessToken_clientCredentialsGrant_withRefreshToken()
$this->assertEquals(time()+$a->getAccessTokenTTL(), $v['expires']);
}

}
}
41 changes: 39 additions & 2 deletions tests/authorization/PasswordGrantTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,25 @@ public function test_issueAccessToken_passwordGrant_missingClientId()
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_passwordGrant_missingClientPassword()
public function test_issueAccessToken_passwordGrant_emptyClientId()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\Password());

$request = new League\OAuth2\Server\Util\Request(array(), $_POST);
$a->setRequest($request);

$a->issueAccessToken(array(
'grant_type' => 'password',
'client_id' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_passwordGrant_missingClientSecret()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\Password());
Expand All @@ -55,6 +73,25 @@ public function test_issueAccessToken_passwordGrant_missingClientPassword()
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 0
*/
public function test_issueAccessToken_passwordGrant_emptyClientSecret()
{
$a = $this->returnDefault();
$a->addGrantType(new League\OAuth2\Server\Grant\Password());

$request = new League\OAuth2\Server\Util\Request(array(), $_POST);
$a->setRequest($request);

$a->issueAccessToken(array(
'grant_type' => 'password',
'client_id' => 1234,
'client_secret' => ''
));
}

/**
* @expectedException League\OAuth2\Server\Exception\ClientException
* @expectedExceptionCode 8
Expand Down Expand Up @@ -614,4 +651,4 @@ function test_issueAccessToken_passwordGrant_withRefreshToken()
$this->assertEquals(time()+$a->getAccessTokenTTL(), $v['expires']);
}

}
}
Loading

0 comments on commit 8e54ee1

Please sign in to comment.