From 27f8d24fef222efb7479d8f3dccf35bb7b262800 Mon Sep 17 00:00:00 2001 From: Alessio Fabiani Date: Tue, 19 Apr 2022 19:22:37 +0200 Subject: [PATCH] [Fixes #9115] Advanced Workflow: permissions assigned to managers/members should be filtered by owner and resource group metadata only (#9119) * [Fixes #9106] Implement API for compact permissions * [CircleCi] Fix tests * [CircleCi] Fix tests * [CircleCi] Fix tests * [CircleCi] Fix tests * [CircleCi] Fix tests * [Fixes #9114] Group managers are assigned Manage permission on adding a group with any permission in share permissions form * [Fixes #9115] Advanced Workflow: permissions assigned to managers/members should be filtered by owner and resource group metadata only * - Fixes: The "get_all_level_info" forcibly and wrongly writes manages into the "perm_spec" - Fixes: The "update_metadata" does not forward information about group and approval status change to the update method * - Fixes: promotion/demotion consider the owner's group also --- geonode/layers/views.py | 7 +++++ geonode/resource/manager.py | 6 +++- geonode/security/models.py | 61 +++++++++++++------------------------ geonode/security/utils.py | 8 +---- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/geonode/layers/views.py b/geonode/layers/views.py index e3f61ed91a6..71a385add74 100644 --- a/geonode/layers/views.py +++ b/geonode/layers/views.py @@ -726,10 +726,17 @@ def dataset_metadata( tb = traceback.format_exc() logger.error(tb) + vals = {} + if 'group' in dataset_form.changed_data: + vals['group'] = dataset_form.cleaned_data.get('group') + if any([x in dataset_form.changed_data for x in ['is_approved', 'is_published']]): + vals['is_approved'] = dataset_form.cleaned_data.get('is_approved', layer.is_approved) + vals['is_published'] = dataset_form.cleaned_data.get('is_published', layer.is_published) resource_manager.update( layer.uuid, instance=layer, notify=True, + vals=vals, extra_metadata=json.loads(dataset_form.cleaned_data['extra_metadata']) ) return HttpResponse(json.dumps({'message': message})) diff --git a/geonode/resource/manager.py b/geonode/resource/manager.py index 1c1fa4d0e3c..b448537974e 100644 --- a/geonode/resource/manager.py +++ b/geonode/resource/manager.py @@ -375,7 +375,11 @@ def update(self, uuid: str, /, instance: ResourceBase = None, xml_file: str = No try: _resource.save(notify=notify) resourcebase_post_save(_resource.get_real_instance()) - _resource.set_permissions() + _resource.set_permissions( + created=False, + approval_status_changed=(vals is not None and any([x in vals for x in ['is_approved', 'is_published']])), + group_status_changed=(vals is not None and 'group' in vals) + ) if _resource.state != enumerations.STATE_INVALID: _resource.set_processing_state(enumerations.STATE_PROCESSED) except Exception as e: diff --git a/geonode/security/models.py b/geonode/security/models.py index dcf8ba36f49..0dc5de4f8f0 100644 --- a/geonode/security/models.py +++ b/geonode/security/models.py @@ -92,31 +92,6 @@ def get_all_level_info(self): resource = self.get_self_resource() users = get_users_with_perms(resource) groups = get_groups_with_perms(resource, attach_perms=True) - if groups: - for group in groups: - try: - group_profile = GroupProfile.objects.get(slug=group.name) - managers = group_profile.get_managers() - if managers: - for manager in managers: - if manager not in users and not manager.is_superuser and \ - manager != resource.owner: - users[manager] = ADMIN_PERMISSIONS + VIEW_PERMISSIONS + DOWNLOAD_PERMISSIONS - except GroupProfile.DoesNotExist: - tb = traceback.format_exc() - logger.debug(tb) - if resource.group: - try: - group_profile = GroupProfile.objects.get(slug=resource.group.name) - managers = group_profile.get_managers() - if managers: - for manager in managers: - if manager not in users and not manager.is_superuser and \ - manager != resource.owner: - users[manager] = ADMIN_PERMISSIONS + VIEW_PERMISSIONS + DOWNLOAD_PERMISSIONS - except GroupProfile.DoesNotExist: - tb = traceback.format_exc() - logger.debug(tb) info = { 'users': users, @@ -171,28 +146,34 @@ def get_group_managers(self, group=None): """ obj_groups = [] obj_group_managers = [] - user_groups = get_user_groups(self.owner, group=group) - if user_groups: - for _user_group in user_groups: - if not skip_registered_members_common_group(Group.objects.get(name=_user_group)): - try: - _group_profile = GroupProfile.objects.get(slug=_user_group) - managers = _group_profile.get_managers() - if managers: - for manager in managers: - if manager not in obj_group_managers and not manager.is_superuser: - obj_group_managers.append(manager) - except GroupProfile.DoesNotExist: - tb = traceback.format_exc() - logger.debug(tb) + if group: + user_groups = get_user_groups(self.owner, group=group) + if user_groups: + for _user_group in user_groups: + if not skip_registered_members_common_group(Group.objects.get(name=_user_group)): + try: + _group_profile = GroupProfile.objects.get(slug=_user_group) + managers = _group_profile.get_managers() + if managers: + for manager in managers: + if manager not in obj_group_managers and not manager.is_superuser: + obj_group_managers.append(manager) + except GroupProfile.DoesNotExist: + tb = traceback.format_exc() + logger.debug(tb) if self.group: obj_groups.append(self.group) for x in self.owner.groupmember_set.all(): if x.group.slug != groups_settings.REGISTERED_MEMBERS_GROUP_NAME: obj_groups.append(x.group.group) + managers = x.group.get_managers() + if managers: + for manager in managers: + if manager not in obj_group_managers and not manager.is_superuser: + obj_group_managers.append(manager) - return obj_groups, obj_group_managers + return list(set(obj_groups)), list(set(obj_group_managers)) def set_default_permissions(self, owner=None, created=False): """ diff --git a/geonode/security/utils.py b/geonode/security/utils.py index dd8f58ddb1c..425cf05fbd4 100644 --- a/geonode/security/utils.py +++ b/geonode/security/utils.py @@ -162,7 +162,6 @@ def get_resources_with_perms(user, filter_options={}, shortcut_kwargs={}): """ Returns resources a user has access to. """ - from geonode.base.models import ResourceBase if settings.SKIP_PERMS_FILTER: @@ -210,7 +209,6 @@ def get_geoapp_subtypes(): Returns a list of geoapp subtypes. eg ['geostory'] """ - from geonode.geoapps.models import GeoApp subtypes = [] @@ -678,12 +676,8 @@ def set_group_member_permissions(user, group, role): ["base.view_resourcebase", "base.change_resourcebase"], any_perm=True) .filter(group=group.group) - .exclude(owner=user) ) - # A.F.: By including 'group.resources()' here, we will look also for resources - # having permissions related to the current 'group' and not only the ones assigned - # to the 'group' through the metadata settings. - _resources = set([_r for _r in queryset.iterator()] + [_r for _r in group.resources()]) + _resources = set([_r for _r in queryset.iterator()]) if len(_resources) == 0: queryset = ( get_objects_for_user(