[java-idp-plugin-duo] branch main updated: JDUO-79 - Duo integration objects violate null/init constraints
Phil Smart
philip.smart at jisc.ac.uk
Fri Nov 24 16:32:48 UTC 2023
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=dc5cd8e09048fec0e4e556d26d177dcd7b290c23
The following commit(s) were added to refs/heads/main by this push:
new dc5cd8e0 JDUO-79 - Duo integration objects violate null/init constraints
dc5cd8e0 is described below
commit dc5cd8e09048fec0e4e556d26d177dcd7b290c23
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Nov 24 16:32:41 2023 +0000
JDUO-79 - Duo integration objects violate null/init constraints
- Guard nonnull getters
- Fix tests that did not initialize the default OIDC integration
https://shibboleth.atlassian.net/browse/JDUO-79
---
.../authn/duo/DefaultDuoOIDCIntegration.java | 80 ++++++++++-----------
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 3 +-
.../authn/duo/DefaultDuoOIDCIntegrationTest.java | 82 ++++++++++++++++++++++
.../authn/duo/impl/AbstractDuoActionTest.java | 4 +-
.../duo/impl/DefaultDuoOIDCClientRegistryTest.java | 23 +++---
.../authn/duo/impl/DualDuoIntegrationStrategy.java | 57 +++++++++------
.../impl/DuoAudienceClaimLookupStrategyTest.java | 5 +-
.../plugin/authn/duo/impl/DuoAuthnFlowTest.java | 40 +++++++----
.../duo/impl/DuoIssuerClaimLookupStrategyTest.java | 1 +
.../authn/duo/impl/DuoOIDCAuthnControllerTest.java | 14 +++-
.../duo/impl/ExchangeCodeForDuoTokenTest.java | 4 ++
.../authn/duo/impl/ValidateTokenSignatureTest.java | 5 ++
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 28 ++++++--
.../duo/sdk/impl/DuoSDKClientFactoryTest.java | 5 +-
14 files changed, 254 insertions(+), 97 deletions(-)
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
index 5e983dbc..a89b9336 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
@@ -16,7 +16,6 @@ package net.shibboleth.idp.plugin.authn.duo;
import java.security.Principal;
import java.util.Collection;
-import java.util.Collections;
import java.util.Objects;
import java.util.Set;
@@ -33,6 +32,7 @@ import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
@@ -88,7 +88,7 @@ public final class DefaultDuoOIDCIntegration
/** Constructor. */
public DefaultDuoOIDCIntegration() {
supportedPrincipals = new Subject();
- allowedOrigins = Collections.emptySet();
+ allowedOrigins = CollectionSupport.emptySet();
}
/**
@@ -97,21 +97,21 @@ public final class DefaultDuoOIDCIntegration
* @param hosts the hostnames to allow.
*/
public synchronized void setAllowedOrigins(@Nullable @NonnullElements final Collection<String> hosts) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
- allowedOrigins = Set.copyOf(StringSupport.normalizeStringCollection(
+ checkSetterPreconditions();
+ allowedOrigins = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(
Constraint.isNotNull(hosts, "Types cannot be null")));
}
@Override
@Nonnull @NotLive @Unmodifiable public synchronized Set<String> getAllowedOrigins() {
//set is unmodifiable and string is immutable - so not live.
- return Collections.unmodifiableSet(allowedOrigins);
+ return CollectionSupport.copyToSet(allowedOrigins);
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getAPIHost() {
+ checkComponentActive();
+ assert apiHost != null;
return apiHost;
}
@@ -121,14 +121,14 @@ public final class DefaultDuoOIDCIntegration
* @param host API host
*/
public synchronized void setAPIHost(@Nonnull @NotEmpty final String host) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
apiHost = Constraint.isNotNull(StringSupport.trimOrNull(host), "API host cannot be null or empty");
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getHealthCheckEndpoint() {
+ checkComponentActive();
+ assert healthEndpoint != null;
return healthEndpoint;
}
@@ -138,15 +138,15 @@ public final class DefaultDuoOIDCIntegration
* @param endpoint the endpoint.
*/
public synchronized void setHealthCheckEndpoint(@Nonnull @NotEmpty final String endpoint) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
healthEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Health check endpoint cannot be null or empty");
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getAuthorizeEndpoint() {
+ checkComponentActive();
+ assert authorizeEndpoint != null;
return authorizeEndpoint;
}
@@ -156,15 +156,15 @@ public final class DefaultDuoOIDCIntegration
* @param endpoint the endpoint.
*/
public synchronized void setAuthorizeEndpoint(@Nonnull @NotEmpty final String endpoint) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
authorizeEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Authorize endpoint cannot be null or empty");
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getTokenEndpoint() {
+ checkComponentActive();
+ assert tokenEndpoint != null;
return tokenEndpoint;
}
@@ -174,9 +174,7 @@ public final class DefaultDuoOIDCIntegration
* @param endpoint the endpoint.
*/
public synchronized void setTokenEndpoint(@Nonnull @NotEmpty final String endpoint) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
tokenEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Token endpoint cannot be null or empty");
}
@@ -192,9 +190,7 @@ public final class DefaultDuoOIDCIntegration
* @param uri the redirect_uri
*/
public synchronized void setRegisteredRedirectURI(@Nullable final String uri) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
registeredRedirectURI = StringSupport.trimOrNull(uri);
}
@@ -213,7 +209,8 @@ public final class DefaultDuoOIDCIntegration
@Override
public synchronized void setRedirectURIIfAbsent(
- @Nonnull @NotEmpty final String computedRedirectURI){
+ @Nonnull @NotEmpty final String computedRedirectURI){
+ // Specifically do not check if component has been initialized. This can change during use.
Constraint.isNotEmpty(computedRedirectURI, "Computed redirect URI can not be null or empty");
if (redirectURI == null) {
@@ -228,14 +225,14 @@ public final class DefaultDuoOIDCIntegration
* @param id the client identifier.
*/
public synchronized void setClientId(@Nonnull @NotEmpty final String id) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
clientId = Constraint.isNotNull(StringSupport.trimOrNull(id), "ClientID cannot be null or empty");
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getClientId() {
+ checkComponentActive();
+ assert clientId != null;
return clientId;
}
@@ -245,14 +242,14 @@ public final class DefaultDuoOIDCIntegration
* @param key secret key
*/
public synchronized void setSecretKey(@Nonnull @NotEmpty final String key) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
secretKey = Constraint.isNotNull(StringSupport.trimOrNull(key), "Secret key cannot be null or empty");
}
@Override
@NonnullAfterInit @NotEmpty public synchronized String getSecretKey() {
+ checkComponentActive();
+ assert secretKey != null;
return secretKey;
}
@@ -260,7 +257,9 @@ public final class DefaultDuoOIDCIntegration
@Override
@Nonnull @NonnullElements @Unmodifiable
public synchronized <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
- return supportedPrincipals.getPrincipals(c);
+ final Set<T> result = supportedPrincipals.getPrincipals(c);
+ assert result != null;
+ return result;
}
/**
@@ -274,9 +273,7 @@ public final class DefaultDuoOIDCIntegration
*/
public synchronized <T extends Principal> void setSupportedPrincipals(
@Nullable @NonnullElements final Collection<T> principals) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
-
+ checkSetterPreconditions();
supportedPrincipals.getPrincipals().clear();
if (principals != null && !principals.isEmpty()) {
@@ -286,14 +283,17 @@ public final class DefaultDuoOIDCIntegration
@Override
protected void doInitialize() throws ComponentInitializationException {
- if (getAPIHost() == null || getClientId() == null || getSecretKey() == null
- || getHealthCheckEndpoint() == null || getAuthorizeEndpoint() == null
- || getTokenEndpoint() == null || (getRegisteredRedirectURI() == null
- && getAllowedOrigins().isEmpty())) {
- throw new ComponentInitializationException("API host, clientId, secret key,"
- + "token endpoint, health check endpoint, authorization endpoint, and one of "
- + "redirectURI or allowed redirect URI origins must be set");
+ synchronized (this) {
+ if (apiHost == null || clientId == null || secretKey == null
+ || healthEndpoint == null || authorizeEndpoint == null
+ || tokenEndpoint == null || (registeredRedirectURI == null
+ && allowedOrigins.isEmpty())) {
+ throw new ComponentInitializationException("API host, clientId, secret key,"
+ + "token endpoint, health check endpoint, authorization endpoint, and one of "
+ + "redirectURI or allowed redirect URI origins must be set");
+ }
}
+
}
@Override
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
index a79acd4b..cfa7e347 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
@@ -23,7 +23,8 @@ import net.shibboleth.shared.annotation.constraint.NotEmpty;
/**
* Interface to a particular Duo OIDC integration point. In part replaces
- * OIDC metadata, as that is not supported by Duo.
+ * OIDC metadata, as that is not supported by Duo. Implementations must override the {@link #equals(Object)}
+ * and {@link #hashCode()} methods appropriately (e.g., using the clientId as a way to differentiate between objects).
*/
public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
diff --git a/idp-duo-api/src/test/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegrationTest.java b/idp-duo-api/src/test/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegrationTest.java
index acc1c653..fac30382 100644
--- a/idp-duo-api/src/test/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegrationTest.java
+++ b/idp-duo-api/src/test/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegrationTest.java
@@ -82,7 +82,89 @@ public class DefaultDuoOIDCIntegrationTest {
integration.initialize();
}
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoclientId() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setRegisteredRedirectURI("/callback");
+ integration.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoSecret() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoTokenEndpoint() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+
+ integration.setHealthCheckEndpoint("/health");
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoHealthEndpoint() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoAuthzEndpoint() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoApiHost() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
index 30fb5e56..40f82746 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
@@ -80,6 +80,8 @@ public abstract class AbstractDuoActionTest {
protected DuoOIDCAuthenticationContext dc;
+ protected DefaultDuoOIDCIntegration integ;
+
/**
* <p>Setup the relevant contexts per method execution.</p>
*
@@ -886,7 +888,7 @@ public abstract class AbstractDuoActionTest {
* @return a dummy Duo integration.
*/
@Nonnull protected DefaultDuoOIDCIntegration createDummyDuoIntegration() {
- final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
+ integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost(API_HOST);
integ.setClientId(CLIENT_ID);
integ.setRegisteredRedirectURI(REDIRECT_URI);
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
index 297958e5..3ce30685 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
@@ -70,9 +70,13 @@ public class DefaultDuoOIDCClientRegistryTest {
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("host.com");
+ integ.setAuthorizeEndpoint("https://host.com/authz");
+ integ.setTokenEndpoint("https://host.com/token");
+ integ.setHealthCheckEndpoint("https://host.com/health");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
integ.setRegisteredRedirectURI("http://localhost/");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ.initialize();
final DuoOIDCClient client = registry.getClientOrCreate(integ);
final DuoOIDCClient clientTwo = registry.getClientOrCreate(integ);
@@ -80,11 +84,19 @@ public class DefaultDuoOIDCClientRegistryTest {
assertEquals(client, clientTwo);
// set to a different value
- integ.setClientId("DIU6GEFWG5LIUBVV2M3B");
+ final DefaultDuoOIDCIntegration integ2 = new DefaultDuoOIDCIntegration();
+ integ2.setAPIHost("host.com");
+ integ2.setAuthorizeEndpoint("https://host.com/authz");
+ integ2.setTokenEndpoint("https://host.com/token");
+ integ2.setHealthCheckEndpoint("https://host.com/health");
+ integ2.setClientId("DIU6GEFWG5LIUBVV2M3P");
+ integ2.setRegisteredRedirectURI("http://localhost/");
+ integ2.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ2.setClientId("DIU6GEFWG5LIUBVV2M3B");
+ integ2.initialize();
// should get a new client.
- final DuoOIDCClient clientThree = registry.getClientOrCreate(integ);
+ final DuoOIDCClient clientThree = registry.getClientOrCreate(integ2);
- System.out.println("Client: " + client + " Client2: " + clientTwo + " Client3: " + clientThree);
assertNotSame(client, clientThree);
}
@@ -129,11 +141,6 @@ public class DefaultDuoOIDCClientRegistryTest {
final DuoOIDCClient client = f.get();
System.out.println("client:"+client);
}
-
-
-
-
-
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DualDuoIntegrationStrategy.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DualDuoIntegrationStrategy.java
index e3d9f0f9..53e69b97 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DualDuoIntegrationStrategy.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DualDuoIntegrationStrategy.java
@@ -22,6 +22,7 @@ import org.testng.Assert;
import net.shibboleth.idp.plugin.authn.duo.DefaultDuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.profile.context.RelyingPartyContext;
+import net.shibboleth.shared.component.ComponentInitializationException;
/**
* Function for testing multiple duo integrations.
@@ -32,28 +33,42 @@ public class DualDuoIntegrationStrategy implements Function<ProfileRequestContex
@SuppressWarnings("null")
public DuoOIDCIntegration apply(final ProfileRequestContext prc) {
- final DefaultDuoOIDCIntegration int1 = new DefaultDuoOIDCIntegration();
- int1.setAPIHost("host.com");
- int1.setClientId(DuoAuthnFlowTest.FIRST_INTEGRATION_CLIENT_ID);
- int1.setRegisteredRedirectURI("http://localhost/");
- int1.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
-
- final DefaultDuoOIDCIntegration int2 = new DefaultDuoOIDCIntegration();
- int2.setAPIHost("host.com");
- int2.setClientId(DuoAuthnFlowTest.SECOND_INTEGRATION_CLIENT_ID);
- int2.setRegisteredRedirectURI("http://localhost/");
- int2.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
-
- if (DuoAuthnFlowTest.FIRST_INTEGRATION_SP.equals(
- prc.getSubcontext(RelyingPartyContext.class).getRelyingPartyId())) {
- return int1;
- } else if (DuoAuthnFlowTest.SECOND_INTEGRATION_SP.equals(
- prc.getSubcontext(RelyingPartyContext.class).getRelyingPartyId())){
- return int2;
+ try {
+ final DefaultDuoOIDCIntegration int1 = new DefaultDuoOIDCIntegration();
+ int1.setAPIHost("host.com");
+ int1.setAuthorizeEndpoint("https://host.com/authz");
+ int1.setTokenEndpoint("https://host.com/token");
+ int1.setHealthCheckEndpoint("https://host.com/health");
+ int1.setClientId(DuoAuthnFlowTest.FIRST_INTEGRATION_CLIENT_ID);
+ int1.setRegisteredRedirectURI("http://localhost/");
+ int1.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ int1.initialize();
+
+ final DefaultDuoOIDCIntegration int2 = new DefaultDuoOIDCIntegration();
+ int2.setAPIHost("host.com");
+ int2.setAuthorizeEndpoint("https://host.com/authz");
+ int2.setTokenEndpoint("https://host.com/token");
+ int2.setHealthCheckEndpoint("https://host.com/health");
+ int2.setClientId(DuoAuthnFlowTest.SECOND_INTEGRATION_CLIENT_ID);
+ int2.setRegisteredRedirectURI("http://localhost/");
+ int2.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ int2.initialize();
+
+ if (DuoAuthnFlowTest.FIRST_INTEGRATION_SP.equals(
+ prc.getSubcontext(RelyingPartyContext.class).getRelyingPartyId())) {
+ return int1;
+ } else if (DuoAuthnFlowTest.SECOND_INTEGRATION_SP.equals(
+ prc.getSubcontext(RelyingPartyContext.class).getRelyingPartyId())){
+ return int2;
+ }
+ //fail if none chosen.
+ Assert.fail();
+ return null;
+ } catch (final ComponentInitializationException e) {
+ //fail if can not init the integrations.
+ Assert.fail(e.getMessage());
+ return null;
}
- //fail if none chosen.
- Assert.fail();
- return null;
}
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
index 2b6e642d..10c8138d 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
@@ -30,9 +30,10 @@ public class DuoAudienceClaimLookupStrategyTest extends AbstractDuoActionTest{
}
@Test
- public void applySuccess() {
+ public void applySuccess() throws ComponentInitializationException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
final String audience = strategy.apply(prc,new JWTClaimsSet.Builder().build());
final var integration = dc.getIntegration();
assertNotNull(integration);
@@ -41,7 +42,7 @@ public class DuoAudienceClaimLookupStrategyTest extends AbstractDuoActionTest{
}
@Test
- public void applyNoDuoContext() {
+ public void applyNoDuoContext() throws ComponentInitializationException {
final String audience = strategy.apply(prc,new JWTClaimsSet.Builder().build());
assertEquals(audience, null);
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
index 319a5bc0..24f3c6ce 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
@@ -52,6 +52,7 @@ import net.shibboleth.idp.saml.authn.principal.AuthnContextClassRefPrincipal;
import net.shibboleth.profile.context.RelyingPartyContext;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
+import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
@@ -305,9 +306,10 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
* correctly add the ACR to the set of principals.
*
* @throws DuoClientException on error.
+ * @throws ComponentInitializationException on error
*/
@Test
- public void testContextToPrincipalMappingStrategy() throws DuoClientException {
+ public void testContextToPrincipalMappingStrategy() throws DuoClientException, ComponentInitializationException {
setFlowPath(FLOW);
setFlowModelResources(flowResources);
setSubflows(subflows);
@@ -347,7 +349,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
integ.setHealthCheckEndpoint("/health");
integ.setTokenEndpoint("/token");
integ.setRegisteredRedirectURI("http://localhost/authorization-callback");
-
+ integ.initialize();
duoContext.setIntegration(integ);
//add the mock client as was not added by the populate stage
@@ -379,10 +381,15 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
- /** Test the Duo flow from the external authorization request to the end of the flow.
- * @throws DuoClientException if the client can not be created.*/
+ /**
+ *
+ * Test the Duo flow from the external authorization request to the end of the flow.
+ *
+ * @throws DuoClientException if the client can not be created.
+ * @throws ComponentInitializationException on error
+ */
@Test
- public void testDuoAuthnFlowFromAuthorizationCallback() throws DuoClientException {
+ public void testDuoAuthnFlowFromAuthorizationCallback() throws DuoClientException, ComponentInitializationException {
setRemoveDefaultContextCleanupHook(true);
setFlowPath(FLOW);
@@ -421,7 +428,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
integ.setHealthCheckEndpoint("/health");
integ.setTokenEndpoint("/token");
integ.setRegisteredRedirectURI("http://localhost/authorization-callback");
-
+ integ.initialize();
duoContext.setIntegration(integ);
@@ -453,13 +460,17 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
}
- /** Test the Duo flow from the external authorization request to the end of the flow with a requested
+ /**
+ * Test the Duo flow from the external authorization request to the end of the flow with a requested
* principal set.
*
- * @throws DuoClientException if the client can not be created.*/
+ * @throws DuoClientException if the client can not be created.
+ * @throws ComponentInitializationException on error
+ */
//TODO: finish this, AFD is not checked in the dummy flow, and we are not checking auth result principals.
@Test
- public void testDuoAuthnFlowFromAuthorizationCallbackWithRPC() throws DuoClientException {
+ public void testDuoAuthnFlowFromAuthorizationCallbackWithRPC()
+ throws DuoClientException, ComponentInitializationException {
setRemoveDefaultContextCleanupHook(true);
setFlowPath(FLOW);
@@ -498,7 +509,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
integ.setHealthCheckEndpoint("/health");
integ.setTokenEndpoint("/token");
integ.setRegisteredRedirectURI("http://localhost/authorization-callback");
-
+ integ.initialize();
duoContext.setIntegration(integ);
//add the mock client as was not added by the populate stage
@@ -548,9 +559,12 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
* Test the Duo flow from the external authorization request. This should fail, as forced authn is
* requested but the auth_time is from a previous authentication (to far in the past).
*
- * @throws DuoClientException if the client can not be created.*/
+ * @throws DuoClientException if the client can not be created.
+ * @throws ComponentInitializationException on error
+ */
@Test
- public void testDuoAuthnFlowFromAuthorizationCallbackForceAuthnFailure() throws DuoClientException {
+ public void testDuoAuthnFlowFromAuthorizationCallbackForceAuthnFailure()
+ throws DuoClientException, ComponentInitializationException {
setFlowPath(FLOW);
setFlowModelResources(flowResources);
@@ -589,7 +603,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
integ.setAuthorizeEndpoint("/authorize");
integ.setHealthCheckEndpoint("/health");
integ.setTokenEndpoint("/token");
-
+ integ.initialize();
duoContext.setIntegration(integ);
//add the mock client as was not added by the populate stage
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
index 2e583ade..97eaca12 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
@@ -46,6 +46,7 @@ public class DuoIssuerClaimLookupStrategyTest extends AbstractDuoActionTest{
public void applySuccess() throws ComponentInitializationException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
//set a different path for testing.
final String issuer = strategy.apply(prc,new JWTClaimsSet.Builder().build());
final var integration = dc.getIntegration();
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
index cc2b8c2c..1069bdfc 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
@@ -64,6 +64,7 @@ import net.shibboleth.idp.session.IdPSession;
import net.shibboleth.idp.session.context.SessionContext;
import net.shibboleth.idp.ui.context.RelyingPartyUIContext;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.component.ComponentInitializationException;
/**
* Tests for the {@link DuoOIDCAuthnController}.
@@ -127,7 +128,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
@Test
@SuppressWarnings("null")
public void testSuccessfulAuthorizeRequest() throws Exception {
-
+
final MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/authorize")
.param("conversation", "e1s1"))
.andDo(print())
@@ -266,8 +267,9 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
* IdP's configuration of the {@link ServletContextAttributeExporter}.
*
* @throws DuoClientException on error creating the duoclient.
+ * @throws ComponentInitializationException on error
*/
- private void exportServletContextAttributes() throws DuoClientException {
+ private void exportServletContextAttributes() throws DuoClientException, ComponentInitializationException {
final FlowExecutorImpl mockFlowExecutor = Mockito.mock(FlowExecutorImpl.class);
final FlowExecutionRepository mockFlowExecutionRepo = Mockito.mock(FlowExecutionRepository.class);
@@ -292,8 +294,10 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
*
* @return a profile request context.
* @throws DuoClientException on error creating the duo client
+ * @throws ComponentInitializationException on error
*/
- @Nonnull private ProfileRequestContext buildProfileRequestContext() throws DuoClientException {
+ @Nonnull private ProfileRequestContext buildProfileRequestContext()
+ throws DuoClientException, ComponentInitializationException {
final ProfileRequestContext prc = new ProfileRequestContext();
final AuthenticationContext ac = new AuthenticationContext();
@@ -305,9 +309,13 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost(API_HOST);
+ integ.setTokenEndpoint(API_HOST+"/token");
+ integ.setHealthCheckEndpoint(API_HOST+"/health");
+ integ.setAuthorizeEndpoint(API_HOST+"/authz");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
integ.setRegisteredRedirectURI("http://localhost/");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ.initialize();
dc.setUsername("jdoe");
dc.setIntegration(integ);
dc.setClient(new MockDuoOIDCClient_OK(integ));
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
index e4db179d..90d1f03c 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
@@ -47,6 +47,7 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
public void testExecuteSuccess() throws ComponentInitializationException, DuoRegistryException, DuoClientException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
//add an auth code
dc.setAuthorizationCode("testcode");
final var integration = dc.getIntegration();
@@ -66,6 +67,7 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
// blank the auth code.
dc.setAuthorizationCode(null);
final var integration = dc.getIntegration();
@@ -85,6 +87,7 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
//do not set client
//dc.setClient(new MockDuoOIDCClient_OK(dc.getIntegration()));
@@ -101,6 +104,7 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
dc.setAuthorizationCode("testcode");
//blank username
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
index 02976f22..f7965214 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
@@ -72,6 +72,7 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
public final void testNoneSignature() throws ComponentInitializationException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
addAttemptedFlow("authn/DuoOIDC");
dc.setAuthToken(createPlainDummyToken(DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW,"Login Succesful",CLIENT_ID,
Instant.now().plus(1,ChronoUnit.MINUTES),Instant.now(), Instant.now(),
@@ -92,6 +93,7 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
public final void testUnsuportedSignature() throws ComponentInitializationException, EncodingException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
addAttemptedFlow("authn/DuoOIDC");
//unsupported asymmetric RSA algorithm
@@ -116,6 +118,7 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
public final void testValidSignature() throws ComponentInitializationException, EncodingException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
addAttemptedFlow("authn/DuoOIDC");
final String headerJson = "{\"typ\": \"JWT\",\"alg\": \"HS256\"}";
@@ -143,6 +146,7 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
public final void testInvalidSignature() throws ComponentInitializationException, EncodingException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
addAttemptedFlow("authn/DuoOIDC");
final String headerJson = "{\"typ\": \"JWT\",\"alg\": \"HS256\"}";
@@ -169,6 +173,7 @@ public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
public final void testSignatureNotPresent() throws ComponentInitializationException, EncodingException {
addDuoContext();
addDuoIntegrationToContext();
+ integ.initialize();
addAttemptedFlow("authn/DuoOIDC");
final String headerJson = "{\"typ\": \"JWT\",\"alg\": \"HS256\"}";
diff --git a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
index 308e0467..53d10350 100644
--- a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
+++ b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
@@ -98,13 +98,17 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
try {
if (caCerts == null) {
//will use the default certs in the Client if the caCerts are null
- client = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
+ final Client newClient = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
integration.getAPIHost(), integration.getRedirectURI()).setUseDuoCodeAttribute(false).build();
+ assert newClient != null;
+ client = newClient;
} else {
- client = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
+ final Client newClient = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
integration.getAPIHost(), integration.getRedirectURI()).setCACerts(
caCerts.toArray(new String[caCerts.size()]))
.setUseDuoCodeAttribute(false).build();
+ assert newClient != null;
+ client = newClient;
}
} catch (final DuoException e) {
//wrap exception and throw
@@ -122,8 +126,11 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
if (response == null) {
throw new DuoClientException("Duo health check response was null");
}
- return healthCheckResponseConverter.apply(response);
-
+ final DuoHealthCheck check = healthCheckResponseConverter.apply(response);
+ if (check == null) {
+ throw new DuoClientException("Converted Duo health check response was null");
+ }
+ return check;
} catch (final DuoException e) {
//wrap duo specific exception.
@@ -148,7 +155,9 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
Constraint.isNotEmpty(state, "State can not be null or empty");
//does not support the nonce or redirect_uri override
try {
- return client.createAuthUrl(username, state);
+ final String authUrl = client.createAuthUrl(username, state);
+ assert authUrl != null;
+ return authUrl;
} catch (final DuoException e) {
//wrap duo specific exception.
throw new DuoClientException(e);
@@ -190,9 +199,11 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
@Override
public DuoHealthCheck apply(@Nullable final HealthCheckResponse response) {
assert response != null;
+ final Integer timestamp = response.getResponse().getTimestamp();
+ assert timestamp != null;
return DuoHealthCheck.builder().withStatus(response.getStat()).withCode(response.getCode())
.withMessage(response.getMessage()).withMessageDetail(response.getMessage_detail())
- .withResponse(new DuoHealthCheckResponse(response.getResponse().getTimestamp()))
+ .withResponse(new DuoHealthCheckResponse(timestamp))
.withTimestamp(response.getTimestamp()).build();
}
@@ -222,9 +233,12 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
try {
final String duoTokenAsJson = objectMapper.writeValueAsString(t);
final JWTClaimsSet claims = JWTClaimsSet.parse(duoTokenAsJson);
+ assert claims != null;
//re-sign the JWT using an incompatible key for the given algorithm!
//FIXME please change this Duo!
- return JWSAssemblyUtils.assembleMacJws(JWSAlgorithm.HS512,claims,
+ final JWSAlgorithm algo = JWSAlgorithm.HS512;
+ assert algo != null;
+ return JWSAssemblyUtils.assembleMacJws(algo, claims,
JWSAssemblyUtils.getSecretBytes(integ.getSecretKey()));
} catch (final JsonProcessingException | ParseException | JOSEException | EncodingException e) {
diff --git a/idp-duo-sdk-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactoryTest.java b/idp-duo-sdk-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactoryTest.java
index 94e38fe8..161b0f36 100644
--- a/idp-duo-sdk-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactoryTest.java
+++ b/idp-duo-sdk-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactoryTest.java
@@ -39,6 +39,7 @@ import org.testng.annotations.Test;
import net.shibboleth.idp.plugin.authn.duo.DefaultDuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
+import net.shibboleth.shared.component.ComponentInitializationException;
/** Test for the DuoSDKClientFactory.*/
public class DuoSDKClientFactoryTest {
@@ -75,9 +76,10 @@ public class DuoSDKClientFactoryTest {
* Test creation.
*
* @throws DuoClientException on error.
+ * @throws ComponentInitializationException on error
*/
@Test
- public final void testCreateInstance() throws DuoClientException {
+ public final void testCreateInstance() throws DuoClientException, ComponentInitializationException {
final List<String> certs = new ArrayList<>();
certs.add("sha256/I/Lt/z7ekCWanjD0Cvj5EqXls2lOaThEA0H2Bg4BT/o=");
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
@@ -88,6 +90,7 @@ public class DuoSDKClientFactoryTest {
integ.setAuthorizeEndpoint("/oauth/v1/authorize");
integ.setTokenEndpoint("/oauth/v1/token");
integ.setHealthCheckEndpoint("/oauth/v1/health_check");
+ integ.initialize();
final var registeredRedirect = integ.getRegisteredRedirectURI();
assertNotNull(registeredRedirect);
assert registeredRedirect != null;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list