[PR] fix(acl): keep the account whitelist in the update response [rocketmq-dashboard]

["zjncs (via GitHub)" <gi...@apache.org>]

zjncs opened a new pull request, #4122:
URL: https://github.com/apache/rocketmq-dashboard/pull/4122

   ### Motivation
   
   `AclService.updateUser` (local plain-access path) carefully preserves every existing field of the account through a partial update — `accessKey`, `secretKey`, `admin`, `clusters`, `gmtCreate` — but the merge builder omits `whiteRemoteAddress`:
   
   ```java
   AclUserVO merged = AclUserVO.builder()
           .id(existing.getId())
           .username(...)
           .accessKey(existing.getAccessKey())
           .secretKey(existing.getSecretKey())
           .admin(...)
           .clusters(...)
           .gmtCreate(existing.getGmtCreate())
           .build();
   ```
   
   `whiteRemoteAddress` is a real, persisted field of local plain-access accounts (`rmq_acl_user.white_remote_address`, written by `createAndUpdatePlainAccessConfig`, read back by `toUserVO`). The stored row survives the update (MyBatis-Plus `updateById` skips null columns), but `replaceUser` returns the merged VO, so `POST /api/acl/users/update` responds with `whiteRemoteAddress: null` after a harmless rename — the API reports the account's IP whitelist as gone even though it is still enforced.
   
   ### Modifications
   
   Carry `existing.getWhiteRemoteAddress()` through the merge, like every other preserved field.
   
   ### Verification
   
   New test `updateUserShouldKeepTheExistingWhiteRemoteAddress` (model: `updateUserShouldSaveExistingUser`): existing user with `whiteRemoteAddress = "10.0.1.0/24"`, rename via DTO.
   - Before the fix: fails — `whiteRemoteAddress` in the response is `null`.
   - After the fix: passes.
   - ACL suite regression: `mvn -f server/pom.xml test -Dtest='AclServiceTest,MybatisPlusAclRepositoryTest,AclControllerTest'` → **Tests run: 114, Failures: 0, Errors: 0**.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(acl): keep the account whitelist in the update response [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins commented on PR #4122:
URL: https://github.com/apache/rocketmq-dashboard/pull/4122#issuecomment-5601979563

   Closing as folded into #4113, which was merged into `rocketmq-studio` as 0c54d985.
   
   Your diagnosis and your fix here were correct and are now on the branch: `AclService.updateUser` merges the incoming account onto the stored one but never carried `whiteRemoteAddress` across, so the update response reported a null whitelist even though `MybatisPlusAclRepository.toUserEntity` does not map `white_remote_address` at all — meaning the column is untouched by `replaceUser` and this was purely a response-shape defect. That matters in practice because `web/src/pages/instance/acl.tsx` replaces its local row with the response and does not refetch, so the whitelist visibly disappeared until the next reload. The one-line `.whiteRemoteAddress(existing.getWhiteRemoteAddress())` in the merged builder is exactly what shipped.
   
   The reason this landed inside #4113 rather than on its own: both PRs fix the same ACL account read/write path, arrived within an hour of each other from the same author, and neither touches a file the other does. We group related same-author changes into one commit on the target PR so the branch history stays readable, and #4113 was the target here. Your `updateUserShouldKeepTheExistingWhiteRemoteAddress` test came across with the fix and passes as part of `AclServiceTest` (65 tests green).
   
   Nothing further needed from you — thank you for the fix and for the precise write-up of why a null whitelist in the response is user-visible rather than cosmetic.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(acl): keep the account whitelist in the update response [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins closed pull request #4122: fix(acl): keep the account whitelist in the update response
URL: https://github.com/apache/rocketmq-dashboard/pull/4122


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org