agrafix / Spock

Another Haskell web framework for rapid development
https://www.spock.li
678 stars 56 forks source link

sc_sessionExpandTTL only expands expiration on server #50

Closed agrafix closed 8 years ago

agrafix commented 9 years ago

The cookies expiration date should be modified, too

ondrap commented 8 years ago

Just found that too - IMO it would suffice to change in Web.Spock.Internal.SessionManager the sessionMiddleware to just set the cookie header uncoditionally (or conditionally on the sc_sessionExpandTTL). BTW: is there any use case for having sc_sessionExpandTTL set to False?

ondrap commented 8 years ago

Just found out this solution wouldn't probably play out well with regenerateSesssionId... It seems to me that multiple setCookie on the same cookie will result in multiple Set-Cookie headers being sent. Maybe some mechanism to send only the last cookie name would make sense?

agrafix commented 8 years ago

sc_sessionExpandTTL = False is useful in cases where you want a fixed session length. I needed this for a project of mine. I'm still trying to figure out the best way to handle this, hence the issue... The biggest issue for me are the possible race conditions so I'd really like to write some tests first showing the issue and cornercases and then implement a solution. I'd also like to take a peek at other web frameworks how they handle this. I'm on vacation for a week, but if you come up with a good solution including tests I'll happily merge your PR :+1:

ondrap commented 8 years ago

What about setting the session cookie life time to infinity (i.e. 'long time' - or 'until the user closes the browser) and forget about the problem? The session lifetime management is done on the server anyway and upon login the regenerateSessionId should be used anyway (maybe this should be stressed in the documentation). This could be configurable in the SessionCfg. Are there any drawbacks?

agrafix commented 8 years ago

That was what I was thinking about too... Maybe also implement that if the framework stumbles app-on an invalid cookie it will delete it. Would you like to go for that?

elfeck commented 8 years ago

I would like to see a fix for this as well. ondrap's solution seems okay to me.

agrafix commented 8 years ago

@elfeck Yap! Would you like to create a PR?

elfeck commented 8 years ago

@agrafix Although I have little experience the inner workings of Spock and real-world haskell, I will at least look into it.

agrafix commented 8 years ago

It should be pretty straight forward, you probably only have to touch these lines: https://github.com/agrafix/Spock/blob/305c29cd3250f33bce0597310e45f7696820b367/Spock/src/Web/Spock/Internal/SessionManager.hs#L186-L196 .