jenkins-bot has submitted this change and it was merged.
Change subject: Cancel duplicate authorizations for a single order ID
......................................................................
Cancel duplicate authorizations for a single order ID
Donors can somehow submit the Adyen iframe multiple times without
our generating new order IDs / merchant references. If we get
duplicate authorizations, mark the donor details as captured after
capturing the first, then cancel all subsequent authorizations.
We send the pending queue message before the donor sees the credit
card form, so we assume that if no message exists, this is a duplicate
authorization and the original has already been sent to Civi.
Bug: T129935
Change-Id: I2b338164461130c3afd0d91efbbd7cfbeec1b680
---
M CrmLink/Messages/DonationInterfaceMessage.php
M PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
M PaymentProviders/Adyen/Tests/MockAdyenPaymentsAPI.php
M PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
M PaymentProviders/Amazon/Tests/phpunit/ApiTest.php
M Tests/BaseSmashPigUnitTestCase.php
6 files changed, 285 insertions(+), 54 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/CrmLink/Messages/DonationInterfaceMessage.php
b/CrmLink/Messages/DonationInterfaceMessage.php
index 6db0360..3f9c391 100644
--- a/CrmLink/Messages/DonationInterfaceMessage.php
+++ b/CrmLink/Messages/DonationInterfaceMessage.php
@@ -6,6 +6,7 @@
* Message sent to the 'cc-limbo' queue when a payment has been initiated and
sent off to the gateway.
*/
class DonationInterfaceMessage extends KeyedOpaqueStorableObject {
+ public $captured = '';
public $city = '';
public $city_2 = '';
public $comment = '';
diff --git a/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
b/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
index d6d5987..7323071 100644
--- a/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
+++ b/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
@@ -26,10 +26,10 @@
protected $avsResult;
protected $cvvResult;
// Actions to take after examining capture request and queue message
- const ACTION_IGNORE = 'ignore'; // no donor info or auth already
captured, do nothing
const ACTION_PROCESS = 'process'; // all clear to capture payment
const ACTION_REJECT = 'reject'; // very likely fraud - cancel the
authorization
const ACTION_REVIEW = 'review'; // potential fraud - do not capture now
+ const ACTION_DUPLICATE = 'duplicate'; // probable duplicate - cancel
the authorization
public static function factory( Authorisation $authMessage ) {
$obj = new ProcessCaptureRequestJob();
@@ -54,7 +54,11 @@
);
// Determine if a message exists in the pending queue; if it
does not then
- // this payment has already been sent to the verified queue.
+ // this payment has already been sent to the verified queue. If
it does,
+ // we need to check $capture_requested in case we have
requested a capture
+ // but have not yet received notification of capture success.
Either case can
+ // occur when a donor submits their credit card details
multiple times against
+ // a single order ID. We should cancel all the duplicate
authorizations.
Logger::debug( 'Attempting to locate associated message in
pending queue.' );
/**
* @var \SmashPig\Core\DataStores\KeyedOpaqueDataStore
@@ -64,51 +68,58 @@
$success = true;
$action = $this->determineAction( $queueMessage );
- if ( $action == self::ACTION_PROCESS ) {
- // Tell the pending queue to keep the message around
for the RecordCaptureJob
- $pendingQueue->queueIgnoreObject();
-
- // Attempt to capture the payment
- $api = $this->getApi();
- Logger::info(
- "Attempting capture API call for currency
'{$this->currency}', " .
- "amount '{$this->amount}', reference
'{$this->pspReference}'."
- );
- $captureResult = $api->capture( $this->currency,
$this->amount, $this->pspReference );
-
- if ( $captureResult ) {
- // Success!
+ switch( $action ) {
+ case self::ACTION_PROCESS:
+ // Attempt to capture the payment
+ $api = $this->getApi();
Logger::info(
- "Successfully captured payment!
Returned reference: '{$captureResult}'. " .
- 'Leaving pending message in
queue for record capture job.');
- } else {
- // Some kind of error in the request. We should
keep the pending
- // message, complain loudly, and move this
capture job to the
- // damaged queue.
- Logger::error(
- "Failed to capture payment on account
'{$this->account}' with reference " .
- "'{$this->pspReference}' and
correlation id '{$this->correlationId}'.",
- $queueMessage
+ "Attempting capture API call for
currency '{$this->currency}', " .
+ "amount '{$this->amount}', reference
'{$this->pspReference}'."
);
- $success = false;
- }
- } else if ( $action == self::ACTION_REJECT ) {
- Logger::debug( "Cancelling authorization with reference
'{$this->pspReference}'" );
- $api = $this->getApi();
- $result = $api->cancel( $this->pspReference );
- if ( $result ) {
- Logger::debug( "Successfully cancelled
authorization" );
- } else {
- // Not a big deal
- Logger::warning( "Failed to cancel
authorization, it will remain in the payment console" );
- }
- // Delete the queue message whatever happened
- $pendingQueue->queueAckObject();
- $pendingQueue->removeObjectsById( $this->correlationId
);
- } else {
- // Not cancelling, just leaving the authorization in
the console for review.
- // Put the donor details back on the pending queue.
- $pendingQueue->queueIgnoreObject();
+ $captureResult = $api->capture(
$this->currency, $this->amount, $this->pspReference );
+
+ if ( $captureResult ) {
+ // Success!
+ Logger::info(
+ "Successfully captured payment!
Returned reference: '{$captureResult}'. " .
+ 'Marking pending queue
message as captured.'
+ );
+ $pendingQueue->queueAckObject();
+ $queueMessage->captured = true;
+ $pendingQueue->addObject( $queueMessage
);
+ } else {
+ // Some kind of error in the request.
We should keep the pending
+ // message, complain loudly, and move
this capture job to the
+ // damaged queue.
+ Logger::error(
+ "Failed to capture payment on
account '{$this->account}' with reference " .
+
"'{$this->pspReference}' and correlation id '{$this->correlationId}'.",
+ $queueMessage
+ );
+ $pendingQueue->queueIgnoreObject();
+ $success = false;
+ }
+ break;
+ case self::ACTION_REJECT:
+ $this->cancelAuthorization();
+ // Delete the fraudy donor details
+ $pendingQueue->queueAckObject();
+ $pendingQueue->removeObjectsById(
$this->correlationId );
+ break;
+ case self::ACTION_DUPLICATE:
+ // We have already captured one payment for
this donation attempt, so
+ // cancel the duplicate authorization. If there
is a pending message,
+ // leave it intact for the legitimate
RecordCaptureJob.
+ $this->cancelAuthorization();
+ if ( $queueMessage ) {
+ $pendingQueue->queueIgnoreObject();
+ }
+ break;
+ case self::ACTION_REVIEW:
+ // Don't capture the payment right now, but
leave the donor details in
+ // the pending queue in case the authorization
is captured via the console.
+ $pendingQueue->queueIgnoreObject();
+ break;
}
Logger::leaveContext();
@@ -124,7 +135,14 @@
"ID '{$this->correlationId}'.",
$queueMessage
);
- return self::ACTION_IGNORE;
+ return self::ACTION_DUPLICATE;
+ }
+ if ( $queueMessage->captured ) {
+ Logger::info(
+ "Duplicate PSP Reference
'{$this->pspReference}' for correlation ID '{$this->correlationId}'.",
+ $queueMessage
+ );
+ return self::ACTION_DUPLICATE;
}
return $this->getRiskAction( $queueMessage );
}
@@ -178,4 +196,16 @@
$api->setAccount( $this->account );
return $api;
}
+
+ protected function cancelAuthorization() {
+ Logger::debug( "Cancelling authorization with reference
'{$this->pspReference}'" );
+ $api = $this->getApi();
+ $result = $api->cancel( $this->pspReference );
+ if ( $result ) {
+ Logger::debug( "Successfully cancelled authorization" );
+ } else {
+ // Not a big deal
+ Logger::warning( "Failed to cancel authorization, it
will remain in the payment console" );
+ }
+ }
}
diff --git a/PaymentProviders/Adyen/Tests/MockAdyenPaymentsAPI.php
b/PaymentProviders/Adyen/Tests/MockAdyenPaymentsAPI.php
index 4c44a92..04a7eec 100644
--- a/PaymentProviders/Adyen/Tests/MockAdyenPaymentsAPI.php
+++ b/PaymentProviders/Adyen/Tests/MockAdyenPaymentsAPI.php
@@ -7,6 +7,9 @@
protected $account = '';
protected $returnCode = false;
+ public $captured = array();
+ public $cancelled = array();
+
public function __construct( $returnCode ) {
$this->returnCode = $returnCode;
}
@@ -25,6 +28,11 @@
* @returns bool|string The return code set in the constructor.
*/
public function capture( $currency, $amount, $pspReference ) {
+ $this->captured[] = array(
+ 'currency' => $currency,
+ 'amount' => $amount,
+ 'pspReference' => $pspReference,
+ );
return $this->returnCode;
}
@@ -36,6 +44,7 @@
* @returns bool|string The return code set in the constructor.
*/
public function cancel( $pspReference ) {
+ $this->cancelled[] = $pspReference;
return $this->returnCode;
}
}
diff --git a/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
b/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
index 499ab9e..17ec73c 100644
--- a/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
+++ b/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
@@ -14,9 +14,11 @@
* on the pending queue, add an antifraud message, and return true.
*/
public function testSuccessfulCapture() {
- $this->setConfig( __DIR__ . '/../config_test_success.php',
'adyen' );
- $antifraudQueue = Configuration::getDefaultConfig()->object(
'data-store/antifraud', true );
- $pendingQueue = Configuration::getDefaultConfig()->object(
'data-store/pending', true );
+ $config = $this->setConfig( __DIR__ .
'/../config_test_success.php', 'adyen' );
+ $antifraudQueue = $config->object( 'data-store/antifraud', true
);
+ $pendingQueue = $config->object( 'data-store/pending', true );
+ $api = $config->object( 'payment-provider/adyen/api', true );
+
$pendingQueue->addObject(
KeyedOpaqueStorableObject::fromJsonProxy(
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
@@ -27,17 +29,198 @@
'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation',
file_get_contents( __DIR__ . '/../Data/auth.json' )
);
+
$job = ProcessCaptureRequestJob::factory( $auth );
$this->assertTrue( $job->execute() );
+
+ $donorData = $pendingQueue->queueGetObject( null,
$auth->correlationId );
$this->assertNotNull(
- $pendingQueue->queueGetObject( null,
$auth->correlationId ),
+ $donorData,
'RequestCaptureJob did not leave donor data on pending
queue'
);
+ $this->assertTrue(
+ $donorData->captured,
+ 'RequestCaptureJob did not mark donor data as captured'
+ );
+
+ $this->assertEquals(
+ array(
+ 'currency' => 'USD',
+ 'amount' => 10,
+ 'pspReference' => '762895314225',
+ ),
+ $api->captured[0],
+ 'RequestCaptureJob did not make the right capture call'
+ );
+
+ // Blank correlation ID on antifraud messages
+ $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
$this->assertNotNull(
- // Blank correlation ID on antifraud messages
- $antifraudQueue->queueGetObject( null, "" ),
+ $antifraudMessage,
'RequestCaptureJob did not send antifraud message'
);
+ $this->assertEquals(
+ 'process',
+ $antifraudMessage->validation_action,
+ 'Successful capture should get "process" validation
action'
+ );
+ }
+
+ /**
+ * When AVS and CVV scores push the donation over the review threshold,
+ * we should not capture the payment, but leave the donor details.
+ */
+ public function testReviewThreshold() {
+ $config = $this->setConfig( __DIR__ .
'/../config_test_success.php', 'adyen' );
+ $antifraudQueue = $config->object( 'data-store/antifraud', true
);
+ $pendingQueue = $config->object( 'data-store/pending', true );
+ $api = $config->object( 'payment-provider/adyen/api', true );
+
+ $pendingQueue->addObject(
+ KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
+ file_get_contents( __DIR__ .
'/../Data/pending.json' )
+ )
+ );
+ $auth = KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation',
+ file_get_contents( __DIR__ . '/../Data/auth.json' )
+ );
+
+ $auth->avsResult = '1'; // Bad zip code pushes us over review
+
+ $job = ProcessCaptureRequestJob::factory( $auth );
+ $this->assertTrue( $job->execute() );
+
+ $donorData = $pendingQueue->queueGetObject( null,
$auth->correlationId );
+ $this->assertNotNull(
+ $donorData,
+ 'RequestCaptureJob did not leave donor data for review'
+ );
+ $this->assertNotEquals(
+ true,
+ $donorData->captured,
+ 'RequestCaptureJob marked donor data above review
threshold as captured'
+ );
+
+ $this->assertEmpty(
+ $api->captured,
+ 'RequestCaptureJob tried to capture above review
threshold'
+ );
+
+ $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
+ $this->assertNotNull(
+ $antifraudMessage,
+ 'RequestCaptureJob did not send antifraud message'
+ );
+ $this->assertEquals(
+ 'review',
+ $antifraudMessage->validation_action,
+ 'Suspicious auth should get "review" validation action'
+ );
+ }
+
+ /**
+ * When AVS and CVV scores push the donation over the reject threshold,
+ * we should cancel the authorization and delete the donor details.
+ */
+ public function testRejectThreshold() {
+ $config = $this->setConfig( __DIR__ .
'/../config_test_success.php', 'adyen' );
+ $antifraudQueue = $config->object( 'data-store/antifraud', true
);
+ $pendingQueue = $config->object( 'data-store/pending', true );
+ $api = $config->object( 'payment-provider/adyen/api', true );
+
+ $pendingQueue->addObject(
+ KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
+ file_get_contents( __DIR__ .
'/../Data/pending.json' )
+ )
+ );
+ $auth = KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation',
+ file_get_contents( __DIR__ . '/../Data/auth.json' )
+ );
+
+ $auth->avsResult = '2'; // No match at all
+ $auth->cvvResult = '2'; // CVV is also wrong
+
+ $job = ProcessCaptureRequestJob::factory( $auth );
+ $this->assertTrue( $job->execute() );
+
+ $donorData = $pendingQueue->queueGetObject( null,
$auth->correlationId );
+ $this->assertNull(
+ $donorData,
+ 'RequestCaptureJob should delete fraudy donor data'
+ );
+
+ $this->assertEmpty(
+ $api->captured,
+ 'RequestCaptureJob tried to capture above reject
threshold'
+ );
+ $this->assertEquals(
+ $auth->pspReference,
+ $api->cancelled[0],
+ 'Did not cancel the fraudulent authorization'
+ );
+
+ $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
+ $this->assertNotNull(
+ $antifraudMessage,
+ 'RequestCaptureJob did not send antifraud message'
+ );
+ $this->assertEquals(
+ 'reject',
+ $antifraudMessage->validation_action,
+ 'Obvious fraud should get "reject" validation action'
+ );
+ }
+
+ /**
+ * When two authorizations come in with the same merchant reference, we
+ * should cancel the second one and leave the donor details in pending.
+ */
+ public function testDuplicateAuthorisation() {
+ $config = $this->setConfig( __DIR__ .
'/../config_test_success.php', 'adyen' );
+ $pendingQueue = $config->object( 'data-store/pending', true );
+ $api = $config->object( 'payment-provider/adyen/api', true );
+
+ $pendingQueue->addObject(
+ KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
+ file_get_contents( __DIR__ .
'/../Data/pending.json' )
+ )
+ );
+ $auth1 = KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation',
+ file_get_contents( __DIR__ . '/../Data/auth.json' )
+ );
+ $job1 = ProcessCaptureRequestJob::factory( $auth1 );
+ $job1->execute();
+
+ $this->assertEquals( 1, count( $api->captured ), 'Set up
failed' );
+
+ $auth2 = KeyedOpaqueStorableObject::fromJsonProxy(
+
'SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation',
+ file_get_contents( __DIR__ . '/../Data/auth.json' )
+ );
+ $auth2->pspReference = mt_rand( 1000000000, 10000000000 );
+ $job2 = ProcessCaptureRequestJob::factory( $auth2 );
+ $this->assertTrue(
+ $job2->execute(),
+ 'Duplicate auths should not clutter damage queue'
+ );
+
+ $this->assertEquals( 1, count( $api->captured ), 'Captured a
duplicate!' );
+ $this->assertEquals(
+ $auth2->pspReference,
+ $api->cancelled[0],
+ 'Did not cancel the right authorization'
+ );
+
+ $this->assertNotNull(
+ $pendingQueue->queueGetObject( null,
$auth1->correlationId ),
+ 'Capture job should leave donor details on queue'
+ );
}
}
diff --git a/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php
b/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php
index 1107383..448b049 100644
--- a/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php
+++ b/PaymentProviders/Amazon/Tests/phpunit/ApiTest.php
@@ -10,8 +10,8 @@
public function setUp() {
parent::setUp();
- $this->setConfig( __DIR__ . '/../config_test.php', 'amazon' );
- $this->mockClient = Context::get()->getConfiguration()->object(
'payments-client', true );
+ $config = $this->setConfig( __DIR__ . '/../config_test.php',
'amazon' );
+ $this->mockClient = $config->object( 'payments-client', true );
$this->mockClient->calls = array();
$this->mockClient->returns = array();
$this->mockClient->exceptions = array();
diff --git a/Tests/BaseSmashPigUnitTestCase.php
b/Tests/BaseSmashPigUnitTestCase.php
index 8bb5b3e..bd55db7 100644
--- a/Tests/BaseSmashPigUnitTestCase.php
+++ b/Tests/BaseSmashPigUnitTestCase.php
@@ -20,6 +20,13 @@
return json_decode( file_get_contents( $path ), true );
}
+ /**
+ * Set a test configuration and initialize the context
+ *
+ * @param string $configPath path to configuration override file
+ * @param string $configNode node to use for configuration overrides
+ * @return Configuration
+ */
function setConfig( $configPath = null, $configNode = 'default' ) {
$defaultConfig = __DIR__ . '/../config_defaults.php';
$config = new Configuration(
@@ -34,5 +41,6 @@
Logger::init( 'test', 'debug', $config );
self::$loggerCreated = true;
}
+ return $config;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/277427
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b338164461130c3afd0d91efbbd7cfbeec1b680
Gerrit-PatchSet: 5
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
