strangerstudios / paid-memberships-pro

The Trusted Membership Platform That Grows with You: Restrict access to content and charge recurring subscriptions using Stripe, PayPal, and more. Fully open source. 100% GPL.
https://www.paidmembershipspro.com
Other
469 stars 358 forks source link

PPE: failed account creation when session is lost #1338

Closed mircobabini closed 7 months ago

mircobabini commented 4 years ago

Describe the bug The PayPal Express gateway works and the order is marked as "success" when the users get back to the site, thanks to the routine in /preheaders/checkout.php. But the creation of the user fails, because the pmpro_checkout_new_user_array filter populates the $new_user_array getting the values from the session, but the session can be empty because of many of reasons.

To Reproduce Steps to reproduce the behavior:

CASE 1:

  1. Go to your checkout page
  2. Fill all the fields and proceed
  3. Once on paypal.com checkout page, copy the browser URL
  4. Open an anonymous tab, or change the browser, whatever
  5. Paste the link and complete the checkout on paypal.com
  6. When you will be back to the site... boom: "Your payment was accepted, but there was an error setting up your account. Please contact us."

/preheaders/checkout.php, lines from 515 to 538

Also, there are some other possible scenario, not the same case but... same result: CASE 2) When you set up with a username (ex. Dude123) and you spend a lot of time on the paypal.com page. Somebody else use the same username and complete the checkout faster then you... when you will get back to the site, boom: username already in use. Fuck them! CASE 3) When you set up with a username longer than 60 chars. Easily fixable with a fields check on checkout page. CASE n) In general, every other possible error on the wp_insert_user is messing with the checkout flow.

I know these are rare cases, but I'm managing a site with over 200 new memberships a day. Not so rare for us, we had 35 cases in the last 15 days (CASE 1). I was able to reproduce the issue just last night and added the debug script this morning, then I don't know HOW they did it, but those 35 users was able to land on the checkout page after the payment WITH NOTHING in the session. That's a fact, check the screenshots below. And this report with code:

<?php
add_filter( 'pmpro_checkout_new_user_array', function( $new_user_array ){
    print_r( $new_user_array );
}, 11 );

/**
Array
(
    [user_login] => 
    [user_pass] => 
    [user_email] => 
    [first_name] => 
    [last_name] => 
)
*/

First of all, the manual user creation process for these cases is not so clean (create the user, associate the user id to the order, manually edit the memberships_users table...). Also, very frequently users tries again to subscribe and they possibly succeed. That's good, they have everything set up by themself, but there are two success subscriptions created at the gateway. Then, I must manually cancel the old subscription and refund it... very confusing for the user who receives messages from PayPal. Needs some customer care and they don't like when weird things happens with their money, expecially with recurring payments out of their control (because of their ignorance, of course).

This issue is more serius with non-english speaking countries, because the automatic payments related emails from PayPal are in English. Do you think is possible to send a locale to PayPal in order to teach them to use a different locale when send emails to a subscribed user? Don't think so, but I can be wrong.

Screenshots issue_checkout_page_error.png issue_order_status_no_user.png

Expected behavior I know that the flow (payment first, then creating the user) is a fact that we can't change easily and by the way it doesn't solve the CASE 1, for instance. By the way there's the IPN [txn_type] => recurring_payment_profile_created which is unused at the moment, may it be a fallback for these cases? Don't know.

What I think I can do to solve it in my case, but must define something for all...: 1) If session found

That's all. I think in this way I will solve every possible scenario.

Isolating the problem (mark completed items with an [x]):

WordPress Environment

``` WordPress latest version, PHP7.2, PMPro latest version, a lot of addons (latest versions) and custom code writter by me. But it's not because of the custom code :) ```
mircobabini commented 4 years ago

Mh, what if we save the session data on a custom table?

wp_pmpro_memberships_sessions with: ID, session_id, data (serialized)

Thanks to the gateway response I know the order_id, then I know the session_id, then I know what's inside the session... it's a solid server provided session fallback when the client provided session is empty, isn't it?

mircobabini commented 4 years ago

CASE 4) "We actually just had a case on our own site last night where it just looks like someone checked out on PayPal then waited a few hours before clicking the confirm button, at which point the session was gone" @ideadude

CASE 5) I’m on paypal and share with you the payment link on whatsapp, you will go though it and the user creation fails

Ideas

mircobabini commented 4 years ago

Just created a FALLBACK FIX for this here.

Of course it's a fallback. If session is lost OR if the wp_insert_user fails are two different cases and should be handled in PayPal Express gateway checkout code.

mircobabini commented 4 years ago

Collected much more cases here. The most frequent case is about people completing PayPal after 10+ hours, when the session is obviously lost. I'm actually creating the wp user automatically:

I will complete the fallback fix above adding some email address storing earlier.

mircobabini commented 4 years ago

I completed the fallback fix above. In two words, this is the fallback:

hbcondo commented 3 years ago

@mircobabini, thanks for documenting this huge bug! What are some of the symptoms of this issue? We just noticed that the database table pmpro_membership_orders.user_id is set to 0 when this occurs.

mircobabini commented 3 years ago

The main issue is that the user is not created, this means that the user_id will be certainly set to 0. But please note that having user_id set to 0 may also depend by other factors. Try to search in the users for someone with the same first or last name as filled in the order checkout data. If exists, you are not facing this issue. Instead you should check why the user is created but not associated to the order.

mircobabini commented 3 years ago

Just prepared a new snippet which makes use of order metas instead of cookies.

This one requires the action pmpro_before_commit_express_checkout which is not integrated into core yet => PR: #1852

https://gist.github.com/mircobabini/bf9b65010124e4ab1e04c0c634681364

asadalarma commented 2 years ago

Can you please check if this issue is related to this or not ! do the upgrading the version or paid version will solve problem or not ! https://github.com/strangerstudios/paid-memberships-pro/issues/2184

andrewlimaza commented 2 years ago

There is also this PR that may fix the issue -> https://github.com/strangerstudios/paid-memberships-pro/pull/2197

andrewlimaza commented 1 year ago

I'll leave this issue open for a bit before closing it. I think with PayPal Express it's best to skip the confirmation page upon returning whenever possible. User accounts are created before payment processing is done, so it should fix this issue even upon returning as the user would be logged-in at this point.

Please keep us posted if anything has changed or the issue is still present.

dparker1005 commented 7 months ago

PPE no longer stores checkout information in sessions. It is now stored in order meta.