diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java index 6bd1a64e..6d68193f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java @@ -256,7 +256,15 @@ public class ReviewPortalAppService { Map userNsRoles) { ReviewTask anchor = reviewTaskRepository.findById(reviewTaskId) .orElseThrow(() -> new DomainNotFoundException("review_task.not_found", reviewTaskId)); - if (!canViewReview(anchor, userId, normalizeRoles(userNsRoles))) { + Namespace namespace = namespaceRepository.findById(anchor.getNamespaceId()) + .orElseThrow(() -> new DomainNotFoundException( + "namespace.not_found", anchor.getNamespaceId())); + if (!reviewService.canReviewNamespace( + anchor, + userId, + namespace.getType(), + normalizeRoles(userNsRoles), + platformRoles(userId))) { throw new DomainForbiddenException("review.no_permission"); } 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 index 6142d33a..e734c191 100644 --- 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 @@ -353,7 +353,7 @@ class ReviewPortalControllerTest { given(rbacService.getUserRoleCodes("reviewer-1")).willReturn(Set.of("SKILL_ADMIN")); given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); - given(reviewService.canViewReview( + given(reviewService.canReviewNamespace( latest, "reviewer-1", namespace.getType(), @@ -371,6 +371,32 @@ class ReviewPortalControllerTest { .andExpect(jsonPath("$.data[1].id").value(8)); } + @Test + void listReviewAttempts_forbidsSubmitterWithoutReviewerRole() throws Exception { + ReviewTask ownAttempt = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.REJECTED); + setField(ownAttempt, "skillId", 30L); + setField(ownAttempt, "skillVersion", "1.0.0"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("author-1", List.of(new NamespaceMember( + 20L, "author-1", NamespaceRole.MEMBER))); + given(rbacService.getUserRoleCodes("author-1")).willReturn(Set.of()); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(ownAttempt)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(reviewService.canReviewNamespace( + ownAttempt, + "author-1", + namespace.getType(), + Map.of(20L, NamespaceRole.MEMBER), + Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/12/attempts").with(auth("author-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(reviewTaskRepository, never()) + .findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0"); + } + @Test void listReviewAttempts_forbidsUnrelatedUser() throws Exception { ReviewTask latest = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.PENDING); @@ -381,7 +407,7 @@ class ReviewPortalControllerTest { given(rbacService.getUserRoleCodes("other-user")).willReturn(Set.of()); given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); - given(reviewService.canViewReview( + given(reviewService.canReviewNamespace( latest, "other-user", namespace.getType(), diff --git a/web/e2e/rejected-version-republish.spec.ts b/web/e2e/rejected-version-republish.spec.ts index 492c55ac..5a014ac9 100644 --- a/web/e2e/rejected-version-republish.spec.ts +++ b/web/e2e/rejected-version-republish.spec.ts @@ -26,6 +26,8 @@ test.describe('Rejected version replacement (Real API)', () => { test('re-publishes the same version after rejection', async ({ page, browser }, testInfo) => { const consoleErrors: string[] = [] const pageErrors: string[] = [] + const adminConsoleErrors: string[] = [] + const adminPageErrors: string[] = [] page.on('console', (message) => { if (message.type() === 'error') consoleErrors.push(message.text()) }) @@ -36,6 +38,10 @@ test.describe('Rejected version replacement (Real API)', () => { const adminContext = await browser.newContext() const adminPage = await adminContext.newPage() + adminPage.on('console', (message) => { + if (message.type() === 'error') adminConsoleErrors.push(message.text()) + }) + adminPage.on('pageerror', (error) => adminPageErrors.push(error.message)) const adminBuilder = new E2eTestDataBuilder(adminPage, testInfo) await loginWithCredentials(adminPage, adminCredentials(), testInfo) await adminBuilder.init() @@ -141,6 +147,8 @@ test.describe('Rejected version replacement (Real API)', () => { )) expect(unexpectedConsoleErrors).toEqual([]) expect(pageErrors).toEqual([]) + expect(adminConsoleErrors).toEqual([]) + expect(adminPageErrors).toEqual([]) } finally { await adminBuilder.cleanup() await adminContext.close()