Merge pull request #114 from CESNET/refactor

refactor: Some login in sources/modifiers constructors moved to Claim…

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/ClaimInitContext.java

@@ -0,0 +1,38 @@
+package cz.muni.ics.oidc.server.claims;
+
+import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
+
+import java.util.Properties;
+
+@Slf4j
+public class ClaimInitContext {
+
+    private final String propertyPrefix;
+
+    private final Properties properties;
+
+    @Getter
+    private final String claimName;
+
+    public ClaimInitContext(String propertyPrefix, Properties properties, String claimName) {
+        this.propertyPrefix = propertyPrefix;
+        this.properties = properties;
+        this.claimName = claimName;
+    }
+
+    public String getProperty(String suffix, String defaultValue) {
+        return properties.getProperty(propertyPrefix + '.' + suffix, defaultValue);
+    }
+
+    public Long getLongProperty(String suffix, Long defaultValue) {
+        String propKey = propertyPrefix + '.' + suffix;
+        String prop = properties.getProperty(propertyPrefix + "." + suffix);
+        try {
+            return Long.parseLong(prop);
+        } catch (NumberFormatException e) {
+            log.warn("Could not parse value '{}' for property '{}' as Long", prop, propKey);
+        }
+        return defaultValue;
+    }
+}

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/ClaimModifierInitContext.java

@@ -1,6 +1,8 @@
 package cz.muni.ics.oidc.server.claims;
 
 import java.util.Properties;
+
+import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -9,32 +11,18 @@ import lombok.extern.slf4j.Slf4j;
  * @author Martin Kuba <makub@ics.muni.cz>
  */
 @Slf4j
-public class ClaimModifierInitContext {
+@Getter
+public class ClaimModifierInitContext extends ClaimInitContext {
 
-	private final String propertyPrefix;
-	private final Properties properties;
-	private final String claimName;
 	private final String modifierName;
 
 	public ClaimModifierInitContext(String propertyPrefix, Properties properties, String claimName, String modifierName) {
-		this.propertyPrefix = propertyPrefix;
+		super(propertyPrefix, properties, claimName);
-		this.properties = properties;
+
-		this.claimName = claimName;
 		this.modifierName = modifierName;
+
 		log.debug("{}:{} - context: property prefix for modifier configured to '{}'",
 				claimName, modifierName, propertyPrefix);
 	}
 
-	public String getClaimName() {
-		return claimName;
-	}
-
-	public String getModifierName() {
-		return modifierName;
-	}
-
-	public String getProperty(String suffix, String defaultValue) {
-		return properties.getProperty(propertyPrefix + '.' + suffix, defaultValue);
-	}
-
 }

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/ClaimSourceInitContext.java

@@ -3,6 +3,8 @@ package cz.muni.ics.oidc.server.claims;
 import cz.muni.ics.jwt.signer.service.JWTSigningAndValidationService;
 import cz.muni.ics.oidc.server.configurations.PerunOidcConfig;
 import java.util.Properties;
+
+import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -11,13 +13,11 @@ import lombok.extern.slf4j.Slf4j;
  * @author Martin Kuba <makub@ics.muni.cz>
  */
 @Slf4j
-public class ClaimSourceInitContext {
+@Getter
+public class ClaimSourceInitContext extends ClaimInitContext {
 
 	private final PerunOidcConfig perunOidcConfig;
 	private final JWTSigningAndValidationService jwtService;
-	private final String propertyPrefix;
-	private final Properties properties;
-	private final String claimName;
 
 	public ClaimSourceInitContext(PerunOidcConfig perunOidcConfig,
 								  JWTSigningAndValidationService jwtService,
@@ -25,39 +25,12 @@ public class ClaimSourceInitContext {
 								  Properties properties,
 								  String claimName)
 	{
+		super(propertyPrefix, properties, claimName);
+
 		this.perunOidcConfig = perunOidcConfig;
 		this.jwtService = jwtService;
-		this.propertyPrefix = propertyPrefix;
+
-		this.properties = properties;
-		this.claimName = claimName;
 		log.debug("{} - context: property prefix for modifier configured to '{}'", claimName, propertyPrefix);
 	}
 
-	public String getClaimName() {
-		return claimName;
-	}
-
-	public String getProperty(String suffix, String defaultValue) {
-		return properties.getProperty(propertyPrefix + "." + suffix, defaultValue);
-	}
-
-	public Long getLongProperty(String suffix, Long defaultValue) {
-		String propKey = propertyPrefix + '.' + suffix;
-		String prop = properties.getProperty(propertyPrefix + "." + suffix);
-		try {
-			return Long.parseLong(prop);
-		} catch (NumberFormatException e) {
-			log.warn("Could not parse value '{}' for property '{}' as Long", prop, propKey);
-		}
-		return defaultValue;
-	}
-
-	public JWTSigningAndValidationService getJwtService() {
-		return jwtService;
-	}
-
-	public PerunOidcConfig getPerunOidcConfig() {
-		return perunOidcConfig;
-	}
-
 }

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/ClaimUtils.java

@@ -19,19 +19,25 @@ public class ClaimUtils {
         return StringUtils.hasText(propertyName);
     }
 
-    public static String fillStringPropertyOrNoVal(String suffix, ClaimSourceInitContext ctx) {
+    public static String fillStringMandatoryProperty(String suffix, ClaimInitContext ctx, String claimName) {
-        return fillStringPropertyOrNoVal(ctx.getProperty(suffix, NO_VALUE));
+        String filled = fillStringPropertyOrDefaultVal(ctx.getProperty(suffix, NO_VALUE), NO_VALUE);
+
+        if (filled == null) {
+            throw new IllegalArgumentException(claimName + " - missing mandatory configuration option: " + suffix);
+        }
+
+        return filled;
     }
 
-    public static String fillStringPropertyOrNoVal(String suffix, ClaimModifierInitContext ctx) {
+    public static String fillStringPropertyOrDefaultVal(String suffix, ClaimInitContext ctx, String defaultVal) {
-        return fillStringPropertyOrNoVal(ctx.getProperty(suffix, NO_VALUE));
+        return fillStringPropertyOrDefaultVal(ctx.getProperty(suffix, NO_VALUE), defaultVal);
     }
 
-    private static String fillStringPropertyOrNoVal(String prop) {
+    private static String fillStringPropertyOrDefaultVal(String prop, String defaultVal) {
         if (StringUtils.hasText(prop)) {
             return prop;
         } else {
-            return NO_VALUE;
+            return defaultVal;
         }
     }
 

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/modifiers/GroupNamesAARCFormatModifier.java

@@ -35,14 +35,10 @@ public class GroupNamesAARCFormatModifier extends ClaimModifier {
 
 	public GroupNamesAARCFormatModifier(ClaimModifierInitContext ctx) {
 		super(ctx);
-		this.prefix = ClaimUtils.fillStringPropertyOrNoVal(PREFIX, ctx);
+
-		if (!ClaimUtils.isPropSet(this.prefix)) {
+		this.prefix = ClaimUtils.fillStringMandatoryProperty(PREFIX, ctx, getClaimName());
-			throw new IllegalArgumentException(getUnifiedName() + " - missing mandatory configuration option: " + PREFIX);
+		this.authority = ClaimUtils.fillStringMandatoryProperty(AUTHORITY, ctx, getClaimName());
-		}
+
-		this.authority = ClaimUtils.fillStringPropertyOrNoVal(AUTHORITY, ctx);
-		if (!ClaimUtils.isPropSet(this.authority)) {
-			throw new IllegalArgumentException(getUnifiedName() + " - missing mandatory configuration option: " + AUTHORITY);
-		}
 		log.debug("{}:{}(modifier) - prefix: '{}', authority: '{}'", getClaimName(), getModifierName(), prefix, authority);
 	}
 

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/sources/EntitlementSource.java

@@ -57,19 +57,14 @@ public class EntitlementSource extends GroupNamesSource {
 
 	public EntitlementSource(ClaimSourceInitContext ctx) {
 		super(ctx);
-		this.forwardedEntitlements = ClaimUtils.fillStringPropertyOrNoVal(FORWARDED_ENTITLEMENTS, ctx);
+
-		this.resourceCapabilities = ClaimUtils.fillStringPropertyOrNoVal(RESOURCE_CAPABILITIES, ctx);
+		this.forwardedEntitlements = ClaimUtils.fillStringPropertyOrDefaultVal(FORWARDED_ENTITLEMENTS, ctx, null);
-		this.facilityCapabilities = ClaimUtils.fillStringPropertyOrNoVal(FACILITY_CAPABILITIES, ctx);
+		this.resourceCapabilities = ClaimUtils.fillStringPropertyOrDefaultVal(RESOURCE_CAPABILITIES, ctx, null);
-		this.prefix = ClaimUtils.fillStringPropertyOrNoVal(PREFIX, ctx);
+		this.facilityCapabilities = ClaimUtils.fillStringPropertyOrDefaultVal(FACILITY_CAPABILITIES, ctx, null);
-		if (!ClaimUtils.isPropSet(this.prefix)) {
+
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: " +
+		this.prefix = ClaimUtils.fillStringMandatoryProperty(PREFIX, ctx, getClaimName());
-					PREFIX);
+		this.authority = ClaimUtils.fillStringMandatoryProperty(AUTHORITY, ctx, getClaimName());
-		}
+
-		this.authority = ClaimUtils.fillStringPropertyOrNoVal(AUTHORITY, ctx);
-		if (!ClaimUtils.isPropSet(this.authority)) {
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: " +
-					AUTHORITY);
-		}
 		log.debug("{} - forwardedEntitlements: '{}', resourceCapabilities: '{}', facilityCapabilities: '{}', " +
 				"prefix: '{}', authority: '{}'", getClaimName(), forwardedEntitlements, resourceCapabilities,
 				facilityCapabilities, prefix, authority);

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/sources/ExtractValuesByDomainSource.java

@@ -35,16 +35,10 @@ public class ExtractValuesByDomainSource extends ClaimSource {
 
 	public ExtractValuesByDomainSource(ClaimSourceInitContext ctx) {
 		super(ctx);
-		this.domain = ClaimUtils.fillStringPropertyOrNoVal(EXTRACT_BY_DOMAIN, ctx);
+
-		if (!ClaimUtils.isPropSet(this.domain)) {
+		this.domain = ClaimUtils.fillStringMandatoryProperty(EXTRACT_BY_DOMAIN, ctx, getClaimName());
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: "
+		this.attributeName = ClaimUtils.fillStringMandatoryProperty(ATTRIBUTE_NAME, ctx, getClaimName());
-					+ EXTRACT_BY_DOMAIN);
+
-		}
-		this.attributeName = ClaimUtils.fillStringPropertyOrNoVal(ATTRIBUTE_NAME, ctx);
-		if (!ClaimUtils.isPropSet(this.attributeName)) {
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: "
-					+ ATTRIBUTE_NAME);
-		}
 		log.debug("{} - domain: '{}', attributeName: '{}'", getClaimName(), domain, attributeName);
 	}
 
@@ -56,35 +50,29 @@ public class ExtractValuesByDomainSource extends ClaimSource {
 	@Override
 	public JsonNode produceValue(ClaimSourceProduceContext pctx) {
 		JsonNode result = NullNode.getInstance();
-		if (!ClaimUtils.isPropSet(domain)) {
+		PerunAttributeValue attributeValue = pctx.getAttrValues().get(attributeName);
-			log.trace("{} - no domain set, return empty JSON", domain);
-			result = NullNode.getInstance();
-		} else if (!ClaimUtils.isPropSetAndHasAttribute(attributeName, pctx)) {
-			log.trace("{} - no attributeName set, return empty JSON", domain);
-			result = NullNode.getInstance();
-		} else {
-			PerunAttributeValue attributeValue = pctx.getAttrValues().get(attributeName);
-			if (attributeValue != null) {
-				JsonNode attributeValueJson = attributeValue.valueAsJson();
-				if (attributeValueJson.isTextual() && hasDomain(attributeValueJson.textValue(), domain)) {
-					log.trace("{} - found domain in string value: '{}'", getClaimName(), attributeValueJson);
-					result = attributeValueJson;
-				} else if (attributeValueJson.isArray()) {
-					ArrayNode arrayNode = (ArrayNode) attributeValueJson;
-					JsonNodeFactory factory = JsonNodeFactory.instance;
-					ArrayNode arr = new ArrayNode(factory);
 
-					for (int i = 0; i < arrayNode.size(); i++) {
+		if (attributeValue != null) {
-						String subValue = arrayNode.get(i).textValue();
+			JsonNode attributeValueJson = attributeValue.valueAsJson();
-						if (hasDomain(subValue, domain)) {
+			if (attributeValueJson.isTextual() && hasDomain(attributeValueJson.textValue(), domain)) {
-							log.trace("{} - found domain in array sub-value: '{}'", getClaimName(), subValue);
+				log.trace("{} - found domain in string value: '{}'", getClaimName(), attributeValueJson);
-							arr.add(subValue);
+				result = attributeValueJson;
-						}
+			} else if (attributeValueJson.isArray()) {
+				ArrayNode arrayNode = (ArrayNode) attributeValueJson;
+				JsonNodeFactory factory = JsonNodeFactory.instance;
+				ArrayNode arr = new ArrayNode(factory);
+
+				for (int i = 0; i < arrayNode.size(); i++) {
+					String subValue = arrayNode.get(i).textValue();
+					if (hasDomain(subValue, domain)) {
+						log.trace("{} - found domain in array sub-value: '{}'", getClaimName(), subValue);
+						arr.add(subValue);
 					}
-					result = arr;
 				}
+				result = arr;
 			}
 		}
+
 		log.debug("{} - produced value for user({}): '{}'", getClaimName(), pctx.getPerunUserId(), result);
 		return result;
 	}

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/sources/IsCesnetEligibleClaimSource.java

@@ -40,15 +40,10 @@ public class IsCesnetEligibleClaimSource extends ClaimSource {
 
     public IsCesnetEligibleClaimSource(ClaimSourceInitContext ctx) {
         super(ctx);
-        this.sourceAttr = ClaimUtils.fillStringPropertyOrNoVal(SOURCE_ATTR_NAME, ctx);
+
-        if (!ClaimUtils.isPropSet(sourceAttr)) {
+        this.sourceAttr = ClaimUtils.fillStringMandatoryProperty(SOURCE_ATTR_NAME, ctx, getClaimName());
-            throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: " +
+        this.valueFormat = ClaimUtils.fillStringPropertyOrDefaultVal(VALUE_FORMAT, ctx, DEFAULT_FORMAT);
-                    SOURCE_ATTR_NAME);
+
-        }
-        this.valueFormat = ClaimUtils.fillStringPropertyOrNoVal(VALUE_FORMAT, ctx);
-        if (!ClaimUtils.isPropSet(valueFormat)) {
-            this.valueFormat = DEFAULT_FORMAT;
-        }
         log.debug("{} - sourceAttr: '{}', valueFormat: '{}'", getClaimName(), sourceAttr, valueFormat);
     }
 

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/sources/PerunAttributeClaimSource.java

@@ -31,10 +31,9 @@ public class PerunAttributeClaimSource extends ClaimSource {
 
 	public PerunAttributeClaimSource(ClaimSourceInitContext ctx) {
 		super(ctx);
-		this.attributeName = ClaimUtils.fillStringPropertyOrNoVal(ATTRIBUTE, ctx);
+
-		if (!ClaimUtils.isPropSet(this.attributeName)) {
+		this.attributeName = ClaimUtils.fillStringMandatoryProperty(ATTRIBUTE, ctx, getClaimName());
-			throw new IllegalArgumentException("Missing mandatory configuration option - " + ATTRIBUTE);
+
-		}
 		log.debug("{} - attributeName: '{}'", getClaimName(), attributeName);
 	}
 

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/claims/sources/TwoArrayAttributesClaimSource.java

@@ -36,16 +36,10 @@ public class TwoArrayAttributesClaimSource extends ClaimSource {
 
 	public TwoArrayAttributesClaimSource(ClaimSourceInitContext ctx) {
 		super(ctx);
-		this.attribute1Name = ClaimUtils.fillStringPropertyOrNoVal(ATTRIBUTE_1, ctx);
+
-		if (!ClaimUtils.isPropSet(this.attribute1Name)) {
+		this.attribute1Name = ClaimUtils.fillStringMandatoryProperty(ATTRIBUTE_1, ctx, getClaimName());
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: " +
+		this.attribute2Name = ClaimUtils.fillStringMandatoryProperty(ATTRIBUTE_2, ctx, getClaimName());
-					ATTRIBUTE_1);
+
-		}
-		this.attribute2Name = ClaimUtils.fillStringPropertyOrNoVal(ATTRIBUTE_2, ctx);
-		if (!ClaimUtils.isPropSet(this.attribute2Name)) {
-			throw new IllegalArgumentException(getClaimName() + " - missing mandatory configuration option: " +
-					ATTRIBUTE_2);
-		}
 		log.debug("{} - attribute1Name: '{}', attribute2Name: '{}'", getClaimName(), attribute1Name, attribute2Name);
 	}
 

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/FiltersUtils.java

@@ -274,6 +274,18 @@ public class FiltersUtils {
 		FiltersUtils.redirectUnapproved(request, response, clientIdentifier, redirectUrl);
 	}
 
+	public static String fillStringMandatoryProperty(String propertyName,
+													 String filterName,
+													 PerunRequestFilterParams params) {
+		String filled = params.getProperty(propertyName);
+
+		if (!StringUtils.hasText(filled)) {
+			throw new IllegalArgumentException("No value configured for '" + propertyName + "' in filter " + filterName);
+		}
+
+		return filled;
+	}
+
 	private static void redirectToRegistrationForm(HttpServletRequest request, HttpServletResponse response,
 												   String clientIdentifier, Facility facility, PerunUser user) {
 		Map<String, String> params = new HashMap<>();

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/impl/PerunEnsureVoMember.java

@@ -6,6 +6,7 @@ import cz.muni.ics.oidc.models.PerunAttributeValue;
 import cz.muni.ics.oidc.server.adapters.PerunAdapter;
 import cz.muni.ics.oidc.server.configurations.PerunOidcConfig;
 import cz.muni.ics.oidc.server.filters.FilterParams;
+import cz.muni.ics.oidc.server.filters.FiltersUtils;
 import cz.muni.ics.oidc.server.filters.PerunRequestFilter;
 import cz.muni.ics.oidc.server.filters.PerunRequestFilterParams;
 import cz.muni.ics.oidc.web.controllers.ControllerUtils;
@@ -54,17 +55,14 @@ public class PerunEnsureVoMember extends PerunRequestFilter {
     public PerunEnsureVoMember(PerunRequestFilterParams params) {
         super(params);
         BeanUtil beanUtil = params.getBeanUtil();
+
         this.perunOidcConfig = beanUtil.getBean(PerunOidcConfig.class);
         this.perunAdapter = beanUtil.getBean(PerunAdapter.class);
         this.filterName = params.getFilterName();
-        this.triggerAttr = params.getProperty(TRIGGER_ATTR);
+
-        if (!StringUtils.hasText(triggerAttr)) {
+        this.triggerAttr = FiltersUtils.fillStringMandatoryProperty(TRIGGER_ATTR, filterName, params);
-            throw new IllegalArgumentException("No value configured for '" + TRIGGER_ATTR + "' in filter " + filterName);
+        this.voDefsAttr = FiltersUtils.fillStringMandatoryProperty(VO_DEFS_ATTR, filterName, params);
-        }
+
-        this.voDefsAttr = params.getProperty(VO_DEFS_ATTR);
-        if (!StringUtils.hasText(voDefsAttr)) {
-            throw new IllegalArgumentException("No value configured for '" + VO_DEFS_ATTR + "' in filter " + filterName);
-        }
         this.loginUrlAttr = params.getProperty(LOGIN_URL_ATTR);
         log.debug("{} - initialized filter: {}", filterName, this);
     }