ericrasmussen / pyramid_redis_sessions Goto Github PK

View Code? Open in Web Editor NEW

43.0 7.0 26.0 230 KB

Pyramid web framework session factory backed by Redis

License: Other

Python 89.08% Makefile 5.72% Batchfile 5.21%

pyramid_redis_sessions's People

Contributors

Stargazers

Watchers

pyramid_redis_sessions's Issues

Ability to omit some requests

It will be nice to have an ability to omit some requests, or just turn off refreshing redis timeout for some of them (maybe some view decorator?).

the package calls EXISTS and then GET, this should be a single GET

The key is checked with EXISTS, and if it passes GET is used to retrieve it.

This should be refactored to just make a single call to GET. The redis server shouldn't be sent unnecessary traffic.

Docs url not found error

I noticed documentation url http://pyramid-redis-sessions.readthedocs.org/en/latest/index.html leads to not found page, from PyPi and github as well.

can't install with pip

I can install using easy_install, but when I install using pip:

    virtualenv /tmp/foo
    source /tmp/foo/bin/activate
    pip install pyramid_redis_sessions

I get this error:

    Downloading/unpacking pyramid-redis-sessions
      Could not find a version that satisfies the requirement pyramid-redis-sessions (from versions: 0.9b, 0.9b1, 0.9b2, 1.0a1)
    Cleaning up...
    No distributions matching the version for pyramid-redis-sessions
    Storing debug log for failure in /home/username/.pip/pip.log

pip.log says:

------------------------------------------------------------
/tmp/foo/bin/pip run on Tue Feb 25 09:43:02 2014
Downloading/unpacking pyramid-redis-sessions
  Getting page https://pypi.python.org/simple/pyramid_redis_sessions/
  URLs to search for versions for pyramid-redis-sessions:
  * https://pypi.python.org/simple/pyramid_redis_sessions/
  Analyzing links from page https://pypi.python.org/simple/pyramid_redis_sessions/
    Found link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b.tar.gz#md5=043eff32cb6a9fc5981ddf5ae24b4345 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version: 0.9b
    Found link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b1.tar.gz#md5=6707382d7bee61578be0c6781ebda0e7 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version: 0.9b1
    Found link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b2.tar.gz#md5=c75e554bdcc706b3d865b87f2cb1b290 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version: 0.9b2
    Found link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-1.0a1.tar.gz#md5=98ff79948b87c55ed9b87cad1601a2f6 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version: 1.0a1
    Skipping link http://docs.pylonsproject.org/projects/pyramid/en/latest/api/interfaces.html#pyramid.interfaces.ISession (from https://pypi.python.org/simple/pyramid_redis_sessions/); unknown archive format: .html
  Ignoring link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b.tar.gz#md5=043eff32cb6a9fc5981ddf5ae24b4345 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version 0.9b is a pre-release (use --pre to allow).
  Ignoring link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b1.tar.gz#md5=6707382d7bee61578be0c6781ebda0e7 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version 0.9b1 is a pre-release (use --pre to allow).
  Ignoring link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-0.9b2.tar.gz#md5=c75e554bdcc706b3d865b87f2cb1b290 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version 0.9b2 is a pre-release (use --pre to allow).
  Ignoring link https://pypi.python.org/packages/source/p/pyramid_redis_sessions/pyramid_redis_sessions-1.0a1.tar.gz#md5=98ff79948b87c55ed9b87cad1601a2f6 (from https://pypi.python.org/simple/pyramid_redis_sessions/), version 1.0a1 is a pre-release (use --pre to allow).
  Could not find a version that satisfies the requirement pyramid-redis-sessions (from versions: 0.9b, 0.9b1, 0.9b2, 1.0a1)
Cleaning up...
  Removing temporary dir /tmp/foo/build...
No distributions matching the version for pyramid-redis-sessions
Exception information:
Traceback (most recent call last):
  File "/tmp/foo/local/lib/python2.7/site-packages/pip/basecommand.py", line 122, in main
    status = self.run(options, args)
  File "/tmp/foo/local/lib/python2.7/site-packages/pip/commands/install.py", line 270, in run
    requirement_set.prepare_files(finder, force_root_egg_info=self.bundle, bundle=self.bundle)
  File "/tmp/foo/local/lib/python2.7/site-packages/pip/req.py", line 1157, in prepare_files
    url = finder.find_requirement(req_to_install, upgrade=self.upgrade)
  File "/tmp/foo/local/lib/python2.7/site-packages/pip/index.py", line 330, in find_requirement
    raise DistributionNotFound('No distributions matching the version for %s' % req)
DistributionNotFound: No distributions matching the version for pyramid-redis-sessions

HTTP Only should default to True

Current best practice insists that Session IDs be stored HTTP Only to prevent hijacking of the cookie via content injection. This should be the default.

Configuration of sessions based on url no longer works

If you use the redis.url config settings. The options for the connection defaulted here are in turn passed into here throwing this traceback:

2013-03-22 10:20:52,628 ERROR [waitress][Dummy-2] Exception when serving /favicon.ico
Traceback (most recent call last):
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/waitress/channel.py", line 329, in service
    task.service()
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/waitress/task.py", line 173, in service
    self.execute()
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/waitress/task.py", line 380, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/router.py", line 251, in __call__
    response = self.invoke_subrequest(request, use_tweens=True)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/router.py", line 227, in invoke_subrequest
    response = handle_request(request)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/tweens.py", line 21, in excview_tween
    response = handler(request)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid_tm/__init__.py", line 50, in tm_tween
    userid = unauthenticated_userid(request)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/security.py", line 89, in unauthenticated_userid
    return policy.unauthenticated_userid(request)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/authentication.py", line 1040, in unauthenticated_userid
    return request.session.get(self.userid_key)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/decorator.py", line 39, in __get__
    val = self.wrapped(inst)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/pyramid/request.py", line 350, in session
    return factory(self)
  File "/opt/webapp/clusterflunk/src/pyramid-redis-sessions/pyramid_redis_sessions/__init__.py", line 172, in factory
    redis = get_default_connection(request, url=url, **redis_options)
  File "/opt/webapp/clusterflunk/src/pyramid-redis-sessions/pyramid_redis_sessions/connection.py", line 81, in get_default_connection
    redis = redis_client.from_url(url, **redis_options)
  File "/opt/webapp/clusterflunk/lib/python2.7/site-packages/redis/client.py", line 264, in from_url
    password=url.password, **kwargs)
TypeError: type object got multiple values for keyword argument 'password'

invalidation workflow is broken

As reported by whiteroses in #pyramid, it's currently not possible to mutate a session after invalidating it which is possibly the most common workflow that invalidation is intended to support. The issue appears to be that a call to invalidate() adds a callback to delete the cookie, and later once the session is mutated again (in the same request) the changes are lost as the cookie is still deleted.

def login(request):
    request.session.invalidate()
    request.session['user_id'] = user_id

test_factory_parameters_used_to_set_cookie test failing

test_factory_parameters_used_to_set_cookie is failing on master branch.

WebOb renamed the key argument in webob.response.set_cookie and webob.response.delete_cookie to name. It's been deprecated for a while but it was removed in 1.7.

redis `set`+`expire` could just be `setex`

setex does both [http://redis.io/commands/SETEX] in a single call. The performane difference in python is non-existent, but it's much better for the server.

It requires Redis 2.0 -- which was released in 2010.

I'll generate a PR.

(I'm doing tickets and PRs for everything separate so you can pick & choose if there are any issues).

DeprecationWarning: "charset" & "errors" (Redis v2.10.5)

using pyramid-redis-sessions version v1.0.1 with redis v2.10.5, i'm getting two warnings:

redis/client.py:408: DeprecationWarning: "charset" is deprecated. Use "encoding" instead '"charset" is deprecated. Use "encoding" instead'))

/redis/client.py:412: DeprecationWarning: "errors" is deprecated. Use "encoding_errors" instead '"errors" is deprecated. Use "encoding_errors" instead'))

the package talks to redis too much.

The TTL is set on every attribute access.
The session is serialized on every attribute write.

This should only happen once per request.

Addressed in #63

Adjust Session Timeout

I'm using adjust_session_timeout as described in the docs, and my users are still getting logged out after a short amount of time. Here are my settings:

clusterflunk.remember_me_timeout = 2592000
redis.sessions.cookie_max_age = 2592000

and here is the code where the timeout time is adjusted:

if payload['remember_me']:
    session.adjust_timeout_for_session(settings['clusterflunk.remember_me_timeout'])

config.include doesn't work if you haven't set any redis.sessions options in the ini file

While I figured out from reading the code that the 'secret' option has no default (and that makes sense), there should be a check and an error message, not this unhelpful traceback.

  File "/Users/jon/code/aisleplanner/aisleplanner/__init__.py", line 32, in main
    config.include('pyramid_redis_sessions')
  File "/Users/jon/code/aisleplanner/localpy/lib/python2.7/site-packages/pyramid/config/__init__.py", line 737, in include
    c(configurator)
  File "/Users/jon/code/aisleplanner/localpy/lib/python2.7/site-packages/pyramid_redis_sessions/__init__.py", line 25, in includeme
    session_factory = session_factory_from_settings(config.registry.settings)
  File "/Users/jon/code/aisleplanner/localpy/lib/python2.7/site-packages/pyramid_redis_sessions/__init__.py", line 34, in session_factory_from_settings
    return RedisSessionFactory(**options)
TypeError: RedisSessionFactory() takes at least 1 argument (0 given)

[Documentation] How do I configure via Paste deploy?

Next, configure pyramid_redis_sessions via your Paste config file.
-- Getting Started guide

The Pyramid INIs have multiple sections. Below which section should the redis settings reside? Or should they own their own section?

support clear() and new cookie signing

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.

How do I deal with redis upper-bound requirements?

I'm doing my first steps in redis and promptly get an error for python setup.py develop:

error: Installed distribution redis 2.10.3 conflicts with requirement redis<=2.9.1

My Pyramid setup contains these requirements:

requires = [... "redis", "pyramid_redis_sessions"]

I intend do use redit for other middleware tasks (i.e. caching) so I'd love to have the newest version. I changed the requires.txt in the EGG-INFO folder to redis>=2.4.11 and reran setup.py to silence the error but I have no idea what will break (aside from deployment on the target platform).

What should I do?

use a config option to disable GET/SET on TTL

stubbing the ticket, I can handle this.

If redis is configured to act as an LRU cache with server-side expire policies, there is no need to set the EXPIRE/SETEX or even use any TTL data.

Having an option to disable this would:

cut down on wire traffic
cut down on memory used by redis (the expire data would not need to be maintained)

Session invalidates upon redirect from site

I have some code that starts Twitter authenticated, by storing off some session keys, then redirecting to Twitter. Once Twitter redirects back to my site, the session has invalidated, and the values are no longer available. This is only happening after upgrading to master.

Possible bug: constructor overwriting reify method

in session.py, line 84 in the constructor says:

self._session_state = self._make_session_state(....

However on line 90 of the same file, there already is a @reify method also called _session_state in the same class, so the way I am reading this, as soon as the constructor is run, that method is "replaced" with an instance variable.

This doesn't sound right.

...

Seems to be vulnerable to the pickle security issue (remote code execution)

I am using Pyramid and Beaker at the moment for sessions, but Beaker is currently also vulnerable to the pickle issue found in so many Python frameworks:

http://www.balda.ch/posts/2013/Jun/23/python-web-frameworks-pickle/

Django has fixed it now, but Beaker and Pyramid's built-in session haven't

I was hoping that pyramid_redis_sessions would not be vulnerable to this security vulverability, but a quick grep of the code shows, that it too is using pickle instead of json for serialisation.

If this is an issue, can we please get this fixed by using JSON for serialisation, not pickle

potential issue with deserialization

I recently fixed this in my fork, and wanted to warn you here:

If an error occurs in deserialization during from_redis https://github.com/ericrasmussen/pyramid_redis_sessions/blob/master/pyramid_redis_sessions/session.py#L142-L147 , it will bubble up as a fatal exception for the request under pyramid.

It's not likely, but possible. It will always happen if you change the session (de)serialization methods and haven't cleared out existing sessions yet, or somehow your Redis data gets corrupted (which can happen after a reboot if you have low disk space).

I addressed it by catching exceptions during deserialization and raising an InvalidSession error (https://github.com/jvanasco/pyramid_redis_sessions/blob/custom_deployment/pyramid_redis_sessions/session.py#L203-L223).

My session initialization function is slightly different than yours -- I just use a GET to avoid the call to EXISTS, and create a new session if the id was invalid, so it was a quick fix for me. https://github.com/jvanasco/pyramid_redis_sessions/blob/custom_deployment/pyramid_redis_sessions/__init__.py#L222-L250

Option for not update expiration of session

Sometimes, session cache values which were fetched from database server. So, in this case, session should be invalidate/delete after expire time (in configuration file *.ini) and this option should be declared in configuration file too (for example, fixed_timeout = boolean). I mean, if timeout is set 1 hour, the session will be deleted after exact 1 four later, no matter what its last accessed time.

What do you think about my suggestion?

Does not have concurrency control (locking or optimistic) so race conditions are possible

I see no sign of locking here. Session is a shared resource. To avoid losing data and to have predictable results, concurrency control is needed. In the case of Redis it means you need to use locking, so that parallel requests using same session don't overwrite each other's session data.
Google "session race condition" to get plenty of information on this topic.

The irony of the fact is using exclusive locks partly defeats the whole idea of using fast session backend such as Redis: with locking parallel requests will effectively wait in a queue. But of course this will only affect same session requests. This is one of the reasons why I think database-backed sessions are not such a bad idea after all. Atleast, when we will have pyramid_retry finally working.

Session ID generation is insecure

The session ID generator at https://github.com/ericrasmussen/pyramid_redis_sessions/blob/master/pyramid_redis_sessions/util.py#L41 uses a non-cryptographic source of randomness in a predictable fashion in order to generate session keys:

    when = time.time()
    period = 1
    this_period = int(when - (when % period))
    rand = random.randint(0, 99999999)
    global _CURRENT_PERIOD
    if this_period != _CURRENT_PERIOD:
        _CURRENT_PERIOD = this_period
    source = to_binary('%s%s%s' % (rand, when, pid))
    session_id = sha1(source).hexdigest()
    return session_id

This poor-mans PRNG produces relatively predictable nonces to feed the SHA1 algorithm (keyspace of 99999999_len(str(time.time() % 1))_65535) which is significantly smaller (8.5*10^13) than the correct key space of 256^20 for that nonce size. It may also be vulnerable to timing attacks and poor system clocks.

In addition, this code makes use of a single SHA1 op. In total this reduces the session ID entropy well below that recommended as the minimum by OWASP

https://www.owasp.org/index.php/Session_Management_Cheat_Sheet#Session_ID_Properties

The session ID should be generated by using a proper cryptographic random source such as os.urandom. It may be advisable to protect the source by using a double SHA-256 operation.

I strongly recommend having session code reviewed by someone with a suitable security background.

Session without timeout

Please add support for inifinit session, it'll be very useful.

redis-py 2.10 changed "errors" connection keyword argument to "encoding_errors"

The latest version of redis-py changed the keyword argument "errors" to "encoding_errorrs" resulting in the following error message when attempting to use request.session

Traceback (most recent call last):
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/waitress/channel.py", line 337, in service
    task.service()
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/waitress/task.py", line 173, in service
    self.execute()
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/waitress/task.py", line 392, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/router.py", line 242, in __call__
    response = self.invoke_subrequest(request, use_tweens=True)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/router.py", line 217, in invoke_subrequest
    response = handle_request(request)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid_debugtoolbar/toolbar.py", line 165, in toolbar_tween
    return handler(request)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/tweens.py", line 21, in excview_tween
    response = handler(request)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/router.py", line 163, in handle_request
    response = view_callable(context, request)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/config/views.py", line 355, in rendered_view
    result = view(context, request)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/config/views.py", line 501, in _requestonly_view
    response = view(request)
  File "/home/mmartinez/test/test/views.py", line 6, in my_view
    request.session
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/decorator.py", line 37, in __get__
    val = self.wrapped(inst)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid/request.py", line 200, in session
    return factory(self)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/pyramid_redis_sessions/__init__.py", line 225, in factory
    is_new_session = redis.get(session_id) is None
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/redis/client.py", line 807, in get
    return self.execute_command('GET', name)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/redis/client.py", line 526, in execute_command
    connection = pool.get_connection(command_name, **options)
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/redis/connection.py", line 819, in get_connection
    connection = self.make_connection()
  File "/home/mmartinez/env/sandbox/lib/python2.7/site-packages/redis/connection.py", line 828, in make_connection
    return self.connection_class(**self.connection_kwargs)
TypeError: __init__() got an unexpected keyword argument 'errors'

suggestion: multiple, configurable, request properties

I'm looking at ditching Beaker, and wanted to make a suggestion/request. (i'd be wiling to code this too).

I would like the request property to be configurable, and to be able to run multiple sessions on a single request.

A practical example would be adding a secondary https-only session as request.session_https

synchronize to current redis-py

The package is pinned to redis<=2.9.1 (i.e., redis-py), and the changelog says this is due to an API incompatibility in the 2.10 series (redis/redis-py#510). That bug is now closed, so can the pin be removed?

This implementation can not handle nested/multilevel dictionaries properly

I caught this a month ago in my fork as the issue for many bugs during integration testing -- the current implementation does not catch nested dict values changing.

eg the following code will not automatically persist:

Request 1: sesssion.data = {'foo': {'bar': 1}}
Request 2: sesssion.data['foo']['bar'] = 2

Persisting Request 2 to redis requires an explicit call to session.changed

I handled it in my fork by calculating a md5 hash of the from_redis string on from_redis (https://github.com/jvanasco/pyramid_redis_sessions/blob/custom_deployment/pyramid_redis_sessions/session.py#L189-L198). If an explicit persist is not requested in my finished callback (https://github.com/jvanasco/pyramid_redis_sessions/blob/custom_deployment/pyramid_redis_sessions/__init__.py#L354-L371), I check to see if the current hashed value is different via a should_persist query (https://github.com/jvanasco/pyramid_redis_sessions/blob/custom_deployment/pyramid_redis_sessions/session.py#L52-L61)

there are probably better ways to detect change, but calculating an md5 value is negligible and quick to implement.

i backported the test case for your version (attached)

 TestRedisSession(unittest.TestCase):
    ...
    def test_dict_multilevel(self):
        inst = self._set_up_session_in_Redis_and_makeOne(session_id='test1')
        inst['dict'] = {'foo': {'bar': 1}}
        get_from_inst = inst['dict']['foo']['bar']
        self.assertEqual(get_from_inst, 1)
        session_dict_in_redis = inst.from_redis()['managed_dict']
        get_from_redis = session_dict_in_redis['dict']['foo']['bar']
        self.assertEqual(get_from_redis, 1)
        inst['dict']['foo']['bar'] = 2
        ## requires a call to `changed()` under current system
        # inst.changed()
        session_dict_in_redis2 = inst.from_redis()['managed_dict']
        get_from_redis2 = session_dict_in_redis2['dict']['foo']['bar']
        self.assertEqual(get_from_redis2, 2)

Session Key Prefix

In my opinion there should be an easy way to set a prefix, that gets appended to all redis keys. This makes for easily distinguishable keys.

Is the generated session the same one been pass via the request.session property?

Hi,
In order for multiple cookie/session support direct references to request.session (RE: set_cookie, cookie_callback) will obviously fail (sessions are managed via a sessionmanager property itself attached to the request object - WIP).
Please see review changes to compare here.
Currently testing and appears to be ok. Please advise if I've missed something or discuss.
Thanks.

ericrasmussen / pyramid_redis_sessions Goto Github PK

pyramid_redis_sessions's People

Contributors

Stargazers

Watchers

Forkers

pyramid_redis_sessions's Issues

Recommend Projects

Recommend Topics

Recommend Org