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

Multi-domain: Improve cookie handling #20685

Closed
chrisbreiding opened this issue Mar 18, 2022 · 4 comments · Fixed by #22320
Closed

Multi-domain: Improve cookie handling #20685

chrisbreiding opened this issue Mar 18, 2022 · 4 comments · Fixed by #22320
Assignees
Labels
topic: cy.origin Problems or enhancements related to cy.origin command

Comments

@chrisbreiding
Copy link
Contributor

chrisbreiding commented Mar 18, 2022

What would you like?

The SameSite cookie handling implemented in #20450 is a stop-gap measure to make it work for the experimental release of multi-domain, coercing multi-domain cookies to have SameSite=None.

For the GA release of multi-domain support, instead of mutating cookies, the strategy should be the following:

  • Recognize multi-domain cookies that the browser won't set but would be set if the user's app were not in an iframe
  • Capture those cookies in our own cookie jar*
  • Change the header to X-Set-Cookie so it's clear the cookies are not being set by standard means
  • When a request is made to a domain for which we have stored cookies and it's appropriate to attach them based on its SameSite policy, do so

* I did a proof of concept where I set the cookies on the browser's cookie jar. In that case, they do get set but still aren't attached to requests, so we still have to attach them ourselves. Getting the cookies from automation to attach them worked fine in Chrome, but would not work in Firefox. Further investigation into fixing that could be done, but there's also increased latency per request involved in that approach too, whereas using our own cookie jar should be much faster. We just need to make sure it behaves like the browser's (e.g. unsetting the cookie when expired).

NOTE: Make sure to update the feature overview readme after changing this behavior, since it references the old behavior.

Why is this needed?

We need cookies to work as they would normally for the sake of authentication in multi-domain. The experimental release version is not ideal, since it relies on mutating all cookies to be SameSite=None and also needs a potentially brittle workaround for Firefox localhost cookies.

The proposed solution can be more nuanced. It can set cookies from responses and attach cookies to requests the way the browser would in a non-iframe situation. The user will still be able to see the cookies in the DevTools Network tab on requests as they were sent by the response instead of our mutated version.

Other

No response

@mjhenkes
Copy link
Member

We believe this update will also correct the following issues:

#21363
#21201
#21186 (comment)
#21186 (comment)
#21186 (comment)

@ganeshac123
Copy link

@chrisbreiding @cy-veronica @cypress-app-bot Any update on this ticket, we are waiting for release?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 23, 2022

The code for this is done in cypress-io/cypress#22320, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 28, 2022

Released in 10.3.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.3.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic: cy.origin Problems or enhancements related to cy.origin command
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants