I'm trying to set up a site with PayPal and UC Recurring and some other nonsense that's not really relevant. Here's what is!

This error?

We were unable to process your credit card payment. Please verify your card details and try again.  If the problem persists, contact us to complete your order.

Totally vague and unhelpful. :(

However, if I hack uc_paypal.module and toss the following in the uc_paypal_wpp_charge() function:

  ...
  $nvp_response = uc_paypal_api_request($nvp_request, variable_get('uc_paypal_wpp_server', 'https://api-3t.sandbox.paypal.com/nvp'));
  # HACK. DON'T DO THIS. IT WILL BARF CODE AT YOU.
  var_dump($nvp_response);
  die;
  ...

Buried in that glop I get back is the extremely specific and helpful:

["L_LONGMESSAGE1"]=> string(96) "There's an error with this transaction. Please enter a valid postal code in the billing address."

Is there a way for uc_paypal.module (and, presumably, other payment gateway modules who all store their errors in a different format) to percolate that more specific error up the stack so that uc_credit.module can display it to an end user, and they have some clue how to proceed?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave’s picture

I've solved this in another payment module (uc_protx_vsp_direct) by parsing the gateway response and setting $GLOBALS['conf']['uc_credit_fail_message'] to a more helpful message where possible, it's a pretty horrible hack but it works.

TR’s picture

This comes up from time-to-time, and at some point I wrote an eloquent explanation of why it shouldn't be changed. I can't find that now, but let me summarize:

When you log on to a computer (or a Drupal site for that matter) you are asked for a name and password. You have to get both right to log on. If you get one wrong, you are not told which one. This is deliberate; Telling you which one is wrong is the same as confirming that the other one is correct. The same goes for credit card information, only more so. If you tell the customer his ZIP code was entered incorrectly, that tells a potential identity thief exactly what part of the stolen card information needs to be changed to have that credit card work on *any* site. Not only do you not want to expose to the customer what validations are being made, but you don't want to give a malicious customer a test bed for verifying *and correcting* stolen card information.

IMO, there is only a slight loss of usability by simply telling a customer that his/her data is incorrect, please check it carefully. This slight loss of usability is more than compensated for by the increase in security, and besides this is how all other e-commerce sites work - the customer should be accustomed to looking at the data and determining what's wrong. And this information is not random stuff like order numbers - it's name/address/phone number data that should be excessively familiar to the customer.

That said, I support logging more detail in the watchdog to help the admin figure out what's going on.

longwave’s picture

The message in webchick's example appears to be a validation failure rather than an authentication failure; asking for a "valid postal code" suggests that the postal code is not valid at all for the given country, rather than failing a specific test for the customer's card. Most payment gateways appear to already work to avoid fraud in the way TR describes, returning generic "not authorised" responses when the bank refuses the card, but detailed, useful messages when part of the supplied details are invalid rather than incorrect.

I think it's also worth pointing out that the default message is misleading in saying "card details", as it's often not what the customer thinks of as card details (number, expiry date, CVV) that caused the failure. Perhaps the word "card" should be removed at least?

longwave’s picture

Further to this, PayPal actually suggest you should display the "long message" field to the user:

When displaying an error message for a failed transaction to your customer, PayPal suggests one of two methods:

1. Map the PayPal error code number to your own custom message. (Recommended)
2. Display PayPal's long message directly to your customer. We do not recommend showing the short message back to the customer.

(from https://www.paypalobjects.com/IntegrationCenter/ic_uk-direct-payment.html)

webchick’s picture

TR: Ok, I can see that perspective. But longwave is correct that in this case I had entered "12345" for the zip code as I always did, only I had chosen Canada as the country. I naturally assumed that it wasn't doing any address validation, since "12345" is not a valid zip code for Alabama, either. Then, when I got this error on the dev site I spent the next hour trying to figure out how I had configured PayPal wrong, what firewall rules were in place, etc. before I finally gave up and started throwing var_dump()s around and figured out what the problem actually was. :\

So yeah, some additional clarity in watchdog() at the very least would definitely be helpful, and probably would've shaved at least 20 minutes off that hour. But IMO, I'd just display the message, especially if that's what PayPal wants you (as an implementer) to do.

webchick’s picture

And yes, the term "card details" was definitely the start of the whole rabbit chase. Had it just said "details" I might've tested "H0H0H0" instead. That's a nice suggestion, longwave.

webchick’s picture

Also, fwiw, here's what Amazon does, as a point of reference:

Address data

It lays out pretty explicitly which part of the address/phone number I've messed up. I have no idea why it takes it three passes to do this, however. :P

Errors indicate city, state, and postal code are invalid
Once semi-valid location data entered, errors about address
once semi-valid address entered, errors about zip code and phone

Credit cards

It does tell me when I've entered just blatantly wrong values:

Flags invalid CC and expired card

But it does NOT validate plausible information during the "add a card" step at all:
Visa from smokey the bear

Uh. Even more interesting, it lets me through the entire transaction. WTF?

Your order has been placed

I assume my mailinator address will have some email telling me I suck later. Or maybe this only worked because I entered one of the universal test CC numbers. Interesting.

Well that was fun. :)

The point being, though, the #1 e-commerce site in the world (and the biggest security attack lightning rod in the universe) does validate some basic stuff, like making sure the address (by itself) is right, the credit card details (by themselves) are right, just like Drupal will tell you if you forgot to fill out your username field. But I do totally see the perspective that we should not tell people the combination of a plausible billing address + plausible credit card which part is right or wrong, and use a vague error similar to the login form when that happens.

Make sense?

longwave’s picture

As I said, payment gateways I've worked with already seem to take care of the "plausible details but failed authentication" scenario by returning a generic message, to avoid fraud in exactly the way TR describes; why would they send a human readable response that could help to enable credit card fraud, if a poor implementation of their API may end up showing this to the customer?

The only use cases I can think of where the payment gateway would want to inform an administrator but not the end user is where payments would always fail, such as "payment gateway username incorrect" or similar, but this would show up when the administrator is initially testing the site. At the end of the day, if a bank wants to refuse a transaction but doesn't want to explain exactly why to an end user, it's none of the store owner's business either.

TR’s picture

I think there are two separate things going on here: 1) form validation, and 2) card details authentication. My argument applies only to the authentication.

Ubercart doesn't do any field validation for the address, and does only minimal validation for the credit card number. Ubercart checks the length of the card number and validates the checksum with the Luhn algorithm, so if you enter a 1-digit credit card number in Ubercart and you should get a form error with that field highlighted telling you it's an incorrect number, just like Amazon. But entering an invalid ZIP code won't fail on the Ubercart end.

It seems that PayPal does perform some address validation on its end before attempting to charge the card, so if we can distinguish a PayPal validation error from a card authorization error (a decline), then it would be good to pass on the info to the customer with a form_set_error() like you said in the original post. I would still be against pointing out which field was wrong in the case where the card was declined because the billing address didn't match the address on record, the CVV was wrong, the expiration date didn't match, the cardholder's name didn't match, etc.

TR’s picture

Re: #8
I haven't worked with PayPal's API in many years - it seems it's a lot more sophisticated now. But I have written a number of payment gateways recently for minor players in the field, and those didn't perform validation. They just gave me back an accept or decline, along with a reason for decline like ZIP didn't match. It's in that context that I say the error shouldn't be passed on to the customer. I don't think we can assume all payment gateways will sanitize the error message in the case of simple validation errors. But if we can distinguish a validation error (you entered 4 digits for your ZIP, we were expecting 5) from an authentication error (you entered a New York ZIP code but your card is registered to a person living in Arizona), then I think we should show the validation error.

longwave’s picture

Take a look at the "gateway decline" errors at https://cms.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=develo... - the "long message" is what they recommend you show to the end user.

The only two I can see that you may want to filter out according to your argument are 10756 "The transaction cannot be processed. The country and billing address associated with this credit card do not match." and maybe 15004 "This transaction cannot be processed. Please enter a valid Credit Card Verification Number." - but PayPal's docs still suggest that you should show these messages to the customer if they are returned. All the other ones, especially the "invalid data" ones, are much more helpful to the customer than the generic (and badly worded) message currently displayed by Ubercart, and won't be of assistance to fraudsters.

travist’s picture

Here is a patch on what I did to solve this issue.

It has been tested to work very well with Authorize.net. Not sure about PayPal.

longwave’s picture

Status: Active » Needs work

That patch unfortunately changes the return value of uc_payment_process() from boolean to array, which will break existing modules that currently assume the return value is a boolean.

travist’s picture

It shouldn't break those modules. I made it reverse compatible, where it checks the type returned from the payment module.. Like so..

if ( (is_bool($result) && !$result) || (is_array($result) && !$result['success']) )  {

This way, if they return the old way, it just works the way it always worked, otherwise, it will add more information to the message.

Let me know your thoughts.

Travis.

longwave’s picture

uc_payment_process() now returns an array instead of FALSE if the payment fails. If a contrib module does this:

$result = uc_payment_process(...);
if ($result) {
  // payment succeeded, so do some processing
}

then they will incorrectly execute the if() block if payment failed, as $result will now be a non-empty array, which casts to TRUE despite the fact that $result['success'] is FALSE.

travist’s picture

I see what you are saying, and yeah, that is a problem. I was under the impression that most modules would not call uc_payment_process since they can implement a charge using the payment hooks. What contrib modules actually call uc_payment_process? This seems like a function that should ONLY be called by uc_credit.module which this patch addresses.

I still understand though, that this changes the API, but I argue that this function should not be a public API. It would be worth knowing if any contribs actually call this function.

Just my thoughts.

Travis.

longwave’s picture

The Sage Pay Token module I wrote (part of uc_protx_vsp_direct) uses it, at least:
http://drupalcode.org/viewvc/drupal/contributions/modules/uc_protx_vsp_d...

Allowing the function to return either a boolean or an array is ugly IMO, so I think a better fix is needed, and I don't think we should break compatibility with other modules that may use it - especially as this involves something as important as payment processing.

longwave’s picture

Its usage is also documented and explained at http://www.commerceguys.com/resources/articles/122 so I suspect others may use it too.

travist’s picture

Fair enough. I agree with you. This one may need to wait until an API change can be made. I will think about another way to do this in the mean time.

Thanks for your time.

Travis.

longwave’s picture

The approach I mentioned in #1 works, but is still ugly - but doesn't need an API change. Perhaps an option should be introduced to uc_credit such as "show detailed payment gateway error messages to the customer", which payment modules can check before overriding that global?

longwave’s picture

Committed my suggestion from #3 to change "card details" to just "details". Leaving as "needs work" because this could still be improved.

bcmiller0’s picture

Re-rolled patch for #13 to apply clean to 2.7 ubercart. Realize this isn't the direction things will go based on above comments, but to keep our build working from our drush make file. I've re-rolled #13 to be used in our build process, till we can spend the time to use the suggestions by longwave above to cleanup our approach.

bcmiller0’s picture

Re-rolled, as missed one line... Plan to cleanup later as discussed, but per our dev process this will allow us to upgrade to 2.7 and use our drush make build.

drupalfever’s picture

Here are my two cents.

I agree that the gateway error message should not be shown to the customer for security reasons.

However, an e-mail could be sent to the administrator, which would contain the gateway error message and the information about the customer having trouble with the transaction. Alternatively, an internal Drupal alert could be issued that only the Administrator would have access to. This alert would have the same information proposed for the e-mail.

I think that this gateway error message is important information that the main developer, or at list the store owner, should be informed about. There could be an error message being generated by the gateway that is unrelated to the information being provided by the customer. Keeping the developer in the dark is never a good thing.

I found this thread because I am having problems with the Authorize.net gateway. They upgraded their certificate to SHA-2 and the gateway stopped working. I had to jump through hoops to figure out what the problem was because I didn't have the benefit of seeing the error message that the gateway was generating.

Sorry! I have a tendency of being long-winded. I'm working on it.

geduozhao’s picture

Issue summary: View changes

Same Authorize.net gateway issue.

TR’s picture

Version: 6.x-2.x-dev » 7.x-3.x-dev
Category: Bug report » Feature request

@drupalfever: If you're going to work on this, please do so for 7.x-3.x. We're not adding new features to 6.x-2.x any more.