[java-identity-provider] 01/03: IDP-1811, IDP-1817 Push Attribute Information collection down.

Rod Widdowson rdw at steadingsoftware.com
Wed May 5 15:43:25 UTC 2021


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

rdw pushed a commit to branch dev/IDP-1811
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=600c0dbbc2ad57843d4db37ed336daa942cb8516

commit 600c0dbbc2ad57843d4db37ed336daa942cb8516
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed May 5 14:10:57 2021 +0100

    IDP-1811, IDP-1817 Push Attribute Information collection down.
    
    https://issues.shibboleth.net/jira/browse/IDP-1811
    https://issues.shibboleth.net/jira/browse/IDP-1818
    
    The work is now done in the consent flow, making it
    an optional step in attribute resolution.
---
 .../idp/flows/intercept/attribute-release-flow.xml |   4 +-
 .../impl/AbstractAttributeDisplayFunction.java     |  53 ++++++++--
 .../impl/AttributeDisplayDescriptionFunction.java  |  13 ++-
 .../logic/impl/AttributeDisplayNameFunction.java   |  14 ++-
 ...ttributeDisplayNameDescriptionFunctionTest.java | 114 +++++++++++++++++++--
 5 files changed, 170 insertions(+), 28 deletions(-)

diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/intercept/attribute-release-flow.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/intercept/attribute-release-flow.xml
index a82a988fb..887cf62da 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/intercept/attribute-release-flow.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/intercept/attribute-release-flow.xml
@@ -125,9 +125,9 @@
             <evaluate expression="flowRequestContext.getExternalContext().getNativeRequest()" result="viewScope.request" />
             <evaluate expression="flowRequestContext.getExternalContext().getNativeResponse()" result="viewScope.response" />
             <evaluate expression="opensamlProfileRequestContext" result="viewScope.profileRequestContext" />
-            <evaluate expression="new net.shibboleth.idp.consent.logic.impl.AttributeDisplayNameFunction(flowRequestContext.getExternalContext().getNativeRequest(), FallbackLanguages)" 
+            <evaluate expression="new net.shibboleth.idp.consent.logic.impl.AttributeDisplayNameFunction(flowRequestContext.getExternalContext().getNativeRequest(), FallbackLanguages, flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.AttributeRegistryService'))"
                                                                                                result="viewScope.attributeDisplayNameFunction" />
-            <evaluate expression="new net.shibboleth.idp.consent.logic.impl.AttributeDisplayDescriptionFunction(flowRequestContext.getExternalContext().getNativeRequest(), FallbackLanguages)" 
+            <evaluate expression="new net.shibboleth.idp.consent.logic.impl.AttributeDisplayDescriptionFunction(flowRequestContext.getExternalContext().getNativeRequest(), FallbackLanguages, flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.AttributeRegistryService'))"
                                                                                                result="viewScope.attributeDisplayDescriptionFunction" />
             <evaluate expression="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.consent.context.ConsentContext))" result="viewScope.consentContext" />
             <evaluate expression="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.consent.context.AttributeReleaseContext))" result="viewScope.attributeReleaseContext" />
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AbstractAttributeDisplayFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AbstractAttributeDisplayFunction.java
index c3714626f..504e6864c 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AbstractAttributeDisplayFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AbstractAttributeDisplayFunction.java
@@ -18,6 +18,7 @@
 package net.shibboleth.idp.consent.logic.impl;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Locale.LanguageRange;
@@ -31,13 +32,20 @@ import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.ext.spring.util.SpringSupport;
 import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.transcoding.AttributeTranscoderRegistry;
 import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
+import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.service.ReloadableService;
+import net.shibboleth.utilities.java.support.service.ServiceableComponent;
 
 /**
  * Abstract Function which returns {@link Locale}-aware information about an attribute. The abstract method
- * {@link #getDisplayInfo(IdPAttribute)} returns the information selected from the attribute. This function defaults to
- * returning the attribute ID if no information is selected from the attribute for the desired locales.
+ * {@link #getDisplayInfo(AttributeTranscoderRegistry, IdPAttribute)} returns the information selected for
+ * the attribute from the transcoder.
+ * 
+ * This function defaults to returning the attribute ID if no information is selected from the attribute
+ * for the desired locales.
  */
 public abstract class AbstractAttributeDisplayFunction implements Function<IdPAttribute, String> {
 
@@ -47,17 +55,25 @@ public abstract class AbstractAttributeDisplayFunction implements Function<IdPAt
     /** The tags for the fallback languages. */
     @Nonnull @Unmodifiable private final List<Locale.LanguageRange> defaultLanguageRange;
 
+    /** Cache of already looked up values. */
+    @Nonnull private Map<IdPAttribute, Map<Locale, String>> cachedInfo = new HashMap<>();
+
+    /** How to do the lookup. */
+    @Nonnull private ReloadableService<AttributeTranscoderRegistry> transcoder;
     
     /**
      * Constructor.
      * 
      * @param request {@link HttpServletRequest} used to get preferred languages
      * @param defaultLanguages list of fallback languages in order of decreasing preference
+     * @param transcoderService the attribute transcoder service
      */
     public AbstractAttributeDisplayFunction(@Nonnull final HttpServletRequest request,
-            @Nullable final List<String> defaultLanguages) {
+            @Nullable final List<String> defaultLanguages,
+            final ReloadableService<AttributeTranscoderRegistry> transcoderService) {
 
         languageRange = SpringSupport.getLanguageRange(request);
+        transcoder = Constraint.isNotNull(transcoderService, "Injected transocde service should be non-null");
         if (defaultLanguages == null || defaultLanguages.isEmpty()) {
             defaultLanguageRange = Collections.emptyList();
         } else {
@@ -75,8 +91,22 @@ public abstract class AbstractAttributeDisplayFunction implements Function<IdPAt
         if (input == null) {
             return "N/A";
         }
-        
-        final Map<Locale, String> displayInfo = getDisplayInfo(input);
+
+        Map<Locale, String> displayInfo = cachedInfo.get(input);
+        if (displayInfo == null) {
+            ServiceableComponent<AttributeTranscoderRegistry> component = null;
+            try {
+                component = transcoder.getServiceableComponent();
+                if (component != null) {
+                    displayInfo = getDisplayInfo(component.getComponent(), input);
+                }
+            } finally {
+                if (component != null) {
+                    component.unpinComponent();
+                }
+            }
+            cachedInfo.put(input, displayInfo);
+        }
         
         Locale locale = Locale.lookup(languageRange, displayInfo.keySet());
         if (locale == null) {
@@ -85,14 +115,21 @@ public abstract class AbstractAttributeDisplayFunction implements Function<IdPAt
         if (locale == null) {
             return input.getId();
         }
-        return displayInfo.get(locale);
+        final String result = displayInfo.get(locale);
+        if (result == null) {
+            return input.getId();
+        }
+        return result;
     }
 
     /**
      * Get the information to be displayed from the attribute.
      * 
-     * @param input the attribute to consider
+     * @param registry the {@link AttributeTranscoderRegistry} to ask.
+     * @param attribute the attribute to consider
      * @return the map of locale dependent information to be displayed
      */
-    @Nonnull protected abstract Map<Locale, String> getDisplayInfo(@Nonnull final IdPAttribute input);
+    @Nonnull protected abstract Map<Locale, String> getDisplayInfo(
+            @Nonnull final AttributeTranscoderRegistry registry,
+            @Nonnull final IdPAttribute attribute);
 }
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
index 3ce732da6..c2f743b9b 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
@@ -26,6 +26,8 @@ import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.transcoding.AttributeTranscoderRegistry;
+import net.shibboleth.utilities.java.support.service.ReloadableService;
 
 /**
  * Function which returns the locale-aware display description of an attribute, defaulting to the
@@ -38,14 +40,17 @@ public class AttributeDisplayDescriptionFunction extends AbstractAttributeDispla
      * 
      * @param request {@link HttpServletRequest} used to get preferred languages
      * @param defaultLangauages list of fallback languages in order of decreasing preference
+     * @param transcoderService the attribute transcoder service
      */
     public AttributeDisplayDescriptionFunction(@Nonnull final HttpServletRequest request,
-            @Nullable final List<String> defaultLangauages) {
-        super(request, defaultLangauages);
+            @Nullable final List<String> defaultLangauages,
+            final ReloadableService<AttributeTranscoderRegistry> transcoderService) {
+        super(request, defaultLangauages, transcoderService);
     }
 
     /** {@inheritDoc} */
-    @Override @Nonnull protected Map<Locale, String> getDisplayInfo(@Nonnull final IdPAttribute attribute) {
-        return attribute.getDisplayDescriptions();
+    protected Map<Locale, String> getDisplayInfo( @Nonnull final AttributeTranscoderRegistry registry,
+            @Nonnull final IdPAttribute attribute) {
+        return registry.getDescriptions(attribute);
     }
 }
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameFunction.java
index a65b8215a..a5f1073b7 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameFunction.java
@@ -26,6 +26,8 @@ import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.transcoding.AttributeTranscoderRegistry;
+import net.shibboleth.utilities.java.support.service.ReloadableService;
 
 /**
  * Function which returns the locale-aware display name of an attribute, defaulting to the
@@ -38,14 +40,18 @@ public class AttributeDisplayNameFunction extends AbstractAttributeDisplayFuncti
      * 
      * @param request {@link HttpServletRequest} used to get preferred languages
      * @param defaultLangauages list of fallback languages in order of decreasing preference
+     * @param transcoderService the attribute transcoder service
      */
     public AttributeDisplayNameFunction(@Nonnull final HttpServletRequest request,
-            @Nullable final List<String> defaultLangauages) {
-        super(request, defaultLangauages);
+            @Nullable final List<String> defaultLangauages,
+            final ReloadableService<AttributeTranscoderRegistry> transcoderService) {
+        super(request, defaultLangauages, transcoderService);
     }
 
     /** {@inheritDoc} */
-    @Override @Nonnull protected Map<Locale, String> getDisplayInfo(@Nonnull final IdPAttribute attribute) {
-        return attribute.getDisplayNames();
+    @Override @Nonnull protected Map<Locale, String> getDisplayInfo(
+            @Nonnull final AttributeTranscoderRegistry registry,
+            @Nonnull final IdPAttribute attribute) {
+        return  registry.getDisplayNames(attribute);
     }
 }
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameDescriptionFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameDescriptionFunctionTest.java
index 944d5465d..db5dc86ed 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameDescriptionFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayNameDescriptionFunctionTest.java
@@ -17,7 +17,9 @@
 
 package net.shibboleth.idp.consent.logic.impl;
 
+import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -27,6 +29,11 @@ import java.util.function.Function;
 import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.transcoding.AttributeTranscoderRegistry;
+import net.shibboleth.idp.attribute.transcoding.TranscodingRule;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.service.ReloadableService;
+import net.shibboleth.utilities.java.support.service.ServiceableComponent;
 
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.testng.Assert;
@@ -36,10 +43,13 @@ import org.testng.annotations.Test;
 /**
  * {@link AttributeDisplayNameFunction} and {@link AttributeDisplayDescriptionFunction} unit tests.
  */
+ at SuppressWarnings("javadoc")
 public class AttributeDisplayNameDescriptionFunctionTest {
     
     private IdPAttribute testAttribute;
 
+    private MockService service = new MockService();
+
     @BeforeClass public void constructAttribute() {
         final IdPAttribute attr = new IdPAttribute("What");
 
@@ -71,38 +81,38 @@ public class AttributeDisplayNameDescriptionFunctionTest {
     }
     
     @Test public void testNameHttpOnly() {
-        Function<IdPAttribute, String> func = new AttributeDisplayNameFunction(getMockRequest("fr", "de", "en"), null);
+        Function<IdPAttribute, String> func = new AttributeDisplayNameFunction(getMockRequest("fr", "de", "en"), null, service);
         Assert.assertEquals(func.apply(testAttribute), "FR locale Name");
 
-        func = new AttributeDisplayNameFunction(getMockRequest("pt", "es"), null);
+        func = new AttributeDisplayNameFunction(getMockRequest("pt", "es"), null, service);
         Assert.assertEquals(func.apply(testAttribute), testAttribute.getId());
     }
 
     @Test public void testNameWithDefault() {
         List<String> fallback = List.of("en", "fr", "de");
         
-        Function<IdPAttribute, String> func = new AttributeDisplayNameFunction(getMockRequest("fr", "de", "en"), fallback);
+        Function<IdPAttribute, String> func = new AttributeDisplayNameFunction(getMockRequest("fr", "de", "en"), fallback, service);
         Assert.assertEquals(func.apply(testAttribute), "FR locale Name");
 
-        func = new AttributeDisplayNameFunction(getMockRequest("pt", "es"), fallback);
+        func = new AttributeDisplayNameFunction(getMockRequest("pt", "es"), fallback, service);
         Assert.assertEquals(func.apply(testAttribute), "EN locale Name");
     }
 
     @Test public void testDescHttpOnly() {
-        Function<IdPAttribute, String> func = new AttributeDisplayDescriptionFunction(getMockRequest("fr", "de", "en"), null);
+        Function<IdPAttribute, String> func = new AttributeDisplayDescriptionFunction(getMockRequest("fr", "de", "en"), null, service);
         Assert.assertEquals(func.apply(testAttribute), "FR locale Description");
 
-        func = new AttributeDisplayDescriptionFunction(getMockRequest("pt", "es"), null);
+        func = new AttributeDisplayDescriptionFunction(getMockRequest("pt", "es"), null, service);
         Assert.assertEquals(func.apply(testAttribute), testAttribute.getId());
     }
 
     @Test public void testDescWithDefault() {
         List<String> fallback = List.of("en", "fr", "de");
         
-        Function<IdPAttribute, String> func = new AttributeDisplayDescriptionFunction(getMockRequest("fr", "de", "en"), fallback);
+        Function<IdPAttribute, String> func = new AttributeDisplayDescriptionFunction(getMockRequest("fr", "de", "en"), fallback, service);
         Assert.assertEquals(func.apply(testAttribute), "FR locale Description");
 
-        func = new AttributeDisplayDescriptionFunction(getMockRequest("pt", "es"), fallback);
+        func = new AttributeDisplayDescriptionFunction(getMockRequest("pt", "es"), fallback, service);
         Assert.assertEquals(func.apply(testAttribute), "EN locale Description");
     }
 
@@ -112,10 +122,94 @@ public class AttributeDisplayNameDescriptionFunctionTest {
         fallback.add("");
         fallback.add("fr");
         
-        Function<IdPAttribute, String> displayNameFunc = new AttributeDisplayNameFunction(getMockRequest("pt", "es"), fallback);
+        Function<IdPAttribute, String> displayNameFunc = new AttributeDisplayNameFunction(
+                getMockRequest("pt", "es"), fallback, service);
         Assert.assertEquals(displayNameFunc.apply(testAttribute), "FR locale Name");
         
-        Function<IdPAttribute, String> descFunc = new AttributeDisplayDescriptionFunction(getMockRequest("pt", "es"), fallback);
+        Function<IdPAttribute, String> descFunc = new AttributeDisplayDescriptionFunction(
+                getMockRequest("pt", "es"), fallback, service);
         Assert.assertEquals(descFunc.apply(testAttribute), "FR locale Description");
     }
+
+    private final static class MockService implements
+        ReloadableService<AttributeTranscoderRegistry>,
+        ServiceableComponent<AttributeTranscoderRegistry>,
+        AttributeTranscoderRegistry
+    {
+
+        /** {@inheritDoc} */
+        public boolean isInitialized() {
+            return true;
+        }
+
+        /** {@inheritDoc} */
+        public void initialize() throws ComponentInitializationException {
+        }
+
+        /** {@inheritDoc} */
+        public Instant getLastSuccessfulReloadInstant() {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public Instant getLastReloadAttemptInstant() {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public Throwable getReloadFailureCause() {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public void reload() {
+        }
+
+        /** {@inheritDoc} */
+        public ServiceableComponent<AttributeTranscoderRegistry> getServiceableComponent() {
+            return this;
+        }
+
+        /** {@inheritDoc} */
+        public String getId() {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public Map<Locale, String> getDisplayNames(IdPAttribute attribute) {
+            return attribute.getDisplayNames();
+        }
+
+        /** {@inheritDoc} */
+        public Map<Locale, String> getDescriptions(IdPAttribute attribute) {
+            return attribute.getDisplayDescriptions();
+        }
+
+        /** {@inheritDoc} */
+        public Collection<TranscodingRule> getTranscodingRules(IdPAttribute from, Class<?> to) {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public <T> Collection<TranscodingRule> getTranscodingRules(T from) {
+            return null;
+        }
+
+        /** {@inheritDoc} */
+        public AttributeTranscoderRegistry getComponent() {
+            return this;
+        }
+
+        /** {@inheritDoc} */
+        public void pinComponent() {
+        }
+
+        /** {@inheritDoc} */
+        public void unpinComponent() {
+        }
+
+        /** {@inheritDoc} */
+        public void unloadComponent() {
+        }
+    }
 }

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


More information about the commits mailing list