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

cy.request unable to handle Same-Site: None cookie for cross origin requests #6757

Closed
jakub-bao opened this issue Mar 17, 2020 · 9 comments · Fixed by #6778
Closed

cy.request unable to handle Same-Site: None cookie for cross origin requests #6757

jakub-bao opened this issue Mar 17, 2020 · 9 comments · Fixed by #6778
Assignees
Labels
topic: cookies 🍪 type: enhancement Requested enhancement of existing feature

Comments

@jakub-bao
Copy link

jakub-bao commented Mar 17, 2020

Facts:

  1. cy.request should persist cookies received in response headers. It does it just fine except this used case described below.
  2. Google just published a new update to chrome browsers which breaks cookie persistance for cross origin requests
  3. This is a new issue which just occurred after receiving a chrome update this weekend. This only affects my cypress environment. When testing manually on same version of chrome it's just fine.

Behaviour before chrome update (expected behaviour):

  1. I send a cy.request to login to our server before running tests:
Cypress.Commands.add('loginAs', (userType)=>{
    cy.request({
        method: 'POST', 
        url: `${baseUrl}dhis-web-commons-security/login.action`,
        body: `j_username=cypress-${userType}&j_password=${testPassword}`,
        headers: {
            'authority': baseUrl,
            'origin': baseUrl,
            'referer': `${baseUrl}dhis-web-commons-security/login.action`,
            'content-type': 'application/x-www-form-urlencoded',
        }
    }).then((response)=>{ 
        console.log(response);
    });
});
  1. This was just fine and it would set a cookie for chrome browser.
    image

  2. Then I can run my tests against a local instance of my app at http://localhost:3003 while the app requests cross-origin resources from dev.xxxx.org. The server dev.xxxx.org knows I am logged in because with each request my Chrome provides session id cookie.

Behavior after chrome update (current behavior)

  1. I do my login via cy.request as in the case above. But cypress fails to persist the cookie due to the following Chrome security complains:
A cookie associated with a cross-site resource at https://dev.xxxx.org/ was set without the 'SameSite' attribute. It has been blocked, as Chrome now only delivers cookies with cross-site requests if they are set with SameSite=None and Secure. You can review cookies in developer tools under Application>Storage>Cookies and see more details at https://www.chromestatus.com/feature/5088147346030592 and https://www.chromestatus.com/feature/5633521622188032.

Long story short: Chrome is complaining it can't store my session id cookie because it's missing SameSite=None and Secure attributes.

The problem is that the attributes are NOT missing. Look at raw output here:
image

Conclusions

I believe that cypress swallows the Same-Site attribute while persisting cookies from cy.request

A hint is is also that cy.setCookie is completely missing Same-Site parameter:
image

Temporary solution

  1. Downgrade Chrome 80.0.3987.132
  2. Use Electron instead

Versions

  1. Chrome 80.0.3987.122 or 80.0.3987.132. It was fine with earlier versions of Chrome including (80.*)
  2. Cypress 4.2.0 and 4.1.0
  3. OS: Ubuntu 19.10
@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label Mar 18, 2020
@jennifer-shehane
Copy link
Member

Issue to allow sameSite parameter for cy.setCookie() is here: #2437

@jakub-bao
Copy link
Author

@jennifer-shehane Good. That might be a good first step. However, I think it still won't solve this issue. It might need extra work.

@flotwig
Copy link
Contributor

flotwig commented Mar 18, 2020

Here is the latest draft spec I can find for SameSite: https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-05

It's still not a standard, but I guess it might as well be since the browsers are shipping it as stable 😛

The Cypress networking layer doesn't know about SameSite and will discard it from incoming requests that are manually handled (like cy.visit and cy.request). I'll work on implementing that along with the work required for #2437.

@cypress-bot cypress-bot bot added stage: work in progress There is an open PR for this issue [WIP] and removed stage: needs investigating Someone from Cypress needs to look at this labels Mar 18, 2020
@jennifer-shehane jennifer-shehane added the type: enhancement Requested enhancement of existing feature label Mar 19, 2020
@jakub-bao
Copy link
Author

@flotwig

The Cypress networking layer doesn't know about SameSite and will discard it from incoming requests that are manually handled (like cy.visit and cy.request). I'll work on implementing that along with the work required for #2437.

Great! That sounds awesome! Thank you!

FYI, temporary workaround: Switch to Electron browser.

@flotwig
Copy link
Contributor

flotwig commented Mar 20, 2020

Curious that Electron works for you, we're shipping Electron 8.1.1 which includes Chromium 80.0.3987.141, which is > the versions you mentioned using. I wonder why that is.

@jklingen
Copy link

@flotwig the new SameSite behavior is not bound to a specific version, but rather "being rolled out to Chrome 80 Stable users through gradually increasing rollouts": https://www.chromium.org/updates/same-site

You can explicitely enable/disable the behavior in chrome://flags, though.

@jakub-bao
Copy link
Author

@jklingen Awesome! That's amazing.

I was running of Electron. But honestly Chrome is so much better for test running. Due to it's DevTools debugger. Electron has it too but let's be honest :)

For anyone experiencing this issue. You can temporarily disable the new security features in chrome://flags here:

image

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress There is an open PR for this issue [WIP] labels Mar 27, 2020
@cypress-bot cypress-bot bot added the stage: pending release There is a closed PR for this issue label Mar 30, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 30, 2020

The code for this is done in cypress-io/cypress#6778, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Mar 30, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 30, 2020

Released in 4.3.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.3.0, please open a new issue.

@cypress-bot cypress-bot bot removed the stage: pending release There is a closed PR for this issue label Mar 30, 2020
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic: cookies 🍪 type: enhancement Requested enhancement of existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants