From b17870ee07c18d48f753fa266c5b5cb98872d887 Mon Sep 17 00:00:00 2001 From: Jakob Englisch Date: Fri, 24 Jun 2016 17:22:21 +0200 Subject: [PATCH] LogoutConfigurer: only allow suitable http methods --- .../web/configurers/LogoutConfigurer.java | 8 +++- .../configurers/LogoutConfigurerTests.groovy | 42 +++++++++++++------ 2 files changed, 37 insertions(+), 13 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 86171bf384..f092451563 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 @@ -34,6 +34,7 @@ import org.springframework.security.web.authentication.logout.SecurityContextLog import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -345,7 +346,12 @@ public final class LogoutConfigurer> extends this.logoutRequestMatcher = new AntPathRequestMatcher(this.logoutUrl, "POST"); } else { - this.logoutRequestMatcher = new AntPathRequestMatcher(this.logoutUrl); + this.logoutRequestMatcher = new OrRequestMatcher( + new AntPathRequestMatcher(this.logoutUrl, "GET"), + new AntPathRequestMatcher(this.logoutUrl, "POST"), + new AntPathRequestMatcher(this.logoutUrl, "PUT"), + new AntPathRequestMatcher(this.logoutUrl, "DELETE") + ); } return this.logoutRequestMatcher; } 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 705cd27ab4..9724cd4d82 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 @@ -15,19 +15,15 @@ */ package org.springframework.security.config.annotation.web.configurers -import org.springframework.beans.factory.BeanCreationException -import org.springframework.context.annotation.Configuration +import org.springframework.http.HttpStatus import org.springframework.security.config.annotation.AnyObjectPostProcessor import org.springframework.security.config.annotation.BaseSpringSpec -import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder import org.springframework.security.config.annotation.web.builders.HttpSecurity import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter -import org.springframework.security.config.annotation.web.configurers.LogoutConfigurerTests.RememberMeNoLogoutHandler; import org.springframework.security.web.authentication.RememberMeServices import org.springframework.security.web.authentication.logout.LogoutFilter -import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; -import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository; +import org.springframework.security.web.authentication.logout.LogoutSuccessHandler import org.springframework.security.web.util.matcher.RequestMatcher /** @@ -91,14 +87,25 @@ class LogoutConfigurerTests extends BaseSpringSpec { } } - def "SEC-2311: Logout allows other methods if CSRF is disabled"() { + def "Logout allows other methods if CSRF is disabled"() { when: loadConfig(CsrfDisabledConfig) - request.method = "GET" + request.method = method request.servletPath = "/logout" findFilter(LogoutFilter).doFilter(request,response,chain) then: - response.redirectedUrl == "/login?logout" + response.status == httpStatus.value() + response.redirectedUrl == url + where: + method | httpStatus | url + "GET" | HttpStatus.FOUND | "/login?logout" + "POST" | HttpStatus.FOUND | "/login?logout" + "PUT" | HttpStatus.FOUND | "/login?logout" + "DELETE" | HttpStatus.FOUND | "/login?logout" + "OPTIONS" | HttpStatus.OK | null + "HEAD" | HttpStatus.OK | null + "TRACE" | HttpStatus.OK | null + } @EnableWebSecurity @@ -113,14 +120,25 @@ class LogoutConfigurerTests extends BaseSpringSpec { } - def "SEC-2311: Logout allows other methods if CSRF is disabled with custom logout URL"() { + def "Logout allows other methods if CSRF is disabled with custom logout URL"() { when: loadConfig(CsrfDisabledCustomLogoutUrlConfig) - request.method = "GET" + request.method = method request.servletPath = "/custom/logout" findFilter(LogoutFilter).doFilter(request,response,chain) then: - response.redirectedUrl == "/login?logout" + response.status == httpStatus.value() + response.redirectedUrl == url + where: + method | httpStatus | url + "GET" | HttpStatus.FOUND | "/login?logout" + "POST" | HttpStatus.FOUND | "/login?logout" + "PUT" | HttpStatus.FOUND | "/login?logout" + "DELETE" | HttpStatus.FOUND | "/login?logout" + "OPTIONS" | HttpStatus.OK | null + "HEAD" | HttpStatus.OK | null + "TRACE" | HttpStatus.OK | null + } @EnableWebSecurity