[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 <[email protected]> (cherry picked from commit 0c71f23)
GeoNode · Jul 6, 2021 · 9e28a29 · 9e28a29
1 parent 701e68a
commit 9e28a29
Show file tree

Hide file tree

Showing 14 changed files with 213 additions and 38 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -55,12 +55,6 @@ <h3>{% trans "Service Resources" %} <span class="badge">{{ total_resources }}</s
                         {% endif %}
                     </tr>
                 {% endfor %}
-                {% for service in services %}
-                    <tr>
-                        <td><a href='{%  url "service_detail" service.id %}'>{{service.title|striptags}}</a></td>
-                        <td>{{service.abstract|striptags}}</td>
-                    </tr>
-                {% endfor %}
             </table>
             </div>
         {% endif %}
@@ -104,22 +98,22 @@ <h3>{% trans "Service Resources" %} <span class="badge">{{ total_resources }}</s
 {% endblock %}
 {% block sidebar %}
 
-
     {% get_obj_perms request.user for service as "resource_perms" %}
 
-    {% comment %}{% if "change_service" in resource_perms or "remove_service" in resource_perms or "change_service_permissions" in resource_perms %}{% endcomment %}
+    {% if can_add_resorces or "add_resourcebase" in resource_perms or "change_resourcebase_metadata" in resource_perms or "change_service_permissions" in resource_perms %}
     <ul class="list-group">
         <li class="list-group-item"><h3>{% trans "Manage" %}</h3></li>
-
-        {% comment %}{% if "change_service" in resource_perms %}{% endcomment %}
+        {% if "change_resourcebase_metadata" in resource_perms %}
             <li class="list-group-item"><a class="btn btn-default btn-md btn-block" href="{% url "edit_service" service.id %}">{% trans "Edit Service Metadata" %}</a></li>
+        {% endif %}
+        {% if can_add_resorces %}
             <li class="list-group-item"><a id="harvestResources" class="btn btn-default btn-md btn-block" href="{% url "harvest_resources" service.id %}">{% trans "Import Service Resources" %}</a></li>
-        {% 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 %}
             <li class="list-group-item"><a class="btn btn-default btn-md btn-block" href="{% url "remove_service" service.id %}">{% trans "Remove Service" %}</a></li>
-        {% comment %}{% endif %}{% endcomment %}
+        {% endif %}
       </ul>
-    {% 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
@@ -1,11 +1,12 @@
 {% extends "services/services_base.html" %}
 {% load i18n %}
+{% load base_tags %}
 
 {% block title %} {% trans "Services" %} -- {{ block.super }} {% endblock %}
 
 {% block body_outer %}
 <div class="page-header">
-  {% if user.is_authenticated and not READ_ONLY_MODE %}
+  {% if user.is_authenticated and not READ_ONLY_MODE and can_add_resources %}
     <a href="{% url "register_service" %}" class="btn btn-primary pull-right">{% trans "Add Remote Service" %}</a>
   {% endif %}
   <h2>{% trans "Remote Services" %}</h2>
@@ -20,9 +21,13 @@ <h2>{% trans "Remote Services" %}</h2>
   </thead>
   {% for service in services %}
   <tr>
-	<td><a href='{% url "service_detail" service.id %}'>{{ service.title }} <span class="badge">{{ service.layer_set.count }}</span></a></td>
-	<td><a href='{{ service.base_url }}' target="_blank" rel="noopener noreferrer">{{ service.base_url }}</a></td>
-    <td>{{ service.service_type }}</td>
+	  <td>
+      <a href='{% url "service_detail" service.id %}'>{{ service.title }} <span class="badge">{% get_layer_count_by_services service.id request.user %}</span></a>
+    </td>
+	  <td>
+      <a href='{{ service.base_url }}' target="_blank" rel="noopener noreferrer">{{ service.base_url }}</a>
+      </td>
+      <td>{{ service.service_type }}</td>
   </tr>
   {% endfor %}
 </table>

diff --git a/geonode/services/templates/services/service_resources_harvest.html b/geonode/services/templates/services/service_resources_harvest.html
@@ -75,7 +75,9 @@ <h3>{% trans "Import resources" %}
                                 <button id="btnClearFilter" type="button" class="btn btn-default">{% trans "Clear Filter" %}</button>
                                 <a class="btn btn-default"
                                    href="{% url "service_detail" service.id %}">{% trans "Back to service details" %}</a>
+                                {% if can_add_resources or "add_resourcebase_from_service" in resource_perms or 'add_resourcebase_from_service' in permissions_list %}
                                 <input type="submit" class="btn btn-primary" value="{% trans "Import Resources" %}"/>
+                                {% endif %}
                             </div>
                         </div>
                     </form>

diff --git 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)