cookies icon indicating copy to clipboard operation
cookies copied to clipboard

this.keys but no opts means cookies don't get signed

Open jergason opened this issue 10 years ago • 4 comments

In Cookie.prototype.set (https://github.com/expressjs/cookies/blob/2dcb71f130a7eaafd16e71b9af70debe11d4c93f/lib/cookies.js#L69), the signed variable is true if opts.signed or this.keys is truthy. However, the check for whether to sign keys or not also checks if opts exists.

This means that if this.keys is truthy, but opts is undefined, the signed variable will be true but the key still won't be signed. My expectation is they key should be signed if signed == true, and it shouldn't depend on the existence of opts.

jergason avatar Dec 09 '14 15:12 jergason

That seems to make sense, though I'm not sure how easy this will be to fix without including it in the major version bump, because I wonder how many people have the keys set but no options given. I'll have to think about it some. Also input from @jonathanong

dougwilson avatar Dec 09 '14 16:12 dougwilson

Ha was gonna remove that in the next major

jonathanong avatar Dec 09 '14 16:12 jonathanong

Cool, that was my suspicion. It seems weird that the existence of keys caused cookies to automatically sign. So I would almost say that the current behavior is intended: you only get signed cookies if you sent {signed: true} to .set. Does that sound right @jonathanong ?

dougwilson avatar Dec 09 '14 17:12 dougwilson

Current behavior is intended :( I wanted the next version to have .sign() and .encrypt() as separate methods

jonathanong avatar Dec 09 '14 17:12 jonathanong