Skip to content

Commit

Permalink
Issue #38 Support duplicate notifications.
Browse files Browse the repository at this point in the history
Only the status is checked to determine if this is a duplicate.
If the remove system, or a mittle attacker, is not trying to
change the status, then we do not need to worry about the
duplicate. It does not result in a database update.
  • Loading branch information
judgej committed Apr 3, 2017
1 parent 0273a35 commit 3b6e148
Showing 1 changed file with 21 additions and 8 deletions.
29 changes: 21 additions & 8 deletions src/Academe/SagePay/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function notification($post, $redirect_url)
// Get the transaction record.
// A transaction object should already have been injected to look this up.

if ( $retStatus == 'OK' && $this->getTransactionModel() === null) {
if ($retStatus == 'OK' && $this->getTransactionModel() === null) {
// Internal error.
$retStatus = 'INVALID';
$retStatusDetail = 'Internal error (missing transaction object)';
Expand All @@ -168,11 +168,24 @@ public function notification($post, $redirect_url)
}
}

$duplicate_notification = false;

if ($this->getField('VendorTxCode') !== null) {
// If the stored status is not PENDING, then we *have* had a notification
// for it already.

if ($this->getField('Status') != 'PENDING') {
// Already processed status.
$retStatus = 'INVALID';
$retStatusDetail = 'Transaction has already been processed';
// Issue #38 If this is a duplicate notification, then so long as the
// status is the same as the first time, we can return an "OK". However,
// we do not update the existing record in storage.

if ($this->getField('Status') != $Status) {
// Already processed status, and a different status has been sent.
$retStatus = 'INVALID';
$retStatusDetail = 'Transaction has already been processed';
} else {
$duplicate_notification = true;
}
} elseif ($VPSTxId != $this->getField('VPSTxId')) {
// Mis-matching VPSTxId values.
$retStatus = 'INVALID';
Expand All @@ -185,9 +198,9 @@ public function notification($post, $redirect_url)
}

// With some of the major checks done, let's dig a little deeper into
// the transaction to see if it has been tampered with. The anit-tamper
// the transaction to see if it has been tampered with. The anti-tamper
// checks allows us to used a non-secure connection for the .
if ($retStatus == 'OK') {
if ($retStatus == 'OK' && ! $duplicate_notification) {
// Gather some additional parameters, making sure they are all set (defaulting to '').
// Derive this list from the transaction metadata, with flag "tamper" set.

Expand Down Expand Up @@ -247,7 +260,7 @@ public function notification($post, $redirect_url)
}

// If still a success, then all tests have passed.
if ($retStatus == 'OK') {
if ($retStatus == 'OK' && ! $duplicate_notification) {
// We found a PENDING transaction, so update it.
// We don't want to be updating the local transaction in any other circumstance.
// However, we might want to log the errors somewhere else.
Expand Down Expand Up @@ -280,7 +293,7 @@ public function notification($post, $redirect_url)

// Save the transaction record to local storage.
// We don't want to throw exceptions here; SagePay must get its response.
try{
try {
$this->save();
}
catch (Exception\RuntimeException $e) {
Expand Down

0 comments on commit 3b6e148

Please sign in to comment.