[java-shib-metadata] branch main updated: IDP-2288 - Injection of beans into BeanPostProcessor causes warnings

Scott Cantor cantor.2 at osu.edu
Thu Oct 24 13:21:52 UTC 2024


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-shib-metadata.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-metadata.git;a=commit;h=bdba93735d61d1bfa175271319ee7b64a95b9b94

The following commit(s) were added to refs/heads/main by this push:
     new bdba9373 IDP-2288 - Injection of beans into BeanPostProcessor causes warnings
bdba9373 is described below

commit bdba93735d61d1bfa175271319ee7b64a95b9b94
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Oct 24 09:21:49 2024 -0400

    IDP-2288 - Injection of beans into BeanPostProcessor causes warnings
    
    https://shibboleth.atlassian.net/browse/IDP-2288
    
    Migrate ByReference filter into Spring parser also to fix ordering.
---
 .../impl/ByReferenceMetadataFilterBridge.java      |  28 ++++-
 .../metadata/AbstractMetadataProviderParser.java   |  92 ++++++++++------
 .../ByReferenceFilterBeanPostProcessor.java        | 116 ---------------------
 3 files changed, 87 insertions(+), 149 deletions(-)

diff --git a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ByReferenceMetadataFilterBridge.java b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ByReferenceMetadataFilterBridge.java
index 5021bd5a..a9837d81 100644
--- a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ByReferenceMetadataFilterBridge.java
+++ b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ByReferenceMetadataFilterBridge.java
@@ -29,6 +29,7 @@ import org.opensaml.saml.metadata.resolver.filter.impl.ByReferenceMetadataFilter
 import org.slf4j.Logger;
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
 
 import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.primitive.LoggerFactory;
@@ -37,17 +38,36 @@ import net.shibboleth.shared.primitive.LoggerFactory;
  * This is a bridge filter that uses Spring to locate extant {@link ByReferenceMetadataFilter}
  * objects to run.
  */
-public class ByReferenceMetadataFilterBridge extends AbstractMetadataFilter {
+public class ByReferenceMetadataFilterBridge extends AbstractMetadataFilter implements ApplicationContextAware {
 
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(ByReferenceMetadataFilterBridge.class);
 
+    /** Whether to actually enable this filter. */
+    private boolean enabled;
+    
     /** Application context. */
     @Nullable private ApplicationContext applicationContext;
     
     /** Chain to wrap the beans obtained from the context. */
     @Nullable private MetadataFilterChain filterChain;
     
+    /** Constructor. */
+    public ByReferenceMetadataFilterBridge() {
+        enabled = true;
+    }
+    
+    /**
+     * Sets whether to enable this filter.
+     * 
+     * <p>Defaults to true.</p>
+     * 
+     * @param flag flag to set
+     */
+    public void setEnabled(final boolean flag) {
+        enabled = flag;
+    }
+    
     /**
      * Set the containing {@link ApplicationContext}.
      * 
@@ -61,6 +81,10 @@ public class ByReferenceMetadataFilterBridge extends AbstractMetadataFilter {
     @Nullable public XMLObject filter(@Nullable final XMLObject metadata, @Nonnull final MetadataFilterContext context)
             throws FilterException {
 
+        if (!enabled) {
+            return metadata;
+        }
+        
         MetadataFilterChain chain = null;
         
         synchronized(this) {
@@ -83,7 +107,7 @@ public class ByReferenceMetadataFilterBridge extends AbstractMetadataFilter {
             }
         }
         
-        return chain != null ? chain.filter(metadata, context) : metadata;
+        return chain.filter(metadata, context);
     }
 
 }
\ No newline at end of file
diff --git a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/AbstractMetadataProviderParser.java b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/AbstractMetadataProviderParser.java
index c40e24a1..030e358f 100644
--- a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/AbstractMetadataProviderParser.java
+++ b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/AbstractMetadataProviderParser.java
@@ -32,6 +32,7 @@ import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
+import net.shibboleth.idp.saml.metadata.impl.ByReferenceMetadataFilterBridge;
 import net.shibboleth.idp.saml.metadata.impl.MetadataProviderContainer;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.primitive.LoggerFactory;
@@ -70,14 +71,26 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
     /** Logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractMetadataProviderParser.class);
 
+    /** Bean ID for defaulted {@link ByReferenceMetadataFilterBridge} instance. */
+    @Nullable private final String byReferenceBridgeRef;
+
     /** Bean ID for defaulted {@link AutoWiredNodeProcessingMetadataFilter} instance. */
     @Nullable private final String autoWiredNodeProcessingRef;
     
     /** Constructor. */
     public AbstractMetadataProviderParser() {
-        // Will warn later if not set.
+        byReferenceBridgeRef = getCustomProperty(AbstractMetadataProviderParser.class.getName()
+                + ".ByReferenceMetadataFilterBridge.bean", null);
+        if (byReferenceBridgeRef == null) {
+            log.warn("No bean ID installed to auto-install a ByReference bridge filter");
+        }
+
         autoWiredNodeProcessingRef = getCustomProperty(AbstractMetadataProviderParser.class.getName()
                 + ".AutoWiredNodeProcessingMetadataFilter.bean", null);
+        if (autoWiredNodeProcessingRef == null) {
+            log.warn("No bean ID installed to auto-install a NodeProcessing filter");
+        }
+                
     }
     
     /**
@@ -229,6 +242,7 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
         processMetadataFilters(element, parserContext, builder);
     }
 
+// Checkstyle: CyclomaticComplexity OFF
     /**
      * Handle set up of metadata filters with chaining and auto-install of node processing filter.
      * 
@@ -240,46 +254,62 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
             @Nonnull final BeanDefinitionBuilder builder) {
         
         final List<Element> filters = ElementSupport.getChildElements(element, METADATA_FILTER_ELEMENT_NAME);
+
+        if (isChaining(element)) {
+            if (!filters.isEmpty()) {
+                log.warn("MetadataFilter is not valid for {}", CHAINING_PROVIDER_ELEMENT_NAME.getLocalPart());
+            }
+            return;
+        }
         
         if (!filters.isEmpty()) {
-            
-            if (!isChaining(element)) {
-                if (filters.size() == 1 && autoWiredNodeProcessingRef == null) {
-                    // Install directly and log the fact that we can't auto-install ours.
-                    log.warn("No bean ID installed to auto-install a NodeProcessing filter");
-                    builder.addPropertyValue("metadataFilter",
-                            SpringSupport.parseCustomElement(filters.get(0), parserContext, builder, false));
-                } else if (filters.size() > 1 || autoWiredNodeProcessingRef != null) {
-                    // Wrap in a chaining filter.
-                    final BeanDefinitionBuilder chainBuilder =
-                            BeanDefinitionBuilder.genericBeanDefinition(MetadataFilterChain.class);
-                    final ManagedList<BeanDefinition> deployerFilters =
-                            SpringSupport.parseCustomElements(filters, parserContext, chainBuilder);
-                    
+            if (filters.size() == 1 && byReferenceBridgeRef == null && autoWiredNodeProcessingRef == null) {
+                // Install directly.
+                builder.addPropertyValue("metadataFilter",
+                        SpringSupport.parseCustomElement(filters.get(0), parserContext, builder, false));
+            } else {
+                // Wrap deployer filters plus the built-ins in a chaining filter.
+                final BeanDefinitionBuilder chainBuilder =
+                        BeanDefinitionBuilder.genericBeanDefinition(MetadataFilterChain.class);
+                final ManagedList<BeanDefinition> deployerFilters =
+                        SpringSupport.parseCustomElements(filters, parserContext, chainBuilder);
+                
+                if (byReferenceBridgeRef != null || autoWiredNodeProcessingRef != null) {
+                    // Wire up a chaining filter with the deployer filters AND either/both of our built-ins.
+                    // ByRef comes before NodeProcessor.
+                    final ManagedList<Object> filtersToInstall = new ManagedList<>();
+                    filtersToInstall.addAll(deployerFilters);
+                    if (byReferenceBridgeRef != null) {
+                        filtersToInstall.add(new RuntimeBeanReference(byReferenceBridgeRef));
+                    }
                     if (autoWiredNodeProcessingRef != null) {
-                        final ManagedList<Object> filtersToInstall = new ManagedList<>();
-                        assert autoWiredNodeProcessingRef != null;
                         filtersToInstall.add(new RuntimeBeanReference(autoWiredNodeProcessingRef));
-                        filtersToInstall.addAll(deployerFilters);
-                        chainBuilder.addPropertyValue("filters", filtersToInstall);
-                    } else {
-                        log.warn("No bean ID installed to auto-install a NodeProcessing filter");
-                        chainBuilder.addPropertyValue("filters", deployerFilters);
                     }
-                    builder.addPropertyValue("metadataFilter", chainBuilder.getBeanDefinition());
+                    chainBuilder.addPropertyValue("filters", filtersToInstall);
+                } else {
+                    // No built-ins available, so just wire up the deployer filters.
+                    chainBuilder.addPropertyValue("filters", deployerFilters);
                 }
-            } else {
-                log.warn("MetadataFilter is not valid for {}", CHAINING_PROVIDER_ELEMENT_NAME.getLocalPart());
+                // Use the new chaining filter as the actual provider filter.
+                builder.addPropertyValue("metadataFilter", chainBuilder.getBeanDefinition());
             }
-        } else if (autoWiredNodeProcessingRef != null) {
-            if (!isChaining(element)) {
-                assert autoWiredNodeProcessingRef != null;
-                builder.addPropertyReference("metadataFilter", autoWiredNodeProcessingRef);
+        } else if (byReferenceBridgeRef != null || autoWiredNodeProcessingRef != null) {
+            // Wire up a chaining filter with either/both of our built-ins.
+            // ByRef comes before NodeProcessor.
+            final ManagedList<Object> filtersToInstall = new ManagedList<>();
+            if (byReferenceBridgeRef != null) {
+                filtersToInstall.add(new RuntimeBeanReference(byReferenceBridgeRef));
             }
-        } else {
-            log.warn("No bean ID installed to auto-install a NodeProcessing filter");
+            if (autoWiredNodeProcessingRef != null) {
+                filtersToInstall.add(new RuntimeBeanReference(autoWiredNodeProcessingRef));
+            }
+            final BeanDefinitionBuilder chainBuilder =
+                    BeanDefinitionBuilder.genericBeanDefinition(MetadataFilterChain.class);
+            chainBuilder.addPropertyValue("filters", filtersToInstall);
+            builder.addPropertyValue("metadataFilter", chainBuilder.getBeanDefinition());
         }
     }
+// Checkstyle: CyclomaticComplexity ON
     
     /**
      * Process predicate-related options.
diff --git a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/ByReferenceFilterBeanPostProcessor.java b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/ByReferenceFilterBeanPostProcessor.java
deleted file mode 100644
index f7860dba..00000000
--- a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/ByReferenceFilterBeanPostProcessor.java
+++ /dev/null
@@ -1,116 +0,0 @@
-/*
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package net.shibboleth.spring.metadata;
-
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver;
-import org.opensaml.saml.metadata.resolver.MetadataResolver;
-import org.opensaml.saml.metadata.resolver.filter.MetadataFilter;
-import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain;
-import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.config.BeanPostProcessor;
-import org.springframework.context.ApplicationContext;
-import org.springframework.context.ApplicationContextAware;
-import org.springframework.core.Ordered;
-
-import net.shibboleth.idp.saml.metadata.impl.ByReferenceMetadataFilterBridge;
-import net.shibboleth.shared.collection.CollectionSupport;
-
-/**
- * A {@link BeanPostProcessor} for {@link MetadataResolver} beans that ensures a {@link ByReferenceMetadataFilterBridge}
- * is attached.
- * 
- * @since 4.0.0
- */
-public class ByReferenceFilterBeanPostProcessor implements BeanPostProcessor, ApplicationContextAware, Ordered {
-
-    /** Whether to enable the processor. */
-    private boolean enabled;
-    
-    /** Spring context. */
-    @Nullable private ApplicationContext applicationContext;
-
-    /** Constructor. */
-    public ByReferenceFilterBeanPostProcessor() {
-        enabled = true;
-    }
-    
-    /**
-     * Set whether to enable the processor.
-     * 
-     * @param flag flag to set
-     */
-    public void setEnabled(final boolean flag) {
-        enabled = flag;
-    }
-    
-    /** {@inheritDoc} */
-    public int getOrder() {
-        return HIGHEST_PRECEDENCE;
-    }
-
-    /** {@inheritDoc} */
-    public void setApplicationContext(@Nullable final ApplicationContext context) throws BeansException {
-        applicationContext = context;
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    @Nonnull public Object postProcessBeforeInitialization(@Nonnull final Object bean, @Nonnull final String beanName) {
-        
-        // Do not attach to beans which just include other ones.
-        if (!enabled || !(bean instanceof MetadataResolver) || bean instanceof ChainingMetadataResolver) {
-            return bean;
-        }
-
-        final MetadataResolver resolver = (MetadataResolver) bean;
-
-        boolean filterAttached = false;
-
-        final MetadataFilter filter = resolver.getMetadataFilter();
-        if (filter instanceof ByReferenceMetadataFilterBridge) {
-            filterAttached = true;
-        } else if (filter instanceof MetadataFilterChain) {
-            filterAttached = ((MetadataFilterChain) filter).getFilters().stream().anyMatch(
-                    f -> f instanceof ByReferenceMetadataFilterBridge);
-        }
-
-        if (!filterAttached) {
-            final ByReferenceMetadataFilterBridge filterToAttach = new ByReferenceMetadataFilterBridge();
-            filterToAttach.setApplicationContext(applicationContext);
-
-            if (filter == null) {
-                resolver.setMetadataFilter(filterToAttach);
-            } else if (filter instanceof MetadataFilterChain) {
-                ((MetadataFilterChain) filter).getFilters().add(filterToAttach);
-            } else {
-                final MetadataFilterChain chain = new MetadataFilterChain();
-                chain.setFilters(CollectionSupport.listOf(filter, filterToAttach));
-                resolver.setMetadataFilter(chain);
-            }
-        }
-
-        return resolver;
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    @Nonnull public Object postProcessAfterInitialization(@Nonnull final Object bean, @Nonnull final String beanName) {
-        return bean;
-    }
-
-}
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list