From 6085f51e84b90fd414b09438d7752f32931b006d Mon Sep 17 00:00:00 2001 From: Jeroen Vermeulen Date: Mon, 8 Jan 2024 18:38:31 +0100 Subject: [PATCH 1/4] Recursively encode and sort URL parameters before signing. Fixes #596 --- src/OAuth/OAuth1/Signature/Signature.php | 30 +++++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/OAuth/OAuth1/Signature/Signature.php b/src/OAuth/OAuth1/Signature/Signature.php index 23711d31..990d6612 100644 --- a/src/OAuth/OAuth1/Signature/Signature.php +++ b/src/OAuth/OAuth1/Signature/Signature.php @@ -53,11 +53,7 @@ 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 = $this->urlEncodeArray(array_merge($queryStringData, $params)); // determine base uri $baseUri = $uri->getScheme() . '://' . $uri->getRawAuthority(); @@ -70,11 +66,12 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST') $baseString = strtoupper($method) . '&'; $baseString .= rawurlencode($baseUri) . '&'; - $baseString .= rawurlencode($this->buildSignatureDataString($signatureData)); + $baseString .= rawurlencode(http_build_query($signatureData, '', '&')); return base64_encode($this->hash($baseString)); } + /** * @return string */ @@ -120,4 +117,25 @@ protected function hash($data) ); } } + + /** + * URL encodes both keys and values in an array, recusively. + * Sorts the array by key after encoding. + * + * @param array $data + * @return array + */ + protected function urlEncodeArray(array $data) + { + $result = []; + foreach ($data as $key => $value) { + if (is_array($value)) { + $result[rawurlencode($key)] = $this->rawUrlEncodeArray($value); + } else { + $result[rawurlencode($key)] = rawurlencode($value); + } + } + ksort($result); + return $result; + } } From 54b07ee1cc9576bfe449d8ad75f4fd705d0acf1a Mon Sep 17 00:00:00 2001 From: Jeroen Vermeulen Date: Mon, 8 Jan 2024 18:50:30 +0100 Subject: [PATCH 2/4] Leave the work of URL encoding to http_build_query() --- src/OAuth/OAuth1/Signature/Signature.php | 26 ++---------------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/OAuth/OAuth1/Signature/Signature.php b/src/OAuth/OAuth1/Signature/Signature.php index 990d6612..6349939b 100644 --- a/src/OAuth/OAuth1/Signature/Signature.php +++ b/src/OAuth/OAuth1/Signature/Signature.php @@ -53,7 +53,7 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST') { parse_str($uri->getQuery(), $queryStringData); - $signatureData = $this->urlEncodeArray(array_merge($queryStringData, $params)); + $signatureData = array_merge($queryStringData, $params); // determine base uri $baseUri = $uri->getScheme() . '://' . $uri->getRawAuthority(); @@ -66,12 +66,11 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST') $baseString = strtoupper($method) . '&'; $baseString .= rawurlencode($baseUri) . '&'; - $baseString .= rawurlencode(http_build_query($signatureData, '', '&')); + $baseString .= http_build_query($signatureData, '', '&', PHP_QUERY_RFC3986); return base64_encode($this->hash($baseString)); } - /** * @return string */ @@ -117,25 +116,4 @@ protected function hash($data) ); } } - - /** - * URL encodes both keys and values in an array, recusively. - * Sorts the array by key after encoding. - * - * @param array $data - * @return array - */ - protected function urlEncodeArray(array $data) - { - $result = []; - foreach ($data as $key => $value) { - if (is_array($value)) { - $result[rawurlencode($key)] = $this->rawUrlEncodeArray($value); - } else { - $result[rawurlencode($key)] = rawurlencode($value); - } - } - ksort($result); - return $result; - } } From 49d98d597add063c07a6fa9d4fc9cf29c35429e5 Mon Sep 17 00:00:00 2001 From: Jeroen Vermeulen Date: Mon, 8 Jan 2024 19:09:19 +0100 Subject: [PATCH 3/4] Restored double URL Encode Fixes #596 --- src/OAuth/OAuth1/Signature/Signature.php | 37 ++++++++++--------- tests/Unit/OAuth1/Signature/SignatureTest.php | 7 ---- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/OAuth/OAuth1/Signature/Signature.php b/src/OAuth/OAuth1/Signature/Signature.php index 6349939b..afe27fc7 100644 --- a/src/OAuth/OAuth1/Signature/Signature.php +++ b/src/OAuth/OAuth1/Signature/Signature.php @@ -54,6 +54,7 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST') parse_str($uri->getQuery(), $queryStringData); $signatureData = array_merge($queryStringData, $params); + $this->ksortRecursive($signatureData); // determine base uri $baseUri = $uri->getScheme() . '://' . $uri->getRawAuthority(); @@ -66,27 +67,12 @@ public function getSignature(UriInterface $uri, array $params, $method = 'POST') $baseString = strtoupper($method) . '&'; $baseString .= rawurlencode($baseUri) . '&'; - $baseString .= http_build_query($signatureData, '', '&', PHP_QUERY_RFC3986); + // 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 */ @@ -116,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; + } } diff --git a/tests/Unit/OAuth1/Signature/SignatureTest.php b/tests/Unit/OAuth1/Signature/SignatureTest.php index 0f1fd062..c6c3696e 100644 --- a/tests/Unit/OAuth1/Signature/SignatureTest.php +++ b/tests/Unit/OAuth1/Signature/SignatureTest.php @@ -41,7 +41,6 @@ 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 @@ -79,7 +78,6 @@ 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 @@ -117,7 +115,6 @@ 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 @@ -155,7 +152,6 @@ 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 @@ -196,7 +192,6 @@ 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 @@ -237,7 +232,6 @@ 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 @@ -277,7 +271,6 @@ 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 From 2e6691032c807c961af1ac6180efb9642c5c7373 Mon Sep 17 00:00:00 2001 From: Jeroen Vermeulen Date: Mon, 8 Jan 2024 22:41:02 +0100 Subject: [PATCH 4/4] Updated test coverage --- tests/Unit/OAuth1/Signature/SignatureTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Unit/OAuth1/Signature/SignatureTest.php b/tests/Unit/OAuth1/Signature/SignatureTest.php index c6c3696e..b1431a4d 100644 --- a/tests/Unit/OAuth1/Signature/SignatureTest.php +++ b/tests/Unit/OAuth1/Signature/SignatureTest.php @@ -44,6 +44,7 @@ public function testSetTokenSecret(): void * @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 */ @@ -81,6 +82,7 @@ public function testGetSignatureBareUri(): void * @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 */ @@ -118,6 +120,7 @@ public function testGetSignatureWithQueryString(): void * @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 */ @@ -155,6 +158,7 @@ public function testGetSignatureWithAuthority(): void * @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 */ @@ -195,6 +199,7 @@ public function testGetSignatureWithBarePathNonExplicitTrailingHostSlash(): void * @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 */ @@ -235,6 +240,7 @@ public function testGetSignatureWithBarePathWithExplicitTrailingHostSlash(): voi * @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 */ @@ -274,6 +280,7 @@ public function testGetSignatureNoTokenSecretSet(): void * @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 */