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 216c2e320 fix(acl): answer a duplicate ACL username with 409 instead
of 500 (#5072)
216c2e320 is described below
commit 216c2e320fd6849c3ed5eb5cc534e32579601e45
Author: 烤化の初雪 <[email protected]>
AuthorDate: Thu Oct 1 17:27:59 2026 +0800
fix(acl): answer a duplicate ACL username with 409 instead of 500 (#5072)
`rmq_acl_user` carries `uk_username` and `uk_access_key`, but neither
write path translated the violation: the DuplicateKeyException travelled
to the generic advice and the console showed "Internal Server Error" for
what is a client mistake. Creating a user whose name exists, or renaming
a user onto one, both reach it.
Translate it the way the plain-access write in the same repository
already does, and cover both paths.
Co-authored-by: unbridled-41
<[email protected]>
---
.../instance/acl/MybatisPlusAclRepository.java | 25 +++++++++++--
.../instance/acl/MybatisPlusAclRepositoryTest.java | 42 ++++++++++++++++++++++
2 files changed, 65 insertions(+), 2 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 a7d7894df..a848bdbcf 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
@@ -149,7 +149,11 @@ public class MybatisPlusAclRepository implements
AclRepository {
if (entity.getId() != null && userMapper.selectById(entity.getId()) !=
null) {
userMapper.updateById(entity);
} else {
- userMapper.insert(entity);
+ try {
+ userMapper.insert(entity);
+ } catch (DuplicateKeyException exception) {
+ throw aclUserConflict(user.getUsername());
+ }
user.setId(entity.getId());
}
return user;
@@ -164,7 +168,14 @@ public class MybatisPlusAclRepository implements
AclRepository {
}
RmqAclUser entity = toUserEntity(user);
entity.setGmtCreate(existing.getGmtCreate());
- if (userMapper.updateById(entity) == 0) {
+ int updated;
+ try {
+ updated = userMapper.updateById(entity);
+ } catch (DuplicateKeyException exception) {
+ // Renaming onto an existing username hits `uk_username` just as
creating one does.
+ throw aclUserConflict(user.getUsername());
+ }
+ if (updated == 0) {
return Optional.empty();
}
if (user.getClusters() != null && entity.getClusters() == null) {
@@ -180,6 +191,16 @@ public class MybatisPlusAclRepository implements
AclRepository {
return id != null && userMapper.deleteById(id) > 0;
}
+ /**
+ * The {@code rmq_acl_user} unique keys ({@code uk_username}, {@code
uk_access_key}) make a
+ * duplicate a client mistake rather than a server fault: without this the
generic advice answers
+ * every one of them with 500 "Internal Server Error". Same answer as the
sibling duplicate-key
+ * paths ({@code createAndUpdatePlainAccessConfig} below, {@code
CloudCredentialService}).
+ */
+ private BusinessException aclUserConflict(String username) {
+ return new BusinessException(409, "ACL user already exists: " +
username);
+ }
+
/**
* Summarizes the ACL accounts provisioned in the dashboard store for a
cluster. This is a
* store-level view, not a live broker query: accounts are read from
{@code rmq_acl_user} /
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 0d8948afb..b78d4e340 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
@@ -255,6 +255,48 @@ class MybatisPlusAclRepositoryTest {
verify(userMapper, never()).update(any(), any());
}
+ @Test
+ void saveUserShouldReportADuplicateUsernameAsAConflict() {
+ when(userMapper.insert(any(RmqAclUser.class)))
+ .thenThrow(new
org.springframework.dao.DuplicateKeyException("uk_username"));
+
+ AclUserVO user = AclUserVO.builder()
+ .username("svc-a")
+ .accessKey("access-key")
+ .secretKey("secret-key")
+ .build();
+
+ // `rmq_acl_user` carries `uk_username`: a second user with the same
name is the operator's
+ // mistake, and the console has to read it as one (409) rather than as
a server failure.
+ assertThatThrownBy(() -> repository.saveUser(user))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("ACL user already exists: svc-a")
+ .satisfies(ex -> assertThat(((BusinessException)
ex).getCode()).isEqualTo(409));
+ }
+
+ @Test
+ void replaceUserShouldReportARenameOntoAnExistingUsernameAsAConflict() {
+ RmqAclUser existing = new RmqAclUser();
+ existing.setId(1L);
+ existing.setGmtCreate(LocalDateTime.of(2026, 1, 1, 0, 0));
+ when(userMapper.selectById(1L)).thenReturn(existing);
+ when(userMapper.updateById(any(RmqAclUser.class)))
+ .thenThrow(new
org.springframework.dao.DuplicateKeyException("uk_username"));
+
+ AclUserVO replacement = AclUserVO.builder()
+ .id(1L)
+ .username("taken")
+ .accessKey("access-key")
+ .secretKey("secret-key")
+ .build();
+
+ // The edit form renames a user, so the same unique key is reachable
from the update path.
+ assertThatThrownBy(() -> repository.replaceUser(replacement))
+ .isInstanceOf(BusinessException.class)
+ .hasMessage("ACL user already exists: taken")
+ .satisfies(ex -> assertThat(((BusinessException)
ex).getCode()).isEqualTo(409));
+ }
+
@Test
void replaceRuleShouldExplicitlyClearActionsWhenListIsEmpty() {
RmqAclRule existing = new RmqAclRule();
