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

Add canMakePayment() method #380

Merged
merged 1 commit into from
Dec 14, 2016
Merged

Add canMakePayment() method #380

merged 1 commit into from
Dec 14, 2016

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Dec 14, 2016

This closes #310, and closes #316 (which it is roughly based on), and closes #367 by intentionally allowing user agent leeway in restricting repeated calls to canMakePayment().


The main difference from #316 is in allowing user agents maximum flexibility in protecting against fingerprinting, instead of trying to prescribe an algorithm for doing so. It also contains a number of fixes to the algorithm.

This closes w3c#310, and closes w3c#316 (which it is roughly based on), and closes w3c#367 by intentionally allowing user agent leeway in restricting repeated calls to canMakePayment().
@zkoch zkoch requested a review from adrianba December 14, 2016 19:47
Copy link
Contributor

@adrianba adrianba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine to merge but we should add a note to the spec explaining that "supports" in the canMakePayment case may be different from "supports" in the show() case, because some implementations may only return true if there are payment credientials stored.

@zkoch zkoch merged commit 33edb3b into w3c:gh-pages Dec 14, 2016
@domenic domenic deleted the canmakepayment branch December 15, 2016 00:02
@burdges
Copy link

burdges commented Dec 15, 2016

I'd do think "returns true" should be considered a valid algorithm for browsers that seek to provide maximum security, although the InvalidStateError makes sense, so it's not literally "always true" I guess.

@rsolomakhin
Copy link
Collaborator

@domenic: Excellent patch! Thank you for sending this in.

@adrianba: +1 to distinguish "supports" of .canMakePayment() and .show().

@burdges: I don't quite follow about maximum security. Can you elaborate?

@domenic
Copy link
Collaborator Author

domenic commented Dec 15, 2016

Thanks @rsolomakhin! @adrianba and @rsolomakhin, for clarifying "supports", probably that is best done on top of #382, which itself clarifies "supports" for show(). I can work on something, although I'm building up quite the dependency list of unmerged PRs :)

MXEBot pushed a commit to mirror/chromium that referenced this pull request Dec 17, 2016
Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7AQAJ

Spec:
https://w3c.github.io/browser-payment-api/#canmakepayment-method

Added in:
w3c/payment-request#380

BUG=664619

Review-Url: https://codereview.chromium.org/2579793003
Cr-Commit-Position: refs/heads/master@{#438995}
(cherry picked from commit 6412949)

Review-Url: https://codereview.chromium.org/2581683006 .
Cr-Commit-Position: refs/branch-heads/2924@{#526}
Cr-Branched-From: 3a87aec-refs/heads/master@{#433059}
@burdges
Copy link

burdges commented Jan 5, 2017

There is an unacceptable risk to the user's privacy here @rsolomakhin so a browser that wishes to protect the user's privacy must prevent .canMakePayment() from returning any information.

There are two heavy handed ways browsers could do this, either disable the payments API all together, or disable all payment methods except one the Browser vender chooses, ala the Brave project's in-browser payments.

Instead, we should provide a less heavy handed route whereby a browser can effectively disable .canMakePayment() in some way so that many merchants still work. There are several options :

  • .canMakePayment() invokes .show() immediately.
  • .canMakePayment() always returns false, so merchants who call it cannot use the payment API.
  • .canMakePayment() always returns true, possibly resulting in customers seeing the wrong invocation.
  • .canMakePayment() throws an exception indicating that it detected an attack on the user by the page. This would happen if the page called .canMakePayment() too many times, but a more secure browser could simply do it anytime .canMakePayment() gets called. Ideally, the browser should warn the user that the page did something nefarious.

@domenic
Copy link
Collaborator Author

domenic commented Jan 5, 2017

@burdges I think several of your suggestions are already implemented by https://w3c.github.io/browser-payment-api/#canmakepayment-method :

  • step 3 allows returning a rejected promise with QuotaExceededError (your 4)
  • step 6.1, modulo some above-mentioned ambiguity in the definition of "supports", allows always returning false or true (your 2 and 3)

I don't think your 1 is a good idea, as asking if you can make a payment should not show a payment; that defeats the purpose of the API.

@burdges
Copy link

burdges commented Jan 5, 2017

I missed this conversation over "supports", sounds good.

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