Use TTL sessions for removing expired sessions, set browser-session-length expiring cookies (maxAge: null) to expire after 2 weeks of inactivity #50

Merged
merged 5 commits into from Dec 19, 2012

Conversation

Projects
None yet
3 participants
Contributor

aroman commented Dec 17, 2012

Based on the discussion in #46, I've replaced the previous implementation of using setInterval and a mongo query to remove expired sessions, and moved to using mongodb's native collection TTLs.

Please note, while these changes are not strictly "breaking", they do change some functionality (for the better!). Specifically:

  • sessions with no specific expiry (which is the default -- maxAge defaults to null) are now set to expire after 2 weeks of inactivity. This seems to be the industry standard practice, and is the default in projects such as django. If there is a need, it would of course be possible to make this time configurable.
  • clear_interval has been made obsolete (and thus removed). This now defaults to the server-side value of every 60 seconds
  • collection TTLs only went stable in mongodb 2.2. So that means sessions will only be removed on mongodb >= 2.2. If this is a deal-breaker for you (it isn't for my selfish needs), I would be happy to fallback to your "hand-made" expiry system if connected to a mongodb server running >= 2.2. Or perhaps configurable in the connect-mongo sessionStore options.

Additionally, the two tests that dealt with testing the "manual" TTL system have been removed, since that system is no longer being used.

Oh, and I added a section to the README explaining some the rationale behind this functionality, and linked to further reading.

aroman added some commits Dec 17, 2012

@aroman aroman (Fixes: #46) Use TTL sessions for removing expired sessions, set brow…
…ser-session-length expiring cookies (maxAge: null) to expire after 2 weeks of inactivity
0ccfb74
@aroman aroman Only return collection after ensuring the TTL index 64ccda5
Contributor

aroman commented Dec 17, 2012

Just noticed the Travis build failed... it was running the test suite and tried to connect to a mongodb instance on the localhost, which obviously doesn't exist. How have you been running the test suite on travis such that it passes (as master does now?)

Collaborator

kcbanner commented Dec 18, 2012

This looks good, I'm going to verify the Travis build system and I'll merge this ASAP. Thanks so much!

Contributor

aroman commented Dec 19, 2012

Great! I managed to fix the travis build -- looks like you need to explicitly add monogdb as a service in the travis config file. Build is passing happily now! :)

@kcbanner kcbanner added a commit that referenced this pull request Dec 19, 2012

@kcbanner kcbanner Merge pull request #50 from aroman/master
Use TTL sessions for removing expired sessions, set browser-session-length expiring cookies (maxAge: null) to expire after 2 weeks of inactivity
9445612

@kcbanner kcbanner merged commit 9445612 into jdesboeufs:master Dec 19, 2012

1 check passed

default The Travis build passed
Details
Collaborator

kcbanner commented Dec 19, 2012

I'll publish a new version soon.

Could you pls update the npm package?

Thanks

Owner

aroman replied Jan 2, 2013

Unfortunately it seems that @kcbanner is the only one with commit access to the repo. He has previously stated that he's been quite busy with other projects. That said, you're very free to depend on my fork of connect-mongo (which has since been merged to trunk). I currently do this in my own project, which is running in production.

(You can depend on packages directly from git/github in your npm package.json.)

Hey guys. I'll do a push soon, either today or tomorrow.

Great, thanks!

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