thephpleague / omnipay

A framework agnostic, multi-gateway payment processing library for PHP 5.6+
http://omnipay.thephpleague.com/
MIT License
5.92k stars 928 forks source link

Better Bank Support #245

Open johnnye opened 9 years ago

johnnye commented 9 years ago

At the moment there are 3 tickets open that relate to bank based payment systems:

I'm treating "bank based" as Electronic Funds Transfers so directDebit/sepa, ACH, SWIFT (if anyone supports that), etc.

I've only been digging into the internals of Omnipay over the past two weeks so take my opinions as you wish.

It seems to me though that Omnipay is very much a card based payments interface, and therefore there are somethings that are hard or inappropriate to do with the current interface. I really like the suggestion in #206 about a BankAccount object, but I wonder if that goes far enough to fully incorporate bank based payments. One of the current issues is that Card based payments are very common, only a subset of companies need to handle bank based payments.

Rather the dirtying the methods available through the current Gateway - would it be a better idea to create a BankGateway (AbstractBankGateway) interface. This interface deals with a BankAccount object - in the same way that the current Gateway expects a CreditCard object.

This BankGateway should be able to collect and distribute payments if needed. However it should be able to combine the existing Gateway with the BankGateway to get an end to end payments solution using Omnipay. Think about Stripe Marketplace API (https://stripe.com/us/marketplaces) - I would use the current Gateway to collect payments, but the BankGateway to interact with their TransfersAPI (https://stripe.com/docs/tutorials/sending-transfers). This object also shouldn't be tied to one national or economic blocks banking system(EU/US).

What I'm suggesting might not be compatible with the aims of Omnipay, and likely isn't sanely thought through yet, but in my current frame of mind seems fairly sensible.

greydnls commented 9 years ago

If I understand what you're suggesting correctly, you're talking about creating separate ACH gateways.

For instance, for Authorize.Net that package would provide the current AuthorizeNet_AIM and AuthorizeNet_SIM gateways, as well as an AuthoirzeNet_ACH gateway? Is that correct?

Just want to make sure I'm understanding where you're going with this fully.

aderuwe commented 9 years ago

+1

Haven't been involved for a long time (new job with no need for omnipay at this moment, and no time to keep involved otherwise) but I really missed this functionality when I was.

(Not commenting on the technical details here, just the concept.)

johnnye commented 9 years ago

@kayladnls not Exactly - though that might be an option. My suggestion is that there is a new base class for ACH gateways.

So, AuthorizeNet_ACH would implement BankGateway.

BankGateway would have methods for payouts, creating bank objects, collecting money from bank accounts etc.

It allows the current gateway to remain clean and useful for the majority of users. It also allows those of us that need to interact with banks a place to implement specific functionality that these type of bank based systems need.

johnnye commented 9 years ago

I tried to flesh this out a bit more, and ended up in a tangle about if this was a good idea or not. This Gist is what I ended up with: https://gist.github.com/johnnye/e4858e311c1cdd15b606

Current Issues with the Omnipay interface:

  1. It is tied to cards as a funding source quite closely.
  2. All actions don't have a complete set of opposite actions. As a payments API we want it to enable us to take money, but for some users they need the ability to distribute money too.

Solutions:

  1. Create a generic type that Cards can extend, this could then be extended for BankAccounts and Wallets (think about a payment provider that holds a clients money on your behalf) or other funding sources.
  2. The current Omnipay interface works really well for taking customers money with cards. So these methods extend well to the idea of taking money from a bank account, in this case it makes sense to argue for common gateway no matter the type. However, there isn't any support for payouts, or deposits at the moment in Omnipay. There is an argument IMHO for splitting this out into it's own area, maybe BankGateway is too specific. It is more common to distribute money to bank accounts rather then credit cards.

If there is a method for creating a deposit or payout, then there should also be a method for cancelling it. This could add a lot of weight to the current API if it isn't split out. There are cases where provider APIs use different endpoints for cards and banks, so it could become difficult for gateway developers to integrate the two payment methods without performing lots of convoluted checking.

I can see the benefits of splitting, and of keeping it the same, in the long run I think splitting might give an advantage for adopting more complex payment gateways.

greydnls commented 9 years ago

One of the things I want to do with V3 is to decouple the current CreditCard object into two separate objects: Customer and CreditCard. Currently, they're very intertwined. I could see creating a number of different payment types that can work with the Customer object and be passed into Gateways for use similar to what you outlined in your Gist.

I think I would much rather have the gateway be separate, rather than using the getBankGateway() function that you outlined.

Example:


$gateway = Omnipay::create('stripe');
$gateway->setApiKey('11111111');
$gateway->purchase();

$bankGateway = Omnipay::create('stripe_bank'):
$bankGateway->purchase();

It would seem to me that if you were offering your customers the ability to pay via credit card or bank transfer, it would be beneficial to keep the API for both gateways as close as possible. But I'd love your feedback on that. Does that not mesh well with your use case?

As for the functions you outlined in the BankGateway, I'm not sold on the idea of createAccount() and deleteAccount(). I don't know if that's something that necessarily should be covered by a payment gateway library.

johnnye commented 9 years ago

Separating the customer and their payment method would be an excellent move I think, it also reflects how most of the payment providers and code is structured.

Keeping both gateway interfaces as similar as possible seems a sensible choice too, purchase() is a good example of this. How would this work with payments to users or some other bank based calls, or would they have to be replicated in both and just not used?

I misnamed createAccount() in my gist, it might be better named validateAccount(), it was the end of my work day when I wrote it. There needs to be a place in the gateway to validate bank account details. At the moment there is basic card validation in the CreditCard class which does some sanity checking on the dates and uses the Luhn checking for the number. Unfortunately validating bank details is going to be far more complicated. There are some simple sanity checks that can take place, like this for IBAN. Checking validity of a UK bank account is much more complicated (PDF), and ACH check sum. In this light, it seems that some APIs also provide a way to validate a bank account number (like so in GoCardless Pro, the stripe transfer API seems to do this for you server side, but only supports the US at the moment - else where is closed beta).

As for deleteAccount(), how would / should an Omnipay user deal with the following scenario?

without a way to update or delete accounts in Ominpay, I would have to write those calls to the payment API myself. I think this should be available to all payment methods - the reason being is it's easy to keep a payment method on file now that lots of payment providers are acting like card vaults.

greydnls commented 9 years ago

I may be understanding what you're saying about deleteAccount(). When you say "my system" are you talking about the app you've written to work with Omnipay (marketplace, subscription service or similar)?

I think you are, and if that's the case, why would removing a credit card from your system require a call to a payment API? Are you talking about the ability to remove it from the "vault" within the payment gateway? I know that Braintree does something similar with their Vault and I could see a use case there.

Or, perhaps I've misunderstood entirely?

johnnye commented 9 years ago

Absolutely correct.

"my system" - is an imaginary web system. The scenario still holds for keeping cards on file if you're a web store and wanted to implement a non-patented low volume of clicks purchase system that is nothing like amazons patented One Click. Nothing like it.

Stripe methods for this - https://stripe.com/docs/api#delete_card Braintree - https://developers.braintreepayments.com/javascript+php/reference/request/payment-method/delete

greydnls commented 9 years ago

It sounds like it's something like this: https://github.com/thephpleague/omnipay-paypal/blob/master/src/RestGateway.php#L469-L490

If so, I could definitely see creating such a feature.

johnnye commented 9 years ago

Exactly what I'd like to use.

As an aside life would be easier to just use stripe, unified payment types just introduced. https://stripe.com/blog/unifying-payment-types-in-the-api

AlexM4H commented 7 years ago

Any further updates or development for a BankAccount or Customer Object in the actual OmniPay library? I am interested in a solution for SEPA bank wire transfer via EBICS protocol.

delatbabel commented 7 years ago

At the moment I don't believe that any of the core omnipay team are working on this, although a PR for version 3.0 would be looked at.