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.

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: Check for non-3DS response before attempting 3DS parse #3391

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

britth
Copy link
Contributor

@britth britth commented Oct 14, 2019

Based on the Redsys docs, it seems that when you attempt 3DS but
3DS is not available, you will receive a direct response to your
request. Currently, we look under specific namespaces related to
the 3DS action step when performing 3DS, which would cause an error
if the service returned a direct response. This PR updates the
parse method to first look for the path //RETORNOXML/OPERACION
and if present (and successful), tries to parse the response.
Otherwise, we proceed to the 3DS specific parsing, or error
message if there was an issue.

Unfortunately, there's no way to test this in the Redsys sandbox,
but existing functionality appears in tact with this change.

Remote:
22 tests, 70 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit:
36 tests, 112 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@britth britth requested a review from a team October 17, 2019 13:24
Copy link
Contributor

@bayprogrammer bayprogrammer left a comment

Choose a reason for hiding this comment

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

👍 Code looks good. Ran the tests locally. The redsys_sha356_test.rb and redsys_test.rb unit tests all pass for me. remote_redsys_sha256_test.rb passes for me as well, but remote_redsys_test.rb had some failures. Those fail for master as well, so they are not related to these updates to the RedsysGateway adapter.

@britth
Copy link
Contributor Author

britth commented Oct 21, 2019

@bayprogrammer thanks for the review! Yeah, the remote_redsys_test file tests non-sha256 requests, which are technically deprecated, so we probably want to just remove those once we officially get rid of support in AM

Based on the Redsys docs, it seems that when you attempt 3DS but
3DS is not available, you will receive a direct response to your
request. Currently, we look under specific namespaces related to
the 3DS action step when performing 3DS, which would cause an error
if the service returned a direct response. This PR updates the
parse method to first look for the path //RETORNOXML/OPERACION
and if present (and successful), tries to parse the response.
Otherwise, we proceed to the 3DS specific parsing, or error
message if there was an issue.

Unfortunately, there's no way to test this in the Redsys sandbox,
but existing functionality appears in tact with this change.

Remote:
22 tests, 70 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit:
36 tests, 112 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@britth britth merged commit b7ead62 into activemerchant:master Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants