Credorax: add 3DS2 MPI auth data support

[bayprogrammer]

Credorax: add 3DS2 MPI auth data support

Unit:
22 tests, 102 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
25 tests, 45 assertions, 10 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
60% passed

NOTE: Failing tests also failing in current master as of time of this run (and these same 10 were passing only a couple hours ago). The Credorax test infrastructure with respect to the test Visa credentials seems very unstable (and possibly depenedent on the time of day the transactions are attempted), see:

#3040 (comment)

Today, it seemed like they passed before 17:00 London time, and started failing with "Transaction has been declined." at or around 17:00 London time. Shuffling card numbers is futile, just leaving these failures in place since there is no predictable and reliable way to ensure they always pass.

ECS-382

Closes #3274

[molbrown]

Assuming I know very little about 3DS2, three_ds_version is only included in 3DS2 requests? If so, why does this need to be a parameter, or can it be defined based on the presence of three_ds_2? Or included inside of three_ds_2?

[bayprogrammer]

Those are great questions! I initially was thinking of embedding the version directly in the hash, rather than leaving it outside. There are two main reasons for the approach I settled on and propose here:

First, as to why include it at all, is that there are different sub-versions of 3DS 2.x, and while only 1.0 or 2.0 are currently valid version strings for Credorax, and only needs to be passed in for 2.0 requests, that could change in the future. I would like to avoid hard-coding the version string, even if I could get away with doing so currently, to make as few assumptions on use and future evolution of the Credorax API as possible.

Conceptually, the specific version of 3DS (even minor revisions within 3DS 2.x) can affect how a gateway wishes to receive the information. By requiring the version to be passed by the user, we can leave the rest of the ActiveMerchant API consistent (letting the version number, where applicable, affect how we interpret and include 3DS information in the outgoing request).

Secondly, while admittedly we aren't (until here with this PR) explicitly using the three_ds_version option in ActiveMerchant for any other gateway adapter, from our internal Core application we are passing it this way for Adyen and Barclay already. In terms of general API design, my thinking is that--since the version could be useful for any version of 3DS, not only version 2.0--it makes sense to go ahead and begin to explicitly adopt and make use of this top-level option we've been including already.

See Adyen and Barclaycard Smartpay for examples which are similarly structured with respect to the three_ds_2 hash (and the corresponding generic Gateway code in Core for how we send the three_ds_version option currently, even if otherwise unused hitherto).

As a side-note, while we don't have any formally established 3DS2 field standardization within ActiveMerchant project-wide as yet (and we'd need to coordinate with Shopify to accomplish such), I am hoping this and future gateway adapters we contribute 3DS2 related functionality to will provide a clear and consistent pattern that can eventually lead to such conventions being widely used and adopted throughout ActiveMerchant.

Definetely open to changing this design if it doesn't make sense though. Do you think this design could lead to unexpected confusion or difficulty obtaining wider acceptance?

[molbrown]

Having the perspective for making space for versions 2.x and beyond really helps me to get behind this. I think it totally makes sense. I got caught up in not seeing three_ds_version applied for the 1.0 case. Does Credorax default to 1.0 if no version is sent, but the other desired fields are? Would there be cases when both of these if statements are true? Again, knows little about 3DS2/ Credorax.

[bayprogrammer]

Yes, Credorax assumes 1.0 unless you opt into 2.0 by way of passing extra values in 3ds_dstxnid and 3ds_version. I'm not certain what they would actually do if someone was inconsistent (passing 3ds_version as 1.0 but then inserting the parameters formatted as required for a 2.0 request), but the documentation is clear on this point (emphasis mine):

When authentication is done using 3-DSecure 2.0:

1. The XID sub-field is not required as part of the i8 paramater. Instead send “none”.
2. In addition, send the following parameters as part of the request:

• Parameter name: 3ds_version
  • Description: Indicates whether the 3D Secure protocol version is 1.0 or 2.0
  • Format: [0-9]
  • Min,Max: 3,3
• Parameter name: 3ds_dstrxid
  • Description: 3DS Directory server transaction ID. Must be sent if 3ds_version = 2.0 and i8 is used.
  • Format: [0-9A-Za-z,-]
  • Min,Max: 34

[bayprogrammer]

@filipebarcos Gave me some excellent feedback via Slack. Here's his thoughts (quoted with permission):

Something we have in house already and many gateways are using in AM is the pattern:

 three_d_secure: {
   three_d_secure_version: '2.1.0',
   eci: '01',
   cavv: 'a-cavv',
   ds_transaction_id: 'a-ds-trans-id',
   xid: 'a-xid',
 }
}

I’d argue that authentication_value is a better name than cavv but I don’t mind keeping the way we have, as cavv. Some examples of gateways using those patterns:

• https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/payflow.rb#L227-L238
• https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/authorize_net.rb#L571-L576
• https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/quickpay/quickpay_v10.rb#L221-L225
• https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/worldpay.rb#L303-L310
• https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/braintree_blue.rb#L632-L638

This is great to learn about. I would prefer to remain consistent with those examples (including adopting cavv for the authorization value field option), since we do have a fairly well established pattern emerging in ActiveMerchant already for passing these 3DS authentication values (including how to pass in the specific 3DS version). It will not be difficult for me to update this PR to match those patterns.

We also discussed drafting a Wiki article documenting these patterns and linking to them from the README. I will take a first stab at drafting such notes.

[jknipp]

+1 for consistency

[bayprogrammer]

Looks like the field name we're using inside the :three_d_secure sub-options hash for the 3DS version is :version, not :three_d_secure_version .

See https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/gateways/worldpay.rb#L305

Going to match the WorldPay example and use :version (which makes the most sense to me anyway since it's nested inside the 3ds hash already giving it a scope).

[bayprogrammer]

I've updated this PR to adopt the 3DS options conventions used in the other gateways per #3274 (comment)

Ready for re-review.

[bayprogrammer]

Addendum, as suspected, Credorax Visa-related tests pass in the morning. 🍓

Results as of ~10:00 EDT:

Unit:
22 tests, 102 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
25 tests, 72 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

[molbrown]

This looks good to me 👍