diff --git a/packages/driver/cypress/fixtures/auth/cookie-login.html b/packages/driver/cypress/fixtures/auth/cookie-login.html new file mode 100644 index 0000000000..4a439d400b --- /dev/null +++ b/packages/driver/cypress/fixtures/auth/cookie-login.html @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + diff --git a/packages/driver/cypress/fixtures/multi-domain.html b/packages/driver/cypress/fixtures/multi-domain.html index f41b916962..bd497c4ab5 100644 --- a/packages/driver/cypress/fixtures/multi-domain.html +++ b/packages/driver/cypress/fixtures/multi-domain.html @@ -19,6 +19,8 @@ http://www.foobar.com:3500/fixtures/files-form.html http://www.foobar.com:3500/fixtures/errors.html http://www.foobar.com:3500/fixtures/screenshots.html + Login with Social + Go to Welcome diff --git a/packages/driver/cypress/integration/e2e/multi-domain/cookie_login_spec.ts b/packages/driver/cypress/integration/e2e/multi-domain/cookie_login_spec.ts new file mode 100644 index 0000000000..b4c815c4fd --- /dev/null +++ b/packages/driver/cypress/integration/e2e/multi-domain/cookie_login_spec.ts @@ -0,0 +1,96 @@ +// @ts-ignore / session support is needed for visiting about:blank between tests +describe('multi-domain - cookie login', { experimentalSessionSupport: true }, () => { + const verifyLoggedIn = (username) => { + cy.get('h1') + .invoke('text') + .should('equal', `Welcome, ${username}!`) + } + + it('works in a session', () => { + cy.session('ZJohnson', () => { + cy.visit('/fixtures/multi-domain.html') + cy.get('[data-cy="cookie-login"]').click() + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="username"]').type('ZJohnson') + cy.get('[data-cy="login"]').click() + }) + }, { + validate () { + cy.getCookie('user').its('value').should('equal', 'ZJohnson') + }, + }) + + cy.visit('/welcome') + verifyLoggedIn('ZJohnson') + }) + + describe('SameSite handling', () => { + beforeEach(() => { + cy.visit('/fixtures/multi-domain.html') + cy.get('[data-cy="cookie-login"]').click() + }) + + it('works with no SameSite, no Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="username"]').type('AJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('AJohnson') + }) + + it('works with SameSite=None, Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="sameSite"]').select('None') + cy.get('[data-cy="secure"]').check() + cy.get('[data-cy="username"]').type('BJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('BJohnson') + }) + + it('works with SameSite=None, no Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="sameSite"]').select('None') + cy.get('[data-cy="username"]').type('CJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('CJohnson') + }) + + it('works with SameSite=Lax, Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="sameSite"]').select('Lax') + cy.get('[data-cy="secure"]').check() + cy.get('[data-cy="username"]').type('DJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('DJohnson') + }) + + it('works with SameSite=Strict, Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="sameSite"]').select('Strict') + cy.get('[data-cy="secure"]').check() + cy.get('[data-cy="username"]').type('EJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('EJohnson') + }) + + it('works with invalid SameSite, Secure', () => { + cy.switchToDomain('http://foobar.com:3500', () => { + cy.get('[data-cy="sameSite"]').select('Invalid') + cy.get('[data-cy="secure"]').check() + cy.get('[data-cy="username"]').type('FJohnson') + cy.get('[data-cy="login"]').click() + }) + + verifyLoggedIn('FJohnson') + }) + }) +}) diff --git a/packages/driver/cypress/plugins/server.js b/packages/driver/cypress/plugins/server.js index 84da935835..91694154bc 100644 --- a/packages/driver/cypress/plugins/server.js +++ b/packages/driver/cypress/plugins/server.js @@ -10,7 +10,6 @@ const multer = require('multer') const upload = multer({ dest: 'cypress/_test-output/' }) const PATH_TO_SERVER_PKG = path.dirname(require.resolve('@packages/server')) -const { getPathToDist } = require('@packages/resolve-dist') const httpPorts = [3500, 3501] const httpsPort = 3502 @@ -27,6 +26,7 @@ const createApp = (port) => { }) app.use(require('cors')()) + app.use(require('cookie-parser')()) app.use(require('compression')()) app.use(bodyParser.urlencoded({ extended: false })) app.use(bodyParser.json()) @@ -190,9 +190,67 @@ const createApp = (port) => { .send('server error') }) - app.get('/cypress_multi_domain_runner.js', (req, res) => { - res.type('application/javascript') - res.sendFile(getPathToDist('runner', 'cypress_multi_domain_runner.js')) + const getCookieAdditions = ({ sameSite, secure }) => { + let additions = '' + + if (sameSite) additions += `; SameSite=${sameSite}` + + if (secure) additions += '; Secure' + + return additions + } + + app.get('/cookie-login', (req, res) => { + const { username, redirect } = req.query + + res + .header('Set-Cookie', `user=${username}${getCookieAdditions(req.query)}`) + .redirect(302, `/verify-cookie-login?username=${username}&redirect=${redirect}`) + }) + + app.get('/verify-cookie-login', (req, res) => { + if (!req.cookies.user) { + return res + .status(403) + .send('

Not logged in

') + } + + const { username, redirect } = req.query + + res.send(` + + +

Redirecting ${username}...

+ + + + `) + }) + + app.get('/login', (req, res) => { + const { username } = req.query + + if (!username) { + return res.send('

Must specify username to log in

') + } + + // can't use res.cookie() because it won't allow setting an invalid + // SameSite value, which we want to test + res + .header('Set-Cookie', `user=${username}${getCookieAdditions(req.query)}`) + .redirect(302, '/welcome') + }) + + app.get('/welcome', (req, res) => { + if (!req.cookies.user) { + return res.send('

No user found

') + } + + res.send(`

Welcome, ${req.cookies.user}!

`) }) let _var = '' diff --git a/packages/driver/package.json b/packages/driver/package.json index 61ead24b61..ff5ad4829a 100644 --- a/packages/driver/package.json +++ b/packages/driver/package.json @@ -46,6 +46,7 @@ "chokidar-cli": "2.1.0", "clone": "2.1.2", "compression": "1.7.4", + "cookie-parser": "1.4.5", "core-js-pure": "3.21.0", "cors": "2.8.5", "cypress-multi-reporters": "1.4.0", diff --git a/packages/driver/src/cypress/cy.ts b/packages/driver/src/cypress/cy.ts index 7a4fd0906b..cefeb207b8 100644 --- a/packages/driver/src/cypress/cy.ts +++ b/packages/driver/src/cypress/cy.ts @@ -936,9 +936,6 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert // TODO: handle no longer error when ended early cy.doneEarly() - // if using multi-domain, unbind any listeners waiting for a done() callback to come from cross domain - // @ts-ignore - Cypress.multiDomainCommunicator.emit('unbind:done:called') originalDone(err) // return null else we there are situations diff --git a/packages/network/lib/cors.ts b/packages/network/lib/cors.ts index 498d2b2383..0a56b96ecc 100644 --- a/packages/network/lib/cors.ts +++ b/packages/network/lib/cors.ts @@ -84,6 +84,15 @@ export function urlMatchesOriginPolicyProps (urlStr, props) { return _.isEqual(parsedUrl, props) } +export function urlOriginsMatch (url1, url2) { + if (!url1 || !url2) return false + + const parsedUrl1 = parseUrlIntoDomainTldPort(url1) + const parsedUrl2 = parseUrlIntoDomainTldPort(url2) + + return _.isEqual(parsedUrl1, parsedUrl2) +} + export function urlMatchesOriginProtectionSpace (urlStr, origin) { const normalizedUrl = uri.addDefaultPort(urlStr).format() const normalizedOrigin = uri.addDefaultPort(origin).format() diff --git a/packages/network/lib/uri.ts b/packages/network/lib/uri.ts index 1d88cf47c1..e7e8d83f7b 100644 --- a/packages/network/lib/uri.ts +++ b/packages/network/lib/uri.ts @@ -6,7 +6,7 @@ // - https://nodejs.org/api/url.html#url_url_format_urlobject import _ from 'lodash' -import url from 'url' +import url, { URL } from 'url' // yup, protocol contains a: ':' colon // at the end of it (-______________-) @@ -87,3 +87,19 @@ export function addDefaultPort (urlToCheck) { export function getPath (urlToCheck) { return url.parse(urlToCheck).path } + +const localhostIPRegex = /^127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$/ + +export function isLocalhost (url: URL) { + return ( + // https://datatracker.ietf.org/doc/html/draft-west-let-localhost-be-localhost#section-2 + url.hostname === 'localhost' + || url.hostname.endsWith('.localhost') + // [::1] is the IPv6 localhost address + // See https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.3 + || url.hostname === '[::1]' + // 127.0.0.0/8 are considered localhost for IPv4 + // See https://datatracker.ietf.org/doc/html/rfc5735 (Page 3) + || localhostIPRegex.test(url.hostname) + ) +} diff --git a/packages/network/test/unit/uri_spec.ts b/packages/network/test/unit/uri_spec.ts new file mode 100644 index 0000000000..3d8fe82c95 --- /dev/null +++ b/packages/network/test/unit/uri_spec.ts @@ -0,0 +1,40 @@ +import { expect } from 'chai' +import { URL } from 'url' + +import { uri } from '../../lib' + +describe('lib/uri', () => { + context('.isLocalhost', () => { + it('http://localhost is localhost', () => { + expect(uri.isLocalhost(new URL('http://localhost'))).to.be.true + }) + + it('https://localhost is localhost', () => { + expect(uri.isLocalhost(new URL('https://localhost'))).to.be.true + }) + + it('http://127.0.0.1 is localhost', () => { + expect(uri.isLocalhost(new URL('http://127.0.0.1'))).to.be.true + }) + + it('http://127.0.0.9 is localhost', () => { + expect(uri.isLocalhost(new URL('http://127.0.0.9'))).to.be.true + }) + + it('http://[::1] is localhost', () => { + expect(uri.isLocalhost(new URL('http://[::1]'))).to.be.true + }) + + it('http://128.0.0.1 is NOT localhost', () => { + expect(uri.isLocalhost(new URL('http://128.0.0.1'))).to.be.false + }) + + it('http:foobar.com is NOT localhost', () => { + expect(uri.isLocalhost(new URL('http:foobar.com'))).to.be.false + }) + + it('https:foobar.com is NOT localhost', () => { + expect(uri.isLocalhost(new URL('https:foobar.com'))).to.be.false + }) + }) +}) diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts index b1c94901c4..1b176391c5 100644 --- a/packages/proxy/lib/http/index.ts +++ b/packages/proxy/lib/http/index.ts @@ -18,6 +18,7 @@ import type { Request, Response } from 'express' import RequestMiddleware from './request-middleware' import ResponseMiddleware from './response-middleware' import { DeferredSourceMapCache } from '@packages/rewriter' +import type { Browser } from '@packages/server/lib/browsers/types' export const debugVerbose = Debug('cypress-verbose:proxy:http') @@ -43,7 +44,10 @@ type HttpMiddlewareCtx = { debug: Debug.Debugger middleware: HttpMiddlewareStacks deferSourceMapRewrite: (opts: { js: string, url: string }) => string + getCurrentBrowser: () => Browser | Partial & Pick | null getPreRequest: (cb: GetPreRequestCb) => void + getPreviousAUTRequestUrl: Http['getPreviousAUTRequestUrl'] + setPreviousAUTRequestUrl: Http['setPreviousAUTRequestUrl'] } & T export const defaultMiddleware = { @@ -55,6 +59,7 @@ export const defaultMiddleware = { export type ServerCtx = Readonly<{ config: CyServer.Config & Cypress.Config shouldCorrelatePreRequests?: () => boolean + getCurrentBrowser: () => Browser | Partial & Pick | null getFileServerToken: () => string getRemoteState: CyServer.getRemoteState getRenderedHTMLOrigins: Http['getRenderedHTMLOrigins'] @@ -78,7 +83,7 @@ const READONLY_MIDDLEWARE_KEYS: (keyof HttpMiddlewareThis<{}>)[] = [ 'skipMiddleware', ] -type HttpMiddlewareThis = HttpMiddlewareCtx & ServerCtx & Readonly<{ +export type HttpMiddlewareThis = HttpMiddlewareCtx & ServerCtx & Readonly<{ buffers: HttpBuffers next: () => void @@ -194,6 +199,7 @@ export class Http { config: CyServer.Config shouldCorrelatePreRequests: () => boolean deferredSourceMapCache: DeferredSourceMapCache + getCurrentBrowser: () => Browser | Partial & Pick | null getFileServerToken: () => string getRemoteState: () => any middleware: HttpMiddlewareStacks @@ -203,6 +209,7 @@ export class Http { socket: CyServer.Socket serverBus: EventEmitter renderedHTMLOrigins: {[key: string]: boolean} = {} + previousAUTRequestUrl?: string constructor (opts: ServerCtx & { middleware?: HttpMiddlewareStacks }) { this.buffers = new HttpBuffers() @@ -210,6 +217,7 @@ export class Http { this.config = opts.config this.shouldCorrelatePreRequests = opts.shouldCorrelatePreRequests || (() => false) + this.getCurrentBrowser = opts.getCurrentBrowser this.getFileServerToken = opts.getFileServerToken this.getRemoteState = opts.getRemoteState this.middleware = opts.middleware @@ -230,6 +238,7 @@ export class Http { buffers: this.buffers, config: this.config, shouldCorrelatePreRequests: this.shouldCorrelatePreRequests, + getCurrentBrowser: this.getCurrentBrowser, getFileServerToken: this.getFileServerToken, getRemoteState: this.getRemoteState, request: this.request, @@ -247,6 +256,8 @@ export class Http { }) }, getRenderedHTMLOrigins: this.getRenderedHTMLOrigins, + getPreviousAUTRequestUrl: this.getPreviousAUTRequestUrl, + setPreviousAUTRequestUrl: this.setPreviousAUTRequestUrl, getPreRequest: (cb) => { this.preRequests.get(ctx.req, ctx.debug, cb) }, @@ -280,6 +291,14 @@ export class Http { return this.renderedHTMLOrigins } + getPreviousAUTRequestUrl = () => { + return this.previousAUTRequestUrl + } + + setPreviousAUTRequestUrl = (url) => { + this.previousAUTRequestUrl = url + } + async handleSourceMapRequest (req: Request, res: Response) { try { const sm = await this.deferredSourceMapCache.resolve(req.params.id, req.headers) @@ -296,6 +315,7 @@ export class Http { reset () { this.buffers.reset() + this.setPreviousAUTRequestUrl(undefined) } setBuffer (buffer) { diff --git a/packages/proxy/lib/http/response-middleware.ts b/packages/proxy/lib/http/response-middleware.ts index b0bdee09da..4f0915256a 100644 --- a/packages/proxy/lib/http/response-middleware.ts +++ b/packages/proxy/lib/http/response-middleware.ts @@ -1,18 +1,20 @@ import _ from 'lodash' import charset from 'charset' import type { CookieOptions } from 'express' -import { cors, concatStream, httpUtils } from '@packages/network' +import { cors, concatStream, httpUtils, uri } from '@packages/network' import type { CypressIncomingRequest, CypressOutgoingResponse } from '@packages/proxy' import debugModule from 'debug' -import type { HttpMiddleware } from '.' +import type { HttpMiddleware, HttpMiddlewareThis } from '.' import iconv from 'iconv-lite' import type { IncomingMessage, IncomingHttpHeaders } from 'http' import { InterceptResponse } from '@packages/net-stubbing' import { PassThrough, Readable } from 'stream' import * as rewriter from './util/rewriter' import zlib from 'zlib' +import { URL } from 'url' +import type { Browser } from '@packages/server/lib/browsers/types' -export type ResponseMiddleware = HttpMiddleware<{ +interface ResponseMiddlewareProps { /** * Before using `res.incomingResStream`, `prepareResStream` can be used * to remove any encoding that prevents it from being returned as plain text. @@ -23,7 +25,9 @@ export type ResponseMiddleware = HttpMiddleware<{ isGunzipped: boolean incomingRes: IncomingMessage incomingResStream: Readable -}> +} + +export type ResponseMiddleware = HttpMiddleware const debug = debugModule('cypress:proxy:http:response-middleware') @@ -381,11 +385,89 @@ const MaybePreventCaching: ResponseMiddleware = function () { this.next() } +const determineIfNeedsMultiDomainHandling = (ctx: HttpMiddlewareThis) => { + const previousAUTRequestUrl = ctx.getPreviousAUTRequestUrl() + + // A cookie needs multi-domain handling if it's an AUT request and + // either 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-domain -> primary-domain, we don't + // recognize the request as cross-origin + return ( + !!ctx.req.isAUTFrame && + ( + (previousAUTRequestUrl && !cors.urlOriginsMatch(previousAUTRequestUrl, ctx.req.proxiedUrl)) + || !reqMatchesOriginPolicy(ctx.req, ctx.getRemoteState()) + ) + ) +} + +interface EnsureSameSiteNoneProps { + cookie: string + browser: Browser | { family: string | null } + isLocalhost: boolean + url: URL +} + +const cookieSameSiteRegex = /SameSite=(\w+)/i +const cookieSecureRegex = /Secure/i +const cookieSecureSemicolonRegex = /;\s*Secure/i + +const ensureSameSiteNone = ({ cookie, browser, isLocalhost, url }: EnsureSameSiteNoneProps) => { + debug('original cookie: %s', cookie) + + if (cookieSameSiteRegex.test(cookie)) { + debug('change cookie to SameSite=None') + cookie = cookie.replace(cookieSameSiteRegex, 'SameSite=None') + } else { + debug('add SameSite=None to cookie') + cookie += '; SameSite=None' + } + + const isFirefox = browser.family === 'firefox' + + // Secure is required for SameSite=None cookies to be set in secure contexts + // (https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy), + // but will not allow the cookie to be set in an insecure context. + // Normally http://localhost is considered a secure context (see + // https://w3c.github.io/webappsec-secure-contexts/#localhost), but Firefox + // does not consider the Cypress-launched browser to be a secure context (see + // https://github.com/cypress-io/cypress/issues/18217). For that reason, we + // remove Secure from http://localhost cookies in Firefox. + if (cookieSecureRegex.test(cookie)) { + if (isFirefox && isLocalhost && url.protocol === 'http:') { + debug('remove Secure from cookie') + cookie = cookie.replace(cookieSecureSemicolonRegex, '') + } + } else if (!isFirefox || url.protocol === 'https:') { + debug('add Secure to cookie') + cookie += '; Secure' + } + + debug('resulting cookie: %s', cookie) + + return cookie +} + const CopyCookiesFromIncomingRes: ResponseMiddleware = function () { const cookies: string | string[] | undefined = this.incomingRes.headers['set-cookie'] if (cookies) { - ([] as string[]).concat(cookies).forEach((cookie) => { + const needsMultiDomainHandling = ( + this.config.experimentalMultiDomain + && determineIfNeedsMultiDomainHandling(this) + ) + const browser = this.getCurrentBrowser() || { family: null } + const url = new URL(this.req.proxiedUrl) + const isLocalhost = uri.isLocalhost(url) + + debug('force SameSite=None?', needsMultiDomainHandling) + + ;([] as string[]).concat(cookies).forEach((cookie) => { + if (needsMultiDomainHandling) { + cookie = ensureSameSiteNone({ cookie, browser, isLocalhost, url }) + } + try { this.res.append('Set-Cookie', cookie) } catch (err) { @@ -501,6 +583,12 @@ const GzipBody: ResponseMiddleware = function () { } const SendResponseBodyToClient: ResponseMiddleware = function () { + if (this.req.isAUTFrame) { + // track the previous AUT request URL so we know if the next requests + // is cross-origin + this.setPreviousAUTRequestUrl(this.req.proxiedUrl) + } + this.incomingResStream.pipe(this.res).on('error', this.onError) this.res.on('end', () => this.end()) } diff --git a/packages/proxy/test/integration/net-stubbing.spec.ts b/packages/proxy/test/integration/net-stubbing.spec.ts index a3ee110267..580cb30da3 100644 --- a/packages/proxy/test/integration/net-stubbing.spec.ts +++ b/packages/proxy/test/integration/net-stubbing.spec.ts @@ -38,6 +38,7 @@ context('network stubbing', () => { netStubbingState, config, middleware: defaultMiddleware, + getCurrentBrowser: () => ({ family: 'chromium' }), getRemoteState: () => remoteState, getFileServerToken: () => 'fake-token', request: new Request(), diff --git a/packages/proxy/test/unit/http/response-middleware.spec.ts b/packages/proxy/test/unit/http/response-middleware.spec.ts index 5021ce7fba..700bd94899 100644 --- a/packages/proxy/test/unit/http/response-middleware.spec.ts +++ b/packages/proxy/test/unit/http/response-middleware.spec.ts @@ -478,6 +478,368 @@ describe('http/response-middleware', function () { } } }) + + describe('CopyCookiesFromIncomingRes', function () { + const { CopyCookiesFromIncomingRes } = ResponseMiddleware + + it('appends cookies on the response when an array', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'set-cookie': ['cookie1=value1', 'cookie2=value2'], + }, + }, + res: { + append: appendStub, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledTwice + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie1=value1') + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie2=value2') + }) + + it('appends cookies on the response when a string', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'set-cookie': 'cookie=value', + }, + }, + res: { + append: appendStub, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('is a noop when cookies are undefined', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + res: { + append: appendStub, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).not.to.be.called + }) + + describe('SameSite', function () { + it('forces SameSite=None when an AUT request and does not match origin policy', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + req: { + isAUTFrame: true, + }, + res: { + append: appendStub, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value; SameSite=None; Secure') + }) + + it('forces SameSite=None when an AUT request and last AUT request was a different origin', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + req: { + isAUTFrame: true, + proxiedUrl: 'https://site2.com', + }, + res: { + append: appendStub, + }, + getPreviousAUTRequestUrl () { + return 'https://different.site' + }, + getRemoteState () { + // nonsense, but it's the simplest way to match origin policy + return { + strategy: 'file', + origin: 'http', + } + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value; SameSite=None; Secure') + }) + + it('does not force SameSite=None if not an AUT request', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + res: { + append: appendStub, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('does not force SameSite=None if the first AUT request', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + req: { + isAUTFrame: true, + }, + res: { + append: appendStub, + }, + getRemoteState () { + // nonsense, but it's the simplest way to match origin policy + return { + strategy: 'file', + origin: 'http', + } + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('does not force SameSite=None if an AUT request but not cross-origin', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + req: { + isAUTFrame: true, + }, + res: { + append: appendStub, + }, + getRemoteState () { + // nonsense, but it's the simplest way to match origin policy + return { + strategy: 'file', + origin: 'http', + } + }, + }) + + ctx.getPreviousAUTRequestUrl = () => ctx.req.proxiedUrl + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + it('does not force SameSite=None if experimental flag is off', async function () { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': 'cookie=value', + }, + }, + req: { + isAUTFrame: true, + }, + res: { + append: appendStub, + }, + config: { + experimentalMultiDomain: false, + }, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledWith('Set-Cookie', 'cookie=value') + }) + + describe('cookie modification scenarios', function () { + const makeScenarios = (output, flippedOutput?) => { + return [ + ['SameSite=Strict; Secure', output], + ['SameSite=Strict', output], + ['SameSite=Lax; Secure', output], + ['SameSite=Lax', output], + ['SameSite=Invalid; Secure', output], + ['SameSite=Invalid', output], + ['SameSite=None', output], + ['', output], + // When there's Secure and no SameSite, it ends up as + // "Secure; SameSite=None" instead of "Secure" being second + ['Secure', flippedOutput || output], + ] + } + + const withFirefox = { + getCurrentBrowser: () => ({ family: 'firefox' }), + } + + describe('not Firefox', function () { + makeScenarios('SameSite=None; Secure', 'Secure; SameSite=None').forEach(([input, output]) => { + it(`${input} -> ${output}`, async function () { + const { appendStub, ctx } = prepareContextWithCookie(`cookie=value${input ? '; ' : ''}${input}`) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', `cookie=value; ${output}`) + }) + }) + }) + + describe('Firefox + non-localhost', function () { + makeScenarios('SameSite=None; Secure', 'Secure; SameSite=None').forEach(([input, output]) => { + it(`${input} -> ${output}`, async function () { + const { appendStub, ctx } = prepareContextWithCookie(`cookie=value${input ? '; ' : ''}${input}`, { + req: { proxiedUrl: 'https://foobar.com' }, + ...withFirefox, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', `cookie=value; ${output}`) + }) + }) + }) + + describe('Firefox + https://localhost', function () { + makeScenarios('SameSite=None; Secure', 'Secure; SameSite=None').forEach(([input, output]) => { + it(`${input} -> ${output}`, async function () { + const { appendStub, ctx } = prepareContextWithCookie(`cookie=value${input ? '; ' : ''}${input}`, { + req: { proxiedUrl: 'https://localhost:3500' }, + ...withFirefox, + }) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', `cookie=value; ${output}`) + }) + }) + }) + + describe('Firefox + http://localhost', function () { + makeScenarios('SameSite=None').forEach(([input, output]) => { + it(`${input} -> ${output}`, async function () { + const { appendStub, ctx } = prepareContextWithCookie(`cookie=value${input ? '; ' : ''}${input}`, withFirefox) + + await testMiddleware([CopyCookiesFromIncomingRes], ctx) + + expect(appendStub).to.be.calledOnce + expect(appendStub).to.be.calledWith('Set-Cookie', `cookie=value; ${output}`) + }) + }) + }) + }) + }) + + function prepareContext (props) { + return { + incomingRes: { + headers: {}, + ...props.incomingRes, + }, + res: { + headers: {}, + on () {}, + ...props.res, + }, + req: { + proxiedUrl: 'http:127.0.0.1:3501/multi-domain.html', + headers: {}, + ...props.req, + }, + incomingResStream: { + pipe () { + return { on () {} } + }, + }, + config: { + experimentalMultiDomain: true, + }, + getCurrentBrowser () { + return { family: 'chromium' } + }, + getPreviousAUTRequestUrl () {}, + getRemoteState () { + return { + strategy: 'foo', + } + }, + debug () {}, + onError (error) { + throw error + }, + ..._.omit(props, 'incomingRes', 'res', 'req'), + } + } + + function prepareContextWithCookie (cookie, props: any = {}) { + const appendStub = sinon.stub() + const ctx = prepareContext({ + incomingRes: { + headers: { + 'content-type': 'text/html', + 'set-cookie': cookie, + }, + }, + req: { + isAUTFrame: true, + ...props.req, + }, + res: { + append: appendStub, + }, + ..._.omit(props, 'incomingRes', 'res', 'req'), + }) + + return { appendStub, ctx } + } + }) }) // beforeEach(function () { diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts index 85856bfe8b..d7dd6322f7 100644 --- a/packages/server/lib/browsers/cdp_automation.ts +++ b/packages/server/lib/browsers/cdp_automation.ts @@ -3,8 +3,10 @@ import _ from 'lodash' import Bluebird from 'bluebird' import type { Protocol } from 'devtools-protocol' -import { cors } from '@packages/network' +import { cors, uri } from '@packages/network' import debugModule from 'debug' +import { URL } from 'url' + import type { Automation } from '../automation' import type { ResourceType, BrowserPreRequest, BrowserResponseReceived } from '@packages/proxy' @@ -174,10 +176,25 @@ const ffToStandardResourceTypeMap: { [ff: string]: ResourceType } = { 'webmanifest': 'manifest', } +export interface CdpOptions { + sendDebuggerCommandFn: SendDebuggerCommand + onFn: OnFn + automation: Automation + experimentalMultiDomain: boolean +} + export class CdpAutomation { - constructor (private sendDebuggerCommandFn: SendDebuggerCommand, onFn: OnFn, private automation: Automation) { - onFn('Network.requestWillBeSent', this.onNetworkRequestWillBeSent) - onFn('Network.responseReceived', this.onResponseReceived) + sendDebuggerCommandFn: SendDebuggerCommand + automation: Automation + experimentalMultiDomain: boolean + + constructor (options: CdpOptions) { + this.sendDebuggerCommandFn = options.sendDebuggerCommandFn + this.automation = options.automation + this.experimentalMultiDomain = options.experimentalMultiDomain + + options.onFn('Network.requestWillBeSent', this.onNetworkRequestWillBeSent) + options.onFn('Network.responseReceived', this.onResponseReceived) } async enable () { @@ -238,8 +255,23 @@ export class CdpAutomation { urls: [url], }) .then((result: Protocol.Network.GetCookiesResponse) => { + const isLocalhost = uri.isLocalhost(new URL(url)) + return normalizeGetCookies(result.cookies) .filter((cookie) => { + // Chrome returns all cookies for a URL, even if they wouldn't normally + // be sent with a request. This standardizes it by filtering out ones + // that are secure but not on a secure context + + if (this.experimentalMultiDomain) { + // localhost is considered a secure context (even when http:) + // and it's required for multi-domain when visiting a secondary + // domain so that all its cookies are sent. This may be a + // breaking change, so put it behind the flag for now. Need to + // investigate further when we remove the experimental flag. + return !(cookie.secure && url.startsWith('http:') && !isLocalhost) + } + return !(url.startsWith('http:') && cookie.secure) }) }) diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index d253425def..d1ae7c8716 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -434,8 +434,13 @@ const _handlePausedRequests = async (client) => { }) } -const _setAutomation = async (client, automation) => { - const cdpAutomation = new CdpAutomation(client.send, client.on, automation) +const _setAutomation = async (client, automation, options) => { + const cdpAutomation = new CdpAutomation({ + sendDebuggerCommandFn: client.send, + onFn: client.on, + automation, + experimentalMultiDomain: options.experimentalMultiDomain, + }) await cdpAutomation.enable() @@ -623,7 +628,7 @@ export = { throw new Error(`Cypress requires at least Chrome 64.\n\nDetails:\n${err.message}`) }) - await this._setAutomation(criClient, automation) + await this._setAutomation(criClient, automation, options) // monkey-patch the .kill method to that the CDP connection is closed const originalBrowserKill = launchedBrowser.kill diff --git a/packages/server/lib/browsers/electron.js b/packages/server/lib/browsers/electron.js index d9df9a71dc..af8ce03842 100644 --- a/packages/server/lib/browsers/electron.js +++ b/packages/server/lib/browsers/electron.js @@ -51,7 +51,12 @@ const _getAutomation = async function (win, options, parent) { }) } - const automation = new CdpAutomation(sendCommand, on, parent) + const automation = new CdpAutomation({ + sendDebuggerCommandFn: sendCommand, + onFn: on, + automation: parent, + experimentalMultiDomain: options.experimentalMultiDomain, + }) await automation.enable() diff --git a/packages/server/lib/browsers/firefox-util.ts b/packages/server/lib/browsers/firefox-util.ts index 6e42b974d5..562ee47591 100644 --- a/packages/server/lib/browsers/firefox-util.ts +++ b/packages/server/lib/browsers/firefox-util.ts @@ -101,10 +101,15 @@ const attachToTabMemory = Bluebird.method((tab) => { }) }) -async function setupRemote (remotePort, automation, onError) { +async function setupRemote (remotePort, automation, options) { const wsUrl = await protocol.getWsTargetFor(remotePort, 'Firefox') - const criClient = await CriClient.create(wsUrl, onError) - const cdpAutomation = new CdpAutomation(criClient.send, criClient.on, automation) + const criClient = await CriClient.create(wsUrl, options.onError) + const cdpAutomation = new CdpAutomation({ + sendDebuggerCommandFn: criClient.send, + onFn: criClient.on, + automation, + experimentalMultiDomain: options.experimentalMultiDomain, + }) await cdpAutomation.enable() } @@ -176,16 +181,16 @@ export default { setup ({ automation, extensions, - onError, url, marionettePort, foxdriverPort, remotePort, + options, }) { return Bluebird.all([ this.setupFoxdriver(foxdriverPort), this.setupMarionette(extensions, url, marionettePort), - remotePort && setupRemote(remotePort, automation, onError), + remotePort && setupRemote(remotePort, automation, options), ]) }, diff --git a/packages/server/lib/browsers/firefox.ts b/packages/server/lib/browsers/firefox.ts index 97e5fe2db9..9c102afd2c 100644 --- a/packages/server/lib/browsers/firefox.ts +++ b/packages/server/lib/browsers/firefox.ts @@ -510,7 +510,14 @@ export async function open (browser: Browser, url, options: any = {}, automation }) try { - await firefoxUtil.setup({ automation, extensions: launchOptions.extensions, url, foxdriverPort, marionettePort, remotePort, onError: options.onError }) + await firefoxUtil.setup({ automation, + extensions: launchOptions.extensions, + url, + foxdriverPort, + marionettePort, + remotePort, + options, + }) } catch (err) { errors.throw('FIREFOX_COULD_NOT_CONNECT', err) } diff --git a/packages/server/lib/open_project.ts b/packages/server/lib/open_project.ts index e6d97baadf..8a3b02232e 100644 --- a/packages/server/lib/open_project.ts +++ b/packages/server/lib/open_project.ts @@ -148,6 +148,7 @@ export class OpenProject { chromeWebSecurity: cfg.chromeWebSecurity, isTextTerminal: cfg.isTextTerminal, downloadsFolder: cfg.downloadsFolder, + experimentalMultiDomain: cfg.experimentalMultiDomain, }) // if we don't have the isHeaded property diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index 7bab674d5c..91e5244a85 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -39,7 +39,7 @@ import type { LaunchArgs } from './open_project' // and are required when creating a project. type ReceivedCypressOptions = Pick - & Pick // TODO: Figure out how to type this better. + & Pick // TODO: Figure out how to type this better. export interface Cfg extends ReceivedCypressOptions { projectRoot: string diff --git a/packages/server/lib/server-base.ts b/packages/server/lib/server-base.ts index 7a0dc17385..07a1f1c17b 100644 --- a/packages/server/lib/server-base.ts +++ b/packages/server/lib/server-base.ts @@ -216,7 +216,7 @@ export abstract class ServerBase { return this._getRemoteState() } - this.createNetworkProxy(config, getRemoteState, shouldCorrelatePreRequests) + this.createNetworkProxy({ config, getCurrentBrowser, getRemoteState, shouldCorrelatePreRequests }) if (config.experimentalSourceRewriting) { createInitialWorkers() @@ -304,7 +304,7 @@ export abstract class ServerBase { return e } - createNetworkProxy (config, getRemoteState, shouldCorrelatePreRequests) { + createNetworkProxy ({ config, getCurrentBrowser, getRemoteState, shouldCorrelatePreRequests }) { const getFileServerToken = () => { return this._fileServer.token } @@ -314,6 +314,7 @@ export abstract class ServerBase { this._networkProxy = new NetworkProxy({ config, shouldCorrelatePreRequests, + getCurrentBrowser, getRemoteState, getFileServerToken, socket: this.socket, diff --git a/packages/server/test/integration/server_spec.js b/packages/server/test/integration/server_spec.js index a5398efba6..f6b5976368 100644 --- a/packages/server/test/integration/server_spec.js +++ b/packages/server/test/integration/server_spec.js @@ -89,6 +89,7 @@ describe('Server', () => { createRoutes, specsStore: new SpecsStore({}, 'e2e'), testingType: 'e2e', + getCurrentBrowser: () => null, }) .spread(async (port) => { const automationStub = { diff --git a/packages/server/test/integration/websockets_spec.js b/packages/server/test/integration/websockets_spec.js index 645cf1a045..6f1bcb32e6 100644 --- a/packages/server/test/integration/websockets_spec.js +++ b/packages/server/test/integration/websockets_spec.js @@ -40,6 +40,7 @@ describe('Web Sockets', () => { createRoutes, specsStore: new SpecsStore({}, 'e2e'), testingType: 'e2e', + getCurrentBrowser: () => null, }) .then(async () => { const automationStub = { diff --git a/packages/server/test/performance/proxy_performance_spec.js b/packages/server/test/performance/proxy_performance_spec.js index 6a281b2a8e..0afca66d23 100644 --- a/packages/server/test/performance/proxy_performance_spec.js +++ b/packages/server/test/performance/proxy_performance_spec.js @@ -358,6 +358,7 @@ describe('Proxy Performance', function () { createRoutes, specsStore: new SpecsStore({}, 'e2e'), testingType: 'e2e', + getCurrentBrowser: () => null, }) }), ) diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts index 93f35da92f..d2b7a288f2 100644 --- a/packages/server/test/unit/browsers/cdp_automation_spec.ts +++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts @@ -1,5 +1,6 @@ const { expect, sinon } = require('../../spec_helper') +import { Automation } from '../../../lib/automation' import { CdpAutomation, _cookieMatches, @@ -71,7 +72,12 @@ context('lib/browsers/cdp_automation', () => { this.sendDebuggerCommand = sinon.stub() this.onFn = sinon.stub() - this.automation = new CdpAutomation(this.sendDebuggerCommand, this.onFn) + this.automation = new CdpAutomation({ + sendDebuggerCommandFn: this.sendDebuggerCommand, + onFn: this.onFn, + automation: {} as Automation, + experimentalMultiDomain: false, + }) await this.automation.enable() diff --git a/packages/server/test/unit/socket_spec.js b/packages/server/test/unit/socket_spec.js index b157662051..b5fc100b87 100644 --- a/packages/server/test/unit/socket_spec.js +++ b/packages/server/test/unit/socket_spec.js @@ -47,6 +47,7 @@ describe('lib/socket', () => { createRoutes, specsStore: new SpecsStore({}, 'e2e'), testingType: 'e2e', + getCurrentBrowser: () => null, }) .then(() => { this.options = { @@ -597,6 +598,7 @@ describe('lib/socket', () => { createRoutes, specsStore: new SpecsStore({}, 'e2e'), testingType: 'e2e', + getCurrentBrowser: () => null, }) .then(() => { this.automation = new Automation(this.cfg.namespace, this.cfg.socketIoCookie, this.cfg.screenshotsFolder)