Skip to content
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

Permissions Policy Integration (formalize nested iframe support) #78

Merged
merged 3 commits into from
Jun 9, 2021

Conversation

johannhof
Copy link
Member

This is a first stab at integrating permissions policy and support nested iframes in the spec, see #10 and #12 . A few notes:

  • I'm happy to bikeshed on the permission name ("request-storage-access"). It's important to note that this policy only controls requesting storage access, it does not tell user agents with persistent/passive storage (see Active or passive storage access after explicit user opt-in #2) how to behave if an allow=none attribute was added after an iframe received storage access.
  • We're now using the "*" default allowlist which @annevk intended to deprecate. We went back and fort on this but ultimately "*" captures the reality of current implementations best, especially considering that WebKit does not have PP support (and thus implictly default to "*") for the time being. If we had started from scratch on this then maybe "self" would have been the best option, but personally I don't see that happening without WebKit support. Let me know if anyone disagrees.
  • If I understand correctly this would also allow sites to use the PP header to globally reject all rSA calls, which is fixing FeaturePolicy type header so top level site can ask for maximum privacy implementation #56

@johannhof johannhof requested review from hober and annevk May 5, 2021 15:44
@@ -202,7 +200,7 @@ When invoked on {{Document}} |doc|, the <dfn export method for=Document><code>re
1. Let |p| be [=a new promise=].
1. If this algorithm was invoked when |doc|'s {{Window}} object did not have [=transient activation=], [=reject=] and return |p|.
1. If |doc|'s [=Document/browsing context=] is a [=top-level browsing context=], [=/resolve=] and return |p|.
1. If |doc|'s [=Document/browsing context=]'s [=parent browsing context=] is not a [=top-level browsing context=], [=reject=] and return |p|.
1. If |doc|'s [=relevant settings object=] is not [=allowed to use=] the `"request-storage-access"` permission, [=reject=] and return |p|.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"allowed to use" operates on a document I believe so you need to grab a document from the environment settings object here.

Copy link
Collaborator

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reluctantly approving.

@clelland do we keep a registry of these values someplace? Any thoughts from you on this?

storage-access.bs Outdated Show resolved Hide resolved
@johannhof
Copy link
Member Author

@hober @johnwilander I'll merge this, let me know if you still have any concerns and we can figure those out in follow-ups

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants