From e90b0a4a4169b8b6b24cbc6b162609e7393eddbb Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Tue, 6 Sep 2022 15:41:23 -0400 Subject: [PATCH 1/7] test: add correct cookie_behavior assertions before work on server (currently failing) --- .../e2e/e2e/origin/cookie_behavior.cy.ts | 285 +++++++++++------- 1 file changed, 179 insertions(+), 106 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 4726fb353f17..d0e6c81554c7 100644 --- a/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts +++ b/packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts @@ -41,7 +41,6 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { } beforeEach(() => { - // FIXME: clearing cookies in the browser currently does not clear cookies in the server-side cookie jar cy.clearCookies() }) @@ -182,14 +181,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does NOT attach same-site cookies to request if "omit" credentials option is specified', () => { cy.intercept(`${originUrl}/test-request`, (req) => { - // current expected assertion with server side cookie jar is set from previous test - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion, regardless of server side cookie jar - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -214,14 +209,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does NOT set same-site cookies from request if "omit" credentials option is specified', () => { cy.intercept(`${originUrl}/test-request`, (req) => { - // current expected assertion with server side cookie jar is set from previous test - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion, regardless of server side cookie jar - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -250,10 +241,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { describe('same site / cross origin', () => { describe('XMLHttpRequest', () => { - // withCredentials option should have no effect on same-site requests, even though the request is cross-origin - it('sets and attaches same-site cookies to request, even though request is cross-origin', () => { + it('does NOT set and attach same-site cookies to request when the request is cross-origin', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -283,17 +273,107 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) }) + + it('sets cookie on same-site request if withCredentials is true, but does not attach to same-site request if withCredentials is false', () => { + cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request`, (req) => { + expect(req['headers']['cookie']).to.equal(undefined) + + req.reply({ + statusCode: 200, + }) + }).as('cookieCheck') + + cy.visit('/fixtures/primary-origin.html') + cy.get(`a[data-cy="cookie-${scheme}"]`).click() + + // cookie jar should now mimic http://foobar.com:3500 / https://foobar.com:3502 as top + cy.origin(originUrl, { + args: { + scheme, + crossOriginPort, + }, + }, ({ scheme, crossOriginPort }) => { + cy.window().then((win) => { + // do NOT set the cookie in the browser + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/set-cookie-credentials?cookie=foo1=bar1; Domain=foobar.com`, 'xmlHttpRequest', true)) + }) + + // 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').its('value').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.window().then((win) => { + // but send the cookies in the request + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request`, 'xmlHttpRequest')) + }) + + cy.wait('@cookieCheck') + }) + }) + + it('sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true', () => { + cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { + expect(req['headers']['cookie']).to.equal('foo1=bar1') + + req.reply({ + statusCode: 200, + }) + }).as('cookieCheck') + + cy.visit('/fixtures/primary-origin.html') + cy.get(`a[data-cy="cookie-${scheme}"]`).click() + + // cookie jar should now mimic http://foobar.com:3500 / https://foobar.com:3502 as top + cy.origin(originUrl, { + args: { + scheme, + crossOriginPort, + }, + }, ({ scheme, crossOriginPort }) => { + cy.window().then((win) => { + // do NOT set the cookie in the browser + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/set-cookie-credentials?cookie=foo1=bar1; Domain=foobar.com`, 'xmlHttpRequest', true)) + }) + + // 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').its('value').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.window().then((win) => { + // but send the cookies in the request + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, 'xmlHttpRequest', true)) + }) + + cy.wait('@cookieCheck') + }) + }) }) describe('fetch', () => { - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does not set same-site cookies from request nor send same-site cookies by default (same-origin)', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -345,7 +425,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) // assert cookie value is actually set in the browser - // current expected assertion. NOTE: This SHOULD be consistent + // current expected assertion. if (Cypress.isBrowser('firefox')) { // firefox actually sets the cookie correctly cy.getCookie('foo1').its('value').should('equal', 'bar1') @@ -353,7 +433,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.getCookie('foo1').its('value').should('equal', null) } - // FIXME: ideally, browser should have access to this cookie + // 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') @@ -363,14 +443,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -399,7 +475,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.getCookie('foo1').its('value').should('equal', null) } - // FIXME: ideally, browser should have access to this cookie + // 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') @@ -411,15 +487,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 // this should have the same effect as same-origin option for same-site/cross-origin requests, but adding here incase our implementation is not consistent it('does not set or send same-site cookies if "omit" credentials option is specified', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') - - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, }) @@ -453,7 +524,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { describe('XMLHttpRequest', () => { it('does NOT set or send cookies with request by default', () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -484,14 +555,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // can only set third-party SameSite=None with Secure attribute, which is only possibly over https if (scheme === 'https') { - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does set cookie if withCredentials is true, but does not send cookie if withCredentials is false', () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('bar1=baz1') - - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -514,7 +580,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists, though it is set in the browser + // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. cy.getCookie('bar1').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -555,7 +621,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { return cy.wrap(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, though it is set in the browser + // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. cy.getCookie('bar1').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -574,9 +640,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { describe('fetch', () => { ['same-origin', 'omit'].forEach((credentialOption) => { - it(`does NOT set or send cookies with request by credentials is ${credentialOption}`, () => { + it(`does NOT set or send cookies with request if credentials is ${credentialOption}`, () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -608,18 +674,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { }) }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it(`does set cookie if credentials is "include", but does not send cookie if credentials is ${credentialOption}`, () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - // current expected assertion - if (scheme === 'https') { - expect(req['headers']['cookie']).to.equal('bar1=baz1') - } else { - expect(req['headers']['cookie']).to.equal('') - } - - // future expected assertion for both http / https - // expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -643,7 +700,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists, though it is set in the browser + // FIXME: cy.getCookie does not believe this cookie exists. Should be fixed in https://github.com/cypress-io/cypress/pull/23643. cy.getCookie('bar1').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -689,7 +746,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // 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 + // 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').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -931,14 +988,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does NOT attach same-site cookies to request if "omit" credentials option is specified', () => { cy.intercept('/test-request', (req) => { - // current expected assertion with server side cookie jar is set from previous test - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion, regardless of server side cookie jar - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -958,14 +1011,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does NOT set same-site cookies from request if "omit" credentials option is specified', () => { cy.intercept('/test-request', (req) => { - // current expected assertion with server side cookie jar is set from previous test - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion, regardless of server side cookie jar - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -989,10 +1038,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { describe('same site / cross origin', () => { describe('XMLHttpRequest', () => { - // withCredentials option should have no effect on same-site requests, even though the request is cross-origin - it('sets and attaches same-site cookies to request, even though request is cross-origin', () => { + it('does NOT set and attach same-site cookies to request when the request is cross-origin', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -1010,17 +1058,64 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) + + it('sets cookie on same-site request if withCredentials is true, but does not attach to same-site request if withCredentials is false', () => { + cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request`, (req) => { + expect(req['headers']['cookie']).to.equal(undefined) + + req.reply({ + statusCode: 200, + }) + }).as('cookieCheck') + + cy.visit(`${scheme}://www.foobar.com:${sameOriginPort}`) + cy.window().then((win) => { + // do NOT set the cookie in the browser + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/set-cookie-credentials?cookie=foo1=bar1; Domain=foobar.com`, 'xmlHttpRequest', true)) + }) + + // firefox actually sets the cookie correctly + cy.getCookie('foo1').its('value').should('equal', 'bar1') + + cy.window().then((win) => { + // but send the cookies in the request + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request`, 'xmlHttpRequest')) + }) + + cy.wait('@cookieCheck') + }) + + it('sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true', () => { + cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { + expect(req['headers']['cookie']).to.equal('foo1=bar1') + + req.reply({ + statusCode: 200, + }) + }).as('cookieCheck') + + cy.visit(`${scheme}://www.foobar.com:${sameOriginPort}`) + cy.window().then((win) => { + // do NOT set the cookie in the browser + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/set-cookie-credentials?cookie=foo1=bar1; Domain=foobar.com`, 'xmlHttpRequest', true)) + }) + + cy.getCookie('foo1').its('value').should('equal', 'bar1') + + cy.window().then((win) => { + // but send the cookies in the request + return cy.wrap(makeRequest(win, `${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, 'xmlHttpRequest', true)) + }) + + cy.wait('@cookieCheck') + }) }) describe('fetch', () => { - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does not set same-site cookies from request nor send same-site cookies by default (same-origin)', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -1061,14 +1156,10 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -1088,15 +1179,11 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 // this should have the same effect as same-origin option for same-site/cross-origin requests, but adding here incase our implementation is not consistent it('does not set or send same-site cookies if "omit" credentials option is specified', () => { cy.intercept(`${scheme}://app.foobar.com:${crossOriginPort}/test-request-credentials`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('foo1=bar1') + expect(req['headers']['cookie']).to.equal(undefined) - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') req.reply({ statusCode: 200, }) @@ -1120,7 +1207,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { describe('XMLHttpRequest', () => { it('does NOT set or send cookies with request by default', () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -1141,14 +1228,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // can only set third-party SameSite=None with Secure attribute, which is only possibly over https if (scheme === 'https') { - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it('does set cookie if withCredentials is true, but does not send cookie if withCredentials is false', () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - // current expected assertion - expect(req['headers']['cookie']).to.equal('bar1=baz1') - - // future expected assertion - // expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -1162,7 +1244,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists, though it is 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').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -1193,7 +1275,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { return cy.wrap(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, though it is 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').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -1213,7 +1295,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { ['same-origin', 'omit'].forEach((credentialOption) => { it(`does NOT set or send cookies with request by credentials is ${credentialOption}`, () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -1234,18 +1316,9 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { cy.wait('@cookieCheck') }) - // FIXME: @see https://github.com/cypress-io/cypress/issues/23551 it(`does set cookie if credentials is "include", but does not send cookie if credentials is ${credentialOption}`, () => { cy.intercept(`${scheme}://www.barbaz.com:${sameOriginPort}/test-request`, (req) => { - // current expected assertion - if (scheme === 'https') { - expect(req['headers']['cookie']).to.equal('bar1=baz1') - } else { - expect(req['headers']['cookie']).to.equal('') - } - - // future expected assertion for both http / https - // expect(req['headers']['cookie']).to.equal('') + expect(req['headers']['cookie']).to.equal(undefined) req.reply({ statusCode: 200, @@ -1259,7 +1332,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // assert cookie value is actually set in the browser if (scheme === 'https') { - // FIXME: cy.getCookie does not believe this cookie exists, though it is 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').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https @@ -1295,7 +1368,7 @@ describe('Cookie Behavior with experimentalSessionAndOrigin=true', () => { // 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 + // 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').its('value').should('equal', null) // can only set third-party SameSite=None with Secure attribute, which is only possibly over https From 54b5943ed5b6ba3ef655c78061ae335235af4732 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Wed, 21 Sep 2022 19:19:41 -0400 Subject: [PATCH 2/7] chore: add types needed in the socket and middlewares --- packages/proxy/lib/types.ts | 12 ++++++++++++ packages/server/lib/server-base.ts | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/proxy/lib/types.ts b/packages/proxy/lib/types.ts index 57b7b2625022..6d3ccb4e269e 100644 --- a/packages/proxy/lib/types.ts +++ b/packages/proxy/lib/types.ts @@ -13,6 +13,8 @@ export type CypressIncomingRequest = Request & { responseTimeout?: number followRedirect?: boolean isAUTFrame: boolean + requestedWith?: RequestResourceType + credentialsLevel?: RequestCredentialLevel } export type RequestResourceType = 'fetch' | 'xhr' @@ -21,6 +23,16 @@ export type RequestCredentialLevel = 'same-origin' | 'include' | 'omit' | boolea export type CypressWantsInjection = 'full' | 'fullCrossOrigin' | 'partial' | false +export type AppliedCredentialByUrlAndResourceMap = Map> + +export type GetCredentialLevelOfRequest = (url: string, optionalResourceType?: RequestResourceType) => { + resourceType: RequestResourceType + credentialStatus: RequestCredentialLevel +} + /** * An outgoing response to an incoming request to the Cypress web server. */ diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index 38a9f7be1dcd..1e52365d7d20 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -14,7 +14,7 @@ import la from 'lazy-ass' import type httpsProxy from '@packages/https-proxy' import { netStubbingState, NetStubbingState } from '@packages/net-stubbing' import { agent, clientCertificates, cors, httpUtils, uri } from '@packages/network' -import { NetworkProxy, BrowserPreRequest } from '@packages/proxy' +import { NetworkProxy, BrowserPreRequest, RequestResourceType, RequestCredentialLevel, AppliedCredentialByUrlAndResourceMap } from '@packages/proxy' import type { SocketCt } from './socket-ct' import * as errors from './errors' import Request from './request' From 5f47fadea9e4505d09f6fab27876df7fb8bb60bf Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Wed, 7 Sep 2022 12:54:50 -0400 Subject: [PATCH 3/7] feat: add socket code to server-base (no tests here) to be used in request/response middleware --- packages/proxy/lib/http/index.ts | 10 ++++ packages/server/lib/server-base.ts | 87 ++++++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index 7263e2bba2bc..6d8eaef117ca 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -5,6 +5,8 @@ import type { CypressIncomingRequest, CypressOutgoingResponse, BrowserPreRequest, + AppliedCredentialByUrlAndResourceMap, + GetCredentialLevelOfRequest, } from '@packages/proxy' import Debug from 'debug' import chalk from 'chalk' @@ -73,6 +75,8 @@ export type ServerCtx = Readonly<{ getFileServerToken: () => string getCookieJar: () => CookieJar remoteStates: RemoteStates + appliedCredentialByUrlAndResourceMap: AppliedCredentialByUrlAndResourceMap + getCredentialLevelOfRequest: GetCredentialLevelOfRequest getRenderedHTMLOrigins: Http['getRenderedHTMLOrigins'] netStubbingState: NetStubbingState middleware: HttpMiddlewareStacks @@ -222,6 +226,8 @@ export class Http { request: any socket: CyServer.Socket serverBus: EventEmitter + appliedCredentialByUrlAndResourceMap: AppliedCredentialByUrlAndResourceMap + getCredentialLevelOfRequest: GetCredentialLevelOfRequest renderedHTMLOrigins: {[key: string]: boolean} = {} autUrl?: string getCookieJar: () => CookieJar @@ -240,6 +246,8 @@ export class Http { this.socket = opts.socket this.request = opts.request this.serverBus = opts.serverBus + this.appliedCredentialByUrlAndResourceMap = opts.appliedCredentialByUrlAndResourceMap + this.getCredentialLevelOfRequest = opts.getCredentialLevelOfRequest this.getCookieJar = opts.getCookieJar if (typeof opts.middleware === 'undefined') { @@ -267,6 +275,8 @@ export class Http { netStubbingState: this.netStubbingState, socket: this.socket, serverBus: this.serverBus, + appliedCredentialByUrlAndResourceMap: this.appliedCredentialByUrlAndResourceMap, + getCredentialLevelOfRequest: this.getCredentialLevelOfRequest, getCookieJar: this.getCookieJar, debug: (formatter, ...args) => { if (!debugVerbose.enabled) return diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index 1e52365d7d20..3b75c4575154 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -8,6 +8,7 @@ import express, { Express } from 'express' import http from 'http' import httpProxy from 'http-proxy' import _ from 'lodash' +import md5 from 'md5' import type { AddressInfo } from 'net' import url from 'url' import la from 'lazy-ass' @@ -38,6 +39,10 @@ import type { AutomationCookie } from './automation/cookies' const debug = Debug('cypress:server:server-base') +const hashUrl = (url: string): string => { + return md5(decodeURIComponent(url)) +} + const _isNonProxiedRequest = (req) => { // proxied HTTP requests have a URL like: "http://example.com/foo" // non-proxied HTTP requests have a URL like: "/foo" @@ -123,6 +128,7 @@ export abstract class ServerBase { protected _graphqlWS?: WebSocketServer protected _eventBus: EventEmitter protected _remoteStates: RemoteStates + protected _appliedCredentialByUrlAndResourceMap: AppliedCredentialByUrlAndResourceMap private getCurrentBrowser: undefined | (() => Browser) constructor () { @@ -141,6 +147,8 @@ export abstract class ServerBase { fileServerPort: this._fileServer?.port(), } }) + + this._appliedCredentialByUrlAndResourceMap = new Map() } ensureProp = ensureProp @@ -173,6 +181,42 @@ export abstract class ServerBase { return this._remoteStates } + get appliedCredentialByUrlAndResourceMap () { + return this._appliedCredentialByUrlAndResourceMap + } + + getCredentialLevelOfRequest (url: string, optionalResourceType?: RequestResourceType): { + resourceType: RequestResourceType + credentialStatus: RequestCredentialLevel + } { + const hashKey = hashUrl(url) + + debug(`credentials request received for request url ${url}, hashKey ${hashKey}`) + let value: { + resourceType: RequestResourceType + credentialStatus: RequestCredentialLevel + } | undefined + + const credentialsObj = this.appliedCredentialByUrlAndResourceMap.get(hashKey) + + if (credentialsObj) { + // remove item from queue + value = credentialsObj?.shift() + debug(`credential value found ${value}`) + } + + // if value is undefined for any reason, apply defaults and assume xhr if no optionalResourceType + // optionalResourceType should be provided with CDP based browsers, so at least we have a fallback that is more accurate + if (value === undefined) { + value = { + resourceType: optionalResourceType || 'xhr', + credentialStatus: optionalResourceType === 'fetch' ? 'same-origin' : false, + } + } + + return value + } + setupCrossOriginRequestHandling () { this._eventBus.on('cross:origin:automation:cookies', (cookies: AutomationCookie[]) => { this.socket.localBus.once('cross:origin:automation:cookies:received', () => { @@ -181,6 +225,35 @@ export abstract class ServerBase { this.socket.toDriver('cross:origin:automation:cookies', cookies) }) + + this.socket.localBus.on('request:sent:with:credentials', ( + { url, + resourceType, + credentialStatus, + }: { + url: string + resourceType: RequestResourceType + credentialStatus: RequestCredentialLevel + }, + ) => { + const hashKey = hashUrl(url) + + debug(`credentials request stored for request url ${url}, resourceType ${resourceType}, hashKey ${hashKey}`) + + let urlHashValue = this.appliedCredentialByUrlAndResourceMap.get(hashKey) + + if (!urlHashValue) { + this.appliedCredentialByUrlAndResourceMap.set(hashKey, [{ + resourceType, + credentialStatus, + }]) + } else { + urlHashValue.push({ + resourceType, + credentialStatus, + }) + } + }) } abstract createServer ( @@ -218,7 +291,12 @@ export abstract class ServerBase { clientCertificates.loadClientCertificateConfig(config) - this.createNetworkProxy({ config, getAutomation, remoteStates: this._remoteStates, shouldCorrelatePreRequests }) + this.createNetworkProxy({ + config, getAutomation, + remoteStates: this._remoteStates, + appliedCredentialByUrlAndResourceMap: this._appliedCredentialByUrlAndResourceMap, + shouldCorrelatePreRequests, + }) if (config.experimentalSourceRewriting) { createInitialWorkers() @@ -310,7 +388,7 @@ export abstract class ServerBase { return e } - createNetworkProxy ({ config, getAutomation, remoteStates, shouldCorrelatePreRequests }) { + createNetworkProxy ({ config, getAutomation, remoteStates, appliedCredentialByUrlAndResourceMap, shouldCorrelatePreRequests }) { const getFileServerToken = () => { return this._fileServer.token } @@ -328,6 +406,8 @@ export abstract class ServerBase { netStubbingState: this.netStubbingState, request: this.request, serverBus: this._eventBus, + appliedCredentialByUrlAndResourceMap, + getCredentialLevelOfRequest: this.getCredentialLevelOfRequest, }) } @@ -341,6 +421,7 @@ export abstract class ServerBase { this.networkProxy.reset() this.netStubbingState.reset() this._remoteStates.reset() + this._appliedCredentialByUrlAndResourceMap.clear() } const io = this.socket.startListening(this.server, automation, config, options) @@ -475,7 +556,7 @@ export abstract class ServerBase { reset () { this._networkProxy?.reset() - + this._appliedCredentialByUrlAndResourceMap.clear() const baseUrl = this._baseUrl ?? '' return this._remoteStates.set(baseUrl) From 27e639cf18286dcbb25e778cae5030d3813b9894 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Mon, 19 Sep 2022 14:54:49 -0400 Subject: [PATCH 4/7] feat: fill out the ExtractCypressMetadataHeaders implementation --- packages/proxy/lib/http/request-middleware.ts | 19 +++ .../test/unit/http/request-middleware.spec.ts | 141 ++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index 5635f493e436..b4ecb20e0a8a 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -3,6 +3,7 @@ import { blocked, cors } from '@packages/network' import { InterceptRequest } from '@packages/net-stubbing' import type { HttpMiddleware } from './' import { getSameSiteContext, addCookieJarCookiesToRequest } from './util/cookies' +import { doesTopNeedToBeSimulated } from './util/top-simulation' // do not use a debug namespace in this file - use the per-request `this.debug` instead // available as cypress-verbose:proxy:http @@ -23,6 +24,7 @@ const LogRequest: RequestMiddleware = function () { const ExtractCypressMetadataHeaders: RequestMiddleware = function () { this.req.isAUTFrame = !!this.req.headers['x-cypress-is-aut-frame'] + const requestIsXhrOrFetch = this.req.headers['x-cypress-request'] if (this.req.headers['x-cypress-is-aut-frame']) { delete this.req.headers['x-cypress-is-aut-frame'] @@ -33,6 +35,23 @@ const ExtractCypressMetadataHeaders: RequestMiddleware = function () { delete this.req.headers['x-cypress-request'] } + if (!this.config.experimentalSessionAndOrigin || + !doesTopNeedToBeSimulated(this) || + // this should be unreachable happen as the x-cypress-request header is only attached if the resource type is 'xhr' + // inside the extension or electron equivalent. This should only be needed for defensive purposes + (requestIsXhrOrFetch !== 'true' && requestIsXhrOrFetch !== 'xhr' && requestIsXhrOrFetch !== 'fetch')) { + this.next() + + return + } + + this.debug(`looking up credentials for ${this.req.proxiedUrl}`) + let { resourceType, credentialStatus } = this.getCredentialLevelOfRequest(this.req.proxiedUrl, requestIsXhrOrFetch !== 'true' ? requestIsXhrOrFetch : undefined) + + this.debug(`credentials calculated for ${resourceType}:${credentialStatus}`) + + this.req.requestedWith = resourceType + this.req.credentialsLevel = credentialStatus this.next() } diff --git a/packages/proxy/test/unit/http/request-middleware.spec.ts b/packages/proxy/test/unit/http/request-middleware.spec.ts index cfb6672c9e39..5f8c2c880219 100644 --- a/packages/proxy/test/unit/http/request-middleware.spec.ts +++ b/packages/proxy/test/unit/http/request-middleware.spec.ts @@ -1,6 +1,7 @@ import _ from 'lodash' import RequestMiddleware from '../../../lib/http/request-middleware' import { expect } from 'chai' +import sinon from 'sinon' import { testMiddleware } from './helpers' import { CypressIncomingRequest, CypressOutgoingResponse } from '../../../lib' import { HttpBuffer, HttpBuffers } from '../../../lib/http/util/buffers' @@ -86,6 +87,146 @@ describe('http/request-middleware', () => { expect(ctx.req.headers['x-cypress-request']).not.to.exist }) }) + + it('does not set requestedWith or credentialLevel on the request if the the experimentalSessionAndOrigin flag is off', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: false, + }, + req: { + headers: { + 'x-cypress-request': 'true', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.req.requestedWith).not.to.exist + expect(ctx.req.credentialsLevel).not.to.exist + }) + }) + + it('does not set requestedWith or credentialLevel on the request if top does NOT need to be simulated', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: true, + }, + getAUTUrl: sinon.stub().returns(undefined), + req: { + headers: { + 'x-cypress-request': 'true', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.req.requestedWith).not.to.exist + expect(ctx.req.credentialsLevel).not.to.exist + }) + }) + + it('does not set requestedWith or credentialLevel on the request if x-cypress-request has invalid values', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: true, + }, + getAUTUrl: sinon.stub().returns('http://localhost:8080'), + remoteStates: { + isPrimaryOrigin: sinon.stub().returns(false), + }, + req: { + headers: { + 'x-cypress-request': 'sub_frame', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.req.requestedWith).not.to.exist + expect(ctx.req.credentialsLevel).not.to.exist + }) + }) + + // CDP can determine whether or not the request is xhr | fetch, but the extension or electron cannot + it('provides getCredentialLevelOfRequest with resourceType if able to determine from header (xhr)', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: true, + }, + getAUTUrl: sinon.stub().returns('http://localhost:8080'), + remoteStates: { + isPrimaryOrigin: sinon.stub().returns(false), + }, + getCredentialLevelOfRequest: sinon.stub().returns({}), + req: { + proxiedUrl: 'http://localhost:8080', + headers: { + 'x-cypress-request': 'xhr', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.getCredentialLevelOfRequest).to.have.been.calledWith('http://localhost:8080', `xhr`) + }) + }) + + // CDP can determine whether or not the request is xhr | fetch, but the extension or electron cannot + it('provides getCredentialLevelOfRequest with resourceType if able to determine from header (fetch)', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: true, + }, + getAUTUrl: sinon.stub().returns('http://localhost:8080'), + remoteStates: { + isPrimaryOrigin: sinon.stub().returns(false), + }, + getCredentialLevelOfRequest: sinon.stub().returns({}), + req: { + proxiedUrl: 'http://localhost:8080', + headers: { + 'x-cypress-request': 'fetch', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.getCredentialLevelOfRequest).to.have.been.calledWith('http://localhost:8080', `fetch`) + }) + }) + + it('sets the resourceType and credentialsLevel on the request from whatever is returned by getCredentialLevelOfRequest if conditions apply', async () => { + const ctx = { + config: { + experimentalSessionAndOrigin: true, + }, + getAUTUrl: sinon.stub().returns('http://localhost:8080'), + remoteStates: { + isPrimaryOrigin: sinon.stub().returns(false), + }, + getCredentialLevelOfRequest: sinon.stub().returns({ + resourceType: 'fetch', + credentialStatus: 'same-origin', + }), + req: { + proxiedUrl: 'http://localhost:8080', + headers: { + 'x-cypress-request': 'true', + }, + } as Partial, + } + + await testMiddleware([ExtractCypressMetadataHeaders], ctx) + .then(() => { + expect(ctx.req.requestedWith).to.equal('fetch') + expect(ctx.req.credentialsLevel).to.equal('same-origin') + }) + }) }) describe('MaybeSimulateSecHeaders', () => { From 384d0e49f96306b1a2ececf9e4e6eb36f44d8bff Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Wed, 7 Sep 2022 14:33:44 -0400 Subject: [PATCH 5/7] feat: add attach cookie logic to requests based on xhr/fetch requests --- packages/proxy/lib/http/request-middleware.ts | 14 ++- .../test/unit/http/request-middleware.spec.ts | 85 ++++++++++++++++--- 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index b4ecb20e0a8a..a6d0ffcdf46d 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -2,7 +2,7 @@ import _ from 'lodash' import { blocked, cors } from '@packages/network' import { InterceptRequest } from '@packages/net-stubbing' import type { HttpMiddleware } from './' -import { getSameSiteContext, addCookieJarCookiesToRequest } from './util/cookies' +import { getSameSiteContext, addCookieJarCookiesToRequest, shouldAttachAndSetCookies } from './util/cookies' import { doesTopNeedToBeSimulated } from './util/top-simulation' // do not use a debug namespace in this file - use the per-request `this.debug` instead @@ -71,9 +71,16 @@ const MaybeSimulateSecHeaders: RequestMiddleware = function () { } const MaybeAttachCrossOriginCookies: RequestMiddleware = function () { + if (!this.config.experimentalSessionAndOrigin || !doesTopNeedToBeSimulated(this)) { + return this.next() + } + + // Top needs to be simulated since the AUT is in a cross origin state. Get the requestedWith and credentials and see what cookies need to be attached const currentAUTUrl = this.getAUTUrl() + const shouldCookiesBeAttachedToRequest = shouldAttachAndSetCookies(this.req.proxiedUrl, currentAUTUrl, this.req.requestedWith, this.req.credentialsLevel, this.req.isAUTFrame) - if (!this.config.experimentalSessionAndOrigin || !currentAUTUrl) { + this.debug(`should cookies be attached to request?: ${shouldCookiesBeAttachedToRequest}`) + if (!shouldCookiesBeAttachedToRequest) { return this.next() } @@ -89,7 +96,8 @@ const MaybeAttachCrossOriginCookies: RequestMiddleware = function () { this.debug('existing cookies on request from cookie jar: %s', applicableCookiesInCookieJar.join('; ')) this.debug('add cookies to request from header: %s', cookiesOnRequest.join('; ')) - this.req.headers['cookie'] = addCookieJarCookiesToRequest(applicableCookiesInCookieJar, cookiesOnRequest) + // if the cookie header is empty (i.e. ''), set it to undefined for expected behavior + this.req.headers['cookie'] = addCookieJarCookiesToRequest(applicableCookiesInCookieJar, cookiesOnRequest) || undefined this.debug('cookies being sent with request: %s', this.req.headers['cookie']) this.next() diff --git a/packages/proxy/test/unit/http/request-middleware.spec.ts b/packages/proxy/test/unit/http/request-middleware.spec.ts index 5f8c2c880219..d5c75cfedbda 100644 --- a/packages/proxy/test/unit/http/request-middleware.spec.ts +++ b/packages/proxy/test/unit/http/request-middleware.spec.ts @@ -7,6 +7,7 @@ import { CypressIncomingRequest, CypressOutgoingResponse } from '../../../lib' import { HttpBuffer, HttpBuffers } from '../../../lib/http/util/buffers' import { RemoteStates } from '@packages/server/lib/remote_states' import { CookieJar } from '@packages/server/lib/util/cookies' +import { HttpMiddlewareThis } from '../../../lib/http' describe('http/request-middleware', () => { it('exports the members in the correct order', () => { @@ -327,9 +328,67 @@ describe('http/request-middleware', () => { expect(ctx.req.headers['cookie']).to.equal('request=cookie') }) - it('prepends cookie jar cookies to request', async () => { + it('is a noop if does not need to simulate top', async () => { const ctx = await getContext() + ctx.req.isAUTFrame = false + ctx.remoteStates.isPrimaryOrigin.returns(true), + + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) + + expect(ctx.req.headers['cookie']).to.equal('request=cookie') + }) + + it('is a noop if cookies do NOT need to be attached to request', async () => { + const ctx = await getContext(['request=cookie'], ['jar=cookie'], 'http://foobar.com', 'http://app.foobar.com') + + ctx.req.requestedWith = 'fetch' + ctx.req.credentialsLevel = 'omit' + + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) + + expect(ctx.req.headers['cookie']).to.equal('request=cookie') + }) + + it(`allows setting cookies on request if resource type cannot be determined, but comes from the AUT frame (likely in the case of documents or redirects)`, async function () { + const ctx = await getContext([], ['jar=cookie'], 'http://foobar.com/index.html', 'http://app.foobar.com/index.html') + + ctx.req.requestedWith = undefined + ctx.req.credentialsLevel = undefined + ctx.req.isAUTFrame = true + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) + + expect(ctx.req.headers['cookie']).to.equal('jar=cookie') + }) + + it(`otherwise, does not allow setting cookies if request type cannot be determined and is not from the AUT and is cross-origin`, async function () { + const ctx = await getContext([], ['jar=cookie'], 'http://foobar.com/index.html', 'http://app.foobar.com/index.html') + + ctx.req.requestedWith = undefined + ctx.req.credentialsLevel = undefined + ctx.req.isAUTFrame = false + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) + + expect(ctx.req.headers['cookie']).to.be.undefined + }) + + it('sets the cookie header to undefined if no cookies exist on the request, none in the jar, but cookies should be attached', async () => { + const ctx = await getContext([], [], 'http://foobar.com', 'http://app.foobar.com') + + ctx.req.requestedWith = 'xhr' + ctx.req.credentialsLevel = true + + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) + + expect(ctx.req.headers['cookie']).to.equal(undefined) + }) + + it('prepends cookie jar cookies to request', async () => { + const ctx = await getContext(['request=cookie'], ['jar=cookie'], 'http://foobar.com', 'http://app.foobar.com') + + ctx.req.requestedWith = 'fetch' + ctx.req.credentialsLevel = 'include' + await testMiddleware([MaybeAttachCrossOriginCookies], ctx) expect(ctx.req.headers['cookie']).to.equal('jar=cookie; request=cookie') @@ -383,7 +442,7 @@ describe('http/request-middleware', () => { describe('does not add request cookie to request if cookie exists in jar, and preserves duplicate cookies when same key/value if', () => { describe('subdomain and TLD', () => { it('matches hierarchy', async () => { - const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com', 'jar=cookie2; Domain=foobar.com', 'jar=cookie3; Domain=exclude.foobar.com'], 'http://app.foobar.com/generic') + const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com', 'jar=cookie2; Domain=foobar.com', 'jar=cookie3; Domain=exclude.foobar.com'], 'http://app.foobar.com/generic', 'http://app.foobar.com/generic') await testMiddleware([MaybeAttachCrossOriginCookies], ctx) @@ -391,7 +450,7 @@ describe('http/request-middleware', () => { }) it('matches hierarchy and gives order to the cookie that was created first', async () => { - const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com;', 'jar=cookie2; Domain=.foobar.com;'], 'http://app.foobar.com/generic') + const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com;', 'jar=cookie2; Domain=.foobar.com;'], 'http://app.foobar.com/generic', 'http://app.foobar.com/generic') const cookies = ctx.getCookieJar().getCookies('http://app.foobar.com/generic', 'strict') @@ -405,7 +464,7 @@ describe('http/request-middleware', () => { }) it('matches hierarchy and gives order to the cookie with the most specific path, regardless of creation time', async () => { - const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com; Path=/generic', 'jar=cookie2; Domain=.foobar.com;'], 'http://app.foobar.com/generic') + const ctx = await getContext(['jar=cookie', 'request=cookie'], ['jar=cookie1; Domain=app.foobar.com; Path=/generic', 'jar=cookie2; Domain=.foobar.com;'], 'http://app.foobar.com/generic', 'http://app.foobar.com/generic') const cookies = ctx.getCookieJar().getCookies('http://app.foobar.com/generic', 'strict') @@ -433,7 +492,7 @@ describe('http/request-middleware', () => { 'jar=cookie9; Domain=exclude.foobar.com; Path=/generic/specific', ] - const ctx = await getContext(['request=cookie'], cookieJarCookies, 'http://app.foobar.com/generic/specific') + const ctx = await getContext(['request=cookie'], cookieJarCookies, 'http://app.foobar.com/generic/specific', 'http://app.foobar.com/generic/specific') const cookies = ctx.getCookieJar().getCookies('http://app.foobar.com/generic', 'strict') @@ -448,12 +507,12 @@ describe('http/request-middleware', () => { }) }) - async function getContext (requestCookieStrings = ['request=cookie'], cookieJarStrings = ['jar=cookie'], autAndRequestUrl = 'http://foobar.com') { + async function getContext (requestCookieStrings = ['request=cookie'], cookieJarStrings = ['jar=cookie'], autUrl = 'http://foobar.com', requestUrl = 'http://foobar.com') { const cookieJar = new CookieJar() await Promise.all(cookieJarStrings.map(async (cookieString) => { try { - await cookieJar._cookieJar.setCookie(cookieString, autAndRequestUrl) + await cookieJar._cookieJar.setCookie(cookieString, requestUrl) } catch (e) { // likely doesn't match the url policy, path, or is another type of cookie mismatch return @@ -461,18 +520,20 @@ describe('http/request-middleware', () => { })) return { - getAUTUrl: () => autAndRequestUrl, + getAUTUrl: () => autUrl, getCookieJar: () => cookieJar, + remoteStates: { + isPrimaryOrigin: sinon.stub().returns(false), + }, config: { experimentalSessionAndOrigin: true }, req: { - proxiedUrl: autAndRequestUrl, + proxiedUrl: requestUrl, isAUTFrame: true, headers: { - - cookie: requestCookieStrings.join('; '), + cookie: requestCookieStrings.join('; ') || undefined, }, }, - } + } as HttpMiddlewareThis } }) From b1f0d26c22b1774d1c7c44d3f60b2d0a128c8e5b Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Wed, 7 Sep 2022 17:09:31 -0400 Subject: [PATCH 6/7] feat: add attaching cookies to response logic w/ tests --- .../proxy/lib/http/response-middleware.ts | 46 +- packages/proxy/lib/http/util/cookies.ts | 28 +- .../unit/http/response-middleware.spec.ts | 727 +++++++++++++++++- 3 files changed, 759 insertions(+), 42 deletions(-) diff --git a/packages/proxy/lib/http/response-middleware.ts b/packages/proxy/lib/http/response-middleware.ts index 0a71202c3212..230f828ea469 100644 --- a/packages/proxy/lib/http/response-middleware.ts +++ b/packages/proxy/lib/http/response-middleware.ts @@ -4,7 +4,7 @@ import type Debug from 'debug' import type { CookieOptions } from 'express' import { cors, concatStream, httpUtils } from '@packages/network' import type { CypressIncomingRequest, CypressOutgoingResponse } from '@packages/proxy' -import type { HttpMiddleware, HttpMiddlewareThis } from '.' +import type { HttpMiddleware } from '.' import iconv from 'iconv-lite' import type { IncomingMessage, IncomingHttpHeaders } from 'http' import { InterceptResponse } from '@packages/net-stubbing' @@ -13,6 +13,7 @@ import * as rewriter from './util/rewriter' import zlib from 'zlib' import { URL } from 'url' import { CookiesHelper } from './util/cookies' +import { doesTopNeedToBeSimulated } from './util/top-simulation' interface ResponseMiddlewareProps { /** @@ -370,48 +371,33 @@ const MaybePreventCaching: ResponseMiddleware = function () { this.next() } -const checkIfNeedsCrossOriginHandling = (ctx: HttpMiddlewareThis) => { - const currentAUTUrl = ctx.getAUTUrl() - - // A cookie needs cross origin handling if the request itself is - // cross-origin or the origins between requests don't match, - // since the browser won't set them in that case and if it's - // secondary-origin -> primary-origin, we don't recognize the request as cross-origin - return ( - ctx.config.experimentalSessionAndOrigin - && ( - (currentAUTUrl && !cors.urlOriginsMatch(currentAUTUrl, ctx.req.proxiedUrl)) - || !ctx.remoteStates.isPrimaryOrigin(ctx.req.proxiedUrl) - ) - ) -} - -const CopyCookiesFromIncomingRes: ResponseMiddleware = async function () { +const MaybeCopyCookiesFromIncomingRes: ResponseMiddleware = async function () { const cookies: string | string[] | undefined = this.incomingRes.headers['set-cookie'] if (!cookies || !cookies.length) { return this.next() } - // Cross-origin Cookie Handling + // Simulated Top Cookie Handling // --------------------------- // - We capture cookies sent by responses and add them to our own server-side // tough-cookie cookie jar. All request cookies are captured, since any - // future request could be cross-origin even if the response that sets them + // future request could be cross-origin in the context of top, even if the response that sets them // is not. // - If we sent the cookie header, it may fail to be set by the browser // (in most cases). However, we cannot determine all the cases in which Set-Cookie - // will currently fail, and currently is best to set optimistically until #23551 is addressed. + // will currently fail. We try to address this in our tough cookie jar + // by only setting cookies that would otherwise work in the browser if the AUT url was top // - We also set the cookies through automation so they are available in the // browser via document.cookie and via Cypress cookie APIs - // (e.g. cy.getCookie). This is only done for cross-origin responses, since - // non-cross-origin responses will be successfully set in the browser - // automatically. + // (e.g. cy.getCookie). This is only done when the AUT url and top do not match responses, + // since AUT and Top being same origin will be successfully set in the browser + // automatically as expected. // - In the request middleware, we retrieve the cookies for a given URL // and attach them to the request, like the browser normally would. // tough-cookie handles retrieving the correct cookies based on domain, // path, etc. It also removes cookies from the cookie jar if they've expired. - const needsCrossOriginHandling = checkIfNeedsCrossOriginHandling(this) + const doesTopNeedSimulating = doesTopNeedToBeSimulated(this) const appendCookie = (cookie: string) => { // always call 'Set-Cookie' in the browser as cross origin or same site requests @@ -425,7 +411,7 @@ const CopyCookiesFromIncomingRes: ResponseMiddleware = async function () { } } - if (!this.config.experimentalSessionAndOrigin) { + if (!this.config.experimentalSessionAndOrigin || !doesTopNeedSimulating) { ([] as string[]).concat(cookies).forEach((cookie) => { appendCookie(cookie) }) @@ -440,7 +426,9 @@ const CopyCookiesFromIncomingRes: ResponseMiddleware = async function () { request: { url: this.req.proxiedUrl, isAUTFrame: this.req.isAUTFrame, - needsCrossOriginHandling, + doesTopNeedSimulating, + resourceType: this.req.requestedWith, + credentialLevel: this.req.credentialsLevel, }, }) @@ -454,7 +442,7 @@ const CopyCookiesFromIncomingRes: ResponseMiddleware = async function () { const addedCookies = await cookiesHelper.getAddedCookies() - if (!needsCrossOriginHandling || !addedCookies.length) { + if (!addedCookies.length) { return this.next() } @@ -599,7 +587,7 @@ export default { OmitProblematicHeaders, MaybePreventCaching, MaybeStripDocumentDomainFeaturePolicy, - CopyCookiesFromIncomingRes, + MaybeCopyCookiesFromIncomingRes, MaybeSendRedirectToClient, CopyResponseStatusCode, ClearCyInitialCookie, diff --git a/packages/proxy/lib/http/util/cookies.ts b/packages/proxy/lib/http/util/cookies.ts index 6efac68a6f22..199b4b3f41cb 100644 --- a/packages/proxy/lib/http/util/cookies.ts +++ b/packages/proxy/lib/http/util/cookies.ts @@ -11,7 +11,9 @@ type SiteContext = 'same-origin' | 'same-site' | 'cross-site' interface RequestDetails { url: string isAUTFrame: boolean - needsCrossOriginHandling: boolean + doesTopNeedSimulating: boolean + resourceType?: RequestResourceType + credentialLevel?: RequestCredentialLevel } /** @@ -162,6 +164,7 @@ export class CookiesHelper { debug: Debug.Debugger defaultDomain: string sameSiteContext: 'strict' | 'lax' | 'none' + siteContext: SiteContext previousCookies: Cookie[] = [] constructor ({ cookieJar, currentAUTUrl, request, debug }) { @@ -170,6 +173,7 @@ export class CookiesHelper { this.request = request this.debug = debug this.sameSiteContext = getSameSiteContext(currentAUTUrl, request.url, request.isAUTFrame) + this.siteContext = calculateSiteContext(this.request.url, this.currentAUTUrl || '') const parsedRequestUrl = new URL(request.url) @@ -180,7 +184,7 @@ export class CookiesHelper { // this plays a part in adding cross-origin cookies to the browser via // automation. if the request doesn't need cross-origin handling, this // is a noop - if (!this.request.needsCrossOriginHandling) return + if (!this.request.doesTopNeedSimulating) return this.previousCookies = this.cookieJar.getAllCookies() } @@ -189,7 +193,7 @@ export class CookiesHelper { // this plays a part in adding cross-origin cookies to the browser via // automation. if the request doesn't need cross-origin handling, this // is a noop - if (!this.request.needsCrossOriginHandling) return [] + if (!this.request.doesTopNeedSimulating) return [] const afterCookies = this.cookieJar.getAllCookies() @@ -208,10 +212,28 @@ export class CookiesHelper { // because Secure is required for SameSite=None. not all browsers currently // currently enforce this, but they're heading in that direction since // it's now the standard, so this is more future-proof + // TODO: in the future we may want to check for https, which might be tricky since localhost is considered a secure context if (!toughCookie || (toughCookie.sameSite === 'none' && !toughCookie.secure)) { return } + // cross site cookies cannot set lax/strict cookies in the browser for xhr/fetch requests (but ok with navigation/document requests) + if (this.request.resourceType && this.siteContext === 'cross-site' && toughCookie.sameSite !== 'none') { + this.debug(`cannot set cookie with SameSite=${toughCookie.sameSite} when site context is ${this.siteContext}`) + + return + } + + // don't set the cookie in our own cookie jar if the cookie would otherwise fail being set in the browser if the AUT Url + // was actually top. This prevents cookies from being applied to our cookie jar when they shouldn't, preventing possible security implications. + const shouldSetCookieGivenSiteContext = shouldAttachAndSetCookies(this.request.url, this.currentAUTUrl, this.request.resourceType, this.request.credentialLevel, this.request.isAUTFrame) + + if (!shouldSetCookieGivenSiteContext) { + this.debug(`not setting cookie for ${this.request.url} with simulated top ${ this.currentAUTUrl} for ${ this.request.resourceType}:${this.request.credentialLevel}, cookie: ${toughCookie}`) + + return + } + try { this.cookieJar.setCookie(toughCookie, this.request.url, this.sameSiteContext) } catch (err) { diff --git a/packages/proxy/test/unit/http/response-middleware.spec.ts b/packages/proxy/test/unit/http/response-middleware.spec.ts index 078b01a791b8..9f476139dd4f 100644 --- a/packages/proxy/test/unit/http/response-middleware.spec.ts +++ b/packages/proxy/test/unit/http/response-middleware.spec.ts @@ -19,7 +19,7 @@ describe('http/response-middleware', function () { 'OmitProblematicHeaders', 'MaybePreventCaching', 'MaybeStripDocumentDomainFeaturePolicy', - 'CopyCookiesFromIncomingRes', + 'MaybeCopyCookiesFromIncomingRes', 'MaybeSendRedirectToClient', 'CopyResponseStatusCode', 'ClearCyInitialCookie', @@ -584,8 +584,8 @@ describe('http/response-middleware', function () { } }) - describe('CopyCookiesFromIncomingRes', function () { - const { CopyCookiesFromIncomingRes } = ResponseMiddleware + describe('MaybeCopyCookiesFromIncomingRes', function () { + const { MaybeCopyCookiesFromIncomingRes } = ResponseMiddleware it('appends cookies on the response when an array', async function () { const { appendStub, ctx } = prepareSameOriginContext({ @@ -596,7 +596,7 @@ describe('http/response-middleware', function () { }, }) - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(appendStub).to.be.calledTwice expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1') @@ -606,7 +606,7 @@ describe('http/response-middleware', function () { it('appends cookies on the response when a string', async function () { const { appendStub, ctx } = prepareSameOriginContext() - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(appendStub).to.be.calledOnce expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') @@ -620,15 +620,722 @@ describe('http/response-middleware', function () { }, }) - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(appendStub).not.to.be.called }) - it('does not send cross:origin:automation:cookies if request does not need cross-origin handling', async () => { + it('is a noop in the cookie jar when top does NOT need simulating', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + incomingRes: { + headers: { + 'set-cookie': 'cookie=value', + }, + }, + }) + + ctx.getAUTUrl = () => 'http://www.foobar.com/index.html' + // set the primaryOrigin to true to signal we do NOT need to simulate top + ctx.remoteStates.isPrimaryOrigin = () => true + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('is a noop in the cookie jar when experimentalSessionAndOrigin is false', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + incomingRes: { + headers: { + 'set-cookie': 'cookie=value', + }, + }, + }) + + ctx.config.experimentalSessionAndOrigin = false + + // a case where top would need to be simulated, but the experimental flag is off + ctx.getAUTUrl = () => 'http://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + describe('same-origin', () => { + ['same-origin', 'include'].forEach((credentialLevel) => { + it(`sets first-party cookie context in the jar when simulating top if credentials included with fetch with credential ${credentialLevel}`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'fetch', + credentialsLevel: credentialLevel, + proxiedUrl: 'https://www.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://www.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://www.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top, just sets a third party cookie + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://www.foobar.com/test-request', 'strict') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + + ;[true, false].forEach((credentialLevel) => { + it(`sets first-party cookie context in the jar when simulating top if withCredentials ${credentialLevel} with xhr`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'xhr', + credentialsLevel: credentialLevel, + proxiedUrl: 'https://www.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://www.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://www.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top, just sets a third party cookie + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://www.foobar.com/test-request', 'strict') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + + it(`sets no cookies if fetch level is omit`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'fetch', + credentialsLevel: 'omit', + proxiedUrl: 'https://www.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should not work as this wouldn't be set in the browser if the AUT url was top + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://www.foobar.com/test-request', 'strict') + + // should not work as this wouldn't be set in the browser if the AUT url was top + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://www.foobar.com/test-request', 'strict') + + // should not work as this wouldn't be set in the browser if the AUT url was top + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://www.foobar.com/test-request', 'strict') + + // return these to the browser, even though they are likely to fail setting anyway + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + + describe('same-site', () => { + it('sets first-party cookie context in the jar when simulating top if credentials included with fetch via include', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'fetch', + credentialsLevel: 'include', + proxiedUrl: 'https://app.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://app.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://app.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top, just sets a third party cookie + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://app.foobar.com/test-request', 'strict') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + + it('sets first-party cookie context in the jar when simulating top if credentials true with xhr', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'xhr', + credentialsLevel: true, + proxiedUrl: 'https://app.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://app.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://app.foobar.com/test-request', 'strict') + + // should work as this would be set in the browser if the AUT url was top, just sets a third party cookie + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://app.foobar.com/test-request', 'strict') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + + ;['same-origin', 'omit'].forEach((credentialLevel) => { + it(`sets no cookies if fetch level is ${credentialLevel}`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a same-site request that has the ability to set first-party cookies in the browser + requestedWith: 'fetch', + credentialsLevel: credentialLevel, + proxiedUrl: 'https://app.foobar.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should not work as this wouldn't be set in the browser if the AUT url was top + expect(cookieJar.setCookie).not.to.have.been.called + + // return these to the browser, even though they are likely to fail setting anyway + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1; SameSite=Strict') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2; SameSite=Lax') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + }) + + describe('cross-site', () => { + it('sets third-party cookie context in the jar when simulating top if credentials included with fetch', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a cross-site request that has the ability to set cookies in the browser + requestedWith: 'fetch', + credentialsLevel: 'include', + proxiedUrl: 'https://www.barbaz.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should not work as this wouldn't be set in the browser if the AUT url was top anyway + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://www.barbaz.com/test-request', 'none') + + // should not work as this wouldn't be set in the browser if the AUT url was top anyway + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://www.barbaz.com/test-request', 'none') + + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://www.barbaz.com/test-request', 'none') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + + ;['same-origin', 'omit'].forEach((credentialLevel) => { + it(`does NOT set third-party cookie context in the jar when simulating top if credentials ${credentialLevel} with fetch`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a cross-site request that has the ability to set cookies in the browser + requestedWith: 'fetch', + credentialsLevel: credentialLevel, + proxiedUrl: 'https://www.barbaz.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + + // send to browser anyway even though these will likely fail to be set + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + + it('sets third-party cookie context in the jar when simulating top if withCredentials true with xhr', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a cross-site request that has the ability to set cookies in the browser + requestedWith: 'xhr', + credentialsLevel: true, + proxiedUrl: 'https://www.barbaz.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + // should not work as this wouldn't be set in the browser if the AUT url was top anyway + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie1', + value: 'value1', + sameSite: 'strict', + }), 'https://www.barbaz.com/test-request', 'none') + + // should not work as this wouldn't be set in the browser if the AUT url was top anyway + expect(cookieJar.setCookie).not.to.have.been.calledWith(sinon.match({ + key: 'cookie2', + value: 'value2', + sameSite: 'lax', + }), 'https://www.barbaz.com/test-request', 'none') + + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie3', + value: 'value3', + sameSite: 'none', + }), 'https://www.barbaz.com/test-request', 'none') + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + + it('does not set third-party cookie context in the jar when simulating top if withCredentials false with xhr', async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a cross-site request that has the ability to set cookies in the browser + requestedWith: 'xhr', + credentialsLevel: false, + proxiedUrl: 'https://www.barbaz.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1; SameSite=Strict', 'cookie2=value2; SameSite=Lax', 'cookie3=value3; SameSite=None; Secure'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'http://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + + // send to the browser, even though the browser will NOT set this cookie + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None; Secure') + }) + }) + + it(`does NOT set third-party cookie context in the jar if secure cookie is not enabled`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + // a cross-site request that has the ability to set cookies in the browser + requestedWith: 'xhr', + credentialsLevel: true, + proxiedUrl: 'https://www.barbaz.com/test-request', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie3=value3; SameSite=None'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + + // send to browser anyway even though these will likely fail to be set + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie3=value3; SameSite=None') + }) + + it(`allows setting cookies if request type cannot be determined, but comes from the AUT frame (likely in the case of documents or redirects)`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + isAUTFrame: true, + proxiedUrl: 'https://www.barbaz.com/index.html', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie=value'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).to.have.been.calledWith(sinon.match({ + key: 'cookie', + value: 'value', + sameSite: 'lax', + }), 'https://www.barbaz.com/index.html', 'lax') + + // send to browser anyway even though these will likely fail to be set + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it(`otherwise, does not allow setting cookies if request type cannot be determined and is not from the AUT and is cross-origin`, async function () { + const appendStub = sinon.stub() + + const cookieJar = { + getAllCookies: () => [{ key: 'cookie', value: 'value' }], + setCookie: sinon.stub(), + } + + const ctx = prepareContext({ + cookieJar, + res: { + append: appendStub, + }, + req: { + proxiedUrl: 'https://www.barbaz.com/some-image.png', + }, + incomingRes: { + headers: { + 'set-cookie': ['cookie=value'], + }, + }, + }) + + // a case where top would need to be simulated + ctx.getAUTUrl = () => 'https://www.foobar.com/index.html' + ctx.remoteStates.isPrimaryOrigin = () => false + + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) + + expect(cookieJar.setCookie).not.to.have.been.called + + // send to browser anyway even though these will likely fail to be set + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('does not send cross:origin:automation:cookies if request does not need top simulation', async () => { const { ctx } = prepareSameOriginContext() - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(ctx.serverBus.emit).not.to.be.called }) @@ -647,7 +1354,7 @@ describe('http/response-middleware', function () { }, }) - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(ctx.serverBus.emit).not.to.be.called }) @@ -673,7 +1380,7 @@ describe('http/response-middleware', function () { // that we move on once receiving this event ctx.serverBus.once.withArgs('cross:origin:automation:cookies:received').yields() - await testMiddleware([CopyCookiesFromIncomingRes], ctx) + await testMiddleware([MaybeCopyCookiesFromIncomingRes], ctx) expect(ctx.serverBus.emit).to.be.calledWith('cross:origin:automation:cookies') From bfa5e855d4006207254fa50203bbada594f0f670 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Thu, 22 Sep 2022 16:01:08 -0400 Subject: [PATCH 7/7] Update packages/proxy/lib/http/request-middleware.ts Co-authored-by: Matt Henkes --- packages/proxy/lib/http/request-middleware.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts index a6d0ffcdf46d..b37b1aecd5b9 100644 --- a/packages/proxy/lib/http/request-middleware.ts +++ b/packages/proxy/lib/http/request-middleware.ts @@ -37,8 +37,8 @@ const ExtractCypressMetadataHeaders: RequestMiddleware = function () { if (!this.config.experimentalSessionAndOrigin || !doesTopNeedToBeSimulated(this) || - // this should be unreachable happen as the x-cypress-request header is only attached if the resource type is 'xhr' - // inside the extension or electron equivalent. This should only be needed for defensive purposes + // this should be unreachable, as the x-cypress-request header is only attached if the resource type is 'xhr' + // inside the extension or electron equivalent. This is only needed for defensive purposes. (requestIsXhrOrFetch !== 'true' && requestIsXhrOrFetch !== 'xhr' && requestIsXhrOrFetch !== 'fetch')) { this.next()