Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sign URL with array param #597

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix sign URL with array param #597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6085f51
Recursively encode and sort URL parameters before signing.
jeroenvermeulen Jan 8, 2024
54b07ee
Leave the work of URL encoding to http_build_query()
jeroenvermeulen Jan 8, 2024
49d98d5
Restored double URL Encode
jeroenvermeulen Jan 8, 2024
2e66910
Updated test coverage
jeroenvermeulen Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 21 additions & 22 deletions src/OAuth/OAuth1/Signature/Signature.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,8 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST')
{
parse_str($uri->getQuery(), $queryStringData);

foreach (array_merge($queryStringData, $params) as $key => $value) {
$signatureData[rawurlencode($key)] = rawurlencode($value);
}

ksort($signatureData);
$signatureData = array_merge($queryStringData, $params);
$this->ksortRecursive($signatureData);

// determine base uri
$baseUri = $uri->getScheme() . '://' . $uri->getRawAuthority();
Expand All @@ -70,27 +67,12 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST')

$baseString = strtoupper($method) . '&';
$baseString .= rawurlencode($baseUri) . '&';
$baseString .= rawurlencode($this->buildSignatureDataString($signatureData));
// The url paramaters are first encoded induvidually by http_build_query, then the result is encoded again.
$baseString .= rawurlencode(http_build_query($signatureData, '', '&', PHP_QUERY_RFC3986));

return base64_encode($this->hash($baseString));
}

/**
* @return string
*/
protected function buildSignatureDataString(array $signatureData)
{
$signatureString = '';
$delimiter = '';
foreach ($signatureData as $key => $value) {
$signatureString .= $delimiter . $key . '=' . $value;

$delimiter = '&';
}

return $signatureString;
}

/**
* @return string
*/
Expand Down Expand Up @@ -120,4 +102,21 @@ protected function hash($data)
);
}
}

/**
* Rescursively sorts an array by key.
* @param string $data
*
* @return string
*/
protected function ksortRecursive(&$array, $sort_flags = SORT_REGULAR) {
if (!is_array($array)) {
return false;
}
ksort($array, $sort_flags);
foreach ($array as &$arr) {
$this->ksortRecursive($arr, $sort_flags);
}
return true;
}
}
14 changes: 7 additions & 7 deletions tests/Unit/OAuth1/Signature/SignatureTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ public function testSetTokenSecret(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -79,10 +79,10 @@ public function testGetSignatureBareUri(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -117,10 +117,10 @@ public function testGetSignatureWithQueryString(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -155,10 +155,10 @@ public function testGetSignatureWithAuthority(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -196,10 +196,10 @@ public function testGetSignatureWithBarePathNonExplicitTrailingHostSlash(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -237,10 +237,10 @@ public function testGetSignatureWithBarePathWithExplicitTrailingHostSlash(): voi

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down Expand Up @@ -277,10 +277,10 @@ public function testGetSignatureNoTokenSecretSet(): void

/**
* @covers \OAuth\OAuth1\Signature\Signature::__construct
* @covers \OAuth\OAuth1\Signature\Signature::buildSignatureDataString
* @covers \OAuth\OAuth1\Signature\Signature::getSignature
* @covers \OAuth\OAuth1\Signature\Signature::getSigningKey
* @covers \OAuth\OAuth1\Signature\Signature::hash
* @covers \OAuth\OAuth1\Signature\Signature::ksortRecursive
* @covers \OAuth\OAuth1\Signature\Signature::setHashingAlgorithm
* @covers \OAuth\OAuth1\Signature\Signature::setTokenSecret
*/
Expand Down