From 9e28a29a53cd4017ac664abb6b4df34effa0207a Mon Sep 17 00:00:00 2001 From: mattiagiupponi <51856725+mattiagiupponi@users.noreply.github.com> Date: Tue, 6 Jul 2021 10:31:16 +0200 Subject: [PATCH] [Fixes #7718] Permissions assignments on Remote Services (#7719) * [Backport Resolves #7392] Fix upload/replace/append layer * [Fixes #7718] Permissions assignments on Remote Services * [Fixes #7718] Permissions assignments on Remote Services * [Fixes #7718] Permissions assignments on Remote Services * [Fixes #7718] Permissions assignments on Remote Services * [Fixes #7718] Pep8 issues fixed * [Fixes #7718] Permissions assignments on Remote Services * [Fixes #7718] remove unused imports * [Fixes #7718] Fix broken migrations * [CircelCI] Tests fix * [Fixes #7718] db startup error * [Fixes #7718] Fix impovements from ISSUE * Update views.py * Update service_detail.html * [Fixes #7718] Fix count layers on services list, now is based on visible resources Co-authored-by: afabiani (cherry picked from commit 0c71f23194afdc4f0e6b8251e8e155ad313b2c50) --- geonode/base/populate_test_data.py | 8 +-- geonode/base/templatetags/base_tags.py | 7 ++ .../0033_set_contributors_permissions.py | 37 ++++++++++ geonode/security/models.py | 3 +- geonode/security/permissions.py | 9 ++- geonode/security/utils.py | 13 +++- .../migrations/0044_auto_20210628_0955.py | 17 +++++ .../migrations/0045_auto_20210629_1355.py | 17 +++++ geonode/services/models.py | 8 +++ .../templates/services/service_detail.html | 22 +++--- .../templates/services/service_list.html | 13 ++-- .../services/service_resources_harvest.html | 2 + geonode/services/tests.py | 27 ++++++++ geonode/services/views.py | 68 ++++++++++++++++--- 14 files changed, 213 insertions(+), 38 deletions(-) create mode 100644 geonode/people/migrations/0033_set_contributors_permissions.py create mode 100644 geonode/services/migrations/0044_auto_20210628_0955.py create mode 100644 geonode/services/migrations/0045_auto_20210629_1355.py diff --git a/geonode/base/populate_test_data.py b/geonode/base/populate_test_data.py index dd0e9f7690a..5b4ce63ca35 100644 --- a/geonode/base/populate_test_data.py +++ b/geonode/base/populate_test_data.py @@ -33,7 +33,6 @@ from django.contrib.auth.models import Permission, Group from django.core.serializers import serialize from django.contrib.auth import get_user_model -from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile from geonode import geoserver # noqa @@ -156,12 +155,7 @@ def create_models(type=None, integration=False): map_data, user_data, people_data, layer_data, document_data = create_fixtures() anonymous_group, created = Group.objects.get_or_create(name='anonymous') cont_group, created = Group.objects.get_or_create(name='contributors') - ctype = ContentType.objects.get_for_model(cont_group) - perm, created = Permission.objects.get_or_create( - codename='base_addresourcebase', - name='Can add resources', - content_type=ctype - ) + perm = Permission.objects.get(codename='add_resourcebase') cont_group.permissions.add(perm) logger.debug("[SetUp] Get or create user admin") u, created = get_user_model().objects.get_or_create(username='admin') diff --git a/geonode/base/templatetags/base_tags.py b/geonode/base/templatetags/base_tags.py index e5c630341a6..7fc059fd737 100644 --- a/geonode/base/templatetags/base_tags.py +++ b/geonode/base/templatetags/base_tags.py @@ -424,3 +424,10 @@ def display_change_perms_button(resource, user, perms): return True else: return not getattr(settings, 'ADMIN_MODERATE_UPLOADS', False) + +@register.simple_tag +def get_layer_count_by_services(service_id, user): + return get_visible_resources( + queryset=Layer.objects.filter(remote_service=service_id), + user=user + ).count() diff --git a/geonode/people/migrations/0033_set_contributors_permissions.py b/geonode/people/migrations/0033_set_contributors_permissions.py new file mode 100644 index 00000000000..5f01e398647 --- /dev/null +++ b/geonode/people/migrations/0033_set_contributors_permissions.py @@ -0,0 +1,37 @@ +# Assign the contributors group to users according to #7364 + +from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType +from django.db import migrations +from django.db.migrations.operations import RunPython +from geonode.base.models import ResourceBase + + +def assign_permissions_to_contributors(apps, schema_editor): + contributors = Group.objects.filter(name='contributors') + if contributors.exists(): + contr_obj = contributors.first() + perm, _ = Permission.objects.get_or_create( + name='Can add resources', + codename='add_resourcebase', + content_type=ContentType.objects.get_for_model(ResourceBase) + ) + contr_obj.permissions.add(perm) + perm_exists = contr_obj.permissions.filter(codename='base_addresourcebase') + if perm_exists.exists(): + contr_obj.permissions.remove(perm_exists.first()) + contr_obj.save() + perm_to_remove = Permission.objects.filter(codename='base_addresourcebase') + if perm_to_remove.exists(): + perm_to_remove.delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0032_set_contributors_group'), + ] + + operations = [ + RunPython(assign_permissions_to_contributors) + ] diff --git a/geonode/security/models.py b/geonode/security/models.py index 46d32588038..8c462e5218c 100644 --- a/geonode/security/models.py +++ b/geonode/security/models.py @@ -46,6 +46,7 @@ ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS, VIEW_PERMISSIONS, + SERVICE_PERMISSIONS ) from .utils import ( @@ -435,7 +436,7 @@ def get_user_perms(self, user): config = Configuration.load() ctype = ContentType.objects.get_for_model(self) - PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS + PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS + SERVICE_PERMISSIONS resource_perms = Permission.objects.filter( codename__in=PERMISSIONS_TO_FETCH, diff --git a/geonode/security/permissions.py b/geonode/security/permissions.py index 2191bd0ef63..fe65b63f73c 100644 --- a/geonode/security/permissions.py +++ b/geonode/security/permissions.py @@ -1,6 +1,6 @@ # Permissions mapping PERMISSIONS = { - 'base_addresourcebase': 'add_resource', + 'add_resourcebase': 'add_resource', } # The following permissions will be filtered out when READ_ONLY mode is active @@ -26,3 +26,10 @@ 'change_layer_data', 'change_layer_style' ] + +SERVICE_PERMISSIONS = [ + "add_service", + "delete_service", + "change_resourcebase_metadata", + "add_resourcebase_from_service" +] diff --git a/geonode/security/utils.py b/geonode/security/utils.py index 07f9748c804..ea158b9c038 100644 --- a/geonode/security/utils.py +++ b/geonode/security/utils.py @@ -113,10 +113,10 @@ def get_users_with_perms(obj): """ Override of the Guardian get_users_with_perms """ - from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) + from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS, SERVICE_PERMISSIONS) ctype = ContentType.objects.get_for_model(obj) permissions = {} - PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS + PERMISSIONS_TO_FETCH = VIEW_PERMISSIONS + ADMIN_PERMISSIONS + LAYER_ADMIN_PERMISSIONS + SERVICE_PERMISSIONS for perm in Permission.objects.filter(codename__in=PERMISSIONS_TO_FETCH, content_type_id=ctype.id): permissions[perm.id] = perm.codename @@ -654,7 +654,7 @@ def _get_gf_services(layer, perms): def set_owner_permissions(resource, members=None): """assign all admin permissions to the owner""" - from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS) + from .permissions import (VIEW_PERMISSIONS, ADMIN_PERMISSIONS, LAYER_ADMIN_PERMISSIONS, SERVICE_PERMISSIONS) if resource.polymorphic_ctype: # Owner & Manager Admin Perms admin_perms = VIEW_PERMISSIONS + ADMIN_PERMISSIONS @@ -675,6 +675,13 @@ def set_owner_permissions(resource, members=None): for user in members: assign_perm(perm, user, resource.layer) + if resource.polymorphic_ctype.name == 'service': + for perm in SERVICE_PERMISSIONS: + assign_perm(perm, resource.owner, resource.service) + if members: + for user in members: + assign_perm(perm, user, resource.service) + def remove_object_permissions(instance): """Remove object permissions on given resource. diff --git a/geonode/services/migrations/0044_auto_20210628_0955.py b/geonode/services/migrations/0044_auto_20210628_0955.py new file mode 100644 index 00000000000..fe244034fcd --- /dev/null +++ b/geonode/services/migrations/0044_auto_20210628_0955.py @@ -0,0 +1,17 @@ +# Generated by Django 2.2.20 on 2021-06-28 09:55 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('services', '0043_auto_20210519_1308'), + ] + + operations = [ + migrations.AlterModelOptions( + name='service', + options={'permissions': (('add_resourcebase_from_service', 'Can add resources to Service'),)}, + ), + ] diff --git a/geonode/services/migrations/0045_auto_20210629_1355.py b/geonode/services/migrations/0045_auto_20210629_1355.py new file mode 100644 index 00000000000..9aa1f7ffd99 --- /dev/null +++ b/geonode/services/migrations/0045_auto_20210629_1355.py @@ -0,0 +1,17 @@ +# Generated by Django 2.2.20 on 2021-06-29 13:55 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('services', '0044_auto_20210628_0955'), + ] + + operations = [ + migrations.AlterModelOptions( + name='service', + options={'permissions': (('add_resourcebase_from_service', 'Can add resources to Service'), ('change_resourcebase_metadata', 'Can change resources metadata'))}, + ), + ] diff --git a/geonode/services/models.py b/geonode/services/models.py index 198b39c8a80..2578f24194b 100644 --- a/geonode/services/models.py +++ b/geonode/services/models.py @@ -181,6 +181,14 @@ def probe_service(self): except Exception: return 404 + class Meta: + # custom permissions, + # change and delete are standard in django-guardian + permissions = ( + ('add_resourcebase_from_service', 'Can add resources to Service'), + ('change_resourcebase_metadata', 'Can change resources metadata'), + ) + class ServiceProfileRole(models.Model): diff --git a/geonode/services/templates/services/service_detail.html b/geonode/services/templates/services/service_detail.html index 2dea724cc24..3df1de23add 100644 --- a/geonode/services/templates/services/service_detail.html +++ b/geonode/services/templates/services/service_detail.html @@ -55,12 +55,6 @@

{% trans "Service Resources" %} {{ total_resources }} {% endfor %} - {% for service in services %} - - {{service.title|striptags}} - {{service.abstract|striptags}} - - {% endfor %} {% endif %} @@ -104,22 +98,22 @@

{% trans "Service Resources" %} {{ total_resources }}

  • {% trans "Manage" %}

    • - - {% comment %}{% if "change_service" in resource_perms %}{% endcomment %} + {% if "change_resourcebase_metadata" in resource_perms %}
  • {% trans "Edit Service Metadata" %}
    • + {% endif %} + {% if can_add_resorces %}
  • {% trans "Import Service Resources" %}
    • - {% comment %}{% endif %}{% endcomment %} - {% comment %}{% if "remove_service" or "delete_service" in resource_perms %}{% endcomment %} + {% endif %} + {% if "remove_service" in permissions_list or "delete_service" in permissions_list or 'delete_resourcebase' in permissions_list %}
  • {% trans "Remove Service" %}
    • - {% comment %}{% endif %}{% endcomment %} + {% endif %} - {% comment %}{% endif %}{% endcomment %} + {% endif %} {% endblock %} {% block extra_script %} {{ block.super }} diff --git a/geonode/services/templates/services/service_list.html b/geonode/services/templates/services/service_list.html index ddb00650769..d45d2d391cf 100644 --- a/geonode/services/templates/services/service_list.html +++ b/geonode/services/templates/services/service_list.html @@ -1,11 +1,12 @@ {% extends "services/services_base.html" %} {% load i18n %} +{% load base_tags %} {% block title %} {% trans "Services" %} -- {{ block.super }} {% endblock %} {% block body_outer %}
    - {% if user.is_authenticated and not READ_ONLY_MODE %} + {% if user.is_authenticated and not READ_ONLY_MODE and can_add_resources %} {% trans "Add Remote Service" %} {% endif %}

    {% trans "Remote Services" %}

    @@ -20,9 +21,13 @@

    {% trans "Remote Services" %}

    {% for service in services %} - {{ service.title }} {{ service.layer_set.count }} - {{ service.base_url }} - {{ service.service_type }} + + {{ service.title }} {% get_layer_count_by_services service.id request.user %} + + + {{ service.base_url }} + + {{ service.service_type }} {% endfor %} diff --git a/geonode/services/templates/services/service_resources_harvest.html b/geonode/services/templates/services/service_resources_harvest.html index 7195485e9bc..22fac47b973 100644 --- a/geonode/services/templates/services/service_resources_harvest.html +++ b/geonode/services/templates/services/service_resources_harvest.html @@ -75,7 +75,9 @@

    {% trans "Import resources" %} {% trans "Back to service details" %} + {% if can_add_resources or "add_resourcebase_from_service" in resource_perms or 'add_resourcebase_from_service' in permissions_list %} + {% endif %}

    diff --git a/geonode/services/tests.py b/geonode/services/tests.py index 8f0d8884430..3081cd10ea0 100644 --- a/geonode/services/tests.py +++ b/geonode/services/tests.py @@ -18,6 +18,7 @@ # ######################################################################### import logging +from geonode.services.enumerations import WMS, INDEXED from django.contrib.staticfiles.testing import StaticLiveServerTestCase from django.test import Client from selenium import webdriver @@ -882,3 +883,29 @@ def test_harvest_resources(self): self.selenium.find_element_by_id('btn-id-filter').click() # self.selenium.find_element_by_id('option_atlantis:tiger_roads_tiger_roads').click() # self.selenium.find_element_by_tag_name('form').submit() + + +class TestServiceViews(GeoNodeBaseTestSupport): + def setUp(self): + self.user = 'admin' + self.passwd = 'admin' + self.admin = get_user_model().objects.get(username='admin') + self.sut, _ = Service.objects.get_or_create( + type=WMS, + name='Bogus', + title='Pocus', + owner=self.admin, + method=INDEXED, + metadata_only=True, + base_url='http://bogus.pocus.com/ows') + self.sut.clear_dirty_state() + + def test_user_admin_can_access_to_page(self): + self.client.login(username='admin', password='admin') + response = self.client.get(reverse('services')) + self.assertEqual(response.status_code, 200) + + def test_invalid_user_cannot_access_to_page(self): + self.client.login(username='bobby', password='bobby') + response = self.client.get(reverse('services')) + self.assertEqual(response.status_code, 302) diff --git a/geonode/services/views.py b/geonode/services/views.py index 2c75d67b3cb..5412c5369e0 100644 --- a/geonode/services/views.py +++ b/geonode/services/views.py @@ -18,6 +18,8 @@ # ######################################################################### +from guardian.shortcuts import assign_perm +from geonode.security.utils import get_visible_resources import logging from django.conf import settings @@ -64,10 +66,14 @@ def service_proxy(request, service_id): def services(request): """This view shows the list of all registered services""" + return render( request, "services/service_list.html", - {"services": Service.objects.all()} + { + "services": Service.objects.all(), + "can_add_resources": request.user.has_perm('base.add_resourcebase') + } ) @@ -86,7 +92,7 @@ def register_service(request): raise Http404(str(e)) service.save() service.keywords.add(*service_handler.get_keywords()) - service.set_default_permissions() + if service_handler.indexing_method == enumerations.CASCADED: service_handler.create_cascaded_store() request.session[service_handler.url] = service_handler @@ -128,6 +134,13 @@ def harvest_resources_handle_get(request, service, handler): available_resources = handler.get_resources() is_sync = getattr(settings, "CELERY_TASK_ALWAYS_EAGER", False) errored_state = False + _ = _perms_info_json(service) + + perms_list = list( + service.get_self_resource().get_user_perms(request.user) + .union(service.get_user_perms(request.user)) + ) + already_harvested = HarvestJob.objects.values_list( "resource_id", flat=True).filter(service=service, status=enumerations.PROCESSED) if available_resources: @@ -161,7 +174,10 @@ def harvest_resources_handle_get(request, service, handler): "requested": request.GET.getlist("resource_list"), "is_sync": is_sync, "errored_state": errored_state, + "can_add_resources": request.user.has_perm('base.add_resourcebase'), "filter_row": filter_row, + "permissions_list": perms_list + } ) return result @@ -187,6 +203,16 @@ def harvest_resources_handle_post(request, service, handler): else: logger.warning( f"resource {id} already has a harvest job") + # assign permission of the resource to the user + perms = [ + 'view_resourcebase', 'download_resourcebase', + 'change_resourcebase_metadata', 'change_resourcebase', + 'delete_resourcebase' + ] + layer = Layer.objects.filter(alternate=id) + if layer.exists(): + for perm in perms: + assign_perm(perm, request.user, layer.first().get_self_resource()) msg_async = _("The selected resources are being imported") msg_sync = _("The selected resources have been imported") messages.add_message( @@ -204,6 +230,7 @@ def harvest_resources_handle_post(request, service, handler): @login_required def harvest_resources(request, service_id): + service = get_object_or_404(Service, pk=service_id) try: handler = request.session[service.base_url] @@ -274,12 +301,36 @@ def rescan_service(request, service_id): @login_required def service_detail(request, service_id): """This view shows the details of a service""" - service = get_object_or_404(Service, pk=service_id) + + services = Service.objects.filter(resourcebase_ptr_id=service_id) + + if not services.exists(): + messages.add_message( + request, + messages.ERROR, + _("You dont have enougth rigths to see the resource detail") + ) + return redirect( + reverse("services") + ) + service = services.first() + + permissions_json = _perms_info_json(service) + + perms_list = list( + service.get_self_resource().get_user_perms(request.user) + .union(service.get_user_perms(request.user)) + ) + + already_imported_layers = get_visible_resources( + queryset=Layer.objects.filter(remote_service=service), + user=request.user + ) resources_being_harvested = HarvestJob.objects.filter(service=service) - already_imported_layers = Layer.objects.filter(remote_service=service) + service_list = service.service_set.all() - all_resources = (list(resources_being_harvested) + - list(already_imported_layers) + list(service_list)) + all_resources = (list(resources_being_harvested) + list(already_imported_layers) + list(service_list)) + paginator = Paginator( all_resources, getattr(settings, "CLIENT_RESULTS_LIMIT", 25), @@ -309,10 +360,11 @@ def service_detail(request, service_id): context={ "service": service, "layers": already_imported_layers, - "services": (r for r in resources if isinstance(r, Service)), "resource_jobs": ( r for r in resources if isinstance(r, HarvestJob)), - "permissions_json": _perms_info_json(service), + "permissions_json": permissions_json, + "permissions_list": perms_list, + "can_add_resorces": request.user.has_perm('base.add_resourcebase'), "resources": resources, "total_resources": len(already_imported_layers), }