[java-idp-oidc] 25/44: JOIDC-5 Refactoring + polishing for client secret resolution from Props.
Henri Mikkonen
henri.mikkonen at iki.fi
Thu Oct 22 13:08:37 UTC 2020
This is an automated email from the git hooks/post-receive script.
hjmikkon pushed a commit to branch main
in repository java-idp-oidc.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=d315bb18b89bed6139fd58a77fdd60a099de1efd
commit d315bb18b89bed6139fd58a77fdd60a099de1efd
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Jun 26 12:32:14 2020 +0300
JOIDC-5 Refactoring + polishing for client secret resolution from Props.
https://issues.shibboleth.net/jira/browse/JOIDC-5
---
.../impl/AbstractClientSecretValueResolver.java | 29 ++++++
.../impl/PropertiesClientSecretValueResolver.java | 78 ++++++++------
.../impl/BaseClientSecretValueResolverTest.java | 74 ++++++++++++++
.../PropertiesClientSecretValueResolverTest.java | 112 +++++++++++++++++++++
.../src/test/resources/conf/global-oidc.xml | 4 +-
5 files changed, 264 insertions(+), 33 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/AbstractClientSecretValueResolver.java b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/AbstractClientSecretValueResolver.java
new file mode 100644
index 00000000..52216471
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/AbstractClientSecretValueResolver.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017 - 2020, GÉANT
+ *
+ * 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 org.geant.idpextension.oidc.metadata.impl;
+
+import org.geant.idpextension.oidc.metadata.resolver.ClientSecretValueResolver;
+
+import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
+
+/**
+ * Base class for {@link ClientSecretValueResolver}s.
+ */
+public abstract class AbstractClientSecretValueResolver extends AbstractIdentifiableInitializableComponent
+ implements ClientSecretValueResolver {
+
+}
diff --git a/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolver.java b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolver.java
index 3ce0743a..399b0091 100644
--- a/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolver.java
+++ b/idp-oidc-extension-impl/src/main/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolver.java
@@ -21,15 +21,16 @@ import java.util.Collections;
import java.util.Properties;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.geant.idpextension.oidc.criterion.ClientSecretReferenceCriterion;
-import org.geant.idpextension.oidc.metadata.resolver.ClientSecretValueResolver;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.core.io.Resource;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.collection.LazySet;
-import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
@@ -39,36 +40,44 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
/**
* A client secret value resolver that fetches the values from the given Properties file.
*/
-public class PropertiesClientSecretValueResolver extends AbstractIdentifiableInitializableComponent
- implements ClientSecretValueResolver {
+public class PropertiesClientSecretValueResolver extends AbstractClientSecretValueResolver {
/** Class logger. */
private final Logger log = LoggerFactory.getLogger(PropertiesClientSecretValueResolver.class);
- /** The properties file. */
- @Nonnull private Properties properties;
+ /** The properties resource containing client secret values. */
+ @NonnullAfterInit private Resource propertiesResource;
/**
- * Constructor.
- *
- * @param resource The properties resource.
+ * Set the properties resource containing client secret values.
+ *
+ * @param resource The properties resource containing client secret values.
*/
- public PropertiesClientSecretValueResolver(@Nonnull final Resource resource) {
- super();
- Constraint.isNotNull(resource, "Properties resource cannot be null");
- properties = new Properties();
+ public void setResource(@Nonnull final Resource resource) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ propertiesResource = Constraint.isNotNull(resource, "Properties resource cannot be null");
+ final Properties properties = new Properties();
try {
- properties.load(resource.getInputStream());
+ properties.load(propertiesResource.getInputStream());
} catch (IOException e) {
- log.warn("Could not read the properties from the given resource", e);
+ log.error("Could not read the properties from the given resource", e);
throw new ConstraintViolationException("The properties resource must be readable");
}
}
- @Override
+ /**
+ * Get the properties resource containing client secret values.
+ *
+ * @return The properties resource containing client secret values.
+ */
+ public @NonnullAfterInit Resource getResource() {
+ return propertiesResource;
+ }
+
/** {@inheritDoc} */
- public Iterable<String> resolve(final CriteriaSet criteria) throws ResolverException {
- final String value = fetchValue(criteria);
+ @Override
+ public @Nonnull Iterable<String> resolve(@Nonnull final CriteriaSet criteria) throws ResolverException {
+ final String value = resolveSingle(criteria);
if (value != null) {
final LazySet<String> result = new LazySet<>();
result.add(value);
@@ -77,30 +86,37 @@ public class PropertiesClientSecretValueResolver extends AbstractIdentifiableIni
return Collections.emptySet();
}
- @Override
/** {@inheritDoc} */
- public String resolveSingle(final CriteriaSet criteria) throws ResolverException {
- return fetchValue(criteria);
- }
-
- /**
- * Fetch the value from properties.
- *
- * @param criteria The criteria containing {@link ClientSecretReferenceCriterion}.
- * @return The value found from properties, or null if no value was found.
- */
- protected String fetchValue(final CriteriaSet criteria) {
+ @Override
+ public @Nullable String resolveSingle(@Nonnull final CriteriaSet criteria) throws ResolverException {
ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
final ClientSecretReferenceCriterion criterion = criteria.get(ClientSecretReferenceCriterion.class);
+ final Properties properties = new Properties();
+ try {
+ properties.load(propertiesResource.getInputStream());
+ } catch (IOException e) {
+ log.error("Could not read the properties from the resource", e);
+ throw new ResolverException(e);
+ }
return properties.getProperty(criterion.getSecretReference());
}
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
+
+ if (getResource() == null) {
+ throw new ComponentInitializationException("Properties resource cannot be null");
+ }
+ }
/** {@inheritDoc} */
@Override
protected void doDestroy() {
- properties = null;
+ propertiesResource = null;
super.doDestroy();
}
diff --git a/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/BaseClientSecretValueResolverTest.java b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/BaseClientSecretValueResolverTest.java
new file mode 100644
index 00000000..be823959
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/BaseClientSecretValueResolverTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2017 - 2020, GÉANT
+ *
+ * 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 org.geant.idpextension.oidc.metadata.impl;
+
+import org.geant.idpextension.oidc.criterion.ClientSecretReferenceCriterion;
+import org.geant.idpextension.oidc.metadata.resolver.ClientSecretValueResolver;
+import org.testng.annotations.Test;
+
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.DestroyedComponentException;
+import net.shibboleth.utilities.java.support.component.UninitializedComponentException;
+import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
+import net.shibboleth.utilities.java.support.resolver.ResolverException;
+
+/**
+ * Base unit tests for all implementations of {@link ClientSecretValueResolver}.
+ *
+ * @param <T> The implementation to be tested.
+ */
+public abstract class BaseClientSecretValueResolverTest<T extends AbstractClientSecretValueResolver> {
+
+ T resolver;
+
+ @Test(expectedExceptions = UninitializedComponentException.class)
+ public void resolveSingle_shouldThrowIfCalledBeforeInit() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(false);
+ resolver.resolveSingle(buildCriteriaSet("ignored"));
+ }
+
+ @Test(expectedExceptions = DestroyedComponentException.class)
+ public void resolveSingle_shouldThrowIfCalledAfterDestroy() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ resolver.destroy();
+ resolver.resolveSingle(buildCriteriaSet("ignored"));
+ }
+
+ @Test(expectedExceptions = UninitializedComponentException.class)
+ public void resolve_shouldThrowIfCalledBeforeInit() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(false);
+ resolver.resolve(buildCriteriaSet("ignored"));
+ }
+
+ @Test(expectedExceptions = DestroyedComponentException.class)
+ public void resolve_shouldThrowIfCalledAfterDestroy() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ resolver.destroy();
+ resolver.resolve(buildCriteriaSet("ignored"));
+ }
+
+ protected CriteriaSet buildCriteriaSet(final String secretReference) {
+ final ClientSecretReferenceCriterion criterion = new ClientSecretReferenceCriterion(secretReference);
+ return new CriteriaSet(criterion);
+ }
+
+ protected abstract T buildResolver(boolean init) throws ComponentInitializationException;
+}
diff --git a/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolverTest.java b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolverTest.java
new file mode 100644
index 00000000..91e16311
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/org/geant/idpextension/oidc/metadata/impl/PropertiesClientSecretValueResolverTest.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017 - 2020, GÉANT
+ *
+ * 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 org.geant.idpextension.oidc.metadata.impl;
+
+import java.util.Iterator;
+
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.Resource;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.UnmodifiableComponentException;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+import net.shibboleth.utilities.java.support.resolver.ResolverException;
+
+/**
+ * Unit tests for {@link PropertiesClientSecretValueResolver}.
+ */
+public class PropertiesClientSecretValueResolverTest
+ extends BaseClientSecretValueResolverTest<PropertiesClientSecretValueResolver> {
+
+ Resource resource;
+
+ String clientSecretKeyReference = "mockClientSecretKey";
+ String clientSecretValue = "mockClientSecretmockClientSecretmockClientSecret";
+
+ @BeforeMethod
+ public void init() {
+ resource = new ClassPathResource("org/geant/idpextension/oidc/metadata/impl/client-secret-test.properties");
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void initialize_shouldThrowWhenResourceNotSet() throws ComponentInitializationException {
+ resolver = new PropertiesClientSecretValueResolver();
+ resolver.setId("mockId");
+ resolver.initialize();
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void setResource_shouldThrowIfResourceIsNul() {
+ resolver = new PropertiesClientSecretValueResolver();
+ resolver.setResource(null);
+ }
+
+ @Test(expectedExceptions = UnmodifiableComponentException.class)
+ public void setResource_shouldThrowAfterInit() throws ComponentInitializationException {
+ resolver = buildResolver(true);
+ resolver.setResource(resource);
+ }
+
+ @Test
+ public void resolveSingle_shouldReturnNullWhenPropertyNotFound() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ Assert.assertNull(resolver.resolveSingle(buildCriteriaSet("not_found")));
+ }
+
+ @Test
+ public void resolveSingle_shouldReturnPropertyWhenFound() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ Assert.assertEquals(resolver.resolveSingle(buildCriteriaSet(clientSecretKeyReference)), clientSecretValue);
+ }
+
+ @Test
+ public void resolve_shouldReturnEmptyIteratorWhenPropertyNotFound() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ final Iterable<String> iterable = resolver.resolve(buildCriteriaSet("not_found"));
+ Assert.assertNotNull(iterable);
+ Assert.assertFalse(iterable.iterator().hasNext());
+ }
+
+ @Test
+ public void resolve_shouldReturnSingleValueIteratorWhenPropertyFound() throws ResolverException,
+ ComponentInitializationException {
+ resolver = buildResolver(true);
+ final Iterable<String> iterable = resolver.resolve(buildCriteriaSet(clientSecretKeyReference));
+ Assert.assertNotNull(iterable);
+ final Iterator<String> iterator = iterable.iterator();
+ Assert.assertTrue(iterator.hasNext());
+ Assert.assertEquals(iterator.next(), clientSecretValue);
+ Assert.assertFalse(iterator.hasNext());
+ }
+
+ protected PropertiesClientSecretValueResolver buildResolver(boolean init) throws ComponentInitializationException {
+ resolver = new PropertiesClientSecretValueResolver();
+ resolver.setId("mockId");
+ resolver.setResource(resource);
+ if (init) {
+ resolver.initialize();
+ }
+ return resolver;
+ }
+
+}
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 317d1851..e568ce17 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
@@ -31,8 +31,8 @@
</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" />
+ class="org.geant.idpextension.oidc.metadata.impl.PropertiesClientSecretValueResolver"
+ p: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" />
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list