Skip to content

Commit 9ecf277

Browse files
committed
MAGETWO-31999: oAuth issue [from github]
- Fixed oauth validation to return aggregated error message - Fixed to return HTTP 400 instead of 500 for missing parameters - Fixed to return HTTP 400 for invalid token length
1 parent 41fae5a commit 9ecf277

File tree

5 files changed

+33
-9
lines changed

5 files changed

+33
-9
lines changed

dev/tests/api-functional/testsuite/Magento/Webapi/Authentication/RestTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public function testGetAccessTokenConsumerMismatch()
175175

176176
/**
177177
* @expectedException \Exception
178-
* @expectedExceptionMessage HTTP/1.1 401
178+
* @expectedExceptionMessage HTTP/1.1 400
179179
*/
180180
public function testAccessApiInvalidAccessToken()
181181
{

dev/tests/unit/testsuite/Magento/Integration/Oauth/OauthTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ public function testGetRequestTokenConsumerKeyNotFound()
217217
public function testGetRequestTokenOutdatedConsumerKey()
218218
{
219219
$this->_setupConsumer();
220+
$this->_setupNonce();
220221
$this->_consumerMock
221222
->expects($this->any())
222223
->method('isValidForTokenExchange')
@@ -521,7 +522,7 @@ public function testGetAccessTokenParameterAbsent()
521522
/**
522523
* \Magento\Framework\Oauth\OauthInterface::ERR_TOKEN_REJECTED
523524
*
524-
* @expectedException \Magento\Framework\Oauth\Exception
525+
* @expectedException \Magento\Framework\Oauth\OauthInputException
525526
*/
526527
public function testGetAccessTokenTokenRejected()
527528
{

lib/internal/Magento/Framework/Oauth/Helper/Request.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,9 @@ public function prepareErrorResponse(\Exception $exception, \Zend_Controller_Res
196196
$responseCode = self::HTTP_UNAUTHORIZED;
197197
} elseif ($exception instanceof \Magento\Framework\Oauth\OauthInputException) {
198198
$responseCode = self::HTTP_BAD_REQUEST;
199+
if ($errorMsg == \Magento\Framework\Oauth\OauthInputException::DEFAULT_MESSAGE) {
200+
$errorMsg = $exception->getAggregatedErrorMessage();
201+
}
199202
} else {
200203
$errorMsg = 'internal_error&message=' . ($errorMsg ? $errorMsg : 'empty_message');
201204
$responseCode = self::HTTP_INTERNAL_ERROR;

lib/internal/Magento/Framework/Oauth/Oauth.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,9 @@ public static function getSupportedSignatureMethods()
6060
*/
6161
public function getRequestToken($params, $requestUrl, $httpMethod = 'POST')
6262
{
63-
$this->_validateVersionParam($params['oauth_version']);
63+
$this->_validateProtocolParams($params);
6464
$consumer = $this->_tokenProvider->getConsumerByKey($params['oauth_consumer_key']);
6565
$this->_tokenProvider->validateConsumer($consumer);
66-
$this->_nonceGenerator->validateNonce($consumer, $params['oauth_nonce'], $params['oauth_timestamp']);
67-
6866
$this->_validateSignature($params, $consumer->getSecret(), $httpMethod, $requestUrl);
6967

7068
return $this->_tokenProvider->createRequestToken($consumer);
@@ -219,9 +217,9 @@ protected function _validateVersionParam($version)
219217
* @param array $protocolParams
220218
* @param array $requiredParams
221219
* @return void
222-
* @throws Exception|OauthInputException
220+
* @throws OauthInputException
223221
*/
224-
protected function _validateProtocolParams($protocolParams, $requiredParams)
222+
protected function _validateProtocolParams($protocolParams, $requiredParams = [])
225223
{
226224
// validate version if specified.
227225
if (isset($protocolParams['oauth_version'])) {
@@ -246,7 +244,7 @@ protected function _validateProtocolParams($protocolParams, $requiredParams)
246244
$protocolParams['oauth_token']
247245
)
248246
) {
249-
throw new Exception('Token is not the correct length');
247+
throw new OauthInputException('Token is not the correct length');
250248
}
251249

252250
// Validate signature method.
@@ -275,10 +273,14 @@ protected function _validateProtocolParams($protocolParams, $requiredParams)
275273
*/
276274
protected function _checkRequiredParams($protocolParams, $requiredParams)
277275
{
276+
$exception = new OauthInputException();
278277
foreach ($requiredParams as $param) {
279278
if (!isset($protocolParams[$param])) {
280-
throw new OauthInputException(OauthInputException::REQUIRED_FIELD, ['fieldName' => $param]);
279+
$exception->addError(OauthInputException::REQUIRED_FIELD, ['fieldName' => $param]);
281280
}
282281
}
282+
if ($exception->wasErrorAdded()) {
283+
throw $exception;
284+
}
283285
}
284286
}

lib/internal/Magento/Framework/Oauth/OauthInputException.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,22 @@
1212
*/
1313
class OauthInputException extends InputException
1414
{
15+
/**
16+
* Get error messages as a single comma separated string
17+
*
18+
* @return string
19+
*/
20+
public function getAggregatedErrorMessage()
21+
{
22+
$errors = [];
23+
foreach ($this->getErrors() as $error) {
24+
// Clean up any trailing period
25+
$errors[] = rtrim($error->getMessage(), '.');
26+
}
27+
$errorMsg = '';
28+
if (!empty($errors)) {
29+
$errorMsg = implode(', ', $errors);
30+
}
31+
return $errorMsg;
32+
}
1533
}

0 commit comments

Comments
 (0)