[java-identity-provider] 04/06: IDP-2069 Null handling

Rod Widdowson rdw at steadingsoftware.com
Fri Feb 10 13:50:21 UTC 2023


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=60d464d26b8e0d625f9a988cfde70203e54b2024

commit 60d464d26b8e0d625f9a988cfde70203e54b2024
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu Feb 9 16:26:30 2023 +0000

    IDP-2069 Null handling
    
    https://shibboleth.atlassian.net/browse/IDP-2069
    
    Clean idp-authn-api (but not entirely)
    One API (AbstractCredentialValidator) changes from @nonnull to @nullable
---
 .../idp/authn/AbstractCredentialValidator.java     | 12 ++++++--
 .../idp/authn/AbstractExtractionAction.java        |  8 +++--
 .../AbstractSubjectCanonicalizationAction.java     |  1 +
 ...bstractUsernamePasswordCredentialValidator.java |  2 ++
 .../idp/authn/AuthenticationFlowDescriptor.java    |  2 +-
 .../idp/authn/ExternalAuthentication.java          |  5 ++--
 .../authn/MultiFactorAuthenticationTransition.java |  5 ++--
 .../config/LDAPAuthenticationFactoryBean.java      | 34 +++++++++++++++++-----
 .../idp/authn/context/SubjectContext.java          |  4 ++-
 .../idp/authn/duo/BasicDuoIntegration.java         | 10 ++++++-
 .../principal/AbstractPrincipalSerializer.java     |  1 +
 .../principal/GenericPrincipalSerializer.java      |  5 +++-
 .../principal/ProxyAuthenticationPrincipal.java    |  4 ++-
 .../authn/principal/SimplePrincipalSerializer.java |  8 +++--
 14 files changed, 78 insertions(+), 23 deletions(-)

diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
index 35d12ae50..46406db52 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
@@ -90,7 +90,15 @@ public abstract class AbstractCredentialValidator extends AbstractIdentifiedInit
     @Override
     @Nonnull @NonnullElements @Unmodifiable @NotLive public <T extends Principal> Set<T> getSupportedPrincipals(
             @Nonnull final Class<T> c) {
-        return customPrincipals != null ? customPrincipals.getPrincipals(c) : CollectionSupport.emptySet();
+        final Subject localCopy = customPrincipals;
+        if (localCopy == null) {
+            return CollectionSupport.emptySet();
+        }
+        final Set<T> result = localCopy.getPrincipals(c);
+        if (result == null) {
+            return CollectionSupport.emptySet();
+        }
+        return result;
     }
     
     /**
@@ -103,7 +111,7 @@ public abstract class AbstractCredentialValidator extends AbstractIdentifiedInit
         checkSetterPreconditions();
         
         if (principals != null) {
-            final Collection<Principal> copy = Set.copyOf(principals);
+            final Collection<Principal> copy = CollectionSupport.copyToSet(principals);
             if (!copy.isEmpty()) {
                 customPrincipals = new Subject();
                 customPrincipals.getPrincipals().addAll(copy);
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractExtractionAction.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractExtractionAction.java
index 744523005..a484cbe87 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractExtractionAction.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractExtractionAction.java
@@ -124,7 +124,11 @@ public abstract class AbstractExtractionAction extends AbstractAuthenticationAct
      * 
      * @return  the result of applying the expressions
      */
-    @Nonnull @NotEmpty protected String applyTransforms(@Nonnull @NotEmpty final String input) {
+    @Nullable @NotEmpty protected String applyTransforms(@Nullable final String input) {
+        
+        if (input == null) {
+            return null;
+        }
         
         String s = input;
         
@@ -155,7 +159,7 @@ public abstract class AbstractExtractionAction extends AbstractAuthenticationAct
                 log.debug("{} Result of replacement is '{}'", getLogPrefix(), s);
             }
         }
-        
+
         return s;
     }
 
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractSubjectCanonicalizationAction.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractSubjectCanonicalizationAction.java
index 55ab2b7f8..10d71389b 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractSubjectCanonicalizationAction.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractSubjectCanonicalizationAction.java
@@ -247,6 +247,7 @@ public abstract class AbstractSubjectCanonicalizationAction
             }
         }
         
+        assert s != null;
         return s;
     }
     
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractUsernamePasswordCredentialValidator.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractUsernamePasswordCredentialValidator.java
index d16ff8eb3..bdff45b39 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractUsernamePasswordCredentialValidator.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractUsernamePasswordCredentialValidator.java
@@ -304,6 +304,7 @@ public abstract class AbstractUsernamePasswordCredentialValidator extends Abstra
         }
         
         if (transforms.isEmpty()) {
+            assert s != null;
             return s;
         }
         
@@ -318,6 +319,7 @@ public abstract class AbstractUsernamePasswordCredentialValidator extends Abstra
             }
         }
         
+        assert s != null;
         return s;
     }
 
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
index 31b314465..c09f66175 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
@@ -561,7 +561,7 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
             supportedPrincipals.getPrincipals().clear();
             
             stringBasedPrincipals.forEach(v -> {
-                assert principalServiceManager != null;
+                assert principalServiceManager != null && v != null;
                 final Principal p = principalServiceManager.principalFromString(v);
                 if (p != null) {
                     supportedPrincipals.getPrincipals().add(p);
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/ExternalAuthentication.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/ExternalAuthentication.java
index 0d03f031d..b620b683d 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/ExternalAuthentication.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/ExternalAuthentication.java
@@ -139,8 +139,9 @@ public abstract class ExternalAuthentication {
         url.append(baseLocation.indexOf('?') == -1 ? '?' : '&');
         url.append(CONVERSATION_KEY).append('=').append(
                 UrlEscapers.urlFormParameterEscaper().escape(conversationValue));
-        
-        return url.toString();
+        final String result = url.toString();
+        assert result != null;
+        return result;
     }
     
     /**
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/MultiFactorAuthenticationTransition.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/MultiFactorAuthenticationTransition.java
index 9c8d5c311..daf0a3e82 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/MultiFactorAuthenticationTransition.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/MultiFactorAuthenticationTransition.java
@@ -66,8 +66,9 @@ public class MultiFactorAuthenticationTransition {
      * @return flow determination strategy
      */
     @Nonnull public Function<ProfileRequestContext,String> getNextFlowStrategy(@Nonnull @NotEmpty final String event) {
-        if (nextFlowStrategyMap.containsKey(event)) {
-            return nextFlowStrategyMap.get(event);
+        final Function<ProfileRequestContext,String>  result = nextFlowStrategyMap.get(event); 
+        if (result != null) {
+            return result;
         }
         return FunctionSupport.constant(null);
     }
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
index 448d5bcb4..d086ce235 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
@@ -26,6 +26,7 @@ import javax.annotation.Nullable;
 import com.google.common.base.MoreObjects;
 import net.shibboleth.idp.authn.TemplateSearchDnResolver;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 import org.apache.velocity.app.VelocityEngine;
@@ -620,6 +621,14 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
     public void setUserFilter(final String filter) {
         userFilter = filter;
     }
+    
+    /**
+     *Get the {@link #userFilter}.
+     * @return the userfilter
+     */
+    @Nonnull private String getUserFilter() {
+        return Constraint.isNotNull(userFilter, "p:userFilter must be specified");
+    }
 
     /** Set {@link #subtreeSearch}.
      * @param b what to set
@@ -649,6 +658,15 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
         velocityEngine = engine;
     }
 
+    /**
+     *Get the {@link #velocityEngine}.
+     * @return the velocityEngine
+     */
+    @Nonnull private VelocityEngine getVelocityEngine() {
+        return Constraint.isNotNull(velocityEngine, "p:velocityEngine must be specified");
+    }
+
+
     /** Set {@link #bindDn}.
      * @param dn what to set
      */
@@ -909,16 +927,16 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
         switch (authenticatorType) {
         case BIND_SEARCH:
             if (disablePooling) {
-                final TemplateSearchDnResolver bindSearchDnResolver = new TemplateSearchDnResolver(velocityEngine,
-                        userFilter);
+                final TemplateSearchDnResolver bindSearchDnResolver = new TemplateSearchDnResolver(getVelocityEngine(),
+                        getUserFilter());
                 bindSearchDnResolver.setBaseDn(baseDn);
                 bindSearchDnResolver.setSubtreeSearch(subtreeSearch);
                 bindSearchDnResolver.setConnectionFactory(new DefaultConnectionFactory(createConnectionConfig(
                         new BindConnectionInitializer(bindDn, new Credential(bindDnCredential)))));
                 authenticator.setDnResolver(bindSearchDnResolver);
             } else {
-                final TemplateSearchDnResolver bindSearchDnResolver = new TemplateSearchDnResolver(velocityEngine,
-                        userFilter);
+                final TemplateSearchDnResolver bindSearchDnResolver = new TemplateSearchDnResolver(getVelocityEngine(),
+                        getUserFilter());
                 bindSearchDnResolver.setBaseDn(baseDn);
                 bindSearchDnResolver.setSubtreeSearch(subtreeSearch);
                 bindSearchDnResolver.setConnectionFactory(createPooledConnectionFactory("dn-search-pool",
@@ -939,15 +957,15 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
             break;
         case ANON_SEARCH:
             if (disablePooling) {
-                final TemplateSearchDnResolver anonSearchDnResolver = new TemplateSearchDnResolver(velocityEngine,
-                        userFilter);
+                final TemplateSearchDnResolver anonSearchDnResolver = new TemplateSearchDnResolver(getVelocityEngine(),
+                        getUserFilter());
                 anonSearchDnResolver.setBaseDn(baseDn);
                 anonSearchDnResolver.setSubtreeSearch(subtreeSearch);
                 anonSearchDnResolver.setConnectionFactory(new DefaultConnectionFactory(createConnectionConfig()));
                 authenticator.setDnResolver(anonSearchDnResolver);
             } else {
-                final TemplateSearchDnResolver anonSearchDnResolver = new TemplateSearchDnResolver(velocityEngine,
-                        userFilter);
+                final TemplateSearchDnResolver anonSearchDnResolver = new TemplateSearchDnResolver(getVelocityEngine(),
+                        getUserFilter());
                 anonSearchDnResolver.setBaseDn(baseDn);
                 anonSearchDnResolver.setSubtreeSearch(subtreeSearch);
                 anonSearchDnResolver.setConnectionFactory(createPooledConnectionFactory("dn-search-pool",
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/SubjectContext.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/SubjectContext.java
index e206c8c5f..2a12223ca 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/SubjectContext.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/SubjectContext.java
@@ -31,6 +31,7 @@ import net.shibboleth.shared.annotation.constraint.Live;
 import net.shibboleth.shared.annotation.constraint.NonnullElements;
 import net.shibboleth.shared.annotation.constraint.NotLive;
 import net.shibboleth.shared.annotation.constraint.Unmodifiable;
+import net.shibboleth.shared.collection.CollectionSupport;
 
 import org.opensaml.messaging.context.BaseContext;
 
@@ -133,7 +134,8 @@ public final class SubjectContext extends BaseContext {
         return authenticationResults.values()
                 .stream()
                 .map(AuthenticationResult::getSubject)
-                .collect(Collectors.toUnmodifiableList());
+                .collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableList())).
+                get();
     }
     
 }
\ No newline at end of file
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/duo/BasicDuoIntegration.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/duo/BasicDuoIntegration.java
index b7161aa1d..7c578c9c6 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/duo/BasicDuoIntegration.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/duo/BasicDuoIntegration.java
@@ -63,6 +63,8 @@ public class BasicDuoIntegration extends AbstractInitializableComponent implemen
 
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getAPIHost() {
+        checkComponentActive();
+        assert apiHost != null;
         return apiHost;
     }
     
@@ -93,6 +95,8 @@ public class BasicDuoIntegration extends AbstractInitializableComponent implemen
 
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getIntegrationKey() {
+        checkComponentActive();
+        assert integrationKey != null;
         return integrationKey;
     }
     
@@ -108,6 +112,8 @@ public class BasicDuoIntegration extends AbstractInitializableComponent implemen
 
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getSecretKey() {
+        checkComponentActive();
+        assert secretKey != null;
         return secretKey;
     }
     
@@ -124,7 +130,9 @@ public class BasicDuoIntegration extends AbstractInitializableComponent implemen
     /** {@inheritDoc} */
     @Nonnull @NonnullElements @Unmodifiable
     public <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
-        return supportedPrincipals.getPrincipals(c);
+        final Set<T> result = supportedPrincipals.getPrincipals(c);
+        assert result != null;
+        return result;
     }
     
     /**
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/AbstractPrincipalSerializer.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/AbstractPrincipalSerializer.java
index b3199e19d..40ce081c9 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/AbstractPrincipalSerializer.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/AbstractPrincipalSerializer.java
@@ -50,6 +50,7 @@ public abstract class AbstractPrincipalSerializer<Type> extends AbstractInitiali
      */
     public AbstractPrincipalSerializer() {
         final JsonProvider provider = JsonProvider.provider();
+        assert provider != null;
         generatorFactory = provider.createGeneratorFactory(null);
         readerFactory = provider.createReaderFactory(null);
     }
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/GenericPrincipalSerializer.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/GenericPrincipalSerializer.java
index 62d14f574..6619c9a00 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/GenericPrincipalSerializer.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/GenericPrincipalSerializer.java
@@ -68,6 +68,7 @@ public class GenericPrincipalSerializer extends AbstractPrincipalSerializer<Stri
     @Nonnull @NotEmpty private static final String PRINCIPAL_NAME_FIELD = "nam";
 
     /** Pattern used to determine if input is supported. */
+    @SuppressWarnings("null")
     @Nonnull private static final Pattern JSON_PATTERN = Pattern.compile("^\\{\"typ\":.*,\"nam\":.*\\}$");
 
     /** Class logger. */
@@ -139,7 +140,9 @@ public class GenericPrincipalSerializer extends AbstractPrincipalSerializer<Stri
                 
             gen.writeEnd();
         }
-        return sink.toString();
+        final String result = sink.toString();
+        assert result != null;
+        return result;
     }
 
     /** {@inheritDoc} */
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
index af491e876..bf5b5a880 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
@@ -82,7 +82,9 @@ public class ProxyAuthenticationPrincipal implements Principal, Predicate<Profil
     
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getName() {
-        return authorities.toString();
+        final String result = authorities.toString();
+        assert result != null;
+        return result;
     }
     
     /**
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/SimplePrincipalSerializer.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/SimplePrincipalSerializer.java
index 7002b080a..651a7b4f9 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/SimplePrincipalSerializer.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/SimplePrincipalSerializer.java
@@ -95,7 +95,9 @@ public class SimplePrincipalSerializer<T extends Principal> extends AbstractPrin
                 .write(fieldName, getName(principal))
                 .writeEnd();
         }
-        return sink.toString();
+        final String result = sink.toString();
+        assert result != null;
+        return result;
     }
     
     /**
@@ -108,7 +110,9 @@ public class SimplePrincipalSerializer<T extends Principal> extends AbstractPrin
      * @throws IOException if an error occurs
      */
     @Nonnull @NotEmpty protected String getName(@Nonnull final Principal principal) throws IOException {
-        return principal.getName();
+        final String result = principal.getName();
+        assert result != null;
+        return result;
     }
 
     /** {@inheritDoc} */

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


More information about the commits mailing list