Bug 229406 – Add "payment" permissions policy

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Marcos Caceres]

The Payment Request API [1] defines defines a policy-controlled feature identified by the string "payment" [permissions-policy]. Its default allowlist is 'self'.

However, WebKit currently doesn't include "payment" as policy controlled feature. 


[1] https://www.w3.org/TR/payment-request/#permissions-policy

[Marcos Caceres]

Created attachment 436180 [details]
Patch

[Marcos Caceres]

Lacking tests right now, for reasons we discussed on Slack... with a little guidance, I should be able to add them.

[youenn fablet]

We have some WebKit specific tests, for instance LayoutTests/http/tests/media/media-stream/enumerate-devices-iframe-allow-attribute.html.
We also have WPT LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-default-feature-policy.https.html for instance.

[Marcos Caceres]

(In reply to youenn fablet from comment #3)
> We have some WebKit specific tests, for instance
> LayoutTests/http/tests/media/media-stream/enumerate-devices-iframe-allow-
> attribute.html.
> We also have WPT
> LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-
> default-feature-policy.https.html for instance.

Thanks Youenn! I'll create some tests off those.

[Devin Rousso]

Comment on attachment 436180 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436180&action=review

> Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp:294
> +    if (!isFeaturePolicyAllowedByDocumentAndAllOwners(FeaturePolicy::Type::Payment, document, LogFeaturePolicyFailure::Yes))

Rather than doing this here, I wonder if this should be moved to `PaymentSession::canCreateSession` so that stuff like `ApplePaySession.canMakePayments` would also throw unless the `"payment"` feature policy is allowed.  This would also cover someone trying to use the Apple Pay JS API <https://developer.apple.com/documentation/apple_pay_on_the_web/apple_pay_js_api>, which I'd imagine should also be covered by this.

[Sam Sneddon [:gsnedders]]

is this just a dupe of Bug 167417? though given work has been happening here, maybe we should just forward-dupe that to here

[Marcos Caceres]

(In reply to Sam Sneddon [:gsnedders] from comment #6)
> is this just a dupe of Bug 167417? 

Yes, indeed. Although the scope of this one is a little bigger (it's the iframe and respecting the http header).  

> though given work has been happening
> here, maybe we should just forward-dupe that to here

Yes, that be great.

[Radar WebKit Bug Importer]

<rdar://problem/82518272>

[Marcos Caceres]

Created attachment 437018 [details]
Patch

[Sam Sneddon [:gsnedders]]

*** Bug 167417 has been marked as a duplicate of this bug. ***

[Devin Rousso]

Comment on attachment 437018 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437018&action=review

r=me

> Source/WebCore/Modules/applepay/PaymentSession.cpp:54
> +        return Exception { SecurityError, "Payment Request API is disabled Permissions Policy."_s };

Can we make this error into an actual sentence?  Maybe `"Third-party iframes are not allowed to request payments unless explicitly allowed via Feature-Policy (payment)"`?

> Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp:34
> +#include "FeaturePolicy.h"

NIT: I don't think this is needed anymore

[Marcos Caceres]

Created attachment 437065 [details]
Patch

[EWS]

Committed r281885 (241215@main): <https://commits.webkit.org/241215@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437065 [details].

[Brad]

Hi glad to see progress here! This doesn't seem to resolve this duplicate issue: https://bugs.webkit.org/show_bug.cgi?id=226345.

The issue is that https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/Modules/applepay/PaymentSession.cpp?rev=281885#L66 still requires the origins of ancestors frames to match. This seems reasonable to remove now that payments policy is enforced (and is not available to third parties by default).

[Marcos Caceres]

(In reply to Brad from comment #14)
> Hi glad to see progress here! This doesn't seem to resolve this duplicate
> issue: https://bugs.webkit.org/show_bug.cgi?id=226345.
> 
> The issue is that
> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/Modules/applepay/
> PaymentSession.cpp?rev=281885#L66 still requires the origins of ancestors
> frames to match. This seems reasonable to remove now that payments policy is
> enforced (and is not available to third parties by default).

Yes, that's definitely true. I'm guessing that's a product decision and not a bug? 

It would be great to get input from Apple Pay folks on the above, as there may be technical or security issues around the same origin enforcement.

[Brad]

Sure is there an update from them? I reopened the other ticket.

From the developer perspective, it makes it difficult to build a safe Apple Pay integration when using third party vendors, which need to be trusted to process payments but not to receive access to sensitive user data and cookies on the top level origin.

This top level origin restriction made sense from a security perspective before the "payment" permissions policy was implemented, to prevent arbitrary iframes from asking for payments that the top origin didn't intend. But now that this attribute works and is default denied, it seems safe to allow top origins to delegate the ability to their own payment vendors in specific iframes.