Skip to content

fix(acl): carry the ACL user whitelist through user upserts - #2928

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-acl-user-whitelist
Open

fix(acl): carry the ACL user whitelist through user upserts#2928
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-acl-user-whitelist

Conversation

@yyqdbngt

@yyqdbngt yyqdbngt commented Sep 1, 2026

Copy link
Copy Markdown

Summary

  • map whiteRemoteAddress in MybatisPlusAclRepository.toUserEntity so the generic user upsert path persists the whitelist
  • add capture-based regression tests for saveUser and replaceUser

Why

toUserVO reads white_remote_address from the row, but toUserEntity never wrote it back: every upsert through the generic saveUser/replaceUser path silently dropped the whitelist carried on the VO. The plain-access endpoint worked around this by building its entity manually, but any caller of the generic upsert with a set whitelist lost it without an error. Existing updateUser behavior is unchanged because the merged VO carries a null whitelist and MyBatis-Plus omits null fields from updateById.

Testing

  • cd server && mvn -q -Dtest=MybatisPlusAclRepositoryTest test — all tests pass, including the new saveUserShouldPersistTheWhiteRemoteAddress and replaceUserShouldCarryTheWhiteRemoteAddressIntoTheUpdate
  • cd server && mvn -q -Dtest=AclServiceTest test — all tests pass (regression for the update flow)

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Carries the ACL user whitelist through user upsert operations so it is not silently dropped. Good bug fix — prevents ACL configuration loss during user updates.

Findings

  • [Info] The fix correctly preserves the whitelist field during the upsert path.

Suggestions

No blocking issues. Test coverage verifies the whitelist survives the round-trip.


Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants