You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by li...@apache.org on 2026/09/07 09:54:44 UTC
(rocketmq-dashboard) branch rocketmq-studio updated: fix(acl): assign cleared list columns explicitly when updating users and rules (#3342)
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 12d8f9c15 fix(acl): assign cleared list columns explicitly when updating users and rules (#3342)
12d8f9c15 is described below
commit 12d8f9c152f884431fa7873a92093bda8b318ad3
Author: 烤化の初雪 <ya...@outlook.com>
AuthorDate: Mon Sep 7 17:54:37 2026 +0800
fix(acl): assign cleared list columns explicitly when updating users and rules (#3342)
MyBatis-Plus omits null entity fields from updateById, so an update that
clears a user's cluster bindings (or a rule's actions) produced a null
column value that was skipped, silently retaining the previous CSV list
while the API response reported the cleared value. Assign the column
explicitly for the cleared case, mirroring the existing workaround for
white_remote_address.
Co-authored-by: unbridled-41 <17...@users.noreply.github.com>
---
.../instance/acl/MybatisPlusAclRepository.java | 27 ++++++--
.../instance/acl/MybatisPlusAclRepositoryTest.java | 72 ++++++++++++++++++++++
2 files changed, 94 insertions(+), 5 deletions(-)
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 8168211e6..681fe84a4 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
@@ -16,6 +16,7 @@
*/
package org.apache.rocketmq.studio.instance.acl;
+import com.baomidou.mybatisplus.core.mapper.BaseMapper;
import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
@@ -79,6 +80,7 @@ public class MybatisPlusAclRepository implements AclRepository {
}
@Override
+ @Transactional
public Optional<AclRuleVO> replaceRule(AclRuleVO rule) {
RmqAclRule existing = ruleMapper.selectById(rule.getId());
if (existing == null) {
@@ -89,6 +91,10 @@ public class MybatisPlusAclRepository implements AclRepository {
if (ruleMapper.updateById(entity) == 0) {
return Optional.empty();
}
+ if (rule.getActions() != null && entity.getActions() == null) {
+ // Clearing the actions list must persist as a null column; updateById skips null fields.
+ clearColumn(ruleMapper, entity.getId(), "actions");
+ }
rule.setGmtCreate(existing.getGmtCreate());
return Optional.of(rule);
}
@@ -144,6 +150,7 @@ public class MybatisPlusAclRepository implements AclRepository {
}
@Override
+ @Transactional
public Optional<AclUserVO> replaceUser(AclUserVO user) {
RmqAclUser existing = userMapper.selectById(user.getId());
if (existing == null) {
@@ -154,6 +161,10 @@ public class MybatisPlusAclRepository implements AclRepository {
if (userMapper.updateById(entity) == 0) {
return Optional.empty();
}
+ if (user.getClusters() != null && entity.getClusters() == null) {
+ // Clearing the cluster bindings must persist as a null column; updateById skips null fields.
+ clearColumn(userMapper, entity.getId(), "clusters");
+ }
user.setGmtCreate(existing.getGmtCreate());
return Optional.of(user);
}
@@ -231,11 +242,8 @@ public class MybatisPlusAclRepository implements AclRepository {
if (existing != null) {
userMapper.updateById(entity);
if (entity.getWhiteRemoteAddress() == null) {
- // MyBatis-Plus omits null entity fields from updateById. Assign this column
- // explicitly so clearing the whitelist does not silently retain its old value.
- userMapper.update(null, new UpdateWrapper<RmqAclUser>()
- .eq("id", entity.getId())
- .set("white_remote_address", null));
+ // Clearing the whitelist must persist as a null column; updateById skips null fields.
+ clearColumn(userMapper, entity.getId(), "white_remote_address");
}
} else {
try {
@@ -274,6 +282,15 @@ public class MybatisPlusAclRepository implements AclRepository {
return trimmed.isEmpty() ? null : trimmed;
}
+ /**
+ * MyBatis-Plus {@code updateById} omits null entity fields, so an update that clears a column
+ * to null is silently skipped and the previous value is retained. Assign the column explicitly
+ * so emptied list/whitelist columns persist as null.
+ */
+ private static <T> void clearColumn(BaseMapper<T> mapper, Long id, String column) {
+ mapper.update(null, new UpdateWrapper<T>().eq("id", id).set(column, null));
+ }
+
private void upsertPlainAccessRules(PlainAccessConfigVO config) {
ruleMapper.delete(new QueryWrapper<RmqAclRule>()
.eq("principal", config.getAccessKey())
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 ee5c17f90..569d592ad 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
@@ -209,6 +209,78 @@ class MybatisPlusAclRepositoryTest {
verify(userMapper, never()).insert(any(RmqAclUser.class));
}
+ @Test
+ void replaceUserShouldExplicitlyClearClusterBindingsWhenListIsEmpty() {
+ RmqAclUser existing = new RmqAclUser();
+ existing.setId(1L);
+ existing.setClusters("cluster-a,cluster-b");
+ existing.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
+ when(userMapper.selectById(1L)).thenReturn(existing);
+ when(userMapper.updateById(any(RmqAclUser.class))).thenReturn(1);
+ when(userMapper.update(isNull(), any(UpdateWrapper.class))).thenReturn(1);
+
+ AclUserVO replacement = AclUserVO.builder()
+ .id(1L)
+ .username("svc-a")
+ .accessKey("access-key")
+ .secretKey("secret-key")
+ .clusters(List.of())
+ .build();
+
+ assertThat(repository.replaceUser(replacement)).isPresent();
+
+ @SuppressWarnings("rawtypes")
+ ArgumentCaptor<UpdateWrapper> captor = ArgumentCaptor.forClass(UpdateWrapper.class);
+ verify(userMapper).update(isNull(), captor.capture());
+ assertThat(captor.getValue().getSqlSet()).contains("clusters");
+ assertThat(captor.getValue().getParamNameValuePairs()).containsValue(null);
+ }
+
+ @Test
+ void replaceUserShouldKeepClusterBindingsWhenNoneProvided() {
+ RmqAclUser existing = new RmqAclUser();
+ existing.setId(1L);
+ existing.setClusters("cluster-a");
+ existing.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
+ when(userMapper.selectById(1L)).thenReturn(existing);
+ when(userMapper.updateById(any(RmqAclUser.class))).thenReturn(1);
+
+ AclUserVO replacement = AclUserVO.builder()
+ .id(1L)
+ .username("renamed")
+ .build();
+
+ assertThat(repository.replaceUser(replacement)).isPresent();
+
+ verify(userMapper, never()).update(any(), any());
+ }
+
+ @Test
+ void replaceRuleShouldExplicitlyClearActionsWhenListIsEmpty() {
+ RmqAclRule existing = new RmqAclRule();
+ existing.setId(1L);
+ existing.setActions("PUB,SUB");
+ existing.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
+ when(ruleMapper.selectById(1L)).thenReturn(existing);
+ when(ruleMapper.updateById(any(RmqAclRule.class))).thenReturn(1);
+ when(ruleMapper.update(isNull(), any(UpdateWrapper.class))).thenReturn(1);
+
+ AclRuleVO replacement = AclRuleVO.builder()
+ .id(1L)
+ .principal("svc-a")
+ .resource("orders")
+ .actions(List.of())
+ .build();
+
+ assertThat(repository.replaceRule(replacement)).isPresent();
+
+ @SuppressWarnings("rawtypes")
+ ArgumentCaptor<UpdateWrapper> captor = ArgumentCaptor.forClass(UpdateWrapper.class);
+ verify(ruleMapper).update(isNull(), captor.capture());
+ assertThat(captor.getValue().getSqlSet()).contains("actions");
+ assertThat(captor.getValue().getParamNameValuePairs()).containsValue(null);
+ }
+
@Test
void upsertShouldAssignUniqueRuleIdPerPermission() {
when(userMapper.selectList(any(QueryWrapper.class))).thenReturn(List.of());