Skip to content

Commit

Permalink
Signed queries don't break the request for other middlewares
Browse files Browse the repository at this point in the history
`$request->getBody()->getContents()` consumes the body without rewinding.
Repeated calls, by other middlewares, will return empty string. So
other middlewares will think the whole request is empty, and GraphQL
will throw.

To avoid this, we should rewind the stream. However, not all streams are
rewindable and would throw exceptions. To avoid that, we return a new
request identical to original with a brand-new stream for body.
  • Loading branch information
PowerKiKi committed Nov 11, 2023
1 parent c5c7e6b commit 89a55ce
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
38 changes: 26 additions & 12 deletions src/Middleware/SignedQueryMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Cake\Chronos\Chronos;
use Ecodev\Felix\Validator\IPRange;
use Exception;
use Laminas\Diactoros\CallbackStream;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
Expand Down Expand Up @@ -44,18 +45,18 @@ public function __construct(
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
if ($this->required) {
$this->verify($request);
$request = $this->verify($request);
}

return $handler->handle($request);
}

private function verify(ServerRequestInterface $request): void
private function verify(ServerRequestInterface $request): ServerRequestInterface
{
$signature = $request->getHeader('X-Signature')[0] ?? '';
if (!$signature) {
if ($this->isAllowedIp($request)) {
return;
return $request;
}

throw new Exception('Missing `X-Signature` HTTP header in signed query', 403);
Expand All @@ -66,10 +67,11 @@ private function verify(ServerRequestInterface $request): void
$hash = $m['hash'];

$this->verifyTimestamp($timestamp);
$this->verifyHash($request, $timestamp, $hash);
} else {
throw new Exception('Invalid `X-Signature` HTTP header in signed query', 403);

return $this->verifyHash($request, $timestamp, $hash);
}

throw new Exception('Invalid `X-Signature` HTTP header in signed query', 403);
}

private function verifyTimestamp(string $timestamp): void
Expand All @@ -83,33 +85,45 @@ private function verifyTimestamp(string $timestamp): void
}
}

private function verifyHash(ServerRequestInterface $request, string $timestamp, string $hash): void
private function verifyHash(ServerRequestInterface $request, string $timestamp, string $hash): ServerRequestInterface
{
$operations = $this->getOperations($request);
['request' => $request, 'operations' => $operations] = $this->getOperations($request);
$payload = $timestamp . $operations;

foreach ($this->keys as $key) {
$computedHash = hash_hmac('sha256', $payload, $key);
if ($hash === $computedHash) {
return;
return $request;
}
}

throw new Exception('Invalid signed query', 403);
}

private function getOperations(ServerRequestInterface $request): mixed
/**
* @return array{request: ServerRequestInterface, operations: string}
*/
private function getOperations(ServerRequestInterface $request): array
{
$contents = $request->getBody()->getContents();

if ($contents) {
return $contents;
return [
// Pseudo-rewind the request, even if non-rewindable, so the next
// middleware still accesses the stream from the beginning
'request' => $request->withBody(new CallbackStream(fn () => $contents)),
'operations' => $contents,
];
}

$parsedBody = $request->getParsedBody();
if (is_array($parsedBody)) {
$operations = $parsedBody['operations'] ?? null;
if ($operations) {
return $operations;
return [
'request' => $request,
'operations' => $operations,
];
}
}

Expand Down
7 changes: 6 additions & 1 deletion tests/Middleware/SignedQueryMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Laminas\Diactoros\Response;
use Laminas\Diactoros\ServerRequest;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;

class SignedQueryMiddlewareTest extends TestCase
Expand Down Expand Up @@ -59,7 +60,11 @@ private function process(array $keys, bool $required, string $ip, string $body,
$handler = $this->createMock(RequestHandlerInterface::class);
$handler->expects($expectExceptionMessage ? self::never() : self::once())
->method('handle')
->willReturn(new Response());
->willReturnCallback(function (ServerRequestInterface $incomingRequest) use ($body) {
self::assertSame($body, $incomingRequest->getBody()->getContents(), 'the original body content is still available for next middlewares');

return new Response();
});

$middleware = new SignedQueryMiddleware($keys, ['1.2.3.4', '2a01:198:603:0::/65'], $required);

Expand Down

0 comments on commit 89a55ce

Please sign in to comment.