privacycg / storage-access

The Storage Access API
https://privacycg.github.io/storage-access/
212 stars 27 forks source link

Simplify WebDriver API for blocking cross-site cookies #162

Open johannhof opened 1 year ago

johannhof commented 1 year ago

We currently have a testdriver.set_storage_access API specified in the SAA spec that can be used to selectively enable cross-site cookie blocking for a given origin. Somewhat confusingly, this doesn't give out an SAA grant or permission, but instead is intended to change the underlying browser mechanics of cross-site cookie blocking to allow- or deny-list that origin.

This idea has never had many supporters outside of Chromium and other browser vendors have not implemented the API so far. We've recently discovered some issues with the API in Chromium requiring a major refactor to keep it working as intended, and are now also not sure we like the API in its current form.

The API was so far not used for exempting origins from cross-site cookie blocking, but only for selectively denying test origins access to cross-site cookies in Chromium, which does not block cross-site cookies by default. As such, having a way to enforce cross-site cookie blocking for a single test seems to be the only functionality we really need.

@annevk had previously expressed concerns that such an API would only be useful to browsers that do not block cross-site cookies by default. I think that's a fair point, which is why I want to emphasize two things in this proposed redesign:

Anne's original suggestion was to have a Chrome-only configuration file somewhere, but having discussed this with @foolip and @jgraham it seems much more difficult to achieve such a setup in a way that would give us the same results as this WebDriver API. As such, I think it's preferable to update and simplify the existing WebDriver API instead.

Proposal

My rough idea is an interface like this:

testdriver.require_cross_site_cookie_blocking();

which would communicate to a user agent that the test being run is incompatible with an environment where cross-site cookies are allowed.

And the "cleanup" variant of that is:

testdriver.reset_cross_site_cookie_blocking();

(names up for bikeshedding), which would reset the blocking behavior to the UA default.

I'd love to get thoughts from @annevk @bvandersloot-mozilla and @cfredric. If this seems like an acceptable compromise to you, I'm happy to start writing up the spec parts of it.

cfredric commented 1 year ago

I'm supportive of this, probably unsurprisingly. I like that the API design gives no indication that "allow third-party cookies" is a supported state, since all major browsers are moving toward blocking third-party cookie access by default.

annevk commented 1 year ago

Could you elaborate on the issue with Chromium running all tests as if cross-site cookies are blocked?

With this proposal we might end up with a number of tests that work in all browsers (and that also depends on how these methods function in non-supporting user agents), but we will still have a large number of "broken" tests.

johannhof commented 1 year ago

Right, eventually we'd like to have Chromium run all tests with cross-site cookies blocked. However, that is a much larger effort both technically and (especially) organizationally, and it's not even clear whether this is something our team would be driving.

In the meantime, to make iterative improvements and to support the effort of increasing the number of tests without cross-site cookies enabled, having this capability would be helpful.

johannhof commented 1 year ago

We discussed this at the editor's meeting and ended up re-iterating on two things that we'd like to ensure when committing to this API:

I'll try to prototype this in Chrome to see if this is achievable with the proposed model and report back for follow-up discussion if not.

gsnedders commented 1 year ago

Would testdriver.require_cross_site_cookie_blocking set a flag that persists across navigations? What if the browser crashes and the cleanup function never gets called?

johannhof commented 1 year ago

Would testdriver.require_cross_site_cookie_blocking set a flag that persists across navigations?

I guess I was thinking that it could turn on cookie blocking globally, could that be an issue with tests running in parallel? Otherwise we might be able to store some extra information on the browsing context (or what the new term is) to ensure that it is sustained across navigations.

What if the browser crashes and the cleanup function never gets called?

I don't think this setting should persist across restarts.

gsnedders commented 1 year ago

This sounds like something that the test runner itself should own, rather than it being controlled via JS. wptrunner and (I believe) Blink's run_web_tests.py have ways to set preferences for certain groups of tests, and this doesn't seem meaningfully different from that. Which seems to come back to:

Anne's original suggestion was to have a Chrome-only configuration file somewhere, but having discussed this with @foolip and @jgraham it seems much more difficult to achieve such a setup in a way that would give us the same results as this WebDriver API. As such, I think it's preferable to update and simplify the existing WebDriver API instead.

IMO, this probably makes more sense to have as a thread on https://github.com/web-platform-tests/rfcs, rather than here, because it sounds like there's prior discussion (not linked) that I don't know the context of, and it really is a question of how we change the semantics of the test runner and less a CG matter.