Merge pull request #7 from paynl/feature/remove-fallback

Revert "Add fallback verification method to the HMAC implementation"
paynl · Feb 6, 2024 · 8d51401 · 8d51401
2 parents 4c96682 + b34c182
commit 8d51401
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 57 deletions.
diff --git a/README.md b/README.md
@@ -23,12 +23,12 @@ composer require paynl/request-signing
 
 The `PayNL\RequestSigning\RequestSigningService` simplifies both signing and verifying requests, avoiding the need for manual class instantiation. 
 
-The constructor function of this service requires an array of _SingingMethods_ that you wish to support. Each signing method has different needs and functionality, and which one(s) you opt to support falls under your discretion.
+The constructor function of this service requires an array of _SigningMethods_ that you wish to support. Each signing method has different needs and functionality, and which one(s) you opt to support falls under your discretion.
 
 By providing the service with the array of these chosen methods, the service can handle the configuration and functionality. This decouples the setup process from your main application logic, allowing a more streamlined integration.
 
 ### Signing
-The below code snippet shows the basis of singing a request using this package:
+The below code snippet shows the basis of signing a request using this package:
 
 ```php
 use PayNL\RequestSigning\RequestSigningService;
@@ -73,7 +73,7 @@ $requestValid = $signingService->verify($request);
 
 The request signing service and the underlying signing / verifying methods may throw exceptions when unexpected values are encountered, these are:
 - SignatureKeyNotFound, this exception must be thrown when the implementation of the `PayNL\RequestSigning\Repository\SignatureKeyRepositoryInterface` can not find the key based on the provided id;
-- UnknownSigningMethodException, this exception will be thrown by the singing / verifying methods when they are requested to sign / verify a request with an algorithm they do not support;
+- UnknownSigningMethodException, this exception will be thrown by the signing / verifying methods when they are requested to sign / verify a request with an algorithm they do not support;
 - UnsupportedHashingAlgorithmException, this exception will be thrown by the `PayNL\RequestSigning\RequestSigningService` when it is requested to sign / verify a request with a method it doesn't support.
 
 ### Supported Signing / Verifying methods

diff --git a/src/Methods/HmacSignature.php b/src/Methods/HmacSignature.php
@@ -37,7 +37,7 @@ public function __construct(HmacSignatureKeyRepositoryInterface $signatureKeyRep
      */
     public function sign(RequestInterface $request, string $keyId, string $algorithm): RequestInterface
     {
-        $signatureData = $this->generateSignature((string)$request->getBody(), $keyId, $algorithm);
+        $signatureData = $this->generateSignature((string) $request->getBody(), $keyId, $algorithm);
 
         return $request
             ->withHeader(RequestSigningMethodInterface::SIGNATURE_HEADER, $signatureData->getSignature())
@@ -90,10 +90,10 @@ public function verify(RequestInterface $request): bool
             $algorithm = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_ALGORITHM_HEADER);
 
             $generatedSignature = $this
-                ->generateSignature((string)$request->getBody(), $keyId, $algorithm)
+                ->generateSignature((string) $request->getBody(), $keyId, $algorithm)
                 ->getSignature();
 
-            return hash_equals($signature, $generatedSignature) || $this->doFallbackVerification($request);
+            return hash_equals($signature, $generatedSignature);
         } catch (Throwable $throwable) {
             // We do nothing with the exception. The request stays marked as "invalid".
         }
@@ -105,40 +105,4 @@ public function supports(string $method): bool
     {
         return strtolower(self::METHOD_NAME) === strtolower($method);
     }
-
-    /**
-     * Since we've moved away from hash() signing in favor of hash_hmac we should (for now) support the
-     * hash verification method to keep older implementations valid
-     *
-     * @throws UnsupportedHashingAlgorithmException
-     * @throws SignatureKeyNotFoundException
-     */
-    private function doFallbackVerification(RequestInterface $request): bool
-    {
-        $keyId = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_KEY_ID_HEADER);
-        $signature = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_HEADER);
-        $algorithm = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_ALGORITHM_HEADER);
-
-        $key = $this->signatureKeyRepository->findOneById($keyId);
-
-        $algorithm = strtolower($algorithm);
-
-        if (in_array($algorithm, hash_algos()) === false) {
-            throw UnsupportedHashingAlgorithmException::forAlgorithm($algorithm);
-        }
-
-        $generatedSignature = hash(
-            $algorithm,
-            implode(
-                ':',
-                [
-                    $key->getId(),
-                    $key->getSecret(),
-                    (string)$request->getBody(),
-                ]
-            )
-        );
-
-        return hash_equals($signature, $generatedSignature);
-    }
 }
diff --git a/tests/Unit/Methods/HmacSignatureTest.php b/tests/Unit/Methods/HmacSignatureTest.php
@@ -122,21 +122,6 @@ public function testItReturnsFalseForAnHashingAlgorithm(): void
         $this->assertFalse($hmacSignature->verify($request));
     }
 
-    public function testItCanVerifyUsingTheFallbackMethod(): void
-    {
-        // First, we'll create a dummy request
-        $request = $this->getSignedDummyRequest()->withHeader(
-            RequestSigningMethodInterface::SIGNATURE_HEADER,
-            '6086a395c7fe9b47f47cee4623b76cc4cc79cb3f44e968091edc447cfe7e7533dc2564f49b22bc1f225c24f172be0d2a5b5893e0825c6fde782a63f3e43ce7d1'
-        );
-
-        // Next, we'll instantiate the HmacSignature class
-        $hmacSignature = new HmacSignature($this->getKeyRepository($this->getDummySignatureKey()));
-
-        // Next, we'll verify that this request passes its verification
-        $this->assertTrue($hmacSignature->verify($request));
-    }
-
     private function getDummyRequest(): RequestInterface
     {
         return new Request('POST', 'https://pay.nl');