From 86a1226c904c9727fca4492e049b34332fb0abba Mon Sep 17 00:00:00 2001 From: kittimzhe Date: Wed, 14 Oct 2026 11:16:30 +0800 Subject: [PATCH 1/2] [Fix-18699][API] Fix inverted session expiry check in createSessionIfAbsent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reuse the existing session when it is still valid and replace it when it has expired — the previous condition did the opposite, so a second successful login deleted the user's live session (killing their other tab/devices) while a timed-out session was refreshed and reused. --- .../api/service/impl/SessionServiceImpl.java | 4 +- .../api/service/SessionServiceTest.java | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java index b476fd1db6f3..cf18932a91dd 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java @@ -71,7 +71,9 @@ public Session createSessionIfAbsent(User user) { } } session = sessionList.get(0); - if (isSessionExpire(session)) { + // An unexpired session is reused (its last-login time refreshed); an + // expired one is deleted so a new session is created below + if (!isSessionExpire(session)) { session.setLastLoginTime(new Date()); sessionDao.updateById(session); return session; diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java index 659900306956..13800c1e6313 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java @@ -120,6 +120,51 @@ public void testIsSessionExpireUsesConfiguredTimeout() { Assertions.assertTrue(sessionService.isSessionExpire(expired)); } + /** + * #18699: an unexpired session must be reused on re-login, not deleted and + * replaced (the expired/unexpired branches were inverted before the fix) + */ + @Test + public void testCreateSessionIfAbsentReusesUnexpiredSession() { + User user = new User(); + user.setUserType(UserType.GENERAL_USER); + user.setId(1); + // fixture session's lastLoginTime is 40 days in the future -> unexpired + Mockito.when(sessionDao.queryByUserId(1)).thenReturn(getSessions()); + + Session session = sessionService.createSessionIfAbsent(user); + + Assertions.assertNotNull(session); + Assertions.assertEquals(sessionId, session.getId()); + Mockito.verify(sessionDao).updateById(session); + Mockito.verify(sessionDao, Mockito.never()).deleteById(Mockito.anyString()); + Mockito.verify(sessionDao, Mockito.never()).insert(Mockito.any(Session.class)); + } + + /** + * #18699: an expired session must be replaced by a new one on re-login, + * not refreshed and reused (the expired/unexpired branches were inverted + * before the fix) + */ + @Test + public void testCreateSessionIfAbsentReplacesExpiredSession() { + User user = new User(); + user.setUserType(UserType.GENERAL_USER); + user.setId(1); + Session expired = getSession(); + expired.setLastLoginTime(DateUtils.add(new Date(), Calendar.DAY_OF_MONTH, -40)); + List sessions = new ArrayList<>(); + sessions.add(expired); + Mockito.when(sessionDao.queryByUserId(1)).thenReturn(sessions); + + Session session = sessionService.createSessionIfAbsent(user); + + Assertions.assertNotNull(session); + Assertions.assertNotEquals(sessionId, session.getId()); + Mockito.verify(sessionDao).deleteById(sessionId); + Mockito.verify(sessionDao).insert(Mockito.any(Session.class)); + } + private Session getSession() { Session session = new Session(); session.setId(sessionId); From ea05d54ae5289606f8d0c3df12839f1ef1b339b4 Mon Sep 17 00:00:00 2001 From: kittimzhe Date: Fri, 23 Oct 2026 17:30:41 +0800 Subject: [PATCH 2/2] Remove redundant comments per review --- .../api/service/impl/SessionServiceImpl.java | 2 -- .../dolphinscheduler/api/service/SessionServiceTest.java | 9 --------- 2 files changed, 11 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java index cf18932a91dd..522e67f2f7a0 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java @@ -71,8 +71,6 @@ public Session createSessionIfAbsent(User user) { } } session = sessionList.get(0); - // An unexpired session is reused (its last-login time refreshed); an - // expired one is deleted so a new session is created below if (!isSessionExpire(session)) { session.setLastLoginTime(new Date()); sessionDao.updateById(session); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java index 13800c1e6313..383ac73b3c54 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/SessionServiceTest.java @@ -120,10 +120,6 @@ public void testIsSessionExpireUsesConfiguredTimeout() { Assertions.assertTrue(sessionService.isSessionExpire(expired)); } - /** - * #18699: an unexpired session must be reused on re-login, not deleted and - * replaced (the expired/unexpired branches were inverted before the fix) - */ @Test public void testCreateSessionIfAbsentReusesUnexpiredSession() { User user = new User(); @@ -141,11 +137,6 @@ public void testCreateSessionIfAbsentReusesUnexpiredSession() { Mockito.verify(sessionDao, Mockito.never()).insert(Mockito.any(Session.class)); } - /** - * #18699: an expired session must be replaced by a new one on re-login, - * not refreshed and reused (the expired/unexpired branches were inverted - * before the fix) - */ @Test public void testCreateSessionIfAbsentReplacesExpiredSession() { User user = new User();