camaraproject / CarrierBillingCheckOut

Repository to describe, develop, document and test the Carrier Billing Check Out API family
Apache License 2.0
9 stars 9 forks source link

Enhancements in GET payment endpoints #111

Closed PedroDiez closed 6 months ago

PedroDiez commented 9 months ago

Problem description

Be more accurate in the description of these endpoints to indicate they are refering in the context of a user phone number. Add an exception when phone number cannot be deducted from Access Token context

Possible evolution

1) Endpoint GET /payments

Update endpoint description:

Retrieve a list of payments of a given phone number and their details based on some filtering criteria. Only payments recently performed (e.g. in the last hour) by the API client invoking this method will be returned; time window for availability will be a configuration in API Server.

Add this exception to this endpoint:

2) Endpoint GET /payments/{paymentId}

Update endpoint description: Retrieve payment details for a given payment implicitly associated to a given phone number

Add this exception to this endpoint:

Additional context N/A

PedroDiez commented 8 months ago

Not covered in 11/OCT meeting. To be reviewed in next meeting and check offline

PedroDiez commented 8 months ago

25/OCT: I would like to take advantage of this and reformulate some how the proposal

bigludo7 commented 8 months ago

25/OCT: I would like to take advantage of this and reformulate some how the proposal

Yes agreed. As said I'm note sure about using the token associated with the phone number.

For me the critical point is strictly enforce that a API consumer (identified with credentials) can only query and consult the payments it created. After this is perfectly valid for me to an API consumer, let say a VoD app for exemple, to browse payment for its servers and passing msisdn for example.

A good topic to discuss....

PedroDiez commented 8 months ago

Hi,

Probably here we need to consider both business scenarios where there is user phone number involved and no user phone number involved.

Think we can have two approaches (regardless the final description provided in the GET endpoints).

retrievePayments When Access Token is issued for a given user phone number, the list of payments returned would be only the ones associated to that user phone number and API client. When Access Token is not associated to a user phone number, therefore only associated to API client the list of payments returned would be all the ones managed by that API client.

retrievePayment When Access Token is issued for a given user phone number, the payment details would be returned in case the paymentId is associated to that user phone number and API client, otherwise 404 NOT_FOUND will be returned. When Access Token is not associated to a user phone number, the payment details are returned in case the API client managed that payment.

My preference would be the first approach (to avoid the sending of personal information in a query parameter, that if not wrong was commented in the past in an issue not a good option due to security reasons https://github.com/camaraproject/WorkingGroups/issues/212).

bigludo7 commented 8 months ago

Thanks @PedroDiez

I like also the approach 1. We able there to retrieve all payments for one given user for one given service and retrieve all payments for all user for one given service. We are not able to retrieve all the payment performed for one given user on all his/her services but I'm not sure this is a valid request.

OK for me to have approach 1 in the API documentation.

PedroDiez commented 7 months ago

Regarding analysis if additional error should be managed. Think also new excepction is required in both endpoints.

When Access token is issued in the context of a user, a unique phone number is not explictly indicated.

HTTP 403 Phone Number cannot be deducted from access token context.("code": "CARRIER_BILLING.INVALID_TOKEN_CONTEXT","message": "User phone number cannot be deducted from access token context.").

PedroDiez commented 7 months ago

Commented during meeting 21/NOV and aligned to be considered in the next PR. Also with the consideration of new exception commented