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

Add support for sameSite in cookie-related commands #6828

Merged
merged 20 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions cli/schema/cypress.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@
],
"default": "bundled",
"description": "If set to 'system', Cypress will try to find a Node.js executable on your path to use when executing your plugins. Otherwise, Cypress will use the Node version bundled with Cypress."
},
"experimentalGetCookiesSameSite": {
"type": "boolean",
"default": false,
"description": "If `true`, Cypress will add `sameSite` values to the objects yielded from `cy.setCookie()`, `cy.getCookie()`, and `cy.getCookies()`. This will become the default behavior in Cypress 5.0."
}
}
}
10 changes: 10 additions & 0 deletions cli/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2450,6 +2450,12 @@ declare namespace Cypress {
* @default { runMode: 1, openMode: null }
*/
firefoxGcInterval: Nullable<number | { runMode: Nullable<number>, openMode: Nullable<number> }>
/**
* If `true`, Cypress will add `sameSite` values to the objects yielded from `cy.setCookie()`,
* `cy.getCookie()`, and `cy.getCookies()`. This will become the default behavior in Cypress 5.0.
* @default false
*/
experimentalGetCookiesSameSite: boolean
}

interface PluginConfigOptions extends ConfigOptions {
Expand Down Expand Up @@ -2594,12 +2600,15 @@ declare namespace Cypress {
onAnyAbort(route: RouteOptions, proxy: any): void
}

type SameSiteStatus = 'no_restriction' | 'strict' | 'lax'

interface SetCookieOptions extends Loggable, Timeoutable {
path: string
domain: string
secure: boolean
httpOnly: boolean
expiry: number
sameSite: SameSiteStatus
}

/**
Expand Down Expand Up @@ -4701,6 +4710,7 @@ declare namespace Cypress {
httpOnly: boolean
secure: boolean
expiry?: string
sameSite?: SameSiteStatus
}

interface EnqueuedCommand {
Expand Down
4 changes: 1 addition & 3 deletions packages/desktop-gui/cypress/integration/settings_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -687,9 +687,7 @@ describe('Settings', () => {
// do not overwrite the shared object reference -
// because it is used by the app's code.
this.win.experimental.names.experimentalCoolFeature = 'Cool Feature'
this.win.experimental.summaries.experimentalCoolFeature = `
Enables super cool feature from Cypress where you can see the cool feature
`
this.win.experimental.summaries.experimentalCoolFeature = 'Enables super cool feature from Cypress where you can see the cool feature'
})

const hasLearnMoreLink = () => {
Expand Down
9 changes: 8 additions & 1 deletion packages/desktop-gui/src/lib/markdown-renderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,17 @@ export default class MarkdownRenderer extends React.PureComponent {
}

render () {
let renderFn = md.render

if (this.props.noParagraphWrapper) {
// prevent markdown-it from wrapping the output in a <p> tag
renderFn = md.renderInline
}

return (
<span ref={(node) => this.node = node}
dangerouslySetInnerHTML={{
__html: md.render(this.props.markdown),
__html: renderFn.call(md, this.props.markdown),
}}>
</span>
)
Expand Down
5 changes: 3 additions & 2 deletions packages/desktop-gui/src/settings/experiments.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react'
import { observer } from 'mobx-react'
import ipc from '../lib/ipc'
import { getExperiments } from '@packages/server/lib/experiments'
import MarkdownRenderer from '../lib/markdown-renderer'

const openHelp = (e) => {
e.preventDefault()
Expand All @@ -27,14 +28,14 @@ const Experiments = observer(({ project }) => {
_.map(experiments, (experiment, i) => (
<li className='experiment' key={i}>
<h5>
{experiment.name}
<MarkdownRenderer markdown={experiment.name} noParagraphWrapper/>
<span className={`experiment-status-sign ${experiment.enabled ? 'enabled' : ''}`}>
{experiment.enabled ? 'ON' : 'OFF'}
</span>
</h5>
<div className='experiment-desc'>
<p className="text-muted">
{experiment.summary}
<MarkdownRenderer markdown={experiment.summary} noParagraphWrapper/>
</p>
<div className='experiment-status'>
<code>{experiment.key}</code>
Expand Down
6 changes: 3 additions & 3 deletions packages/desktop-gui/src/settings/settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@

.experiment-intro {
padding-bottom: 15px;
margin-bottom: 0px;
margin-bottom: 0px;
border-bottom: 1px solid #ddd;
}

Expand All @@ -265,15 +265,15 @@

p {
width: 80%;
margin-bottom: 0;
margin-bottom: 0;
}

.experiment-status {
margin-left: auto;
}
}


.experiment-status-sign {
margin-left: 10px;
color: #9d9ea9;
Expand Down
63 changes: 62 additions & 1 deletion packages/driver/src/cy/commands/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const $utils = require('../../cypress/utils')
const $errUtils = require('../../cypress/error_utils')
const $Location = require('../../cypress/location')

const COOKIE_PROPS = 'name value path secure httpOnly expiry domain'.split(' ')
const COOKIE_PROPS = 'name value path secure httpOnly expiry domain sameSite'.split(' ')

const commandNameRe = /(:)(\w)/

Expand Down Expand Up @@ -33,7 +33,37 @@ const mergeDefaults = function (obj) {
return merge(obj)
}

// from https://developer.chrome.com/extensions/cookies#type-SameSiteStatus
// note that `unspecified` is purposely omitted - Firefox and Chrome set
// different defaults, and Firefox lacks support for `unspecified`, so
// `undefined` is used in lieu of `unspecified`
// @see https://bugzilla.mozilla.org/show_bug.cgi?id=1624668
const VALID_SAMESITE_VALUES = ['no_restriction', 'lax', 'strict']

const normalizeSameSite = (sameSite) => {
if (_.isUndefined(sameSite)) {
return sameSite
}

if (_.isString(sameSite)) {
sameSite = sameSite.toLowerCase()
}

if (sameSite === 'none') {
// "None" is the value sent in the header for `no_restriction`, so allow it here for convenience
sameSite = 'no_restriction'
}

return sameSite
}

module.exports = function (Commands, Cypress, cy, state, config) {
const maybeStripSameSiteProp = (cookie) => {
if (cookie && !Cypress.config('experimentalGetCookiesSameSite')) {
delete cookie.sameSite
}
}

const automateCookies = function (event, obj = {}, log, timeout) {
const automate = () => {
return Cypress.automation(event, mergeDefaults(obj))
Expand Down Expand Up @@ -146,6 +176,8 @@ module.exports = function (Commands, Cypress, cy, state, config) {

return automateCookies('get:cookie', { name }, options._log, options.timeout)
.then((resp) => {
maybeStripSameSiteProp(resp)

options.cookie = resp

return resp
Expand Down Expand Up @@ -181,6 +213,10 @@ module.exports = function (Commands, Cypress, cy, state, config) {

return automateCookies('get:cookies', _.pick(options, 'domain'), options._log, options.timeout)
.then((resp) => {
if (Array.isArray(resp)) {
resp.forEach(maybeStripSameSiteProp)
}

options.cookies = resp

return resp
Expand Down Expand Up @@ -225,12 +261,37 @@ module.exports = function (Commands, Cypress, cy, state, config) {

const onFail = options._log

cookie.sameSite = normalizeSameSite(cookie.sameSite)

if (!_.isUndefined(cookie.sameSite) && !VALID_SAMESITE_VALUES.includes(cookie.sameSite)) {
$errUtils.throwErrByPath('setCookie.invalid_samesite', {
onFail,
args: {
value: options.sameSite, // for clarity, throw the error with the user's unnormalized option
validValues: VALID_SAMESITE_VALUES,
},
})
}

// cookies with SameSite=None must also set Secure
// @see https://web.dev/samesite-cookies-explained/#changes-to-the-default-behavior-without-samesite
if (cookie.sameSite === 'no_restriction' && cookie.secure !== true) {
$errUtils.throwErrByPath('setCookie.secure_not_set_with_samesite_none', {
onFail,
args: {
value: options.sameSite, // for clarity, throw the error with the user's unnormalized option
},
})
}

if (!_.isString(name) || !_.isString(value)) {
$errUtils.throwErrByPath('setCookie.invalid_arguments', { onFail })
}

return automateCookies('set:cookie', cookie, options._log, options.timeout)
.then((resp) => {
maybeStripSameSiteProp(resp)

options.cookie = resp

return resp
Expand Down
17 changes: 17 additions & 0 deletions packages/driver/src/cypress/error_messages.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,23 @@ module.exports = {
message: "#{cmd('setCookie')} must be passed an RFC-6265-compliant cookie value. You passed:\n\n`{{value}}`"
docsUrl: "https://on.cypress.io/setcookie"
}
invalid_samesite: ({ validValues, value }) => {
message: """
If a `sameSite` value is supplied to #{cmd('setCookie')}, it must be a string from the following list:
> #{validValues.join(', ')}
You passed:
> #{format(value)}
"""
docsUrl: "https://on.cypress.io/setcookie"
}
secure_not_set_with_samesite_none: ({ validValues, value }) => {
message: """
Only cookies with the `secure` flag set to `true` can use `sameSite: '{{value}}'`.

Pass `secure: true` to #{cmd('setCookie')} to set a cookie with `sameSite: '{{value}}'`.
"""
docsUrl: "https://on.cypress.io/setcookie"
}

should:
chainer_not_found: "The chainer `{{chainer}}` was not found. Could not build assertion."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ describe "src/cy/commands/cookies", ->
}).then ->
expect(Cypress.automation).to.be.calledWith(
"set:cookie",
{ domain: "localhost", name: "foo", value: "bar", path: "/", secure: false, httpOnly: false, expiry: 12345 }
{ domain: "localhost", name: "foo", value: "bar", path: "/", secure: false, httpOnly: false, expiry: 12345, sameSite: undefined }
)

it "can change options", ->
Expand All @@ -384,7 +384,7 @@ describe "src/cy/commands/cookies", ->
}).then ->
expect(Cypress.automation).to.be.calledWith(
"set:cookie",
{ domain: "brian.dev.local", name: "foo", value: "bar", path: "/foo", secure: true, httpOnly: true, expiry: 987 }
{ domain: "brian.dev.local", name: "foo", value: "bar", path: "/foo", secure: true, httpOnly: true, expiry: 987, sameSite: undefined }
)

it "does not mutate options", ->
Expand All @@ -394,6 +394,33 @@ describe "src/cy/commands/cookies", ->
cy.setCookie("foo", "bar", {}).then ->
expect(options).deep.eq({})

it "can set cookies with sameSite", ->
Cypress.automation.restore()
Cypress.utils.addTwentyYears.restore()

Cypress.sinon.stub(Cypress, 'config').callThrough()
.withArgs('experimentalGetCookiesSameSite').returns(true)

cy.setCookie('one', 'bar', { sameSite: 'none', secure: true })
cy.getCookie('one').should('include', { sameSite: 'no_restriction' })

cy.setCookie('two', 'bar', { sameSite: 'no_restriction', secure: true })
cy.getCookie('two').should('include', { sameSite: 'no_restriction' })

cy.setCookie('three', 'bar', { sameSite: 'Lax' })
cy.getCookie('three').should('include', { sameSite: 'lax' })

cy.setCookie('four', 'bar', { sameSite: 'Strict' })
cy.getCookie('four').should('include', { sameSite: 'strict' })

cy.setCookie('five', 'bar')

## @see https://bugzilla.mozilla.org/show_bug.cgi?id=1624668
if Cypress.isBrowser('firefox')
cy.getCookie('five').should('include', { sameSite: 'no_restriction' })
else
cy.getCookie('five').should('not.have.property', 'sameSite')

describe "timeout", ->
it "sets timeout to Cypress.config(responseTimeout)", ->
Cypress.config("responseTimeout", 2500)
Expand Down Expand Up @@ -497,6 +524,39 @@ describe "src/cy/commands/cookies", ->

cy.setCookie("foo", 123)

it "when an invalid samesite prop is supplied", (done) ->
cy.on "fail", (err) =>
lastLog = @lastLog

expect(@logs.length).to.eq(1)
expect(lastLog.get("error").message).to.eq """
If a `sameSite` value is supplied to `cy.setCookie()`, it must be a string from the following list:
> no_restriction, lax, strict
You passed:
> bad
"""
expect(lastLog.get("error").docsUrl).to.eq "https://on.cypress.io/setcookie"
expect(lastLog.get("error")).to.eq(err)
done()

cy.setCookie('foo', 'bar', { sameSite: 'bad' })

it "when samesite=none is supplied and secure is not set", (done) ->
cy.on "fail", (err) =>
lastLog = @lastLog

expect(@logs.length).to.eq(1)
expect(lastLog.get("error").message).to.eq """
Only cookies with the `secure` flag set to `true` can use `sameSite: 'None'`.

Pass `secure: true` to `cy.setCookie()` to set a cookie with `sameSite: 'None'`.
"""
expect(lastLog.get("error").docsUrl).to.eq "https://on.cypress.io/setcookie"
expect(lastLog.get("error")).to.eq(err)
done()

cy.setCookie('foo', 'bar', { sameSite: 'None' })

context "when setting an invalid cookie", ->
it "throws an error if the backend responds with an error", (done) ->
err = new Error("backend could not set cookie")
Expand All @@ -520,7 +580,7 @@ describe "src/cy/commands/cookies", ->

Cypress.automation
.withArgs("set:cookie", {
domain: "localhost", name: "foo", value: "bar", path: "/", secure: false, httpOnly: false, expiry: 12345
domain: "localhost", name: "foo", value: "bar", path: "/", secure: false, httpOnly: false, expiry: 12345, sameSite: undefined
})
.resolves({
name: "foo", value: "bar", domain: "localhost", path: "/", secure: true, httpOnly: false
Expand Down
2 changes: 1 addition & 1 deletion packages/extension/app/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const browser = require('webextension-polyfill')
const client = require('./client')
const { getCookieUrl } = require('../lib/util')

const COOKIE_PROPS = ['url', 'name', 'path', 'secure', 'domain']
const COOKIE_PROPS = ['url', 'name', 'path', 'secure', 'domain', 'sameSite']
const GET_ALL_PROPS = COOKIE_PROPS.concat(['session', 'storeId'])
const SET_PROPS = COOKIE_PROPS.concat(['value', 'httpOnly', 'expirationDate'])

Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/automation/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const debug = require('debug')('cypress:server:cookies')

// match the w3c webdriver spec on return cookies
// https://w3c.github.io/webdriver/webdriver-spec.html#cookies
const COOKIE_PROPERTIES = 'name value path domain secure httpOnly expiry hostOnly'.split(' ')
const COOKIE_PROPERTIES = 'name value path domain secure httpOnly expiry hostOnly sameSite'.split(' ')

const normalizeCookies = (cookies) => {
return _.map(cookies, normalizeCookieProps)
Expand Down
Loading