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

Issue with cy.request and cookies #6628

Closed
vfernandestoptal opened this issue Mar 3, 2020 · 20 comments · Fixed by #6821 or Flygsand/site#83
Closed

Issue with cy.request and cookies #6628

vfernandestoptal opened this issue Mar 3, 2020 · 20 comments · Fixed by #6821 or Flygsand/site#83

Comments

@vfernandestoptal
Copy link

Current behavior:

Cookies received in response to cy.request() call, with a Domain value set to the parent domain of the target domain, are not being sent in subsequent cy.request() or cy.visit() calls.
This started happening with Cypress 3.5.0 and it's still happening with newer 4.x versions

First cy.request() sets the cookie:
Screenshot from 2020-03-03 16-27-13

Second cy.request() doesn't send the cookie:
Screenshot from 2020-03-03 16-27-26

Desired behavior:

The cookies should be sent, like it is doing with Cypress 3.4.1

Test code to reproduce

https://github.com/vfernandestoptal/cypress-broken-cookies

Versions

Manjaro Linux kernel 5.5.6-1
Cypress 3.5.0 and higher
Chrome Version 80.0.3987.122 (Official Build) (64-bit)

@VesterDe
Copy link

VesterDe commented Mar 3, 2020

I'm having the same issue with version 4.1.0

For me, logout is failing to set the auth cookie to null, because /logout redirects to /login, and login responds to the auth cookie incorrectly by keeping it set to true.

My real problem though, is that this isn't consistent, and works most of the time... I was wondering if cy.request based cookie setting is maybe async in some way?

@jennifer-shehane
Copy link
Member

I will have to verify with @flotwig but I believe this may be a duplicate of #5688 It's a bit hard to clarify since the reproducible example from that issue is not public.

@corwinstephen
Copy link

@jennifer-shehane yeah, I think it's the same issue

@vfernandestoptal
Copy link
Author

Any progress on this issue? Anything else I can do to help fix it?

@flotwig
Copy link
Contributor

flotwig commented Mar 20, 2020

@vfernandestoptal If you have a reproducible example that I can run to see the issue locally, that would be most helpful. I've tried to reproduce the issue by following what is described in the OP but have no had success. So an example GitHub repo or something that showcases the issue would be ideal.

@corwinstephen
Copy link

@vfernandestoptal what stack are you using?

@vfernandestoptal
Copy link
Author

@vfernandestoptal If you have a reproducible example that I can run to see the issue locally, that would be most helpful. I've tried to reproduce the issue by following what is described in the OP but have no had success. So an example GitHub repo or something that showcases the issue would be ideal.

Do you mean you tried out the sample repo - https://github.com/vfernandestoptal/cypress-broken-cookies.git - and it works for you?

@flotwig
Copy link
Contributor

flotwig commented Mar 20, 2020

@vfernandestoptal Sweet, thank you, I actually missed that link the first time around 🤦‍♂️ It does reproduce the issue. This is a huge help, I will look into this and see if I can figure out the issue.

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Mar 20, 2020
@cypress-bot cypress-bot bot added stage: work in progress There is an open PR for this issue [WIP] and removed stage: ready for work The issue is reproducible and in scope labels Mar 23, 2020
@flotwig
Copy link
Contributor

flotwig commented Mar 23, 2020

@vfernandestoptal I tracked down the issue, there is a bug with cookie parsing related to unknown TLDs. If you change the domain in your test to local.cypress.com instead of local.cypress.test, it will begin to work (in 4.2.0 at least). Check #6821 for details, there should be a fix out soon.

@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 23, 2020
@VesterDe
Copy link

I'm curious if this will help me, we're using an .io domain for local testing, but we run on port :5000.

Might there be anything there as well?

@corwinstephen
Copy link

corwinstephen commented Mar 23, 2020 via email

@polga
Copy link

polga commented Mar 24, 2020

I was having this issue as well, with a cookie domain of .app.localhost:8088 (to support subdomains). But once I changed the cookie domain to slug.app.localhost, it worked fine even on port 8088.

@flotwig
Copy link
Contributor

flotwig commented Mar 24, 2020

Okay, just tried for a little bit to reproduce issues with specifying a non-default port on a cookie in 4.2.0 by running the 2_cookies_spec e2e test with USE_HIGH_PORTS=1 set, to force it to use non-default ports. But nothing is failing.

If someone can share a reproducible example where the test passes on the default HTTP port (80/443) but fails on a non-default port, I'd be happy to look into the root cause of why that is happening. Otherwise, it's possible it may be a red herring for #6821.


@VesterDe can you share the domain you're using, and if any part of it is in the public suffix list?

@VesterDe
Copy link

@flotwig During all possible workflows for us (local + CI), Cypress encounters these domains:

http://development.127.0.0.1.our-domain.io:5000/
http://ci.127.0.0.1.our-domain.io:5050/
http://testing.127.0.0.1.our-domain.io:5050/

@flotwig
Copy link
Contributor

flotwig commented Mar 24, 2020

@VesterDe hmm, I'm not able to reproduce an issue with cookies using domains like that, can you please open a new issue describing what is happening in more detail, and with a reproducible example if possible?

@VesterDe
Copy link

@flotwig It's not very consistent, it works fine 90% of the time, then sometimes cookie values are just wrong... If I manage to catch it happening at some point, I'll try to provide more info.

@cypress-bot cypress-bot bot added stage: work in progress There is an open PR for this issue [WIP] and removed stage: needs review The PR code is done & tested, needs review labels Mar 25, 2020
@9odzilla
Copy link

Not sure if this is the same issue. My e2e tests have started to fail because of 400 requests. Without any code changes to tests when I inspect network requests there are no cookies so no auth is sent in the headers. This only happens after my first test passes. I think this issue was masked because our UI code had an auth token accidently committed to code but has recently been removed.

@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 25, 2020
@DanWahlin
Copy link

DanWahlin commented Mar 26, 2020

Thanks @flotwig (in reference to #6821)! I think this is the exact issue I've been fighting with Node/Passport/Express/Cookie-Session. I've been having to add a dev setting for the cookie to make Cypress pick up a login cookie correctly for our codewithdan.local domain (used locally of course). For production I'd then set the cookie the "correct" way so that it could be used by subdomains. Anyway, sounds like this will fix the issue - looking forward to the release.

For anyone hitting the issue here was my workaround. I'm hoping this will be unnecessary with this fix though.

// Handle setting session cookie
const cookieDomain = '.' + config.domain;
console.log('Cookie Domain: ', cookieDomain);

const cookieOptions = {
    name: 'auth-cookie',
    secret: '...'
};

if (process.env.NODE_ENV === 'production') {
    // Needed for production!!!! Otherwise www.codewithdan.com will be the cookie's
    // domain which will break thing when an author clicks a course in My Account.
    // Without this they'll be asked to login again.
    cookieOptions.domain = cookieDomain;
}
else {
    // needed for Cypress to login correctly
    cookieOptions.cookie = { domain: cookieDomain } 
}

app.use(cookieSession(cookieOptions));

@cypress-bot cypress-bot bot added stage: pending release There is a closed PR for this issue and removed stage: needs review The PR code is done & tested, needs review labels Mar 26, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 26, 2020

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

@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.
Projects
None yet
8 participants