[Fixes #5779] Data edition permissions set in GeoNode for a layer are…

… not applied on the WFS (#6641)

geonode/proxy/views.py

@@ -183,12 +183,13 @@ def proxy(request, url=None, response_callback=None,
             '%s%s' % (settings.SITEURL, 'geoserver'),
             ogc_server_settings.LOCATION.rstrip('/'))
 
-    response, content = http_client.request(_url,
-                                            method=request.method,
-                                            data=_data,
-                                            headers=headers,
-                                            timeout=timeout,
-                                            user=request.user)
+    response, content = http_client.request(
+        _url,
+        method=request.method,
+        data=_data,
+        headers=headers,
+        timeout=timeout,
+        user=request.user)
     content = response.content or response.reason
     status = response.status_code
     content_type = response.headers.get('Content-Type')

geonode/security/models.py

@@ -300,7 +300,10 @@ def set_permissions(self, perm_spec, created=False):
                     # Set the GeoFence Rules
                     if settings.OGC_SERVER['default'].get("GEOFENCE_SECURITY_ENABLED", False):
                         if self.polymorphic_ctype.name == 'layer':
-                            sync_geofence_with_guardian(self.layer, perms, user=user)
+                            group_perms = None
+                            if 'groups' in perm_spec and len(perm_spec['groups']) > 0:
+                                group_perms = perm_spec['groups']
+                            sync_geofence_with_guardian(self.layer, perms, user=_user, group_perms=group_perms)
 
         # All the other groups
         if 'groups' in perm_spec and len(perm_spec['groups']) > 0:

geonode/security/tests.py

@@ -420,7 +420,7 @@ def test_perm_specs_synchronization(self):
             3. Set permissions to a group of users
             4. Try to sync a layer from GeoServer
         """
-        layer = Layer.objects.first()
+        layer = Layer.objects.filter(storeType='dataStore').first()
         self.client.login(username='admin', password='admin')
 
         # Reset GeoFence Rules
@@ -447,6 +447,86 @@ def test_perm_specs_synchronization(self):
         _log("3. geofence_rules_count: %s " % geofence_rules_count)
         self.assertEqual(geofence_rules_count, 7)
 
+        # FULL WFS-T
+        perm_spec = {
+            'users': {
+                'bobby': [
+                    'view_resourcebase',
+                    'download_resourcebase',
+                    'change_layer_style',
+                    'change_layer_data']
+            },
+            'groups': []
+        }
+        layer.set_permissions(perm_spec)
+        geofence_rules_count = get_geofence_rules_count()
+        self.assertEqual(geofence_rules_count, 10)
+
+        rules_objs = get_geofence_rules(entries=10)
+        _deny_wfst_rule_exists = False
+        for rule in rules_objs['rules']:
+            if rule['service'] == "WFS" and \
+            rule['userName'] == 'bobby' and \
+            rule['request'] == "TRANSACTION":
+                _deny_wfst_rule_exists = rule['access'] == 'DENY'
+                break
+        self.assertFalse(_deny_wfst_rule_exists)
+
+        # NO WFS-T
+        # - order it important
+        perm_spec = {
+            'users': {
+                'bobby': [
+                    'view_resourcebase',
+                    'download_resourcebase',
+                ]
+            },
+            'groups': []
+        }
+        layer.set_permissions(perm_spec)
+        geofence_rules_count = get_geofence_rules_count()
+        self.assertEqual(geofence_rules_count, 13)
+
+        rules_objs = get_geofence_rules(entries=13)
+        _deny_wfst_rule_exists = False
+        _deny_wfst_rule_position = -1
+        _allow_wfs_rule_position = -1
+        for cnt, rule in enumerate(rules_objs['rules']):
+            if rule['service'] == "WFS" and \
+            rule['userName'] == 'bobby' and \
+            rule['request'] == "TRANSACTION":
+                _deny_wfst_rule_exists = rule['access'] == 'DENY'
+                _deny_wfst_rule_position = cnt
+            elif rule['service'] == "WFS" and \
+            rule['userName'] == 'bobby' and \
+            (rule['request'] is None or rule['request'] == '*'):
+                _allow_wfs_rule_position = cnt
+        self.assertTrue(_deny_wfst_rule_exists)
+        self.assertTrue(_allow_wfs_rule_position > _deny_wfst_rule_position)
+
+        # NO WFS
+        perm_spec = {
+            'users': {
+                'bobby': [
+                    'view_resourcebase',
+                ]
+            },
+            'groups': []
+        }
+        layer.set_permissions(perm_spec)
+        geofence_rules_count = get_geofence_rules_count()
+        self.assertEqual(geofence_rules_count, 7)
+
+        rules_objs = get_geofence_rules(entries=7)
+        _deny_wfst_rule_exists = False
+        for rule in rules_objs['rules']:
+            if rule['service'] == "WFS" and \
+            rule['userName'] == 'bobby' and \
+            rule['request'] == "TRANSACTION":
+                _deny_wfst_rule_exists = rule['access'] == 'DENY'
+                break
+        self.assertFalse(_deny_wfst_rule_exists)
+
         perm_spec = {'users': {}, 'groups': {'bar': ['view_resourcebase']}}
         layer.set_permissions(perm_spec)
         geofence_rules_count = get_geofence_rules_count()
@@ -460,6 +540,11 @@ def test_perm_specs_synchronization(self):
         self.assertEqual(geofence_rules_count, 5)
 
         # Testing GeoLimits
+        # Reset GeoFence Rules
+        purge_geofence_all()
+        geofence_rules_count = get_geofence_rules_count()
+        self.assertEqual(geofence_rules_count, 0)
+        layer = Layer.objects.first()
         # grab bobby
         bobby = get_user_model().objects.get(username="bobby")
 
@@ -482,7 +567,9 @@ def test_perm_specs_synchronization(self):
 
         rules_objs = get_geofence_rules(entries=5)
         self.assertEqual(len(rules_objs['rules']), 5)
-        for rule in rules_objs['rules']:
+        # Order is important
+        _limit_rule_position = -1
+        for cnt, rule in enumerate(rules_objs['rules']):
             if rule['service'] is None:
                 self.assertEqual(rule['userName'], 'bobby')
                 self.assertEqual(rule['workspace'], 'CA')
@@ -495,6 +582,10 @@ def test_perm_specs_synchronization(self):
 146.7000276171853 -42.53655428642583, 146.7110139453067 -43.07256577359489, \
 145.9804231249952 -43.05651288026286, 145.8046418749977 -42.49606500060302)))')
                 self.assertEqual(rule_limits['catalogMode'], 'MIXED')
+                _limit_rule_position = cnt
+            elif rule['userName'] == 'bobby':
+                # When there's a limit rule, "*" must be the first one
+                self.assertTrue(_limit_rule_position < cnt)
 
         geo_limit, _ = GroupGeoLimit.objects.get_or_create(
             group=GroupProfile.objects.get(group__name='bar'),
@@ -515,20 +606,26 @@ def test_perm_specs_synchronization(self):
 
         rules_objs = get_geofence_rules(entries=6)
         self.assertEqual(len(rules_objs['rules']), 6)
-        for rule in rules_objs['rules']:
+        # Order is important
+        _limit_rule_position = -1
+        for cnt, rule in enumerate(rules_objs['rules']):
             if rule['roleName'] == 'ROLE_BAR':
-                self.assertEqual(rule['service'], None)
-                self.assertEqual(rule['userName'], None)
-                self.assertEqual(rule['workspace'], 'CA')
-                self.assertEqual(rule['layer'], 'CA')
-                self.assertEqual(rule['access'], 'LIMIT')
-
-                self.assertTrue('limits' in rule)
-                rule_limits = rule['limits']
-                self.assertEqual(rule_limits['allowedArea'], 'MULTIPOLYGON (((145.8046418749977 -42.49606500060302, \
+                if rule['service'] is None:
+                    self.assertEqual(rule['userName'], None)
+                    self.assertEqual(rule['workspace'], 'CA')
+                    self.assertEqual(rule['layer'], 'CA')
+                    self.assertEqual(rule['access'], 'LIMIT')
+
+                    self.assertTrue('limits' in rule)
+                    rule_limits = rule['limits']
+                    self.assertEqual(rule_limits['allowedArea'], 'MULTIPOLYGON (((145.8046418749977 -42.49606500060302, \
 146.7000276171853 -42.53655428642583, 146.7110139453067 -43.07256577359489, \
 145.9804231249952 -43.05651288026286, 145.8046418749977 -42.49606500060302)))')
-                self.assertEqual(rule_limits['catalogMode'], 'MIXED')
+                    self.assertEqual(rule_limits['catalogMode'], 'MIXED')
+                    _limit_rule_position = cnt
+                else:
+                    # When there's a limit rule, "*" must be the first one
+                    self.assertTrue(_limit_rule_position < cnt)
 
         # Change Layer Type and SRID in order to force GeoFence allowed-area reprojection
         _original_storeType = layer.storeType
@@ -543,21 +640,28 @@ def test_perm_specs_synchronization(self):
 
         rules_objs = get_geofence_rules(entries=6)
         self.assertEqual(len(rules_objs['rules']), 6)
-        for rule in rules_objs['rules']:
+        # Order is important
+        _limit_rule_position = -1
+        for cnt, rule in enumerate(rules_objs['rules']):
             if rule['roleName'] == 'ROLE_BAR':
-                self.assertEqual(rule['service'], None)
-                self.assertEqual(rule['userName'], None)
-                self.assertEqual(rule['workspace'], 'CA')
-                self.assertEqual(rule['layer'], 'CA')
-                self.assertEqual(rule['access'], 'LIMIT')
-
-                self.assertTrue('limits' in rule)
-                rule_limits = rule['limits']
-                self.assertEqual(
-                    rule_limits['allowedArea'], 'MULTIPOLYGON (((145.8046418749977 -42.49606500060302, 146.7000276171853 \
+                if rule['service'] is None:
+                    self.assertEqual(rule['service'], None)
+                    self.assertEqual(rule['userName'], None)
+                    self.assertEqual(rule['workspace'], 'CA')
+                    self.assertEqual(rule['layer'], 'CA')
+                    self.assertEqual(rule['access'], 'LIMIT')
+
+                    self.assertTrue('limits' in rule)
+                    rule_limits = rule['limits']
+                    self.assertEqual(
+                        rule_limits['allowedArea'], 'MULTIPOLYGON (((145.8046418749977 -42.49606500060302, 146.7000276171853 \
 -42.53655428642583, 146.7110139453067 -43.07256577359489, 145.9804231249952 \
 -43.05651288026286, 145.8046418749977 -42.49606500060302)))')
-                self.assertEqual(rule_limits['catalogMode'], 'MIXED')
+                    self.assertEqual(rule_limits['catalogMode'], 'MIXED')
+                    _limit_rule_position = cnt
+                else:
+                    # When there's a limit rule, "*" must be the first one
+                    self.assertTrue(_limit_rule_position < cnt)
 
         layer.storeType = _original_storeType
         layer.srid = _original_srid

geonode/security/utils.py

@@ -532,24 +532,41 @@ def set_geofence_all(instance):
 
 
 @on_ogc_backend(geoserver.BACKEND_PACKAGE)
-def sync_geofence_with_guardian(layer, perms, user=None, group=None):
+def sync_geofence_with_guardian(layer, perms, user=None, group=None, group_perms=None):
     """
     Sync Guardian permissions to GeoFence.
     """
     _layer_name = layer.name if layer and hasattr(layer, 'name') else layer.alternate.split(":")[0]
     _layer_workspace = get_layer_workspace(layer)
     # Create new rule-set
     gf_services = {}
-    gf_services["*"] = 'download_resourcebase' in perms and \
-        ('view_resourcebase' in perms or 'change_layer_style' in perms)
     gf_services["WMS"] = 'view_resourcebase' in perms or 'change_layer_style' in perms
     gf_services["GWC"] = 'view_resourcebase' in perms or 'change_layer_style' in perms
     gf_services["WFS"] = ('download_resourcebase' in perms or 'change_layer_data' in perms) \
         and layer.is_vector()
     gf_services["WCS"] = ('download_resourcebase' in perms or 'change_layer_data' in perms) \
         and not layer.is_vector()
     gf_services["WPS"] = 'download_resourcebase' in perms or 'change_layer_data' in perms
+    gf_services["*"] = 'download_resourcebase' in perms and \
+        ('view_resourcebase' in perms or 'change_layer_style' in perms)
 
+    gf_requests = {}
+    if 'change_layer_data' not in perms:
+        _skip_perm = False
+        if user and group_perms:
+            if isinstance(user, string_types):
+                user = get_user_model().objects.get(username=user)
+            user_groups = list(user.groups.all().values_list('name', flat=True))
+            for _group, _perm in group_perms.items():
+                if 'change_layer_data' in _perm and _group in user_groups:
+                    _skip_perm = True
+                    break
+        if not _skip_perm:
+            gf_requests["WFS"] = {
+                "TRANSACTION": False,
+                "LOCKFEATURE": False,
+                "GETFEATUREWITHLOCK": False
+            }
     _user = None
     _group = None
     _disable_layer_cache = False
@@ -579,6 +596,11 @@ def sync_geofence_with_guardian(layer, perms, user=None, group=None):
         # delete_layer_cache('{}:{}'.format(_layer_workspace, _layer_name))
         filters = None
         formats = None
+        # Re-order dictionary
+        # - if geo-limits have been defined for this user/group, the "*" rule must be the first one
+        gf_services_limits_first = {"*": gf_services.pop('*')}
+        gf_services_limits_first.update(gf_services)
+        gf_services = gf_services_limits_first
     else:
         filters = [{
             "styleParameterFilter": {
@@ -602,18 +624,30 @@ def sync_geofence_with_guardian(layer, perms, user=None, group=None):
                 _wkt = None
                 if users_geolimits and users_geolimits.count():
                     _wkt = users_geolimits.last().wkt
+                if service in gf_requests:
+                    for request, enabled in gf_requests[service].items():
+                        _update_geofence_rule(layer, _layer_name, _layer_workspace,
+                                              service, request=request, user=_user, allow=enabled)
                 _update_geofence_rule(layer, _layer_name, _layer_workspace, service, user=_user, geo_limit=_wkt)
             elif not _group:
                 logger.debug("Adding to geofence the rule: %s %s *" % (layer, service))
                 _wkt = None
                 if anonymous_geolimits and anonymous_geolimits.count():
                     _wkt = anonymous_geolimits.last().wkt
+                if service in gf_requests:
+                    for request, enabled in gf_requests[service].items():
+                        _update_geofence_rule(layer, _layer_name, _layer_workspace,
+                                              service, request=request, user=_user, allow=enabled)
                 _update_geofence_rule(layer, _layer_name, _layer_workspace, service, geo_limit=_wkt)
             if _group:
                 logger.debug("Adding 'group' to geofence the rule: %s %s %s" % (layer, service, _group))
                 _wkt = None
                 if groups_geolimits and groups_geolimits.count():
                     _wkt = groups_geolimits.last().wkt
+                if service in gf_requests:
+                    for request, enabled in gf_requests[service].items():
+                        _update_geofence_rule(layer, _layer_name, _layer_workspace,
+                                              service, request=request, group=_group, allow=enabled)
                 _update_geofence_rule(layer, _layer_name, _layer_workspace, service, group=_group, geo_limit=_wkt)
     if not getattr(settings, 'DELAYED_SECURITY_SIGNALS', False):
         set_geofence_invalidate_cache()
@@ -682,7 +716,7 @@ def remove_object_permissions(instance):
 
 
 def _get_geofence_payload(layer, layer_name, workspace, access, user=None, group=None,
-                          service=None, geo_limit=None):
+                          service=None, request=None, geo_limit=None):
     highest_priority = get_highest_priority()
     root_el = etree.Element("Rule")
     username_el = etree.SubElement(root_el, "userName")
@@ -702,6 +736,9 @@ def _get_geofence_payload(layer, layer_name, workspace, access, user=None, group
     if service is not None and service != "*":
         service_el = etree.SubElement(root_el, "service")
         service_el.text = service
+    if request is not None and request != "*":
+        service_el = etree.SubElement(root_el, "request")
+        service_el.text = request
     if service and service == "*" and geo_limit is not None and geo_limit != "":
         access_el = etree.SubElement(root_el, "access")
         access_el.text = "LIMIT"
@@ -716,15 +753,19 @@ def _get_geofence_payload(layer, layer_name, workspace, access, user=None, group
     return etree.tostring(root_el)
 
 
-def _update_geofence_rule(layer, layer_name, workspace, service, user=None, group=None, geo_limit=None):
+def _update_geofence_rule(layer, layer_name, workspace,
+                          service, request=None,
+                          user=None, group=None,
+                          geo_limit=None, allow=True):
     payload = _get_geofence_payload(
         layer=layer,
         layer_name=layer_name,
         workspace=workspace,
-        access="ALLOW",
+        access="ALLOW" if allow else "DENY",
         user=user,
         group=group,
         service=service,
+        request=request,
         geo_limit=geo_limit
     )
     logger.debug("request data: {}".format(payload))