From 41039d48188fd058736eec03d10e0f6b2606ac35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 10:01:32 +0200 Subject: [PATCH 1/4] [#2990] Create 403 on organization_index if user has no permission --- ckan/logic/action/get.py | 12 ++++++++---- ckan/tests/controllers/test_organization.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index e57e4a1819d..200981aeccb 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -26,6 +26,7 @@ import ckan.authz as authz from ckan.common import _ +from ckan.lib import base log = logging.getLogger('ckan.logic') @@ -518,10 +519,13 @@ def organization_list(context, data_dict): :rtype: list of strings ''' - _check_access('organization_list', context, data_dict) - data_dict['groups'] = data_dict.pop('organizations', []) - data_dict.setdefault('type', 'organization') - return _group_or_org_list(context, data_dict, is_org=True) + try: + _check_access('organization_list', context, data_dict) + data_dict['groups'] = data_dict.pop('organizations', []) + data_dict.setdefault('type', 'organization') + return _group_or_org_list(context, data_dict, is_org=True) + except logic.NotAuthorized: + base.abort(403, _('You are not authorized to see a list of organizations')) def group_list_authz(context, data_dict): diff --git a/ckan/tests/controllers/test_organization.py b/ckan/tests/controllers/test_organization.py index ce8a875928b..0db260cb38e 100644 --- a/ckan/tests/controllers/test_organization.py +++ b/ckan/tests/controllers/test_organization.py @@ -61,6 +61,21 @@ def test_all_fields_saved(self): assert_equal(group['description'], 'Sciencey datasets') +class TestOrganizationList(helpers.FunctionalTestBase): + def setup(self): + super(TestOrganizationList, self).setup() + self.app = helpers._get_test_app() + self.user = factories.User() + self.user_env = {'REMOTE_USER': self.user['name'].encode('ascii')} + self.organization_list_url = url_for(controller='organization', + action='index') + + def test_error_message_shown_when_no_organization_list_permission(self, mock_check_access): + response = self.app.get(url=self.organization_list_url, + extra_environ=self.user_env) + assert response.status_int == 403 + + class TestOrganizationRead(helpers.FunctionalTestBase): def setup(self): super(TestOrganizationRead, self).setup() From 0fa410b710e8d06c2d1e698772237dfa937e5d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 11:57:40 +0200 Subject: [PATCH 2/4] [#2990] Add test for organization_list - The test makes sure that the response is a 403 - Mosly this is to make sure that the view renders at all and that it does not throw an exception in the backend anymore like it used to --- ckan/tests/controllers/test_organization.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ckan/tests/controllers/test_organization.py b/ckan/tests/controllers/test_organization.py index 0db260cb38e..549a92aa9f1 100644 --- a/ckan/tests/controllers/test_organization.py +++ b/ckan/tests/controllers/test_organization.py @@ -1,6 +1,7 @@ from bs4 import BeautifulSoup from nose.tools import assert_equal, assert_true from routes import url_for +from mock import patch from ckan.tests import factories, helpers from ckan.tests.helpers import webtest_submit, submit_and_follow, assert_in @@ -70,10 +71,11 @@ def setup(self): self.organization_list_url = url_for(controller='organization', action='index') + @patch('ckan.logic.auth.get.organization_list', return_value={'success': False}) def test_error_message_shown_when_no_organization_list_permission(self, mock_check_access): response = self.app.get(url=self.organization_list_url, - extra_environ=self.user_env) - assert response.status_int == 403 + extra_environ=self.user_env, + status=403) class TestOrganizationRead(helpers.FunctionalTestBase): From bcc644f67dcc983a127f31ce9fc44d6b6924c711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 14:39:35 +0200 Subject: [PATCH 3/4] [#2990] Move check for permission to controller --- ckan/controllers/group.py | 1 + ckan/logic/action/get.py | 11 ++++------- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index 61f6a76b20f..b01050d37fc 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -159,6 +159,7 @@ def index(self): sort_by = c.sort_by_selected = request.params.get('sort') try: self._check_access('site_read', context) + self._check_access('group_list', context) except NotAuthorized: abort(403, _('Not authorized to see this page')) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 200981aeccb..2a65d4d49ee 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -519,13 +519,10 @@ def organization_list(context, data_dict): :rtype: list of strings ''' - try: - _check_access('organization_list', context, data_dict) - data_dict['groups'] = data_dict.pop('organizations', []) - data_dict.setdefault('type', 'organization') - return _group_or_org_list(context, data_dict, is_org=True) - except logic.NotAuthorized: - base.abort(403, _('You are not authorized to see a list of organizations')) + _check_access('organization_list', context, data_dict) + data_dict['groups'] = data_dict.pop('organizations', []) + data_dict.setdefault('type', 'organization') + return _group_or_org_list(context, data_dict, is_org=True) def group_list_authz(context, data_dict): From 3c802655579f2e94087626f55f3d09ed6531b20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20H=C3=BChne?= Date: Fri, 20 May 2016 14:48:12 +0200 Subject: [PATCH 4/4] [#2990] remove stray import --- ckan/logic/action/get.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 2a65d4d49ee..e57e4a1819d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -26,7 +26,6 @@ import ckan.authz as authz from ckan.common import _ -from ckan.lib import base log = logging.getLogger('ckan.logic')