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

Feature/pi protocol 4 #165

Closed
wants to merge 17 commits into from

Conversation

benjam-es
Copy link
Contributor

@benjam-es benjam-es commented Jul 9, 2021

This is a similar pull request dealing with #43 to replace pull request #164.

In addition, this is my working version for the newer 3d secure version.

Example request data (I have an initialiseData() method for this)

     $this->requestData = [
            'amount' => $this->transaction_amount * 100,
            'currency' => ‘GBP’,
            'returnUrl' => url('my-completion-url', null, true),
            'profile' => 'LOW',
            'apply3DSecure' => 'UseMSPSetting',
            'language' => config('app.locale', 'en'),
            'merchantSessionKey' => request()->input('merchant-session-key'),
            'cardIdentifier' => request()->input('card-identifier'),
            'tokenSave' => true,
            'card' => $card = new CreditCard([
                'billingFirstName' => $this->customer->forename,
                'billingLastName' => $this->customer->surname,
                'billingAddress1' => $this->billing->address1,
                'billingAddress2' => $this->billing->address2,
                'billingCity' => $this->billing->city,
                'billingPostcode' => $this->billing->postcode,
                'billingCountry' => $this->billing->country_code ?: 'GB',

                'shippingName' => $this->shipping->recipient_name,
                'shippingAddress1' => $this->shipping->address1,
                'shippingAddress2' => $this->shipping->address2,
                'shippingCity' => $this->shipping->city,
                'shippingPostcode' => $this->shipping->postcode,
                'shippingCountry' => $this->shipping->country_code ?: 'GB',
                'email' => $this->billing->email ?? $this->user->email,
            ]),
            'strongCustomerAuthentication' => [
                'website' => url('/', null, true),
                'notificationURL' => url('my-completion-url', null, true),
                'browserIP' => $userIp,
                'browserAcceptHeader' => $_SERVER['HTTP_ACCEPT'] ?? 'text/html, application/json',
                'browserJavascriptEnabled' => (bool) request()->input('browser.browserJavascriptEnabled', 0),
                'browserJavaEnabled' => (bool) request()->input('browser.browserJavaEnabled', 0),
                'browserLanguage' => substr(request()->input('browser.browserLanguage', 'en-GB'), 0, 5),
                'browserColorDepth' => (int) request()->input('browser.browserColorDepth', 24),
                'browserScreenHeight' => request()->input('browser.browserScreenHeight', null),
                'browserScreenWidth' => request()->input('browser.browserScreenWidth', null),
                'browserTZ' => request()->input('browser.browserTZ', 0),
                'browserUserAgent' => request()->input('browser.browserUserAgent', 'Unknown'),
                'challengeWindowSize' => 'Large',
                'transType' => 'GoodsAndServicePurchase',
            ],
            'credentialType' => [
                'cofUsage' => 'First',
                'initiatedType' => 'CIT',
            ],
            'transactionid' => $latest_transaction_id,
            'MD' => encrypt($latest_transaction_id),
            'description' => 'Order Description’,
        ];

Initialise the sage rest gateway

    private function initialiseSage()
    {
        $this->sage = new RestServerGateway;

        $this->sage->setVendor(config('sage.vendor'));
        $this->sage->setUsername(config('sage.api_username'));
        $this->sage->setPassword(config('sage.api_password'));
        $this->sage->setTestMode(config('sage.testmode'));
    }

Get a merchant session token for use within the frontend

    public function getMerchantSessionKey()
    {
        $this->initialiseSage();

        $response = $this->sage->createMerchantSessionKey()->send();

        if ($response->isSuccessful()) {
            return $response->getData();
        }
        return [];
    }

On a purchase end point (the sage pi code would use merchant session token, and user details to get a card token)

$response = $this->sage->purchase($this->requestData)->send();

...

if ($response->isSuccessful()) {
    // success (save order, trigger emails, redirect to success page)
} elseif ($response->isRedirect()) {
    // handle response data, save gateway reference


    // This will start a 3dsecure redirect
    return $response->redirect();
}

...

// Otherwise error

On another end point for 'completion' to recieve the return from 3dsecure

$postedData = request()->input();


// decrypt returned id for my transaction ID and find transaction by this ID

...

$post3dresponse = $this->sage->complete(['transactionId' => $this->transaction->gateway_payment_reference])->send();

if (!$post3dresponse->isSuccessful()) {
    // Mark as failed
}

// Find transaction
$response = $this->sage->getTransaction(['transactionId' => $this->transaction->gateway_payment_reference])->send();

if ($response->isSuccessful()) {
    // Save order & Success flow
} else {
    // Error process
}

Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
Signed-off-by: Ben James <[email protected]>
@JoshStaff
Copy link

Interested in integrating along side this for our implementation. We would rather commit to this repository & get it working here mind you.

Is there anything outstanding work wise on this PR? I'm happy to review it. Worth noting that I'll likely be tinkering with this locally while we develop.

More than happy to either bring #169 into this PR, or offer the same assistance for that PR.

Would be nice to get these pull requests moving as there is a lot of value in them. @barryvdh

src/RestServerGateway.php Outdated Show resolved Hide resolved
src/RestServerGateway.php Show resolved Hide resolved
@benjam-es
Copy link
Contributor Author

@JoshStaff I just hacked together something to get my implementation working. It would probably need reviewing in detail to ensure it is written well enough, and inline with the rest of the package.

Was intended as a starting point to help the maintainer along, but I never really heard back since July.

@benjam-es
Copy link
Contributor Author

@JoshStaff.
Part of the problem is that I wanted to originally incorporate a way to work with Sagepay PI, so I hacked that together, then I needed protocol 4 to also work with my PI code, so this pulled request is already covering two things, which isn't ideal.

In a perfect world, we would need a maintainer to merge one of the pull requests for protocol 4, then a separate pull request for Opayo PI, and then finally compatibility with PHP 8.x

@benjam-es
Copy link
Contributor Author

PHP 8 couple of tests broken

@barryvdh
Copy link
Member

barryvdh commented Feb 9, 2022

I'm not familiar with this gateway. What do you think @judgej ?

@judgej
Copy link
Member

judgej commented Feb 10, 2022

I needed to get SagePay Pi (now Opayo Pi) fixed on an existing project and ended up with this https://github.com/academe/opayo-pi I may be of some help. It's not Omnipay, but comes from an old hand-written PSR-7 package, before any OpenAPI descriptions were available. Starting from scratch today, I would use the OpenAPI Generator project to create the PSR-7 DTOs for all the messages, then wrap those up in an Omnipay package.

Now, the process for how this gateway works, is pushing the boundaries of Omnipay's normal processes, so it feels like putting a square peg into a round hole to create something that works, but is not an easy swap for other gateways. But maybe it's the round holes that need expanding and squaring a bit, so a kind of evolution?

I would personally not try to squeeze Opayo Pi into this Server/Direct/Form Omnipay package. There is really nothing that they share with Pi to make the effort worth while. I would start with a fresh package, and hopefuly there is something useful in the link above. A separate package would be so much simpler to manage IMO.

I'll help as much as I can, but really overloaded with project work at the moment (and very aware of the looming 3DS v2 technical debt deadlines). I'm happy testing/merging, but it's got to be in small steps so it's clear exactly what is being changed. I know it's too easy to end up putting lots of changes in one PR, then it's hard to pick it apart. But that's how it goes :-)

Not sure if that has helped at all?

@judgej
Copy link
Member

judgej commented Feb 10, 2022

I think what's missing is the documentation on how to use the Pi (aka "rest", though it's not really) API. There is lots of new requests and models in this PR. It needs at least a quickstart few paragraphs to try it out and see how it works.

So, if there are any details available on how to use the Pi API, I will give it a try.

@benjam-es
Copy link
Contributor Author

Part of the problem is that I hacked together something to work for PI, and then had to add in the changes for protocol v4.

For everyone else, there may be a simpler solution for v4 (and 3DS V2), without using the Payment Integration (PI) integration.

@benjam-es
Copy link
Contributor Author

benjam-es commented Feb 10, 2022

@judgej If you wanted to use #163 as a base, and use mine to fill any gaps you might have, I'll happily help get the package to a point where users will be able to carry on using it when 3DS v2 kicks in fully.

That would allow you to hopefully address the upcoming cutoff point, without the PI side muddying the waters

@adam-openplay
Copy link

@JoshStaff and I ended up forking this for our own purposes. Unfortunately it's probably not in a state where it can be merged upstream, but anyone is welcome to take a look at it and reuse anything that might be useful: https://github.com/openplayuk/omnipay-opayo

Changes include:

  • PHP8 support
  • @benjam-es' PI implementation, with a few additions and fixes to support deferred transactions and some issues we found around 3DSv2
  • Renaming SagePay to Opayo (the biggest change which introduces a BC break and makes the merge difficult, sorry about that!)

We've followed the omnipay interface as much as possible, and the parameters used for the requests simply map to what's in the Opayo docs https://developer-eu.elavon.com/docs/opayo/spec/api-reference-0. However, we haven't added specific documentation to this package as it has for the other implementations.

@judgej I can see the merit to breaking this out into its own omnipay package because, as you say, there's very little shared with the Server/Direct/Form implementations and in fact some of the bugs we found were due to relying on an overlap that didn't actually exist - even things like transaction status values are different.

Unfortunately, like you, we just don't have the capacity at the moment to maintain this and just had to get something working for our needs but, again, anyone else is welcome to run with it if they can make sense of it 😅

@benjam-es
Copy link
Contributor Author

@judgej i've updated the original comment at the top from the pull request with some idea of what would be used for a PI transaction

@benjam-es
Copy link
Contributor Author

@adam-openplay Any chance you can create a branch from your fork, and revert the name changes (just in a temporary throw away branch)... will make it easier to read the diff without so many name only changes

@adam-openplay
Copy link

@adam-openplay Any chance you can create a branch from your fork, and revert the name changes (just in a temporary throw away branch)... will make it easier to read the diff without so many name only changes

@benjam-es this is a very quick and dirty revert with conflicts resolved so no guarantee it will actually work but hopefully a cleaner diff https://github.com/openplayuk/omnipay-opayo/tree/temp-revert-opayo-rename

@benjam-es
Copy link
Contributor Author

All of the changes look good to me on first scan

benjam-es/omnipay-sagepay@feature/pi-protocol-4...openplayuk:temp-revert-opayo-rename

If anyone else who doesn't use Sagepay/Opayo PI could test out Adam's version, then that might be a good reference point for thephpleague to pull from.

@barryvdh
Copy link
Member

So is this a new feature? Let's stick to 4.x gateway support + PHP8 support first and tag that. Merged #173 for now.

@benjam-es
Copy link
Contributor Author

I'll close my pull requests, as there are better options for protocol 4 which all seem to address it well, and hopefully others will test. Hopefully that will close most/all protocol 4 and PI issues/pull requests.

Worth revisiting the PI option, maybe with Adam's code further in a separate pull request as you say above.

@benjam-es benjam-es closed this Feb 11, 2022
@benjam-es
Copy link
Contributor Author

@adam-openplay any chance you are able to merge in the changes from #173 into your version (including the temp branch)?

Would mean your package would track the up to date master here, and then I could potentially refer back to yours for an office "PI" pull request back here

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.

5 participants