From ec95d63557ce94e4b72d620efb31314748932701 Mon Sep 17 00:00:00 2001 From: Mike Wiesner Date: Mon, 21 Dec 2009 17:15:44 +0000 Subject: [PATCH] SES-24: SpnegoAuthenticationProcessingFilter should call AuthenticationSuccessHandler and AuthenticationFailureHandler --- .../SpnegoAuthenticationProcessingFilter.java | 53 ++++++++++++++++--- ...egoAuthenticationProcessingFilterTest.java | 53 +++++++++++++++++-- 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/spring-security-kerberos-core/src/main/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilter.java b/spring-security-kerberos-core/src/main/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilter.java index 3796d81..2cc6722 100644 --- a/spring-security-kerberos-core/src/main/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilter.java +++ b/spring-security-kerberos-core/src/main/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilter.java @@ -32,6 +32,8 @@ import org.springframework.security.core.codec.Base64; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.extensions.kerberos.KerberosServiceAuthenticationProvider; import org.springframework.security.extensions.kerberos.KerberosServiceRequestToken; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.util.Assert; import org.springframework.web.filter.GenericFilterBean; @@ -93,9 +95,15 @@ import org.springframework.web.filter.GenericFilterBean; */ public class SpnegoAuthenticationProcessingFilter extends GenericFilterBean { - private AuthenticationManager authenticationManager; - @Override + private AuthenticationManager authenticationManager; + private AuthenticationSuccessHandler successHandler; + private AuthenticationFailureHandler failureHandler; + + + /* (non-Javadoc) + * @see javax.servlet.Filter#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse, javax.servlet.FilterChain) + */ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { HttpServletRequest request = (HttpServletRequest) req; @@ -120,12 +128,19 @@ public class SpnegoAuthenticationProcessingFilter extends GenericFilterBean { // That shouldn't happen, as it is most likely a wrong configuration on the server side logger.warn("Negotiate Header was invalid: "+header, e); SecurityContextHolder.clearContext(); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - response.flushBuffer(); + if (failureHandler != null) { + failureHandler.onAuthenticationFailure(request, response, e); + } + else { + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + response.flushBuffer(); + } return; } - SecurityContextHolder.getContext() - .setAuthentication(authentication); + if (successHandler != null) { + successHandler.onAuthenticationSuccess(request, response, authentication); + } + SecurityContextHolder.getContext().setAuthentication(authentication); } chain.doFilter(request, response); @@ -140,6 +155,31 @@ public class SpnegoAuthenticationProcessingFilter extends GenericFilterBean { public void setAuthenticationManager(AuthenticationManager authenticationManager) { this.authenticationManager = authenticationManager; } + + /** + * This handler is called after a successful authentication. + * One can add additional authentication behavior by setting this.
+ * Default is null, which means nothing additional happens + * + * @param successHandler + */ + public void setSuccessHandler(AuthenticationSuccessHandler successHandler) { + this.successHandler = successHandler; + } + + /** + * This handler is called after a failure authentication. + * In most cases you only get Kerberos/SPNEGO failures with a wrong server + * or network configurations and not during runtime. If the client encounters + * an error, he will just stop the communication with server and therefore + * this handler will not be called in this case.
+ * Default is null, which means that the Filter returns the HTTP 500 code + * + * @param failureHandler + */ + public void setFailureHandler(AuthenticationFailureHandler failureHandler) { + this.failureHandler = failureHandler; + } /* (non-Javadoc) * @see org.springframework.web.filter.GenericFilterBean#afterPropertiesSet() @@ -150,4 +190,5 @@ public class SpnegoAuthenticationProcessingFilter extends GenericFilterBean { Assert.notNull(this.authenticationManager, "authenticationManager must be specified"); } + } diff --git a/spring-security-kerberos-core/src/test/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilterTest.java b/spring-security-kerberos-core/src/test/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilterTest.java index b0050f1..45d0bfc 100644 --- a/spring-security-kerberos-core/src/test/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilterTest.java +++ b/spring-security-kerberos-core/src/test/java/org/springframework/security/extensions/kerberos/web/SpnegoAuthenticationProcessingFilterTest.java @@ -18,7 +18,10 @@ package org.springframework.security.extensions.kerberos.web; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import java.io.IOException; + import javax.servlet.FilterChain; +import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -30,9 +33,12 @@ import org.junit.Test; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.extensions.kerberos.KerberosServiceRequestToken; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; /** * Test class for {@link SpnegoAuthenticationProcessingFilter} @@ -50,6 +56,8 @@ public class SpnegoAuthenticationProcessingFilterTest { private HttpServletRequest request; private HttpServletResponse response; private FilterChain chain; + private AuthenticationSuccessHandler successHandler; + private AuthenticationFailureHandler failureHandler; // data private static final byte[] TEST_TOKEN = "TestToken".getBytes(); @@ -58,10 +66,11 @@ public class SpnegoAuthenticationProcessingFilterTest { AuthorityUtils.createAuthorityList("ROLE_ADMIN"), TEST_TOKEN); private static final String HEADER = "Authorization"; private static final String TOKEN_PREFIX = "Negotiate "; + private static final BadCredentialsException BCE = new BadCredentialsException(""); @Before - public void before() { + public void before() throws Exception { // mocking authenticationManager = mock(AuthenticationManager.class); filter = new SpnegoAuthenticationProcessingFilter(); @@ -69,10 +78,24 @@ public class SpnegoAuthenticationProcessingFilterTest { request = mock(HttpServletRequest.class); response = mock(HttpServletResponse.class); chain = mock(FilterChain.class); + filter.afterPropertiesSet(); } @Test public void testEverythingWorks() throws Exception { + everythingWorks(); + } + + @Test + public void testEverythingWorksWithHandlers() throws Exception { + createHandler(); + everythingWorks(); + verify(successHandler).onAuthenticationSuccess(request, response, AUTHENTICATION); + verify(failureHandler, never()).onAuthenticationFailure(any(HttpServletRequest.class), + any(HttpServletResponse.class), any(AuthenticationException.class)); + } + + private void everythingWorks() throws IOException, ServletException { // stubbing when(request.getHeader(HEADER)).thenReturn(TOKEN_PREFIX+TEST_TOKEN_BASE64); when(authenticationManager.authenticate(new KerberosServiceRequestToken(TEST_TOKEN))).thenReturn(AUTHENTICATION); @@ -82,6 +105,8 @@ public class SpnegoAuthenticationProcessingFilterTest { verify(chain).doFilter(request, response); assertEquals(AUTHENTICATION, SecurityContextHolder.getContext().getAuthentication()); } + + @Test public void testNoHeader() throws Exception { @@ -95,17 +120,39 @@ public class SpnegoAuthenticationProcessingFilterTest { @Test public void testAuthenticationFails() throws Exception { + authenticationFails(); + verify(response).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + + @Test + public void testAuthenticationFailsWithHandlers() throws Exception { + createHandler(); + authenticationFails(); + verify(failureHandler).onAuthenticationFailure(request, response, BCE); + verify(successHandler, never()).onAuthenticationSuccess(any(HttpServletRequest.class), + any(HttpServletResponse.class), any(Authentication.class)); + verify(response, never()).setStatus(anyInt()); + } + + private void authenticationFails() throws IOException, ServletException { // stubbing when(request.getHeader(HEADER)).thenReturn(TOKEN_PREFIX+TEST_TOKEN_BASE64); - when(authenticationManager.authenticate(any(Authentication.class))).thenThrow(new BadCredentialsException("")); + when(authenticationManager.authenticate(any(Authentication.class))).thenThrow(BCE); // testing filter.doFilter(request, response, chain); // chain should stop here and it should send back a 500 // future version should call some error handler verify(chain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class)); - verify(response).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } + + private void createHandler() { + successHandler = mock(AuthenticationSuccessHandler.class); + failureHandler = mock(AuthenticationFailureHandler.class); + filter.setSuccessHandler(successHandler); + filter.setFailureHandler(failureHandler); + } + @After public void after() {