From 47011eb9e20a9a11e6976a83341e37a12fb7572f Mon Sep 17 00:00:00 2001 From: Eleftheria Stein Date: Thu, 12 Mar 2020 12:11:14 -0400 Subject: [PATCH] Polish transfer session's max inactive interval Issue: gh-2693 --- .../SessionFixationProtectionStrategy.java | 8 +- ...SessionFixationProtectionStrategyTest.java | 81 ------------------- ...ultSessionAuthenticationStrategyTests.java | 32 +++++++- 3 files changed, 36 insertions(+), 85 deletions(-) delete mode 100644 web/src/test/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategyTest.java diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java index 588de9e77c..30794362b2 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java +++ b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2020 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. @@ -90,7 +90,7 @@ public class SessionFixationProtectionStrategy extends } Map attributesToMigrate = extractAttributes(session); - int originMaxInactiveInterval = session.getMaxInactiveInterval(); + int maxInactiveIntervalToMigrate = session.getMaxInactiveInterval(); session.invalidate(); session = request.getSession(true); // we now have a new session @@ -100,7 +100,9 @@ public class SessionFixationProtectionStrategy extends } transferAttributes(attributesToMigrate, session); - session.setMaxInactiveInterval(originMaxInactiveInterval); + if (migrateSessionAttributes) { + session.setMaxInactiveInterval(maxInactiveIntervalToMigrate); + } return session; } diff --git a/web/src/test/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategyTest.java b/web/src/test/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategyTest.java deleted file mode 100644 index 579b15b471..0000000000 --- a/web/src/test/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategyTest.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright 2002-2013 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.session; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; - -import java.util.Arrays; -import java.util.List; -import javax.servlet.http.HttpSession; - -import org.junit.Before; -import org.junit.Test; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.mock.web.MockHttpSession; -import org.springframework.security.core.Authentication; - -public class SessionFixationProtectionStrategyTest { - - private Authentication authentication; - private MockHttpServletRequest httpServletRequest; - private MockHttpServletResponse httpServletResponse; - private MockHttpSession httpSession; - private SessionFixationProtectionStrategy sessionFixationProtectionStrategy; - - @Before - public void setUp() { - this.authentication = mock(Authentication.class); - this.httpServletRequest = new MockHttpServletRequest(); - this.httpServletResponse = new MockHttpServletResponse(); - this.httpSession = new MockHttpSession(); - this.httpServletRequest.setSession(httpSession); - this.sessionFixationProtectionStrategy = new SessionFixationProtectionStrategy(); - } - - @Test - public void createsANewSessionWithAllAttributesTransferredAndTheSessionMaxInactiveInterval() { - String name = "jaswanth"; - List hobbies = Arrays.asList("reading", "blah"); - httpSession.setAttribute("name", name); - httpSession.setAttribute("hobbies", hobbies); - httpSession.setMaxInactiveInterval(2480); - - sessionFixationProtectionStrategy.onAuthentication(authentication, httpServletRequest, httpServletResponse); - - HttpSession newHttpSession = httpServletRequest.getSession(false); - assertThat(httpSession.hashCode()).isNotEqualTo(newHttpSession.hashCode()); - assertThat(newHttpSession.getAttribute("name")).isEqualTo(name); - assertThat(newHttpSession.getAttribute("hobbies")).isEqualTo(hobbies); - assertThat(newHttpSession.getMaxInactiveInterval()).isEqualTo(2480); - } - - @Test - public void shouldNotTransferAttributesIfNotRequested() { - httpSession.setAttribute("name", "jaswanth"); - httpSession.setMaxInactiveInterval(2480); - this.sessionFixationProtectionStrategy.setMigrateSessionAttributes(false); - - sessionFixationProtectionStrategy.onAuthentication(authentication, httpServletRequest, httpServletResponse); - - HttpSession newHttpSession = httpServletRequest.getSession(false); - assertThat(httpSession.hashCode()).isNotEqualTo(newHttpSession.hashCode()); - assertThat(newHttpSession.getAttributeNames().hasMoreElements()).isFalse(); - assertThat(newHttpSession.getMaxInactiveInterval()).isEqualTo(2480); - } -} diff --git a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java b/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java index c462675749..3fd780609a 100644 --- a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java +++ b/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2020 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. @@ -161,4 +161,34 @@ public class DefaultSessionAuthenticationStrategyTests { assertThat(request.getSession(false)).isNotNull(); } + @Test + public void onAuthenticationWhenMigrateSessionAttributesTrueThenMaxInactiveIntervalIsMigrated() { + SessionFixationProtectionStrategy strategy = new SessionFixationProtectionStrategy(); + HttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(); + session.setMaxInactiveInterval(1); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onAuthentication(mockAuthentication, request, + new MockHttpServletResponse()); + + assertThat(request.getSession().getMaxInactiveInterval()).isEqualTo(1); + } + + @Test + public void onAuthenticationWhenMigrateSessionAttributesFalseThenMaxInactiveIntervalIsNotMigrated() { + SessionFixationProtectionStrategy strategy = new SessionFixationProtectionStrategy(); + strategy.setMigrateSessionAttributes(false); + HttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(); + session.setMaxInactiveInterval(1); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onAuthentication(mockAuthentication, request, + new MockHttpServletResponse()); + + assertThat(request.getSession().getMaxInactiveInterval()).isNotEqualTo(1); + } }