joeyzee-97 opened a new issue, #18699: URL: https://github.com/apache/dolphinscheduler/issues/18699
### Search before asking - [x] I searched the repository's issues and pull requests and did not find a direct report of the re-login behavior described below. Related reports are linked below. ### What happened There appears to be an inverted expiry condition in `SessionServiceImpl#createSessionIfAbsent` in **3.4.3** and the current **dev** branch. Please confirm whether this is a regression or intentional login policy. The method currently: - Refreshes and reuses the existing session when `isSessionExpire(session)` is **true**. - Deletes the existing session and creates a new session when it is **false**. However, `isSessionExpire` returns true when the elapsed time is greater than or equal to the session timeout. This means an unexpired session is invalidated on another successful login, whereas an expired session ID is refreshed and reused. For a user logging in through a second browser/device, the source-level consequence is that the first browser's valid session ID is deleted. This report concerns that behavior and its historical intent; it is not a request to introduce independent per-device sessions. Source references: - [3.4.3: login branch and expiry predicate](https://github.com/apache/dolphinscheduler/blob/3.4.3/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java) - [dev pinned at 7cead6c895dbd0bc9e04ea2300c22c6bdff4fcb9](https://github.com/apache/dolphinscheduler/blob/7cead6c895dbd0bc9e04ea2300c22c6bdff4fcb9/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java) ### What you expected to happen If the historical behavior is intended, a successful re-login should reuse/refresh an unexpired session and replace an expired session with a fresh ID. If the intended policy is instead "the latest login invalidates previous logins", please confirm it and clarify why expired session IDs are reused rather than replaced. I have not found an explicit design statement establishing a single-device policy. ### How to reproduce #### Isolated source-method reproduction performed I extracted the **unchanged method bodies** of `createSessionIfAbsent` and `isSessionExpire` from both references above and executed them with Java 21 in a small harness. The harness provides an in-memory DAO and minimal `User`, `Session`, constants/configuration, and collection-helper stand-ins. The timeout was 2 hours, and there was exactly one existing session for the user. Both references produced the same results: | Existing session age | Expired? | Reused old ID? | DAO operations | | --- | --- | --- | --- | | 1 minute | false | no | delete old session, insert new session | | 3 hours | true | yes | update old session, no insert | Assertions checking those observed operations passed for both source versions. This verifies the branch behavior in isolation; **I have not run the complete DolphinScheduler test suite or a deployed two-browser reproduction**. #### Suggested regression tests in the existing SessionServiceTest These tests express the historical reuse policy and are proposed for maintainer confirmation. They have not been run in the full project: ```java @Test void testReloginReusesUnexpiredSession() { User user = new User(); user.setId(1); Session existing = Session.builder() .id("existing-session") .userId(1) .lastLoginTime(new Date(System.currentTimeMillis() - 60_000L)) .build(); Mockito.when(sessionDao.queryByUserId(1)) .thenReturn(java.util.Collections.singletonList(existing)); Session result = sessionService.createSessionIfAbsent(user); Assertions.assertEquals(existing.getId(), result.getId()); Mockito.verify(sessionDao, Mockito.never()).deleteById(existing.getId()); } @Test void testReloginReplacesExpiredSession() { User user = new User(); user.setId(1); Session existing = Session.builder() .id("expired-session") .userId(1) .lastLoginTime(new Date(System.currentTimeMillis() - 3L * 60 * 60 * 1000)) .build(); Mockito.when(sessionDao.queryByUserId(1)) .thenReturn(java.util.Collections.singletonList(existing)); Session result = sessionService.createSessionIfAbsent(user); Assertions.assertNotEquals(existing.getId(), result.getId()); Mockito.verify(sessionDao).deleteById(existing.getId()); } ``` For an integration reproduction, log into browser A, record its session ID, log into browser B with the same account before timeout, and then make an authenticated request from A using its original session ID. Based on the source, that ID should have been removed. These integration steps are provided for validation and were not executed in my environment. ### Anything else The history suggests a possible regression: 1. Before [PR #16989](https://github.com/apache/dolphinscheduler/pull/16989), the method named `isSessionExpire` used `elapsed <= timeout`. Despite the misleading name, the login caller therefore reused an unexpired session. See [the parent revision](https://github.com/apache/dolphinscheduler/blob/4416548/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/SessionServiceImpl.java). 2. PR #16989 fixed [Issue #16627](https://github.com/apache/dolphinscheduler/issues/16627) and changed the predicate to `elapsed >= timeout`, but retained the same login branch. The PR describes an expiry-check fix, rather than a change to concurrent-login policy. 3. [PR #18653](https://github.com/apache/dolphinscheduler/pull/18653), merged into dev, made the timeout configurable but left the login branch unchanged. If preserving the earlier reuse policy is correct, the login branch appears to need `!isSessionExpire(session)`. That proposed change is subject to maintainer confirmation of the intended policy. The current `testCreateSession` only asserts that the returned session and its ID are non-null; it does not assert reuse/replacement behavior. ### Version 3.4.3; also source-checked on dev at `7cead6c895dbd0bc9e04ea2300c22c6bdff4fcb9` on 2026-10-13. ### Are you willing to submit PR? - [ ] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
