treated possible infinite loop in ap collection

1 files changed, 43 insertions, 16 deletions

diff --git a/id/server/idserverlib/src/main/java/at/gv/egovernment/moa/id/protocols/stork2/AttributeCollector.java b/id/server/idserverlib/src/main/java/at/gv/egovernment/moa/id/protocols/stork2/AttributeCollector.java
index 6b7769c49..2735fde68 100644
--- a/id/server/idserverlib/src/main/java/at/gv/egovernment/moa/id/protocols/stork2/AttributeCollector.java
+++ b/id/server/idserverlib/src/main/java/at/gv/egovernment/moa/id/protocols/stork2/AttributeCollector.java
@@ -27,6 +27,7 @@ import org.opensaml.common.impl.SecureRandomIdentifierGenerator;
 import eu.stork.peps.auth.commons.IPersonalAttributeList;
 import eu.stork.peps.auth.commons.PEPSUtil;
 import eu.stork.peps.auth.commons.PersonalAttribute;
+import eu.stork.peps.auth.commons.PersonalAttributeList;
 import eu.stork.peps.auth.commons.STORKAuthnRequest;
 import eu.stork.peps.auth.commons.STORKAuthnResponse;
 import eu.stork.peps.auth.engine.STORKSAMLEngine;
@@ -63,18 +64,18 @@ public class AttributeCollector implements IAction {
         for (AttributeProvider current : AttributeProviderFactory.getConfiguredPlugins(oaParam.getStorkAPs()))
             try {
                 newAttributes = current.parse(httpReq);
+                
+                // stop as soon as we hit a capable plugin
+                break;
             } catch (UnsupportedAttributeException e1) {
                 // the current provider cannot find anything familiar within the
                 // provided httpreq. Try the next one.
-                // TODO check the loop
             }
 
         if (null == newAttributes) {
             // we do not have a provider which is capable of fetching something
             // from the received httpreq.
-            // TODO should we continue with the next attribute?
             Logger.error("No attribute could be retrieved from the response the attribute provider gave us.");
-            throw new MOAIDException("stork.11", null);
         }
 
         // - fetch the container
@@ -87,11 +88,8 @@ public class AttributeCollector implements IAction {
             throw new MOAIDException("stork.11", null);
         }
 
-
-
         // - insert the embedded attribute(s) into the container
-        for (PersonalAttribute current : newAttributes)
-            container.getResponse().getPersonalAttributeList().add(current);
+        addOrUpdateAll(container.getResponse().getPersonalAttributeList(), newAttributes);
 
         // see if we need some more attributes
         return processRequest(container, httpReq, httpResp, moasession, oaParam);
@@ -119,23 +117,35 @@ public class AttributeCollector implements IAction {
         try {
             // for each attribute still missing
             for (PersonalAttribute currentAttribute : missingAttributes) {
-                // - check if we can find a suitable AttributeProvider Plugin
+            	
+				/*
+				 * prefill attributes with "notAvailable". If we get them later, we override the value and status.
+				 * This way, there is no error case in which an attribute is left unanswered.
+				 */
+				IPersonalAttributeList aquiredAttributes = new PersonalAttributeList();
+				currentAttribute.setStatus("notAvailable");
+				addOrUpdateAll(container.getResponse().getPersonalAttributeList(), aquiredAttributes);
+
+            	// - check if we can find a suitable AttributeProvider Plugin
                 for (AttributeProvider currentProvider : AttributeProviderFactory.getConfiguredPlugins(oaParam.getStorkAPs())) {
                     try {
                         // - hand over control to the suitable plugin
-                        IPersonalAttributeList aquiredAttributes = currentProvider.acquire(currentAttribute, moasession);
-
-                        // - add the aquired attribute to the container
-                        for (PersonalAttribute current : aquiredAttributes)
-                            container.getResponse().getPersonalAttributeList().add(current);
+                        aquiredAttributes = currentProvider.acquire(currentAttribute, moasession);
+                        break;
                     } catch (UnsupportedAttributeException e) {
                         // ok, try the next attributeprovider
                     } catch (MOAIDException e) {
                         // the current plugin had an error. Try the next one.
-                        // TODO we might want to add the non-fetchable attribute as "NotAvailable" to prevent an infinite loop
                     }
-
                 }
+                
+				// check if we could fetch the attribute
+				if (null == aquiredAttributes) {
+					// if not
+					Logger.error("We have no suitable plugin for obtaining the attribute '" + currentAttribute.getName() + "'");
+				} else
+					// else, update any existing attributes
+					addOrUpdateAll(container.getResponse().getPersonalAttributeList(), aquiredAttributes);
             }
 
             // build response
@@ -159,7 +169,6 @@ public class AttributeCollector implements IAction {
                 // add container-key to redirect embedded within the return URL
                 e.getAp().performRedirect(AuthConfigurationProvider.getInstance().getPublicURLPrefix() + "/dispatcher?mod=id_stork2&action=AttributeCollector&" + ARTIFACT_ID + "=" + newArtifactId, container.getRequest().getSpCountry(), request, response, oaParam);
 
-
             } catch (Exception e1) {
                 // TODO should we return the response as is to the PEPS?
                 Logger.error("Error putting incomplete Stork response into temporary storage", e1);
@@ -238,6 +247,24 @@ public class AttributeCollector implements IAction {
             Logger.error("Velocity error: " + e.getMessage());
         }
     }
+    
+    /**
+     * Adds or updates all {@link PersonalAttribute} objects given in {@code source} to/in {@code target}.
+     *
+     * @param target the target
+     * @param source the source
+     */
+	private void addOrUpdateAll(IPersonalAttributeList target, IPersonalAttributeList source) {
+		for (PersonalAttribute current : source) {
+			// check if we need to update the current pa
+			if (target.containsKey(current.getName())) {
+				target.get(current.getName()).setStatus(current.getStatus());
+				target.get(current.getName()).setValue(current.getValue());
+				target.get(current.getName()).setComplexValue(current.getComplexValue());
+			} else
+				target.add(current);
+		}
+	}
 
     /* (non-Javadoc)
      * @see at.gv.egovernment.moa.id.moduls.IAction#needAuthentication(at.gv.egovernment.moa.id.moduls.IRequest, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)