Cross-origin iframes and feature policy. #282
Conversation
Prohibit cross-origin iframes from installing payment handlers, unless allowed by the feature policy.
|
@marcoscaceres: Am I holding it right? |
Before this patch, any iframe could install a Payment Handler by calling `paymentManager.instruments.set()`. This patch checks for the feature policy "payment" before allowing any operations on `paymentManager.instruments`. After this patch, a cross-origin iframe will reject all operations on `paymentManager.instruments` by default. The parent context can explicitly allow the iframe to use the Payment Handler API through feature policy. This can be accomplished via the iframe attribute `allow="payment"`, for example. Note that the same feature policy controls access to Payment Request API as well. Discussion: w3c/payment-handler#281 Spec change: w3c/payment-handler#282 Payment Handlers are behind a flag: chrome://flags/#service-worker-payment-apps Manual test: https://rsolomakhin.github.io/pr/apps/iframe/ Bug: 828948 Change-Id: I0259555692fa0b215d3700c233b3687724e665cb Reviewed-on: https://chromium-review.googlesource.com/1005275 Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org> Reviewed-by: Ganggui Tang <gogerald@chromium.org> Cr-Commit-Position: refs/heads/master@{#550629}
index.html
Outdated
| Iframes | ||
| </h2> | ||
| <ul> | ||
| <li>Cross-origin iframes should not be able to register payment |
There was a problem hiding this comment.
"should not" doesn't read well here, and neither does "iframes". Perhaps just state that:
The top-level document needs to explicitly grant access to nested browsing contexts via the "payment" feature policy.
I've filed w3c/webappsec-permissions-policy#154 so we can actually link to "payment".
index.html
Outdated
| @@ -426,6 +426,10 @@ <h2> | |||
| <li>Return <var>p</var> and perform the remaining steps in | |||
| parallel: | |||
| </li> | |||
| <li>If the <a>document</a> is not <a>allowed to use</a> the | |||
There was a problem hiding this comment.
Nit: drop "policy-controlled-feature", it's redundant.
index.html
Outdated
| @@ -426,6 +426,10 @@ <h2> | |||
| <li>Return <var>p</var> and perform the remaining steps in | |||
| parallel: | |||
| </li> | |||
| <li>If the <a>document</a> is not <a>allowed to use</a> the | |||
| policy-controlled-feature <code>payment</code>, reject <var>p</var> | |||
| with <a>SecurityError</a>. | |||
There was a problem hiding this comment.
Here, you want:
"<a>SecurityError</a>" <a>DOMException</a>.You can take the dfns from payment request.
|
@marcoscaceres Please take another look! I've taken your suggestions. |
|
Generally, this looks good. Nit: I think you can probably drop all the "policy-controlled-feature"s throughout. |
|
@marcoscaceres: I've applied your fixes throughout. OK to merge? |
|
Hi @rsolomakhin , Are we ready to merge this pull request? Ian |
|
@ianbjacobs: I'm ready to go, just need a review from @marcoscaceres, I think. |
|
So, editorially it looks fine. Any plans to finish the checklist above before merging tho? I assume there are tests already. |
|
I wonder if we should get a second pair of eyes on the Feature Policy stuff before we merge? I don't know that spec very well, and having assurance from one of the editors of that spec that we've not missed anything might be good. Just a thought. |
|
I will work on more web platform tests eventually, but not this week. We can wait for more web platform tests from Chrome to land before merging this in. |
|
Now that PaymentInstruments API is removed, the only way to install a payment handler is during a transaction ("just-in-time") through |
Prohibit cross-origin iframes from installing payment handlers, unless
allowed by the feature policy.
closes #281
The following tasks have been completed:
Implementation commitment:
Preview | Diff