diff --git a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy index 69e624183c..4c4bab3b99 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy @@ -419,7 +419,7 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { 'form-login'() } createAppContext() - def handlers = getFilter(LogoutFilter).handlers + def handlers = getFilter(LogoutFilter).handler.logoutHandlers expect: handlers[2] instanceof CookieClearingLogoutHandler diff --git a/config/src/test/groovy/org/springframework/security/config/http/RememberMeConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/RememberMeConfigTests.groovy index 77a8b782ea..7217cd1166 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/RememberMeConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/RememberMeConfigTests.groovy @@ -101,7 +101,7 @@ class RememberMeConfigTests extends AbstractHttpConfigTests { createAppContext(AUTH_PROVIDER_XML) - List logoutHandlers = FieldUtils.getFieldValue(getFilter(LogoutFilter.class), "handlers"); + List logoutHandlers = FieldUtils.getFieldValue(getFilter(LogoutFilter.class), "handler").logoutHandlers; Map ams = appContext.getBeansOfType(ProviderManager.class); ProviderManager am = (ams.values() as List).find { it instanceof ProviderManager && it.providers.size() == 2} RememberMeAuthenticationProvider rmp = am.providers.find { it instanceof RememberMeAuthenticationProvider} @@ -124,7 +124,7 @@ class RememberMeConfigTests extends AbstractHttpConfigTests { createAppContext(AUTH_PROVIDER_XML) def rememberMeServices = rememberMeServices() - List logoutHandlers = getFilter(LogoutFilter.class).handlers + List logoutHandlers = getFilter(LogoutFilter.class).handler.logoutHandlers expect: rememberMeServices diff --git a/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy index bd3ffd9647..0f0e98df6b 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy @@ -169,8 +169,8 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { getFilter(SessionManagementFilter.class) != null sessionRegistryIsValid(); - concurrentSessionFilter.handlers.size() == 1 - def logoutHandler = concurrentSessionFilter.handlers[0] + concurrentSessionFilter.handlers.logoutHandlers.size() == 1 + def logoutHandler = concurrentSessionFilter.handlers.logoutHandlers[0] logoutHandler instanceof SecurityContextLogoutHandler logoutHandler.invalidateHttpSession @@ -190,7 +190,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { List filters = getFilters("/someurl") ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) - def logoutHandlers = concurrentSessionFilter.handlers + def logoutHandlers = concurrentSessionFilter.handlers.logoutHandlers then: 'ConcurrentSessionFilter contains the customized LogoutHandlers' logoutHandlers.size() == 3 @@ -216,7 +216,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { List filters = getFilters("/someurl") ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) - def logoutHandlers = concurrentSessionFilter.handlers + def logoutHandlers = concurrentSessionFilter.handlers.logoutHandlers then: 'SecurityContextLogoutHandler and RememberMeServices are in ConcurrentSessionFilter logoutHandlers' !filters.find { it instanceof LogoutFilter } @@ -238,7 +238,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { List filters = getFilters("/someurl") ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) - def logoutHandlers = concurrentSessionFilter.handlers + def logoutHandlers = concurrentSessionFilter.handlers.logoutHandlers then: 'Only SecurityContextLogoutHandler is found in ConcurrentSessionFilter logoutHandlers' !filters.find { it instanceof LogoutFilter } diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandler.java new file mode 100644 index 0000000000..c37ccf1f27 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandler.java @@ -0,0 +1,58 @@ +/* + * 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.Arrays; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.security.core.Authentication; +import org.springframework.util.Assert; + +/** + * Performs a logout through all the {@link LogoutHandler} implementations. + * If any exception is thrown by + * {@link #logout(HttpServletRequest, HttpServletResponse, Authentication)}, + * next element in {@link #logoutHandlers} is not invoked. + * + * @author Eddú Meléndez + * @since 4.2.0 + */ +public final class CompositeLogoutHandler implements LogoutHandler { + + private final List logoutHandlers; + + public CompositeLogoutHandler(LogoutHandler... logoutHandlers) { + Assert.notEmpty(logoutHandlers, "LogoutHandlers are required"); + this.logoutHandlers = Arrays.asList(logoutHandlers); + } + + public CompositeLogoutHandler(List logoutHandlers) { + Assert.notEmpty(logoutHandlers, "LogoutHandlers are required"); + this.logoutHandlers = logoutHandlers; + } + + @Override + public void logout(HttpServletRequest request, HttpServletResponse response, Authentication authentication) { + for (LogoutHandler handler : this.logoutHandlers) { + handler.logout(request, response, authentication); + } + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/LogoutFilter.java b/web/src/main/java/org/springframework/security/web/authentication/logout/LogoutFilter.java index 75ef026a44..29f0e01a23 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/logout/LogoutFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/LogoutFilter.java @@ -17,8 +17,6 @@ package org.springframework.security.web.authentication.logout; import java.io.IOException; -import java.util.Arrays; -import java.util.List; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -49,6 +47,7 @@ import org.springframework.web.filter.GenericFilterBean; * which constructor was used. * * @author Ben Alex + * @author Eddú Meléndez */ public class LogoutFilter extends GenericFilterBean { @@ -57,7 +56,7 @@ public class LogoutFilter extends GenericFilterBean { private RequestMatcher logoutRequestMatcher; - private final List handlers; + private LogoutHandler handler; private final LogoutSuccessHandler logoutSuccessHandler; // ~ Constructors @@ -71,16 +70,14 @@ public class LogoutFilter extends GenericFilterBean { */ public LogoutFilter(LogoutSuccessHandler logoutSuccessHandler, LogoutHandler... handlers) { - Assert.notEmpty(handlers, "LogoutHandlers are required"); - this.handlers = Arrays.asList(handlers); + this.handler = new CompositeLogoutHandler(handlers); Assert.notNull(logoutSuccessHandler, "logoutSuccessHandler cannot be null"); this.logoutSuccessHandler = logoutSuccessHandler; setFilterProcessesUrl("/logout"); } public LogoutFilter(String logoutSuccessUrl, LogoutHandler... handlers) { - Assert.notEmpty(handlers, "LogoutHandlers are required"); - this.handlers = Arrays.asList(handlers); + this.handler = new CompositeLogoutHandler(handlers); Assert.isTrue( !StringUtils.hasLength(logoutSuccessUrl) || UrlUtils.isValidRedirectUrl(logoutSuccessUrl), @@ -109,9 +106,7 @@ public class LogoutFilter extends GenericFilterBean { + "' and transferring to logout destination"); } - for (LogoutHandler handler : handlers) { - handler.logout(request, response, auth); - } + this.handler.logout(request, response, auth); logoutSuccessHandler.onLogoutSuccess(request, response, auth); diff --git a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java index bfdffee9a6..4d726e50b4 100644 --- a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java +++ b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java @@ -42,6 +42,7 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.AuthenticationEntryPoint; +import org.springframework.security.web.authentication.logout.CompositeLogoutHandler; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.util.Assert; @@ -81,6 +82,7 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory { private AuthenticationEntryPoint authenticationEntryPoint; private AuthenticationManager authenticationManager; private List logoutHandlers; + private LogoutHandler logoutHandler; HttpServlet3RequestFactory(String rolePrefix) { this.rolePrefix = rolePrefix; @@ -250,12 +252,13 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory { "logoutHandlers is null, so allowing original HttpServletRequest to handle logout"); super.logout(); return; + } else { + HttpServlet3RequestFactory.this.logoutHandler = new + CompositeLogoutHandler(handlers); } Authentication authentication = SecurityContextHolder.getContext() .getAuthentication(); - for (LogoutHandler logoutHandler : handlers) { - logoutHandler.logout(this, this.response, authentication); - } + HttpServlet3RequestFactory.this.logoutHandler.logout(this, this.response, authentication); } private boolean isAuthenticated() { diff --git a/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java b/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java index 87a30eddaf..75d6fd77d4 100644 --- a/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java +++ b/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java @@ -32,6 +32,7 @@ import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; +import org.springframework.security.web.authentication.logout.CompositeLogoutHandler; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler; import org.springframework.security.web.util.UrlUtils; @@ -59,6 +60,7 @@ import org.springframework.web.filter.GenericFilterBean; *

* * @author Ben Alex + * @author Eddú Meléndez */ public class ConcurrentSessionFilter extends GenericFilterBean { // ~ Instance fields @@ -66,8 +68,8 @@ public class ConcurrentSessionFilter extends GenericFilterBean { private SessionRegistry sessionRegistry; private String expiredUrl; - private LogoutHandler[] handlers = new LogoutHandler[] { new SecurityContextLogoutHandler() }; private RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); + private LogoutHandler handlers = new CompositeLogoutHandler(new SecurityContextLogoutHandler()); // ~ Methods // ======================================================================================================== @@ -142,14 +144,11 @@ public class ConcurrentSessionFilter extends GenericFilterBean { private void doLogout(HttpServletRequest request, HttpServletResponse response) { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - for (LogoutHandler handler : handlers) { - handler.logout(request, response, auth); - } + this.handlers.logout(request, response, auth); } public void setLogoutHandlers(LogoutHandler[] handlers) { - Assert.notNull(handlers); - this.handlers = handlers; + this.handlers = new CompositeLogoutHandler(handlers); } public void setRedirectStrategy(RedirectStrategy redirectStrategy) { diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandlerTests.java new file mode 100644 index 0000000000..e77cf35eb9 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/authentication/logout/CompositeLogoutHandlerTests.java @@ -0,0 +1,134 @@ +/* + * 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.Arrays; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.InOrder; + +import org.springframework.security.core.Authentication; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * @author Eddú Meléndez + */ +public class CompositeLogoutHandlerTests { + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void buildEmptyCompositeLogoutHandlerThrowsException() { + this.exception.expect(IllegalArgumentException.class); + this.exception.expectMessage("LogoutHandlers are required"); + new CompositeLogoutHandler(); + } + + @Test + public void buildCompositeLogoutHandlerWithArray() { + LogoutHandler[] logoutHandlers = {new SecurityContextLogoutHandler()}; + LogoutHandler handler = new CompositeLogoutHandler(logoutHandlers); + assertThat(ReflectionTestUtils.getField(handler, "logoutHandlers")).isNotNull(); + assertThat(((List)ReflectionTestUtils.getField(handler, + "logoutHandlers")).size()) + .isEqualTo(1); + } + + @Test + public void buildCompositeLogoutHandlerWithList() { + LogoutHandler securityContextLogoutHandler = new SecurityContextLogoutHandler(); + List logoutHandlers = Arrays.asList(securityContextLogoutHandler); + LogoutHandler handler = new CompositeLogoutHandler(logoutHandlers); + assertThat(ReflectionTestUtils.getField(handler, "logoutHandlers")).isNotNull(); + assertThat(((List)ReflectionTestUtils.getField(handler, + "logoutHandlers")).size()) + .isEqualTo(1); + } + + @Test + public void callLogoutHandlersSuccessfully() { + LogoutHandler securityContextLogoutHandler = mock(SecurityContextLogoutHandler.class); + LogoutHandler csrfLogoutHandler = mock(SecurityContextLogoutHandler.class); + + List logoutHandlers = Arrays.asList(securityContextLogoutHandler, csrfLogoutHandler); + LogoutHandler handler = new CompositeLogoutHandler(logoutHandlers); + assertThat(ReflectionTestUtils.getField(handler, "logoutHandlers")).isNotNull(); + assertThat(((List)ReflectionTestUtils.getField(handler, "logoutHandlers")).size()).isEqualTo(2); + + handler.logout(mock(HttpServletRequest.class), mock(HttpServletResponse.class), mock(Authentication.class)); + + verify(securityContextLogoutHandler, times(1)).logout(any(HttpServletRequest.class), any(HttpServletResponse.class), any(Authentication.class)); + verify(csrfLogoutHandler, times(1)).logout(any(HttpServletRequest.class), any(HttpServletResponse.class), any(Authentication.class)); + } + + @Test + public void callLogoutHandlersThrowException() { + LogoutHandler firstLogoutHandler = mock(FirstLogoutHandler.class); + LogoutHandler secondLogoutHandler = mock(SecondLogoutHandler.class); + + doThrow(new IllegalArgumentException()).when(firstLogoutHandler).logout(any(HttpServletRequest.class), any(HttpServletResponse.class), any(Authentication.class)); + + List logoutHandlers = Arrays.asList(firstLogoutHandler, secondLogoutHandler); + LogoutHandler handler = new CompositeLogoutHandler(logoutHandlers); + assertThat(ReflectionTestUtils.getField(handler, "logoutHandlers")).isNotNull(); + assertThat(((List)ReflectionTestUtils.getField(handler, "logoutHandlers")).size()).isEqualTo(2); + + try { + handler.logout(mock(HttpServletRequest.class), mock(HttpServletResponse.class), mock(Authentication.class)); + } catch (IllegalArgumentException ex) { + // Do nothing + } finally { + InOrder logoutHandlersInOrder = inOrder(firstLogoutHandler, secondLogoutHandler); + + logoutHandlersInOrder.verify(firstLogoutHandler, times(1)).logout(any(HttpServletRequest.class), any(HttpServletResponse.class), any(Authentication.class)); + logoutHandlersInOrder.verify(secondLogoutHandler, never()).logout(any(HttpServletRequest.class), any(HttpServletResponse.class), any(Authentication.class)); + } + } + + static class FirstLogoutHandler implements LogoutHandler { + + @Override + public void logout(HttpServletRequest request, HttpServletResponse response, Authentication authentication) { + + } + } + + static class SecondLogoutHandler implements LogoutHandler { + + @Override + public void logout(HttpServletRequest request, HttpServletResponse response, Authentication authentication) { + + } + } + +}