Check for null client info from thread local to avoid NPE

Error observed during deployment when users bounced to new server. Indicates session replication probably not working but might as well get exception with message rather than NPE.
apereo · Aug 11, 2020 · b827485 · b827485
1 parent 3d9a737
commit b827485
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 1 deletion.
diff --git a/...ookie-api/src/main/java/org/apereo/cas/web/support/mgmr/DefaultCasCookieValueManager.java b/...ookie-api/src/main/java/org/apereo/cas/web/support/mgmr/DefaultCasCookieValueManager.java
@@ -56,6 +56,11 @@ protected String buildCompoundCookieValue(final String givenCookieValue, final H
         return builder.toString();
     }
 
+
+    /**
+     * Make sure cookie is used from same IP and with same user-agent as when cookie created.
+     * Client info (with original client ip) may be null if cluster failover occurs and session replication not working.
+     */
     @Override
     protected String obtainValueFromCompoundCookie(final String cookieValue, final HttpServletRequest request) {
         val cookieParts = Splitter.on(String.valueOf(COOKIE_FIELD_SEPARATOR)).splitToList(cookieValue);
@@ -76,9 +81,14 @@ protected String obtainValueFromCompoundCookie(final String cookieValue, final H
         }
 
         val clientInfo = ClientInfoHolder.getClientInfo();
+        if (clientInfo == null) {
+            throw new InvalidCookieException("Unable to match required remote address "
+                    + remoteAddr + " because client ip at time of cookie creation unknown");
+        }
+
         if (!remoteAddr.equals(clientInfo.getClientIpAddress())) {
             throw new InvalidCookieException("Invalid cookie. Required remote address "
-                + remoteAddr + " does not match " + clientInfo.getClientIpAddress());
+                    + remoteAddr + " does not match " + clientInfo.getClientIpAddress());
         }
 
         val agent = HttpRequestUtils.getHttpServletRequestUserAgent(request);

diff --git a/...ookie-api/src/test/java/org/apereo/cas/web/support/DefaultCasCookieValueManagerTests.java b/...ookie-api/src/test/java/org/apereo/cas/web/support/DefaultCasCookieValueManagerTests.java
@@ -105,4 +105,14 @@ public void verifyBadAgent() {
         assertThrows(InvalidCookieException.class, () -> mgr.obtainCookieValue("something@"
             + ClientInfoHolder.getClientInfo().getClientIpAddress() + "@agent", new MockHttpServletRequest()));
     }
+
+    @Test
+    public void verifyMissingClientInfo() {
+        val props = new TicketGrantingCookieProperties();
+        val mgr = new DefaultCasCookieValueManager(CipherExecutor.noOp(), props);
+        ClientInfoHolder.clear();
+        assertThrows(InvalidCookieException.class, () -> mgr.obtainCookieValue("something@"
+                + CLIENT_IP + "@" + USER_AGENT, new MockHttpServletRequest()));
+    }
+
 }