Closed (fixed)
Project:
Ubercart
Version:
6.x-2.x-dev
Component:
User Interface
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
13 Dec 2009 at 06:16 UTC
Updated:
23 Sep 2011 at 14:21 UTC
Jump to comment: Most recent file
Comments
Comment #1
giorgio79 commentedI just checked if the user optionally fills out username and password.
In that case the user is logged in!
So this seems like a bug, as the user should be logged in, even if Ubercart assigns a username and password to them...
Comment #2
Chad_Dupuis commentedsub
Comment #3
artatac commentedsub
Comment #4
vitis commentedsub
Comment #5
rszrama commentedThere's a separate setting for automatically logging in the user. It already exists but I don't believe it's enabled by default. You might do some testing to make sure it works for you as expected... otherwise I'm marking this as "won't fix" since I'm not sure there's anything here to fix.
Comment #6
giorgio79 commentedThanks Ryan, yes, I have that enabled.
As I mentioned, if the user fills out the username and password on the checkout pane, it works fine, and the user is logged in!
However, if the user does not, and Ubercart is assigning it, it does not work, meaning the user is not logged in.
Test it out! It is quite confusing when you read it.
So just to recap on checkout settings:
ENABLED Enable anonymous checkout (users can checkout without logging in).
ENABLED Login users when new customer accounts are created at checkout.
ENABLED New customer accounts will be set to active.
Now, entering username and pass is optional, email is still compulsory.
1. If you enter username and pass (in addition to email), you are logged in.
2. If you don't enter username and pass (only email), then ubercart creates them, but you wont be logged in.
In case 2 user should be logged in.
Comment #7
rszrama commentedAhh, ok. I didn't gather from your issue that you had the auto-login setting enabled. Guess we'll need to look into it... it should work either way. : )
Comment #8
Hossam commentedsub
Comment #9
giorgio79 commentedI see a lot of people subscribed, could one of you please confirm this issue as well?
Comment #10
Hossam commentedI think we are all having this issue, personally I do.
Comment #11
giorgio79 commentedI have been trying to investigate this.
As I understand
function uc_cart_complete_sale is handling the user login and reg in uc_cart.module file.
It seems login is set in the code to false automatically like, this
and later on it checks if it is enabled, yet the code does not update $login anywhere, so this code is never executed:
I tried setting $login = TRUE, but that did not help, so there seem to be a multitude of issues working together and breaking this functionality.
Also found this:
http://www.drupal.org/node/335025
Where they are using user_authenticate instead of drupal_execute to login the user.
Any help is appreciated.
Comment #12
giorgio79 commentedAlright, the anonymous login works fine with the test credit card gateway. So it is important to mention that this happens for me with Paypal WPS.
Comment #13
giorgio79 commentedI also notice in uc_paypal_pages.inc
uc_cart_complete_sale($order);
should be
uc_cart_complete_sale($order, variable_get('uc_new_customer_login', FALSE));
as per the uc checkout files, otherwise anonymous login will never be enabled for paypal payment
Reported this in a separate issue #668038: call uc_cart_complete_sale consistently across gateways
I fixed this in my code, still anonymous login does not work, so there still is something wrong.
I Was looking at how the credit module handles login, but did not find any calls to uc_cart_complete_sale so at the moment this is a mystery to me.
Comment #14
giorgio79 commentedI hope it is ok if I put this on critical, given a core feature is not working. This is also a show stopper for my projects, as a smooth checkout ensures low refunds, and happy customers. :)
Comment #15
giorgio79 commentedCrap, I think I just figured this out, based on this issue
#369958: anonymous user check out with setting to create new account fails... because email address not returned from paypal?
Basically, the email I used on the ubercart site on checkout did not match the test customer email on paypal... This caused the user not to be logged in.
I guess there are use cases where the user has a different email for paypal but I think I am going to sleep now :)
Phew...
Comment #16
WebNewCastle commentedFor those who may still be having this issue, you may want to check when you are testing to see if your test purchase account may actually be logged in. That is, if you get through to the completed order message / pane asking if you want to log-in, try clicking on the log in link (/user) if you didn't already to see if you aren't logged in.
When I was testing this out and using the default completed order pane, I found this message presented by the newly created account was actually logged in - even though the messaging appeared otherwise.
Comment #17
bob_b commentedHi
On localhost users get logged in, even though order status never gets further than pending. As soon as I move to a live site though I get:
I fixed the duplicate emails with conditional actions, but new users are still not being logged in.
I made sure, and this is not the case
This is not the case either. I am not logged in wither way
Also doen't help me, but this could have something to do with it because the user is logged in only when PayPal can't send anything back (localhost)
Anyway, I hope this can be fixed! :)
Comment #18
bob_b commented*please disregard the following - see edits below*
I just had a breakthrough!
I changed the Input formats for the checkout messages (/admin/store/settings/checkout/edit/messages) to a Full HTML filter without pathologic enabled, and new users are now logged in!
EDIT: It's *almost* all working fine for me now. If a new user uses Paypal with a credit card, they are not logged in. They are instead taken to the "Checkout completion for existing users: " and says it has attached their order to the account it found matching the email address. I think this is because of auto return not working when you use a credit card. It works fine when you use a Paypal account.
EDIT: ** INPUT FORMATS WERE NO THE PROBLEM! ** I changed the currency at the same time that I changed the input formats, and didn't realize till now that it was the currency that fixed the problem! Sorry!
Comment #19
smscotten commentedbob_b, can you be more specific? How were the currencies the problem? Was there a mismatch? What did you change the currency from and to? I'm using US Dollars for everything and new users aren't being logged in.
Comment #20
smscotten commented@giorgio79: Did you actually get your users logged in when checking out through PayPal?
It seems that there is nothing I can do to make this work. I've made the changes to uc_paypal_pages.inc and uc_cart.module as giorgio79 suggested. I've made sure that I use the same email address in both Ubercart and PayPal, I've generated new sandbox addresses and not reused addresses, run an order through the live PayPal server instead of the sandbox, and I have checked to see that I'm really not logged in; that it's not just a message telling me to log in falsely as WebNewCastle suggested.
I'm at the end of my rope. I've spent two full days trying to figure out why this feature does not work as advertised, but I'm stumped.
I'm setting this back to "active" because the issue is clearly not just a duplicate of the issue with mismatched email addresses.
Comment #21
giorgio79 commentedI decided to use Paypal Buttons for Ubercart.
Comment #22
bob_b commentedHi Splicer
It's really confusing, and I'm not 100% sure that it was the currencies, but heres what happened:
I had a store configured with USD, and was using a PayPal Pandbox account with a US credit card. When I changed from using USD in the store to AUD, but still keeping the US Paypal account, it seemed to be working. It stopped working when I changed back.
I first noticed this when I took my "previously working" Ubercart setup and went live; that was the first time the paypal accounts changed. Playing around the next day in Sandbox it seemed that currencies were the cause.
Please note though, theres a chance that it could have been something else, as I was in a real rush at the time.
In the end I just gave up and disabled "automatically log in new users".
Comment #23
valante commentedI can confirm the following scenario:
1. System is set to auto login, active new users etc.
2. Site requires confirmation email
2. A new user enters email but not the optional username and password
3. Pays through PayPal Website payments
If user entered the *same* email address on the PayPal page, he is taken to the "New users" completion page and is logged in.
If user entered a different email address--I think it's a very likely scenario--he is taken to the "Existing users" completion page, and is sent a verification email before he becomes active.
Grrr....
Edit: Confirmation email is courtesy of LoginToboggan. And now no matter what combination I try, I can't get it to even display the new user completion text....
Comment #24
codepeak-1 commentedsub
Comment #25
codepeak-1 commentedMade a custom module to fix this issue, will share the code later on when it's tested enough.
Comment #26
Gham commentedHi .. wondering if you've finished testing this yet ?
H.
Comment #27
drupeo commentedHere's what I know about this issue. Using WPS, with the following shop settings:
* Enable anonymous checkout (users can checkout without logging in): TRUE
* Send new customers a separate e-mail with their account details: TRUE
* Login users when new customer accounts are created at checkout: TRUE
* New customer accounts will be set to active: TRUE
* Require e-mail verification when a visitor creates an account: FALSE
If you have PayPal’s AutoReturn switch turned on it hits uc_paypal/wps/complete/ORDERID just after it hits the IPN. Since both methods end up calling uc_cart_complete_sale(), and it is usually the IPN doing this first, the IPN call auto creates the user, but it does not pass the $LOGIN parameter as the IPN is not the HTTP connection that represents the user to login. Milliseconds later the Return URL’s invocation of uc_cart_complete_sale() finds that the user account exists, so the auto logon code isn’t called (since it’s within the code to create the user).
The $login check needs to be moved to after the logic to create a new user, possibly checking the created time of the user in the db, or a flag in their profile etc. The session cannot be used though because the IPN could be creating the user.
Comment #28
Keith Hurst commentedsubscribing, having exactly the same issue.
Comment #29
kalm commentedSubscribing, I'm having the same problem as well.
This issue is really making the checkout process very confusing for the customer as they buy a digital download but cannot download it as they are not logged in.
As a temporary (very messy) solution on my site I'm going to try a hack of making a login / register block and CSS hacking it to cover up the checkout button so that users cant checkout until logged in, unless anyone else can come up with a solution.
Comment #30
that0n3guy commentedsubscribe....
Comment #31
torgospizzaSubscribing. Does this issue still exist in 2.4? Seems like a pretty ridiculous logic bug. @kalm, can you write a patch I can test?
Comment #32
that0n3guy commentedYes, I believe this is still an issue.
Comment #33
torgospizzaComment #34
that0n3guy commentedthanks torgos.. shoulda did that. This and the duplicate email thing is driving me crazy...
Comment #35
that0n3guy commentedOh wait.... if I uncheck "Require e-mail verification when a visitor creates an account" in admin/user/settings then this works just fine.
BUT, it makes sense that if someone gave me money, that is good enough to verify their account. So this should override that setting.
Money > Email verification :)
Edit again 9/25... so it "worked" like 4 times but not it doesn't. But I noticed that when it "worked" (meaning I was logged in) it would not mark my order as complete.
So basically, I'm logged in, but order isn't complete... or not logged in but order is complete. I'm still trying to figure out why it does it sometimes, but not others...
Comment #36
that0n3guy commentedOk, after many many tests, it seems like a timing issue... not sure why.
The 2 different "timings" are: click back to the site (from paypal) as fast as you can, or click back to the site after 5 or so seconds (or auto-return to site after 5 or so seconds).
If the user clicks back to the site right away:
The user is autologged in but the order never makes it to complete. It makes it to payment received.
If the user clicks back to the site after 5 or so seconds or is auto returned (paypal setting):
The user is not auto logged in but the order goes to complete.
Please note that I am using this patch (http://drupal.org/node/644538#comment-3001168) for most of my testing.
Also note that I am using a different email with paypal than on ubercart.
Comment #37
torgospizzaYou've double-checked your Conditional Actions right? Payment Received is technically a "post-checkout" status. It seems to me like I would make sure you have a CA that is set to trigger "when a status is updated" and have it check the balance. Also you can restrict it to only the "payment received" status, so that if an order is Payment Received, but the balance is $0, you can use CA to update it to Completed.
It's not the best workaround, and I'd like to see it fixed in uc_paypal. IIRC there is actually a hard-coded uc_complete_order('payment_received') in either uc_paypal or the paypal_ipn include file. I would do a grep in those files for the word "payment_received" - I can't remember exactly what it is or when it gets fired, but I think it might be the culprit here.
Comment #38
that0n3guy commentedI've triple checked by Cond. actions... I stuck some dsm's all over inside of ca.module to see what was firing and what wasn't... I think that sometimes uc_checkout_complete_paid is stopped because it sees its balance not as zero, which causes it not to be set as completed.
Here is my question for you, the ipn is sent from paypal at some random time after the user completes the purchase on the paypal end... right? If this is so, then the login of new users should not be called by the uc_paypal_ipn function. It should be called on the uc_paypal_complete (the url that paypal sends the user back to) function.
I've seen it take 20 minutes for an ipn to finally get to my site. Or I've seen it send way before the user gets back to the site. In both cases the user isn't logged in.
Also, why would do we have to "wait" for the ipn to say that the payment is complete or recieved? If your coming back from paypal, doesn't that mean you paid?
Comment #39
torgospizzaWaiting for the IPN is just a fact of life when it comes to PayPal WPS, although I've seen some interesting uses of it, such as on the woot.com site. It seems like their system is a little better, in that you come back to their site after visiting PayPal, and THEN you have to complete the transaction. At least I think this is WPS, it could be WPP however. But regardless, it doesn't matter if you are coming back from the PayPal site - it's required that the IPN contact your site to complete the transaction.
I think, technically, they don't have to wait for the IPN to trigger a completed status before being logged in - I mean, that's what it sounds like from what you are saying. Truthfully, the two should be mutually exclusive. You should be able to complete checkout, get logged in, and then see the status of your latest order, even if the IPN hasn't come into your site yet. I'd be interested in seeing where exactly the login functionality occurs.
So IMO the way it should work is, user completes checkout, is logged in, they can go to their order history, and maybe they see a PayPal order still in "Pending" - when the IPN comes in, suddenly that order is Completed (which they see if they refresh) and they can go about their business.
Since I don't have anon checkout enabled yet I can't test that but I can try and do some digging if it'll help.
Comment #40
that0n3guy commentedOk... I think I figured it out. In the whole process the function "uc_cart_complete_sale" is called twice. Once by the IPN and once by the person returning to the site. The function does several things:
- creates the new user if necessary
- returns the content on the "complete page"
- login the user if new user
- calls up conditional actions up to do their thing
So let me give you a quick example:
You buy something. That day paypal is on their game so they get the IPN back to your site before you get back. So the IPN creates a new user and tries to log that user in (but can't because its just code... not an actual browser, so its really logging in paypal). It also sets the complete page message as a new user (which only "paypal" will see... not you).
So now you make it back to the site. The function is run again and it thinks your an existing user, not a new user. Since it thinks your NOT and existing user, it doesn't create a new user (which is good) and it doesn't set the right complete page text (which is bad), it doesn't login the user (which is bad) and it calls up conditional actions (again, possibly a reason for this http://drupal.org/node/644538).
If you make it back to the site before the IPN, it works, but thats iffy.
Comment #41
that0n3guy commentedOk, attached is a patch. It has some duplicate code kinda like:
The reason for that is that I wanted to fix the paypal wps without screwing up the other payment methods.
This fixes the user is not logged in issue as well as http://drupal.org/node/644538 for paypal wps.
Comment #42
that0n3guy commentedThe patch above looks like I made a million changes, but really I only change a couple little things:
- changed the conditional action "Update order status upon checkout completion with full payment" so that it had its own trigger and wasn't associated with the admin & customer emails. (helps with duplicate emails)
- made the above conditional action only triggered by the IPN and not the user (only if your using paypal wps).
- gave paypal_wps its own section in the uc_cart_complete_sale function.
- gave uc_cart_complete_sale the ability to detect when its the IPN flagging it.
- changed the way the user is auto-logged-in if the IPN gets back to the site before the user.
Comment #43
torgospizzaLooks like a hell of a patch. Andy, Lyle, TR, care to review this? I'll test on dev and production soon, too.
Comment #44
tanjerine commentedsubscribing. wish me luck. i'm going to apply it right now and test.
Comment #45
that0n3guy commented@torgosPizza, Like I said above, I really didn't change that much. It just looks like it in the patch because I duplicated stuff and changed multiple files.
I have it running on a dev site with zero problems so far. Its soon to go production.
Comment #46
torgospizzaSweet, glad to hear it. Nice work. :)
Comment #47
rosemeria commentedHello thatOn3guy,
I have tested your patch and I am getting a "white screen" on my return from paypal(sandbox) on the /cart/checkout/complete page. It is a very large patch - but I performed the patch changes twice, thinking it might be a typo. Maybe you can send me your uc_cart.module file and I can check again -- to confirm that I have no typos in my code.
Order status on order report: payment received
User:none (no user account was made for new user)
tanjerine - how did your test go?
I would like my new customers to be already logged-in to their new accounts upon returning from paypal!
Hope we can get this working... thanks for all the hard work...
Aloha,
Rose
Comment #48
longwave@rosemeria: it's easier to apply patches automatically using software than do them by hand! See http://drupal.org/patch/apply or http://drupal.org/node/620014 for more information.
Comment #49
that0n3guy commentedHmm... I just tested the patch the "normal way" (ie
patch < 0001-fixed-duplicate-emails-and-new-user-login-for-paypal.patch) and it failed a bunch of times.But I created the patch with git so this is how you apply the patch with git:
- download ubercart 2.4
- Go into the ubercart directory and then start the git fun:
All that initialized a git repo, commits the code and downloads the patch.... To apply it:
Please note that this patches 3 files, not just uc_cart.module.
My attempt at creating a patch using diff (like shown here: http://drupal.org/patch/create) resulted in a patch that would still fail by applying it "normally". but the git method works...
Comment #50
longwaveHad a quick look at this but I'm not sure this is the right approach. Hardcoding the check for paypal_wps into uc_cart_complete_sale() seems wrong, and also there's a dd() debugging statement left behind in your code - maybe the source of the WSOD that rosemeria is seeing, if they don't have devel installed.
Why does the IPN function have to call uc_cart_complete_sale() at all? Is it a common enough case that users pay at PayPal but then never return to the site to see the completion page?
Comment #51
torgospizzaWhy does the IPN function have to call uc_cart_complete_sale() at all? Is it a common enough case that users pay at PayPal but then never return to the site to see the completion page?
In my experience, yes, users will click the link to go back to the site maybe half the time. I've heard reports that this causes the IPN to fail - I have no proof of this, but it seems to happen often enough to warrant some kind of guarantee that the PayPal order goes to completion.
Comment #52
that0n3guy commentedtorgospizza,
I wondered why it had to call it as well, your right, it really doesn't need to call it. But, we have to call something like ca_pull_trigger('uc_checkout_complete', $order, $account) to flag it as complete and to run whatever other conditional actions are around. That function needs $account, which the ipn function doesnt have.
Thats not a problem if the ipn gets back to the site after the user does (because the users account is created when the user returns) as you could just user_load from the user in the $order. But if the user gets back after the ipn (or at the same time) then the ipn needs to create the account.
So if the IPN needs to create the account, why doesnt it just use uc_cart_complete_sale() instead of copying the user creation code into the ipn function.
Either way you still need to modify the ca_ stuff so that you don't get duplicate notifications. You also still need to do some of the other mods so that it logs the user in if the user is new but the account is already created by the ipn.
Comment #53
torgospizzaI wonder if we can at least add the $user->uid to the form that gets posted to PayPal? I know there is a "custom" field that is returned in the IPN response code... I wonder if we can use this to host that uid data so we can fire some CA events?
EDIT: This might also help with some of our other issues, including the uc_recurring stuff.
Comment #54
longwaveDoes #423546: Using PayPal with Anonymous checkout gives merged not new account still apply, and is it relevant here?
Comment #55
that0n3guy commentedSooo... I looked through the patch that longwave posted. I like the way cha0s did that. Attached is patch with cha0s's patch and a couple other little things that gets the user logged in correctly.
This patch:
- still has duplicate emails if the user gets back to the site before paypal does.
- MIGHT screw with other payment methods a little because it uses new user info out of $order instead of $_SESSION:
Oh, this is patched off 2.4 of ubercart with git.
Comment #56
krabbe commentedsubscribe
Comment #57
djflux commentedBTW, you can use "patch" to patch your source if you use the -p option.
Here's how I patched using the patch in #41:
The -p option removes the number of pathes from the front of each patch section that you specify (e.g. -p1 removes one patch which makes a/payment/uc_payment/uc_payment.ca.inc becomes payment/uc_payment/uc_payment.ca.inc).
Applied patch in #41 to my test environment after removing the dd() line in the patch and will report my results when I can test. Hopefully this patch also resolves the duplicate stock decrement as described in #644538: Duplicate order notification e-mail, and duplicate stock decrement
Comment #58
tanjerine commented@rosemeria: (sorry just saw your comment) .. the tests went well. the patch fixed the duplicate email bug that the site was experiencing.
Comment #59
that0n3guy commented@tanjerine, patch at #55 is a better patch (it has cha0s magic in it :P ) but it doesn't solve the duplicate email issue.
Comment #60
tanjerine commented@that0n3guy, i would love some cha0s magic in the patch i'm using ... but unfortunately, my client freaks out at the duplicate emails. :(
Comment #61
torgospizzaAre the dupe emails something that can be fixed by tweaking a Conditional Action?
Comment #62
that0n3guy commented@tanjerine, yeah I've been doing more and more ubercart sites lately so maybe I'll get around to doing the dup. emails fixed w/ the other patch.
Comment #63
tanjerine commented@that0n3guy that would be cool! i'm back at working with ubercart again but more in the capacity of cleaning up other people's code (inherited code) but i'm hoping to poke around the patch too when i have some down time. thanks! :)
Comment #64
dbrant commentedSo, I think I'm having the same issue. For me, however, a new user account never even gets created after PayPal redirects back to my site. The order just gets assigned to the anonymous user (0), and the original user information gets lost. Is this the same issue?
I'm pretty sure I've configured everything correctly... what else could I be missing?
- YES to "enable anonymous checkout"
- YES to "login new users after checkout"
- NO to "require email verification for new accounts"
- tried it with HTTPS and without
Comment #65
that0n3guy commentedAlright, posted a patch that fixes this and duplicate email issue: http://drupal.org/node/644538#comment-3771270
Its a mod of #55 above w/ duplicate email fixes in it.
Comment #66
that0n3guy commentedJust an update in here, I created a new patch here: http://drupal.org/node/644538#comment-3973274 .
I consider the old one in-secure now (see: http://drupal.org/node/644538#comment-3972176).
Comment #67
YK85 commentedsubscribing
Comment #68
mrogers commentedsubscribing
Comment #69
hanoiithis should be sorted out in the last patch on #644538: Duplicate order notification e-mail, and duplicate stock decrement
Comment #71
hanoiiAs mentioned in #69, a patch that attempts to fix this as well is in #644538-291: Duplicate order notification e-mail, and duplicate stock decrement, tests on that patch will be really appreciated.
Comment #72
hanoiiComment #73
longwaveThis has been fixed in 6.x-2.x-dev as part of #1192018: Duplicate order notification e-mail, and duplicate stock decrement