Merge pull request #50497 from dmarteau/fix-FilterFeatures

Fix unwanted side effect with QgsAccessControl filter cache

python/server/auto_generated/qgsaccesscontrol.sip.in

@@ -42,8 +42,17 @@ Constructor
     void resolveFilterFeatures( const QList<QgsMapLayer *> &layers );
 %Docstring
 Resolve features' filter of layers
+The method fetch filter's expressions returned from access control plugins and
+and combine them to a unique expression for each layer.
+The resulted expressions are stored in cache for efficiency; between each requests, the cache
+must be cleared using ':py:func:`~QgsAccessControl.unresolveFilterFeatures`'.
 
 :param layers: to filter
+%End
+
+    void unresolveFilterFeatures();
+%Docstring
+Clear expression's cache computed from `resolveFilterFeatures`
 %End
 
     virtual void filterFeatures( const QgsVectorLayer *layer, QgsFeatureRequest &filterFeatures ) const;

src/server/qgsaccesscontrol.cpp

@@ -58,6 +58,13 @@ QString QgsAccessControl::resolveFilterFeatures( const QgsVectorLayer *layer ) c
   return expression;
 }
 
+//! Clear feature's filter of layers
+void QgsAccessControl::unresolveFilterFeatures()
+{
+  mFilterFeaturesExpressions.clear();
+  mResolved = false;
+}
+
 //! Filter the features of the layer
 void QgsAccessControl::filterFeatures( const QgsVectorLayer *layer, QgsFeatureRequest &featureRequest ) const
 {

src/server/qgsaccesscontrol.h

@@ -76,10 +76,20 @@ class SERVER_EXPORT QgsAccessControl : public QgsFeatureFilterProvider
 
     /**
      * Resolve features' filter of layers
+     * The method fetch filter's expressions returned from access control plugins and
+     * and combine them to a unique expression for each layer.
+     * The resulted expressions are stored in cache for efficiency; between each requests, the cache
+     * must be cleared using 'unresolveFilterFeatures()'.
+     *
      * \param layers to filter
      */
     void resolveFilterFeatures( const QList<QgsMapLayer *> &layers );
 
+    /**
+     *  Clear expression's cache computed from `resolveFilterFeatures`
+     */
+    void unresolveFilterFeatures();
+
     /**
      * Filter the features of the layer
      * \param layer the layer to control

src/server/qgsserver.cpp

@@ -399,6 +399,16 @@ void QgsServer::handleRequest( QgsServerRequest &request, QgsServerResponse &res
 
     response.clear();
 
+    // Clean up qgis access control filter's cache to prevent side effects
+    // across requests
+#ifdef HAVE_SERVER_PYTHON_PLUGINS
+    QgsAccessControl *accessControls = sServerInterface->accessControls();
+    if ( accessControls )
+    {
+      accessControls->unresolveFilterFeatures();
+    }
+#endif
+
     // Pass the filters to the requestHandler, this is needed for the following reasons:
     // Allow server request to call sendResponse plugin hook if enabled
     QgsFilterResponseDecorator responseDecorator( sServerInterface->filters(), response );

tests/src/python/CMakeLists.txt

@@ -530,6 +530,7 @@ if (WITH_SERVER)
   ADD_PYTHON_TEST(PyQgsServerAccessControlWFS test_qgsserver_accesscontrol_wfs.py)
   ADD_PYTHON_TEST(PyQgsServerAccessControlWCS test_qgsserver_accesscontrol_wcs.py)
   ADD_PYTHON_TEST(PyQgsServerAccessControlWFSTransactional test_qgsserver_accesscontrol_wfs_transactional.py)
+  ADD_PYTHON_TEST(PyQgsServerAccessControlFixFiltersCache test_qgsserver_accesscontrol_fix_filters.py)
   ADD_PYTHON_TEST(PyQgsServerCacheManager test_qgsserver_cachemanager.py)
   ADD_PYTHON_TEST(PyQgsServerWMTS test_qgsserver_wmts.py)
   ADD_PYTHON_TEST(PyQgsServerWFS test_qgsserver_wfs.py)

tests/src/python/test_qgsserver_accesscontrol_fix_filters.py

@@ -0,0 +1,70 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for QgsServer.
+
+From build dir, run: ctest -R PyQgsServerAccessControlWFS -V
+
+.. note:: This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+"""
+__author__ = 'David Marteau'
+__date__ = '10/09/2022'
+__copyright__ = 'Copyright 2022, The QGIS Project'
+
+from qgis.testing import unittest
+import urllib.request
+import urllib.parse
+import urllib.error
+from test_qgsserver_accesscontrol import TestQgsServerAccessControl, XML_NS
+
+
+class TestQgsServerAccessControlFixFilters(TestQgsServerAccessControl):
+
+    def test_wfs_getfeature_fix_feature_filters(self):
+        wfs_query_string = "&".join(["%s=%s" % i for i in list({
+            "MAP": urllib.parse.quote(self.projectPath),
+            "SERVICE": "WFS",
+            "VERSION": "1.0.0",
+            "REQUEST": "GetFeature",
+            "TYPENAME": "Hello_Filter",
+            "EXP_FILTER": "pkuid = 1"
+        }.items())])
+
+        wms_query_string = "&".join(["%s=%s" % i for i in list({
+            "MAP": urllib.parse.quote(self.projectPath),
+            "SERVICE": "WMS",
+            "VERSION": "1.1.1",
+            "REQUEST": "GetMap",
+            "FORMAT": "image/png",
+            "LAYERS": "Hello_Filter",
+            "BBOX": "-16817707,-6318936.5,5696513,16195283.5",
+            "HEIGHT": "500",
+            "WIDTH": "500",
+            "SRS": "EPSG:3857"
+        }.items())])
+
+        # Execute an unrestricted wfs request
+        response, headers = self._get_fullaccess(wfs_query_string)
+        self.assertTrue(
+            str(response).find("<qgs:pk>1</qgs:pk>") != -1,
+            "No result in GetFeature\n%s" % response)
+
+        # Execute a restricted WMS request
+        # That will store the filter expression in cache
+        response, headers = self._get_restricted(wms_query_string)
+        self.assertTrue(headers.get("Content-Type") == "image/png")
+
+        # Execute an unrestricted wfs request again
+        # We must have same result as the first time
+        #
+        # This test will fail if we do not clear the filter's cache
+        # before each requests.
+        response, headers = self._get_fullaccess(wfs_query_string)
+        self.assertTrue(
+            str(response).find("<qgs:pk>1</qgs:pk>") != -1,
+            "No result in GetFeature\n%s" % response)
+
+
+if __name__ == "__main__":
+    unittest.main()