search

data-govt-nz / ckanext-security

A CKAN extension to hold various security improvements for CKAN
GNU Affero General Public License v3.0
25 stars 32 forks source link

support ckan 2.7 and redis #18

Closed ebuckley closed 5 years ago

ebuckley commented 6 years ago

Includes changes to support moving from memcache to redis, and support for ckan 2.7.

Original PR for redis support: https://github.com/data-govt-nz/ckanext-security/pull/16

Todo before release

ebuckley commented 6 years ago

@anotheredward I have been integrating this with ckan 2.7 and noticed a couple cases where a 403 or 500 error would cause when doing a Form POST (I.E login page).

Can you code check and help manual test this change. (should be able to check it from my docker-compose PR 'coming soon'.)

ebuckley commented 6 years ago

The related PR for testing this branch is here @anotheredward https://github.com/mediasuitenz/catalogue.data.govt.nz/pull/7

anotheredward commented 6 years ago

Known limitation of the old approach:

michab23 commented 6 years ago

Hi all, I have been integrating this with ckan 2.7.2 and get the following error: Exception happened during processing of request from ('127.0.0.1', 57496) Traceback (most recent call last): File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/httpserver.py", line 1068, in process_request_in_thread self.finish_request(request, client_address) File "/usr/lib/python2.7/SocketServer.py", line 331, in finish_request self.RequestHandlerClass(request, client_address, self) File "/usr/lib/python2.7/SocketServer.py", line 652, in init self.handle() File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/httpserver.py", line 442, in handle BaseHTTPRequestHandler.handle(self) File "/usr/lib/python2.7/BaseHTTPServer.py", line 340, in handle self.handle_one_request() File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/httpserver.py", line 437, in handle_one_request self.wsgi_execute() File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/httpserver.py", line 287, in wsgi_execute self.wsgi_start_response) File "/usr/lib/gov_ckan/default/src/ckan/ckan/config/middleware/init.py", line 136, in call return self.apps[app_name](environ, start_response) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/cascade.py", line 130, in call return self.apps[-1](environ, start_response) File "/usr/lib/gov_ckan/default/src/ckan/ckan/config/middleware/common_middleware.py", line 61, in call return self.app(environ, start_response) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/paste/registry.py", line 379, in call app_iter = self.application(environ, start_response) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/Beaker-1.6.5.post1-py2.7.egg/beaker/middleware.py", line 155, in call return self.wrap_app(environ, session_start_response) File "/usr/lib/gov_ckan/default/src/ckan/ckanext/ckanext-security/ckanext/security/middleware.py", line 66, in call resp = self.add_new_token(resp) File "/usr/lib/gov_ckan/default/src/ckan/ckanext/ckanext-security/ckanext/security/middleware.py", line 92, in add_new_token self.cache.set(self.session.id, token) File "/usr/lib/gov_ckan/default/src/ckan/ckanext/ckanext-security/ckanext/security/cache/clients.py", line 17, in set return self.client.set(self.prefix + key, value) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/client.py", line 1072, in set return self.execute_command('SET', pieces) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/client.py", line 572, in execute_command connection.send_command(args) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/connection.py", line 563, in send_command self.send_packed_command(self.pack_command(*args)) File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/connection.py", line 538, in send_packed_command self.connect() File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/connection.py", line 446, in connect self.on_connect() File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/connection.py", line 520, in on_connect if nativestr(self.read_response()) != 'OK': File "/usr/lib/gov_ckan/default/local/lib/python2.7/site-packages/redis/connection.py", line 582, in read_response raise response ResponseError: invalid DB index

Any ideas? Regards

anotheredward commented 6 years ago

@michab23 Looks like your redis configuration might be incorrect, potentially this line: ckanext.security.redis.db = 1 should be this ckanext.security.redis.db = 0 See this for more details: https://github.com/shellphish/driller/issues/1

This branch still requires a write-up, but the current recommendation is to have a seperate redis instance that behaves the same as memcached did (Fixed size, Deletes Least Recently Used) , or to use memcached.

This is to avoid eventually running out of space, as Redis doesn't delete entries and has no fixed size by default.

camfindlay commented 5 years ago

@ebuckley @anotheredward - worth looking at getting this merged? Do we need to hold a separate branch for any pre-2.7 users?

camfindlay commented 5 years ago

@ebuckley @anotheredward - worth looking at getting this merged? Do we need to hold a separate branch for any pre-2.7 users?

camfindlay commented 5 years ago

@ebuckley @anotheredward - worth looking at getting this merged? Do we need to hold a separate branch for any pre-2.7 users?

camfindlay commented 5 years ago

@ebuckley @anotheredward - worth looking at getting this merged? Do we need to hold a separate branch for any pre-2.7 users?

camfindlay commented 5 years ago

@ebuckley @anotheredward - worth looking at getting this merged? Do we need to hold a separate branch for any pre-2.7 users?

anotheredward commented 5 years ago

@camfindlay I think we need to write some tests around this to avoid future issues, should have some time in the not too distant future to investigate this issue and make sure it doesn't affect future users :)