[java-idp-plugin-duo] branch main updated: JDUO-34 - Improve thread safety guarantees
Phil Smart
philip.smart at jisc.ac.uk
Fri Mar 5 12:18:49 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=b2e409ca240e0fc9ed125431061882d7fad62570
The following commit(s) were added to refs/heads/main by this push:
new b2e409c JDUO-34 - Improve thread safety guarantees
b2e409c is described below
commit b2e409ca240e0fc9ed125431061882d7fad62570
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Mar 5 12:18:46 2021 +0000
JDUO-34 - Improve thread safety guarantees
- Improve locking/synchronisation
- Some cleanup
- Some JavaDoc improvements
https://issues.shibboleth.net/jira/browse/JDUO-34
---
.../authn/duo/AbstractDuoAuthenticationAction.java | 3 +-
.../plugin/authn/duo/AbstractDuoOIDCClient.java | 13 +-
.../authn/duo/DefaultDuoOIDCIntegration.java | 5 +-
.../idp/plugin/authn/duo/DuoOIDCAuthAPI.java | 4 +
.../idp/plugin/authn/duo/DuoOIDCClient.java | 2 +-
.../authn/duo/DuoOIDCClientCapabilities.java | 2 +-
.../idp/plugin/authn/duo/DuoOIDCClientFactory.java | 4 +-
.../plugin/authn/duo/DuoOIDCClientRegistry.java | 2 +
.../idp/plugin/authn/duo/DuoOIDCIntegration.java | 5 +-
.../idp/plugin/authn/duo/URISupport.java | 2 +-
.../duo/context/DuoOIDCAuthenticationContext.java | 13 +-
.../idp/plugin/authn/duo/model/DuoHealthCheck.java | 133 ++++++++++++++-------
.../authn/duo/model/DuoHealthCheckResponse.java | 26 ++++
.../duo/impl/DefaultDuoOIDCClientRegistry.java | 22 ++--
.../duo/impl/DuoIssuerClaimLookupStrategy.java | 30 ++++-
.../authn/duo/impl/DuoOIDCAuthnController.java | 25 +++-
.../idp/plugin/authn/duo/impl/DuoSupport.java | 2 +
.../duo/impl/PopulateDuoAuthenticationContext.java | 16 ++-
.../impl/ValidateDuoTokenAuthenticationResult.java | 5 +-
.../plugin/authn/duo/impl/ValidateTokenClaims.java | 15 +--
.../authn/duo/impl/ValidateTokenSignature.java | 4 +-
.../flows/authn/DuoOIDC/duo-oidc-authn-beans.xml | 3 +-
.../plugin/authn/duo/nimbus/impl/NimbusClient.java | 2 +
.../authn/duo/nimbus/impl/NimbusClientFactory.java | 35 +++++-
.../authn/duo/nimbus/impl/TokenResponse.java | 63 ++++++----
.../authn/duo/sdk/impl/DuoSDKClientAdaptor.java | 15 +--
.../authn/duo/sdk/impl/DuoSDKClientFactory.java | 4 +-
27 files changed, 317 insertions(+), 138 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 35802e0..8799c11 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
@@ -75,9 +75,10 @@ public abstract class AbstractDuoAuthenticationAction extends AbstractAuthentica
*
* @param strategy lookup strategy
*/
- public void setDuoContextLookupStrategy(
+ public synchronized void setDuoContextLookupStrategy(
@Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
duoContextLookupStrategy = Constraint.isNotNull(strategy, "DuoContextLookuplookup strategy cannot be null");
}
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 d63feb4..9cf4f25 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,30 +20,33 @@ package net.shibboleth.idp.plugin.authn.duo;
import java.util.UUID;
import javax.annotation.Nonnull;
-import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
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.
+ *
+ * <p>Client's are shared amongst requests and hence possibly threads, and therefore must be thread-safe</p>
*/
- at Immutable
+ at ThreadSafe
public abstract class AbstractDuoOIDCClient implements DuoOIDCClient{
/** The client instance UUID for identification.*/
- @Nonnull @NotEmpty private final String clientId;
+ @GuardedBy("this") @Nonnull @NotEmpty private final String clientId;
/** Constructor.*/
public AbstractDuoOIDCClient() {
clientId = UUID.randomUUID().toString();
}
- @Override @Nonnull public String getClientId() {
+ @Override @Nonnull public final synchronized String getClientId() {
return clientId;
}
- @Override @Nonnull public DuoOIDCClientCapabilities getCapabilities() {
+ @Override @Nonnull public final synchronized DuoOIDCClientCapabilities getCapabilities() {
return this;
}
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 f925e99..17e0803 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
@@ -47,7 +47,7 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
* Data wrapper for use with Duo OIDC integrations. Holds shared-state.
*
* <p>Despite most fields being 'effectively immutable' once published by Spring, the redirectURI is allowed
- * to change, but in a thread-safe manor.</p>
+ * to change, but in a guarded, shared-thread-safe manor.</p>
*
*/
@ThreadSafe
@@ -65,7 +65,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
/** Secret key. */
@GuardedBy("this") @NonnullAfterInit @NotEmpty private String secretKey;
- /** The redirect_uri to send the client after authorisation .*/
+ /** The used (by clients) redirect_uri to send the client after authorisation .*/
@GuardedBy("this") @Nullable private String redirectURI;
/** A statically set (pre-registered) redirectURI to send the client to after authorisation.*/
@@ -106,6 +106,7 @@ public final class DefaultDuoOIDCIntegration extends AbstractInitializableCompon
@Override
@Nonnull @NotLive @Unmodifiable public synchronized Set<String> getAllowedOrigins() {
+ //set is unmodifiable and string is immutable - so not live.
return Collections.unmodifiableSet(allowedOrigins);
}
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 3d97560..0448dbd 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
@@ -18,12 +18,16 @@
package net.shibboleth.idp.plugin.authn.duo;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
/**
* Constants defined in the Duo OIDC Auth API.
*/
+ at Immutable
+ at ThreadSafe
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 9cb0638..af28ab7 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
@@ -26,7 +26,7 @@ import net.shibboleth.idp.plugin.authn.duo.model.DuoHealthCheck;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
/**
- * A client for handling Duo OIDC 2FA interactions. Clients are required to be thread-safe.
+ * A client for handling Duo OIDC 2FA interactions. Clients are shared and required to be thread-safe.
*/
public interface DuoOIDCClient extends DuoOIDCClientCapabilities{
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 c19718c..cf42550 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
@@ -26,7 +26,7 @@ public interface DuoOIDCClientCapabilities {
* <p>Does this client support the OIDC nonce parameter.</p>
*
* <p>If the client does support a nonce, it <b>must</b> be included by the client in the authorisation request URL,
- * where it must then be returned by the OP in the id_token as part of the 2FA result.</p>
+ * where it must then be returned by the provider in the id_token as part of the 2FA result.</p>
*
* @return true iff the client supports the nonce parameter, false otherwise.
*/
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 64ad8d9..d86e5d9 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
@@ -20,13 +20,15 @@ package net.shibboleth.idp.plugin.authn.duo;
import javax.annotation.Nonnull;
/**
- * Abstract factory for creating {@link DuoOIDCClient} instances.
+ * Abstract factory for creating singleton {@link DuoOIDCClient} instances.
*/
public interface DuoOIDCClientFactory {
/**
* Create an {@link DuoOIDCClient} instance from the supplied integration.
*
+ * <p>The client should be fully initialised and safely published.</p>
+ *
* @param integration the duo integration used to instantiate the client.
*
* @return the created Duo client, never {@code null}.
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 92ee9b1..b3194eb 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
@@ -21,6 +21,8 @@ import javax.annotation.Nonnull;
/**
* A registry of {@link DuoOIDCClient}s for {@link DuoOIDCIntegration}s.
+ *
+ * <p>The registry *must* be thread-safe.</p>
*/
public interface DuoOIDCClientRegistry {
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 9b1c357..72fba30 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
@@ -30,7 +30,7 @@ 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.
+ * OIDC metadata, as that is not supported by Duo.
*/
public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
@@ -58,7 +58,8 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
/**
* 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.
+ * @return a set of unmodifiable allowed origins.
+ * Never {@literal null} but could be empty.
*/
@Nonnull @NotLive @Unmodifiable Set<String> getAllowedOrigins();
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
index 143dca2..9987eac 100644
--- 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
@@ -45,7 +45,7 @@ public final class URISupport {
*
* @throws URISyntaxException if the URI can not be constructed.
*/
- @Nonnull public static URI buildURIIgnoreDefaultPorts(@Nonnull final String scheme,
+ @Nonnull public static final URI buildURIIgnoreDefaultPorts(@Nonnull final String scheme,
@Nonnull final String host, @Nonnull final int port,
@Nonnull final String path) throws URISyntaxException {
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 ec69e0c..386953a 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
@@ -19,6 +19,7 @@ package net.shibboleth.idp.plugin.authn.duo.context;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.NotThreadSafe;
import org.opensaml.messaging.context.BaseContext;
@@ -31,10 +32,13 @@ import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
/**
* <p>Mutable Context that carries the Duo integration, request-response state, authorization code,
* and Duo authentication result token.</p>
+ *
+ * <p>As with other context classes, holds state, is not thread-safe and should be thread-confined.</p>
*
* @parent {@link AuthenticationContext}
* @added After extracting the Duo integration for the given authentication request.
*/
+ at NotThreadSafe
public final class DuoOIDCAuthenticationContext extends BaseContext {
/** Username. */
@@ -49,20 +53,20 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
/** A randomly generated 32 character minimum String returned in the Duo 2FA authorization response.*/
@Nullable private String responseState;
- /** String value used to associate a Client session with an ID Token, and to mitigate replay attacks.*/
+ /** String value used to associate a Client session with an ID Token to mitigate replay attacks.*/
@Nullable private String nonce;
/** The authorization code return from the Duo authorization request.*/
@Nullable private String authCode;
- /** The JWT token received from Duo as a result of 2FA.*/
+ /** The JWT token received from Duo as a result of 2FA. Token *must* be signed.*/
@Nullable private JWT authToken;
/** 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 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.
*/
@@ -249,7 +253,8 @@ public final class DuoOIDCAuthenticationContext extends BaseContext {
* @param duoIntegration the integration
* @return this context
*/
- public DuoOIDCAuthenticationContext setIntegration(@Nullable final DuoOIDCIntegration duoIntegration) {
+ @Nonnull public DuoOIDCAuthenticationContext setIntegration(
+ @Nullable final DuoOIDCIntegration duoIntegration) {
integration = duoIntegration;
return this;
}
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 bacfcbb..a8a7884 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
@@ -20,6 +20,7 @@ package net.shibboleth.idp.plugin.authn.duo.model;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.ThreadSafe;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -37,6 +38,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
* <p>Is immutable, can only be built using the builder and not changed thereafter.</p>
*/
@Immutable
+ at ThreadSafe
@JsonDeserialize(builder=DuoHealthCheck.Builder.class)
@JsonIgnoreProperties(ignoreUnknown = true)
public final class DuoHealthCheck {
@@ -58,6 +60,21 @@ public final class DuoHealthCheck {
/** The error response detailed message.*/
@Nullable private final String messageDetail;
+
+
+ /**
+ * Private constructor, can only be called by this builder.
+ *
+ * @param builder the builder to build the instance with
+ */
+ private DuoHealthCheck(final Builder builder) {
+ this.status = builder.status;
+ this.response = builder.response;
+ this.code = builder.code;
+ this.timestamp = builder.timestamp;
+ this.message = builder.message;
+ this.messageDetail = builder.messageDetail;
+ }
/**
@@ -122,21 +139,6 @@ public final class DuoHealthCheck {
status, response, code, timestamp, message, messageDetail);
}
-
-
- /**
- * Private constructor, can only be called by this builder.
- *
- * @param builder the builder to build the instance with
- */
- private DuoHealthCheck(Builder builder) {
- this.status = builder.status;
- this.response = builder.response;
- this.code = builder.code;
- this.timestamp = builder.timestamp;
- this.message = builder.message;
- this.messageDetail = builder.messageDetail;
- }
/**
* Creates builder to build {@link DuoHealthCheck}.
@@ -147,23 +149,67 @@ public final class DuoHealthCheck {
return new Builder();
}
-
+ /** Status builder.*/
public interface IStatusStage {
- public IBuildStage withStatus(String status);
+
+ /**
+ * Set the status.
+ *
+ * @param status the status
+ * @return the builder.
+ */
+ public IBuildStage withStatus(final String status);
}
-
+ /** Required fields builder.*/
public interface IBuildStage {
- public IBuildStage withResponse(DuoHealthCheckResponse response);
-
- public IBuildStage withCode(String code);
-
- public IBuildStage withTimestamp(String timestamp);
-
- public IBuildStage withMessage(String message);
-
- public IBuildStage withMessageDetail(String messageDetail);
-
+
+ /**
+ * Set the response.
+ *
+ * @param response the response.
+ *
+ * @return the builder.
+ */
+ public IBuildStage withResponse(final DuoHealthCheckResponse response);
+
+ /**
+ * Set the code.
+ *
+ * @param code the code
+ * @return the builder.
+ */
+ public IBuildStage withCode(final String code);
+
+ /**
+ * Set the timestamp.
+ *
+ * @param timestamp the timestamp
+ * @return the builder.
+ */
+ public IBuildStage withTimestamp(final String timestamp);
+
+ /**
+ * Set the message.
+ *
+ * @param message the message
+ * @return the builder.
+ */
+ public IBuildStage withMessage(final String message);
+
+ /**
+ * Set the messageDetail.
+ *
+ * @param messageDetail the messageDetail
+ * @return the builder.
+ */
+ public IBuildStage withMessageDetail(final String messageDetail);
+
+ /**
+ * Build the health check response.
+ *
+ * @return the health check response.
+ */
public DuoHealthCheck build();
}
@@ -173,16 +219,23 @@ public final class DuoHealthCheck {
@JsonPOJOBuilder(buildMethodName = "build",withPrefix = "with")
@JsonIgnoreProperties(ignoreUnknown = true)
public static final class Builder implements IStatusStage, IBuildStage {
+
+ /** The status.*/
private String status;
+ /** The response.*/
private DuoHealthCheckResponse response;
+ /** The code.*/
private String code;
+ /** The timestamp.*/
private String timestamp;
+ /** The message.*/
private String message;
+ /** The message detail.*/
private String messageDetail;
private Builder() {
@@ -190,43 +243,43 @@ public final class DuoHealthCheck {
@Override
@JsonProperty("stat")
- public IBuildStage withStatus(String status) {
- this.status = status;
+ public IBuildStage withStatus(final String stat) {
+ this.status = stat;
return this;
}
@Override
@JsonProperty("response")
- public IBuildStage withResponse(DuoHealthCheckResponse response) {
- this.response = response;
+ public IBuildStage withResponse(final DuoHealthCheckResponse resp) {
+ this.response = resp;
return this;
}
@Override
@JsonProperty("code")
- public IBuildStage withCode(String code) {
- this.code = code;
+ public IBuildStage withCode(final String codeIn) {
+ this.code = codeIn;
return this;
}
@Override
@JsonProperty("timestamp")
- public IBuildStage withTimestamp(String timestamp) {
- this.timestamp = timestamp;
+ public IBuildStage withTimestamp(final String timestampIn) {
+ this.timestamp = timestampIn;
return this;
}
@Override
@JsonProperty("message")
- public IBuildStage withMessage(String message) {
- this.message = message;
+ public IBuildStage withMessage(final String msg) {
+ this.message = msg;
return this;
}
@Override
@JsonProperty("message_detail")
- public IBuildStage withMessageDetail(String messageDetail) {
- this.messageDetail = messageDetail;
+ public IBuildStage withMessageDetail(final String msgDetail) {
+ this.messageDetail = msgDetail;
return this;
}
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 61d780f..30784ad 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
@@ -1,7 +1,26 @@
+/*
+ * 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.model;
import javax.annotation.Nonnull;
import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.ThreadSafe;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -10,12 +29,19 @@ import com.fasterxml.jackson.annotation.JsonProperty;
* Represents part of the {@link DuoHealthCheck} response.
*/
@Immutable
+ at ThreadSafe
@JsonIgnoreProperties(ignoreUnknown = true)
public final class DuoHealthCheckResponse {
/** The response timestamp in seconds since Unix EPOCH.*/
@Nonnull private final Integer timestamp;
+ /**
+ *
+ * Constructor.
+ *
+ * @param time the timestamp.
+ */
public DuoHealthCheckResponse(@Nonnull @JsonProperty("timestamp") final Integer time) {
timestamp = time;
}
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 0b08596..a5a6aaa 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
@@ -47,21 +47,23 @@ 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} singleton instance.</p>
+ * or existing {@link DuoOIDCClient} (assumed thread-safe) singleton instance.</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>Clients are created and registered one-to-one against a {@link DuoOIDCIntegration}. Once created, the same client
+ * instance is reused for the same given Duo 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>The {@link DuoOIDCIntegration} is the 'key' to the map holding the clients. It therefore should decide its
+ * own 'business key' using the {@link #equals(Object)} and {@link #hashCode()} method appropriately. For example,
+ * the {@link DefaultDuoOIDCIntegration} uses the clientID as its key. The key should be immutable.</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. The client is properly published once created.</p>
+ * be created for the same integration. The client is safely published by the map.</p>
+ *
+ * <p>The registry itself is effectively immutable once published by spring.</p>
*
*/
@ThreadSafe
@@ -82,6 +84,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
/** Constructor.*/
public DefaultDuoOIDCClientRegistry() {
+ //initial capacity is 1, as that is the most likely number ever to be created
clientRegistry = new ConcurrentHashMap<DuoOIDCIntegration, DuoOIDCClient>(1);
clientRegistryMappingFunction = new CreateNewClientMappingFunction();
}
@@ -93,6 +96,8 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
*/
public synchronized void setClientFactory(@Nonnull final DuoOIDCClientFactory factory) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
clientFactory = Constraint.isNotNull(factory, "Duo client factory can not be null");
}
@@ -111,7 +116,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
Constraint.isNotNull(integration, "Duo integration can not be null");
try {
- //this is an atomic call, avoiding the need to manually synchronise here e.g. two clients should never
+ //this is an atomic call, avoiding the need to externally 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 '{}'",
@@ -136,6 +141,7 @@ public class DefaultDuoOIDCClientRegistry extends AbstractIdentifiableInitializa
* throws a {@link DuoClientInitializationException} if the factory can not create the client. This function
* executes within the concurrent hashmap lock.
*/
+ @ThreadSafe
private class CreateNewClientMappingFunction implements Function<DuoOIDCIntegration, DuoOIDCClient> {
/** Class logger. */
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 7cd482b..b15f31e 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
@@ -21,6 +21,8 @@ import java.util.function.BiFunction;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -33,11 +35,13 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
- * Find the issuer from the {@link DuoOIDCIntegration}. Returns null if not found.
+ * Find the issuer from the {@link DuoOIDCIntegration}. An issuer contains the scheme, host, and optionally, port
+ * and path components that identify the id_token issuer. 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>
+ * <p>Is effectively immutable if {@link #setIssuerPath(String)} is only be called during initialisation e.g.
+ * by spring, otherwise it can mutate, but in thread-safe way.</p>
*/
+ at ThreadSafe
public class DuoIssuerClaimLookupStrategy implements BiFunction<ProfileRequestContext, JWTClaimsSet, String> {
/** HTTPS scheme protocol.*/
@@ -47,16 +51,30 @@ public class DuoIssuerClaimLookupStrategy implements BiFunction<ProfileRequestCo
@Nonnull @NotEmpty public static final String DEFAULT_ISSUER_PATH = "/oauth/v1/token";
/** The URL path component of the issuer.*/
- @Nonnull @NotEmpty private String issuerPath;
+ @Nonnull @NotEmpty @GuardedBy("this") private String issuerPath;
+
+ /** Constructor.*/
+ public DuoIssuerClaimLookupStrategy() {
+ issuerPath = DEFAULT_ISSUER_PATH;
+ }
/**
* Sets the issuer URL path component.
*
* @param path the issuer path
*/
- public void setIssuerPath(@Nonnull @NotEmpty final String path) {
+ public synchronized void setIssuerPath(@Nonnull @NotEmpty final String path) {
issuerPath = Constraint.isNotEmpty(path, "Issuer URL path cannot be null or empty");
}
+
+ /**
+ * Internal, synchronized, method for returning the issuerPath.
+ *
+ * @return the issuer path.
+ */
+ private synchronized String getIssuerPath() {
+ return issuerPath;
+ }
/** {@inheritDoc} */
@Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
@@ -75,7 +93,7 @@ public class DuoIssuerClaimLookupStrategy implements BiFunction<ProfileRequestCo
if (duoIntegration == null) {
return null;
}
- return HTTPS+duoIntegration.getAPIHost()+issuerPath;
+ return HTTPS+duoIntegration.getAPIHost()+getIssuerPath();
}
}
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 cf40c5c..1f08320 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
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -54,8 +55,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
*
* <p>The controller initiates the Duo OIDC authorization code grant flow and accepts the authorization
* code response.</p>
+ *
+ * <p>Is effectively immutable once published by Spring. Is a thread-safe singleton.</p>
*
*/
+ at ThreadSafe
@Controller
@RequestMapping("%{idp.duo.oidc.externalAuthnPath:/Authn/Duo/2FA}")
public class DuoOIDCAuthnController extends AbstractInitializableComponent{
@@ -70,8 +74,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
@Nonnull @NotEmpty public static final String DUO_CODE_PARAMETER = "duo_code";
/** The name of the Http parameter that stores the state value.*/
- @Nonnull @NotEmpty public static final String STATE_PARAMETER = "state";
-
+ @Nonnull @NotEmpty public static final String STATE_PARAMETER = "state";
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoOIDCAuthnController.class);
@@ -91,12 +94,22 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
*
* @param strategy lookup strategy
*/
- public void setDuoContextLookupStrategy(
+ public synchronized void setDuoContextLookupStrategy(
@Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
duoContextLookupStrategy = Constraint.isNotNull(strategy, "DuoContext lookup strategy cannot be null");
}
+ /**
+ * Internally synchronized method for accessing the Duo context lookup strategy.
+ *
+ * @return the duo context lookup strategy.
+ */
+ private synchronized Function<ProfileRequestContext,DuoOIDCAuthenticationContext> getDuoContextLookupStrategy() {
+ return duoContextLookupStrategy;
+ }
+
/**
* 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.
@@ -114,7 +127,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
final String key = ExternalAuthentication.startExternalAuthentication(httpRequest);
final ProfileRequestContext prc = ExternalAuthentication.getProfileRequestContext(key, httpRequest);
- final DuoOIDCAuthenticationContext duoContext = duoContextLookupStrategy.apply(prc);
+ final DuoOIDCAuthenticationContext duoContext = getDuoContextLookupStrategy().apply(prc);
if (duoContext == null) {
log.error("No Duo context to use in initiating a Duo 2FA request");
httpRequest.setAttribute(ExternalAuthentication.AUTHENTICATION_ERROR_KEY, EventIds.INVALID_PROFILE_CTX);
@@ -176,7 +189,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
final String code = httpRequest.getParameter(CODE_PARAMETER);
final String state = httpRequest.getParameter(STATE_PARAMETER);
- //if duo's webSDK becomes OAuth2.0 complaint again, remove this
+ //if duo's webSDK becomes OAuth2.0 complaint again, remove this code path
final String duoCode = httpRequest.getParameter(DUO_CODE_PARAMETER);
if (state == null || (code == null && duoCode == null)) {
@@ -199,7 +212,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
final ProfileRequestContext prc = ExternalAuthentication.getProfileRequestContext(key, httpRequest);
- final DuoOIDCAuthenticationContext duoContext = duoContextLookupStrategy.apply(prc);
+ final DuoOIDCAuthenticationContext duoContext = getDuoContextLookupStrategy().apply(prc);
if (duoContext == null) {
log.error("No Duo authentication context to store the Duo 2FA response");
httpRequest.setAttribute(ExternalAuthentication.AUTHENTICATION_ERROR_KEY, EventIds.INVALID_PROFILE_CTX);
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
index 827e438..920958b 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
@@ -20,6 +20,7 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.security.SecureRandom;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.binary.Hex;
@@ -30,6 +31,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
/**
* Helper methods for Duo 2FA.
*/
+ at ThreadSafe
public final class DuoSupport {
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 5430ef1..27f55f8 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
@@ -95,8 +95,9 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param duoRegistry the registry
*/
- public void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
+ public synchronized void setClientRegistry(@Nonnull final DuoOIDCClientRegistry duoRegistry) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
clientRegistry = Constraint.isNotNull(duoRegistry,"DuoClient registry can not be null");
}
@@ -106,8 +107,10 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public void setUsernameLookupStrategy(@Nonnull final Function<ProfileRequestContext, String> strategy) {
+ public synchronized void setUsernameLookupStrategy(
+ @Nonnull final Function<ProfileRequestContext, String> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
usernameLookupStrategy = Constraint.isNotNull(strategy, "Username lookup strategy cannot be null");
}
@@ -118,9 +121,10 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy the creation strategy.
*/
- public void setRedirectURICreationStrategy(
+ public synchronized void setRedirectURICreationStrategy(
@Nonnull final BiFunction<HttpServletRequest, DuoOIDCIntegration, String> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
redirectURICreationStrategy = Constraint.isNotNull(strategy, "RedirectURI"
+ " creation strategy cannot be null");
@@ -131,9 +135,10 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public void setDuoContextCreationStrategy(
+ public synchronized void setDuoContextCreationStrategy(
@Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
duoAuthContextCreationStrategy = Constraint.isNotNull(strategy, "DuoAuthenticationContext"
+ " creation strategy cannot be null");
@@ -144,9 +149,10 @@ public class PopulateDuoAuthenticationContext extends AbstractAuthenticationActi
*
* @param strategy lookup strategy
*/
- public void setDuoIntegrationLookupStrategy(
+ public synchronized void setDuoIntegrationLookupStrategy(
@Nonnull final Function<ProfileRequestContext, DuoOIDCIntegration> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
duoIntegrationLookupStrategy = Constraint.isNotNull(strategy, "DuoIntegration lookup strategy cannot be null");
}
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 755e4e2..de077b3 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
@@ -90,7 +90,8 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
*
* @return the mapping hook
*/
- @Nullable public Function<ProfileRequestContext,Collection<Principal>> getContextToPrincipalMappingStrategy() {
+ @Nullable public synchronized Function<ProfileRequestContext,Collection<Principal>>
+ getContextToPrincipalMappingStrategy() {
return contextToPrincipalMappingStrategy;
}
@@ -100,7 +101,7 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
*
* @param hook principal mapping hook
*/
- public void setContextToPrincipalMappingStrategy(@Nullable final
+ public synchronized void setContextToPrincipalMappingStrategy(@Nullable final
Function<ProfileRequestContext,Collection<Principal>> hook) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
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 c2b68a9..501e6b7 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
@@ -97,24 +97,16 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
throw new ComponentInitializationException("Duo ClaimSet Validator cannot be null");
}
}
-
- /**
- * Get the cleanup hook to execute after either a successful or unsuccessful validation.
- *
- * @return cleanup hook
- */
- @Nullable public Consumer<ProfileRequestContext> getCleanupHook() {
- return cleanupHook;
- }
-
+
/**
* Set the cleanup hook to execute after either a successful or unsuccessful claims validation.
*
* @param hook cleanup hook
*
*/
- public void setCleanupHook(@Nullable final Consumer<ProfileRequestContext> hook) {
+ public synchronized void setCleanupHook(@Nullable final Consumer<ProfileRequestContext> hook) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
cleanupHook = hook;
}
@@ -127,6 +119,7 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
public synchronized void setClaimsValidator(
@Nonnull final JWTClaimsValidation validator) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
claimsValidator = Constraint.isNotNull(validator, "Claims validator cannot be null");
}
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 1cfa2f5..509b8ae 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
@@ -105,8 +105,10 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
*
* @param algo the JWS signature algorithm.
*/
- public void setSignatureAlgorithm(@Nonnull final JWSAlgorithm algo) {
+ public synchronized void setSignatureAlgorithm(@Nonnull final JWSAlgorithm algo) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
Constraint.isNotNull(algo, "Signature algorithm can not be null");
if (!SUPPORTED_SIGNATURE_FAMILY.contains(algo)) {
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 a37340e..4e7ec04 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
@@ -210,7 +210,8 @@
<bean id="shibboleth.authn.DuoOIDC.jwt.DefaultAudienceLookupStrategy"
class="net.shibboleth.idp.plugin.authn.duo.impl.DuoAudienceClaimLookupStrategy"/>
- <!-- These represent the default set of id_token claims which are **required** by OIDC -->
+ <!-- These represent the default set of id_token claims which are **required** by OIDC
+ https://openid.net/specs/openid-connect-core-1_0.html#IDToken -->
<util:set id="shibboleth.authn.DuoOIDC.DefaultRequiredOIDCClaims">
<value>iss</value>
<value>sub</value>
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 ee107b7..17e00fb 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
@@ -24,6 +24,7 @@ import java.text.ParseException;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.ThreadSafe;
import org.apache.http.HttpResponse;
@@ -55,6 +56,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* A Duo client using the Nimbus OIDC library. Can/should only be instantiated by the corresponding factory method.
*/
@ThreadSafe
+ at Immutable
final class NimbusClient extends AbstractDuoOIDCClient{
/** The only supported client assertion type.*/
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 751d82e..8c4d564 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
@@ -60,7 +60,7 @@ public class NimbusClientFactory extends AbstractInitializableComponent implemen
@Override
public DuoOIDCClient createInstance(@Nonnull final DuoOIDCIntegration integration) throws DuoClientException {
- return new NimbusClient(integration, httpClient, httpClientSecurityParameters, objectMapper);
+ return new NimbusClient(integration, getHttpClient(), getHttpClientSecurityParameters(), getObjectMapper());
}
/** {@inheritDoc} */
@@ -68,19 +68,46 @@ public class NimbusClientFactory extends AbstractInitializableComponent implemen
protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
- if (httpClient == null) {
+ if (getHttpClient() == null) {
throw new ComponentInitializationException("HttpClient cannot be null");
}
- if (objectMapper == null) {
+ if (getObjectMapper() == null) {
throw new ComponentInitializationException("ObjectMapper cannot be null");
}
}
+
+ /**
+ * Internally synchronized method for returning the HttpClient.
+ *
+ * @return the http client.
+ */
+ @NonnullAfterInit private synchronized HttpClient getHttpClient() {
+ return httpClient;
+ }
+
+ /**
+ * Internally synchronized method for returning the http client security parameters.
+ *
+ * @return the http client security parameters.
+ */
+ @Nullable private synchronized HttpClientSecurityParameters getHttpClientSecurityParameters() {
+ return httpClientSecurityParameters;
+ }
+
+ /**
+ * Internally synchronized method for returning the object mapper.
+ *
+ * @return the object mapper.
+ */
+ @NonnullAfterInit private synchronized ObjectMapper getObjectMapper() {
+ return objectMapper;
+ }
/**
* Set the {@link HttpClient} to use for contacting Duo.
*
- * @param client HttpClient
+ * @param client the http client.
*/
public synchronized void setHttpClient(@Nonnull final HttpClient client) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/TokenResponse.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/TokenResponse.java
index 213fcc7..dfcfdfc 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/TokenResponse.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/TokenResponse.java
@@ -68,6 +68,21 @@ public final class TokenResponse {
/** The scope requested by the client.*/
@Nullable private final String scope;
+ /**
+ *
+ * Constructor.
+ *
+ * @param builder the builder.
+ */
+ private TokenResponse(final Builder builder) {
+ this.idToken = builder.idToken;
+ this.accessToken = builder.accessToken;
+ this.tokenType = builder.tokenType;
+ this.refreshToken = builder.refreshToken;
+ this.expiresIn = builder.expiresIn;
+ this.scope = builder.scope;
+ }
+
/**
* Get the ID token.
*
@@ -130,14 +145,7 @@ public final class TokenResponse {
+ ", tokenType=" + tokenType + ", expiresIn=" + expiresIn + ", scope=" + scope + "]";
}
- private TokenResponse(final Builder builder) {
- this.idToken = builder.idToken;
- this.accessToken = builder.accessToken;
- this.tokenType = builder.tokenType;
- this.refreshToken = builder.refreshToken;
- this.expiresIn = builder.expiresIn;
- this.scope = builder.scope;
- }
+
/**
* Creates builder to build {@link TokenResponse}.
@@ -148,22 +156,22 @@ public final class TokenResponse {
return new Builder();
}
-
+ /** Token stage interface.*/
public interface IIdTokenStage {
public IAccessTokenStage withIdToken(final String idToken);
}
-
+ /** Access token stage interface.*/
public interface IAccessTokenStage {
public ITokenTypeStage withAccessToken(final String accessToken);
}
-
+ /** Token type stage interface.*/
public interface ITokenTypeStage {
public IBuildStage withTokenType(final String tokenType);
}
-
+ /** Build stage interface.*/
public interface IBuildStage {
public IBuildStage withRefreshToken(final String refreshToken);
@@ -180,16 +188,23 @@ public final class TokenResponse {
@JsonPOJOBuilder(buildMethodName = "build",withPrefix = "with")
@JsonIgnoreProperties(ignoreUnknown = true)
public static final class Builder implements IIdTokenStage, IAccessTokenStage, ITokenTypeStage, IBuildStage {
+
+ /** The id token.*/
private String idToken;
+ /** The access token.*/
private String accessToken;
+ /** The token type.*/
private String tokenType;
+ /** The refresh token - not supported by Duo.*/
private String refreshToken;
+ /** The expiry.*/
private Integer expiresIn;
+ /** The scope.*/
private String scope;
private Builder() {
@@ -197,43 +212,43 @@ public final class TokenResponse {
@Override
@JsonProperty("id_token")
- public IAccessTokenStage withIdToken(final String idToken) {
- this.idToken = idToken;
+ public IAccessTokenStage withIdToken(final String token) {
+ this.idToken = token;
return this;
}
@Override
@JsonProperty("access_token")
- public ITokenTypeStage withAccessToken(final String accessToken) {
- this.accessToken = accessToken;
+ public ITokenTypeStage withAccessToken(final String accessTokenIn) {
+ this.accessToken = accessTokenIn;
return this;
}
@Override
@JsonProperty("token_type")
- public IBuildStage withTokenType(final String tokenType) {
- this.tokenType = tokenType;
+ public IBuildStage withTokenType(final String type) {
+ this.tokenType = type;
return this;
}
@Override
@JsonProperty("refresh_token")
- public IBuildStage withRefreshToken(final String refreshToken) {
- this.refreshToken = refreshToken;
+ public IBuildStage withRefreshToken(final String refresh) {
+ this.refreshToken = refresh;
return this;
}
@Override
@JsonProperty("expires_in")
- public IBuildStage withExpiresIn(final Integer expiresIn) {
- this.expiresIn = expiresIn;
+ public IBuildStage withExpiresIn(final Integer expires) {
+ this.expiresIn = expires;
return this;
}
@Override
@JsonProperty("scope")
- public IBuildStage withScope(final String scope) {
- this.scope = scope;
+ public IBuildStage withScope(final String scopeIn) {
+ this.scope = scopeIn;
return this;
}
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 e052605..3ade8ad 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
@@ -25,6 +25,7 @@ import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.ThreadSafe;
import org.slf4j.Logger;
@@ -59,10 +60,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* <p>This is package private, and can/should only be instantiated by the {@link DuoSDKClientFactory}.</p>
*/
@ThreadSafe
+ at Immutable
final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
/** Class logger. */
- @Nonnull private Logger log = LoggerFactory.getLogger(DuoSDKClientAdaptor.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientAdaptor.class);
/** The wrapped Duo native client.*/
@Nonnull private final Client client;
@@ -188,13 +190,9 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
@Override
public DuoHealthCheck apply(@Nonnull final HealthCheckResponse response) {
- Integer responseTimestamp = null;
- if (response.getResponse()!=null) {
- responseTimestamp = response.getResponse().getTimestamp();
- }
return DuoHealthCheck.builder().withStatus(response.getStat()).withCode(response.getCode())
.withMessage(response.getMessage()).withMessageDetail(response.getMessage_detail())
- .withResponse(new DuoHealthCheckResponse(responseTimestamp))
+ .withResponse(new DuoHealthCheckResponse(response.getResponse().getTimestamp()))
.withTimestamp(response.getTimestamp()).build();
}
@@ -230,10 +228,7 @@ final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
log.error("Could not convert Duo Token to a Nimbus JWT Token",e);
return null;
}
- }
-
-
-
+ }
}
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 4e8178c..8ae345d 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
@@ -22,6 +22,7 @@ import java.util.List;
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;
@@ -31,7 +32,6 @@ 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;
@@ -39,7 +39,7 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
/** Abstract factory implementation for the {@link DuoSDKClientAdaptor}. */
- at ThreadSafeAfterInit
+ at ThreadSafe
public class DuoSDKClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory{
/** Class logger. */
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list