Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve document.cookie patch #23643

Merged
merged 44 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
075def8
patches/cookies.js -> patches/cookies.ts
chrisbreiding Aug 24, 2022
3d1976f
wip - optimistically update document.cookie
chrisbreiding Aug 29, 2022
6e3e1b4
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Aug 31, 2022
38b8bed
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Sep 1, 2022
846679b
revert firefox automation changes
chrisbreiding Sep 7, 2022
b23cc8d
fix: optimistically update document.cookie value when possible
chrisbreiding Sep 7, 2022
cc625db
convert back to js
chrisbreiding Sep 7, 2022
2227311
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Sep 7, 2022
d106c4a
fix test
chrisbreiding Sep 7, 2022
39e85d3
use default value
chrisbreiding Sep 7, 2022
c2d18ee
refactor how state is used/reset
chrisbreiding Sep 7, 2022
97355ac
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Sep 7, 2022
5f0b0ea
remove app build script
chrisbreiding Sep 7, 2022
483e914
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Sep 8, 2022
69fb577
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Oct 3, 2022
2ca2a46
wip
chrisbreiding Oct 5, 2022
ed4203d
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Oct 5, 2022
5d8bb52
wip
chrisbreiding Oct 6, 2022
517d430
wire up cy commands, get cookie login spec passing
chrisbreiding Oct 7, 2022
e728aed
convert cookies patch to typescript
chrisbreiding Oct 7, 2022
dc231b0
improve injection logic
chrisbreiding Oct 13, 2022
512a268
use cookie jar to store document.cookie cookies
chrisbreiding Oct 13, 2022
d8a4186
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Oct 13, 2022
8e3227e
remove polling for automation cookies
chrisbreiding Oct 14, 2022
b90c040
remove need for injection globals
chrisbreiding Oct 14, 2022
02b7ac2
add test for spec bridge existing before page load
chrisbreiding Oct 14, 2022
0300279
cleanup
chrisbreiding Oct 14, 2022
54255fc
set document.cookie cookies in server-side jar as well as automation
chrisbreiding Oct 17, 2022
7ddbc2b
only send one event to set automation cookies
chrisbreiding Oct 17, 2022
f2a1d0e
fix unit tests
chrisbreiding Oct 17, 2022
dd631eb
update injection to match previous whitespace
chrisbreiding Oct 17, 2022
e951c1f
put back listener
chrisbreiding Oct 17, 2022
5cb6c15
remove comments
chrisbreiding Oct 17, 2022
2102ad7
fix typescript issue
chrisbreiding Oct 17, 2022
f9bdea5
fix unit tests
chrisbreiding Oct 18, 2022
23a31de
fix xhr cookies
chrisbreiding Oct 18, 2022
5b7822e
improve simulated cookies logic
chrisbreiding Oct 18, 2022
12277d6
removed obsolete code
chrisbreiding Oct 18, 2022
f1441f0
upsense before:stability:release
chrisbreiding Oct 18, 2022
8c9d60f
refactor cross:origin:cookies handling
chrisbreiding Oct 18, 2022
7b91ba5
use import type
chrisbreiding Oct 18, 2022
1c91af7
update comments
chrisbreiding Oct 18, 2022
1dc801f
remove no-longer-used event listener
chrisbreiding Oct 18, 2022
bb2e37e
Merge branch 'develop' into issue-23531-document-cookie-timing
chrisbreiding Oct 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 13 additions & 28 deletions packages/app/src/runner/event-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { EventEmitter } from 'events'
import type { MobxRunnerStore } from '@packages/app/src/store/mobx-runner-store'
import type MobX from 'mobx'
import type { LocalBusEmitsMap, LocalBusEventMap, DriverToLocalBus, SocketToDriverMap } from './event-manager-types'

import type { RunState, CachedTestState, AutomationElementId, FileDetails, ReporterStartInfo, ReporterRunState } from '@packages/types'

import { logger } from './logger'
Expand Down Expand Up @@ -40,7 +39,7 @@ const driverToSocketEvents = 'backend:request automation:request mocha recorder:
const driverTestEvents = 'test:before:run:async test:after:run'.split(' ')
const driverToLocalEvents = 'viewport:changed config stop url:changed page:loading visit:failed visit:blank cypress:in:cypress:runner:event'.split(' ')
const socketRerunEvents = 'runner:restart watched:file:changed'.split(' ')
const socketToDriverEvents = 'net:stubbing:event request:event script:error cross:origin:automation:cookies'.split(' ')
const socketToDriverEvents = 'net:stubbing:event request:event script:error cross:origin:cookies'.split(' ')
const localToReporterEvents = 'reporter:log:add reporter:log:state:changed reporter:log:remove'.split(' ')

/**
Expand Down Expand Up @@ -698,32 +697,18 @@ export class EventManager {
log?.set(attrs)
})

// This message comes from the AUT, not the spec bridge.
// This is called in the event that cookies are set in a cross origin AUT prior to attaching a spec bridge.
Cypress.primaryOriginCommunicator.on('aut:set:cookie', ({ cookie, href }, _origin, source) => {
const { superDomain } = Cypress.Location.create(href)
const automationCookie = Cypress.Cookies.toughCookieToAutomationCookie(Cypress.Cookies.parse(cookie), superDomain)

Cypress.automation('set:cookie', automationCookie).then(() => {
// It's possible the source has already unloaded before this event has been processed.
source?.postMessage({ event: 'cross:origin:aut:set:cookie' }, '*')
})
.catch(() => {
// unlikely there will be errors, but ignore them in any case, since
// they're not user-actionable
})
})

// This message comes from the AUT, not the spec bridge.
// This is called in the event that cookies are retrieved in a cross origin AUT prior to attaching a spec bridge.
Cypress.primaryOriginCommunicator.on('aut:get:cookie', async ({ href }, _origin, source) => {
const { superDomain } = Cypress.Location.create(href)

const cookies = await Cypress.automation('get:cookies', { superDomain })

// It's possible the source has already unloaded before this event has been processed.
source?.postMessage({ event: 'cross:origin:aut:get:cookie', cookies }, '*')
})
// This message comes from the AUT, not the spec bridge. This is called in
// the event that cookies are set via document.cookie in a cross origin
// AUT prior to attaching a spec bridge.
Cypress.primaryOriginCommunicator.on(
'aut:set:cookie',
(options: { cookie, url: string, sameSiteContext: string }) => {
// unlikely there will be errors, but ignore them in any case, since
// they're not user-actionable
Cypress.automation('set:cookie', options.cookie).catch(() => {})
Cypress.backend('cross:origin:set:cookie', options).catch(() => {})
},
)

// The window.top should not change between test reloads, and we only need to bind the message event when Cypress is recreated
// Forward all message events to the current instance of the multi-origin communicator
Expand Down
122 changes: 100 additions & 22 deletions packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,13 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
const expires = (new Date()).toUTCString()

cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="localhostCookieProps"]').type(`Expires=${expires}`)
cy.get('[data-cy="cookieProps"]').type(`Expires=${expires}`)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', () => {
cy.clearCookie('user')
cy.origin('http://www.idp.com:3501', () => {
cy.wait(1000) // give cookie time to expire
cy.reload()
cy.document().its('cookie').should('not.include', 'user=')
})
})
Expand Down Expand Up @@ -502,15 +503,18 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
cy.getCookie('user').should('be.null')
})

it('past max-age -> not accessible via document.cookie', () => {
// expiring cookies set by automation don't seem to get unset appropriately
// in Firefox. this issue doesn't seem to be specific to cross-origin tests,
// as it happens even using cy.setCookie()
it('past max-age -> not accessible via document.cookie', { browser: '!firefox' }, () => {
cy.get('[data-cy="cookie-login-land-on-idp"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="localhostCookieProps"]').type('Max-Age=1')
cy.get('[data-cy="cookieProps"]').type('Max-Age=1')
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', () => {
cy.origin('http://www.idp.com:3501', () => {
cy.wait(1500) // give cookie time to expire
cy.reload()
cy.document().its('cookie').should('not.include', 'user=')
Expand Down Expand Up @@ -664,14 +668,15 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
})

it('gets cookie set by http request', () => {
cy.get('[data-cy="cookie-login-land-on-idp"]').click()
cy.get('[data-cy="cookie-login-land-on-document-cookie"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', { args: { username } }, ({ username }) => {
cy.document().its('cookie').should('include', `user=${username}`)
cy.origin('http://www.idp.com:3501', { args: { username } }, ({ username }) => {
cy.get('[data-cy="doc-cookie"]').invoke('text')
.should('include', `user=${username}`)
})
})

Expand All @@ -698,20 +703,20 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
})

it('cookie properties are preserved when set via automation', () => {
cy.get('[data-cy="cross-origin-secondary-link"]').click()
cy.origin('http://www.foobar.com:3500', () => {
cy.get('[data-cy="cookie-https"]').click()
cy.origin('https://www.foobar.com:3502', () => {
cy.document().then((doc) => {
doc.cookie = 'key=value; SameSite=Strict; Path=/foo'
doc.cookie = 'key=value; SameSite=Strict; Secure; Path=/fixtures'
})

cy.getCookie('key').then((cookie) => {
expect(Cypress._.omit(cookie, 'expiry')).to.deep.equal({
domain: '.foobar.com',
domain: '.www.foobar.com',
httpOnly: false,
name: 'key',
path: '/foo',
path: '/fixtures',
sameSite: 'strict',
secure: false,
secure: true,
value: 'value',
})
})
Expand Down Expand Up @@ -743,7 +748,7 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
doc.cookie = 'key2=value2'
})

cy.document().its('cookie').should('equal', 'key2=value2; key1=value1')
cy.document().its('cookie').should('equal', 'key1=value1; key2=value2')
})
})

Expand All @@ -754,31 +759,50 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
doc.cookie = 'key=value'
})

// it can take a small amount of time for the cookie to make it to
// automation, but it's unlikely a user will encounter this issue
// since they'd pretty much have to write this exact test. making it
// wait a second is probably overkill, but purposefully keeping the
// wait long to avoid this test becoming flaky
cy.wait(1000)
cy.getCookie('key').its('value').should('equal', 'value')
})
})

it('returns cookie set by cy.setCookie()', () => {
cy.get('[data-cy="cookie-login-land-on-idp"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3501', () => {
cy.setCookie('foo', 'bar')
cy.document().its('cookie').should('include', 'foo=bar')
})
})

it('no longer returns cookie after cy.clearCookie()', () => {
cy.get('[data-cy="cookie-login-land-on-idp"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', () => {
cy.origin('http://www.idp.com:3501', () => {
cy.clearCookie('user')
cy.document().its('cookie').should('equal', '')
})
})

it('no longer returns cookie after cy.clearCookies()', () => {
it('no longer returns cookies after cy.clearCookies()', () => {
cy.get('[data-cy="cookie-login-land-on-idp"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', () => {
cy.origin('http://www.idp.com:3501', () => {
cy.clearCookies()
cy.document().its('cookie').should('equal', '')
})
Expand All @@ -791,7 +815,7 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3500', { args: { username } }, ({ username }) => {
cy.origin('http://www.idp.com:3501', { args: { username } }, ({ username }) => {
cy.document().then((doc) => {
doc.cookie = 'key=value'
})
Expand All @@ -810,12 +834,12 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
})

cy.get('[data-cy="document-cookie"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.origin('http://www.foobar.com:3500', () => {
cy.document().its('cookie').should('include', 'name=value')
cy.get('[data-cy="doc-cookie"]').invoke('text').should('equal', 'name=value')
cy.getCookie('name').then((cookie) => {
expect(Cypress._.omit(cookie, 'expiry')).to.deep.equal({
domain: '.foobar.com',
domain: '.www.foobar.com',
httpOnly: false,
name: 'name',
path: '/',
Expand All @@ -826,5 +850,59 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
})
})
})

it('preserves duplicate cookie keys', () => {
cy.get('[data-cy="cookie-login-land-on-document-cookie"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3501', () => {
// ensure we've redirected to the right page
cy.url().should('not.include', 'http://www.idp.com:3501/verify-cookie-login')
cy.document().then((doc) => {
doc.cookie = 'key=value1; domain=www.idp.com'
doc.cookie = 'key=value2; domain=idp.com'
})

// order of the cookies differs depending on browser, so just
// ensure that each one is there
cy.document().its('cookie').should('include', 'key=value1')
cy.document().its('cookie').should('include', 'key=value2')
})
})

it('setting cookie preserves cookies on subsequent page loads', () => {
cy.get('[data-cy="cross-origin-secondary-link"]').click()
cy.origin('http://www.foobar.com:3500', () => {
cy.document().then((doc) => {
doc.cookie = 'key=value'
})

cy.document().its('cookie').should('equal', 'key=value')
cy.wait(500)
cy.reload()
cy.document().its('cookie').should('equal', 'key=value')
})
})

// the spec bridge will likely already exist in this spec when running
// all the tests together, but this ensures the behavior in case it's run
// alone or if we implement spec bridge removal in the future
it('works when spec bridge is set up prior to page load', () => {
cy.origin('http://www.idp.com:3501', () => {})

cy.get('[data-cy="cookie-login-land-on-document-cookie"]').click()
cy.origin('http://www.foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="login"]').click()
})

cy.origin('http://www.idp.com:3501', { args: { username } }, ({ username }) => {
cy.get('[data-cy="doc-cookie"]').invoke('text')
.should('include', `user=${username}`)
})
})
})
})
7 changes: 5 additions & 2 deletions packages/driver/cypress/fixtures/auth/document-cookie.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
<head>
</head>
<body>
<p id='test' data-cy="doc-cookie">should be replaced</p>
<p>
<strong>document.cookie</strong>:
<span data-cy="doc-cookie">should be replaced</span>
</p>
<script>
document.cookie = 'name=value'

document.getElementById('test').innerText = document.cookie
document.querySelector('[data-cy="doc-cookie"]').innerText = document.cookie
</script>
</body>
</html>
10 changes: 6 additions & 4 deletions packages/driver/cypress/fixtures/primary-origin.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
<li><a data-cy="xhr-fetch-requests-onload" href="http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html?fireOnload=true">http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html onLoad</a></li>
<li><a data-cy="xhr-fetch-requests" href="http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html">http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html</a></li>
<li><a data-cy="integrity-link" href="http://www.foobar.com:3500/fixtures/scripts-with-integrity.html">http://www.foobar.com:3500/fixtures/scripts-with-integrity.html</a></li>
<li><a data-cy="cookie-http" href="http://www.foobar.com:3500/fixtures/secondary-origin.html">Visit foobar.com over http</a></li>
<li><a data-cy="cookie-https" href="https://www.foobar.com:3502/fixtures/secondary-origin.html">Visit foobar.com over https</a></li>
<li><a data-cy="document-cookie" href="http://www.foobar.com:3500/fixtures/auth/document-cookie.html">http://www.foobar.com:3500/fixtures/auth/document-cookie.html</a></li>
<li><a data-cy="cookie-login">Login with Social</a></li>
<li><a data-cy="cookie-login-https">Login with Social (https)</a></li>
<li><a data-cy="cookie-login-subdomain">Login with Social (subdomain)</a></li>
<li><a data-cy="cookie-login-alias">Login with Social (aliased localhost)</a></li>
<li><a data-cy="cookie-login-override">Login with Social (cookie override)</a></li>
<li><a data-cy="cookie-login-land-on-idp">Login with Social (lands on idp)</a></li>
<li><a data-cy="cookie-http" href="http://www.foobar.com:3500/fixtures/secondary-origin.html">Visit foobar.com over http</a></li>
<li><a data-cy="cookie-https" href="https://www.foobar.com:3502/fixtures/secondary-origin.html">Visit foobar.com over https</a></li>
<li><a data-cy="document-cookie" href="http://www.foobar.com:3500/fixtures/auth/document-cookie.html">http://www.foobar.com:3500/fixtures/auth/document-cookie.html</a></li>
<li><a data-cy="cookie-login-land-on-document-cookie">Login with Social (lands on document.cookie)</a></li>
</ul>
<script>
function setHref (dataCy, redirect2, primaryQueryAddition = '') {
Expand All @@ -45,7 +46,8 @@
setHref('cookie-login-subdomain', 'http://localhost:3500/login&subdomain=true')
setHref('cookie-login-alias', 'http://localhost:3500/login&alias=true')
setHref('cookie-login-override', 'https://localhost:3502/login', '&override=true')
setHref('cookie-login-land-on-idp', 'http://www.idp.com:3500/welcome')
setHref('cookie-login-land-on-idp', 'http://www.idp.com:3501/welcome')
setHref('cookie-login-land-on-document-cookie', 'http://www.idp.com:3501/fixtures/auth/document-cookie.html')
</script>
</body>
</html>
15 changes: 6 additions & 9 deletions packages/driver/src/cross-origin/cypress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const createCypress = () => {

const autWindow = findWindow()

if (autWindow) {
if (autWindow && !autWindow.Cypress) {
attachToWindow(autWindow)
}
})
Expand Down Expand Up @@ -165,21 +165,18 @@ const attachToWindow = (autWindow: Window) => {

const cy = Cypress.cy

// this communicates to the injection code that Cypress is now available so
// it can safely subscribe to Cypress events, etc
// @ts-ignore
autWindow.__attachToCypress(Cypress)
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved

Cypress.state('window', autWindow)
Cypress.state('document', autWindow.document)

if (Cypress && Cypress.config('experimentalModifyObstructiveThirdPartyCode')) {
patchFormElementSubmit(autWindow)
}

Cypress.removeAllListeners('app:timers:reset')
Cypress.removeAllListeners('app:timers:pause')

// @ts-expect-error - the injected code adds the cypressTimersReset function to window
Cypress.on('app:timers:reset', autWindow.cypressTimersReset)
// @ts-ignore - the injected code adds the cypressTimersPause function to window
Cypress.on('app:timers:pause', autWindow.cypressTimersPause)

cy.urlNavigationEvent('before:load')

cy.overrides.wrapNativeMethods(autWindow)
Expand Down
Loading