-
Notifications
You must be signed in to change notification settings - Fork 23.1k
Update and clarify Permissions-Policy usage #42534
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
Conversation
|
Preview URLs (3 pages)
(comment last updated: 2026-01-26 20:47:11) |
|
@bban160 You're right in all your main points:
Two points thought:
So a suggestion.
Does that make sense? |
This is not actually correct. I was initially confused about how Permissions Policy propagates as well, but I now believe I understand it correctly after reading numerous specs over the past week and testing in Chromium and Firefox:
This is very unintuitive, and I do not believe it is documented in plain English on MDN or any other site at the moment. I would like to contribute this to MDN in the future, though I do not mind if someone else gets to it first. That tangent aside, the part that relates to this PR is that
You are right. I should have looked more closely at the framing of the article. This PR is not the proper venue for this discussion, but I believe the ideal solution would be to rewrite most of the pages concerning Permissions Policy to better reflect the actual standard. Which is to say, the HTTP response header and iframe With all that in mind, would you like me to still make the changes you suggested? My main goal with this PR is to be a (relatively) simple fix of the incorrect/misleading information. I noticed a few other pages have the exact same issue and plan to use this PR as a template to fix those, but not until receiving feedback on this first one. The broader overhaul is something I would like to do eventually but do not have the bandwidth to take on right now. It will also take time to complete and probably deserves more discussion of its own. |
|
Arrggg. After reading the spec yesterday I had come to the conclusion there was a different default for if the header wasn't specified (at all) vs the usual default allowlist that would apply if the header was sent but the specific feature was not set. Reading today I have no idea how I came to that conclusion. I think your summary matches the spec here: https://w3c.github.io/webappsec-permissions-policy/#default-allowlists - putting it down below to summarise my thoughts:
Hope that makes sense, and I haven't confused both of us again. |
This is not correct. If the default is
See above. If it worked this way, it would be an untenable situation in terms of website breakage. Before Permissions Policy existed, you only needed the
No worries, I fully sympathize with the unintuitiveness. If it helps, you can run |
|
Thanks very much @bban160 I'd been misreading the spec to mean that the parent must allow the feature for the origin of the child in order for the feature to be delegated. Once I understand that the parent can delgate the feature if it has it itself, this does indeed make sense - also hanks for the test method, which I used to verify all this. But I am still confused. Looking at the example as it is now, we have: Permissions-Policy: fullscreen=(self "https://example.com")From our conversation above, if Also in the this case you still need to specify In other words, what's the point of specifying anything other than * or self on the parent? |
|
@miketaylr You might want to be involved in this discussion. The assertion here is that:
|
|
I will circle back to this PR after finding a better way to illustrate the rules (probably a flowchart). Other MDN readers will benefit from this as well. |
So, if you have a header If you have no header at all, fullscreen can be delegated to frames from any origin. If you have a header |
My usecase for this is probably quite underrepresented on MDN. I host a lot of software, mostly web apps, written by other people. It's very easy for me to control HTTP headers in my reverse proxy, but modifying the HTML itself can be a significant maintenance burden. So it is important in my usecase to implement security hardening via headers, while modifying HTML is only done out of necessity. You can see specific examples of my methodology on my website (sorry for the shameless plug, but it really is relevant here). A notable example would be autofill, where I do not want to risk user PII being automatically delivered to a third-party frame without my explicit approval (remember: the HTML may not be written by me). I also use Content-Security-Policy to limit third-party frames, but that is beyond the scope of this discussion. |
|
@bban160 Ah, my test case on this was broken.
We do probably need a few more words of explanation in each case - i.e. in the second case, note that |
|
@hamishwillee Sure, I think we are on the same page now. I will finish this PR and some other small ones first, then tackle the main docs which will be a much larger PR. Since a few other pages are in the exact same situation as this page and need the exact same changes, should I integrate them into this PR? Or do a separate PR for each one? |
After you've done the main docs you may wish to cross link here, or reduce what you say here because it is all said elsewhere. For that reason I'd be tempted to do this first, then the main docs, then another new PR which does all the rest. That's just a suggestion though. If you want to do all of them now in one PR I won't be complaining - but it is likely you won't get this all reviewed for a bit in that case, as I'm on holiday for 2 weeks from Friday. |
|
Yes, the plan is to modify quite a few pages alongside the main docs update to reduce redundancy. Since we are on the same page now with this PR, I'll go ahead and integrate the remaining pages with the same changeset (they are so identical that reviewing one means all of them have been reviewed). |
Well yes, but more churn since they will all have to be reviewed twice if there is modification after the main docs are updated. |
|
I understand it will be more churn, but revising the main docs will take a lot longer (I have some unanswered questions I still need to research myself). Right now I am more concerned with fixing the incorrect and doing so quickly. The larger changes will follow once I finish researching and think about the best way to structure the docs. It's not ideal, but that's the best I can offer right now. |
fc2b9ae to
97939d4
Compare
|
@hamishwillee Ready for review. bluetooth, fullscreen, and geolocation are updated as we discussed. For compute-pressure, it's just a fix of the header syntax. It could have been a separate PR, but it's a tiny change and fits under this PR anyway. |
|
@chrisdavidmills I'm on holiday for two weeks. If you happen to have bandwidth for this then please do review. Otherwise I'll look when I return. @bban160 has a better understanding of this than me, though finally I do get it. |
chrisdavidmills
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there @bban160. I've stepped in as reviewer on this one, given that @hamishwillee is on vacation for a couple of weeks (lucky man!). This is looking pretty good, but I did have a few comments, which apply to all of the three main pages you've updated, given that the example is the same in each case.
Let me know what you think.
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/fullscreen/index.md
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/fullscreen/index.md
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/geolocation/index.md
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/geolocation/index.md
Show resolved
Hide resolved
|
@chrisdavidmills Should I split out the |
You can do it if you want, but I don't mind either way. I don't think it will take us more than a few days to resolve this review. |
|
@chrisdavidmills The compute-pressure change now lives in #42793 since I haven't had time to work on this yet. That PR however got assigned to hamishwillee, so I guess it might not be merged any sooner unless you want to take over that PR too. |
I've taken it over ;-) |
99cd516 to
7b36211
Compare
Thanks very much! Looks like you two are having fun :-) |
chrisdavidmills
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, @bban160! This is really close now; I've just got a couple of minor language suggestions for you to look at (the same two suggestions on each page).
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/bluetooth/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/fullscreen/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/fullscreen/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/geolocation/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/http/reference/headers/permissions-policy/geolocation/index.md
Outdated
Show resolved
Hide resolved
7b36211 to
d393519
Compare
chrisdavidmills
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, let's get this merged. Thanks again, @bban160, for all your work on this.
Description + Motivation
Regarding the Permissions-Policy: fullscreen directive page:
The "General example" is incomplete. Cross-origin iframes must have
allow="fullscreen"in addition to their origin being allowlisted in the Permissions-Policy HTTP response header.The "With an <iframe> element" example is invalid. The
Permissions-Policy: fullscreen=(self)HTTP response header prevents other origins from using fullscreen. The iframeallowattribute cannot grant fullscreen access because that would violate the most restrictive subset inheritance strategy.After correcting the above, it seemed appropriate to recategorize which one is the "general example" (more widely applicable) and which is the secondary example (more niche).
Additional details
I tested both Chromium and Firefox to confirm the updated examples are correct (aside from Firefox ignoring the
Permissions-Policyheader due to lack of support).