Add "has storage access" boolean to environment #10990
Conversation
|
To clarify, is this change integrating https://privacycg.github.io/storage-access/#ua-state into HTML? cc @cfredric If so, that seems fine by me, but I wonder if it should be part of a larger port of the storage access API (🍾 ?) |
|
@domenic would you be okay with this as an incremental step? |
|
Different Dom(e|i)nic here, but I'm curious about the "browser support". You marked all browsers as supportive in whatwg/fetch#1807, and I can't really imagine a browser being supportive of that overall effort but objecting on this PR's supplementary bit. I guess my general question is: are all browsers supportive of what exists in https://privacycg.github.io/storage-access/ (https://privacycg.github.io/storage-access/#ua-state specifically)? Second, should we expect another PR similar to this one to upstream the same bit for "source snapshot params"? What are the plans for that?
This is true from the perspective of the universe of WHATWG standards. But it'd be good to clarify in the commit message/OP that the purpose of this PR is to expose something that will be manipulated and set to true from other specifications, and just needs to appear here so that Fetch can reference it directly. |
|
All browsers are supportive. And yes, there would be further PRs to upstream the remainder of Storage Access. Pretty much all of Storage Access will end up in HTML. Good point on being super clear in the commit message. |
Got it. In that case, I am at least supportive of this. |
|
I'm a little unclear why this is being done in small pieces, and I worry that we'll get stuck halfway. And that the status quo will be confusing during the transition period. But, I'm happy to delegate this to other editors who are more involved in the storage access work. |
|
To be concrete about why I'm scared we might get stuck halfway:
Again, I'm happy to delegate this, but I just want to voice this concern. The alternative model would be having synchronized PRs ready across HTML, Fetch, and Storage Access API, all landable within the same day, with the Storage Access API PRs being removal PRs that remove anything that's now redundant with HTML + Fetch. |
|
To be clear, I think we should not land this before the other two HTML PRs are ready or the Fetch PR is ready to land. I think those together form an incremental step to a better cookie understanding. Landing each in isolation would be wrong. Potentially we could combine those three HTML PRs into one. Not sure what @bvandersloot-mozilla thinks about that. |
I agree with this entirely. Sorry if that wasn't clear @domenic.
I'm okay combining all of the HTML PRs into one if it helps with clarity. I had them all separate to make them easier on the Editors to reason about in isolation. Your wish is my rebase, just say the word. |
|
Ah cool, that sounds great! I'm glad to hear the plan is to land everything at once. I don't have an opinion on 1 PR vs. 3. |
|
+1 on that this is also my understanding :) |
This is a concept that originated in the Storage Access API, where it has been stuck because of spec issues between Fetch and 6265bis. To un-logjam this, I've started whatwg/fetch#1807. That depends on this bit existing. This patch adds the bit, which remains false, and does nothing.
|
I rebased this and addressed the formatting concerns. I also updated Domenic's comment above to point to a replacement PR for the one that was not maintained. |
| <dt>A <dfn data-x="concept-environment-has-storage-access" export for="environment">has storage | ||
| access</dfn></dt> | ||
| <dd><p>A boolean that indicates whether the environment has access to unpartitioned cookies. It | ||
| is initially false.</p></dd> |
There was a problem hiding this comment.
Can I ask why this is on environment instead of environment settings object? Fetch only seems to use it on request clients, which are ESOs.
There was a problem hiding this comment.
If I remember correctly, I put it on environment because Storage Access API's changes to set up a window environment settings object need to access the has storage access bool on reservedEnvironment which is an environment (rather than environment settings object).
There was a problem hiding this comment.
Ah, thanks, I only checked the Fetch situation, not the SAA situation.
However, I'm still confused, because I can't see any spec that accesses the reserved client's has storage access.
There was a problem hiding this comment.
I might be missing what you're saying, but the intent was for this part of SAA to set the reserved client's has storage access: https://privacycg.github.io/storage-access/#navigation:~:text=When%20creating%20request,the%20following%20hold%3A
There was a problem hiding this comment.
Right, I'm saying that there is a spec which sets the bit. But there is no spec that reads the bit. So setting the bit is just dead code, from what I can tell.
There was a problem hiding this comment.
As far as I can tell, the request's reserved client is assigned to the navigation params's reserved environment in the final step of create navigation params by fetching. That's called from step 4.2 of attempt to populate the history entry's document (as long as no other navigation params was provided). Step 5.6 of that algorithm calls load a document, which calls load an HTML document, which calls create and initialize a Document object. Step 7.10 of that algorithm is to set up the window environment settings object using the navigation params' reserved environment, which is the other algorithm that SAA modified to read the has storage access bit. So I think the plumbing from write-site to read-site is intact.
There was a problem hiding this comment.
I see. So it seems like this information is being used to populate from the fetching, to the new document creation.
Such information is best stored in navigation params, not on the client. The client represents a window or worker that needs to be shown to the service worker APIs. Whereas the navigation params represents information used to create new documents.
Furthermore, I think this means the current PR has a pretty bad a bug. Since you are putting this information on the client (which is an environment settings object), you are failing to capture this information for cases where you are navigating from the initial about:blank. Because navigations from the initial about:blank reuse the same Window/environment settings object as the initial about:blank page. It's only the Document that is new.
It seems to me that "has storage access" should live on document. And it can be put there by also tracking it in navigation params, to capture from source document -> new document. A good model to follow is policy container.
I wonder, actually, if "has storage access" should live in the policy container? The main impact of that would be to automatically propagate it correctly to tricky cases like about:blank, srcdoc, blob URL documents, data: URL documents, workers, etc. per https://html.spec.whatwg.org/#determining-navigation-params-policy-container and https://html.spec.whatwg.org/#initialize-worker-policy-container
What do you think?
There was a problem hiding this comment.
Using navigation params makes sense to me. I'm unsure about the practical impact of not propagating the bool from the initial about:blank page (is it possible for the has storage access bool to become true on the initial about:blank page?), but in principle SAA ought to use the right mechanisms for things, so updating that seems good.
I don't know enough about the differences between environment vs document lifetimes, nor what policy container is or how it could be used, so I'll go back to lurking and defer to the SAA editors to manage this PR.
| <dt>A <dfn data-x="concept-environment-has-storage-access" export for="environment">has storage | ||
| access</dfn></dt> |
There was a problem hiding this comment.
As the person who originally picked has storage access as the name for this bool, I'd now advocate for something like eligible for storage-access instead 😅
IMO, has storage access turned out to be a bit of a misnomer, since it's not authoritative for whether the environment really has access to cookies and other unpartitioned storage; the bool just indicates whether the environment has "opted in" or not. The UA's cookie store still has the ability to deny access due to user settings, for example. (See https://privacycg.github.io/storage-access/#cookies.)
Naming this eligible for storage-access would match the corresponding field on request, which conveys the same opt-in signal and is initialized based on environment's has storage access bool.
(We could also reuse the storage access eligibility enum instead of a bool, which was Anne's suggestion when I added the field to request.)
There was a problem hiding this comment.
Hmm, while I think for requests it's accurate to say they are eligible, as other factors are in play, that's less clear to me for environments. They have the storage access bit.
What end user setting would come into play after they get the bit, but wouldn't have come into play while obtaining the bit? If anything it seems like it should have come into play when obtaining the bit.
There was a problem hiding this comment.
The UA should check end user settings both when setting the bit, and when "using" the bit. Failure to check the settings when using the bit is a TOCTOU bug, since the end user might have changed the settings after the bit was set.
There was a problem hiding this comment.
Check-to-use within the same page lifetime seems very fair and like it could cause unexpected bugs in websites if their cookie access flip-flops while a user toggles a checkbox in a browser settings tab.
There was a problem hiding this comment.
Hmm, I think the bug would be not reloading the website when such consequential choices are made.
There was a problem hiding this comment.
Forcing a reload when settings change is one solution; re-checking settings at time-of-use is another option that doesn't require reloads.
Do implementations today reload the affected pages when end user settings are changed?
There was a problem hiding this comment.
I'm not aware of implementations reloading, but they should. Re-checking at time-of-use is not an actual solution as the website already had access to the relevant state before the end user changed the setting. Without reload the end user will either make the website non-functional or think they are preventing something that they're actually not.
There was a problem hiding this comment.
Strictly speaking, simply reloading the page isn't a full solution either because sites can store identifiers in partitioned storage. Among other things, the UA would really have to unload all documents, clear all data for all sites (both partitioned and unpartitioned), and then let the user resume browsing.
But that would be disruptive and annoying to users, and would be a powerful incentive against changing the UA's cookie settings to make them more private. I'd rather avoid that disincentive, since it doesn't seem like a practical way to increase user privacy, IMO. My intuition is that we can get most of the privacy benefits (without most of the annoyance) by simply blocking cookies when user settings say to.
But either way, we're getting into product-level settings behavior decisions now, which aren't standardized; and as far as standards go, you're the editors of the SAA spec and HTML standard. If you prefer to keep the old has storage access name, that's fine by me.
There was a problem hiding this comment.
Firefox currently reloads along the most common UX path users disable third-party cookie blocking.
There was a problem hiding this comment.
"They should reload" feels like a weak argument to me when it's not normatively required. I'm pretty sure that no user agent (including Firefox, IIRC) forces a reload in all cases where cookie settings or permissions change.
With that said, this feels like the bikesheddiest conversation I've seen in a long time, and I'm glad to support either of these two names :P
This is a concept that originated in the Storage Access API, where it has been stuck because of spec issues between Fetch and 6265bis. To un-logjam this, I've started whatwg/fetch#1807. That depends on this bit existing.
This patch adds the bit, which remains false, and does nothing.
(See WHATWG Working Mode: Changes for more details.)
/webappapis.html ( diff )