Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass SKU instead of Total #879

Open
ianbjacobs opened this issue Sep 3, 2019 · 12 comments
Open

Pass SKU instead of Total #879

ianbjacobs opened this issue Sep 3, 2019 · 12 comments

Comments

@ianbjacobs
Copy link
Collaborator

ianbjacobs commented Sep 3, 2019

Hi all,

@rsolomakhin passed on this use case today, so recording it here.

Google Play Store requires the merchant to creates SKUs and specify SKUs instead of amounts during the payment flow. There's also a feature to request the price of the SKUs before payment, so that merchant can display the price in their app. Can we add this feature to Payment Request API in the next version? Sample code:

const request = new PaymentRequest([{supportedMethods: 'https://play.google.com/store'}], {total: {sku: '1234567890'}});
const details = await request.getDetailsFromSku();
showAmounts(details);
const response = await request.show();
@adrianhopebailie
Copy link
Collaborator

It's not clear to me why translating SKUs to price should be handled by PaymentRequest?

It feels to me like the group has expressed recently that we have already tightly coupled too many things to "payment" with this API.

i.e. Collecting address, email, shipping are more related to "checkout" than payments. As a result PR API is only really useful to many merchants in the guest checkout scenario.

What about we change this to:

const total = getTotalFromSku();
const request = new PaymentRequest([{supportedMethods:'https://play.google.com/store'}], {total});
const response = await request.show();

@davidbenoit
Copy link

Would there be a method of getting new details if the currency changes? For example, if I were to detect that the method of payment should be billed in currency X vs Y, I want new amounts to be displayed, and a new total calculated. Possible?

@danyao
Copy link
Collaborator

danyao commented Sep 5, 2019

I agree with @adrianhopebailie that Payment Request API should not be directly involved in translating SKUs to prices. SKU is a payment-method specific concept. I think we should keep Payment Request API generic.

I like the getTotalFromSku() direction. But here's a missing piece: the merchant still needs to figure out how to implement getTotalFromSku(), which involves a cross-origin communication to the payment app. Given that Payment Request API is meant to establish a communication pipe between the merchant and the payment app, can we expand it to support this use case?

What about something like this:

const emptyDetails = {} // make 'total' optional, so it can be omitted here
const request = new PaymentRequest([{supportedMethods:'https://play.google.com/store'}], emptyDetails);

// New channel for merchant to query selected payment app. This returns a promise that
// resolves to a payment-method defined blob. Merchant can turn that into a `PaymentDetails`:
let detailsPromise = request.query(
    'https://play.google.com/store',
    {querySkuRequest: { sku: '123456'}}
).then((response) => {
    return {
      total: {
        label: "total",
        amount: {
          currency: response.querySkuTotalResponse.currency,
          value: response.querySkuTotalResponse.amount,
        },
      },
    };
  });

request.show(detailsPromise);

To make this work, Payment Handler API will expose a QueryEvent, which is triggered on the payment app's service worker when merchant calls request.query().

The obvious concern is that this may be too generic: we're allowing arbitrary exchange of data between merchants and payment apps. On the other hand, a payment app can choose to decline a query from an untrusted merchant, using the merchant validation mechanism.

@adrianhopebailie
Copy link
Collaborator

I hadn't made the connection that this is actually requesting the handler to provide payment details.

That's an interesting use case. Does it relate to w3c/payment-handler#337 in that case?

If so it seems like we could think about solving this more abstractly, i.e. let payment handlers specify any of the details they are able to provide.

Something like (building on the example from @rsolomakhin):

registration.paymentManager.capabilities.set(
    ['requestShipping', 'requestPayerEmail', 'setTotal'])
  .then(handleDoneSettingCapabilities)
  .catch(handleError);

This would imply some additional matching criteria for Payment Handlers, i.e. if the PaymentRequest is created without a total, then only handlers that can setTotal should be options.

I realise this doesn't allow the website to know the total until the PaymentRequest is invoked (.show()) but that seems preferable from a privacy perspective.

@ianbjacobs
Copy link
Collaborator Author

Hi all,

What if total is made available by a promise resolved later?

Payment Request could generically support such a mechanism, and how the promise resolves could be payment-method specific. The payment handler does whatever it does, then provides the merchant with the result, and the merchant can then update the payment request.

I was also wondering whether such an approach could be used for the $0 total use cases (e.g., ride sharing) or some others.

I've not thought it through but wanted to mention the concept before we meet face-to-face.

Ian

@danyao
Copy link
Collaborator

danyao commented Sep 11, 2019

@adrianhopebailie: I haven't thought of this use case as an analogy of w3c/payment-handler#337 so it's cool that you point this out! I see a clear benefit of building on top of the capabilities registration mechanism: this limits the information exchange between merchant and payment handler to well-defined entities (i.e. shipping, contact, total), which is probably good for security.

I worry slightly about the capabilities list growing too long over time (which a generic pipe would be simpler) - but it's just a hunch now. I have no use cases to back that up.

I think there may still be a legitimate use case where the website needs to know the total before show(): e.g. to display the price to the user. Why do you think this has worse privacy property?

@ianbjacobs: can you remind me what is the ride sharing $0 total use case? I thought there is usually a cost estimate when starting a ride.

@ianbjacobs
Copy link
Collaborator Author

@danyao, I was referring to:
#858

Ian

@cyberphone
Copy link

How does this match a system like Saturn which requires that payment data is signed? The current demo application which builds on PaymentRequest only uses server-provided static data so I haven't had any opportunities (or reasons) for looking into this in detail. The data element of supportedInstruments is currently used for dealing with signed payment data.

@danyao
Copy link
Collaborator

danyao commented Sep 11, 2019

Ah thanks @ianbjacobs. Yes I believe #858 is raising essentially the same issue as this thread. Should we combine them?

Just to add to the list of use cases: donation is another one where optional 'total' can be useful. PayPal currently has a flow where website simply adds a "donate" button. When user clicks on it, it takes them to a PayPal page where they can enter a custom amount. The web payments equivalent would be website creating a PaymentRequest without a total, then user completing the amount in the payment handler's UI.

@cyberphone: IIUC, Saturn [1] is about establishing an e2e encrypted channel between the merchant bank and the user bank without having to rely on intermediaries. It does so by attaching a merchant bank signed object in the PaymentRequest which the user bank can verify. Then user bank returns an encrypted payment credentials only the merchant bank can process. Is this roughly correct?

If so, I believe the optional total proposal here is orthogonal to Saturn because "total" is just another piece of information that the user bank (represented by the payment handler) sends back in the response. It can either be included in the encrypted blob in PaymentResponse.details or as plaint text outside. Either option has the same security property because "total" by itself is not enough to perpetrate an attack.

[1] https://cyberphone.github.io/doc/saturn/enhanced-four-corner-model.pdf

@cyberphone
Copy link

cyberphone commented Sep 11, 2019

Hi @danyao Saturn is about a lot of things but the thing I'm talking about is step 2 in:
https://cyberphone.github.io/doc/saturn/enhanced-four-corner-model.pdf
That is, it is the Merchant who signs the payment request data (amount, currency and few other things as well like type of payment [direct, gas station, etc, etc]). If amount is updated for whatever reason, the payment request data must be recreated including the signature.

The verification of the Merchant is performed in the subsequent step which is only between the Merchant and the User's bank.

Saturn depends on statically provisioned, account specific keys in the Mobile Device so there is no direct communication between the User (Mobile Device) and the User's Bank during a payment operation.

@ianbjacobs
Copy link
Collaborator Author

@danyao,

It felt to me that #858 are different use cases, so I didn't combine the issues. But it may well be that we have a solution that addresses them both. So I didn't combine the issues yet, but we have now cross-referenced them.

@cyberphone
Copy link

@danyao et al. I have just scrapped(!) signing PaymentRequest data. By building on https://tools.ietf.org/html/draft-rundgren-json-canonicalization-scheme-09 for creating a hash of the JSON input data, signing it together with a bunch of related objects, and finally encrypting the result as performed in step 3, followed by the Merchant's signature in step 4, the scheme appears to anyway be "air-tight" from a security point of view. I.e. the signature was redundant.

However, I would still be a bit concerned about limitations on what you can update in an event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants