Metadata manager now only log errors when some of the providers fails…

… during initialization instead of failing completely.
sannies · Mar 21, 2011 · 1126cfb · 1126cfb
1 parent ceab3ef
commit 1126cfb
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 39 deletions.
diff --git a/...core/src/main/java/org/springframework/security/saml/metadata/CachingMetadataManager.java b/...core/src/main/java/org/springframework/security/saml/metadata/CachingMetadataManager.java
@@ -17,7 +17,6 @@
 import org.opensaml.saml2.metadata.EntityDescriptor;
 import org.opensaml.saml2.metadata.provider.MetadataProvider;
 import org.opensaml.saml2.metadata.provider.MetadataProviderException;
-import org.opensaml.saml2.metadata.provider.ObservableMetadataProvider;
 
 import java.util.HashMap;
 import java.util.List;
@@ -68,11 +67,9 @@ public CachingMetadataManager(List<MetadataProvider> providers) throws MetadataP
 
     /**
      * Guaranteed to be called by the superclass as part of the initialization.
-     *
-     * @throws MetadataProviderException
      */
     @Override
-    public void refreshMetadata() throws MetadataProviderException {
+    public void refreshMetadata() {
 
         try {
 

diff --git a/.../saml2-core/src/main/java/org/springframework/security/saml/metadata/MetadataManager.java b/.../saml2-core/src/main/java/org/springframework/security/saml/metadata/MetadataManager.java
@@ -134,13 +134,9 @@ public void destroy() {
 
     /**
      * Method can be repeatedly called to browse all configured providers and load SP and IDP names which
-     * are supported by them. In case an error occurs during initialization the old configuration stays.
-     * <p/>
-     * In case
-     *
-     * @throws MetadataProviderException error parsing data
+     * are supported by them. Providers which fail during initialization are ignored for this refresh.
      */
-    public void refreshMetadata() throws MetadataProviderException {
+    public void refreshMetadata() {
 
         try {
 
@@ -167,8 +163,16 @@ public void refreshMetadata() throws MetadataProviderException {
 
                 for (MetadataProvider provider : getProviders()) {
 
-                    log.debug("Refreshing metadata provider {}", provider.toString());
-                    initializeProvider(provider);
+                    try {
+
+                        log.debug("Refreshing metadata provider {}", provider.toString());
+                        initializeProvider(provider);
+
+                    } catch (MetadataProviderException e) {
+
+                        log.error("Initialization of metadata provider {} failed, provider will be ignored", provider, e);
+
+                    }
 
                 }
 
@@ -191,11 +195,39 @@ public void refreshMetadata() throws MetadataProviderException {
 
     }
 
+    /**
+     * Adds a new metadata provider to the managed list.
+     *
+     * @param newProvider provider
+     * @throws MetadataProviderException in case provider can't be added
+     */
     @Override
     public void addMetadataProvider(MetadataProvider newProvider) throws MetadataProviderException {
 
-        super.addMetadataProvider(newProvider);
-        setRefreshRequired(true);
+        try {
+
+            lock.writeLock().lock();
+            super.addMetadataProvider(newProvider);
+            setRefreshRequired(true);
+
+        } finally {
+            lock.writeLock().unlock();
+        }
+
+    }
+
+    @Override
+    public void removeMetadataProvider(MetadataProvider provider) {
+
+        try {
+
+            lock.writeLock().lock();
+            super.removeMetadataProvider(provider);
+            setRefreshRequired(true);
+
+        } finally {
+            lock.writeLock().unlock();
+        }
 
     }
 
@@ -210,7 +242,7 @@ private void initializeProvider(MetadataProvider provider) throws MetadataProvid
 
             if (roleDescriptor != null) {
                 if (idpName.contains(key)) {
-                    throw new MetadataProviderException("Metadata contains two entities with the same entityID: " + key);
+                    log.warn("Provider {} contains entity {} with IDP which was already contained in another metadata provider and will be ignored", provider, key);
                 } else {
                     idpName.add(key);
                 }
@@ -219,7 +251,7 @@ private void initializeProvider(MetadataProvider provider) throws MetadataProvid
             roleDescriptor = provider.getRole(key, SPSSODescriptor.DEFAULT_ELEMENT_NAME, SAMLConstants.SAML20P_NS);
             if (roleDescriptor != null) {
                 if (spName.contains(key)) {
-                    throw new MetadataProviderException("Metadata contains two entities with the same entityID: " + key);
+                    log.warn("Provider {} contains entity {} which SP which was already contained in another metadata provider and will be ignored", provider, key);
                 } else {
                     spName.add(key);
                 }
@@ -238,12 +270,15 @@ private void initializeProvider(MetadataProvider provider) throws MetadataProvid
 
                     // Verify alias is unique
                     if (aliasSet.contains(alias)) {
-                        throw new MetadataProviderException("Alias " + alias + " is not unique for all entities, please make sure alias is either not set or unique");
+
+                        log.warn("Provider {} contains alias {} which is not unique and will be ignored", provider, alias);
+
                     } else {
+
                         aliasSet.add(alias);
-                    }
+                        log.debug("Local entity {} available under alias {}", key, alias);
 
-                    log.debug("Local entity {} available under alias {}", key, alias);
+                    }
 
                 } else {
 
@@ -427,30 +462,12 @@ public String getDefaultIDP() throws MetadataProviderException {
     }
 
     /**
-     * Sets name of IDP to be used as default. In case the IDP is not present (wasn't loaded from any
-     * metadata provider) runtime exception is thrown.
+     * Sets name of IDP to be used as default.
      *
      * @param defaultIDP IDP to set as default
      */
     public void setDefaultIDP(String defaultIDP) {
-
-        /*try { // TODO
-
-            lock.writeLock().lock();
-
-            if (isIDPValid(defaultIDP)) {
-                this.defaultIDP = defaultIDP;
-                return;
-            }
-
-            throw new IllegalArgumentException("Attempt to set nonexistent IDP as a default: " + defaultIDP);
-
-        } finally {
-
-            lock.writeLock().unlock();
-
-        }*/
-
+        this.defaultIDP = defaultIDP;
     }
 
     /**