yyqdbngt opened a new pull request, #5354: URL: https://github.com/apache/rocketmq-dashboard/pull/5354
### Which Issue(s) This PR Fixes - Fixes #5306 ### Brief Description The only session-revocation lever the console had was per account: `StudioUserController.revokeSessions` (base `StudioUserController.java:92`) delegates to `AuthService.revokeSessionsForUser` (base `AuthService.java:292`), which pins the statement to one `user_id` in `AuthService.revokeUserSessions` (base `AuthService.java:513`). Answering a suspected credential leak therefore meant opening every user's session drawer in turn, and an interrupted sweep left the incident half-contained. This change adds the bulk action behind the same admin-only surface: - `AuthService.revokeAllSessions(Long excludingUserId)` — one `UPDATE` over the *same* active-session predicate the per-user revoke uses (`revoked_at IS NULL AND expires_at > now`), so it is idempotent and never touches sessions that are already revoked or absolutely expired. A non-null `excludingUserId` adds `user_id <> ?` to that predicate. - `revokeUserSessions` now delegates to the shared private `revokeActiveSessions(userId, excludingUserId)`, so the per-user and the bulk revocation cannot drift apart; the per-user predicate itself is unchanged (`user_id = ?` is still added to the same three conditions, only the order of the generated conditions differs). - `POST /api/studio-users/sessions/revoke-all` returns the existing `StudioUserSessionRevokeVO`: `revokedSessionCount` is the number of invalidated rows and `userId` names the spared account, so the client can state who was spared. The endpoint is a non-`GET` under `/api/studio-users/**`, which `AuthInterceptor` already gates to administrators; readers get 403 before the handler, with no statement executed. - The acting administrator is read from `AuthenticatedUserContext.currentUserId()` (through `InstanceIds.parseLongOrNull`, the existing helper for numeric ids crossing the `String` API boundary), so the responder cannot sign themselves out mid-response. A request that carries no principal — login is not required by the deployment — passes `null` and spares nobody, which is the system-triggered case the issue asks for. - The database-backed contract mirrors the per-user revoke exactly: in the properties-based (no database) mode both throw the same `IllegalStateException`. Deliberately out of scope here, both because they live outside this change: the overview-card action and its confirmation dialog (`web/**`), and an audit row for the bulk revocation — operation-audit coverage for these controllers is proposed separately (#5311). Nothing else about the endpoint surface changes; `GET /sessions/overview` and the per-user routes are untouched. ### How Did You Test This Change? `StudioUserSessionRevokeAllTest` (new, 4 tests) runs a `StudioUserController` slice with the production `AuthInterceptor` active and a **real** `AuthService` over a mocked session mapper, so the statement the endpoint writes is what is asserted, not just the call: - `revokingEverySessionSparesTheOperatorWhoPulledTheLever` — an authenticated administrator (`userId=1`) revoking everything: response `data.userId=1`, `data.revokedSessionCount=12` (the mapper's affected-row count), and the captured `UpdateWrapper` sets `revoked_at`, filters `revoked_at IS NULL AND expires_at > now` and carries `user_id` with `1L` as its parameter, i.e. the operator is excluded rather than included. - `aCallWithoutAPrincipalRevokesEverySessionThereIs` — the same endpoint with login not required: the statement carries no `user_id` condition at all and reports the mapper's count. - `aReaderCannotReachTheBulkRevocation` — a non-admin principal is answered 403 and the session mapper is never touched. - `theBulkRevocationKeepsThePerUserContractOfADatabaseBackedDeployment` — the properties-based service refuses the bulk revocation with the same message the per-user revoke produces (`requires database persistence`). ```powershell $ cd server && mvn -B -ntp test -Dtest=StudioUserSessionRevokeAllTest [ERROR] Tests run: 4, Failures: 3, Errors: 0, Skipped: 0 <- before the fix [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 <- after the fix [INFO] BUILD SUCCESS ``` Raw red run (test file only, base revision): ``` [ERROR] Tests run: 4, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 12.19 s <<< FAILURE! -- in org.apache.rocketmq.studio.auth.StudioUserSessionRevokeAllTest java.lang.AssertionError: Status expected:<200> but was:<404> java.lang.AssertionError: Status expected:<200> but was:<404> [ERROR] StudioUserSessionRevokeAllTest.aCallWithoutAPrincipalRevokesEverySessionThereIs:149 Status expected:<200> but was:<404> [ERROR] StudioUserSessionRevokeAllTest.revokingEverySessionSparesTheOperatorWhoPulledTheLever:129 Status expected:<200> but was:<404> [ERROR] StudioUserSessionRevokeAllTest.theBulkRevocationKeepsThePerUserContractOfADatabaseBackedDeployment:176 java.lang.IllegalStateException: Method not found: org.apache.rocketmq.studio.auth.AuthService.revokeAllSessions(java.lang.Object) [ERROR] Tests run: 4, Failures: 3, Errors: 0, Skipped: 0 [INFO] BUILD FAILURE ``` (The reader test passes before and after: `AuthInterceptor` already answers 403 for any non-`GET` on `/api/studio-users/**`, which is what that test pins for the new route.) Green run after the change, with the neighbouring auth and session classes: ```powershell $ cd server && mvn -B -ntp test "-Dtest=StudioUserSessionRevokeAllTest,StudioUserControllerTest,AuthServiceDatabaseTest,AuthServiceTest,AuthControllerTest,AuthServicePasswordChangeIntegrationTest" [INFO] You have 0 Checkstyle violations. [INFO] Tests run: 67, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` The full server suite was also run to check for regressions in the sessions that were already green: ```powershell $ cd server && mvn -B -ntp test [INFO] Tests run: 3326, Failures: 8, Errors: 27, Skipped: 4 ``` (3326 = the 3322 tests of the base revision plus the 4 new ones; the 8 failures and 27 errors are the pre-existing MySQL-8 / external-CLI classes, identical to the base revision.) ### Checklist - [x] One coherent change; unrelated modifications are not bundled in - [x] Commit subject follows Conventional Commits - [x] Tests added or updated for non-trivial changes - [ ] New UI text has both Chinese and English entries under `web/src/i18n/` — does not apply, this change adds no UI text (the overview-card action is out of scope here) - [ ] Architecture constraints stay green — no ArchUnit rule class exists in the repository and this change adds none; the maven runs above report 0 checkstyle violations - [x] New source files carry the ASF license header - [ ] Documentation touched where behaviour changed — not ticked: the endpoint is documented in the description above; `docs/` is outside the file set this change was scoped to, so no document was edited there -- 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]
