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...

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

Ian

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?

No comments on the Basic Card Payment spec.

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.

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.

@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

@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 :-)

@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!

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.

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 https://github.com/w3c/webpayments-method-identifiers/issues/9#issuecomment-246436501 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 https://github.com/w3c/remote-playback/issues/74#issue-220897112 . (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.

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/

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

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.