diff --git a/core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java b/core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java index 6ca07fc6f3..9d0c77674b 100644 --- a/core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java +++ b/core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java @@ -35,12 +35,15 @@ import org.apache.shiro.subject.Subject; import org.apache.shiro.subject.SubjectContext; import org.apache.shiro.subject.support.DefaultSubjectContext; +import org.apache.shiro.subject.support.DelegatingSubject; import org.apache.shiro.util.CollectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.Serializable; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; /** * The Shiro framework's default concrete implementation of the {@link SecurityManager} interface, @@ -302,7 +305,17 @@ public Subject login(Subject subject, AuthenticationToken token) throws Authenti * @param subject Subject */ protected void beforeSuccessfulLogin(Subject subject) { - stopSession(subject); + Session session = subject.getSession(false); + if (session != null) { + Map attributes = new HashMap<>(); + session.getAttributeKeys().forEach(key -> attributes.put(key, session.getAttribute(key))); + stopSession(subject); + var newSession = subject.getSession(); + var keys = newSession.getAttributeKeys(); + attributes.entrySet().stream() + .filter(entry -> !keys.contains(entry.getKey())) + .forEach(entry -> newSession.setAttribute(entry.getKey(), entry.getValue())); + } } protected void onSuccessfulLogin(AuthenticationToken token, AuthenticationInfo info, Subject subject) { @@ -603,6 +616,9 @@ protected void stopSession(Subject subject) { Session s = subject.getSession(false); if (s != null) { s.stop(); + if (subject instanceof DelegatingSubject) { + ((DelegatingSubject) subject).sessionStopped(); + } } } diff --git a/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java b/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java index b96c97d1b2..f439177078 100644 --- a/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java +++ b/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java @@ -379,7 +379,7 @@ public void logout() { } } - private void sessionStopped() { + public void sessionStopped() { this.session = null; } diff --git a/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java b/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java index c9dfcc6ce2..f38fabacbf 100644 --- a/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java +++ b/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java @@ -36,6 +36,7 @@ import java.util.concurrent.Callable; import static org.apache.shiro.env.BasicIniEnvironment.INI_REALM_NAME; +import static org.assertj.core.api.Assertions.assertThat; import static org.easymock.EasyMock.createNiceMock; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -156,6 +157,8 @@ void testRunAs() { //login as user1 Subject subject = new Subject.Builder(sm).buildSubject(); subject.login(new UsernamePasswordToken("user1", "user1")); + // duplicate login, test for https://github.com/apache/shiro/issues/2704 + subject.login(new UsernamePasswordToken("user1", "user1")); assertFalse(subject.isRunAs()); assertEquals("user1", subject.getPrincipal()); @@ -223,6 +226,36 @@ void testRunAs() { LifecycleUtils.destroy(sm); } + @Test + void sessionAttributesSurviveLoginSessionRotation() { + Ini ini = new Ini(); + Ini.Section users = ini.addSection("users"); + users.put("user1", "user1,role1"); + users.put("user2", "user2,role2"); + users.put("user3", "user3,role3"); + SecurityManager sm = new BasicIniEnvironment(ini).getSecurityManager(); + Subject subject = new Subject.Builder(sm).buildSubject(); + + subject.login(new UsernamePasswordToken("user1", "user1")); + subject.logout(); + + Session preLoginSession = subject.getSession(true); + preLoginSession.setAttribute("tenantId", "ACME"); + Serializable preLoginSessionId = preLoginSession.getId(); + + subject.login(new UsernamePasswordToken("user1", "user1")); + assertThat(subject.isAuthenticated()).isTrue(); + + Session postLoginSession = subject.getSession(false); + assertThat(postLoginSession).isNotNull(); + + assertThat(preLoginSessionId).as("session ID should change on login (session fixation protection)") + .isNotEqualTo(postLoginSession.getId()); + assertThat(postLoginSession.getAttribute("tenantId")) + .as("session attributes set before login must survive session rotation") + .isEqualTo("ACME"); + } + @Test void testToString() { // given