[java-idp-plugin-duo] branch main updated: Improve Javadoc, syntax, and some log statements
Phil Smart
philip.smart at jisc.ac.uk
Tue Sep 8 15:48:27 UTC 2020
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=fcabc77069d931f887e979ea85e74dd2340c5187
The following commit(s) were added to refs/heads/main by this push:
new fcabc77 Improve Javadoc, syntax, and some log statements
fcabc77 is described below
commit fcabc77069d931f887e979ea85e74dd2340c5187
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Sep 8 16:48:22 2020 +0100
Improve Javadoc, syntax, and some log statements
---
idp-duo-api/pom.xml | 2 +-
.../authn/duo/AbstractDuoAuthenticationAction.java | 1 +
.../authn/duo/DefaultDuoOIDCIntegration.java | 29 ++++++++++------------
.../duo/DuoClientInitializationException.java | 3 +++
.../idp/plugin/authn/duo/DuoException.java | 5 +---
.../idp/plugin/authn/duo/DuoOIDCAuthAPI.java | 1 +
.../idp/plugin/authn/duo/DuoOIDCClient.java | 10 ++++----
.../idp/plugin/authn/duo/DuoOIDCClientFactory.java | 2 +-
.../plugin/authn/duo/DuoOIDCClientRegistry.java | 12 ++++-----
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 2 +-
.../idp/plugin/authn/duo/DuoRegistryException.java | 3 +++
.../duo/context/DuoOIDCAuthenticationContext.java | 5 ++--
.../idp/plugin/authn/duo/context/package-info.java | 2 +-
.../idp/plugin/authn/duo/model/package-info.java | 2 +-
.../idp/plugin/authn/duo/package-info.java | 2 +-
.../idp/plugin/authn/duo/DuoOIDCDescription.java | 1 -
.../duo/impl/DefaultDuoOIDCClientRegistry.java | 26 +++++++++++--------
.../authn/duo/impl/DuoOIDCAuthnController.java | 21 ++++++++++------
.../authn/duo/impl/ExchangeCodeForDuoToken.java | 7 +++---
.../authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java | 17 +++++++------
.../duo/impl/PopulateDuoAuthenticationContext.java | 2 +-
.../authn/duo/impl/ValidateDuoResponseState.java | 7 +++---
.../authn/duo/impl/ValidateDuoTokenAudience.java | 3 ++-
.../impl/ValidateDuoTokenAuthenticationResult.java | 4 +--
.../impl/ValidateDuoTokenAuthenticationTime.java | 6 ++++-
.../duo/impl/ValidateDuoTokenExpirationTime.java | 4 ++-
.../authn/duo/impl/ValidateDuoTokenIssuedAt.java | 4 ++-
.../authn/duo/impl/ValidateDuoTokenIssuer.java | 8 +++---
.../authn/duo/impl/ValidateDuoTokenSubject.java | 2 ++
.../duo/impl/DefaultDuoOIDCClientRegistryTest.java | 8 +++---
.../duo/impl/ExchangeCodeForDuoTokenTest.java | 2 +-
.../duo/impl/ValidateDuoResponseStateTest.java | 6 ++---
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 10 +++++---
.../authn/duo/sdk/impl/DuoSDKClientFactory.java | 8 +++---
34 files changed, 129 insertions(+), 98 deletions(-)
diff --git a/idp-duo-api/pom.xml b/idp-duo-api/pom.xml
index 1cce03f..663f2e2 100644
--- a/idp-duo-api/pom.xml
+++ b/idp-duo-api/pom.xml
@@ -64,7 +64,7 @@
</manifestEntries>
<manifestSections>
<manifestSection>
- <name>net/shibboleth/idp/plugin/duo</name>
+ <name>net/shibboleth/idp/plugin/authn/duo/</name>
<manifestEntries>
<Implementation-Title>${project.artifactId}</Implementation-Title>
<Implementation-Version>${project.version}</Implementation-Version>
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoAuthenticationAction.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoAuthenticationAction.java
index dca6004..fdb8f56 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoAuthenticationAction.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/AbstractDuoAuthenticationAction.java
@@ -62,6 +62,7 @@ public abstract class AbstractDuoAuthenticationAction extends AbstractAuthentica
/** Constructor.*/
public AbstractDuoAuthenticationAction() {
+ //prc -> ac -> dc
duoContextLookupStrategy = new ChildContextLookup<>(DuoOIDCAuthenticationContext.class).
compose(new ChildContextLookup<>(AuthenticationContext.class));
}
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 4f46943..d8b8966 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
@@ -37,7 +37,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
/**
- * Wrapper for use with Duo OIDC integrations.
+ * Mutable wrapper for use with Duo OIDC integrations.
*/
public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
@@ -88,20 +88,11 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
redirectURI = Constraint.isNotEmpty(uri, "Redirect_uri cannot be null or empty");
}
- /**
- * Get the redirect_uri.
- *
- * @return the redirect_uri.
- */
+ /** {@inheritDoc} */
@Nonnull @NotEmpty public String getRedirectURI() {
return redirectURI;
}
-
- /** {@inheritDoc} */
- @Nonnull @NotEmpty public String getClientId() {
- return clientId;
- }
-
+
/**
* Set the client ID to use.
*
@@ -112,12 +103,12 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
clientId = Constraint.isNotNull(StringSupport.trimOrNull(id), "ClientID cannot be null or empty");
}
-
+
/** {@inheritDoc} */
- @Nonnull @NotEmpty public String getSecretKey() {
- return secretKey;
+ @Nonnull @NotEmpty public String getClientId() {
+ return clientId;
}
-
+
/**
* Set the secret key to use.
*
@@ -128,6 +119,12 @@ public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent im
secretKey = Constraint.isNotNull(StringSupport.trimOrNull(key), "Secret key cannot be null or empty");
}
+
+ /** {@inheritDoc} */
+ @Nonnull @NotEmpty public String getSecretKey() {
+ return secretKey;
+ }
+
/** {@inheritDoc} */
@Nonnull @NonnullElements @Unmodifiable
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoClientInitializationException.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoClientInitializationException.java
index 55a694c..85d4aa9 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoClientInitializationException.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoClientInitializationException.java
@@ -17,9 +17,12 @@
package net.shibboleth.idp.plugin.authn.duo;
+import javax.annotation.concurrent.ThreadSafe;
+
/**
* Indicates an error during registry processing.
*/
+ at ThreadSafe
public class DuoClientInitializationException extends RuntimeException{
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoException.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoException.java
index f9bfb19..58c4771 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoException.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoException.java
@@ -28,10 +28,7 @@ public class DuoException extends Exception{
/** Serial UID. */
private static final long serialVersionUID = -2380145079984333546L;
- /**
- * Constructor.
- *
- */
+ /** Constructor. */
public DuoException() {
super();
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 916bdb3..d17d7a7 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
@@ -24,6 +24,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
/**
* Constants defined in the Duo OIDC Auth API.
*/
+//TODO: Same as DuoAuthAPI in the core IdP, should we just re-used?
public final class 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 3fe7333..f6ba97a 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
@@ -30,7 +30,7 @@ public interface DuoOIDCClient {
/**
* Check the health of the Duo 2FA endpoint.
*
- * @return the heath check response
+ * @return the heath check response, never {@literal null}.
*
* @throws DuoClientException if there is an error returning the health check response
*/
@@ -40,9 +40,9 @@ public interface DuoOIDCClient {
* Constructs a redirection URL string with query parameters required to initiate a Duo 2FA request.
*
* @param username The user to be authenticated by Duo.
- * @param state A randomly generated 32 character String, which is relied back to the client.
+ * @param state A randomly generated minimum 22 character String, which is relied back to the client.
*
- * @return the redirect URL
+ * @return the redirect URL as a string, never {@literal null}.
*
* @throws DuoClientException if there is an error creating the authentication URL.
*/
@@ -57,9 +57,9 @@ public interface DuoOIDCClient {
* about the authentication.
* @param username The user to be authenticated by Duo.
*
- * @return the token
+ * @return the token, never {@literal null}.
*
- * @throws DuoClientException if there is an error exchaining the auth_code for a token result.
+ * @throws DuoClientException if there is an error exchanging the auth_code for a token result.
*/
@Nonnull DuoAuthToken exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
@Nonnull final String username) throws DuoClientException;
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientFactory.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientFactory.java
index d9307d5..a46bbf6 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientFactory.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCClientFactory.java
@@ -29,7 +29,7 @@ public interface DuoOIDCClientFactory {
*
* @param integration the duo integration used to instantiate the client.
*
- * @return the created Duo client.
+ * @return the created Duo client, never {@literal null}.
*
* @throws DuoClientException if there is an error creating the Duo client.
*/
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 f973dd9..fc3715d 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
@@ -26,20 +26,20 @@ import javax.annotation.Nonnull;
public interface DuoOIDCClientRegistry {
/**
- * Get a Duo client for the given Duo integration.
+ * <p>Retrieve an existing, or create a new, Duo client for the given Duo integration.</p>
*
* <p>implSpec: Only one client should exist per integration i.e. given the same integration,
- * the same client should be returned.</p>
+ * the same client instance should be returned.</p>
*
- * <p>implNote: Clients can either be pre-registered, or lazy-initialised when first needed.</p>
+ * <p>implNote: Clients could either be pre-registered, or lazy-initialised when first needed.</p>
*
- * @param integration the Duo integration to find a client for. Never {@literal null}.
+ * @param integration the Duo integration to find a client for, never {@literal null}.
*
- * @return a {@link DuoOIDCClient} appropriate for this integration. Never {@literal null}.
+ * @return a {@link DuoOIDCClient} appropriate for this integration, never {@literal null}.
*
* @throws DuoRegistryException if there is an issue locating or creating a Duo client.
*/
- @Nonnull DuoOIDCClient getIntegrationClientOrCreate(@Nonnull final DuoOIDCIntegration integration)
+ @Nonnull DuoOIDCClient getClientOrCreate(@Nonnull final DuoOIDCIntegration integration)
throws DuoRegistryException;
}
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 f41a42c..b9e3c12 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
@@ -53,7 +53,7 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
*
* @return the redirect_uri
*/
- @Nonnull String getRedirectURI();
+ @Nonnull @NotEmpty String getRedirectURI();
}
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoRegistryException.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoRegistryException.java
index 4131dd8..6103089 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoRegistryException.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoRegistryException.java
@@ -17,9 +17,12 @@
package net.shibboleth.idp.plugin.authn.duo;
+import javax.annotation.concurrent.ThreadSafe;
+
/**
* Indicates an error during registry processing.
*/
+ at ThreadSafe
public class DuoRegistryException extends DuoException{
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 76cc234..30888af 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
@@ -28,7 +28,8 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoAuthToken;
/**
- * <p>Context that carries the Duo integration and Duo authentication result token.</p>
+ * <p>Context that carries the Duo integration, request-response state, authorization code,
+ * and Duo authentication result token.</p>
*
* @parent {@link AuthenticationContext}
* @added After extracting the Duo integration for the given authentication request.
@@ -56,7 +57,7 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
/** The Duo OIDC client to use for the lifetime of this request.*/
@Nullable private DuoOIDCClient client;
- /** Constructor. */
+ /** Public no-arg constructor to allow auto-creation. */
public DuoOIDCAuthenticationContext() {
}
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/context/package-info.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/package-info.java
similarity index 94%
rename from idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/context/package-info.java
rename to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/package-info.java
index 603c095..d3b1f2e 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/context/package-info.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/context/package-info.java
@@ -16,4 +16,4 @@
*/
/** Duo OIDC 2FA login flow API context classes. */
-package net.shbboleth.idp.plugin.authn.duo.context;
+package net.shibboleth.idp.plugin.authn.duo.context;
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/model/package-info.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/package-info.java
similarity index 94%
rename from idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/model/package-info.java
rename to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/package-info.java
index e4b99f2..b527b64 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/model/package-info.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/package-info.java
@@ -16,4 +16,4 @@
*/
/** Duo OIDC 2FA login flow API model classes. */
-package net.shbboleth.idp.plugin.authn.duo.model;
+package net.shibboleth.idp.plugin.authn.duo.model;
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/package-info.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/package-info.java
similarity index 95%
rename from idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/package-info.java
rename to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/package-info.java
index 64be76d..0273347 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/package-info.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/package-info.java
@@ -16,4 +16,4 @@
*/
/** Duo OIDC 2FA login flow API classes. */
-package net.shbboleth.idp.plugin.authn.duo;
+package net.shibboleth.idp.plugin.authn.duo;
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCDescription.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCDescription.java
index eaf1f66..edf68da 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCDescription.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCDescription.java
@@ -30,7 +30,6 @@ import org.springframework.core.io.ClassPathResource;
import net.shibboleth.idp.plugin.AbstractPluginDescription;
import net.shibboleth.idp.plugin.PluginVersion;
-import net.shibboleth.idp.plugin.authn.duo.Version;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
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 65397b1..14142a8 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
@@ -48,12 +48,19 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* or existing {@link DuoOIDCClient} instance.</p>
*
* <p>Clients are created and registered against a {@link DuoOIDCIntegration}. Once created the client is
- * re-used for the lifetime of the IdP. The {@link DuoOIDCIntegration} should decide it's own business key (using the
- * equals and hashcode method appropriately), the {@link DefaultDuoOIDCIntegration} key is the clientID.</p>
+ * reused for the lifetime of the IdP. </p>
*
- * <p>Supports lazy intilization of clients when they are first requested. A single, configurable, client factory
+ * <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 it's key.</p>
+ *
+ * <p>Supports lazy initialization of clients when they are first requested. A single, configurable, client factory
* is called to initilize 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>
+ *
*/
@ThreadSafe
public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializableComponent
@@ -84,13 +91,12 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
*/
public void setClientFactory(@Nonnull final DuoOIDCClientFactory factory) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
- Constraint.isNotNull(factory, "Duo client factory can not be null");
- clientFactory = factory;
+ clientFactory = Constraint.isNotNull(factory, "Duo client factory can not be null");
}
-
+ /** {@inheritDoc} */
@Override
- @Nonnull public DuoOIDCClient getIntegrationClientOrCreate(@Nonnull final DuoOIDCIntegration integration)
+ @Nonnull public DuoOIDCClient getClientOrCreate(@Nonnull final DuoOIDCIntegration integration)
throws DuoRegistryException {
Constraint.isNotNull(integration, "Duo integration can not be null");
@@ -111,7 +117,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
super.doInitialize();
if (clientFactory == null) {
- throw new ComponentInitializationException("Duo Client Factory cannot be null");
+ throw new ComponentInitializationException("A Duo Client Factory must be configured and cannot be null");
}
}
@@ -125,7 +131,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
@Nonnull private final Logger log = LoggerFactory.getLogger(CreateNewClientMappingFunction.class);
@Override
- public DuoOIDCClient apply(@Nonnull final DuoOIDCIntegration integration){
+ @Nonnull public DuoOIDCClient apply(@Nonnull final DuoOIDCIntegration integration){
try {
log.debug("Creating a new Duo client for integration '{}', using factory type '{}'",integration
@@ -134,7 +140,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
} catch (final DuoClientException e) {
//wrap the exception in a runtime exception.
throw new DuoClientInitializationException("Could not initialise "
- + "the DuoClient for the client "+integration.getClientId(),e);
+ + "the DuoClient for the integration with clientId "+integration.getClientId(),e);
}
}
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 0cc1a50..43ddb1f 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
@@ -43,13 +43,16 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
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} mechanism.</p>
+ * <p>MVC controller for managing Duo 2FA exchanges implemented as an {@link ExternalAuthentication}
+ * mechanism.</p>
*
- * <p>The controller initiates the Duo OIDC authorization code grant flow and accepts the authorization code response.</p>
+ * <p>The controller initiates the Duo OIDC authorization code grant flow and accepts the authorization
+ * code response.</p>
*
*/
@Controller
@@ -85,11 +88,13 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
*/
public void setDuoContextLookupStrategy(
@Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
- duoContextLookupStrategy = Constraint.isNotNull(strategy, "DuoContextLookuplookup strategy cannot be null");
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ duoContextLookupStrategy = Constraint.isNotNull(strategy, "DuoContext lookup strategy cannot be null");
}
/**
- * Start the Duo ODIC authorization code flow.
+ * Start the Duo ODIC authorization code flow. The SWF execution key is encoded in the state parameter
+ * so it can be extracted on return from Duo.
*
* @param httpRequest servlet request
* @param httpResponse servlet response
@@ -112,9 +117,10 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
return;
}
- final DuoOIDCIntegration integration = duoContext.getIntegration();
+
log.trace("Starting Duo 2FA for integration client '{}' and user '{}'",
- integration.getClientId(), duoContext.getUsername());
+ duoContext.getIntegration() != null ? duoContext.getIntegration().getClientId():
+ "none", duoContext.getUsername());
try {
final DuoOIDCClient client = duoContext.getClient();
if (client == null) {
@@ -124,7 +130,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
//TODO: could use a type of replay cache and storage service?
final String nonce = DuoSupport.generateNonce(32);
final String state = DuoSupport.generateState(nonce, key);
- //store only the nonce as the request state. The key is only used to resume the flow.
+ //store only the nonce component as the request state. The key component is only used to resume the flow.
duoContext.setRequestState(nonce);
final String authURL = client.createAuthUrl(duoContext.getUsername(), state);
httpResponse.sendRedirect(authURL);
@@ -164,7 +170,6 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
throw new ExternalAuthenticationException("Flow execution key component could not be found in the "
+ "returned state, unable to resume the flow execution",e);
}
- log.trace("Duo OIDC callback has flow execution key '{}'",key);
final ProfileRequestContext prc = ExternalAuthentication.getProfileRequestContext(key, httpRequest);
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 68a35c6..9a005db 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
@@ -35,10 +35,11 @@ import net.shibboleth.idp.plugin.authn.duo.model.DuoAuthToken;
/**
* Action to exchange the authorization code in the Duo 2FA response for a Duo id_token that describes the result
- * of 2FA. Adds the token to the Duo context.
+ * of 2FA. Once obtained, adds the token to the Duo context.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link AuthnEventIds#AUTHN_EXCEPTION}
+ * @event {@link AuthnEventIds#NO_CREDENTIALS}
* @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null and
* AuthenticationContext.getSubcontect(DuoOIDCAuthenticationContext.class,false)!=null</pre>
* @post Add the Duo authentication token to the context.
@@ -65,13 +66,13 @@ public class ExchangeCodeForDuoToken extends AbstractDuoAuthenticationAction{
if (code == null) {
log.error("{} Duo 2FA authorization code is not available in the response",getLogPrefix());
//FIXME: maybe our own exception and switch in the flow here.
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+ ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
return;
}
final String username = duoContext.getUsername();
if (username == null) {
log.error("{} Username is not available in the Duo context",getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+ ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
return;
}
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 6c7162c..cb5848d 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
@@ -52,6 +52,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
/** Event signalling that the Duo 2FA endpoints are not available. */
+ //TODO: Unused?
@Nonnull @NotEmpty public static final String DUO_UNAVAILABLE = "DuoUnavailable";
/** Class logger. */
@@ -67,9 +68,9 @@ public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
if (client == null) {
throw new DuoClientException("Duo client is null, has the context been created correctly?");
}
- //Native duo client will throw an exception if anything other than OK is returned.
+ //Native duo SDK will throw an exception if anything other than OK is returned.
final DuoHealthCheck healthCheckResponse = client.healthCheck();
- log.trace("Duo health check response '{}'",healthCheckResponse);
+ log.trace("{} Duo health check response '{}'",getLogPrefix(),healthCheckResponse);
if (healthCheckResponse == null) {
//no response.
@@ -86,16 +87,16 @@ public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
} else if (DuoOIDCAuthAPI.DUO_RESPONSE_STATUS_FAIL.equalsIgnoreCase(healthCheckResponse.getStatus())) {
//2FA is unavailable for the given client integration.
log.info("{} Duo 2FA health check failed, current status '{}',"
- + "code '{}', message '{}', message detail '{}'",getLogPrefix(),
- healthCheckResponse.getStatus(),healthCheckResponse.getCode(),
- healthCheckResponse.getMessage(),healthCheckResponse.getMessageDetail());
- throw new DuoClientException("Duo 2FA health check responded with a failure status "+
+ + " message '{}', message detail '{}'",getLogPrefix(),
+ healthCheckResponse.getStatus(),healthCheckResponse.getMessage(),
+ healthCheckResponse.getMessageDetail());
+ throw new DuoClientException("Duo 2FA health check responded with a failure status of: "+
healthCheckResponse.getMessage());
} else {
log.info("{} Duo health check response contained an unknown status of '{}', "
- + "code '{}', message '{}', message detail '{}'",getLogPrefix(),
- healthCheckResponse.getStatus(),healthCheckResponse.getCode(),
+ + " message '{}', message detail '{}'",getLogPrefix(),
+ healthCheckResponse.getStatus(),
healthCheckResponse.getMessage(),healthCheckResponse.getMessageDetail());
throw new DuoClientException("Duo 2FA health check returned an unknown status response: "+
healthCheckResponse.getMessage());
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 5b79983..6f83b86 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
@@ -165,7 +165,7 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
//Configure the Duo client for the established integration
try {
- DuoOIDCClient client = clientRegistry.getIntegrationClientOrCreate(duoIntegration);
+ final DuoOIDCClient client = clientRegistry.getClientOrCreate(duoIntegration);
context.setClient(client);
} catch (final DuoRegistryException e) {
log.warn("{} No DuoClient established (located or created) for "
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseState.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseState.java
index 480a762..1f2de4e 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseState.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseState.java
@@ -35,7 +35,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
* matches that in the 2FA request.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
- * @event {@link AuthnEventIds#AUTHN_EXCEPTION}
+ * @event {@link AuthnEventIds#INVALID_CREDENTIALS}
* @pre <pre>
* ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
* </pre>
@@ -50,6 +50,7 @@ public class ValidateDuoResponseState extends AbstractDuoAuthenticationAction {
/** Class logger. */
@Nonnull @NotEmpty private final Logger log = LoggerFactory.getLogger(ValidateDuoResponseState.class);
+ /** {@inheritDoc} */
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@Nonnull final DuoOIDCAuthenticationContext duoContext) {
@@ -60,13 +61,13 @@ public class ValidateDuoResponseState extends AbstractDuoAuthenticationAction {
if (duoContext.getRequestState() == null || duoContext.getResponseState() == null) {
log.error("{} The state parameter was not present in either the request or response, "
+ "state is mandatory for Duo 2FA requests",getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+ ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_CREDENTIALS);
return;
}
if (!duoContext.getRequestState().equals(duoContext.getResponseState())) {
log.error("{} Duo request state did not match response state, has it been tampered with!",
getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+ ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_CREDENTIALS);
//blank request and response for safety
blankState(duoContext);
return;
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAudience.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAudience.java
index b3831d1..b426786 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAudience.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAudience.java
@@ -72,6 +72,7 @@ public class ValidateDuoTokenAudience extends AbstractDuoAuthenticationAction {
return true;
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -93,7 +94,7 @@ public class ValidateDuoTokenAudience extends AbstractDuoAuthenticationAction {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
return;
}
- log.debug("{} Token has the correct audience '{}' for this client",getLogPrefix(),audience);
+ log.trace("{} Token has the correct audience '{}' for this client",getLogPrefix(),audience);
//audience is fine
}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
index d176ef0..86eb33d 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
@@ -163,8 +163,8 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
subject.getPrincipals().addAll(duoContext.getIntegration().getSupportedPrincipals(Principal.class));
//add any further principals from a function hook that can inspect the Duo response.
- //If the mapping strategy is set, the defaults are not copied over from the flow. Hence,
- //these will be added only to those added above.
+ //If the mapping strategy is set, the defaults should not be copied over from the flow. Hence,
+ //these will be added only to those added above (this is configured in the XML).
if (getContextToPrincipalMappingStrategy() != null) {
final Collection<Principal> mapped = getContextToPrincipalMappingStrategy().apply(prc);
if (mapped != null) {
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationTime.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationTime.java
index a7f64b0..cf3c26a 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationTime.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationTime.java
@@ -41,7 +41,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* <p> An action that checks if auth_time (when the End-User authentication took place) stored within the id_token
* is within a valid expiration window.</p>
*
- * <p> This only applies to forced authentication requests, to ensure active 2FA was performed.</p>
+ * <p> This only applies to forced authentication requests to ensure active 2FA was performed.</p>
*
* @pre <pre>
* ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
@@ -103,6 +103,7 @@ public class ValidateDuoTokenAuthenticationTime extends AbstractDuoAuthenticatio
clockSkew = Constraint.isNotNull(skew, "Clock skew cannot be null");
}
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -118,6 +119,7 @@ public class ValidateDuoTokenAuthenticationTime extends AbstractDuoAuthenticatio
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -154,7 +156,9 @@ public class ValidateDuoTokenAuthenticationTime extends AbstractDuoAuthenticatio
"{} Authentication required (forced) but has expired: auth_time was '{}', expired at: '{}', current time: '{}'",
getLogPrefix(), authTime, expiration, now);
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+ return;
}
+ //is OK.
}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenExpirationTime.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenExpirationTime.java
index 80a6da2..6464e3b 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenExpirationTime.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenExpirationTime.java
@@ -37,7 +37,7 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
- * An action that verifies the expiration time (exp) of an id_token. If the expiration time has past
+ * An action that verifies the expiration time (exp) of an id_token. If the expiration time has past,
* the id_token must not be accepted. A few minutes of leeway is allowed - clock skew.
* See section 3.1.3.7 of OpenID Connect core 1.0.
*
@@ -83,6 +83,7 @@ public class ValidateDuoTokenExpirationTime extends AbstractDuoAuthenticationAct
clockSkew = Constraint.isNotNull(skew, "Clock skew cannot be null");
}
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -98,6 +99,7 @@ public class ValidateDuoTokenExpirationTime extends AbstractDuoAuthenticationAct
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuedAt.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuedAt.java
index 3fba868..c46a27c 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuedAt.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuedAt.java
@@ -89,6 +89,7 @@ public class ValidateDuoTokenIssuedAt extends AbstractDuoAuthenticationAction {
iatMaxClockSkew = clockSkew;
}
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -104,6 +105,7 @@ public class ValidateDuoTokenIssuedAt extends AbstractDuoAuthenticationAction {
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -118,7 +120,7 @@ public class ValidateDuoTokenIssuedAt extends AbstractDuoAuthenticationAction {
}
final Instant now = Instant.now();
- //double to long conversation with rounding!
+ //double to long conversation with rounding! it is unlikely this field will contain a decimal value
final Instant iat = Instant.ofEpochSecond(Math.round(iatEpochSeconds));
final Duration iatDifference = Duration.between(now, iat).abs();
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuer.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuer.java
index 5cba9d4..9243ce6 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuer.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenIssuer.java
@@ -38,12 +38,12 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* <p>An action that verifies the issuer (iss) of the id_token exactly matches that of the configured
* Duo token provider. See section 3.1.3.7 of OpenID Connect core 1.0.</p>
*
- * <p>This is specific to the Duo implementation, and mimics the native Duo client issuer validation.</p>
+ * <p>The logic here is specific to the Duo implementation, and mimics the native Duo client issuer validation.</p>
*
* @pre
*
* <pre>
- * ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
+ * ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
* </pre>
*
* @pre
@@ -91,6 +91,7 @@ public class ValidateDuoTokenIssuer extends AbstractDuoAuthenticationAction {
issuerPath = Constraint.isNotNull(path, "Issuer URL path cannot be null");
}
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -106,6 +107,7 @@ public class ValidateDuoTokenIssuer extends AbstractDuoAuthenticationAction {
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -130,7 +132,7 @@ public class ValidateDuoTokenIssuer extends AbstractDuoAuthenticationAction {
}
final String issuerFromIntegration = HTTPS+apiHost+issuerPath;
- log.trace("{} Token issuers is '{}', expected '{}'",getLogPrefix(),issuer, issuerFromIntegration);
+ log.trace("{} Token issuer is '{}', expected '{}'",getLogPrefix(),issuer, issuerFromIntegration);
if (!issuer.equals(issuerFromIntegration)) {
log.error("{} Token issuer differs from that expected, issuer is '{}', expected '{}'",getLogPrefix(),
issuer,issuerFromIntegration);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenSubject.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenSubject.java
index 1768beb..207adc2 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenSubject.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenSubject.java
@@ -58,6 +58,7 @@ public class ValidateDuoTokenSubject extends AbstractDuoAuthenticationAction {
/** The Duo authentication token. */
@Nullable private DuoAuthToken token;
+ /** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
@@ -73,6 +74,7 @@ public class ValidateDuoTokenSubject extends AbstractDuoAuthenticationAction {
}
+ /** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
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 e07bdb6..536c6a2 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
@@ -74,15 +74,15 @@ public class DefaultDuoOIDCClientRegistryTest {
integ.setRedirectURI("http://localhost/");
integ.setSecretKey("rFvDfPul27v3Wew2zb6xRPzAJewJ34MP2w8UitPh");
- final DuoOIDCClient client = registry.getIntegrationClientOrCreate(integ);
- final DuoOIDCClient clientTwo = registry.getIntegrationClientOrCreate(integ);
+ final DuoOIDCClient client = registry.getClientOrCreate(integ);
+ final DuoOIDCClient clientTwo = registry.getClientOrCreate(integ);
assertEquals(client, clientTwo);
// set to a different value
integ.setClientId("DIU6GEFWG5LIUBVV2M3B");
// should get a new client.
- final DuoOIDCClient clientThree = registry.getIntegrationClientOrCreate(integ);
+ final DuoOIDCClient clientThree = registry.getClientOrCreate(integ);
System.out.println("Client: " + client + " Client2: " + clientTwo + " Client3: " + clientThree);
assertNotSame(client, clientThree);
@@ -109,7 +109,7 @@ public class DefaultDuoOIDCClientRegistryTest {
final Collection<Future<DuoOIDCClient>> futures = new ArrayList<>(2);
for (int t = 0; t < 2; ++t) {
- futures.add(service.submit(()->registry.getIntegrationClientOrCreate(integ)));
+ futures.add(service.submit(()->registry.getClientOrCreate(integ)));
}
for (final Future<DuoOIDCClient> f : futures) {
final DuoOIDCClient client = f.get();
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 67dcaa3..c3b2b89 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,7 +73,7 @@ public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
action.initialize();
final Event event = action.execute(src);
- assertEventId(event, AuthnEventIds.AUTHN_EXCEPTION);
+ assertEventId(event, AuthnEventIds.NO_CREDENTIALS);
}
}
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 ad58f37..f4a3f98 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
@@ -64,7 +64,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
addAttemptedFlow("authn/DuoOIDC");
action.initialize();
final Event event = action.execute(src);
- assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
+ assertEventId(event,AuthnEventIds.INVALID_CREDENTIALS);
}
/* Test Duo 2FA response validation, no state in response. */
@@ -76,7 +76,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
dc.setRequestState(DuoSupport.generateNonce(32));
action.initialize();
final Event event = action.execute(src);
- assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
+ assertEventId(event,AuthnEventIds.INVALID_CREDENTIALS);
}
/* Test Duo 2FA response validation, no state in request.*/
@@ -88,7 +88,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
dc.setResponseState(DuoSupport.generateNonce(32));
action.initialize();
final Event event = action.execute(src);
- assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
+ assertEventId(event,AuthnEventIds.INVALID_CREDENTIALS);
}
}
diff --git a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
index 7b1e279..df6e528 100644
--- a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
+++ b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
@@ -23,7 +23,7 @@ import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
- * <p>An object adaptor class for bridging between the Duo SDK implementation
+ * <p>An Object Adaptor class for bridging between the Duo SDK implementation
* and the internal {@link DuoOIDCClient} interface.</p>
*
* <p>This is package private, and can only be instantiated by the {@link DuoSDKClientFactory}.</p>
@@ -44,7 +44,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
*
* Constructor. Initialises the native Duo SDK client.
*
- * @param integration the Duo integration to initilize the client from. Never {@literal null}.
+ * @param integration the Duo integration to initialize the client from. Never {@literal null}.
* @param caCerts the list of CA Certificates used to validate connections to Duo. Can be {@literal null}.
*
* @throws DuoClientException if there is an error instantiating the client
@@ -65,6 +65,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
caCerts.toArray(new String[caCerts.size()]));
}
} catch (final DuoException e) {
+ //wrap exception and throw
throw new DuoClientException(e);
}
@@ -81,6 +82,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
healthCheckResponseConverter = Constraint.isNotNull(converter, "Health Check response converter can not be null");
}
+ /** {@inheritDoc} */
@Override
@Nonnull public DuoHealthCheck healthCheck() throws DuoClientException {
try {
@@ -98,6 +100,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
}
+ /** {@inheritDoc} */
@Override
@Nonnull public String createAuthUrl(@Nonnull final String username, @Nonnull final String state)
throws DuoClientException {
@@ -109,6 +112,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
}
}
+ /** {@inheritDoc} */
@Override
@Nonnull public DuoAuthToken exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
@Nonnull final String username)
@@ -120,7 +124,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
}
return tokenResponseConverter.apply(token);
} catch (final DuoException e) {
- //wrap duo specific exception.
+ //wrap duo specific exception.
throw new DuoClientException(e);
}
}
diff --git a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
index 69d5572..06577a2 100644
--- a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
+++ b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
@@ -15,21 +15,19 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
-/**
- * Abstract factory implementation for the {@link DuoSDKClientAdaptor}.
- */
+/** Abstract factory implementation for the {@link DuoSDKClientAdaptor}. */
public class DuoSDKClientFactory implements DuoOIDCClientFactory{
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientFactory.class);
- /** List of CA Certificate pins. If null, the clients default set are used.*/
+ /** List of CA Certificate pins. If null, the client's default set are used.*/
@Nullable private List<String> caCerts;
/**
* Sets the list of CA certificate pins used to verify the Duo client connection
- * to the API host. If null, the client will use the internal defaults.
+ * to the API host. If {@literal null}, the client will use the internal defaults.
*
* @param certs the list of certificate pins.
*/
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list