Skip to content

Commit

Permalink
fix: same site cookie context and duplicate cookies (#23438)
Browse files Browse the repository at this point in the history
* test: refactor and add tests in the cors package

* fix: add areUrlsSameSite method to cookies package and fix
sameSiteContext calculation method and add tests

* fix: always use Set-Cookie optimistically whether or not we keep track of the cookie or not in the server side cookie jar

* chore: add failing unit tests for postpending cookies

* chore: add tough cookie integration tests to verify we append cookies appropriately to request header Cookie

* fix: do not duplicate cookies in request if existing in the cookie jar. Add additional tests to verify expected behavior

* test: add cookie behavior tests that document current expected behavior vs what spec behavior should/will be

* test: add misc tests that check for cookie order

* chore: update debug logs in request to discern cookies

* test: fix assertions in firefox as same-site cookies are actually set correctly

* fix test incorrect assertions. cookies currently exist in primary that are same-site regardless of browser

* skip SameSite=none test in firefox as we currently low insecure samesite none cookies in firefox

* chore: apply suggestions from code review

* chore: change expects to expect

* chore: add documentation for why we need an additional HTTPS port

* remove X-Set-Cookie fixmes
  • Loading branch information
AtofStryker authored Sep 8, 2022
1 parent 1aa9793 commit 9cdb33b
Show file tree
Hide file tree
Showing 12 changed files with 2,028 additions and 170 deletions.
1 change: 1 addition & 0 deletions packages/driver/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export default defineConfig({
'experimentalStudio': true,
'hosts': {
'*.foobar.com': '127.0.0.1',
'*.barbaz.com': '127.0.0.1',
'*.idp.com': '127.0.0.1',
'localalias': '127.0.0.1',
},
Expand Down
1,435 changes: 1,435 additions & 0 deletions packages/driver/cypress/e2e/e2e/origin/cookie_behavior.cy.ts

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ describe('cy.origin - cookie login', () => {
verifyIdpNotLoggedIn({ expectNullCookie: false })
})

it('SameSite=None -> not logged in', () => {
// FIXME: Currently in Firefox, the default cookie setting in the extension is no_restriction, which can be set with Secure=false.
it('SameSite=None -> not logged in', { browser: '!firefox' }, () => {
cy.origin('http://foobar.com:3500', { args: { username } }, ({ username }) => {
cy.get('[data-cy="username"]').type(username)
cy.get('[data-cy="cookieProps"]').type('SameSite=None')
Expand Down
2 changes: 2 additions & 0 deletions packages/driver/cypress/fixtures/primary-origin.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
<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>
</ul>
<script>
function setHref (dataCy, redirect2, primaryQueryAddition = '') {
Expand Down
37 changes: 31 additions & 6 deletions packages/driver/cypress/plugins/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const upload = multer({ dest: 'cypress/_test-output/' })
const PATH_TO_SERVER_PKG = path.dirname(require.resolve('@packages/server'))

const httpPorts = [3500, 3501]
const httpsPort = 3502
const httpsPorts = [3502, 3503]

const createApp = (port) => {
const app = express()
Expand Down Expand Up @@ -283,6 +283,10 @@ const createApp = (port) => {
res.send(`<html><body><h1>Welcome, ${user}!</h1></body></html>`)
})

app.get('/test-request', (req, res) => {
res.sendStatus(200)
})

app.get('/set-cookie', (req, res) => {
const { cookie } = req.query

Expand All @@ -291,6 +295,23 @@ const createApp = (port) => {
.sendStatus(200)
})

app.get('/test-request-credentials', (req, res) => {
res
.setHeader('Access-Control-Allow-Origin', req['headers']['origin'])
.setHeader('Access-Control-Allow-Credentials', 'true')
.sendStatus(200)
})

app.get('/set-cookie-credentials', (req, res) => {
const { cookie } = req.query

res
.setHeader('Access-Control-Allow-Origin', req['headers']['origin'])
.setHeader('Access-Control-Allow-Credentials', 'true')
.append('Set-Cookie', cookie)
.sendStatus(200)
})

let _var = ''

app.get('/set-var', (req, res) => {
Expand Down Expand Up @@ -323,10 +344,14 @@ httpPorts.forEach((port) => {
})
})

const httpsApp = createApp(httpsPort)
const httpsServer = httpsProxy.httpsServer(httpsApp)
// Have two HTTPS ports in order to test same-site cookie behavior in `cookie_behavior.cy.ts` for experimentalSessionAndOrigin
// Cookies can be same site if the port is different, and we need a way to test this E2E style to make sure we implement cookie handling correctly
httpsPorts.forEach((port) => {
const httpsApp = createApp(port)
const httpsServer = httpsProxy.httpsServer(httpsApp)

httpsServer.listen(httpsPort, () => {
// eslint-disable-next-line no-console
return console.log('Express server listening on port', httpsPort, '(HTTPS)')
return httpsServer.listen(port, () => {
// eslint-disable-next-line no-console
return console.log('Express server listening on port', port, '(HTTPS)')
})
})
Loading

5 comments on commit 9cdb33b

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 9cdb33b Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.7.1/linux-x64/develop-9cdb33b4c6415cf5ea3f95fce5b62339de3a51a0/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 9cdb33b Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.7.1/linux-arm64/develop-9cdb33b4c6415cf5ea3f95fce5b62339de3a51a0/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 9cdb33b Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.7.1/darwin-x64/develop-9cdb33b4c6415cf5ea3f95fce5b62339de3a51a0/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 9cdb33b Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.7.1/win32-x64/develop-9cdb33b4c6415cf5ea3f95fce5b62339de3a51a0/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 9cdb33b Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.7.1/darwin-arm64/develop-9cdb33b4c6415cf5ea3f95fce5b62339de3a51a0/cypress.tgz

Please sign in to comment.