When making account creation compulsory is rather pointless.

Steps to reproduce:

  • admin > user management > user settings
  • disable email verification
  • admin > ecommerce settings > store
  • set anon purchasing policy to flexible or registered only
  • admin > site building > modules
  • enable cart block somewhere on the site
  • Start another browser
  • Add 1 or more items to the cart
  • Go to checkout
  • Register a new account
  • Look at cart in side bar and swear a few of times
  • Scratch head
  • Go buy from another site

I am unsure if the bug is in drupal core, ec cart or ec payment - I will leave you to sort that out

This has the potential lose business, which isn't good. I have only put this at normal, even though I think it is pretty critical, but I don't know the convention.

Sorry if this a dupe, I tried searching first :(

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

skwashd’s picture

Status: Active » Needs review
FileSize
663 bytes

I noticed this bug in ecommerce, it turns out that the problem was in user.module, reassigning. One of the ec devs (gordonh) helped with the patch.

gordon’s picture

Project: e-Commerce » Drupal core
Version: 5.x-3.0 » 5.1
Component: cart » user system
Priority: Normal » Critical
FileSize
540 bytes

When the user setting "Require e-mail verification when a visitor creates an account" is not enabled the user is logged in without calling hook_user('login'). This is a problem for E-Commerce when it is trying to move the cart from the anonymous user to the newly created user, and thus the cart is lost.

gordon’s picture

Title: Creating account wipes out cart contents » Registrations without email verification doesn't call hook_user('login')
drumm’s picture

Status: Needs review » Needs work

Does not apply.

gordon’s picture

Version: 5.1 » 6.x-dev
Status: Needs work » Needs review
FileSize
760 bytes

Actually this was for 5.1, but here is a version for 6-dev

gordon’s picture

Assigned: Unassigned » gordon

Bump, I need to get this into HEAD so that it can be backported to Drupal 5.x

This is giving me headaches for Drupal 5.x E-Commerce users

sammys’s picture

Version: 6.x-dev » 5.1

This is a problem with Drupal 5. A very serious problem at that. Why has this not been dealt with?

gordon’s picture

Version: 5.1 » 6.x-dev

After speaking with Killes I was informed that I needed to get this into 6.x-dev first and then he would include it in 5.x-dev.

I know this is a big problem in 5.1, but I need to get this into 6.x first, and then it can fix our problem.

Dries’s picture

This looks OK, but can you sprinkle that code with some code comments? Thanks.

gordon’s picture

FileSize
792 bytes

Here is an update to the patch for HEAD.

JohnAlbin’s picture

I think this points at a more general problem: hook_user('login') isn’t called consistently and neither are the other tasks associated with a successful login.

All of the appropriate tasks occur in user_login_submit() which is called when a user completes the user login form:

  • making a note in watchdog
  • updating user's login timestamp
  • firing hook_user('login)
  • and generating new session ID

But not all of those tasks are completed in the other places that users are logged in: user_pass_reset() (one time password), user_register_submit() (register and login immediately), blogapi_validate_user(), drupal_login().

Those login tasks should be called the same place where the actual login occurs; namely where the user is loaded into global $user in user_authenticate()

I’ve attached a patch that consolidates those tasks into one place.

Not generating a new session ID when logging in is a security issue. Isn’t it? :-s

drumm’s picture

I'd rather go with a much simpler change than the patch in #11 for 5.x.

gordon’s picture

Yes my patch just fixes the missing call, which is much better, esp for 5.2

JohnAlbin’s picture

Status: Needs review » Reviewed & tested by the community

I’ve moved the patch in #11 to “User login/authentication tasks not performed consistently” so we can get Gordon’s issue into 5.x quicker. :-)

I’ve looked closely at Gordon’s patch in #10 and it is RTBC.

Gábor Hojtsy’s picture

Version: 6.x-dev » 5.x-dev

So it looks that this patch is against 5.x and the 6.x stuff which needed more fixes is moved into another patch.

drumm’s picture

Status: Reviewed & tested by the community » Needs work

#10's patch does not apply in 5.x.

gordon’s picture

Status: Needs work » Reviewed & tested by the community

#2 is for 5.x and #10 is for HEAD

drumm’s picture

Status: Reviewed & tested by the community » Needs work

Neither does #2.

gordon’s picture

Status: Needs work » Needs review
FileSize
764 bytes

I have rerolled that patch and should be working now.

drumm’s picture

Status: Needs review » Fixed

Committed to 5.x.

gordon’s picture

Version: 5.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

This needs to be ported to 6.x, see if #10 commits

JohnAlbin’s picture

Version: 6.x-dev » 5.x-dev
Status: Patch (to be ported) » Fixed

Gordon, I’m already working on getting it fixed in 6.x. The patch in http://drupal.org/node/152497 includes your fix as well as the more general problem I described in #11 above.

Anonymous’s picture

Status: Fixed » Closed (fixed)