Skip to content

Commit

Permalink
Fix WMS layer access control check
Browse files Browse the repository at this point in the history
  • Loading branch information
@dmarteau
dmarteau committed Jul 11, 2024
1 parent 7c32a89 commit 6236202
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 19 deletions.
73 changes: 58 additions & 15 deletions src/server/services/wms/qgswmsrendercontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,19 @@ void QgsWmsRenderContext::setParameters( const QgsWmsParameters &parameters )

searchLayersToRender();
removeUnwantedLayers();
checkLayerReadPermissions();

std::reverse( mLayersToRender.begin(), mLayersToRender.end() );
}

void QgsWmsRenderContext::addLayerToRender( QgsMapLayer *layer, bool queryLayer )
{
if ( checkLayerReadPermissions( layer, queryLayer ) )
{
mLayersToRender.append( layer );
}
}


void QgsWmsRenderContext::setFlag( const Flag flag, const bool on )
{
if ( on )
Expand Down Expand Up @@ -186,6 +194,7 @@ qreal QgsWmsRenderContext::dotsPerMm() const

QStringList QgsWmsRenderContext::flattenedQueryLayers( const QStringList &layerNames ) const
{

QStringList result;
std::function <QStringList( const QString &name )> findLeaves = [ & ]( const QString & name ) -> QStringList
{
Expand Down Expand Up @@ -335,7 +344,7 @@ void QgsWmsRenderContext::initLayerGroupsRecursive( const QgsLayerTreeGroup *gro
{
if ( !groupName.isEmpty() )
{
mLayerGroups[groupName] = QList<QgsMapLayer *>();
QList<QgsMapLayer *> layerGroup;
const auto projectLayerTreeRoot { mProject->layerTreeRoot() };
const auto treeGroupLayers { group->findLayers() };
// Fast track if there is no custom layer order,
Expand All @@ -344,7 +353,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, false ) )
{
layerGroup.push_back( layer );
}
}
}
else
Expand All @@ -354,16 +367,25 @@ void QgsWmsRenderContext::initLayerGroupsRecursive( const QgsLayerTreeGroup *gro
QList<QgsMapLayer *> groupLayersList;
for ( const auto &tl : treeGroupLayers )
{
groupLayersList << tl->layer();
auto layer = tl->layer();
if ( checkLayerReadPermissions( layer, false ) )
{
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() )
Expand Down Expand Up @@ -442,10 +464,12 @@ void QgsWmsRenderContext::searchLayersToRender()
{
const QList<QgsMapLayer *> layers = mNicknameLayers.values( layerName );
for ( QgsMapLayer *lyr : layers )
{
if ( !mLayersToRender.contains( lyr ) )
{
mLayersToRender.append( lyr );
addLayerToRender( lyr, true );
}
}
}
}

Expand All @@ -456,10 +480,12 @@ void QgsWmsRenderContext::searchLayersToRender()
{
const QList<QgsMapLayer *> layers = mNicknameLayers.values( layerName );
for ( QgsMapLayer *lyr : layers )
{
if ( !mLayersToRender.contains( lyr ) )
{
mLayersToRender.append( lyr );
addLayerToRender( lyr, true );
}
}
}
}
}
Expand Down Expand Up @@ -495,7 +521,10 @@ void QgsWmsRenderContext::searchLayersToRenderSld()
if ( mNicknameLayers.contains( lname ) )
{
mSlds[lname] = namedElem;
mLayersToRender.append( mNicknameLayers.values( lname ) );
for ( const auto layer : mNicknameLayers.values( lname ) )
{
addLayerToRender( layer, true );
}
}
else if ( mLayerGroups.contains( lname ) )
{
Expand Down Expand Up @@ -540,7 +569,7 @@ void QgsWmsRenderContext::searchLayersToRenderStyle()
{
// to delete later
mExternalLayers.append( layer.release() );
mLayersToRender.append( mExternalLayers.last() );
addLayerToRender( mExternalLayers.last(), true );
}
}
else if ( mNicknameLayers.contains( nickname ) )
Expand All @@ -550,7 +579,10 @@ void QgsWmsRenderContext::searchLayersToRenderStyle()
mStyles[nickname] = style;
}

mLayersToRender.append( mNicknameLayers.values( nickname ) );
for ( const auto layer : mNicknameLayers.values( nickname ) )
{
addLayerToRender( layer, true );
}
}
else if ( mLayerGroups.contains( nickname ) )
{
Expand All @@ -575,7 +607,10 @@ void QgsWmsRenderContext::searchLayersToRenderStyle()

for ( const auto &name : layersFromGroup )
{
mLayersToRender.append( mNicknameLayers.values( name ) );
for ( const auto layer : mNicknameLayers.values( name ) )
{
addLayerToRender( layer, false );
}
}
}
else
Expand Down Expand Up @@ -852,17 +887,25 @@ bool QgsWmsRenderContext::isExternalLayer( const QString &name ) const
return false;
}

void QgsWmsRenderContext::checkLayerReadPermissions()
bool QgsWmsRenderContext::checkLayerReadPermissions( QgsMapLayer *layer, bool queryLayer )
{
#ifdef HAVE_SERVER_PYTHON_PLUGINS
for ( const auto layer : mLayersToRender )
if ( !accessControl()->layerReadPermission( layer ) )
{
if ( !accessControl()->layerReadPermission( layer ) )
QString msg = QStringLiteral( "Checking forbidden access for layer: %1" ).arg( layer->name() );
QgsMessageLog::logMessage( msg, "Server", Qgis::MessageLevel::Info );

// Check if a layer has been requested explicitly
// In this case raise a forbidden access exception
if ( queryLayer )
{
throw QgsSecurityException( QStringLiteral( "You are not allowed to access to the layer: %1" ).arg( layer->name() ) );
throw QgsSecurityException( QStringLiteral( "Your are not allowed to access the layer %1" ).arg( layer->name() ) );
}
return false;

}
#endif
return true;
}

#ifdef HAVE_SERVER_PYTHON_PLUGINS
Expand Down
4 changes: 3 additions & 1 deletion src/server/services/wms/qgswmsrendercontext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ namespace QgsWms
void searchLayersToRenderStyle();
void removeUnwantedLayers();

void checkLayerReadPermissions();
void addLayerToRender( QgsMapLayer *layer, bool queryLayer );

bool checkLayerReadPermissions( QgsMapLayer *layer, bool queryLayer );

bool layerScaleVisibility( const QString &name ) const;

Expand Down
23 changes: 22 additions & 1 deletion tests/src/python/test_qgsserver_accesscontrol_wms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,31 @@ def test_wms_getmap_grp(self):
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('<ServiceException code="Security">') != -1,
str(response).find('<ServiceException code="LayerNotDefined">') != -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),
Expand Down
2 changes: 1 addition & 1 deletion tests/src/python/test_qgsserver_accesscontrol_wms_getlegendgraphic.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def test_wms_getlegendgraphic_country_grp(self):
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('<ServiceException code="Security">') != -1,
str(response).find('<ServiceException code="LayerNotDefined">') != -1,
"Not allowed GetLegendGraphic"
)

Expand Down
2 changes: 1 addition & 1 deletion tests/testdata/qgis_server_accesscontrol/project_grp.qgs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3742,7 +3742,7 @@ def my_form_open(dialog, layer, feature):
<WMSRequestDefinedDataSources type="bool">false</WMSRequestDefinedDataSources>
<WMSRestrictedComposers type="QStringList"/>
<WMSRestrictedLayers type="QStringList"/>
<WMSRootName type="QString"></WMSRootName>
<WMSRootName type="QString">project_grp</WMSRootName>
<WMSSegmentizeFeatureInfoGeometry type="bool">false</WMSSegmentizeFeatureInfoGeometry>
<WMSServiceAbstract type="QString">Simple test app.</WMSServiceAbstract>
<WMSServiceCapabilities type="bool">true</WMSServiceCapabilities>
Expand Down

0 comments on commit 6236202

Please sign in to comment.