[java-oidc-common] branch main updated: Revert JCOMOIDC-89

Phil Smart philip.smart at jisc.ac.uk
Wed Oct 25 15:19:05 UTC 2023


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

philsmart pushed a commit to branch main
in repository java-oidc-common.

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

The following commit(s) were added to refs/heads/main by this push:
     new d6eb6e6  Revert JCOMOIDC-89
d6eb6e6 is described below

commit d6eb6e6eb70386ab33cfb23dfbf418141dbee3e0
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Oct 25 16:18:29 2023 +0100

    Revert JCOMOIDC-89
---
 .../AbstractNullableComponentAwareFactoryBean.java |  84 ---------
 .../AbstractNullableCredentialFactoryBean.java     | 203 ---------------------
 .../credential/BasicJWKCredentialFactoryBean.java  |  72 +++-----
 .../BasicJWKCredentialFactoryBeanTest.java         |  80 --------
 4 files changed, 24 insertions(+), 415 deletions(-)

diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableComponentAwareFactoryBean.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableComponentAwareFactoryBean.java
deleted file mode 100644
index 2725e84..0000000
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableComponentAwareFactoryBean.java
+++ /dev/null
@@ -1,84 +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.oidc.security.credential;
-
-import javax.annotation.Nullable;
-
-import org.springframework.beans.factory.BeanCreationException;
-import org.springframework.beans.factory.FactoryBean;
-import org.springframework.beans.factory.config.AbstractFactoryBean;
-
-import net.shibboleth.shared.component.DestructableComponent;
-import net.shibboleth.shared.component.InitializableComponent;
-import net.shibboleth.shared.primitive.LoggerFactory;
-
-/**
- * 
- * A Factory bean which is aware of the component lifecycle interfaces.
- *
- * <p>Unlike other bean factories, this factory allows {@code null} beans to be returned via its 
- * {@link #createInstance()} method. Ultimately, a FactoryBean implementation may return {@code null} objects (see 
- * {@link FactoryBean#getObject()) and so this is compatible with Spring's Factory Bean interface even though 
- * it is not directly compatible with the {@code non-null} contract of the {@link AbstractFactoryBean#createInstance()} 
- * method. </p>
- * 
- * @param <T> The type to implement
- */
-public abstract class AbstractNullableComponentAwareFactoryBean<T> extends AbstractFactoryBean<T> {       
-    
-    /** {@inheritDoc}. Call our destroy method if apposite. */
-    @Override protected void destroyInstance(@Nullable final T instance) throws Exception {
-        super.destroyInstance(instance);
-        if (instance instanceof final DestructableComponent destructableComponent) {
-            destructableComponent.destroy();
-        }
-    }
-
-    /**
-     *  Call the child class to create the object, then initialize it apposite.
-     * 
-     *  <p>This method is allowed to return {@code null} even though it overrides a superclass method which 
-     *  is {@code non-null}. Spring {@link FactoryBean FactoryBeans} can return {@code null} objects, 
-     *  see {@link FactoryBean#getObject()}. </p>
-     * 
-     *  {@inheritDoc}.
-     */
-    @SuppressWarnings("null")
-    @Override
-    @Nullable protected final T createInstance() throws Exception {
-        if (!isSingleton()) {
-            LoggerFactory.getLogger(AbstractNullableComponentAwareFactoryBean.class).error(
-                    "Configuration error: {} should not be used to create prototype beans."
-                            + "  Destroy is never called for prototype beans",
-                            AbstractNullableComponentAwareFactoryBean.class);
-            throw new BeanCreationException("Do not use AbstractComponentAwareFactoryBean to create prototype beans");
-        }
-        final T theBean = doCreateInstance();
-        if (theBean instanceof final InitializableComponent initComponent) {
-            initComponent.initialize();
-        }
-        return theBean;
-    }
-
-    /**
-     * Implementation method to create the bean instance.
-     * 
-     * @return the bean. Can be {@code null} if the bean was not created but no error was signalled.
-     * 
-     * @throws Exception if there was an unrecoverable error constructing the bean.
-     */
-    @Nullable protected abstract T doCreateInstance() throws Exception;
-
-}
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableCredentialFactoryBean.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableCredentialFactoryBean.java
deleted file mode 100644
index 895dcac..0000000
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/AbstractNullableCredentialFactoryBean.java
+++ /dev/null
@@ -1,203 +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.oidc.security.credential;
-
-import java.util.List;
-
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-import org.opensaml.security.credential.Credential;
-import org.slf4j.Logger;
-import org.springframework.beans.factory.BeanCreationException;
-
-import net.shibboleth.shared.primitive.LoggerFactory;
-
-/**
- * A factory bean to accumulate the information pertaining to an {@link Credential}. 
- * 
- * <p>Note, {@link #doCreateInstance()} is allowed to return {@code null} if {@code throwIfCredentialIsNull} is 'false' 
- * (the default throws an exception if the credential is {@code null}).</p>
- * 
- * @param <T> the type of credential to create.
- */
-public abstract class AbstractNullableCredentialFactoryBean<T extends Credential> 
-                                                extends AbstractNullableComponentAwareFactoryBean<T> {
-    
-    /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractNullableCredentialFactoryBean.class);
-
-    /** Usage type of the credential. */
-    @Nullable private String usageType;
-
-    /** Names for the key represented by the credential. */
-    @Nullable  private List<String> keyNames;
-
-    /** Identifier for the owner of the credential. */
-    @Nullable private String entityID;
-    
-    /** The privateKey Password (if any). */
-    @Nullable private char[] privateKeyPassword;
-
-    /** For logging: The description of the source of the configuration.*/
-    @Nullable private String configDescription;
-    
-    /** Should the factory throw an exception if the created credential is null?.*/
-    private boolean throwIfCredentialIsNull;
-    
-    
-    /** Constructor. */
-    protected AbstractNullableCredentialFactoryBean() {
-        throwIfCredentialIsNull = true;
-    }    
-    
-    /**
-     * Should the factory throw an exception if the credential is {@code null}?
-     *  
-     * @param flag the flag
-     */
-    public void setThrowIfCredentialIsNull(final boolean flag) {
-        throwIfCredentialIsNull = flag;
-    }
-    
-    /**
-     * Get the flag to determine if the factory should throw an exception if the credential is {@code null}.
-     * 
-     * @return true if the factory should throw an exception if the credential is null, false otherwise. 
-     */
-    public boolean isThrowIfCredentialIsNull() {
-        return throwIfCredentialIsNull;
-    }
-
-   
-    /**
-     * Gets the names for the key represented by the credential.
-     * 
-     * @return names for the key represented by the credential
-     */
-    @Nullable public List<String> getKeyNames() {
-        return keyNames;
-    }
-
-    /**
-     * Gets the usage type of the credential.
-     * 
-     * @return usage type of the credential
-     */
-    @Nullable public String getUsageType() {
-        return usageType;
-    }
-
-    /**
-     * Get the entity ID of the credential.
-     * 
-     * @return the entity ID
-     */
-    @Nullable public String getEntityID() {
-        return entityID;
-    }
-
-    /**
-     * Sets the names for the key represented by the credential.
-     * 
-     * @param names names for the key represented by the credential
-     */
-    public void setKeyNames(@Nullable final List<String> names) {
-        keyNames = names;
-    }
-
-    /**
-     * Sets the usage type of the credential.
-     * 
-     * @param type usage type of the credential
-     */
-    public void setUsageType(@Nullable final String type) {
-        if (null != type) {
-            usageType = type.toUpperCase();
-        } else {
-            usageType = type;
-        }
-    }
-
-    /**
-     * Set the entity ID of the credential.
-     * 
-     * @param newEntityID the entity ID
-     */
-    public void setEntityID(@Nullable final String newEntityID) {
-        entityID = newEntityID;
-    }
-    
-    /**
-     * Get the password for the private key.
-     * 
-     * @return Returns the privateKeyPassword.
-     */
-    @Nullable public char[] getPrivateKeyPassword() {
-        return privateKeyPassword;
-    }
-
-    /**
-     * Set the password for the private key.
-     * 
-     * @param password The password to set.
-     */
-    public void setPrivateKeyPassword(@Nullable final char[] password) {
-        if (null != password && password.length > 0) {
-            privateKeyPassword = password;
-        } else {
-            privateKeyPassword = null;
-        }
-    }
-
-    /** For logging, get the description of the resource that defined this bean.
-     * 
-     * @return Returns the description.
-     */
-    @Nullable public String getConfigDescription() {
-        return configDescription;
-    }
-
-    /** For logging, set the description of the resource that defined this bean.
-     * 
-     * @param desc what to set.
-     */
-    public void setConfigDescription(@Nullable final String desc) {
-        configDescription = desc;
-    }
-    
-    
-    /** {@inheritDoc} */
-    @Override
-    @Nullable protected T doCreateInstance() throws Exception {
-        final T credential = doCreateCredential();
-        if (credential == null && isThrowIfCredentialIsNull()) {
-            log.debug("Credential was null, must not be null (if allowable set throwIfCredentialIsNull "
-                    + "to 'false')");
-            throw new BeanCreationException("Null credential");
-        }
-        return credential;
-    }
-    
-    /**
-     * Implementation method to create the {@link Credential} instance. 
-     * 
-     * @return the credential, or {@code null}.
-     * 
-     * @throws Exception if there was an unrecoverable error constructing the credential.
-     */
-    @Nullable protected abstract T doCreateCredential() throws Exception;
-    
-}
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBean.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBean.java
index dd83f4e..42b26fa 100644
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBean.java
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBean.java
@@ -16,16 +16,15 @@ package net.shibboleth.oidc.security.credential;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.security.PrivateKey;
-import java.security.PublicKey;
 import java.text.ParseException;
 import java.util.List;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import javax.crypto.SecretKey;
 
+import org.opensaml.spring.credential.AbstractCredentialFactoryBean;
 import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.core.io.Resource;
 
@@ -38,23 +37,23 @@ import com.nimbusds.jose.jwk.OctetSequenceKey;
 import net.shibboleth.oidc.security.CredentialConversionUtil;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.logic.Constraint;
-import net.shibboleth.shared.primitive.DeprecationSupport;
-import net.shibboleth.shared.primitive.DeprecationSupport.ObjectType;
-import net.shibboleth.shared.primitive.LoggerFactory;
 
 /** 
  * Factory bean for Basic JSON Web Keys (JWK).
  * 
  * @since 2.2.0
  */
-public class BasicJWKCredentialFactoryBean extends AbstractNullableCredentialFactoryBean<BasicJWKCredential> {
+public class BasicJWKCredentialFactoryBean extends AbstractCredentialFactoryBean<BasicJWKCredential> {
 
     /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(BasicJWKCredentialFactoryBean.class);
+    private final Logger log = LoggerFactory.getLogger(BasicJWKCredentialFactoryBean.class);
 
     /** Where the private key is to be found. */
     @Nullable private Resource jwkResource;
     
+    /** Should the factory throw an exception if the resource is null?.*/
+    private boolean failIfResourceIsNull;
+    
     /**
      * The JCA algorithm name to use if the key to be converted is a symmetric key.
      * Default is AES.
@@ -63,7 +62,7 @@ public class BasicJWKCredentialFactoryBean extends AbstractNullableCredentialFac
     
     /** Constructor.*/
     public BasicJWKCredentialFactoryBean() {
-        super();
+        failIfResourceIsNull = true;
         symmetricKeyAlgorithm = "AES";
     }
     
@@ -78,15 +77,12 @@ public class BasicJWKCredentialFactoryBean extends AbstractNullableCredentialFac
     }
     
     /**
-     * Should the factory throw an exception if the resource is null?
+     * Should the factory throw an exeception if the resource is null?
      *  
      * @param flag the flag
      */
-    @Deprecated(since="3.1.0", forRemoval=true)
     public void setFailIfResourceIsNull(final boolean flag) {
-        DeprecationSupport.warn(ObjectType.METHOD, "setFailIfResourceIsNull",
-                "oidc-credentials.xml", "setThrowIfCredentialIsNull");
-        setThrowIfCredentialIsNull(flag);
+        failIfResourceIsNull = flag;
     }
 
     /**
@@ -100,54 +96,34 @@ public class BasicJWKCredentialFactoryBean extends AbstractNullableCredentialFac
 
     /** {@inheritDoc} */
     @Override
-    @Nullable protected BasicJWKCredential doCreateCredential() throws Exception {
-        
-        final Resource jwkResourceLocal = jwkResource;
-        if (jwkResourceLocal == null) {
+    protected BasicJWKCredential doCreateInstance() throws Exception {
+
+        if (jwkResource == null && failIfResourceIsNull) {
+            log.error("{}: No JWK credential provided", getConfigDescription());
+            throw new BeanCreationException("No JWK credential provided");
+        } else if (jwkResource == null) {
             return null;
         }
-        
-        assert jwkResourceLocal != null;
         JWK jwk = null;
         BasicJWKCredential jwkCredential = null;
-        
-        try (InputStream is = jwkResourceLocal.getInputStream()) {
-            
+        try (InputStream is = jwkResource.getInputStream()) {
             jwk = JWK.parse(new String(ByteStreams.toByteArray(is)));
             jwkCredential = new BasicJWKCredential();
-            
             if (jwk.getKeyType() == KeyType.EC || jwk.getKeyType() == KeyType.RSA) {
                 if (jwk.isPrivate()) {
-                    final PrivateKey privateKey = ((AsymmetricJWK) jwk).toPrivateKey();
-                    if (privateKey != null) {
-                        jwkCredential.setPrivateKey(privateKey);
-                    } else {
-                        throw new BeanCreationException("Private key type found, but no key supplied");
-                    }                    
+                    jwkCredential.setPrivateKey(((AsymmetricJWK) jwk).toPrivateKey());
                 }
-                final PublicKey publicKey = ((AsymmetricJWK) jwk).toPublicKey();
-                if (publicKey != null) {
-                    jwkCredential.setPublicKey(publicKey);
-                } else {
-                    throw new BeanCreationException("Public key type found, but no key supplied");
-                }                
+                jwkCredential.setPublicKey(((AsymmetricJWK) jwk).toPublicKey());
             } else if (jwk.getKeyType() == KeyType.OCT) {
-                final SecretKey secretKey = ((OctetSequenceKey) jwk).toSecretKey(symmetricKeyAlgorithm);
-                if (secretKey != null) {
-                    jwkCredential.setSecretKey(secretKey);
-                } else {
-                    throw new BeanCreationException("Secret key type found, but no key supplied");
-                }
+                jwkCredential.setSecretKey(((OctetSequenceKey) jwk).toSecretKey(symmetricKeyAlgorithm));
             } else {
-                throw new BeanCreationException("Unsupported KeyFile at " + jwkResourceLocal.getDescription());
+                throw new BeanCreationException("Unsupported KeyFile at " + jwkResource.getDescription());
             }
         } catch (final IOException | ParseException e) {
-            log.error("{}: Could not decode KeyFile at {}: {}", getConfigDescription(), 
-                    jwkResourceLocal.getDescription(), e);
-            throw new BeanCreationException("Could not decode provided KeyFile " + 
-                    jwkResourceLocal.getDescription(), e);
+            log.error("{}: Could not decode KeyFile at {}: {}", getConfigDescription(), jwkResource.getDescription(),
+                    e);
+            throw new BeanCreationException("Could not decode provided KeyFile " + jwkResource.getDescription(), e);
         }
-        
         jwkCredential.setUsageType(CredentialConversionUtil.getUsageType(jwk));
         jwkCredential.setEntityId(getEntityID());
         jwkCredential.setAlgorithm(jwk.getAlgorithm());
diff --git a/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBeanTest.java b/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBeanTest.java
deleted file mode 100644
index 30730ef..0000000
--- a/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/BasicJWKCredentialFactoryBeanTest.java
+++ /dev/null
@@ -1,80 +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.oidc.security.credential;
-
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
-
-import java.nio.charset.StandardCharsets;
-
-import org.springframework.beans.factory.BeanCreationException;
-import org.springframework.core.io.ByteArrayResource;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-/**
- * Tests for the {@link BasicJWKCredentialFactoryBean}.
- */
-public class BasicJWKCredentialFactoryBeanTest {
-
-    /** Factory to test.*/
-    private BasicJWKCredentialFactoryBean factory;
-
-    /** Example EC JWK taken from JWK spec.*/
-    private final String keyResource = 
-                         """
-                         {
-                             "alg":"EC",
-                             "crv":"P-256",
-                             "kty": "EC",
-                             "x":"MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4",
-                             "y":"4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM",
-                             "use":"enc",
-                             "kid":"1"
-                          }
-                          """;
-
-    @BeforeMethod
-    public void setup() throws Exception {
-        factory = new BasicJWKCredentialFactoryBean();
-    }
-
-    @Test(expectedExceptions = BeanCreationException.class)
-    public void testNullJWKResource_Fail() throws Exception {
-        factory.setThrowIfBeanIsNull(true);
-        factory.setResource(null);
-        factory.afterPropertiesSet();
-        factory.getObject();
-    }
-    
-    @Test
-    public void testNullJWKResource_Success() throws Exception {
-        factory.setThrowIfBeanIsNull(false);
-        factory.setResource(null);
-        factory.afterPropertiesSet();
-        final var cred = factory.getObject();
-        assertNull(cred);
-    }
-
-    @Test
-    public void testBeanCreated() throws Exception {
-        factory.setThrowIfBeanIsNull(true);
-        factory.setResource(new ByteArrayResource(keyResource.getBytes(StandardCharsets.UTF_8)));
-        factory.afterPropertiesSet();
-        final var credential = factory.getObject();
-        assertNotNull(credential);
-    }
-
-}

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


More information about the commits mailing list