Skip to content

feat: adds optional sessionID getter #40

Closed
tim-dev wants to merge 2 commits into
expressjs:masterfrom
tim-dev:master
Closed

feat: adds optional sessionID getter #40
tim-dev wants to merge 2 commits into
expressjs:masterfrom
tim-dev:master

Conversation

@tim-dev

@tim-dev tim-dev commented May 22, 2014

Copy link
Copy Markdown

Allows an alternative to only using the cookie to get the sessionID. Designed mostly for API interfaces who don't want to store cookies, and would rather use headers or query params.

@jonathanong

Copy link
Copy Markdown
Member

Uhhh this kind of sounds like a terrible idea because people could just hijack other peoples sessions.

If it's an API, why don't you just use auth or an api key or something?

@MichaelCereda

Copy link
Copy Markdown

Yes @jonathanong, also in my opinion adding a getter is asking for throubles..

@joewagner

Copy link
Copy Markdown
Member

The idea of abstracting the way express-session gets –and sets– the sesisonID from/to the client seems like a reasonable idea, but I don't think this is the way to do it. The getting and setting of the sessionId, however its implemented, should be handled by a single module.
I would be interested to see something that opened the possibility for a developer to connect any new models for state management easily as a module.
All that would potentially be moot if a big rewrite as discussed in #1 happens, or we decided to rethink the whole plugins system.

@jonathanong

Copy link
Copy Markdown
Member

Closing because the consensus is -1.

@joewagner I agree this module needs a lot of refactoring

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants