diff --git a/geonode/base/admin.py b/geonode/base/admin.py index 5f0e1a39822..4b68a37512f 100755 --- a/geonode/base/admin.py +++ b/geonode/base/admin.py @@ -99,7 +99,7 @@ def set_user_and_group_dataset_permission(modeladmin, request, queryset): } form = UserAndGroupPermissionsForm({ - 'permission_type': ('r', ), + 'permission_type': 'read', 'mode': 'set', 'ids': ids, }) diff --git a/geonode/base/forms.py b/geonode/base/forms.py index 5c241b1562b..b87c84f9009 100644 --- a/geonode/base/forms.py +++ b/geonode/base/forms.py @@ -638,13 +638,13 @@ def __init__(self, *args, **kwargs): layers = forms.ModelMultipleChoiceField( queryset=Dataset.objects.all(), required=False) - permission_type = forms.MultipleChoiceField( + permission_type = forms.ChoiceField( required=True, - widget=forms.CheckboxSelectMultiple, + widget=forms.RadioSelect, choices=( - ('r', 'Read'), - ('w', 'Write'), - ('d', 'Download'), + ('read', 'Read'), + ('edit', 'Edit'), + ('download', 'Download') ), ) mode = forms.ChoiceField( diff --git a/geonode/base/templates/base/user_and_group_permissions.html b/geonode/base/templates/base/user_and_group_permissions.html index e8dec70707d..14490cbb8c0 100644 --- a/geonode/base/templates/base/user_and_group_permissions.html +++ b/geonode/base/templates/base/user_and_group_permissions.html @@ -11,7 +11,7 @@

{% trans "Batch Edit" %}

-
+ {% csrf_token %} {{ form|as_bootstrap }}
diff --git a/geonode/base/views.py b/geonode/base/views.py index ae59dfe49af..993281cdf4b 100644 --- a/geonode/base/views.py +++ b/geonode/base/views.py @@ -99,7 +99,7 @@ def user_and_group_permission(request, model): form = UserAndGroupPermissionsForm(request.POST) ids = ids.split(",") if form.is_valid(): - resources_names = [layer.name for layer in form.cleaned_data.get('layers')] + resources_names = [layer.title for layer in form.cleaned_data.get('layers')] users_usernames = [user.username for user in model_class.objects.filter( id__in=ids)] if model == 'profile' else None groups_names = [group_profile.group.name for group_profile in model_class.objects.filter( @@ -121,13 +121,13 @@ def user_and_group_permission(request, model): if permissions_names: set_permissions.apply_async( - (permissions_names, resources_names, users_usernames, groups_names, delete_flag)) + ([permissions_names], resources_names, users_usernames, groups_names, delete_flag)) return HttpResponseRedirect( get_url_for_app_model(model, model_class)) form = UserAndGroupPermissionsForm({ - 'permission_type': ('r', ), + 'permission_type': 'read', 'mode': 'set', }) return render( diff --git a/geonode/layers/management/commands/set_layers_permissions.py b/geonode/layers/management/commands/set_layers_permissions.py index 72e569e3e0f..0e23584de7e 100644 --- a/geonode/layers/management/commands/set_layers_permissions.py +++ b/geonode/layers/management/commands/set_layers_permissions.py @@ -17,10 +17,15 @@ # ######################################################################### -from django.core.management.base import BaseCommand +import logging from argparse import RawTextHelpFormatter + +from django.core.management.base import BaseCommand +from geonode.layers.models import Dataset from geonode.layers.utils import set_datasets_permissions +logger = logging.getLogger("geonode.layers.management.set_layers_permissions") + class Command(BaseCommand): @@ -48,67 +53,82 @@ def create_parser(self, *args, **kwargs): def add_arguments(self, parser): parser.add_argument( - '-r', - '--resources', - dest='resources', - nargs='*', + "-r", + "--resources", + dest="resources", + nargs="*", type=str, - default=None, - help='Resources names for which permissions will be assigned to. ' - 'Default value: None (all the layers will be considered). ' - 'Multiple choices can be typed with white space separator.' - 'A Note: names with white spaces must be typed inside quotation marks.' + default=[], + help="Resources IDs for which permissions will be assigned to. Default value: [] (all the layers will be considered). " ) parser.add_argument( - '-p', - '--permission', - dest='permission', + "-p", + "--permission", + dest="permission", type=str, default=None, - help='Permissions to be assigned. ' - 'Allowed values are: read (r), write (w), download (d) and owner (o).' + help="Permissions to be assigned. " "Allowed values are: view, download, edit and manage.", ) parser.add_argument( - '-u', - '--users', - dest='users', - nargs='*', + "-u", + "--users", + dest="users", + nargs="*", type=str, - default=None, - help='Users for which permissions will be assigned to. ' - 'Multiple choices can be typed with white space separator.' + default=[], + help="Users for which permissions will be assigned to. " + "Multiple choices can be typed with comma separator.", ) parser.add_argument( - '-g', - '--groups', - dest='groups', - nargs='*', + "-g", + "--groups", + dest="groups", + nargs="*", type=str, - default=None, - help='Groups for which permissions will be assigned to. ' - 'Multiple choices can be typed with white space separator.' + default=[], + help="Groups for which permissions will be assigned to. " + "Multiple choices can be typed with comma separator.", ) parser.add_argument( - '-d', - '--delete', - dest='delete_flag', - action='store_true', + "-d", + "--delete", + dest="delete_flag", + action="store_true", default=False, - help='Delete permission if it exists.' + help="Delete permission if it exists.", ) def handle(self, *args, **options): # Retrieving the arguments - resources_names = options.get('resources') - permissions_name = options.get('permission') - users_usernames = options.get('users') - groups_names = options.get('groups') - delete_flag = options.get('delete_flag') + permissions_name = options.get("permission").replace(" ", "") + + resources_pk = [x.replace(" ", "") for x in options.get("resources", [])] + if resources_pk: + resources_pk = resources_pk[0].split(",") + else: + resources_pk = [x for x in Dataset.objects.values_list('pk', flat=True)] + + users_usernames = [x.replace(" ", "") for x in options.get("users", [])] + if users_usernames: + users_usernames = users_usernames[0].split(",") + + groups_names = [x.replace(" ", "") for x in options.get("groups", [])] + if groups_names: + groups_names = groups_names[0].split(",") + + delete_flag = options.get("delete_flag") + + if isinstance(permissions_name, list): + # it accept one kind of permission per request + raise Exception("Only one permission name must be specified") + + if not users_usernames and not groups_names: + raise Exception("Groups or Usernames must be specified") + set_datasets_permissions( permissions_name, - resources_names, - users_usernames, - groups_names, - delete_flag, - verbose=True + resources_names=resources_pk, + users_usernames=users_usernames, + groups_names=groups_names, + delete_flag=delete_flag ) diff --git a/geonode/layers/tests.py b/geonode/layers/tests.py index 479d06113a2..4cb50f90267 100644 --- a/geonode/layers/tests.py +++ b/geonode/layers/tests.py @@ -18,6 +18,7 @@ ######################################################################### import io +import itertools import os import shutil import gisdata @@ -35,6 +36,7 @@ from django.test.client import RequestFactory from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile +from django.core.management import call_command from django.contrib.auth.models import Group from django.contrib.gis.geos import Polygon from django.db.models import Count @@ -902,7 +904,7 @@ def test_assign_remove_permissions(self): perm_spec = layer.get_all_level_info() self.assertNotIn(get_user_model().objects.get(username="norman"), perm_spec["users"]) - utils.set_datasets_permissions("write", resources_names=[layer.name], users_usernames=["norman"], delete_flag=False, verbose=True) + utils.set_datasets_permissions("edit", resources_names=[layer.name], users_usernames=["norman"], delete_flag=False, verbose=True) perm_spec = layer.get_all_level_info() _c = 0 if "users" in perm_spec: @@ -2017,3 +2019,114 @@ def test_dataset_time_form_should_raise_error_if_invalid_payload(self): self.assertFalse(form.is_valid()) self.assertTrue('presentation' in form.errors) self.assertEqual("Select a valid choice. INVALID_PRESENTATION_VALUE is not one of the available choices.", form.errors['presentation'][0]) + + +class SetLayersPermissionsCommand(GeoNodeBaseTestSupport): + ''' + Unittest to ensure that the management command "set_layers_permissions" + behaves as expected + ''' + + def test_user_get_the_download_permissions_for_the_selected_dataset(self): + ''' + Given a user, the compact perms and the resource id, it shoul set the + permissions for the selected resource + ''' + try: + expected_perms = {'view_resourcebase', 'download_resourcebase'} + + dataset, args, username, opts = self._create_arguments(perms_type='download') + call_command('set_layers_permissions', *args, **opts) + + self._assert_perms(expected_perms, dataset, username) + finally: + if dataset: + dataset.delete() + + def test_user_get_the_view_permissions_for_the_selected_dataset(self): + ''' + Given a user, the compact perms and the resource id, it shoul set the + permissions for the selected resource + ''' + try: + expected_perms = {'view_resourcebase'} + dataset, args, username, opts = self._create_arguments(perms_type='view') + + call_command('set_layers_permissions', *args, **opts) + + self._assert_perms(expected_perms, dataset, username) + + finally: + if dataset: + dataset.delete() + + def test_user_get_the_edit_permissions_for_the_selected_dataset(self): + ''' + Given a user, the compact perms and the resource id, it shoul set the + permissions for the selected resource + ''' + try: + expected_perms = { + 'view_resourcebase', + 'change_dataset_style', + 'download_resourcebase', + 'change_resourcebase_metadata', + 'change_dataset_data', + 'change_resourcebase' + } + + dataset, args, username, opts = self._create_arguments(perms_type='edit') + + call_command('set_layers_permissions', *args, **opts) + + self._assert_perms(expected_perms, dataset, username) + finally: + if dataset: + dataset.delete() + + def test_user_get_the_manage_permissions_for_the_selected_dataset(self): + ''' + Given a user, the compact perms and the resource id, it shoul set the + permissions for the selected resource + ''' + try: + expected_perms = { + 'delete_resourcebase', + 'change_resourcebase', + 'view_resourcebase', + 'change_resourcebase_permissions', + 'change_dataset_style', + 'change_resourcebase_metadata', + 'publish_resourcebase', + 'change_dataset_data', + 'download_resourcebase' + } + + dataset, args, username, opts = self._create_arguments(perms_type='manage') + + call_command('set_layers_permissions', *args, **opts) + + self._assert_perms(expected_perms, dataset, username) + finally: + if dataset: + dataset.delete() + + def _create_arguments(self, perms_type): + dataset = create_single_dataset('dataset_for_management_command') + args = [] + username = get_user_model().objects.exclude(username='admin').exclude(username='AnonymousUser').first().username + opts = { + "permission": perms_type, + "users": [username], + "resources": str(dataset.id) + } + + return dataset, args, username, opts + + def _assert_perms(self, expected_perms, dataset, username): + dataset.refresh_from_db() + + perms = dataset.get_all_level_info() + self.assertTrue(username in [user.username for user in perms['users']]) + actual = set(itertools.chain.from_iterable([perms for user, perms in perms['users'].items() if user.username == username])) + self.assertSetEqual(expected_perms, actual) diff --git a/geonode/layers/utils.py b/geonode/layers/utils.py index f263282bdf6..0ecd0d52a82 100644 --- a/geonode/layers/utils.py +++ b/geonode/layers/utils.py @@ -21,6 +21,7 @@ """ # Standard Modules +import copy import re import os import glob @@ -41,6 +42,7 @@ from django.utils.translation import ugettext as _ from django.core.exceptions import ObjectDoesNotExist, SuspiciousFileOperation from geonode.layers.api.exceptions import InvalidDatasetException +from geonode.security.permissions import PermSpec, PermSpecCompact from geonode.storage.manager import storage_manager # Geonode functionality from geonode.base.models import Region @@ -419,195 +421,84 @@ def surrogate_escape_string(input_string, source_character_set): def set_datasets_permissions(permissions_name, resources_names=None, users_usernames=None, groups_names=None, delete_flag=False, verbose=False): + # here to avoid circular import + from geonode.resource.manager import resource_manager # Processing information - if not resources_names: - # If resources is None we consider all the existing layer - resources = Dataset.objects.all() - else: - try: - resources = Dataset.objects.filter(Q(title__in=resources_names) | Q(name__in=resources_names)) - except Dataset.DoesNotExist: - logger.warning( - f'No resources have been found with these names: {", ".join(resources_names)}.' - ) - if not resources: - logger.warning("No resources have been found. No update operations have been executed.") - else: - # PERMISSIONS - if not permissions_name: - logger.error("No permissions have been provided.") + resources_as_pk = [] + for el in resources_names or []: + if isinstance(el, str) and not el.isnumeric(): + res = Dataset.objects.filter(Q(title=el) | Q(name=el)) + if res.exists(): + resources_as_pk.append(res.first().pk) else: - permissions = [] - if permissions_name.lower() in ('read', 'r'): - if not delete_flag: - permissions = READ_PERMISSIONS - else: - permissions = READ_PERMISSIONS + WRITE_PERMISSIONS \ - + DOWNLOAD_PERMISSIONS + OWNER_PERMISSIONS - elif permissions_name.lower() in ('write', 'w'): - if not delete_flag: - permissions = READ_PERMISSIONS + WRITE_PERMISSIONS - else: - permissions = WRITE_PERMISSIONS - elif permissions_name.lower() in ('download', 'd'): - if not delete_flag: - permissions = READ_PERMISSIONS + DOWNLOAD_PERMISSIONS - else: - permissions = DOWNLOAD_PERMISSIONS - elif permissions_name.lower() in ('owner', 'o'): - if not delete_flag: - permissions = READ_PERMISSIONS + WRITE_PERMISSIONS \ - + DOWNLOAD_PERMISSIONS + OWNER_PERMISSIONS - else: - permissions = OWNER_PERMISSIONS - if not permissions: - logger.error( - "Permission must match one of these values: read (r), write (w), download (d), owner (o)." - ) - else: - if not users_usernames and not groups_names: - logger.error( - "At least one user or one group must be provided." - ) - else: - # USERS - users = [] - if users_usernames: - User = get_user_model() - for _user in users_usernames: - try: - if isinstance(_user, str): - user = User.objects.get(username=_user) - else: - user = User.objects.get(username=_user.username) - users.append(user) - except User.DoesNotExist: - logger.warning( - f'The user {_user} does not exists. ' - 'It has been skipped.' - ) - # GROUPS - groups = [] - if groups_names: - for group_name in groups_names: - try: - group = Group.objects.get(name=group_name) - groups.append(group) - except Group.DoesNotExist: - logger.warning( - f'The group {group_name} does not exists. ' - 'It has been skipped.' - ) - if not users and not groups: - logger.error( - 'Neither users nor groups corresponding to the typed names have been found. ' - 'No update operations have been executed.' + resources_as_pk.append(el) + + not_found = [] + final_perms_payload = {} + + for rpk in resources_as_pk: + resource = Dataset.objects.filter(pk=rpk) + if not resource.exists(): + not_found.append(rpk) + logger.error(f"Resource named: {rpk} not found, skipping....") + continue + else: + # creating the payload from the CompactPermissions like we do in the UI. + # the result will be a payload with the compact permissions list required + # for the selected resource + resource = resource.first() + # getting the actual permissions available for the dataset + original_perms = PermSpec(resource.get_all_level_info(), resource) + new_perms_payload = {"organizations": [], "users": [], "groups": []} + # if the username is specified, we add them to the payload with the compact + # perm value + if users_usernames: + User = get_user_model() + for _user in users_usernames: + try: + new_perms_payload["users"].append( + {"id": User.objects.get(username=_user).pk, "permissions": permissions_name} + ) + except User.DoesNotExist: + logger.warning(f"The user {_user} does not exists. " "It has been skipped.") + # GROUPS + # if the group is specified, we add them to the payload with the compact + # perm value + if groups_names: + for group_name in groups_names: + try: + new_perms_payload["groups"].append( + {"id": Group.objects.get(name=group_name).pk, "permissions": permissions_name} ) - else: - # RESOURCES - for resource in resources: - # Existing permissions on the resource - perm_spec = resource.get_all_level_info() - if verbose: - logger.info( - f"Initial permissions info for the resource {resource.title}: {perm_spec}" - ) - print( - f"Initial permissions info for the resource {resource.title}: {perm_spec}" - ) - for u in users: - _user = u - # Add permissions - if not delete_flag: - # Check the permission already exists - if _user not in perm_spec["users"] and _user.username not in perm_spec["users"]: - perm_spec["users"][_user] = permissions - else: - if _user.username in perm_spec["users"]: - u_perms_list = perm_spec["users"][_user.username] - del (perm_spec["users"][_user.username]) - perm_spec["users"][_user] = u_perms_list - - try: - u_perms_list = perm_spec["users"][_user] - base_set = set(u_perms_list) - target_set = set(permissions) - perm_spec["users"][_user] = list(base_set | target_set) - except KeyError: - perm_spec["users"][_user] = permissions - - # Delete permissions - else: - # Skip resource owner - if _user != resource.owner: - if _user in perm_spec["users"]: - u_perms_set = set() - for up in perm_spec["users"][_user]: - if up not in permissions: - u_perms_set.add(up) - perm_spec["users"][_user] = list(u_perms_set) - else: - logger.warning( - f"The user {_user.username} does not have " - f"any permission on the dataset {resource.title}. " - "It has been skipped." - ) - else: - logger.warning( - f"Warning! - The user {_user.username} is the " - f"layer {resource.title} owner, " - "so its permissions can't be changed. " - "It has been skipped." - ) - for g in groups: - _group = g - # Add permissions - if not delete_flag: - # Check the permission already exists - if _group not in perm_spec["groups"] and _group.name not in perm_spec["groups"]: - perm_spec["groups"][_group] = permissions - else: - if _group.name in perm_spec["groups"]: - g_perms_list = perm_spec["groups"][_group.name] - del (perm_spec["groups"][_group.name]) - perm_spec["groups"][_group] = g_perms_list - - try: - g_perms_list = perm_spec["groups"][_group] - base_set = set(g_perms_list) - target_set = set(permissions) - perm_spec["groups"][_group] = list(base_set | target_set) - except KeyError: - perm_spec["groups"][_group] = permissions - - # Delete permissions - else: - if g in perm_spec["groups"]: - g_perms_set = set() - for gp in perm_spec["groups"][g]: - if gp not in permissions: - g_perms_set.add(gp) - perm_spec["groups"][g] = list(g_perms_set) - else: - logger.warning( - f"The group {g.name} does not have any permission " - f"on the dataset {resource.title}. " - "It has been skipped." - ) - # Set final permissions - from geonode.resource.manager import resource_manager - resource_manager.set_permissions(resource.uuid, instance=resource, permissions=perm_spec) - - if verbose: - logger.info( - f"Final permissions info for the resource {resource.title}: {perm_spec}" - ) - print( - f"Final permissions info for the resource {resource.title}: {perm_spec}" - ) - if verbose: - logger.info("Permissions successfully updated!") - print("Permissions successfully updated!") + except Group.DoesNotExist: + logger.warning(f"The group {group_name} does not exists. " "It has been skipped.") + # Using the compact permissions payload to calculate the permissions + # that we want to give for each user/group + # This part is in common with the permissions API + new_compact_perms = PermSpecCompact(new_perms_payload, resource) + copy_compact_perms = copy.deepcopy(new_compact_perms) + + perms_spec_compact_resource = PermSpecCompact(original_perms.compact, resource) + perms_spec_compact_resource.merge(new_compact_perms) + + final_perms_payload = perms_spec_compact_resource.extended + # if the delete flag is set, we must delete the permissions for the input user/group + if delete_flag: + # since is a delete operation, we must remove the users/group from the resource + # so this will return the updated dict without the user/groups to be removed + final_perms_payload["users"] = { + _user: _perms + for _user, _perms in perms_spec_compact_resource.extended["users"].items() + if _user not in copy_compact_perms.extended["users"] + } + final_perms_payload["groups"] = { + _group: _perms + for _group, _perms in perms_spec_compact_resource.extended["groups"].items() + if _user not in copy_compact_perms.extended["groups"] + } + + # calling the resource manager to set the permissions + resource_manager.set_permissions(resource.uuid, instance=resource, permissions=final_perms_payload) def get_uuid_handler(): diff --git a/geonode/people/tests.py b/geonode/people/tests.py index 2e12697b906..8f96b70d4cf 100644 --- a/geonode/people/tests.py +++ b/geonode/people/tests.py @@ -54,7 +54,7 @@ def setUp(self): self.layers = Dataset.objects.all()[:3] self.dataset_ids = [layer.pk for layer in self.layers] self.user_ids = ",".join(str(element.pk) for element in get_user_model().objects.all()[:3]) - self.permission_type = ("r", "w", "d") + self.permission_type = ("read", "download", "edit") self.groups = Group.objects.all()[:3] self.group_ids = ",".join(str(element.pk) for element in self.groups) diff --git a/geonode/people/urls.py b/geonode/people/urls.py index fbb23e5a634..e27ece68650 100644 --- a/geonode/people/urls.py +++ b/geonode/people/urls.py @@ -35,6 +35,6 @@ url(r'^forgotname', views.forgot_username, name='forgot_username'), url(r'^autocomplete/$', login_required(ProfileAutocomplete.as_view()), name='autocomplete_profile'), - url(r'^layer/permission/$', + url(r'^dataset/permission/$', SetUserLayerPermission.as_view(), name='set_user_dataset_permissions'), ] diff --git a/geonode/settings.py b/geonode/settings.py index 0857e2e1824..e82ac2b8fec 100644 --- a/geonode/settings.py +++ b/geonode/settings.py @@ -2123,6 +2123,7 @@ def get_geonode_catalogue_service(): "sync_geonode_maps", "importlayers", "set_all_datasets_metadata", + "set_layers_permissions", ] + ast.literal_eval(os.getenv('MANAGEMENT_COMMANDS_EXPOSED_OVER_HTTP ', '[]')))