jenkins-bot has submitted this change and it was merged.
Change subject: Show TY page on dead session if potentially paid
......................................................................
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 paypal_gateway/express_checkout/paypal_express.adapter.php
3 files changed, 52 insertions(+), 17 deletions(-)
Approvals:
Cdentinger: Looks good to me, approved
jenkins-bot: Verified
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 33a13fe..f5f3a48 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,28 @@
$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. We don't want
to do
+ // any post-processing if we're not sure whether we
actually
+ // originated this attempt, so we return right after.
+ $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 +422,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 +541,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 string $logReason Logged explanation for redirect
+ */
+ protected function displayThankYouPage( $logReason ) {
+ $thankYouPage = ResultPages::getThankYouPage( $this->adapter );
+ $this->logger->info( "Displaying thank you page $thankYouPage
for status $logReason." );
+ $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/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: merged
Gerrit-Change-Id: If4112da339f4dd32666c5ff5dfb9729627dac2a0
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: XenoRyet <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
