Skip to content

Commit c0eaedc

Browse files
glaubinixglaubinix
authored
Merge pull request #90 from packagist/api-signature-v2-cover-query-params
Sign URL query parameters in API request signatures (v2)
2 parents b658e61 + a0e5893 commit c0eaedc

2 files changed

Lines changed: 112 additions & 2 deletions

File tree

‎src/HttpClient/Plugin/RequestSignature.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ class RequestSignature implements Plugin
1616
{
1717
use Plugin\VersionBridgePlugin;
1818

19+
const SIGNATURE_VERSION = '2';
20+
1921
/** @var string */
2022
private $key;
2123
/** @var string */
@@ -48,6 +50,8 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca
4850
'key' => $this->key,
4951
'timestamp' => $this->getTimestamp(),
5052
'cnonce' => $this->getNonce(),
53+
'version' => self::SIGNATURE_VERSION,
54+
'query' => $this->normalizeQueryString($request->getUri()->getQuery()),
5155
];
5256

5357
$content = (string) $request->getBody();
@@ -56,10 +60,11 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca
5660
}
5761

5862
$request = $request->withHeader('Authorization', sprintf(
59-
'PACKAGIST-HMAC-SHA256 Key=%s, Timestamp=%s, Cnonce=%s, Signature=%s',
63+
'PACKAGIST-HMAC-SHA256 Key=%s, Timestamp=%s, Cnonce=%s, Version=%s, Signature=%s',
6064
$params['key'],
6165
$params['timestamp'],
6266
$params['cnonce'],
67+
$params['version'],
6368
$this->generateSignature($request, $params)
6469
));
6570

@@ -94,4 +99,21 @@ private function normalizeParameters(array $params)
9499

95100
return http_build_query($params, '', '&', PHP_QUERY_RFC3986);
96101
}
102+
103+
/**
104+
* @param string $queryString
105+
* @return string
106+
*/
107+
private function normalizeQueryString($queryString)
108+
{
109+
if ($queryString === '') {
110+
return '';
111+
}
112+
113+
$queryParams = [];
114+
parse_str($queryString, $queryParams);
115+
uksort($queryParams, 'strcmp');
116+
117+
return http_build_query($queryParams, '', '&', PHP_QUERY_RFC3986);
118+
}
97119
}

‎tests/HttpClient/Plugin/RequestSignatureTest.php‎

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace PrivatePackagist\ApiClient\HttpClient\Plugin;
1111

1212
use GuzzleHttp\Psr7\Request;
13+
use Psr\Http\Message\RequestInterface;
1314

1415
class RequestSignatureTest extends PluginTestCase
1516
{
@@ -39,7 +40,7 @@ public function testPrefixRequestPath()
3940
'POST',
4041
'/packages/?foo=bar',
4142
[
42-
'Authorization' => ["PACKAGIST-HMAC-SHA256 Key={$this->key}, Timestamp={$this->timestamp}, Cnonce={$this->nonce}, Signature=a6wxBLYrmz4Mwmv/TKBZR5WHFcSCRbsny2frobJMt24="],
43+
'Authorization' => ["PACKAGIST-HMAC-SHA256 Key={$this->key}, Timestamp={$this->timestamp}, Cnonce={$this->nonce}, Version=2, Signature=rzwvwGS17Qcmk8UqTefJCHCV188x1/e1iBWG2pB4z1M="],
4344
],
4445
json_encode(['foo' => 'bar'])
4546
);
@@ -49,6 +50,93 @@ public function testPrefixRequestPath()
4950
$this->assertEquals($expected->getHeaders(), $promise->wait(true)->getHeaders());
5051
}
5152

53+
public function testSignatureCoversQueryString()
54+
{
55+
$requestA = new Request('GET', '/packages/?page=1&limit=10');
56+
$requestB = new Request('GET', '/packages/?page=2&limit=10');
57+
58+
$signatureA = $this->extractSignature($this->plugin->handleRequest($requestA, $this->next, $this->first)->wait(true));
59+
$signatureB = $this->extractSignature($this->plugin->handleRequest($requestB, $this->next, $this->first)->wait(true));
60+
61+
$this->assertNotSame($signatureA, $signatureB);
62+
}
63+
64+
public function testSignatureIgnoresQueryParamOrder()
65+
{
66+
$requestA = new Request('GET', '/packages/?page=1&limit=10');
67+
$requestB = new Request('GET', '/packages/?limit=10&page=1');
68+
69+
$signatureA = $this->extractSignature($this->plugin->handleRequest($requestA, $this->next, $this->first)->wait(true));
70+
$signatureB = $this->extractSignature($this->plugin->handleRequest($requestB, $this->next, $this->first)->wait(true));
71+
72+
$this->assertSame($signatureA, $signatureB);
73+
}
74+
75+
public function testQueryParamWithAuthFieldNameDoesNotShadowAuthIdentity()
76+
{
77+
$request = new Request('GET', '/packages/?key=evil&version=1');
78+
79+
$header = $this->plugin->handleRequest($request, $this->next, $this->first)->wait(true)->getHeader('Authorization')[0];
80+
81+
$this->assertStringContainsString("Key={$this->key}", $header);
82+
$this->assertStringContainsString('Version=2', $header);
83+
}
84+
85+
public function testEmptyQueryStringMatchesNoQueryString()
86+
{
87+
$withoutQuery = new Request('GET', '/packages/');
88+
$withEmptyQuery = new Request('GET', '/packages/?');
89+
90+
$signatureA = $this->extractSignature($this->plugin->handleRequest($withoutQuery, $this->next, $this->first)->wait(true));
91+
$signatureB = $this->extractSignature($this->plugin->handleRequest($withEmptyQuery, $this->next, $this->first)->wait(true));
92+
93+
$this->assertSame($signatureA, $signatureB);
94+
}
95+
96+
public function testEmptyValueQueryParamIsSigned()
97+
{
98+
$withoutParam = new Request('GET', '/packages/');
99+
$withEmptyValue = new Request('GET', '/packages/?foo=');
100+
$withValuelessParam = new Request('GET', '/packages/?foo');
101+
102+
$baseline = $this->extractSignature($this->plugin->handleRequest($withoutParam, $this->next, $this->first)->wait(true));
103+
$emptyValue = $this->extractSignature($this->plugin->handleRequest($withEmptyValue, $this->next, $this->first)->wait(true));
104+
$valueless = $this->extractSignature($this->plugin->handleRequest($withValuelessParam, $this->next, $this->first)->wait(true));
105+
106+
$this->assertNotSame($baseline, $emptyValue);
107+
$this->assertSame($emptyValue, $valueless);
108+
}
109+
110+
public function testUrlEncodingIsCanonical()
111+
{
112+
$percent = new Request('GET', '/packages/?foo=hello%20world');
113+
$plus = new Request('GET', '/packages/?foo=hello+world');
114+
115+
$signaturePercent = $this->extractSignature($this->plugin->handleRequest($percent, $this->next, $this->first)->wait(true));
116+
$signaturePlus = $this->extractSignature($this->plugin->handleRequest($plus, $this->next, $this->first)->wait(true));
117+
118+
$this->assertSame($signaturePercent, $signaturePlus);
119+
}
120+
121+
public function testV2GetDeterministicSignature()
122+
{
123+
$request = new Request('GET', 'https://localhost/api/packages/?limit=1&page=2');
124+
$expected = "PACKAGIST-HMAC-SHA256 Key={$this->key}, Timestamp={$this->timestamp}, Cnonce={$this->nonce}, Version=2, Signature=";
125+
126+
$actual = $this->plugin->handleRequest($request, $this->next, $this->first)->wait(true)->getHeader('Authorization')[0];
127+
128+
$this->assertStringStartsWith($expected, $actual);
129+
$this->assertStringEndsWith('Signature=RLb/mPCONIcfPp3+Ink+jtxNU6VKyeasf7Zdd7kNO+A=', $actual);
130+
}
131+
132+
private function extractSignature(RequestInterface $request)
133+
{
134+
$header = $request->getHeader('Authorization')[0];
135+
preg_match('/Signature=([^,]+)/', $header, $matches);
136+
137+
return $matches[1];
138+
}
139+
52140
public function testPrefixRequestPathSmoke()
53141
{
54142
$request = new Request('POST', '/packages/?foo=bar', [], json_encode(['foo' => 'bar']));

0 commit comments

Comments
 (0)