paypal_payment_pps: HTTP 404 on return uri

Thanks for reporting this problem! Can you start with writing a test that exposes this bug? We can then prove the fix actually works by simply running the test.

Thanks! I have one question: you created a completely new test case, while there already is a test case for PPS payment execution. Is there a reason why you did not add the IPN test there?

HI, could you please post some instructions on how to apply the patch. I would greatly appreciate it.

Thank you for you fast / swift reply.

Sherwin

Hi

can confirm this issue on two independent sites.

it OCCURS using paypal standard and NOT with paypal express.

on returning after paypal on url ... paypal_payment_pps/return

get message 'Access denied' you are not authorized to access this page.

when the message is displayed the payment still occurs correctly and shows as completed in payment reference field.

can close the 'access denied' page and continue on with the original node and save the page.

Configuration items:
Drupal 7.22
Domain Access
Payment v7.x-1.8
paypal for payment
plus other stuff

have not looked into it any further.

Cheers

What if we just decouple the existence of an IPN in the database from its validation? PayPalPaymentIPNController::validate() would no longer check for an existing IPN, but instead just validate the IPN's contents, and paypal_payment_ipn_post() would check if the received IPN already exists in the database, before acknowledging and saving it.

Never mind.

I'm puzzling with a patch a bit.

A short review of the patch in #18. I'm working on a patch that addresses these issues.

1. +++ b/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc
   @@ -95,7 +95,7 @@ abstract class PayPalPaymentIPNController {
   -    $data = '';
   +    $data = array();
   

   Good catch, but not in the scope of this patch.

2. +++ b/paypal_payment_ipn/includes/PayPalPaymentIPNController.inc
   @@ -136,29 +136,24 @@ abstract class PayPalPaymentIPNController {
   +    if (!isset($ipn_variables['txn_id']) || !isset($ipn_variables['invoice'])) {
   

   We no longer need the txn_id.

3. +++ b/paypal_payment_ipn/paypal_payment_ipn.module
   @@ -26,16 +26,38 @@ function paypal_payment_ipn_menu() {
   +  if (PayPalPaymentIPNController::validate($ipn_variables) && PayPalPaymentIPNController::acknowledge($ipn_variables)) {
   

   Validation before acknowledgement makes sense in a way, but there was *some* documentation that said you had to acknowledge every IPN you got regardless of its contents. Say you create a payment that is executed, and then deleted. You will still be getting IPNs for it, but validation will fail, because the invoice_id no longer matches a payment ID. However, PayPal requires you to confirm that you received the IPN.

4. +++ b/paypal_payment_ipn/paypal_payment_ipn.module
   @@ -26,16 +26,38 @@ function paypal_payment_ipn_menu() {
   +function paypal_payment_ipn_get_decoded_data() {
   +  $ipn_variables = $_POST;
   

   Ideally $_POST would be passed on as a function argument, because we do not want a tight coupling between logic and anything outside the direct Drupal environment. When testing, for instance, the data does not come from POST.

5. +++ b/paypal_payment_ipn_test/paypal_payment_ipn_test.module
   @@ -60,4 +60,13 @@ function paypal_payment_ipn_test_paypal_server() {
   +function paypal_pament_ipn_finish_callback(Payment $payment) {
   

   This function is never used.

6. +++ b/paypal_payment_pps/paypal_payment_pps.info
   @@ -7,6 +7,7 @@ core = 7.x
   +files[] = tests/PayPalPaymentPPSPaymentIPNExecution.test
   

   This file was not added to the patch.

2) the txn_id key was only used to check if an IPN already existed in the DB. That is no longer done, so we don't need to check for the key's existence anymore.

5) If it's part pf paypal_payment_ipn_test, it will need to be namespaced as such.

Also, I assigned the issue to myself to aggregate the results of the patches and our discussions into one smaller patch. Yours unfortunately tries to do too many things that make understanding the solution harder to do.

Just a quick reply while making dinner: your patch does not only try to solve the problem, but also clean things up a bit. Parts of the code definitely need some cleaning up, but let's not do that here, so we can focus on fixing the problem. Too much code in the patch that is no part of the fix can distract us from seeing the important bits.

I'm hoping to dig into this a bit more later tonight. The biggest problem (which I believe you found as well), is that the communication with the remote server is not properly tested. paypal_payment_pps_test already reroutes requests to Paypal's servers, but that will need to be extended.

What is the best way to go about this problem? Could I use one of the patches or is there a new patch (or release) on the way? I can confirm this problem still exists in 7.x-1.1, unless I missed something.

I tried the patch in #24 but found som problems that i fixed.

1. function paypal_payment_ipn_process() is actually a Drupal hook_process() and gets run on every request. Replaced function name to paypal_payment_ipn_process_data().
2. business was not set in the variables from paypal receiver_email seemed to be the right variable name. I don't know if something has changed in the API or if this just was a typing error?
   Old

   return strtolower($ipn_variables['business']) == strtolower($payment->method->controller_data['email_address']);
   

   New

   return strtolower($ipn_variables['receiver_email']) == strtolower($payment->method->controller_data['email_address']);
   

Thanks for looking into this! Let's keep this issue focused on the HTTP version problem and fix different problems in different issues. Can you post a new patch that doesn't contain all the other fixes?

I just had to re-roll the patch to work with -p1 (instead of -p2). It did the job for me.

I think this should be it, made the namespace fit and got rid of some refactoring (the one that fixed 6 nested control structures for instance.) Didn't test it yet I'm going to wait for the automated testing to pass it. I'm not sure what is going on in #32

Feedback on the test is:
Access denied
You are not authorized to access this page.

My understanding of the paypal protocol for PPS is still a bit limited, but I've made the tests in #33 pass. For that I've simply modified PayPalPaymentIPNController::validate() to not check for an existing database entry (ie. any valid post array is fine).

Xano: We no longer need the txn_id.

Do you mean we don't need the txn_id at all (not even store it) or do you mean we don't need it for validation?

This time rolling the patch with git format-patch :) -- The interdiff is still the same.

The existing test caught the change in behavior of ::validate() -- I've changed the assertion to reflect the new behavior: validate() only checks for a valid $_POST-array not for whether it has been saved already or not.

I'm still getting a 403 when returning from the paypal payment page. The reason is that I'm sent there via a GET-request. I think it's time to consult some API documentation.

Ok I've found the reason. It seems that there is several ways for paypal to return.

<meta http-equiv="refresh" content="5;url=http://server/paypal_payment/pps/return"/>

This one will redirect after 5 seconds without sending the post variables.

setTimeout("document.forms.merchantredirectform.submit()", 4000);

This one will redirect after 4 seconds sending the post variables.

Since my test-site doesn't have HTTPS my firefox asks me whether I really want to submit the form. This stalls the submission long enough for the meta-tag to take effect - leading to a 403 error.

I've just tested out the patch at #40 and I'm still getting the same 404 error.

I'm not getting a 404 error.

Tried patch from #40 and I'm getting a 403 "Not Authorized" error.

Hi all,

First at all sorry for my approximative english...
Secondly sorry for my approximative explanation, even I'm a developper and I can understand technical things, I have big problems when I need technical talking... so :

I just spent a few days trying to solve this issue, especially with the information given by berliner (main problem), torotil at #42 and of course Xano at #31 .
After 2 days of dpm, watchdog and headaches, I could not really go on with the issue... So I decided to investigate how this works in commerce_payment and commerce_paypal module. Two more days of dpm, watchdog and headaches and I noticed that the same problem exists in commerce_paypal! Thank's torotil to point on this here #42 . So why it work with commerce_paypal payments?

I noticed that in the function commerce_payment_redirect_pane_checkout_form (that is in our case more or less the equivalent of paypal_payment_pps_return and paypal_payment_pps_return_access) they don't deal with the $_POST variable but they pass the order and other stuff as a params to the function and then they directlly deals with the order who contains a value named payment_redirect_key that was stored just before the submission to PayPal.
I wrote a patch that seems to work but I'm not really sure... so if someone can test it and give feed-back I will appreciate.

Thanks everyone for the works done.

Little changes...

Humm...

Sorry guys, it seems that my patch does not work after I activate the IPN in PayPal settings...

I keep trying...

Any updates on this issue?

FWIW, I was able to get this working in sandbox mode by applying the patch in #8.

Slightly off topic but I thought worth noting in case anybody else is having issues with the status not updating properly. It's probably common knowledge but just in case it helps someone else, here is a little gotcha. Note this is specifically in relation to sandbox mode:

* For the payment method email address, make sure the email address you enter here is one set up in sandbox mode (you can see a list of these by signing in to your developer account and clicking on 'Accounts' (under 'Sandbox') . I initially set this to the actual PayPal account email address and couldn't understand why the response (when completing payment) was always 'Pending'. I dug a bit further and realised that the 'pending_reason' value was 'unilateral' which means the payment was made to an email address that is not registered/confirmed. Full details on all the return values (and what they mean) are here: https://developer.paypal.com/docs/classic/ipn/integration-guide/IPNandPD...
* The payment method 'Capture' option needs to be 'Automatic' (at least in my case). Whenever I set this to 'Manual' I was never able to complete the transaction.

These are just my 2 cents in case anybody else finds themselves banging their head against the wall trying to figure out why payment wasn't being completed when it was a successful transaction!

Any updates? I applied the patch in #8 and I still receive the access denied.

A little more digging turned up the issue for me (I believe anyways).

I am using the PayPal Sandbox and it doesn't send the invoice back. paypal_payment_pps_return_access returns FALSE if this is missing.

Here is what the Sandbox was returning:

tx:9UR90742E29782031
st:Completed
amt:80.00
cc:USD
cm:
item_number:

Once I tested with production account settings it worked perfectly. Please note that this is still after the patch in #8 was applied.

Greetings!

I've spent a whole bunch of time with debugging the #46 patch, when I finally realized, it can not be the solution anyway... but I've learned a lot during this time about how this should be work and because of that, I think I've found the root of this problem.

In the original code the PayPalPaymentIPNController::validate() function was used to validate whether the user should have access to the PPN's return page in paypal_payment_pps_return_access(), which is a mistake... This function should be used only for IPN calls, because as the first comment says in the function "// Make sure this IPN was not processed before." , so if the Paypal returned the payment information through the IPN first (which usually happens) and the user try to return to the site from the Paypal, then s/he gets an access denied page, because this validation function returns FALSE, since the Payment information already arrived already and processed through the IPN call, so there is nothing to do with this PPS call.
So I've replaced the paypal_payment_pps_return_access() function (as you can see, this is quite the same as the PayPalPaymentIPNController::validate(), without the problematic part) and I've updated the paypal_payment_pps_return() function as well (moved the other part of the validation there).

Feedbacks and reviews are warmly welcomed! Thx!

Previous patch was made for the latest stable, here is the updated one for the latest dev.

Minor fix.

This solved the problem for me. $ipn_variables here is actually $_POST, but PayPal doesn't send 'business' back in the post data. Instead, it sends 'receiver_email'. I was am unable to determine if this is a configuration issue or a change to PayPal's API which I cannot find documentation for, but comparing against only the 'business' field threw an error on screen (unable to find index business of $ipn_variables) and denied access as paypal_payment_pps_return_access failed.

Uploaded the same patch twice for some reason. Sorry.

@aapis, For whatever reason, your patches in #56 aren't showing up when I follow the links.

Hello guys.

I'm implementing a payment system on a site that uses this module. At the end of the payment, the same error registered here occurs on the project.

Is there any news about this? Why the patches in #56 aren't displayed?

Can anyone help us, please?

Please try my patch from #55. #56 contains empty patch files...

Tks, I will try it.

I think this will be resolved with #2550693: IPN problem with PayPal's payment review since PayPalPaymentIPNController::validate() now allows multiple calls with the same invoice.

The original issue should be solved with the latest release. There is perhaps still the issue with the 403 as described in #42. I think it's better to open a separate issue for this though.

The issue is not solved using paypal_payment 7.x-1.2 with PPS payment method and Payment Form Field on drupal 7.54

@coopernet Please file a new issue and include steps to reproduce it with the newer versions. Thanks!