cookieParser docs may require some extra text #1607

Closed
Pomax opened this Issue May 7, 2013 · 6 comments

3 participants

@Pomax

The current online documentation leaves some questions pertaining to using the cookieParser. The current text states that "Optionally you may enabled signed cookie support by passing a secret string." implying that without such a string, secure cookies won't work, regardless of whether a cookie has a 'secret' set. If this is the case, it would be good to mention this explicitly. It also doesn't explain what happens when both cookieParser and a cookie have a secret string set. Which one is used, does a cookie still need a secret, or will it fall back to the cookieparser secret, etc.

(filed because we had great difficulty getting cookies to work across subdomains, despite using matching cookie secret strings. Adding a nonsense string to the cookieParser suddenly made things work, which is not the most intuitive behaviour when there are already secret strings encoded in the cookies.)

@Pomax Pomax referenced this issue in mozilla/login.webmaker.org May 7, 2013
Merged

cross-wm-tool cookie management #47

@badunk

I believe that is the case - if there is no string, secure cookies won't work.

It also doesn't explain what happens when both cookieParser and a cookie have a secret string set. Which one is > used, does a cookie still need a secret, or will it fall back to the cookieparser secret, etc.

What do you mean both the cookieParser and a cookie? Are you using cookie based sessions? If so, according to the source code, I believe the session store defers to the cookie's secret code if set. This part is probably undocumented.

@Pomax

yeah, so something like:

app.use(express.cookieParser("happy random string"));
app.use(express.sessionCookie({
  ...
  secret: "other happy string",
  ...
}));

We were seeing it not pick up signed cookies at all without a string, despite using 'secret' (so that's a bit odd and definitely needs explicit mention. You'd think that even if you don't tell it to use secure cookies, the cookie itself having a secret would force signed cookie checking).

If the cookieParser defers to the cookie's own "secret" value, then that makes the cookieParser's constructor string essentially moot. Would a "true" value also work?

@badunk

If you look at here: https://github.com/senchalabs/connect/blob/master/lib/middleware/cookieParser.js#L43, req.secret is being defined by cookieParser. Then, it should use req.secret if secret is not passed in sessionCookie: https://github.com/senchalabs/connect/blob/master/lib/middleware/session.js#L225

I think your assumption is correct - you don't have to explicitly tell it to use signed cookies. Its implied when you pass it a string.

How were you seeing that the signed cookies didn't work? Have you tried the following?:

app.use(express.cookieParser("happy random string"));
app.use(express.sessionCookie());
@Pomax

We were using it in a super-cookie setting, with a login.domain.com setting a cookie for ".domain.com", under name "abc", with other apps on the same domain but different subdomains then reading it in using express.sessionCookie({name: 'abc', ...}). We got stuck on a lot of different things (the weirdest being that if we ran cookieParser(...) with a hardcoded string, things worked, but if we used habitat and env.get("SESSION_SECRET") it would break and the cookies weren't recognized as the same. We'd see two cookies for a consumer app with different domains, even if the environment variables were the same string across instances, and were both running on heroku). I think we went the wrong way round, first trying with a cookie secret but no cookieParser secret, then with both secret-enabled, and then we got so confused by the hardcoded string issue that we decided to try to come up with an alternative that didn't rely on cookies.

Looking at the code you linked, it seems like apps can only do either all signed, or all unsigned cookies... it might be less confusing, and possibly offer devs more freedom, to let the cookieParser always set up both parser types, and then simply pick based on what was set; no cookie.secret, no parser.secret -> parse unsigned, cookie.secret | parser.secret -> parse signed

@badunk

Your case w/ cookie sharing across sub domains is interesting...no idea why it shouldn't work. I don't see any code w/ the parsing that would differ between environments.

By parser, I assume you mean the session store. The session store actually requires that a secret be set (either by the cookieParser or fed into itself as an option). This is for obvious security reasons.

@jonathanong
expressjs member

hey guys, feel free to add docs in https://github.com/visionmedia/expressjs.com

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