[java-identity-provider] 01/02: IDP-2069 Null handling

Rod Widdowson rdw at steadingsoftware.com
Sat Feb 25 17:01:54 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=42cfcf1d3ac527970d29e581557128fe66a7061f

commit 42cfcf1d3ac527970d29e581557128fe66a7061f
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Feb 25 16:54:04 2023 +0000

    IDP-2069 Null handling
    
    https://shibboleth.atlassian.net/browse/IDP-2069
    
    Cleanup idp-session-impl.  Tests still TBD.
---
 .../idp/session/impl/DestroySessions.java          | 29 +++++++-----
 .../idp/session/impl/DetectIdentitySwitch.java     |  4 +-
 .../impl/ExtractActiveAuthenticationResults.java   |  6 ++-
 .../impl/PopulateLogoutPropagationContext.java     | 15 ++++---
 .../PopulateMultiRPContextFromLogoutContext.java   | 21 ++++++---
 .../idp/session/impl/SaveLogoutContext.java        |  3 +-
 .../session/impl/SelectLogoutPropagationFlow.java  |  7 +--
 .../idp/session/impl/StorageBackedIdPSession.java  | 51 +++++++++++++---------
 .../UpdateSessionWithAuthenticationResult.java     | 38 +++++++++++-----
 .../session/impl/UpdateSessionWithSPSession.java   |  5 ++-
 10 files changed, 117 insertions(+), 62 deletions(-)

diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DestroySessions.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DestroySessions.java
index 7dc2e8389..ef6fa0f2e 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DestroySessions.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DestroySessions.java
@@ -22,12 +22,12 @@ import java.util.function.Function;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.opensaml.messaging.context.BaseContext;
 import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
-import net.shibboleth.shared.primitive.LoggerFactory;
 
 import com.google.common.base.Predicates;
 
@@ -40,6 +40,7 @@ import net.shibboleth.idp.session.context.SessionContext;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * Profile action that destroys any {@link IdPSession}s found in a {@link LogoutContext}.
@@ -135,8 +136,8 @@ public class DestroySessions extends AbstractProfileAction {
             return false;
         }
         
-        logoutContext = logoutContextLookupStrategy.apply(profileRequestContext);
-        if (logoutContext == null || logoutContext.getIdPSessions().isEmpty()) {
+        final LogoutContext lc = logoutContext = logoutContextLookupStrategy.apply(profileRequestContext);
+        if (lc == null || lc.getIdPSessions().isEmpty()) {
             log.debug("{} No LogoutContext or IdPSessions found, nothing to do", getLogPrefix());
             return false;
         }
@@ -150,26 +151,34 @@ public class DestroySessions extends AbstractProfileAction {
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
         
-        for (final IdPSession session : logoutContext.getIdPSessions()) {
+        final LogoutContext lc = logoutContext;
+        assert lc != null;
+        final SessionContext sc = sessionContext;
+        for (final IdPSession session : lc.getIdPSessions()) {
+            assert session!= null;
+            final IdPSession idpSession = sc != null ? sc.getIdPSession() : null;
             log.debug("{} Attempting destruction of session {}", getLogPrefix(), session.getId());
             
-            final boolean unbind = sessionContext != null && sessionContext.getIdPSession() != null
-                    ? sessionContext.getIdPSession().equals(session)
-                            : false;
+            final boolean unbind = idpSession != null ? idpSession.equals(session) : false;
             if (unbind) {
-                sessionContext.getParent().removeSubcontext(sessionContext);
+                assert sc != null;
+                final BaseContext parent = sc.getParent();
+                assert parent != null;
+                parent.removeSubcontext(sc);
                 sessionContext = null;
             }
             
             try {
-                sessionManager.destroySession(session.getId(), unbind);
+                final String id = session.getId();
+                assert id != null;
+                sessionManager.destroySession(id, unbind);
             } catch (final SessionException e) {
                 log.error("{} Error destroying session", getLogPrefix(), e);
                 ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
             }
         }
         
-        logoutContext.getIdPSessions().clear();
+        lc.getIdPSessions().clear();
     }
     
 }
\ No newline at end of file
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DetectIdentitySwitch.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DetectIdentitySwitch.java
index 59b842383..c60c38595 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DetectIdentitySwitch.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/DetectIdentitySwitch.java
@@ -176,7 +176,9 @@ public class DetectIdentitySwitch extends AbstractAuthenticationAction {
                 idpSession.getPrincipalName());
         
         try {
-            sessionManager.destroySession(idpSession.getId(), true);
+            final String id = idpSession.getId();
+            assert id != null;
+            sessionManager.destroySession(id, true);
         } catch (final SessionException e) {
             log.error("{} Error destroying session {}", getLogPrefix(), idpSession.getId(), e);
             ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ExtractActiveAuthenticationResults.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ExtractActiveAuthenticationResults.java
index 95109dbdb..39dcbcfe4 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ExtractActiveAuthenticationResults.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ExtractActiveAuthenticationResults.java
@@ -105,15 +105,17 @@ public class ExtractActiveAuthenticationResults extends AbstractAuthenticationAc
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final AuthenticationContext authenticationContext) {
 
+        final IdPSession theSession = session;
+        assert theSession != null;
         if (authenticationContext.getHintedName() == null) {
-            authenticationContext.setHintedName(session.getPrincipalName());
+            authenticationContext.setHintedName(theSession.getPrincipalName());
         }
         
         final Instant now = Instant.now();
         final Duration maxAge = authenticationContext.getMaxAge();
         
         final List<AuthenticationResult> actives = new ArrayList<>();
-        for (final AuthenticationResult result : session.getAuthenticationResults()) {
+        for (final AuthenticationResult result : theSession.getAuthenticationResults()) {
             final AuthenticationFlowDescriptor descriptor =
                     authenticationContext.getPotentialFlows().get(result.getAuthenticationFlowId());
             if (descriptor == null) {
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateLogoutPropagationContext.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateLogoutPropagationContext.java
index 0bf846188..c66b7683c 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateLogoutPropagationContext.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateLogoutPropagationContext.java
@@ -253,6 +253,8 @@ public class PopulateLogoutPropagationContext extends AbstractProfileAction {
      */
     @Nonnull private SPSession getSessionByValue(@Nonnull final String sessionVal) throws MessageDecodingException {
         try {
+            // checked in calling method.
+            assert dataSealer != null;
             final String decrypted = dataSealer.unwrap(sessionVal);
             final int pos = decrypted.indexOf(':');
             if (pos <= 0) {
@@ -260,17 +262,20 @@ public class PopulateLogoutPropagationContext extends AbstractProfileAction {
             }
 
             final String sessionClassName = decrypted.substring(0,  pos);
-
-            final StorageSerializer<? extends SPSession> spSessionSerializer =
-                    spSessionSerializerRegistry.lookup(Class.forName(sessionClassName).asSubclass(SPSession.class));
+            final Class<? extends SPSession> claz = Class.forName(sessionClassName).asSubclass(SPSession.class);
+            assert claz != null;
+            // checked in calling method.
+            assert spSessionSerializerRegistry != null;
+            final StorageSerializer<? extends SPSession> spSessionSerializer = spSessionSerializerRegistry.lookup(claz);
             if (spSessionSerializer == null) {
                 throw new MessageDecodingException("No serializer registered for session type: " + sessionClassName);
             }
 
             // Deserialize starting past the colon delimiter. The fields are mostly irrelevant here,
             // we're just after the session data itself.
-            return spSessionSerializer.deserialize(
-                    1, "session", "key", decrypted.substring(pos + 1), System.currentTimeMillis());
+            final String sub = decrypted.substring(pos + 1);
+            assert sub != null;
+            return spSessionSerializer.deserialize(1, "session", "key", sub, System.currentTimeMillis());
         } catch (final ClassNotFoundException | IOException | DataSealerException e) {
             throw new MessageDecodingException("Error deserializing encrypted SPSession", e);
         }
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateMultiRPContextFromLogoutContext.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateMultiRPContextFromLogoutContext.java
index 06f7fe852..970e39896 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateMultiRPContextFromLogoutContext.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateMultiRPContextFromLogoutContext.java
@@ -74,7 +74,7 @@ public class PopulateMultiRPContextFromLogoutContext extends AbstractProfileActi
     @Nonnull private Function<ProfileRequestContext,LogoutContext> logoutContextLookupStrategy;
     
     /** Role to resolve metadata for. */
-    @NonnullAfterInit private QName role; 
+    @Nonnull private QName role; 
     
     /** {@link LogoutContext} to process. */
     @Nullable private LogoutContext logoutCtx;
@@ -148,21 +148,28 @@ public class PopulateMultiRPContextFromLogoutContext extends AbstractProfileActi
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
         checkComponentActive();
         
+        final LogoutContext lCtx = logoutCtx;
+        assert lCtx != null;
         final MultiRelyingPartyContext multiCtx = new MultiRelyingPartyContext();
         profileRequestContext.addSubcontext(multiCtx, true);
         
-        for (final String relyingPartyId : logoutCtx.getSessionMap().keySet()) {
+        for (final String relyingPartyId : lCtx.getSessionMap().keySet()) {
+            assert relyingPartyId != null;
             final RelyingPartyContext rpCtx = new RelyingPartyContext();
             rpCtx.setRelyingPartyId(relyingPartyId);
             multiCtx.addRelyingPartyContext(LABEL, rpCtx);
             
-            final EntityIdCriterion entityIdCriterion = new EntityIdCriterion(rpCtx.getRelyingPartyId());
+            final String id = rpCtx.getRelyingPartyId();
+            assert id != null;
+            final EntityIdCriterion entityIdCriterion = new EntityIdCriterion(id);
+            
             final EntityRoleCriterion roleCriterion = new EntityRoleCriterion(role);
             
             ProtocolCriterion protocolCriterion = null;
-            final SPSession spSession = logoutCtx.getSessions(relyingPartyId).iterator().next();
-            if (spSession.getProtocol() != null) {
-                protocolCriterion = new ProtocolCriterion(spSession.getProtocol());
+            final SPSession spSession = lCtx.getSessions(relyingPartyId).iterator().next();
+            final String protocol = spSession.getProtocol();
+            if (protocol != null) {
+                protocolCriterion = new ProtocolCriterion(protocol);
             }
             
             final CriteriaSet criteria = new CriteriaSet(entityIdCriterion, protocolCriterion, roleCriterion);
@@ -180,7 +187,7 @@ public class PopulateMultiRPContextFromLogoutContext extends AbstractProfileActi
                     continue;
                 }
 
-                final SAMLMetadataContext metadataCtx = rpCtx.getSubcontext(SAMLMetadataContext.class, true);
+                final SAMLMetadataContext metadataCtx = rpCtx.getOrCreateSubcontext(SAMLMetadataContext.class);
                 metadataCtx.setEntityDescriptor((EntityDescriptor) roleMetadata.getParent());
                 metadataCtx.setRoleDescriptor(roleMetadata);
 
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SaveLogoutContext.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SaveLogoutContext.java
index 35fc1b373..ff2f2eae9 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SaveLogoutContext.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SaveLogoutContext.java
@@ -34,6 +34,7 @@ import org.springframework.webflow.execution.RequestContext;
 import java.util.function.Function;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 /**
  * Stores the {@link LogoutContext} in the servlet session to facilitate lookup by logout propagation flows.
@@ -67,7 +68,7 @@ public class SaveLogoutContext extends AbstractProfileAction {
     
     /** {@inheritDoc} */
     @Override
-    @Nonnull protected Event doExecute(@Nonnull final RequestContext springRequestContext,
+    @Nullable protected Event doExecute(@Nonnull final RequestContext springRequestContext,
             @Nonnull final ProfileRequestContext profileRequestContext) {
 
         final LogoutContext logoutContext = logoutContextLookup.apply(profileRequestContext);
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SelectLogoutPropagationFlow.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SelectLogoutPropagationFlow.java
index 42bd81350..42dcf239f 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SelectLogoutPropagationFlow.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/SelectLogoutPropagationFlow.java
@@ -95,9 +95,10 @@ public class SelectLogoutPropagationFlow extends AbstractProfileAction {
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_POTENTIAL_FLOW);
             return;
         }
-
-        log.debug("{} Selecting logout propagation flow {}", getLogPrefix(), flow.getId());
-        ActionSupport.buildEvent(profileRequestContext, flow.getId());
+        final String flowId = flow.getId();
+        assert flowId != null;
+        log.debug("{} Selecting logout propagation flow {}", getLogPrefix(), flowId);
+        ActionSupport.buildEvent(profileRequestContext, flowId);
     }
     
 }
\ No newline at end of file
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
index 138482e1a..5df7b05e4 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
@@ -31,6 +31,7 @@ import net.shibboleth.idp.authn.AuthenticationFlowDescriptor;
 import net.shibboleth.idp.authn.AuthenticationResult;
 import net.shibboleth.idp.session.AbstractIdPSession;
 import net.shibboleth.idp.session.SPSession;
+import net.shibboleth.idp.session.SPSessionSerializerRegistry;
 import net.shibboleth.idp.session.SessionException;
 import net.shibboleth.shared.annotation.constraint.Live;
 import net.shibboleth.shared.annotation.constraint.NonnullElements;
@@ -169,7 +170,9 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
             final Map.Entry<String,Optional<AuthenticationResult>> entry = entries.next();
             if (entry.getValue().isEmpty()) {
                 try {
-                    final AuthenticationResult result = loadAuthenticationResultFromStorage(entry.getKey());
+                    final String key = entry.getKey();
+                    assert key != null;
+                    final AuthenticationResult result = loadAuthenticationResultFromStorage(key);
                     if (result != null) {
                         entry.setValue(Optional.of(result));
                     } else {
@@ -202,7 +205,7 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
         
         // Load and add to map.
         try {
-            result = loadAuthenticationResultFromStorage(trimmed);
+            result = loadAuthenticationResultFromStorage(Constraint.isNotNull(trimmed, "FlowID was empty"));
             if (result != null) {
                 doAddAuthenticationResult(result);
             } else {
@@ -337,7 +340,9 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
                 final Map.Entry<String, Optional<SPSession>> entry = entries.next();
                 if (entry.getValue().isEmpty()) {
                     try {
-                        final SPSession result = loadSPSessionFromStorage(entry.getKey());
+                        final String key = entry.getKey();
+                        assert key != null;
+                        final SPSession result = loadSPSessionFromStorage(key);
                         if (result != null) {
                             entry.setValue(Optional.of(result));
                         } else {
@@ -372,7 +377,7 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
             
             // Load and add to map.
             try {
-                result = loadSPSessionFromStorage(trimmed);
+                result = loadSPSessionFromStorage(Constraint.isNotNull(trimmed, "ServiceId was empty"));
                 if (result != null) {
                     doAddSPSession(result);
                 } else {
@@ -400,7 +405,7 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
             try {
                 // Prime things to make sure any previous instance from this SP is loaded so
                 // we know to remove it.
-                getSPSession(spSession.getId());
+                getSPSession(Constraint.isNotNull(spSession.getId(), "SessionID was empty"));
 
                 // Store the record.
                 if (!saveSPSessionToStorage(spSession) && !sessionManager.isMaskStorageFailure()) {
@@ -448,7 +453,7 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
         if (super.removeSPSession(spSession)) {
             try {
                 // Remove the separate record.
-                sessionManager.getStorageService().delete(getId(), getSPSessionStorageKey(spSession.getId()));
+                sessionManager.getStorageService().delete(getId(), getSPSessionStorageKey(Constraint.isNotNull(spSession.getId(), "SessionID was empty")));
             } catch (final IOException e) {
                 log.error("Exception removing SPSession record for IdP session {} and service {}", getId(),
                         spSession.getId(), e);
@@ -633,11 +638,12 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
             }
             
             final String sessionClassName = record.getValue().substring(0,  pos);
-            
+            final SPSessionSerializerRegistry registry = Constraint.isNotNull(sessionManager.getSPSessionSerializerRegistry(), "Session Serializer Registry not set up");
+
             // Look up the serializer instance for that class type.
-            final StorageSerializer<? extends SPSession> spSessionSerializer =
-                    sessionManager.getSPSessionSerializerRegistry().lookup(
-                            Class.forName(sessionClassName).asSubclass(SPSession.class));
+            final Class<? extends SPSession> claz = Class.forName(sessionClassName).asSubclass(SPSession.class);
+            assert claz != null;
+            final StorageSerializer<? extends SPSession> spSessionSerializer = registry.lookup(claz);
             if (spSessionSerializer == null) {
                 throw new IOException("No serializer registered for SPSession type " + sessionClassName);
             }
@@ -667,15 +673,18 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
     private boolean saveSPSessionToStorage(@Nonnull final SPSession session) throws IOException {
         log.debug("Saving SPSession for service {} in session {}", session.getId(), getId());
 
+        final SPSessionSerializerRegistry registry = Constraint.isNotNull(sessionManager.getSPSessionSerializerRegistry(), "Session Serializer Registry not set up");
         // Look up the serializer instance for that class type.
+        final Class<? extends SPSession> claz = session.getClass();
+        assert claz != null;
         final StorageSerializer<SPSession> spSessionSerializer =
-                (StorageSerializer<SPSession>) sessionManager.getSPSessionSerializerRegistry().lookup(
-                        session.getClass());
+                (StorageSerializer<SPSession>) registry.lookup(claz);
         if (spSessionSerializer == null) {
             throw new IOException("No serializer registered for SPSession type " + session.getClass().getName());
         }
-
-        final String key = getSPSessionStorageKey(session.getId());
+        final String id = session.getId();
+        assert id != null;
+        final String key = getSPSessionStorageKey(id);
         
         // Prefix the class name to the serialized data.
         final StringBuilder builder = new StringBuilder(session.getClass().getName());
@@ -687,12 +696,12 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
             boolean success = false;
             do {
                 final Instant exp = session.getExpirationInstant().plus(sessionManager.getSessionSlop());
-                success = sessionManager.getStorageService().create(getId(), key, builder.toString(),
-                        exp.toEpochMilli());
+                final String builtString = builder.toString();
+                assert builtString != null; 
+                success = sessionManager.getStorageService().create(getId(), key, builtString, exp.toEpochMilli());
                 if (!success) {
                     // The record already exists, so we need to overwrite via an update.
-                    success = sessionManager.getStorageService().update(getId(), key, builder.toString(),
-                            exp.toEpochMilli());
+                    success = sessionManager.getStorageService().update(getId(), key, builtString, exp.toEpochMilli());
                 }
             } while (!success && attempts-- > 0);
             
@@ -712,12 +721,14 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
      * Convert a service identifier into a suitable key for the underlying storage service.
      * 
      * @param serviceId the service identifier
-     * 
+     * = 
      * @return  an appropriately sized storage key
      */
     @Nonnull @NotEmpty private String getSPSessionStorageKey(@Nonnull @NotEmpty final String serviceId) {
         if (serviceId.length() > sessionManager.getStorageService().getCapabilities().getKeySize()) {
-            return DigestUtils.sha256Hex(serviceId);
+            final String result = DigestUtils.sha256Hex(serviceId);
+            assert result != null;
+            return result;
         }
         return serviceId;
     }
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithAuthenticationResult.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithAuthenticationResult.java
index 7d49139c4..1e6487e35 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithAuthenticationResult.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithAuthenticationResult.java
@@ -27,11 +27,11 @@ import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
-import net.shibboleth.shared.primitive.LoggerFactory;
 
 import com.google.common.base.Predicates;
 
 import net.shibboleth.idp.authn.AbstractAuthenticationAction;
+import net.shibboleth.idp.authn.AuthenticationResult;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.SubjectContext;
 import net.shibboleth.idp.session.IdPSession;
@@ -41,6 +41,7 @@ import net.shibboleth.idp.session.context.SessionContext;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * An authentication action that establishes a record of the {@link net.shibboleth.idp.authn.AuthenticationResult}
@@ -149,6 +150,7 @@ public class UpdateSessionWithAuthenticationResult extends AbstractAuthenticatio
             }
             
             // We can only do work if a session exists or a non-empty SubjectContext exists.
+            assert sessionCtx != null;
             return sessionCtx.getIdPSession() != null || (subjectCtx != null && subjectCtx.getPrincipalName() != null);
         }
         
@@ -159,8 +161,10 @@ public class UpdateSessionWithAuthenticationResult extends AbstractAuthenticatio
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final AuthenticationContext authenticationContext) {
+        final SessionContext sc = sessionCtx;
+        assert sc != null;;
 
-        final IdPSession session = sessionCtx.getIdPSession();
+        final IdPSession session = sc.getIdPSession();
         if (session != null) {
             try {
                 updateIdPSession(authenticationContext, session);
@@ -172,8 +176,9 @@ public class UpdateSessionWithAuthenticationResult extends AbstractAuthenticatio
             try {
                 createIdPSession(authenticationContext);
             } catch (final SessionException e) {
-                log.error("{} Error creating session for principal {}", getLogPrefix(),
-                        subjectCtx.getPrincipalName(), e);
+                assert subjectCtx != null;
+                final String principalName = subjectCtx.getPrincipalName();
+                log.error("{} Error creating session for principal {}", getLogPrefix(), principalName, e);
                 ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
             }
         }
@@ -192,17 +197,19 @@ public class UpdateSessionWithAuthenticationResult extends AbstractAuthenticatio
     private void updateIdPSession(@Nonnull final AuthenticationContext authenticationContext,
             @Nonnull final IdPSession session) throws SessionException {
         
+        final AuthenticationResult ar = authenticationContext.getAuthenticationResult();
+        assert ar != null;
         if (authenticationContext.getAttemptedFlow() != null) {
             if (authenticationContext.isResultCacheable()) {
                 log.debug("{} Adding new AuthenticationResult for flow {} to existing session {}", getLogPrefix(),
-                        authenticationContext.getAuthenticationResult().getAuthenticationFlowId(), session.getId());
-                session.addAuthenticationResult(authenticationContext.getAuthenticationResult());
+                        ar.getAuthenticationFlowId(), session.getId());
+                session.addAuthenticationResult(ar);
             }
         } else {
             log.debug("{} Updating activity time on reused AuthenticationResult for flow {} in existing session {}",
-                    getLogPrefix(), authenticationContext.getAuthenticationResult().getAuthenticationFlowId(),
+                    getLogPrefix(), ar.getAuthenticationFlowId(),
                     session.getId());
-            session.updateAuthenticationResultActivity(authenticationContext.getAuthenticationResult());
+            session.updateAuthenticationResultActivity(ar);
         }
     }
     
@@ -215,11 +222,20 @@ public class UpdateSessionWithAuthenticationResult extends AbstractAuthenticatio
     private void createIdPSession(@Nonnull final AuthenticationContext authenticationContext)
             throws SessionException {
 
-        log.debug("{} Creating new session for principal {}", getLogPrefix(), subjectCtx.getPrincipalName());
+        final SessionContext sc = sessionCtx;
+        final SubjectContext sbc = subjectCtx;
+        assert sbc != null && sc != null;;
+        final String principalName = sbc.getPrincipalName();
+        assert principalName != null;
+        log.debug("{} Creating new session for principal {}", getLogPrefix(), principalName);
         
-        sessionCtx.setIdPSession(sessionManager.createSession(subjectCtx.getPrincipalName()));
+        sc.setIdPSession(sessionManager.createSession(principalName));
         if (authenticationContext.isResultCacheable()) {
-            sessionCtx.getIdPSession().addAuthenticationResult(authenticationContext.getAuthenticationResult());
+            final AuthenticationResult ar = authenticationContext.getAuthenticationResult();
+            assert ar != null;
+            final IdPSession idPSession = sc.getIdPSession();
+            assert idPSession != null;
+            idPSession.addAuthenticationResult(ar);
         }
     }
 }
\ No newline at end of file
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithSPSession.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithSPSession.java
index db3c31701..3bbf0b055 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithSPSession.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/UpdateSessionWithSPSession.java
@@ -62,7 +62,7 @@ public class UpdateSessionWithSPSession extends AbstractProfileAction {
     @Nonnull private final Logger log = LoggerFactory.getLogger(UpdateSessionWithSPSession.class);
     
     /** A function that returns the {@link SPSession} to add. */
-    @Nonnull private Function<ProfileRequestContext,SPSession> spSessionCreationStrategy;
+    @NonnullAfterInit private Function<ProfileRequestContext,SPSession> spSessionCreationStrategy;
 
     /** SessionManager. */
     @NonnullAfterInit private SessionManager sessionManager;
@@ -147,8 +147,9 @@ public class UpdateSessionWithSPSession extends AbstractProfileAction {
             log.debug("{} SPSession was not returned, nothing to do", getLogPrefix());
             return;
         }
-        
+        assert sessionCtx != null;
         final IdPSession idpSession = sessionCtx.getIdPSession();
+        assert idpSession != null;
         try {
             log.debug("{} Adding new SPSession for relying party {} to existing session {}", getLogPrefix(),
                     spSession.getId(), idpSession.getId());

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


More information about the commits mailing list