[java-idp-plugin-duo] branch main updated: Remove unnecessary synchronisation on prototype beans

Phil Smart philip.smart at jisc.ac.uk
Wed Mar 10 15:08:13 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=a5c03d7c5b471b3fad0f1a77db6a621584edea08

The following commit(s) were added to refs/heads/main by this push:
       new  a5c03d7   Remove unnecessary synchronisation on prototype beans
a5c03d7 is described below

commit a5c03d7c5b471b3fad0f1a77db6a621584edea08
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Mar 10 15:08:10 2021 +0000

    Remove unnecessary synchronisation on prototype beans
    
    Add final to strategies and factories that should not be extended.
    Add some additional ThreadSafe annotations.
    JavaDoc typo or two.
---
 .../plugin/authn/duo/AbstractDuoAuthenticationAction.java   |  4 ++--
 .../idp/plugin/authn/duo/AbstractDuoOIDCClient.java         | 13 +++++--------
 .../net/shibboleth/idp/plugin/authn/duo/URISupport.java     |  2 ++
 .../authn/duo/impl/DuoAudienceClaimLookupStrategy.java      |  2 +-
 .../plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java  |  2 +-
 .../shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java    |  3 +--
 .../authn/duo/impl/DuoUsernameClaimLookupStrategy.java      |  2 +-
 .../duo/impl/ValidateDuoTokenAuthenticationResult.java      |  7 ++++---
 .../idp/plugin/authn/duo/impl/ValidateTokenClaims.java      |  4 ++--
 .../idp/plugin/authn/duo/impl/ValidateTokenSignature.java   |  2 +-
 .../plugin/authn/duo/impl/ValidateDuoResponseStateTest.java |  2 +-
 .../plugin/authn/duo/nimbus/impl/NimbusClientFactory.java   |  2 +-
 .../plugin/authn/duo/nimbus/impl/NimbusClientSupport.java   |  2 ++
 13 files changed, 24 insertions(+), 23 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 8799c11..adb4874 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
@@ -63,7 +63,7 @@ public abstract class AbstractDuoAuthenticationAction extends AbstractAuthentica
         
     
     /** Constructor.*/
-    public AbstractDuoAuthenticationAction() {
+    protected AbstractDuoAuthenticationAction() {
         //prc -> ac -> dc
         duoContextLookupStrategy = new ChildContextLookup<>(DuoOIDCAuthenticationContext.class).
                 compose(new ChildContextLookup<>(AuthenticationContext.class));
@@ -75,7 +75,7 @@ public abstract class AbstractDuoAuthenticationAction extends AbstractAuthentica
      * 
      * @param strategy lookup strategy
      */
-    public synchronized void setDuoContextLookupStrategy(
+    public void setDuoContextLookupStrategy(
             @Nonnull final Function<ProfileRequestContext,DuoOIDCAuthenticationContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
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 9cf4f25..fed6324 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,8 +20,6 @@ package net.shibboleth.idp.plugin.authn.duo;
 import java.util.UUID;
 
 import javax.annotation.Nonnull;
-import javax.annotation.concurrent.GuardedBy;
-import javax.annotation.concurrent.ThreadSafe;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 
@@ -29,24 +27,23 @@ 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>
+ * <p>Client's are shared amongst requests - and hence possibly threads - and therefore must be thread-safe</p>
  */
- at ThreadSafe
 public abstract class AbstractDuoOIDCClient implements DuoOIDCClient{
     
     /** The client instance UUID for identification.*/
-    @GuardedBy("this") @Nonnull @NotEmpty private final String clientId;
+    @Nonnull @NotEmpty private final String clientId;
     
     /** Constructor.*/
-    public AbstractDuoOIDCClient() {
+    protected AbstractDuoOIDCClient() {
         clientId = UUID.randomUUID().toString();
     }
     
-    @Override @Nonnull public final synchronized String getClientId() {
+    @Override @Nonnull public final String getClientId() {
         return clientId;
     }
     
-    @Override @Nonnull public final synchronized DuoOIDCClientCapabilities getCapabilities() {
+    @Override @Nonnull public final DuoOIDCClientCapabilities getCapabilities() {
         return this;
     }
 
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 9987eac..a3e6e4d 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
@@ -21,10 +21,12 @@ import java.net.URI;
 import java.net.URISyntaxException;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
 
 import org.apache.http.client.utils.URIBuilder;
 
 /** URL support class.*/
+ at ThreadSafe
 public final class URISupport {
     
     /** Private constructor.*/
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
index e22bec2..2fbff21 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategy.java
@@ -36,7 +36,7 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
  * Returns null if it fails to find the clientID. Used for JWT ID Token audience claims verification.
  */
 @ThreadSafe
-public class DuoAudienceClaimLookupStrategy implements BiFunction<ProfileRequestContext, JWTClaimsSet, String> {
+public final class DuoAudienceClaimLookupStrategy implements BiFunction<ProfileRequestContext, JWTClaimsSet, String> {
 
     @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
             @Nonnull final JWTClaimsSet cliams) {
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
index 18e141c..ee889c0 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategy.java
@@ -35,7 +35,7 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
  * Find the nonce from the {@link DuoAuthenticationContext}. Returns null if not found.
  */
 @ThreadSafe
-public class DuoNonceClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
+public final class DuoNonceClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
 
     @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
             @Nonnull final JWTClaimsSet cliams) {
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 920958b..f97b0c3 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
@@ -125,8 +125,7 @@ public final class DuoSupport {
         if (stateSplit.length!=2) {
             throw new DuoException("State does not contain the nonce component");
         }
-        final String nonce = stateSplit[0];
-        return nonce;
+        return stateSplit[0];
     }
 
 }
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
index a7f6278..0034c54 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategy.java
@@ -35,7 +35,7 @@ import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
  * Find the authenticating principals username from the {@link DuoAuthenticationContext}. Returns null if not found.
  */
 @ThreadSafe
-public class DuoUsernameClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
+public final class DuoUsernameClaimLookupStrategy implements BiFunction<ProfileRequestContext,JWTClaimsSet, String> {
 
     @Override @Nullable public String apply(@Nonnull final ProfileRequestContext context,
             @Nonnull final JWTClaimsSet cliams) {
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 de077b3..ca350c5 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
@@ -81,7 +81,7 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
     /** Attempted username. */
     @Nullable @NotEmpty private String username;
     
-    /** Hook to map context information, often Duo factors in the Duo token, to principal collections.*/
+    /** Hook to map context information (often Duo factors in the Duo token) to principal collections.*/
     @Nullable private Function<ProfileRequestContext,Collection<Principal>> contextToPrincipalMappingStrategy; 
     
     /**
@@ -90,7 +90,7 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
      * 
      * @return the mapping hook
      */
-    @Nullable public synchronized Function<ProfileRequestContext,Collection<Principal>> 
+    @Nullable public Function<ProfileRequestContext,Collection<Principal>> 
                 getContextToPrincipalMappingStrategy() {
         return contextToPrincipalMappingStrategy;
     }
@@ -101,9 +101,10 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractValidationActi
      * 
      * @param hook principal mapping hook
      */
-    public synchronized void setContextToPrincipalMappingStrategy(@Nullable final 
+    public void setContextToPrincipalMappingStrategy(@Nullable final 
             Function<ProfileRequestContext,Collection<Principal>> hook) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
         contextToPrincipalMappingStrategy = hook;
     }
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 501e6b7..bc98680 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
@@ -104,7 +104,7 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
      * @param hook cleanup hook
      * 
      */
-    public synchronized void setCleanupHook(@Nullable final Consumer<ProfileRequestContext> hook) {
+    public void setCleanupHook(@Nullable final Consumer<ProfileRequestContext> hook) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
@@ -116,7 +116,7 @@ public class ValidateTokenClaims extends AbstractDuoAuthenticationAction {
      * 
      * @param validator the claims validator.
      */
-    public synchronized void setClaimsValidator(
+    public void setClaimsValidator(
             @Nonnull final JWTClaimsValidation validator) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
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 509b8ae..9ef614a 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,7 +105,7 @@ public class ValidateTokenSignature extends AbstractDuoAuthenticationAction {
      * 
      * @param algo the JWS signature algorithm.
      */
-    public synchronized void setSignatureAlgorithm(@Nonnull final JWSAlgorithm algo) {
+    public void setSignatureAlgorithm(@Nonnull final JWSAlgorithm algo) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
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 fce5292..47e10b1 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
@@ -56,7 +56,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
         assertNull(event);
     }
     
-    /* Test Duo 2FA response validation, falied.*/
+    /* Test Duo 2FA response validation, failed.*/
     @Test 
     public void testExecuteFailed() throws ComponentInitializationException {
         addDuoContext();
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 7d89ae1..9af8e4e 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
@@ -47,7 +47,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * </p>
  */
 @ThreadSafe
-public class NimbusClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory {
+public final class NimbusClientFactory extends AbstractInitializableComponent implements DuoOIDCClientFactory {
 
     /** HttpClient for contacting Duo. */
     @GuardedBy("this") @NonnullAfterInit private HttpClient httpClient;
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
index cb6834d..6fc3522 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClientSupport.java
@@ -23,6 +23,7 @@ import java.time.Duration;
 import java.util.Date;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
 
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWSAlgorithm;
@@ -37,6 +38,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 /** 
  * Helper methods for working with Duo using Nimbus.
  */
+ at ThreadSafe
 public final class NimbusClientSupport {
     
     /** private constructor.*/

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list