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

Simplify WebDriver API for blocking cross-site cookies #162

Open
johannhof opened this issue Feb 1, 2023 · 9 comments
Open

Simplify WebDriver API for blocking cross-site cookies #162

johannhof opened this issue Feb 1, 2023 · 9 comments
Assignees
Labels
resolve before graduation These issues need to be resolved before the spec graduates from the CG

Comments

@johannhof
Copy link
Member

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:

  • This is an intentional stopgap to ensure that Chrome/Edge/Others can maintain interop with Firefox and Safari during their transition to blocking cross-site cookies.
  • The API should be designed to prevent writing tests that are incompatible with browsers which are already blocking cross-site cookies by default but can't support the API for whatever reason. It should behave in a way that strictly encourages adoption of cross-site cookie blocking.

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.

@johannhof johannhof added the resolve before graduation These issues need to be resolved before the spec graduates from the CG label Feb 1, 2023
@johannhof johannhof self-assigned this Feb 1, 2023
@cfredric
Copy link
Contributor

cfredric commented Feb 6, 2023

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
Copy link
Collaborator

annevk commented Feb 6, 2023

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
Copy link
Member Author

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
Copy link
Member Author

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:

  • We should make it as clear as possible that it's only meant for browsers that have not yet deprecated cross-site cookies. In comments, naming and semantics it should be clear that this will have no effect in browsers with blocked cross-site cookies by default.
  • The API should ideally just function (i.e. do nothing) out of the box in those browsers, without requiring extra work from their side.

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
Copy link

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
Copy link
Member Author

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolve before graduation These issues need to be resolved before the spec graduates from the CG
Projects
None yet
Development

No branches or pull requests

5 participants
@gsnedders @annevk @johannhof @cfredric and others