Merge pull request #143 from academe/issue131

Issue131 fix - meant to merge this in a while ago, but been really busy with other things.

Author: judgej

README.md

@@ -34,7 +34,7 @@ Table of Contents
          * [Server Notification Handler](#server-notification-handler)
       * [Sage Pay Form Methods](#sage-pay-form-methods)
          * [Form Authorize](#form-authorize)
-         * [Form completeAuthorise](#form-completeauthorise)
+         * [Form completeAuthorize](#form-completeauthorize)
          * [Form Purchase](#form-purchase)
       * [Sage Pay Shared Methods (Direct and Server)](#sage-pay-shared-methods-direct-and-server)
          * [Repeat Authorize/Purchase](#repeat-authorizepurchase)
@@ -682,26 +682,38 @@ $response = $gateway->authorize([
 ```
 
 The `$response` will be a `POST` redirect, which will take the user to the gateway.
-At the gateway, the user will authenticate or authorise their credit card,
+At the gateway, the user will authenticate or authorize their credit card,
 perform any 3D Secure actions that may be requested, then will return to the
 merchant site.
 
-### Form completeAuthorise
+Like `Server` and `Direct`, you can use either the `DEFERRED` or the `AUTHENTICATE`
+method to reserve the amount.
+
+### Form completeAuthorize
 
 To get the result details, the transaction is "completed" on the
 user's return. This will be at your `returnUrl` endpoint:
 
 ```php
 // The result will be read and decrypted from the return URL (or failure URL)
-// query parameters:
+// query parameters.
+// You MUST provide the original expected transactionId, which is validated
+// against the transactionId provided in the server request.
+// This prevents different payments getting mixed up.
 
-$result = $gateway->completeAuthorize()->send();
+$completeRequest = $gateway->completeAuthorize(['transactionId' => $originalTransactionId]);
+$result = $completeRequest->send();
 
 $result->isSuccessful();
 $result->getTransactionReference();
 // etc.
 ```
 
+Note that if `send()` throws an exception here due to a `transactionId` mismatch,
+you can still access the decryoted data that was brought back with the user as
+`$completeRequest->getData()`.
+You will need to log this for later analysis.
+
 If you already have the encrypted response string, then it can be passed in.
 However, you would normally leave it for the driver to read it for you from
 the current server request, so the following would not normally be necessary:
@@ -728,9 +740,6 @@ In a future release, the `completeAuthorize()` method will expect the
 `transactionId` to be supplied and it must match before it will
 return a success status.
 
-Like `Server` and `Direct`, you can use either the `DEFERRED` or the `AUTHENTICATE`
-method to reserve the amount.
-
 ### Form Purchase
 
 This is the same as `authorize()`, but the `purchase()` request is used instead,

src/Message/Form/CompleteAuthorizeRequest.php

@@ -9,6 +9,7 @@
 use Omnipay\SagePay\Message\AbstractRequest;
 use Omnipay\SagePay\Message\Response as GenericResponse;
 use Omnipay\Common\Exception\InvalidResponseException;
+use Omnipay\Common\Exception\InvalidRequestException;
 
 class CompleteAuthorizeRequest extends AbstractRequest
 {
@@ -72,14 +73,37 @@ public function getData()
 
     /**
      * Nothing to send to gateway - we have the result data in the server request.
+     *
+     * @throws InvalidResponseException
+     * @throws InvalidResponseException
      */
     public function sendData($data)
     {
+        $this->response = new GenericResponse($this, $data);
+
+        // Issue #131: confirm the response is for the transaction ID we are
+        // expecting, and not replayed from another transaction.
+
+        $originalTransactionId = $this->getTransactionId();
+        $returnedTransactionId = $this->response->getTransactionId();
+
+        if (empty($originalTransactionId)) {
+            throw new InvalidRequestException('Missing expected transactionId parameter');
+        }
+
+        if ($originalTransactionId !== $returnedTransactionId) {
+            throw new InvalidResponseException(sprintf(
+                'Unexpected transactionId; expected "%s" received "%s"',
+                $originalTransactionId,
+                $returnedTransactionId
+            ));
+        }
+
         // The Response in the current namespace conflicts with
         // the Response in the namespace one level down, but only
         // for PHP 5.6. This alias works around it.
 
-        return $this->response = new GenericResponse($this, $data);
+        return $this->response;
     }
 
     /**

tests/FormGatewayTest.php

@@ -34,6 +34,7 @@ public function setUp()
 
         $this->completePurchaseOptions = [
             'encryptionKey' => '2f52208a25a1facf',
+            'transactionId' => 'phpne-demo-53922585',
         ];
     }
 
@@ -235,7 +236,12 @@ public function testCompletePurchaseSuccess()
 
         $this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
 
-        $response = $this->gateway->completePurchase($this->completePurchaseOptions)->send();
+        $options = $this->completePurchaseOptions;
+
+        // Switch to the transaction ID actually encrypted in the server request.
+        $options['transactionId'] = 'phpne-demo-56260425';
+
+        $response = $this->gateway->completePurchase($options)->send();
 
         $this->assertTrue($response->isSuccessful());
         $this->assertFalse($response->isRedirect());
@@ -266,4 +272,44 @@ public function testCompletePurchaseSuccess()
             $response->getData()
         );
     }
+
+    /**
+     * The wrong transaction ID is supplied with the server request.
+     *
+     * @expectedException Omnipay\Common\Exception\InvalidResponseException
+     */
+    public function testCompletePurchaseReplayAttack()
+    {
+        //$this->expectException(Complicated::class);
+
+        // Set the "crypt" query parameter.
+
+        $this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
+
+        // These options contain a different transactionId from the once expected.
+
+        $options = $this->completePurchaseOptions;
+
+        $response = $this->gateway->completePurchase($options)->send();
+    }
+
+    /**
+     * The missing expected transaction ID supplied by the app.
+     *
+     * @expectedException Omnipay\Common\Exception\InvalidRequestException
+     */
+    public function testCompletePurchaseMissingExpectedParam()
+    {
+        //$this->expectException(Complicated::class);
+
+        // Set the "crypt" query parameter.
+
+        $this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
+
+        $options = $this->completePurchaseOptions;
+
+        unset($options['transactionId']);
+
+        $response = $this->gateway->completePurchase($options)->send();
+    }
 }