ericrasmussen / pyramid_redis_sessions

Pyramid web framework session factory backed by Redis
Other
43 stars 26 forks source link

support clear() and new cookie signing #38

Open mmerickel opened 10 years ago

mmerickel commented 10 years ago

I put these in the same ticket because they are both things that would break bw-compat.

First of all, webob 1.3.1 introduced the new webob.cookies.SignedCookieProfile object which manages a signed cookie. It's being used by the pyramid.session.SignedCookieSessionFactory for all of the cookie management and I'd like to see pyramid_redis_sessions use it as well. Unfortunately it uses a slightly different signing algorithm from Pyramid's signed_serialize and signed_deserialize so there is a potential hazard there.

Secondly, currently some data like the timeout is stored in the managed_dict rather than alongside. This means if a user does clear(), some critical metadata can be lost from the session. I think it'd be an improvement to follow the SignedCookieSessionFactory mechanism again and treat the session blob as a 3-tuple cookie_val = (managed_dict, created, timeout). Or even just a dict if you want it to be more extensible. Changing the format of the blob can be done in a bw-compat way if necessary.

I'd suggest implementing these features prior to 1.0 and just tell users that when they upgrade that any active sessions will be invalid. If you wait, it'd be more important to try to create bw-compat shims for each or (sadly) not implement the features.

ericrasmussen commented 10 years ago

Short update: @whiteroses addressed the metadata issue in PR #37.

I haven't had the time/energy to look into SignedCookieSessionFactory, and I don't want to hold up releasing the fixes with the invalidation workflow and metadata, so I'll leave this issue open for now.

If anyone in #pyramid is looking small ways to get involved, feel free to send them my way and I'll work with them on this!