fix some possible problems with STORK configuration in config-GUI

5 files changed, 134 insertions, 79 deletions

diff --git a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/GeneralStorkConfig.java b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/GeneralStorkConfig.java
index d8bb0179c..e71bad299 100644
--- a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/GeneralStorkConfig.java
+++ b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/GeneralStorkConfig.java
@@ -25,6 +25,8 @@ package at.gv.egovernment.moa.id.configuration.data;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.log4j.Logger;
+
 import at.gv.egovernment.moa.id.commons.db.dao.config.deprecated.AuthComponentGeneral;
 import at.gv.egovernment.moa.id.commons.db.dao.config.deprecated.CPEPS;
 import at.gv.egovernment.moa.id.commons.db.dao.config.deprecated.ForeignIdentities;
@@ -34,8 +36,6 @@ import at.gv.egovernment.moa.id.commons.db.dao.config.deprecated.StorkAttribute;
 import at.gv.egovernment.moa.id.config.webgui.exception.ConfigurationException;
 import at.gv.egovernment.moa.id.configuration.config.ConfigurationProvider;
 
-import org.apache.log4j.Logger;
-
 public class GeneralStorkConfig {
 
 	private List<CPEPS> cpepslist;
@@ -61,7 +61,9 @@ public class GeneralStorkConfig {
     	
     public void parse(MOAIDConfiguration config) {
         log.info("Initializing general Stork config");
-
+  
+        cpepslist = new ArrayList<CPEPS>();
+		attributes = new ArrayList<StorkAttribute>();
 
 		if (config != null) {
 			AuthComponentGeneral auth = config.getAuthComponentGeneral();
@@ -71,15 +73,14 @@ public class GeneralStorkConfig {
 
 				if (foreign != null) {
 					STORK stork = foreign.getSTORK();
-					cpepslist = new ArrayList<CPEPS>();
-					attributes = new ArrayList<StorkAttribute>();
-					
+										
 					if (stork != null) {
 						// deep clone all the things
-						// to foreclose lazyloading session timeouts
-						
-						for(CPEPS current : stork.getCPEPS()) {
-							cpepslist.add(current);
+						// to foreclose lazyloading session timeouts						
+						if (stork.getCPEPS() != null) {
+							for(CPEPS current : stork.getCPEPS()) {
+								cpepslist.add(current);
+							}
 						}
 						
 						List<StorkAttribute> tmp = stork.getAttributes();
@@ -91,19 +92,26 @@ public class GeneralStorkConfig {
 
 						try {
 							qaa = stork.getQualityAuthenticationAssuranceLevel();
+							
 						} catch(NullPointerException e) {
 							qaa = 4;
 						}
 					}
 					
-					if (cpepslist.isEmpty())
-						cpepslist = null;
-					
-					if(attributes.isEmpty())
-						attributes.add(new StorkAttribute());
 				}
 			}
 		}
+		
+		if (cpepslist.isEmpty()) {
+			CPEPS defaultCPEPS = new CPEPS();
+			defaultCPEPS.setCountryCode("CC");
+			defaultCPEPS.setURL("http://");
+			defaultCPEPS.setSupportsXMLSignature(true);
+			cpepslist.add(defaultCPEPS );
+		
+		}
+		if(attributes.isEmpty())
+			attributes.add(new StorkAttribute());
 	}
 
     public List<CPEPS> getRawCPEPSList() {
diff --git a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/oa/OASTORKConfig.java b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/oa/OASTORKConfig.java
index 33277af07..c0e1eaaf7 100644
--- a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/oa/OASTORKConfig.java
+++ b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/data/oa/OASTORKConfig.java
@@ -41,6 +41,7 @@ import at.gv.egovernment.moa.id.config.webgui.exception.ConfigurationException;
 import at.gv.egovernment.moa.id.configuration.auth.AuthenticatedUser;
 import at.gv.egovernment.moa.id.configuration.config.ConfigurationProvider;
 import at.gv.egovernment.moa.id.configuration.validation.oa.OASTORKConfigValidation;
+import at.gv.egovernment.moa.util.MiscUtil;
 //import at.gv.egovernment.moa.id.protocols.stork2.AttributeProviderFactory;
 
 public class OASTORKConfig implements IOnlineApplicationData{
@@ -120,8 +121,10 @@ public class OASTORKConfig implements IOnlineApplicationData{
 				
 
 				enabledCitizenCountries = new ArrayList<String>();
-				for(CPEPS current : config.getCPEPS())
-					enabledCitizenCountries.add(current.getCountryCode());
+				if (config.getCPEPS() != null) {
+					for(CPEPS current : config.getCPEPS())
+						enabledCitizenCountries.add(current.getCountryCode());
+				}
 
 				// prepare attribute helper list
 				attributes = new ArrayList<AttributeHelper>();
@@ -130,9 +133,11 @@ public class OASTORKConfig implements IOnlineApplicationData{
                 		for(StorkAttribute current : dbconfig.getAuthComponentGeneral().getForeignIdentities().getSTORK().getAttributes()) {
                 			AttributeHelper tmp = null;
 
-                			for(OAStorkAttribute sepp : config.getOAAttributes())
-                				if(sepp.getName().equals(current.getName()))
-                					tmp = new AttributeHelper(sepp);
+                			if (config.getOAAttributes() != null) {
+                				for(OAStorkAttribute sepp : config.getOAAttributes())
+                					if(sepp.getName() != null && sepp.getName().equals(current.getName()))
+                						tmp = new AttributeHelper(sepp);
+                			}
 
                 			if(null == tmp)
                 				tmp = new AttributeHelper(current);
@@ -248,12 +253,14 @@ public class OASTORKConfig implements IOnlineApplicationData{
 			
 			if (generalConfStorkAttr != null) {
 				for(StorkAttribute currentAttribute : generalConfStorkAttr)
-					if(currentAttribute.getName().equals(current.getName())) {
+					if(MiscUtil.isNotEmpty(currentAttribute.getName()) && 
+							currentAttribute.getName().equals(current.getName())) {
 						if(current.isUsed() || currentAttribute.isMandatory()) {
 							OAStorkAttribute tmp = new OAStorkAttribute();
 							tmp.setName(current.getName());
 							tmp.setMandatory(current.isMandatory());
 							result.add(tmp);
+							
 						}
 						break;
 					}
diff --git a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/struts/action/EditGeneralConfigAction.java b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/struts/action/EditGeneralConfigAction.java
index 504b598c0..d12c1342c 100644
--- a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/struts/action/EditGeneralConfigAction.java
+++ b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/struts/action/EditGeneralConfigAction.java
@@ -542,42 +542,77 @@ public class EditGeneralConfigAction extends BasicAction {
 			forcon.setURL(moaconfig.getSzrgwURL());
 		}
 		
-            ForeignIdentities foreign = dbauth.getForeignIdentities();
-			if (foreign != null) {
-				STORK stork = foreign.getSTORK();
-				if (stork == null) {
-					stork = new STORK();
-					foreign.setSTORK(stork);
-					
-				}
-
-                try {
-                log.error("QAAAA " + storkconfig.getDefaultQaa());
-				stork.setQualityAuthenticationAssuranceLevel(storkconfig.getDefaultQaa());
+        ForeignIdentities foreign = dbauth.getForeignIdentities();
+		if (foreign != null) {
+			STORK stork = foreign.getSTORK();
+			if (stork == null) {
+				stork = new STORK();
+				foreign.setSTORK(stork);
 				
-				if (storkconfig.getAttributes() != null)
-					stork.setAttributes(storkconfig.getAttributes());
-				
-				else
+			}
+
+            try {
+            	log.error("QAAAA " + storkconfig.getDefaultQaa());
+            	stork.setQualityAuthenticationAssuranceLevel(storkconfig.getDefaultQaa());
+			
+				if (storkconfig.getAttributes() != null) {
+					List<StorkAttribute> dbStorkAttr = stork.getAttributes();
+					if (dbStorkAttr == null) {
+						dbStorkAttr = new ArrayList<StorkAttribute>();
+						stork.setAttributes(dbStorkAttr);
+						
+					}
+					
+					for (StorkAttribute attr : storkconfig.getAttributes()) {
+						if (attr != null && MiscUtil.isNotEmpty(attr.getName()))
+							dbStorkAttr.add(attr);
+						
+						else
+							log.info("Remove null or empty STORK attribute");
+					}
+											
+				} else
 					stork.setAttributes((List<StorkAttribute>) (new ArrayList<StorkAttribute>()));
 				
-				if (storkconfig.getCpepslist() != null)
-
-					stork.setCPEPS(storkconfig.getCpepslist());
-				else
-					stork.setCPEPS((List<CPEPS>) (new ArrayList<CPEPS>()));
+				if (storkconfig.getCpepslist() != null) {
+					List<CPEPS> dbStorkCPEPS = stork.getCPEPS();
+					if (dbStorkCPEPS == null) {
+						dbStorkCPEPS = new ArrayList<CPEPS>();
+						stork.setCPEPS(dbStorkCPEPS);
+					}
+					
+					for (CPEPS cpeps : storkconfig.getCpepslist()) {
+						if (cpeps != null && MiscUtil.isNotEmpty(cpeps.getURL()) &&
+								MiscUtil.isNotEmpty(cpeps.getCountryCode())) {
+							
+							if (cpeps.getCountryCode().equals("CC") && 
+									cpeps.getURL().equals("http://"))
+								log.info("Remove dummy STORK CPEPS entry.");
+								
+							else							
+								dbStorkCPEPS.add(cpeps);
+							
+						} else
+							log.info("Remove null or emtpy STORK CPEPS configuration");
+					}
 				
-                } catch (Exception e) {
-                    e.printStackTrace();
-                }
-
-                try{
-                   log.info("CPEPS LIST: " + storkconfig.getCpepslist().size() );
-                    log.trace("CPEPS 1:" + storkconfig.getCpepslist().get(0).getCountryCode() +storkconfig.getCpepslist().get(0).getURL());
-                }   catch (Exception ex) {
-                    log.info("CPEPS LIST is null");
-                }
-			}
+				} else
+					stork.setCPEPS((List<CPEPS>) (new ArrayList<CPEPS>()));
+			
+            } catch (Exception e) {
+                e.printStackTrace();
+                
+            }
+
+            try{
+               log.info("CPEPS LIST: " + storkconfig.getCpepslist().size() );
+               log.trace("CPEPS 1:" + storkconfig.getCpepslist().get(0).getCountryCode() +storkconfig.getCpepslist().get(0).getURL());
+               
+            }   catch (Exception ex) {
+                log.info("CPEPS LIST is null");
+                
+            }
+		}
 
 		if (MiscUtil.isNotEmpty(moaconfig.getMandateURL())) {
 			OnlineMandates dbmandate = dbauth.getOnlineMandates();
diff --git a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/MOAConfigValidator.java b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/MOAConfigValidator.java
index d4e8e957d..617e9cf51 100644
--- a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/MOAConfigValidator.java
+++ b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/MOAConfigValidator.java
@@ -367,7 +367,7 @@ public class MOAConfigValidator {
 		check = form.getSsoTarget();
 		if (MiscUtil.isEmpty(check)) {
 			log.info("Empty SSO Target");
-			errors.add(LanguageHelper.getErrorString("validation.general.sso.target.empty", request));
+			//errors.add(LanguageHelper.getErrorString("validation.general.sso.target.empty", request));
 			
 		} else {
 			if (!ValidationHelper.isValidAdminTarget(check)) {
diff --git a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/StorkConfigValidator.java b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/StorkConfigValidator.java
index b73859d81..6b5c51e3f 100644
--- a/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/StorkConfigValidator.java
+++ b/id/ConfigWebTool/src/main/java/at/gv/egovernment/moa/id/configuration/validation/moaconfig/StorkConfigValidator.java
@@ -47,31 +47,33 @@ public class StorkConfigValidator {
 							errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.cc",
 									new Object[] {check}, request ));
 					}
+					
+					// check url
+					check = current.getURL();
+					if (MiscUtil.isNotEmpty(check)) {
+						if (!ValidationHelper.validateURL(check)) {
+							log.info("CPEPS config URL is invalid : " + check);
+							errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.url", request));
+						}
+					} else {
+						log.warn("CPEPS config url is empty : " + check);
+						errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.empty",
+								new Object[] {check}, request ));
+					}
+					
 				} else {
 					log.warn("CPEPS config countrycode is empty : " + check);
-					errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.empty",
-							new Object[] {check}, request ));
+//					errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.empty",
+//							new Object[] {check}, request ));
 				}
 
-				// check url
-				check = current.getURL();
-				if (MiscUtil.isNotEmpty(check)) {
-					if (!ValidationHelper.validateURL(check)) {
-						log.info("CPEPS config URL is invalid : " + check);
-						errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.url", request));
-					}
-				} else {
-					log.warn("CPEPS config url is empty : " + check);
-					errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.empty",
-							new Object[] {check}, request ));
-				}
 			}
 
 			if (form.getCpepslist() != null) {
 				// ensure uniqueness of country code
 				for (CPEPS one : form.getCpepslist())
 					for (CPEPS another : form.getCpepslist())
-						if (null != one && null != another)
+						if (null != one && null != another && one.getCountryCode() != null)
 							if (!one.equals(another) && one.getCountryCode().equals(another.getCountryCode())) {
 								errors.add(LanguageHelper.getErrorString("validation.stork.cpeps.duplicate", request));
 								break;
@@ -90,17 +92,20 @@ public class StorkConfigValidator {
 		// check attributes
 		if (MiscUtil.isNotEmpty(form.getAttributes())) {
 			for(StorkAttribute check : form.getAttributes()) {
-				String tmp = check.getName().replace("eidas/attributes/", ""); // since eIDaS attributes come with a "/", we need to exclude them from validation. TODO Or should we require the admin to escape them in the UI?
-				if (ValidationHelper.containsPotentialCSSCharacter(tmp, true)) { 
-					log.warn("default attributes contains potentail XSS characters: " + check);
-					errors.add(LanguageHelper.getErrorString("validation.stork.requestedattributes",
-							new Object[] {ValidationHelper.getPotentialCSSCharacter(true)}, request ));
-				}
-				if(!tmp.toLowerCase().matches("^[A-Za-z]*$")) {
-						log.warn("default attributes do not match the requested format : " + check);
+				if (check != null && MiscUtil.isNotEmpty(check.getName())) {
+					String tmp = check.getName().replace("eidas/attributes/", ""); // since eIDaS attributes come with a "/", we need to exclude them from validation. TODO Or should we require the admin to escape them in the UI?
+					if (ValidationHelper.containsPotentialCSSCharacter(tmp, true)) { 
+						log.warn("default attributes contains potentail XSS characters: " + check);
 						errors.add(LanguageHelper.getErrorString("validation.stork.requestedattributes",
-								new Object[] {check}, request ));
-				}
+								new Object[] {ValidationHelper.getPotentialCSSCharacter(true)}, request ));
+					}
+					if(!tmp.toLowerCase().matches("^[A-Za-z]*$")) {
+							log.warn("default attributes do not match the requested format : " + check);
+							errors.add(LanguageHelper.getErrorString("validation.stork.requestedattributes",
+									new Object[] {check}, request ));
+					}
+					
+				} 
 			}
 			
 		//TODO: STORK attributes check if no attribute is set