From cea0cf92600d0b6df9da86155bc4e53e507c0d9f Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 26 Sep 2013 11:38:16 -0500 Subject: [PATCH] SEC-2243: Remove additional Debug Filter --- .../annotation/web/builders/DebugFilter.java | 191 ------------------ .../annotation/web/builders/WebSecurity.java | 1 + ...SecurityDebugBeanFactoryPostProcessor.java | 6 +- .../EnableWebSecurityTests.groovy | 2 +- .../configurers/NamespaceDebugTests.groovy | 2 +- ...tyDebugBeanFactoryPostProcessorTest.groovy | 1 + .../config/http/MiscHttpConfigTests.groovy | 3 +- .../security/web}/debug/DebugFilter.java | 8 +- .../security/web}/debug/Logger.java | 10 +- .../security/web}/debug/DebugFilterTest.java | 9 +- 10 files changed, 26 insertions(+), 207 deletions(-) delete mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/builders/DebugFilter.java rename {config/src/main/java/org/springframework/security/config => web/src/main/java/org/springframework/security/web}/debug/DebugFilter.java (94%) rename {config/src/main/java/org/springframework/security/config => web/src/main/java/org/springframework/security/web}/debug/Logger.java (80%) rename {config/src/test/java/org/springframework/security/config => web/src/test/java/org/springframework/security/web}/debug/DebugFilterTest.java (90%) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/DebugFilter.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/DebugFilter.java deleted file mode 100644 index 2370ae6854..0000000000 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/DebugFilter.java +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright 2002-2013 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.config.annotation.web.builders; - -import java.io.IOException; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.List; - -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.security.web.FilterChainProxy; -import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.util.UrlUtils; - -/** - * Spring Security debugging filter. - *

- * Logs information (such as session creation) to help the user understand how requests are being handled - * by Spring Security and provide them with other relevant information (such as when sessions are being created). - * - * - * @author Luke Taylor - * @author Rob Winch - * @since 3.1 - */ -class DebugFilter implements Filter { - private static final String ALREADY_FILTERED_ATTR_NAME = DebugFilter.class.getName().concat(".FILTERED"); - - private final FilterChainProxy fcp; - private final Logger logger = new Logger(); - - public DebugFilter(FilterChainProxy fcp) { - this.fcp = fcp; - } - - public final void doFilter(ServletRequest srvltRequest, ServletResponse srvltResponse, FilterChain filterChain) - throws ServletException, IOException { - - if (!(srvltRequest instanceof HttpServletRequest) || !(srvltResponse instanceof HttpServletResponse)) { - throw new ServletException("DebugFilter just supports HTTP requests"); - } - HttpServletRequest request = (HttpServletRequest) srvltRequest; - HttpServletResponse response = (HttpServletResponse) srvltResponse; - - List filters = getFilters(request); - logger.log("Request received for '" + UrlUtils.buildRequestUrl(request) + "':\n\n" + - request + "\n\n" + - "servletPath:" + request.getServletPath() + "\n" + - "pathInfo:" + request.getPathInfo() + "\n\n" + - formatFilters(filters)); - - if (request.getAttribute(ALREADY_FILTERED_ATTR_NAME) == null) { - invokeWithWrappedRequest(request, response, filterChain); - } else { - fcp.doFilter(request, response, filterChain); - } - } - - private void invokeWithWrappedRequest(HttpServletRequest request, - HttpServletResponse response, FilterChain filterChain) throws IOException, ServletException { - request.setAttribute(ALREADY_FILTERED_ATTR_NAME, Boolean.TRUE); - request = new DebugRequestWrapper(request); - try { - fcp.doFilter(request, response, filterChain); - } - finally { - request.removeAttribute(ALREADY_FILTERED_ATTR_NAME); - } - } - - String formatFilters(List filters) { - StringBuilder sb = new StringBuilder(); - sb.append("Security filter chain: "); - if (filters == null) { - sb.append("no match"); - } else if (filters.isEmpty()) { - sb.append("[] empty (bypassed by security='none') "); - } else { - sb.append("[\n"); - for (Filter f : filters) { - sb.append(" ").append(f.getClass().getSimpleName()).append("\n"); - } - sb.append("]"); - } - - return sb.toString(); - } - - private List getFilters(HttpServletRequest request) { - for (SecurityFilterChain chain : fcp.getFilterChains()) { - if (chain.matches(request)) { - return chain.getFilters(); - } - } - - return null; - } - - public void init(FilterConfig filterConfig) throws ServletException { - } - - public void destroy() { - } -} - -class DebugRequestWrapper extends HttpServletRequestWrapper { - private static final Logger logger = new Logger(); - - public DebugRequestWrapper(HttpServletRequest request) { - super(request); - } - - @Override - public HttpSession getSession() { - boolean sessionExists = super.getSession(false) != null; - HttpSession session = super.getSession(); - - if (!sessionExists) { - logger.log("New HTTP session created: " + session.getId(), true); - } - - return session; - } - - @Override - public HttpSession getSession(boolean create) { - if (!create) { - return super.getSession(create); - } - return getSession(); - } -} - -/** - * Controls output for the Spring Security debug feature. - * - * @author Luke Taylor - * @since 3.1 - */ -final class Logger { - final static Log logger = LogFactory.getLog("Spring Security Debugger"); - - void log(String message) { - log(message, false); - } - - void log(String message, boolean dumpStack) { - StringBuilder output = new StringBuilder(256); - output.append("\n\n************************************************************\n\n"); - output.append(message).append("\n"); - - if (dumpStack) { - StringWriter os = new StringWriter(); - new Exception().printStackTrace(new PrintWriter(os)); - StringBuffer buffer = os.getBuffer(); - // Remove the exception in case it scares people. - int start = buffer.indexOf("java.lang.Exception"); - buffer.replace(start, start + 19, ""); - output.append("\nCall stack: \n").append(os.toString()); - } - - output.append("\n\n************************************************************\n\n"); - - logger.info(output.toString()); - } -} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java index a6b48d8c04..c9d3bafa36 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java @@ -40,6 +40,7 @@ import org.springframework.security.web.access.DefaultWebInvocationPrivilegeEval import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; +import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.util.RequestMatcher; diff --git a/config/src/main/java/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessor.java b/config/src/main/java/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessor.java index 7c93f80ef9..f510272ce1 100644 --- a/config/src/main/java/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessor.java +++ b/config/src/main/java/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessor.java @@ -15,6 +15,8 @@ */ package org.springframework.security.config.debug; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -22,15 +24,17 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.security.config.BeanIds; +import org.springframework.security.web.debug.DebugFilter; /** * @author Luke Taylor * @author Rob Winch */ public class SecurityDebugBeanFactoryPostProcessor implements BeanDefinitionRegistryPostProcessor { + private final Log logger = LogFactory.getLog(getClass()); public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { - Logger.logger.warn("\n\n" + + logger.warn("\n\n" + "********************************************************************\n" + "********** Security debugging is enabled. *************\n" + "********** This may include sensitive information. *************\n" + diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/EnableWebSecurityTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/EnableWebSecurityTests.groovy index f384d0779c..10affc9652 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/EnableWebSecurityTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/EnableWebSecurityTests.groovy @@ -23,9 +23,9 @@ import org.springframework.security.authentication.AuthenticationManager import org.springframework.security.authentication.UsernamePasswordAuthenticationToken import org.springframework.security.config.annotation.BaseSpringSpec import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder -import org.springframework.security.config.annotation.web.builders.DebugFilter; import org.springframework.security.config.annotation.web.builders.HttpSecurity import org.springframework.security.web.authentication.AnonymousAuthenticationFilter +import org.springframework.security.web.debug.DebugFilter; class EnableWebSecurityTests extends BaseSpringSpec { diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceDebugTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceDebugTests.groovy index 6aceb9a947..a139f1233a 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceDebugTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceDebugTests.groovy @@ -29,7 +29,6 @@ import org.springframework.security.access.ConfigAttribute import org.springframework.security.authentication.AnonymousAuthenticationToken import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.config.annotation.BaseSpringSpec -import org.springframework.security.config.annotation.web.builders.DebugFilter; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.core.Authentication; @@ -49,6 +48,7 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.NullSecurityContextRepository; import org.springframework.security.web.context.SecurityContextPersistenceFilter +import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.jaasapi.JaasApiIntegrationFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.AntPathRequestMatcher diff --git a/config/src/test/groovy/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessorTest.groovy b/config/src/test/groovy/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessorTest.groovy index ed9fe8899c..43eb213f58 100644 --- a/config/src/test/groovy/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessorTest.groovy +++ b/config/src/test/groovy/org/springframework/security/config/debug/SecurityDebugBeanFactoryPostProcessorTest.groovy @@ -18,6 +18,7 @@ package org.springframework.security.config.debug import org.springframework.security.config.BeanIds import org.springframework.security.config.http.AbstractHttpConfigTests import org.springframework.security.web.FilterChainProxy; +import org.springframework.security.web.debug.DebugFilter; class SecurityDebugBeanFactoryPostProcessorTest extends AbstractHttpConfigTests { diff --git a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy index 3a4432ef4f..372bf2a41e 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy @@ -55,6 +55,7 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi import org.springframework.security.web.context.HttpSessionSecurityContextRepository import org.springframework.security.web.context.SecurityContextPersistenceFilter import org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter; +import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.jaasapi.JaasApiIntegrationFilter import org.springframework.security.web.savedrequest.HttpSessionRequestCache import org.springframework.security.web.savedrequest.RequestCacheAwareFilter @@ -620,7 +621,7 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { request.addParameter("j_username", "bob"); request.addParameter("j_password", "bobspassword"); then: "App context creation and login request succeed" - Filter debugFilter = appContext.getBean(BeanIds.SPRING_SECURITY_FILTER_CHAIN); + DebugFilter debugFilter = appContext.getBean(BeanIds.SPRING_SECURITY_FILTER_CHAIN); debugFilter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); appListener.events.size() == 2 appListener.authenticationEvents.size() == 2 diff --git a/config/src/main/java/org/springframework/security/config/debug/DebugFilter.java b/web/src/main/java/org/springframework/security/web/debug/DebugFilter.java similarity index 94% rename from config/src/main/java/org/springframework/security/config/debug/DebugFilter.java rename to web/src/main/java/org/springframework/security/web/debug/DebugFilter.java index 0a041e91c9..ccbc1a5688 100644 --- a/config/src/main/java/org/springframework/security/config/debug/DebugFilter.java +++ b/web/src/main/java/org/springframework/security/web/debug/DebugFilter.java @@ -1,4 +1,4 @@ -package org.springframework.security.config.debug; +package org.springframework.security.web.debug; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; @@ -28,7 +28,7 @@ import java.util.*; * @author Rob Winch * @since 3.1 */ -class DebugFilter implements Filter { +public final class DebugFilter implements Filter { private static final String ALREADY_FILTERED_ATTR_NAME = DebugFilter.class.getName().concat(".FILTERED"); private final FilterChainProxy fcp; @@ -48,7 +48,7 @@ class DebugFilter implements Filter { HttpServletResponse response = (HttpServletResponse) srvltResponse; List filters = getFilters(request); - logger.log("Request received for '" + UrlUtils.buildRequestUrl(request) + "':\n\n" + + logger.info("Request received for '" + UrlUtils.buildRequestUrl(request) + "':\n\n" + request + "\n\n" + "servletPath:" + request.getServletPath() + "\n" + "pathInfo:" + request.getPathInfo() + "\n\n" + @@ -121,7 +121,7 @@ class DebugRequestWrapper extends HttpServletRequestWrapper { HttpSession session = super.getSession(); if (!sessionExists) { - logger.log("New HTTP session created: " + session.getId(), true); + logger.info("New HTTP session created: " + session.getId(), true); } return session; diff --git a/config/src/main/java/org/springframework/security/config/debug/Logger.java b/web/src/main/java/org/springframework/security/web/debug/Logger.java similarity index 80% rename from config/src/main/java/org/springframework/security/config/debug/Logger.java rename to web/src/main/java/org/springframework/security/web/debug/Logger.java index 3488906ad9..e70d6f6cf8 100644 --- a/config/src/main/java/org/springframework/security/config/debug/Logger.java +++ b/web/src/main/java/org/springframework/security/web/debug/Logger.java @@ -1,4 +1,4 @@ -package org.springframework.security.config.debug; +package org.springframework.security.web.debug; import java.io.PrintWriter; import java.io.StringWriter; @@ -13,13 +13,13 @@ import org.apache.commons.logging.LogFactory; * @since 3.1 */ final class Logger { - final static Log logger = LogFactory.getLog("Spring Security Debugger"); + private static final Log logger = LogFactory.getLog("Spring Security Debugger"); - void log(String message) { - log(message, false); + public void info(String message) { + info(message, false); } - void log(String message, boolean dumpStack) { + public void info(String message, boolean dumpStack) { StringBuilder output = new StringBuilder(256); output.append("\n\n************************************************************\n\n"); output.append(message).append("\n"); diff --git a/config/src/test/java/org/springframework/security/config/debug/DebugFilterTest.java b/web/src/test/java/org/springframework/security/web/debug/DebugFilterTest.java similarity index 90% rename from config/src/test/java/org/springframework/security/config/debug/DebugFilterTest.java rename to web/src/test/java/org/springframework/security/web/debug/DebugFilterTest.java index f1f55fa8e4..73e87351df 100644 --- a/config/src/test/java/org/springframework/security/config/debug/DebugFilterTest.java +++ b/web/src/test/java/org/springframework/security/web/debug/DebugFilterTest.java @@ -1,4 +1,4 @@ -package org.springframework.security.config.debug; +package org.springframework.security.web.debug; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyString; @@ -22,6 +22,9 @@ import org.powermock.core.classloader.annotations.PrepareOnlyThisForTest; import org.powermock.modules.junit4.PowerMockRunner; import org.powermock.reflect.internal.WhiteboxImpl; import org.springframework.security.web.FilterChainProxy; +import org.springframework.security.web.debug.DebugFilter; +import org.springframework.security.web.debug.DebugRequestWrapper; +import org.springframework.security.web.debug.Logger; /** * @@ -60,7 +63,7 @@ public class DebugFilterTest { public void doFilterProcessesRequests() throws Exception { filter.doFilter(request, response, filterChain); - verify(logger).log(anyString()); + verify(logger).info(anyString()); verify(request).setAttribute(requestAttr, Boolean.TRUE); verify(fcp).doFilter(requestCaptor.capture(), eq(response), eq(filterChain)); assertEquals(DebugRequestWrapper.class,requestCaptor.getValue().getClass()); @@ -75,7 +78,7 @@ public class DebugFilterTest { filter.doFilter(request, response, filterChain); - verify(logger).log(anyString()); + verify(logger).info(anyString()); verify(fcp).doFilter(request, response, filterChain); verify(this.request,never()).removeAttribute(requestAttr); }