mojaloop / project

Repo to track product development issues for the Mojaloop project.
Other
24 stars 15 forks source link

FSPIOP-URI coming incorrect in PUT/transfers from P2P Happy path #1065

Closed sri-miriyala closed 4 years ago

sri-miriyala commented 4 years ago

Summary: PUT /transfers sending fspiop-uri to include the fsp name which should not. This is a bug in the P2P happy path.

Severity: High

Priority: Medium

Pull Requests:

Expected Behavior FSP name should not be included in FSPIOP-URI header. It should only include /transfers/{ID}, for example, this should be like below: fspiop-uri: "/transfers/54647c06-6e4d-43c2-9b67-2834eaba474c"

Steps to Reproduce

  1. Send POST /quotes from P2P happy path
  2. Send POST /transfers from P2P happy path
  3. check the postman console for the final response at payerfsp. I see the below: fspiop-uri: "/payerfsp/transfers/54647c06-6e4d-43c2-9b67-2834eaba474c"
Screen Shot 2019-11-18 at 11 32 32 AM

Specifications

Request: curl -X POST \ http://dev1-ml-api-adapter.mojaloop.live/transfers \ -H 'Accept: application/vnd.interoperability.transfers+json;version=1' \ -H 'Accept-Encoding: gzip, deflate' \ -H 'Cache-Control: no-cache' \ -H 'Connection: keep-alive' \ -H 'Content-Length: 1062' \ -H 'Content-Type: application/vnd.interoperability.transfers+json;version=1.0' \ -H 'Date: Mon, 18 Nov 2019 16:41:44 GMT' \ -H 'FSPIOP-Destination: payeefsp' \ -H 'FSPIOP-HTTP-Method: POST' \ -H 'FSPIOP-Signature: {"signature":"iU4GBXSfY8twZMj1zXX1CTe3LDO8Zvgui53icrriBxCUF_wltQmnjgWLWI4ZUEueVeOeTbDPBZazpBWYvBYpl5WJSUoXi14nVlangcsmu2vYkQUPmHtjOW-yb2ng6_aPfwd7oHLWrWzcsjTF-S4dW7GZRPHEbY_qCOhEwmmMOnE1FWF1OLvP0dM0r4y7FlnrZNhmuVIFhk_pMbEC44rtQmMFv4pm4EVGqmIm3eyXz0GkX8q_O1kGBoyIeV_P6RRcZ0nL6YUVMhPFSLJo6CIhL2zPm54Qdl2nVzDFWn_shVyV0Cl5vpcMJxJ--O_Zcbmpv6lxqDdygTC782Ob3CNMvg\",\"protectedHeader\":\"eyJhbGciOiJSUzI1NiIsIkZTUElPUC1VUkkiOiIvdHJhbnNmZXJzIiwiRlNQSU9QLUhUVFAtTWV0aG9kIjoiUE9TVCIsIkZTUElPUC1Tb3VyY2UiOiJPTUwiLCJGU1BJT1AtRGVzdGluYXRpb24iOiJNVE5Nb2JpbGVNb25leSIsIkRhdGUiOiIifQ"}' \ -H 'FSPIOP-Source: payerfsp' \ -H 'FSPIOP-URI: /transfers' \ -H 'Host: dev1-ml-api-adapter.mojaloop.live' \ -H 'Postman-Token: 6b2cc3dc-02cc-4527-b729-5b275df22a2a,87c07d06-bf09-4aad-9e32-d67bb80fc6bb' \ -H 'User-Agent: PostmanRuntime/7.19.0' \ -H 'cache-control: no-cache' \ -d '{ "transferId": "54647c06-6e4d-43c2-9b67-2834eaba474c", "payerFsp": "payerfsp", "payeeFsp": "payeefsp", "amount": { "amount": "1.11", "currency": "USD" }, "expiration": "2019-11-18T16:24:15.292Z", "ilpPacket": "AQAAAAAAAADIEHByaXZhdGUucGF5ZWVmc3CCAiB7InRyYW5zYWN0aW9uSWQiOiIyZGY3NzRlMi1mMWRiLTRmZjctYTQ5NS0yZGRkMzdhZjdjMmMiLCJxdW90ZUlkIjoiMDNhNjA1NTAtNmYyZi00NTU2LThlMDQtMDcwM2UzOWI4N2ZmIiwicGF5ZWUiOnsicGFydHlJZEluZm8iOnsicGFydHlJZFR5cGUiOiJNU0lTRE4iLCJwYXJ0eUlkZW50aWZpZXIiOiIyNzcxMzgwMzkxMyIsImZzcElkIjoicGF5ZWVmc3AifSwicGVyc29uYWxJbmZvIjp7ImNvbXBsZXhOYW1lIjp7fX19LCJwYXllciI6eyJwYXJ0eUlkSW5mbyI6eyJwYXJ0eUlkVHlwZSI6Ik1TSVNETiIsInBhcnR5SWRlbnRpZmllciI6IjI3NzEzODAzOTExIiwiZnNwSWQiOiJwYXllcmZzcCJ9LCJwZXJzb25hbEluZm8iOnsiY29tcGxleE5hbWUiOnt9fX0sImFtb3VudCI6eyJjdXJyZW5jeSI6IlVTRCIsImFtb3VudCI6IjIwMCJ9LCJ0cmFuc2FjdGlvblR5cGUiOnsic2NlbmFyaW8iOiJERVBPU0lUIiwic3ViU2NlbmFyaW8iOiJERVBPU0lUIiwiaW5pdGlhdG9yIjoiUEFZRVIiLCJpbml0aWF0b3JUeXBlIjoiQ09OU1VNRVIiLCJyZWZ1bmRJbmZvIjp7fX19", "condition": "HOr22-H3AfTDHrSkPjJtVPRdKouuMkDXTR4ejlQa8Ks" }'

Tasks:

elnyry-sam-k commented 4 years ago

Hi @sri-miriyala - a couple of questions..

  1. Can you please point to the Specification or standard which says the value / pattern here is wrong, thereby indicating that this is a bug?
  2. Whats the value of the FSPIOP-URI value coming in the Fulfil request from the Payee? Is the Switch here just forwarding what it got or is it generating a new value? It would be interesting if the Switch is generating this.

For #1 above, the description from the Signature doc says this: "A customized parameter FSPIOP-URI that represents the URI path and query parameters of HTTP request message of the APIs must be present."

sri-miriyala commented 4 years ago

@elnyry included the curl request in the details at the bottom. In the POST request sending FSPIOP-URI is "/transfers"

elnyry-sam-k commented 4 years ago

@sri-miriyala my question was about the PUT /transfers/{ID} call from the Payee FSP, the Fulfil request, not the POST call

sri-miriyala commented 4 years ago

@elnyry here are the logs from payeefsp sim: scheme-adapter_1 | msg: 'Executing HTTP PUT: { method: \'PUT\',\n uri: \'http://extgw.public.tips-sandbox.live:8280/fsp/1.0/transfers/c7379f2a-32f1-4744-bcfc-4d0a034b3f58\',\n headers: \n { \'content-type\': \'application/vnd.interoperability.transfers+json;version=1.0\',\n date: \'Tue, 19 Nov 2019 20:53:54 GMT\',\n \'fspiop-source\': \'payeefsp\',\n \'fspiop-destination\': \'payerfsp\',\n Authorization: \'Bearer e29975a8-5938-3409-8839-3c2e45db2303\',\n \'fspiop-http-method\': \'PUT\',\n \'fspiop-uri\': \'/transfers/c7379f2a-32f1-4744-bcfc-4d0a034b3f58\',\n \'fspiop-signature\': \'{"signature":"YTl-yV2K1z73wBNGssAClKq2wEQ095ZJRyD2fwHXoXFuINri7VfcDynl2BzbKOhKQg36lME6RImWFoz1t3TMpvJ3lUiQHBmRRAoWABI9xKsrBP8uNLZTu0PVLKB0nVwfSYObaqwO-eBoSCCrZ6Ygcq6Fy5DegkcoZuc5apboXoAGha8FeZ_5T4u-M4xnEekpPwtdlY-WF6pdFqyo6Mbk7bHfQ-X7nMbAhI0BNI5Ss8Rgg70T7z0hYrklFVfwVp4k96CqQrRPMUJNVDf0HxlViqAR2RyHFwpYDbYR4VwJRyJXdx6ez4HkILFcNnnS5vWLPBBZdJjKptAJVDPzQJoMRg","protectedHeader":"eyJhbGciOiJSUzI1NiIsIkZTUElPUC1VUkkiOiIvdHJhbnNmZXJzL2M3Mzc5ZjJhLTMyZjEtNDc0NC1iY2ZjLTRkMGEwMzRiM2Y1OCIsIkZTUElPUC1IVFRQLU1ldGhvZCI6IlBVVCIsIkZTUElPUC1Tb3VyY2UiOiJwYXllZWZzcCIsIkZTUElPUC1EZXN0aW5hdGlvbiI6InBheWVyZnNwIiwiRGF0ZSI6IlR1ZSwgMTkgTm92IDIwMTkgMjA6NTM6NTQgR01UIn0"}\' },\n body: \'{"completedTimestamp":"2019-11-19T20:53:54.929Z","transferState":"COMMITTED","fulfilment":"xDNI_OVBD9VF5CiwIn4jOmti7LIJInqv2pizvPBj1mo"}\',\n agent: \n Agent {\n domain: null,\n _events: { free: [Function] },\n _eventsCount: 1,\n _maxListeners: undefined,\n defaultPort: 80,\n protocol: \'http:\',\n options: { path: null },\n requests: {},\n sockets: { \'sim:3000:\': [Array] },\n freeSockets: {},\n keepAliveMsecs: 1000,\n keepAlive: false,\n maxSockets: Infinity,\n maxFreeSockets: 256 },\n resolveWithFullResponse: true,\n simple: false }', scheme-adapter_1 | timestamp: '2019-11-19T20:53:54.932Z' } scheme-adapter_1 | { app: 'mojaloop-sdk-inbound-api',

I see there is no fsp name in the fspiop-uri.

elnyry-sam-k commented 4 years ago

@sri-miriyala this is not the FSP name being added to the FSPIOP-URI.. Its the actual value thats derived as the URI for that endpoint, based on the value provided during on-boarding (or updated later)..

I just did a GET on {{HOST_CENTRAL_LEDGER}}/participants/payerfsp/endpoints on the dev1 envt and here is the output

[
    {
        "type": "FSPIOP_CALLBACK_URL_PARTICIPANT_PUT",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/participants/{{partyIdType}}/{{partyIdentifier}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTICIPANT_PUT_ERROR",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/participants/{{partyIdType}}/{{partyIdentifier}}/error"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTICIPANT_BATCH_PUT",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/participants/{{requestId}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTICIPANT_BATCH_PUT_ERROR",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/participants/{{requestId}}/error"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTIES_GET",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/parties/{{partyIdType}}/{{partyIdentifier}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTIES_PUT",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/parties/{{partyIdType}}/{{partyIdentifier}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_PARTIES_PUT_ERROR",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/parties/{{partyIdType}}/{{partyIdentifier}}/error"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_QUOTES",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_TRANSFER_POST",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/transfers"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_TRANSFER_PUT",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/transfers/{{transferId}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_TRANSFER_ERROR",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/transfers/{{transferId}}/error"
    },
    {
        "type": "NET_DEBIT_CAP_ADJUSTMENT_EMAIL",
        "value": "sridevi.miriyala@modusbox.com"
    },
    {
        "type": "SETTLEMENT_TRANSFER_POSITION_CHANGE_EMAIL",
        "value": "sridevi.miriyala@modusbox.com"
    },
    {
        "type": "NET_DEBIT_CAP_THRESHOLD_BREACH_EMAIL",
        "value": "sridevi.miriyala@modusbox.com"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_BULK_TRANSFER_POST",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/bulkTransfers"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_BULK_TRANSFER_PUT",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/bulkTransfers/{{id}}"
    },
    {
        "type": "FSPIOP_CALLBACK_URL_BULK_TRANSFER_ERROR",
        "value": "http://dev1-simulator.mojaloop.live/payerfsp/bulkTransfers/{{id}}/error"
    }
]

As you can see above, for the PUT on /transfers - FSPIOP_CALLBACK_URL_TRANSFER_PUT, the value is: http://dev1-simulator.mojaloop.live/payerfsp/transfers/{{transferId}}. In this, the value derived for the URI is: /payerfsp/transfers/{{transferId}}, since http is the protocol and dev1-simulator.mojaloop.live comes under hostname; this applies to the PUT /transfers error as well.

There's just one issue to think about here, with the Switch possibly generating this header value, instead of just passing along the incoming value. Let me get back to you on this one, if thats indeed the case here.

elnyry-sam-k commented 4 years ago

Following-up, the Switch is setting the value for FSPIOP-URI to the URI value based on the end-point as I mentioned above. The spec doesn't talk about it, but I think we agreed to just relay the value between the two FSPs. (Its however, different for the messages that originate at the Switch, in which case, its generated dynamically based on the endpoint)

bushjames commented 4 years ago

The switch must not change any of the JWS related headers when forwarding requests. These are FSPIOP-Signature, FSPIOP-URI, FSPIOP-HTTP-Method, Date, FSPIOP-Source, FSPIOP-Destination. Altering the value of any one of these will cause JWS validation at the recipient to fail.

sri-miriyala commented 4 years ago

This is the screenshot for API spec

Screen Shot 2019-11-27 at 1 16 52 PM
sri-miriyala commented 4 years ago

Yes @bushjames .... that was the reason I opened the bug. Payee sim was setting FSPIOP-URI as /transfers/{ID} and it is being over written by switch as /payerfsp/transfers/{ID}

elnyry-sam-k commented 4 years ago

This is the screenshot for API spec

Screen Shot 2019-11-27 at 1 16 52 PM

@sri-miriyala It was an example value that you posted. Its not a rule or a specification. I already posted above two things:

  1. Establishing that the Switch is updating the value here and that the fix is being put in place (added to current Sprint, fixed. Will be included in next Helm release).
  2. The text from specification about the FSPIOP-URI header.

We cannot take example values and pass them on or consider them as Validation or Specification rules.

Here's the list of column names of the table (Table-1) from which you posted the snapshot: image

sri-miriyala commented 4 years ago

@elnyry I see this behavior on the -ve cases where the fspiop-uri coming with a fspname prefixed. Can we pls re-open the bug?

elnyry-sam-k commented 4 years ago

@sri-miriyala yes please. I've re-opened the bug, can you please provide more details with requests (text not snapshots), so that it can be reproduced and looked into?

sri-miriyala commented 4 years ago

@elnyry , it is the regular block transfer scenario that reproduces this. But all the other error scenarios as well where the error is generated in switch. for now, can you pls try the block transfer scenario form the GoldenPath collection.

amarmodus commented 4 years ago
Screenshot 2020-03-17 14 12 10
elnyry-sam-k commented 4 years ago

@sri-miriyala @amarmodus @bushjames @rmothilal - Thanks!

We've agreed that there is no regression now. Further discussion to continue on this issue: https://github.com/mojaloop/project/issues/1264