From 5082a046262913548a5e01c557d3271e074ce051 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 19 Sep 2013 16:05:26 -0500 Subject: [PATCH] SEC-2311: LogoutConfigurer allows other HTTP methods if CSRF is disabled --- .../web/configurers/LogoutConfigurer.java | 34 ++++++++++---- .../configurers/LogoutConfigurerTests.groovy | 47 +++++++++++++++++++ 2 files changed, 73 insertions(+), 8 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 c590a04e65..77763a860e 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 @@ -64,7 +64,8 @@ public final class LogoutConfigurer> extends Ab private SecurityContextLogoutHandler contextLogoutHandler = new SecurityContextLogoutHandler(); private String logoutSuccessUrl = "/login?logout"; private LogoutSuccessHandler logoutSuccessHandler; - private RequestMatcher logoutRequestMatcher = new AntPathRequestMatcher("/logout", "POST"); + private String logoutUrl = "/logout"; + private RequestMatcher logoutRequestMatcher; private boolean permitAll; private boolean customLogoutSuccess; @@ -98,8 +99,10 @@ public final class LogoutConfigurer> extends Ab } /** - * The URL that triggers log out to occur on HTTP POST. The default is - * "/logout". + * The URL that triggers log out to occur (default is "/logout"). If CSRF + * protection is enabled (default), then the request must also be a POST. + * This means that by default POST "/logout" is required to trigger a log + * out. If CSRF protection is disabled, then any HTTP method is allowed. * *

* It is considered best practice to use an HTTP POST on any action that @@ -110,13 +113,16 @@ public final class LogoutConfigurer> extends Ab *

* * @see #logoutRequestMatcher(RequestMatcher) + * @see HttpSecurity#csrf() * * @param logoutUrl * the URL that will invoke logout. * @return the {@link LogoutConfigurer} for further customization */ public LogoutConfigurer logoutUrl(String logoutUrl) { - return logoutRequestMatcher(new AntPathRequestMatcher(logoutUrl, "POST")); + this.logoutRequestMatcher = null; + this.logoutUrl = logoutUrl; + return this; } /** @@ -219,7 +225,7 @@ public final class LogoutConfigurer> extends Ab public void init(H http) throws Exception { if(permitAll) { PermitAllSupport.permitAll(http, this.logoutSuccessUrl); - PermitAllSupport.permitAll(http, this.logoutRequestMatcher); + PermitAllSupport.permitAll(http, this.getLogoutRequestMatcher(http)); } DefaultLoginPageViewFilter loginPageGeneratingFilter = http.getSharedObject(DefaultLoginPageViewFilter.class); @@ -230,7 +236,7 @@ public final class LogoutConfigurer> extends Ab @Override public void configure(H http) throws Exception { - LogoutFilter logoutFilter = createLogoutFilter(); + LogoutFilter logoutFilter = createLogoutFilter(http); http.addFilter(logoutFilter); } @@ -268,15 +274,27 @@ public final class LogoutConfigurer> extends Ab * instances, the {@link #logoutSuccessHandler(LogoutSuccessHandler)} and * the {@link #logoutUrl(String)}. * + * @param http the builder to use * @return the {@link LogoutFilter} to use. * @throws Exception */ - private LogoutFilter createLogoutFilter() throws Exception { + private LogoutFilter createLogoutFilter(H http) throws Exception { logoutHandlers.add(contextLogoutHandler); LogoutHandler[] handlers = logoutHandlers.toArray(new LogoutHandler[logoutHandlers.size()]); LogoutFilter result = new LogoutFilter(getLogoutSuccessHandler(), handlers); - result.setLogoutRequestMatcher(logoutRequestMatcher); + result.setLogoutRequestMatcher(getLogoutRequestMatcher(http)); result = postProcess(result); return result; } + + @SuppressWarnings("unchecked") + private RequestMatcher getLogoutRequestMatcher(H http) { + if(logoutRequestMatcher != null) { + return logoutRequestMatcher; + } + if(http.getConfigurer(CsrfConfigurer.class) != null) { + this.logoutRequestMatcher = new AntPathRequestMatcher(this.logoutUrl, "POST"); + } + return new AntPathRequestMatcher(this.logoutUrl); + } } \ 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 2eb84d5252..632c371a92 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 @@ -67,4 +67,51 @@ class LogoutConfigurerTests extends BaseSpringSpec { .logout() } } + + def "SEC-2311: Logout allows other methods if CSRF is disabled"() { + when: + loadConfig(CsrfDisabledConfig) + request.method = "GET" + request.servletPath = "/logout" + findFilter(LogoutFilter).doFilter(request,response,chain) + then: + response.redirectedUrl == "/login?logout" + } + + @Configuration + @EnableWebSecurity + static class CsrfDisabledConfig extends WebSecurityConfigurerAdapter { + + @Override + protected void configure(HttpSecurity http) throws Exception { + http + .csrf().disable() + .logout() + } + } + + + def "SEC-2311: Logout allows other methods if CSRF is disabled with custom logout URL"() { + when: + loadConfig(CsrfDisabledCustomLogoutUrlConfig) + request.method = "GET" + request.servletPath = "/custom/logout" + findFilter(LogoutFilter).doFilter(request,response,chain) + then: + response.redirectedUrl == "/login?logout" + } + + @Configuration + @EnableWebSecurity + static class CsrfDisabledCustomLogoutUrlConfig extends WebSecurityConfigurerAdapter { + + @Override + protected void configure(HttpSecurity http) throws Exception { + http + .logout() + .logoutUrl("/custom/logout") + .and() + .csrf().disable() + } + } }