prevent resetting card data when reviewing form

10 replies [Last post]

Wed, 08/27/2008 - 15:15

BenStallings

Offline

Joined: 03/13/2008

Juice: 91

Hello! My employer has directed me to "fix" what may have been intended as a security feature...

If you are filling out the checkout form and fail to fill a required field, or put an invalid value in a field, you are returned to the checkout form with a warning. The credit card number is replaced with (Last 4) and your last four digits. So far so good.

The trouble is, the rest of your card data is reset -- the card owner, the expiration date, and the CVV -- and there's no indication that it has been reset, so that users tend to submit the form a second time without filling in those blanks again and get another error.

So my challenge is to keep these fields from getting cleared. Looking at uc_credit.module, I don't see where they are getting cleared... the cc_owner field should have a #default_value of $order->payment_details['cc_owner'] ... does the lack of data in that field mean that $order->payment_details is not set at that stage in the process?

Please advise. Thanks!

Tue, 09/02/2008 - 14:41

#1 View page

Ryan

Offline

Joined: 08/07/2007

Juice: 15438

Re: prevent resetting card data when reviewing form

Interesting... so it's not setting the CVV to --- or something? I don't know that I tested it with the owner field enabled or not. Can you verify what version of UC you're running and maybe confirm the bug on the Livetest to make sure we're not dealing with an old bug?

Wed, 09/03/2008 - 15:55

#2 View page

BenStallings

Offline

Joined: 03/13/2008

Juice: 91

it's on the Livetest too

Yes, I can confirm it on the Livetest. If I go to checkout, and I don't fill in one or more required fields, but I do enter a credit card number and expiration date and CVV, and I click Review Order, the cart number is replaced by the last 4 digits, but the CVV field is cleared (not ---) and the expiration date is reset to January 2008, and there is no indication on the page that this information has been reset (besides the fields themselves), so that if I now fill in all the other required fields I missed the first time and resubmit the form, I get another error telling me to fill in the credit card info I already filled in before!

So it is not just my site; it's either a bug or a feature of Ubercart. But since you ask, we're running 5.x-1.3.

Thanks in advance, Ryan.

Wed, 09/10/2008 - 15:42

#3 View page

Ryan

Offline

Joined: 08/07/2007

Juice: 15438

Re: it's on the Livetest too

So... I found out this is actually a tough nut to crack. I've spent half the day on this and simply have to move on. Fixing your initial problem was simple... but it turned up the fact that the session variable storing encrypted CC data wasn't being unset properly when checkout form validation failed. I'm hard clearing that now in hook_exit() on non-checkout pages, but this is less than ideal. I'm keeping an unset line in the credit card form builder func, but the problem is changes made to the session on the initial pageload through that function aren't actually affecting the session. I think it's something about the timing of the page request w/ $(document).ready() in jQuery. It's eluding me for now, but at least we're a step closer to the ideal.

Wed, 09/10/2008 - 16:36

#4 View page

BenStallings

Offline

Joined: 03/13/2008

Juice: 91

where?

When you say you've fixed the initial problem and brought us a step closer to the ideal, where is that fixed version of the module? In the 5.x-1.3-rc1 release on the downloads page? Or just on your computer? I'm eager to try it. Thanks again, Ryan!

Wed, 09/10/2008 - 17:05

#5 View page

Ryan

Offline

Joined: 08/07/2007

Juice: 15438

Re: where?

It's committed to Bzr and will be in the 1.4 release.

Wed, 09/10/2008 - 17:08

#6 View page

kerunt

Offline

Joined: 05/05/2008

Juice: 158

BenStallings wrote:When you

BenStallings wrote:

It's probably in the latest version in the Bazaar: http://www.ubercart.org/bazaar

Thu, 09/25/2008 - 17:34

#7 View page

Ryan

Offline

Joined: 08/07/2007

Juice: 15438

AHA! I fixed it. >:) Took

AHA! I fixed it.

Took long enough. Someone else raised the issue and I was able to work a fix together that involves calling the payment method's cart-process function during the form alter stage of the rendering. That's a rough way of referring to the problem which is almost too much to describe in a quick forum post.

Basically...

1. Page load renders the form, including all hook_form_alter()s. Credit module doesn't see any CC data in the session that needs to be moved to the form so it does nothing. However, CC data was entered by the customer and will get processed during the validation stage.
2. We need to validate the form... everything happens except a required field was left unfilled. DAAAAAA! Drupal displays the form as is, meaning the cc module has no change to alter the data into the form...
3. So when the AJAX to render the payment details section is called, it has no CC data to plug into the fields. A fluke of the rendering logic presents this is as though it's truncated the number... but it's lieing.
4. So when the form gets submitted, there's no CC data being processed and we fail.

My fix involves changing step 1 a little bit so that the form alter runs the credit module's validation routine a little early, which is where the data gets put into the SESSION for storage in the form. This still happens just like normal, and normal CC validation gets processed later that can cause everything to fail if, say, an invalid CC number was put in.

So... after 10 hours of getting nowhere, the last bright idea has brought this issue to a close. The final fix will be in the 1.5 release - hoping for tomorrow, but no promises. It's in bzr for sure.

Thu, 09/25/2008 - 17:52

#8 View page

Ryan

Offline

Joined: 08/07/2007

Juice: 15438

Re: AHA! I fixed it. >:) Took

Alrighty... just committed this and I'm attaching a patch if you need to review it / commit the patch manually. Please let me know if there's anything missing as I'm assuming this is now 100%. Many thanks to Acquia for sponsoring the last development needed to bring this patch into reality.

Geez... this is seriously the most awful bug I've had to resolve. I think I spent close to 10 hours mucking through this issue. Let's hear it for persistence.

Attachment	Size
credit_fix.patch.txt	4.45 KB

Fri, 09/26/2008 - 09:42

#9 View page

BenStallings

Offline

Joined: 03/13/2008

Juice: 91

Hooray!

That's awesome! Thank you, Ryan!!! And Smartphone Magazine iPhone Life thanks you, too!

Mon, 02/23/2009 - 16:40

#10 View page

kris

Offline

Joined: 02/09/2009

Juice: 30

Re: Hooray!

I think I have this exact same issue, but I'm running Ubercart 2.x, so that patch doesn't work for me.

If I'm on the checkout page, and I click the checkout link in the nav bar, the page reloads, resetting the credit card expiration date, and cvv number, and displaying "(Last4)1111" (or whatever the last 4 digits happen to be) for card number. If the user enters in their cvv number and expiration date and hits submit at this point, they will be told they entered an invalid credit card number as it tries to submit (Last4)1111 as the number.

I don't mind that it doesn't store the number. However, I do mind that it leads the user to believe it is storing it, if it's not. Is there some fix for this? Any help would be great! Thanks!

Ubercart

Navigation

User login