session icon indicating copy to clipboard operation
session copied to clipboard

Added support for passing session ID using HTTP header

Open alexey-detr opened this issue 10 years ago • 36 comments

These changes add new option headerName that can be used to optional passing session ID using custom HTTP header.

Already discussed this issue in #77

alexey-detr avatar Aug 19 '14 16:08 alexey-detr

Hi @alexey-detr I'm so sorry I have not paid attention to this PR :( So I was looking over it, and to me, it seems a little weird for someone to be wanting to read from a header the signed session ID value, and especially to want to write out a response header with this value. an you explain to me a little bit about your use-case that inspired you to implement this?

dougwilson avatar Oct 17 '14 05:10 dougwilson

Hi! If you remember we already discussed it in #77. Actually it is a very rare use case when requests are sending with AJAX + CORS. I'm still not sure if such implementation should be in this package, because it doesn't look solid. But this feature is necessary for my project so I created fork with private branch for my needs.

alexey-detr avatar Oct 17 '14 06:10 alexey-detr

If you remember we already discussed it in #77.

Sorry, I forgot :) There are just so many things to remember, and it's mainly just hard to remember people's GitHub handles and put together with convos, sorry!!

Ok. So I understand what you are saying. Basically, to me, it sounds like you want to be able to have something you can use as a Bearer token (think OAuth) and have that token load up a session using this module. I'll look into making this possible, even if you may have to add a little bit of code in your app to do it, but then you won't have to maintain a fork for no reason :)

dougwilson avatar Oct 17 '14 14:10 dougwilson

+1 for this pull request.

dhollenbeck avatar Oct 27 '14 19:10 dhollenbeck

+1 Could be usefull when a page in called from a script (not from a browser). In this case (php client of a restfull api for example) it is easier to send a header instead of setup a cookie

kappuccino avatar Nov 13 '14 08:11 kappuccino

What if we look to see if req.sessionId has been set by an earlier piece of middleware, and use that if it exists? We could basically just do this That would allow the use of a Authorization: Bearer as well as an X-* type header. For @alexey-detr use case the getHeader and setHeader functions would live in middleware before this module, and they would set req.sessionId appropriately.

This would require a good explanation in the docs, but I think it would be more extensible.

joewagner avatar Nov 13 '14 17:11 joewagner

@JoeWagner I try your solution it is working and consists in :

  • A middleware before express-session to catch a specific header (x-session-id) and inject it in req.sessionID
  • A patch of express-session to check if req.sessionID exists (copy & past of your solution) JoeWagner@fc55f78
  • A third middleware to set x-session-id header in the response headers

This last middleware allow the client to grab the session ID easily (no need to parse the set-cookie header — still here but not uses in my case)

In my opinion, a better solution could be to merge all this in expressjs-session middleware and add an extra parameter in configuration object (the header name)

if someone is interested, i can propose my solution wrapped in a pull request :beers:

kappuccino avatar Nov 15 '14 13:11 kappuccino

+1 for this pull request, I need this feature for my single web page application that uses socket.io + expressjs from another server.

kscc25 avatar Dec 06 '14 17:12 kscc25

+1

JSteunou avatar Dec 16 '14 17:12 JSteunou

I am ready to make the pull request but my unit test is not ready. I make a first request with no header and i catch the session-id in response headers. then I make another request with session-id in my "request headers"... but i do not figure how to do it.

it('should modify cookie when changed to large value', function(done){ request(app) .get('/') ....

any idea ?

kappuccino avatar Dec 17 '14 06:12 kappuccino

@kappuccino You can see how I did it here https://github.com/alexey-detr/session/commit/3c9ef961785b2d86c435af92ec7b4fce2be304fa#diff-306b67b120079e997613897e45b88194R109

alexey-detr avatar Dec 17 '14 07:12 alexey-detr

@alexey-detr thank you ! if you have already implemented the "session-id via header" why not to make a pull request ?

kappuccino avatar Dec 17 '14 07:12 kappuccino

@kappuccino But I already did so. This thread is a PR or I done something wrong?

alexey-detr avatar Dec 17 '14 07:12 alexey-detr

my apology. you're right https://github.com/expressjs/session/pulls sound your request has not been pulled yet sorry

kappuccino avatar Dec 17 '14 07:12 kappuccino

@kappuccino that's ok, I'm a newbie in making pull requests, so probably I could made something wrong. I'm also waiting for request to be pulled in or this feature will be implemented in another way.

alexey-detr avatar Dec 17 '14 08:12 alexey-detr

@alexey-detr you should update your PR because it cannot be merged at the moment.

This pull request contains merge conflicts that must be resolved.

JSteunou avatar Dec 17 '14 10:12 JSteunou

@JSteunou thank you for advice! PR should be updated now.

alexey-detr avatar Dec 17 '14 13:12 alexey-detr

Travis says "This pull request can be automatically merged by project collaborators."

sounds good guys !

kappuccino avatar Dec 17 '14 14:12 kappuccino

+1

Vitaliy-Yarovuy avatar Jan 08 '15 00:01 Vitaliy-Yarovuy

Guys I have updated the PR. Now it is possible to specify cookie option as null, so cookies will not be ever sent with responses. Tried to implement it when cookie option is not specified at all, but it breaks backward compatibility. Setting cookie as null allows to use only headers to pass session id.

alexey-detr avatar Jan 11 '15 15:01 alexey-detr

So.. when's this getting pulled in?

stephenliberty avatar Feb 24 '15 10:02 stephenliberty

+1 Would be great to have this in shortly, don't want to re-implement it on my own when it's pretty much already being presented here on a silver platter.

BonsaiDen avatar Feb 24 '15 14:02 BonsaiDen

FWIW, the express-mysql-session package (and maybe others) specifically look for cookies.. so this doesn't work for them. Issues should be opened on those packages once this finally makes it in..

stephenliberty avatar Feb 24 '15 18:02 stephenliberty

FYI for those coming by here waiting for this to be implemented, for now the only work-around is to add the session ID in the req.signedCookies object provided by cookie-parser module.

dougwilson avatar Apr 25 '15 02:04 dougwilson

+1 to have this in shortly

hazemsq avatar Oct 07 '15 12:10 hazemsq

any news on that?

simllll avatar Jan 23 '16 14:01 simllll

+1

lime-green avatar Feb 11 '16 02:02 lime-green

bump

alexprice1 avatar Jun 07 '16 20:06 alexprice1

Very much interested in this. It's been mentioned this is a 'rare' case. I'm familiar with no other way to authenticate and persist sessions against an API cross domain, as cookies can't be shared. Please merge or address this situation.

jasonbodily avatar Sep 14 '16 20:09 jasonbodily

Please merge or address this situation.

Just looking through the PR, there was a comment on the name of the option, which was only partially changed; the docs (where the comment still remains) hasn't been updated. Also, the PR currently has conflicts and can't be merged just as a general comment.

Even then, there are multiple threads discussing this topic, at in at least one of them, it was said I would rather accept an PR that allowed a user to read the session ID out in whatever way they choose, instead of still locking them into the decisions of this library. For example, you are locked into Cookies because that's who this module did; this PR would now just lock you into Cookie or a Header, which is not a generally better situation, just a quick Band-Aid that should be better addressed in some PR, please.

dougwilson avatar Sep 14 '16 21:09 dougwilson