fix: tighten archived namespace visibility

This commit is contained in:
yun-zhi-ztl 2026-03-16 14:38:40 +08:00 • committed by Xudong Sun
parent b30311a537
commit 5c42f05486
8 changed files with 227 additions and 60 deletions

View file

@ -68,8 +68,10 @@ public class NamespaceController extends BaseApiController {
}
@GetMapping("/namespaces/{slug}")
public ApiResponse<NamespaceResponse> getNamespace(@PathVariable String slug) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
public ApiResponse<NamespaceResponse> getNamespace(@PathVariable String slug,
@RequestAttribute(value = "userId", required = false) String userId,
@RequestAttribute(value = "userNsRoles", required = false) Map<Long, NamespaceRole> userNsRoles) {
Namespace namespace = namespaceService.getNamespaceBySlugForRead(slug, userId, userNsRoles != null ? userNsRoles : Map.of());
return ok("response.success.read", NamespaceResponse.from(namespace));
}

View file

@ -43,11 +43,11 @@ public class NamespaceMemberCandidateService {
@Transactional(readOnly = true)
public List<NamespaceCandidateUserResponse> 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());
}

View file

@ -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<Long, NamespaceRole> 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<Skill> matchedSkills = result.skillIds().isEmpty()
? List.of()
: skillRepository.findByIdIn(result.skillIds());
Map<Long, Skill> skillsById = matchedSkills.stream()
.collect(Collectors.toMap(Skill::getId, Function.identity()));
List<Long> latestVersionIds = matchedSkills.stream()
.map(Skill::getLatestVersionId)
.filter(java.util.Objects::nonNull)
.distinct()
.toList();
Map<Long, SkillVersion> versionsById = latestVersionIds.isEmpty()
? Map.of()
: skillVersionRepository.findByIdIn(latestVersionIds).stream()
.collect(Collectors.toMap(SkillVersion::getId, Function.identity()));
List<Long> namespaceIds = matchedSkills.stream()
.map(Skill::getNamespaceId)
.distinct()
.toList();
Map<Long, Namespace> namespacesById = namespaceIds.isEmpty()
? Map.of()
: namespaceRepository.findByIdIn(namespaceIds).stream()
.collect(Collectors.toMap(Namespace::getId, Function.identity()));
Map<Long, String> namespaceSlugsById = namespacesById.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getSlug()));
List<SkillSummaryResponse> 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<Long, NamespaceRole> 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<Long, NamespaceRole> 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<Long, NamespaceRole> 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<SkillSummaryResponse> 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<SkillSummaryResponse> 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<SkillSummaryResponse> mapVisibleSkillSummaries(
List<Long> skillIds,
String userId,
Map<Long, NamespaceRole> userNsRoles) {
if (skillIds.isEmpty()) {
return List.of();
}
List<Skill> matchedSkills = skillRepository.findByIdIn(skillIds);
Map<Long, Skill> skillsById = matchedSkills.stream()
.collect(Collectors.toMap(Skill::getId, Function.identity()));
List<Long> latestVersionIds = matchedSkills.stream()
.map(Skill::getLatestVersionId)
.filter(java.util.Objects::nonNull)
.distinct()
.toList();
Map<Long, SkillVersion> versionsById = latestVersionIds.isEmpty()
? Map.of()
: skillVersionRepository.findByIdIn(latestVersionIds).stream()
.collect(Collectors.toMap(SkillVersion::getId, Function.identity()));
List<Long> namespaceIds = matchedSkills.stream()
.map(Skill::getNamespaceId)
.distinct()
.toList();
Map<Long, Namespace> namespacesById = namespaceIds.isEmpty()
? Map.of()
: namespaceRepository.findByIdIn(namespaceIds).stream()
.collect(Collectors.toMap(Namespace::getId, Function.identity()));
Map<Long, String> 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<Long, SkillVersion> versionsById,

View file

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

View file

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

View file

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

View file

@ -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<Long, NamespaceRole> 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());
}

View file

@ -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";