Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/318015
Change subject: WIP: show TY page on dead session if potentially paid
......................................................................
WIP: show TY page on dead session if potentially paid
Donors whose sessions time out are making duplicate donations.
For processors who capture the payment without our poking them during
resultswitcher processing, we should never show a fail page due to
a dead session.
TODO: figure out whether missing payments-init messages for these
donors are a problem.
Bug: T120228
Change-Id: If4112da339f4dd32666c5ff5dfb9729627dac2a0
---
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/globalcollect.adapter.php
M paypal_gateway/express_checkout/paypal_express.adapter.php
4 files changed, 57 insertions(+), 17 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/15/318015/1
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 33a13fe..6a6890b 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -354,14 +354,9 @@
//no longer letting people in without these things. If this is
//preventing you from doing something, you almost certainly
want to be
//somewhere else.
- $forbidden = false;
+ $deadSession = false;
if ( !$this->adapter->session_hasDonorData() ) {
- $forbidden = true;
- $f_message = 'No active donation in the session';
- }
-
- if ( $forbidden ) {
- wfHttpError( 403, 'Forbidden', wfMessage(
'donate_interface-error-http-403' )->text() );
+ $deadSession = true;
}
$oid = $this->adapter->getData_Unstaged_Escaped( 'order_id' );
@@ -379,11 +374,6 @@
$sessionOrderStatus[$oid] = 'liberated';
$request->setSessionData( 'order_status',
$sessionOrderStatus );
$this->logger->info( "Resultswitcher: Popping out of
iframe for Order ID " . $oid );
- //TODO: Move the $forbidden check back to the beginning
of this if block, once we know this doesn't happen a lot.
- //TODO: If we get a lot of these messages, we need to
redirect to something more friendly than FORBIDDEN, RAR RAR RAR.
- if ( $forbidden ) {
- $this->logger->error( "Resultswitcher: $oid
SHOULD BE FORBIDDEN. Reason: $f_message" );
- }
$this->getOutput()->allowClickjacking();
$this->getOutput()->addModules( 'iframe.liberator' );
return;
@@ -391,8 +381,26 @@
$this->setHeaders();
- if ( $forbidden ){
- throw new RuntimeException( "Resultswitcher: Request
forbidden. " . $f_message . " Adapter Order ID: $oid" );
+ if ( $deadSession ){
+ if ( $this->adapter->isReturnProcessingRequired() ) {
+ wfHttpError( 403, 'Forbidden', wfMessage(
'donate_interface-error-http-403' )->text() );
+ throw new RuntimeException(
+ 'Resultswitcher: Request forbidden. No
active donation in the session. ' .
+ "Adapter Order ID: $oid"
+ );
+ }
+ // If it's possible for a donation to go through
without our
+ // having to do additional processing in the result
switcher,
+ // we don't want to falsely claim it failed just
because we
+ // lost the session data. We also don't want to give any
+ // information to scammers hitting this page with no
session,
+ // so we always show the thank you page.
+ $this->logger->warning(
+ 'Resultswitcher: session is dead, but the ' .
+ 'donor may have made a successful payment.'
+ );
+ $this->displayThankYouPage( 'dead session' );
+ return;
}
$this->logger->info( "Resultswitcher: OK to process Order ID: "
. $oid );
@@ -412,9 +420,7 @@
switch ( $status ) {
case FinalStatus::COMPLETE:
case FinalStatus::PENDING:
- $thankYouPage = ResultPages::getThankYouPage(
$this->adapter );
- $this->logger->info( "Displaying thank you page
$thankYouPage for status $status." );
- $this->getOutput()->redirect( $thankYouPage );
+ $this->displayThankYouPage( $status );
return;
}
$this->logger->info( "Displaying fail page for final
status $status" );
@@ -533,4 +539,13 @@
// Maybe ask $form_obj for a title so different errors can show
different titles
$this->getOutput()->setPageTitle( wfMessage(
'donate_interface-make-your-donation' ) );
}
+
+ /**
+ * @param $reason
+ */
+ protected function displayThankYouPage( $reason ) {
+ $thankYouPage = ResultPages::getThankYouPage( $this->adapter );
+ $this->logger->info( "Displaying thank you page $thankYouPage
for status $reason." );
+ $this->getOutput()->redirect( $thankYouPage );
+ }
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 67f016a..624684f 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1442,6 +1442,17 @@
}
/**
+ * Whether donation processing depends on additional processing on-wiki
+ * at the donor's return from a payment processor. This is used to
+ * determine whether we should show fail pages on session timeouts.
+ *
+ * @return bool true when on-wiki post-processing is required.
+ */
+ public function isReturnProcessingRequired() {
+ return false;
+ }
+
+ /**
* Check the response for general sanity - e.g. correct data format,
keys exists
* @return boolean true if response looks sane
*/
diff --git a/globalcollect_gateway/globalcollect.adapter.php
b/globalcollect_gateway/globalcollect.adapter.php
index 1550afa..34a6c1b 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -1006,6 +1006,13 @@
}
/**
+ * @return bool true, we need to SET_PAYMENT when donors return.
+ */
+ public function isReturnProcessingRequired() {
+ return true;
+ }
+
+ /**
* Parse the response to get the status. Not sure if this should return
a bool, or something more... telling.
*
* @param DOMDocument $response The response XML loaded into a
DOMDocument
diff --git a/paypal_gateway/express_checkout/paypal_express.adapter.php
b/paypal_gateway/express_checkout/paypal_express.adapter.php
index 6cc85d7..4b545fb 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -376,6 +376,13 @@
}
/**
+ * @return bool true, we need to DoExpressCheckoutPayment when donors
return
+ */
+ public function isReturnProcessingRequired() {
+ return true;
+ }
+
+ /**
* TODO: DRY with AstroPay; handle ProcessReturn like other transactions
*/
public function processResponse( $response ) {
--
To view, visit https://gerrit.wikimedia.org/r/318015
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If4112da339f4dd32666c5ff5dfb9729627dac2a0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
