[java-idp-plugin-duo] branch main updated: JDUO-31 - Auto-determine idp.duo.oidc.redirectURL
Phil Smart
philip.smart at jisc.ac.uk
Thu Feb 25 17:14:01 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=e610441d6941df41e709369fce59b4b5010cc1cd
The following commit(s) were added to refs/heads/main by this push:
new e610441 JDUO-31 - Auto-determine idp.duo.oidc.redirectURL
e610441 is described below
commit e610441d6941df41e709369fce59b4b5010cc1cd
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Feb 25 17:13:59 2021 +0000
JDUO-31 - Auto-determine idp.duo.oidc.redirectURL
- Changes to support HTTP Host header driven redirectURL determination
from an allow list, if one is not pre-registered.
- Javdoc fixes
- Annotation cleanup
- Config cleanup
- Some concurrency cleanup
https://issues.shibboleth.net/jira/browse/JDUO-31
---
idp-duo-api/pom.xml | 10 +
.../plugin/authn/duo/AbstractDuoOIDCClient.java | 2 +
.../authn/duo/DefaultDuoOIDCIntegration.java | 184 ++++++++++++------
.../idp/plugin/authn/duo/DuoOIDCAuthAPI.java | 6 +
.../idp/plugin/authn/duo/DuoOIDCClient.java | 24 ++-
.../authn/duo/DuoOIDCClientCapabilities.java | 11 ++
.../plugin/authn/duo/DuoOIDCClientRegistry.java | 3 +-
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 49 ++++-
.../idp/plugin/authn/duo/URISupport.java | 84 +++++++++
.../duo/context/DuoOIDCAuthenticationContext.java | 31 ++-
idp-duo-api/src/test/resources/logback-test.xml | 41 ++++
.../duo/impl/DefaultDuoOIDCClientRegistry.java | 48 +++--
.../impl/DefaultRedirectURICreationStrategy.java | 101 ++++++++++
.../duo/impl/DuoAudienceClaimLookupStrategy.java | 8 +-
.../duo/impl/DuoIssuerClaimLookupStrategy.java | 6 +-
.../duo/impl/DuoNonceClaimLookupStrategy.java | 6 +-
.../authn/duo/impl/DuoOIDCAuthnController.java | 15 +-
.../duo/impl/DuoUsernameClaimLookupStrategy.java | 6 +-
.../authn/duo/impl/ExchangeCodeForDuoToken.java | 4 +-
.../authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java | 7 +-
.../duo/impl/PopulateDuoAuthenticationContext.java | 63 ++++++-
.../plugin/authn/duo/impl/ValidateTokenClaims.java | 1 +
.../flows/authn/DuoOIDC/duo-oidc-authn-beans.xml | 37 ++--
.../flows/authn/DuoOIDC/duo-oidc-authn-flow.xml | 2 +-
.../impl/AbstractAuthnXmlFlowExecutionTests.java | 4 +-
.../authn/duo/impl/AbstractDuoActionTest.java | 102 +++++++++-
.../duo/impl/DefaultDuoOIDCClientRegistryTest.java | 22 +++
.../DefaultRedirectURICreationStrategyTest.java | 188 +++++++++++++++++++
.../authn/duo/impl/DualDuoIntegrationStrategy.java | 4 +-
.../plugin/authn/duo/impl/DuoAuthnFlowTest.java | 9 +-
.../authn/duo/impl/DuoOIDCAuthnControllerTest.java | 4 +-
.../duo/impl/ExchangeCodeForDuoTokenTest.java | 35 ++++
.../impl/PopulateDuoAuthenticationContextTest.java | 208 ++++++++++++++++++++-
.../duo/impl/ValidateDuoResponseStateTest.java | 17 +-
.../ValidateDuoTokenAuthenticationResultTest.java | 20 ++
.../plugin/authn/mock/MockDuoOIDCClient_FAIL.java | 15 +-
.../plugin/authn/mock/MockDuoOIDCClient_OK.java | 15 +-
.../mock/MockDuoOIDCClient_OK_OLD_AUTH_TIME.java | 16 +-
.../authn/mock/MockDuoOIDCClient_UNKNOWN.java | 15 +-
.../plugin/authn/duo/nimbus/impl/NimbusClient.java | 36 ++--
.../authn/duo/nimbus/impl/NimbusClientFactory.java | 15 +-
.../authn/duo/nimbus/impl/NimbusClientSupport.java | 48 +++--
.../duo/nimbus/conf/authn/duo-oidc.properties | 7 +
.../duo/nimbus/impl/NimbusClientSupportTest.java | 5 +-
.../authn/duo/nimbus/impl/NimbusClientTest.java | 6 +-
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 46 +++--
.../authn/duo/sdk/impl/DuoSDKClientFactory.java | 32 +++-
.../authn/duo/sdk/conf/authn/duo-oidc.properties | 7 +
pom.xml | 5 +
49 files changed, 1402 insertions(+), 228 deletions(-)
diff --git a/idp-duo-api/pom.xml b/idp-duo-api/pom.xml
index 7d1f99a..3830625 100644
--- a/idp-duo-api/pom.xml
+++ b/idp-duo-api/pom.xml
@@ -45,7 +45,17 @@
<artifactId>jackson-datatype-jsr310</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>javax.servlet</groupId>
+ <artifactId>javax.servlet-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
<!-- Test dependencies -->
+ <dependency>
+ <groupId>${spring.groupId}</groupId>
+ <artifactId>spring-test</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoOIDCClient.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoOIDCClient.java
index 011bf1e..d63feb4 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoOIDCClient.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoOIDCClient.java
@@ -20,6 +20,7 @@ package net.shibboleth.idp.plugin.authn.duo;
import java.util.UUID;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.Immutable;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
@@ -27,6 +28,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
* Abstract base class for {@link DuoOIDCClient} implementations. Handles the clientId and
* retrieval of the client's capabilities.
*/
+ at Immutable
public abstract class AbstractDuoOIDCClient implements DuoOIDCClient{
/** The client instance UUID for identification.*/
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 ef9f0c8..3d57f3e 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
@@ -17,69 +17,124 @@
package net.shibboleth.idp.plugin.authn.duo;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.security.Principal;
import java.util.Collection;
+import java.util.Collections;
import java.util.Objects;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import javax.security.auth.Subject;
+import javax.servlet.http.HttpServletRequest;
+
+import org.slf4j.Logger;
+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.ThreadSafeAfterInit;
+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;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
+import net.shibboleth.utilities.java.support.component.InitializableComponent;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
/**
- * Wrapper for use with Duo OIDC integrations. Class is an initializable component and is hence
- * made effectively immutable once initialized.
+ * Data wrapper for use with Duo OIDC integrations. Holds shared-state.
+ *
+ * Is an {@link InitializableComponent} component and is hence
+ * made 'effectively immutable' once initialized and properly published by spring.
+ * FIXME
+ * Despite most fields being 'effectively immutable' once published by Spring, the redirectURI is allowed
+ * to change, but in a thread-safe manor.
+ *
+ * Is capable of deriving the redirectURI from the server request checked against a known list of allowed hosts.
*/
- at ThreadSafeAfterInit
-public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
+ at ThreadSafe
+public final class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
+
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoOIDCIntegration.class);
/** API host. */
- @NonnullAfterInit @NotEmpty private String apiHost;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String apiHost;
/** Integration key. */
- @NonnullAfterInit @NotEmpty private String clientId;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String clientId;
/** Secret key. */
- @NonnullAfterInit @NotEmpty private String secretKey;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String secretKey;
/** The redirect_uri to send the client after authorisation .*/
- @NonnullAfterInit @NotEmpty private String redirectURI;
+ @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;
/** The URL path to the health endpoint.*/
- @NonnullAfterInit @NotEmpty private String healthEndpoint;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String healthEndpoint;
/** The URL path to the authorization endpoint.*/
- @NonnullAfterInit @NotEmpty private String authorizeEndpoint;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String authorizeEndpoint;
/** The URL path to the token endpoint.*/
- @NonnullAfterInit @NotEmpty private String tokenEndpoint;
+ @GuardedBy("this") @NonnullAfterInit @NotEmpty private String tokenEndpoint;
/** Container for supported principals. */
- @Nonnull private final Subject supportedPrincipals;
+ @GuardedBy("this") @Nonnull private final Subject supportedPrincipals;
/** Constructor. */
public DefaultDuoOIDCIntegration() {
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 Set<String> getAllowedOrigins() {
+ return Collections.unmodifiableSet(allowedOrigins);
}
- /** {@inheritDoc} */
- @Nonnull @NotEmpty public String getAPIHost() {
+ @Override
+ @Nonnull @NotEmpty public synchronized String getAPIHost() {
return apiHost;
}
+
+ /**
+ * Set the API host to use.
+ *
+ * @param host API host
+ */
+ public synchronized void setAPIHost(@Nonnull @NotEmpty final String host) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ apiHost = Constraint.isNotNull(StringSupport.trimOrNull(host), "API host cannot be null or empty");
+ }
@Override
- @Nonnull @NotEmpty public String getHealthCheckEndpoint() {
+ @Nonnull @NotEmpty public synchronized String getHealthCheckEndpoint() {
return healthEndpoint;
}
@@ -88,7 +143,7 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
*
* @param endpoint the endpoint.
*/
- public void setHealthCheckEndpoint(@Nonnull @NotEmpty final String endpoint) {
+ public synchronized void setHealthCheckEndpoint(@Nonnull @NotEmpty final String endpoint) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
healthEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
@@ -96,7 +151,7 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
}
@Override
- @Nonnull @NotEmpty public String getAuthorizeEndpoint() {
+ @Nonnull @NotEmpty public synchronized String getAuthorizeEndpoint() {
return authorizeEndpoint;
}
@@ -105,7 +160,7 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
*
* @param endpoint the endpoint.
*/
- public void setAuthorizeEndpoint(@Nonnull @NotEmpty final String endpoint) {
+ public synchronized void setAuthorizeEndpoint(@Nonnull @NotEmpty final String endpoint) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
authorizeEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
@@ -113,7 +168,7 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
}
@Override
- @Nonnull @NotEmpty public String getTokenEndpoint() {
+ @Nonnull @NotEmpty public synchronized String getTokenEndpoint() {
return tokenEndpoint;
}
@@ -122,38 +177,62 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
*
* @param endpoint the endpoint.
*/
- public void setTokenEndpoint(@Nonnull @NotEmpty final String endpoint) {
+ public synchronized void setTokenEndpoint(@Nonnull @NotEmpty final String endpoint) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
tokenEndpoint = Constraint.isNotNull(StringSupport.trimOrNull(endpoint),
"Token endpoint cannot be null or empty");
}
-
+
/**
- * Set the API host to use.
+ * Set the redirect_uri to use.
*
- * @param host API host
+ * @param uri the redirect_uri
*/
- public void setAPIHost(@Nonnull @NotEmpty final String host) {
+ public synchronized void setRedirectURI(@Nullable final String uri) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- apiHost = Constraint.isNotNull(StringSupport.trimOrNull(host), "API host cannot be null or empty");
+
+ redirectURI = StringSupport.trimOrNull(uri);
}
-
+
+ @Override
+ @Nullable public synchronized String getRedirectURI() {
+ return redirectURI;
+ }
+
/**
* Set the redirect_uri to use.
*
* @param uri the redirect_uri
*/
- public void setRedirectURI(@Nonnull @NotEmpty final String uri) {
+ public synchronized void setRegisteredRedirectURI(@Nullable final String uri) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- redirectURI = Constraint.isNotEmpty(uri, "Redirect_uri cannot be null or empty");
+
+ registeredRedirectURI = StringSupport.trimOrNull(uri);
}
- /** {@inheritDoc} */
- @Nonnull @NotEmpty public String getRedirectURI() {
- return redirectURI;
+ @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 on the first request as '{}'",computedRedirectURI);
+ redirectURI = computedRedirectURI;
+ }
}
/**
@@ -161,14 +240,14 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
*
* @param id the client identifier.
*/
- public void setClientId(@Nonnull @NotEmpty final String id) {
+ public synchronized void setClientId(@Nonnull @NotEmpty final String id) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
clientId = Constraint.isNotNull(StringSupport.trimOrNull(id), "ClientID cannot be null or empty");
}
- /** {@inheritDoc} */
- @Nonnull @NotEmpty public String getClientId() {
+ @Override
+ @Nonnull @NotEmpty public synchronized String getClientId() {
return clientId;
}
@@ -177,21 +256,21 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
*
* @param key secret key
*/
- public void setSecretKey(@Nonnull @NotEmpty final String key) {
+ public synchronized void setSecretKey(@Nonnull @NotEmpty final String key) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
secretKey = Constraint.isNotNull(StringSupport.trimOrNull(key), "Secret key cannot be null or empty");
}
- /** {@inheritDoc} */
- @Nonnull @NotEmpty public String getSecretKey() {
+ @Override
+ @Nonnull @NotEmpty public synchronized String getSecretKey() {
return secretKey;
}
- /** {@inheritDoc} */
+ @Override
@Nonnull @NonnullElements @Unmodifiable
- public <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
+ public synchronized <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
return supportedPrincipals.getPrincipals(c);
}
@@ -204,7 +283,7 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
* @param <T> a type of principal to add, if not generic
* @param principals supported principals to include
*/
- public <T extends Principal> void setSupportedPrincipals(
+ public synchronized <T extends Principal> void setSupportedPrincipals(
@Nullable @NonnullElements final Collection<T> principals) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
@@ -215,24 +294,22 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
}
}
- /** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
- if (apiHost == null || clientId == null || secretKey == null
- || redirectURI == null || healthEndpoint == null || authorizeEndpoint == null
- || tokenEndpoint == null) {
- throw new ComponentInitializationException("API host, clientId, secret key, redirect_uri,"
- + "token endpoint, health check endpoint and authorization endpoint must be set");
+ if (getAPIHost() == null || getClientId() == null || getSecretKey() == null
+ || getHealthCheckEndpoint() == null || getAuthorizeEndpoint() == null
+ || getTokenEndpoint() == null || (getRedirectURI() == null && getAllowedOrigins() == 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");
}
}
- /** {@inheritDoc} */
@Override
public int hashCode() {
- return Objects.hash(clientId);
+ return Objects.hash(getClientId());
}
- /** {@inheritDoc} */
@Override
public boolean equals(final Object obj) {
if (this == obj) {
@@ -245,10 +322,9 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
return false;
}
final DefaultDuoOIDCIntegration other = (DefaultDuoOIDCIntegration) obj;
- return Objects.equals(clientId, other.clientId);
+ return Objects.equals(getClientId(), other.getClientId());
}
- /** {@inheritDoc} */
@Override
public String toString() {
final StringBuilder builder = new StringBuilder();
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCAuthAPI.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCAuthAPI.java
index c5ea760..3d97560 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCAuthAPI.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCAuthAPI.java
@@ -54,6 +54,12 @@ public final class DuoOIDCAuthAPI {
/** The name of the JSON result status message property.*/
@Nonnull @NotEmpty public static final String DUO_AUTH_RESULT_STATUS_MSG_JSON_OBJECT = "status_msg";
+ /** The final part of the IdP URL path which is called after authorization.*/
+ @Nonnull @NotEmpty public static final String CALLBACK_PATH_SEGMENT = "/duo-callback";
+
+ /** The final part of the IdP URL path which is called to begin authorization.*/
+ @Nonnull @NotEmpty public static final String AUTHORIZE_PATH_SEGMENT = "/authorize";
+
/** Constructor. */
private DuoOIDCAuthAPI() {
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClient.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClient.java
index f5605c1..9cb0638 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClient.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClient.java
@@ -38,11 +38,11 @@ public interface DuoOIDCClient extends DuoOIDCClientCapabilities{
@Nonnull DuoOIDCClientCapabilities getCapabilities();
/**
- * Check the health of the Duo 2FA endpoint.
+ * Check the health of the Duo 2FA endpoint and the clients configuration.
*
* @return the heath check response, never {@code null}.
*
- * @throws DuoClientException if there is an error returning the health check response
+ * @throws DuoClientException if there is an error returning the health check response.
*/
@Nonnull DuoHealthCheck healthCheck() throws DuoClientException;
@@ -56,13 +56,19 @@ public interface DuoOIDCClient extends DuoOIDCClientCapabilities{
* @param nonce a randomly generated (minimum 22 character) cryptographically secure nonce that is
* replayed in the id_token. Can be {@code null} if not supported by the client as indicated
* by the clients described capabilities.
+ * @param redirectURIOverride the redirectURI to use in the authorization request. If not {@literal null}
+ * and the client supports dynamic redirect URIs, it should override any redirect_uri held
+ * internally to the client e.g. from the {@link DuoOIDCIntegration}. If the client
+ * does not support dynamic redirect URIs, it can be ignored in favour of one held internally.
*
- * @return the redirect URL as a string, never {@code null}.
+ * @return the authorization redirect URL as a string, never {@code null}.
*
* @throws DuoClientException if there is an error creating the authentication URL.
*/
- @Nonnull String createAuthUrl(@Nonnull @NotEmpty final String username, @Nonnull @NotEmpty final String state,
- @Nullable final String nonce) throws DuoClientException;
+ @Nonnull @NotEmpty String createAuthUrl(@Nonnull @NotEmpty final String username,
+ @Nonnull @NotEmpty final String state, @Nullable final String nonce,
+ @Nullable final String redirectURIOverride) throws DuoClientException;
+
/**
* Exchanges the authorizaton code for a signed Json Web Token (JWT) which contains information pertaining to
@@ -71,15 +77,19 @@ public interface DuoOIDCClient extends DuoOIDCClientCapabilities{
*
* @param code An authentication identifier which is exchanged (per OAuth2.0 spec) with Duo for a token.
* the token can be used to determine if authentication was successful as well as obtain meta-data
- * about the authentication, never {@code null}.
+ * about the authentication, never {@code null}. *
* @param username The user to be authenticated by Duo, never {@code null}.
+ * @param redirectURIOverride the redirectURI to use in the code exchange request. If not {@literal null}
+ * and the client supports dynamic redirect URIs, it should override any redirect_uri held
+ * internally to the client e.g. from the {@link DuoOIDCIntegration}. If the client
+ * does not support dynamic redirect URIs, it can be ignored in favour of one held internally.
*
* @return the **signed** JWT, never {@code null}.
*
* @throws DuoClientException if there is an error exchanging the auth_code for a token result.
*/
@Nonnull JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
- @Nonnull final String username) throws DuoClientException;
+ @Nonnull final String username, @Nullable final String redirectURIOverride) throws DuoClientException;
/**
* Get an identifier for this client e.g. a UUID. Only for logging and debugging.
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 78b0a78..c19718c 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
@@ -31,5 +31,16 @@ 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();
}
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientRegistry.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientRegistry.java
index f2b21ff..92ee9b1 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientRegistry.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientRegistry.java
@@ -25,7 +25,8 @@ import javax.annotation.Nonnull;
public interface DuoOIDCClientRegistry {
/**
- * <p>Retrieve an existing, or create a new, Duo client for the given Duo integration.</p>
+ * <p>Retrieve an existing, or create a new, Duo client for the given Duo integration.
+ * The client should be fully initialised and safely published on return.</p>
*
* <p>implSpec: Only one client should exist per integration i.e. given the same integration,
* the same client instance should be returned.</p>
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 da8800c..fee033e 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,17 +17,22 @@
package net.shibboleth.idp.plugin.authn.duo;
+
+import java.util.Set;
+
import javax.annotation.Nonnull;
-import javax.annotation.concurrent.Immutable;
+import javax.annotation.Nullable;
+import javax.servlet.http.HttpServletRequest;
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.
*/
- at Immutable
public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
/**
@@ -52,11 +57,43 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
@Nonnull @NotEmpty String getSecretKey();
/**
- * Get the redirect_uri to direct the client to after authorisation.
+ * Get a list of origins that allowed to appear in computed redirect URIs.
+ *
+ * @return a set of 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.
*
- * @return the redirect_uri
+ * @return the pre-registered redirect_uri.
*/
- @Nonnull @NotEmpty String getRedirectURI();
+ @Nullable String getRegisteredRedirectURI();
+
+ /**
+ * Has a redirectURI been pre-registered i.e. set in the properties file.
+ *
+ * @return true iff {@link #getPreregisteredRedirectURI()} is not null, false otherwise.
+ */
+ @Nullable boolean isRedirectURIPreregistered();
/**
* Get the path of the health check endpoint.
@@ -79,5 +116,7 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
*/
@Nonnull @NotEmpty String getTokenEndpoint();
+
+
}
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/URISupport.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/URISupport.java
new file mode 100644
index 0000000..bd569e9
--- /dev/null
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/URISupport.java
@@ -0,0 +1,84 @@
+/*
+ * 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 java.net.URI;
+import java.net.URISyntaxException;
+
+import javax.annotation.Nonnull;
+
+import org.apache.http.client.utils.URIBuilder;
+
+/** URL support class.*/
+public final class URISupport {
+
+ /** Private constructor.*/
+ private URISupport() {
+
+ }
+
+ /**
+ * Build a {@link URI} from the given parameters. If the scheme is either
+ * 'http' or 'https' with their respective default port, the port is set to -1.
+ *
+ * @param scheme the scheme
+ * @param host the hostname
+ * @param port the port
+ * @param path the path
+ *
+ * @return a fully built URI from the given parameters.
+ *
+ * @throws URISyntaxException if the URI can not be constructed.
+ */
+ @Nonnull public static URI buildURIIgnoreDefaultPorts(@Nonnull final String scheme,
+ @Nonnull final String host, @Nonnull final int port,
+ @Nonnull final String path) throws URISyntaxException {
+
+ int usedPort = port;
+ if ("http".equalsIgnoreCase(scheme)) {
+ // ignore port iff using the default http port
+ if (port == 80) {
+ usedPort = -1;
+ }
+ } else if ("https".equalsIgnoreCase(scheme)) {
+ // ignore port iff using the default https port
+ if (port == 443) {
+ usedPort = -1;
+ }
+ }
+ final URI uri = new URIBuilder()
+ .setScheme(scheme)
+ .setHost(host)
+ .setPort(usedPort)
+ .setPath(path)
+ .build();
+ return uri;
+ }
+
+ @Nonnull public static String buildOrigin(@Nonnull final URI uri) throws URISyntaxException {
+
+ if (uri.getPort() == -1) {
+ //is the default port (or is not defined), do not include
+ return new URI(String.format("%s://%s", uri.getScheme(),uri.getHost())).toString();
+ } else {
+ return new URI(String.format("%s://%s:%s", uri.getScheme(),uri.getHost(),uri.getPort())).toString();
+ }
+
+ }
+
+}
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/DuoOIDCAuthenticationContext.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/DuoOIDCAuthenticationContext.java
index d9bc15b..ec69e0c 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/DuoOIDCAuthenticationContext.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/DuoOIDCAuthenticationContext.java
@@ -60,12 +60,41 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
/** The Duo OIDC client to use for the lifetime of this authentication request.*/
@Nullable private DuoOIDCClient client;
+
+ /**
+ * A request bound redirect URI that was determined at runtime. Used to override the URI inside
+ * a {@link DuoOIDCIntegration} for supported clients. Allows per-request redirects
+ * e.g. useful if one IdP instance is fronted by different virtual hosts.
+ */
+ @Nullable private String redirectURIOverride;
/** Public no-arg constructor to allow auto-creation. */
public DuoOIDCAuthenticationContext() {
}
+ /**
+ * Get the override redirect URI.
+ *
+ * @return the override redirect URI.
+ */
+ @Nullable public String getRedirectURIOverride() {
+ return redirectURIOverride;
+ }
+
+ /**
+ * Set the override redirect URI.
+ *
+ * @param override the override redirect URI.
+ *
+ * @return this context.
+ */
+ @Nonnull public DuoOIDCAuthenticationContext setRedirectURIOverride(
+ @Nullable final String override) {
+ redirectURIOverride = override;
+ return this;
+ }
+
/**
* Get the nonce.
*
@@ -82,7 +111,7 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
*
* @return this context.
*/
- @Nonnull public DuoOIDCAuthenticationContext setNonce(@Nonnull final String oidcNonce) {
+ @Nonnull public DuoOIDCAuthenticationContext setNonce(@Nullable final String oidcNonce) {
nonce = oidcNonce;
return this;
}
diff --git a/idp-duo-api/src/test/resources/logback-test.xml b/idp-duo-api/src/test/resources/logback-test.xml
new file mode 100644
index 0000000..6ddfb8a
--- /dev/null
+++ b/idp-duo-api/src/test/resources/logback-test.xml
@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<configuration>
+
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder class="ch.qos.logback.classic.encoder.PatternLayoutEncoder">
+ <pattern>%level [%logger:%line] - %msg%n</pattern>
+ <charset>UTF-8</charset>
+ </encoder>
+ </appender>
+
+ <logger name="net.shibboleth" level="DEBUG" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+
+ <logger name="net.shibboleth.idp.plugin.authn.duo" level="TRACE" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+ <logger name="net.shibboleth.idp.ext.authn.duo.impl.Saml2FlowTest" level="DEBUG" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+
+ <logger name="org.springframework.webflow" level="TRACE" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+
+ <logger name="org.springframework.beans.factory.xml.XmlBeanDefinitionReader" level="TRACE" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+
+ <logger name="org.springframework.binding.mapping.impl" level="DEBUG" additivity="false">
+ <appender-ref ref="STDOUT" />
+ </logger>
+
+ <root>
+ <level value="INFO" />
+ <appender-ref ref="STDOUT" />
+ </root>
+
+</configuration>
\ No newline at end of file
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistry.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistry.java
index 933c6a2..0b08596 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistry.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistry.java
@@ -23,6 +23,9 @@ import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -37,7 +40,6 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoRegistryException;
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.ThreadSafeAfterInit;
import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -45,24 +47,24 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
/**
* <p>The default Duo Client registry for mapping a {@link DuoOIDCIntegration} to either a new
- * or existing {@link DuoOIDCClient} instance.</p>
+ * or existing {@link DuoOIDCClient} singleton instance.</p>
*
- * <p>Clients are created and registered against a {@link DuoOIDCIntegration}. Once created the client is
- * reused for the lifetime of the IdP. </p>
+ * <p>Clients are created and registered against a {@link DuoOIDCIntegration}. Once created, the same client
+ * instance is reused for the pertaining integration for the lifetime of the IdP. </p>
*
* <p>The {@link DuoOIDCIntegration} should decide it's own 'business key' using the
* {@link #equals(Object)} and {@link #hashCode()} method appropriately. The {@link DefaultDuoOIDCIntegration}
* uses the clientID as its key.</p>
*
- * <p>Supports lazy initialization of clients when they are first requested. A single, configurable, client factory
- * is called to initialize new clients.</p>
+ * <p>Supports thread-safe lazy initialization of clients when they are first requested. A single, configurable,
+ * client factory is called to initialize new clients.</p>
*
* <p>Initialization and fetching is thread safe thanks to the use of a {@link ConcurrentMap} and its
* {@link ConcurrentMap#computeIfAbsent(Object, Function)} operation. This guarantees that two clients should never
- * be created for the same integration.</p>
+ * be created for the same integration. The client is properly published once created.</p>
*
*/
- at ThreadSafeAfterInit
+ at ThreadSafe
public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializableComponent
implements DuoOIDCClientRegistry {
@@ -70,13 +72,13 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoOIDCClientRegistry.class);
/** Factory to produce Duo clients.*/
- @NonnullAfterInit private DuoOIDCClientFactory clientFactory;
+ @GuardedBy("this") @Nullable @NonnullAfterInit private DuoOIDCClientFactory clientFactory;
/** Registry of Duo client to Duo integration.*/
@Nonnull @NonnullElements private final ConcurrentMap<DuoOIDCIntegration, DuoOIDCClient> clientRegistry;
/** Function for creating a DuoClient from a DuoIntegration. */
- @Nonnull private Function<DuoOIDCIntegration, DuoOIDCClient> clientRegistryMappingFunction;
+ @Nonnull private final Function<DuoOIDCIntegration, DuoOIDCClient> clientRegistryMappingFunction;
/** Constructor.*/
public DefaultDuoOIDCClientRegistry() {
@@ -89,20 +91,28 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
*
* @param factory the factory.
*/
- public void setClientFactory(@Nonnull final DuoOIDCClientFactory factory) {
+ public synchronized void setClientFactory(@Nonnull final DuoOIDCClientFactory factory) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
clientFactory = Constraint.isNotNull(factory, "Duo client factory can not be null");
}
+
+ /**
+ * Internally synchronized method for obtaining the 'current' client factory.
+ *
+ * @return the client factory.
+ */
+ @Nullable private synchronized DuoOIDCClientFactory getClientFactory() {
+ return clientFactory;
+ }
- /** {@inheritDoc} */
@Override
@Nonnull public DuoOIDCClient getClientOrCreate(@Nonnull final DuoOIDCIntegration integration)
throws DuoRegistryException {
Constraint.isNotNull(integration, "Duo integration can not be null");
try {
- //this is an atomic call, avoiding the need to synchronise here e.g. two clients should never
- //be created for the same integration.
+ //this is an atomic call, avoiding the need to manually synchronise here e.g. two clients should never
+ //be created for the same integration. Client is properly published once created.
final DuoOIDCClient client = clientRegistry.computeIfAbsent(integration,clientRegistryMappingFunction);
log.debug("Duo registry returning the DuoClient instance '{}' of type '{}'",
client.getClientId(),client.getClass().getCanonicalName());
@@ -113,18 +123,18 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
}
- /** {@inheritDoc} */
@Override protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
- if (clientFactory == null) {
+ if (getClientFactory() == null) {
throw new ComponentInitializationException("A Duo Client Factory must be configured and cannot be null");
}
}
/**
* A function for creating a new Duo client from the configured client factory for the given Duo integration.
- * throws a {@link DuoClientInitializationException} if the factory can not create the client.
+ * throws a {@link DuoClientInitializationException} if the factory can not create the client. This function
+ * executes within the concurrent hashmap lock.
*/
private class CreateNewClientMappingFunction implements Function<DuoOIDCIntegration, DuoOIDCClient> {
@@ -136,8 +146,8 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
try {
log.debug("Creating a new Duo client for integration '{}', using factory type '{}'",integration
- ,clientFactory.getClass().getTypeName());
- return clientFactory.createInstance(integration);
+ ,getClientFactory().getClass().getTypeName());
+ return getClientFactory().createInstance(integration);
} catch (final DuoClientException e) {
//wrap the exception in a runtime exception.
throw new DuoClientInitializationException("Could not initialise "
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
new file mode 100644
index 0000000..44fef74
--- /dev/null
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategy.java
@@ -0,0 +1,101 @@
+/*
+ * 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.impl;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.function.BiFunction;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.ThreadSafe;
+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.URISupport;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+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;
+ * <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>
+ *
+ * <p>Returns null if one can not be constructed.</p>
+ *
+ * <p>Is thread-safe and immutable</p>
+ */
+ at ThreadSafe
+ at Immutable
+public final class DefaultRedirectURICreationStrategy
+ implements BiFunction<HttpServletRequest, DuoOIDCIntegration, String>{
+
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultRedirectURICreationStrategy.class);
+
+ /** The path, excluding the context and servlet paths, to the Duo callback handler.*/
+ @Nonnull @NotEmpty private final String callbackServletPath;
+
+ /**
+ * Constructor.
+ *
+ * @param callbackPath the path segment relative to the servlet path of the callback endpoint.
+ */
+ public DefaultRedirectURICreationStrategy(
+ @Nonnull @NotEmpty 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) {
+
+ final String redirectFromIntegration = integration.getRegisteredRedirectURI();
+ if (redirectFromIntegration != null) {
+ log.trace("Using redirect_uri '{}' from the integration", redirectFromIntegration);
+ return redirectFromIntegration;
+ }
+ try {
+ final URI uri = URISupport.buildURIIgnoreDefaultPorts(request.getScheme(),
+ request.getServerName(),
+ request.getServerPort(),
+ request.getContextPath()+request.getServletPath()+callbackServletPath);
+
+ final String origin = URISupport.buildOrigin(uri);
+ if (!integration.getAllowedOrigins().contains(origin)) {
+ log.warn("The 'origin' of the computed redirect_uri ('{}') is not allowed",origin);
+ return null;
+ }
+ return uri.toString();
+ } catch (final URISyntaxException e) {
+ log.warn("Unable to generate redirectURI",e);
+ }
+ return null;
+
+ }
+
+}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
index c9204bc..e22bec2 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
@@ -1,4 +1,4 @@
-/*
+/*
* 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
@@ -20,6 +20,8 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.util.function.BiFunction;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -33,10 +35,10 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
* Looks up the audience from the clientID in the {@link DuoOIDCIntegration} inside the context.
* Returns null if it fails to find the clientID. Used for JWT ID Token audience claims verification.
*/
+ at ThreadSafe
public class DuoAudienceClaimLookupStrategy implements BiFunction<ProfileRequestContext, JWTClaimsSet, String> {
- /** {@inheritDoc} */
- @Override public String apply(@Nonnull final ProfileRequestContext context,
+ @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
@Nonnull final JWTClaimsSet cliams) {
final AuthenticationContext authnContext = context.getSubcontext(AuthenticationContext.class);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategy.java
index 62e0001..7cd482b 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategy.java
@@ -20,6 +20,7 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.util.function.BiFunction;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -33,6 +34,9 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
/**
* Find the issuer from the {@link DuoOIDCIntegration}. Returns null if not found.
+ *
+ * <p>{@link #setIssuerPath(String)} should only be called during initialisation e.g. by spring, otherwise
+ * thread-safety is compromised.</p>
*/
public class DuoIssuerClaimLookupStrategy implements BiFunction<ProfileRequestContext, JWTClaimsSet, String> {
@@ -55,7 +59,7 @@ public class DuoIssuerClaimLookupStrategy implements BiFunction<ProfileRequestCo
}
/** {@inheritDoc} */
- @Override public String apply(@Nonnull final ProfileRequestContext context,
+ @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
@Nonnull final JWTClaimsSet cliams) {
final AuthenticationContext authnContext = context.getSubcontext(AuthenticationContext.class);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
index e685384..18e141c 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
@@ -20,6 +20,8 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.util.function.BiFunction;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -32,10 +34,10 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
/**
* Find the nonce from the {@link DuoAuthenticationContext}. Returns null if not found.
*/
+ at ThreadSafe
public class DuoNonceClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
- /** {@inheritDoc} */
- @Override public String apply(@Nonnull final ProfileRequestContext context,
+ @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
@Nonnull final JWTClaimsSet cliams) {
final AuthenticationContext authnContext = context.getSubcontext(AuthenticationContext.class);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
index bfe6079..cf40c5c 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
@@ -39,12 +39,14 @@ import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
import net.shibboleth.idp.plugin.authn.duo.DuoException;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
+import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
+
/**
*
* <p>MVC controller for managing Duo 2FA exchanges implemented as an {@link ExternalAuthentication}
@@ -70,6 +72,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
/** The name of the Http parameter that stores the state value.*/
@Nonnull @NotEmpty public static final String STATE_PARAMETER = "state";
+
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoOIDCAuthnController.class);
@@ -104,7 +107,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
* @throws ExternalAuthenticationException if an error occurs
* @throws IOException if an I/O error occurs
*/
- @GetMapping("/authorize")
+ @GetMapping(DuoOIDCAuthAPI.AUTHORIZE_PATH_SEGMENT)
public void authorizationRequest(@Nonnull final HttpServletRequest httpRequest,
@Nonnull final HttpServletResponse httpResponse) throws ExternalAuthenticationException, IOException {
@@ -136,14 +139,16 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
duoContext.setRequestState(nonce);
String authURL;
- //if an OIDC id_token nonce is supported, add it to the authz request
+ //if an OIDC id_token nonce is supported, add it to the authz request and context
if (client.getCapabilities().isSupportsNonce()) {
final String oidcNonce = DuoSupport.generateNonce(36);
- authURL = client.createAuthUrl(duoContext.getUsername(), state, oidcNonce);
+ authURL = client.createAuthUrl(duoContext.getUsername(), state, oidcNonce,
+ duoContext.getRedirectURIOverride());
duoContext.setNonce(oidcNonce);
} else {
- authURL = client.createAuthUrl(duoContext.getUsername(), state, null);
+ authURL = client.createAuthUrl(duoContext.getUsername(), state, null,
+ duoContext.getRedirectURIOverride());
}
httpResponse.sendRedirect(authURL);
@@ -164,7 +169,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
* @throws ExternalAuthenticationException if an error occurs
* @throws IOException if an I/O error occurs
*/
- @GetMapping("/duo-callback")
+ @GetMapping(DuoOIDCAuthAPI.CALLBACK_PATH_SEGMENT)
public void authorizationCallback(@Nonnull final HttpServletRequest httpRequest,
@Nonnull final HttpServletResponse httpResponse) throws ExternalAuthenticationException, IOException {
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
index ca7113d..a7f6278 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
@@ -20,6 +20,8 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.util.function.BiFunction;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -32,10 +34,10 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
/**
* Find the authenticating principals username from the {@link DuoAuthenticationContext}. Returns null if not found.
*/
+ at ThreadSafe
public class DuoUsernameClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
- /** {@inheritDoc} */
- @Override public String apply(@Nonnull final ProfileRequestContext context,
+ @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
@Nonnull final JWTClaimsSet cliams) {
final AuthenticationContext authnContext = context.getSubcontext(AuthenticationContext.class);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoToken.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoToken.java
index 22249f9..354cdbe 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoToken.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoToken.java
@@ -53,7 +53,6 @@ public class ExchangeCodeForDuoToken extends AbstractDuoAuthenticationAction{
@Nonnull private final Logger log = LoggerFactory.getLogger(ExchangeCodeForDuoToken.class);
- /** {@inheritDoc} */
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@Nonnull final DuoOIDCAuthenticationContext duoContext) {
@@ -79,7 +78,8 @@ public class ExchangeCodeForDuoToken extends AbstractDuoAuthenticationAction{
}
try {
- final JWT token = client.exchangeAuthorizationCodeFor2FAResult(code,username);
+ final JWT token = client.exchangeAuthorizationCodeFor2FAResult(
+ code, username, duoContext.getRedirectURIOverride());
if (log.isDebugEnabled()) {
//avoid parsing claims if debug not enabled,
//if debug is enabled and parsing fails here, you will get different behaviour than if
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java
index b37999d..a368523 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java
@@ -37,7 +37,9 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
/**
* An action that checks the health of the Duo 2FA endpoint for the established Duo integration.
- * An {@link AuthnEventIds#AUTHN_EXCEPTION} event is emitted if the health point is unreachable or unhealthy.
+ * An {@link AuthnEventIds#AUTHN_EXCEPTION} event is emitted if the health endpoint is unreachable
+ * or unhealthy (unhealthy can also mean the client details are incorrect e.g. the client is not
+ * registered with Duo).
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link EventIds#IO_ERROR}
@@ -54,7 +56,6 @@ public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
/** Class logger. */
@Nonnull @NotEmpty private final Logger log = LoggerFactory.getLogger(HealthCheckDuoOIDCAuthAPI.class);
- /** {@inheritDoc} */
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@Nonnull final DuoOIDCAuthenticationContext duoContext) {
@@ -76,7 +77,7 @@ public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
//These checks are redundant if using the Duo WebSDK v4 client as it throws an exception if not 'OK'.
//They are still included to be compatible with other implementations which return the full response.
if (DuoOIDCAuthAPI.DUO_RESPONSE_STATUS_OK.equalsIgnoreCase(healthCheckResponse.getStatus())) {
- log.trace("{} Duo 2FA endpoints are healthy!",getLogPrefix());
+ log.trace("{} Duo 2FA endpoints and client are healthy!",getLogPrefix());
return;
} else if (DuoOIDCAuthAPI.DUO_RESPONSE_STATUS_FAIL.equalsIgnoreCase(healthCheckResponse.getStatus())) {
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 55dca14..5430ef1 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
@@ -17,9 +17,11 @@
package net.shibboleth.idp.plugin.authn.duo.impl;
+import java.util.function.BiFunction;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.servlet.http.HttpServletRequest;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
import org.opensaml.profile.action.ActionSupport;
@@ -31,7 +33,6 @@ 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.authn.duo.DuoIntegration;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientRegistry;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
@@ -46,7 +47,11 @@ import net.shibboleth.utilities.java.support.logic.FunctionSupport;
/**
* An action to create (or lookup) and populate the {@link DuoOIDCAuthenticationContext}
- * with the username, chosen {@link DuoIntegration}, and {@link DuoOIDCClient} appropriate for this request.
+ * with the username, chosen {@link DuoOIDCIntegration}, and {@link DuoOIDCClient} appropriate for this request.
+ *
+ * <p>Also determines the usable redirect_uri, either from one registered, or computed from the
+ * HTTP request. Is set once, before the client is constructed, for every client. If however, the client supports
+ * dynamic use of the redirect_uri, it is also set into the context for use downstream.</p>
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
@@ -68,6 +73,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;
+
/** The registry for locating the DuoClient for the established integration.*/
@NonnullAfterInit private DuoOIDCClientRegistry clientRegistry;
@@ -87,7 +95,9 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param duoRegistry the registry
*/
- public void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
+ public void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
clientRegistry = Constraint.isNotNull(duoRegistry,"DuoClient registry can not be null");
}
@@ -102,6 +112,20 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
usernameLookupStrategy = Constraint.isNotNull(strategy, "Username lookup strategy cannot be null");
}
+ /**
+ * Set the redirect URI creation strategy. The strategy is free to use or create a redirectURI based
+ * either on runtime parameters, or static information in the {@link DuoOIDCIntegration}.
+ *
+ * @param strategy the creation strategy.
+ */
+ public void setRedirectURICreationStrategy(
+ @Nonnull final BiFunction<HttpServletRequest, DuoOIDCIntegration, String> strategy) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ redirectURICreationStrategy = Constraint.isNotNull(strategy, "RedirectURI"
+ + " creation strategy cannot be null");
+ }
+
/**
* Set the strategy used to locate the {@link DuoOIDCAuthenticationContext} to operate on.
*
@@ -134,6 +158,9 @@ 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} */
@@ -156,6 +183,28 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
}
context.setIntegration(duoIntegration);
+ final HttpServletRequest request = getHttpServletRequest();
+ if (request == null) {
+ log.warn("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+ 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());
@@ -168,6 +217,13 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
try {
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);
@@ -177,5 +233,6 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
log.debug("Created Duo authentication context for '{}'",username);
}
+
}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
index 623234f..c2b68a9 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaims.java
@@ -131,6 +131,7 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
claimsValidator = Constraint.isNotNull(validator, "Claims validator cannot be null");
}
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
index 5edd6d7..a37340e 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
@@ -14,13 +14,15 @@
<bean class="net.shibboleth.ext.spring.config.IdentifiableBeanPostProcessor" />
<bean class="net.shibboleth.idp.profile.impl.ProfileActionBeanPostProcessor" />
+
+ <!-- location of the Duo OIDC IdP servlet -->
+ <bean id="shibboleth.authn.DuoOIDC.externalServletPath" class="java.lang.String"
+ c:_0="%{idp.duo.oidc.externalAuthnPath:/Authn/Duo/2FA}" />
- <bean id="shibboleth.authn.DuoOIDC.externalAuthnPath" class="java.lang.String"
- c:_0="servletRelative:%{idp.duo.oidc.externalAuthnPath:/Authn/Duo/2FA/}" />
+ <!-- location of the Duo OIDC IdP servlet authorization endpoint releative to the servlet -->
+ <bean id="shibboleth.authn.DuoOIDC.externalAuthorizationPath" class="java.lang.String"
+ c:_0="servletRelative:#{getObject('shibboleth.authn.DuoOIDC.externalServletPath')}#{T(net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI).AUTHORIZE_PATH_SEGMENT}" />
- <!-- Default strategy function to obtain the external path. -->
- <bean id="shibboleth.authn.DuoOIDC.externalAuthnPathStrategy" parent="shibboleth.Functions.Constant"
- c:target-ref="shibboleth.authn.DuoOIDC.externalAuthnPath" />
<!-- Default Duo Integration -->
<bean id="shibboleth.authn.DuoOIDC.DuoIntegration"
@@ -28,15 +30,17 @@
p:APIHost="%{idp.duo.oidc.apiHost:none}"
p:clientId="%{idp.duo.oidc.clientId:none}"
p:secretKey="%{idp.duo.oidc.secretKey:none}"
- p:redirectURI="%{idp.duo.oidc.redirectURL:none}"
+ p:registeredRedirectURI="%{idp.duo.oidc.redirectURL:}"
p:healthCheckEndpoint="%{idp.duo.oidc.endpoint.health:/oauth/v1/health_check}"
p:tokenEndpoint="%{idp.duo.oidc.endpoint.token:/oauth/v1/token}"
- p:authorizeEndpoint="%{idp.duo.oidc.endpoint.authorize:/oauth/v1/authorize}" />
+ p:authorizeEndpoint="%{idp.duo.oidc.endpoint.authorize:/oauth/v1/authorize}"
+ p:allowedOrigins="%{idp.duo.oidc.redirecturl.allowedOrigins:}" />
<bean id="shibboleth.authn.DuoOIDC.DuoIntegrationStrategy" parent="shibboleth.Functions.Constant"
c:target-ref="shibboleth.authn.DuoOIDC.DuoIntegration" />
<!-- Default "optional" non-browser integration. -->
- <bean id="shibboleth.authn.DuoOIDC.NonBrowser.DuoIntegration" class="net.shibboleth.idp.authn.duo.BasicDuoIntegration"
+ <bean id="shibboleth.authn.DuoOIDC.NonBrowser.DuoIntegration"
+ class="net.shibboleth.idp.authn.duo.BasicDuoIntegration"
p:APIHost="%{idp.duo.oidc.nonbrowser.apiHost:%{idp.duo.oidc.apiHost:none}}"
p:integrationKey="%{idp.duo.oidc.nonbrowser.integrationKey:none}"
p:secretKey="%{idp.duo.oidc.nonbrowser.secretKey:none}" />
@@ -104,9 +108,16 @@
<!-- Duo OIDC AuthAPI beans -->
<bean id="PopulateDuoAuthenticationContext" scope="prototype"
class="net.shibboleth.idp.plugin.authn.duo.impl.PopulateDuoAuthenticationContext"
+ p:httpServletRequest-ref="shibboleth.HttpServletRequest"
p:duoIntegrationLookupStrategy-ref="shibboleth.authn.DuoOIDC.DuoIntegrationStrategy"
+ p:redirectURICreationStrategy-ref="shibboleth.authn.DuoOIDC.RedirectURICreationStrategy"
p:usernameLookupStrategy-ref="shibboleth.authn.DuoOIDC.UsernameLookupStrategy"
p:clientRegistry-ref="shibboleth.authn.DuoOIDC.clientRegistry" />
+
+ <bean id="shibboleth.authn.DuoOIDC.RedirectURICreationStrategy"
+ class="net.shibboleth.idp.plugin.authn.duo.impl.DefaultRedirectURICreationStrategy"
+ c:_0="#{getObject('shibboleth.authn.DuoOIDC.externalServletPath')}#{T(net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI).CALLBACK_PATH_SEGMENT}"
+ />
<bean id="HealthCheckDuoOIDCAuthAPI" scope="prototype"
class="net.shibboleth.idp.plugin.authn.duo.impl.HealthCheckDuoOIDCAuthAPI" />
@@ -123,8 +134,10 @@
<bean id="ValidateTokenClaims" scope="prototype"
class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateTokenClaims"
- p:cleanupHook="#{getObject('shibboleth.authn.DuoOIDC.jwt.claims.CleanUpHook') ?: getObject('shibboleth.authn.DuoOIDC.jwt.claims.DefaultCleanupHook')}"
- p:claimsValidator="#{getObject('shibboleth.authn.DuoOIDC.DuoTokenClaimsVerifier') ?: getObject('shibboleth.authn.DuoOIDC.DefaultDuoTokenClaimsVerifier')}"/>
+ p:cleanupHook="#{getObject('shibboleth.authn.DuoOIDC.jwt.claims.CleanUpHook')
+ ?: getObject('shibboleth.authn.DuoOIDC.jwt.claims.DefaultCleanupHook')}"
+ p:claimsValidator="#{getObject('shibboleth.authn.DuoOIDC.DuoTokenClaimsVerifier')
+ ?: getObject('shibboleth.authn.DuoOIDC.DefaultDuoTokenClaimsVerifier')}"/>
<!-- OIDC claims verification -->
<bean id="shibboleth.authn.DuoOIDC.DefaultDuoTokenClaimsVerifier"
@@ -134,10 +147,10 @@
<bean id="requiredClaimsValidator"
class="net.shibboleth.oidc.security.jwt.claims.impl.RequiredClaimsValidator"
p:requiredClaims="#{getObject('shibboleth.authn.DuoOIDC.RequiredOIDCClaims') ?:
- getObject('shibboleth.authn.DuoOIDC.DefaultRequiredOIDCClaims')}"/>
+ getObject('shibboleth.authn.DuoOIDC.DefaultRequiredOIDCClaims')}"/>
<bean id="issuerClaimValidator"
class="net.shibboleth.oidc.security.jwt.claims.impl.ExactMatchClaimsValidator"
- p:claimName="iss"
+ p:claimName="#{T(net.shibboleth.oidc.security.jwt.claims.impl.JWTClaims).ISSUER_CLAIM.claimName}"
p:valueToMatchLookupStrategy="#{getObject('shibboleth.authn.DuoOIDC.jwt.IssuerLookupStrategy') ?:
getObject('shibboleth.authn.DuoOIDC.jwt.DefaultIssuerLookupStrategy')}"/>
<bean id="audienceClaimValidator"
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
index 33c4246..978fa27 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
@@ -32,7 +32,7 @@
</action-state>
<view-state id="Duo2FAAuthorizationRequest"
- view="externalRedirect:#{T(net.shibboleth.idp.authn.ExternalAuthentication).getExternalRedirect(flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.authn.DuoOIDC.externalAuthnPathStrategy').apply(opensamlProfileRequestContext) + '/authorize', flowExecutionContext.getKey().toString())}">
+ view="externalRedirect:#{T(net.shibboleth.idp.authn.ExternalAuthentication).getExternalRedirect(flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.authn.DuoOIDC.externalAuthorizationPath'), flowExecutionContext.getKey().toString())}">
<attribute name="csrf_excluded" value="true" type="boolean" />
<on-render>
<evaluate
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractAuthnXmlFlowExecutionTests.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractAuthnXmlFlowExecutionTests.java
index 531c37b..8f3bb25 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractAuthnXmlFlowExecutionTests.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractAuthnXmlFlowExecutionTests.java
@@ -289,7 +289,7 @@ public abstract class AbstractAuthnXmlFlowExecutionTests extends CustomAbstractX
final Converter<String,Duration> durationConverter = new Converter<String,Duration>(){
@Override
- public Duration convert(String source) {
+ public Duration convert(final String source) {
if (source.startsWith("P") || source.startsWith("-P")) {
return DOMTypeSupport.stringToDuration(source.trim());
}
@@ -316,7 +316,7 @@ public abstract class AbstractAuthnXmlFlowExecutionTests extends CustomAbstractX
builderContext.registerBean("shibboleth.authn.DuoOIDC.CleanUpHook", new Consumer<ProfileRequestContext>() {
@Override
- public void accept(ProfileRequestContext t) {
+ public void accept(final ProfileRequestContext t) {
return;
}});
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 83a9a17..712541e 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
@@ -132,6 +132,42 @@ public abstract class AbstractDuoActionTest {
return null;
}
+ /**
+ * Create a dummy Duo plain (no sig or enc) JWT token with no auth_result.
+ *
+ * @param authResult the authentication result e.g. allow.
+ * @param authResultMessage a user friendly result message.
+ * @param aud audience
+ * @param exp expiration time.
+ * @param iat issued at.
+ * @param authTime the auth time.
+ * @param apiHost the api host
+ * @param factor the duo factor
+ * @return the duo auth token.
+ */
+ protected JWT createPlainDummyTokenNoAuthResult(@Nonnull final String authResult,
+ @Nonnull final String authResultMessage, @Nonnull final String aud,
+ @Nonnull final Instant exp, @Nonnull final Instant iat,
+ @Nonnull final Instant authTime, @Nonnull final String apiHost,
+ @Nonnull final String factor) {
+
+ final String jwtJson = createJWTJsonWithNoAuthResult(authResult,authResultMessage,aud,
+ exp, iat, authTime, apiHost, factor);
+
+ try {
+
+ final JWT jwt = new PlainJWT(new PlainHeader().toBase64URL(),
+ new Base64URL(Base64Support.encodeURLSafe(jwtJson.getBytes())));
+ //test the claims exist by calling it
+ jwt.getJWTClaimsSet();
+ return jwt;
+ } catch (final EncodingException | ParseException e) {
+ fail("Error creating the Mock JWT",e);
+ }
+ fail();
+ return null;
+ }
+
/**
* Create a dummy Duo plain (no sig or enc) JWT token which is missing the 'exp' and 'aud' claims.
*
@@ -392,6 +428,70 @@ public abstract class AbstractDuoActionTest {
return jwtJson;
}
+ /**
+ * Create a JWT token in JSON without the auth_result element.
+ *
+ * @param authResult the authentication result e.g. allow.
+ * @param authResultMessage a user friendly result message.
+ * @param aud audience
+ * @param exp expiration time.
+ * @param iat issued at.
+ * @param authTime the auth time.
+ * @param apiHost the api host.
+ * @param factor the factor.
+ * @return the duo auth token.
+ */
+ private String createJWTJsonWithNoAuthResult(@Nonnull final String authResult,
+ @Nonnull final String authResultMessage, @Nonnull final String aud,
+ @Nonnull final Instant exp, @Nonnull final Instant iat,
+ @Nonnull final Instant authTime, @Nonnull final String apiHost,
+ @Nonnull final String factor) {
+ final String jwtJson = "{\n" +
+ " \"iss\": \"https://"+apiHost+"/oauth/v1/token\",\n" +
+ " \"sub\": \"jdoe\",\n" +
+ " \"preferred_username\": \"jdoe\",\n" +
+ " \"aud\": \""+aud+"\",\n" +
+ " \"exp\": "+Math.toIntExact(exp.getEpochSecond()) +",\n" +
+ " \"iat\": "+(double)iat.getEpochSecond()+",\n" +
+ " \"auth_time\": "+authTime.getEpochSecond()+",\n" +
+ " \"auth_context\": {\n" +
+ " \"result\": \"success\",\n" +
+ " \"timestamp\": 1599749128,\n" +
+ " \"auth_device\": {\n" +
+ " \"ip\": \"82.17.89.232\",\n" +
+ " \"name\": \"+44 7852 119881\",\n" +
+ " \"location\": {\n" +
+ " \"state\": \"Wales\",\n" +
+ " \"city\": \"Cardiff\",\n" +
+ " \"country\": \"United Kingdom\"\n" +
+ " }\n" +
+ " },\n" +
+ " \"txid\": \"1684599c-bb16-4250-af85-904291bfe7cc\",\n" +
+ " \"event_type\": \"authentication\",\n" +
+ " \"reason\": \"user_approved\",\n" +
+ " \"access_device\": {\n" +
+ " \"hostname\": null,\n" +
+ " \"ip\": \"82.17.89.232\",\n" +
+ " \"location\": {\n" +
+ " \"state\": \"Wales\",\n" +
+ " \"city\": \"Cardiff\",\n" +
+ " \"country\": \"United Kingdom\"\n" +
+ " }\n" +
+ " },\n" +
+ " \"application\": {\n" +
+ " \"key\": \"DIU6GEFWG5LIUTVV2M3N\",\n" +
+ " \"name\": \"Shibboleth Integration Testing\"\n" +
+ " },\n" +
+ " \"factor\": \""+factor+"\",\n" +
+ " \"user\": {\n" +
+ " \"key\": \"DUGL8U46QGJSOUJWG59W\",\n" +
+ " \"name\": \"philsmart\"\n" +
+ " }\n" +
+ " }\n" +
+ "}";
+ return jwtJson;
+ }
+
/**
* Create a JWT token in JSON using the claims arguments present, but missing the 'aud' and
* 'exp' claims.
@@ -565,7 +665,7 @@ public abstract class AbstractDuoActionTest {
*/
protected void assertEventId(@Nullable final Event event, @Nonnull final String eventId) {
assertNotNull(event);
- assertEquals(eventId, event.getId());
+ assertEquals(event.getId(),eventId);
}
/** Add the Duo authentication context to the authentication context.*/
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 5e82dd6..0aae48c 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
@@ -19,6 +19,7 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
+import static org.mockito.ArgumentMatchers.any;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotSame;
@@ -29,11 +30,16 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
+import org.mockito.Mockito;
import org.testng.annotations.BeforeMethod;
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.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;
@@ -87,6 +93,22 @@ public class DefaultDuoOIDCClientRegistryTest {
System.out.println("Client: " + client + " Client2: " + clientTwo + " Client3: " + clientThree);
assertNotSame(client, clientThree);
}
+
+ @Test(expectedExceptions = DuoRegistryException.class)
+ public final void testClientFailedToCreate() throws DuoRegistryException,
+ ComponentInitializationException, DuoClientException {
+
+ final DuoOIDCClientFactory mockClientFactory = Mockito.mock(DuoOIDCClientFactory.class);
+ Mockito.when(mockClientFactory.createInstance(any(DuoOIDCIntegration.class)))
+ .thenThrow(DuoClientException.class);
+
+ registry.setClientFactory(mockClientFactory);
+ registry.initialize();
+
+ //should throw
+ final DuoOIDCClient client = registry.getClientOrCreate(Mockito.mock(DuoOIDCIntegration.class));
+
+ }
//@test is not useful as an automated test
public final void testThreadSafetyCaching() throws Exception {
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
new file mode 100644
index 0000000..136c0e3
--- /dev/null
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
@@ -0,0 +1,188 @@
+/*
+ * 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.impl;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.Set;
+
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+import org.springframework.mock.web.MockHttpServletRequest;
+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;
+
+
+/** Tests for the DefaultDuoOIDCIntegration class.*/
+public class DefaultRedirectURICreationStrategyTest {
+
+ /** Static callback path from servlet request.*/
+ private static final String CALLBACK_PATH = "/Authn/Duo/2FA/"+DuoOIDCAuthAPI.CALLBACK_PATH_SEGMENT;
+
+ /** The action to test. */
+ private DefaultRedirectURICreationStrategy strategy;
+
+ /** A dummy integration to use.*/
+ @Mock
+ private DuoOIDCIntegration integration;
+
+ /** HTTP request mock.*/
+ private MockHttpServletRequest request;
+
+
+ @BeforeMethod
+ public void setUp() throws Exception {
+ request = new MockHttpServletRequest();
+ MockitoAnnotations.openMocks(this);
+ }
+
+ @Test
+ public final void testComputedRedirectAllowed() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "allowed.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("https://allowed.com"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, "https://allowed.com"+CALLBACK_PATH);
+
+ }
+
+ @Test
+ public final void testComputedRedirectDisallowed() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "not-allowed.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("https://allowed.com"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, null);
+
+ }
+
+ @Test
+ public final void testComputedRedirectAllowedHTTPSCustomPort() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "allowed.com");
+ request.setServerPort(8443);
+ request.setScheme("https");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("https://allowed.com:8443"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, "https://allowed.com:8443"+CALLBACK_PATH);
+
+ }
+
+ @Test
+ public final void testComputedRedirectAllowedHTTPCustomPort() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "allowed.com");
+ request.setServerPort(8080);
+ request.setScheme("http");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("http://allowed.com:8080"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, "http://allowed.com:8080"+CALLBACK_PATH);
+
+ }
+
+ @Test
+ public final void testComputedRedirectAllowedNullPort() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "allowed.com");
+ request.setServerPort(-1);
+ request.setScheme("http");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("http://allowed.com"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, "http://allowed.com"+CALLBACK_PATH);
+
+ }
+
+ @Test
+ public final void testComputedRedirectDisallowedOnPort() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "allowed.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("https://allowed.com:8443"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, null);
+
+ }
+
+ @Test
+ public final void testPreregisteredURL() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ //request is irrelevant
+ request.addHeader("Host", "notused.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn("https://registered.com"+CALLBACK_PATH);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(true);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, "https://registered.com"+CALLBACK_PATH);
+
+ }
+
+ @Test
+ public final void testComputedRedirectBadHostname() throws Exception {
+
+ strategy = new DefaultRedirectURICreationStrategy(CALLBACK_PATH);
+
+ request.addHeader("Host", "<script>inject</script>");
+ request.setServerPort(80);
+ request.setScheme("http");
+ Mockito.when(integration.getAllowedOrigins()).thenReturn(Set.of("http://allowed.com"));
+ Mockito.when(integration.getRegisteredRedirectURI()).thenReturn(null);
+ Mockito.when(integration.isRedirectURIPreregistered()).thenReturn(false);
+ final String redirect = strategy.apply(request, integration);
+ assertEquals(redirect, null);
+
+ }
+
+
+}
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 1a3cc68..b64d860 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
@@ -37,13 +37,13 @@ public class DualDuoIntegrationStrategy implements Function<ProfileRequestContex
final DefaultDuoOIDCIntegration int1 = new DefaultDuoOIDCIntegration();
int1.setAPIHost("host.com");
int1.setClientId(DuoAuthnFlowTest.FIRST_INTEGRATION_CLIENT_ID);
- int1.setRedirectURI("http://localhost/");
+ int1.setRegisteredRedirectURI("http://localhost/");
int1.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
final DefaultDuoOIDCIntegration int2 = new DefaultDuoOIDCIntegration();
int2.setAPIHost("host.com");
int2.setClientId(DuoAuthnFlowTest.SECOND_INTEGRATION_CLIENT_ID);
- int2.setRedirectURI("http://localhost/");
+ int2.setRegisteredRedirectURI("http://localhost/");
int2.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
if (DuoAuthnFlowTest.FIRST_INTEGRATION_SP.equals(
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 30d7035..3f1e8a3 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
@@ -109,7 +109,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
setClientFactory(new MockDuoOIDCClientFactory_FAIL_Client());
final Map<String,String> mockProperties = Map.of(
- "idp.duo.oidc.redirectUri","http://localhost/callback",
+ "idp.duo.oidc.redirectURL","http://localhost/callback",
"idp.duo.oidc.apiHost","api-c9f24c5a.duosecurity.com",
"idp.duo.oidc.clientId","DIU6GEFWG5LIUBVV2M3P",
"idp.duo.oidc.secretKey","rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh",
@@ -140,7 +140,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
setClientFactory(new MockDuoOIDCClientFactory_OK_Client());
final Map<String,String> mockProperties = Map.of(
- "idp.duo.oidc.redirectUri","http://localhost/authorization-callback",
+ "idp.duo.oidc.redirectURL","http://localhost/authorization-callback",
"idp.duo.oidc.apiHost","api-c9f24c5a.duosecurity.com",
"idp.duo.oidc.clientId","DIU6GEFWG5LIUBVV2M3P",
"idp.duo.oidc.secretKey","rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh",
@@ -171,7 +171,8 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
setSubflows(subflows);
setClientFactory(new MockDuoOIDCClientFactory_OK_Client());
- final Map<String,String> mockProperties = Map.of(
+ final Map<String,String> mockProperties = Map.of(
+ "idp.duo.oidc.redirectURL","http://localhost/authorization-callback",
"idp.duo.oidc.clientFactoryBean","shibboleth.authn.DuoOIDC.test.clientFactory",
"idp.duo.oidc.user.config","duo-oidc-authn-config-multiple-integrations.xml");
@@ -253,7 +254,7 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
//setClientFactory(new MockDuoOIDCClientFactory_OK_Client());
final Map<String,String> mockProperties = Map.of(
- "idp.duo.oidc.redirectUri","http://localhost/authorization-callback",
+ "idp.duo.oidc.redirectURL","http://localhost/authorization-callback",
"idp.duo.oidc.apiHost","api-c9f24c5a.duosecurity.com",
"idp.duo.oidc.clientId","DIU6GEFWG5LIUBVV2M3P",
"idp.duo.oidc.secretKey","rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh",
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 0076366..d856e94 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
@@ -178,7 +178,9 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
@Test
public void testSuccessfulAuthorizeRequest() throws Exception {
- MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/authorize").param("conversation", "e1s1")).andDo(print())
+ final MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/authorize")
+ .param("conversation", "e1s1"))
+ .andDo(print())
.andExpect(status().is3xxRedirection()).andReturn();
assertNotNull(result.getResponse().getHeader("Location"));
//basic check of the URL - contains the API host
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 9c3b00e..73daf4e 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
@@ -73,5 +73,40 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
assertEventId(event, AuthnEventIds.NO_CREDENTIALS);
}
+
+ /* Test ExchangeCodeForDuoToken with no client.*/
+ @Test
+ public void testExecuteNoClient()
+ throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+
+ addDuoContext();
+ addDuoIntegrationToContext();
+
+ //do not set client
+ //dc.setClient(new MockDuoOIDCClient_OK(dc.getIntegration()));
+ action.initialize();
+ final Event event = action.execute(src);
+
+ assertEventId(event, AuthnEventIds.AUTHN_EXCEPTION);
+ }
+
+ /* Test ExchangeCodeForDuoToken with no username.*/
+ @Test
+ public void testExecuteNoUsername()
+ throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+
+ addDuoContext();
+ addDuoIntegrationToContext();
+ dc.setAuthorizationCode("testcode");
+
+ //blank username
+ dc.setUsername(null);
+
+ dc.setClient(new MockDuoOIDCClient_OK(dc.getIntegration()));
+ action.initialize();
+ final Event event = action.execute(src);
+
+ assertEventId(event, AuthnEventIds.NO_CREDENTIALS);
+ }
}
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 838a56a..c29a2ee 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
@@ -17,10 +17,12 @@
package net.shibboleth.idp.plugin.authn.duo.impl;
+import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import org.mockito.Mockito;
import org.opensaml.profile.action.EventIds;
+import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.webflow.execution.Event;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -28,9 +30,14 @@ import org.testng.annotations.Test;
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.context.DuoOIDCAuthenticationContext;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import static org.mockito.ArgumentMatchers.any;
/**
* Tests for the {@link PopulateDuoAuthenticationContext} strategy.
@@ -39,6 +46,9 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
/** The action to test. */
private PopulateDuoAuthenticationContext action;
+
+ /** HTTP request mock.*/
+ private MockHttpServletRequest request;
/**
* Setup.
@@ -48,34 +58,90 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
@BeforeMethod public void setUp() throws Exception {
super.setup();
action = new PopulateDuoAuthenticationContext();
-
+ request = new MockHttpServletRequest();
+
}
/**
- * Test successful execution.
+ * Test successful execution assuming a client that does not support dynamic redirect URIs.
*
* @throws ComponentInitializationException on error.
* @throws DuoClientException on error.
* @throws DuoRegistryException one error.
*/
@Test
- public void testExecuteSuccess() throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+ public void testExecuteSuccessWithStaticClient()
+ throws ComponentInitializationException, DuoRegistryException, DuoClientException {
final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
//set the duo integration strategy to lookup this
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);
+ action.initialize();
+ final Event event = action.execute(src);
+ //success here is a null event
+ assertNull(event);
+ }
+
+ /**
+ * Test successful execution assuming a client that does not support dynamic redirect URIs.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoClientException on error.
+ * @throws DuoRegistryException one error.
+ */
+ @Test
+ public void testExecuteSuccessWithDynamicClient()
+ throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ //cleanout pre-registered redirect
+ integ.setRedirectURI(null);
+ //set the duo integration strategy to lookup this
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ action.setHttpServletRequest(request);
+ //set http params
+ request.addHeader("Host", "example.com");
+ request.setServerPort(443);
+ request.setScheme("https");
+ //mock
+ 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);
action.initialize();
final Event event = action.execute(src);
//success here is a null event
assertNull(event);
+ assertEquals(integ.getRedirectURI(), "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ assertEquals(ac.getSubcontext(DuoOIDCAuthenticationContext.class)
+ .getRedirectURIOverride(), "https://example.com/idp/profile/Authn/Duo/2FA/callback");
}
+
+
+
+
/**
* Test unsuccessful execution if there is no duo integration specified.
*
@@ -84,27 +150,32 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
* @throws DuoRegistryException on error.
*/
@Test
- public void testExecuteNoDuoIntegration() throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+ public void testExecuteNoDuoIntegration()
+ throws ComponentInitializationException, DuoRegistryException, DuoClientException {
final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
action.setClientRegistry(mockClientRegistry);
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ action.setHttpServletRequest(request);
action.initialize();
final Event event = action.execute(src);
- //success here is a null event
assertEventId(event, EventIds.INVALID_PROFILE_CTX);
}
/**
- * Test unsuccessful execution if there is no duo client registry specified.
+ * Test unsuccessful execution if there is no duo client registry or
+ * redirectURI creation strategy specified.
*
* @throws ComponentInitializationException on error.
* @throws DuoClientException on error.
* @throws DuoRegistryException on error.
*/
@Test(expectedExceptions = ComponentInitializationException.class)
- public void testExecuteNoDuoClientRegistry() throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+ public void testExecuteNoDuoClientRegistry() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
action.initialize();
}
@@ -117,19 +188,136 @@ public class PopulateDuoAuthenticationContextTest extends AbstractDuoActionTest{
* @throws DuoRegistryException on error.
*/
@Test
- public void testExecuteNullUsername() throws ComponentInitializationException, DuoRegistryException, DuoClientException {
+ public void testExecuteNullUsername() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
//set the duo integration strategy to lookup this
action.setDuoIntegrationLookupStrategy(prc -> integ);
//lookup a username
action.setUsernameLookupStrategy(prc -> null);
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ action.setHttpServletRequest(request);
final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
action.setClientRegistry(mockClientRegistry);
action.initialize();
final Event event = action.execute(src);
- //success here is a null event
assertEventId(event, AuthnEventIds.NO_CREDENTIALS);
}
+
+ /**
+ * Test unsuccessful execution if there is a null redirectURI.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoClientException on error.
+ * @throws DuoRegistryException on error.
+ */
+ @Test
+ public void testExecuteNullRedirectURI() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ //set the duo integration strategy to lookup this
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+ action.setRedirectURICreationStrategy((http,duoInteg) -> null);
+ action.setHttpServletRequest(request);
+ final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
+
+ action.setClientRegistry(mockClientRegistry);
+ action.initialize();
+ final Event event = action.execute(src);
+ assertEventId(event,EventIds.INVALID_PROFILE_CTX);
+ }
+
+ /**
+ * Test unsuccessful execution if there is an exception thrown by the client.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoClientException on error.
+ * @throws DuoRegistryException on error.
+ */
+ @Test
+ public void testExecuteClientException() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ //set the duo integration strategy to lookup this
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ action.setHttpServletRequest(request);
+ final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
+ //throw when asking for a client
+ Mockito.when(mockClientRegistry.getClientOrCreate(any(DuoOIDCIntegration.class)))
+ .thenThrow(DuoRegistryException.class);
+
+ action.setClientRegistry(mockClientRegistry);
+ action.initialize();
+ final Event event = action.execute(src);
+ assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
+ }
+
+ /**
+ * Test unsuccessful execution if there is no HttpServletRequest.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoClientException on error.
+ * @throws DuoRegistryException on error.
+ */
+ @Test
+ public void testExecuteNoHttpRequest() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ //set the duo integration strategy to lookup this
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+
+ //do not set http request
+ //action.setHttpServletRequest(request);
+
+ final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
+
+
+ action.setClientRegistry(mockClientRegistry);
+ action.initialize();
+ final Event event = action.execute(src);
+ assertEventId(event, EventIds.INVALID_PROFILE_CTX);
+ }
+
+ /**
+ * Test unsuccessful execution if there is no HttpServletRequest.
+ *
+ * @throws ComponentInitializationException on error.
+ * @throws DuoClientException on error.
+ * @throws DuoRegistryException on error.
+ */
+ @Test
+ public void testExecuteNoDuoContext() throws ComponentInitializationException,
+ DuoRegistryException, DuoClientException {
+ final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+ //set the duo integration strategy to lookup this
+ action.setDuoIntegrationLookupStrategy(prc -> integ);
+ //lookup a username
+ action.setUsernameLookupStrategy(prc -> "jdoe");
+ action.setRedirectURICreationStrategy((http,duoInteg)
+ -> "https://example.com/idp/profile/Authn/Duo/2FA/callback");
+ action.setDuoContextCreationStrategy(prc -> null);
+
+ action.setHttpServletRequest(request);
+
+ final DuoOIDCClientRegistry mockClientRegistry = Mockito.mock(DuoOIDCClientRegistry.class);
+
+
+ action.setClientRegistry(mockClientRegistry);
+ action.initialize();
+ final Event event = action.execute(src);
+ assertEventId(event, EventIds.INVALID_PROFILE_CTX);
+ }
}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
index 0f7abb2..fce5292 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
@@ -56,6 +56,21 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
assertNull(event);
}
+ /* Test Duo 2FA response validation, falied.*/
+ @Test
+ public void testExecuteFailed() throws ComponentInitializationException {
+ addDuoContext();
+ addDuoIntegrationToContext();
+ addAttemptedFlow("authn/DuoOIDC");
+ final String nonce = DuoSupport.generateNonce(32);
+ dc.setRequestState(nonce);
+ dc.setResponseState("different");
+ action.initialize();
+ final Event event = action.execute(src);
+ //null event is success.
+ assertEventId(event,AuthnEventIds.NO_CREDENTIALS);
+ }
+
/* Test Duo 2FA response validation, no state in either request or response.*/
@Test
public void testExecuteNoStateInRequestOrResponse() throws ComponentInitializationException {
@@ -81,7 +96,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
/* Test Duo 2FA response validation, no state in request.*/
@Test
- public void NO_CREDENTIALS() throws ComponentInitializationException {
+ public void testExecuteNoStateInRequest() throws ComponentInitializationException {
addDuoContext();
addDuoIntegrationToContext();
addAttemptedFlow("authn/DuoOIDC");
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
index 9dd4804..a976fea 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
@@ -83,6 +83,26 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
assertNull(event);
}
+ /**
+ * Test successful execution.
+ *
+ * @throws ComponentInitializationException on error.
+ */
+ @Test
+ public void testExecuteNoAuthResult() throws ComponentInitializationException {
+ addDuoContext();
+ addDuoIntegrationToContext();
+ addAttemptedFlow("authn/DuoOIDC");
+ dc.setAuthToken(createPlainDummyTokenNoAuthResult(DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW,"Login Succesful",CLIENT_ID,
+ Instant.now().plus(1,ChronoUnit.MINUTES),Instant.now(), Instant.now(), "api.duosecurity.com", "duo_push"));
+ dc.setUsername("jdoe");
+ action.initialize();
+
+ final Event event = action.execute(src);
+ // success here is a null event
+ assertEventId(event, AuthnEventIds.AUTHN_EXCEPTION);
+ }
+
/**
* Test the correct invalid authentication state is signalled when no Duo context is supplied.
*
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 3b143cc..f6bdd08 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
@@ -65,7 +65,8 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
}
@Override
- public String createAuthUrl(final String username, final String state, final String nonce) throws DuoClientException {
+ public String createAuthUrl(final String username, final String state,
+ final String nonce, final String redirect) throws DuoClientException {
return format("https://%s%s?scope=openid&response_type=code&redirect_uri=%s&client_id=%s&request=%s",
integration.getAPIHost(),"/oauth/v1/authorize", integration.getRedirectURI(),
integration.getClientId(), "JWT");
@@ -73,7 +74,7 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
@Override
public JWT exchangeAuthorizationCodeFor2FAResult(final String code,
- final String username) throws DuoClientException {
+ final String username, final String redirect) throws DuoClientException {
final String jwtJson = "{\n" +
" \"iss\": \"https://"+integration.getAPIHost()+"oauth/v1/token\",\n" +
@@ -145,6 +146,11 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
public boolean isSupportsNonce() {
return false;
}
+
+ @Override
+ public boolean isSupportsDynamicRedirectURI() {
+ return false;
+ }
@Override
public DuoOIDCClientCapabilities getCapabilities() {
@@ -154,6 +160,11 @@ public class MockDuoOIDCClient_FAIL implements DuoOIDCClient{
public boolean isSupportsNonce() {
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 27670e7..26164e5 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
@@ -62,7 +62,8 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
}
@Override
- public String createAuthUrl(final String username, final String state,final String nonce) throws DuoClientException {
+ public String createAuthUrl(final String username, final String state,
+ final String nonce, final String redirect) throws DuoClientException {
return format("https://%s%s?scope=openid&response_type=code&redirect_uri=%s&client_id=%s&request=%s",
integration.getAPIHost(),"/oauth/v1/authorize", integration.getRedirectURI(),
integration.getClientId(), "JWT");
@@ -70,7 +71,7 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
@Override
public JWT exchangeAuthorizationCodeFor2FAResult(final String code,
- final String username) throws DuoClientException {
+ final String username, final String redirect) throws DuoClientException {
final String jwtJson = "{\n" +
" \"iss\": \"https://"+integration.getAPIHost()+"/oauth/v1/token\",\n" +
@@ -143,6 +144,11 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
return false;
}
+ @Override
+ public boolean isSupportsDynamicRedirectURI() {
+ return false;
+ }
+
@Override
public DuoOIDCClientCapabilities getCapabilities() {
return new DuoOIDCClientCapabilities() {
@@ -151,6 +157,11 @@ public class MockDuoOIDCClient_OK implements DuoOIDCClient{
public boolean isSupportsNonce() {
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 aeb6f20..3d23873 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
@@ -64,8 +64,8 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
}
@Override
- public String createAuthUrl(
- final String username, final String state,final String nonce) throws DuoClientException {
+ public String createAuthUrl(final String username, final String state,
+ final String nonce, final String redirect) throws DuoClientException {
return format("https://%s%s?scope=openid&response_type=code&redirect_uri=%s&client_id=%s&request=%s",
integration.getAPIHost(),"/oauth/v1/authorize", integration.getRedirectURI(),
integration.getClientId(), "JWT");
@@ -73,7 +73,7 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
@Override
public JWT exchangeAuthorizationCodeFor2FAResult(final String code,
- final String username) throws DuoClientException {
+ final String username, final String redirect) throws DuoClientException {
final String jwtJson = "{\n" +
" \"iss\": \"https://"+integration.getAPIHost()+"/oauth/v1/token\",\n" +
@@ -146,6 +146,11 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
return false;
}
+ @Override
+ public boolean isSupportsDynamicRedirectURI() {
+ return false;
+ }
+
@Override
public DuoOIDCClientCapabilities getCapabilities() {
return new DuoOIDCClientCapabilities() {
@@ -154,6 +159,11 @@ public class MockDuoOIDCClient_OK_OLD_AUTH_TIME implements DuoOIDCClient{
public boolean isSupportsNonce() {
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 22c188b..f2be23d 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
@@ -66,7 +66,8 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
}
@Override
- public String createAuthUrl(final String username, final String state,final String nonce) throws DuoClientException {
+ public String createAuthUrl(final String username, final String state,
+ final String nonce, final String redirect) throws DuoClientException {
return format("https://%s%s?scope=openid&response_type=code&redirect_uri=%s&client_id=%s&request=%s",
integration.getAPIHost(),"/oauth/v1/authorize", integration.getRedirectURI(),
integration.getClientId(), "JWT");
@@ -74,7 +75,7 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
@Override
public JWT exchangeAuthorizationCodeFor2FAResult(final String code,
- final String username) throws DuoClientException {
+ final String username, final String redirect) throws DuoClientException {
final String jwtJson = "{\n" +
" \"iss\": \"https://"+integration.getAPIHost()+"oauth/v1/token\",\n" +
" \"sub\": \""+SUB+"\",\n" +
@@ -145,6 +146,11 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
return false;
}
+ @Override
+ public boolean isSupportsDynamicRedirectURI() {
+ return false;
+ }
+
@Override
public DuoOIDCClientCapabilities getCapabilities() {
return new DuoOIDCClientCapabilities() {
@@ -153,6 +159,11 @@ public class MockDuoOIDCClient_UNKNOWN implements DuoOIDCClient{
public boolean isSupportsNonce() {
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 14e245c..ee107b7 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
@@ -65,7 +65,7 @@ final class NimbusClient extends AbstractDuoOIDCClient{
@Nonnull @NotEmpty private static final String HTTPS = "https";
/** Class logger. */
- @Nonnull private Logger log = LoggerFactory.getLogger(NimbusClient.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(NimbusClient.class);
/** The integration to help generate the JWT.*/
@Nonnull private final DuoOIDCIntegration duoIntegration;
@@ -77,7 +77,7 @@ final class NimbusClient extends AbstractDuoOIDCClient{
@Nullable private final HttpClientSecurityParameters httpClientSecurityParameters;
/** JSON object mapper. */
- @Nonnull private ObjectMapper objectMapper;
+ @Nonnull private final ObjectMapper objectMapper;
/**
@@ -98,7 +98,6 @@ final class NimbusClient extends AbstractDuoOIDCClient{
//We do not validate the integration parameters e.g. because we know Duo use an invalid secret key
}
- /** {@inheritDoc} */
@Override
@Nonnull public DuoHealthCheck healthCheck() throws DuoClientException {
try {
@@ -109,7 +108,8 @@ final class NimbusClient extends AbstractDuoOIDCClient{
final RequestBuilder rb =
RequestBuilder.post().setUri(uri).addParameter("client_id",duoIntegration.getClientId())
.addParameter("client_assertion",
- NimbusClientSupport.createJWS(uri.toString(), duoIntegration));
+ NimbusClientSupport.createJWS(uri.toString(),
+ duoIntegration.getClientId(), duoIntegration.getSecretKey()));
return executeRequest(rb.build(), new TypeReference<DuoHealthCheck>() {});
@@ -119,10 +119,10 @@ final class NimbusClient extends AbstractDuoOIDCClient{
}
}
- /** {@inheritDoc} */
@Override
@Nonnull public String createAuthUrl(@Nonnull @NotEmpty final String username,
- @Nonnull @NotEmpty final String state, @Nonnull final String nonce) throws DuoClientException {
+ @Nonnull @NotEmpty final String state, @Nonnull 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");
Constraint.isNotEmpty(nonce, "Nonce can not be null or empty for this client");
@@ -130,7 +130,11 @@ final class NimbusClient extends AbstractDuoOIDCClient{
Constraint.isLessThan(1025, state.length(),"State must be at maximum 1024 characters");
try {
- final String request = NimbusClientSupport.createJWSRequestObject(duoIntegration,state, username);
+ final String redirectURI = redirectURIOverride != null ?
+ redirectURIOverride : duoIntegration.getRedirectURI();
+
+ final String request = NimbusClientSupport.createJWSRequestObject(
+ duoIntegration.getClientId(), redirectURI, duoIntegration.getSecretKey(), state, username);
final URI uri = new URIBuilder()
.setScheme(HTTPS)
@@ -139,22 +143,21 @@ final class NimbusClient extends AbstractDuoOIDCClient{
.setParameter("scope", "openid")
.setParameter("nonce", nonce)
.setParameter("response_type", "code")
- .setParameter("redirect_uri", duoIntegration.getRedirectURI())
+ .setParameter("redirect_uri", redirectURI)
.setParameter("client_id", duoIntegration.getClientId())
.setParameter("request", request).build();
return uri.toString();
} catch (final URISyntaxException e) {
- log.error("Error performing a Duo health check",e);
+ log.error("Unable to create a Duo authorization URL",e);
throw new DuoClientException(e);
}
}
- /** {@inheritDoc} */
@Override
public JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
- @Nonnull final String username) throws DuoClientException {
+ @Nonnull final String username, @Nullable final String redirectURIOverride) throws DuoClientException {
Constraint.isNotEmpty(code, "Auth_code can not be null");
try {
@@ -166,10 +169,12 @@ final class NimbusClient extends AbstractDuoOIDCClient{
RequestBuilder.post().setUri(uri)
.addParameter("grant_type","authorization_code")
.addParameter("code",code)
- .addParameter("redirect_uri",duoIntegration.getRedirectURI())
+ .addParameter("redirect_uri",redirectURIOverride != null
+ ? redirectURIOverride : duoIntegration.getRedirectURI())
.addParameter("client_assertion_type",CLIENT_ASSERTION_TYPE)
.addParameter("client_assertion",
- NimbusClientSupport.createJWS(uri.toString(),duoIntegration));
+ NimbusClientSupport.createJWS(uri.toString(),duoIntegration.getClientId(),
+ duoIntegration.getSecretKey()));
final TokenResponse response = executeRequest(rb.build(),new TypeReference<TokenResponse>() {});
log.trace("Duo token response: '{}'",response);
@@ -233,5 +238,10 @@ final class NimbusClient extends AbstractDuoOIDCClient{
public boolean isSupportsNonce() {
return true;
}
+
+ @Override
+ public boolean isSupportsDynamicRedirectURI() {
+ return true;
+ }
}
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactory.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactory.java
index fe76c85..751d82e 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactory.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientFactory.java
@@ -19,6 +19,7 @@ package net.shibboleth.idp.plugin.authn.duo.nimbus.impl;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
import org.apache.http.client.HttpClient;
import org.opensaml.security.httpclient.HttpClientSecurityParameters;
@@ -30,6 +31,7 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientFactory;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -44,16 +46,17 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* Each client instance shares the same, thread-safe, http client and object mapper.
* </p>
*/
+ at ThreadSafeAfterInit
public class NimbusClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory {
/** HttpClient for contacting Duo. */
- @NonnullAfterInit private HttpClient httpClient;
+ @GuardedBy("this") @NonnullAfterInit private HttpClient httpClient;
/** HTTP client security parameters. */
- @Nullable private HttpClientSecurityParameters httpClientSecurityParameters;
+ @GuardedBy("this") @Nullable private HttpClientSecurityParameters httpClientSecurityParameters;
/** JSON object mapper. */
- @NonnullAfterInit private ObjectMapper objectMapper;
+ @GuardedBy("this") @NonnullAfterInit private ObjectMapper objectMapper;
@Override
public DuoOIDCClient createInstance(@Nonnull final DuoOIDCIntegration integration) throws DuoClientException {
@@ -79,7 +82,7 @@ public class NimbusClientFactory extends AbstractInitializableComponent implemen
*
* @param client HttpClient
*/
- public void setHttpClient(@Nonnull final HttpClient client) {
+ public synchronized void setHttpClient(@Nonnull final HttpClient client) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -91,7 +94,7 @@ public class NimbusClientFactory extends AbstractInitializableComponent implemen
*
* @param params the new client security parameters
*/
- public void setHttpClientSecurityParameters(@Nullable final HttpClientSecurityParameters params) {
+ public synchronized void setHttpClientSecurityParameters(@Nullable final HttpClientSecurityParameters params) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
@@ -103,7 +106,7 @@ public class NimbusClientFactory extends AbstractInitializableComponent implemen
*
* @param mapper object mapper
*/
- public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
+ public synchronized void setObjectMapper(@Nonnull final ObjectMapper mapper) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
index 1526503..cb6834d 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
@@ -29,7 +29,6 @@ import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jwt.JWTClaimsSet;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
-import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.codec.EncodingException;
@@ -68,21 +67,24 @@ public final class NimbusClientSupport {
* Only supports the HS512 JWS algorithm.
* </p>
*
- * @param duoIntegration the integration used to construct the JWT
+ * @param clientID the client identifier
+ * @param redirectURI the redirectURI
+ * @param secret the client secret
* @param state the state
* @param username the subject of the authentication
*
* @throws DuoClientException on error constructing the JWS request object
*
* @return a signed JWT
- */
- //TODO this method and the below should be inside oidc-commons, and merged into a single API
- @Nonnull static String createJWSRequestObject(@Nonnull final DuoOIDCIntegration duoIntegration,
- @Nonnull @NotEmpty final String state, @Nonnull final String username) throws DuoClientException{
+ */
+ @Nonnull static String createJWSRequestObject(@Nonnull @NotEmpty final String clientID,
+ @Nonnull @NotEmpty final String redirectURI, @Nonnull @NotEmpty final String secret,
+ @Nonnull @NotEmpty final String state, @Nonnull @NotEmpty final String username) throws DuoClientException{
- Constraint.isNotNull(duoIntegration, "Duo Integration can not be null");
+ Constraint.isNotEmpty(clientID, "ClientID can not be null or empty");
+ Constraint.isNotEmpty(redirectURI, "RedirectURI can not be null or empty");
Constraint.isNotEmpty(state, "State can not be null or empty");
- Constraint.isNotNull(username, "username can not be null");
+ Constraint.isNotEmpty(username, "username can not be null or empty");
final Date expiration = new Date();
expiration.setTime(expiration.getTime() + Duration.ofHours(1).toMillis());
@@ -91,15 +93,15 @@ public final class NimbusClientSupport {
final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder()
.expirationTime(expiration)
.claim("scope", "openid")
- .claim("client_id", duoIntegration.getClientId())
- .claim("redirect_uri", duoIntegration.getRedirectURI())
+ .claim("client_id", clientID)
+ .claim("redirect_uri", redirectURI)
.claim("state", state)
.claim("duo_uname", username)
.claim("response_type", "code")
.build();
return JWSAssemblyUtils.assembleMacJwsAsString(
- JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(duoIntegration.getSecretKey()));
+ JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(secret));
} catch (final JOSEException | EncodingException | ParseException e) {
throw new DuoClientException(e);
@@ -108,13 +110,14 @@ public final class NimbusClientSupport {
}
/**
- * Create a signed JWT using the audience and Duo integration supplied.
+ * Create a signed JWT using the audience and client ID supplied.
* <p>
* Only supports the HS512 JWS algorithm.
* </p>
*
* @param aud the audience of the JWT.
- * @param duoIntegration the integration used to construct the JWT.
+ * @param clientID the client identifier
+ * @param secret the client secret
*
* @throws DuoClientException on error constructing the JWS object
*
@@ -122,10 +125,11 @@ public final class NimbusClientSupport {
*
*/
@Nonnull static String createJWS(@Nonnull final String aud,
- @Nonnull final DuoOIDCIntegration duoIntegration) throws DuoClientException{
+ @Nonnull final String clientID, @Nonnull final String secret) throws DuoClientException{
- Constraint.isNotNull(duoIntegration, "Duo Integration can not be null");
- Constraint.isNotNull(aud, "Audience can not be null");//'
+ Constraint.isNotNull(clientID, "Duo clientId can not be null");
+ Constraint.isNotNull(secret, "Duo secret can not be null");
+ Constraint.isNotNull(aud, "Audience can not be null");
final Date expiration = new Date();
expiration.setTime(expiration.getTime() + Duration.ofHours(1).toMillis());
@@ -133,24 +137,18 @@ public final class NimbusClientSupport {
try {
final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder()
.expirationTime(expiration)
- .issuer(duoIntegration.getClientId())
- .subject(duoIntegration.getClientId())
+ .issuer(clientID)
+ .subject(clientID)
.audience(aud)
.jwtID(NimbusClientSupport.generateJWTId(32))
.build();
return JWSAssemblyUtils.assembleMacJwsAsString(
- JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(duoIntegration.getSecretKey()));
+ JWSAlgorithm.HS512,claimsSet,JWSAssemblyUtils.getSecretBytes(secret));
} catch (final EncodingException | JOSEException | ParseException e) {
throw new DuoClientException(e);
}
}
-
-
-
-
-
-
}
diff --git a/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/conf/authn/duo-oidc.properties b/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/conf/authn/duo-oidc.properties
index 87faac3..e0c680f 100644
--- a/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/conf/authn/duo-oidc.properties
+++ b/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/conf/authn/duo-oidc.properties
@@ -17,6 +17,13 @@ idp.authn.DuoOIDC.supportedPrincipals = \
idp.duo.oidc.apiHost = hostname
idp.duo.oidc.clientId = clientid
idp.duo.oidc.redirectURL = https://<hostname>:<port>/idp/profile/Authn/Duo/2FA/duo-callback
+
+# If a redirect_uri is not explicitly declared above, one can be inferred from each
+# request's Host header. To avoid Host header injection attacks, the allowed origins
+# must be specified here. Origins are comma seperated. Do not specify the port when
+# using the default ports.
+# idp.duo.oidc.redirecturl.allowedOrigins = https://localhost
+
# We suggest defining this in credentials/secrets.properties
#idp.duo.oidc.secretKey = key
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 1b3dac4..6e78c0e 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
@@ -62,7 +62,8 @@ public class NimbusClientSupportTest {
/* Simple test that a JWS string is created and contains 3 components, header.payload.signature */
@Test
public void testCreateJWSRequestObject() throws DuoClientException {
- final String jws = NimbusClientSupport.createJWSRequestObject(integ, STATE, USERNAME);
+ final String jws = NimbusClientSupport.createJWSRequestObject(integ.getClientId(),
+ integ.getRedirectURI(),integ.getSecretKey(), STATE, USERNAME);
assertNotNull(jws);
assertEquals(jws.split("\\.").length,3);
}
@@ -70,7 +71,7 @@ public class NimbusClientSupportTest {
/* Simple test that a JWS string is created and contains 3 components, header.payload.signature */
@Test
public void testCreateJWS() throws DuoClientException {
- final String jws = NimbusClientSupport.createJWS(AUD,integ);
+ final String jws = NimbusClientSupport.createJWS(AUD,integ.getClientId(),integ.getSecretKey());
assertNotNull(jws);
assertEquals(jws.split("\\.").length,3);
}
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 6d0ac07..fab8ad6 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
@@ -182,7 +182,7 @@ public class NimbusClientTest {
// create new client with mock response
client = new NimbusClient(integ, httpClient, null, new ObjectMapper());
- final JWT jwt = client.exchangeAuthorizationCodeFor2FAResult("code", "jdoe");
+ final JWT jwt = client.exchangeAuthorizationCodeFor2FAResult("code", "jdoe",null);
assertNotNull(jwt);
assertEquals(jwt.getJWTClaimsSet().getSubject(),"jdoe");
@@ -211,7 +211,7 @@ public class NimbusClientTest {
// create new client with mock response
client = new NimbusClient(integ, httpClient, null, new ObjectMapper());
- final JWT jwt = client.exchangeAuthorizationCodeFor2FAResult("code", "jdoe");
+ final JWT jwt = client.exchangeAuthorizationCodeFor2FAResult("code", "jdoe",null);
}
@@ -315,7 +315,7 @@ public class NimbusClientTest {
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");
+ final String authUrl = client.createAuthUrl("jdoe", "RRFSFERGERGSDDSXXXXQWERG","a_nonce",null);
assertNotNull(authUrl);
assertTrue(authUrl.contains("response_type"));
assertTrue(authUrl.contains("client_id"));
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 8add2ba..e052605 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
@@ -71,10 +71,10 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
* Function to map the native Duo {@link HealthCheckResponse} object to the
* interface {@link DuoHealthCheck} object.
*/
- @Nonnull private Function<HealthCheckResponse,DuoHealthCheck> healthCheckResponseConverter;
+ @Nonnull private final Function<HealthCheckResponse,DuoHealthCheck> healthCheckResponseConverter;
/** Function to map the native Duo {@link Token} object to the interface {@link JWT} object.*/
- @Nonnull private BiFunction<Token, DuoOIDCIntegration, JWT> tokenResponseConverter;
+ @Nonnull private final BiFunction<Token, DuoOIDCIntegration, JWT> tokenResponseConverter;
/** Save off the integration to help generate the JWT.*/
@Nonnull private final DuoOIDCIntegration duoIntegration;
@@ -112,17 +112,6 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
}
-
- /**
- * Set the health check response converter.
- *
- * @param converter the converter.
- */
- public void setHealthCheckResponseConverter(@Nonnull final
- Function<HealthCheckResponse,DuoHealthCheck> converter) {
- healthCheckResponseConverter = Constraint.isNotNull(converter,
- "Health Check response converter can not be null");
- }
/** {@inheritDoc} */
@Override
@@ -142,14 +131,21 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
}
- /** {@inheritDoc} */
+ /**
+ * {@inheritDoc}
+ *
+ * <p>The Duo WebSDK Client does not support either the {@code nonce} or {@code redirectURIOverride}
+ * parameter.</p>
+ *
+ */
@Override
@Nonnull public String createAuthUrl(@Nonnull @NotEmpty final String username,
- @Nonnull @NotEmpty final String state, @Nullable 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");
- //does not support the nonce.
+ //does not support the nonce or redirect_uri override
try {
return client.createAuthUrl(username, state);
} catch (final DuoException e) {
@@ -158,10 +154,15 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
}
}
- /** {@inheritDoc} */
+ /**
+ * {@inheritDoc}
+ *
+ * <p>The Duo WebSDK Client does not support the {@code redirectURIOverride} parameter.</p>
+ *
+ */
@Override
@Nonnull public JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
- @Nonnull final String username)
+ @Nonnull final String username, @Nullable final String redirectURIOverride)
throws DuoClientException {
Constraint.isNotEmpty(code, "Auth_code can not be null");
try {
@@ -204,9 +205,10 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
* create a **signed** JWT. As the signature is not returned from the Duo SDK and the flow requires a signed JWT,
* a new HMAC signature is computed using the integrations secret key.
*/
+ @ThreadSafe
private final class DefaultTokenResponseConverter implements BiFunction<Token, DuoOIDCIntegration, JWT>{
- /** JSON object mapper. */
+ /** Thread-safe JSON object mapper. */
@Nonnull private ObjectMapper objectMapper;
/** Constructor. */
@@ -241,5 +243,11 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
}
+ @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 94a1f7c..4e8178c 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
@@ -21,6 +21,7 @@ import java.util.List;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -30,42 +31,55 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClientFactory;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
+import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
+import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
/** Abstract factory implementation for the {@link DuoSDKClientAdaptor}. */
-public class DuoSDKClientFactory implements DuoOIDCClientFactory{
+ at ThreadSafeAfterInit
+public class DuoSDKClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientFactory.class);
/** List of CA Certificate pins. If null, the client's default set are used.*/
- @Nullable @NonnullElements @Unmodifiable private List<String> caCerts;
-
-
+ @GuardedBy("this") @Nullable @NonnullElements @Unmodifiable private List<String> caCerts;
+
/**
* Sets the list of CA certificate pins used to verify the Duo client connection
* to the API host. If {@code null}, the client will use the internal defaults.
*
* @param certs the list of certificate pins.
*/
- public void setCaCerts(@Nullable final List<String> certs) {
+ public synchronized void setCaCerts(@Nullable final List<String> certs) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
//check if null, as the native duo client uses the internal defaults if null
- //but would respect an empty list.
+ //but will respect an empty list.
if (certs != null) {
caCerts = List.copyOf(StringSupport.normalizeStringCollection(certs));
} else {
caCerts = null;
- }
-
+ }
+ }
+
+ /**
+ * Get the caCerts, guarded by this objects monitor.
+ *
+ * @return the caCerts.
+ */
+ private synchronized List<String> getCaCerts(){
+ return caCerts;
}
@Override
@Nonnull public DuoOIDCClient createInstance(@Nonnull final DuoOIDCIntegration integration)
throws DuoClientException {
//every integration shares the same list of caCert pins.
- return new DuoSDKClientAdaptor(integration, caCerts);
+ return new DuoSDKClientAdaptor(integration, getCaCerts());
}
}
diff --git a/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/conf/authn/duo-oidc.properties b/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/conf/authn/duo-oidc.properties
index d5e0ef2..880ee0c 100644
--- a/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/conf/authn/duo-oidc.properties
+++ b/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/conf/authn/duo-oidc.properties
@@ -17,6 +17,13 @@ idp.authn.DuoOIDC.supportedPrincipals = \
idp.duo.oidc.apiHost = hostname
idp.duo.oidc.clientId = clientid
idp.duo.oidc.redirectURL = https://<hostname>:<port>/idp/profile/Authn/Duo/2FA/duo-callback
+
+# If a redirect_uri is not explicitly declared above, one can be inferred from the
+# Host header of the 'first' request. To avoid Host header injection attacks, the
+# allowed origins must be specified here. Origins are comma seperated. Do not specify
+# the port when using default ports.
+# idp.duo.oidc.redirecturl.allowedOrigins = https://localhost
+
# We suggest defining this in credentials/secrets.properties
#idp.duo.oidc.secretKey = key
diff --git a/pom.xml b/pom.xml
index 9b19a3b..33723a6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -87,6 +87,11 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<dependencyManagement>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list