From 5c42f0548619ff13f631a469bc67a4ebf28e6cf5 Mon Sep 17 00:00:00 2001 From: yun-zhi-ztl <15071461069@163.com> Date: Mon, 16 Mar 2026 14:38:40 +0800 Subject: [PATCH] fix: tighten archived namespace visibility --- .../portal/NamespaceController.java | 6 +- .../NamespaceMemberCandidateService.java | 6 +- .../service/SkillSearchAppService.java | 143 +++++++++++------- .../NamespacePortalControllerTest.java | 15 ++ .../NamespaceMemberCandidateServiceTest.java | 23 +++ .../service/SkillSearchAppServiceTest.java | 52 ++++++- .../domain/namespace/NamespaceService.java | 24 ++- .../namespace/NamespaceServiceTest.java | 18 ++- 8 files changed, 227 insertions(+), 60 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java index ed20b758..4c00fc10 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java @@ -68,8 +68,10 @@ public class NamespaceController extends BaseApiController { } @GetMapping("/namespaces/{slug}") - public ApiResponse getNamespace(@PathVariable String slug) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); + public ApiResponse getNamespace(@PathVariable String slug, + @RequestAttribute(value = "userId", required = false) String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + Namespace namespace = namespaceService.getNamespaceBySlugForRead(slug, userId, userNsRoles != null ? userNsRoles : Map.of()); return ok("response.success.read", NamespaceResponse.from(namespace)); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespaceMemberCandidateService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespaceMemberCandidateService.java index daa2b1b6..0401a7c8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespaceMemberCandidateService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespaceMemberCandidateService.java @@ -43,11 +43,11 @@ public class NamespaceMemberCandidateService { @Transactional(readOnly = true) public List searchCandidates(String slug, String search, String operatorUserId, int size) { Namespace namespace = namespaceService.getNamespaceBySlug(slug); + if (namespaceAccessPolicy.isImmutable(namespace)) { + throw new DomainBadRequestException("error.namespace.system.immutable", namespace.getSlug()); + } namespaceService.assertAdminOrOwner(namespace.getId(), operatorUserId); if (!namespaceAccessPolicy.canManageMembers(namespace)) { - if (namespaceAccessPolicy.isImmutable(namespace)) { - throw new DomainBadRequestException("error.namespace.system.immutable", namespace.getSlug()); - } throw new DomainBadRequestException("error.namespace.readonly", namespace.getSlug()); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java index aab4b04e..2e92f77a 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; @@ -29,16 +30,19 @@ public class SkillSearchAppService { private final SkillRepository skillRepository; private final NamespaceRepository namespaceRepository; private final SkillVersionRepository skillVersionRepository; + private final NamespaceService namespaceService; public SkillSearchAppService( SearchQueryService searchQueryService, SkillRepository skillRepository, NamespaceRepository namespaceRepository, - SkillVersionRepository skillVersionRepository) { + SkillVersionRepository skillVersionRepository, + NamespaceService namespaceService) { this.searchQueryService = searchQueryService; this.skillRepository = skillRepository; this.namespaceRepository = namespaceRepository; this.skillVersionRepository = skillVersionRepository; + this.namespaceService = namespaceService; } public record SearchResponse( @@ -57,64 +61,18 @@ public class SkillSearchAppService { String userId, Map userNsRoles) { - Long namespaceId = resolveNamespaceId(namespaceSlug); + Long namespaceId = resolveNamespaceId(namespaceSlug, userId, userNsRoles); SearchVisibilityScope scope = buildVisibilityScope(userId, userNsRoles); - SearchQuery query = new SearchQuery( - keyword, - namespaceId, - scope, - sortBy != null ? sortBy : "newest", - page, - size - ); - - SearchResult result = searchQueryService.search(query); - List matchedSkills = result.skillIds().isEmpty() - ? List.of() - : skillRepository.findByIdIn(result.skillIds()); - Map skillsById = matchedSkills.stream() - .collect(Collectors.toMap(Skill::getId, Function.identity())); - - List latestVersionIds = matchedSkills.stream() - .map(Skill::getLatestVersionId) - .filter(java.util.Objects::nonNull) - .distinct() - .toList(); - Map versionsById = latestVersionIds.isEmpty() - ? Map.of() - : skillVersionRepository.findByIdIn(latestVersionIds).stream() - .collect(Collectors.toMap(SkillVersion::getId, Function.identity())); - - List namespaceIds = matchedSkills.stream() - .map(Skill::getNamespaceId) - .distinct() - .toList(); - Map namespacesById = namespaceIds.isEmpty() - ? Map.of() - : namespaceRepository.findByIdIn(namespaceIds).stream() - .collect(Collectors.toMap(Namespace::getId, Function.identity())); - Map namespaceSlugsById = namespacesById.entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getSlug())); - - List skills = result.skillIds().stream() - .map(skillsById::get) - .filter(java.util.Objects::nonNull) - .filter(skill -> namespaceVisible(skill.getNamespaceId(), namespacesById, userId, userNsRoles)) - .map(skill -> toSummaryResponse(skill, versionsById, namespaceSlugsById)) - .toList(); - - return new SearchResponse(skills, skills.size(), result.page(), result.size()); + return searchVisibleSkills(keyword, namespaceId, sortBy != null ? sortBy : "newest", page, size, userId, userNsRoles, scope); } - private Long resolveNamespaceId(String namespaceSlug) { + private Long resolveNamespaceId(String namespaceSlug, String userId, Map userNsRoles) { if (namespaceSlug == null || namespaceSlug.isBlank()) { return null; } - return namespaceRepository.findBySlug(namespaceSlug) - .map(com.iflytek.skillhub.domain.namespace.Namespace::getId) - .orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", namespaceSlug)); + return namespaceService.getNamespaceBySlugForRead(namespaceSlug, userId, userNsRoles != null ? userNsRoles : Map.of()).getId(); } private SearchVisibilityScope buildVisibilityScope(String userId, Map userNsRoles) { @@ -135,6 +93,89 @@ public class SkillSearchAppService { return new SearchVisibilityScope(userId, memberNamespaceIds, adminNamespaceIds); } + private SearchResponse searchVisibleSkills( + String keyword, + Long namespaceId, + String sortBy, + int page, + int size, + String userId, + Map userNsRoles, + SearchVisibilityScope scope) { + int batchSize = Math.max(size, 20); + long rawTotal = Long.MAX_VALUE; + int rawPage = 0; + long visibleSeen = 0; + int visibleStart = page * size; + List pageItems = new java.util.ArrayList<>(); + + while ((long) rawPage * batchSize < rawTotal) { + SearchResult result = searchQueryService.search(new SearchQuery( + keyword, + namespaceId, + scope, + sortBy, + rawPage, + batchSize + )); + rawTotal = result.total(); + List visibleBatch = mapVisibleSkillSummaries(result.skillIds(), userId, userNsRoles); + for (SkillSummaryResponse item : visibleBatch) { + if (visibleSeen >= visibleStart && pageItems.size() < size) { + pageItems.add(item); + } + visibleSeen++; + } + if (result.skillIds().isEmpty()) { + break; + } + rawPage++; + } + + return new SearchResponse(pageItems, visibleSeen, page, size); + } + + private List mapVisibleSkillSummaries( + List skillIds, + String userId, + Map userNsRoles) { + if (skillIds.isEmpty()) { + return List.of(); + } + + List matchedSkills = skillRepository.findByIdIn(skillIds); + Map skillsById = matchedSkills.stream() + .collect(Collectors.toMap(Skill::getId, Function.identity())); + + List latestVersionIds = matchedSkills.stream() + .map(Skill::getLatestVersionId) + .filter(java.util.Objects::nonNull) + .distinct() + .toList(); + Map versionsById = latestVersionIds.isEmpty() + ? Map.of() + : skillVersionRepository.findByIdIn(latestVersionIds).stream() + .collect(Collectors.toMap(SkillVersion::getId, Function.identity())); + + List namespaceIds = matchedSkills.stream() + .map(Skill::getNamespaceId) + .distinct() + .toList(); + Map namespacesById = namespaceIds.isEmpty() + ? Map.of() + : namespaceRepository.findByIdIn(namespaceIds).stream() + .collect(Collectors.toMap(Namespace::getId, Function.identity())); + Map namespaceSlugsById = namespacesById.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getSlug())); + + return skillIds.stream() + .map(skillsById::get) + .filter(java.util.Objects::nonNull) + .filter(skill -> namespaceVisible(skill.getNamespaceId(), namespacesById, userId, userNsRoles)) + .map(skill -> toSummaryResponse(skill, versionsById, namespaceSlugsById)) + .toList(); + } + private SkillSummaryResponse toSummaryResponse( Skill skill, Map versionsById, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java index dd5d41d9..38c533b8 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java @@ -82,6 +82,21 @@ class NamespacePortalControllerTest { .andExpect(jsonPath("$.data[0].currentUserRole").value("OWNER")); } + @Test + void getNamespace_hidesArchivedNamespaceFromAnonymousUsers() throws Exception { + Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ARCHIVED, NamespaceType.TEAM); + given(namespaceService.getNamespaceBySlugForRead("team-a", null, Map.of())).willThrow( + new com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException( + "error.namespace.slug.notFound", + "team-a" + ) + ); + + mockMvc.perform(get("/api/v1/namespaces/team-a")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + @Test void archiveNamespace_returnsUpdatedNamespace() throws Exception { Namespace archived = namespace(1L, "team-a", NamespaceStatus.ARCHIVED, NamespaceType.TEAM); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespaceMemberCandidateServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespaceMemberCandidateServiceTest.java index 7d7b2a81..46e55f9f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespaceMemberCandidateServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespaceMemberCandidateServiceTest.java @@ -92,6 +92,29 @@ class NamespaceMemberCandidateServiceTest { service.searchCandidates("team-a", "ali", "owner-1", 10)); } + @Test + void searchCandidates_shouldRejectGlobalNamespaceBeforeMembershipChecks() { + Namespace namespace = new Namespace("global", "Global", "system"); + setField(namespace, "id", 1L); + namespace.setType(com.iflytek.skillhub.domain.namespace.NamespaceType.GLOBAL); + + NamespaceMemberCandidateService service = new NamespaceMemberCandidateService( + namespaceService, + namespaceAccessPolicy, + namespaceMemberRepository, + userAccountRepository + ); + + when(namespaceService.getNamespaceBySlug("global")).thenReturn(namespace); + when(namespaceAccessPolicy.isImmutable(namespace)).thenReturn(true); + + DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> + service.searchCandidates("global", "ali", "guest-1", 10)); + + assertEquals("error.namespace.system.immutable", exception.messageCode()); + verify(namespaceService, org.mockito.Mockito.never()).assertAdminOrOwner(1L, "guest-1"); + } + private void setField(Object target, String fieldName, Object value) { try { java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java index 7d34088f..755e04b1 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java @@ -1,8 +1,10 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; @@ -20,6 +22,7 @@ import java.util.Map; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -37,11 +40,14 @@ class SkillSearchAppServiceTest { @Mock private SkillVersionRepository skillVersionRepository; + @Mock + private NamespaceService namespaceService; + private SkillSearchAppService service; @BeforeEach void setUp() { - service = new SkillSearchAppService(searchQueryService, skillRepository, namespaceRepository, skillVersionRepository); + service = new SkillSearchAppService(searchQueryService, skillRepository, namespaceRepository, skillVersionRepository, namespaceService); } @Test @@ -64,6 +70,50 @@ class SkillSearchAppServiceTest { assertEquals(0, response.total()); } + @Test + void search_shouldFillVisiblePageAcrossArchivedNamespaceResults() { + Skill archivedSkill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC); + setField(archivedSkill, "id", 10L); + Skill visibleSkill = new Skill(2L, "visible-skill", "owner-1", SkillVisibility.PUBLIC); + setField(visibleSkill, "id", 11L); + + Namespace archivedNamespace = new Namespace("archived-team", "Archived Team", "owner-1"); + setField(archivedNamespace, "id", 1L); + archivedNamespace.setStatus(NamespaceStatus.ARCHIVED); + Namespace activeNamespace = new Namespace("team-a", "Team A", "owner-1"); + setField(activeNamespace, "id", 2L); + activeNamespace.setStatus(NamespaceStatus.ACTIVE); + + when(searchQueryService.search(org.mockito.ArgumentMatchers.any())) + .thenReturn(new SearchResult(List.of(10L, 11L), 2, 0, 20)); + when(skillRepository.findByIdIn(List.of(10L, 11L))).thenReturn(List.of(archivedSkill, visibleSkill)); + when(namespaceRepository.findByIdIn(List.of(1L, 2L))).thenReturn(List.of(archivedNamespace, activeNamespace)); + + SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 1, null, null); + + assertEquals(1, response.items().size()); + assertEquals("visible-skill", response.items().getFirst().slug()); + assertEquals(1, response.total()); + } + + @Test + void search_shouldHideArchivedNamespaceFilterForAnonymousUsers() { + Namespace archivedNamespace = new Namespace("archived-team", "Archived Team", "owner-1"); + setField(archivedNamespace, "id", 1L); + archivedNamespace.setStatus(NamespaceStatus.ARCHIVED); + when(namespaceService.getNamespaceBySlugForRead("archived-team", null, Map.of())).thenThrow( + new com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException( + "error.namespace.slug.notFound", + "archived-team" + ) + ); + + assertThrows( + com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException.class, + () -> service.search("skill", "archived-team", "newest", 0, 20, null, Map.of()) + ); + } + private void setField(Object target, String fieldName, Object value) { try { java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceService.java index 071f92d7..fef5ea3d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceService.java @@ -5,6 +5,8 @@ import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.util.Map; + @Service public class NamespaceService { @@ -44,8 +46,9 @@ public class NamespaceService { String operatorUserId) { Namespace namespace = namespaceRepository.findById(namespaceId) .orElseThrow(() -> new DomainBadRequestException("error.namespace.id.notFound", namespaceId)); + assertNotImmutable(namespace); assertAdminOrOwner(namespaceId, operatorUserId); - assertMutable(namespace); + assertWritable(namespace); if (displayName != null) { namespace.setDisplayName(displayName); @@ -65,6 +68,17 @@ public class NamespaceService { .orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", slug)); } + public Namespace getNamespaceBySlugForRead(String slug, String userId, Map userNsRoles) { + Namespace namespace = getNamespaceBySlug(slug); + if (namespace.getStatus() != NamespaceStatus.ARCHIVED) { + return namespace; + } + if (userId != null && userNsRoles != null && userNsRoles.containsKey(namespace.getId())) { + return namespace; + } + throw new DomainBadRequestException("error.namespace.slug.notFound", slug); + } + public Namespace getNamespace(Long namespaceId) { return namespaceRepository.findById(namespaceId) .orElseThrow(() -> new DomainBadRequestException("error.namespace.id.notFound", namespaceId)); @@ -85,9 +99,17 @@ public class NamespaceService { } void assertMutable(Namespace namespace) { + assertNotImmutable(namespace); + assertWritable(namespace); + } + + void assertNotImmutable(Namespace namespace) { if (namespaceAccessPolicy.isImmutable(namespace)) { throw new DomainBadRequestException("error.namespace.system.immutable", namespace.getSlug()); } + } + + private void assertWritable(Namespace namespace) { if (!namespaceAccessPolicy.canMutateSettings(namespace)) { throw new DomainBadRequestException("error.namespace.readonly", namespace.getSlug()); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/NamespaceServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/NamespaceServiceTest.java index d3127c52..48953f90 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/NamespaceServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/NamespaceServiceTest.java @@ -133,14 +133,28 @@ class NamespaceServiceTest { Namespace namespace = new Namespace("global", "Global", "system"); namespace.setType(NamespaceType.GLOBAL); when(namespaceRepository.findById(namespaceId)).thenReturn(Optional.of(namespace)); - when(namespaceMemberRepository.findByNamespaceIdAndUserId(namespaceId, operatorUserId)) - .thenReturn(Optional.of(new NamespaceMember(namespaceId, operatorUserId, NamespaceRole.OWNER))); when(namespaceAccessPolicy.isImmutable(namespace)).thenReturn(true); assertThrows(DomainBadRequestException.class, () -> namespaceService.updateNamespace(namespaceId, "Name", "Desc", null, operatorUserId)); } + @Test + void updateNamespace_shouldRejectGlobalNamespaceMutationBeforeMembershipChecks() { + Long namespaceId = 1L; + String operatorUserId = "user-404"; + Namespace namespace = new Namespace("global", "Global", "system"); + namespace.setType(NamespaceType.GLOBAL); + when(namespaceRepository.findById(namespaceId)).thenReturn(Optional.of(namespace)); + when(namespaceAccessPolicy.isImmutable(namespace)).thenReturn(true); + + DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> + namespaceService.updateNamespace(namespaceId, "Name", "Desc", null, operatorUserId)); + + assertEquals("error.namespace.system.immutable", exception.messageCode()); + verify(namespaceMemberRepository, never()).findByNamespaceIdAndUserId(namespaceId, operatorUserId); + } + @Test void getNamespaceBySlug_shouldReturnNamespace() { String slug = "test-slug";