From 439b0be58e257ac315fbccd9ed60e7770e33d572 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 26 Feb 2008 14:54:29 +0000 Subject: [PATCH] SEC-462: 302 redirect is not usable for SOAP clients http://jira.springframework.org/browse/SEC-462 --- .../security/ui/AbstractProcessingFilter.java | 27 ++++++++-- .../ui/AbstractProcessingFilterTests.java | 53 ++++++++++++++----- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java index c0cdd924a1..edc34f06f2 100644 --- a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java @@ -210,13 +210,15 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl private boolean migrateInvalidatedSessionAttributes = true; private boolean allowSessionCreation = true; + + private boolean serverSideRedirect = false; //~ Methods ======================================================================================================== public void afterPropertiesSet() throws Exception { Assert.hasLength(filterProcessesUrl, "filterProcessesUrl must be specified"); Assert.hasLength(defaultTargetUrl, "defaultTargetUrl must be specified"); - Assert.hasLength(authenticationFailureUrl, "authenticationFailureUrl must be specified"); +// Assert.hasLength(authenticationFailureUrl, "authenticationFailureUrl must be specified"); Assert.notNull(authenticationManager, "authenticationManager must be specified"); Assert.notNull(rememberMeServices, "rememberMeServices cannot be null"); Assert.notNull(targetUrlResolver, "targetUrlResolver cannot be null"); @@ -343,7 +345,7 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl } protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, - Authentication authResult) throws IOException { + Authentication authResult) throws IOException, ServletException { if (logger.isDebugEnabled()) { logger.debug("Authentication success: " + authResult.toString()); } @@ -437,7 +439,7 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl } protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, - AuthenticationException failed) throws IOException { + AuthenticationException failed) throws IOException, ServletException { SecurityContextHolder.getContext().setAuthentication(null); if (logger.isDebugEnabled()) { @@ -463,8 +465,14 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl onUnsuccessfulAuthentication(request, response, failed); rememberMeServices.loginFail(request, response); - - sendRedirect(request, response, failureUrl); + + if (failureUrl == null) { + response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authentication Failed:" + failed.getMessage()); + } else if (serverSideRedirect){ + request.getRequestDispatcher(failureUrl).forward(request, response); + } else { + sendRedirect(request, response, failureUrl); + } } protected String determineFailureUrl(HttpServletRequest request, AuthenticationException failed) { @@ -601,4 +609,13 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl public void setTargetUrlResolver(TargetUrlResolver targetUrlResolver) { this.targetUrlResolver = targetUrlResolver; } + + /** + * Tells if we are to do a server side include of the error URL instead of a 302 redirect. + * + * @param serverSideRedirect + */ + public void setServerSideRedirect(boolean serverSideRedirect) { + this.serverSideRedirect = serverSideRedirect; + } } diff --git a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java index dcc9842871..e3b857265d 100644 --- a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java @@ -293,20 +293,6 @@ public class AbstractProcessingFilterTests extends TestCase { assertEquals(sessionPreAuth, request.getSession()); } - public void testStartupDetectsInvalidAuthenticationFailureUrl() throws Exception { - AbstractProcessingFilter filter = new MockAbstractProcessingFilter(); - filter.setAuthenticationManager(new MockAuthenticationManager()); - filter.setDefaultTargetUrl("/"); - filter.setFilterProcessesUrl("/j_spring_security_check"); - - try { - filter.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("authenticationFailureUrl must be specified", expected.getMessage()); - } - } - public void testStartupDetectsInvalidAuthenticationManager() throws Exception { AbstractProcessingFilter filter = new MockAbstractProcessingFilter(); filter.setAuthenticationFailureUrl("/failed.jsp"); @@ -547,6 +533,45 @@ public class AbstractProcessingFilterTests extends TestCase { assertNull(request.getSession(false)); } + /** + * SEC-462 + */ + public void testLoginErrorWithNoFailureUrlSendsUnauthorizedStatus() throws Exception { + MockHttpServletRequest request = createMockRequest(); + + MockFilterConfig config = new MockFilterConfig(null, null); + MockFilterChain chain = new MockFilterChain(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(false); + filter.setDefaultTargetUrl("http://monkeymachine.co.uk/"); + + executeFilterInContainerSimulator(config, filter, request, response, chain); + + assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + } + + /** + * SEC-462 + */ + public void testServerSideRedirectForwardsToFailureUrl() throws Exception { + MockHttpServletRequest request = createMockRequest(); + + MockFilterConfig config = new MockFilterConfig(null, null); + MockFilterChain chain = new MockFilterChain(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(false); + filter.setDefaultTargetUrl("http://monkeymachine.co.uk/"); + filter.setAuthenticationFailureUrl("/error"); + filter.setServerSideRedirect(true); + + executeFilterInContainerSimulator(config, filter, request, response, chain); + + assertEquals("/error", response.getForwardedUrl()); + } + + //~ Inner Classes ================================================================================================== private class MockAbstractProcessingFilter extends AbstractProcessingFilter {