[java-identity-provider] 03/07: IDP-1361 Deprecate <SourceAttributes> for TemplateAttrDef
Rod Widdowson
rdw at steadingsoftware.com
Sat Dec 8 09:27:26 EST 2018
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch maint-3.4
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=5088ed299b97ce68bb0e44df24a0a39b4f16e43c
commit 5088ed299b97ce68bb0e44df24a0a39b4f16e43c
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Nov 20 11:41:01 2018 +0000
IDP-1361 Deprecate <SourceAttributes> for TemplateAttrDef
https://issues.shibboleth.net/jira/browse/IDP-1361
This involves both deprecating the definition, but also using the
input attributes and checking that if the inpuit attributes are
specificied then they are also in the dependencies
---
.../ad/impl/TemplateAttributeDefinition.java | 48 ++++++++++++++++++----
.../resolver/ad/impl/TemplateAttributeTest.java | 30 +++++++++++---
.../ad/impl/TemplateAttributeDefinitionParser.java | 11 ++++-
3 files changed, 73 insertions(+), 16 deletions(-)
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
index 61a294d..c3cfbd2 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
@@ -22,6 +22,9 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.HashSet;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -34,6 +37,9 @@ import net.shibboleth.idp.attribute.UnsupportedAttributeTypeException;
import net.shibboleth.idp.attribute.resolver.AbstractAttributeDefinition;
import net.shibboleth.idp.attribute.resolver.PluginDependencySupport;
import net.shibboleth.idp.attribute.resolver.ResolutionException;
+import net.shibboleth.idp.attribute.resolver.ResolverAttributeDefinitionDependency;
+import net.shibboleth.idp.attribute.resolver.ResolverDataConnectorDependency;
+import net.shibboleth.idp.attribute.resolver.ResolverPluginDependency;
import net.shibboleth.idp.attribute.resolver.context.AttributeResolutionContext;
import net.shibboleth.idp.attribute.resolver.context.AttributeResolverWorkContext;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
@@ -175,9 +181,7 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
throw new ComponentInitializationException(getLogPrefix() + " no velocity engine was configured");
}
- if (sourceAttributes.isEmpty()) {
- log.info("{} No Source Attributes supplied, was this intended?", getLogPrefix());
- }
+ checkSourceAttributes();
if (null == templateText) {
// V2 compatibility - define our own template
@@ -197,6 +201,33 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
template = Template.fromTemplate(engine, templateText);
}
+ /**
+ * Check the provided source attributes against the provided dependencies.
+ */
+ private void checkSourceAttributes() {
+ if (sourceAttributes.isEmpty()) {
+ return;
+ }
+
+ final Set<String> dependencyAttributeNames = new HashSet<>(getDependencies().size());
+ for (final ResolverPluginDependency dependency: getDependencies()) {
+ if (dependency instanceof ResolverAttributeDefinitionDependency) {
+ dependencyAttributeNames.add( dependency.getDependencyPluginId());
+ } else if (dependency instanceof ResolverDataConnectorDependency) {
+ final ResolverDataConnectorDependency dc = (ResolverDataConnectorDependency) dependency;
+ if (dc.isAllAttributes()) {
+ return;
+ }
+ }
+ }
+
+ for (final String s: sourceAttributes) {
+ if (!dependencyAttributeNames.contains(s)) {
+ log.warn("{} Source Attribute {} is not provided as a dependency", getLogPrefix(),s);
+ }
+ }
+ }
+
/** {@inheritDoc} */
@Override @Nonnull protected IdPAttribute doAttributeDefinitionResolve(
@Nonnull final AttributeResolutionContext resolutionContext,
@@ -279,13 +310,12 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
final Map<String, List<IdPAttributeValue<?>>> dependencyAttributes =
PluginDependencySupport.getAllAttributeValues(workContext, getDependencies());
-
int valueCount = 0;
boolean valueCountSet = false;
- for (final String attributeName : sourceAttributes) {
+ for (final Entry<String, List<IdPAttributeValue<?>>> entry : dependencyAttributes.entrySet() ) {
- List<IdPAttributeValue<?>> attributeValues = dependencyAttributes.get(attributeName);
+ List<IdPAttributeValue<?>> attributeValues = entry.getValue();
if (null == attributeValues) {
attributeValues = Collections.emptyList();
}
@@ -295,12 +325,12 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
valueCountSet = true;
} else if (attributeValues.size() != valueCount) {
final String msg = getLogPrefix() + " All source attributes used in"
- + " TemplateAttributeDefinition must have the same number of values: '" + attributeName + "'" ;
- log.error(msg);
+ + " TemplateAttributeDefinition must have the same number of values: '" + entry.getKey() + "'" ;
+ log.error("{} {}", getLogPrefix(), msg);
throw new ResolutionException(msg);
}
- sourceValues.put(attributeName, attributeValues.iterator());
+ sourceValues.put(entry.getKey(), attributeValues.iterator());
}
return valueCount;
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
index 30ae8b2..616ae4a 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
@@ -204,7 +204,6 @@ public class TemplateAttributeTest {
// ds.add(TestSources.makeResolverPluginDependency(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_CONNECTOR,
// TestSources.STATIC_ATTRIBUTE_NAME));
templateDef.setDependencies(ds);
- templateDef.setSourceAttributes(Collections.singletonList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
templateDef.initialize();
final Set<AttributeDefinition> attrDefinitions = new LazySet<>();
@@ -233,6 +232,27 @@ public class TemplateAttributeTest {
* @throws ComponentInitializationException if it goes wrong.
*/
@Test public void templateWithValues() throws ResolutionException, ComponentInitializationException {
+ templateWithValues(false);
+ }
+
+ /**
+ * Test resolution of an template script with data generated from the attributes, but with
+ * explicit setting of source attributes.
+ *
+ * @throws ResolutionException if it goes wrong.
+ * @throws ComponentInitializationException if it goes wrong.
+ */
+ @Test public void templateWithValuesTestSources() throws ResolutionException, ComponentInitializationException {
+ templateWithValues(false);
+ }
+
+ /** Worker function for the templateWithValues and templateWithValuesTestSources tests.
+ *
+ * @param setSources whether to all {@link TemplateAttributeDefinition#setSourceAttributes(List)}
+ * @throws ResolutionException if it goes wrong.
+ * @throws ComponentInitializationException if it goes wrong.
+ */
+ private final void templateWithValues(boolean setSources) throws ResolutionException, ComponentInitializationException {
final String name = TEST_ATTRIBUTE_BASE_NAME + "3";
@@ -247,8 +267,9 @@ public class TemplateAttributeTest {
ds.add(TestSources.makeResolverPluginDependency(TestSources.STATIC_CONNECTOR_NAME,
TestSources.DEPENDS_ON_SECOND_ATTRIBUTE_NAME));
templateDef.setDependencies(ds);
- templateDef.setSourceAttributes(Arrays.asList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR,
- TestSources.DEPENDS_ON_SECOND_ATTRIBUTE_NAME));
+ if (setSources) {
+ templateDef.setSourceAttributes(Collections.singletonList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
+ }
templateDef.initialize();
final Set<AttributeDefinition> attrDefinitions = new LazySet<>();
@@ -286,7 +307,6 @@ public class TemplateAttributeTest {
ds.add(TestSources.makeResolverPluginDependency(TestSources.STATIC_ATTRIBUTE_NAME,
TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
templateDef.setDependencies(ds);
- templateDef.setSourceAttributes(Arrays.asList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
templateDef.initialize();
final List<IdPAttributeValue<?>> values = new ArrayList<>();
@@ -331,7 +351,6 @@ public class TemplateAttributeTest {
TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
ds.add(TestSources.makeResolverPluginDependency(otherDefName, otherAttrName));
templateDef.setDependencies(ds);
- templateDef.setSourceAttributes(Arrays.asList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR, otherAttrName));
templateDef.initialize();
final Set<AttributeDefinition> attrDefinitions = new LazySet<>();
@@ -363,7 +382,6 @@ public class TemplateAttributeTest {
ds.add(TestSources.makeResolverPluginDependency(TestSources.STATIC_ATTRIBUTE_NAME,
TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
templateDef.setDependencies(ds);
- templateDef.setSourceAttributes(Collections.singletonList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
templateDef.initialize();
diff --git a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
index ef30672..b229346 100644
--- a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
+++ b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
@@ -25,6 +25,8 @@ import javax.xml.namespace.QName;
import net.shibboleth.idp.attribute.resolver.ad.impl.TemplateAttributeDefinition;
import net.shibboleth.idp.attribute.resolver.spring.impl.AttributeResolverNamespaceHandler;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
import net.shibboleth.utilities.java.support.xml.ElementSupport;
@@ -79,7 +81,11 @@ public class TemplateAttributeDefinitionParser extends AbstractWarningAttributeD
final List<Element> templateElements = ElementSupport.getChildElements(config, TEMPLATE_ELEMENT_NAME_AD);
templateElements.addAll(ElementSupport.getChildElements(config, TEMPLATE_ELEMENT_NAME_RESOLVER));
- if (null != templateElements && templateElements.size() >= 1) {
+ if (null == templateElements || templateElements.isEmpty()) {
+ DeprecationSupport.warnOnce(ObjectType.ELEMENT, "Missing " + TEMPLATE_ELEMENT_NAME_RESOLVER.getLocalPart(),
+ parserContext.getReaderContext().getResource().getDescription(),
+ "by providing an explicit template");
+ } else {
if (templateElements.size() > 1) {
log.warn("{} Too many <Template> elements, taking the first");
}
@@ -94,6 +100,9 @@ public class TemplateAttributeDefinitionParser extends AbstractWarningAttributeD
ElementSupport.getChildElements(config, SOURCE_ATTRIBUTE_ELEMENT_NAME_AD);
sourceAttributeElements.addAll(ElementSupport.getChildElements(config, SOURCE_ATTRIBUTE_ELEMENT_NAME_RESOLVER));
if (null != sourceAttributeElements) {
+ DeprecationSupport.warnOnce(ObjectType.ELEMENT, SOURCE_ATTRIBUTE_ELEMENT_NAME_RESOLVER.getLocalPart(),
+ parserContext.getReaderContext().getResource().getDescription(),
+ "by using <InputAttributeDefinition> and <InputDataConnector>");
final List<String> sourceAttributes = new ManagedList<>(sourceAttributeElements.size());
for (final Element element : sourceAttributeElements) {
sourceAttributes.add(StringSupport.trimOrNull(element.getTextContent()));
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list