mojaloop / mojaloop-specification

This repo contains the specification document set of the Open API for FSP Interoperability
https://docs.mojaloop.io/api
Other
20 stars 40 forks source link

Add "echo data" in the quote response and transfer request #14

Open adrianhopebailie opened 5 years ago

adrianhopebailie commented 5 years ago

If #13 is implemented to align the API with ILPv4 it will be necessary to define a data element in the callback of the /quote API to carry data sent by the payee that it expects to be unchanged and returned in the subsequent /transfer API call. (Currently payee systems will use the encoded ILP packet)

This field will be used by the payee to generate the fulfilment and condition so it must be returned unchanged to the payee in order to fulfil the transfer. To satisfy this requirement it is recommended that the field is base64 encoded.

The content of the field is at the discretion of the payee and MAY contain data (such as the quote and transaction details) that it can use to ensure the fulfilment and condition are cryptographically bound to the transaction.

henrka commented 5 years ago

This change request is very abstract in its current state. It is difficult for me to comment on this change without any more detail. Can you please add some more detail regarding how you would like this to look like in the API?

Why does the field need to be base64 encoded if it is just one field? If there were several elements then I could understand the need for base64 encoding to preserve structure of the data in JSON, but for just one element there should be no need to preserve structure. You cannot guarantee that an element is unchanged just by having it base64 encoded. To guarantee that the element is unchanged you should have end to end signing of the element.

adrianhopebailie commented 5 years ago

@ehenrka the quote callback would be:

Name Cardinality Type Description
transferAmount 1 Money The amount of Money that the Payer FSP should transfer to the Payee FSP.
payeeReceiveAmount 0..1 Money The amount of Money that the Payee should receive in the end-to-end transaction. Optional as the Payee FSP might not want to disclose any optional Payee fees.
payeeFspFee 0..1 Money Payee FSP’s part of the transaction fee.
payeeFspCommission 0..1 Money Transaction commission from the Payee FSP.
expiration 1 DateTime Date and time until when the quotation is valid and can be honored when used in the subsequent transaction.
geoCode 0..1 GeoCode Longitude and Latitude of the Payee. Can be used to detect fraud.
echoData 0..1 BinaryString Opaque data provided by the payee that must be echoed back unchanged in the transfer
condition 1 IlpCondition The condition that must be attached to the transfer by the Payer.
extensionList 0..1 ExtensionList Optional extension, specific to deployment

and the transfer request:

Name Cardinality Type Description
transferId 1 CorrelationId The common ID between the FSPs and the optional Switch for the transfer object, decided by the Payer FSP. The ID should be reused for resends of the same transfer. A new ID should be generated for each new transfer.
payeeFsp 1 FspId Payee FSP in the proposed financial transaction.
payerFsp 1 FspId Payer FSP in the proposed financial transaction.
amount 1 Money The transfer amount to be sent.
transaction 1 Transaction The end-to-end transaction.
echoData 0..1 BinaryString Data provided by the Payee during quoting that is echoed back unchanged.
condition 1 IlpCondition The condition that must be fulfilled to commit the transfer.
expiration 1 DateTime Expiration can be set to get a quick failure expiration of the transfer. The transfer should be rolled back if no fulfilment is delivered before this time.
extensionList 0..1 ExtensionList Optional extension, specific to deployment.

Note that the ILP Packet is removed from the quote callback as it is no longer passed in the transfer (See #13).

However, this means the payee no longer has some data it can use as input to the algorithm for generating the fulfilment and condition.

Previously the payee created the Transaction object, used this to encode an ILP Packet and then used the raw bytes of the ILP Packet to create a Fulfilment and then hashed this to get a Condition.

The new proposal allows the payee to use whatever transaction data it chooses as the input to this sequence. The scheme may mandate that it include the Transaction so that this is cryptographically bound to the condition and fulfillment but the API does not mandate that.

All the API mandates (to ensure interoperability) is that the same data is sent in the /POST transfers. It is up to the payee to validate this data and also regenerate the fulfilment and condition from this data to ensure the integrity of the transfer with regard to the original quote.

This system allows the payee to even encrypt this data if they choose to as it is opaque to the payer.

You asked:

Why does the field need to be base64 encoded if it is just one field? If there were several elements then I could understand the need for base64 encoding to preserve structure of the data in JSON, but for just one element there should be no need to preserve structure.

The goal is to provide an envelope for the payee to put the data into. Since the API is text encoded (data is sent as JSON) it is safer for the field to be explicitly base64 encoded to prevent unintended encoding and decoding by JSON parsers along the way. As the name states, no system other than the payee should even bother decoding this, it should just be echoed back.

You cannot guarantee that an element is unchanged just by having it base64 encoded. To guarantee that the element is unchanged you should have end to end signing of the element.

The requirement is not that the data is guaranteed to be unmodified but rather that the transfer will be invalid if the payee detects that it has been. I.e. The payee might choose to sign or encrypt the data but this is left to them as it has no impact on interoperability.

henrka commented 5 years ago

Thank you @adrianhopebailie for providing additional details.

There are other fields which could have more of an impact if the data was unintendedly encoded and decoded improperly by JSON parsers along the way. I'm having troubles justifying the use of base64 for just this element. This would be the only element in the API that uses BinaryString of undefined length as the IlpPacket would now be removed (IlpCondition and IlpFulfilment uses BinaryString32).

Otherwise I'm fine with the change.

adrianhopebailie commented 5 years ago

@ehenrka - given the requirements; an opaque data element that must be identical (byte for byte) when sent in the quote callback and the transfer request, I think this SHOULD be sent as binary data.

Can you suggest a better way to do this than base64 encoding?

henrka commented 5 years ago

@adrianhopebailie Our existing signature functionality which is based on JWS also requires identical data (byte for byte). This means that if the data which is signature protected is modified on its way the signature validation will fail, thus there is already the same requirement (byte for byte identical) for all data.

To have the data opaque, an FSP can encode it in whatever way they like (e.g. base64 or some other encoding). My suggestion would be that we recommend base64 encoding in the spec, but keep it open to the FSP which encoding that they would like as this data should only be used by the Payee FSP anyways. We can still keep a similar regex, e.g. ^[A-Za-z0-9-_=\/,.:~+]+$ (existing BinaryString is ^[A-Za-z0-9-_]+[=]{0,2}$). This would leave it up to the implementation which base64 variant to use and if the ending padding characters should be present or not, without us describing it in the spec (the current text in the spec led to some problems as one implementer interpreted it incorrectly).

(Minor edit to add + as allowed character in regex)

adrianhopebailie commented 5 years ago

@ehenrka what about making this just UTF8 text with a safe size limit?

Then implementations can put whatever they want. If they want to use binary data they can base64 encode it or send it as hex.

henrka commented 5 years ago

@adrianhopebailie Sounds good to me!

MichaelJBRichards commented 5 years ago

I agree. It seems to me that the data is for the sole use of the payee, and it is up to the payee to decide both whether or not to include an encoded version of the data it wants to check, and which elements of the transfer should be included in the check it performs (whether or not an encoded version is passed.) A given scheme might, of course, decide to mandate the form of this check; but, for me, that's not the same as saying that the API definition needs to codify it.

I assume also that the response from the payee to a quote request will need to include the Transaction object, since it's the payee who defines the Transaction object. Is that correct?

henrka commented 12 months ago

I assume also that the response from the payee to a quote request will need to include the Transaction object, since it's the payee who defines the Transaction object. Is that correct?

Picking up this old question now as we started to discuss moving to ILPv4 again. It is the Payee FSP that currently creates the transaction object. But, you could question if it is necessary to be sent as the Payer FSP already knows the terms and the transaction ID for the quote which it is has requested.

Now, if we should support the use case of allowing the Payee to change the terms somehow as you have asked for Michael, then we would need to include the changed terms somehow so that the Payer FSP is informed.