This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 0c54d985f fix(acl): read rules by exact principal and keep the
whitelist on update (#4113)
0c54d985f is described below
commit 0c54d985f5d3f3cab068183226210ecb4a6ee5c0
Author: Zhao Jianing <[email protected]>
AuthorDate: Wed Sep 9 20:39:40 2026 +0800
fix(acl): read rules by exact principal and keep the whitelist on update
(#4113)
Two ACL account defects, folded into one change because they touch the same
account read/write path.
`toPlainAccessConfig` read an account's rules through `ruleQuery()`, whose
principal filter is a substring `LIKE`. Accounts whose access keys contain one
another were merged: reading `svc-a` also returned the rules of `svc-a-v2`, and
because `upsertPlainAccessRules` deletes by exact principal before writing, the
absorbed rules were written back verbatim onto the wrong account. The read now
uses an exact `eq("principal", ...)` wrapper with the same ordering, mirroring
the delete.
`AclService.updateUser` merged the incoming account onto the stored one but
never carried `whiteRemoteAddress` across, so the update response reported a
null whitelist even though `replaceUser` does not write that column at all. The
web ACL page replaces its local row with that response without refetching, so
the whitelist visibly disappeared until the next reload. The merged builder now
keeps the existing value.
Folded in from #4122, which made the same one-line whitelist fix
independently; that PR is closed as superseded by this one.
---
.../rocketmq/studio/instance/acl/AclService.java | 1 +
.../instance/acl/MybatisPlusAclRepository.java | 8 ++++-
.../studio/instance/acl/AclServiceTest.java | 15 +++++++++
.../instance/acl/MybatisPlusAclRepositoryTest.java | 37 ++++++++++++++++++++++
4 files changed, 60 insertions(+), 1 deletion(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java
index 31bf49c19..97b4b8dbe 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclService.java
@@ -208,6 +208,7 @@ public class AclService {
.secretKey(existing.getSecretKey())
.admin(user.getAdmin() == null ? existing.isAdmin() :
user.getAdmin())
.clusters(user.getClusters() == null ? existing.getClusters()
: user.getClusters())
+ .whiteRemoteAddress(existing.getWhiteRemoteAddress())
.gmtCreate(existing.getGmtCreate())
.build();
AclUserVO saved = aclRepository.replaceUser(merged)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
index 681fe84a4..0b569c922 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepository.java
@@ -342,7 +342,13 @@ public class MybatisPlusAclRepository implements
AclRepository {
}
private PlainAccessConfigVO toPlainAccessConfig(AclUserVO user) {
- List<AclRuleVO> userRules =
ruleMapper.selectList(ruleQuery(user.getAccessKey(), null, null, null, null))
+ // Exact principal match, mirroring the delete in
upsertPlainAccessRules: the
+ // substring LIKE in ruleQuery would also absorb rules of other
accounts whose
+ // accessKey contains this one (e.g. "svc-a" also matching "svc-a-v2").
+ List<AclRuleVO> userRules = ruleMapper.selectList(new
QueryWrapper<RmqAclRule>()
+ .eq("principal", user.getAccessKey())
+ .orderByDesc("gmt_create")
+ .orderByDesc("id"))
.stream()
.map(MybatisPlusAclRepository::toRuleVO)
.collect(Collectors.toList());
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclServiceTest.java
index 472d60ee3..b82582214 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclServiceTest.java
@@ -542,6 +542,21 @@ class AclServiceTest {
eq("username=newuser, admin=true"), eq("SUCCESS"), eq(null));
}
+ @Test
+ void updateUserShouldKeepTheExistingWhiteRemoteAddress() {
+ existingUser.setWhiteRemoteAddress("10.0.1.0/24");
+ UpdateAclUserDTO input = new UpdateAclUserDTO();
+ input.setId("1");
+ input.setUsername("renamed");
+
+
when(aclRepository.findUserById(1L)).thenReturn(Optional.of(existingUser));
+ when(aclRepository.replaceUser(any(AclUserVO.class))).thenAnswer(inv
-> Optional.of(inv.getArgument(0)));
+
+ AclUserVO result = aclService.updateUser(input, null);
+
+ assertThat(result.getWhiteRemoteAddress()).isEqualTo("10.0.1.0/24");
+ }
+
@Test
void updateUserShouldPreserveAdminWhenNotProvided() {
AclUserVO adminUser = AclUserVO.builder()
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
index 569d592ad..0d8948afb 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/MybatisPlusAclRepositoryTest.java
@@ -551,6 +551,28 @@ class MybatisPlusAclRepositoryTest {
assertThat(captor.getValue().getClusters()).isEqualTo("cluster-a,cluster-b");
}
+ @Test
+ void examineShouldNotAbsorbRulesOfAccountsWhoseAccessKeysOverlap() {
+ RmqAclUser user = userEntity(1L, "svc-a",
CredentialUtils.encodeBase64("secret-a-value"));
+
when(userMapper.selectList(any(QueryWrapper.class))).thenReturn(List.of(user));
+ RmqAclRule ownRule = plainRuleEntity("svc-a", "orders", "Topic",
"PUB");
+ RmqAclRule otherAccountRule = plainRuleEntity("svc-a-v2", "payments",
"Topic", "SUB");
+ // Simulate SQL semantics: a substring LIKE on the principal matches
both
+ // accounts, an exact equality only the requested one.
+
when(ruleMapper.selectList(any(QueryWrapper.class))).thenAnswer(invocation -> {
+ QueryWrapper<RmqAclRule> query = invocation.getArgument(0);
+ return query.getSqlSegment().contains("LIKE")
+ ? List.of(ownRule, otherAccountRule)
+ : List.of(ownRule);
+ });
+
+ AclClusterConfigVO config =
repository.examineBrokerClusterAclConfig("cluster-a");
+
+ assertThat(config.getAccounts()).hasSize(1);
+ PlainAccessConfigVO account = config.getAccounts().get(0);
+ assertThat(account.getTopicPerms()).containsExactly("orders=PUB");
+ }
+
private static RmqAclUser userEntity(Long id, String accessKey, String
encodedSecret) {
RmqAclUser entity = new RmqAclUser();
entity.setId(id);
@@ -561,4 +583,19 @@ class MybatisPlusAclRepositoryTest {
entity.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
return entity;
}
+
+ private static RmqAclRule plainRuleEntity(String principal, String
resource,
+ String resourceType, String actions) {
+ RmqAclRule rule = new RmqAclRule();
+ rule.setPrincipal(principal);
+ rule.setResource(resource);
+ rule.setResourceType(resourceType);
+ rule.setResourcePattern("LITERAL");
+ rule.setActions(actions);
+ rule.setDecision("ALLOW");
+ rule.setScope("*");
+ rule.setAclVersion("2.0");
+ rule.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
+ return rule;
+ }
}
