Skip to content

Commit f05a2ee

Browse files
PowerKiKiPowerKiKi
committed
SignedQueryMiddleware includes the key name in request attribute
The attribute will only be present if a key was actually used successfully. Allowed IP or bots usage will not include it.
1 parent 20a6986 commit f05a2ee

File tree

2 files changed

+65
-13
lines changed

2 files changed

+65
-13
lines changed

‎src/Middleware/SignedQueryMiddleware.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
*
2222
* The signature is valid for a limited time only, ~15 minutes.
2323
*
24+
* If valid the `$request` will have a new attribute `"Ecodev\Felix\Middleware\SignedQueryMiddleware"` whose value is
25+
* the name (`int|string`) of the key used to successfully validate the request.
26+
*
2427
* The signature syntax is:
2528
*
2629
* ```ebnf
@@ -91,10 +94,10 @@ private function verifyHash(ServerRequestInterface $request, string $timestamp,
9194
['request' => $request, 'operations' => $operations] = $this->getOperations($request);
9295
$payload = $timestamp . $operations;
9396

94-
foreach ($this->keys as $key) {
95-
$computedHash = hash_hmac('sha256', $payload, $key);
97+
foreach ($this->keys as $name => $value) {
98+
$computedHash = hash_hmac('sha256', $payload, $value);
9699
if ($hash === $computedHash) {
97-
return $request;
100+
return $request->withAttribute($this::class, $name);
98101
}
99102
}
100103

‎tests/Middleware/SignedQueryMiddlewareTest.php‎

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,28 @@ protected function tearDown(): void
2828
/**
2929
* @dataProvider dataProviderQuery
3030
*/
31-
public function testRequiredSignedQuery(array $keys, string $body, ?array $parsedBody, string $signature, string $expectExceptionMessage = '', string $ip = ''): void
32-
{
33-
$this->process($keys, true, $ip, $body, $parsedBody, $signature, $expectExceptionMessage);
31+
public function testRequiredSignedQuery(
32+
array $keys,
33+
string $body,
34+
?array $parsedBody,
35+
string $signature,
36+
string|int|null $keyName = null,
37+
string $expectExceptionMessage = '',
38+
string $ip = '',
39+
): void {
40+
$this->process($keys, true, $ip, $body, $parsedBody, $signature, $keyName, $expectExceptionMessage);
3441
}
3542

3643
/**
3744
* @dataProvider dataProviderQuery
3845
*/
39-
public function testNonRequiredSignedQuery(array $keys, string $body, ?array $parsedBody, string $signature): void
40-
{
41-
$this->process($keys, false, '', $body, $parsedBody, $signature, '');
46+
public function testNonRequiredSignedQuery(
47+
array $keys,
48+
string $body,
49+
?array $parsedBody,
50+
string $signature,
51+
): void {
52+
$this->process($keys, false, '', $body, $parsedBody, $signature, null, '');
4253
}
4354

4455
public static function dataProviderQuery(): iterable
@@ -51,13 +62,23 @@ public static function dataProviderQuery(): iterable
5162
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
5263
null,
5364
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
65+
0,
66+
];
67+
68+
yield 'simple with key name' => [
69+
['my custom key name' => $key1],
70+
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
71+
null,
72+
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
73+
'my custom key name',
5474
];
5575

5676
yield 'simple but wrong key' => [
5777
[$key2],
5878
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
5979
null,
6080
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
81+
null,
6182
'Invalid signed query',
6283
];
6384

@@ -66,20 +87,23 @@ public static function dataProviderQuery(): iterable
6687
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
6788
null,
6889
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
90+
1,
6991
];
7092

7193
yield 'simple but slightly in the past' => [
7294
[$key1],
7395
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
7496
null,
7597
'v1.1577951100.7d3b639703584e3ea4c68b30a37b56bcf94d19ccdc11c7f05a737c4e7e663a6c',
98+
0,
7699
];
77100

78101
yield 'simple but too much in the past' => [
79102
[$key1],
80103
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
81104
null,
82105
'v1.1577951099.' . str_repeat('a', 64),
106+
null,
83107
'Signed query is expired',
84108
];
85109

@@ -88,13 +112,15 @@ public static function dataProviderQuery(): iterable
88112
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
89113
null,
90114
'v1.1577978100.b6fb50cd1aa3974ec9df0c320bf32ff58a28f6fc2040aa13e529a7ef57212e49',
115+
0,
91116
];
92117

93118
yield 'simple but too much in the future' => [
94119
[$key1],
95120
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
96121
null,
97122
'v1.1577978101.' . str_repeat('a', 64),
123+
null,
98124
'Signed query is expired',
99125
];
100126

@@ -103,7 +129,7 @@ public static function dataProviderQuery(): iterable
103129
'[{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }},{"operationName":"Configuration","variables":{"key":"announcement-active"},"query":"query Configuration($key: String!) { configuration(key: $key)}"}]',
104130
null,
105131
'v1.1577964600.566fafed794d956d662662b0df3d88e5c0a1e52e19111c08cc122f64a54bd8ec',
106-
132+
0,
107133
];
108134

109135
yield 'file upload' => [
@@ -114,6 +140,7 @@ public static function dataProviderQuery(): iterable
114140
'map' => '{"1":["variables.input.file"]}',
115141
],
116142
'v1.1577964600.69dd1f396016e284afb221966ae5e61323a23222f2ad2a5086e4ba2354f99e58',
143+
0,
117144
];
118145

119146
yield 'file upload will ignore map and uploaded file to sign' => [
@@ -125,13 +152,15 @@ public static function dataProviderQuery(): iterable
125152
1 => 'fake file',
126153
],
127154
'v1.1577964600.69dd1f396016e284afb221966ae5e61323a23222f2ad2a5086e4ba2354f99e58',
155+
0,
128156
];
129157

130158
yield 'no header' => [
131159
[$key1],
132160
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
133161
null,
134162
'',
163+
null,
135164
'Missing `X-Signature` HTTP header in signed query',
136165
];
137166

@@ -140,6 +169,7 @@ public static function dataProviderQuery(): iterable
140169
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
141170
null,
142171
'foo',
172+
null,
143173
'Invalid `X-Signature` HTTP header in signed query',
144174
];
145175

@@ -148,6 +178,7 @@ public static function dataProviderQuery(): iterable
148178
'',
149179
null,
150180
'v1.1577964600.' . str_repeat('a', 64),
181+
null,
151182
'Invalid signed query',
152183
];
153184

@@ -156,6 +187,7 @@ public static function dataProviderQuery(): iterable
156187
'',
157188
null,
158189
'v1.1577964600.ff8a9f2bc8090207b824d88251ed8e9d39434607d86e0f0b2837c597d6642c26',
190+
0,
159191
'',
160192
];
161193

@@ -164,6 +196,7 @@ public static function dataProviderQuery(): iterable
164196
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
165197
null,
166198
'',
199+
'no-attribute-at-all',
167200
'',
168201
'1.2.3.4',
169202
];
@@ -173,6 +206,7 @@ public static function dataProviderQuery(): iterable
173206
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
174207
null,
175208
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
209+
null,
176210
'Invalid signed query',
177211
'1.2.3.4',
178212
];
@@ -182,6 +216,7 @@ public static function dataProviderQuery(): iterable
182216
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
183217
null,
184218
'',
219+
null,
185220
'Missing `X-Signature` HTTP header in signed query',
186221
'66.249.70.134',
187222
];
@@ -191,6 +226,7 @@ public static function dataProviderQuery(): iterable
191226
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
192227
null,
193228
'v1.1577951099.20177a7face4e05a75c4b2e41bc97a8225f420f5b7bb1709dd5499821dba0807',
229+
0,
194230
'',
195231
'66.249.70.134',
196232
];
@@ -200,6 +236,7 @@ public static function dataProviderQuery(): iterable
200236
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
201237
null,
202238
'v1.1577951099' . str_repeat('a', 64),
239+
null,
203240
'Invalid `X-Signature` HTTP header in signed query',
204241
'66.249.70.134',
205242
];
@@ -209,6 +246,7 @@ public static function dataProviderQuery(): iterable
209246
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
210247
null,
211248
'',
249+
'no-attribute-at-all',
212250
'',
213251
'40.77.188.165',
214252
];
@@ -218,6 +256,7 @@ public static function dataProviderQuery(): iterable
218256
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
219257
null,
220258
'v1.1577951099.20177a7face4e05a75c4b2e41bc97a8225f420f5b7bb1709dd5499821dba0807',
259+
null,
221260
'Signed query is expired',
222261
'40.77.188.165',
223262
];
@@ -227,6 +266,7 @@ public static function dataProviderQuery(): iterable
227266
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
228267
null,
229268
'v1.1577951099' . str_repeat('a', 64),
269+
null,
230270
'Invalid `X-Signature` HTTP header in signed query',
231271
'40.77.188.165',
232272
];
@@ -239,8 +279,16 @@ public function testThrowIfNoKeys(): void
239279
new SignedQueryMiddleware([], []);
240280
}
241281

242-
private function process(array $keys, bool $required, string $ip, string $body, ?array $parsedBody, string $signature, string $expectExceptionMessage): void
243-
{
282+
private function process(
283+
array $keys,
284+
bool $required,
285+
string $ip,
286+
string $body,
287+
?array $parsedBody,
288+
string $signature,
289+
string|int|null $keyName,
290+
string $expectExceptionMessage,
291+
): void {
244292
$request = new ServerRequest(['REMOTE_ADDR' => $ip]);
245293
$request = $request->withBody(new CallbackStream(fn () => $body))->withParsedBody($parsedBody);
246294

@@ -251,8 +299,9 @@ private function process(array $keys, bool $required, string $ip, string $body,
251299
$handler = $this->createMock(RequestHandlerInterface::class);
252300
$handler->expects($expectExceptionMessage ? self::never() : self::once())
253301
->method('handle')
254-
->willReturnCallback(function (ServerRequestInterface $incomingRequest) use ($body) {
302+
->willReturnCallback(function (ServerRequestInterface $incomingRequest) use ($required, $body, $keyName) {
255303
self::assertSame($body, $incomingRequest->getBody()->getContents(), 'the original body content is still available for next middlewares');
304+
self::assertSame($required ? $keyName : 'no-attribute-at-all', $incomingRequest->getAttribute(SignedQueryMiddleware::class, 'no-attribute-at-all'), 'the name of the key used');
256305

257306
return new Response();
258307
});

0 commit comments

Comments
 (0)