diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java index 270372be..66d276d0 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java @@ -4,10 +4,13 @@ import com.iflytek.skillhub.auth.rbac.RbacService; import com.iflytek.skillhub.controller.BaseApiController; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.review.PromotionRequest; import com.iflytek.skillhub.domain.review.PromotionRequestRepository; import com.iflytek.skillhub.domain.review.PromotionService; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; @@ -19,6 +22,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.web.bind.annotation.*; +import java.util.Map; import java.util.Set; @RestController @@ -32,6 +36,7 @@ public class PromotionController extends BaseApiController { private final NamespaceRepository namespaceRepository; private final UserAccountRepository userAccountRepository; private final RbacService rbacService; + private final ReviewPermissionChecker permissionChecker; public PromotionController(PromotionService promotionService, PromotionRequestRepository promotionRequestRepository, @@ -40,6 +45,7 @@ public class PromotionController extends BaseApiController { NamespaceRepository namespaceRepository, UserAccountRepository userAccountRepository, RbacService rbacService, + ReviewPermissionChecker permissionChecker, ApiResponseFactory responseFactory) { super(responseFactory); this.promotionService = promotionService; @@ -49,15 +55,18 @@ public class PromotionController extends BaseApiController { this.namespaceRepository = namespaceRepository; this.userAccountRepository = userAccountRepository; this.rbacService = rbacService; + this.permissionChecker = permissionChecker; } @PostMapping public ApiResponse submitPromotion( @RequestBody PromotionRequestDto request, - @RequestAttribute("userId") String userId) { + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { PromotionRequest promotion = promotionService.submitPromotion( request.sourceSkillId(), request.sourceVersionId(), - request.targetNamespaceId(), userId); + request.targetNamespaceId(), userId, + userNsRoles != null ? userNsRoles : Map.of()); return ok("response.success.create", toResponse(promotion)); } @@ -89,9 +98,8 @@ public class PromotionController extends BaseApiController { @RequestParam(defaultValue = "20") int size, @RequestAttribute("userId") String userId) { Set platformRoles = rbacService.getUserRoleCodes(userId); - boolean hasAdminRole = platformRoles.contains("SKILL_ADMIN") || platformRoles.contains("SUPER_ADMIN"); - if (!hasAdminRole) { - return ok("response.success.read", PageResponse.from(Page.empty())); + if (!permissionChecker.canListPendingPromotions(platformRoles)) { + throw new DomainForbiddenException("promotion.no_permission"); } Page requests = promotionRequestRepository.findByStatus( ReviewTaskStatus.PENDING, PageRequest.of(page, size)); @@ -99,8 +107,14 @@ public class PromotionController extends BaseApiController { } @GetMapping("/{id}") - public ApiResponse getPromotionDetail(@PathVariable Long id) { + public ApiResponse getPromotionDetail( + @PathVariable Long id, + @RequestAttribute("userId") String userId) { PromotionRequest promotion = promotionRequestRepository.findById(id).orElseThrow(); + Set platformRoles = rbacService.getUserRoleCodes(userId); + if (!permissionChecker.canReadPromotion(promotion, userId, platformRoles)) { + throw new DomainForbiddenException("promotion.no_permission"); + } return ok("response.success.read", toResponse(promotion)); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java index 742c263a..1b92da5b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java @@ -6,9 +6,11 @@ import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.review.ReviewService; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; import com.iflytek.skillhub.domain.review.ReviewTask; import com.iflytek.skillhub.domain.review.ReviewTaskRepository; import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; @@ -34,6 +36,7 @@ public class ReviewController extends BaseApiController { private final NamespaceRepository namespaceRepository; private final UserAccountRepository userAccountRepository; private final RbacService rbacService; + private final ReviewPermissionChecker permissionChecker; public ReviewController(ReviewService reviewService, ReviewTaskRepository reviewTaskRepository, @@ -42,6 +45,7 @@ public class ReviewController extends BaseApiController { NamespaceRepository namespaceRepository, UserAccountRepository userAccountRepository, RbacService rbacService, + ReviewPermissionChecker permissionChecker, ApiResponseFactory responseFactory) { super(responseFactory); this.reviewService = reviewService; @@ -51,16 +55,19 @@ public class ReviewController extends BaseApiController { this.namespaceRepository = namespaceRepository; this.userAccountRepository = userAccountRepository; this.rbacService = rbacService; + this.permissionChecker = permissionChecker; } @PostMapping public ApiResponse submitReview( @RequestBody ReviewTaskRequest request, - @RequestAttribute("userId") String userId) { - SkillVersion sv = skillVersionRepository.findById(request.skillVersionId()) - .orElseThrow(); - Skill skill = skillRepository.findById(sv.getSkillId()).orElseThrow(); - ReviewTask task = reviewService.submitReview(request.skillVersionId(), skill.getNamespaceId(), userId); + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + ReviewTask task = reviewService.submitReview( + request.skillVersionId(), + userId, + userNsRoles != null ? userNsRoles : Map.of() + ); return ok("response.success.create", toResponse(task)); } @@ -104,7 +111,18 @@ public class ReviewController extends BaseApiController { @RequestParam Long namespaceId, @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId) { + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + Namespace namespace = namespaceRepository.findById(namespaceId).orElseThrow(); + Set platformRoles = rbacService.getUserRoleCodes(userId); + if (!permissionChecker.canManageNamespaceReviews( + namespaceId, + namespace.getType(), + userNsRoles != null ? userNsRoles : Map.of(), + platformRoles)) { + throw new DomainForbiddenException("review.no_permission"); + } + Page tasks = reviewTaskRepository.findByNamespaceIdAndStatus( namespaceId, ReviewTaskStatus.PENDING, PageRequest.of(page, size)); return ok("response.success.read", PageResponse.from(tasks.map(this::toResponse))); @@ -121,8 +139,21 @@ public class ReviewController extends BaseApiController { } @GetMapping("/{id}") - public ApiResponse getReviewDetail(@PathVariable Long id) { + public ApiResponse getReviewDetail( + @PathVariable Long id, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { ReviewTask task = reviewTaskRepository.findById(id).orElseThrow(); + Namespace namespace = namespaceRepository.findById(task.getNamespaceId()).orElseThrow(); + Set platformRoles = rbacService.getUserRoleCodes(userId); + if (!permissionChecker.canReadReview( + task, + userId, + namespace.getType(), + userNsRoles != null ? userNsRoles : Map.of(), + platformRoles)) { + throw new DomainForbiddenException("review.no_permission"); + } return ok("response.success.read", toResponse(task)); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java new file mode 100644 index 00000000..2e57b0f7 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java @@ -0,0 +1,201 @@ +package com.iflytek.skillhub.controller; + +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.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.PromotionRequest; +import com.iflytek.skillhub.domain.review.PromotionRequestRepository; +import com.iflytek.skillhub.domain.review.PromotionService; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +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.http.MediaType; +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 org.springframework.test.web.servlet.request.RequestPostProcessor; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +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.get; +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 +@AutoConfigureMockMvc +@ActiveProfiles("test") +class PromotionPortalControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private PromotionService promotionService; + + @MockBean + private PromotionRequestRepository promotionRequestRepository; + + @MockBean + private SkillRepository skillRepository; + + @MockBean + private SkillVersionRepository skillVersionRepository; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private com.iflytek.skillhub.domain.namespace.NamespaceRepository namespaceRepository; + + @MockBean + private UserAccountRepository userAccountRepository; + + @MockBean + private RbacService rbacService; + + @MockBean + private ReviewPermissionChecker permissionChecker; + + @Test + void submitPromotion_passesNamespaceRolesToService() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(5L, "user-1", NamespaceRole.ADMIN))); + given(promotionService.submitPromotion(10L, 20L, 30L, "user-1", Map.of(5L, NamespaceRole.ADMIN))) + .willReturn(request); + stubPromotionResponse(request); + + mockMvc.perform(post("/api/v1/promotions") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"sourceSkillId\":10,\"sourceVersionId\":20,\"targetNamespaceId\":30}") + .with(csrf()) + .with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(1L)); + } + + @Test + void listPendingPromotions_forbidsRegularUser() throws Exception { + stubNamespaceRoles("user-1", List.of()); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canListPendingPromotions(Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/promotions/pending").with(auth("user-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(promotionRequestRepository, never()).findByStatus(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + } + + @Test + void getPromotionDetail_allowsSubmitter() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-1", List.of()); + given(promotionRequestRepository.findById(1L)).willReturn(Optional.of(request)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canReadPromotion(request, "user-1", Set.of())).willReturn(true); + stubPromotionResponse(request); + + mockMvc.perform(get("/api/v1/promotions/1").with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.submittedBy").value("user-1")); + } + + @Test + void getPromotionDetail_forbidsUnrelatedUser() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-9", List.of()); + given(promotionRequestRepository.findById(1L)).willReturn(Optional.of(request)); + given(rbacService.getUserRoleCodes("user-9")).willReturn(Set.of()); + given(permissionChecker.canReadPromotion(request, "user-9", Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/promotions/1").with(auth("user-9"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + private void stubPromotionResponse(PromotionRequest request) { + Skill skill = new Skill(5L, "skill-a", request.getSubmittedBy(), SkillVisibility.PUBLIC); + setField(skill, "id", request.getSourceSkillId()); + SkillVersion version = new SkillVersion(request.getSourceSkillId(), "1.0.0", request.getSubmittedBy()); + setField(version, "id", request.getSourceVersionId()); + Namespace sourceNamespace = new Namespace("team-a", "Team A", "owner-1"); + setField(sourceNamespace, "id", 5L); + Namespace targetNamespace = new Namespace("global", "Global", "owner-2"); + setField(targetNamespace, "id", request.getTargetNamespaceId()); + UserAccount submitter = new UserAccount(request.getSubmittedBy(), "Submitter", "submitter@example.com", ""); + + given(skillRepository.findById(request.getSourceSkillId())).willReturn(Optional.of(skill)); + given(skillVersionRepository.findById(request.getSourceVersionId())).willReturn(Optional.of(version)); + given(namespaceRepository.findById(5L)).willReturn(Optional.of(sourceNamespace)); + given(namespaceRepository.findById(request.getTargetNamespaceId())).willReturn(Optional.of(targetNamespace)); + given(userAccountRepository.findById(request.getSubmittedBy())).willReturn(Optional.of(submitter)); + } + + private void stubNamespaceRoles(String userId, List members) { + given(namespaceMemberRepository.findByUserId(userId)).willReturn(members); + } + + private RequestPostProcessor auth(String userId) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of() + ); + UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_USER")) + ); + return authentication(authenticationToken); + } + + private PromotionRequest createPromotionRequest(Long id, String submittedBy) { + PromotionRequest request = new PromotionRequest(10L, 20L, 30L, submittedBy); + setField(request, "id", id); + setField(request, "status", ReviewTaskStatus.PENDING); + return request; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java new file mode 100644 index 00000000..44551a8e --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java @@ -0,0 +1,213 @@ +package com.iflytek.skillhub.controller; + +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.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; +import com.iflytek.skillhub.domain.review.ReviewService; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +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.http.MediaType; +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 org.springframework.test.web.servlet.request.RequestPostProcessor; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +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.get; +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 +@AutoConfigureMockMvc +@ActiveProfiles("test") +class ReviewPortalControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private ReviewService reviewService; + + @MockBean + private ReviewTaskRepository reviewTaskRepository; + + @MockBean + private SkillRepository skillRepository; + + @MockBean + private SkillVersionRepository skillVersionRepository; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private com.iflytek.skillhub.domain.namespace.NamespaceRepository namespaceRepository; + + @MockBean + private UserAccountRepository userAccountRepository; + + @MockBean + private RbacService rbacService; + + @MockBean + private ReviewPermissionChecker permissionChecker; + + @Test + void submitReview_passesNamespaceRolesToService() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(20L, "user-1", NamespaceRole.MEMBER))); + given(reviewService.submitReview(100L, "user-1", Map.of(20L, NamespaceRole.MEMBER))).willReturn(task); + stubReviewResponse(task); + + mockMvc.perform(post("/api/v1/reviews") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"skillVersionId\":100}") + .with(csrf()) + .with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(1L)); + } + + @Test + void listPendingReviews_forbidsNamespaceMember() throws Exception { + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(20L, "user-1", NamespaceRole.MEMBER))); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canManageNamespaceReviews( + 20L, + namespace.getType(), + Map.of(20L, NamespaceRole.MEMBER), + Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/pending") + .param("namespaceId", "20") + .with(auth("user-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(reviewTaskRepository, never()).findByNamespaceIdAndStatus(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + } + + @Test + void getReviewDetail_allowsSubmitter() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-1", List.of()); + given(reviewTaskRepository.findById(1L)).willReturn(Optional.of(task)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canReadReview(task, "user-1", namespace.getType(), Map.of(), Set.of())).willReturn(true); + stubReviewResponse(task); + + mockMvc.perform(get("/api/v1/reviews/1").with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.submittedBy").value("user-1")); + } + + @Test + void getReviewDetail_forbidsUnrelatedUser() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-9", List.of()); + given(reviewTaskRepository.findById(1L)).willReturn(Optional.of(task)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-9")).willReturn(Set.of()); + given(permissionChecker.canReadReview(task, "user-9", namespace.getType(), Map.of(), Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/1").with(auth("user-9"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + private void stubReviewResponse(ReviewTask task) { + SkillVersion version = new SkillVersion(30L, "1.0.0", task.getSubmittedBy()); + setField(version, "id", task.getSkillVersionId()); + Skill skill = new Skill(task.getNamespaceId(), "skill-a", task.getSubmittedBy(), SkillVisibility.PUBLIC); + setField(skill, "id", 30L); + UserAccount submitter = new UserAccount(task.getSubmittedBy(), "Submitter", "submitter@example.com", ""); + + given(skillVersionRepository.findById(task.getSkillVersionId())).willReturn(Optional.of(version)); + given(skillRepository.findById(30L)).willReturn(Optional.of(skill)); + given(namespaceRepository.findById(task.getNamespaceId())).willReturn(Optional.of(createNamespace(task.getNamespaceId(), "team-a"))); + given(userAccountRepository.findById(task.getSubmittedBy())).willReturn(Optional.of(submitter)); + } + + private void stubNamespaceRoles(String userId, List members) { + given(namespaceMemberRepository.findByUserId(userId)).willReturn(members); + } + + private RequestPostProcessor auth(String userId) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of() + ); + UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_USER")) + ); + return authentication(authenticationToken); + } + + private ReviewTask createReviewTask(Long id, Long namespaceId, String submittedBy) { + ReviewTask task = new ReviewTask(100L, namespaceId, submittedBy); + setField(task, "id", id); + setField(task, "status", ReviewTaskStatus.PENDING); + return task; + } + + private Namespace createNamespace(Long id, String slug) { + Namespace namespace = new Namespace(slug, "Team", "owner-1"); + setField(namespace, "id", id); + return namespace; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} 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 01d4d581..919d847f 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 @@ -3,6 +3,7 @@ package com.iflytek.skillhub.domain.review; import com.iflytek.skillhub.domain.event.SkillPublishedEvent; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceType; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; @@ -15,6 +16,7 @@ import org.springframework.transaction.annotation.Transactional; import java.time.LocalDateTime; import java.util.ConcurrentModificationException; import java.util.List; +import java.util.Map; import java.util.Set; @Service @@ -46,7 +48,8 @@ public class PromotionService { @Transactional public PromotionRequest submitPromotion(Long sourceSkillId, Long sourceVersionId, - Long targetNamespaceId, String userId) { + Long targetNamespaceId, String userId, + Map userNamespaceRoles) { Skill sourceSkill = skillRepository.findById(sourceSkillId) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", sourceSkillId)); @@ -61,6 +64,10 @@ public class PromotionService { throw new DomainBadRequestException("promotion.version_not_published", sourceVersionId); } + if (!permissionChecker.canSubmitPromotion(sourceSkill, userId, userNamespaceRoles)) { + throw new DomainForbiddenException("promotion.submit.no_permission"); + } + Namespace targetNamespace = namespaceRepository.findById(targetNamespaceId) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", targetNamespaceId)); 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 0cc89df1..4983a4a7 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 @@ -2,6 +2,7 @@ package com.iflytek.skillhub.domain.review; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.skill.Skill; import org.springframework.stereotype.Component; import java.util.Map; @@ -10,6 +11,26 @@ import java.util.Set; @Component public class ReviewPermissionChecker { + public boolean canSubmitReview(Long namespaceId, + Map userNamespaceRoles) { + NamespaceRole role = userNamespaceRoles.get(namespaceId); + return role == NamespaceRole.OWNER + || role == NamespaceRole.ADMIN + || role == NamespaceRole.MEMBER; + } + + public boolean canManageNamespaceReviews(Long namespaceId, + NamespaceType namespaceType, + Map userNamespaceRoles, + Set platformRoles) { + if (namespaceType == NamespaceType.GLOBAL) { + return hasPlatformReviewRole(platformRoles); + } + + NamespaceRole role = userNamespaceRoles.get(namespaceId); + return role == NamespaceRole.OWNER || role == NamespaceRole.ADMIN; + } + /** * Check if a user can review a ReviewTask. * @@ -31,16 +52,37 @@ public class ReviewPermissionChecker { } // Global namespace: only SKILL_ADMIN or SUPER_ADMIN - if (namespaceType == NamespaceType.GLOBAL) { - return platformRoles.contains("SKILL_ADMIN") - || platformRoles.contains("SUPER_ADMIN"); + return canManageNamespaceReviews( + task.getNamespaceId(), + namespaceType, + userNamespaceRoles, + platformRoles + ); + } + + public boolean canReadReview(ReviewTask task, + String userId, + NamespaceType namespaceType, + Map userNamespaceRoles, + Set platformRoles) { + return task.getSubmittedBy().equals(userId) + || canManageNamespaceReviews( + task.getNamespaceId(), + namespaceType, + userNamespaceRoles, + platformRoles + ); + } + + public boolean canSubmitPromotion(Skill sourceSkill, + String userId, + Map userNamespaceRoles) { + if (sourceSkill.getOwnerId().equals(userId)) { + return true; } - // Team namespace: namespace ADMIN or OWNER - NamespaceRole role = userNamespaceRoles.get( - task.getNamespaceId()); - return role == NamespaceRole.ADMIN - || role == NamespaceRole.OWNER; + NamespaceRole role = userNamespaceRoles.get(sourceSkill.getNamespaceId()); + return role == NamespaceRole.OWNER || role == NamespaceRole.ADMIN; } /** @@ -54,6 +96,21 @@ public class ReviewPermissionChecker { if (request.getSubmittedBy().equals(userId)) { return false; } + return hasPlatformReviewRole(platformRoles); + } + + public boolean canListPendingPromotions(Set platformRoles) { + return hasPlatformReviewRole(platformRoles); + } + + public boolean canReadPromotion(PromotionRequest request, + String userId, + Set platformRoles) { + return request.getSubmittedBy().equals(userId) + || canListPendingPromotions(platformRoles); + } + + private boolean hasPlatformReviewRole(Set platformRoles) { return platformRoles.contains("SKILL_ADMIN") || platformRoles.contains("SUPER_ADMIN"); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index fb2482af..8e2f6308 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -17,7 +17,6 @@ import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.time.Instant; import java.time.LocalDateTime; import java.util.ConcurrentModificationException; import java.util.Map; @@ -48,18 +47,27 @@ public class ReviewService { } @Transactional - public ReviewTask submitReview(Long skillVersionId, Long namespaceId, String userId) { + public ReviewTask submitReview(Long skillVersionId, + String userId, + Map userNamespaceRoles) { SkillVersion skillVersion = skillVersionRepository.findById(skillVersionId) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", skillVersionId)); + Skill skill = skillRepository.findById(skillVersion.getSkillId()) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); + if (skillVersion.getStatus() != SkillVersionStatus.DRAFT) { throw new DomainBadRequestException("review.submit.not_draft", skillVersionId); } + if (!permissionChecker.canSubmitReview(skill.getNamespaceId(), userNamespaceRoles)) { + throw new DomainForbiddenException("review.submit.no_permission"); + } + skillVersion.setStatus(SkillVersionStatus.PENDING_REVIEW); skillVersionRepository.save(skillVersion); - ReviewTask task = new ReviewTask(skillVersionId, namespaceId, userId); + ReviewTask task = new ReviewTask(skillVersionId, skill.getNamespaceId(), userId); try { return reviewTaskRepository.save(task); } catch (DataIntegrityViolationException e) { 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 7670a76d..b1a8ea76 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 @@ -121,6 +121,7 @@ class PromotionServiceTest { when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(globalNs)); when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) .thenReturn(Optional.empty()); @@ -132,7 +133,7 @@ class PromotionServiceTest { }); PromotionRequest result = promotionService.submitPromotion( - SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID); + SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of()); assertNotNull(result); assertEquals(SOURCE_SKILL_ID, result.getSourceSkillId()); @@ -147,7 +148,7 @@ class PromotionServiceTest { when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -156,7 +157,7 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -169,7 +170,7 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sv)); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -182,39 +183,99 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sv)); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenTargetNamespaceNotFound() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenTargetNamespaceNotGlobal() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(createTeamNamespace())); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenDuplicatePendingExists() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(createGlobalNamespace())); when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) .thenReturn(Optional.of(createPendingPromotion())); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID)); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); + } + + @Test + void shouldThrowWhenSubmitterIsNotOwnerOrNamespaceAdmin() { + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(permissionChecker.canSubmitPromotion( + sourceSkill, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.MEMBER))) + .thenReturn(false); + + assertThrows(DomainForbiddenException.class, + () -> promotionService.submitPromotion( + SOURCE_SKILL_ID, + SOURCE_VERSION_ID, + TARGET_NAMESPACE_ID, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.MEMBER) + )); + verify(promotionRequestRepository, never()).save(any(PromotionRequest.class)); + } + + @Test + void shouldAllowNamespaceAdminToSubmitPromotionForForeignSkill() { + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + Namespace globalNs = createGlobalNamespace(); + + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(permissionChecker.canSubmitPromotion( + sourceSkill, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.ADMIN))) + .thenReturn(true); + when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(globalNs)); + when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) + .thenReturn(Optional.empty()); + when(promotionRequestRepository.save(any(PromotionRequest.class))) + .thenAnswer(inv -> inv.getArgument(0)); + + PromotionRequest result = promotionService.submitPromotion( + SOURCE_SKILL_ID, + SOURCE_VERSION_ID, + TARGET_NAMESPACE_ID, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.ADMIN) + ); + + assertNotNull(result); } } 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 f3d43cd9..48a3aadd 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 @@ -2,6 +2,8 @@ package com.iflytek.skillhub.domain.review; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; import org.junit.jupiter.api.Test; import java.util.Map; @@ -81,6 +83,53 @@ class ReviewPermissionCheckerTest { // --- canReviewPromotion tests --- + @Test + void memberCanSubmitReview() { + assertTrue(checker.canSubmitReview(10L, Map.of(10L, NamespaceRole.MEMBER))); + } + + @Test + void outsiderCannotSubmitReview() { + assertFalse(checker.canSubmitReview(10L, Map.of())); + } + + @Test + void teamAdminCanManagePendingReviewList() { + assertTrue(checker.canManageNamespaceReviews( + 10L, NamespaceType.TEAM, Map.of(10L, NamespaceRole.ADMIN), Set.of())); + } + + @Test + void submitterCanReadOwnReview() { + ReviewTask task = new ReviewTask(1L, 10L, "user-1"); + assertTrue(checker.canReadReview(task, "user-1", + NamespaceType.TEAM, Map.of(), Set.of())); + } + + @Test + void ownerCanSubmitPromotion() { + Skill sourceSkill = new Skill(10L, "skill-a", "user-1", SkillVisibility.PUBLIC); + assertTrue(checker.canSubmitPromotion(sourceSkill, "user-1", Map.of())); + } + + @Test + void teamAdminCanSubmitPromotionForForeignSkill() { + Skill sourceSkill = new Skill(10L, "skill-a", "user-2", SkillVisibility.PUBLIC); + assertTrue(checker.canSubmitPromotion(sourceSkill, "user-1", + Map.of(10L, NamespaceRole.ADMIN))); + } + + @Test + void submitterCanReadOwnPromotion() { + PromotionRequest req = new PromotionRequest(1L, 1L, 1L, "user-1"); + assertTrue(checker.canReadPromotion(req, "user-1", Set.of())); + } + + @Test + void skillAdminCanListPendingPromotions() { + assertTrue(checker.canListPendingPromotions(Set.of("SKILL_ADMIN"))); + } + @Test void skillAdminCanReviewPromotion() { PromotionRequest req = new PromotionRequest(1L, 1L, 1L, "user-2"); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java index 297a7701..c9325329 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java @@ -104,11 +104,20 @@ class ReviewServiceTest { @Test void shouldSubmitReviewSuccessfully() { SkillVersion sv = createDraftSkillVersion(); + Skill skill = createSkill(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(permissionChecker.canSubmitReview( + NAMESPACE_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))).thenReturn(true); ReviewTask savedTask = createPendingReviewTask(); when(reviewTaskRepository.save(any(ReviewTask.class))).thenReturn(savedTask); - ReviewTask result = reviewService.submitReview(SKILL_VERSION_ID, NAMESPACE_ID, USER_ID); + ReviewTask result = reviewService.submitReview( + SKILL_VERSION_ID, + USER_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER) + ); assertNotNull(result); assertEquals(SkillVersionStatus.PENDING_REVIEW, sv.getStatus()); @@ -121,27 +130,50 @@ class ReviewServiceTest { when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, NAMESPACE_ID, USER_ID)); + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenStatusNotDraft() { SkillVersion sv = createPendingReviewSkillVersion(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(createSkill())); assertThrows(DomainBadRequestException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, NAMESPACE_ID, USER_ID)); + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))); } @Test void shouldThrowOnDuplicateSubmission() { SkillVersion sv = createDraftSkillVersion(); + Skill skill = createSkill(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(permissionChecker.canSubmitReview( + NAMESPACE_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))).thenReturn(true); when(reviewTaskRepository.save(any(ReviewTask.class))) .thenThrow(new DataIntegrityViolationException("duplicate")); assertThrows(DomainBadRequestException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, NAMESPACE_ID, USER_ID)); + () -> reviewService.submitReview( + SKILL_VERSION_ID, + USER_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER) + )); + } + + @Test + void shouldThrowWhenSubmitterLacksNamespaceMembership() { + SkillVersion sv = createDraftSkillVersion(); + Skill skill = createSkill(); + when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(permissionChecker.canSubmitReview(NAMESPACE_ID, Map.of())).thenReturn(false); + + assertThrows(DomainForbiddenException.class, + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of())); + verify(reviewTaskRepository, never()).save(any(ReviewTask.class)); } }