WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
265130
New allow="payment" attribute does not work in nested iFrame lacking the src attribute
https://bugs.webkit.org/show_bug.cgi?id=265130
Summary
New allow="payment" attribute does not work in nested iFrame lacking the src ...
RLambert
Reported
2023-11-20 06:14:21 PST
As of Safari 17 (Release Notes [
https://developer.apple.com/documentation/safari-release-notes/safari-17-release-notes#Apple-Pay
]), ApplePay is supported within cross-origin iframes with the allow="payment" attribute. This attribute enables apple pay even when nesting iframes, so long as the each frame in the chain has this same allow="payment" attribute. Unfortunately, the chain appears to break when a parent iframe lacks a src attribute. I believe this is because the rule also requires a secure https host. I noticed this behavior when testing the new allow="payment" attribute within a Google Ad which nests content within an iframe like this example: <iframe id="google_ads_iframe_/6782/GenAptTherapy/Post/Right_Rail/Right_Rail_001_0" name="google_ads_iframe_/6782/GenAptTherapy/Post/Right_Rail/Right_Rail_001_0" title="3rd party ad content" width="300" height="600" scrolling="no" marginwidth="0" marginheight="0" frameborder="0" role="region" aria-label="Advertisement" tabindex="0" allow="payment; attribution-reporting" style="border: 0px; vertical-align: bottom;" data-load-complete="true" data-google-container-id="2"> // #document // ... our iframe containing allow="payment" attribute <iframe allow="payment src="
https://example.com
" /> </iframe> Notice how the parent lacks a "src" attribute. It works fine when a src attribute is applied to the parent. In the console, I see complaints about the request to Apple Pay failing and a complaint about the parent frame not being secure (because it lacks a src attribute). My proposal is to allow the payment attribute when the parent frame lacks the src attribute.
Attachments
Diff to address bug 265130
(2.28 KB, patch)
2023-11-29 16:44 PST
,
Abrar Rahman Protyasha
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2023-11-27 06:15:14 PST
<
rdar://problem/118831152
>
Abrar Rahman Protyasha
Comment 2
2023-11-28 12:29:28 PST
Thanks for the report, RLambert! I too suspect that we trip here because we can't assert that some ancestor frame is secure. As a workaround (and because you already have a reproducing case), can you try adding `src=about:blank` to the iframes? I believe that allows it to inherit the origin of the containing document. I suppose we can replace HTTPS protocol check in PaymentSession like so: ``` if (!documentLoader.response().url().protocolIs("https"_s) && !documentLoader.response().url() == aboutBlankURL()) ``` but this weakens our security measures, so I don't think this is the right move.
Abrar Rahman Protyasha
Comment 3
2023-11-28 12:30:49 PST
(In reply to Abrar Rahman Protyasha from
comment #2
)
> ``` > if (!documentLoader.response().url().protocolIs("https"_s) && > !documentLoader.response().url() == aboutBlankURL()) > ```
Pardon the typos in the snippet.
Abrar Rahman Protyasha
Comment 4
2023-11-28 12:31:08 PST
(In reply to Abrar Rahman Protyasha from
comment #2
)
> ``` > if (!documentLoader.response().url().protocolIs("https"_s) && > !documentLoader.response().url() == aboutBlankURL()) > ```
Pardon the typos in the snippet.
RLambert
Comment 5
2023-11-28 12:44:44 PST
(In reply to Abrar Rahman Protyasha from
comment #2
)
> Thanks for the report, RLambert! I too suspect that we trip here because we > can't assert that some ancestor frame is secure. As a workaround (and > because you already have a reproducing case), can you try adding > `src=about:blank` to the iframes? I believe that allows it to inherit the > origin of the containing document. > > I suppose we can replace HTTPS protocol check in PaymentSession like so: > > ``` > if (!documentLoader.response().url().protocolIs("https"_s) && > !documentLoader.response().url() == aboutBlankURL()) > ``` > > but this weakens our security measures, so I don't think this is the right > move.
Thank you for the feedback. I did try to use the about:blank workaround on an HTTPS page as you suggested but that complained about a lack of secure origin since about:blank is not HTTPS. Happy to try any other solution, but about:blank doesn't seem to work in current state.
Abrar Rahman Protyasha
Comment 6
2023-11-28 12:54:30 PST
(In reply to RLambert from
comment #5
)
> Happy to try any other solution, but about:blank doesn't seem to work in > current state.
Ok, that's a good data point. (no other thoughts at this moment)
Abrar Rahman Protyasha
Comment 7
2023-11-29 16:44:31 PST
Created
attachment 468812
[details]
Diff to address
bug 265130
So, I think we can consider the frame secure if its owner is secure, so we should be consulting `SecurityPolicy::shouldInheritSecurityOriginFromOwner` here. A diff like this will probably work, but I haven't introduced any testing for this.
RLambert
Comment 8
2023-11-30 05:47:32 PST
(In reply to Abrar Rahman Protyasha from
comment #7
)
> Created
attachment 468812
[details]
> Diff to address
bug 265130
> > So, I think we can consider the frame secure if its owner is secure, so we > should be consulting `SecurityPolicy::shouldInheritSecurityOriginFromOwner` > here. A diff like this will probably work, but I haven't introduced any > testing for this.
Yeah I agree, matching the content policy of the parent seems like a nice solution so long as the child has no `src` attribute Happy to help test it when there is a version available with those changes.
RLambert
Comment 9
2023-12-05 07:56:10 PST
(In reply to Abrar Rahman Protyasha from
comment #7
)
> Created
attachment 468812
[details]
> Diff to address
bug 265130
> > So, I think we can consider the frame secure if its owner is secure, so we > should be consulting `SecurityPolicy::shouldInheritSecurityOriginFromOwner` > here. A diff like this will probably work, but I haven't introduced any > testing for this.
@Abrar is there a PR on
https://github.com/WebKit/WebKit
associated with this?
Abrar Rahman Protyasha
Comment 10
2023-12-05 22:46:06 PST
No, sorry, not yet. I haven't had time to address the bug. I plan to get to this soon though.
Abrar Rahman Protyasha
Comment 11
2023-12-05 22:51:52 PST
Clarification in case the attached diff muddied the meaning of "address the bug" -- by that I meant verifying the fix does what it does, introducing test coverage, etc.
RLambert
Comment 12
2023-12-15 11:56:47 PST
(In reply to Abrar Rahman Protyasha from
comment #10
)
> No, sorry, not yet. I haven't had time to address the bug. I plan to get to > this soon though.
Friendly nudge / reminder on this one - would be an awesome feature I feel and unblock some things on our roadmap. Really appreciate it!
Anthony
Comment 13
2024-01-18 13:24:13 PST
I'm facing a similar issue. Has there been any updates @Abrar Rahman Protyahsa?
Andrew Schneider
Comment 14
2024-01-23 12:16:01 PST
This is also directly impacting potential sales for my small company. Please can this fix be prioritized. Thanks so much!
Allyson
Comment 15
2024-02-20 11:17:18 PST
I'm facing the same issue as well. Any updates here? Thanks for the help!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug