diff --git a/src/server/services/wms/qgswmsrendercontext.cpp b/src/server/services/wms/qgswmsrendercontext.cpp index e58a1527704..aad8f3ad9ab 100644 --- a/src/server/services/wms/qgswmsrendercontext.cpp +++ b/src/server/services/wms/qgswmsrendercontext.cpp @@ -47,11 +47,21 @@ void QgsWmsRenderContext::setParameters( const QgsWmsParameters ¶meters ) searchLayersToRender(); removeUnwantedLayers(); - checkLayerReadPermissions(); std::reverse( mLayersToRender.begin(), mLayersToRender.end() ); } +bool QgsWmsRenderContext::addLayerToRender( QgsMapLayer *layer ) +{ + const bool allowed = checkLayerReadPermissions( layer ); + if ( allowed ) + { + mLayersToRender.append( layer ); + } + return allowed; +} + + void QgsWmsRenderContext::setFlag( const Flag flag, const bool on ) { if ( on ) @@ -186,6 +196,7 @@ qreal QgsWmsRenderContext::dotsPerMm() const QStringList QgsWmsRenderContext::flattenedQueryLayers( const QStringList &layerNames ) const { + QStringList result; std::function findLeaves = [ & ]( const QString & name ) -> QStringList { @@ -335,7 +346,7 @@ void QgsWmsRenderContext::initLayerGroupsRecursive( const QgsLayerTreeGroup *gro { if ( !groupName.isEmpty() ) { - mLayerGroups[groupName] = QList(); + QList layerGroup; const auto projectLayerTreeRoot { mProject->layerTreeRoot() }; const auto treeGroupLayers { group->findLayers() }; // Fast track if there is no custom layer order, @@ -344,7 +355,11 @@ void QgsWmsRenderContext::initLayerGroupsRecursive( const QgsLayerTreeGroup *gro { for ( const auto &tl : treeGroupLayers ) { - mLayerGroups[groupName].push_back( tl->layer() ); + auto layer = tl->layer(); + if ( checkLayerReadPermissions( layer ) ) + { + layerGroup.push_back( layer ); + } } } else @@ -354,16 +369,25 @@ void QgsWmsRenderContext::initLayerGroupsRecursive( const QgsLayerTreeGroup *gro QList groupLayersList; for ( const auto &tl : treeGroupLayers ) { - groupLayersList << tl->layer(); + auto layer = tl->layer(); + if ( checkLayerReadPermissions( layer ) ) + { + groupLayersList << layer; + } } for ( const auto &l : projectLayerOrder ) { if ( groupLayersList.contains( l ) ) { - mLayerGroups[groupName].push_back( l ); + layerGroup.push_back( l ); } } } + + if ( !layerGroup.empty() ) + { + mLayerGroups[groupName] = layerGroup; + } } for ( const QgsLayerTreeNode *child : group->children() ) @@ -442,10 +466,16 @@ void QgsWmsRenderContext::searchLayersToRender() { const QList layers = mNicknameLayers.values( layerName ); for ( QgsMapLayer *lyr : layers ) + { if ( !mLayersToRender.contains( lyr ) ) { - mLayersToRender.append( lyr ); + if ( !addLayerToRender( lyr ) ) + { + throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( lyr->name() ) ); + + } } + } } } @@ -456,10 +486,16 @@ void QgsWmsRenderContext::searchLayersToRender() { const QList layers = mNicknameLayers.values( layerName ); for ( QgsMapLayer *lyr : layers ) + { if ( !mLayersToRender.contains( lyr ) ) { - mLayersToRender.append( lyr ); + if ( !addLayerToRender( lyr ) ) + { + throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( lyr->name() ) ); + + } } + } } } } @@ -495,7 +531,13 @@ void QgsWmsRenderContext::searchLayersToRenderSld() if ( mNicknameLayers.contains( lname ) ) { mSlds[lname] = namedElem; - mLayersToRender.append( mNicknameLayers.values( lname ) ); + for ( const auto layer : mNicknameLayers.values( lname ) ) + { + if ( !addLayerToRender( layer ) ) + { + throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( layer->name() ) ); + } + } } else if ( mLayerGroups.contains( lname ) ) { @@ -540,7 +582,12 @@ void QgsWmsRenderContext::searchLayersToRenderStyle() { // to delete later mExternalLayers.append( layer.release() ); - mLayersToRender.append( mExternalLayers.last() ); + auto lyr = mExternalLayers.last(); + if ( !addLayerToRender( lyr ) ) + { + throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( lyr->name() ) ); + + } } } else if ( mNicknameLayers.contains( nickname ) ) @@ -550,7 +597,13 @@ void QgsWmsRenderContext::searchLayersToRenderStyle() mStyles[nickname] = style; } - mLayersToRender.append( mNicknameLayers.values( nickname ) ); + for ( const auto layer : mNicknameLayers.values( nickname ) ) + { + if ( !addLayerToRender( layer ) ) + { + throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( layer->name() ) ); + } + } } else if ( mLayerGroups.contains( nickname ) ) { @@ -575,7 +628,10 @@ void QgsWmsRenderContext::searchLayersToRenderStyle() for ( const auto &name : layersFromGroup ) { - mLayersToRender.append( mNicknameLayers.values( name ) ); + for ( const auto layer : mNicknameLayers.values( name ) ) + { + addLayerToRender( layer ); + } } } else @@ -852,17 +908,18 @@ bool QgsWmsRenderContext::isExternalLayer( const QString &name ) const return false; } -void QgsWmsRenderContext::checkLayerReadPermissions() +bool QgsWmsRenderContext::checkLayerReadPermissions( QgsMapLayer *layer ) { #ifdef HAVE_SERVER_PYTHON_PLUGINS - for ( const auto layer : mLayersToRender ) + if ( !accessControl()->layerReadPermission( layer ) ) { - if ( !accessControl()->layerReadPermission( layer ) ) - { - throw QgsSecurityException( QStringLiteral( "You are not allowed to access to the layer: %1" ).arg( layer->name() ) ); - } + QString msg = QStringLiteral( "Checking forbidden access for layer: %1" ).arg( layer->name() ); + QgsMessageLog::logMessage( msg, "Server", Qgis::MessageLevel::Info ); + return false; } #endif + Q_UNUSED( layer ) + return true; } #ifdef HAVE_SERVER_PYTHON_PLUGINS diff --git a/src/server/services/wms/qgswmsrendercontext.h b/src/server/services/wms/qgswmsrendercontext.h index eca1e1c0249..414939246b1 100644 --- a/src/server/services/wms/qgswmsrendercontext.h +++ b/src/server/services/wms/qgswmsrendercontext.h @@ -292,7 +292,17 @@ namespace QgsWms void searchLayersToRenderStyle(); void removeUnwantedLayers(); - void checkLayerReadPermissions(); + /** + * Adds the layer to the list of layers to be rendered if the layer is readable + * Returns true if the layer is readable, false otherwise + */ + bool addLayerToRender( QgsMapLayer *layer ); + + /** + * Check layer read permissions + * Returns true if the layer is readable, false otherwise + */ + bool checkLayerReadPermissions( QgsMapLayer *layer ); bool layerScaleVisibility( const QString &name ) const; diff --git a/tests/src/python/test_qgsserver_accesscontrol_wms.py b/tests/src/python/test_qgsserver_accesscontrol_wms.py index cb03d9bc977..55c45896be8 100644 --- a/tests/src/python/test_qgsserver_accesscontrol_wms.py +++ b/tests/src/python/test_qgsserver_accesscontrol_wms.py @@ -276,10 +276,31 @@ class TestQgsServerAccessControlWMS(TestQgsServerAccessControl): headers.get("Content-Type"), "text/xml; charset=utf-8", f"Content type for GetMap is wrong: {headers.get('Content-Type')}") self.assertTrue( - str(response).find('') != -1, + str(response).find('') != -1, "Not allowed do a GetMap on Country_grp" ) + # Check group ACL. + # The whole group should not fail since it contains + # allowed layers. + query_string = "&".join(["%s=%s" % i for i in list({ + "MAP": urllib.parse.quote(self.projectPath), + "SERVICE": "WMS", + "VERSION": "1.1.1", + "REQUEST": "GetMap", + "LAYERS": "project_grp", + "STYLES": "", + "FORMAT": "image/png", + "BBOX": "-16817707,-6318936.5,5696513,16195283.5", + "HEIGHT": "500", + "WIDTH": "500", + "SRS": "EPSG:3857" + }.items())]) + response, headers = self._get_restricted(query_string) + self.assertEqual( + headers.get("Content-Type"), "image/png", + f"Content type for GetMap is wrong: {headers.get('Content-Type')}") + def test_wms_getfeatureinfo_hello(self): query_string = "&".join(["%s=%s" % i for i in list({ "MAP": urllib.parse.quote(self.projectPath), diff --git a/tests/src/python/test_qgsserver_accesscontrol_wms_getlegendgraphic.py b/tests/src/python/test_qgsserver_accesscontrol_wms_getlegendgraphic.py index 3175b006dd5..36a54040723 100644 --- a/tests/src/python/test_qgsserver_accesscontrol_wms_getlegendgraphic.py +++ b/tests/src/python/test_qgsserver_accesscontrol_wms_getlegendgraphic.py @@ -98,7 +98,7 @@ class TestQgsServerAccessControlWMSGetlegendgraphic(TestQgsServerAccessControl): headers.get("Content-Type"), "text/xml; charset=utf-8", f"Content type for GetMap is wrong: {headers.get('Content-Type')}") self.assertTrue( - str(response).find('') != -1, + str(response).find('') != -1, "Not allowed GetLegendGraphic" ) diff --git a/tests/testdata/qgis_server_accesscontrol/project_grp.qgs b/tests/testdata/qgis_server_accesscontrol/project_grp.qgs index 637e924f52e..8aa1e5f6896 100644 --- a/tests/testdata/qgis_server_accesscontrol/project_grp.qgs +++ b/tests/testdata/qgis_server_accesscontrol/project_grp.qgs @@ -3742,7 +3742,7 @@ def my_form_open(dialog, layer, feature): false - + project_grp false Simple test app. true