From 5de89c793335b5921b0bf6ab2ae8da772bcdae3a Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Wed, 15 Feb 2023 19:04:40 -0500 Subject: [PATCH 1/4] fix: fix the cookie_behavior tests by syncing cookies immediately if the application is already stable --- .../e2e/e2e/origin/cookie_behavior.cy.ts | 92 ++++--------------- .../driver/src/cross-origin/events/cookies.ts | 18 +++- 2 files changed, 35 insertions(+), 75 deletions(-) diff --git a/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts b/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts index 6e0965aada61..626e4f8a3a56 100644 --- a/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts +++ b/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts @@ -251,17 +251,7 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { // though request is cross origin, site should have access directly to cookie because it is same site // assert cookie value is actually set in the browser - // current expected assertion. NOTE: This SHOULD be consistent - if (Cypress.isBrowser('firefox')) { - // firefox actually sets the cookie correctly - cy.getCookie('foo1').its('value').should('equal', 'bar1') - } else { - cy.getCookie('foo1').should('equal', null) - } - - // FIXME: Ideally, browser should have access to this cookie. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - // future expected assertion - // cy.getCookie('foo1').its('value').should('equal', 'bar1') + cy.getCookie('foo1').its('value').should('equal', 'bar1') cy.window().then((win) => { // but send the cookies in the request @@ -298,17 +288,7 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { // though request is cross origin, site should have access directly to cookie because it is same site // assert cookie value is actually set in the browser - // current expected assertion. NOTE: This SHOULD be consistent - if (Cypress.isBrowser('firefox')) { - // firefox actually sets the cookie correctly - cy.getCookie('foo1').its('value').should('equal', 'bar1') - } else { - cy.getCookie('foo1').should('equal', null) - } - - // FIXME: Ideally, browser should have access to this cookie. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - // future expected assertion - // cy.getCookie('foo1').its('value').should('equal', 'bar1') + cy.getCookie('foo1').its('value').should('equal', 'bar1') cy.window().then((win) => { // but send the cookies in the request @@ -376,17 +356,7 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { }) // assert cookie value is actually set in the browser - // current expected assertion. - if (Cypress.isBrowser('firefox')) { - // firefox actually sets the cookie correctly - cy.getCookie('foo1').its('value').should('equal', 'bar1') - } else { - cy.getCookie('foo1').should('equal', null) - } - - // FIXME: Ideally, browser should have access to this cookie. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - // future expected assertion - // cy.getCookie('foo1').its('value').should('equal', 'bar1') + cy.getCookie('foo1').its('value').should('equal', 'bar1') cy.window().then((win) => { return cy.wrap(window.makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, 'fetch', 'include')) @@ -418,17 +388,7 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { }) // assert cookie value is actually set in the browser - // current expected assertion. NOTE: This SHOULD be consistent - if (Cypress.isBrowser('firefox')) { - // firefox actually sets the cookie correctly - cy.getCookie('foo1').its('value').should('equal', 'bar1') - } else { - cy.getCookie('foo1').should('equal', null) - } - - // FIXME: Ideally, browser should have access to this cookie. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - // future expected assertion - // cy.getCookie('foo1').its('value').should('equal', 'bar1') + cy.getCookie('foo1').its('value').should('equal', 'bar1') cy.window().then((win) => { return cy.wrap(window.makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, 'fetch')) @@ -529,14 +489,11 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/set-cookie-credentials?cookie=bar1=baz1; Domain=barbaz.com; SameSite=None; Secure`, 'xmlHttpRequest', true)) }) - // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - cy.getCookie('bar1').should('equal', null) - // can only set third-party SameSite=None with Secure attribute, which is only possibly over https - - //expected future assertion - // cy.getCookie('bar1').its('value').should('equal', 'baz1') + // assert cookie value is actually set in the browser, even if in a different domain + cy.getCookie('bar1', { + domain: 'barbaz.com', + }).its('value').should('equal', 'baz1') } else { cy.getCookie('bar1').should('equal', null) } @@ -572,12 +529,10 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/set-cookie-credentials?cookie=bar1=baz1; Domain=barbaz.com; SameSite=None; Secure`, 'xmlHttpRequest', true)) }) - // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - cy.getCookie('bar1').should('equal', null) - // can only set third-party SameSite=None with Secure attribute, which is only possibly over https - - //expected future assertion - // cy.getCookie('bar1').its('value').should('equal', 'baz1') + // assert cookie value is actually set in the browser, even if in a different domain + cy.getCookie('bar1', { + domain: 'barbaz.com', + }).its('value').should('equal', 'baz1') cy.window().then((win) => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/test-request-credentials`, 'xmlHttpRequest', true)) @@ -649,14 +604,11 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/set-cookie-credentials?cookie=bar1=baz1; Domain=barbaz.com; SameSite=None; Secure`, 'fetch', 'include')) }) - // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - cy.getCookie('bar1').should('equal', null) - // can only set third-party SameSite=None with Secure attribute, which is only possibly over https - - //expected future assertion - // cy.getCookie('bar1').its('value').should('equal', 'baz1') + // assert cookie value is actually set in the browser, even if in a different domain + cy.getCookie('bar1', { + domain: 'barbaz.com', + }).its('value').should('equal', 'baz1') } else { cy.getCookie('bar1').should('equal', null) } @@ -695,14 +647,10 @@ describe('Cookie Behavior', { browser: '!webkit' }, () => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/set-cookie-credentials?cookie=bar1=baz1; Domain=barbaz.com; SameSite=None; Secure`, 'fetch', 'include')) }) - // assert cookie value is actually set in the browser - - // FIXME: cy.getCookie does not believe this cookie exists, though it is set in the browser. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. - cy.getCookie('bar1').should('equal', null) - // can only set third-party SameSite=None with Secure attribute, which is only possibly over https - - //expected future assertion - // cy.getCookie('bar1').its('value').should('equal', 'baz1') + // assert cookie value is actually set in the browser, even if in a different domain + cy.getCookie('bar1', { + domain: 'barbaz.com', + }).its('value').should('equal', 'baz1') cy.window().then((win) => { return cy.wrap(window.makeRequest(win, `${scheme}://www.barbaz.com:${sameOriginPort}/test-request-credentials`, 'fetch', 'include')) diff --git a/packages/driver/src/cross-origin/events/cookies.ts b/packages/driver/src/cross-origin/events/cookies.ts index ed8303a65bdc..926450777991 100644 --- a/packages/driver/src/cross-origin/events/cookies.ts +++ b/packages/driver/src/cross-origin/events/cookies.ts @@ -25,8 +25,8 @@ export const handleCrossOriginCookies = (Cypress: ICypress) => { // this event allows running a handler before stability is released. // this prevents subsequent commands from running until the cookies // are set via automation - // @ts-ignore - Cypress.once('before:stability:release', () => { + + const addCookiesCallback = () => { const cookies = cookiesToSend cookiesToSend = [] @@ -37,6 +37,18 @@ export const handleCrossOriginCookies = (Cypress: ICypress) => { .catch(() => { // errors here can be ignored as they're not user-actionable }) - }) + } + + // if the application is already stable, sync the cookies to the automation client immediately + if (cy.state('isStable')) { + addCookiesCallback() + } else { + // otherwise, wait until stability is achieved + // this event allows running a handler before stability is released. + // this prevents subsequent commands from running until the cookies + // are set via automation + // @ts-ignore + Cypress.once('before:stability:release', addCookiesCallback) + } }) } From 392349ecf5aefa30e1efbe94ad5e2f45de82575e Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Thu, 16 Feb 2023 11:34:51 -0500 Subject: [PATCH 2/4] chore: add changelog entry --- cli/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 974f0ad1ab56..1ec1f0c2b3fb 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -6,6 +6,7 @@ _Released 03/1/2023 (PENDING)_ **Bugfixes:** - Fixed an issue where cookies were being duplicated with the same hostname, but a prepended dot. Fixed an issue where cookies may not be expiring correctly. Fixes [#25174](https://github.com/cypress-io/cypress/issues/25174), [#25205](https://github.com/cypress-io/cypress/issues/25205) and [#25495](https://github.com/cypress-io/cypress/issues/25495). +- Fixed an issue where cookies weren't being synced when the application was stable. Fixed in [#25855](https://github.com/cypress-io/cypress/pull/25855). Fixes [#25835](https://github.com/cypress-io/cypress/issues/25835). ## 12.6.0 From 80016409f29e0435590ec2a81dbe4c16567216ea Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Thu, 16 Feb 2023 11:35:00 -0500 Subject: [PATCH 3/4] [run ci] From 66b323f74c1c652349ed399a983b3476a0f3d5f5 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Fri, 17 Feb 2023 16:43:37 -0500 Subject: [PATCH 4/4] chore: address comments from code review --- packages/driver/src/cross-origin/events/cookies.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/driver/src/cross-origin/events/cookies.ts b/packages/driver/src/cross-origin/events/cookies.ts index 926450777991..bd4306d61077 100644 --- a/packages/driver/src/cross-origin/events/cookies.ts +++ b/packages/driver/src/cross-origin/events/cookies.ts @@ -22,11 +22,7 @@ export const handleCrossOriginCookies = (Cypress: ICypress) => { waitingToSend = true - // this event allows running a handler before stability is released. - // this prevents subsequent commands from running until the cookies - // are set via automation - - const addCookiesCallback = () => { + const syncCookiesViaAutomation = () => { const cookies = cookiesToSend cookiesToSend = [] @@ -41,14 +37,14 @@ export const handleCrossOriginCookies = (Cypress: ICypress) => { // if the application is already stable, sync the cookies to the automation client immediately if (cy.state('isStable')) { - addCookiesCallback() + syncCookiesViaAutomation() } else { // otherwise, wait until stability is achieved // this event allows running a handler before stability is released. // this prevents subsequent commands from running until the cookies // are set via automation // @ts-ignore - Cypress.once('before:stability:release', addCookiesCallback) + Cypress.once('before:stability:release', syncCookiesViaAutomation) } }) }