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 6b4b97b98 fix(credentials): preserve concurrent field updates during
rotation (#5254)
6b4b97b98 is described below
commit 6b4b97b98fc66508676ec3c6fc48153c1ffcf2fb
Author: zmuxuny <[email protected]>
AuthorDate: Fri Oct 9 23:39:35 2026 -0700
fix(credentials): preserve concurrent field updates during rotation (#5254)
Rotating a cloud credential read the whole row, changed the secret and
wrote the row back, so a
concurrent rename was lost. The update is now a conditional SET of only the
fields being changed,
across the interface, the implementation and the service.
26 tests green, 0 checkstyle violations.
---
.../credential/CloudCredentialRepository.java | 3 +-
.../credential/CloudCredentialService.java | 25 +++----
.../MybatisPlusCloudCredentialRepository.java | 11 ++-
...loudCredentialPartialUpdateIntegrationTest.java | 64 +++++++++++++++++
.../credential/CloudCredentialServiceTest.java | 82 +++++++++++++++++++++-
.../MybatisPlusCloudCredentialRepositoryTest.java | 46 ++++++++++++
6 files changed, 210 insertions(+), 21 deletions(-)
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialRepository.java
index 44a03085f..f61f105b7 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialRepository.java
@@ -33,7 +33,8 @@ public interface CloudCredentialRepository {
CloudCredentialVO save(CloudCredentialVO credential);
- boolean replace(CloudCredentialVO credential);
+ /** A null field is unchanged; an empty remark clears the stored remark. */
+ boolean updateFields(Long id, String name, String secretKey, String
remark);
boolean deleteById(Long id);
}
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialService.java
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialService.java
index 1d30f548a..6c5e729e8 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialService.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialService.java
@@ -117,28 +117,21 @@ public class CloudCredentialService {
throw new BusinessException(400, "Cloud credential id is
required");
}
log.info("Updating cloud credential id={}", request.getId());
- CloudCredentialVO existing =
credentialRepository.findById(request.getId())
+ credentialRepository.findById(request.getId())
.orElseThrow(() -> new BusinessException(404, "Cloud
credential not found: " + request.getId()));
if (request.getName() != null && request.getName().isBlank()) {
throw new BusinessException(400, "Cloud credential name cannot be
blank");
}
- if (request.getName() != null) {
- existing.setName(request.getName());
- }
- if (request.getSecretKey() != null &&
!request.getSecretKey().isBlank()) {
- existing.setSecretKey(request.getSecretKey());
- }
- if (request.getRemark() != null) {
- existing.setRemark(request.getRemark());
- }
- existing.setGmtModified(LocalDateTime.now());
- if (!credentialRepository.replace(existing)) {
+ String secretKey = StringUtils.hasText(request.getSecretKey()) ?
request.getSecretKey() : null;
+ if (!credentialRepository.updateFields(request.getId(),
request.getName(), secretKey, request.getRemark())) {
throw new BusinessException(404, "Cloud credential not found: " +
request.getId());
}
- invalidateCloudClients(existing);
- recordAudit("UPDATE_CLOUD_CREDENTIAL", "CLOUD_CREDENTIAL",
String.valueOf(existing.getId()), null,
- credentialAuditDetail(existing));
- return maskAccessKey(existing);
+ CloudCredentialVO updated =
credentialRepository.findById(request.getId())
+ .orElseThrow(() -> new BusinessException(404, "Cloud
credential not found: " + request.getId()));
+ invalidateCloudClients(updated);
+ recordAudit("UPDATE_CLOUD_CREDENTIAL", "CLOUD_CREDENTIAL",
String.valueOf(updated.getId()), null,
+ credentialAuditDetail(updated));
+ return maskAccessKey(updated);
}
public void delete(Long id) {
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepository.java
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepository.java
index b010fb3e0..ccb8a1c83 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepository.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepository.java
@@ -17,6 +17,7 @@
package org.apache.rocketmq.studio.provider.credential;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
import org.apache.rocketmq.studio.common.domain.PageResult;
import org.apache.rocketmq.studio.common.domain.enums.InstanceVendor;
@@ -98,8 +99,14 @@ public class MybatisPlusCloudCredentialRepository implements
CloudCredentialRepo
}
@Override
- public boolean replace(CloudCredentialVO credential) {
- return credential.getId() != null &&
credentialMapper.updateById(toEntity(credential)) > 0;
+ public boolean updateFields(Long id, String name, String secretKey, String
remark) {
+ UpdateWrapper<RmqCloudCredential> update = new
UpdateWrapper<RmqCloudCredential>()
+ .eq("id", id)
+ .set(name != null, "name", name)
+ .set(secretKey != null, "secret_key",
CredentialUtils.encodeBase64(secretKey))
+ .set(remark != null, "remark", remark)
+ .set("gmt_modified", LocalDateTime.now());
+ return credentialMapper.update(null, update) > 0;
}
@Override
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialPartialUpdateIntegrationTest.java
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialPartialUpdateIntegrationTest.java
new file mode 100644
index 000000000..4431f07fb
--- /dev/null
+++
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialPartialUpdateIntegrationTest.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.provider.credential;
+
+import org.apache.rocketmq.studio.common.util.CredentialUtils;
+import org.apache.rocketmq.studio.persistence.entity.RmqCloudCredential;
+import org.apache.rocketmq.studio.persistence.mapper.RmqCloudCredentialMapper;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+
+import java.time.LocalDateTime;
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@SpringBootTest(properties = {"studio.auth.login-required=false"})
+class CloudCredentialPartialUpdateIntegrationTest {
+
+ @Autowired
+ private CloudCredentialRepository repository;
+
+ @Autowired
+ private RmqCloudCredentialMapper mapper;
+
+ @Test
+ void nameOnlyUpdateAfterRotationPreservesSecretTest() {
+ RmqCloudCredential row = new RmqCloudCredential();
+ row.setName("production");
+ row.setVendor("ALIYUN");
+ row.setAccessKey("integration-" + UUID.randomUUID());
+ row.setSecretKey(CredentialUtils.encodeBase64("old-secret"));
+ row.setGmtCreate(LocalDateTime.now());
+ row.setGmtModified(LocalDateTime.now());
+ mapper.insert(row);
+ try {
+ assertThat(repository.updateFields(row.getId(), null,
"new-secret", null)).isTrue();
+ assertThat(repository.updateFields(row.getId(), "renamed", null,
"")).isTrue();
+
+ RmqCloudCredential persisted = mapper.selectById(row.getId());
+ assertThat(persisted.getName()).isEqualTo("renamed");
+
assertThat(persisted.getSecretKey()).isEqualTo(CredentialUtils.encodeBase64("new-secret"));
+ assertThat(persisted.getRemark()).isEmpty();
+
assertThat(repository.findById(row.getId()).orElseThrow().getSecretKey())
+ .isEqualTo("new-secret");
+ } finally {
+ mapper.deleteById(row.getId());
+ }
+ }
+}
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialServiceTest.java
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialServiceTest.java
index 4f5a1e027..544c26f94 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialServiceTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/CloudCredentialServiceTest.java
@@ -34,6 +34,13 @@ import org.springframework.dao.DuplicateKeyException;
import java.util.List;
import java.util.Optional;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -186,7 +193,7 @@ class CloudCredentialServiceTest {
stored.setAccessKey("LTAI5tUpdateKey000000001");
stored.setSecretKey("old-secret");
when(credentialRepository.findById(1L)).thenReturn(Optional.of(stored));
-
when(credentialRepository.replace(any(CloudCredentialVO.class))).thenReturn(true);
+ when(credentialRepository.updateFields(1L, null, "new-secret",
null)).thenReturn(true);
UpdateCloudCredentialDTO request = new UpdateCloudCredentialDTO();
request.setId(1L);
@@ -194,11 +201,82 @@ class CloudCredentialServiceTest {
service.update(request);
+ verify(credentialRepository).updateFields(1L, null, "new-secret",
null);
verify(aliyunClientFactory).invalidateCredential(1L);
verify(operationAuditService).record(eq("UPDATE_CLOUD_CREDENTIAL"),
eq("CLOUD_CREDENTIAL"),
eq("1"), eq(null), eq("name=null, vendor=ALIYUN"),
eq("SUCCESS"), eq(null));
}
+ @Test
+ void concurrentNameEditShouldNotUndoSecretRotationTest() throws Exception {
+ CloudCredentialVO original = new CloudCredentialVO();
+ original.setId(1L);
+ original.setName("production");
+ original.setVendor(InstanceVendor.ALIYUN);
+ original.setAccessKey("LTAI5tUpdateKey000000001");
+ original.setSecretKey("old-secret");
+ AtomicReference<CloudCredentialVO> persisted = new
AtomicReference<>(original);
+ AtomicInteger reads = new AtomicInteger();
+ CountDownLatch bothReadOldRow = new CountDownLatch(2);
+ CountDownLatch rotationSaved = new CountDownLatch(1);
+
+ when(credentialRepository.findById(1L)).thenAnswer(invocation -> {
+ CloudCredentialVO snapshot = copyCredential(persisted.get());
+ if (reads.incrementAndGet() <= 2) {
+ bothReadOldRow.countDown();
+ assertThat(bothReadOldRow.await(5, TimeUnit.SECONDS)).isTrue();
+ }
+ return Optional.of(snapshot);
+ });
+ when(credentialRepository.updateFields(eq(1L), any(), any(),
any())).thenAnswer(invocation -> {
+ String name = invocation.getArgument(1);
+ String secretKey = invocation.getArgument(2);
+ String remark = invocation.getArgument(3);
+ if (name != null) {
+ assertThat(rotationSaved.await(5, TimeUnit.SECONDS)).isTrue();
+ }
+ persisted.updateAndGet(current -> {
+ CloudCredentialVO patched = copyCredential(current);
+ if (name != null) patched.setName(name);
+ if (secretKey != null) patched.setSecretKey(secretKey);
+ if (remark != null) patched.setRemark(remark);
+ return patched;
+ });
+ if (secretKey != null) rotationSaved.countDown();
+ return true;
+ });
+
+ UpdateCloudCredentialDTO rotation = new UpdateCloudCredentialDTO();
+ rotation.setId(1L);
+ rotation.setSecretKey("new-secret");
+ UpdateCloudCredentialDTO rename = new UpdateCloudCredentialDTO();
+ rename.setId(1L);
+ rename.setName("renamed");
+
+ ExecutorService pool = Executors.newFixedThreadPool(2);
+ try {
+ Future<?> rotationResult = pool.submit(() ->
service.update(rotation));
+ Future<?> renameResult = pool.submit(() -> service.update(rename));
+ rotationResult.get(10, TimeUnit.SECONDS);
+ renameResult.get(10, TimeUnit.SECONDS);
+ } finally {
+ pool.shutdownNow();
+ }
+ assertThat(persisted.get().getName()).isEqualTo("renamed");
+ assertThat(persisted.get().getSecretKey()).isEqualTo("new-secret");
+ }
+
+ private static CloudCredentialVO copyCredential(CloudCredentialVO source) {
+ CloudCredentialVO copy = new CloudCredentialVO();
+ copy.setId(source.getId());
+ copy.setName(source.getName());
+ copy.setVendor(source.getVendor());
+ copy.setAccessKey(source.getAccessKey());
+ copy.setSecretKey(source.getSecretKey());
+ copy.setRemark(source.getRemark());
+ return copy;
+ }
+
@Test
void updateShouldNotRecreateConcurrentlyDeletedCredentialTest() {
CloudCredentialVO stored = new CloudCredentialVO();
@@ -206,7 +284,7 @@ class CloudCredentialServiceTest {
stored.setVendor(InstanceVendor.TENCENT);
stored.setAccessKey("AKIDexample");
when(credentialRepository.findById(1L)).thenReturn(Optional.of(stored));
- when(credentialRepository.replace(stored)).thenReturn(false);
+ when(credentialRepository.updateFields(1L, "renamed", null,
null)).thenReturn(false);
UpdateCloudCredentialDTO request = new UpdateCloudCredentialDTO();
request.setId(1L);
request.setName("renamed");
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepositoryTest.java
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepositoryTest.java
index b9b9ffa00..6bad1398c 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepositoryTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/provider/credential/MybatisPlusCloudCredentialRepositoryTest.java
@@ -19,10 +19,12 @@ package org.apache.rocketmq.studio.provider.credential;
import com.baomidou.mybatisplus.core.conditions.Wrapper;
import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
import org.apache.rocketmq.studio.common.domain.enums.InstanceVendor;
import org.apache.rocketmq.studio.common.exception.BusinessException;
+import org.apache.rocketmq.studio.common.util.CredentialUtils;
import org.apache.rocketmq.studio.persistence.entity.RmqCloudCredential;
import org.apache.rocketmq.studio.persistence.mapper.RmqCloudCredentialMapper;
import org.junit.jupiter.api.Test;
@@ -37,6 +39,7 @@ import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.when;
@@ -110,6 +113,49 @@ class MybatisPlusCloudCredentialRepositoryTest {
assertThat(query.getParamNameValuePairs()).containsValue("%credential%");
}
+ @Test
+ void updateFieldsShouldLeaveOmittedCredentialColumnsUntouchedTest() {
+ when(credentialMapper.update(isNull(),
any(Wrapper.class))).thenReturn(1);
+
+ assertThat(repository.updateFields(1L, "renamed", null,
null)).isTrue();
+
+ ArgumentCaptor<Wrapper<RmqCloudCredential>> queryCaptor =
ArgumentCaptor.forClass(Wrapper.class);
+ verify(credentialMapper).update(isNull(), queryCaptor.capture());
+ UpdateWrapper<RmqCloudCredential> update =
(UpdateWrapper<RmqCloudCredential>) queryCaptor.getValue();
+ assertThat(update.getSqlSet()).contains("name", "gmt_modified")
+ .doesNotContain("secret_key", "remark");
+
assertThat(update.getParamNameValuePairs().values()).contains("renamed");
+ }
+
+ @Test
+ void updateFieldsShouldEncodeOnlyTheSuppliedSecretTest() {
+ when(credentialMapper.update(isNull(),
any(Wrapper.class))).thenReturn(1);
+
+ assertThat(repository.updateFields(1L, null, "new-secret",
null)).isTrue();
+
+ ArgumentCaptor<Wrapper<RmqCloudCredential>> queryCaptor =
ArgumentCaptor.forClass(Wrapper.class);
+ verify(credentialMapper).update(isNull(), queryCaptor.capture());
+ UpdateWrapper<RmqCloudCredential> update =
(UpdateWrapper<RmqCloudCredential>) queryCaptor.getValue();
+ assertThat(update.getSqlSet()).contains("secret_key", "gmt_modified")
+ .doesNotContain("name", "remark");
+ assertThat(update.getParamNameValuePairs().values())
+ .contains(CredentialUtils.encodeBase64("new-secret"))
+ .doesNotContain("new-secret");
+ }
+
+ @Test
+ void updateFieldsShouldAllowClearingTheRemarkTest() {
+ when(credentialMapper.update(isNull(),
any(Wrapper.class))).thenReturn(1);
+
+ assertThat(repository.updateFields(1L, null, null, "")).isTrue();
+
+ ArgumentCaptor<Wrapper<RmqCloudCredential>> queryCaptor =
ArgumentCaptor.forClass(Wrapper.class);
+ verify(credentialMapper).update(isNull(), queryCaptor.capture());
+ UpdateWrapper<RmqCloudCredential> update =
(UpdateWrapper<RmqCloudCredential>) queryCaptor.getValue();
+
assertThat(update.getSqlSet()).contains("remark").doesNotContain("secret_key");
+ assertThat(update.getParamNameValuePairs().values()).contains("");
+ }
+
private RmqCloudCredential entity(Long id, String name, String vendor) {
RmqCloudCredential entity = new RmqCloudCredential();
entity.setId(id);
