From 738f8b33b852832c21ea3ba776a6eea4707b8042 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 13:56:52 +0800 Subject: [PATCH 1/6] fix(review): allow super admins to approve own promotions --- .../skillhub/domain/review/ReviewPermissionChecker.java | 2 +- .../domain/review/ReviewPermissionCheckerTest.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java index bc5dae42..634e5893 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java @@ -107,7 +107,7 @@ public class ReviewPermissionChecker { String userId, Set platformRoles) { if (request.getSubmittedBy().equals(userId)) { - return false; + return platformRoles.contains("SUPER_ADMIN"); } return hasPlatformReviewRole(platformRoles); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java index 21fc18dc..020744b2 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java @@ -198,4 +198,12 @@ class ReviewPermissionCheckerTest { assertFalse(checker.canReviewPromotion(req, userId, Set.of("SKILL_ADMIN"))); } + + @Test + void superAdminCanReviewOwnPromotion() { + String userId = "user-2"; + PromotionRequest req = new PromotionRequest(1L, 1L, 1L, userId); + assertTrue(checker.canReviewPromotion(req, userId, + Set.of("SUPER_ADMIN"))); + } } From 9644d50fbb0436fb8e342a1cd0a0db377e7f8ce4 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 14:02:48 +0800 Subject: [PATCH 2/6] Revert "fix(review): allow super admins to approve own promotions" This reverts commit 738f8b33b852832c21ea3ba776a6eea4707b8042. --- .../skillhub/domain/review/ReviewPermissionChecker.java | 2 +- .../domain/review/ReviewPermissionCheckerTest.java | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java index 634e5893..bc5dae42 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java @@ -107,7 +107,7 @@ public class ReviewPermissionChecker { String userId, Set platformRoles) { if (request.getSubmittedBy().equals(userId)) { - return platformRoles.contains("SUPER_ADMIN"); + return false; } return hasPlatformReviewRole(platformRoles); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java index 020744b2..21fc18dc 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java @@ -198,12 +198,4 @@ class ReviewPermissionCheckerTest { assertFalse(checker.canReviewPromotion(req, userId, Set.of("SKILL_ADMIN"))); } - - @Test - void superAdminCanReviewOwnPromotion() { - String userId = "user-2"; - PromotionRequest req = new PromotionRequest(1L, 1L, 1L, userId); - assertTrue(checker.canReviewPromotion(req, userId, - Set.of("SUPER_ADMIN"))); - } } From e35e99ae598ca331317a52b2ef3fae2d46cdea65 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 14:44:08 +0800 Subject: [PATCH 3/6] fix(auth): backfill bootstrap admin role binding --- .../bootstrap/BootstrapAdminInitializer.java | 25 +++++++++----- .../BootstrapAdminInitializerTest.java | 33 +++++++++++++++++-- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java index f8e7fab5..b440ab8c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java @@ -63,8 +63,13 @@ public class BootstrapAdminInitializer implements ApplicationRunner { log.info("Bootstrap admin is disabled"); return; } - if (localCredentialRepository.existsByUsernameIgnoreCase(bootstrapAdminProperties.getUsername())) { - log.info("Bootstrap admin already exists, skipping"); + LocalCredential existingCredential = localCredentialRepository + .findByUsernameIgnoreCase(bootstrapAdminProperties.getUsername()) + .orElse(null); + if (existingCredential != null + && !bootstrapAdminProperties.getUserId().equals(existingCredential.getUserId())) { + log.info("Bootstrap admin username '{}' is already bound to another user, skipping", + bootstrapAdminProperties.getUsername()); return; } @@ -83,13 +88,15 @@ public class BootstrapAdminInitializer implements ApplicationRunner { admin = userAccountRepository.save(admin); // 2. Create local credential (username/password) - localCredentialRepository.save( - new LocalCredential( - admin.getId(), - bootstrapAdminProperties.getUsername(), - passwordEncoder.encode(bootstrapAdminProperties.getPassword()) - ) - ); + if (existingCredential == null) { + localCredentialRepository.save( + new LocalCredential( + admin.getId(), + bootstrapAdminProperties.getUsername(), + passwordEncoder.encode(bootstrapAdminProperties.getPassword()) + ) + ); + } // 3. Assign SUPER_ADMIN role Role superAdmin = roleRepository.findByCode("SUPER_ADMIN") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java index a151d2a8..276e9925 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java @@ -73,7 +73,6 @@ class BootstrapAdminInitializerTest { setField(superAdminRole, "id", 1L); setField(superAdminRole, "code", "SUPER_ADMIN"); - when(localCredentialRepository.existsByUsernameIgnoreCase("admin")).thenReturn(false); when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.empty()); when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); when(passwordEncoder.encode("ChangeMe!2026")).thenReturn("encoded-password"); @@ -111,7 +110,8 @@ class BootstrapAdminInitializerTest { @Test void shouldSkipWhenBootstrapAdminCredentialAlreadyExists() { bootstrapAdminProperties.setEnabled(true); - when(localCredentialRepository.existsByUsernameIgnoreCase("admin")).thenReturn(true); + LocalCredential conflictingCredential = new LocalCredential("someone-else", "admin", "encoded-password"); + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(conflictingCredential)); initializer.run(new DefaultApplicationArguments(new String[0])); @@ -121,13 +121,40 @@ class BootstrapAdminInitializerTest { verify(namespaceMemberRepository, never()).save(any(NamespaceMember.class)); } + @Test + void shouldStillEnsureRoleAndMembershipWhenBootstrapCredentialExistsForConfiguredUser() throws Exception { + bootstrapAdminProperties.setEnabled(true); + Namespace global = new Namespace("global", "Global", "system"); + setField(global, "id", 1L); + + Role superAdminRole = new Role(); + setField(superAdminRole, "id", 1L); + setField(superAdminRole, "code", "SUPER_ADMIN"); + + LocalCredential existingCredential = new LocalCredential("docker-admin", "admin", "encoded-password"); + + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(existingCredential)); + when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.empty()); + when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(roleRepository.findByCode("SUPER_ADMIN")).thenReturn(Optional.of(superAdminRole)); + when(userRoleBindingRepository.findByUserId("docker-admin")).thenReturn(List.of()); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "docker-admin")).thenReturn(Optional.empty()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(localCredentialRepository, never()).save(any(LocalCredential.class)); + verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); + verify(namespaceMemberRepository).save(any(NamespaceMember.class)); + } + @Test void shouldSkipWhenBootstrapAdminIsDisabled() { bootstrapAdminProperties.setEnabled(false); initializer.run(new DefaultApplicationArguments(new String[0])); - verify(localCredentialRepository, never()).existsByUsernameIgnoreCase(any()); + verify(localCredentialRepository, never()).findByUsernameIgnoreCase(any()); verify(userAccountRepository, never()).save(any(UserAccount.class)); } From 082d97a69d092af635cca333e6079a71d309eb36 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 16:23:31 +0800 Subject: [PATCH 4/6] fix(review): handle promotion target skill conflicts --- .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../domain/review/PromotionService.java | 27 ++++++++++++++++++- .../domain/review/PromotionServiceTest.java | 23 ++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 1fea61ca..68540961 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -167,3 +167,4 @@ validation.auth.password.reset.email.invalid=Email format is invalid validation.auth.password.reset.code.notBlank=Verification code cannot be blank validation.auth.password.reset.code.invalid=Verification code must be 6 digits validation.auth.password.reset.newPassword.notBlank=New password cannot be blank +promotion.target_skill_conflict=The target global skill "{0}" already exists diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 05bd0905..c1f782f0 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -167,3 +167,4 @@ validation.auth.password.reset.email.invalid=邮箱格式不正确 validation.auth.password.reset.code.notBlank=验证码不能为空 validation.auth.password.reset.code.invalid=验证码必须为 6 位数字 validation.auth.password.reset.newPassword.notBlank=新密码不能为空 +promotion.target_skill_conflict=目标全局技能“{0}”已存在 diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java index eb533b8a..67d70c0f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java @@ -16,6 +16,7 @@ import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.*; import jakarta.persistence.EntityManager; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -207,6 +208,8 @@ public class PromotionService { SkillVersion sourceVersion = skillVersionRepository.findById(approvedRequest.getSourceVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", approvedRequest.getSourceVersionId())); + assertTargetSkillNotExists(approvedRequest, sourceSkill); + // Create new skill in global namespace Skill newSkill = new Skill(approvedRequest.getTargetNamespaceId(), sourceSkill.getSlug(), sourceSkill.getOwnerId(), SkillVisibility.PUBLIC); @@ -215,7 +218,11 @@ public class PromotionService { newSkill.setSourceSkillId(sourceSkill.getId()); newSkill.setCreatedBy(reviewerId); newSkill.setUpdatedBy(reviewerId); - newSkill = skillRepository.save(newSkill); + try { + newSkill = skillRepository.save(newSkill); + } catch (DataIntegrityViolationException ex) { + throw duplicateTargetSkillConflict(sourceSkill.getSlug(), ex); + } // Create new version copying metadata from source SkillVersion newVersion = new SkillVersion(newSkill.getId(), sourceVersion.getVersion(), @@ -264,6 +271,24 @@ public class PromotionService { return savedRequest; } + private void assertTargetSkillNotExists(PromotionRequest approvedRequest, Skill sourceSkill) { + skillRepository.findByNamespaceIdAndSlugAndOwnerId( + approvedRequest.getTargetNamespaceId(), + sourceSkill.getSlug(), + sourceSkill.getOwnerId() + ).ifPresent(existing -> { + throw duplicateTargetSkillConflict(sourceSkill.getSlug(), null); + }); + } + + private DomainBadRequestException duplicateTargetSkillConflict(String slug, Exception cause) { + DomainBadRequestException ex = new DomainBadRequestException("promotion.target_skill_conflict", slug); + if (cause != null) { + ex.initCause(cause); + } + return ex; + } + /** * Rejects a pending promotion request without changing the source skill. */ diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java index ea00a449..0df9342d 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java @@ -19,6 +19,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import java.time.Clock; import java.time.Instant; @@ -511,6 +512,28 @@ class PromotionServiceTest { () -> promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN"))); } + @Test + void shouldTranslateDuplicateTargetSkillIntoBadRequest() { + PromotionRequest pr = createPendingPromotion(); + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + + when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); + when(promotionRequestRepository.updateStatusWithVersion( + PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", null, pr.getVersion())) + .thenReturn(1); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(skillRepository.save(any(Skill.class))) + .thenThrow(new DataIntegrityViolationException("duplicate key value violates unique constraint")); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, + () -> promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN"))); + + assertEquals("promotion.target_skill_conflict", ex.messageCode()); + } + @Test void shouldCopyDisplayNameAndSummaryToNewSkill() { PromotionRequest pr = createPendingPromotion(); From 2334aa6a5a27d248195d443b2efd8c6bfc69e6b0 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 20:02:41 +0800 Subject: [PATCH 5/6] fix(review): avoid stale promotion approval save --- .../PromotionApprovalFlowIntegrationTest.java | 193 ++++++++++++++++++ .../domain/review/PromotionService.java | 5 +- .../domain/review/PromotionServiceTest.java | 34 ++- 3 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java new file mode 100644 index 00000000..44d39712 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java @@ -0,0 +1,193 @@ +package com.iflytek.skillhub.controller.portal; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.governance.GovernanceNotificationService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.review.PromotionRequest; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.PromotionRequestJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillVersionJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import com.iflytek.skillhub.notification.service.NotificationDispatcher; +import java.time.Instant; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest(classes = SkillhubApplication.class) +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class PromotionApprovalFlowIntegrationTest { + + private static final String SUBMITTER_ID = "promotion-owner"; + private static final String REVIEWER_ID = "docker-admin"; + + @Autowired + private MockMvc mockMvc; + + @Autowired + private UserAccountJpaRepository userAccountRepository; + + @Autowired + private NamespaceJpaRepository namespaceRepository; + + @Autowired + private SkillJpaRepository skillRepository; + + @Autowired + private SkillVersionJpaRepository skillVersionRepository; + + @Autowired + private PromotionRequestJpaRepository promotionRequestRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private RbacService rbacService; + + @MockBean + private GovernanceNotificationService governanceNotificationService; + + @MockBean + private NotificationDispatcher notificationDispatcher; + + @BeforeEach + void setUp() { + when(rbacService.getUserRoleCodes(REVIEWER_ID)).thenReturn(Set.of("SUPER_ADMIN")); + } + + @Test + void approvePromotion_persistsTargetSkillAndReturnsSuccessForBootstrapAdmin() throws Exception { + PromotionGraph graph = createPromotionGraph(); + + mockMvc.perform(post("/api/web/promotions/" + graph.request().getId() + "/approve") + .contentType("application/json") + .content("{\"comment\":\"ship it\"}") + .with(authentication(portalAuth(REVIEWER_ID, "SUPER_ADMIN"))) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(graph.request().getId())) + .andExpect(jsonPath("$.data.status").value("APPROVED")) + .andExpect(jsonPath("$.data.reviewedBy").value(REVIEWER_ID)) + .andExpect(jsonPath("$.data.reviewComment").value("ship it")); + + PromotionRequest savedRequest = promotionRequestRepository.findAllById(List.of(graph.request().getId())) + .stream() + .findFirst() + .orElseThrow(); + assertThat(savedRequest.getStatus()).isEqualTo(ReviewTaskStatus.APPROVED); + assertThat(savedRequest.getReviewedBy()).isEqualTo(REVIEWER_ID); + assertThat(savedRequest.getTargetSkillId()).isNotNull(); + + Skill targetSkill = skillRepository.findAllById(List.of(savedRequest.getTargetSkillId())) + .stream() + .findFirst() + .orElseThrow(); + assertThat(targetSkill.getNamespaceId()).isEqualTo(graph.globalNamespace().getId()); + assertThat(targetSkill.getSlug()).isEqualTo(graph.sourceSkill().getSlug()); + assertThat(targetSkill.getSourceSkillId()).isEqualTo(graph.sourceSkill().getId()); + assertThat(targetSkill.getLatestVersionId()).isNotNull(); + + List targetVersions = skillVersionRepository.findBySkillId(targetSkill.getId()); + assertThat(targetVersions).hasSize(1); + assertThat(targetVersions.get(0).getStatus()).isEqualTo(SkillVersionStatus.PUBLISHED); + } + + private PromotionGraph createPromotionGraph() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + + userAccountRepository.saveAndFlush( + new UserAccount(SUBMITTER_ID, "Promotion Owner", "owner-" + suffix + "@example.com", null) + ); + userAccountRepository.saveAndFlush( + new UserAccount(REVIEWER_ID, "Admin", "admin-" + suffix + "@example.com", null) + ); + + Namespace globalNamespace = new Namespace("global-" + suffix, "Global " + suffix, REVIEWER_ID); + globalNamespace.setType(NamespaceType.GLOBAL); + globalNamespace = namespaceRepository.saveAndFlush(globalNamespace); + + Namespace teamNamespace = new Namespace("team-" + suffix, "Team " + suffix, SUBMITTER_ID); + teamNamespace = namespaceRepository.saveAndFlush(teamNamespace); + + Skill sourceSkill = new Skill(teamNamespace.getId(), "promote-skill-" + suffix, SUBMITTER_ID, SkillVisibility.PUBLIC); + sourceSkill.setDisplayName("Promote Skill " + suffix); + sourceSkill.setSummary("Used to verify promotion approval flow."); + sourceSkill.setCreatedBy(SUBMITTER_ID); + sourceSkill.setUpdatedBy(SUBMITTER_ID); + sourceSkill = skillRepository.saveAndFlush(sourceSkill); + + SkillVersion sourceVersion = new SkillVersion(sourceSkill.getId(), "1.0.0", SUBMITTER_ID); + sourceVersion.setStatus(SkillVersionStatus.PUBLISHED); + sourceVersion.setPublishedAt(Instant.now()); + sourceVersion.setRequestedVisibility(SkillVisibility.PUBLIC); + sourceVersion.setParsedMetadataJson("{\"name\":\"promotion-flow\"}"); + sourceVersion.setManifestJson("{\"version\":\"1.0.0\"}"); + sourceVersion = skillVersionRepository.saveAndFlush(sourceVersion); + + sourceSkill.setLatestVersionId(sourceVersion.getId()); + sourceSkill.setUpdatedBy(SUBMITTER_ID); + sourceSkill = skillRepository.saveAndFlush(sourceSkill); + + PromotionRequest request = promotionRequestRepository.saveAndFlush( + new PromotionRequest(sourceSkill.getId(), sourceVersion.getId(), globalNamespace.getId(), SUBMITTER_ID) + ); + + return new PromotionGraph(globalNamespace, sourceSkill, sourceVersion, request); + } + + private UsernamePasswordAuthenticationToken portalAuth(String userId, String... roles) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of(roles) + ); + List authorities = java.util.Arrays.stream(roles) + .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) + .toList(); + return new UsernamePasswordAuthenticationToken(principal, null, authorities); + } + + private record PromotionGraph(Namespace globalNamespace, + Skill sourceSkill, + SkillVersion sourceVersion, + PromotionRequest request) { + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java index 67d70c0f..4ce5d3c9 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java @@ -198,9 +198,8 @@ public class PromotionService { if (updated == 0) { throw new ConcurrentModificationException("Promotion request was modified concurrently"); } - syncPromotionRequestState(request, ReviewTaskStatus.APPROVED, reviewerId, comment); - entityManager.detach(request); - PromotionRequest approvedRequest = request; + PromotionRequest approvedRequest = promotionRequestRepository.findById(promotionId) + .orElseThrow(() -> new DomainNotFoundException("promotion.not_found", promotionId)); Skill sourceSkill = skillRepository.findById(approvedRequest.getSourceSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", approvedRequest.getSourceSkillId())); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java index 0df9342d..8586f96d 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java @@ -120,6 +120,16 @@ class PromotionServiceTest { return pr; } + private PromotionRequest approvedPromotion(PromotionRequest original, String comment) { + PromotionRequest approved = createPendingPromotion(); + approved.setStatus(ReviewTaskStatus.APPROVED); + approved.setReviewedBy(REVIEWER_ID); + approved.setReviewComment(comment); + approved.setReviewedAt(Instant.now(CLOCK)); + setField(approved, "version", original.getVersion() + 1); + return approved; + } + private List createSourceFiles() { return List.of( new SkillFile(SOURCE_VERSION_ID, "main.py", 500L, "text/x-python", "sha1", "storage/key1"), @@ -348,6 +358,7 @@ class PromotionServiceTest { @Test void shouldNotifySubmitterWhenPromotionApproved() { PromotionRequest request = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(request, "ok"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); Skill newSkill = new Skill(TARGET_NAMESPACE_ID, "my-skill", REVIEWER_ID, SkillVisibility.PUBLIC); @@ -355,7 +366,8 @@ class PromotionServiceTest { SkillVersion newVersion = new SkillVersion(NEW_SKILL_ID, sourceVersion.getVersion(), REVIEWER_ID); setField(newVersion, "id", NEW_VERSION_ID); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(request)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(request), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(request, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", null, request.getVersion())) @@ -365,6 +377,7 @@ class PromotionServiceTest { when(skillRepository.save(any(Skill.class))).thenReturn(newSkill); when(skillVersionRepository.save(any(SkillVersion.class))).thenReturn(newVersion); when(skillFileRepository.findByVersionId(SOURCE_VERSION_ID)).thenReturn(List.of()); + when(promotionRequestRepository.save(approvedRequest)).thenReturn(approvedRequest); promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN")); @@ -394,11 +407,13 @@ class PromotionServiceTest { @Test void shouldApprovePromotionSuccessfully() { PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "LGTM"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); List sourceFiles = createSourceFiles(); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "LGTM", null, pr.getVersion())) @@ -417,7 +432,7 @@ class PromotionServiceTest { }); when(skillFileRepository.findByVersionId(SOURCE_VERSION_ID)).thenReturn(sourceFiles); when(skillFileRepository.saveAll(anyList())).thenAnswer(inv -> inv.getArgument(0)); - when(promotionRequestRepository.save(pr)).thenReturn(pr); + when(promotionRequestRepository.save(approvedRequest)).thenReturn(approvedRequest); PromotionRequest result = promotionService.approvePromotion( PROMOTION_ID, REVIEWER_ID, "LGTM", Set.of("SKILL_ADMIN")); @@ -468,8 +483,8 @@ class PromotionServiceTest { assertEquals(REVIEWER_ID, event.publisherId()); // Verify targetSkillId updated on promotion request - verify(promotionRequestRepository).save(pr); - assertEquals(NEW_SKILL_ID, pr.getTargetSkillId()); + verify(promotionRequestRepository).save(approvedRequest); + assertEquals(NEW_SKILL_ID, approvedRequest.getTargetSkillId()); } @Test @@ -515,10 +530,12 @@ class PromotionServiceTest { @Test void shouldTranslateDuplicateTargetSkillIntoBadRequest() { PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "ok"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", null, pr.getVersion())) @@ -537,10 +554,12 @@ class PromotionServiceTest { @Test void shouldCopyDisplayNameAndSummaryToNewSkill() { PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "ok"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion(any(), any(), any(), any(), any(), any())).thenReturn(1); when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); @@ -566,6 +585,7 @@ class PromotionServiceTest { assertEquals("My Skill", newSkill.getDisplayName()); assertEquals("A test skill", newSkill.getSummary()); } + } @Nested From a6f8956549243296e82d4719f523163991434c48 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 15 Apr 2026 15:49:42 +0800 Subject: [PATCH 6/6] fix(auth): preserve bootstrap admin profile on backfill --- .../bootstrap/BootstrapAdminInitializer.java | 28 +++++++++-------- .../BootstrapAdminInitializerTest.java | 30 +++++++++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java index b440ab8c..2d70a060 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java @@ -73,19 +73,23 @@ public class BootstrapAdminInitializer implements ApplicationRunner { return; } - // 1. Create admin user account + // 1. Create admin user account. When the credential already belongs to the + // configured bootstrap user, only backfill role/membership and preserve + // any existing profile fields managed elsewhere. UserAccount admin = userAccountRepository.findById(bootstrapAdminProperties.getUserId()) - .orElseGet(() -> userAccountRepository.save( - new UserAccount( - bootstrapAdminProperties.getUserId(), - bootstrapAdminProperties.getDisplayName(), - bootstrapAdminProperties.getEmail(), - null - ) - )); - admin.setDisplayName(bootstrapAdminProperties.getDisplayName()); - admin.setEmail(bootstrapAdminProperties.getEmail()); - admin = userAccountRepository.save(admin); + .orElse(null); + if (admin == null) { + admin = userAccountRepository.save(new UserAccount( + bootstrapAdminProperties.getUserId(), + bootstrapAdminProperties.getDisplayName(), + bootstrapAdminProperties.getEmail(), + null + )); + } else if (existingCredential == null) { + admin.setDisplayName(bootstrapAdminProperties.getDisplayName()); + admin.setEmail(bootstrapAdminProperties.getEmail()); + admin = userAccountRepository.save(admin); + } // 2. Create local credential (username/password) if (existingCredential == null) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java index 276e9925..ace28d26 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java @@ -148,6 +148,36 @@ class BootstrapAdminInitializerTest { verify(namespaceMemberRepository).save(any(NamespaceMember.class)); } + @Test + void shouldPreserveExistingUserProfileWhenBackfillingRoleAndMembership() throws Exception { + bootstrapAdminProperties.setEnabled(true); + Namespace global = new Namespace("global", "Global", "system"); + setField(global, "id", 1L); + + Role superAdminRole = new Role(); + setField(superAdminRole, "id", 1L); + setField(superAdminRole, "code", "SUPER_ADMIN"); + + LocalCredential existingCredential = new LocalCredential("docker-admin", "admin", "encoded-password"); + UserAccount existingUser = new UserAccount("docker-admin", "Existing Admin", "existing-admin@example.com", null); + + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(existingCredential)); + when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.of(existingUser)); + when(roleRepository.findByCode("SUPER_ADMIN")).thenReturn(Optional.of(superAdminRole)); + when(userRoleBindingRepository.findByUserId("docker-admin")).thenReturn(List.of()); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "docker-admin")).thenReturn(Optional.empty()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + assertEquals("Existing Admin", existingUser.getDisplayName()); + assertEquals("existing-admin@example.com", existingUser.getEmail()); + verify(userAccountRepository, never()).save(any(UserAccount.class)); + verify(localCredentialRepository, never()).save(any(LocalCredential.class)); + verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); + verify(namespaceMemberRepository).save(any(NamespaceMember.class)); + } + @Test void shouldSkipWhenBootstrapAdminIsDisabled() { bootstrapAdminProperties.setEnabled(false);