[java-idp-plugin-duo] branch main updated: Cleanup the DuoOIDCIntegration interface
Phil Smart
philip.smart at jisc.ac.uk
Wed Mar 10 16:12:03 UTC 2021
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=eed7affce23379f8c92b42c95753079087a80c17
The following commit(s) were added to refs/heads/main by this push:
new eed7aff Cleanup the DuoOIDCIntegration interface
eed7aff is described below
commit eed7affce23379f8c92b42c95753079087a80c17
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Mar 10 16:12:01 2021 +0000
Cleanup the DuoOIDCIntegration interface
- Create an extended DuoOIDCIntegration interface to support the
dynamic redirect_uri parameters, leaving the original interface intact.
This allows for simpler integration wrappers such as the new (but not
used by default) SimpleDuoOIDCIntegration.
- Includes some JavaDoc fixes.
---
.../authn/duo/DefaultDuoOIDCIntegration.java | 33 +++----
.../authn/duo/DuoOIDCClientCapabilities.java | 14 +--
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 42 +-------
...gration.java => DynamicDuoOIDCIntegration.java} | 79 +++------------
...egration.java => SimpleDuoOIDCIntegration.java} | 106 +++++---------------
.../authn/duo/DefaultDuoOIDCIntegrationTest.java | 92 ++++++++++++++++++
.../impl/DefaultRedirectURICreationStrategy.java | 13 +--
.../duo/impl/PopulateDuoAuthenticationContext.java | 107 +++++++++++++--------
.../duo/impl/DefaultDuoOIDCClientRegistryTest.java | 6 +-
.../DefaultRedirectURICreationStrategyTest.java | 5 +-
.../impl/PopulateDuoAuthenticationContextTest.java | 70 +++++++++++---
.../plugin/authn/mock/MockDuoOIDCClient_FAIL.java | 11 +--
.../plugin/authn/mock/MockDuoOIDCClient_OK.java | 8 --
.../mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java | 8 --
.../authn/mock/MockDuoOIDCClient_UNKNOWN.java | 8 --
.../plugin/authn/duo/nimbus/impl/NimbusClient.java | 26 ++---
.../duo/nimbus/impl/NimbusClientFactoryTest.java | 7 +-
.../duo/nimbus/impl/NimbusClientSupportTest.java | 10 +-
.../authn/duo/nimbus/impl/NimbusClientTest.java | 33 ++++++-
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 22 ++---
.../authn/duo/sdk/impl/DuoSDKClientFactory.java | 2 +-
.../duo/sdk/impl/DuoSDKClientFactoryTest.java | 12 ++-
22 files changed, 351 insertions(+), 363 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 17e0803..8da13f7 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
@@ -51,7 +51,8 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
*
*/
@ThreadSafe
-public final class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
+public final class DefaultDuoOIDCIntegration
+ extends AbstractInitializableComponent implements DynamicDuoOIDCIntegration{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoOIDCIntegration.class);
@@ -111,7 +112,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getAPIHost() {
+ @NonnullAfterInit @NotEmpty public synchronized String getAPIHost() {
return apiHost;
}
@@ -127,7 +128,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getHealthCheckEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getHealthCheckEndpoint() {
return healthEndpoint;
}
@@ -144,7 +145,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getAuthorizeEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getAuthorizeEndpoint() {
return authorizeEndpoint;
}
@@ -161,7 +162,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getTokenEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getTokenEndpoint() {
return tokenEndpoint;
}
@@ -176,17 +177,6 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
tokenEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Token endpoint cannot be null or empty");
}
-
- /**
- * Set the redirect_uri to use.
- *
- * @param uri the redirect_uri
- */
- public synchronized void setRedirectURI(@Nullable final String uri) {
- ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- redirectURI = StringSupport.trimOrNull(uri);
- }
@Override
@Nullable public synchronized String getRedirectURI() {
@@ -223,7 +213,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
Constraint.isNotEmpty(computedRedirectURI, "Computed redirect URI can not be null or empty");
if (redirectURI == null) {
- log.debug("Redirect_uri is being set as '{}'",computedRedirectURI);
+ log.debug("Integration redirect_uri is being pinned to '{}'",computedRedirectURI);
redirectURI = computedRedirectURI;
}
}
@@ -240,7 +230,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getClientId() {
+ @NonnullAfterInit @NotEmpty public synchronized String getClientId() {
return clientId;
}
@@ -256,7 +246,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getSecretKey() {
+ @NonnullAfterInit @NotEmpty public synchronized String getSecretKey() {
return secretKey;
}
@@ -291,9 +281,10 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
protected void doInitialize() throws ComponentInitializationException {
if (getAPIHost() == null || getClientId() == null || getSecretKey() == null
|| getHealthCheckEndpoint() == null || getAuthorizeEndpoint() == null
- || getTokenEndpoint() == null || (getRedirectURI() == null && getAllowedOrigins() == 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"
+ + "token endpoint, health check endpoint, authorization endpoint, and one of "
+ "redirectURI or allowed redirect URI origins must be set");
}
}
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientCapabilities.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientCapabilities.java
index cf42550..f5ab2a3 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientCapabilities.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientCapabilities.java
@@ -30,17 +30,7 @@ public interface DuoOIDCClientCapabilities {
*
* @return true iff the client supports the nonce parameter, false otherwise.
*/
- boolean isSupportsNonce();
-
-
- /**
- * <p>Does this client support a redirectURI supplied dynamically per-request.</p>
- *
- * <p>If the client does, a redirectURI will be supplied (and should be used)
- * when constructing the authorization redirect, and during auth_code for id_token exchange.</p>
- *
- * @return true iff the client supports a per-request redirectURI, false otherwise.
- */
- boolean isSupportsDynamicRedirectURI();
+ boolean isSupportsNonce();
+
}
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 72fba30..2339e87 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
@@ -17,16 +17,11 @@
package net.shibboleth.idp.plugin.authn.duo;
-
-import java.util.Set;
-
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
-import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
/**
* Interface to a particular Duo OIDC integration point. In part replaces
@@ -53,47 +48,14 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
*
* @return the secret key
*/
- @Nonnull @NotEmpty String getSecretKey();
-
- /**
- * Get a list of origins that allowed to appear in computed redirect URIs.
- *
- * @return a set of unmodifiable allowed origins.
- * Never {@literal null} but could be empty.
- */
- @Nonnull @NotLive @Unmodifiable Set<String> getAllowedOrigins();
-
- /**
- * <p>Set the redirectURI from one in a thread-safe way.</p>
- *
- * <p>This is a sate changing operation, and should be thread-safe in its operation, as the
- * integration will be shared amongst threads e.g. syncrhonize the method call.</p>
- *
- * @param computedRedirectURI the runtime computed redirectURI.
- *
- */
- void setRedirectURIIfAbsent(@Nonnull @NotEmpty final String computedRedirectURI);
+ @Nonnull @NotEmpty String getSecretKey();
/**
* Get the runtime redirectURI to direct the client to after authorisation.
*
* @return the redirectURI
*/
- @Nullable String getRedirectURI();
-
- /**
- * Get the static pre-registered redirect_uri.
- *
- * @return the pre-registered redirect_uri.
- */
- @Nullable String getRegisteredRedirectURI();
-
- /**
- * Has a redirectURI been pre-registered i.e. set in the properties file.
- *
- * @return true iff {@link #getRegisteredRedirectURI()} is not null, false otherwise.
- */
- @Nullable boolean isRedirectURIPreregistered();
+ @Nullable String getRedirectURI();
/**
* Get the path of the health check endpoint.
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/DynamicDuoOIDCIntegration.java
similarity index 56%
copy from idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
copy to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DynamicDuoOIDCIntegration.java
index 72fba30..7b0289b 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/DynamicDuoOIDCIntegration.java
@@ -15,72 +15,32 @@
* limitations under the License.
*/
-package net.shibboleth.idp.plugin.authn.duo;
+package net.shibboleth.idp.plugin.authn.duo;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
-import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
/**
- * Interface to a particular Duo OIDC integration point. In part replaces
- * OIDC metadata, as that is not supported by Duo.
+ * Specialised {@link DuoOIDCIntegration} that allows redirectURIs to be pre-registered or computed dynamically
+ * against a list of allowed origins.
*/
-public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
+public interface DynamicDuoOIDCIntegration extends DuoOIDCIntegration{
/**
- * Get the name of the API host to contact.
- *
- * @return name of API host
- */
- @Nonnull @NotEmpty String getAPIHost();
-
- /**
- * Get the clientId key.
- *
- * @return the integration key
- */
- @Nonnull @NotEmpty String getClientId();
-
- /**
- * Get the secret key.
- *
- * @return the secret key
- */
- @Nonnull @NotEmpty String getSecretKey();
-
- /**
- * Get a list of origins that allowed to appear in computed redirect URIs.
+ * Get a list of URL origins that are allowed to appear in computed redirect URIs.
*
* @return a set of unmodifiable allowed origins.
* Never {@literal null} but could be empty.
*/
@Nonnull @NotLive @Unmodifiable Set<String> getAllowedOrigins();
- /**
- * <p>Set the redirectURI from one in a thread-safe way.</p>
- *
- * <p>This is a sate changing operation, and should be thread-safe in its operation, as the
- * integration will be shared amongst threads e.g. syncrhonize the method call.</p>
- *
- * @param computedRedirectURI the runtime computed redirectURI.
- *
- */
- void setRedirectURIIfAbsent(@Nonnull @NotEmpty final String computedRedirectURI);
-
- /**
- * Get the runtime redirectURI to direct the client to after authorisation.
- *
- * @return the redirectURI
- */
- @Nullable String getRedirectURI();
-
/**
* Get the static pre-registered redirect_uri.
*
@@ -93,30 +53,19 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
*
* @return true iff {@link #getRegisteredRedirectURI()} is not null, false otherwise.
*/
- @Nullable boolean isRedirectURIPreregistered();
+ boolean isRedirectURIPreregistered();
- /**
- * Get the path of the health check endpoint.
- *
- * @return the path of the health check endpoint
- */
- @Nonnull @NotEmpty String getHealthCheckEndpoint();
/**
- * Get the path of the authorization endpoint.
+ * <p>Set the redirectURI from the one given in a thread-safe way.</p>
*
- * @return the path of the authorization endpoint
- */
- @Nonnull @NotEmpty String getAuthorizeEndpoint();
-
- /**
- * Get the path of the token endpoint.
+ * <p>Once set, {@link #getRedirectURI()} should return the value of {@literal computedRedirectURI}.</p>
+ *
+ * <p>This is a sate changing operation, and should be thread-safe in its operation, as the
+ * integration will be shared amongst threads e.g. synchronize the method call.</p>
+ *
+ * @param computedRedirectURI the runtime computed redirectURI.
*
- * @return the path of the token endpoint;
*/
- @Nonnull @NotEmpty String getTokenEndpoint();
-
-
-
-
+ void setRedirectURIIfAbsent(@Nonnull @NotEmpty final String computedRedirectURI);
}
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/SimpleDuoOIDCIntegration.java
similarity index 68%
copy from idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
copy to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/SimpleDuoOIDCIntegration.java
index 17e0803..5ddc4d0 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/SimpleDuoOIDCIntegration.java
@@ -19,7 +19,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;
@@ -35,7 +34,6 @@ import org.slf4j.LoggerFactory;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
@@ -44,17 +42,14 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
/**
- * Data wrapper for use with Duo OIDC integrations. Holds shared-state.
- *
- * <p>Despite most fields being 'effectively immutable' once published by Spring, the redirectURI is allowed
- * to change, but in a guarded, shared-thread-safe manor.</p>
- *
+ * A data wrapper for use with Duo OIDC integrations which does not support redirectURI generation.
*/
@ThreadSafe
-public final class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
+public final class SimpleDuoOIDCIntegration
+ extends AbstractInitializableComponent implements DuoOIDCIntegration{
/** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoOIDCIntegration.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(SimpleDuoOIDCIntegration.class);
/** API host. */
@GuardedBy("this") @NonnullAfterInit @NotEmpty private String apiHost;
@@ -66,13 +61,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
@GuardedBy("this") @NonnullAfterInit @NotEmpty private String secretKey;
/** The used (by clients) redirect_uri to send the client after authorisation .*/
- @GuardedBy("this") @Nullable private String redirectURI;
-
- /** A statically set (pre-registered) redirectURI to send the client to after authorisation.*/
- @GuardedBy("this") @Nullable private String registeredRedirectURI;
-
- /** A set of 'allowed' origins that can be used as the scheme, host, and port portion of the redirectURI.*/
- @GuardedBy("this") @Nonnull @NonnullElements @Unmodifiable private Set<String> allowedOrigins;
+ @GuardedBy("this") @Nullable private String redirectURI;
/** The URL path to the health endpoint.*/
@GuardedBy("this") @NonnullAfterInit @NotEmpty private String healthEndpoint;
@@ -87,34 +76,17 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
@GuardedBy("this") @Nonnull private final Subject supportedPrincipals;
/** Constructor. */
- public DefaultDuoOIDCIntegration() {
+ public SimpleDuoOIDCIntegration() {
supportedPrincipals = new Subject();
- allowedOrigins = Collections.emptySet();
}
- /**
- * Set the origins that are allowed to form the scheme, host, and port part of a computed redirect_uri.
- *
- * @param hosts the hostnames to allow.
- */
- public synchronized void setAllowedOrigins(@Nullable @NonnullElements final Collection<String> hosts) {
- ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- allowedOrigins = Set.copyOf(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);
- }
@Override
- @Nonnull @NotEmpty public synchronized String getAPIHost() {
+ @NonnullAfterInit @NotEmpty public synchronized String getAPIHost() {
return apiHost;
}
-
+
/**
* Set the API host to use.
*
@@ -127,7 +99,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getHealthCheckEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getHealthCheckEndpoint() {
return healthEndpoint;
}
@@ -144,7 +116,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getAuthorizeEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getAuthorizeEndpoint() {
return authorizeEndpoint;
}
@@ -161,7 +133,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getTokenEndpoint() {
+ @NonnullAfterInit @NotEmpty public synchronized String getTokenEndpoint() {
return tokenEndpoint;
}
@@ -176,17 +148,6 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
tokenEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Token endpoint cannot be null or empty");
}
-
- /**
- * Set the redirect_uri to use.
- *
- * @param uri the redirect_uri
- */
- public synchronized void setRedirectURI(@Nullable final String uri) {
- ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- redirectURI = StringSupport.trimOrNull(uri);
- }
@Override
@Nullable public synchronized String getRedirectURI() {
@@ -194,39 +155,16 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
/**
- * Set the redirect_uri to use.
+ * Set the redirect URI.
*
- * @param uri the redirect_uri
+ * @param url the url.
*/
- public synchronized void setRegisteredRedirectURI(@Nullable final String uri) {
+ public synchronized void setRedirectURI(@Nonnull final String url) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- registeredRedirectURI = StringSupport.trimOrNull(uri);
- }
-
- @Override
- @Nullable public synchronized String getRegisteredRedirectURI() {
- return registeredRedirectURI;
- }
-
- @Override
- public synchronized boolean isRedirectURIPreregistered() {
- if (getRegisteredRedirectURI() == null) {
- return false;
- }
- return true;
- }
-
- @Override
- @Nonnull @NotEmpty public synchronized void setRedirectURIIfAbsent(
- @Nonnull @NotEmpty final String computedRedirectURI){
- Constraint.isNotEmpty(computedRedirectURI, "Computed redirect URI can not be null or empty");
- if (redirectURI == null) {
- log.debug("Redirect_uri is being set as '{}'",computedRedirectURI);
- redirectURI = computedRedirectURI;
- }
+ redirectURI = Constraint.isNotNull(StringSupport.trimOrNull(url), "Redirect URI cannot be null or empty");;
}
+
/**
* Set the client ID to use.
@@ -240,7 +178,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getClientId() {
+ @NonnullAfterInit @NotEmpty public synchronized String getClientId() {
return clientId;
}
@@ -256,7 +194,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
}
@Override
- @Nonnull @NotEmpty public synchronized String getSecretKey() {
+ @NonnullAfterInit @NotEmpty public synchronized String getSecretKey() {
return secretKey;
}
@@ -291,10 +229,10 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
protected void doInitialize() throws ComponentInitializationException {
if (getAPIHost() == null || getClientId() == null || getSecretKey() == null
|| getHealthCheckEndpoint() == null || getAuthorizeEndpoint() == null
- || getTokenEndpoint() == null || (getRedirectURI() == null && getAllowedOrigins() == null)) {
+ || getTokenEndpoint() == null || getRedirectURI() == null) {
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");
+ + "token endpoint, health check endpoint, authorization endpoint, and "
+ + "redirectURI must be set");
}
}
@@ -314,7 +252,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
if (getClass() != obj.getClass()) {
return false;
}
- final DefaultDuoOIDCIntegration other = (DefaultDuoOIDCIntegration) obj;
+ final SimpleDuoOIDCIntegration other = (SimpleDuoOIDCIntegration) obj;
return Objects.equals(getClientId(), other.getClientId());
}
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
new file mode 100644
index 0000000..e4c0dfc
--- /dev/null
+++ b/idp-duo-api/src/test/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegrationTest.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You 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.idp.plugin.authn.duo;
+
+import org.testng.annotations.Test;
+
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+
+import java.util.Set;
+
+/** Test for the DefaultDuoOIDCIntegration. */
+public class DefaultDuoOIDCIntegrationTest {
+
+ /**
+ * Try to initialize an integration with a registered redirectURI, but without the allowed origins set.
+ *
+ * @throws ComponentInitializationException on error
+ */
+ @Test
+ public void testEverythingSetWithRegisteredRedirectURI() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setRegisteredRedirectURI("/callback");
+ integration.initialize();
+ }
+
+ /**
+ * Try to initialize an integration without the registered redirectURI, but with the allowed origins set.
+ *
+ * @throws ComponentInitializationException on error
+ */
+ @Test
+ public void testEverythingSetWithAllowedOrigins() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ integration.setAllowedOrigins(Set.of("https://host.com"));
+ integration.initialize();
+ }
+
+ /**
+ * Try to initialize an integration but without setting the registered redirectURI or the
+ * allowed origins list. Should throw.
+ *
+ * @throws ComponentInitializationException on error
+ */
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testNoRedirectURIOrAllowedHost() throws ComponentInitializationException {
+
+ final DefaultDuoOIDCIntegration integration = new DefaultDuoOIDCIntegration();
+ integration.setAPIHost("host.com");
+ integration.setAuthorizeEndpoint("/authorize");
+ integration.setClientId("CLIENTID");
+ integration.setSecretKey("secret");
+ integration.setTokenEndpoint("/token");
+ integration.setHealthCheckEndpoint("/health");
+ //must have either RegisteredRedirectURI or the allowedOrigins set so one can be computed
+ //integration.setRegisteredRedirectURI("/callback");
+ integration.initialize();
+ }
+
+
+
+
+
+}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategy.java
index 7d61ee1..011fce4 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategy.java
@@ -30,8 +30,9 @@ import javax.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
+import net.shibboleth.idp.plugin.authn.duo.DynamicDuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.URISupport;
+import net.shibboleth.utilities.java.support.annotation.ParameterName;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -39,7 +40,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* Constructive, pure, function that returns a redirect_uri from one of (ordered):
* <ol>
* <li>A pre-registered redirect_uri on the Duo integration,
- * {@link DuoOIDCIntegration#getRegisteredRedirectURI}. Or, if none are pre-registered;
+ * {@link DynamicDuoOIDCIntegration#getRegisteredRedirectURI()}. Or, if none are pre-registered;
* <li>Derived from the HTTP Servlet request server parameters, checking the origin
* against an allowed set of origins from the Duo integration - to prevent Host header injection.
* </ol>
@@ -51,7 +52,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
@ThreadSafe
@Immutable
public final class DefaultRedirectURICreationStrategy
- implements BiFunction<HttpServletRequest, DuoOIDCIntegration, String>{
+ implements BiFunction<HttpServletRequest, DynamicDuoOIDCIntegration, String>{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultRedirectURICreationStrategy.class);
@@ -65,14 +66,14 @@ public final class DefaultRedirectURICreationStrategy
* @param callbackPath the path segment relative to the servlet path of the callback endpoint.
*/
public DefaultRedirectURICreationStrategy(
- @Nonnull @NotEmpty final String callbackPath) {
+ @Nonnull @NotEmpty @ParameterName(name="callbackPath") final String callbackPath) {
callbackServletPath = Constraint.isNotNull(callbackPath,"Duo Call back path can not be null");
}
@Override
@Nullable public String apply(@Nonnull final HttpServletRequest request,
- @Nonnull final DuoOIDCIntegration integration) {
+ @Nonnull final DynamicDuoOIDCIntegration integration) {
final String redirectFromIntegration = integration.getRegisteredRedirectURI();
if (redirectFromIntegration != null) {
@@ -92,7 +93,7 @@ public final class DefaultRedirectURICreationStrategy
}
return uri.toString();
} catch (final URISyntaxException e) {
- log.warn("Unable to generate redirectURI",e);
+ log.warn("Unable to generate redirectURI, {}",e.getMessage());
}
return null;
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContext.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContext.java
index 27f55f8..a9fee3c 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContext.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContext.java
@@ -21,6 +21,7 @@ import java.util.function.BiFunction;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
@@ -33,10 +34,11 @@ import org.slf4j.LoggerFactory;
import net.shibboleth.idp.authn.AbstractAuthenticationAction;
import net.shibboleth.idp.authn.AuthnEventIds;
import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.idp.plugin.authn.duo.DuoException;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientRegistry;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
-import net.shibboleth.idp.plugin.authn.duo.DuoRegistryException;
+import net.shibboleth.idp.plugin.authn.duo.DynamicDuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
import net.shibboleth.idp.session.context.navigate.CanonicalUsernameLookupStrategy;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
@@ -73,8 +75,9 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
/** Lookup strategy for Duo integration. */
@Nonnull private Function<ProfileRequestContext, DuoOIDCIntegration> duoIntegrationLookupStrategy;
- /** Strategy used to compute the redirectURI from the given Duo integration.*/
- @NonnullAfterInit private BiFunction<HttpServletRequest, DuoOIDCIntegration, String> redirectURICreationStrategy;
+ /** Strategy used to compute the redirectURI from the given Duo integration if supported.*/
+ @Nullable
+ private BiFunction<HttpServletRequest, DynamicDuoOIDCIntegration, String> redirectURICreationStrategy;
/** The registry for locating the DuoClient for the established integration.*/
@NonnullAfterInit private DuoOIDCClientRegistry clientRegistry;
@@ -95,7 +98,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param duoRegistry the registry
*/
- public synchronized void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
+ public void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -107,7 +110,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public synchronized void setUsernameLookupStrategy(
+ public void setUsernameLookupStrategy(
@Nonnull final Function<ProfileRequestContext, String> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -121,8 +124,8 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy the creation strategy.
*/
- public synchronized void setRedirectURICreationStrategy(
- @Nonnull final BiFunction<HttpServletRequest, DuoOIDCIntegration, String> strategy) {
+ public void setRedirectURICreationStrategy(
+ @Nonnull final BiFunction<HttpServletRequest, DynamicDuoOIDCIntegration, String> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -135,7 +138,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public synchronized void setDuoContextCreationStrategy(
+ public void setDuoContextCreationStrategy(
@Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -149,7 +152,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public synchronized void setDuoIntegrationLookupStrategy(
+ public void setDuoIntegrationLookupStrategy(
@Nonnull final Function<ProfileRequestContext, DuoOIDCIntegration> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -164,9 +167,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
if (clientRegistry == null) {
throw new ComponentInitializationException("Duo Client Registry cannot be null");
}
- if (redirectURICreationStrategy == null) {
- throw new ComponentInitializationException("Redirect URI creation strategy cannot be null");
- }
+
}
/** {@inheritDoc} */
@@ -196,21 +197,6 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
return;
}
- // A redirectURI is built for every request even if the client only sets it internally,
- // immutable, during the first request.
- final String redirectURI = redirectURICreationStrategy.apply(request,duoIntegration);
- if (redirectURI == null) {
- log.warn("{} A redirect_uri was not registered, or could not be computed", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- return;
- }
-
- // set the redirectURI iff not already set. This should happen at least once on the first
- // request (whichever thread gets here first), but not thereafter. This allows it to be
- // computed from the request, and is compatible with clients that do not support per-request
- // redirect_uri overrides (if required).
- duoIntegration.setRedirectURIIfAbsent(redirectURI);
-
final String username = usernameLookupStrategy.apply(profileRequestContext);
if (username == null) {
log.warn("{} No principal name available to initiate a Duo 2FA request", getLogPrefix());
@@ -219,20 +205,15 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
}
context.setUsername(username);
- //Configure the Duo client for the established integration
try {
+ computeAndStoreRedirectURIIfSupported(duoIntegration, request, context);
+
+ //Configure the Duo client for the established integration
final DuoOIDCClient client = clientRegistry.getClientOrCreate(duoIntegration);
context.setClient(client);
- // If supported, set a redirect override on the context - so it can be used per-request.
- if (client.getCapabilities().isSupportsDynamicRedirectURI() &&
- !duoIntegration.isRedirectURIPreregistered()) {
- log.debug("Redirect_uri was not pre-registered and client supports dynamic redirect_uris, "
- + "setting the redirect_uri to '{}",redirectURI);
- context.setRedirectURIOverride(redirectURI);
- }
- } catch (final DuoRegistryException e) {
- log.warn("{} No DuoClient established (located or created) for "
- + "this integration", getLogPrefix(),e);
+
+ } catch (final DuoException e) {
+ log.warn("{} Unable to establish a Duo Client for the given integration", getLogPrefix(),e);
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
return;
}
@@ -240,5 +221,53 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
log.debug("Created Duo authentication context for '{}'",username);
}
-
+ /**
+ * <p>For {@link DynamicDuoOIDCIntegration DynamicDuoOIDCIntegrations}, apply the redirect_uri creation
+ * strategy to compute a redirect_uri to use.</p>
+ *
+ * <p>The redirect_uri is computed for each request, but is only set once as the usable redirect_uri
+ * on the integration itself i.e. for the client to read using {@link DuoOIDCIntegration#getRedirectURI()}.
+ * This allows all clients to see a computed (by the {@code redirectURICreationStrategy}) redirect_uri
+ * from the first request onward.</p>
+ *
+ * <p>The computed redirect_uri is also added to the context as an override redirect_uri which - if
+ * supported by the client - can be used dynamically when creating authorization or token exchange
+ * requests.</p>
+ *
+ * @param duoIntegration the Duo integration pertaining to this request.
+ * @param request the http servlet request.
+ * @param context the Duo authentication context to store the computed override redirect_uri.
+ *
+ * @throws DuoException if the redirect_uri could not be created by the strategy.
+ */
+ private void computeAndStoreRedirectURIIfSupported(@Nonnull final DuoOIDCIntegration duoIntegration,
+ @Nonnull final HttpServletRequest request,
+ @Nonnull final DuoOIDCAuthenticationContext context) throws DuoException {
+
+ if (duoIntegration instanceof DynamicDuoOIDCIntegration) {
+
+ if (redirectURICreationStrategy == null) {
+ throw new DuoException("A dynamic DuoOIDC integration was supplied, but the redirect URI"
+ + " creation strategy was null. Please set a redirect URI creation strategy.");
+ }
+
+ final String redirectURI = redirectURICreationStrategy.apply(request,
+ (DynamicDuoOIDCIntegration)duoIntegration);
+ if (redirectURI == null) {
+ throw new DuoException("A redirect_uri was not registered, or could not be computed");
+ }
+
+ // set the redirectURI iff not already set. This should happen at least once on the first
+ // request (whichever thread gets here first), but not thereafter. This allows it to be
+ // computed from the request, and is compatible with clients that do not support per-request
+ // redirect_uri overrides (if required).
+ ((DynamicDuoOIDCIntegration)duoIntegration).setRedirectURIIfAbsent(redirectURI);
+
+ //always add to the context for the client to use if it supports dynamic redirect_uris
+ log.trace("{} Adding a dynamic redirect_uri '{}' to the context for the DuoClient to use if "
+ + "supported",getLogPrefix(),redirectURI);
+ context.setRedirectURIOverride(redirectURI);
+ }
+ }
+
}
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 0aae48c..5cd1e0b 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
@@ -25,7 +25,6 @@ import static org.testng.Assert.assertNotSame;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
@@ -38,7 +37,6 @@ 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.idp.plugin.authn.duo.DuoOIDCClientFactory;
-import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientRegistry;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoRegistryException;
import net.shibboleth.idp.plugin.authn.mock.MockDuoOIDCClientFactory_OK_Client;
@@ -77,7 +75,7 @@ public class DefaultDuoOIDCClientRegistryTest {
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("host.com");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
- integ.setRedirectURI("http://localhost/");
+ integ.setRegisteredRedirectURI("http://localhost/");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
final DuoOIDCClient client = registry.getClientOrCreate(integ);
@@ -122,7 +120,7 @@ public class DefaultDuoOIDCClientRegistryTest {
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("host.com");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
- integ.setRedirectURI("http://localhost/");
+ integ.setRegisteredRedirectURI("http://localhost/");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
final ExecutorService service = Executors.newFixedThreadPool(2);
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
index 136c0e3..5cd2b22 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
@@ -29,7 +29,7 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
-import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
+import net.shibboleth.idp.plugin.authn.duo.DynamicDuoOIDCIntegration;
/** Tests for the DefaultDuoOIDCIntegration class.*/
@@ -42,8 +42,7 @@ public class DefaultRedirectURICreationStrategyTest {
private DefaultRedirectURICreationStrategy strategy;
/** A dummy integration to use.*/
- @Mock
- private DuoOIDCIntegration integration;
+ @Mock private DynamicDuoOIDCIntegration integration;
/** HTTP request mock.*/
private MockHttpServletRequest request;
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContextTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContextTest.java
index c29a2ee..53a766c 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContextTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/PopulateDuoAuthenticationContextTest.java
@@ -31,10 +31,10 @@ import net.shibboleth.idp.authn.AuthnEventIds;
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.idp.plugin.authn.duo.DuoOIDCClientCapabilities;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientRegistry;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoRegistryException;
+import net.shibboleth.idp.plugin.authn.duo.SimpleDuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import static org.mockito.ArgumentMatchers.any;
@@ -72,20 +72,26 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
@Test
public void testExecuteSuccessWithStaticClient()
throws ComponentInitializationException, DuoRegistryException, DuoClientException {
- final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
- //set the duo integration strategy to lookup this
+
+ final SimpleDuoOIDCIntegration integ = new SimpleDuoOIDCIntegration();
+ integ.setAPIHost(API_HOST);
+ integ.setClientId(CLIENT_ID);
+ integ.setRedirectURI("https://simple-static.redirect/");
+ integ.setSecretKey(SECRET);
+ integ.setAuthorizeEndpoint(AUTHORIZE_URI);
+ integ.setTokenEndpoint(TOKEN_URI);
+ integ.setHealthCheckEndpoint(HEALTH_URI);
+
+
+ //set the duo integration strategy to lookup
action.setDuoIntegrationLookupStrategy(prc -> integ);
//lookup a username
action.setUsernameLookupStrategy(prc -> "jdoe");
- action.setRedirectURICreationStrategy((http,duoInteg)
- -> "http://example.com/idp/profile/Authn/Duo/2FA/callback");
+
action.setHttpServletRequest(request);
final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
final DuoOIDCClient mockClient = Mockito.mock(DuoOIDCClient.class);
Mockito.when(mockClientRegistry.getClientOrCreate(any(DuoOIDCIntegration.class))).thenReturn(mockClient);
- final DuoOIDCClientCapabilities caps = Mockito.mock(DuoOIDCClientCapabilities.class);
- Mockito.when(mockClient.getCapabilities()).thenReturn(caps);
- Mockito.when(caps.isSupportsDynamicRedirectURI()).thenReturn(false);
action.setClientRegistry(mockClientRegistry);
@@ -93,6 +99,41 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
final Event event = action.execute(src);
//success here is a null event
assertNull(event);
+ //simple client, so no override set
+ assertEquals(ac.getSubcontext(DuoOIDCAuthenticationContext.class)
+ .getRedirectURIOverride(), null);
+ }
+
+ /**
+ * Test that should return an authentication exception if the integration is dynamic
+ * but a redirectURI strategy has not been set.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoRegistryException on error.
+ */
+ @Test
+ public void testExecuteDynamicClientButNoRedirectCreationStrategy()
+ throws ComponentInitializationException, DuoRegistryException {
+
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+
+ request.addHeader("Host", "example.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+ action.setHttpServletRequest(request);
+
+ final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
+ final DuoOIDCClient mockClient = Mockito.mock(DuoOIDCClient.class);
+ Mockito.when(mockClientRegistry.getClientOrCreate(any(DuoOIDCIntegration.class))).thenReturn(mockClient);
+
+ action.setClientRegistry(mockClientRegistry);
+
+ action.initialize();
+ final Event event = action.execute(src);
+ assertEventId(event, AuthnEventIds.AUTHN_EXCEPTION);
}
/**
@@ -107,7 +148,7 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
throws ComponentInitializationException, DuoRegistryException, DuoClientException {
final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
//cleanout pre-registered redirect
- integ.setRedirectURI(null);
+ integ.setRegisteredRedirectURI(null);
//set the duo integration strategy to lookup this
action.setDuoIntegrationLookupStrategy(prc -> integ);
//lookup a username
@@ -123,9 +164,7 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
final DuoOIDCClient mockClient = Mockito.mock(DuoOIDCClient.class);
Mockito.when(mockClientRegistry.getClientOrCreate(any(DuoOIDCIntegration.class))).thenReturn(mockClient);
- final DuoOIDCClientCapabilities caps = Mockito.mock(DuoOIDCClientCapabilities.class);
- Mockito.when(mockClient.getCapabilities()).thenReturn(caps);
- Mockito.when(caps.isSupportsDynamicRedirectURI()).thenReturn(true);
+
action.setClientRegistry(mockClientRegistry);
@@ -166,8 +205,7 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
}
/**
- * Test unsuccessful execution if there is no duo client registry or
- * redirectURI creation strategy specified.
+ * Test unsuccessful execution if there is no duo client registry specified.
*
* @throws ComponentInitializationException on error.
* @throws DuoClientException on error.
@@ -228,7 +266,7 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
action.setClientRegistry(mockClientRegistry);
action.initialize();
final Event event = action.execute(src);
- assertEventId(event,EventIds.INVALID_PROFILE_CTX);
+ assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
}
/**
@@ -291,7 +329,7 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
}
/**
- * Test unsuccessful execution if there is no HttpServletRequest.
+ * Test unsuccessful execution if there is no Duo Context.
*
* @throws ComponentInitializationException on error.
* @throws DuoClientException on error.
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
index f6bdd08..247719f 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_FAIL.java
@@ -146,11 +146,7 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
public boolean isSupportsNonce() {
return false;
}
-
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
+
@Override
public DuoOIDCClientCapabilities getCapabilities() {
@@ -161,10 +157,7 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
+
};
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
index 26164e5..c3bce91 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK.java
@@ -144,10 +144,6 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
@Override
public DuoOIDCClientCapabilities getCapabilities() {
@@ -158,10 +154,6 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
};
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
index 3d23873..29cc093 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java
@@ -146,10 +146,6 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
@Override
public DuoOIDCClientCapabilities getCapabilities() {
@@ -160,10 +156,6 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
};
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
index f2be23d..fa5af4f 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java
@@ -146,10 +146,6 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
@Override
public DuoOIDCClientCapabilities getCapabilities() {
@@ -160,10 +156,6 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
return false;
}
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
};
}
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
index 62e9340..959305e 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
@@ -53,11 +53,11 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
- * A Duo client using the Nimbus OIDC library. Can/should only be instantiated by the corresponding factory method.
+ * A Duo client using the Nimbus OIDC library.
*/
@ThreadSafe
@Immutable
-final public class NimbusClient extends AbstractDuoOIDCClient{
+public final class NimbusClient extends AbstractDuoOIDCClient{
/** The only supported client assertion type.*/
@Nonnull @NotEmpty private static final String CLIENT_ASSERTION_TYPE =
@@ -83,14 +83,16 @@ final public class NimbusClient extends AbstractDuoOIDCClient{
/**
- * Constructor.
+ * Package-private Constructor.
+ *
+ * <p>Should only be instantiated by the {@link NimbusClientFactory}.</p>
*
* @param integration the integration to create the client for, never {@code null}
* @param client the Http client to use to execute HTTP requests, never {@code null}
* @param params any security parameters to use for the Http client, can be {@code null}.
* @param oMapper the JSON object mapper, never {@code null}.
*/
- protected NimbusClient(@Nonnull final DuoOIDCIntegration integration, @Nonnull final HttpClient client,
+ NimbusClient(@Nonnull final DuoOIDCIntegration integration, @Nonnull final HttpClient client,
@Nullable final HttpClientSecurityParameters params, @Nonnull final ObjectMapper oMapper) {
super();
duoIntegration = Constraint.isNotNull(integration,"Nimbus Client requires a non-null Duo Integration");
@@ -116,14 +118,13 @@ final public class NimbusClient extends AbstractDuoOIDCClient{
return executeRequest(rb.build(), new TypeReference<DuoHealthCheck>() {});
} catch (final URISyntaxException e) {
- log.error("Error performing a Duo health check",e);
- throw new DuoClientException(e);
+ throw new DuoClientException("Error performing a Duo health check",e);
}
}
@Override
@Nonnull public String createAuthUrl(@Nonnull @NotEmpty final String username,
- @Nonnull @NotEmpty final String state, @Nonnull final String nonce,
+ @Nonnull @NotEmpty final String state, @Nullable final String nonce,
@Nullable final String redirectURIOverride) throws DuoClientException {
Constraint.isNotEmpty(username, "Username can not be null or empty");
Constraint.isNotEmpty(state, "State can not be null or empty");
@@ -151,8 +152,7 @@ final public class NimbusClient extends AbstractDuoOIDCClient{
return uri.toString();
} catch (final URISyntaxException e) {
- log.error("Unable to create a Duo authorization URL",e);
- throw new DuoClientException(e);
+ throw new DuoClientException("Unable to create a Duo authorization URL",e);
}
}
@@ -183,8 +183,7 @@ final public class NimbusClient extends AbstractDuoOIDCClient{
return SignedJWT.parse(response.getIdToken());
} catch (final URISyntaxException | ParseException e) {
- log.error("Unable to swap auth_code for id_token",e);
- throw new DuoClientException(e);
+ throw new DuoClientException("Unable to swap auth_code for id_token",e);
}
}
@@ -240,10 +239,5 @@ final public class NimbusClient extends AbstractDuoOIDCClient{
public boolean isSupportsNonce() {
return true;
}
-
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return true;
- }
}
diff --git a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactoryTest.java b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactoryTest.java
index 725f5e7..3e5b5f7 100644
--- a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactoryTest.java
+++ b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactoryTest.java
@@ -48,8 +48,13 @@ public class NimbusClientFactoryTest {
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("host.com");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
- integ.setRedirectURI("http://localhost/");
+ integ.setRegisteredRedirectURI("http://localhost/");
+ integ.setHealthCheckEndpoint("/health");
+ integ.setTokenEndpoint("/token");
+ integ.setAuthorizeEndpoint("/authorize");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ.setRedirectURIIfAbsent(integ.getRegisteredRedirectURI());
+ integ.initialize();
final DuoOIDCClient client = factory.createInstance(integ);
assertNotNull(client);
}
diff --git a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupportTest.java b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupportTest.java
index 6e78c0e..4ec718c 100644
--- a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupportTest.java
+++ b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupportTest.java
@@ -30,6 +30,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.utilities.java.support.component.ComponentInitializationException;
/** Tests for the NimbusClientSupport class.*/
public class NimbusClientSupportTest {
@@ -51,12 +52,17 @@ public class NimbusClientSupportTest {
@BeforeMethod
- public void setup() {
+ public void setup() throws ComponentInitializationException {
integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("host.com");
integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
- integ.setRedirectURI("http://localhost/");
+ integ.setRegisteredRedirectURI("http://localhost/");
+ integ.setHealthCheckEndpoint("/health");
+ integ.setTokenEndpoint("/token");
+ integ.setAuthorizeEndpoint("/authorize");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ.setRedirectURIIfAbsent(integ.getRegisteredRedirectURI());
+ integ.initialize();
}
/* Simple test that a JWS string is created and contains 3 components, header.payload.signature */
diff --git a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientTest.java b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientTest.java
index fab8ad6..b983f66 100644
--- a/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientTest.java
+++ b/idp-duo-nimbus-client-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientTest.java
@@ -41,6 +41,9 @@ import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
import javax.annotation.Nonnull;
@@ -149,12 +152,13 @@ public class NimbusClientTest {
integ = new DefaultDuoOIDCIntegration();
integ.setAPIHost("api-c9f24c5a.duosecurity.com");
integ.setClientId("DIU6GEFWG5LIUTVV2M3N");
- integ.setRedirectURI("http://localhost/");
+ integ.setRegisteredRedirectURI("http://localhost/");
integ.setSecretKey("TeXvZxKul47v1Wew2zb6xRPzAJewJ34MP2w8Uith");
integ.setAuthorizeEndpoint("/oauth/v1/authorize");
integ.setTokenEndpoint("/oauth/v1/token");
integ.setHealthCheckEndpoint("/oauth/v1/health_check");
-
+ integ.setRedirectURIIfAbsent(integ.getRegisteredRedirectURI());
+ integ.initialize();
}
@@ -326,6 +330,31 @@ public class NimbusClientTest {
}
+ /**
+ * Test the auth URL is constructed correctly when an override redirectURI is supplied.
+ *
+ * @throws DuoClientException on error.
+ * @throws UnsupportedEncodingException on error.
+ */
+ @Test public void testAuthURLWithOverrideRedirect() throws DuoClientException, UnsupportedEncodingException {
+ final HttpClient httpClient = Mockito.mock(HttpClient.class);
+ // create new client with mock response
+ client = new NimbusClient(integ, httpClient, null, new ObjectMapper());
+ final String authUrl = client.createAuthUrl
+ ("jdoe", "RRFSFERGERGSDDSXXXXQWERG","a_nonce","http://override.redirect/");
+ assertNotNull(authUrl);
+
+ assertTrue(authUrl.contains("response_type"));
+ assertTrue(authUrl.contains("client_id"));
+ assertTrue(authUrl.contains("request"));
+ assertTrue(authUrl.contains("redirect_uri"));
+ assertTrue(authUrl.contains("scope"));
+ assertTrue(authUrl.contains("nonce"));
+ assertTrue(authUrl.contains(
+ URLEncoder.encode("http://override.redirect/",StandardCharsets.UTF_8.toString())));
+
+ }
+
}
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 ee81ee5..0574c60 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
@@ -44,6 +44,7 @@ import com.nimbusds.jwt.JWTClaimsSet;
import net.shibboleth.idp.plugin.authn.duo.AbstractDuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
+import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheckResponse;
@@ -55,12 +56,10 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
/**
* <p>An Object Adaptor class for bridging between the Duo SDK implementation
* and the internal {@link DuoOIDCClient} interface.</p>
- *
- * <p>This is package private, and can/should only be instantiated by the {@link DuoSDKClientFactory}.</p>
*/
@ThreadSafe
@Immutable
-final public class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
+public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientAdaptor.class);
@@ -82,14 +81,16 @@ final public class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
/**
*
- * Constructor. Initialises the native Duo SDK client.
+ * Package-private constructor. Initialises the native Duo SDK client.
+ *
+ * <p>Should only be instantiated by the {@link DuoSDKClientFactory}.</p>
*
* @param integration the Duo integration to initialize the client from. Never {@code null}.
* @param caCerts the list of CA Certificates used to validate connections to Duo. Can be {@code null}.
*
* @throws DuoClientException if there is an error instantiating the client
*/
- protected DuoSDKClientAdaptor(@Nonnull final DuoOIDCIntegration integration,
+ DuoSDKClientAdaptor(@Nonnull final DuoOIDCIntegration integration,
@Nullable final List<String> caCerts) throws DuoClientException {
super();
duoIntegration = Constraint.isNotNull(integration,"Duo SDK Client requires a non-null Duo Integration");
@@ -136,7 +137,7 @@ final public class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
* {@inheritDoc}
*
* <p>The Duo WebSDK Client does not support either the {@code nonce} or {@code redirectURIOverride}
- * parameter.</p>
+ * parameters.</p>
*
*/
@Override
@@ -184,6 +185,7 @@ final public class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
/** Default health check response converter. */
+ @ThreadSafe
private class DefaultHealthCheckResponseConverter implements Function<HealthCheckResponse,DuoHealthCheck>{
@Override
@@ -236,12 +238,4 @@ final public class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
return false;
}
-
- @Override
- public boolean isSupportsDynamicRedirectURI() {
- return false;
- }
-
-
-
}
diff --git a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
index 481a90e..adedf96 100644
--- a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
+++ b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
@@ -40,7 +40,7 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
/** Abstract factory implementation for the {@link DuoSDKClientAdaptor}. */
@ThreadSafe
-public class DuoSDKClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory{
+public final class DuoSDKClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientFactory.class);
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 f9c1a86..4ae954e 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
@@ -87,10 +87,14 @@ public class DuoSDKClientFactoryTest {
final List<String> certs = new ArrayList<>();
certs.add("sha256/I/Lt/z7ekCWanjD0Cvj5EqXls2lOaThEA0H2Bg4BT/o=");
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
- integ.setAPIHost("host.com");
- integ.setClientId("DIU6GEFWG5LIUBVV2M3P");
- integ.setRedirectURI("http://localhost/");
- integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
+ integ.setAPIHost("api-c9f24c5a.duosecurity.com");
+ integ.setClientId("DIU6GEFWG5LIUTVV2M3N");
+ integ.setRegisteredRedirectURI("http://localhost/");
+ integ.setSecretKey("TeXvZxKul47v1Wew2zb6xRPzAJewJ34MP2w8Uith");
+ integ.setAuthorizeEndpoint("/oauth/v1/authorize");
+ integ.setTokenEndpoint("/oauth/v1/token");
+ integ.setHealthCheckEndpoint("/oauth/v1/health_check");
+ integ.setRedirectURIIfAbsent(integ.getRegisteredRedirectURI());
final DuoOIDCClient client = factory.createInstance(integ);
assertNotNull(client);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list