From 9e3d2e2d99f986bce6f3722bcbed993bcc709939 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 14 Jun 2016 16:14:41 -0500 Subject: [PATCH] HTTP Basic default logout ignores text/html This fixes an issue where Chrome sends an accept header of application/xml which triggers an HTTP 204 to be returned Fixes gh-3902 --- .../web/configurers/HttpBasicConfigurer.java | 32 ++++++++++++------- .../ExceptionHandlingConfigurerTests.groovy | 8 +++-- .../configurers/LogoutConfigurerTests.groovy | 17 ++++++++++ 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java index 382434c7be..2a1bedfe62 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java @@ -15,6 +15,7 @@ */ package org.springframework.security.config.annotation.web.configurers; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; @@ -32,10 +33,11 @@ import org.springframework.security.web.authentication.HttpStatusEntryPoint; import org.springframework.security.web.authentication.RememberMeServices; import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; import org.springframework.security.web.authentication.logout.HttpStatusReturningLogoutSuccessHandler; -import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; +import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.accept.ContentNegotiationStrategy; @@ -96,8 +98,8 @@ public final class HttpBasicConfigurer> extends DelegatingAuthenticationEntryPoint defaultEntryPoint = new DelegatingAuthenticationEntryPoint( entryPoints); - defaultEntryPoint.setDefaultEntryPoint(basicAuthEntryPoint); - authenticationEntryPoint = defaultEntryPoint; + defaultEntryPoint.setDefaultEntryPoint(this.basicAuthEntryPoint); + this.authenticationEntryPoint = defaultEntryPoint; } /** @@ -110,8 +112,8 @@ public final class HttpBasicConfigurer> extends * @throws Exception */ public HttpBasicConfigurer realmName(String realmName) throws Exception { - basicAuthEntryPoint.setRealmName(realmName); - basicAuthEntryPoint.afterPropertiesSet(); + this.basicAuthEntryPoint.setRealmName(realmName); + this.basicAuthEntryPoint.afterPropertiesSet(); return this; } @@ -144,23 +146,29 @@ public final class HttpBasicConfigurer> extends return this; } + @Override public void init(B http) throws Exception { registerDefaults(http); } - @SuppressWarnings("unchecked") private void registerDefaults(B http) { ContentNegotiationStrategy contentNegotiationStrategy = http .getSharedObject(ContentNegotiationStrategy.class); if (contentNegotiationStrategy == null) { contentNegotiationStrategy = new HeaderContentNegotiationStrategy(); } - MediaTypeRequestMatcher preferredMatcher = new MediaTypeRequestMatcher( + MediaTypeRequestMatcher restMatcher = new MediaTypeRequestMatcher( contentNegotiationStrategy, MediaType.APPLICATION_ATOM_XML, MediaType.APPLICATION_FORM_URLENCODED, MediaType.APPLICATION_JSON, MediaType.APPLICATION_OCTET_STREAM, MediaType.APPLICATION_XML, MediaType.MULTIPART_FORM_DATA, MediaType.TEXT_XML); - preferredMatcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); + restMatcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); + + RequestMatcher notHtmlMatcher = new NegatedRequestMatcher( + new MediaTypeRequestMatcher(contentNegotiationStrategy, + MediaType.TEXT_HTML)); + RequestMatcher preferredMatcher = new AndRequestMatcher( + Arrays.asList(notHtmlMatcher, restMatcher)); registerDefaultEntryPoint(http, preferredMatcher); registerDefaultLogoutSuccessHandler(http, preferredMatcher); @@ -173,7 +181,7 @@ public final class HttpBasicConfigurer> extends return; } exceptionHandling.defaultAuthenticationEntryPointFor( - postProcess(authenticationEntryPoint), preferredMatcher); + postProcess(this.authenticationEntryPoint), preferredMatcher); } private void registerDefaultLogoutSuccessHandler(B http, RequestMatcher preferredMatcher) { @@ -191,10 +199,10 @@ public final class HttpBasicConfigurer> extends AuthenticationManager authenticationManager = http .getSharedObject(AuthenticationManager.class); BasicAuthenticationFilter basicAuthenticationFilter = new BasicAuthenticationFilter( - authenticationManager, authenticationEntryPoint); - if (authenticationDetailsSource != null) { + authenticationManager, this.authenticationEntryPoint); + if (this.authenticationDetailsSource != null) { basicAuthenticationFilter - .setAuthenticationDetailsSource(authenticationDetailsSource); + .setAuthenticationDetailsSource(this.authenticationDetailsSource); } RememberMeServices rememberMeServices = http.getSharedObject(RememberMeServices.class); if(rememberMeServices != null) { diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurerTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurerTests.groovy index 3051227e6c..0c41e74554 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurerTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurerTests.groovy @@ -91,7 +91,9 @@ class ExceptionHandlingConfigurerTests extends BaseSpringSpec { loadConfig(HttpBasicAndFormLoginEntryPointsConfig) DelegatingAuthenticationEntryPoint delegateEntryPoint = findFilter(ExceptionTranslationFilter).authenticationEntryPoint then: - delegateEntryPoint.entryPoints.keySet().collect {it.contentNegotiationStrategy.class} == [HeaderContentNegotiationStrategy,HeaderContentNegotiationStrategy] + def entryPoints = delegateEntryPoint.entryPoints.keySet() as List + entryPoints[0].requestMatchers[1].contentNegotiationStrategy.class == HeaderContentNegotiationStrategy + entryPoints[1].contentNegotiationStrategy.class == HeaderContentNegotiationStrategy } @EnableWebSecurity @@ -123,7 +125,9 @@ class ExceptionHandlingConfigurerTests extends BaseSpringSpec { loadConfig(OverrideContentNegotiationStrategySharedObjectConfig) DelegatingAuthenticationEntryPoint delegateEntryPoint = findFilter(ExceptionTranslationFilter).authenticationEntryPoint then: - delegateEntryPoint.entryPoints.keySet().collect {it.contentNegotiationStrategy} == [OverrideContentNegotiationStrategySharedObjectConfig.CNS,OverrideContentNegotiationStrategySharedObjectConfig.CNS] + def entryPoints = delegateEntryPoint.entryPoints.keySet() as List + entryPoints[0].contentNegotiationStrategy == OverrideContentNegotiationStrategySharedObjectConfig.CNS + entryPoints[1].requestMatchers[1].contentNegotiationStrategy == OverrideContentNegotiationStrategySharedObjectConfig.CNS } def "Override ContentNegotiationStrategy with @Bean"() { 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 b36a126102..705cd27ab4 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 @@ -197,4 +197,21 @@ class LogoutConfigurerTests extends BaseSpringSpec { @EnableWebSecurity static class LogoutHandlerContentNegotiation extends WebSecurityConfigurerAdapter { } + // gh-3902 + def "logout in chrome is 302"() { + setup: + loadConfig(LogoutHandlerContentNegotiationForChrome) + when: + login() + request.method = 'POST' + request.servletPath = '/logout' + request.addHeader('Accept', 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8') + springSecurityFilterChain.doFilter(request,response,chain) + then: + response.status == 302 + } + + @EnableWebSecurity + static class LogoutHandlerContentNegotiationForChrome extends WebSecurityConfigurerAdapter { + } }