From 9ea722e56d4308a7509e49e300877f343b657aeb Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Wed, 23 Apr 2014 17:56:18 -0400 Subject: [PATCH 1/6] [#1664] package_create+update: fix config permission check Use the same permission checks for both package_update and package_create and check all the applicable config options: anon_create_dataset, create_unowned_dataset and create_dataset_if_not_in_organization --- ckan/logic/auth/create.py | 14 ++++++++++---- ckan/logic/auth/update.py | 15 +++++++++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/ckan/logic/auth/create.py b/ckan/logic/auth/create.py index e9082fc6b77..a14bb15d381 100644 --- a/ckan/logic/auth/create.py +++ b/ckan/logic/auth/create.py @@ -9,11 +9,17 @@ def package_create(context, data_dict=None): user = context['user'] if new_authz.auth_is_anon_user(context): - check1 = new_authz.check_config_permission('anon_create_dataset') + check1 = all(new_authz.check_config_permission(p) for p in ( + 'anon_create_dataset', + 'create_dataset_if_not_in_organization', + 'create_unowned_dataset', + )) else: - check1 = new_authz.check_config_permission('create_dataset_if_not_in_organization') \ - or new_authz.check_config_permission('create_unowned_dataset') \ - or new_authz.has_user_permission_for_some_org(user, 'create_dataset') + check1 = all(new_authz.check_config_permission(p) for p in ( + 'create_dataset_if_not_in_organization', + 'create_unowned_dataset', + )) or new_authz.has_user_permission_for_some_org( + user, 'create_dataset') if not check1: return {'success': False, 'msg': _('User %s not authorized to create packages') % user} diff --git a/ckan/logic/auth/update.py b/ckan/logic/auth/update.py index 176c333a5ff..ac416bbcb93 100644 --- a/ckan/logic/auth/update.py +++ b/ckan/logic/auth/update.py @@ -23,11 +23,18 @@ def package_update(context, data_dict): ) else: # If dataset is not owned then we can edit if config permissions allow - if not new_authz.auth_is_anon_user(context): - check1 = new_authz.check_config_permission( - 'create_dataset_if_not_in_organization') + if new_authz.auth_is_anon_user(context): + check1 = all(new_authz.check_config_permission(p) for p in ( + 'anon_create_dataset', + 'create_dataset_if_not_in_organization', + 'create_unowned_dataset', + )) else: - check1 = new_authz.check_config_permission('anon_create_dataset') + check1 = all(new_authz.check_config_permission(p) for p in ( + 'create_dataset_if_not_in_organization', + 'create_unowned_dataset', + )) or new_authz.has_user_permission_for_some_org( + user, 'create_dataset') if not check1: return {'success': False, 'msg': _('User %s not authorized to edit package %s') % From 746cfc56eb1881a0d0b53191fd44405272f1fa04 Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Thu, 1 May 2014 16:11:06 -0300 Subject: [PATCH 2/6] [#1665] Add new_tests.helpers.change_config() decorator This allows you to temporarily change a CKAN's config value during a test. It'll restore the values to what they were before, after you test is ran. Conflicts: ckan/new_tests/helpers.py --- ckan/new_tests/helpers.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/ckan/new_tests/helpers.py b/ckan/new_tests/helpers.py index e6799fe0ece..42f9c952be2 100644 --- a/ckan/new_tests/helpers.py +++ b/ckan/new_tests/helpers.py @@ -17,8 +17,9 @@ This module is reserved for these very useful functions. ''' -import pylons.config as config import webtest +from pylons import config +import nose.tools import ckan.config.middleware import ckan.model as model @@ -137,3 +138,37 @@ def _get_test_app(): app = ckan.config.middleware.make_app(config['global_conf'], **config) app = webtest.TestApp(app) return app + + +def change_config(key, value): + '''Decorator to temporarily changes Pylons' config to a new value + + This allows you to easily create tests that need specific config values to + be set, making sure it'll be reverted to what it was originally, after your + test is run. + + Usage:: + + @helpers.change_config('ckan.site_title', 'My Test CKAN') + def test_ckan_site_title(self): + assert pylons.config['ckan.site_title'] == 'My Test CKAN' + + :param key: the config key to be changed, e.g. ``'ckan.site_title'`` + :type key: string + + :param value: the new config key's value, e.g. ``'My Test CKAN'`` + :type value: string + ''' + def decorator(func): + def wrapper(*args, **kwargs): + _original_config = config.copy() + config[key] = value + + return_value = func(*args, **kwargs) + + config.clear() + config.update(_original_config) + + return return_value + return nose.tools.make_decorator(func)(wrapper) + return decorator From 318891ef87fb95f91210ed6d8885af4529d1f232 Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Fri, 13 Jun 2014 16:23:33 -0400 Subject: [PATCH 3/6] [#1664] clear auth functions cache when changing config --- ckan/new_tests/helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ckan/new_tests/helpers.py b/ckan/new_tests/helpers.py index 42f9c952be2..3b8f33722a3 100644 --- a/ckan/new_tests/helpers.py +++ b/ckan/new_tests/helpers.py @@ -24,6 +24,7 @@ import ckan.config.middleware import ckan.model as model import ckan.logic as logic +import ckan.new_authz as new_authz def reset_db(): @@ -163,11 +164,13 @@ def decorator(func): def wrapper(*args, **kwargs): _original_config = config.copy() config[key] = value + new_authz.clear_auth_functions_cache() return_value = func(*args, **kwargs) config.clear() config.update(_original_config) + new_authz.clear_auth_functions_cache() return return_value return nose.tools.make_decorator(func)(wrapper) From 66fec03d6acf38f97d192dec55e834f8be518835 Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Fri, 13 Jun 2014 16:30:41 -0400 Subject: [PATCH 4/6] [#1664] tests for anon users --- ckan/new_tests/logic/auth/test_create.py | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/ckan/new_tests/logic/auth/test_create.py b/ckan/new_tests/logic/auth/test_create.py index f5ced243ed0..6ebca0a4d97 100644 --- a/ckan/new_tests/logic/auth/test_create.py +++ b/ckan/new_tests/logic/auth/test_create.py @@ -7,8 +7,34 @@ import ckan.new_tests.helpers as helpers import ckan.new_tests.factories as factories +import ckan.logic.auth.create as auth_create logic = helpers.logic +assert_equals = nose.tools.assert_equals + + +class TestCreateDatasetSettings(object): + def test_anon_cant_create_dataset(self): + response = auth_create.package_create({'user': None}, None) + assert_equals(response['success'], False) + + @helpers.change_config('ckan.auth.anon_create_dataset', True) + def test_anon_can_create_dataset(self): + response = auth_create.package_create({'user': None}, None) + assert_equals(response['success'], True) + + @helpers.change_config('ckan.auth.anon_create_dataset', True) + @helpers.change_config('ckan.auth.create_dataset_if_not_in_organization', + False) + def test_cdnio_overrides_acd(self): + response = auth_create.package_create({'user': None}, None) + assert_equals(response['success'], False) + + @helpers.change_config('ckan.auth.anon_create_dataset', True) + @helpers.change_config('ckan.auth.create_unowned_dataset', False) + def test_cud_overrides_acd(self): + response = auth_create.package_create({'user': None}, None) + assert_equals(response['success'], False) class TestCreate(object): From 64fd260e20508d25eb4fb3e67db7f2627784daf1 Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Mon, 16 Jun 2014 09:21:29 -0400 Subject: [PATCH 5/6] [#1664] tests for logged in users --- ckan/new_tests/logic/auth/test_create.py | 49 ++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/ckan/new_tests/logic/auth/test_create.py b/ckan/new_tests/logic/auth/test_create.py index 6ebca0a4d97..6edf3500ada 100644 --- a/ckan/new_tests/logic/auth/test_create.py +++ b/ckan/new_tests/logic/auth/test_create.py @@ -13,13 +13,13 @@ assert_equals = nose.tools.assert_equals -class TestCreateDatasetSettings(object): - def test_anon_cant_create_dataset(self): +class TestCreateDatasetAnonymousSettings(object): + def test_anon_cant_create(self): response = auth_create.package_create({'user': None}, None) assert_equals(response['success'], False) @helpers.change_config('ckan.auth.anon_create_dataset', True) - def test_anon_can_create_dataset(self): + def test_anon_can_create(self): response = auth_create.package_create({'user': None}, None) assert_equals(response['success'], True) @@ -37,6 +37,49 @@ def test_cud_overrides_acd(self): assert_equals(response['success'], False) +class TestCreateDatasetLoggedInSettings(object): + def setup(self): + helpers.reset_db() + + def test_no_org_user_can_create(self): + user = factories.User() + response = auth_create.package_create({'user': user.name}, None) + assert_equals(response['success'], True) + + @helpers.change_config('ckan.auth.anon_create_dataset', True) + @helpers.change_config('ckan.auth.create_dataset_if_not_in_organization', + False) + def test_no_org_user_cant_create_if_cdnio_false(self): + user = factories.User() + response = auth_create.package_create({'user': user.name}, None) + assert_equals(response['success'], False) + + @helpers.change_config('ckan.auth.anon_create_dataset', True) + @helpers.change_config('ckan.auth.create_unowned_dataset', False) + def test_no_org_user_cant_create_if_cud_false(self): + user = factories.User() + response = auth_create.package_create({'user': user.name}, None) + assert_equals(response['success'], False) + + def test_same_org_user_can_create(self): + user = factories.User() + org_users = {'name': user.name, 'capacity': 'member'} + org = factories.Organization(users=org_users) + dataset = {'name': 'same-org-user-can-create', 'owner_org': org.id} + response = auth_create.package_create({'user': user.name}, dataset) + assert_equals(response['success'], True) + + def test_different_org_user_cant_create(self): + user = factories.User() + org_users = {'name': user.name, 'capacity': 'member'} + org1 = factories.Organization(users=org_users) + org2 = factories.Organization() + dataset = {'name': 'different-org-user-cant-create', + 'owner_org': org2.id} + response = auth_create.package_create({'user': user.name}, dataset) + assert_equals(response['success'], False) + + class TestCreate(object): def setup(self): From 70c63ab2fde30ea545a34ca29bf88590db0ead3f Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Mon, 16 Jun 2014 09:58:09 -0400 Subject: [PATCH 6/6] [#1664] fix tests --- ckan/new_tests/logic/auth/test_create.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/ckan/new_tests/logic/auth/test_create.py b/ckan/new_tests/logic/auth/test_create.py index 6edf3500ada..030ebed5f0c 100644 --- a/ckan/new_tests/logic/auth/test_create.py +++ b/ckan/new_tests/logic/auth/test_create.py @@ -5,6 +5,7 @@ import mock import nose +import ckan.model as core_model import ckan.new_tests.helpers as helpers import ckan.new_tests.factories as factories import ckan.logic.auth.create as auth_create @@ -43,7 +44,7 @@ def setup(self): def test_no_org_user_can_create(self): user = factories.User() - response = auth_create.package_create({'user': user.name}, None) + response = auth_create.package_create({'user': user['name']}, None) assert_equals(response['success'], True) @helpers.change_config('ckan.auth.anon_create_dataset', True) @@ -51,32 +52,34 @@ def test_no_org_user_can_create(self): False) def test_no_org_user_cant_create_if_cdnio_false(self): user = factories.User() - response = auth_create.package_create({'user': user.name}, None) + response = auth_create.package_create({'user': user['name']}, None) assert_equals(response['success'], False) @helpers.change_config('ckan.auth.anon_create_dataset', True) @helpers.change_config('ckan.auth.create_unowned_dataset', False) def test_no_org_user_cant_create_if_cud_false(self): user = factories.User() - response = auth_create.package_create({'user': user.name}, None) + response = auth_create.package_create({'user': user['name']}, None) assert_equals(response['success'], False) def test_same_org_user_can_create(self): user = factories.User() - org_users = {'name': user.name, 'capacity': 'member'} + org_users = [{'name': user['name'], 'capacity': 'editor'}] org = factories.Organization(users=org_users) - dataset = {'name': 'same-org-user-can-create', 'owner_org': org.id} - response = auth_create.package_create({'user': user.name}, dataset) + dataset = {'name': 'same-org-user-can-create', 'owner_org': org['id']} + context = {'user': user['name'], 'model': core_model} + response = auth_create.package_create(context, dataset) assert_equals(response['success'], True) def test_different_org_user_cant_create(self): user = factories.User() - org_users = {'name': user.name, 'capacity': 'member'} + org_users = [{'name': user['name'], 'capacity': 'editor'}] org1 = factories.Organization(users=org_users) org2 = factories.Organization() dataset = {'name': 'different-org-user-cant-create', - 'owner_org': org2.id} - response = auth_create.package_create({'user': user.name}, dataset) + 'owner_org': org2['id']} + context = {'user': user['name'], 'model': core_model} + response = auth_create.package_create(context, dataset) assert_equals(response['success'], False)