Skip to content

Commit

Permalink
[#1210] Add auth_user_obj to context in check_access
Browse files Browse the repository at this point in the history
If not already there or ignore_auth is True, and the context has a user
name, try to see if the user actually exists in the DB, and store the
object (or None if not found) in context['auth_user_obj']. The check if
performed only once per context object.
  • Loading branch information
amercader committed Aug 23, 2013
1 parent 682e6bc commit c60cbbe
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 0 deletions.
9 changes: 9 additions & 0 deletions ckan/logic/__init__.py
Expand Up @@ -208,6 +208,15 @@ def check_access(action, context, data_dict=None):
user = context.get('user')
log.debug('check access - user %r, action %s' % (user, action))

if not 'auth_user_obj' in context:
context['auth_user_obj'] = None

if not context.get('ignore_auth'):
if not context.get('__auth_user_obj_checked'):
context['__auth_user_obj_checked'] = True

This comment has been minimized.

Copy link
@seanh

seanh Aug 29, 2013

Contributor

I'd move this to after the lines below that do the auth user check! :) Not that it should make any difference

This comment has been minimized.

Copy link
@amercader

amercader Aug 29, 2013

Author Member

Fair enough :)

if context.get('user') and not context.get('auth_user_obj'):
context['auth_user_obj'] = model.User.by_name(context['user'])

This comment has been minimized.

Copy link
@seanh

seanh Aug 29, 2013

Contributor

I'm a bit confused about why we need both 'auth_user_obj' and '__auth_user_obj_checked'. Isn't it enough to check whether 'auth_user_obj' is already there or not? I might be missing something

This comment has been minimized.

Copy link
@amercader

amercader Aug 29, 2013

Author Member

If it is an anonymous request ('auth_user_obj' is None) and check_access gets called multiple times we want to avoid querying the DB each time.


if action:
#if action != model.Action.READ and user in
# (model.PSEUDO_USER__VISITOR, ''):
Expand Down
51 changes: 51 additions & 0 deletions ckan/tests/logic/test_init.py
Expand Up @@ -3,6 +3,8 @@
import ckan.model as model
import ckan.logic as logic

from ckan.lib.create_test_data import CreateTestData


class TestMemberLogic(object):
def test_model_name_to_class(self):
Expand All @@ -11,3 +13,52 @@ def test_model_name_to_class(self):
logic.model_name_to_class,
model,
'inexistent_model_name')

class TestCheckAccess(object):


@classmethod
def setup_class(cls):
model.Session.close_all()
model.repo.delete_all()

def setup(self):
model.repo.rebuild_db()

def test_check_access_auth_user_obj_is_set(self):

CreateTestData.create_test_user()

user_name = 'tester'
context = {'user': user_name}

result = logic.check_access('package_create', context)

assert result
assert context['__auth_user_obj_checked']
assert context['auth_user_obj'].name == user_name

def test_check_access_auth_user_obj_is_not_set_when_ignoring_auth(self):

CreateTestData.create_test_user()

user_name = 'tester'
context = {'user': user_name, 'ignore_auth': True}

result = logic.check_access('package_create', context)

assert result
assert '__auth_user_obj_checked' not in context
assert context['auth_user_obj'] is None

def test_check_access_auth_user_obj_is_not_set(self):

user_names = ('unknown_user', '', None,)
for user_name in user_names:
context = {'user': user_name}

result = logic.check_access('package_search', context)

assert result
assert context['__auth_user_obj_checked']
assert context['auth_user_obj'] is None

0 comments on commit c60cbbe

Please sign in to comment.