From 4093690322fc99d8b83a90748e92eeeadabb0051 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 19 Apr 2016 12:00:06 -0500 Subject: [PATCH] Polish Logout Content Negotiation * Rename to DelegatingLogoutSuccessHandler for consistency * Remove JavascriptOriginRequestMatcher in favor of RequestHeaderRequestMatcher Issue gh-3282 --- .../DelegatingLogoutSuccessHandler.java | 77 ++++++++++++ .../RequestMatcherLogoutSuccessHandler.java | 64 ---------- .../JavascriptOriginRequestMatcher.java | 55 -------- .../DelegatingLogoutSuccessHandlerTests.java | 118 ++++++++++++++++++ ...questMatcherLogoutSuccessHandlerTests.java | 117 ----------------- .../JavascriptOriginRequestMatcherTests.java | 58 --------- 6 files changed, 195 insertions(+), 294 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandler.java delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandler.java delete mode 100644 web/src/main/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcher.java create mode 100644 web/src/test/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandlerTests.java delete mode 100644 web/src/test/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandlerTests.java delete mode 100644 web/src/test/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcherTests.java diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandler.java new file mode 100644 index 0000000000..665180b6e3 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandler.java @@ -0,0 +1,77 @@ +/* + * Copyright 2002-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.authentication.logout; + +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.Map; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.security.core.Authentication; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.util.Assert; + +/** + * Delegates to logout handlers based on matched request matchers + * + * @author Shazin Sadakath + * @author Rob Winch + * @since 4.1 + */ +public class DelegatingLogoutSuccessHandler implements LogoutSuccessHandler { + + private final LinkedHashMap matcherToHandler; + + private LogoutSuccessHandler defaultLogoutSuccessHandler; + + public DelegatingLogoutSuccessHandler( + LinkedHashMap matcherToHandler) { + Assert.notEmpty(matcherToHandler, "matcherToHandler cannot be null"); + this.matcherToHandler = matcherToHandler; + } + + @Override + public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException, ServletException { + for (Map.Entry entry : this.matcherToHandler + .entrySet()) { + RequestMatcher matcher = entry.getKey(); + if (matcher.matches(request)) { + LogoutSuccessHandler handler = entry.getValue(); + handler.onLogoutSuccess(request, response, authentication); + return; + } + } + if (this.defaultLogoutSuccessHandler != null) { + this.defaultLogoutSuccessHandler.onLogoutSuccess(request, response, + authentication); + } + } + + /** + * Sets the default {@link LogoutSuccessHandler} if no other handlers available + * + * @param defaultLogoutSuccessHandler the defaultLogoutSuccessHandler to set + */ + public void setDefaultLogoutSuccessHandler( + LogoutSuccessHandler defaultLogoutSuccessHandler) { + this.defaultLogoutSuccessHandler = defaultLogoutSuccessHandler; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandler.java deleted file mode 100644 index 186c4a92d3..0000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandler.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2002-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.web.authentication.logout; - -import org.springframework.http.HttpStatus; -import org.springframework.security.core.Authentication; -import org.springframework.security.web.util.matcher.JavascriptOriginRequestMatcher; -import org.springframework.security.web.util.matcher.NegatedRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.util.Assert; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.LinkedHashMap; -import java.util.Map; - -/** - * Delegates to logout handlers based on matched request matchers - * - * @author Shazin Sadakath - */ -public class RequestMatcherLogoutSuccessHandler implements LogoutSuccessHandler { - - private Map requestMatcherLogoutSuccessHandlers = new LinkedHashMap(); - - public RequestMatcherLogoutSuccessHandler() { - requestMatcherLogoutSuccessHandlers.put(new JavascriptOriginRequestMatcher(), new HttpStatusReturningLogoutSuccessHandler(HttpStatus.NO_CONTENT)); - requestMatcherLogoutSuccessHandlers.put(new NegatedRequestMatcher(new JavascriptOriginRequestMatcher()), new SimpleUrlLogoutSuccessHandler()); - } - - @Override - public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { - for(Map.Entry entry : requestMatcherLogoutSuccessHandlers.entrySet()) { - if(entry.getKey().matches(request)) { - entry.getValue().onLogoutSuccess(request, response, authentication); - break; - } - } - } - - public Map getRequestMatcherLogoutSuccessHandlers() { - return requestMatcherLogoutSuccessHandlers; - } - - public void setRequestMatcherLogoutSuccessHandlers(Map requestMatcherLogoutSuccessHandlers) { - Assert.notNull(requestMatcherLogoutSuccessHandlers, "must not be null"); - this.requestMatcherLogoutSuccessHandlers = requestMatcherLogoutSuccessHandlers; - } -} diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcher.java deleted file mode 100644 index 9799de90c4..0000000000 --- a/web/src/main/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcher.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2002-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.web.util.matcher; - -import javax.servlet.http.HttpServletRequest; - -/** - * Decides whether the Request Originated from Javascript. - * - * @author Shazin Sadakath - */ -public class JavascriptOriginRequestMatcher implements RequestMatcher { - - public static final String HTTP_X_REQUESTED_WITH = "HTTP_X_REQUESTED_WITH"; - public static final String XML_HTTP_REQUEST = "XMLHttpRequest"; - - private String headerName = HTTP_X_REQUESTED_WITH; - private String headerValue = XML_HTTP_REQUEST; - - - @Override - public boolean matches(HttpServletRequest request) { - Object xHttpRequestedWith = request.getHeader(headerName); - return xHttpRequestedWith != null && xHttpRequestedWith.toString().equalsIgnoreCase(headerValue); - } - - public String getHeaderName() { - return headerName; - } - - public void setHeaderName(String headerName) { - this.headerName = headerName; - } - - public String getHeaderValue() { - return headerValue; - } - - public void setHeaderValue(String headerValue) { - this.headerValue = headerValue; - } -} diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandlerTests.java new file mode 100644 index 0000000000..bc6d81eca8 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/authentication/logout/DelegatingLogoutSuccessHandlerTests.java @@ -0,0 +1,118 @@ +/* + * Copyright 2002-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.authentication.logout; + +import java.util.LinkedHashMap; + +import javax.servlet.http.HttpServletRequest; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.core.Authentication; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +/** + * DelegatingLogoutSuccessHandlerTests Tests + * + * @author Shazin Sadakath + * @author Rob Winch + */ +@RunWith(MockitoJUnitRunner.class) +public class DelegatingLogoutSuccessHandlerTests { + + @Mock + RequestMatcher matcher; + @Mock + RequestMatcher matcher2; + @Mock + LogoutSuccessHandler handler; + @Mock + LogoutSuccessHandler handler2; + @Mock + LogoutSuccessHandler defaultHandler; + @Mock + HttpServletRequest request; + @Mock + MockHttpServletResponse response; + @Mock + Authentication authentication; + + DelegatingLogoutSuccessHandler delegatingHandler; + + @Before + public void setup() { + LinkedHashMap matcherToHandler = new LinkedHashMap(); + matcherToHandler.put(this.matcher, this.handler); + matcherToHandler.put(this.matcher2, this.handler2); + this.delegatingHandler = new DelegatingLogoutSuccessHandler(matcherToHandler); + } + + @Test + public void onLogoutSuccessFirstMatches() throws Exception { + this.delegatingHandler.setDefaultLogoutSuccessHandler(this.defaultHandler); + when(this.matcher.matches(this.request)).thenReturn(true); + + this.delegatingHandler.onLogoutSuccess(this.request, this.response, + this.authentication); + + verify(this.handler).onLogoutSuccess(this.request, this.response, + this.authentication); + verifyZeroInteractions(this.matcher2, this.handler2, this.defaultHandler); + } + + @Test + public void onLogoutSuccessSecondMatches() throws Exception { + this.delegatingHandler.setDefaultLogoutSuccessHandler(this.defaultHandler); + when(this.matcher2.matches(this.request)).thenReturn(true); + + this.delegatingHandler.onLogoutSuccess(this.request, this.response, + this.authentication); + + verify(this.handler2).onLogoutSuccess(this.request, this.response, + this.authentication); + verifyZeroInteractions(this.handler, this.defaultHandler); + } + + @Test + public void onLogoutSuccessDefault() throws Exception { + this.delegatingHandler.setDefaultLogoutSuccessHandler(this.defaultHandler); + + this.delegatingHandler.onLogoutSuccess(this.request, this.response, + this.authentication); + + verify(this.defaultHandler).onLogoutSuccess(this.request, this.response, + this.authentication); + verifyZeroInteractions(this.handler, this.handler2); + } + + @Test + public void onLogoutSuccessNoMatchDefaultNull() throws Exception { + + this.delegatingHandler.onLogoutSuccess(this.request, this.response, + this.authentication); + + verifyZeroInteractions(this.handler, this.handler2, this.defaultHandler); + } +} diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandlerTests.java deleted file mode 100644 index a028ec2da4..0000000000 --- a/web/src/test/java/org/springframework/security/web/authentication/logout/RequestMatcherLogoutSuccessHandlerTests.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright 2002-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.web.authentication.logout; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.security.core.Authentication; -import org.springframework.security.web.util.matcher.IpAddressMatcher; -import org.springframework.security.web.util.matcher.JavascriptOriginRequestMatcher; -import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.web.accept.HeaderContentNegotiationStrategy; - -import java.util.LinkedHashMap; -import java.util.Map; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; - -/** - * RequestMatcherLogoutSuccessHandler Tests - * - * @author Shazin Sadakath - */ -@RunWith(MockitoJUnitRunner.class) -public class RequestMatcherLogoutSuccessHandlerTests { - - private RequestMatcherLogoutSuccessHandler customLogoutSuccessHandler = new RequestMatcherLogoutSuccessHandler(); - - @Before - public void init() { - Map requestMatcherLogoutSuccessHandlerMap = new LinkedHashMap(); - requestMatcherLogoutSuccessHandlerMap.put(new IpAddressMatcher("192.168.1.5"), new SimpleUrlLogoutSuccessHandler()); - requestMatcherLogoutSuccessHandlerMap.put(new MediaTypeRequestMatcher(new HeaderContentNegotiationStrategy(), MediaType.APPLICATION_JSON), new HttpStatusReturningLogoutSuccessHandler(HttpStatus.CREATED)); - customLogoutSuccessHandler.setRequestMatcherLogoutSuccessHandlers(requestMatcherLogoutSuccessHandlerMap); - } - - @Test - public void javascriptOriginRequest() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - LogoutSuccessHandler logoutSuccessHandler = new RequestMatcherLogoutSuccessHandler(); - - request.addHeader(JavascriptOriginRequestMatcher.HTTP_X_REQUESTED_WITH, "XMLHttpRequest"); - - logoutSuccessHandler.onLogoutSuccess(request, response, mock(Authentication.class)); - - assertThat(response.getStatus()).isEqualTo(HttpStatus.NO_CONTENT.value()); - } - - @Test - public void nonJavascriptOriginRequest() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - LogoutSuccessHandler logoutSuccessHandler = new RequestMatcherLogoutSuccessHandler(); - - logoutSuccessHandler.onLogoutSuccess(request, response, mock(Authentication.class)); - - assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - } - - @Test - public void customRequestMatcherHandlerMap_IPAddress() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setRemoteAddr("192.168.1.5"); - MockHttpServletResponse response = new MockHttpServletResponse(); - - customLogoutSuccessHandler.onLogoutSuccess(request, response, mock(Authentication.class)); - - assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - } - - @Test - public void customRequestMatcherHandlerMap_AcceptHeader() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.addHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE); - MockHttpServletResponse response = new MockHttpServletResponse(); - - customLogoutSuccessHandler.onLogoutSuccess(request, response, mock(Authentication.class)); - - assertThat(response.getStatus()).isEqualTo(HttpStatus.CREATED.value()); - } - - @Test - public void customRequestMatcherHandlerMap_IPAddressAndAcceptHeader() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setRemoteAddr("192.168.1.5"); - request.addHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE); - MockHttpServletResponse response = new MockHttpServletResponse(); - - customLogoutSuccessHandler.onLogoutSuccess(request, response, mock(Authentication.class)); - - // IPAddressRequestMatcher -> SimpleUrlLogoutSuccessHandler will be invoked first - assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - } - -} diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcherTests.java deleted file mode 100644 index b5ae11d9c1..0000000000 --- a/web/src/test/java/org/springframework/security/web/util/matcher/JavascriptOriginRequestMatcherTests.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2002-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.web.util.matcher; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; -import org.springframework.mock.web.MockHttpServletRequest; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * JavascriptOriginRequestMatcher Tests - * - * @author Shazin Sadakath - */ -@RunWith(MockitoJUnitRunner.class) -public class JavascriptOriginRequestMatcherTests { - - private RequestMatcher matcher = new JavascriptOriginRequestMatcher(); - - @Test - public void javascriptOriginRequest() { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.addHeader(JavascriptOriginRequestMatcher.HTTP_X_REQUESTED_WITH, "XMLHttpRequest"); - - assertThat(matcher.matches(request)).isTrue(); - } - - @Test - public void nonJavascriptOriginRequest_EmptyHeader() { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.addHeader(JavascriptOriginRequestMatcher.HTTP_X_REQUESTED_WITH, ""); - - assertThat(matcher.matches(request)).isFalse(); - } - - @Test - public void nonJavascriptOriginRequest_NotSetHeader() { - MockHttpServletRequest request = new MockHttpServletRequest(); - - assertThat(matcher.matches(request)).isFalse(); - } - -}