From 76ce690363dbe25097836f609ab6f6aca2197b5b Mon Sep 17 00:00:00 2001
From: Dominik Frantisek Bucik <bucik@ics.muni.cz>
Date: Wed, 15 May 2024 15:37:58 +0200
Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20filtering=20of=20groups?=
 =?UTF-8?q?=20in=20access=20control=20filter?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

AccessControl filter (PerunAuthorizationFilter) now support specifying
a resource attribute, which if set and non-null on Resource object,
groups from this resource will not be considered for controlling access.
---
 .../server/adapters/PerunAdapterMethods.java  |  7 ++--
 .../adapters/impl/PerunAdapterImpl.java       |  6 ++--
 .../adapters/impl/PerunAdapterLdap.java       |  6 ++--
 .../server/adapters/impl/PerunAdapterRpc.java | 32 +++++++++----------
 .../impl/PerunAuthorizationFilter.java        | 24 +++++++++++---
 5 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/PerunAdapterMethods.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/PerunAdapterMethods.java
index d2a7cf172..f0120bae2 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/PerunAdapterMethods.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/PerunAdapterMethods.java
@@ -86,11 +86,12 @@ public interface PerunAdapterMethods {
 	 * Perform check if user can access service based on his/her membership
 	 * in groups assigned to facility resources
 	 *
-	 * @param facility Facility object
-	 * @param userId ID of user
+	 * @param facility                  Facility object
+	 * @param userId                    ID of user
+	 * @param accessControlDisabledAttr
 	 * @return TRUE if user can access, FALSE otherwise
 	 */
-	boolean canUserAccessBasedOnMembership(Facility facility, Long userId);
+	boolean canUserAccessBasedOnMembership(Facility facility, Long userId, String accessControlDisabledAttr);
 
 	/**
 	 * Fetch facility attribute values
diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterImpl.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterImpl.java
index 36b09cd6d..1160ca0d2 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterImpl.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterImpl.java
@@ -62,12 +62,12 @@ public class PerunAdapterImpl extends PerunAdapter {
     }
 
     @Override
-    public boolean canUserAccessBasedOnMembership(Facility facility, Long userId) {
+    public boolean canUserAccessBasedOnMembership(Facility facility, Long userId, String accessControlDisabledAttr) {
         try {
-            return this.getAdapterPrimary().canUserAccessBasedOnMembership(facility, userId);
+            return this.getAdapterPrimary().canUserAccessBasedOnMembership(facility, userId, accessControlDisabledAttr);
         } catch (UnsupportedOperationException e) {
             if (this.isCallFallback()) {
-                return this.getAdapterFallback().canUserAccessBasedOnMembership(facility, userId);
+                return this.getAdapterFallback().canUserAccessBasedOnMembership(facility, userId, accessControlDisabledAttr);
             } else {
                 throw e;
             }
diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterLdap.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterLdap.java
index e38c288ac..6f93e3a18 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterLdap.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterLdap.java
@@ -142,14 +142,14 @@ public class PerunAdapterLdap extends PerunAdapterWithMappingServices implements
 	}
 
 	@Override
-	public boolean canUserAccessBasedOnMembership(Facility facility, Long userId) {
-		Set<Long> groupsWithAccessIds = getGroupIdsAssignedToFacility(facility.getId(), null);
+	public boolean canUserAccessBasedOnMembership(Facility facility, Long userId, String accessControlDisabledAttr) {
+		Set<Long> groupsWithAccessIds = getGroupIdsAssignedToFacility(facility.getId(), accessControlDisabledAttr);
 		if (groupsWithAccessIds == null || groupsWithAccessIds.isEmpty()) {
 			return false;
 		}
 
 		Set<Long> userGroupIds = getGroupIdsWhereUserIsMember(userId, null);
-		if (userGroupIds == null || userGroupIds.isEmpty()) {
+		if (userGroupIds.isEmpty()) {
 			return false;
 		}
 
diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterRpc.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterRpc.java
index 66db96adb..05501b4c4 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterRpc.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/adapters/impl/PerunAdapterRpc.java
@@ -132,12 +132,12 @@ public class PerunAdapterRpc extends PerunAdapterWithMappingServices implements
 	}
 
 	@Override
-	public boolean canUserAccessBasedOnMembership(Facility facility, Long userId) {
+	public boolean canUserAccessBasedOnMembership(Facility facility, Long userId, String ignoreAttr) {
 		if (!this.connectorRpc.isEnabled()) {
 			return true;
 		}
 
-		List<Group> activeGroups = getGroupsWhereUserIsActiveByFacility(facility.getId(), userId);
+		Set<Group> activeGroups = getGroupsWhereUserIsActive(facility.getId(), userId, ignoreAttr);
 		return !activeGroups.isEmpty();
 	}
 
@@ -1002,6 +1002,19 @@ public class PerunAdapterRpc extends PerunAdapterWithMappingServices implements
 		return groups;
 	}
 
+	private Set<Group> getGroupsWhereUserIsActiveByFacility(Long facilityId, Long userId) {
+		if (!this.connectorRpc.isEnabled()) {
+			return new HashSet<>();
+		}
+
+		Map<String, Object> map = new LinkedHashMap<>();
+		map.put("facility", facilityId);
+		map.put("user", userId);
+		JsonNode jsonNode = connectorRpc.post(USERS_MANAGER, "getGroupsWhereUserIsActive", map);
+
+		return new HashSet<>(RpcMapper.mapGroups(jsonNode));
+	}
+
 	private Set<Resource> getResourcesAssignedToFacility(Long facilityId, Long userId, String ignoreAttribute) {
 		if (!this.connectorRpc.isEnabled()) {
 			return new HashSet<>();
@@ -1010,7 +1023,7 @@ public class PerunAdapterRpc extends PerunAdapterWithMappingServices implements
 		Set<Resource> result = new HashSet<>();
 		for (Resource resource : resources) {
 			PerunAttributeValue attrValue = getResourceAttributeValue(resource.getId(), ignoreAttribute);
-			if (attrValue == null || attrValue.isNullValue()) {
+			if (attrValue == null || attrValue.isNullValue() || !attrValue.valueAsBoolean()) {
 				result.add(resource);
 			} else {
 				log.debug(
@@ -1251,19 +1264,6 @@ public class PerunAdapterRpc extends PerunAdapterWithMappingServices implements
 		return true;
 	}
 
-	private List<Group> getGroupsWhereUserIsActiveByFacility(Long facilityId, Long userId) {
-		if (!this.connectorRpc.isEnabled()) {
-			return new ArrayList<>();
-		}
-
-		Map<String, Object> map = new LinkedHashMap<>();
-		map.put("facility", facilityId);
-		map.put("user", userId);
-		JsonNode jsonNode = connectorRpc.post(USERS_MANAGER, "getGroupsWhereUserIsActive", map);
-
-		return RpcMapper.mapGroups(jsonNode);
-	}
-
 	private Map<Long, Vo> convertVoListToMap(List<Vo> vos) {
 		if (!this.connectorRpc.isEnabled()) {
 			return new HashMap<>();
diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/impl/PerunAuthorizationFilter.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/impl/PerunAuthorizationFilter.java
index 9c92242b0..40929bae0 100644
--- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/impl/PerunAuthorizationFilter.java
+++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/impl/PerunAuthorizationFilter.java
@@ -31,6 +31,11 @@ import static cz.muni.ics.openid.connect.request.ConnectRequestParameters.STATE;
  *
  * Configuration:
  * - based on the configuration of bean "facilityAttrsConfig"
+ * Configuration of filter (replace [name] part with the name defined for the filter):
+ * <ul>
+ *     <li><b>filter.[name].accessControlDisabledAttr</b> - resource attribute which triggers if resource assigned
+ *     groups should not be used for controlling access. When not specified, all groups will be used.</li>
+ * </ul>
  * @see FacilityAttrsConfig
  * @see cz.muni.ics.oidc.server.filters.AuthProcFilter (basic configuration options)
  *
@@ -39,15 +44,19 @@ import static cz.muni.ics.openid.connect.request.ConnectRequestParameters.STATE;
 @Slf4j
 public class PerunAuthorizationFilter extends AuthProcFilter {
 
+	protected static final String ACCESS_CONTROL_DISABLED_ATTR = "accessControlDisabledAttr";
+
 	private final PerunAdapter perunAdapter;
 	private final FacilityAttrsConfig facilityAttrsConfig;
 	private final PerunOidcConfig config;
+	private final String accessControlDisabledAttr;
 
 	public PerunAuthorizationFilter(AuthProcFilterInitContext ctx) throws ConfigurationException {
 		super(ctx);
 		this.perunAdapter = ctx.getPerunAdapterBean();
 		this.config = ctx.getPerunOidcConfigBean();
 		this.facilityAttrsConfig = ctx.getBeanUtil().getBean(FacilityAttrsConfig.class);
+		this.accessControlDisabledAttr = ctx.getProperty(ACCESS_CONTROL_DISABLED_ATTR, null);
 	}
 
 	@Override
@@ -65,7 +74,7 @@ public class PerunAuthorizationFilter extends AuthProcFilter {
 		}
 
 		return this.decideAccess(facility, user, req, res, params.getClientIdentifier(),
-				perunAdapter, facilityAttrsConfig);
+				perunAdapter, facilityAttrsConfig, accessControlDisabledAttr);
 	}
 
 	@Override
@@ -73,9 +82,14 @@ public class PerunAuthorizationFilter extends AuthProcFilter {
 		return false;
 	}
 
-	private boolean decideAccess(Facility facility, PerunUser user, HttpServletRequest req,
-								 HttpServletResponse response, String clientIdentifier, PerunAdapter perunAdapter,
-								 FacilityAttrsConfig facilityAttrsConfig)
+	private boolean decideAccess(Facility facility,
+								 PerunUser user,
+								 HttpServletRequest req,
+								 HttpServletResponse response,
+								 String clientIdentifier,
+								 PerunAdapter perunAdapter,
+								 FacilityAttrsConfig facilityAttrsConfig,
+								 String accessControlDisabledAttr)
 	{
 		Map<String, PerunAttributeValue> facilityAttributes = perunAdapter.getFacilityAttributeValues(
 				facility, facilityAttrsConfig.getMembershipAttrNames());
@@ -85,7 +99,7 @@ public class PerunAuthorizationFilter extends AuthProcFilter {
 			return true;
 		}
 
-		if (perunAdapter.canUserAccessBasedOnMembership(facility, user.getId())) {
+		if (perunAdapter.canUserAccessBasedOnMembership(facility, user.getId(), accessControlDisabledAttr)) {
 			log.info("{} - user allowed to access the service", getFilterName());
 			return true;
 		} else {
-- 
GitLab