[java-idp-oidc] 08/09: JOIDC-5 Initial integration of client secret value resolver.

Henri Mikkonen henri.mikkonen at iki.fi
Fri Jun 5 13:55:00 UTC 2020


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

hjmikkon pushed a commit to branch dev/JOIDC-5
in repository java-idp-oidc.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=c3731f0243db0cf52611b2b2b37e7ca9f9e4675e

commit c3731f0243db0cf52611b2b2b37e7ca9f9e4675e
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Jun 5 16:53:12 2020 +0300

    JOIDC-5 Initial integration of client secret value resolver.
    
    https://issues.shibboleth.net/jira/browse/JOIDC-5
---
 .../oidc/filter/impl/ClientInformationParser.java  | 17 ++++++-
 .../impl/ClientInformationNodeProcessor.java       | 55 +++++++++++++++-------
 .../schema/idp-oidc-extension-metadata-ext.xsd     |  6 ++-
 .../impl/ClientInformationNodeProcessorTest.java   | 19 +++++++-
 .../src/test/resources/conf/global-oidc.xml        | 20 ++++++++
 .../src/test/resources/conf/metadata-providers.xml |  2 +-
 .../EntityDescriptor-with-oidcmd-clientsecret.xml  |  2 +-
 .../metadata/impl/client-secret-test.properties    |  1 +
 8 files changed, 97 insertions(+), 25 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/profile/spring/relyingparty/metadata/oidc/filter/impl/ClientInformationParser.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/profile/spring/relyingparty/metadata/oidc/filter/impl/ClientInformationParser.java
index 6150f0eb..2e00805d 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/profile/spring/relyingparty/metadata/oidc/filter/impl/ClientInformationParser.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/profile/spring/relyingparty/metadata/oidc/filter/impl/ClientInformationParser.java
@@ -20,12 +20,15 @@ import javax.annotation.Nonnull;
 import javax.xml.namespace.QName;
 
 import org.geant.idpextension.oidc.metadata.impl.ClientInformationNodeProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Element;
 
 import net.shibboleth.idp.profile.spring.relyingparty.metadata.oidc.impl.MetadataNamespaceHandler;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 /**
  * Parser for a <ClientInformation> node processor.
@@ -35,7 +38,10 @@ public class ClientInformationParser extends AbstractSingleBeanDefinitionParser
     /** Element name. */
     @Nonnull public static final QName TYPE_NAME =
             new QName(MetadataNamespaceHandler.NAMESPACE, "ClientInformation");
-
+    
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(ClientInformationParser.class);
+    
     /** {@inheritDoc} */
     @Override protected Class<?> getBeanClass(final Element element) {
         return ClientInformationNodeProcessor.class;
@@ -45,7 +51,14 @@ public class ClientInformationParser extends AbstractSingleBeanDefinitionParser
     @Override
     protected void doParse(final Element element, final ParserContext parserContext,
             final BeanDefinitionBuilder builder) {
-        
+        super.doParse(element, parserContext, builder);
+
+        if (element.hasAttributeNS(null, "keyInfoProvidersRef")) {
+            builder.addConstructorArgReference(StringSupport.trimOrNull(element.getAttributeNS(null,
+                    "keyInfoProvidersRef")));
+        } else {
+            log.error("No 'keyInfoProvidersRef' attribute defined!");
+        }
     }
 
     /** {@inheritDoc} */
diff --git a/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessor.java b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessor.java
index 1db5ff1e..f7036615 100644
--- a/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessor.java
+++ b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessor.java
@@ -25,9 +25,6 @@ import java.util.Set;
 
 import javax.xml.namespace.QName;
 
-import org.geant.idpextension.keyinfo.ext.impl.provider.ClientSecretProvider;
-import org.geant.idpextension.keyinfo.ext.impl.provider.InlineJwksProvider;
-import org.geant.idpextension.keyinfo.ext.impl.provider.JWKSReferenceProvider;
 import org.geant.idpextension.oidc.config.OIDCCoreProtocolConfiguration;
 import org.geant.idpextension.oidc.security.impl.CredentialConversionUtil;
 import org.geant.security.jwk.NimbusSecretCredential;
@@ -47,9 +44,6 @@ import org.opensaml.security.credential.Credential;
 import org.opensaml.xmlsec.keyinfo.KeyInfoCredentialResolver;
 import org.opensaml.xmlsec.keyinfo.impl.BasicProviderKeyInfoCredentialResolver;
 import org.opensaml.xmlsec.keyinfo.impl.KeyInfoProvider;
-import org.opensaml.xmlsec.keyinfo.impl.provider.DSAKeyValueProvider;
-import org.opensaml.xmlsec.keyinfo.impl.provider.InlineX509DataProvider;
-import org.opensaml.xmlsec.keyinfo.impl.provider.RSAKeyValueProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -90,21 +84,20 @@ public class ClientInformationNodeProcessor implements MetadataNodeProcessor {
     /** Class logger. */
     private final Logger log = LoggerFactory.getLogger(ClientInformationNodeProcessor.class);
     
+    /** The {@link KeyInfoCredentialResolver} to be used for the resolution. */
     private KeyInfoCredentialResolver keyInfoCredentialResolver;
     
-    public ClientInformationNodeProcessor() {
-        final List<KeyInfoProvider> keyInfoProviders = new ArrayList<>();
-        keyInfoProviders.add(new DSAKeyValueProvider());
-        keyInfoProviders.add(new RSAKeyValueProvider());
-        keyInfoProviders.add(new InlineX509DataProvider());
-        keyInfoProviders.add(new InlineJwksProvider());
-        keyInfoProviders.add(new JWKSReferenceProvider());
-        keyInfoProviders.add(new ClientSecretProvider());
-
+    /**
+     * Constructor.
+     * 
+     * @param keyInfoProviders The list of key info providers.
+     */
+    public ClientInformationNodeProcessor(final List<KeyInfoProvider> keyInfoProviders) {
         keyInfoCredentialResolver = new BasicProviderKeyInfoCredentialResolver(keyInfoProviders);
 
     }
     
+    /** {@inheritDoc} */
     @Override
     public void process(XMLObject metadataNode) throws FilterException {
         if (metadataNode instanceof SPSSODescriptor) {
@@ -126,7 +119,13 @@ public class ClientInformationNodeProcessor implements MetadataNodeProcessor {
         }
     }
 
-    
+    /**
+     * Converts the entityID of the given {@link SPSSODescriptor} into a {@link ClientID}. The value is fetched from
+     * the {@link EntityDescriptor}, expected to be the parent element of the given role decriptor.
+     * 
+     * @param roleDescriptor The {@link SPSSODescriptor} to be used as a source.
+     * @return The entityID value as {@link ClientID}.
+     */
     protected ClientID parseClientID(final SPSSODescriptor roleDescriptor) {
         if (!roleDescriptor.hasParent() || !(roleDescriptor.getParent() instanceof EntityDescriptor)) {
             log.warn("Unexpected structure, EntityDescriptor not as a parent for OAuthRPRoleDescriptor");
@@ -136,6 +135,13 @@ public class ClientInformationNodeProcessor implements MetadataNodeProcessor {
         return new ClientID(entityDescriptor.getEntityID());
     }
     
+    /**
+     * Fetches the client secret from given the set of {@link Credential}s. The first credential matching the type
+     * {@link NimbusSecretCredential} is used as the source.
+     * 
+     * @param credentials The source set of {@link Credential}s.
+     * @return The client secret as {@link Secret}.
+     */
     protected Secret parseClientSecret(final Iterable<Credential> credentials) {
         for (final Credential credential : credentials) {
             log.trace("Processing credential type {}", credential.getCredentialType());
@@ -148,7 +154,17 @@ public class ClientInformationNodeProcessor implements MetadataNodeProcessor {
         return null;
     }
 
-    protected OIDCClientMetadata populateMetadata(final SPSSODescriptor roleDescriptor, final Iterable<Credential> credentials, final String clientId) {
+    /**
+     * Populates the {@link OIDCClientMetadata} using the values found from the given {@link SPSSODescriptor}, the set
+     * of {@link Credential}s and the client ID.
+     * 
+     * @param roleDescriptor The {@link SPSSODescriptor} to be used as a source.
+     * @param credentials The source set of {@link Credential}s to be used for client secret and remote/local JWKS.
+     * @param clientId The client ID.
+     * @return The {@link OIDCClientMetadata} parsed from the given parameters.
+     */
+    protected OIDCClientMetadata populateMetadata(final SPSSODescriptor roleDescriptor,
+            final Iterable<Credential> credentials, final String clientId) {
         final OIDCClientMetadata metadata = new OIDCClientMetadata();
         final OAuthRPExtensions extensions = getOAuthRPExtensions(roleDescriptor);
         if (extensions != null) {
@@ -192,6 +208,11 @@ public class ClientInformationNodeProcessor implements MetadataNodeProcessor {
         return metadata;
     }
     
+    /**
+     * 
+     * @param roleDescriptor
+     * @return
+     */
     protected OAuthRPExtensions getOAuthRPExtensions(final SPSSODescriptor roleDescriptor) {
         final Extensions extensions = roleDescriptor.getExtensions();
         if (extensions == null) {
diff --git a/idp-oidc-extension-impl/src/main/resources/schema/idp-oidc-extension-metadata-ext.xsd b/idp-oidc-extension-impl/src/main/resources/schema/idp-oidc-extension-metadata-ext.xsd
index 5ed3ce0b..607fa693 100644
--- a/idp-oidc-extension-impl/src/main/resources/schema/idp-oidc-extension-metadata-ext.xsd
+++ b/idp-oidc-extension-impl/src/main/resources/schema/idp-oidc-extension-metadata-ext.xsd
@@ -13,8 +13,10 @@
             </documentation>
         </annotation>
         <complexContent>
-            <extension base="shibmd:MetadataNodeProcessorType"/>
+            <extension base="shibmd:MetadataNodeProcessorType">
+                <attribute name="keyInfoProvidersRef" type="shibmd:string" use="required" />
+            </extension>
         </complexContent>
     </complexType>
 
-</schema>
+</schema>
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessorTest.java b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessorTest.java
index 8563a0a2..98b065a7 100644
--- a/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessorTest.java
+++ b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/ClientInformationNodeProcessorTest.java
@@ -21,6 +21,9 @@ import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.geant.idpextension.keyinfo.ext.impl.provider.ClientSecretProvider;
+import org.geant.idpextension.keyinfo.ext.impl.provider.InlineJwksProvider;
+import org.geant.idpextension.keyinfo.ext.impl.provider.JWKSReferenceProvider;
 import org.opensaml.core.criterion.EntityIdCriterion;
 import org.opensaml.core.xml.XMLObjectBaseTestCase;
 import org.opensaml.saml.criterion.EntityRoleCriterion;
@@ -31,6 +34,10 @@ import org.opensaml.saml.metadata.resolver.impl.FilesystemMetadataResolver;
 import org.opensaml.saml.metadata.resolver.impl.PredicateRoleDescriptorResolver;
 import org.opensaml.saml.saml2.metadata.RoleDescriptor;
 import org.opensaml.saml.saml2.metadata.SPSSODescriptor;
+import org.opensaml.xmlsec.keyinfo.impl.KeyInfoProvider;
+import org.opensaml.xmlsec.keyinfo.impl.provider.DSAKeyValueProvider;
+import org.opensaml.xmlsec.keyinfo.impl.provider.InlineX509DataProvider;
+import org.opensaml.xmlsec.keyinfo.impl.provider.RSAKeyValueProvider;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -59,8 +66,16 @@ public class ClientInformationNodeProcessorTest extends XMLObjectBaseTestCase {
         mdProvider.setParserPool(parserPool);
         mdProvider.setId("test");
         NodeProcessingMetadataFilter filter = new NodeProcessingMetadataFilter();
+        List<KeyInfoProvider> providers = new ArrayList<>();
+        providers.add(new DSAKeyValueProvider());
+        providers.add(new RSAKeyValueProvider());
+        providers.add(new InlineX509DataProvider());
+        providers.add(new InlineJwksProvider());
+        providers.add(new JWKSReferenceProvider());
+        providers.add(new ClientSecretProvider());
+
         List<MetadataNodeProcessor> processors = new ArrayList<>();
-        processors.add(new ClientInformationNodeProcessor());
+        processors.add(new ClientInformationNodeProcessor(providers));
         filter.setNodeProcessors(processors);
         filter.initialize();
         mdProvider.setMetadataFilter(filter);
@@ -91,5 +106,5 @@ public class ClientInformationNodeProcessorTest extends XMLObjectBaseTestCase {
         Assert.assertEquals(clientInformations.get(0).getID().getValue(), "mockSamlClientId");
         Assert.assertEquals(clientInformations.get(0).getOIDCMetadata().getRedirectionURIs().size(), 2);
     }
-
+    
 }
diff --git a/idp-oidc-extension-impl/src/test/resources/conf/global-oidc.xml b/idp-oidc-extension-impl/src/test/resources/conf/global-oidc.xml
index d1ef15ae..317d1851 100644
--- a/idp-oidc-extension-impl/src/test/resources/conf/global-oidc.xml
+++ b/idp-oidc-extension-impl/src/test/resources/conf/global-oidc.xml
@@ -13,6 +13,26 @@
          This file contains global oidc bean definitions.
          This file should be imported to global.xml
     -->
+    
+    <util:list id="shibboleth.oidc.KeyInfoProviders" value-type="org.opensaml.xmlsec.keyinfo.impl.KeyInfoProvider">
+        <bean class="org.opensaml.xmlsec.keyinfo.impl.provider.DSAKeyValueProvider" />
+        <bean class="org.opensaml.xmlsec.keyinfo.impl.provider.InlineX509DataProvider" />
+        <bean class="org.opensaml.xmlsec.keyinfo.impl.provider.RSAKeyValueProvider" />
+        <bean class="org.geant.idpextension.keyinfo.ext.impl.provider.ClientSecretProvider" />
+        <bean class="org.geant.idpextension.keyinfo.ext.impl.provider.ClientSecretReferenceProvider">
+            <constructor-arg ref="shibboleth.oidc.ClientSecretValueResolvers" />
+        </bean>
+        <bean class="org.geant.idpextension.keyinfo.ext.impl.provider.InlineJwksProvider" />
+        <bean class="org.geant.idpextension.keyinfo.ext.impl.provider.JWKSReferenceProvider" />
+    </util:list>
+    
+    <util:list id="shibboleth.oidc.ClientSecretValueResolvers" value-type="org.geant.idpextension.oidc.metadata.resolver.ClientSecretValueResolver">
+        <ref bean="shibboleth.oidc.ClientSecretValueResolvers.Properties" />
+    </util:list>
+
+    <bean id="shibboleth.oidc.ClientSecretValueResolvers.Properties"
+        class="org.geant.idpextension.oidc.metadata.impl.PropertiesFileClientSecretValueResolver"
+        c:resource="classpath:/org/geant/idpextension/oidc/metadata/impl/client-secret-test.properties" />
 
     <!-- Returns true unless subject is already populated in oidc response context. -->
     <bean id="SubjectRequired" class="org.geant.idpextension.oidc.profile.logic.SubjectActivationCondition" />
diff --git a/idp-oidc-extension-impl/src/test/resources/conf/metadata-providers.xml b/idp-oidc-extension-impl/src/test/resources/conf/metadata-providers.xml
index c1cfcded..2681b6ad 100644
--- a/idp-oidc-extension-impl/src/test/resources/conf/metadata-providers.xml
+++ b/idp-oidc-extension-impl/src/test/resources/conf/metadata-providers.xml
@@ -21,7 +21,7 @@
 
     <MetadataProvider id="SP123MD" xsi:type="ResourceBackedMetadataProvider" maxRefreshDelay="PT5M" indexesRef="testbed.MetadataIndexes" resourceRef="exampleMetadata-saml-oidc-clientsecret">
         <MetadataFilter xsi:type="NodeProcessing">
-            <MetadataNodeProcessor xsi:type="oidcext:ClientInformation"/>
+            <MetadataNodeProcessor xsi:type="oidcext:ClientInformation" keyInfoProvidersRef="shibboleth.oidc.KeyInfoProviders"/>
         </MetadataFilter>
     </MetadataProvider>
 
diff --git a/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/EntityDescriptor-with-oidcmd-clientsecret.xml b/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/EntityDescriptor-with-oidcmd-clientsecret.xml
index 57beb35a..c8bde4a4 100644
--- a/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/EntityDescriptor-with-oidcmd-clientsecret.xml
+++ b/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/EntityDescriptor-with-oidcmd-clientsecret.xml
@@ -4,7 +4,7 @@
         <md:KeyDescriptor>
             <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                 <ds:KeyName>mockClientSecret</ds:KeyName>
-                <oidcmd:ClientSecret>mockClientSecretmockClientSecretmockClientSecret</oidcmd:ClientSecret>
+                <oidcmd:ClientSecretReferenceKey>mockClientSecretKey</oidcmd:ClientSecretReferenceKey>
             </ds:KeyInfo>
         </md:KeyDescriptor>
         <md:Extensions>
diff --git a/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/client-secret-test.properties b/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/client-secret-test.properties
new file mode 100644
index 00000000..8f1c15cc
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/resources/org/geant/idpextension/oidc/metadata/impl/client-secret-test.properties
@@ -0,0 +1 @@
+mockClientSecretKey = mockClientSecretmockClientSecretmockClientSecret
\ 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