[java-identity-provider] branch main updated: IDP-2069 Null handling

Rod Widdowson rdw at steadingsoftware.com
Fri Feb 24 11:34:58 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=0d3a6c2cf557b8620f54bdbe0cd3d60cd0adcf70

The following commit(s) were added to refs/heads/main by this push:
     new 0d3a6c2cf IDP-2069 Null handling
0d3a6c2cf is described below

commit 0d3a6c2cf557b8620f54bdbe0cd3d60cd0adcf70
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Feb 24 11:37:54 2023 +0000

    IDP-2069 Null handling
    
    https://shibboleth.atlassian.net/browse/IDP-2069
    
    Clean idp-cas-impl tests still tbd
---
 .../BuildSamlValidationSuccessMessageAction.java   |  4 +-
 .../net/shibboleth/idp/cas/flow/impl/Events.java   |  4 +-
 .../impl/UpdateIdPSessionWithSPSessionAction.java  |  5 ++-
 .../cas/flow/impl/ValidateProxyCallbackAction.java |  6 ++-
 .../idp/cas/flow/impl/ValidateRenewAction.java     |  7 +++-
 .../idp/cas/flow/impl/ValidateTicketAction.java    | 15 +++++---
 .../cas/flow/impl/WriteValidateResponseAction.java | 11 ++++--
 .../cas/proxy/impl/HttpClientProxyValidator.java   | 14 +++++--
 .../cas/service/impl/MetadataServiceRegistry.java  |  2 +
 .../cas/service/impl/ServiceEntityDescriptor.java  |  4 +-
 .../cas/session/impl/CASSPSessionSerializer.java   |  4 +-
 .../idp/cas/ticket/impl/AbstractTicketService.java | 21 ++++++-----
 .../idp/cas/ticket/impl/EncodingTicketService.java | 21 ++++++-----
 .../impl/AbstractTicketSerializer.java             | 44 ++++++++++++++--------
 .../impl/ProxyGrantingTicketSerializer.java        |  7 +++-
 .../serialization/impl/ProxyTicketSerializer.java  |  3 +-
 16 files changed, 113 insertions(+), 59 deletions(-)

diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSamlValidationSuccessMessageAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSamlValidationSuccessMessageAction.java
index 82682d29c..51488fe0b 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSamlValidationSuccessMessageAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSamlValidationSuccessMessageAction.java
@@ -106,7 +106,9 @@ public class BuildSamlValidationSuccessMessageAction extends AbstractOutgoingSam
         final Ticket ticket = getCASTicket(profileRequestContext);
         final TicketState state = ticket.getTicketState();
         if (state == null) {
-            throw new EventException(ProtocolError.IllegalState.name());
+            final String name = ProtocolError.IllegalState.name();
+            assert name != null;
+            throw new EventException(name);
         }
         log.debug("Building SAML response for {} in IdP session {}", request.getService(), state.getSessionId());
 
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/Events.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/Events.java
index 0ee73eaad..4ddeb34e8 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/Events.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/Events.java
@@ -17,6 +17,8 @@
 
 package net.shibboleth.idp.cas.flow.impl;
 
+import javax.annotation.Nonnull;
+
 import org.springframework.webflow.execution.Event;
 
 /**
@@ -46,7 +48,7 @@ public enum Events {
      *
      * @return Spring webflow event.
      */
-    public Event event(final Object source) {
+    @Nonnull public Event event(final Object source) {
         return new Event(source, name());
     }
 }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
index a75e8d859..c665d8643 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
@@ -118,10 +118,13 @@ public class UpdateIdPSessionWithSPSessionAction<RequestType,ResponseType>
         }
         if (session != null) {
             final Instant now = Instant.now();
+            assert now != null;
+            final Instant expiration = now.plus(sessionLifetime); 
+            assert expiration != null;
             final SPSession sps = new CASSPSession(
                     tckt.getService(),
                     now,
-                    now.plus(sessionLifetime),
+                    expiration,
                     tckt.getId());
             log.debug("{} Created SP session {}", getLogPrefix(), sps);
             try {
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateProxyCallbackAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateProxyCallbackAction.java
index 625514e8f..ce8e76c57 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateProxyCallbackAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateProxyCallbackAction.java
@@ -152,13 +152,15 @@ public class ValidateProxyCallbackAction
         @Nonnull final IdentifierGenerationStrategy pgtIOUGenerator = vCfg.getPGTIOUGenerator(profileRequestContext);
         @Nonnull final Instant expiration = Instant.now().plus(vCfg.getTicketValidityPeriod(profileRequestContext));
         @Nonnull final String pgtId = pgtGenerator.generateIdentifier();
+        final String pgtUrl = request.getPgtUrl();
+        assert pgtUrl != null;
         final ProxyGrantingTicket pgt;
         if (ticket instanceof ServiceTicket) {
             pgt = casTicketService.createProxyGrantingTicket(
-                pgtId, expiration, (ServiceTicket) tkt, request.getPgtUrl());
+                pgtId, expiration, (ServiceTicket) tkt, pgtUrl);
         } else {
             pgt = casTicketService.createProxyGrantingTicket(
-                pgtId, expiration, (ProxyTicket) tkt, request.getPgtUrl());
+                pgtId, expiration, (ProxyTicket) tkt, pgtUrl);
         }
         // The ID of the proxy-granting ticket MAY be different from the generated value above.
         // ALWAYS use the value from the ticket object.
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateRenewAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateRenewAction.java
index ba9efbd53..1486a37b8 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateRenewAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateRenewAction.java
@@ -73,8 +73,11 @@ public class ValidateRenewAction extends AbstractCASProtocolAction<TicketValidat
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
 
+        final Ticket localTicket = ticket;
+        final TicketValidationRequest localRequest = request; 
+        assert localTicket != null && localRequest != null;
         if (ticket instanceof ServiceTicket) {
-            if (request.isRenew() != ((ServiceTicket) ticket).isRenew()) {
+            if (localRequest.isRenew() != ((ServiceTicket) localTicket).isRenew()) {
                 log.debug("{} Renew=true requested at validation time but ticket not issued with renew=true",
                         getLogPrefix());
                 ActionSupport.buildEvent(profileRequestContext, ProtocolError.TicketNotFromRenew.event(this));
@@ -82,7 +85,7 @@ public class ValidateRenewAction extends AbstractCASProtocolAction<TicketValidat
             }
         } else {
             // Proxy ticket validation
-            if (request.isRenew()) {
+            if (localRequest.isRenew()) {
                 ActionSupport.buildEvent(profileRequestContext, ProtocolError.RenewIncompatibleWithProxy.event(this));
                 return;
             }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
index 166f17829..eda27667d 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
@@ -107,12 +107,15 @@ public class ValidateTicketAction extends AbstractCASProtocolAction<TicketValida
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
 
+        final TicketValidationRequest localRequest = request;
+        final ValidateConfiguration localValidateConfig = validateConfig;
+        assert localValidateConfig != null && localRequest != null;
         final Ticket ticket;
         try {
-            final String ticketId = request.getTicket();
+            final String ticketId = localRequest.getTicket();
             log.debug("Attempting to validate {}", ticketId);
             if (ticketId.startsWith(LoginConfiguration.DEFAULT_TICKET_PREFIX)) {
-                ticket = casTicketService.removeServiceTicket(request.getTicket());
+                ticket = casTicketService.removeServiceTicket(localRequest.getTicket());
             } else if (ticketId.startsWith(ProxyConfiguration.DEFAULT_TICKET_PREFIX)) {
                 ticket = casTicketService.removeProxyTicket(ticketId);
             } else {
@@ -134,10 +137,10 @@ public class ValidateTicketAction extends AbstractCASProtocolAction<TicketValida
             return;
         }
 
-        if (validateConfig.getServiceComparator(profileRequestContext).compare(
-                ticket.getService(), request.getService()) != 0) {
+        if (localValidateConfig.getServiceComparator(profileRequestContext).compare(
+                ticket.getService(), localRequest.getService()) != 0) {
             log.debug("{} Service issued for {} does not match {}", getLogPrefix(), ticket.getService(),
-                    request.getService());
+                    localRequest.getService());
             ActionSupport.buildEvent(profileRequestContext, ProtocolError.ServiceMismatch.event(this));
             return;
         }
@@ -150,7 +153,7 @@ public class ValidateTicketAction extends AbstractCASProtocolAction<TicketValida
             return;
         }
 
-        log.info("{} Successfully validated {} for {}", getLogPrefix(), request.getTicket(), request.getService());
+        log.info("{} Successfully validated {} for {}", getLogPrefix(), localRequest.getTicket(), localRequest.getService());
         
         if (ticket instanceof ProxyTicket) {
             ActionSupport.buildEvent(profileRequestContext, Events.ProxyTicketValidated.event(this));
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/WriteValidateResponseAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/WriteValidateResponseAction.java
index e50025786..68db3578c 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/WriteValidateResponseAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/WriteValidateResponseAction.java
@@ -30,6 +30,8 @@ import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventException;
 import org.opensaml.profile.context.ProfileRequestContext;
 
+import jakarta.servlet.http.HttpServletResponse;
+
 /**
  * CAS 1.0 protocol response handler.
  *
@@ -76,12 +78,15 @@ public class WriteValidateResponseAction extends
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
 
+        final TicketValidationResponse localResponse = response;
+        final HttpServletResponse servletResponse = getHttpServletResponse();
+        assert localResponse!=null && servletResponse!=null;
         try {
-            getHttpServletResponse().setContentType(CONTENT_TYPE);
-            final PrintWriter output = getHttpServletResponse().getWriter();
+            servletResponse.setContentType(CONTENT_TYPE);
+            final PrintWriter output = servletResponse.getWriter();
             if (success) {
                 output.print("yes\n");
-                output.print(response.getUserName() + '\n');
+                output.print(localResponse.getUserName() + '\n');
             } else {
                 output.print("no\n\n");
             }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/proxy/impl/HttpClientProxyValidator.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/proxy/impl/HttpClientProxyValidator.java
index 1d5740593..30e4028b6 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/proxy/impl/HttpClientProxyValidator.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/proxy/impl/HttpClientProxyValidator.java
@@ -56,6 +56,7 @@ import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.opensaml.saml.criterion.EntityRoleCriterion;
 import org.opensaml.saml.criterion.ProtocolCriterion;
+import org.opensaml.saml.saml2.metadata.EntityDescriptor;
 import org.opensaml.saml.saml2.metadata.SPSSODescriptor;
 import org.opensaml.security.credential.UsageType;
 import org.opensaml.security.criteria.UsageCriterion;
@@ -156,14 +157,18 @@ public class HttpClientProxyValidator implements ProxyValidator {
      */
     protected int connect(@Nonnull final URI uri, @Nonnull final Service service) throws GeneralSecurityException {
         final HttpClientContext clientContext = HttpClientContext.create();
+        assert clientContext != null;
         HttpClientSecuritySupport.marshalSecurityParameters(clientContext, securityParameters, true);
         setCASTLSTrustEngineCriteria(clientContext, uri, service);
         ClassicHttpResponse response = null;
         try {
             log.debug("Attempting to validate CAS proxy callback URI {}", uri);
             final HttpGet request = new HttpGet(uri);
+            assert request != null;
             response = httpClient.executeOpen(null, request, clientContext);
-            HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, request.getScheme());
+            final String scheme = request.getScheme();
+            assert scheme != null;
+            HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, scheme);
             return response.getCode();
         } catch (final ClientProtocolException e) {
             throw new GeneralSecurityException("HTTP protocol error", e);
@@ -195,10 +200,11 @@ public class HttpClientProxyValidator implements ProxyValidator {
      * @param service CAS service
      */
     private static void setCASTLSTrustEngineCriteria(
-            final HttpClientContext context, final URI requestUri, final Service service) {
+            @Nonnull final HttpClientContext context, @Nonnull final URI requestUri, @Nonnull final Service service) {
         final String entityID;
-        if (service.getEntityDescriptor() != null) {
-            entityID = service.getEntityDescriptor().getEntityID();
+        final EntityDescriptor entityDescriptor = service.getEntityDescriptor();
+        if (entityDescriptor != null) {
+            entityID = entityDescriptor.getEntityID();
         } else {
             entityID = service.getName();
         }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/MetadataServiceRegistry.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/MetadataServiceRegistry.java
index c823e9b2b..a9bd76a5a 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/MetadataServiceRegistry.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/MetadataServiceRegistry.java
@@ -156,6 +156,7 @@ public class MetadataServiceRegistry implements ServiceRegistry {
     protected Service create(@Nonnull final String serviceURL, @Nonnull final SPSSODescriptor role) {
         
         final EntityDescriptor entity = (EntityDescriptor) role.getParent();
+        assert entity!=null;
         final XMLObject parent = entity.getParent();
                 
         final Service service = new Service(
@@ -207,6 +208,7 @@ public class MetadataServiceRegistry implements ServiceRegistry {
         
         /** {@inheritDoc} */
         public boolean test(@Nullable final Endpoint endpoint) {
+            assert endpoint != null;
             return LOGIN_BINDING.equals(endpoint.getBinding());
         }
     }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/ServiceEntityDescriptor.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/ServiceEntityDescriptor.java
index 631950bea..a1eb6a8e9 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/ServiceEntityDescriptor.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/service/impl/ServiceEntityDescriptor.java
@@ -195,7 +195,7 @@ public class ServiceEntityDescriptor extends AbstractXMLObject implements Entity
 
     /** {@inheritDoc} */
     @Nonnull public AttributeMap getUnknownAttributes() {
-        return null;
+        throw new UnsupportedOperationException();
     }
 
     /** {@inheritDoc} */
@@ -208,7 +208,7 @@ public class ServiceEntityDescriptor extends AbstractXMLObject implements Entity
      * 
      * {@inheritDoc}
      */
-    public void setCacheDuration(final Duration duration) {
+    public void setCacheDuration(final @Nullable Duration duration) {
         throw new UnsupportedOperationException();
     }
 
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
index 0ae9e76c6..1286d4213 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
@@ -29,6 +29,7 @@ import net.shibboleth.idp.session.AbstractSPSessionSerializer;
 import net.shibboleth.idp.session.SPSession;
 import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.logic.Constraint;
 
 /**
  * JSON serializer for {@link CASSPSession} class.
@@ -61,7 +62,8 @@ public class CASSPSessionSerializer extends AbstractSPSessionSerializer {
     @Override
     @Nonnull protected SPSession doDeserialize(@Nonnull final JsonObject obj, @Nonnull @NotEmpty final String id,
             @Nonnull final Instant creation, @Nonnull final Instant expiration) throws IOException {
-        return new CASSPSession(id, creation, expiration, obj.getString(TICKET_FIELD));
+        final String ticketField = Constraint.isNotNull(obj.getString(TICKET_FIELD), "No ticket field");
+        return new CASSPSession(id, creation, expiration, ticketField);
     }
     
 }
\ No newline at end of file
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
index 23e54d2aa..f8868d77b 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
@@ -165,8 +165,9 @@ public abstract class AbstractTicketService implements TicketService {
      *
      * @return Storage service serializer.
      */
-    protected static <T extends Ticket> StorageSerializer<T> serializer(final Class<T> clazz) {
-        return (StorageSerializer<T>) SERIALIZER_MAP.get(clazz);
+    @Nonnull protected static <T extends Ticket> StorageSerializer<T> serializer(@Nonnull final Class<T> clazz) {
+        final StorageSerializer<T> result = (StorageSerializer<T>) Constraint.isNotNull(SERIALIZER_MAP.get(clazz), "Serializer for " + clazz + " not found");
+        return result;
     }
 
     /**
@@ -175,10 +176,10 @@ public abstract class AbstractTicketService implements TicketService {
      * @param ticket Ticket to store
      * @param <T> Type of ticket.
      */
-    protected <T extends Ticket> void store(final T ticket) {
-        final String context = context(ticket.getClass());
+    protected <T extends Ticket> void store(@Nonnull final T ticket) {
+        final String context = Constraint.isNotNull(context(ticket.getClass()), "Could not find context for ticket of type " + ticket.getClass());
         try {
-            final String sessionId = ticket.getSessionId();
+            final String sessionId = Constraint.isNotNull(ticket.getSessionId(), "No session Id");
             final long expiry = ticket.getExpirationInstant().toEpochMilli();
             log.debug("Storing mapping of {} to {} in context {}", ticket, sessionId, context);
             if (!storageService.create(context, ticket.getId(), sessionId, expiry)) {
@@ -203,11 +204,11 @@ public abstract class AbstractTicketService implements TicketService {
      *
      * @return Ticket or null if ticket not found.
      */
-    protected <T extends Ticket> T read(final String id, final Class<T> clazz) {
+    protected <T extends Ticket> T read(@Nonnull final String id, @Nonnull final Class<T> clazz) {
         log.debug("Reading {}", id);
         final T ticket;
         try {
-            final String context = context(clazz);
+            final String context = Constraint.isNotNull(context(clazz), "Could not find context for ticket of type " + clazz);
             final StorageRecord<T> sessionRecord = storageService.read(context, id);
             if (sessionRecord == null) {
                 log.debug("{} not found in context {}", id, context);
@@ -235,18 +236,18 @@ public abstract class AbstractTicketService implements TicketService {
      *
      * @return Deleted ticket or null if ticket not found.
      */
-    protected <T extends Ticket> T delete(final String id, final Class<T> clazz) {
+    protected <T extends Ticket> T delete(@Nonnull final String id, @Nonnull final Class<T> clazz) {
         final T ticket = read(id, clazz);
         if (ticket == null) {
             return null;
         }
         try {
-            final String context = context(clazz);
+            final String context = Constraint.isNotNull(context(clazz), "Could not find context for ticket of type " + clazz);
             log.debug("Attempting to delete {} from context {}", id, context);
             if (!storageService.delete(context, id)) {
                 log.info("Failed deleting {} from context {}.", id, context);
             }
-            final String sessionId = ticket.getSessionId();
+            final String sessionId = Constraint.isNotNull(ticket.getSessionId(), "No session Id");
             log.debug("Attempting to delete {} from context {}", id, sessionId);
             if (!storageService.delete(sessionId, id)) {
                 log.info("Failed deleting {} from context {}.", id, sessionId);
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/EncodingTicketService.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/EncodingTicketService.java
index 8ea0491cb..b2f32e1d3 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/EncodingTicketService.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/EncodingTicketService.java
@@ -82,15 +82,15 @@ public class EncodingTicketService extends AbstractTicketService {
     private final DataSealer dataSealer;
 
     /** Service ticket prefix. */
-    @NotEmpty
+    @Nonnull @NotEmpty
     private String serviceTicketPrefix = SERVICE_TICKET_PREFIX;
 
     /** Proxy ticket prefix. */
-    @NotEmpty
+    @Nonnull @NotEmpty
     private String proxyTicketPrefix = PROXY_TICKET_PREFIX;
 
     /** Proxy granting ticket prefix. */
-    @NotEmpty
+    @Nonnull @NotEmpty
     private String proxyGrantingTicketPrefix = PROXY_GRANTING_TICKET_PREFIX;
 
     /**
@@ -181,9 +181,8 @@ public class EncodingTicketService extends AbstractTicketService {
         return decode(ProxyTicket.class, id, proxyTicketPrefix);
     }
 
-    @Nullable
     @Override
-    public ProxyGrantingTicket createProxyGrantingTicket(
+    public @Nonnull ProxyGrantingTicket createProxyGrantingTicket(
             @Nonnull final String id,
             @Nonnull final Instant expiry,
             @Nonnull final ServiceTicket serviceTicket,
@@ -229,14 +228,16 @@ public class EncodingTicketService extends AbstractTicketService {
      * 
      * @return ticket encoded ticket
      */
-    private <T extends Ticket> T encode(final Class<T> ticketClass, final T ticket, final String prefix) {
+    @Nonnull  <T extends Ticket> T encode(@Nonnull final Class<T> ticketClass, @Nonnull final T ticket, @Nonnull final String prefix) {
         final String opaque;
         try {
             opaque = dataSealer.wrap(serializer(ticketClass).serialize(ticket), ticket.getExpirationInstant());
         } catch (final Exception e) {
             throw new RuntimeException("Ticket encoding failed", e);
         }
-        return ticketClass.cast(ticket.clone(prefix + '-' + opaque));
+        final T  clone = ticketClass.cast(ticket.clone(prefix + '-' + opaque));
+        assert clone != null;
+        return clone;
     }
 
     /**
@@ -249,9 +250,11 @@ public class EncodingTicketService extends AbstractTicketService {
      * 
      * @return decoded ticket
      */
-    private <T extends Ticket> T decode(final Class<T> ticketClass, final String id, final String prefix) {
+    private <T extends Ticket> T decode(@Nonnull final Class<T> ticketClass, @Nonnull final String id, @Nonnull final String prefix) {
         try {
-            final String decrypted = dataSealer.unwrap(id.substring(prefix.length() + 1));
+            final String subString = id.substring(prefix.length() + 1);
+            assert subString != null;
+            final String decrypted = dataSealer.unwrap(subString);
             return serializer(ticketClass).deserialize(0, NOT_USED, id, decrypted, 0L);
         } catch (final Exception e) {
             log.warn("Ticket decoding failed with error: " + e.getMessage());
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
index d563d2530..e689f2928 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
@@ -29,6 +29,7 @@ import javax.annotation.Nullable;
 import javax.json.Json;
 import javax.json.JsonArray;
 import javax.json.JsonException;
+import javax.json.JsonNumber;
 import javax.json.JsonObject;
 import javax.json.JsonReader;
 import javax.json.JsonReaderFactory;
@@ -44,6 +45,7 @@ import net.shibboleth.idp.cas.ticket.Ticket;
 import net.shibboleth.idp.cas.ticket.TicketState;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 
@@ -86,10 +88,12 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
     @Nonnull private final Logger logger = LoggerFactory.getLogger(AbstractTicketSerializer.class);
 
     /** JSON generator factory. */
+    @SuppressWarnings("null")
     @Nonnull
     private final JsonGeneratorFactory generatorFactory = Json.createGeneratorFactory(null);
 
     /** JSON reader factory. */
+    @SuppressWarnings("null")
     @Nonnull
     private final JsonReaderFactory readerFactory = Json.createReaderFactory(null);
 
@@ -109,17 +113,18 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
             gen.writeStartObject()
                     .write(SERVICE_FIELD, ticket.getService())
                     .write(EXPIRATION_FIELD, ticket.getExpirationInstant().toEpochMilli());
-            
-            if (ticket.getTicketState() != null) {
+            final TicketState state = ticket.getTicketState();
+            if (state != null) {
                 gen.writeStartObject(STATE_FIELD)
-                        .write(SESSION_FIELD, ticket.getTicketState().getSessionId())
-                        .write(PRINCIPAL_FIELD, ticket.getTicketState().getPrincipalName())
-                        .write(AUTHN_INSTANT_FIELD, ticket.getTicketState().getAuthenticationInstant().toEpochMilli())
-                        .write(AUTHN_METHOD_FIELD, ticket.getTicketState().getAuthenticationMethod());
+                        .write(SESSION_FIELD, state.getSessionId())
+                        .write(PRINCIPAL_FIELD, state.getPrincipalName())
+                        .write(AUTHN_INSTANT_FIELD, state.getAuthenticationInstant().toEpochMilli())
+                        .write(AUTHN_METHOD_FIELD, state.getAuthenticationMethod());
                 
-                if (ticket.getTicketState().getConsentedAttributeIds() != null) {
+                final Set<String> consentedIds = state.getConsentedAttributeIds(); 
+                if (consentedIds != null) {
                     gen.writeStartArray(CONSENTED_ATTRS_FIELD);
-                    for (final String id : ticket.getTicketState().getConsentedAttributeIds()) {
+                    for (final String id : consentedIds) {
                         gen.write(id);
                     }
                     gen.writeEnd();
@@ -133,7 +138,9 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
             logger.error("Exception serializing {}", ticket, e);
             throw new IOException("Exception serializing ticket", e);
         }
-        return buffer.toString();
+        final String result = buffer.toString();
+        assert result != null;
+        return result;
     }
 
     @Override
@@ -147,16 +154,23 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
 
         try (final JsonReader reader = readerFactory.createReader(new StringReader(value))) {
             final JsonObject to = reader.readObject();
-            final String service = to.getString(SERVICE_FIELD);
-            final Instant expiry = Instant.ofEpochMilli(to.getJsonNumber(EXPIRATION_FIELD).longValueExact());
+            final String service = Constraint.isNotNull(to.getString(SERVICE_FIELD), "Service field was not present");
+            final Instant expiry = Instant.ofEpochMilli(Constraint.isNotNull(to.getJsonNumber(EXPIRATION_FIELD), "Expriation Field was not present").longValueExact());
+            assert expiry != null;
             final JsonObject so = to.getJsonObject(STATE_FIELD);
             final TicketState state;
             if (so != null) {
+                final String sessionField = Constraint.isNotNull(so.getString(SESSION_FIELD), "Session field was not present");
+                final String principalField = Constraint.isNotNull(so.getString(PRINCIPAL_FIELD), "Principal field was not present");
+                final JsonNumber authnInstantField = Constraint.isNotNull(so.getJsonNumber(AUTHN_INSTANT_FIELD), "Authn Instant field was not present");
+                final Instant authnInstant = Instant.ofEpochMilli(authnInstantField.longValueExact());
+                assert authnInstant!=null;
+                final String authnMethodField = Constraint.isNotNull(so.getString(AUTHN_METHOD_FIELD), "Authn Method field was not present");
                 state = new TicketState(
-                        so.getString(SESSION_FIELD),
-                        so.getString(PRINCIPAL_FIELD),
-                        Instant.ofEpochMilli(so.getJsonNumber(AUTHN_INSTANT_FIELD).longValueExact()),
-                        so.getString(AUTHN_METHOD_FIELD));
+                        sessionField, 
+                        principalField,
+                        authnInstant,
+                        authnMethodField);
                 final JsonValue consent = so.get(CONSENTED_ATTRS_FIELD);
                 if (consent instanceof JsonArray) {
                     final Set<String> idset = new HashSet<>();
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyGrantingTicketSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyGrantingTicketSerializer.java
index bde509080..60f71a4a7 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyGrantingTicketSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyGrantingTicketSerializer.java
@@ -24,6 +24,7 @@ import javax.json.JsonObject;
 import javax.json.stream.JsonGenerator;
 
 import net.shibboleth.idp.cas.ticket.ProxyGrantingTicket;
+import net.shibboleth.shared.logic.Constraint;
 
 /**
  * Serializes proxy-granting tickets in simple field-delimited form.
@@ -55,6 +56,10 @@ public class ProxyGrantingTicketSerializer extends AbstractTicketSerializer<Prox
             @Nonnull final String id,
             @Nonnull final String service,
             @Nonnull final Instant expiry) {
-        return new ProxyGrantingTicket(id, service, expiry, o.getString(PGTURL_FIELD), o.getString(PARENT_FIELD, null));
+        return new ProxyGrantingTicket(id, 
+                service,
+                expiry,
+                Constraint.isNotNull(o.getString(PGTURL_FIELD), "pgtUrl was not present"),
+                o.getString(PARENT_FIELD, null));
     }
 }
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyTicketSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyTicketSerializer.java
index e5469c6c2..5b583e039 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyTicketSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/ProxyTicketSerializer.java
@@ -24,6 +24,7 @@ import javax.json.JsonObject;
 import javax.json.stream.JsonGenerator;
 
 import net.shibboleth.idp.cas.ticket.ProxyTicket;
+import net.shibboleth.shared.logic.Constraint;
 
 /**
  * Proxy ticket storage serializer.
@@ -47,6 +48,6 @@ public class ProxyTicketSerializer extends AbstractTicketSerializer<ProxyTicket>
             @Nonnull final String id,
             @Nonnull final String service,
             @Nonnull final Instant expiry) {
-        return new ProxyTicket(id, service, expiry, o.getString(PGTID_FIELD));
+        return new ProxyTicket(id, service, expiry, Constraint.isNotNull(o.getString(PGTID_FIELD), "pgtId was not present"));
     }
 }

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


More information about the commits mailing list