SEC-1648: added null check for getTargetUrlParameter() in SavedRequestAwareAuthenticationSuccessHandler.onAuthenticationSuccess and updated validation for AbstractAuthenticationTargetUrlRequestHandler.setTargetUrlParameter

web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationTargetUrlRequestHandler.java

@@ -162,7 +162,9 @@ public abstract class AbstractAuthenticationTargetUrlRequestHandler {
      * to null.
      */
     public void setTargetUrlParameter(String targetUrlParameter) {
-        Assert.hasText("targetUrlParameter canot be null or empty");
+        if(targetUrlParameter != null) {
+            Assert.hasText(targetUrlParameter,"targetUrlParameter cannot be empty");
+        }
         this.targetUrlParameter = targetUrlParameter;
     }
 

web/src/main/java/org/springframework/security/web/authentication/SavedRequestAwareAuthenticationSuccessHandler.java

@@ -64,8 +64,8 @@ public class SavedRequestAwareAuthenticationSuccessHandler extends SimpleUrlAuth
 
             return;
         }
-
-        if (isAlwaysUseDefaultTargetUrl() || StringUtils.hasText(request.getParameter(getTargetUrlParameter()))) {
+        String targetUrlParameter = getTargetUrlParameter();
+        if (isAlwaysUseDefaultTargetUrl() || (targetUrlParameter != null && StringUtils.hasText(request.getParameter(targetUrlParameter)))) {
             requestCache.removeRequest(request, response);
             super.onAuthenticationSuccess(request, response, authentication);
 

web/src/test/java/org/springframework/security/web/authentication/SavedRequestAwareAuthenticationSuccessHandlerTests.java

@@ -1,9 +1,15 @@
 package org.springframework.security.web.authentication;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.*;
 
 import org.junit.Test;
-import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.RedirectStrategy;
+import org.springframework.security.web.savedrequest.RequestCache;
+import org.springframework.security.web.savedrequest.SavedRequest;
 
 public class SavedRequestAwareAuthenticationSuccessHandlerTests {
 
@@ -20,4 +26,23 @@ public class SavedRequestAwareAuthenticationSuccessHandlerTests {
             fail("Shouldn't accept default target without leading slash");
         } catch (IllegalArgumentException expected) {}
     }
-}
+
+    @Test
+    public void onAuthenticationSuccessHasSavedRequest() throws Exception {
+        String redirectUrl = "http://localhost/appcontext/page";
+        RedirectStrategy redirectStrategy = mock(RedirectStrategy.class);
+        RequestCache requestCache = mock(RequestCache.class);
+        SavedRequest savedRequest = mock(SavedRequest.class);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        when(savedRequest.getRedirectUrl()).thenReturn(redirectUrl);
+        when(requestCache.getRequest(request, response)).thenReturn(savedRequest);
+
+        SavedRequestAwareAuthenticationSuccessHandler handler = new SavedRequestAwareAuthenticationSuccessHandler();
+        handler.setRequestCache(requestCache);
+        handler.setRedirectStrategy(redirectStrategy);
+        handler.onAuthenticationSuccess(request, response, mock(Authentication.class));
+
+        verify(redirectStrategy).sendRedirect(request, response, redirectUrl);
+    }
+}

web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationSuccessHandlerTests.java

@@ -1,7 +1,6 @@
 package org.springframework.security.web.authentication;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
 import org.junit.Test;
@@ -85,4 +84,26 @@ public class SimpleUrlAuthenticationSuccessHandlerTests {
         assertEquals("https://monkeymachine.co.uk/", response.getRedirectedUrl());
     }
 
+    @Test
+    public void setTargetUrlParameterNullTargetUrlParameter() {
+        SimpleUrlAuthenticationSuccessHandler ash = new SimpleUrlAuthenticationSuccessHandler();
+        ash.setTargetUrlParameter("targetUrl");
+        ash.setTargetUrlParameter(null);
+        assertEquals(null,ash.getTargetUrlParameter());
+    }
+
+    @Test
+    public void setTargetUrlParameterEmptyTargetUrlParameter() {
+        SimpleUrlAuthenticationSuccessHandler ash = new SimpleUrlAuthenticationSuccessHandler();
+
+        try {
+            ash.setTargetUrlParameter("");
+            fail("Expected Exception");
+        }catch(IllegalArgumentException success) {}
+
+        try {
+            ash.setTargetUrlParameter("   ");
+            fail("Expected Exception");
+        }catch(IllegalArgumentException success) {}
+    }
 }