Registrar: Credit/debit card payment solution

[vohmar]

Foreign registrars have only bank transaction option and TransferWise if they know and use it for payments to their credit accounts in .ee registry. To make this process simpler and more flexible.

[vohmar]

For the initial intermediary we have chosen EveryPay.
Access to test account is created to Artur and Martin
API doc: https://every-pay.com/et/dokumentatsioon-2/

[vohmar]

Access to test account created to Maciej

[maciej-szlosarczyk]

I spent some time investigating how EveryPay works and there are few questions that we should answer before moving forward, mostly related to compatibility with banklink and the ability to use different intermediaries in the future (i.e Stripe), especially since it's an open source project.

Given that we have refactoring for banklink planned in issue #646, I think we should plan for the following URL schema and controller inheritance to follow the 7-action Rails convention:

## Payments creation and return from intermediary:
/registrar/invoices/:invoice_id/payments/:payment_channel/ 
A complete controller dedicated to a single intermediary payment creation and return.

## Callbacks handling
/registrar/invoices/:invoice_id/payment_callbacks/:payment_channel/ 
A complete controller for callbacks handling from one single intermediary.

In case of EveryPay, we would use:

GET /registrar/invoices/1233/payments/every_pay/new 
To display info and redirect the user to CC format page.
PUT /registrar/invoices/1233/payments/every_pay/4687ee51e9e1cca682f4b81e961d28
To redirect the customer back to our system, and display relevant info about 
the payment. The last part of the URL is a nonce we set ourselves.
POST /registrar/invoices/1233/payment_callbacks/every_pay/ 
For callbacks, as they send them as POST requests

Similarly, Swedbank, would use:

GET /registrar/invoices/1233/payments/swedbank/new - for the payment redirect
POST /registrar/invoices/1233/payment_callbacks/swedbank/ - for the payment return

I haven't yet figured out how to separate them properly, but I'm thinking we might put all payment gateways that we use here into it's own separate mountable engine with its own git repo. It will be EIS specific, other people can write their own that covers stripe/paypal/etc.

We however need to create a clear interface on handling the payments, which we currently do in a method on BankLink::Request.

---

We also need to decide if and how we want to store info about payments. Most payment providers require some kind of unique identifier to be set on the merchant side to isolate each transaction. It cannot be the invoice reference number, as there might be two payments attached to the same invoice, with one being failed or not completed. We can:

• Create a new table (i.e payments) with a foreign key on invoices and create a new record there every time someone wants to pay an invoice. It should contain only the data that is relevant to us (created_at, updated_at, status, payment_channel/intermediary)
• Don't create anything, and rely on BankTransactions <--> Invoice binding to provide this information. However, this does not give us the information about payments that were not successful.

[vohmar]

Proposal sounds and looks good. Lets proceed with the trackable solution with extra table.

[artur-intech]

Create a new table (i.e payments) with a foreign key on invoices and create a new record there every time someone wants to pay an invoice.

Sounds good to me. Few other things to consider:

• Partial payments (currently it is not possible as far as I remember)
• There should be some way to add payments manually. It actually exists, but as I mentioned in Incorrect business process #636 (comment)

Creating new bank transaction is weird, since we are not bank

So it should be possible to create new payment in UI, not BankTransaction (which probably might be used only for storing transactions associated with imported bank statements. Another questions is where do we store another payment gateways' transactions. I don't think they are BankTransaction-s.

btw, AccountActivity model seems to act as a collection of payments atm, so the complete billing model should be probably changed according to the needs introduced with this (#419) ticket. Also check #447.

---

• URLs themselves look quite readable and self-explanatory, but I wonder if it's possible to reduce nesting level
• Perhaps instead of payment_callbacks we could use something some more meaningful terms, say payments/confirm?
• Also wondering if we could avoid creating new engines, and to limit ourselves to just pure gem

[maciej-szlosarczyk]

Partial payments introduce complexity we probably don't want. I'd avoid them at all cost, otherwise we need to figure out what to do with invoices that are partially paid, which is pretty big deal accounting-wise. Plus, as a customer you are not allowed to change the amount in the credit card payment. If you do that with Javascript, HMAC will be broken and will not be authorized in EveryPay.

I am against payments/confirm as you are not confirming all payments. Some of them are denied, cancelled, etc. Plus, it mixes REST with RPC, which is not a good idea IMHO.

[artur-intech]

invoices that are partially paid, which is pretty big deal accounting-wise.

I don't think there is some accounting software that wouldn't support partial payments or an accountant who would ignore them, but that's up to @vohmar.

"confirm" was just an example. It's up to you how exactly it should be called.

[vohmar]

we do not support partial payments in the registry software at the moment nor do I see any reason to implement it. We have and will continue to handle these rare cases manually usually by paying back the partial payment and asking registrar by making new payment for the correct sum.

[artur-intech]

Perhaps other registries may benefit from this feature?

[vohmar]

I agree, but at this moment it seems to introduce too many complexities. Also as registrars are in control of the billing - creating, paying and canceling the invoices as necessary it is hard to see what is the added value here.

[vohmar]

web UI's do not work after cofiguring the EveryPay - passenger will not start

error message:

[ E 2018-04-26 11:31:51.0187 4709/Tw age/Cor/App/Implementation.cpp:305 ]: Could not spawn process for application /home/registrar/registrar/current: An error occurred while starting up the preloader.
  Error ID: 398791ec
  Error details saved to: /tmp/passenger-error-QaoJgK.html
  Message from application: undefined method strip' for nil:NilClass (NoMethodError)
  /home/registrar/registrar/releases/433/app/models/payments.rb:2:in<module:Payments>'
  /home/registrar/registrar/releases/433/app/models/payments.rb:1:in `<top (required)>'

[maciej-szlosarczyk]

Please add the following to config/application.yml:

# You should list other payment intermediaries here. Each one of them needs their own class in /app/models/payments/
payments_intermediaries: >
  every_pay

It's in the branch's example file as well, I assumed that those are added automatically:
https://github.com/internetee/registry/pull/829/files#diff-318c31f2f8c28eecdbe0119432a98af0

[vohmar]

• logged in with mobile id, made the payment and automatic redirect back to merchant took me back to login screen - the user session was ended at some point. Same problem with credit card and bank link payments

[artur-intech]

@maciej-szlosarczyk

I don't think "payment" is a good naming here.

A payment is an amount of money that is paid to someone, or the act of paying this money.

Wouldn't it be better to call it "Payment gateway" instead? If I understand correctly, the entity in question is a payment facility, not amount of money paid, so Invoice has many Payments (which have nothing to do with payment facility)

• https://en.wikipedia.org/wiki/Payment_gateway

[maciej-szlosarczyk]

@artur-beljajev Indeed, Payment might not be the best name, but I don't think that payment gateway fits here either. Payment gateways are external to us, so we would end up with creating multiple instances of PaymentGateway to send the order to a real gateway such as Stripe or PayPal. It feels rather artificial to me.

I think PaymentOrder might fit nicely (https://en.wikipedia.org/wiki/Payment_order):

Payment order is an international banking term that refers to a directive to a bank or other financial institution from a bank account holder instructing the bank to make a payment or series of payments to a third party.

This is essentially what we do here, we ask the customer to authorize a payment order to pay a specific invoice and then given the state of that payment order, make changes in our system. Plus, it's quite natural for an invoice to have multiple payment orders, differentiated by channels/banks.

[artur-intech]

Payment order ("maksekorraldus" in Estonian) is internal to a bank, so we cannot and should not create it in terms of design. Bank may or may not use it, it's not our business.

To my understanding, what we do here, is that we allow a customer to pay through some facility and how that payment is done should be hidden behind some abstraction, and "Payment gateway" seemed like a good name for that abstraction.

https://github.com/search?l=Ruby&q=payment_gateway&type=Code

P.S. Looking at examples of those orders, I also don't see them connected to our case:

• https://www.google.ee/search?q=payment+order&source=lnms&tbm=isch&sa=X&ved=0ahUKEwjP_Ie03dzaAhULiaYKHaxdDCEQ_AUICigB&biw=2048&bih=1022
• https://e-justice.europa.eu/content_european_payment_order_forms-156-en.do#action

Other info:

• https://en.wikipedia.org/wiki/Payment_processor
• https://blog.bluepay.com/payment-gateway-vs-payment-processor

Not sure what we deal with a payment processor here as well.

[maciej-szlosarczyk]

Payment order ("maksekorraldus" in Estonian) is internal to a bank, so we cannot and should not create it in terms of design. Bank may or may not use it, it's not our business.

That's not true, please read BankLink documentation. Usage of service 1011 is exactly that: creating a payment order that customer cannot change, and sending it over the wire to the bank. They need to be understood by the bank, but that doesn't mean that they are internal.

https://www.seb.ee/sites/default/files/web/files/pangalingi_tehniline_specENG.pdf

The same goes for EveryPay, the customer can only authorize or cancel the pre-filled payment order.

[maciej-szlosarczyk]

I'll change it PaymentOrder, since Payment is ambiguous in this context. PaymentGateway is not a good name for this abstraction because it gravitates towards a singleton or otherwise it does not model real world. There's only one PayPal or EveryPay, not many. Let me use an example to illustrate that:

Imagine that you have a company that sells physical goods and needs to ship them to customers using different transport companies (Postal, DHL, UPS, etc.). You have an Order object that represents items to be sent to a customer. Now, you have a module called ShippingCarriers that represents person or entity that transports the goods from you to the customer. Each ShippingCarrier knows how to prepare a manifest, deliver a package and cancel a delivery. That's roughly analogous to using PaymentGateway as an abstraction to process payments.

A typical interface for that would look close to the following:

order = Order.find_by(params)
shipping_carrier = ShippingCarriers::UPS.new(order)
shipping_carrier.prepare_shipping_manifest
shipping_carrier.deliver_package
shipping_carrier.complete_delivery
shipping_carrier.cancel_delivery

Looks good, but if we were to model real world, it should be replaced with a singleton since there's only one UPS and one Omniva, not many. Now we have a bunch of non-OO procedures:

order = Order.find_by(params)
ShippingCarriers::UPS.prepare_shipping_manifest(order)
ShippingCarriers::UPS.deliver_package(order)
ShippingCarriers::UPS.complete_delivery(order)
ShippingCarriers::UPS.cancel_delivery(order)

IMHO a better choice is to have an object called Shipment that takes an Order and carrier arguments:

order = Order.find_by(params)
# A carrier can also be an injectable object, depending on how complicated the logic is.
shipment = Shipment.new(order, :omniva) 
shipment.prepare_manifest
shipment.deliver
shipment.mark_as_delivered
shipment.cancel

[maciej-szlosarczyk]

Hey @vohmar,

logged in with mobile id, made the payment and automatic redirect back to merchant took me back to login screen - the user session was ended at some point. Same problem with credit card and bank link payments

I'm pretty certain that's how Estonian ID should work (smart/mobile/ID card), otherwise it would be unsafe. When you exit the page to go to pay, you end the session. You need to authenticate again to resume. You can check it at your internet bank: login, close the page, try to reopen the page. You'll see a cache missing message and be redirected back to the login screen.

[vohmar]

I cannot name any services using bank link or credit card payments that require user to login again after the payment is processed. So purely from user experience point of view we should try to retain the session during the credit card and bank link payments.

That said the last update that keeps the action in one tab still arrives to the same result of ending the session and requiring to log in again. Tested with both mobile-id and id-card logins.

[vohmar]

users logged in using pki auth are logged out as well on back to merchant action - same as with mobile-id and id-card users

[artur-intech]

It's worth trying to set same_site_session_cookies to false in application.yml and clearing all cookies.

[maciej-szlosarczyk]

As @artur-beljajev suggested, we need to change staging values as they currently don't allow for post requests to retain session in non-GET requests. They need to be update to the same values as in production, where it works properly.

[maciej-szlosarczyk]

The values in staging are now updated, after you clear out your session, it can be re-tested.

[vohmar]

in production