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

cookie options won't get set #958

Open
m-a-h1 opened this issue Sep 18, 2023 · 13 comments
Open

cookie options won't get set #958

m-a-h1 opened this issue Sep 18, 2023 · 13 comments

Comments

@m-a-h1
Copy link

m-a-h1 commented Sep 18, 2023

i use session with mongo store, but the problem is that cookie setting won't get change,
app.use( session({ name: "session", store: store, secret: process.env.SESSION_SECRET || "", resave: false, saveUninitialized: false, cookie: { httpOnly: true, secure: true, sameSite: "none", } }) );

no matter what the cookie setting is , it always use the default one.
actually when i change the options , cookie option on the database will be save correctly, but cookie will be send with the default options!

@frostyfrog
Copy link

frostyfrog commented Nov 23, 2023

I did some playing around while investigating Wingysam/Christmas-Community#17 and it appears that the cookie options aren't respected at all. Not even the default options are being applied, so we just set the cookie with 0 options (including no path)

Further investigation revealed that the issue that I was running into lies within a different library that was being used, causing the conversion of the cookie object to a normal object

@mirker21
Copy link

mirker21 commented Dec 1, 2023

@frostyfrog Which library was giving you this problem?

@frostyfrog
Copy link

Pouchdb session. They have a 4y old issue on the problem over here:
solzimer/session-pouchdb-store#2

@mirker21
Copy link

mirker21 commented Dec 1, 2023

Ah, nevermind. The issue that I am trying to fix is with a project that doesn't use Pouchdb session, but still uses express-session. I checked in my node_modules for that package, but I couldn't find it. Thank you though!

@t-araujo
Copy link

This is happening to me too!
The default one is applied! I'm using the session with Mongo store as well, and no matter the changes to the options is always the same result!

@oitozero
Copy link

Same here. @m-a-h1 were you able to sort this?

@Tri-Vi
Copy link

Tri-Vi commented Jan 22, 2024

Same here.

@dougwilson
Copy link
Contributor

Someone needs to provide a reproducable example we can run and see the issue or please make a PR. I cannot reproduce the issue so far just guessing at what the code should be to have the issue.

@t-araujo
Copy link

t-araujo commented Feb 1, 2024

@dougwilson In my case I have this code

  const options = {
    secret: process.env.SESSION_SECRET,
    resave: false,
    saveUninitialized: true,
    store: new MongoStore({ mongooseConnection: mongoose.connection, collection: 'session' }),
    cookie: { secure: false },
  };

  if (isProd) {
    options.cookie.secure = true;
  }

Let me know if something is wrong

@dougwilson
Copy link
Contributor

Hi @t-araujo nothing aeems wrong with that. What kind of problem are you having with that code?

@t-araujo
Copy link

t-araujo commented Feb 1, 2024

@dougwilson The problem is the secure flag is not set in the requests. no matter what code I do even if I remove the IF and set secure = true in the options object!

@dougwilson
Copy link
Contributor

Hi @t-araujo interesting. The test suite and my own apps don't seem to have an issue. I wonder, is your app behind a proxy? If it is, have you set up the proxy stuff from https://github.com/expressjs/session?tab=readme-ov-file#cookiesecure or https://github.com/expressjs/session?tab=readme-ov-file#proxy ?

Otherwise I am not sure why it is not qorking for you. If you can perhaps provide a simple app that I can copy and paste and run and provide instructions for how you are calling it I can replicate and see the issue, I can debug it. You are also welcome to debug and determine a fix and make a PR. If you have other ideas for how to move forward different from those, I am happy to hear them too!

@darcy521
Copy link

darcy521 commented May 4, 2024

@dougwilson @m-a-h1 Hey guys, I have encountered the same issue while using express-session and I guess that I found the reason why secure is not working in this middleware.

From my understanding, if we are running our application locally without using HTTPS and Nginx or something similar, secure will not work under the logic of this express-session middleware. Please see the code snippet below:
image
image

In the latest version of Chrome and Firefox HTTPS requirements are ignored when the Secure attribute is set by localhost, please see the MDN documents below:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
image
Chrome permits setting the secure attribute for cookies under HTTP on localhost. However, express-session does not create cookies under HTTP, which is why cookies might not appear even if the secure attribute is correctly set.

Thus, for developers seeking a quick fix under these circumstances, a practical but not optimal solution is to comment out the if condition code from lines 235 to 238, as illustrated above. This adjustment should make the cookie behave as expected.

I wonder if we can refine logic to align with Chrome rules, so I tried to do some 'naive' optimization by checking if application is running locally.

I am aware that there may be some security concerns related to my changes. If possible, could you happen to identify these issues so I can learn from them? Thank you! :)
PR: https://github.com/expressjs/session/pull/982

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants