btlqql opened a new pull request, #4689: URL: https://github.com/apache/rocketmq-dashboard/pull/4689
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. --> ### Brief Description `AuthService.changePassword` answered `401 Current password is incorrect` (server/src/main/java/org/apache/rocketmq/studio/auth/AuthService.java:351) when the submitted current password did not match. The request is already authenticated at that point - the interceptor resolved the session before the controller ran - so the failing input is a payload field, not the session. `GlobalExceptionHandler` turns the exception code straight into the HTTP status (`GlobalExceptionHandler.java:45`), so that answer reaches the browser as `HTTP 401`. In this API `401` means "no valid session", and the shipped client keys on it: any 401 outside `/auth/login` and `/auth/status` makes it drop the stored session and redirect to the login page (`web/src/api/client.ts:103`, `PUBLIC_AUTH_PATHS` at `client.ts:24`). An operator changing their own password who mistypes the current password was therefore logged out of a perfectly valid session instead of being told the password did not match. The Studio user-management dialog calls exactly that endpoint (`web/src/pages/studio/UserManagement.tsx:291`). The status is now `400`, which is what this endpoint already uses for its other payload problem: the same field answers `400` when it is blank (`ChangePasswordDTO.currentPassword` is `@NotBlank`) and an out-of-policy new password answers `400` as well. The message is unchanged, so nothing else in the contract moves; a wrong current password is still rejected before the hash is touched. ### How Did You Test This Change? New test class `AuthPasswordChangeStatusIntegrationTest`. It seeds a user on the `dev` profile (H2 in MySQL compatibility mode fed by `classpath:db/schema.sql`), logs in for real, and drives `POST /api/auth/password` through MockMvc and the registered `AuthInterceptor` with the resulting bearer token. A controller slice would not do: the status under test is produced by the global exception handler. The second case is the control - a blank current password already answered `400` before this change. Before the fix (red), on the unmodified tree: ``` $ cd server && mvn -B -ntp test -Dtest=AuthPasswordChangeStatusIntegrationTest [ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0 [ERROR] AuthPasswordChangeStatusIntegrationTest.aWrongCurrentPasswordIsAPayloadErrorAndLeavesTheSessionUsableTest:92 Status expected:<400> but was:<401> ``` The control case (`aBlankCurrentPasswordIsRejectedAsAPayloadErrorTest`) passes on the base tree, which is the point: the same field, submitted blank, already answered `400`, so the `401` was not a deliberate contract for this input. After the fix (green) - the new class plus the neighbouring controller/service classes: ``` $ cd server && mvn -B -ntp test -Dtest='AuthPasswordChangeStatusIntegrationTest,AuthServiceDatabaseTest,AuthControllerTest,AuthServiceTest,StudioUserControllerTest' [INFO] Tests run: 9, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.auth.AuthControllerTest [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.auth.AuthPasswordChangeStatusIntegrationTest [INFO] Tests run: 27, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.auth.AuthServiceDatabaseTest [INFO] Tests run: 16, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.auth.AuthServiceTest [INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.rocketmq.studio.auth.StudioUserControllerTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS [INFO] You have 0 Checkstyle violations. ``` Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp test` already reports `Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes are the MySQL 8 backed Spring integration tests (`AuthServiceBootstrapIntegrationTest`, `AuthServiceConcurrencyIntegrationTest`, `AuthServiceSessionOverviewIntegrationTest`, `HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`, `NativeAlertEvaluationTransactionTest`, `NotificationOutboxMapperIntegrationTest`, `RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the external-CLI ones (`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are touched by this change. ### Checklist - [x] One coherent change; unrelated modifications are not bundled in - [x] Commit subject follows Conventional Commits (`fix:`) - [x] Tests added or updated for non-trivial changes, test methods named `...Test` - [x] New UI text has both Chinese and English entries under `web/src/i18n/` (no UI text in this change) - [x] Architecture constraints stay green (`mvn test` runs the ArchUnit checks) - [x] New source files carry the ASF license header - [x] Documentation touched where behaviour changed (the error text is unchanged; only the status of a rejected payload moved from 401 to 400, matching the blank-field case) -- 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]
