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

fix: Improve cross-origin cookie handling #22320

Merged
merged 35 commits into from
Jun 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
57f1267
fix: improve cross-origin cookie handling
chrisbreiding Jun 13, 2022
157acf1
Merge branch 'develop' into issue-20685-improve-cookie-handling
chrisbreiding Jun 13, 2022
729133f
get automation in a better way
chrisbreiding Jun 14, 2022
ac6bcb3
refactor cookie handling
chrisbreiding Jun 14, 2022
e657c81
remove obsolete tests
chrisbreiding Jun 14, 2022
9c7d372
extract cookie jar from session code
chrisbreiding Jun 14, 2022
5e10c47
update doc / add comments
chrisbreiding Jun 14, 2022
ceb127e
use proxy aut url instead of remote states
chrisbreiding Jun 14, 2022
d5f816b
get cy.clearCookie() and cy.clearCookies() working
chrisbreiding Jun 14, 2022
12dbc43
refactor tests
chrisbreiding Jun 14, 2022
7a42d38
refactor cookie functions into cookie jar
chrisbreiding Jun 14, 2022
f1cb1dc
Merge branch 'develop' into issue-20685-improve-cookie-handling
chrisbreiding Jun 14, 2022
e1a5b29
cleanup
chrisbreiding Jun 14, 2022
d34f408
remove redundant cookie clearing
chrisbreiding Jun 15, 2022
756af9f
fix import references
chrisbreiding Jun 15, 2022
c989b01
fix issues with cookie jar
chrisbreiding Jun 15, 2022
3c81b36
error if this.next() is called more than once in same middleware func…
chrisbreiding Jun 15, 2022
efe10f3
fix test
chrisbreiding Jun 15, 2022
3c558b5
remove vestigial event
chrisbreiding Jun 16, 2022
52b6874
fix typos and add comments about browser differences
chrisbreiding Jun 16, 2022
a3eabd7
remove console.log
chrisbreiding Jun 16, 2022
7c2dd0f
make not logged in usage consistent in tests
chrisbreiding Jun 16, 2022
0ae6188
fix issues with automation cookies in firefox
chrisbreiding Jun 21, 2022
ab6e987
add comments to clarify various cookie utilities
chrisbreiding Jun 21, 2022
7eee1fa
fix issues caused by stability changes
chrisbreiding Jun 21, 2022
dd0927b
skip test with known issue in firefox
chrisbreiding Jun 22, 2022
f45b180
remove accidentally committed config
chrisbreiding Jun 22, 2022
db8684a
add/improve types
chrisbreiding Jun 22, 2022
95124a3
apply review suggestions
chrisbreiding Jun 22, 2022
c22269b
add middleware unit tests
chrisbreiding Jun 22, 2022
4903cb1
update sameSiteContext comments
chrisbreiding Jun 22, 2022
ba6c0ac
Merge branch 'develop' into issue-20685-improve-cookie-handling
chrisbreiding Jun 23, 2022
235764f
fix handling of conflicting cookies
chrisbreiding Jun 23, 2022
298ac4d
fix test
chrisbreiding Jun 23, 2022
bae7061
fix test
chrisbreiding Jun 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/app/src/runner/event-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const driverToSocketEvents = 'backend:request automation:request mocha recorder:
const driverTestEvents = 'test:before:run:async test:after:run'.split(' ')
const driverToLocalEvents = 'viewport:changed config stop url:changed page:loading visit:failed visit:blank cypress:in:cypress:runner:event'.split(' ')
const socketRerunEvents = 'runner:restart watched:file:changed'.split(' ')
const socketToDriverEvents = 'net:stubbing:event request:event script:error'.split(' ')
const socketToDriverEvents = 'net:stubbing:event request:event script:error cross:origin:automation:cookies'.split(' ')
const localToReporterEvents = 'reporter:log:add reporter:log:state:changed reporter:log:remove'.split(' ')

/**
Expand Down
6 changes: 2 additions & 4 deletions packages/driver/cross-origin-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,9 @@ Similar to **defaults()** methods needing to be called again, some events may ne

## Cookies

Having the **AUT** on a different origin than **top** causes issues with cookies being set for the origin in the **AUT**. Cookies have a **SameSite** attribute that can be set to **Strict**, **Lax**, ****or **None**. If **Strict** or **Lax**, cookies will not be set when the site is in an iframe on a different origin from the **top frame**.
Having the **AUT** on a different origin than **top** causes issues with cookies being set for the origin in the **AUT**. Cookies that would normally be set when a user's app is run outside of Cypress are not set due to it being rendered in an iframe.

In order to counteract this, we currently coerce the **SameSite** value of all cross-origin cookies to be **None**. This is not particularly ideal, since it could lead to unexpected behavior if cookies are set even though they should not be.

There are plans to change the implementation so that coercing the cookies is not necessary, so this behavior will change, but cookies will still need special handling for them to work as expected with a site whose origin is secondary.
In order to counteract this, we utilize the [proxy](../proxy) to capture cookies from cross-origin responses, store them in our own server-side cookie jar, set them in the browser with automation, and then attach them to cross-origin requests where appropriate. This simulates how cookies behave outside of Cypress.

## Unsupported APIs

Expand Down
1 change: 1 addition & 0 deletions packages/driver/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export default defineConfig({
'hosts': {
'*.foobar.com': '127.0.0.1',
'*.idp.com': '127.0.0.1',
'localalias': '127.0.0.1',
},
'reporter': 'cypress-multi-reporters',
'reporterOptions': {
Expand Down
6 changes: 3 additions & 3 deletions packages/driver/cypress/e2e/commands/navigation.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -1987,7 +1987,7 @@ describe('src/cy/commands/navigation', () => {
// on 2nd retry add the DOM element
const win = cy.state('window')

$('<div id=\'does-not-exist\'>does not exist<div>')
$('<div id=\'did-not-exist\'>did not exist<div>')
.appendTo(win.document.body)

break
Expand All @@ -1996,7 +1996,7 @@ describe('src/cy/commands/navigation', () => {
// and on the 3rd retry add the class
cy.state('window')

$('#does-not-exist').addClass('foo')
$('#did-not-exist').addClass('foo')

break
}
Expand All @@ -2011,7 +2011,7 @@ describe('src/cy/commands/navigation', () => {
// make get timeout after 300ms
// but even though our page does not load for 500ms
// this does not time out
.get('#does-not-exist', { timeout: 300 }).should('have.class', 'foo')
.get('#did-not-exist', { timeout: 300 }).should('have.class', 'foo')
})

describe('errors', () => {
Expand Down
Loading