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 bf7f6690 fix: reject ACL rule updates for unknown IDs (#758)
bf7f6690 is described below
commit bf7f6690c1cb3b493996b29be96006a8c49bed51
Author: Rui <[email protected]>
AuthorDate: Mon Aug 3 11:36:53 2026 +0800
fix: reject ACL rule updates for unknown IDs (#758)
---
.../studio/instance/acl/AclRepository.java | 6 +++
.../rocketmq/studio/instance/acl/AclService.java | 6 +--
.../studio/instance/acl/InMemoryAclRepository.java | 25 +++++++++
.../studio/instance/acl/AclControllerTest.java | 20 +++++++
.../studio/instance/acl/AclServiceTest.java | 62 ++++++++++++++++++++--
5 files changed, 111 insertions(+), 8 deletions(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclRepository.java
index bc1a9514..17325b05 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/AclRepository.java
@@ -25,6 +25,12 @@ public interface AclRepository {
AclRuleVO saveRule(AclRuleVO rule);
+ /**
+ * Replaces an existing rule atomically without creating a missing rule.
+ * Implementations must preserve the original creation timestamp.
+ */
+ Optional<AclRuleVO> replaceRule(AclRuleVO rule);
+
void deleteRule(String id);
List<AclUserVO> findUsers();
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 30aa35c2..3fcec4d3 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
@@ -61,10 +61,8 @@ public class AclService {
throw new BusinessException(400, "ACL rule id is required");
}
log.info("Updating ACL rule id={}, principal={}", rule.getId(),
rule.getPrincipal());
- if (rule.getCreatedAt() == null) {
- rule.setCreatedAt(LocalDateTime.now());
- }
- return aclRepository.saveRule(rule);
+ return aclRepository.replaceRule(rule)
+ .orElseThrow(() -> new BusinessException(404, "ACL rule not
found: " + rule.getId()));
}
public void deleteRule(String id) {
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/InMemoryAclRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/InMemoryAclRepository.java
index 3f1fece9..efe6e8f6 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/instance/acl/InMemoryAclRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/instance/acl/InMemoryAclRepository.java
@@ -19,6 +19,7 @@ package org.apache.rocketmq.studio.instance.acl;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
+import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -48,6 +49,30 @@ public class InMemoryAclRepository implements AclRepository {
return rule;
}
+ @Override
+ public Optional<AclRuleVO> replaceRule(AclRuleVO rule) {
+ String id = rule.getId();
+ AclRuleVO replacedRule = rules.computeIfPresent(id,
+ (key, existingRule) -> copyRule(rule, key,
existingRule.getCreatedAt()));
+ log.debug("Replaced ACL rule id={}, replaced={}", id, replacedRule !=
null);
+ return Optional.ofNullable(replacedRule);
+ }
+
+ private AclRuleVO copyRule(AclRuleVO rule, String id, LocalDateTime
createdAt) {
+ return AclRuleVO.builder()
+ .id(id)
+ .principal(rule.getPrincipal())
+ .resource(rule.getResource())
+ .resourceType(rule.getResourceType())
+ .resourcePattern(rule.getResourcePattern())
+ .actions(rule.getActions() == null ? null : new
ArrayList<>(rule.getActions()))
+ .decision(rule.getDecision())
+ .scope(rule.getScope())
+ .aclVersion(rule.getAclVersion())
+ .createdAt(createdAt)
+ .build();
+ }
+
@Override
public void deleteRule(String id) {
rules.remove(id);
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclControllerTest.java
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclControllerTest.java
index 8c6a75ed..9f780696 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclControllerTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/instance/acl/AclControllerTest.java
@@ -18,6 +18,7 @@
package org.apache.rocketmq.studio.instance.acl;
import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.rocketmq.studio.common.exception.BusinessException;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import
org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
@@ -139,6 +140,25 @@ class AclControllerTest {
.andExpect(jsonPath("$.data.decision").value("DENY"));
}
+ @Test
+ void updateRuleShouldReturnNotFoundForUnknownId() throws Exception {
+ AclRuleVO input = AclRuleVO.builder()
+ .id("missing-rule")
+ .principal("user1")
+ .resource("topic-1")
+ .decision("DENY")
+ .build();
+ when(aclService.updateRule(any(AclRuleVO.class)))
+ .thenThrow(new BusinessException(404, "ACL rule not found:
missing-rule"));
+
+ mockMvc.perform(post("/api/acl/rules/update")
+ .contentType(MediaType.APPLICATION_JSON)
+ .content(objectMapper.writeValueAsString(input)))
+ .andExpect(status().isNotFound())
+ .andExpect(jsonPath("$.code").value(404))
+ .andExpect(jsonPath("$.message").value("ACL rule not found:
missing-rule"));
+ }
+
@Test
void deleteRuleShouldPassValidatedRequest() throws Exception {
mockMvc.perform(post("/api/acl/rules/delete")
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 7e64714d..9803eb36 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
@@ -29,6 +29,7 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
+import java.time.LocalDateTime;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
@@ -127,22 +128,75 @@ class AclServiceTest {
}
@Test
- void updateRuleShouldSaveExistingRule() {
+ void updateRuleShouldReplaceExistingRule() {
+ LocalDateTime createdAt = LocalDateTime.of(2026, 1, 1, 0, 0);
AclRuleVO input = AclRuleVO.builder()
.id("rule-1")
.principal("user1")
.resource("topic-1")
.decision("DENY")
+ .createdAt(createdAt)
.build();
- when(aclRepository.saveRule(any(AclRuleVO.class))).thenAnswer(inv ->
inv.getArgument(0));
+ when(aclRepository.replaceRule(input)).thenReturn(Optional.of(input));
AclRuleVO result = aclService.updateRule(input);
assertThat(result.getId()).isEqualTo("rule-1");
- assertThat(result.getCreatedAt()).isNotNull();
+ assertThat(result.getCreatedAt()).isEqualTo(createdAt);
assertThat(result.getDecision()).isEqualTo("DENY");
- verify(aclRepository).saveRule(any(AclRuleVO.class));
+ verify(aclRepository).replaceRule(input);
+ verify(aclRepository, never()).saveRule(any(AclRuleVO.class));
+ }
+
+ @Test
+ void updateRuleShouldRejectUnknownIdInsteadOfCreatingRule() {
+ InMemoryAclRepository repository = new InMemoryAclRepository();
+ AclService service = new AclService(repository);
+ AclRuleVO update = AclRuleVO.builder()
+ .id("missing-rule")
+ .principal("orders")
+ .resource("orders-topic")
+ .decision("DENY")
+ .build();
+
+ assertThatThrownBy(() -> service.updateRule(update))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("ACL rule not found: missing-rule")
+ .satisfies(ex -> assertThat(((BusinessException)
ex).getCode()).isEqualTo(404));
+ assertThat(service.listRules(null, null)).isEmpty();
+ }
+
+ @Test
+ void updateRuleShouldPreserveStoredCreationTimestamp() {
+ InMemoryAclRepository repository = new InMemoryAclRepository();
+ AclService service = new AclService(repository);
+ AclRuleVO created = service.createRule(AclRuleVO.builder()
+ .principal("orders")
+ .resource("orders-topic")
+ .decision("ALLOW")
+ .build());
+ LocalDateTime originalCreatedAt = created.getCreatedAt();
+
+ LocalDateTime clientCreatedAt = originalCreatedAt.plusDays(1);
+ AclRuleVO update = AclRuleVO.builder()
+ .id(created.getId())
+ .principal("orders")
+ .resource("orders-topic")
+ .decision("DENY")
+ .createdAt(clientCreatedAt)
+ .build();
+
+ AclRuleVO updated = service.updateRule(update);
+ AclRuleVO stored = service.listRules(null, null).get(0);
+
+ assertThat(updated.getCreatedAt()).isEqualTo(originalCreatedAt);
+ assertThat(updated.getDecision()).isEqualTo("DENY");
+ assertThat(update.getCreatedAt()).isEqualTo(clientCreatedAt);
+ assertThat(service.listRules(null, null)).hasSize(1);
+ assertThat(stored.getId()).isEqualTo(created.getId());
+ assertThat(stored.getDecision()).isEqualTo("DENY");
+ assertThat(stored.getCreatedAt()).isEqualTo(originalCreatedAt);
}
@Test
