Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redsys: Add exemptions #3354

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Redsys: Add exemptions #3354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* Stripe: Add supported countries [therufs] #3358
* Stripe Payment Intents: Add supported countries [therufs] #3359
* Mundipagg: Append error messages to the message response field [jasonxp] #3353
* Redsys: Add ability to pass sca_exemption and moto fields to request exemptions [britth] #3354

== Version 1.98.0 (Sep 9, 2019)
* Stripe Payment Intents: Add new gateway [britth] #3290
Expand Down
9 changes: 9 additions & 0 deletions lib/active_merchant/billing/gateways/redsys.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def purchase(money, payment, options = {})
add_threeds(data, options) if options[:execute_threed]
data[:description] = options[:description]
data[:store_in_vault] = options[:store]
data[:sca_exemption] = options[:sca_exemption]

commit data, options
end
Expand All @@ -215,6 +216,7 @@ def authorize(money, payment, options = {})
add_threeds(data, options) if options[:execute_threed]
data[:description] = options[:description]
data[:store_in_vault] = options[:store]
data[:sca_exemption] = options[:sca_exemption]

commit data, options
end
Expand Down Expand Up @@ -428,6 +430,7 @@ def build_merchant_data(xml, data, options = {})
xml.DS_MERCHANT_TERMINAL options[:terminal] || @options[:terminal]
xml.DS_MERCHANT_MERCHANTCODE @options[:login]
xml.DS_MERCHANT_MERCHANTSIGNATURE build_signature(data) unless sha256_authentication?
xml.DS_MERCHANT_EXCEP_SCA data[:sca_exemption] if data[:sca_exemption]

# Only when card is present
if data[:card]
Expand All @@ -441,6 +444,12 @@ def build_merchant_data(xml, data, options = {})
xml.DS_MERCHANT_DIRECTPAYMENT 'true'
end

# Set moto flag only if explicitly requested via moto field
# Requires account configuration to be able to use
if options.dig(:moto) && options.dig(:metadata, :manual_entry)
Copy link
Contributor Author

@britth britth Sep 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a moto field in addition to the standard metadata[:manual_entry] since requests will fail with the error SIS0488: Merchant has not configured the payment method "MOTO/Manual payment" if you pass this field without your account being configured. Wondering if I should just remove the check for options.dig(:metadata, :manual_entry) in this case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where exactly is this :metadata, :manual_entry coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per here, that's what I believe shopify has used for determining moto transactions, and was suggested as the standardized field for marking moto. I don't want to use just that field though, since if the account doesn't support moto but this field is passed in, it'll result in a possibly unintended error (but perhaps that means I should just use the separate field altogether?)

Copy link
Contributor

@curiousepic curiousepic Sep 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused; it doesn't seem like the moto field would help much in ensuring that's the case. Are you picturing it being an extra verification that the account is flagged to support moto? If so, I don't feel like that's necessary. We (Spreedly) could map our own separate flag to the manual_entry flag, if that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was, even though we're standardizing on the manual_entry flag, since it's already being used by Shopify I don't want to unintentionally request this exemption for someone if their account isn't configured.

So basically trying to prevent a case where you're marking your payment method as manual_entry as you normally would but you're not actually intending to request an exemption, and the transaction fails because your account isn't set up - does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, I didn't quite understand its interaction with the exemption piece. Thanks for the clarification. I don't have a strong opinion about whether to keep the second condition, I think you're right that it should work as intended with only the first.

xml.DS_MERCHANT_DIRECTPAYMENT 'moto'
end

if data[:threeds]
xml.DS_MERCHANT_EMV3DS data[:threeds].to_json
end
Expand Down
15 changes: 15 additions & 0 deletions test/remote/gateways/remote_redsys_sha256_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ def test_successful_purchase_3ds
assert_equal 'CardConfiguration', response.message
end

# Requires account configuration to allow setting moto flag
def test_purchase_with_moto_flag
response = @gateway.purchase(100, @credit_card, @options.merge(moto: true, metadata: { manual_entry: true }))
assert_equal 'SIS0488 ERROR', response.message
end

def test_successful_3ds_authorize_with_exemption
options = @options.merge(execute_threed: true, terminal: 12)
response = @gateway.authorize(100, @credit_card, options.merge(sca_exemption: 'LWV'))
assert_success response
assert response.params['ds_emv3ds']
assert_equal 'NO_3DS_v2', JSON.parse(response.params['ds_emv3ds'])['protocolVersion']
assert_equal 'CardConfiguration', response.message
end

def test_purchase_with_invalid_order_id
response = @gateway.purchase(100, @credit_card, order_id: "a%4#{generate_order_id}")
assert_success response
Expand Down
19 changes: 18 additions & 1 deletion test/unit/gateways/redsys_sha256_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,28 @@ def test_successful_authorize_with_3ds

def test_3ds_data_passed
stub_comms(@gateway, :ssl_request) do
@gateway.authorize(100, credit_card, { execute_threed: true, order_id: '156201452719', terminal: 12 })
@gateway.authorize(100, credit_card, { execute_threed: true, order_id: '156201452719', terminal: 12, sca_exemption: 'LWV' })
end.check_request do |method, endpoint, data, headers|
assert_match(/iniciaPeticion/, data)
assert_match(/<DS_MERCHANT_TERMINAL>12<\/DS_MERCHANT_TERMINAL>/, data)
assert_match(/\"threeDSInfo\":\"CardData\"/, data)
assert_match(/<DS_MERCHANT_EXCEP_SCA>LWV<\/DS_MERCHANT_EXCEP_SCA>/, data)
end.respond_with(successful_authorize_with_3ds_response)
end

def test_moto_flag_passed
stub_comms(@gateway, :ssl_request) do
@gateway.authorize(100, credit_card, { order_id: '156201452719', moto: true, metadata: { manual_entry: true } })
end.check_request do |method, endpoint, data, headers|
assert_match(/DS_MERCHANT_DIRECTPAYMENT%3Emoto%3C%2FDS_MERCHANT_DIRECTPAYMENT/, data)
end.respond_with(successful_authorize_with_3ds_response)
end

def test_moto_flag_not_passed_if_not_explicitly_requested
stub_comms(@gateway, :ssl_request) do
@gateway.authorize(100, credit_card, { order_id: '156201452719', metadata: { manual_entry: true } })
end.check_request do |method, endpoint, data, headers|
refute_match(/DS_MERCHANT_DIRECTPAYMENT%3Emoto%3C%2FDS_MERCHANT_DIRECTPAYMENT/, data)
end.respond_with(successful_authorize_with_3ds_response)
end

Expand Down