geoserver / geofence

Advanced Authorization Manager for GeoServer
GNU General Public License v2.0
94 stars 55 forks source link

Improve filtering by role #222

Closed etj closed 1 year ago

etj commented 1 year ago

A GeoServer instance may have multiple chained authentication services, providing different role semantics.

As a use case, there is an instance having

Role filtering

In order to make the first auth service work, we must enable "Use GEOSERVER roles to get authorizations".
Please note that such an option requires a list of role names (a text field labeled "comma delimited list of mutually exclusive roles for authorization"): the GeoFenceAccessManager will only consider a role passed in the request instance only if it's in the role name list.
This behaviour was too limiting in some cases where the role names are not static so, as reported in a previous comment and documented in GEOS-10420, an improvement was implemented in order to use the first of the roles passed in the request object (so the role name is dynamic and not pre-determined). Please also note that the previous implementation would only use the last role matching in the intersection role list provided in the "comma delimited list of mutually exclusive roles for authorization" and the role list in the request.

There are several limitations and misleading behaviours in this implementation:

Proposed solution

If "Use GEOSERVER roles to get authorizations" is selected and we have "" as authorization groups, we will pass all* the provided roles to GeoFence.

Code changes

geofence:RuleFilter

The RuleFilter should allow for multiple roles.
Changing the bean could be a risk for back compatibility: consider implementing this change by packing all the role names in a single comma separated textin order to not change the DTO. In this case we should enforce some ordering of the role names, in order to preserve the instance hash.

geoserver:GeofenceAccessManager

The setRuleFilterUserOrRole method should be able to set multiple roles into RuleFilter

geofence:RuleReaderServiceImpl

The logic should be changed in order to use multiple roles passed in the filter.

Pls note the the changes in RuleReaderServiceImpl may require extensive changes: at the moment GeoServer can request authorization either by user or by role, in a mutually exlusive way.

Other changes

"Use GEOSERVER roles to get authorizations" should read "User authentication roles to get authorizations"

etj commented 1 year ago

Created related GeoServer ticket at GEOS-10625

etj commented 1 year ago

This test covers all possible user/role combinations:

https://github.com/geoserver/geofence/blob/56af68376235121db92b9e51e0e013284e7ee0d6/src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/RuleReaderServiceImplTest.java#L725-L813