[#2704] [#2710] Fixed Session fixation-related regressions

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<Object, Object> 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();
+            }
         }
     }
 

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;
     }
 

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