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

Scott Cantor cantor.2 at osu.edu
Wed Oct 23 18:55:41 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=ee59842769637140a84b7f7d5a1c43633535fbbf

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

commit ee59842769637140a84b7f7d5a1c43633535fbbf
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Oct 23 14:55:39 2024 -0400

    IDP-2288 - Injection of beans into BeanPostProcessor causes warnings
    
    https://shibboleth.atlassian.net/browse/IDP-2288
    
    Convert bean post processor into a special Spring-aware filter.
    Auto-attach new filter to resolvers using Spring parser base class.
---
 .../metadata/AbstractMetadataProviderParser.java   |  68 +++++++++--
 .../NodeProcessingAttachingBeanPostProcessor.java  | 130 ---------------------
 .../AutoWiredNodeProcessingMetadataFilter.java     |  65 +++++++++++
 3 files changed, 122 insertions(+), 141 deletions(-)

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 e6768211..c40e24a1 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
@@ -17,12 +17,16 @@ package net.shibboleth.spring.metadata;
 import java.util.List;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 
 import org.opensaml.saml.metadata.resolver.MetadataResolver;
 import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain;
 import org.slf4j.Logger;
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Element;
@@ -37,6 +41,7 @@ import net.shibboleth.shared.spring.util.SpringSupport;
 import net.shibboleth.shared.xml.AttributeSupport;
 import net.shibboleth.shared.xml.DOMTypeSupport;
 import net.shibboleth.shared.xml.ElementSupport;
+import net.shibboleth.spring.metadata.filter.AutoWiredNodeProcessingMetadataFilter;
 
 /**
  * Parser for the MetadataProviderType in the <code>urn:mace:shibboleth:2.0:metadata</code> namespace.
@@ -65,6 +70,16 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
     /** Logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractMetadataProviderParser.class);
 
+    /** Bean ID for defaulted {@link AutoWiredNodeProcessingMetadataFilter} instance. */
+    @Nullable private final String autoWiredNodeProcessingRef;
+    
+    /** Constructor. */
+    public AbstractMetadataProviderParser() {
+        // Will warn later if not set.
+        autoWiredNodeProcessingRef = getCustomProperty(AbstractMetadataProviderParser.class.getName()
+                + ".AutoWiredNodeProcessingMetadataFilter.bean", null);
+    }
+    
     /**
      * Handle attributes which are inappropriate for specific implementations. The chaining metadata provider cannot
      * have "requireValidMetadata" or "failFastInitialization" set, even though they are present in the schema.
@@ -160,7 +175,6 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
         }
     }
 
-// Checkstyle: CyclomaticComplexity OFF
     /**
      * Parse the element into the provider builder. This has the same function as the more usual
      * {@link AbstractSingleBeanDefinitionParser#doParse(Element, ParserContext, BeanDefinitionBuilder)} but it may need
@@ -212,29 +226,61 @@ public abstract class AbstractMetadataProviderParser extends AbstractCustomBeanD
         
         processPredicateOptions(element, parserContext, builder);
 
-        final List<Element> filters =
-                ElementSupport.getChildElements(element, METADATA_FILTER_ELEMENT_NAME);
-        if (null != filters && !filters.isEmpty()) {
+        processMetadataFilters(element, parserContext, builder);
+    }
+
+    /**
+     * Handle set up of metadata filters with chaining and auto-install of node processing filter.
+     * 
+     * @param element the current element being processed
+     * @param parserContext the current parser context
+     * @param builder the current bean definition builder
+     */
+    private void processMetadataFilters(@Nonnull final Element element, @Nonnull final ParserContext parserContext, 
+            @Nonnull final BeanDefinitionBuilder builder) {
+        
+        final List<Element> filters = ElementSupport.getChildElements(element, METADATA_FILTER_ELEMENT_NAME);
+        
+        if (!filters.isEmpty()) {
+            
             if (!isChaining(element)) {
-                if (filters.size() == 1) {
-                    // Install directly.
+                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) {
+                } else if (filters.size() > 1 || autoWiredNodeProcessingRef != null) {
                     // Wrap in a chaining filter.
                     final BeanDefinitionBuilder chainBuilder =
                             BeanDefinitionBuilder.genericBeanDefinition(MetadataFilterChain.class);
-                    chainBuilder.addPropertyValue("filters", SpringSupport.parseCustomElements(filters, parserContext,
-                            chainBuilder));
+                    final ManagedList<BeanDefinition> deployerFilters =
+                            SpringSupport.parseCustomElements(filters, parserContext, chainBuilder);
+                    
+                    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());
                 }
             } else {
                 log.warn("MetadataFilter is not valid for {}", CHAINING_PROVIDER_ELEMENT_NAME.getLocalPart());
             }
+        } else if (autoWiredNodeProcessingRef != null) {
+            if (!isChaining(element)) {
+                assert autoWiredNodeProcessingRef != null;
+                builder.addPropertyReference("metadataFilter", autoWiredNodeProcessingRef);
+            }
+        } else {
+            log.warn("No bean ID installed to auto-install a NodeProcessing filter");
         }
     }
-// Checkstyle: CyclomaticComplexity ON
-
+    
     /**
      * Process predicate-related options.
      * 
diff --git a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/NodeProcessingAttachingBeanPostProcessor.java b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/NodeProcessingAttachingBeanPostProcessor.java
deleted file mode 100644
index 26a5e559..00000000
--- a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/NodeProcessingAttachingBeanPostProcessor.java
+++ /dev/null
@@ -1,130 +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 java.util.Collection;
-import java.util.List;
-
-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.opensaml.saml.metadata.resolver.filter.MetadataNodeProcessor;
-import org.opensaml.saml.metadata.resolver.filter.impl.NodeProcessingMetadataFilter;
-import org.springframework.beans.factory.BeanCreationException;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.config.BeanPostProcessor;
-import org.springframework.core.Ordered;
-
-import net.shibboleth.shared.collection.CollectionSupport;
-import net.shibboleth.shared.component.ComponentInitializationException;
-
-/**
- * A {@link BeanPostProcessor} for {@link MetadataResolver} beans that ensures a {@link NodeProcessingMetadataFilter}
- * containing a set of default {@link MetadataNodeProcessor} plugins is attached.
- * 
- * <p>
- * This is done to ensure that other components function correctly, such as the PKIX trust engine and predicates that
- * depend on group information.
- * </p>
- * 
- * <p>The constructor will auto-wire all free-standing beans, but the property setter can override these.</p>
- */
-public class NodeProcessingAttachingBeanPostProcessor implements BeanPostProcessor, Ordered {
-
-    /** The processors to install. */
-    @Nonnull private List<MetadataNodeProcessor> nodeProcessors;
-
-    /**
-     * Constructor.
-     *
-     * @param processors auto-wired processors to install
-     */
-    @Autowired
-    public NodeProcessingAttachingBeanPostProcessor(@Nullable final Collection<MetadataNodeProcessor> processors) {
-        if (processors != null) {
-            nodeProcessors = CollectionSupport.copyToList(processors);
-        } else {
-            nodeProcessors = CollectionSupport.emptyList();
-        }
-    }
-    
-    /**
-     * Set the {@link MetadataNodeProcessor} instances to auto-attach instead of the auto-wired set.
-     * 
-     * @param processors processors to auto-attach
-     * 
-     * @since 4.1.0
-     */
-    public void setNodeProcessors(@Nullable final Collection<MetadataNodeProcessor> processors) {
-        if (processors != null) {
-            // Replace auto-wired set.
-            nodeProcessors = CollectionSupport.copyToList(processors);
-        }
-    }
-
-    /** {@inheritDoc} */
-    public int getOrder() {
-        return LOWEST_PRECEDENCE;
-    }
-    
-    /** {@inheritDoc} */
-    @Override
-    @Nonnull public Object postProcessBeforeInitialization(@Nonnull final Object bean,
-            @Nonnull final String beanName) {
-        
-        if (nodeProcessors.isEmpty()) {
-            return bean;
-        }
-        
-        // Do not attach to beans which just include other ones.
-        if (!(bean instanceof MetadataResolver) || bean instanceof ChainingMetadataResolver) {
-            return bean;
-        }
-
-        final MetadataResolver resolver = (MetadataResolver) bean;
-
-        final NodeProcessingMetadataFilter filterToAttach = new NodeProcessingMetadataFilter();
-        filterToAttach.setNodeProcessors(nodeProcessors);
-        try {
-            filterToAttach.initialize();
-        } catch (final ComponentInitializationException e) {
-            throw new BeanCreationException("Error initializing NodeProcessingMetadataFilter", e);
-        }
-
-        final MetadataFilter filter = resolver.getMetadataFilter();
-        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
diff --git a/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/filter/AutoWiredNodeProcessingMetadataFilter.java b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/filter/AutoWiredNodeProcessingMetadataFilter.java
new file mode 100644
index 00000000..bf6c1dd2
--- /dev/null
+++ b/shib-metadata-spring/src/main/java/net/shibboleth/spring/metadata/filter/AutoWiredNodeProcessingMetadataFilter.java
@@ -0,0 +1,65 @@
+/*
+ * 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.filter;
+
+import java.util.Collection;
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.opensaml.saml.metadata.resolver.filter.MetadataNodeProcessor;
+import org.opensaml.saml.metadata.resolver.filter.impl.NodeProcessingMetadataFilter;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import net.shibboleth.shared.collection.CollectionSupport;
+
+/**
+ * A specialization of {@link NodeProcessingMetadataFilter} that auto-wires any
+ * {@link MetadataNodeProcessor} it finds and installs them into itself.
+ * 
+ * <p>If the setter is invoked with a non-null list, then that supplants the auto-wired set,
+ * allowing a deployer to optionally configure a bean to supply the list, and if left out
+ * the default applies.</p>
+ * 
+ * <p>The metadata resolver Spring parsing at the top level will auto-install this filter
+ * along with a "known" bean ID to supply the deployer list, replacing an approach
+ * that caused warnings from Spring.</p>
+ */
+public class AutoWiredNodeProcessingMetadataFilter extends NodeProcessingMetadataFilter {
+
+    /**
+     * Constructor.
+     *
+     * @param processors auto-wired processors to install
+     */
+    @Autowired
+    public AutoWiredNodeProcessingMetadataFilter(@Nullable final Collection<MetadataNodeProcessor> processors) {
+        if (processors != null) {
+            setNodeProcessors(CollectionSupport.copyToList(processors));
+        } else {
+            setNodeProcessors(CollectionSupport.emptyList());
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public void setNodeProcessors(@Nullable final List<MetadataNodeProcessor> newProcessors) {
+
+        if (newProcessors != null) {
+            super.setNodeProcessors(newProcessors);
+        }
+    }
+
+}
\ 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