-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
2 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now to just hide them altogether :) Although I think it makes it more challenging for users to decode signed cookies with things like socket.io auth. Something I did with my other package is to have the signed cookie middleware expose a decode method which can decode the signed cookies.
e6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha yup, I'll have to double-check to make sure express is not using
connect.utils
anymore, I think we should be good. I agree, we should have things separated out some more so that it's easier for people to integrate with socket.io, someone really needs to make a module for socket.io thoughe6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apologies if this isn't the right place but I'm in the situation described here, @shtylman would you mind sharing that decode method?
@visionmedia a module for socket.io would be pretty slick! What did you have in mind? (I'd love to contribute...)
e6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The decode method depends on what cookie session middleware that you are using. If you are using the connect cookie session middleware, you will need some form of the code in the parseSignedCookies from the connect utils.
I think it would be nice for the cookieSession() init function to return not just the middleware but also have a .decode property which would be the function it is using to decode the cookie. This would then be passable to other things that need to be able to decode the cookie.
e6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks yes, I'll pretty much copy/paste utils.parseSignedCookies (+ all its deps) for the while being
and yeah, I'm sure TJ will figure something out, exposing a decode function sounds good but seing how often people pair express with socket.io I think it could be good to have a dedicated module like TJ said.
e6ca949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i've just seen people copy/paste stuff from that blog post enough times that im almost confused why it's not a module yet haha, but ideally we keep moving some things out and then aggregate them in connect