Skip to content

Commit b7f1312

Browse files
committed
Issue #131 Force expected transactionId to be supplied for Form complete.
1 parent fe5f59a commit b7f1312

File tree

3 files changed

+77
-4
lines changed

3 files changed

+77
-4
lines changed

README.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -696,9 +696,12 @@ user's return. This will be at your `returnUrl` endpoint:
696696

697697
```php
698698
// The result will be read and decrypted from the return URL (or failure URL)
699-
// query parameters:
699+
// query parameters.
700+
// You MUST provide the original expected transactionId, which is validated
701+
// against the transactionId provided in the server request.
702+
// This prevents different payments getting mixed up.
700703

701-
$result = $gateway->completeAuthorize()->send();
704+
$result = $gateway->completeAuthorize(['transactionId' => $originalTransactionId])->send();
702705

703706
$result->isSuccessful();
704707
$result->getTransactionReference();

src/Message/Form/CompleteAuthorizeRequest.php

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Omnipay\SagePay\Message\AbstractRequest;
1010
use Omnipay\SagePay\Message\Response as GenericResponse;
1111
use Omnipay\Common\Exception\InvalidResponseException;
12+
use Omnipay\Common\Exception\InvalidRequestException;
1213

1314
class CompleteAuthorizeRequest extends AbstractRequest
1415
{
@@ -72,14 +73,37 @@ public function getData()
7273

7374
/**
7475
* Nothing to send to gateway - we have the result data in the server request.
76+
*
77+
* @throws InvalidResponseException
78+
* @throws InvalidResponseException
7579
*/
7680
public function sendData($data)
7781
{
82+
$this->response = new GenericResponse($this, $data);
83+
84+
// Issue #131: confirm the response is for the transaction ID we are
85+
// expecting, and not replayed from another transaction.
86+
87+
$originalTransactionId = $this->getTransactionId();
88+
$returnedTransactionId = $this->response->getTransactionId();
89+
90+
if (empty($originalTransactionId)) {
91+
throw new InvalidRequestException('Missing expected transactionId parameter');
92+
}
93+
94+
if ($originalTransactionId !== $returnedTransactionId) {
95+
throw new InvalidResponseException(sprintf(
96+
'Unexpected transactionId; expected "%s" received "%s"',
97+
$originalTransactionId,
98+
$returnedTransactionId
99+
));
100+
}
101+
78102
// The Response in the current namespace conflicts with
79103
// the Response in the namespace one level down, but only
80104
// for PHP 5.6. This alias works around it.
81105

82-
return $this->response = new GenericResponse($this, $data);
106+
return $this->response;
83107
}
84108

85109
/**

tests/FormGatewayTest.php

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public function setUp()
3434

3535
$this->completePurchaseOptions = [
3636
'encryptionKey' => '2f52208a25a1facf',
37+
'transactionId' => 'phpne-demo-53922585',
3738
];
3839
}
3940

@@ -235,7 +236,12 @@ public function testCompletePurchaseSuccess()
235236

236237
$this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
237238

238-
$response = $this->gateway->completePurchase($this->completePurchaseOptions)->send();
239+
$options = $this->completePurchaseOptions;
240+
241+
// Switch to the transaction ID actually encrypted in the server request.
242+
$options['transactionId'] = 'phpne-demo-56260425';
243+
244+
$response = $this->gateway->completePurchase($options)->send();
239245

240246
$this->assertTrue($response->isSuccessful());
241247
$this->assertFalse($response->isRedirect());
@@ -266,4 +272,44 @@ public function testCompletePurchaseSuccess()
266272
$response->getData()
267273
);
268274
}
275+
276+
/**
277+
* The wrong transaction ID is supplied with the server request.
278+
*
279+
* @expectedException Omnipay\Common\Exception\InvalidResponseException
280+
*/
281+
public function testCompletePurchaseReplayAttack()
282+
{
283+
//$this->expectException(Complicated::class);
284+
285+
// Set the "crypt" query parameter.
286+
287+
$this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
288+
289+
// These options contain a different transactionId from the once expected.
290+
291+
$options = $this->completePurchaseOptions;
292+
293+
$response = $this->gateway->completePurchase($options)->send();
294+
}
295+
296+
/**
297+
* The missing expected transaction ID supplied by the app.
298+
*
299+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
300+
*/
301+
public function testCompletePurchaseMissingExpectedParam()
302+
{
303+
//$this->expectException(Complicated::class);
304+
305+
// Set the "crypt" query parameter.
306+
307+
$this->getHttpRequest()->initialize(['crypt' => '@5548276239c33e937e4d9d847d0a01f4c05f1b71dd5cd32568b6985a6d6834aca672315cf3eec01bb20d34cd1ccd7bdd69a9cd89047f7f875103b46efd8f7b97847eea6b6bab5eb8b61da9130a75fffa1c9152b7d39f77e534ea870281b8e280ea1fdbd49a8f5a7c67d1f512fe7a030e81ae6bd2beed762ad074edcd5d7eb4456a6797911ec78e4d16e7d3ac96b919052a764b7ee4940fd6976346608ad8fed1eb6b0b14d84d802c594b3fd94378a26837df66b328f01cfd144f2e7bc166370bf7a833862173412d2798e8739ee7ef9b0568afab0fc69f66af19864480bf3e74fe2fd2043ec90396e40ab62dc9c1f32dee0e309af7561d2286380ebb497105bde2860d401ccfb4cfcd7047ad32e9408d37f5d0fe9a67bd964d5b138b2546a7d54647467c59384eaa20728cf240c460e36db68afdcf0291135f9d5ff58563f14856fd28534a5478ba2579234b247d0d5862c5742495a2ae18c5ca0d6461d641c5215b07e690280fa3eaf5d392e1d6e2791b181a500964d4bc6c76310e47468ae72edddc3c04d83363514c908624747118']);
308+
309+
$options = $this->completePurchaseOptions;
310+
311+
unset($options['transactionId']);
312+
313+
$response = $this->gateway->completePurchase($options)->send();
314+
}
269315
}

0 commit comments

Comments
 (0)