Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/206307
Change subject: Make resultswitcher processing slightly less funky
......................................................................
Make resultswitcher processing slightly less funky
No need for isResponse() - was only called from response pages
Quit overriding processResponse with incompatible signature
Remove one bit of $_GET inspection from Adyen adapter
Introduce a couple constants
Always redirect to fail or TY - don't stall on resultswitcher
Bug: T90504
Change-Id: I1f9f904728a30e65553f4247e49dca4511d34500
---
M adyen_gateway/adyen.adapter.php
M astropay_gateway/astropay.adapter.php
M astropay_gateway/astropay_resultswitcher.body.php
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M tests/Adapter/Astropay/AstropayTest.php
6 files changed, 97 insertions(+), 100 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/07/206307/1
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index 7592cf3..988167a 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -237,12 +237,6 @@
return $this->getTransactionAllResults();
}
- function isResponse() {
- global $wgRequest;
- $authResult = $wgRequest->getVal( 'authResult' );
- return !empty( $authResult );
- }
-
function getResponseStatus( $response ) {
}
@@ -478,41 +472,40 @@
return $queryvals;
}
- function processResponse( $response = null, &$retryVars = null ) {
- if ( $response === NULL ) { // convert GET data
- $request_vars = $_GET;
-
- $this->logger->info( "Processing user return data: " .
print_r( $request_vars, TRUE ) );
-
- if ( !$this->checkResponseSignature( $request_vars ) ) {
- $this->logger->info( "Bad signature in
response" );
- return 'BAD_SIGNATURE';
- } else {
- $this->logger->debug( "Good signature" );
- }
-
- $gateway_txn_id = isset( $request_vars[ 'pspReference'
] ) ? $request_vars[ 'pspReference' ] : '';
-
- $result_code = isset( $request_vars[ 'authResult' ] ) ?
$request_vars[ 'authResult' ] : '';
- if ( $result_code == 'PENDING' || $result_code ==
'AUTHORISED' ) {
- // Both of these are listed as pending because
we have to submit a capture
- // request on 'AUTHORIZATION' ipn message
receipt.
- $this->logger->info( "User came back as pending
or authorised, placing in pending queue" );
- $this->finalizeInternalStatus( 'pending' );
- }
- else {
- $this->logger->info( "Negative response from
gateway. Full response: " . print_r( $request_vars, TRUE ) );
- $this->finalizeInternalStatus( 'failed' );
- return 'UNKNOWN';
- }
- $this->setTransactionResult( $gateway_txn_id,
'gateway_txn_id' );
- $this->setTransactionResult( $this->getFinalStatus(),
'txn_message' );
- $this->runPostProcessHooks();
- $this->doLimboStompTransaction( TRUE ); // add
antimessage
- return null;
+ function processResponse( $response, &$retryVars = null ) {
+ if ( empty( $response ) || empty ( $response['data'] ) ) {
+ $this->logger->info( "No response from gateway" );
+ return 'NO_RESPONSE';
}
- $this->logger->info( "No response from gateway" );
- return 'NO_RESPONSE';
+ $request_vars = $response['data'];
+ $this->logger->info( "Processing user return data: " . print_r(
$request_vars, TRUE ) );
+
+ if ( !$this->checkResponseSignature( $request_vars ) ) {
+ $this->logger->info( "Bad signature in response" );
+ return self::BAD_SIGNATURE;
+ } else {
+ $this->logger->debug( "Good signature" );
+ }
+
+ $gateway_txn_id = isset( $request_vars[ 'pspReference' ] ) ?
$request_vars[ 'pspReference' ] : '';
+
+ $result_code = isset( $request_vars[ 'authResult' ] ) ?
$request_vars[ 'authResult' ] : '';
+ if ( $result_code == 'PENDING' || $result_code == 'AUTHORISED'
) {
+ // Both of these are listed as pending because we have
to submit a capture
+ // request on 'AUTHORIZATION' ipn message receipt.
+ $this->logger->info( "User came back as pending or
authorised, placing in pending queue" );
+ $this->finalizeInternalStatus( 'pending' );
+ }
+ else {
+ $this->logger->info( "Negative response from gateway.
Full response: " . print_r( $request_vars, TRUE ) );
+ $this->finalizeInternalStatus( 'failed' );
+ return 'UNKNOWN';
+ }
+ $this->setTransactionResult( $gateway_txn_id, 'gateway_txn_id'
);
+ $this->setTransactionResult( $this->getFinalStatus(),
'txn_message' );
+ $this->runPostProcessHooks();
+ $this->doLimboStompTransaction( TRUE ); // add antimessage
+ return null;
}
/**
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 90b2f2e..fe3c070 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -24,7 +24,9 @@
const GATEWAY_NAME = 'Astropay';
const IDENTIFIER = 'astropay';
const GLOBAL_PREFIX = 'wgAstropayGateway';
+
const DUPLICATE_ORDER_ID_ERROR = 'DUPLICATE_ORDER_ID_ERROR';
+ const MISSING_TXN_ID = 'MISSING_TXN_ID';
public function getFormClass() {
return 'Gateway_Form_Mustache';
@@ -474,20 +476,26 @@
return $currencies;
}
- function processResponse( $response = null, &$retryVars = null ) {
+ function processResponse( $response, &$retryVars = null ) {
switch( $this->getCurrentTransaction() ) {
case 'PaymentStatus':
if ( !$this->verifyStatusSignature(
$response['data'] ) ) {
$this->logger->error( 'Bad signature in
response to PaymentStatus call.' );
- return 'BAD_SIGNATURE';
+ return self::BAD_SIGNATURE;
}
break;
case 'ProcessReturn':
- if ( !$this->verifyStatusSignature(
$this->staged_data ) ) {
+ if ( !$this->verifyStatusSignature(
$response['data'] ) ) {
$this->logger->error( 'Bad signature in
data POSTed to resultswitcher' );
- return 'BAD_SIGNATURE';
+ return self::BAD_SIGNATURE;
}
- $status = $this->findCodeAction(
'PaymentStatus', 'result', $this->staged_data['result'] );
+ if ( isset( $response['data']['x_document'] ) )
{
+ $this->setTransactionResult(
$response['data']['x_document'], 'gateway_txn_id' );
+ } else {
+ $this->logger->error( 'Astropay did not
post back their transaction ID in x_document' );
+ return self::MISSING_TXN_ID;
+ }
+ $status = $this->findCodeAction(
'PaymentStatus', 'result', $response['data']['result'] );
$this->logger->info( "Payment status $status
coming back to ResultSwitcher" );
$this->finalizeInternalStatus( $status );
break;
@@ -530,14 +538,5 @@
return strtoupper(
hash_hmac( 'sha256', pack( 'A*', $message ), pack(
'A*', $key ) )
);
- }
-
- function isResponse() {
- // We expect the resultswitcher page has fed us with enough
POSTed
- // params to verify a signature
- return isset( $this->staged_data['result'] ) &&
- isset( $this->staged_data['x_amount'] ) &&
- isset( $this->staged_data['x_invoice'] ) &&
- isset( $this->staged_data['x_control'] );
}
}
diff --git a/astropay_gateway/astropay_resultswitcher.body.php
b/astropay_gateway/astropay_resultswitcher.body.php
index b335908..967f720 100644
--- a/astropay_gateway/astropay_resultswitcher.body.php
+++ b/astropay_gateway/astropay_resultswitcher.body.php
@@ -9,10 +9,6 @@
protected function handleRequest() {
$this->adapter->setCurrentTransaction( 'ProcessReturn' );
-
- $params = $this->getRequest()->getValues();
- $this->adapter->addResponseData( $params );
-
$this->handleResultRequest();
}
}
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 8b64ba4..22327ad 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -444,28 +444,37 @@
if ( $forbidden ){
$this->logger->critical( "Resultswitcher: Request
forbidden. " . $f_message . " Adapter Order ID: $oid" );
+ $this->getOutput()->redirect(
$this->adapter->getFailPage() );
return;
} else {
$this->logger->info( "Resultswitcher: OK to process
Order ID: " . $oid );
}
if ( $this->adapter->checkTokens() ) {
- if ( $this->adapter->isResponse() ) {
- $this->getOutput()->allowClickjacking();
- $this->getOutput()->addModules(
'iframe.liberator' );
- if ( NULL === $this->adapter->processResponse()
) {
- switch (
$this->adapter->getFinalStatus() ) {
- case 'complete':
- case 'pending':
- $this->getOutput()->redirect(
$this->adapter->getThankYouPage() );
- return;
- }
+ $this->getOutput()->allowClickjacking();
+ // FIXME: do we really need this again?
+ $this->getOutput()->addModules( 'iframe.liberator' );
+ // processResponse expects something with data, so
let's feed it
+ // all the GET and POST vars
+ $response = array(
+ 'data' => $this->getRequest()->getValues(),
+ );
+ // TODO: run the whole set of getResponseStatus,
getResponseErrors
+ // and getResponseData first. Maybe do_transaction
with a
+ // communication_type of 'incoming' and a way to
provide the
+ // adapter the GET/POST params harvested here.
+ if ( NULL === $this->adapter->processResponse(
$response ) ) {
+ switch ( $this->adapter->getFinalStatus() ) {
+ case 'complete':
+ case 'pending':
+ $this->getOutput()->redirect(
$this->adapter->getThankYouPage() );
+ return;
}
- $this->getOutput()->redirect(
$this->adapter->getFailPage() );
}
} else {
$this->logger->error( "Resultswitcher: Token Check
Failed. Order ID: $oid" );
}
+ $this->getOutput()->redirect( $this->adapter->getFailPage() );
}
/**
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 260f129..cd2d0fb 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -322,6 +322,11 @@
*/
protected static $globalsCache = array ( );
+ /**
+ * @const string error code for failed signature validation
+ */
+ const BAD_SIGNATURE = 'BAD_SIGNATURE';
+
//ALL OF THESE need to be redefined in the children. Much voodoo
depends on the accuracy of these constants.
const GATEWAY_NAME = 'Donation Gateway';
const IDENTIFIER = 'donation';
@@ -3860,14 +3865,5 @@
}
return json_encode( $logObj );
- }
-
- /**
- * Indicates if the current request is a user returning from the payment
- * processor with some information in the GET/POST.
- * @return boolean
- */
- function isResponse() {
- return false;
}
}
diff --git a/tests/Adapter/Astropay/AstropayTest.php
b/tests/Adapter/Astropay/AstropayTest.php
index 9c57262..6bf2c27 100644
--- a/tests/Adapter/Astropay/AstropayTest.php
+++ b/tests/Adapter/Astropay/AstropayTest.php
@@ -209,18 +209,20 @@
// Next lines mimic Astropay resultswitcher
$gateway->setCurrentTransaction( 'ProcessReturn' );
- $gateway->addResponseData( array(
- 'result' => '9',
- 'x_amount' => '100.00',
- 'x_amount_usd' => '42.05',
- 'x_control' =>
'DDF89085AC70C0B0628150C51D64419D8592769F2439E3936570E26D24881730',
- 'x_description' => 'Donation to the Wikimedia
Foundation',
- 'x_document' => '32869',
- 'x_iduser' => '08feb2d12771bbcfeb86',
- 'x_invoice' => '123456789',
- ) );
+ $response = array(
+ 'data' => array(
+ 'result' => '9',
+ 'x_amount' => '100.00',
+ 'x_amount_usd' => '42.05',
+ 'x_control' =>
'DDF89085AC70C0B0628150C51D64419D8592769F2439E3936570E26D24881730',
+ 'x_description' => 'Donation to the Wikimedia
Foundation',
+ 'x_document' => '32869',
+ 'x_iduser' => '08feb2d12771bbcfeb86',
+ 'x_invoice' => '123456789',
+ )
+ );
- $result = $gateway->processResponse( null );
+ $result = $gateway->processResponse( $response );
$status = $gateway->getFinalStatus();
$this->assertNull( $result );
$this->assertEquals( 'complete', $status );
@@ -235,18 +237,20 @@
$gateway = $this->getFreshGatewayObject( $init );
$gateway->setCurrentTransaction( 'ProcessReturn' );
- $gateway->addResponseData( array(
- 'result' => '8', // rejected by bank
- 'x_amount' => '100.00',
- 'x_amount_usd' => '42.05',
- 'x_control' =>
'706F57BC3E74906B14B1DEB946F027104513797CC62AC0F5107BC98F42D5DC95',
- 'x_description' => 'Donation to the Wikimedia
Foundation',
- 'x_document' => '32869',
- 'x_iduser' => '08feb2d12771bbcfeb86',
- 'x_invoice' => '123456789',
- ) );
+ $response = array(
+ 'data' => array(
+ 'result' => '8', // rejected by bank
+ 'x_amount' => '100.00',
+ 'x_amount_usd' => '42.05',
+ 'x_control' =>
'706F57BC3E74906B14B1DEB946F027104513797CC62AC0F5107BC98F42D5DC95',
+ 'x_description' => 'Donation to the Wikimedia
Foundation',
+ 'x_document' => '32869',
+ 'x_iduser' => '08feb2d12771bbcfeb86',
+ 'x_invoice' => '123456789',
+ )
+ );
- $result = $gateway->processResponse( null );
+ $result = $gateway->processResponse( $response );
$status = $gateway->getFinalStatus();
$this->assertNull( $result );
$this->assertEquals( 'failed', $status );
--
To view, visit https://gerrit.wikimedia.org/r/206307
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f9f904728a30e65553f4247e49dca4511d34500
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
