From 9f9175be931367ad41cd47041096d2a3202b74ee Mon Sep 17 00:00:00 2001 From: Roman Zimmermann Date: Fri, 6 Jun 2014 16:30:07 +0200 Subject: [PATCH] paypal_payment-pps_HTTP_404_on_return_uri-2052361-40 --- .../includes/PayPalPaymentIPNController.inc | 26 +++-- paypal_payment_ipn/paypal_payment_ipn.module | 38 +++++-- .../tests/PayPalPaymentIPNControllerTest.test | 2 +- .../paypal_payment_ipn_test.module | 11 +- paypal_payment_pps/paypal_payment_pps.info | 1 + paypal_payment_pps/paypal_payment_pps.module | 19 +++- .../tests/PayPalPaymentPPSPaymentIPNExecution.test | 111 +++++++++++++++++++++ 7 files changed, 179 insertions(+), 29 deletions(-) create mode 100644 paypal_payment_pps/tests/PayPalPaymentPPSPaymentIPNExecution.test diff --git a/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc b/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc index 7eac9a8..96ee687 100644 --- a/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc +++ b/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc @@ -139,20 +139,18 @@ abstract class PayPalPaymentIPNController { if (isset($ipn_variables['txn_id'])) { // Make sure this IPN was not processed before. $ipn = PayPalPaymentIPNController::load($ipn_variables['txn_id']); - if (!$ipn) { - // Check if the IPN matches a Payment. - if (isset($ipn_variables['invoice'])) { - $pid = self::PID($ipn_variables['invoice']); - if ($pid) { - $payment = entity_load_single('payment', $pid); - if ($payment) { - // Allow payment method controllers to completely take over validation. - if ($payment->method->controller instanceof PayPalPaymentIPNPaymentMethodControllerInterface) { - return $payment->method->controller->PayPalValidateIPNVariables($payment, $ipn_variables); - } - else { - return TRUE; - } + // Check if the IPN matches a Payment. + if (isset($ipn_variables['invoice'])) { + $pid = self::PID($ipn_variables['invoice']); + if ($pid) { + $payment = entity_load_single('payment', $pid); + if ($payment) { + // Allow payment method controllers to completely take over validation. + if ($payment->method->controller instanceof PayPalPaymentIPNPaymentMethodControllerInterface) { + return $payment->method->controller->PayPalValidateIPNVariables($payment, $ipn_variables); + } + else { + return TRUE; } } } diff --git a/paypal_payment_ipn/paypal_payment_ipn.module b/paypal_payment_ipn/paypal_payment_ipn.module index e2f6a0c..7417b9e 100644 --- a/paypal_payment_ipn/paypal_payment_ipn.module +++ b/paypal_payment_ipn/paypal_payment_ipn.module @@ -26,16 +26,38 @@ function paypal_payment_ipn_menu() { } /** - * Processes an IPN request based on POST data. + * Handle POST data from an IPN request. */ function paypal_payment_ipn_post() { - $ipn_variables = $_POST; - foreach ($_POST as $ipn_variable => $value) { - $ipn_variables[$ipn_variable] = rawurldecode($value); - } + $ipn_variables = paypal_payment_ipn_get_decoded_data($_POST); if (PayPalPaymentIPNController::acknowledge($ipn_variables) && PayPalPaymentIPNController::validate($ipn_variables)) { - $ipn = new PayPalPaymentIPN($ipn_variables); - PayPalPaymentIPNController::save($ipn); - PayPalPaymentIPNController::process($ipn_variables); + paypal_payment_ipn_process_data($ipn_variables); + } +} + +/** + * Processes an IPN request based on POST data. + */ +function paypal_payment_ipn_process_data($ipn_variables) { + if (PayPalPaymentIPNController::load($ipn_variables['txn_id'])) { + // The transaction record has already been created and processed. + return; + } + $ipn = new PayPalPaymentIPN($ipn_variables); + PayPalPaymentIPNController::save($ipn); + PayPalPaymentIPNController::process($ipn_variables); +} + +/** + * Decode incoming IPN data. + * + * @param array $ipn_variables + * This is most probably unprocessed POST data. + */ +function paypal_payment_ipn_get_decoded_data($post_data) { + $ipn_variables = array(); + foreach ($post_data as $key => $value) { + $ipn_variables[$key] = rawurldecode($value); } + return $ipn_variables; } diff --git a/paypal_payment_ipn/tests/PayPalPaymentIPNControllerTest.test b/paypal_payment_ipn/tests/PayPalPaymentIPNControllerTest.test index cb3e6ba..8901d8e 100644 --- a/paypal_payment_ipn/tests/PayPalPaymentIPNControllerTest.test +++ b/paypal_payment_ipn/tests/PayPalPaymentIPNControllerTest.test @@ -113,7 +113,7 @@ class PayPalPaymentIPNControllerTest extends PayPalPaymentIPNWebTestCase { $ipn = new PayPalPaymentIPN($ipn_variables); $ipn->pid = PayPalPaymentIPNController::PID($ipn_variables['invoice']); PayPalPaymentIPNController::save($ipn); - $this->assertFalse(PayPalPaymentIPNController::validate($ipn_variables)); + $this->assertTrue(PayPalPaymentIPNController::validate($ipn_variables)); // Test IPN variables for which no Payment exists. $ipn_variables = $this->mockIPNVariables(999); diff --git a/paypal_payment_ipn_test/paypal_payment_ipn_test.module b/paypal_payment_ipn_test/paypal_payment_ipn_test.module index 5ea6ee4..731a7a0 100644 --- a/paypal_payment_ipn_test/paypal_payment_ipn_test.module +++ b/paypal_payment_ipn_test/paypal_payment_ipn_test.module @@ -60,4 +60,13 @@ function paypal_payment_ipn_test_paypal_server() { print 'INVALID'; } drupal_exit(); -} \ No newline at end of file +} + +/** + * Dummy finish callback for paypal_payments + * + * @param Payment $payment + */ +function paypal_payment_ipn_test_finish_callback(Payment $payment) { + $payment->payment_test_finish_callback = TRUE; +} diff --git a/paypal_payment_pps/paypal_payment_pps.info b/paypal_payment_pps/paypal_payment_pps.info index e8aa8f9..0ea3dd8 100644 --- a/paypal_payment_pps/paypal_payment_pps.info +++ b/paypal_payment_pps/paypal_payment_pps.info @@ -7,6 +7,7 @@ core = 7.x PHP = 5.3 files[] = includes/PayPalPaymentPPSPaymentMethodController.inc files[] = tests/PayPalPaymentPPSPaymentExecution.test +files[] = tests/PayPalPaymentPPSPaymentIPNExecution.test files[] = tests/PayPalPaymentPPSPaymentMethodCRUD.test files[] = tests/PayPalPaymentPPSPaymentMethodUI.test files[] = tests/PayPalPaymentWebTestCase.test diff --git a/paypal_payment_pps/paypal_payment_pps.module b/paypal_payment_pps/paypal_payment_pps.module index 9b659bc..508e49d 100644 --- a/paypal_payment_pps/paypal_payment_pps.module +++ b/paypal_payment_pps/paypal_payment_pps.module @@ -251,10 +251,12 @@ function paypal_payment_pps_form_redirect_access(Payment $payment, $account = NU * Return callback. */ function paypal_payment_pps_return() { - $ipn_variables = $_POST; - $ipn = new PayPalPaymentIPN($ipn_variables); - PayPalPaymentIPNController::save($ipn); - PayPalPaymentIPNController::process($ipn_variables); + // Retrieve the POST data that came with this request. + $ipn_variables = paypal_payment_ipn_get_decoded_data($_POST); + // Make sure that ipn data is handled. Normally this is already done when IPN + // calls back in using PAYPAL_IPN_LISTENER_PATH + paypal_payment_ipn_process_data($ipn_variables); + // Now load the payment and perform finishing steps. $payment = entity_load_single('payment', PayPalPaymentIPNController::PID($ipn_variables['invoice'])); $payment->finish(); } @@ -265,7 +267,14 @@ function paypal_payment_pps_return() { * @return bool */ function paypal_payment_pps_return_access() { - return PayPalPaymentIPNController::validate($_POST); + global $user; + $ipn_variables = paypal_payment_ipn_get_decoded_data($_POST); + if (!PayPalPaymentIPNController::validate($ipn_variables)) { + return FALSE; + } + $pid = PayPalPaymentIPNController::PID($ipn_variables['invoice']); + $payment = entity_load_single('payment', $pid); + return $payment->uid == $user->uid; } /** diff --git a/paypal_payment_pps/tests/PayPalPaymentPPSPaymentIPNExecution.test b/paypal_payment_pps/tests/PayPalPaymentPPSPaymentIPNExecution.test new file mode 100644 index 0000000..306b229 --- /dev/null +++ b/paypal_payment_pps/tests/PayPalPaymentPPSPaymentIPNExecution.test @@ -0,0 +1,111 @@ + 'Payment execution with IPN', + 'group' => 'PayPal Payments Standard', + 'description' => 'Test paypal standard payment with IPN listener', + // do not add paypal_payment_pps_test because then there are url + // rewriting confilicts with the paypal_payment_ipn_test module + 'dependencies' => array('paypal_payment_pps', 'paypal_payment_ipn_test'), + ); + } + + function setUp(array $modules = array()) { + parent::setUp(array_merge($modules, array('paypal_payment_pps', 'paypal_payment_ipn_test'))); + } + + /** + * Tests payment execution. + */ + function testPaymentIPNExecution() { + $account = $this->drupalCreateUser(); + + $method_controller = payment_method_controller_load('PayPalPaymentPPSPaymentMethodController'); + $payment_method = $this->paymentMethodCreate($account->uid, $method_controller); + entity_save('payment_method', $payment_method); + + $payment = $this->paymentCreate($account->uid, $payment_method); + entity_save('payment', $payment); + + // must access the created payment entity, and access the real PID + // call the PAYPAL_IPN_LISTENER_PATH, see that ipn record exists + $ipn_variables = $this->mockIPNVariables($payment->pid, TRUE); + $ipn_variables['business'] = $payment->method->controller_data['email_address']; + + $url = PayPalPaymentIPNController::URL(); + + $this->curlExec(array( + CURLOPT_POST => TRUE, + CURLOPT_POSTFIELDS => $ipn_variables, + CURLOPT_URL => $url, + )); + $response = $this->drupalGetContent(); + + $this->assertFalse($response); + $this->assertResponse(200); + + // test that an IPN has been saved + $this->assertTrue(PayPalPaymentIPNController::load($ipn_variables['txn_id'])); + + // login the user, other whise access control in + // paypal_payment_pps_return_access() fails + $this->drupalLogin($account); + + // Will paypal_payment_pps_return_access() grant us access? + $this->assertTrue(PayPalPaymentIPNController::validate($ipn_variables)); + $this->assertTrue(PayPalPaymentIPNController::PID($ipn_variables['invoice'])); + + // call the return url in paypal_payment_pps.module and see that it + // succeeds, this mimiks click on "Return to Shop-Page" in paypal + $this->curlExec(array( + CURLOPT_POST => TRUE, + CURLOPT_POSTFIELDS => $ipn_variables, + CURLOPT_URL => url('paypal_payment_pps/return', array('absolute' => TRUE)), + )); + $response = $this->drupalGetContent(); + + $this->assertFalse($response); + $this->assertResponse(200); + // $this->assertURL(''); + } + + /** + * Create, save, and return a Payment. + * + * @param integer $uid + * The user ID of the payment's owner. + * @param PaymentMethod $payment_method + * An optional payment method to set. Defaults to PaymentMethodUnavailable. + * + * @return Payment + */ + static function paymentCreate($uid, PaymentMethod $payment_method = NULL) { + $payment_method = $payment_method ? $payment_method : new PaymentMethodUnavailable; + $payment = new Payment(array( + 'currency_code' => 'XXX', + 'description' => 'This is the payment description', + 'finish_callback' => 'paypal_payment_ipn_test_finish_callback', + 'method' => $payment_method, + 'uid' => $uid, + )); + $payment->setLineItem(new PaymentLineItem(array( + 'name' => 'foo', + 'amount' => 1.0, + 'tax_rate' => 0.1, + ))); + + return $payment; + } + +} \ No newline at end of file -- 1.8.3.2