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

Support ambiguous key presence #10

Closed
isaacs opened this issue Apr 25, 2012 · 2 comments
Closed

Support ambiguous key presence #10

isaacs opened this issue Apr 25, 2012 · 2 comments

Comments

@isaacs
Copy link
Contributor

isaacs commented Apr 25, 2012

It'd be nice if cookies didn't try to sign things when no keygrip is supplied, or if {signed:true} were the default when keys are present.

That way, you could have code that just passes in {signed:true} all the time, or never, and if keys are there or are not there, it will work as good as it can.

@jed
Copy link
Contributor

jed commented Apr 25, 2012

perhaps a better idea would be to remove keygrip altogether, and instead have keygrip require cookies and provide a identical API but signed-only cookie implementation. thoughts?

@isaacs
Copy link
Contributor Author

isaacs commented Apr 25, 2012

Well, keygrip itself seems like a rather nice set of generic functionality. And having cookies just take a interface-following thing as an argument seems well factored.

The thing is, I'm doing something like this in my app:

if (config.keys) {
  config.keys = new Keygrip(config.keys)
}

// .. then later ..

req.cookies = res.cookies = new Cookies(req, res, config.keys)

And, really, I want keys to be signed by default if there's keys, and not signed by default if there aren't. So I end up doing dumb stuff like this:

res.cookie.set(key, val, { signed: config.keys })

I'll send a pull req. I was just being lazy, it's a trivial change.

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

No branches or pull requests

2 participants