[java-idp-plugin-duo] branch main updated: Improve Javadoc and syntax
Phil Smart
philip.smart at jisc.ac.uk
Thu Nov 12 16:42:32 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=be69e4a1d376e8339a23fe78a95280ace90f16fa
The following commit(s) were added to refs/heads/main by this push:
new be69e4a Improve Javadoc and syntax
be69e4a is described below
commit be69e4a1d376e8339a23fe78a95280ace90f16fa
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Nov 12 16:42:25 2020 +0000
Improve Javadoc and syntax
---
.../plugin/authn/duo/AbstractDuoAuthenticationAction.java | 4 +++-
.../idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java | 1 +
.../shibboleth/idp/plugin/authn/duo/DuoOIDCAuthAPI.java | 4 +++-
.../net/shibboleth/idp/plugin/authn/duo/DuoOIDCClient.java | 8 ++++----
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 4 ++--
.../java/net/shibboleth/idp/plugin/authn/duo/Version.java | 4 ++--
.../authn/duo/context/DuoOIDCAuthenticationContext.java | 2 +-
.../idp/plugin/authn/duo/model/DuoHealthCheck.java | 2 +-
.../idp/plugin/authn/duo/model/DuoHealthCheckResponse.java | 3 +++
.../idp/plugin/authn/duo/impl/DuoJWTClaimsVerifier.java | 14 ++++----------
.../idp/plugin/authn/duo/impl/ExchangeCodeForDuoToken.java | 1 -
.../plugin/authn/duo/impl/HealthCheckDuoOIDCAuthAPI.java | 2 +-
.../duo/impl/ValidateDuoTokenAuthenticationResult.java | 4 +++-
.../idp/plugin/authn/duo/impl/ValidateTokenClaims.java | 1 +
.../idp/plugin/authn/duo/impl/ValidateTokenSignature.java | 13 +++++++++++--
.../idp/plugin/authn/mock/MockDuoOIDCClient_UNKNOWN.java | 5 -----
.../idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 2 +-
.../idp/plugin/authn/duo/nimbus/NimbusClient.java | 10 +++++-----
.../idp/plugin/authn/duo/nimbus/NimbusClientFactory.java | 9 +++------
.../idp/plugin/authn/duo/nimbus/NimbusClientSupport.java | 7 ++++---
20 files changed, 53 insertions(+), 47 deletions(-)
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 fdb8f56..35802e0 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
@@ -42,11 +42,13 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* <p>In addition to the work performed by {@link AbstractAuthenticationAction}, this action also looks up
* and makes available the {@link DuoOIDCAuthenticationContext}.</p>
*
- * <p>Duo 2FA authentication action implementations should override
+ * <p>Duo 2FA authentication action implementations should override the
* {@link #doExecute(ProfileRequestContext, AuthenticationContext, DuoOIDCAuthenticationContext)}
* method.</p>
*
* @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
+ * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class) != null</pre>
+ * @post <pre>AuthenticationContext.getSubcontext(DuoOIDCAuthenticationContext.class) != null</pre>
*/
public abstract class AbstractDuoAuthenticationAction extends AbstractAuthenticationAction {
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 fa70b88..2c0f26f 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
@@ -39,6 +39,7 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
/**
* Wrapper for use with Duo OIDC integrations. Class is made effectively immutable once initialized.
*/
+//TODO: would need more explanation as to why this becomes immutable - otherwise remove.
public class DefaultDuoOIDCIntegration extends AbstractInitializableComponent implements DuoOIDCIntegration{
/** API host. */
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 cefb62e..745b068 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,7 +24,6 @@ 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 {
@@ -54,6 +53,9 @@ 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 name of the authentication time claim.*/
+ @Nonnull @NotEmpty public static final String DUO_AUTH_TIME_CLAIM_NAME = "auth_time";
/** 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 7f2fbec..056a647 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,10 +38,10 @@ public interface DuoOIDCClient {
@Nonnull DuoHealthCheck healthCheck() throws DuoClientException;
/**
- * Constructs a redirection URL string with query parameters required to initiate a Duo 2FA request.
+ * Constructs a redirection URL string with the query parameters required to initiate a Duo 2FA request.
*
* @param username The user to be authenticated by Duo.
- * @param state A randomly generated minimum 22 character String, which is relied back to the client.
+ * @param state A randomly generated minimum 22 character String, which is relayed back to the client.
*
* @return the redirect URL as a string, never {@code null}.
*
@@ -50,7 +50,7 @@ public interface DuoOIDCClient {
@Nonnull String createAuthUrl(@Nonnull final String username, @Nonnull final String state) throws DuoClientException;
/**
- * Exchanges the authorizaton code for a Json Web Token (JWT) which contains information pertaining to
+ * Exchanges the authorizaton code for a signed Json Web Token (JWT) which contains information pertaining to
* the authentication. There is no requirement on the client to verify the tokens authenticity or
* claims - these should be performed elsewhere in the flow. The JWT **must** be signed.
*
@@ -67,7 +67,7 @@ public interface DuoOIDCClient {
@Nonnull final String username) throws DuoClientException;
/**
- * Get an identifier for this client e.g. a UUID.
+ * Get an identifier for this client e.g. a UUID. Only for logging and debugging.
*
* @return the client identifier.
*/
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 9eaecfc..11cdf67 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
@@ -23,8 +23,8 @@ import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
/**
- * Interface to a particular Duo OIDC integration point. In part replaces any
- * OIDC metadata, as that is not supported by Duo.
+ * Interface to a particular Duo OIDC integration point. In part replaces
+ * OIDC metadata as that is not supported by Duo.
*/
public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/Version.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/Version.java
index 68aac8a..36c7c9b 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/Version.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/Version.java
@@ -19,10 +19,10 @@ package net.shibboleth.idp.plugin.authn.duo;
import javax.annotation.Nullable;
-/** Class for getting and printing the version of the IdP. */
+/** Class for getting and printing the version of the plugin. */
public final class Version {
- /** IdP version. */
+ /** Plugin version. */
@Nullable private static final String VERSION = Version.class.getPackage().getImplementationVersion();
/** Constructor. */
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 7e23de3..2522d0f 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
@@ -55,7 +55,7 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
/** The JWT token received from Duo as a result of 2FA.*/
@Nullable private JWT authToken;
- /** The Duo OIDC client to use for the lifetime of this request.*/
+ /** The Duo OIDC client to use for the lifetime of this authentication request.*/
@Nullable private DuoOIDCClient client;
/** Public no-arg constructor to allow auto-creation. */
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheck.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheck.java
index 7a39e89..5965313 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheck.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheck.java
@@ -19,7 +19,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
* <p>Includes a staged builder for fluent generation. Compatible with Jackson
* deserialization.</p>
*
- * <p>Is immutable, can only be built using the builder.</p>
+ * <p>Is immutable, can only be built using the builder and not changed thereafter.</p>
*/
@Immutable
@JsonDeserialize(builder=DuoHealthCheck.Builder.class)
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheckResponse.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheckResponse.java
index ea8a338..61d780f 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheckResponse.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/model/DuoHealthCheckResponse.java
@@ -6,6 +6,9 @@ import javax.annotation.concurrent.Immutable;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
+/**
+ * Represents part of the {@link DuoHealthCheck} response.
+ */
@Immutable
@JsonIgnoreProperties(ignoreUnknown = true)
public final class DuoHealthCheckResponse {
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoJWTClaimsVerifier.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoJWTClaimsVerifier.java
index 23d7499..56641c5 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoJWTClaimsVerifier.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoJWTClaimsVerifier.java
@@ -34,6 +34,7 @@ import com.nimbusds.jwt.proc.BadJWTException;
import com.nimbusds.jwt.proc.DefaultJWTClaimsVerifier;
import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
@@ -46,11 +47,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
*/
@ThreadSafe
public class DuoJWTClaimsVerifier extends DefaultJWTClaimsVerifier<ProfileRequestJWTSecurityContext>{
-
- //TODO: move this to the DuoOIDAuthAPI constants?
- /** The name of the authentication time claim.*/
- @Nonnull public static final String AUTH_TIME_CLAIM_NAME = "auth_time";
-
+
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoJWTClaimsVerifier.class);
@@ -126,9 +123,6 @@ public class DuoJWTClaimsVerifier extends DefaultJWTClaimsVerifier<ProfileReques
private void verifyAuthenticationTime(@Nonnull final JWTClaimsSet claimsSet,
@Nonnull final ProfileRequestJWTSecurityContext context) throws BadJWTException {
- if (context.getPrc() == null) {
- throw new BadJWTException("No profile request context found, can not validate authentication time");
- }
final AuthenticationContext ac = context.getPrc().getSubcontext(AuthenticationContext.class);
if (ac == null) {
throw new BadJWTException("No authentication request context found, can not validate authentication time");
@@ -140,7 +134,7 @@ public class DuoJWTClaimsVerifier extends DefaultJWTClaimsVerifier<ProfileReques
} else {
//forced authn, so check authentication time
try {
- final Date authTimeDate = claimsSet.getDateClaim(AUTH_TIME_CLAIM_NAME);
+ final Date authTimeDate = claimsSet.getDateClaim(DuoOIDCAuthAPI.DUO_AUTH_TIME_CLAIM_NAME);
if (authTimeDate == null) {
throw new BadJWTException("No authentication time found in token");
}
@@ -172,7 +166,7 @@ public class DuoJWTClaimsVerifier extends DefaultJWTClaimsVerifier<ProfileReques
}
/**
- * Verifies the IssuedAt claim exists, and is within a specified window from the current time.
+ * Verifies the IssuedAt claim exists and is within a specified window from the current time.
*
* @param claimsSet the claimset.
*
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 fd23054..22249f9 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
@@ -68,7 +68,6 @@ public class ExchangeCodeForDuoToken extends AbstractDuoAuthenticationAction{
final String code = duoContext.getAuthorizationCode();
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.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 79c9acf..700dfd1 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
@@ -74,7 +74,7 @@ public class HealthCheckDuoOIDCAuthAPI extends AbstractDuoAuthenticationAction{
throw new DuoClientException("Duo 2FA is unavailable, null response from the "
+ "health check endpoint");
}
- //These checks are redundant if using v1.0.0 of the native Duo client as it throws an exception if not 'OK'.
+ //These checks are redundant if using the native Duo 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());
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 480715c..a5acc4d 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
@@ -49,7 +49,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
/**
- * A validation action that checks for a validate {@link DuoAuthToken} and directly produces an
+ * A validation action that checks for a valid {@link DuoAuthToken} and directly produces an
* {@link AuthenticationResult} based on the identity described by the token.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
@@ -132,6 +132,7 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
if (token == null) {
log.error("{} Duo 2FA token is not available", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
+ recordFailure(profileRequestContext);
return false;
}
try {
@@ -143,6 +144,7 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
} catch (final ParseException | DuoException e) {
log.error("{} Claimset of Duo 2FA token is not available", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
+ recordFailure(profileRequestContext);
return false;
}
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 80f3075..48a78f5 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
@@ -88,6 +88,7 @@ import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
* @event {@link net.shibboleth.idp.authn.AuthnEventIds#AUTHN_EXCEPTION}
* @event {@link net.shibboleth.idp.authn.AuthnEventIds#NO_CREDENTIALS}
*/
+//TODO: maybe the claims checker could be injected here?
public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
/** HTTPS scheme protocol.*/
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
index 7d6edd6..95f655d 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignature.java
@@ -47,7 +47,16 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
* Action to validate the JWT signature. The JWT **must** be signed using the HMAC_SHA family,
* any other type including 'none', emits an error back to the flow.
*
- * TODO: conditions
+ *
+ * @event {@link net.shibboleth.idp.authn.AuthnEventIds#NO_CREDENTIALS}
+ * @event {@link net.shibboleth.idp.authn.AuthnEventIds#INVALID_AUTHN_CTX}
+ * @pre <pre>
+ * ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
+ * </pre>
+ *
+ * @pre <pre>
+ * AuthenticationContext.getSubcontext(DuoOIDCAuthenticationContext.class, false) != null
+ * </pre>
*/
public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
@@ -128,7 +137,7 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
} else {
log.debug("{} Token signature is valid; using algorithm family '{}' for client '{}'",
getLogPrefix(),SUPPORTED_SIGNATURE_FAMILY, integration.getClientId());
- //is valid
+ //Valid token
return;
}
} else {
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 c3d1f94..4a17b49 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
@@ -29,12 +29,9 @@ import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.JWSHeader;
import com.nimbusds.jose.JWSSigner;
-import com.nimbusds.jose.PlainHeader;
import com.nimbusds.jose.crypto.MACSigner;
-import com.nimbusds.jose.util.Base64URL;
import com.nimbusds.jwt.JWT;
import com.nimbusds.jwt.JWTClaimsSet;
-import com.nimbusds.jwt.PlainJWT;
import com.nimbusds.jwt.SignedJWT;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
@@ -42,8 +39,6 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
-import net.shibboleth.utilities.java.support.codec.Base64Support;
-import net.shibboleth.utilities.java.support.codec.EncodingException;
/**
* Mock a Duo client which returns an unknown response for the health check and a login failure in the
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 69af93c..8056847 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
@@ -218,7 +218,7 @@ final class DuoSDKClientAdaptor implements DuoOIDCClient{
final String duoTokenAsJson = objectMapper.writeValueAsString(t);
//re-sign the token using the client secret
final JWSSigner signer = new MACSigner(integ.getSecretKey());
- //FIXME: needs to be HS512 (or not?)
+ //needs to be HS512 (or not?) this is only passed internally, so should be fine
final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.HS256).build();
final JWTClaimsSet claims = JWTClaimsSet.parse(duoTokenAsJson);
final SignedJWT signedJWT = new SignedJWT(header,claims);
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClient.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClient.java
index 3d69b09..999305f 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClient.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClient.java
@@ -98,7 +98,7 @@ final class NimbusClient implements DuoOIDCClient{
httpClientSecurityParameters = params;
objectMapper = oMapper;
clientId = UUID.randomUUID().toString();
- //TODO: should we validate the integration here e.g. secret key length, or when being set etc.
+ //We do not validate the integration parameters e.g. because we know Duo use an invalid secret key
}
/** {@inheritDoc} */
@@ -124,9 +124,10 @@ final class NimbusClient implements DuoOIDCClient{
/** {@inheritDoc} */
@Override
- public String createAuthUrl(@Nonnull @NotEmpty final String username,
- @Nonnull final String state) throws DuoClientException {
+ @Nonnull public String createAuthUrl(@Nonnull @NotEmpty final String username,
+ @Nonnull @NotEmpty final String state) throws DuoClientException {
Constraint.isNotEmpty(username, "Username can not be null or empty");
+ Constraint.isNotEmpty(state, "State can not be null or empty");
Constraint.isGreaterThan(21, state.length(), "State must be at least 22 characters");
Constraint.isLessThan(1025, state.length(),"State must be at maximum 1024 characters");
@@ -153,7 +154,7 @@ final class NimbusClient implements DuoOIDCClient{
/** {@inheritDoc} */
@Override
- public JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
+ public SignedJWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
@Nonnull final String username) throws DuoClientException {
Constraint.isNotEmpty(code, "Auth_code can not be null");
@@ -173,7 +174,6 @@ final class NimbusClient implements DuoOIDCClient{
final TokenResponse response = executeRequest(rb.build(),new TypeReference<TokenResponse>() {});
log.trace("Duo token response: '{}'",response);
- //Accepts only a JWS not a JWE from Duo - method can return any.
return SignedJWT.parse(response.getIdToken());
} catch (final URISyntaxException | ParseException e) {
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientFactory.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientFactory.java
index 37ec141..2cdf6dc 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientFactory.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientFactory.java
@@ -47,16 +47,13 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
public class NimbusClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory {
/** HttpClient for contacting Duo. */
- @NonnullAfterInit
- private HttpClient httpClient;
+ @NonnullAfterInit private HttpClient httpClient;
/** HTTP client security parameters. */
- @Nullable
- private HttpClientSecurityParameters httpClientSecurityParameters;
+ @Nullable private HttpClientSecurityParameters httpClientSecurityParameters;
/** JSON object mapper. */
- @NonnullAfterInit
- private ObjectMapper objectMapper;
+ @NonnullAfterInit private ObjectMapper objectMapper;
@Override
public DuoOIDCClient createInstance(@Nonnull final DuoOIDCIntegration integration) throws DuoClientException {
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientSupport.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientSupport.java
index 8548cdb..b7cdecb 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientSupport.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/NimbusClientSupport.java
@@ -40,6 +40,7 @@ import com.nimbusds.jwt.SignedJWT;
import net.shibboleth.idp.plugin.authn.duo.DuoClientException;
import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.codec.EncodingException;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -84,10 +85,10 @@ public final class NimbusClientSupport {
*/
//TODO this method and the below should be nimbus, inside oidc-commons, and merged into a single API
@Nonnull static String createJWSRequestObject(@Nonnull final DuoOIDCIntegration duoIntegration,
- @Nonnull final String state, @Nonnull final String username) throws DuoClientException{
+ @Nonnull @NotEmpty final String state, @Nonnull final String username) throws DuoClientException{
Constraint.isNotNull(duoIntegration, "Duo Integration can not be null");
- Constraint.isNotNull(state, "state can not be null");
+ Constraint.isNotEmpty(state, "State can not be null or empty");
Constraint.isNotNull(username, "username can not be null");
final Date expiration = new Date();
@@ -180,7 +181,7 @@ public final class NimbusClientSupport {
final String signingInput = composeSigningInput(header,payload);
/*
- * A null provider is supplied, as a result the preferred provider that supports the specified
+ * A null JCA provider is supplied, as a result the preferred provider that supports the specified
* algorithm will be used.
*/
final byte[] hmac = HMAC.compute(getJCAAlgorithmName(algorithm), secret,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list