thephpleague / omnipay-sagepay

Sage Pay driver for the Omnipay PHP payment processing library
MIT License
54 stars 78 forks source link

Incorrect Status for Omnipay Sage Pay Implementation #63

Closed danleecreates closed 8 years ago

danleecreates commented 8 years ago

After talking to our server provider when having issues with Sage Pay they seem to think there is an issue with Omnipay which will effect Sage Pay redirecting back to our website (built using Craft Commerce). Here is their message.

After further investigations, we find there is a coding issue in the omnipay implementation. The problematic code is

public function isRedirect() { return isset($this->data['Status']) && '3DAUTH' === $this->data['Status']; }

The Status response is incorrect.

Is this correct?

judgej commented 8 years ago

I think this is correct. The Sage Pay Direct docs say:

If the card and the card issuer are both part of the scheme, the Sage Pay gateway continues with 3D-Authentication by replying to your post with a Status of 3DAUTH.

The Status field is the raw response data returned from Sage Pay. 3DAUTH is the only status that should result in a redirect, so far as I know.

Are you getting a false isRedirect() when you are expecting it to be true?

I don't use Sage Pay Direct personally, due to the additional PCI requirements, but have an account I can test it on.

danleecreates commented 8 years ago

Yes - we are getting a redirect errors in Sage Pay, it wont send us back to the website after filling in payment information. Cancelling the payment halfway through also brings up an error. We have checked the cancel and redirect URLs, they are correct, Sage Pay IP Addresses are all white listed, ports are open - we were then pointed to Omnipay functionality.

Redirect Error: Server error 5003: Internal server error. HTTP error 500: The request was unsuccessful due to an unexpected condition encountered by the server.

Cancel Error: HTTP Status Code: 500 HTTP Status Message: The request was unsuccessful due to an unexpected condition encountered by the server. Error Code :
Error Description :

judgej commented 8 years ago

So it looks like isRedirect() is okay then - if Sage Pay is giving you the 3D Secure URL to go to, and there is a page when you get there, then it is expecting you, that is good.

With Sage Pay Direct (which it looks like you are using - correct me if I'm wrong) there are no back-channels, so open ports for incoming connections from the Sage Pay server are not important. This means all the URLs involved are URLs you will see in your browser.

When a user is sent to the issuing bank (using POST, for 3D Secure) they are sent with a TermURL - that is where they come back to (with some POST data identifying the result) after completing their 3D Secure password. What is that URL set to, and does the user get returned there?

judgej commented 8 years ago

It is important to note that a GET redirect to the test Sage Pay 3D Secure form will work. For live banks however, it must be a POST - some banks do not accept GET. A few people have falling into this trap, with everything working on the test instance, then failing in production.

danleecreates commented 8 years ago

I'm using Sage Pay Server

In the form we are using POST

<input type="hidden" name="action" value="commerce/payments/pay"/>
<input type="hidden" name="redirect" value="{{ siteUrl }}inkjet-flex/customer/order?number={number}"/>
<input type="hidden" name="cancelUrl" value="{{ siteUrl }}inkjet-flex/checkout/payment"/>
<input type="hidden" name="returnUrl" value="{{ siteUrl }}inkjet-flex/customer/order?number={number}">

The URLs seem to be working our logs...

195.170.169.29 - - [25/Jul/2016:13:43:27 +0100] "POST /shop/index.php/actions/commerce/payments/completePayment?commerceTransactionId=39&commerceTransactionHash=4ffbb58717e2da03bdd857305d47fc51 HTTP/1.1" 302 - "-" "SagePay-Notifier/1.0"
195.170.169.29 - - [25/Jul/2016:13:43:27 +0100] "GET /shop/inkjet-flex/checkout/payment HTTP/1.1" 200 33190 "-" "Java/1.8.0_45"

I'm not entirely sure what I'm meant to be checking.

judgej commented 8 years ago

Ah, okay. It was the 3DSECURE response that confused me, as that is only relevant to Sage Pay Direct. With Sage Pay Server the user to taken to the Sage Pay site to fill out a form there, and whether that includes a 3D Secure form there or not, does not have any impact on what gets returned to you. You simply don't get involved in 3D Secure at all, which is a bonus.

However, Sage Pay Server does use the back channel to send a notification.

That notification URL looks okay. The commerceTransactionId GET parameter has been added by your ecommerce software, and that it is used to look up the transaction in the database for logging the result to. It will probably be identical to $_POST['VendorTxCode'].

The commerceTransactionHash is also added by your ecommerce software, and presumably is used to check that commerceTransactionId isn't being faked by someone POSTing to you.

The data from SagePay will all be POST data - the result, some additional metadata, and a hash to also check the results have not been messed with en-route.

Here is an example of a demo handler:

https://github.com/academe/OmniPay-SagePay-Demo/blob/master/sagepay-confirm.php

Note that ALL the handler must return is right down at the last line. The response contains an "OK" status, an optional message that is logged, and the final URL you want to user to go to. Sage Pay is just expecting those three things back, all generated by the $responseMessage->confirm($finalUrl); and that's it.

Let me know if anything in there does not make sense. I've tried to comment it to explain what is going on.

Side Note: TBH the notification message should probably not be called completePurchase() - an acceptNotification message was introduced some time after this driver was written, and it would make much more sense to use that name. But I'll tackle that separately.

danleecreates commented 8 years ago

I appreciate the help. So are you saying my eccomerce software's handler is incorrect? Not sure what I need to be updating to solve this problem.

judgej commented 8 years ago

I don't know - can't say without looking at the code.

However, one big clue is the 302 return code from your handler URL - a redirect. It should not return a redirect. It should return a 200 with the text provided by OmniPay. Could there be some kind of issue with the routing? If it is the notify handler redirecting to "/shop/inkjet-flex/checkout/payment" then that may explain a few things - and looking at the IP addresses, it is Sage Pay being told to go to the user's final payment page - definitely not right.

The notify handler in your software just captures the POST information is has been given, then decides what URL the user needs to go to next. Then it returns the URL to Sage Pay, and it is Sage Pay that does the redirect, since the end user is right there on the Sage Pay site at this point waiting for your notification handler to decide what to do.

judgej commented 8 years ago

I know this is a lot of detail, but that's what debugging payment gateways is like :-) Would be nice to be able to treat all the software like a black box, but you do need to understand what is happening within that black box to diagnose what may be going wrong.

I'm sorry I didn't spot that 302 and those IP addresses earlier.

danleecreates commented 8 years ago

Ok I think it is fair to say I'm officially out of my depth here. Would you be open to looking at the code and rectifying the issue? We can pay for the service, what is the best place to talk this through?

judgej commented 8 years ago

I'm on jason@academe.co.uk

If you are using a third-party plugin to handle this, feel free to email that and I'll have a look through to see if there is anything obvious.

danleecreates commented 8 years ago

Thanks Jason, I have sent you an email. Looks like you are based round the corner from us. 👍

judgej commented 8 years ago

Fixed in PR #69 - this introduces acceptNotification which should be easier and more consistent with other gateways. It will be easier to implement in the Craft Commerce back-end.