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

Send HTMLIFrameElement.allowPaymentRequest to HTML spec #311

Closed
marcoscaceres opened this issue Nov 16, 2016 · 46 comments
Closed

Send HTMLIFrameElement.allowPaymentRequest to HTML spec #311

marcoscaceres opened this issue Nov 16, 2016 · 46 comments

Comments

@marcoscaceres
Copy link
Member

I'm wondering what the rationale is for having an attribute on HTMLIFrameElement (.allowPaymentRequest), instead of using the sandbox attribute (i.e., sandbox='allow-payment-request')?

I can see there is precedence here with .allowFullScreen tho, but consistency would be good.

cc @mikewest @foolip

@foolip
Copy link
Member

foolip commented Nov 16, 2016

Looks like this was already implemented in Chromium: https://codereview.chromium.org/2394473002

It's pretty recent, so shouldn't have reached stable yet, but any change would have to be soon.

Does https://html.spec.whatwg.org/multipage/embedded-content.html#allowed-to-use do what you want for payments? In other words, do want payments to be allowed by default for top-level frames, disallowed by default for any descendant frames, with an attribute required for every hop along the way to opt in?

cc @annevk, and see whatwg/html#1492 for some extra history.

@rsolomakhin
Copy link
Collaborator

It should not go into sandbox, because sandbox is for blocking features that are allowed by default. For example, <iframe> allows form submission and script execution, but <iframe sandbox> does not. We want to disable PaymentRequest in <iframe> by default, which is more similar to how allowfullscreen operates.

@annevk
Copy link
Member

annevk commented Nov 16, 2016

If that's exactly what you want you should use the primitives from HTML and upstream the attribute to HTML. That's probably the simplest solution here.

w3c/mediacapture-main#268 had some discussion about going in a different direction for scenarios mostly like allowfullscreen, but then it got closed without the discussion being really resolved. Sigh.

@marcoscaceres
Copy link
Member Author

@rsolomakhin, good point.

Ok, we should mark this in the spec as a monkey patch and back port it to HTML as @annevk suggests.

So, the only annoying bit is that we have to rely on W3C HTML here because reasons... so need to figure out what the timing would be to get them to pick it up.

@marcoscaceres marcoscaceres changed the title Shouldn't HTMLIFrameElement. allowPaymentRequest go into "sandbox"? Send HTMLIFrameElement.allowPaymentRequest to HTML spec Nov 17, 2016
@halindrome
Copy link
Contributor

Can't we do it as an "extension" to HTML5? Actually, can't we just do this whole spec that way? That's what it is anyway. Then the HTML5 change comes for free.

@mikewest
Copy link
Member

In an ideal world, this would wait for Feature Policy to figure out how it wants to allow folks to enable features for child frames (@igrigorik, etc).

In the actual world, adding an attribute is ugly, but would let you hit your shipping targets. Have y'all worked out what origin you'll be showing in dialogs from cross-origin frames, and generally how you'll explain to users what's going on? We punted on those questions for credential management; they're hard. :(

As far as process goes, I don't think it's worth anyone's time to block on the W3C HTML spec. For webappsec specs, I've generally put together an "Integrations" section that links off to integration points in other specs (https://w3c.github.io/webappsec-csp/#html-integration, for example), upstreamed a patch to WHATWG's HTML (whatwg/html#1618, for example), and filed a bug against the W3C document along with a corresponding ISSUE in the spec text (w3c/html#547 and https://w3c.github.io/webappsec-csp/#issue-5faf83e6, for example). That seems like a reasonable workflow for documents you'd like to publish through the W3C for reasons.

/cc @adanilo

@rsolomakhin
Copy link
Collaborator

We are showing the title, icon, and origin of the top-level frame.

@mikewest
Copy link
Member

@rsolomakhin: Interesting. Do you account for the fact that frames are navigable cross-origin? That is, evil.com can do something like:

var target = document.createElement('iframe');
target.src = "https://amazing-store.com/";
target.onload = _ => {
  for (var i = 0; i < x.frames.length; i++) {
    target.frames[i].location.href = "https://evil.com/grab-credit-cards.html";
  }
};

@rsolomakhin
Copy link
Collaborator

This code would live on evil.com, so we would show the title, icon, and origin of evil.com. Perhaps my language was confusing, because "top-level frame" does not mean the top-most <iframe> element on the page. The term "top-level frame" refers to the HTML page that is not inside of any <iframe> element.

For example, suppose you load https://rsolomakhin.github.io/pr/iframe/ into your website by typing that URL into the address bar. The user agent will show the icon, title, and full URL (usually) of https://rsolomakhin.github.io/pr/iframe/. Here, https://rsolomakhin.github.io/pr/iframe/ is the "top-level frame", even though it's not inside of any <iframe> element.

@adrianhopebailie
Copy link
Collaborator

@rsolomakhin I'm a bit confused

Are you saying that you display the details of the context in which the API was called or the top-level context?

I.e. If merchant.com embeds an iframe from psp.com and the code that calls the payment request API runs inside psp.com do you display merchant.com or psp.com?

@rsolomakhin
Copy link
Collaborator

We display merchant.com in your example.

@adrianhopebailie
Copy link
Collaborator

We display merchant.com in your example.

I think that is @mikewest 's concern. The user isn't aware that they're actually transacting with psp.com. Is there a way to solve this elegantly?

@ianbjacobs
Copy link
Collaborator

@adrianhopebailie,

As a user, I would expect to see merchant.com. I would not expect to see the implementation details of how the merchant operates. I might in some scenario want to be able to dig deeper, but I think at the time of checkout I want to know that I'm in fact buying something from merchant.com.

@rsolomakhin
Copy link
Collaborator

Our reasoning for this is that user sees "merchant.com" in the address bar, regardless of any iframes the website might have embedded on their page. Therefore, we would like payments UI in Chrome to behave similarly. That being said, there's room for variation between user agents. For example, we have also considered showing something like "merchant.com via psp.com" in UI. We're not going down that path at the moment, but I can see other user agents doing this.

@igrigorik
Copy link
Member

In an ideal world, this would wait for Feature Policy to figure out how it wants to allow folks to enable features for child frames (@igrigorik, etc).

Big +1 to that. This is exactly the problem FP is aiming to solve.

Explainer: https://docs.google.com/document/d/1k0Ua-ZWlM_PsFCFdLMa8kaVTo32PeNZ4G7FFHqpFx4E/edit

@clelland and @raymeskhoury are landing patches as we speak. /cc @ojanvafai

@adrianhopebailie
Copy link
Collaborator

Have we considered the security implications of a page that is not in a secure context hosting the payment frame. I can't recall if this is addressed. Because, if I see merchant.com and the page is insecure and has been hijacked so that the iframe is from evil.com then neither the user or the merchant will be able to detect that.

@rsolomakhin
Copy link
Collaborator

In order for Payment Request to be available, the full chain of iframes and the top context need to be in secure context. Each iframe needs an 'allowpaymentrequest' attribute. You can experiment with this using Chrome Canary on Android. Open up http://rsolomakhin.github.io/pr/iframe/ and note that both iframes threw up SecurityErrors, even though both were embedded via https. Then open up https://rsolomakhin.github.io/pr/iframe/ and note that only the bottom iframe is throwing a SecurityError, because it's missing the 'allowpaymentrequest' attribute.

@clelland
Copy link

Nothing in Feature Policy is intended to allow the secure context requirement to be avoided -- we haven't called that out explicitly, but maybe we should. We do allow FP to be specified for some features which don't need HTTPS, but for those that do, we are still going to require a secure chain of frames, each specifying the correct policy.

The allowpaymentrequest iframe attribute is directly mappable to the enable="paymentrequest" attribute syntax, as it allows payment in the nested frame, and for the permission to be delegated further from that frame by including the same attribute in deeper frames.

@adrianhopebailie
Copy link
Collaborator

If it's SecureContext all the way down then I think we're safe 😄

Although, @ianbjacobs , speaking as a user I would want to know if merchant.com has delegated this to anyoneelse.com. I think @rsolomakhin 's suggestions are along the lines of what I was thinking but I guess it's up to the implementers to look after their users.

Another question is how permissions might work? Perhaps a new thread but @rsolomakhin do you envision a permission request user prompt per origin before granting access to the API and if so who would be granted permission in this case merchant.com or psp.com?

@rsolomakhin
Copy link
Collaborator

Do you mean a permission request akin to geolocation? We don't plan to use that for web payments, because geolocation permission allows polling user's location in background, which we do not want for payments. User should approve every transaction. There's only one approval screen, which shows the hostname of the top level context. This screen might also show the hostnames of the embedded iframes, but this security UX should be left to implementers to decide.

If any user agent implements "approve once, pay multiple times" behavior, I would imagine that they request the permission only once. The permission dialog would again state the top level context hostname, in all likelihood.

@adrianhopebailie
Copy link
Collaborator

Do you mean a permission request akin to geolocation? We don't plan to use that for web payments, because geolocation permission allows polling user's location in background, which we do not want for payments. User should approve every transaction.

👍

There's only one approval screen, which shows the hostname of the top level context. This screen might also show the hostnames of the embedded iframes, but this security UX should be left to implementers to decide.

👍

If any user agent implements "approve once, pay multiple times" behavior, I would imagine that they request the permission only once. The permission dialog would again state the top level context hostname, in all likelihood.

👍

I'd hope they only allow repetitive payments from that origin then and not from the origin of the sub-context. Not sure if we need to go into this detail?

@tommythorsen
Copy link
Member

@rsolomakhin and @adrianhopebailie:

Consider the case where you have a main page that is not a merchant, but for instance a news site. It allows its embedded advertisements (contained in iframes) to make use of Web Payments. So, for instance, while reading a sports article, you get an advertisement for a pair of running shoes, and can click a button in the advertisement to buy the shoes directly.

In this case, I don't think it makes sense to display the news site's origin as the owner of the payment request, as it is not that closely connected to the purchase. It makes much more sense to display the iframe's origin.

Opera leans towards displaying the origin of the frame that actually calls the Payment Request in the payment UI, as this was highlighted by our security team as important to protect against fraud. We don't want payment code that runs in an iframe to be able to pretend to the user that the payment belongs to the parent frame.

@ianbjacobs
Copy link
Collaborator

These are all helpful considerations. I am beginning to think that we should call out this topic in the Security Considerations section. In general I would leave the UX to implementers.

@raymeskhoury
Copy link

raymeskhoury commented Nov 22, 2016

@tommythorsen there are different perspectives on this topic. Even if we display the iframe origin, there's an assumption that the user reads it and understands that there is a different site involved, though I don't think that's a strong assumption to make. We've done some research on this topic.

We're heading toward a model in Chrome where the top level origin is the one displayed as this is the one the user understands themselves to be on. Displaying other origins doesn't necessarily help the user understand what's going on and instead can just add to confusion. The top level origin is the one responsible for delegating access to iframes.

With all that said, I share this purely to give a different perspective on that matter and totally support @ianbjacobs "In general I would leave the UX to implementers." :)

@adrianhopebailie
Copy link
Collaborator

Is there scope to differentiate between "delegating" and "allowing" payments in a child frame?

i.e. When you delegate, as in @tommythorsen 's use case, then the origin of the child frame is used and when you are allowing a child-frame to access the API it is for use cases where that frame is handling payments on behalf of the parent so the parent's origin is used.

@esprehn
Copy link

esprehn commented Nov 22, 2016

I'd prefer we punt this a release and wait on the feature policy or permission delegation features. Both are trying to solve this problem in a general way that doesn't keep up the trend of adding more properties to every iframe for each feature. If we ship this now we're going to be stuck with it (maybe forever) and the FP system is very close and being actively worked on. I'd much prefer we leave this out for now, if those features don't materialize quickly this is only a minor delay.

@foolip
Copy link
Member

foolip commented Nov 23, 2016

Yes, that's the implication, but I'd like to hear the pros and cons from those working on payments and feature policy in Chromium, there could be constraints here that rule out that idea.

@marcoscaceres
Copy link
Member Author

I can't find any plans for Gecko to support FP (in Bugzilla, at least). @igrigorik, have you spoken to any Moz folks about it?

@rsolomakhin
Copy link
Collaborator

Given these unresolved conversations, I am disabling allowpaymentrequest tag in Chrome for now. FP seems the best way forward.

@rsolomakhin
Copy link
Collaborator

@marcoscaceres and @adrianba: Can you find out if and when Firefox and Edge plan to implement FeaturePolicy? If the answer is "never" or "not any time soon", then we might need to keep HTMLIFrameElement.allowpaymentrequest until more browsers have support for FeaturePolicy.

@igrigorik
Copy link
Member

Feature-Policy v1 explainer + discussion: w3c/webappsec-permissions-policy#43

@marcoscaceres
Copy link
Member Author

@igrigorik, great to see Mozilla folks have provided feedback.

@rsolomakhin, I'm trying to get clarity on it.

@marcoscaceres
Copy link
Member Author

@rsolomakhin, unfortunately, Mozilla can't have Feature Policy implemented in the timeframe we've allocated to payments. Looks like we will have to stay with .allowPaymentRequest for now, but we should have a plan for Feature Policy to eventually take precedence.

@igrigorik, can we design Feature Policy such that it plays nice with .allowPaymentRequest and also with . allowFullScreen. Hopefully no further attributes will be needed on iframes.

@marcoscaceres
Copy link
Member Author

For those who haven't seen it, @domenic provided a bunch of guidance for how this should be specified here (it also points out a bunch of issues):
w3ctag/design-reviews#147 (comment)

@rsolomakhin
Copy link
Collaborator

Since Mozilla can't have Feature Policy implemented in the timeframe of web payments, I will remove allowPaymentRequest from behind a flag and ship it.

@ojanvafai
Copy link

@marcoscaceres what we were hoping to do in Chrome is to ship with enable="paymentrequest" in the next chrome release and then the full feature policy support in a following release. That way, we get something that's forward compatible, but really not much more work than the allowpaymentrequest attribute.

The one difference is that enable="paymentrequest" intentionally is only allowing paymentrequest for the origin in the src attribute, i.e. a redirect to a different origin wouldn't get access. We'll eventually need the additional featurepolicy attribute to allow all origins in the subframe, i.e. via featurepolicy='{"paymentrequest": ["*"]}'.

But it seems like v1 of payments could get away with just enable="paymentrequest" and we can avoid the proliferation of more allow* attributes. Does that sound OK?

@marcoscaceres
Copy link
Member Author

@ojanvafai, that sounds like it could work - and would allow Mozilla to incrementally add support for FP.

@annevk
Copy link
Member

annevk commented Nov 30, 2016

@ojanvafai I hate to bikeshed but if there's going to be another attribute called "featurepolicy" should we name this one "featureenable"? Or have something else that ties them together? The other problem with first shipping just enable="" is that it means something different if you also define the header. So we might end up in weird situations (i.e., compatibility issues) if Firefox only ships the attribute while Chrome goes ahead and also ships the header.

Also, do we know whether Edge and Safari are on board with feature policy?

@ojanvafai
Copy link

I chatted more with Chrome folks and want to propose the following:

  1. Rename the attributes to "allow" and "allowpolicy". The latter probably could use some bikeshedding, but I like "allow" because it matches the existing allowfullscreen, etc.
  2. For v1, just ship the "allow".
  3. That gives us some breathing room to hash out all the details of v2 that includes:
    -full policy attribute. Maybe we'd call this "allowpolicy", or maybe we just let "allow" accept either syntax?
    -response headers
    -"disallow" attribute that is the corollary to allow

The things we lose here are v1 will have no way to apply policies to:

  1. the top-level page.
  2. an subframe that redirects or otherwise navigates itself to a different origin. A navigation via the iframe's src attribute would work fine though.

I think both of those are totally fine to not have in v1. The important use cases are met and we have a path towards the full thing eventually that meets all the use cases.

@annevk I'm not sure I understand what you mean about the attribute meaning something different if you also supply the header. The header is an complementary way of setting a policy. If you supply both, then the feature would be enabled if you allowed it with either one. The header doesn't change what the attribute does though. Does that make sense?

Re: Edge/Safari: We've reached out to both of them, but haven't heard back. When we talked to Safari folks about the general idea of feature policy months ago, they said that the general idea of enabling/disabling features at the frame level made sense. We didn't have a concrete API proposal at the time though.

@zkoch
Copy link
Contributor

zkoch commented Dec 2, 2016

This topic came up on the WG call this week, and I think there was some skepticism about trying to optimize for FP this early, especially given that we don't yet have clear, broad vendor support for it.

We've chatted more on the Chrome side, and we're okay keeping support for "allowpaymentrequest" as an attribute on the iFrame. Our preference would be that it has the same semantics as a feature policy of {"paymentrequest": ["*"]}, which is how it's currently spec'd.

For future specs that are trying to do something similar (i.e. allow a feature within an iframe), we'll probably bring up this discussion again. 😉

It seems like there is still a desire to upstream the attribute to HTML as @annevk and @marcoscaceres suggest. I'm not quite sure what that means, so does someone else want to handle that? 😃

cc @adrianba

@sideshowbarker
Copy link
Contributor

It seems like there is still a desire to upstream the attribute to HTML as @annevk and @marcoscaceres suggest. I'm not quite sure what that means, so does someone else want to handle that?

I can volunteer to handle it (though also happy if somebody else manages to get to it before me).

sideshowbarker added a commit to whatwg/html that referenced this issue Dec 5, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

Closes w3c/payment-request#311
sideshowbarker added a commit that referenced this issue Dec 5, 2016
Following whatwg/html#2133, which adds the
definition of the <iframe> `allowpaymentrequest` attribute to the HTML
spec, this change updates the Payment Request API to reference HTML for
that definition of `allowpaymentrequest` from HTML.

See also #311
sideshowbarker added a commit to whatwg/html that referenced this issue Dec 5, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

Closes w3c/payment-request#311
See also w3c/payment-request#359
@sideshowbarker
Copy link
Contributor

For adding allowpaymentrequest to HTML: whatwg/html#2133

For updating Payment Request to reference HTML’s definition of allowpaymentrequest: #359

sideshowbarker added a commit to w3c/html that referenced this issue Dec 6, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
stevefaulkner pushed a commit to w3c/html that referenced this issue Dec 6, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
adrianba pushed a commit that referenced this issue Dec 7, 2016
Following whatwg/html#2133, which adds the
definition of the <iframe> `allowpaymentrequest` attribute to the HTML
spec, this change updates the Payment Request API to reference HTML for
that definition of `allowpaymentrequest` from HTML.

See also #311
zcorpan pushed a commit to whatwg/html that referenced this issue Dec 19, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See
w3c/payment-request#311
w3c/payment-request#359
arronei pushed a commit to arronei/html that referenced this issue Apr 17, 2017
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See
w3c/payment-request#311
w3c/payment-request#359
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