From b28c62a6fe9b7f12a6cd152d190ec083412e1d6a Mon Sep 17 00:00:00 2001
From: Nikos Kastamoulas <nk@dsigned.gr>
Date: Tue, 15 Dec 2015 01:21:50 +0200
Subject: [PATCH] SEC-3170: Null check for Java Config of RememberMeServices

Added a null check in LogoutConfigurer.addLogoutHandler() method to
ensure that a logout handler is always provided..
---
 .../web/configurers/LogoutConfigurer.java     |  4 +++-
 .../configurers/LogoutConfigurerTests.groovy  | 23 +++++++++++++++++++
 .../NamespaceRememberMeTests.groovy           |  8 +++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java
index 070da7f26e..a99f883bee 100644
--- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java
+++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java
@@ -33,6 +33,7 @@ import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuc
 import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
+import org.springframework.util.Assert;
 
 /**
  * Adds logout support. Other {@link SecurityConfigurer} instances may invoke
@@ -85,6 +86,7 @@ public final class LogoutConfigurer<H extends HttpSecurityBuilder<H>> extends
 	 * @return the {@link LogoutConfigurer} for further customization
 	 */
 	public LogoutConfigurer<H> addLogoutHandler(LogoutHandler logoutHandler) {
+		Assert.notNull(logoutHandler, "logoutHandler cannot be null");
 		this.logoutHandlers.add(logoutHandler);
 		return this;
 	}
@@ -311,4 +313,4 @@ public final class LogoutConfigurer<H extends HttpSecurityBuilder<H>> extends
 		}
 		return this.logoutRequestMatcher;
 	}
-}
\ No newline at end of file
+}
diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/LogoutConfigurerTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/LogoutConfigurerTests.groovy
index 3ed19f5f24..e79ad29eba 100644
--- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/LogoutConfigurerTests.groovy
+++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/LogoutConfigurerTests.groovy
@@ -15,6 +15,7 @@
  */
 package org.springframework.security.config.annotation.web.configurers
 
+import org.springframework.beans.factory.BeanCreationException
 import org.springframework.context.annotation.Configuration
 import org.springframework.security.config.annotation.AnyObjectPostProcessor
 import org.springframework.security.config.annotation.BaseSpringSpec
@@ -22,6 +23,7 @@ import org.springframework.security.config.annotation.authentication.builders.Au
 import org.springframework.security.config.annotation.web.builders.HttpSecurity
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
+import org.springframework.security.web.authentication.RememberMeServices
 import org.springframework.security.web.authentication.logout.LogoutFilter
 
 /**
@@ -111,4 +113,25 @@ class LogoutConfigurerTests extends BaseSpringSpec {
 				.csrf().disable()
 		}
 	}
+
+	def "SEC-3170: LogoutConfigurer allows null LogoutHandler"() {
+		when:
+			loadConfig(RememberMeNoLogoutHandler)
+			request.method = "GET"
+			request.servletPath = "/logout"
+			findFilter(LogoutFilter).doFilter(request, response, chain)
+		then:
+			thrown(BeanCreationException)
+	}
+
+	@EnableWebSecurity
+	static class RememberMeNoLogoutHandler extends WebSecurityConfigurerAdapter {
+
+		@Override
+		protected void configure(HttpSecurity http) throws Exception {
+			http
+					.rememberMe()
+					.rememberMeServices(Mock(RememberMeServices))
+		}
+	}
 }
diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceRememberMeTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceRememberMeTests.groovy
index ec21d29d8c..fd9a1aecbf 100644
--- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceRememberMeTests.groovy
+++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceRememberMeTests.groovy
@@ -19,7 +19,8 @@ import org.springframework.security.config.annotation.authentication.builders.Au
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.BaseWebConfig;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
-import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
+import org.springframework.security.web.authentication.logout.LogoutHandler;
 
 import javax.servlet.http.Cookie
 
@@ -112,9 +113,12 @@ public class NamespaceRememberMeTests extends BaseSpringSpec {
 		}
 	}
 
+	// See SEC-3170
+	static interface RememberMeServicesLogoutHandler extends RememberMeServices, LogoutHandler{}
+
 	def "http/remember-me@services-ref"() {
 		setup:
-			RememberMeServicesRefConfig.REMEMBER_ME_SERVICES = Mock(RememberMeServices)
+			RememberMeServicesRefConfig.REMEMBER_ME_SERVICES = Mock(RememberMeServicesLogoutHandler)
 		when: "use custom remember-me services"
 			loadConfig(RememberMeServicesRefConfig)
 		then: "custom remember-me services used"