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

Web Payments Working Group Specifications #152

Closed
1 of 3 tasks
ianbjacobs opened this issue Feb 15, 2017 · 14 comments
Closed
1 of 3 tasks

Web Payments Working Group Specifications #152

ianbjacobs opened this issue Feb 15, 2017 · 14 comments
Assignees

Comments

@ianbjacobs
Copy link

ianbjacobs commented Feb 15, 2017

Hello TAG!

I'm requesting a TAG review of:

Read more detail in the early Jan 2017 call for review to the W3C Chairs list.

Primary contact: @ianbjacobs

Further details:

  • Relevant time constraints or deadlines: The Web Payments Working Group next meets face-to-face 23-24 March in Chicago. The meeting is just prior to the IETF meeting, so if someone from the TAG plans to be in Chicago and wants to pop over to the WPWG meeting, let me know. I realize this means there is not a lot of time to raise issues before the FTF meeting, but that would certainly be very welcome. We do not yet have time frame for going to CR, but I anticipate that we will use a portion of the FTF meeting to get our ducks in a row.
  • [ some time ago] I have read and filled out the Self-Review Questionnare on Security and Privacy. The assessment from that time is here.
  • [ a long time ago ] I have reviewed the TAG's API Design Principles

You should also know that...

  • Google, Microsoft, Facebook, Opera, Samsung, and Mozilla have begun to implement.
  • We have some initial work on a test suite but a lot more work to do.

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@travisleithead
Copy link
Contributor

Read through the PaymentRequest API spec. Here's some notes. In general this spec is very functionally complete. I would have loved many more examples, as this API is highly async, and it was tricky building up the mental map of what happens when and how. An organization by "feature" might have made it much more readable, with a lead-in code example :)


In PaymentRequest(methodData, details, options) constructor MUST act as follows Step 5 throws if total.amount.value is negative. Can't the merchant push a negative (return) transaction? (Same with 10.2.2.1.3).

currencySystem - I read the current spec as the user agent doesn't validate this in any way, but passes along this string to a payment app as part of the payment app matching process. At what point would a bogus value fail?

Would love to see another code example around the shipping updates process flow before delving into the details.

Nit: true is there is a pending updateWith() call should be if (typo)

Note: I like that canMakePayment works off post-validated constructor param data. I assume it was considered as a static method as well (in order to avoid allocating the PaymentRequest constructor). The current design centralizes parameter validation and simplifies what canMakePayment would otherwise have to do.

Section 18 (Security): Pass-through strings that may be presented to the user are targets for attack. May want to collect this in the security considerations section. (E.g., some advice like: don't include serialized html strings, etc.) Or describe what sanitization steps the UA may want to take to reject suspicious strings before displaying them...?

Bit of an API mis-alignment: [...] then it should call updateWith() and provide a PaymentDetailsUpdate dictionary, or a promise for one, containing changed values that [...] yet the API signature for updateWith requires a Promise with no option for just passing a PaymentDetailsUpdate dictionary.

In the statement: When setting the payerPhone value, the user agent SHOULD format the phone number to adhere to [E.164]. Otherwise, set it to null. is the intent to set the value to null when the UA cannot format the number into [E.164] format? I wasn't sure how to read that sentence.

In Exposing available payment methods there is mention about repeated calls to discover what payment methods are available, and the claim that The fact that a successful match to a payment method causes a user interface to be displayed mitigates the disclosure risk. I'm not sure I follow--show is the API to trigger the UI--before that paymentMethods dictionary is provided in the PaymentRequest constructor, and the canMakePayment is accessible independently of show--so how is this actually mitigated?


Will move on to Payment Method Identifiers next...

@ianbjacobs
Copy link
Author

Thanks @travisleithead! I've made sure the WPWG knows:
https://lists.w3.org/Archives/Public/public-payments-wg/2017Apr/0011.html

Ian

@travisleithead
Copy link
Contributor

Payment Method identifiers


Typo: e.g., "I only credit cards from brand A and debit cards from brand B"). missing 'accept'?

Is there an expectation that the Payment manifest will be included in this spec at some point before CR entry?


@travisleithead
Copy link
Contributor

No comments on the Basic Card Payment spec.

@zkoch
Copy link

zkoch commented Apr 3, 2017

Thanks, Travis!

In PaymentRequest(methodData, details, options) constructor MUST act as follows Step 5 throws if total.amount.value is negative. Can't the merchant push a negative (return) transaction? (Same with 10.2.2.1.3).

Nope, no negatives. We don't support refunds, which is effectively what a negative payment is. Things get all kinds of complicated in refund-land, so we thought best to avoid it for now.

currencySystem - I read the current spec as the user agent doesn't validate this in any way, but passes along this string to a payment app as part of the payment app matching process. At what point would a bogus value fail?

Good question. I suppose never. Note that we'll be marking currency systems as "at risk" for CR.

I'm not sure I follow--show is the API to trigger the UI--before that paymentMethods dictionary is provided in the PaymentRequest constructor, and the canMakePayment is accessible independently of show--so how is this actually mitigated?

So there are two ways to determine if a user has an available payment method. The first is to just call .show(). If that doesn't throw, a UI is shown, and that means the user has an available way to pay. This makes it a poor "attack" vector since you can't keep iterating through forms of payment if the UI is shown.

Before calling .show(), however, you can call canMakePayment to determine if a form of payment is available before calling .show(). This gives merchants more flexibility on the UI side. For this, we say in the spec: "Optionally, at the user agent's discretion, return a promise rejected with a "QuotaExceededError" DOMException." The idea here is that user agents can, themselves, figure out the best way to address the risk of fingerprinting exposed by canMakePayments.

I'll file bugs for some of the other issues.

@domenic
Copy link
Member

domenic commented Apr 4, 2017

Bit of an API mis-alignment: [...] then it should call updateWith() and provide a PaymentDetailsUpdate dictionary, or a promise for one, containing changed values that [...] yet the API signature for updateWith requires a Promise with no option for just passing a PaymentDetailsUpdate dictionary.

That's how Promise<PaymentDetailsUpdate> is defined in Web IDL, to accept both; the spec is correct as-is.

@ianbjacobs
Copy link
Author

@travisleithead wrote:

"Is there an expectation that the Payment manifest will be included in this spec at some point before CR entry?"

We discussed that question at the recent FTF meeting [1]. We do not plan to merge them before CR. We do not yet know whether we will merge them further down the line.

Ian

[1] https://www.w3.org/2017/03/24-wpwg-minutes#item04

@travisleithead
Copy link
Contributor

travisleithead commented Apr 4, 2017

@domenic thanks for the pointer--I hadn't realized that. My understanding is now that any value to be converted to a Promise<T> is processed by first routing the provided value through the intrinsic Promise.resolve() method, which either resolves right away with the value (e.g., a PaymentDetailsUpdate), or chains up to the provide Promise's then method (or really any object with a then method--doesn't have to be a native Promise type).

Learned something new today :-)

@travisleithead
Copy link
Contributor

travisleithead commented Apr 4, 2017

@zkoch Thanks for the feedback. I hadn't thought that the refunds pipeline was no different from regular payments pipeline, but I can accept that it is. By being extra strict out of the gate, we leave open the opportunity to relax the requirements later if we want to support negative values in a future version of the spec.

Also thanks for filing the other issues in your repro for me!

@dbaron
Copy link
Member

dbaron commented Apr 11, 2017

So I've been bad about getting to this review in a timely manner.

However, I did take a look at the Basic Card Payment spec today, and have a few initial comments that aren't concrete enough to turn into github issues (in addition to the two filed above):

I think the idea of linking to and maintaining a separate registry of approved card network identifiers that's separate from the specification seems like a good thing. I'm not sure that the W3C has a lot of experience doing this sort of list/registry, though. I wonder if they should be a little more centralized within W3C so that W3C itself can keep track of them better if, e.g., the working group in question goes out of existence.

Regarding billingAddress -- I'm curious if there's good practice on whether optional fields in dictionaries should, by convention, be nullable. I don't actually know.

@dbaron
Copy link
Member

dbaron commented Apr 11, 2017

So I looked at the Payment Method Identifiers spec today as well, and filed a bunch of issues (linked just above). But some other thoughts that I haven't yet been able to turn into a spec issue against the spec repository, partly because I don't yet understand what the suite of specs as a whole is doing and where the parts not yet filled in are going to go (so I'd welcome feedback to better target or to assuage these concerns):

It's not clear to me whether w3c/payment-method-id#9 (comment) reflects a group decision that is still partly pending editing, whether some of that is reflected in the two issues in 3. URLs as Payment Method Identifiers (and if so how those issues will be resolved), or whether the spec is out of date relative to that discussion. But I'm concerned about some of the issues raised there, particularly about whether the URLs point to something that has the information needed for a web client to actually interact with the payment system and present a user interface for it. Which is I guess a bigger issue that I should probably bring up:

So I'm going to try to speak here with my TAG hat on and my Mozilla hat off. TAG members are not supposed to be representing their employers interests, and I think what I'm saying here does disagree in some ways with what I would say as Mozilla's AC Rep. (Obviously, even with "hat off" I'm still speaking based on what I've learned in my own experiences.)

One of the things that always attracted me to the Web was that it was an open platform. The specifications were publicly accessible and freely implementable, which meant that one of the Web's strengths was that newcomers could try to improve pieces of the system, whether by adding or improving aspects of the Web, or by bringing the Web to a new platform without permission from anyone else.

There have been a whole bunch of things recently that have moved (quite quickly) towards adding things to the Web that tie big external pieces of technology into the Web in ways that put these characteristics at risk.

EME was probably the first of this bunch (separated by a bit from the others), but was done as part of making a tradeoff between the addition of one external piece of technology (EME) and the removal of another larger one (plugins). This tradeoff had some advantages (sandboxing leading to much less security and privacy risk, smaller and largely-fixed feature set rather than open-ended one) and some disadvantages (no published API that allows new browsers to enter the market by using existing plugins, although this was only the case for plugins on existing operating systems, giving a somewhat more official blessing to DRM).

We recently saw the Remote Playback API, which led to the concerns that the TAG documented in w3c/remote-playback#74 (comment) . (That case is perhaps more clear-cut because it doesn't cross industries as widely.) I worry that this comment may apply to payment methods as well, and particularly to the ability to get in to systems more secure than card payment.

That is, I'm concerned that the payments specifications may have the same problem: that by being too much glue to other systems and not enough of its own technology, they may tie Web content to the use of payment methods that are specific to an operating system, a browser, or both, or ties browsers to payment systems, in ways that inhibit the ability for newcomers to build browsers or bring browsers to new operating systems.

But again, I still don't feel I understand what's going on with the set of specs as a whole.

@ianbjacobs
Copy link
Author

Hi @dbaron,

Thank you for the careful review. I can fill in a couple of blanks:

  1. Payment method identifiers that are URLs will point to Payment Method Manifest Files.
    We are working on a separate specification for those. Our current draft of that spec
    is out of date with respect to "version 2" that we recently discussed at our face-to-face
    meeting:
    https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

  2. The system as a whole intends to support:
    a) Arbitrary payment methods, identified by URLs and described by payment
    method manifest files.
    b) Third party payment apps that register their support for different payment
    methods with the browser. The WG is developing the Payment Handler API [1]
    to that Web apps can request permission from the user to handle
    payment request events for different payment methods. Thus, the ecosystem
    will support both Web-based payment apps (through an open standard)
    and native payment apps (through proprietary APIs).

Ian

[1] https://w3c.github.io/webpayments-payment-apps-api/

@dbaron
Copy link
Member

dbaron commented Apr 27, 2017

Need to split off a separate issue on registries, and then I think the conclusion was to close this.

@travisleithead
Copy link
Contributor

Note: we sent a mail off to the AB to start a conversation about managing registries (generally), which includes the Payment Request Registry. This closes the review from our end.

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

No branches or pull requests

8 participants