From f9688deeeff9bfd352b84e888af4df64af1f98b2 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 4 Apr 2024 16:41:13 +0200 Subject: [PATCH] Server/OAPIF: fix unreported issue with access control layer permissions When 'canRead' property for a published WFS layer was set to False from an access control filter plugin the whole /collections reques was returning a 404 insted of returning the list of published layers (minus the filtered ones). --- src/server/services/wfs3/qgswfs3handlers.cpp | 97 +++++++++++--------- tests/src/python/test_qgsserver_api.py | 48 ++++++++++ 2 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/server/services/wfs3/qgswfs3handlers.cpp b/src/server/services/wfs3/qgswfs3handlers.cpp index 11b9cfdc70d3..2b2ad6b859a7 100644 --- a/src/server/services/wfs3/qgswfs3handlers.cpp +++ b/src/server/services/wfs3/qgswfs3handlers.cpp @@ -478,53 +478,55 @@ void QgsWfs3CollectionsHandler::handleRequest( const QgsServerApiContext &contex continue; } - // Check if the layer is published, raise not found if it is not - checkLayerIsAccessible( layer, context ); - - const std::string title { layer->serverProperties()->title().isEmpty() ? layer->name().toStdString() : layer->serverProperties()->title().toStdString() }; - const QString shortName { layer->serverProperties()->shortName().isEmpty() ? layer->name() : layer->serverProperties()->shortName() }; - data["collections"].push_back( + try { - // identifier of the collection used, for example, in URIs - { "id", shortName.toStdString() }, - // human readable title of the collection - { "title", title }, - // a description of the features in the collection - { "description", layer->serverProperties()->abstract().toStdString() }, - { - "crs", crss - }, - // TODO: "relations" ? + // Check if the layer is published, raise not found if it is not + checkLayerIsAccessible( layer, context ); + + const std::string title{layer->serverProperties()->title().isEmpty() ? layer->name().toStdString() : layer->serverProperties()->title().toStdString()}; + const QString shortName{layer->serverProperties()->shortName().isEmpty() ? layer->name() : layer->serverProperties()->shortName()}; + data["collections"].push_back( { - "extent", { - { - "spatial", { - { "bbox", QgsServerApiUtils::layerExtent( layer ) }, - { "crs", "http://www.opengis.net/def/crs/OGC/1.3/CRS84" }, + // identifier of the collection used, for example, in URIs + { "id", shortName.toStdString() }, + // human readable title of the collection + { "title", title }, + // a description of the features in the collection + { "description", layer->serverProperties()->abstract().toStdString() }, + { + "crs", crss + }, + // TODO: "relations" ? + { + "extent", { + { + "spatial", { + { "bbox", QgsServerApiUtils::layerExtent( layer ) }, + { "crs", "http://www.opengis.net/def/crs/OGC/1.3/CRS84" }, + }, }, - }, - { - "temporal", { - { "interval", QgsServerApiUtils::temporalExtent( layer ) }, - { "trs", "http://www.opengis.net/def/uom/ISO-8601/0/Gregorian" }, + { + "temporal", { + { "interval", QgsServerApiUtils::temporalExtent( layer ) }, + { "trs", "http://www.opengis.net/def/uom/ISO-8601/0/Gregorian" }, + } } } - } - }, - { - "links", { - { - { "href", href( context, QStringLiteral( "/%1/items" ).arg( shortName ), QgsServerOgcApi::contentTypeToExtension( QgsServerOgcApi::ContentType::JSON ) ) }, - { "rel", QgsServerOgcApi::relToString( QgsServerOgcApi::Rel::item ) }, - { "type", QgsServerOgcApi::mimeType( QgsServerOgcApi::ContentType::GEOJSON ) }, - { "title", title + " as GeoJSON" } - }, - { - { "href", href( context, QStringLiteral( "/%1/items" ).arg( shortName ), QgsServerOgcApi::contentTypeToExtension( QgsServerOgcApi::ContentType::HTML ) ) }, - { "rel", QgsServerOgcApi::relToString( QgsServerOgcApi::Rel::item ) }, - { "type", QgsServerOgcApi::mimeType( QgsServerOgcApi::ContentType::HTML ) }, - { "title", title + " as HTML" } - }/* TODO: not sure what these "concepts" are about, neither if they are mandatory + }, + { + "links", { + { + { "href", href( context, QStringLiteral( "/%1/items" ).arg( shortName ), QgsServerOgcApi::contentTypeToExtension( QgsServerOgcApi::ContentType::JSON ) ) }, + { "rel", QgsServerOgcApi::relToString( QgsServerOgcApi::Rel::item ) }, + { "type", QgsServerOgcApi::mimeType( QgsServerOgcApi::ContentType::GEOJSON ) }, + { "title", title + " as GeoJSON" } + }, + { + { "href", href( context, QStringLiteral( "/%1/items" ).arg( shortName ), QgsServerOgcApi::contentTypeToExtension( QgsServerOgcApi::ContentType::HTML ) ) }, + { "rel", QgsServerOgcApi::relToString( QgsServerOgcApi::Rel::item ) }, + { "type", QgsServerOgcApi::mimeType( QgsServerOgcApi::ContentType::HTML ) }, + { "title", title + " as HTML" } + }/* TODO: not sure what these "concepts" are about, neither if they are mandatory { { "href", href( api, context.request(), QStringLiteral( "/%1/concepts" ).arg( shortName ) ) }, { "rel", QgsServerOgcApi::relToString( QgsServerOgcApi::Rel::item ) }, @@ -532,9 +534,14 @@ void QgsWfs3CollectionsHandler::handleRequest( const QgsServerApiContext &contex { "title", "Describe " + title } } */ - } - }, - } ); + } + }, + } ); + } + catch ( QgsServerApiNotFoundError & ) + { + // Skip non-published layers + } } } diff --git a/tests/src/python/test_qgsserver_api.py b/tests/src/python/test_qgsserver_api.py index 32b72664bbbc..b3ed9c605aab 100644 --- a/tests/src/python/test_qgsserver_api.py +++ b/tests/src/python/test_qgsserver_api.py @@ -43,6 +43,7 @@ QgsServerOgcApiHandler, QgsServerQueryStringParameter, QgsServiceRegistry, + QgsAccessControlFilter, ) from qgis.testing import unittest from test_qgsserver import QgsServerTestBase @@ -290,6 +291,21 @@ def setUpClass(cls): cls.maxDiff = None +class RestrictedLayerAccessControl(QgsAccessControlFilter): + """Access control filter to exclude a list of layers by ID, used by WFS3 test""" + + def __init__(self, server_iface, ecxluded_layers=[]): + self.excluded_layers = ecxluded_layers + super(QgsAccessControlFilter, self).__init__(server_iface) + + def layerPermissions(self, layer): + """ Return the layer rights """ + + rights = QgsAccessControlFilter.LayerPermissions() + rights.canRead = layer.id() not in self.excluded_layers + return rights + + class QgsServerAPITest(QgsServerAPITestBase): """ QGIS API server tests""" @@ -451,6 +467,38 @@ def test_wfs3_collections_json(self): project.read(os.path.join(self.temporary_path, 'qgis_server', 'test_project_api.qgs')) self.compareApi(request, project, 'test_wfs3_collections_project.json') + def test_wfs3_collections_json_excluded_layer(self): + """Test WFS3 API collections in json format with an excluded layer""" + + request = QgsBufferServerRequest( + 'http://server.qgis.org/wfs3/collections.json') + project = QgsProject() + project.read(os.path.join(self.temporary_path, 'qgis_server', 'test_project_api.qgs')) + + server = QgsServer() + + response = QgsBufferServerResponse() + server.handleRequest(request, response, project) + self.assertEqual(response.headers()['Content-Type'], 'application/json') + self.assertEqual(response.statusCode(), 200) + result = bytes(response.body()).decode('utf8') + jresult = json.loads(result) + ids = [l['id'] for l in jresult['collections']] + self.assertIn('layer1_with_short_name', ids) + + # Access control filter to exclude a layer + acfilter = RestrictedLayerAccessControl(server.serverInterface(), ['testlayer_c0988fd7_97ca_451d_adbc_37ad6d10583a']) + server.serverInterface().registerAccessControl(acfilter, 100) + + response = QgsBufferServerResponse() + server.handleRequest(request, response, project) + self.assertEqual(response.headers()['Content-Type'], 'application/json') + self.assertEqual(response.statusCode(), 200) + result = bytes(response.body()).decode('utf8') + jresult = json.loads(result) + ids = [l['id'] for l in jresult['collections']] + self.assertNotIn('layer1_with_short_name', ids) + def test_wfs3_collections_html(self): """Test WFS3 API collections in html format""" request = QgsBufferServerRequest(