Bug 265130 - New allow="payment" attribute does not work in nested iFrame lacking the src attribute
Summary: New allow="payment" attribute does not work in nested iFrame lacking the src ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: Safari 17
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-11-20 06:14 PST by RLambert
Modified: 2024-02-20 11:17 PST (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description RLambert 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.
Comment 1 Radar WebKit Bug Importer 2023-11-27 06:15:14 PST
<rdar://problem/118831152>
Comment 2 Abrar Rahman Protyasha 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.
Comment 3 Abrar Rahman Protyasha 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.
Comment 4 Abrar Rahman Protyasha 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.
Comment 5 RLambert 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.
Comment 6 Abrar Rahman Protyasha 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)
Comment 7 Abrar Rahman Protyasha 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.
Comment 8 RLambert 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.
Comment 9 RLambert 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?
Comment 10 Abrar Rahman Protyasha 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.
Comment 11 Abrar Rahman Protyasha 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.
Comment 12 RLambert 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!
Comment 13 Anthony 2024-01-18 13:24:13 PST
I'm facing a similar issue. Has there been any updates @Abrar Rahman Protyahsa?
Comment 14 Andrew Schneider 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!
Comment 15 Allyson 2024-02-20 11:17:18 PST
I'm facing the same issue as well. Any updates here? Thanks for the help!