feat: make cookies have sameSite key by default (#7790)

* feat: make cookies have sameSite key by default

BREAKING CHANGE: modifies the shape of Cookie objects

* update tests

* add deprecation notice

Co-authored-by: Brian Mann <brian.mann86@gmail.com>

cli/schema/cypress.schema.json

@@ -224,11 +224,6 @@
       "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."
-    },
     "experimentalSourceRewriting": {
       "type": "boolean",
       "default": false,

cli/types/cypress.d.ts

@@ -2442,12 +2442,6 @@ 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
     /**
      * Enables AST-based JS/HTML rewriting. This may fix issues caused by the existing regex-based JS/HTML replacement
      * algorithm.

packages/driver/cypress/integration/commands/cookies_spec.js

@@ -474,9 +474,7 @@ describe('src/cy/commands/cookies', () => {
       })
     })
 
-    it('can set cookies with sameSite', {
-      experimentalGetCookiesSameSite: true,
-    }, () => {
+    it('can set cookies with sameSite', () => {
       Cypress.automation.restore()
       Cypress.utils.addTwentyYears.restore()
 

packages/driver/src/cy/commands/cookies.js

@@ -58,12 +58,6 @@ const normalizeSameSite = (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))
@@ -183,8 +177,6 @@ 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
@@ -222,10 +214,6 @@ 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
@@ -299,8 +287,6 @@ module.exports = function (Commands, Cypress, cy, state, config) {
 
       return automateCookies('set:cookie', cookie, options._log, options.timeout)
       .then((resp) => {
-        maybeStripSameSiteProp(resp)
-
         options.cookie = resp
 
         return resp

packages/server/__snapshots__/2_cookies_spec.ts.js

@@ -5,11 +5,10 @@ exports['e2e cookies with baseurl'] = `
   (Run Starting)
 
   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
-  │ Cypress:      1.2.3                                                                            │
-  │ Browser:      FooBrowser 88                                                                    │
-  │ Specs:        1 found (cookies_spec_baseurl.coffee)                                            │
-  │ Searched:     cypress/integration/cookies_spec_baseurl.coffee                                  │
-  │ Experiments:  experimentalGetCookiesSameSite=true                                              │
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (cookies_spec_baseurl.coffee)                                              │
+  │ Searched:   cypress/integration/cookies_spec_baseurl.coffee                                    │
   └────────────────────────────────────────────────────────────────────────────────────────────────┘
 
 

packages/server/lib/config.js

@@ -93,7 +93,8 @@ configKeys.push('componentFolder')
 const breakingConfigKeys = toWords(`\
 videoRecording
 screenshotOnHeadlessFailure
-trashAssetsBeforeHeadlessRuns\
+trashAssetsBeforeHeadlessRuns
+experimentalGetCookiesSameSite\
 `)
 
 // Internal configuration properties the user should be able to overwrite
@@ -105,7 +106,6 @@ browsers\
 // each should start with "experimental" and be camel cased
 // example: experimentalComponentTesting
 const experimentalConfigKeys = toWords(`\
-experimentalGetCookiesSameSite
 experimentalSourceRewriting
 experimentalComponentTesting
 experimentalShadowDomSupport
@@ -174,7 +174,6 @@ const CONFIG_DEFAULTS = {
   componentFolder: 'cypress/component',
   // TODO: example for component testing with subkeys
   // experimentalComponentTesting: { componentFolder: 'cypress/component' }
-  experimentalGetCookiesSameSite: false,
   experimentalSourceRewriting: false,
   experimentalShadowDomSupport: false,
   experimentalFetchPolyfill: false,
@@ -222,7 +221,6 @@ const validationRules = {
   // validation for component testing experiment
   componentFolder: v.isStringOrFalse,
   // experimental flag validation below
-  experimentalGetCookiesSameSite: v.isBoolean,
   experimentalSourceRewriting: v.isBoolean,
   experimentalShadowDomSupport: v.isBoolean,
   experimentalFetchPolyfill: v.isBoolean,
@@ -251,7 +249,10 @@ const validateNoBreakingConfig = (cfg) => {
           return errors.throw('RENAMED_CONFIG_OPTION', key, 'trashAssetsBeforeRuns')
         case 'videoRecording':
           return errors.throw('RENAMED_CONFIG_OPTION', key, 'video')
+        case 'experimentalGetCookiesSameSite':
+          return errors.warning('EXPERIMENTAL_SAMESITE_REMOVED')
         default:
+          throw new Error(`unknown breaking config key ${key}`)
       }
     }
   })

packages/server/lib/errors.js

@@ -923,6 +923,11 @@ const getMsgByType = function (type, arg1 = {}, arg2, arg3) {
         Enable write permissions to this directory to ensure screenshots and videos are stored.
 
         If you don't require screenshots or videos to be stored you can safely ignore this warning.`
+    case 'EXPERIMENTAL_SAMESITE_REMOVED':
+      return stripIndent`\
+        The \`experimentalGetCookiesSameSite\` configuration option was removed in Cypress 5. Yielding the \`sameSite\` property is now the default behavior of the \`cy.cookie\` commands.
+
+        You can safely remove this option from your config.`
     default:
   }
 }

packages/server/lib/experiments.ts

@@ -53,7 +53,6 @@ interface StringValues {
 const _summaries: StringValues = {
   experimentalComponentTesting: 'Framework-specific component testing, uses `componentFolder` to load component specs',
   experimentalSourceRewriting: 'Enables AST-based JS/HTML rewriting. This may fix issues caused by the existing regex-based JS/HTML replacement algorithm.',
-  experimentalGetCookiesSameSite: 'Adds `sameSite` values to the objects yielded from `cy.setCookie()`, `cy.getCookie()`, and `cy.getCookies()`. This will become the default behavior in Cypress 5.0.',
   experimentalFetchPolyfill: 'Polyfills `window.fetch` to enable Network spying and stubbing',
   experimentalShadowDomSupport: 'Enables support for shadow DOM traversal, introduces the `shadow()` command and the `includeShadowDom` option to traversal commands.',
 }
@@ -71,7 +70,6 @@ const _summaries: StringValues = {
 const _names: StringValues = {
   experimentalComponentTesting: 'Component Testing',
   experimentalSourceRewriting: 'Improved source rewriting',
-  experimentalGetCookiesSameSite: 'Set `sameSite` property when retrieving cookies',
   experimentalShadowDomSupport: 'Shadow DOM Support',
   experimentalFetchPolyfill: 'Fetch polyfill',
 }

packages/server/test/e2e/2_cookies_spec.ts

@@ -1,7 +1,7 @@
-const moment = require('moment')
-const parser = require('cookie-parser')
-const e2e = require('../support/helpers/e2e').default
-const humanInterval = require('human-interval')
+import moment from 'moment'
+import parser from 'cookie-parser'
+import e2e from '../support/helpers/e2e'
+import humanInterval from 'human-interval'
 
 const onServer = function (app) {
   app.use(parser())
@@ -194,7 +194,6 @@ describe('e2e cookies', () => {
     // we can remove this extra test case
     e2e.it('with forced SameSite strictness', {
       config: {
-        experimentalGetCookiesSameSite: true,
         baseUrl,
         env: {
           baseUrl,
@@ -248,7 +247,6 @@ describe('e2e cookies', () => {
       ) => {
         e2e.it(`passes with baseurl: ${baseUrl}`, {
           config: {
-            experimentalGetCookiesSameSite: true,
             baseUrl,
             env: {
               baseUrl,

packages/server/test/e2e/4_request_spec.ts

@@ -1,6 +1,6 @@
-const bodyParser = require('body-parser')
-const cookieParser = require('cookie-parser')
-const e2e = require('../support/helpers/e2e').default
+import bodyParser from 'body-parser'
+import cookieParser from 'cookie-parser'
+import e2e from '../support/helpers/e2e'
 
 let counts = null
 

packages/server/test/e2e/5_subdomain_spec.ts

@@ -1,7 +1,7 @@
-const cors = require('cors')
-const parser = require('cookie-parser')
-const session = require('express-session')
-const e2e = require('../support/helpers/e2e').default
+import cors from 'cors'
+import parser from 'cookie-parser'
+import session from 'express-session'
+import e2e from '../support/helpers/e2e'
 
 const onServer = function (app) {
   app.use(parser())
@@ -48,7 +48,7 @@ const onServer = function (app) {
     cookie: {
       sameSite: true,
     },
-  })
+  }) as Function
 
   app.get('/htmlCookies', (req, res) => {
     const {

packages/server/test/support/fixtures/projects/e2e/cypress/integration/cookies_spec_no_baseurl.coffee

@@ -12,6 +12,11 @@ describe "cookies", ->
       })
 
     it "can get all cookies", ->
+      expectedKeys = ["domain", "name", "value", "path", "secure", "httpOnly", "expiry"]
+
+      if Cypress.isBrowser('firefox')
+        expectedKeys.push('sameSite')
+
       cy
         .clearCookie("foo1")
         .setCookie("foo", "bar").then (c) ->
@@ -23,9 +28,7 @@ describe "cookies", ->
           expect(c.secure).to.eq(false)
           expect(c.expiry).to.be.a("number")
 
-          expect(c).to.have.keys(
-            "domain", "name", "value", "path", "secure", "httpOnly", "expiry"
-          )
+          expect(c).to.have.keys(expectedKeys)
         .getCookies()
           .should("have.length", 1)
           .then (cookies) ->
@@ -39,9 +42,7 @@ describe "cookies", ->
             expect(c.secure).to.eq(false)
             expect(c.expiry).to.be.a("number")
 
-            expect(c).to.have.keys(
-              "domain", "name", "value", "path", "secure", "httpOnly", "expiry"
-            )
+            expect(c).to.have.keys(expectedKeys)
         .clearCookies()
           .should("be.null")
         .setCookie("wtf", "bob", {httpOnly: true, path: "/foo", secure: true})
@@ -54,9 +55,7 @@ describe "cookies", ->
           expect(c.secure).to.eq(true)
           expect(c.expiry).to.be.a("number")
 
-          expect(c).to.have.keys(
-            "domain", "name", "value", "path", "secure", "httpOnly", "expiry"
-          )
+          expect(c).to.have.keys(expectedKeys)
         .clearCookie("wtf")
           .should("be.null")
         .getCookie("doesNotExist")

packages/server/test/support/fixtures/projects/e2e/cypress/integration/request_spec.coffee

@@ -20,14 +20,14 @@ describe "redirects + requests", ->
         expect(cookies[0].secure).to.eq(false)
         expect(cookies[0].expiry).to.be.closeTo(oneMinuteFromNow, 5)
 
-        expect(cookies[1]).to.deep.eq({
+        expect(cookies[1]).to.deep.eq(Cypress._.merge({
           domain: "localhost"
           name: "2293-session"
           value: "true"
           httpOnly: false
           path: "/"
           secure: false
-        })
+        }, (if Cypress.isBrowser('firefox') then { sameSite: 'no_restriction' } else {})))
 
   it "visits to a different superdomain will be resolved twice", ->
     cy

packages/server/test/support/fixtures/projects/e2e/cypress/integration/subdomain_spec.coffee

@@ -42,7 +42,7 @@ describe "subdomains", ->
     cy
       .visit("http://domain.foobar.com:2292")
       .getCookies().should("have.length", 1)
-      .getCookie("nomnom").should("deep.eq", {
+      .getCookie("nomnom").should("include", {
         domain: ".foobar.com"
         name: "nomnom"
         value: "good"

packages/server/test/unit/config_spec.js

@@ -1079,6 +1079,17 @@ describe('lib/config', () => {
       })
     })
 
+    // @see https://github.com/cypress-io/cypress/issues/6892
+    it('warns if experimentalGetCookiesSameSite is passed', async function () {
+      const warning = sinon.spy(errors, 'warning')
+
+      await this.defaults('experimentalGetCookiesSameSite', true, {
+        experimentalGetCookiesSameSite: true,
+      })
+
+      expect(warning).to.be.calledWith('EXPERIMENTAL_SAMESITE_REMOVED')
+    })
+
     describe('.resolved', () => {
       it('sets reporter and port to cli', () => {
         const obj = {
@@ -1108,7 +1119,6 @@ describe('lib/config', () => {
             requestTimeout: { value: 5000, from: 'default' },
             responseTimeout: { value: 30000, from: 'default' },
             execTimeout: { value: 60000, from: 'default' },
-            experimentalGetCookiesSameSite: { value: false, from: 'default' },
             experimentalSourceRewriting: { value: false, from: 'default' },
             taskTimeout: { value: 60000, from: 'default' },
             numTestsKeptInMemory: { value: 50, from: 'default' },
@@ -1185,7 +1195,6 @@ describe('lib/config', () => {
             requestTimeout: { value: 5000, from: 'default' },
             responseTimeout: { value: 30000, from: 'default' },
             execTimeout: { value: 60000, from: 'default' },
-            experimentalGetCookiesSameSite: { value: false, from: 'default' },
             experimentalSourceRewriting: { value: false, from: 'default' },
             taskTimeout: { value: 60000, from: 'default' },
             numTestsKeptInMemory: { value: 50, from: 'default' },