Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Reported at http://drupal.org/node/488848#comment-2254242
Comment | File | Size | Author |
---|---|---|---|
#6 | 630064_uc_moneris_gateway_cleanups.patch | 2.83 KB | greggles |
#3 | uc_moneris.diff | 2.49 KB | j0rd |
Comments
Comment #1
j0rd CreditAttribution: j0rd commentedAlright. This patch is bad. This patch allows successfully processed transactions to get shown to the user and ubercart "AVS Failed", even though there transaction has been processed on their credit card.
Here's the use case:
Moneris returns with AVS 'W'. Which it has been on my account. AVS code 'W' is not in the $avs_access array, there for the AVS code marks the transaction as failed, even though the $response_code is less than 49 (a success).
The true way (which needs to superceed any other logic on success/failed) I believe needs to be the response_code. This tells us if we have charged the card or not. If we have charged the card, we need to allow ubercart to display a successful transaction. Otherwise the user will continue to process their card (sucessfully) but being displayed failed messages.
THIS IS BAD BAD BAD and does happen with this code.
Everything needs to be refactored to error on the side of caution.
It should work like this
right now it works like this
this is poor logic and your clients are getting billed and ubercart is not showing successes.
Comment #2
j0rd CreditAttribution: j0rd commentedHere's another bug:
$completed is equal to 'true'
$completed is never equal to 'TRUE'
Again, when you are dealing with credit card processor integrations you need to error in the side of caution and should do something like
str_tolower($completed) == 'true';
Unless for what ever reason PHP string comparisons are case insensitive, but I don't think they are.
Here's what responses look like
I've replaced numbers with D to hide the information.
Here's one with AVS response W
I've also noticed that successful transactions seem to have an AUTHCODE of non 0000000 . I assume this also means it was able to auth the card and charge it.
Comment #3
j0rd CreditAttribution: j0rd commentedHere's my temporary (quick fix) patch.
This modules always been a messy/buggy and I think it should be re-factored and re-written ... but of course I'm not going to do it :D
Thanks for everyone's hard work on the module.
Comment #4
j0rd CreditAttribution: j0rd commentedFrom the MONERIS documentation.
Essentially it says, AVS will be checked and the result returned, but these will not cause transactions to fail. So this is mostly for merchants to do AUTH only and then CAPTURE manually after checking AVS data.
Taken from the last couple pages of Moneris documentation I found from Google: https://esplusqa.moneris.com/connect/en/download/feb05/BATCH/eSELECTplus...
This means that if you've ever got a failed transaction via AVS and Moneris, the card was probably actually processed. I wonder if there's a setting to force moneris to fail hard on certain bad AVS data. Seems kind of stupid on their part to allow really bad AVS to go through, especially when you have to pay extra for this functionality, that doesn't really reduce fraud unless you are vigilant and increase your internal process.
Comment #5
j0rd CreditAttribution: j0rd commentedI was told by my client who has access to the moneris account that while these transactions are processed, they only come across as Pre-Auth's from Moneris and from what I understand no Captures are done.
This means the amount is reserved on the card, but it's never actually charged. It would then need to be manually processed on Moneris for it to bill the customer.
This is an odd way to do things from Moneris and is pretty weird actually from a processors standpoint. I've never seen another one do something like this.
This adds complexity to the Moneris integration. I will be thinking of a proper solution to this issue over the next couple days and hopefully figure something out. I may have to contact Moneris to see what they suggest.
Comment #6
gregglesThis patch doesn't apply directly. See http://drupal.org/patch/create for best practices on creating patches. I've manually applied it and re-rolled.
Ezra and I are supporting a client who uses this module, but are really just taking over maintenance as we can from codexmas (the original author). I agree it probably needs some re-thinking and rewriting (especially for better US/Canada support) but I'm not sure how to best pool resources toward that. If you can post bug reports and patches with explanations for the changes we're certainly happy to review and commit them. From my limited understanding of the gateway, this patch looks good to me.
Comment #7
gregglesI just realized that this will need to be re-rolled to take into account #525268: Malformed XML error on cart/review submit.
Comment #8
j0rd CreditAttribution: j0rd commentedMy module is probably fairly custom right now to get around all the bugs I had found in it previously. I vaguely remember updating.
As for my patch. I'm not sure if it should be committed. Someone needs to sit down and think this out. It appears that failed AVSs while they do "Auth" and reserve the amount on the clients card, do not actually capture the amount reserved. I believe this needs to be done manually.
I have not looked at my patch to factor in that fact yet, so I consider it a bad solution.
Auths can stick around for a couple weeks, so if you have a client who re-attempts a "Failed AVS, Yet Successful Auth" transaction, they could quickly use up the money on their cards. We for instance had a customer who auth'd 4 times before a success, totalling a couple hundred dollars in Auths. Ideally a failed AVS would be a failed transaction and no Auth should stick around. This might require a VOID to get done after a failed AVS, successful Auth transaction.
As with you, I have a client using this module, but I'm not officially supporting them. Some type of warning should be made to the users of this module until a proper solution to this can be created. Someone needs to contact moneris to find out what is actually going on with this workflow and how they recommend this get resolved. This is pretty irregular.
Comment #9
gregglesWell, nobody's going to do that so we have to get by on little patches here and there. That's kind of how open source works, after all.
Regarding a warning, what would it say? "THERE IS NO WARRANTY FOR THE PROGRAM"? That's in every download from d.o and is always the case.
Comment #10
j0rd CreditAttribution: j0rd commentedI agree.
Which is also why I said to revert my patch as I don't think it's best that it's applied to the module. It will allow successful transaction to go through, when the user is "Auth'd" but not "Capture'd". The default way is to allow a failed transaction to go through and "Auth" the user, but not "Capture". I believe this is preferred to my patch, but neither case are ideal. Someone will have to delve further to resolve this issue, but right now for me, it's not a priority in my ToDo list. It may become in a week or so, when I have some free time.
Another issue I just thought of with the Default scenario (Failed, with Auth) is that say your customer has $100 on their card and your product costs $40. They attempt to purchase and fail AVS twice. You've now reserved $80 on the card. Then the customer fixes their address and process again successfully...this transaction will still fail, due to "Lack of Funds", because they've only got $20 left on their card. At least this is how I assume it will work, unless moneris is smart enough to "borrow" one of the previous Auths, but I assume this wouldn't be the case. I had one customer who ran through they're transaction and failed 4 times, before resolving their AVS issue and getting a proper success. If they didn't have a large empty balance on their card, this sale would have been lost.
Hopefully a new user who needs Moneris, or an existing user who has time will resolve this issue properly. Small price to pay, for a module which works for 90% of use cases.
Thanks for all your hard work,
JOrdans
Comment #11
dirksonii CreditAttribution: dirksonii commentedI've applied the patch in #6, and it has not affected the problem whatsoever.
Comment #12
ezra-g CreditAttribution: ezra-g commentedI committed a revised version of this: http://drupal.org/cvs?commit=313324 .