fix(review): restrict version history to reviewers

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-01 14:18:51 +08:00
parent 3e77365a5d
commit a568d22526
3 changed files with 45 additions and 3 deletions

View file

@ -256,7 +256,15 @@ public class ReviewPortalAppService {
Map<Long, NamespaceRole> 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");
}

View file

@ -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(),

View file

@ -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()