From 5341906767e6eaff61573b142b6f965dfe43cc42 Mon Sep 17 00:00:00 2001 From: SSladarov <48349146+SSladarov@users.noreply.github.com> Date: Thu, 30 May 2019 15:38:30 +0200 Subject: [PATCH] Allow to access datasets description page when private (#51) --- .gitignore | 5 +- ckanext/privatedatasets/auth.py | 111 +++++++------ ckanext/privatedatasets/plugin.py | 75 +++++++-- .../templates/snippets/package_item.html | 6 +- .../templates_2.8/snippets/package_item.html | 6 +- ckanext/privatedatasets/tests/test_auth.py | 143 ++++++++++------ ckanext/privatedatasets/tests/test_helpers.py | 1 - ckanext/privatedatasets/tests/test_plugin.py | 157 +++++++++++++----- .../privatedatasets/tests/test_selenium.py | 8 +- test.ini | 1 + 10 files changed, 348 insertions(+), 165 deletions(-) diff --git a/.gitignore b/.gitignore index d382475..0200228 100644 --- a/.gitignore +++ b/.gitignore @@ -20,7 +20,7 @@ var/ *.egg-info/ .installed.cfg *.egg - +.eggs # Installer logs pip-log.txt pip-delete-this-directory.txt @@ -50,3 +50,6 @@ coverage.xml # Sphinx documentation docs/_build/ + +.idea +venv \ No newline at end of file diff --git a/ckanext/privatedatasets/auth.py b/ckanext/privatedatasets/auth.py index 204b999..64d07db 100644 --- a/ckanext/privatedatasets/auth.py +++ b/ckanext/privatedatasets/auth.py @@ -41,41 +41,37 @@ def package_show(context, data_dict): # Not active packages can only be seen by its owners if package.state == 'active': # anyone can see a public package - if not package.private: - return {'success': True} + if package.private: - # if the user has rights to read in the organization or in the group - if package.owner_org: - authorized = authz.has_user_permission_for_group_or_org( - package.owner_org, user, 'read') - else: - authorized = False + acquired = False - # if the user is not authorized yet, we should check if the - # user is in the allowed_users object - if not authorized: - # Init the model - db.init_db(context['model']) + if package.owner_org: + acquired = authz.has_user_permission_for_group_or_org( + package.owner_org, user, 'read') - # Branch not executed if the database return an empty list - if db.AllowedUser.get(package_id=package.id, user_name=user): - authorized = True + if not acquired: + # Init the model + db.init_db(context['model']) - if not authorized: - # Show a flash message with the URL to acquire the dataset - # This message only can be shown when the user tries to access the dataset via its URL (/dataset/...) - # The message cannot be displayed in other pages that uses the package_show function such as - # the user profile page + # Branch not executed if the database return an empty list + if db.AllowedUser.get(package_id=package.id, user_name=user): + acquired = True - if hasattr(package, 'extras') and 'acquire_url' in package.extras and request.path.startswith('/dataset/')\ - and package.extras['acquire_url'] != '': - helpers.flash_notice(_('This private dataset can be acquired. To do so, please click ' + - 'here') % package.extras['acquire_url'], - allow_html=True) + if not acquired: - return {'success': False, 'msg': _('User %s not authorized to read package %s') % (user, package.id)} - else: - return {'success': True} + # Show a flash message with the URL to acquire the dataset + # This message only can be shown when the user tries to access the dataset via its URL (/dataset/...) + # The message cannot be displayed in other pages that uses the package_show function such as + # the user profile page + + if hasattr(package, 'extras') and 'acquire_url' in package.extras and request.path.startswith( + '/dataset/') \ + and package.extras['acquire_url'] != '': + helpers.flash_notice(_('This private dataset can be acquired. To do so, please click ' + + 'here') % package.extras['acquire_url'], + allow_html=True) + + return {'success': True} else: return {'success': False, 'msg': _('User %s not authorized to read package %s') % (user, package.id)} @@ -104,32 +100,49 @@ def package_update(context, data_dict): @tk.auth_allow_anonymous_access def resource_show(context, data_dict): - # This function is needed since CKAN resource_show function uses the default package_show - # function instead of the one defined in the plugin. - # A bug is openend in order to be able to remove this function - # https://github.com/ckan/ckan/issues/1818 - # It's fixed now, so this function can be deleted when the new version is released. - _model = context['model'] - user = context.get('user') - resource = logic_auth.get_resource_object(context, data_dict) + user = context.get('user') + user_obj = context.get('auth_user_obj') + resource = logic_auth.get_resource_object(context, data_dict) # check authentication against package - query = _model.Session.query(_model.Package)\ - .join(_model.ResourceGroup)\ - .join(_model.Resource)\ - .filter(_model.ResourceGroup.id == resource.resource_group_id) - pkg = query.first() - if not pkg: + package_dict = {'id': resource.package_id} + package = logic_auth.get_package_object(context, package_dict) + if not package: raise tk.ObjectNotFound(_('No package found for this resource, cannot check auth.')) - pkg_dict = {'id': pkg.id} - authorized = package_show(context, pkg_dict).get('success') - - if not authorized: - return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)} - else: + if package and user_obj and package.creator_user_id == user_obj.id: return {'success': True} + # active packages can only be seen by its owners + if package.state == 'active': + + # anyone can see a public package + if not package.private: + return {'success': True} + + # if the user has rights to read in the organization or in the group + if package.owner_org: + authorized = authz.has_user_permission_for_group_or_org( + package.owner_org, user, 'read') + else: + authorized = False + + if not authorized: + # Init the model + db.init_db(context['model']) + + # Branch not executed if the database return an empty list + if db.AllowedUser.get(package_id=package.id, user_name=user): + authorized = True + + if not authorized: + return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)} + + else: + return {'success': True} + + else: + return {'success': False, 'msg': _('User %s not authorized to read resource %s') % (user, resource.id)} @tk.auth_allow_anonymous_access def package_acquired(context, data_dict): diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index eb165c4..e092782 100644 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -29,6 +29,7 @@ from flask import Blueprint from ckanext.privatedatasets import auth, actions, constants, converters_validators as conv_val, db, helpers from ckanext.privatedatasets.views import acquired_datasets + HIDDEN_FIELDS = [constants.ALLOWED_USERS, constants.SEARCHABLE] @@ -43,6 +44,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio p.implements(p.IPackageController, inherit=True) p.implements(p.ITemplateHelpers) p.implements(p.IPermissionLabels) + p.implements(p.IResourceController) ###################################################################### ############################ DATASET FORM ############################ @@ -112,16 +114,11 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio def get_auth_functions(self): auth_functions = {'package_show': auth.package_show, 'package_update': auth.package_update, - # 'resource_show': auth.resource_show, + 'resource_show': auth.resource_show, constants.PACKAGE_ACQUIRED: auth.package_acquired, constants.ACQUISITIONS_LIST: auth.acquisitions_list, constants.PACKAGE_DELETED: auth.revoke_access} - # resource_show is not required in CKAN 2.3 because it delegates to - # package_show - if not tk.check_ckan_version(min_version='2.3'): - auth_functions['resource_show'] = auth.resource_show - return auth_functions ###################################################################### @@ -162,11 +159,11 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio ###################################################################### def get_actions(self): - return { - constants.PACKAGE_ACQUIRED: actions.package_acquired, - constants.ACQUISITIONS_LIST: actions.acquisitions_list, - constants.PACKAGE_DELETED: actions.revoke_access - } + action_functions = {constants.PACKAGE_ACQUIRED: actions.package_acquired, + constants.ACQUISITIONS_LIST: actions.acquisitions_list, + constants.PACKAGE_DELETED: actions.revoke_access} + + return action_functions ###################################################################### ######################### IPACKAGECONTROLLER ######################### @@ -244,6 +241,16 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio def after_show(self, context, pkg_dict): + void = False; + + for resource in pkg_dict['resources']: + if resource == {}: + void = True + + if void: + del pkg_dict['resources'] + del pkg_dict['num_resources'] + user_obj = context.get('auth_user_obj') updating_via_api = context.get(constants.CONTEXT_CALLBACK, False) @@ -294,13 +301,29 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio # NotAuthorized exception is risen when the user is not allowed # to read the package. attrs.append('resources') - # Delete self._delete_pkg_atts(result, attrs) return search_results #### + def before_view(self, pkg_dict): + + for resource in pkg_dict['resources']: + + context = { + 'model': model, + 'session': model.Session, + 'user': tk.c.user, + 'user_obj': tk.c.userobj + } + + try: + tk.check_access('resource_show', context, resource) + except tk.NotAuthorized: + pkg_dict['resources'].remove(resource) + pkg_dict = self.before_view(pkg_dict) + return pkg_dict def get_dataset_labels(self, dataset_obj): labels = super(PrivateDatasets, self).get_dataset_labels( @@ -318,6 +341,34 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissio labels.append('searchable') return labels + ###################################################################### + ######################### IRESOURCECONTROLLER ######################## + ###################################################################### + + def before_create(self, context, resource): + pass + + def before_update(self, context, current, resource): + pass + + def before_delete(self, context, resource, resources): + pass + + def before_show(self, resource_dict): + + context = { + 'model': model, + 'session': model.Session, + 'user': tk.c.user, + 'user_obj': tk.c.userobj + } + + try: + tk.check_access('resource_show', context, resource_dict) + except tk.NotAuthorized: + resource_dict.clear() + return resource_dict + ###################################################################### ######################### ITEMPLATESHELPER ########################### ###################################################################### diff --git a/ckanext/privatedatasets/templates/snippets/package_item.html b/ckanext/privatedatasets/templates/snippets/package_item.html index 5db5776..b84b9ab 100755 --- a/ckanext/privatedatasets/templates/snippets/package_item.html +++ b/ckanext/privatedatasets/templates/snippets/package_item.html @@ -26,7 +26,7 @@ Example: {% block package_item_content %}

- {% if package.private and not h.can_read(package) %} + {% if package.private and not owner and not acquired %} {{ _('Private') }} @@ -46,8 +46,8 @@ Example: {% endif %} - {% if package.private and not h.can_read(package) %} - {{ _(h.truncate(title, truncate_title)) }} + {% if package.private and not owner and not acquired %} + {{ h.link_to(h.truncate(title, truncate_title), h.url_for(controller='package', action='read', id=package.name)) }}
{{ h.acquire_button(package) }} {% else %} diff --git a/ckanext/privatedatasets/templates_2.8/snippets/package_item.html b/ckanext/privatedatasets/templates_2.8/snippets/package_item.html index 5db5776..048006e 100755 --- a/ckanext/privatedatasets/templates_2.8/snippets/package_item.html +++ b/ckanext/privatedatasets/templates_2.8/snippets/package_item.html @@ -26,7 +26,7 @@ Example: {% block package_item_content %}

- {% if package.private and not h.can_read(package) %} + {% if package.private and not owner and not acquired%} {{ _('Private') }} @@ -46,8 +46,8 @@ Example: {% endif %} - {% if package.private and not h.can_read(package) %} - {{ _(h.truncate(title, truncate_title)) }} + {% if package.private and not owner and not acquired %} + {{ h.link_to(h.truncate(title, truncate_title), h.url_for(controller='package', action='read', id=package.name)) }}
{{ h.acquire_button(package) }} {% else %} diff --git a/ckanext/privatedatasets/tests/test_auth.py b/ckanext/privatedatasets/tests/test_auth.py index 9e873da..6436f87 100644 --- a/ckanext/privatedatasets/tests/test_auth.py +++ b/ckanext/privatedatasets/tests/test_auth.py @@ -53,7 +53,6 @@ class AuthTest(unittest.TestCase): auth.authz = self._authz auth.tk = self._tk auth.db = self._db - if hasattr(self, '_package_show'): auth.package_show = self._package_show @@ -66,12 +65,12 @@ class AuthTest(unittest.TestCase): # Anonymous user (public) (None, None, None, False, 'active', None, None, None, None, None, True), # Anonymous user (private) - (None, None, None, True, 'active', None, None, None, None, '/', False), - (None, None, '', True, 'active', None, None, '', None, '/', False), + (None, None, None, True, 'active', None, None, None, None, '/', True), + (None, None, '', True, 'active', None, None, '', None, '/', True), # Anonymous user (private). Buy URL not shown - (None, None, None, True, 'active', None, None, None, 'google.es', '/', False), + (None, None, None, True, 'active', None, None, None, 'google.es', '/', True), # Anonymous user (private). Buy URL show - (None, None, None, True, 'active', None, None, None, 'google.es', '/dataset/testds', False), + (None, None, None, True, 'active', None, None, None, 'google.es', '/dataset/testds', True), # The creator can always see the dataset (1, 1, None, False, 'active', None, None, None, None, None, True), (1, 1, None, True, 'active', 'conwet', None, None, None, None, True), @@ -79,25 +78,26 @@ class AuthTest(unittest.TestCase): (1, 1, None, False, 'draft', None, None, None, None, None, True), # Other user (no organizations) (1, 2, 'test', False, 'active', None, None, None, None, None, True), - (1, 2, 'test', True, 'active', None, None, None, 'google.es', '/', False), # Buy MSG not shown - (1, 2, 'test', True, 'active', None, None, None, None, '/dataset/testds', False), # Buy MSG not shown - (1, 2, 'test', True, 'active', None, None, None, 'google.es', '/dataset/testds', False), # Buy MSG shown - (1, 2, 'test', False, 'draft', None, None, None, None, None, False), + (1, 2, 'test', True, 'active', None, None, None, 'google.es', '/', True), # Buy MSG not shown + (1, 2, 'test', True, 'active', None, None, None, None, '/dataset/testds', True), # Buy MSG not shown + (1, 2, 'test', True, 'active', None, None, None, 'google.es', '/dataset/testds', True), # Buy MSG shown + (1, 2, 'test', False, 'draft', None, None, None, None, None, False), # Other user but authorized in the list of authorized users (1, 2, 'test', True, 'active', None, None, True, None, None, True), # Other user and not authorized in the list of authorized users - (1, 2, 'test', True, 'active', None, None, False, 'google.es', '/', False), - (1, 2, 'test', True, 'active', None, None, False, 'google.es', '/dataset/testds', False), + (1, 2, 'test', True, 'active', None, None, False, 'google.es', '/', True), + (1, 2, 'test', True, 'active', None, None, False, 'google.es', '/dataset/testds', True), # Other user with organizations (1, 2, 'test', False, 'active', 'conwet', False, None, None, None, True), - (1, 2, 'test', True, 'active', 'conwet', False, None, None, None, False), + (1, 2, 'test', True, 'active', 'conwet', False, None, None, None, True), (1, 2, 'test', True, 'active', 'conwet', True, None, None, None, True), (1, 2, 'test', True, 'draft', 'conwet', True, None, None, None, False), # Other user with organizations (user is not in the organization) (1, 2, 'test', True, 'active', 'conwet', False, True, None, None, True), - (1, 2, 'test', True, 'active', 'conwet', False, False, None, None, False), - (1, 2, 'test', True, 'active', 'conwet', False, False, 'google.es', '/dataset/testds', False), - (1, 2, 'test', True, 'active', 'conwet', False, False, 'google.es', '/', False) ]) + (1, 2, 'test', True, 'active', 'conwet', False, False, None, None, True), + (1, 2, 'test', True, 'active', 'conwet', False, False, 'google.es', '/dataset/testds', True), + (1, 2, 'test', True, 'active', 'conwet', False, False, 'google.es', '/', True) + ]) def test_auth_package_show(self, creator_user_id, user_obj_id, user, private, state, owner_org, owner_member, db_auth, acquire_url, request_path, authorized): @@ -140,14 +140,14 @@ class AuthTest(unittest.TestCase): # Check the result self.assertEquals(authorized, result['success']) - # Premissions for organization are checked when the dataset is private, it belongs to an organization + # Permissions for organization are checked when the dataset is private, it belongs to an organization # and when the dataset has not been created by the user who is asking for it if private and owner_org and state == 'active' and creator_user_id != user_obj_id: auth.authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'read') else: self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count) - # The databse is only initialized when: + # The database is only initialized when: # * the dataset is private AND # * the dataset is active AND # * the dataset has no organization OR the user does not belong to that organization AND @@ -159,7 +159,7 @@ class AuthTest(unittest.TestCase): self.assertEquals(0, auth.db.init_db.call_count) # Conditions to buy a dataset; It should be private, active and should not belong to any organization - if not authorized and state == 'active' and request_path and request_path.startswith('/dataset/') and acquire_url: + if authorized and state == 'active' and request_path and request_path.startswith('/dataset/') and acquire_url: auth.helpers.flash_notice.assert_called_once() else: self.assertEquals(0, auth.helpers.flash_notice.call_count) @@ -203,56 +203,101 @@ class AuthTest(unittest.TestCase): self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count) @parameterized.expand([ - (True, True), - (True, False), - (False, False), - (False, False) + # if package dont exist + (False, None, None, None, False, 'active', None, None, None, False), + # Anonymous user only can view resources of a public and active package + (True, None, None, None, False, 'active', None, None, None, True), + (True, None, None, None, True, 'active', None, None, None, False), + (True, None, None, '', True, 'active', None, None, None, False), + # The creator can always see the resource + (True, 1, 1, None, False, 'active', None, None, None, True), + (True, 1, 1, None, True, 'active', None, None, None, True), + (True, 1, 1, None, True, 'draft', None, None, None, True), + (True, 1, 1, None, False, 'draft', None, None, None, True), + # Other user (no organizations) + (True, 1, 2, 'test', False, 'active', None, None, None, True), + (True, 1, 2, 'test', True, 'active', None, None, None, False), + (True, 1, 2, 'test', True, 'draft', None, None, None, False), + # Other user but authorized in the list of authorized users + (True, 1, 2, 'test', True, 'active', None, None, True, True), + # Other user and not authorized in the list of authorized users + (True, 1, 2, 'test', True, 'active', None, None, False, False), + # Other user with organizations + (True, 1, 2, 'test', True, 'active', 'conwet', False, None, False), + (True, 1, 2, 'test', True, 'active', 'conwet', True, None, True), + ]) - def test_auth_resource_show(self, exist_pkg=True, authorized_pkg=True): + def test_auth_resource_show(self, exist_pkg, creator_user_id, user_obj_id, user, private, state, owner_org, + owner_member, db_auth, authorized): #Recover the exception auth.tk.ObjectNotFound = self._tk.ObjectNotFound - # Mock the calls - package = MagicMock() - package.id = '1' + # Configure the mocks + if exist_pkg: + returned_package = MagicMock() + returned_package.creator_user_id = creator_user_id + returned_package.private = private + returned_package.state = state + returned_package.owner_org = owner_org + returned_package.extras = {} + else: + returned_package = None - final_query = MagicMock() - final_query.first = MagicMock(return_value=package if exist_pkg else None) + returned_resource = MagicMock() + returned_resource.package_id = 1 - second_join = MagicMock() - second_join.filter = MagicMock(return_value=final_query) + # Configure the database + db_response = [] + if db_auth is True: + out = auth.db.AllowedUser() + out.package_id = 'package_id' + out.user_name = user + db_response.append(out) - first_join = MagicMock() - first_join.join = MagicMock(return_value=second_join) + # Prepare the context + context = {'model': MagicMock()} + if user is not None: + context['user'] = user + if user_obj_id is not None: + context['auth_user_obj'] = MagicMock() + context['auth_user_obj'].id = user_obj_id - query = MagicMock() - query.join = MagicMock(return_value=first_join) + auth.db.AllowedUser.get = MagicMock(return_value=db_response) + auth.logic_auth.get_resource_object = MagicMock(return_value=returned_resource) + auth.logic_auth.get_package_object = MagicMock(return_value=returned_package) + auth.authz.has_user_permission_for_group_or_org = MagicMock(return_value=owner_member) - model = MagicMock() - session = MagicMock() - session.query = MagicMock(return_value=query) - model.Session = session - - # Create the context - context = {} - context['model'] = model - - # Mock the package_show function - self._package_show = auth.package_show - success = True if authorized_pkg else False - auth.package_show = MagicMock(return_value={'success': success}) + # Prepare the context + context = {'model': MagicMock()} + if user is not None: + context['user'] = user + if user_obj_id is not None: + context['auth_user_obj'] = MagicMock() + context['auth_user_obj'].id = user_obj_id if not exist_pkg: self.assertRaises(self._tk.ObjectNotFound, auth.resource_show, context, {}) else: result = auth.resource_show(context, {}) - self.assertEquals(authorized_pkg, result['success']) + self.assertEquals(authorized, result['success']) + + if private and owner_org and state == 'active' and creator_user_id != user_obj_id: + auth.authz.has_user_permission_for_group_or_org.assert_called_once_with(owner_org, user, 'read') + else: + self.assertEquals(0, auth.authz.has_user_permission_for_group_or_org.call_count) + + if private and state == 'active' and (not owner_org or not owner_member) and (creator_user_id != user_obj_id or user_obj_id is None): + # Check that the database has been initialized properly + auth.db.init_db.assert_called_once_with(context['model']) + else: + self.assertEquals(0, auth.db.init_db.call_count) + def test_package_acquired(self): self.assertTrue(auth.package_acquired({}, {})['success']) def test_package_deleted(self): - self.assertTrue(auth.revoke_access({},{})['success']) + self.assertTrue(auth.revoke_access({}, {})['success']) @parameterized.expand([ ({'user': 'user_1'}, {'user': 'user_1'}, True), diff --git a/ckanext/privatedatasets/tests/test_helpers.py b/ckanext/privatedatasets/tests/test_helpers.py index d9f6bb0..f45db8e 100644 --- a/ckanext/privatedatasets/tests/test_helpers.py +++ b/ckanext/privatedatasets/tests/test_helpers.py @@ -39,7 +39,6 @@ class HelpersTest(unittest.TestCase): self._db = helpers.db helpers.db = MagicMock() - self._request = helpers.request helpers.request = MagicMock() diff --git a/ckanext/privatedatasets/tests/test_plugin.py b/ckanext/privatedatasets/tests/test_plugin.py index 49502c1..99afd64 100644 --- a/ckanext/privatedatasets/tests/test_plugin.py +++ b/ckanext/privatedatasets/tests/test_plugin.py @@ -65,19 +65,13 @@ class PluginTest(unittest.TestCase): ('package_show', plugin.auth.package_show), ('package_update', plugin.auth.package_update), ('resource_show', plugin.auth.resource_show), - ('resource_show', plugin.auth.resource_show, True, False), ('package_acquired', plugin.auth.package_acquired), ('acquisitions_list', plugin.auth.acquisitions_list), ('revoke_access', plugin.auth.revoke_access) ]) - def test_auth_function(self, function_name, expected_function, is_ckan_23=False, expected=True): - plugin.tk.check_ckan_version = MagicMock(return_value=is_ckan_23) + def test_auth_function(self, function_name, expected_function): auth_functions = self.privateDatasets.get_auth_functions() - - if expected: - self.assertEquals(auth_functions[function_name], expected_function) - else: - self.assertNotIn(function_name, auth_functions) + self.assertEquals(auth_functions[function_name], expected_function) def test_update_config(self): # Call the method @@ -215,41 +209,73 @@ class PluginTest(unittest.TestCase): self.assertTrue(found) @parameterized.expand([ - (True, 1, 1, False, True, True), - (True, 1, 2, False, True, True), - (True, 1, 1, True, True, True), - (True, 1, 2, True, True, True), - (True, 1, None, None, True, True), - (True, 1, 1, None, True, True), - (True, 1, None, True, True, True), - (True, 1, None, False, True, True), - (False, 1, 1, False, True, True), - (False, 1, 2, False, True, False), - (False, 1, 1, True, True, True), - (False, 1, 2, True, True, True), - (False, 1, None, None, True, False), - (False, 1, 1, None, True, True), - (False, 1, None, True, True, True), - (False, 1, None, False, True, False), - (True, 1, 1, False, False, False), - (True, 1, 2, False, False, False), - (True, 1, 1, True, False, False), - (True, 1, 2, True, False, False), - (True, 1, None, None, False, False), - (True, 1, 1, None, False, False), - (True, 1, None, True, False, False), - (True, 1, None, False, False, False), - (False, 1, 1, False, False, False), - (False, 1, 2, False, False, False), - (False, 1, 1, True, False, False), - (False, 1, 2, True, False, False), - (False, 1, None, None, False, False), - (False, 1, 1, None, False, False), - (False, 1, None, True, False, False), - (False, 1, None, False, False, False), + (True, 1, 1, False, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, 2, False, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, True, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, 2, True, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, None, None, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, None, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, None, True, True, True, [{'id': 1}, {'id': 2}], True), + (True, 1, None, False, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, False, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, 2, False, True, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, True, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, 2, True, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, None, None, True, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, None, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, None, True, True, True, [{'id': 1}, {'id': 2}], True), + (False, 1, None, False, True, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, False, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 2, False, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, True, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 2, True, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, None, None, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, None, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, None, True, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, None, False, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, False, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 2, False, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, True, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 2, True, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, None, None, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, 1, None, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, None, True, False, False, [{'id': 1}, {'id': 2}], True), + (False, 1, None, False, False, False, [{'id': 1}, {'id': 2}], True), + (True, 1, 1, False, True, True, [{}, {}], False), + (True, 1, 2, False, True, True, [{}, {}], False), + (True, 1, 1, True, True, True, [{}, {}], False), + (True, 1, 2, True, True, True, [{}, {}], False), + (True, 1, None, None, True, True, [{}, {}], False), + (True, 1, 1, None, True, True, [{}, {}], False), + (True, 1, None, True, True, True, [{}, {}], False), + (True, 1, None, False, True, True, [{}, {}], False), + (False, 1, 1, False, True, True, [{}, {}], False), + (False, 1, 2, False, True, False, [{}, {}], False), + (False, 1, 1, True, True, True, [{}, {}], False), + (False, 1, 2, True, True, True, [{}, {}], False), + (False, 1, None, None, True, False, [{}, {}], False), + (False, 1, 1, None, True, True, [{}, {}], False), + (False, 1, None, True, True, True, [{}, {}], False), + (False, 1, None, False, True, False, [{}, {}], False), + (True, 1, 1, False, False, False, [{}, {}], False), + (True, 1, 2, False, False, False, [{}, {}], False), + (True, 1, 1, True, False, False, [{}, {}], False), + (True, 1, 2, True, False, False, [{}, {}], False), + (True, 1, None, None, False, False, [{}, {}], False), + (True, 1, 1, None, False, False, [{}, {}], False), + (True, 1, None, True, False, False, [{}, {}], False), + (True, 1, None, False, False, False, [{}, {}], False), + (False, 1, 1, False, False, False, [{}, {}], False), + (False, 1, 2, False, False, False, [{}, {}], False), + (False, 1, 1, True, False, False, [{}, {}], False), + (False, 1, 2, True, False, False, [{}, {}], False), + (False, 1, None, None, False, False, [{}, {}], False), + (False, 1, 1, None, False, False, [{}, {}], False), + (False, 1, None, True, False, False, [{}, {}], False), + (False, 1, None, False, False, False, [{}, {}], False), ]) - def test_packagecontroller_after_show(self, update_via_api, creator_id, user_id, sysadmin, private, fields_expected): - + def test_packagecontroller_after_show(self, update_via_api, creator_id, user_id, sysadmin, private, fields_expected, resources, resources_fields): + context = {'updating_via_cb': update_via_api} if creator_id is not None or sysadmin is not None: @@ -258,7 +284,7 @@ class PluginTest(unittest.TestCase): user.sysadmin = sysadmin context['auth_user_obj'] = user - pkg_dict = {'creator_user_id': creator_id, 'allowed_users': ['a', 'b', 'c'], 'searchable': True, 'acquire_url': 'http://google.es', 'private': private} + pkg_dict = {'creator_user_id': creator_id, 'allowed_users': ['a', 'b', 'c'], 'searchable': True, 'acquire_url': 'http://google.es', 'private': private, 'resources': resources, 'num_resources': 2} # Call the function result = self.privateDatasets.after_show(context, pkg_dict) # Call the function @@ -271,6 +297,14 @@ class PluginTest(unittest.TestCase): else: self.assertFalse(field in result) + fields = ['resources', 'num_resources'] + for field in fields: + if resources_fields: + self.assertTrue(field in result) + else: + self.assertFalse(field in result) + + @parameterized.expand([ ('public', None, 'public'), ('public', 'False', 'private'), @@ -449,3 +483,40 @@ class PluginTest(unittest.TestCase): self.assertEquals(final_search_results['facets'], search_results['facets']) self.assertEquals(final_search_results['elements'], search_results['elements']) + + @parameterized.expand([ + (True,), + (False,) + ]) + def test_package_controller_before_view(self, user_allowed): + + pkg_dict = {'resources': [{'id': 1}, {'id': 2}, {'id': 3}]} + pkg_dict_not_allowed = {'resources': []} + + plugin.tk.check_access.side_effect = None if user_allowed else plugin.tk.NotAuthorized + + result = self.privateDatasets.before_view(pkg_dict) + + if user_allowed: + self.assertEquals(result['resources'], pkg_dict['resources']) + else: + self.assertEquals(result['resources'], pkg_dict_not_allowed['resources']) + + @parameterized.expand([ + (True,), + (False,) + ]) + def test_resource_controller_before_show(self, user_allowed): + + resource_dict = {'id': 1, 'resource_name': 'resource_test'} + + plugin.tk.check_access.side_effect = None if user_allowed else plugin.tk.NotAuthorized + + result = self.privateDatasets.before_show(resource_dict) + + if user_allowed: + self.assertEquals(result['id'], resource_dict['id']) + self.assertEquals(result['resource_name'], resource_dict['resource_name']) + else: + self.assertNotIn('id', result) + self.assertNotIn('resource_name', result) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 44ae530..251d61f 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -270,13 +270,13 @@ class TestSelenium(unittest.TestCase): driver.get(self.base_url + 'dataset/' + dataset_url) if not acquired and private and not in_org: - # If the user has not access to the dataset the 404 error page is displayed - xpath = '//*[@id="content"]/div[2]/article/div/h1' - msg = '404 Not Found' + # If the dataset is private and the user hasnt access to the resources, the field resources dont appear - self.assertEqual(driver.find_element_by_xpath(xpath).text, msg) + self.assertEquals('empty', driver.find_element_by_class_name('empty').get_attribute('class')) + self.assertEqual(self.base_url + 'dataset/%s' % dataset_url, driver.current_url) else: + self.assertEquals('resource-list', driver.find_element_by_class_name('resource-list').get_attribute('class')) self.assertEqual(self.base_url + 'dataset/%s' % dataset_url, driver.current_url) def check_acquired(self, dataset, dataset_url, acquired, private): diff --git a/test.ini b/test.ini index 2fc3608..cfe18ac 100644 --- a/test.ini +++ b/test.ini @@ -4,6 +4,7 @@ host = 0.0.0.0 port = 5000 [app:main] +# use = config:/usr/lib/ckan/default/src/ckan/test-core.ini use = config:./ckan/test-core.ini ckan.site_id = ckanext.privatedatasets.test