From 0eca66d0154daf33dcc7f709ec81fe2812646b3c Mon Sep 17 00:00:00 2001 From: Cazaril Date: Thu, 23 Jun 2016 12:28:09 +0200 Subject: [PATCH 01/22] Added remove users from private dataset API endpoint --- ckanext/privatedatasets/actions.py | 107 +++++++++++++++++++++++++++ ckanext/privatedatasets/auth.py | 4 + ckanext/privatedatasets/constants.py | 1 + ckanext/privatedatasets/plugin.py | 6 +- 4 files changed, 116 insertions(+), 2 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index b26c911..a3706a0 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -194,3 +194,110 @@ def acquisitions_list(context, data_dict): pass return result +def package_deleted(context, request_data): + ''' + API action to be called every time a user deletes a dataset in an external service. + + This API should be called to delete the user from the list of allowed users. + + Since each service can provide a different way of pushing the data, the received + data will be forwarded to the parser set in the preferences. This parser should + return a dict similar to the following one: + {'errors': ["...", "...", ...] + 'users_datasets': [{'user': 'user_name', 'datasets': ['ds1', 'ds2', ...]}, ...]} + 1) 'errors' contains the list of errors. It should be empty if no errors arised + while the notification is parsed + 2) 'users_datasets' is the lists of datasets available for each user (each element + of this list is a dictionary with two fields: user and datasets). + + :parameter request_data: Depends on the parser + :type request_data: dict + + :return: A list of warnings or None if the list of warnings is empty + :rtype: dict + + ''' + log.info('Notification received: %s' % request_data) + + # Check access + plugins.toolkit.check_access(constants.PACKAGE_DELETED, context, request_data) + + # Get the parser from the configuration + class_path = config.get(PARSER_CONFIG_PROP, '') + + if class_path != '': + try: + cls = class_path.split(':') + class_package = cls[0] + class_name = cls[1] + parser_cls = getattr(importlib.import_module(class_package), class_name) + parser = parser_cls() + except Exception as e: + raise plugins.toolkit.ValidationError({'message': '%s: %s' % (type(e).__name__, str(e))}) + else: + raise plugins.toolkit.ValidationError({'message': '%s not configured' % PARSER_CONFIG_PROP}) + + # Parse the result using the parser set in the configuration + # Expected result: {'errors': ["...", "...", ...] + # 'users_datasets': [{'user': 'user_name', 'datasets': ['ds1', 'ds2', ...]}, ...]} + result = parser.parse_notification(request_data) + + warns = [] + + for user_info in result['users_datasets']: + for dataset_id in user_info['datasets']: + + try: + context_pkg_show = context.copy() + context_pkg_show['ignore_auth'] = True + context_pkg_show[constants.CONTEXT_CALLBACK] = True + dataset = plugins.toolkit.get_action('package_show')(context_pkg_show, {'id': dataset_id}) + + # This operation can only be performed with private datasets + # This check is redundant since the package_update function will throw an exception + # if a list of allowed users is included in a public dataset. However, this check + # should be performed in order to avoid strange future exceptions + if dataset.get('private', None) is True: + + # Create the array if it does not exist + if constants.ALLOWED_USERS not in dataset or dataset[constants.ALLOWED_USERS] is None: + dataset[constants.ALLOWED_USERS] = [] + + # Deletes the user only if it is in the list + if user_info['user'] in dataset[constants.ALLOWED_USERS]: + dataset[constants.ALLOWED_USERS].remove(user_info['user']) + context_pkg_update = context.copy() + context_pkg_update['ignore_auth'] = True + + # Set creator as the user who is performing the changes + user_show = plugins.toolkit.get_action('user_show') + creator_user_id = dataset.get('creator_user_id', '') + user_show_context = {'ignore_auth': True} + user = user_show(user_show_context, {'id': creator_user_id}) + context_pkg_update['user'] = user.get('name', '') + + plugins.toolkit.get_action('package_update')(context_pkg_update, dataset) + log.info('Allowed Users deleted correctly') + else: + log.warn('The user %s is already deleted from accessing the %s dataset' % (user_info['user'], dataset_id)) + else: + log.warn('Dataset %s is public. Allowed Users cannot be deleted') + warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) + + except plugins.toolkit.ObjectNotFound: + # If a dataset does not exist in the instance, an error message will be returned to the user. + # However the process won't stop and the process will continue with the remaining datasets. + log.warn('Dataset %s was not found in this instance' % dataset_id) + warns.append('Dataset %s was not found in this instance' % dataset_id) + except plugins.toolkit.ValidationError as e: + # Some datasets does not allow to introduce the list of allowed users since this property is + # only valid for private datasets outside an organization. In this case, a wanr will return + # but the process will continue + # WARN: This exception should not be risen anymore since public datasets are not updated. + message = '%s(%s): %s' % (dataset_id, constants.ALLOWED_USERS, e.error_dict[constants.ALLOWED_USERS][0]) + log.warn(message) + warns.append(message) + + # Return warnings that inform about non-existing datasets + if len(warns) > 0: + return {'warns': warns} \ No newline at end of file diff --git a/ckanext/privatedatasets/auth.py b/ckanext/privatedatasets/auth.py index 186221e..5186531 100644 --- a/ckanext/privatedatasets/auth.py +++ b/ckanext/privatedatasets/auth.py @@ -137,3 +137,7 @@ def package_acquired(context, data_dict): def acquisitions_list(context, data_dict): # Users can get only their acquisitions list return {'success': context['user'] == data_dict['user']} + +def package_deleted(context, data_dict): + # TODO: Check functionality and improve security(if needed) + return {'success': True} \ No newline at end of file diff --git a/ckanext/privatedatasets/constants.py b/ckanext/privatedatasets/constants.py index 8f00449..0eed977 100644 --- a/ckanext/privatedatasets/constants.py +++ b/ckanext/privatedatasets/constants.py @@ -24,3 +24,4 @@ SEARCHABLE = 'searchable' ACQUIRE_URL = 'acquire_url' CONTEXT_CALLBACK = 'updating_via_cb' PACKAGE_ACQUIRED = 'package_acquired' +PACKAGE_DELETED = 'package_deleted' diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index a265010..38cab60 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -112,7 +112,8 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): 'package_update': auth.package_update, # 'resource_show': auth.resource_show, constants.PACKAGE_ACQUIRED: auth.package_acquired, - constants.ACQUISITIONS_LIST: auth.acquisitions_list} + constants.ACQUISITIONS_LIST: auth.acquisitions_list, + constants.PACKAGE_DELETED: auth.package_deleted} # resource_show is not required in CKAN 2.3 because it delegates to # package_show @@ -152,7 +153,8 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): def get_actions(self): return { constants.PACKAGE_ACQUIRED: actions.package_acquired, - constants.ACQUISITIONS_LIST: actions.acquisitions_list + constants.ACQUISITIONS_LIST: actions.acquisitions_list, + constants.PACKAGE_DELETED: actions.package_deleted } ###################################################################### From 5b6bd5971538415cffe5460f4084fefc4c757255 Mon Sep 17 00:00:00 2001 From: Eugenio Gonzalo Date: Fri, 1 Jul 2016 13:24:18 +0200 Subject: [PATCH 02/22] Added tests for the remove users API endpoint. --- ckanext/privatedatasets/actions.py | 2 +- ckanext/privatedatasets/parsers/fiware.py | 3 +- ckanext/privatedatasets/plugin.py | 2 + ckanext/privatedatasets/tests/test_actions.py | 95 ++++++++++++++++++- ckanext/privatedatasets/tests/test_auth.py | 3 + ckanext/privatedatasets/tests/test_plugin.py | 6 +- .../privatedatasets/tests/test_selenium.py | 3 + test.ini | 2 +- 8 files changed, 105 insertions(+), 11 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index a3706a0..af78101 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -300,4 +300,4 @@ def package_deleted(context, request_data): # Return warnings that inform about non-existing datasets if len(warns) > 0: - return {'warns': warns} \ No newline at end of file + return {'warns': warns} diff --git a/ckanext/privatedatasets/parsers/fiware.py b/ckanext/privatedatasets/parsers/fiware.py index 11ded25..eabb761 100644 --- a/ckanext/privatedatasets/parsers/fiware.py +++ b/ckanext/privatedatasets/parsers/fiware.py @@ -27,7 +27,6 @@ from ckan.common import request class FiWareNotificationParser(object): def parse_notification(self, request_data): - my_host = request.host fields = ['customer_name', 'resources'] @@ -61,4 +60,4 @@ class FiWareNotificationParser(object): else: raise tk.ValidationError({'message': 'Invalid resource format'}) - return {'users_datasets': [{'user': user_name, 'datasets': datasets}]} + return {'users_datasets': [{'user': user_name, 'datasets': datasets}]} \ No newline at end of file diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index 38cab60..dd5f231 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -146,6 +146,8 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): return m + + ###################################################################### ############################## IACTIONS ############################## ###################################################################### diff --git a/ckanext/privatedatasets/tests/test_actions.py b/ckanext/privatedatasets/tests/test_actions.py index 62cef0f..ce3f30b 100644 --- a/ckanext/privatedatasets/tests/test_actions.py +++ b/ckanext/privatedatasets/tests/test_actions.py @@ -84,6 +84,7 @@ class ActionsTest(unittest.TestCase): if expected_error: with self.assertRaises(actions.plugins.toolkit.ValidationError) as cm: actions.package_acquired({}, {}) + actions.package_deleted({},{}) self.assertEqual(cm.exception.error_dict['message'], expected_error) else: # Exception is not risen @@ -92,6 +93,17 @@ class ActionsTest(unittest.TestCase): # Checks self.assertEquals(0, actions.plugins.toolkit.get_action.call_count) + if expected_error: + with self.assertRaises(actions.plugins.toolkit.ValidationError) as cm: + actions.package_deleted({},{}) + self.assertEqual(cm.exception.error_dict['message'], expected_error) + else: + # Exception is not risen + self.assertEquals(None, actions.package_deleted({},{})) + + # Checks + self.assertEquals(0, actions.plugins.toolkit.get_action.call_count) + def configure_mocks(self, parse_result, datasets_not_found=[], not_updatable_datasets=[], allowed_users=None, creator_user={'id': '1234', 'name': 'ckan'}): @@ -163,13 +175,9 @@ class ActionsTest(unittest.TestCase): ]) def test_add_users(self, users_info, datasets_not_found, not_updatable_datasets, allowed_users=[]): - parse_result = {'users_datasets': []} + parse_result = {'users_datasets': [{'user': user, 'datasets': users_info[user]} for user in users_info]} creator_user = {'name': 'ckan', 'id': '1234'} - # Transform user_info - for user in users_info: - parse_result['users_datasets'].append({'user': user, 'datasets': users_info[user]}) - parse_notification, package_show, package_update, user_show = self.configure_mocks(parse_result, datasets_not_found, not_updatable_datasets, allowed_users, creator_user) @@ -295,3 +303,80 @@ class ActionsTest(unittest.TestCase): expected_acquired_datasets.append(pkg) self.assertEquals(expected_acquired_datasets, result) + + @parameterized.expand([ + # Simple Test: one user and one dataset + ({'user1': ['ds1']}, [], [], None), #Test with and non-existing list of allowed users + ({'user2': ['ds1']}, [], [], []), #Test remove a non-existing user + ({'user3': ['ds1']}, [], [], ['user3']), #Test remove an existing user + ({'user4': ['ds1']}, [], [], ['another_user']), #Test remove non-existing from an already populated list + ({'user5': ['ds1']}, [], [], ['another_user', 'user5']), + ({'user6': ['ds1']}, ['ds1'], [], None), #Test remove from an unknown place + ({'user61': ['ds1']}, ['ds1'], [], []), + ({'user62': ['ds1']}, ['ds1'], [], ['user6']), + ({'user7': ['ds1']}, [], ['ds1'], []), #Tests deleting from a public dataset + ({'user8': ['ds1']}, [], ['ds1'], ['another_user']), + ({'user9': ['ds1']}, [], ['ds1'], ['another_user', 'another_one']), + ({'user91': ['ds1']}, ['ds1'], ['ds1'], ['another_user', 'another_one']), # Checking the behaviour when the unknown dataset is public + ({'user92': ['ds1']}, ['ds1'], ['ds1'], ['another_user', 'another_one','user92']), + + # # Complex test: some users and some datasets + ({'user1': ['ds1', 'ds2', 'ds3', 'ds4'], 'user2': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], []), + ({'user3': ['ds1', 'ds2', 'ds3', 'ds4'], 'user4': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], ['another_user']), + ({'user5': ['ds1', 'ds2', 'ds3', 'ds4'], 'user6': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], ['another_user', 'another_one']), + ({'user7': ['ds1', 'ds2', 'ds3', 'ds4'], 'user8': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], ['another_user', 'another_one', 'user7']) + ]) + def test_delete_users(self, users_info, datasets_not_found, not_updatable_datasets, allowed_users=[]): + parse_result = {'users_datasets': []} + creator_user = {'name': 'ckan', 'id': '1234'} + + # Transform user_info + for user in users_info: + parse_result['users_datasets'].append({'user': user, 'datasets': users_info[user]}) + + parse_delete, package_show, package_update, user_show = self.configure_mocks(parse_result, + datasets_not_found, not_updatable_datasets, allowed_users, creator_user) + + # Call the function + context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}} + result = actions.package_deleted(context, users_info) + + # Calculate the list of warns + warns = [] + for user_datasets in parse_result['users_datasets']: + for dataset_id in user_datasets['datasets']: + if dataset_id in datasets_not_found: + warns.append('Dataset %s was not found in this instance' % dataset_id) + elif dataset_id in not_updatable_datasets: + # warns.append('%s(%s): %s' % (dataset_id, 'allowed_users', ADD_USERS_ERROR)) + warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) + + expected_result = {'warns': warns} if len(warns) > 0 else None + + # Check that the returned result is as expected + self.assertEquals(expected_result, result) + + # Check that the initial functions (check_access and parse_notification) has been called properly + parse_delete.assert_called_once_with(users_info) + actions.plugins.toolkit.check_access.assert_called_once_with('package_deleted', context, users_info) + + for user_datasets in parse_result['users_datasets']: + for dataset_id in user_datasets['datasets']: + # The show function is always called + context_show = context.copy() + context_show['ignore_auth'] = True + context_show['updating_via_cb'] = True + package_show.assert_any_call(context_show, {'id': dataset_id}) + + # The update function is called only when the show function does not throw an exception and + # when the user is not in the list of allowed users. + if dataset_id not in datasets_not_found and allowed_users is not None and user_datasets['user'] in allowed_users and dataset_id not in not_updatable_datasets: + # Calculate the list of allowed_users + expected_allowed_users = list(allowed_users) + expected_allowed_users.remove(user_datasets['user']) + + context_update = context.copy() + context_update['ignore_auth'] = True + context_update['user'] = creator_user['name'] + + package_update.assert_any_call(context_update, {'id': dataset_id, 'allowed_users': expected_allowed_users, 'private': True, 'creator_user_id': creator_user['id']}) diff --git a/ckanext/privatedatasets/tests/test_auth.py b/ckanext/privatedatasets/tests/test_auth.py index d7c1978..3c26395 100644 --- a/ckanext/privatedatasets/tests/test_auth.py +++ b/ckanext/privatedatasets/tests/test_auth.py @@ -251,6 +251,9 @@ class AuthTest(unittest.TestCase): def test_package_acquired(self): self.assertTrue(auth.package_acquired({}, {})['success']) + def test_package_deleted(self): + self.assertTrue(auth.package_deleted({},{})['success']) + @parameterized.expand([ ({'user': 'user_1'}, {'user': 'user_1'}, True), ({'user': 'user_2'}, {'user': 'user_1'}, False), diff --git a/ckanext/privatedatasets/tests/test_plugin.py b/ckanext/privatedatasets/tests/test_plugin.py index 8b7f78e..1c4d50b 100644 --- a/ckanext/privatedatasets/tests/test_plugin.py +++ b/ckanext/privatedatasets/tests/test_plugin.py @@ -65,7 +65,8 @@ class PluginTest(unittest.TestCase): ('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) + ('acquisitions_list', plugin.auth.acquisitions_list), + ('package_deleted', plugin.auth.package_deleted) ]) 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) @@ -97,7 +98,8 @@ class PluginTest(unittest.TestCase): @parameterized.expand([ ('package_acquired', plugin.actions.package_acquired), - ('acquisitions_list', plugin.actions.acquisitions_list) + ('acquisitions_list', plugin.actions.acquisitions_list), + ('package_deleted', plugin.actions.package_deleted) ]) def test_actions_function(self, function_name, expected_function): actions = self.privateDatasets.get_actions() diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 7346cf1..3339199 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -22,6 +22,7 @@ from selenium import webdriver from selenium.webdriver.support.ui import Select from subprocess import Popen + import ckan.lib.search.index as search_index import ckan.model as model import ckanext.privatedatasets.db as db @@ -70,6 +71,7 @@ class TestSelenium(unittest.TestCase): self.driver = webdriver.Remote(os.environ['WEB_DRIVER_URL'], webdriver.DesiredCapabilities.FIREFOX.copy()) self.base_url = os.environ['CKAN_SERVER_URL'] else: + self.driver = webdriver.Firefox() self.base_url = 'http://127.0.0.1:5000/' @@ -80,6 +82,7 @@ class TestSelenium(unittest.TestCase): self.clearBBDD() self.driver.quit() + def assert_fields_disabled(self, fields): for field in fields: self.assertFalse(self.driver.find_element_by_id(field).is_enabled()) diff --git a/test.ini b/test.ini index 28e2e4f..7a18138 100644 --- a/test.ini +++ b/test.ini @@ -4,7 +4,7 @@ host = 0.0.0.0 port = 5000 [app:main] -use = config:./ckan/test-core.ini +use = config:/usr/lib/ckan/default/src/ckan/test-core.ini ckan.legacy_templates = no ckan.plugins = privatedatasets From 014779a096d1f4890605fa21faa39fc172065e3e Mon Sep 17 00:00:00 2001 From: Cazaril Date: Thu, 7 Jul 2016 14:36:12 +0200 Subject: [PATCH 03/22] Refactor and test in order to comply to PR requirements --- ckanext/privatedatasets/actions.py | 114 ++++-------------- ckanext/privatedatasets/parsers/fiware.py | 3 +- ckanext/privatedatasets/tests/test_actions.py | 20 +-- test.ini | 39 +++++- 4 files changed, 63 insertions(+), 113 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index af78101..dad0b85 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -52,92 +52,8 @@ def package_acquired(context, request_data): :rtype: dict ''' - - log.info('Notification received: %s' % request_data) - - # Check access - plugins.toolkit.check_access(constants.PACKAGE_ACQUIRED, context, request_data) - - # Get the parser from the configuration - class_path = config.get(PARSER_CONFIG_PROP, '') - - if class_path != '': - try: - cls = class_path.split(':') - class_package = cls[0] - class_name = cls[1] - parser_cls = getattr(importlib.import_module(class_package), class_name) - parser = parser_cls() - except Exception as e: - raise plugins.toolkit.ValidationError({'message': '%s: %s' % (type(e).__name__, str(e))}) - else: - raise plugins.toolkit.ValidationError({'message': '%s not configured' % PARSER_CONFIG_PROP}) - - # Parse the result using the parser set in the configuration - # Expected result: {'errors': ["...", "...", ...] - # 'users_datasets': [{'user': 'user_name', 'datasets': ['ds1', 'ds2', ...]}, ...]} - result = parser.parse_notification(request_data) - - warns = [] - - # Introduce the users into the datasets - for user_info in result['users_datasets']: - for dataset_id in user_info['datasets']: - try: - - context_pkg_show = context.copy() - context_pkg_show['ignore_auth'] = True - context_pkg_show[constants.CONTEXT_CALLBACK] = True - dataset = plugins.toolkit.get_action('package_show')(context_pkg_show, {'id': dataset_id}) - - # This operation can only be performed with private datasets - # This check is redundant since the package_update function will throw an exception - # if a list of allowed users is included in a public dataset. However, this check - # should be performed in order to avoid strange future exceptions - if dataset.get('private', None) is True: - - # Create the array if it does not exist - if constants.ALLOWED_USERS not in dataset or dataset[constants.ALLOWED_USERS] is None: - dataset[constants.ALLOWED_USERS] = [] - - # Add the user only if it is not in the list - if user_info['user'] not in dataset[constants.ALLOWED_USERS]: - dataset[constants.ALLOWED_USERS].append(user_info['user']) - context_pkg_update = context.copy() - context_pkg_update['ignore_auth'] = True - - # Set creator as the user who is performing the changes - user_show = plugins.toolkit.get_action('user_show') - creator_user_id = dataset.get('creator_user_id', '') - user_show_context = {'ignore_auth': True} - user = user_show(user_show_context, {'id': creator_user_id}) - context_pkg_update['user'] = user.get('name', '') - - plugins.toolkit.get_action('package_update')(context_pkg_update, dataset) - log.info('Allowed Users added correctly') - else: - log.warn('The user %s is already allowed to access the %s dataset' % (user_info['user'], dataset_id)) - else: - log.warn('Dataset %s is public. Allowed Users cannot be added') - warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) - - except plugins.toolkit.ObjectNotFound: - # If a dataset does not exist in the instance, an error message will be returned to the user. - # However the process won't stop and the process will continue with the remaining datasets. - log.warn('Dataset %s was not found in this instance' % dataset_id) - warns.append('Dataset %s was not found in this instance' % dataset_id) - except plugins.toolkit.ValidationError as e: - # Some datasets does not allow to introduce the list of allowed users since this property is - # only valid for private datasets outside an organization. In this case, a wanr will return - # but the process will continue - # WARN: This exception should not be risen anymore since public datasets are not updated. - message = '%s(%s): %s' % (dataset_id, constants.ALLOWED_USERS, e.error_dict[constants.ALLOWED_USERS][0]) - log.warn(message) - warns.append(message) - - # Return warnings that inform about non-existing datasets - if len(warns) > 0: - return {'warns': warns} + context['method'] = 'grant' + return _process_package(context, request_data) def acquisitions_list(context, data_dict): ''' @@ -194,9 +110,10 @@ def acquisitions_list(context, data_dict): pass return result + def package_deleted(context, request_data): ''' - API action to be called every time a user deletes a dataset in an external service. + API action to be called in order to revoke access grants of an user. This API should be called to delete the user from the list of allowed users. @@ -217,10 +134,15 @@ def package_deleted(context, request_data): :rtype: dict ''' + context['method'] = 'revoke' + return _process_package(context, request_data) + +def _process_package(context, request_data): log.info('Notification received: %s' % request_data) # Check access - plugins.toolkit.check_access(constants.PACKAGE_DELETED, context, request_data) + method = constants.PACKAGE_ACQUIRED if context.get('method') == 'grant' else constants.PACKAGE_DELETED + plugins.toolkit.check_access(method, context, request_data) # Get the parser from the configuration class_path = config.get(PARSER_CONFIG_PROP, '') @@ -263,9 +185,15 @@ def package_deleted(context, request_data): if constants.ALLOWED_USERS not in dataset or dataset[constants.ALLOWED_USERS] is None: dataset[constants.ALLOWED_USERS] = [] + method = context['method'] == 'grant' + present = user_info['user'] in dataset[constants.ALLOWED_USERS] # Deletes the user only if it is in the list - if user_info['user'] in dataset[constants.ALLOWED_USERS]: - dataset[constants.ALLOWED_USERS].remove(user_info['user']) + if (not method and present) or (method and not present): + if method: + dataset[constants.ALLOWED_USERS].append(user_info['user']) + else: + dataset[constants.ALLOWED_USERS].remove(user_info['user']) + context_pkg_update = context.copy() context_pkg_update['ignore_auth'] = True @@ -277,11 +205,11 @@ def package_deleted(context, request_data): context_pkg_update['user'] = user.get('name', '') plugins.toolkit.get_action('package_update')(context_pkg_update, dataset) - log.info('Allowed Users deleted correctly') + log.info('Action -%s access to user- ended successfully' % context['method']) else: - log.warn('The user %s is already deleted from accessing the %s dataset' % (user_info['user'], dataset_id)) + log.warn('Action -%s access to user- not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) else: - log.warn('Dataset %s is public. Allowed Users cannot be deleted') + log.warn('Dataset %s is public. Cannot %s access to users' % (dataset_id, context['method'])) warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) except plugins.toolkit.ObjectNotFound: diff --git a/ckanext/privatedatasets/parsers/fiware.py b/ckanext/privatedatasets/parsers/fiware.py index eabb761..f5cec78 100644 --- a/ckanext/privatedatasets/parsers/fiware.py +++ b/ckanext/privatedatasets/parsers/fiware.py @@ -60,4 +60,5 @@ class FiWareNotificationParser(object): else: raise tk.ValidationError({'message': 'Invalid resource format'}) - return {'users_datasets': [{'user': user_name, 'datasets': datasets}]} \ No newline at end of file + return {'users_datasets': [{'user': user_name, 'datasets': datasets}]} + \ No newline at end of file diff --git a/ckanext/privatedatasets/tests/test_actions.py b/ckanext/privatedatasets/tests/test_actions.py index ce3f30b..50a040e 100644 --- a/ckanext/privatedatasets/tests/test_actions.py +++ b/ckanext/privatedatasets/tests/test_actions.py @@ -64,7 +64,6 @@ class ActionsTest(unittest.TestCase): ]) def test_class_cannot_be_loaded(self, class_path, class_name, path_exist, class_exist, expected_error): - class_package = class_path class_package += ':' + class_name if class_name else '' actions.config = {PARSER_CONFIG_PROP: class_package} @@ -79,12 +78,10 @@ class ActionsTest(unittest.TestCase): actions.importlib.import_module = MagicMock(side_effect=ImportError(IMPORT_ERROR_MSG) if not path_exist else None, return_value=package if path_exist else None) - - # Call the function + if expected_error: with self.assertRaises(actions.plugins.toolkit.ValidationError) as cm: actions.package_acquired({}, {}) - actions.package_deleted({},{}) self.assertEqual(cm.exception.error_dict['message'], expected_error) else: # Exception is not risen @@ -93,17 +90,6 @@ class ActionsTest(unittest.TestCase): # Checks self.assertEquals(0, actions.plugins.toolkit.get_action.call_count) - if expected_error: - with self.assertRaises(actions.plugins.toolkit.ValidationError) as cm: - actions.package_deleted({},{}) - self.assertEqual(cm.exception.error_dict['message'], expected_error) - else: - # Exception is not risen - self.assertEquals(None, actions.package_deleted({},{})) - - # Checks - self.assertEquals(0, actions.plugins.toolkit.get_action.call_count) - def configure_mocks(self, parse_result, datasets_not_found=[], not_updatable_datasets=[], allowed_users=None, creator_user={'id': '1234', 'name': 'ckan'}): @@ -182,7 +168,7 @@ class ActionsTest(unittest.TestCase): datasets_not_found, not_updatable_datasets, allowed_users, creator_user) # Call the function - context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}} + context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}, 'method': 'grant'} result = actions.package_acquired(context, users_info) # Calculate the list of warns @@ -338,7 +324,7 @@ class ActionsTest(unittest.TestCase): datasets_not_found, not_updatable_datasets, allowed_users, creator_user) # Call the function - context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}} + context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}, 'method': 'revoke'} result = actions.package_deleted(context, users_info) # Calculate the list of warns diff --git a/test.ini b/test.ini index 7a18138..71897e6 100644 --- a/test.ini +++ b/test.ini @@ -4,10 +4,45 @@ 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.legacy_templates = no ckan.plugins = privatedatasets ckan.privatedatasets.parser = ckanext.privatedatasets.parsers.fiware:FiWareNotificationParser ckan.privatedatasets.show_acquire_url_on_create = True -ckan.privatedatasets.show_acquire_url_on_edit = True \ No newline at end of file +ckan.privatedatasets.show_acquire_url_on_edit = True +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage + +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage + +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage + +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage + +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage + +sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test +ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test +ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test + +ckan.storage_path=data/storage From 1bb294832e8bbdd9f4f08517846aba8ed6dbc529 Mon Sep 17 00:00:00 2001 From: egonzalo Date: Wed, 2 Nov 2016 11:37:37 +0100 Subject: [PATCH 04/22] Replace package_deleted function name for revoke_access --- ckanext/privatedatasets/actions.py | 7 ++++--- ckanext/privatedatasets/constants.py | 2 +- ckanext/privatedatasets/tests/test_actions.py | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index dad0b85..04818d4 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -111,7 +111,8 @@ def acquisitions_list(context, data_dict): return result -def package_deleted(context, request_data): + +def revoke_access(context, request_data): ''' API action to be called in order to revoke access grants of an user. @@ -189,7 +190,7 @@ def _process_package(context, request_data): present = user_info['user'] in dataset[constants.ALLOWED_USERS] # Deletes the user only if it is in the list if (not method and present) or (method and not present): - if method: + if method: dataset[constants.ALLOWED_USERS].append(user_info['user']) else: dataset[constants.ALLOWED_USERS].remove(user_info['user']) @@ -209,7 +210,7 @@ def _process_package(context, request_data): else: log.warn('Action -%s access to user- not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) else: - log.warn('Dataset %s is public. Cannot %s access to users' % (dataset_id, context['method'])) + log.warn('Dataset %s is public. Cannot %s access to users' % (dataset_id, context['method'])) warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) except plugins.toolkit.ObjectNotFound: diff --git a/ckanext/privatedatasets/constants.py b/ckanext/privatedatasets/constants.py index 0eed977..b53c127 100644 --- a/ckanext/privatedatasets/constants.py +++ b/ckanext/privatedatasets/constants.py @@ -24,4 +24,4 @@ SEARCHABLE = 'searchable' ACQUIRE_URL = 'acquire_url' CONTEXT_CALLBACK = 'updating_via_cb' PACKAGE_ACQUIRED = 'package_acquired' -PACKAGE_DELETED = 'package_deleted' +PACKAGE_DELETED = 'revoke_access' diff --git a/ckanext/privatedatasets/tests/test_actions.py b/ckanext/privatedatasets/tests/test_actions.py index 50a040e..fe0bb76 100644 --- a/ckanext/privatedatasets/tests/test_actions.py +++ b/ckanext/privatedatasets/tests/test_actions.py @@ -306,7 +306,7 @@ class ActionsTest(unittest.TestCase): ({'user91': ['ds1']}, ['ds1'], ['ds1'], ['another_user', 'another_one']), # Checking the behaviour when the unknown dataset is public ({'user92': ['ds1']}, ['ds1'], ['ds1'], ['another_user', 'another_one','user92']), - # # Complex test: some users and some datasets + # Complex test: some users and some datasets ({'user1': ['ds1', 'ds2', 'ds3', 'ds4'], 'user2': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], []), ({'user3': ['ds1', 'ds2', 'ds3', 'ds4'], 'user4': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], ['another_user']), ({'user5': ['ds1', 'ds2', 'ds3', 'ds4'], 'user6': ['ds5', 'ds6', 'ds7']}, ['ds3', 'ds6'], ['ds4', 'ds7'], ['another_user', 'another_one']), @@ -325,7 +325,7 @@ class ActionsTest(unittest.TestCase): # Call the function context = {'user': 'user1', 'model': 'model', 'auth_obj': {'id': 1}, 'method': 'revoke'} - result = actions.package_deleted(context, users_info) + result = actions.revoke_access(context, users_info) # Calculate the list of warns warns = [] @@ -344,7 +344,7 @@ class ActionsTest(unittest.TestCase): # Check that the initial functions (check_access and parse_notification) has been called properly parse_delete.assert_called_once_with(users_info) - actions.plugins.toolkit.check_access.assert_called_once_with('package_deleted', context, users_info) + actions.plugins.toolkit.check_access.assert_called_once_with('revoke_access', context, users_info) for user_datasets in parse_result['users_datasets']: for dataset_id in user_datasets['datasets']: From 6bbd9f5b95f353014937d0c2e34488ed83a76008 Mon Sep 17 00:00:00 2001 From: egonzalo Date: Thu, 3 Nov 2016 10:14:28 +0100 Subject: [PATCH 05/22] Log format change in actions.py --- ckanext/privatedatasets/actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index 04818d4..6a76437 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -206,9 +206,9 @@ def _process_package(context, request_data): context_pkg_update['user'] = user.get('name', '') plugins.toolkit.get_action('package_update')(context_pkg_update, dataset) - log.info('Action -%s access to user- ended successfully' % context['method']) + log.info('Action %s access to database ended successfully' % context['method']) else: - log.warn('Action -%s access to user- not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) + log.warn('Action %s access to database not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) else: log.warn('Dataset %s is public. Cannot %s access to users' % (dataset_id, context['method'])) warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) From 36733538910d1ccde246383054fc756c76cb28ea Mon Sep 17 00:00:00 2001 From: egonzalo Date: Thu, 10 Nov 2016 13:33:05 +0100 Subject: [PATCH 06/22] reword of some log messages --- ckanext/privatedatasets/actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index 6a76437..25eb38e 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -206,9 +206,9 @@ def _process_package(context, request_data): context_pkg_update['user'] = user.get('name', '') plugins.toolkit.get_action('package_update')(context_pkg_update, dataset) - log.info('Action %s access to database ended successfully' % context['method']) + log.info('Action %s access to dataset ended successfully' % context['method']) else: - log.warn('Action %s access to database not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) + log.warn('Action %s access to dataset not completed. The dataset %s already %s access to the user %s' % (context['method'], dataset_id, context['method'], user_info['user'])) else: log.warn('Dataset %s is public. Cannot %s access to users' % (dataset_id, context['method'])) warns.append('Unable to upload the dataset %s: It\'s a public dataset' % dataset_id) From 018b84127ce964c938dab039ec8273e3284a75d4 Mon Sep 17 00:00:00 2001 From: fdelavega Date: Wed, 26 Apr 2017 16:52:53 +0200 Subject: [PATCH 07/22] Fix problem with renamed revoked_access action --- ckanext/privatedatasets/actions.py | 2 +- ckanext/privatedatasets/auth.py | 5 +++-- ckanext/privatedatasets/plugin.py | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ckanext/privatedatasets/actions.py b/ckanext/privatedatasets/actions.py index 25eb38e..fea1461 100644 --- a/ckanext/privatedatasets/actions.py +++ b/ckanext/privatedatasets/actions.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. diff --git a/ckanext/privatedatasets/auth.py b/ckanext/privatedatasets/auth.py index c14ddfb..cb2c5c1 100644 --- a/ckanext/privatedatasets/auth.py +++ b/ckanext/privatedatasets/auth.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. @@ -141,6 +141,7 @@ def acquisitions_list(context, data_dict): # Users can get only their acquisitions list return {'success': context['user'] == data_dict['user']} -def package_deleted(context, data_dict): +@tk.auth_allow_anonymous_access +def revoke_access(context, data_dict): # TODO: Check functionality and improve security(if needed) return {'success': True} \ No newline at end of file diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index dd5f231..4d9bab1 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. @@ -113,7 +113,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): # 'resource_show': auth.resource_show, constants.PACKAGE_ACQUIRED: auth.package_acquired, constants.ACQUISITIONS_LIST: auth.acquisitions_list, - constants.PACKAGE_DELETED: auth.package_deleted} + constants.PACKAGE_DELETED: auth.revoke_access} # resource_show is not required in CKAN 2.3 because it delegates to # package_show @@ -156,7 +156,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): return { constants.PACKAGE_ACQUIRED: actions.package_acquired, constants.ACQUISITIONS_LIST: actions.acquisitions_list, - constants.PACKAGE_DELETED: actions.package_deleted + constants.PACKAGE_DELETED: actions.revoke_access } ###################################################################### From 8d8c28dd859b057785248bfc1bb6b98723d3cfdc Mon Sep 17 00:00:00 2001 From: fdelavega Date: Wed, 26 Apr 2017 17:30:49 +0200 Subject: [PATCH 08/22] Update tests to support revoke_access rename --- ckanext/privatedatasets/tests/test_auth.py | 4 ++-- ckanext/privatedatasets/tests/test_plugin.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_auth.py b/ckanext/privatedatasets/tests/test_auth.py index 7cf01b1..077dec6 100644 --- a/ckanext/privatedatasets/tests/test_auth.py +++ b/ckanext/privatedatasets/tests/test_auth.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. @@ -252,7 +252,7 @@ class AuthTest(unittest.TestCase): self.assertTrue(auth.package_acquired({}, {})['success']) def test_package_deleted(self): - self.assertTrue(auth.package_deleted({},{})['success']) + self.assertTrue(auth.revoke_access({},{})['success']) @parameterized.expand([ ({'user': 'user_1'}, {'user': 'user_1'}, True), diff --git a/ckanext/privatedatasets/tests/test_plugin.py b/ckanext/privatedatasets/tests/test_plugin.py index 1c4d50b..d5230a1 100644 --- a/ckanext/privatedatasets/tests/test_plugin.py +++ b/ckanext/privatedatasets/tests/test_plugin.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. @@ -66,7 +66,7 @@ class PluginTest(unittest.TestCase): ('resource_show', plugin.auth.resource_show, True, False), ('package_acquired', plugin.auth.package_acquired), ('acquisitions_list', plugin.auth.acquisitions_list), - ('package_deleted', plugin.auth.package_deleted) + ('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) @@ -99,7 +99,7 @@ class PluginTest(unittest.TestCase): @parameterized.expand([ ('package_acquired', plugin.actions.package_acquired), ('acquisitions_list', plugin.actions.acquisitions_list), - ('package_deleted', plugin.actions.package_deleted) + ('revoke_access', plugin.actions.revoke_access) ]) def test_actions_function(self, function_name, expected_function): actions = self.privateDatasets.get_actions() From efae19f0fbd16763e19f8ba466bc23ba2af7e267 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Arranz?= Date: Tue, 14 Nov 2017 11:03:25 +0100 Subject: [PATCH 09/22] Add fontawesome 4 css classes --- .../privatedatasets/templates/snippets/package_item.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ckanext/privatedatasets/templates/snippets/package_item.html b/ckanext/privatedatasets/templates/snippets/package_item.html index c1bf7cc..5db5776 100755 --- a/ckanext/privatedatasets/templates/snippets/package_item.html +++ b/ckanext/privatedatasets/templates/snippets/package_item.html @@ -28,19 +28,19 @@ Example:

{% if package.private and not h.can_read(package) %} - + {{ _('Private') }} {% endif %} {% if acquired and not owner %} - + {{ _('Acquired') }} {% endif %} {% if owner %} - + {{ _('Owner') }} {% endif %} From c7eef4df32a218a01df766ff4d4a75bada7fc3c8 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Wed, 15 Nov 2017 19:09:36 +0100 Subject: [PATCH 10/22] Implement IPermissionLabel interface to suppport searchable datasets in v2.7 --- ckanext/privatedatasets/plugin.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index 4d9bab1..1434be2 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -20,6 +20,7 @@ import ckan.lib.search as search import ckan.model as model import ckan.plugins as p +from ckan.lib.plugins import DefaultPermissionLabels import ckan.plugins.toolkit as tk import auth import actions @@ -32,7 +33,7 @@ import helpers as helpers HIDDEN_FIELDS = [constants.ALLOWED_USERS, constants.SEARCHABLE] -class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): +class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm, DefaultPermissionLabels): p.implements(p.IDatasetForm) p.implements(p.IAuthFunctions) @@ -41,6 +42,7 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): p.implements(p.IActions) p.implements(p.IPackageController, inherit=True) p.implements(p.ITemplateHelpers) + p.implements(p.IPermissionLabels) ###################################################################### ############################ DATASET FORM ############################ @@ -291,6 +293,24 @@ class PrivateDatasets(p.SingletonPlugin, tk.DefaultDatasetForm): return search_results + #### + + def get_dataset_labels(self, dataset_obj): + labels = super(PrivateDatasets, self).get_dataset_labels( + dataset_obj) + + if getattr(dataset_obj, 'searchable', False): + labels.append('searchable') + + return labels + + def get_user_dataset_labels(self, user_obj): + labels = super(PrivateDatasets, self).get_user_dataset_labels( + user_obj) + + labels.append('searchable') + return labels + ###################################################################### ######################### ITEMPLATESHELPER ########################### ###################################################################### From 62c8fa4f8f2ad9c8c71946cac31442e6107b1ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Arranz?= Date: Thu, 16 Nov 2017 15:49:46 +0100 Subject: [PATCH 11/22] Add fontawesome 4 css classes --- .../templates/package/snippets/package_basic_fields.html | 4 ++-- .../privatedatasets/templates/snippets/acquire_button.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ckanext/privatedatasets/templates/package/snippets/package_basic_fields.html b/ckanext/privatedatasets/templates/package/snippets/package_basic_fields.html index e2f56f1..b240f46 100644 --- a/ckanext/privatedatasets/templates/package/snippets/package_basic_fields.html +++ b/ckanext/privatedatasets/templates/package/snippets/package_basic_fields.html @@ -49,7 +49,7 @@ {% endfor %} - + {% trans %} Private datasets can only be accessed by certain users, while public datasets can be accessed by anyone. {% endtrans %} @@ -68,7 +68,7 @@ {% endfor %} - + {% trans %} Searchable datasets can be searched by anyone, while not-searchable datasets can only be accessed by entering directly its URL. {% endtrans %} diff --git a/ckanext/privatedatasets/templates/snippets/acquire_button.html b/ckanext/privatedatasets/templates/snippets/acquire_button.html index 82755ff..ca49b29 100644 --- a/ckanext/privatedatasets/templates/snippets/acquire_button.html +++ b/ckanext/privatedatasets/templates/snippets/acquire_button.html @@ -10,6 +10,6 @@ Example: #} - + {{ _('Acquire') }} From 2cba3fbe0f0486d3a5ce52bfb8a62bce4387626d Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Fri, 22 Dec 2017 14:42:22 +0100 Subject: [PATCH 12/22] Configure travis CI for testing --- .travis.yml | 18 +++++++ bin/setup_and_test.sh | 107 ------------------------------------------ bin/travis-build.bash | 47 +++++++++++++++++++ bin/travis-run.sh | 3 ++ 4 files changed, 68 insertions(+), 107 deletions(-) create mode 100644 .travis.yml delete mode 100755 bin/setup_and_test.sh create mode 100755 bin/travis-build.bash create mode 100755 bin/travis-run.sh diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..f1b5c2d --- /dev/null +++ b/.travis.yml @@ -0,0 +1,18 @@ +sudo: required +language: python +python: + - "2.7" +env: + - CKANVERSION=2.6.3 POSTGISVERSION=2 + - CKANVERSION=2.7.0 POSTGISVERSION=2 +services: + - redis-server + - postgresql +install: + - bash bin/travis-build.bash +script: + - sh bin/travis-run.sh +after_success: coveralls +branches: + only: + - develop \ No newline at end of file diff --git a/bin/setup_and_test.sh b/bin/setup_and_test.sh deleted file mode 100755 index c4aabf7..0000000 --- a/bin/setup_and_test.sh +++ /dev/null @@ -1,107 +0,0 @@ -#!/bin/bash -set -xe -trap 'jobs -p | xargs --no-run-if-empty kill' INT TERM EXIT - -export PATH=$PATH:/usr/local/bin -export PIP_DOWNLOAD_CACHE=~/.pip_cache - -WD=`pwd` -DB_HOST_IP=${DB_HOST_IP:=127.0.0.1} -POSTGRES_PORT=${POSTGRES_PORT:=5432} - -echo "Downloading CKAN..." -git clone https://github.com/ckan/ckan -cd ckan -git checkout release-v2.5.3 -cd $WD - - -echo "Checking Solr..." -SOLR_ACTIVE=`nc -z localhost 8983; echo $?` - -if [ $SOLR_ACTIVE -ne 0 ] -then - - echo "Downloading Solr..." - CACHE_DIR=~/.cache - FILE=solr-4.10.4.tgz - SOLAR_UNZIP_FOLDER=solr-4.10.4 - - # If the solar folder does not exist, we have to build it - if [ ! -d "$CACHE_DIR/$SOLAR_UNZIP_FOLDER" ] - then - # Download the solar installation file if it does not exist - wget --no-verbose --timestamping --directory-prefix=$CACHE_DIR http://archive.apache.org/dist/lucene/solr/4.10.4/$FILE - - # Unzip the folder - tar -xf "$CACHE_DIR/$FILE" --directory "$CACHE_DIR" - - # Delete the downloaded tar.gz - rm "$CACHE_DIR/$FILE" - fi - - echo "Configuring and starting Solr..." - ln -s "$CACHE_DIR/$SOLAR_UNZIP_FOLDER" . - mv "$SOLAR_UNZIP_FOLDER/example/solr/collection1/conf/schema.xml" "$SOLAR_UNZIP_FOLDER/example/solr/collection1/conf/schema.xml.bak" - ln -s $WD/ckan/ckan/config/solr/schema.xml "$SOLAR_UNZIP_FOLDER/example/solr/collection1/conf/schema.xml" - cd $SOLAR_UNZIP_FOLDER/example - java -jar start.jar 2>&1 > /dev/null & - cd $WD - -else - echo "Solar is already installed..." -fi - - -echo "Setting up virtualenv..." -virtualenv --no-site-packages virtualenv -source virtualenv/bin/activate -pip install --upgrade pip - -# Force html5lib version to be used -pip install html5lib==0.9999999 - - -echo "Installing CKAN dependencies..." -cd ckan -python setup.py develop -pip install -r requirements.txt -pip install -r dev-requirements.txt -cd .. - - -echo "Removing databases from old executions..." -sudo -u postgres psql -c "DROP DATABASE IF EXISTS datastore_test;" -sudo -u postgres psql -c "DROP DATABASE IF EXISTS ckan_test;" -sudo -u postgres psql -c "DROP USER IF EXISTS ckan_default;" - - -echo "Creating the PostgreSQL user and database..." -sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';" -sudo -u postgres psql -c 'CREATE DATABASE ckan_test WITH OWNER ckan_default;' -sudo -u postgres psql -c 'CREATE DATABASE datastore_test WITH OWNER ckan_default;' - - -echo "Modifying the configuration to setup properly the Postgres port..." -mkdir -p data/storage -echo " -sqlalchemy.url = postgresql://ckan_default:pass@$DB_HOST_IP:$POSTGRES_PORT/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@$DB_HOST_IP:$POSTGRES_PORT/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@$DB_HOST_IP:$POSTGRES_PORT/datastore_test - -ckan.storage_path=data/storage" >> test.ini - - -echo "Initializing the database..." -sed -i "s/\(postgresql:\/\/.\+\)@localhost\(:[0-9]\+\)\?/\1@$DB_HOST_IP:$POSTGRES_PORT/g" ckan/test-core.ini -cd ckan -paster db init -c test-core.ini -cd .. - - -echo "Installing ckanext-privatedatasets and its requirements..." -python setup.py develop - - -echo "Running tests..." -python setup.py nosetests diff --git a/bin/travis-build.bash b/bin/travis-build.bash new file mode 100755 index 0000000..682932d --- /dev/null +++ b/bin/travis-build.bash @@ -0,0 +1,47 @@ +#!/usr/bin/env bash + +set -e + +echo "This is travis-build.bash..." + +echo "Installing the packages that CKAN requires..." +sudo apt-get update -qq +sudo apt-get install solr-jetty + +echo "Installing CKAN and its Python dependencies..." +git clone https://github.com/ckan/ckan +cd ckan +git checkout ckan-$CKANVERSION +python setup.py develop + +sed -i "s|psycopg2==2.4.5|psycopg2==2.7.1|g" requirements.txt + +pip install -r requirements.txt --allow-all-external +pip install -r dev-requirements.txt --allow-all-external +cd - + +echo "Setting up Solr..." +# solr is multicore for tests on ckan master now, but it's easier to run tests +# on Travis single-core still. +# see https://github.com/ckan/ckan/issues/2972 +sed -i -e 's/solr_url.*/solr_url = http:\/\/127.0.0.1:8983\/solr/' ckan/test-core.ini +printf "NO_START=0\nJETTY_HOST=127.0.0.1\nJETTY_PORT=8983\nJAVA_HOME=$JAVA_HOME" | sudo tee /etc/default/jetty +sudo cp ckan/ckan/config/solr/schema.xml /etc/solr/conf/schema.xml +sudo service jetty restart + +echo "Creating the PostgreSQL user and database..." +sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';" +sudo -u postgres psql -c "CREATE USER datastore_default WITH PASSWORD 'pass';" +sudo -u postgres psql -c "CREATE DATABASE ckan_test WITH OWNER ckan_default;" +sudo -u postgres psql -c "CREATE DATABASE datastore_test WITH OWNER ckan_default;" + + +echo "Initialising the database..." +cd ckan +paster db init -c test-core.ini +cd - + +echo "Installing ckanext-privatedatasets and its requirements..." +python setup.py develop + +echo "travis-build.bash is done." \ No newline at end of file diff --git a/bin/travis-run.sh b/bin/travis-run.sh new file mode 100755 index 0000000..d3ba879 --- /dev/null +++ b/bin/travis-run.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +python setup.py nosetests \ No newline at end of file From 19894497b28a5c6ee3f6026460d53b246bf6ebc9 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Fri, 22 Dec 2017 14:52:43 +0100 Subject: [PATCH 13/22] Update testing CKAN versions --- .travis.yml | 4 ++-- ckanext/privatedatasets/plugin.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f1b5c2d..047afe1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,8 @@ language: python python: - "2.7" env: - - CKANVERSION=2.6.3 POSTGISVERSION=2 - - CKANVERSION=2.7.0 POSTGISVERSION=2 + - CKANVERSION=2.6.4 POSTGISVERSION=2 + - CKANVERSION=2.7.2 POSTGISVERSION=2 services: - redis-server - postgresql diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index 1434be2..bfa430d 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -21,6 +21,7 @@ import ckan.lib.search as search import ckan.model as model import ckan.plugins as p from ckan.lib.plugins import DefaultPermissionLabels + import ckan.plugins.toolkit as tk import auth import actions From 76b5bb82d93e002a417db2bd20d9befd0c930535 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Fri, 22 Dec 2017 15:06:10 +0100 Subject: [PATCH 14/22] Configure travis to execute selenium tests --- .travis.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 047afe1..66ba481 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,13 +3,18 @@ language: python python: - "2.7" env: - - CKANVERSION=2.6.4 POSTGISVERSION=2 - CKANVERSION=2.7.2 POSTGISVERSION=2 services: - redis-server - postgresql +addons: + firefox: "46.0" install: - bash bin/travis-build.bash +before_script: + - "export DISPLAY=:99.0" + - "sh -e /etc/init.d/xvfb start" + - sleep 3 # give xvfb some time to start script: - sh bin/travis-run.sh after_success: coveralls From fd7f99a43896d46946cb931de4359d273198b4a9 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Tue, 26 Dec 2017 11:22:24 +0100 Subject: [PATCH 15/22] Include travis and coveralls badges --- .travis.yml | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 66ba481..514dad2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ install: before_script: - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" - - sleep 3 # give xvfb some time to start + - sleep 15 # give xvfb some time to start script: - sh bin/travis-run.sh after_success: coveralls diff --git a/README.md b/README.md index 0f5eb43..db4b019 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -CKAN Private Datasets [![Build Status](https://build.conwet.fi.upm.es/jenkins/buildStatus/icon?job=ckan_privatedatasets-develop)](https://build.conwet.fi.upm.es/jenkins/job/ckan_privatedatasets-develop/) +CKAN Private Datasets [![Build Status](https://travis-ci.org/conwetlab/ckanext-privatedatasets.svg?branch=develop)](https://travis-ci.org/conwetlab/ckanext-privatedatasets) [![Coverage Status](https://coveralls.io/repos/github/conwetlab/ckanext-privatedatasets/badge.svg?branch=develop)](https://coveralls.io/github/conwetlab/ckanext-privatedatasets?branch=develop) ===================== This CKAN extension allows a user to create private datasets that only certain users will be able to see. When a dataset is being created, it's possible to specify the list of users that can see this dataset. In addition, the extension provides an HTTP API that allows to add users programmatically. From a7f7069975a89f1cfa676ab4be8ca236dd0e0896 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Tue, 26 Dec 2017 13:26:16 +0100 Subject: [PATCH 16/22] Include an explicit wait for log in button in selenium tests --- .travis.yml | 2 +- ckanext/privatedatasets/plugin.py | 3 ++- ckanext/privatedatasets/tests/test_selenium.py | 16 +++++++++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 514dad2..66ba481 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ install: before_script: - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" - - sleep 15 # give xvfb some time to start + - sleep 3 # give xvfb some time to start script: - sh bin/travis-run.sh after_success: coveralls diff --git a/ckanext/privatedatasets/plugin.py b/ckanext/privatedatasets/plugin.py index bfa430d..0ab370e 100755 --- a/ckanext/privatedatasets/plugin.py +++ b/ckanext/privatedatasets/plugin.py @@ -20,9 +20,10 @@ import ckan.lib.search as search import ckan.model as model import ckan.plugins as p +import ckan.plugins.toolkit as tk + from ckan.lib.plugins import DefaultPermissionLabels -import ckan.plugins.toolkit as tk import auth import actions import constants diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 3339199..fe7001f 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2014 - 2017 CoNWeT Lab., Universidad Politécnica de Madrid # This file is part of CKAN Private Dataset Extension. @@ -17,12 +17,15 @@ # You should have received a copy of the GNU Affero General Public License # along with CKAN Private Dataset Extension. If not, see . +from __future__ import unicode_literals + from nose_parameterized import parameterized from selenium import webdriver -from selenium.webdriver.support.ui import Select +from selenium.webdriver.common.by import By +from selenium.webdriver.support.ui import Select, WebDriverWait +from selenium.webdriver.support import expected_conditions as EC from subprocess import Popen - import ckan.lib.search.index as search_index import ckan.model as model import ckanext.privatedatasets.db as db @@ -82,7 +85,6 @@ class TestSelenium(unittest.TestCase): self.clearBBDD() self.driver.quit() - def assert_fields_disabled(self, fields): for field in fields: self.assertFalse(self.driver.find_element_by_id(field).is_enabled()) @@ -111,7 +113,11 @@ class TestSelenium(unittest.TestCase): def login(self, username, password): driver = self.driver driver.get(self.base_url) - driver.find_element_by_link_text('Log in').click() + login_btn = WebDriverWait(driver, 15).until( + EC.element_to_be_clickable((By.LINK_TEXT, 'Log in')) + ) + login_btn.click() + driver.find_element_by_id('field-login').clear() driver.find_element_by_id('field-login').send_keys(username) driver.find_element_by_id('field-password').clear() From 1682108037314bcdab7980154e87b8989b5fec50 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Tue, 26 Dec 2017 13:56:41 +0100 Subject: [PATCH 17/22] Remove replicated settings in test.ini --- test.ini | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/test.ini b/test.ini index 71897e6..a0d7e90 100644 --- a/test.ini +++ b/test.ini @@ -6,16 +6,14 @@ port = 5000 [app:main] use = config:./ckan/test-core.ini +ckan.site_url = http://127.0.0.1:5000/ + ckan.legacy_templates = no ckan.plugins = privatedatasets + ckan.privatedatasets.parser = ckanext.privatedatasets.parsers.fiware:FiWareNotificationParser ckan.privatedatasets.show_acquire_url_on_create = True ckan.privatedatasets.show_acquire_url_on_edit = True -sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test - -ckan.storage_path=data/storage sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test @@ -23,26 +21,3 @@ ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/dat ckan.storage_path=data/storage -sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test - -ckan.storage_path=data/storage - -sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test - -ckan.storage_path=data/storage - -sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test - -ckan.storage_path=data/storage - -sqlalchemy.url = postgresql://ckan_default:pass@127.0.0.1:5432/ckan_test -ckan.datastore.write_url = postgresql://ckan_default:pass@127.0.0.1:5432/datastore_test -ckan.datastore.read_url = postgresql://datastore_default:pass@127.0.0.1:5432/datastore_test - -ckan.storage_path=data/storage From f24ec2b4282a6d8f25a7f2e540110484bb3c52a5 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Tue, 26 Dec 2017 15:23:35 +0100 Subject: [PATCH 18/22] Update non-authorized dataset access, new versions show a 404 error --- .../privatedatasets/tests/test_selenium.py | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index fe7001f..d1494db 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -246,18 +246,11 @@ class TestSelenium(unittest.TestCase): driver.get(self.base_url + 'dataset/' + dataset_url) if not acquired and private and not in_org: - xpath = '//div[@id=\'content\']/div/div' - buy_msg = 'This private dataset can be acquired. To do so, please click here' - if acquire_url is not None: - self.assertTrue(driver.find_element_by_xpath(xpath).text.startswith(buy_msg)) - self.assertEquals(acquire_url, driver.find_element_by_link_text('here').get_attribute('href')) - xpath += '[2]' # The unauthorized message is in a different Path - else: - src = driver.page_source - self.assertEquals(None, re.search(buy_msg, src)) + # 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' - self.assertTrue('/user/login' in driver.current_url) - self.assertTrue(driver.find_element_by_xpath(xpath).text.startswith('Unauthorized to read package %s' % dataset_url)) + self.assertEquals(driver.find_element_by_xpath(xpath).text, msg) else: self.assertEquals(self.base_url + 'dataset/%s' % dataset_url, driver.current_url) @@ -283,10 +276,6 @@ class TestSelenium(unittest.TestCase): self.register(user, user, '%s@conwet.com' % user, user) @parameterized.expand([ - # (['user1', 'user2', 'user3'], True, True, ['user2'], 'http://store.conwet.com/'), - # (['user1', 'user2', 'user3'], True, True, ['user3']), - # (['user1', 'user2', 'user3'], False, True, ['user3']), - # (['user1', 'user2', 'user3'], True, False, ['user2']), (['user1', 'user2', 'user3'], True, True, [], 'http://store.conwet.com/'), (['user1', 'user2', 'user3'], True, True, []), (['user1', 'user2', 'user3'], False, True, []), @@ -307,7 +296,9 @@ class TestSelenium(unittest.TestCase): url = get_dataset_url(pkg_name) self.create_ds(pkg_name, 'Example description', ['tag1', 'tag2', 'tag3'], private, searchable, allowed_users, acquire_url, 'http://upm.es', 'UPM Main', 'Example Description', 'CSV') + self.check_ds_values(url, private, searchable, allowed_users, acquire_url) + self.check_user_access(pkg_name, url, True, True, False, private, searchable, acquire_url) self.check_acquired(pkg_name, url, False, private) From acaea8782c4517a1840375db76602d50c4405cea Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Wed, 27 Dec 2017 13:42:23 +0100 Subject: [PATCH 19/22] Update selenium tests, users are not logged out in new version --- ckanext/privatedatasets/tests/test_selenium.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index d1494db..8bcc0ee 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -310,11 +310,6 @@ class TestSelenium(unittest.TestCase): acquired = user in allowed_users self.check_user_access(pkg_name, url, False, acquired, False, private, searchable, acquire_url) - # The user is logged out when they try to access a private dataset and they are not included - # in the list of allowed users. - if not acquired and private: - self.login(user, user) - self.check_acquired(pkg_name, url, acquired, private) @parameterized.expand([ From af8283794c4935ce450143097c5ece3cc1b30785 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Wed, 27 Dec 2017 14:23:20 +0100 Subject: [PATCH 20/22] Update organization selenium tests, user are not logged out --- .../privatedatasets/tests/test_selenium.py | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 8bcc0ee..5d57126 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -444,21 +444,6 @@ class TestSelenium(unittest.TestCase): self.check_ds_values(url_path, dataset['private'], dataset['searchable'], final_users, acquire_url) @parameterized.expand([ - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, True, [], 'http://store.conwet.com/'), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, True, []), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], False, True, []), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, False, []), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, True, ['user3'], 'http://store.conwet.com/'), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, True, ['user3']), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], False, True, ['user3']), - # (['user1', 'user2', 'user3'], [{'name': 'CoNWeT', 'users': ['user2']}], True, False, ['user3']), - - # More complex - # (['user1', 'user2', 'user3', 'user4', 'user5', 'user6'], [{'name': 'CoNWeT', 'users': ['user2', 'user3']}], True, True, ['user4', 'user5'], 'http://store.conwet.com/'), - # (['user1', 'user2', 'user3', 'user4', 'user5', 'user6'], [{'name': 'CoNWeT', 'users': ['user2', 'user3']}], True, True, ['user4', 'user5']), - # (['user1', 'user2', 'user3', 'user4', 'user5', 'user6'], [{'name': 'CoNWeT', 'users': ['user2', 'user3']}], False, True, ['user4', 'user5']), - # (['user1', 'user2', 'user3', 'user4', 'user5', 'user6'], [{'name': 'CoNWeT', 'users': ['user2', 'user3']}], True, False, ['user4', 'user5']), - # Even if user6 is in another organization, he/she won't be able to access the dataset (['user1', 'user2', 'user3', 'user4', 'user5', 'user6'], [{'name': 'CoNWeT', 'users': ['user2', 'user3']}, {'name': 'UPM', 'users': ['user6']}], True, True, ['user4', 'user5'], 'http://store.conwet.com/'), @@ -498,11 +483,6 @@ class TestSelenium(unittest.TestCase): in_org = user in orgs[0]['users'] self.check_user_access(pkg_name, url, False, acquired, in_org, private, searchable, acquire_url) - # The user is logged out when they try to access a private dataset and they are not included - # in the list of allowed users. - if not acquired and private and not in_org: - self.login(user, user) - self.check_acquired(pkg_name, url, acquired, private) def test_bug_16(self): From 958d621ff4e197ab301c3b0cbb617a0e06d18980 Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Wed, 27 Dec 2017 19:07:09 +0100 Subject: [PATCH 21/22] Update selenium tests, new version always displays datasets to owners --- ckanext/privatedatasets/tests/test_selenium.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 5d57126..832d207 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -226,20 +226,20 @@ class TestSelenium(unittest.TestCase): driver = self.driver driver.find_element_by_link_text('Datasets').click() - if searchable: + if searchable or owner: xpath = '//div[@id=\'content\']/div[3]/div/section/div/ul/li/div/h3/span' # Check the label + if owner: + self.assertEqual('OWNER', driver.find_element_by_xpath(xpath).text) if not acquired and private and not in_org: self.assertEqual('PRIVATE', driver.find_element_by_xpath(xpath).text) elif acquired and not owner and private: self.assertEqual('ACQUIRED', driver.find_element_by_xpath(xpath).text) - elif owner: - self.assertEqual('OWNER', driver.find_element_by_xpath(xpath).text) # When a user cannot access a dataset, the link is no longer provided else: - # If the dataset is not searchable, a link to it could not be found in the dataset search page + # If the dataset is not searchable and the user is not the owner, a link to it could not be found in the dataset search page self.assertEquals(None, re.search(dataset_url, driver.page_source)) # Access the dataset From 342a42b4d967076be91548603c672b9f31f13d5f Mon Sep 17 00:00:00 2001 From: Francisco de la Vega Date: Thu, 28 Dec 2017 10:44:12 +0100 Subject: [PATCH 22/22] Update selenium tests, org members can see non searchable datasets --- ckanext/privatedatasets/tests/test_selenium.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ckanext/privatedatasets/tests/test_selenium.py b/ckanext/privatedatasets/tests/test_selenium.py index 832d207..a49a9a5 100644 --- a/ckanext/privatedatasets/tests/test_selenium.py +++ b/ckanext/privatedatasets/tests/test_selenium.py @@ -226,7 +226,7 @@ class TestSelenium(unittest.TestCase): driver = self.driver driver.find_element_by_link_text('Datasets').click() - if searchable or owner: + if searchable or owner or in_org: xpath = '//div[@id=\'content\']/div[3]/div/section/div/ul/li/div/h3/span' # Check the label @@ -313,20 +313,14 @@ class TestSelenium(unittest.TestCase): self.check_acquired(pkg_name, url, acquired, private) @parameterized.expand([ - # (['a'] , 'http://upm.es', 'Allowed users: Name must be at least 2 characters long'), - # (['a a'], 'http://upm.es', 'Allowed users: Url must be purely lowercase alphanumeric (ascii) characters and these symbols: -_'), (['upm', 'a'], 'http://upm.es', 'Allowed users: Must be at least 2 characters long'), (['upm', 'a a a'], 'http://upm.es', 'Allowed users: Must be purely lowercase alphanumeric (ascii) characters and these symbols: -_'), (['upm', 'a?-vz'], 'http://upm.es', 'Allowed users: Must be purely lowercase alphanumeric (ascii) characters and these symbols: -_'), (['thisisaveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylongname'], 'http://upm.es', 'Allowed users: Name must be a maximum of 100 characters long'), (['conwet'], 'ftp://google.es', 'Acquire URL: The URL "ftp://google.es" is not valid.'), - # (['conwet'], 'http://google*.com', 'Acquire URL: The URL "http://google*.com" is not valid.'), - # (['conwet'], 'http://google+.com', 'Acquire URL: The URL "http://google+.com" is not valid.'), - # (['conwet'], 'http://google/.com', 'Acquire URL: The URL "http://google/.com" is not valid.'), (['conwet'], 'google', 'Acquire URL: The URL "google" is not valid.'), (['conwet'], 'http://google', 'Acquire URL: The URL "http://google" is not valid.'), - # (['conwet'], 'http://google:es', 'Acquire URL: The URL "http://google:es" is not valid.'), (['conwet'], 'www.google.es', 'Acquire URL: The URL "www.google.es" is not valid.') ])