-
Notifications
You must be signed in to change notification settings - Fork 27
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
Forward Declared API Addition #100
Conversation
storage-access.bs
Outdated
|
||
In the new [=window=], Alex selects the account to log in with. In response, `https://identity.example` sets a cookie, which by virtue of its [=first-party-site context=] is in its [=unpartitioned data=] and calls `{{Document.requestStorageAccessUnderSite("https://site.example")}}`. Once the {{Promise}} returned by `{{Document/requestStorageAccessUnderSite()}}` resolves, the `https://identity.example` [=window=] closes itself, returning Alex to `https://site.example`. | ||
|
||
When Alex returns to `https://site.example`, the {{Promise}} returned by `{{Document/allowStorageAccessRequestOnSite()}}` has resolved or will resolve shortly. At this point, requests to `https://idnetity.example` will bear the cookie set in the previous [=window=]. |
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.
To be clear, is this saying allowStorageAccessRequestOnSite
would remaining pending until requestStorageAccessUnderSite
is invoked? Note that while authentication flows can happen in popups, they could be done via a full-page redirect.
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.
That was the intent. Given that, I think it would be best to have that Promise resolve once the internal state is set to allow the requestStorageAccessUnderSite
call to succeed.
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.
Right, I don't fully understand why the Promise needs to be left dangling on site.example. What was the reason for that?
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.
I wanted a means to communicate the timing of unpartitioned storage availability in the https://site.example
page. How can an iframe that is waiting for a separate window to grant it unpartitioned storage know when it has that available? I think the answer right now is polling. Perhaps we want a new function, e.g. waitForStorageAccess()
?
Regardless, I think that this promise should resolve once the intemediate state of "ready for a requestStorageAccessUnderSite
call" is set.
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.
Yup, this is #55 and I agree it gets even more interesting with the popup use case.
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.
Thank you for picking this up, @bvandersloot-mozilla! I had a few thoughts/questions.
storage-access.bs
Outdated
|
||
In the new [=window=], Alex selects the account to log in with. In response, `https://identity.example` sets a cookie, which by virtue of its [=first-party-site context=] is in its [=unpartitioned data=] and calls `{{Document.requestStorageAccessUnderSite("https://site.example")}}`. Once the {{Promise}} returned by `{{Document/requestStorageAccessUnderSite()}}` resolves, the `https://identity.example` [=window=] closes itself, returning Alex to `https://site.example`. | ||
|
||
When Alex returns to `https://site.example`, the {{Promise}} returned by `{{Document/allowStorageAccessRequestOnSite()}}` has resolved or will resolve shortly. At this point, requests to `https://idnetity.example` will bear the cookie set in the previous [=window=]. |
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.
Right, I don't fully understand why the Promise needs to be left dangling on site.example. What was the reason for that?
@@ -144,6 +156,8 @@ A <dfn>storage access flag set</dfn> is a set of zero or more of the following f | |||
:: When set, this flag indicates |embedded site| has access to its [=unpartitioned data=] when it's loaded in a [=third party context=] on |top-level site|. | |||
: The <dfn for="storage access flag set" id=was-expressly-denied-storage-access-flag>was expressly denied storage access flag</dfn> | |||
:: When set, this flag indicates that the user expressly denied |embedded site| access to its [=unpartitioned data=] when it's loaded in a [=third party context=] on |top-level site|. | |||
: The <dfn for="has allow request storage access flag set" id=has-allow-request-storage-access-flag>has allow request storage access flag</dfn> | |||
:: When set, this flag indicates |top-level site| is allowing |embedded site| to request access to its [=unpartitioned data=] on |top-level site| from the |embedded site| [=first-party-site context=]. |
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.
I wonder how folks more familiar with spec syntax like @hober or @annevk feel about our usage of "site" in this PR and in SAA overall, and whether it's clear enough what's being referred to. What we really mean is "documents of embedded site" (in a first-party-site context), so maybe we need to introduce this definition separately.
…rought up by @jasonnutter & @johannhof Also fix some typos
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.
Made a number of requested changes.
@johannhof: I believe this is ready for a second look. |
I'm getting into implementing this, and think there may be an arbitrary choice we made that hinders implementation and support of the pop-up case. I'd like to make a change that redefines the order of the flow. It would be easier for the Embedding site to act after the Embedded site, rather than before. This would mean the example would be the following: Alex visits Once navigated, Alex selects the account to log in with. In response, When Alex returns to I don't think this affects use-cases or the privacy properties. |
@bvandersloot-mozilla I think that should be fine. Is the thinking here that by reversing the order, the embedding site would know immediately that when it invokes Excited to see this implemented! |
It's actually dual purpose. There is the ergonomic aspect you identified. There is also an engineering aspect that calling the function that actually grants the permission from inside |
@bvandersloot-mozilla Ah makes sense, sounds good to me! |
Would the above also work in either of these scenarios:
In other words, would the case where a 1p child window (instead of a redirect) is used work and can that child window be redirected to other sites for arbitrary user interactions, prior to closing -- and still result in storage access being granted to embedded |
Absolutely. The time limit floated right now is ~15 minutes, but allows for arbitrary interaction in between and there is no restriction on the paired calls being in the same window. |
Hi @bvandersloot-mozilla, sorry for the delay. I spoke to @johnwilander about this and while as editors we're generally appreciative of this effort we would like to wait with merging/review until there's an initial implementation (e.g. in Firefox) and some of the details have been figured out. Again, thank you for working on this and I'll try to be more responsive here going forward. Feel free to update this PR along the way as you've been doing right now, I think that's a good format for documenting potential changes to the original proposal. |
I also want to note that depending on the timeline we may want to try to standardize parts of the spec (e.g. through a PR to HTML) prior to merging this PR for simplicity, but I think we can figure that out as we progress :) |
Firefox Nightly has an initial implementation gated off behind the preference |
@bvandersloot-mozilla Fantastic, thanks! Will try it out and let you know if how it goes. |
After discussions at TPAC I'm comfortable closing this in favor of #107 or a variation thereof. Given Firefox's interest in FedCM, the focus of this design having been for login scenarios, and the better generality of #107, I feel that is a better angle to take. We can revisit this as a variation that provides stronger prompt-spam guarantees if we get stuck on prompt spam concerns, specifically with an eye for browsers with fewer implicit grant heuristics, e.g. Safari. |
Building upon this explainer and the discussion in #83 , this is a rough draft of a proposal for a forward declared API.
Looking for thoughts, comments, and potential improvements.
Especially rough is how to handle timing between the first and third party pages. I use the language "wait" and "notify" which is not as precise as the rest of the language in the spec, but gets the point across to get discussion rolling.
Preview | Diff