Signed queries don't break the request for other middlewares

`$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.

Author: PowerKiKi

src/Middleware/SignedQueryMiddleware.php

@@ -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;
@@ -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);
@@ -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
@@ -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,
+                ];
             }
         }
 

tests/Middleware/SignedQueryMiddlewareTest.php

@@ -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
@@ -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);