Support from Acquia helps fund testing for Drupal Acquia logo

Comments

j0rd’s picture

Alright. 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

IF success_code
THEN treat everything as a success and perhaps warn about CVV and AVS codes we dont understand in watchdog, so we can resolve them and improve the module
ELSE failed_code
THEN check to see why it failed including CVV and AVS

right now it works like this

IF success_code
THEN mark transaction as successs
IF FAILED CVV
THEN mark transaction as failed
IF FAILED AVS
THEN mark transaction as failed

this is poor logic and your clients are getting billed and ubercart is not showing successes.

j0rd’s picture

Here's another bug:

if (($this_trans_type == 'us_preauth' && $txnType == 'us_purchase') && intval($response_code) < 49 || $completed == 'TRUE') {

$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

<?xml version="1.0" standalone="yes"?><response><receipt><ReceiptId>csDDDDDDDDDD</ReceiptId><ReferenceNum>66DDDDDDDDD</ReferenceNum><ResponseCode>027</ResponseCode><ISO>01</ISO><AuthCode>08DDDD</AuthCode><TransTime>21:36:10</TransTime><TransDate>2009-11-03</TransDate><TransType>01</TransType><Complete>true</Complete><Message>APPROVED * =</Message><TransAmount>28.22</TransAmount><CardType>V</CardType><TransID>3D-D_DD</TransID><TimedOut>false</TimedOut><BankTotals>null</BankTotals><Ticket>null</Ticket><AvsResultCode>Y</AvsResultCode><CvdResultCode>1M</CvdResultCode></receipt></response>

I've replaced numbers with D to hide the information.

Here's one with AVS response W

<ReceiptId>cs1DDDDDDDDDD</ReceiptId><ReferenceNum>661DDDDDDDDDD</ReferenceNum><ResponseCode>027</ResponseCode><ISO>01</ISO><AuthCode>1DDDDZ</AuthCode><TransTime>10:09:35</TransTime><TransDate>2009-11-11</TransDate><TransType>01</TransType><Complete>true</Complete><Message>APPROVED * =</Message><TransAmount>45.15</TransAmount><CardType>M</CardType><TransID>DD-0_DD</TransID><TimedOut>false</TimedOut><BankTotals>null</BankTotals><Ticket>null</Ticket><AvsResultCode>W</AvsResultCode><CvdResultCode>1M</CvdResultCode></receipt></response>

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.

j0rd’s picture

FileSize
2.49 KB

Here'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.

j0rd’s picture

From 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.

18. Appendix D. Additional Information for AVS

The response that is received from AVS verification is intended to provide added security and fraud prevention, but the response itself will not affect the completion of a transaction. Upon receiving a response, the choice to proceed with a transaction is left entirely to the merchant.

Please note that all responses coming back from this verification method are not direct indicators of whether a merchant should complete any particular transaction. The responses should not be used as a strict guideline of which transaction will approve or decline.

NOTE

Please note that AVS verification is only applicable towards Visa, MasterCard, and Discover
transactions. This verification method is not applicable towards AmEx or any other card type.

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.

j0rd’s picture

I 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.

greggles’s picture

Status: Active » Needs review
FileSize
2.83 KB

This 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.

greg@gjkmbp : /opt/local/apache2/htdocs/d6/sites/all/modules/uc_moneris > patch -p0 < uc_moneris.diff
can't find file to patch at input line 4
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -urN uc_moneris_orig/uc_moneris.module uc_moneris/uc_moneris.module
|--- uc_moneris_orig/uc_moneris.module 2009-11-11 14:53:30.000000000 -0500
|+++ uc_moneris/uc_moneris.module 2009-11-11 14:52:48.000000000 -0500
--------------------------
File to patch: uc_moneris.module
patching file uc_moneris.module
Hunk #1 FAILED at 454.
Hunk #2 FAILED at 475.
Hunk #3 FAILED at 501.
Hunk #4 FAILED at 540.
4 out of 4 hunks FAILED -- saving rejects to file uc_moneris.module.rej

greg@gjkmbp : /opt/local/apache2/htdocs/d6/sites/all/modules/uc_moneris >

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.

greggles’s picture

I just realized that this will need to be re-rolled to take into account #525268: Malformed XML error on cart/review submit.

j0rd’s picture

My 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.

greggles’s picture

Someone needs to sit down and think this out.

Well, 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.

j0rd’s picture

I 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

dirksonii’s picture

I've applied the patch in #6, and it has not affected the problem whatsoever.

ezra-g’s picture

Status: Needs review » Fixed

I committed a revised version of this: http://drupal.org/cvs?commit=313324 .

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.