fix(search): include permitted skills in clawhub explore

This commit is contained in:
dongmucat 2026-04-08 13:54:03 +08:00
parent 010c1a4e46
commit 359434470e
5 changed files with 121 additions and 14 deletions

View file

@ -1,8 +1,9 @@
package com.iflytek.skillhub.service;
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.auth.rbac.RbacService;
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.NamespaceService;
import com.iflytek.skillhub.domain.skill.Skill;
import com.iflytek.skillhub.domain.skill.SkillRepository;
@ -12,13 +13,12 @@ import com.iflytek.skillhub.search.SearchQuery;
import com.iflytek.skillhub.search.SearchQueryService;
import com.iflytek.skillhub.search.SearchResult;
import com.iflytek.skillhub.search.SearchVisibilityScope;
import org.springframework.stereotype.Service;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.springframework.stereotype.Service;
/**
* Application service that assembles discovery responses from search matches.
@ -36,18 +36,21 @@ public class SkillSearchAppService {
private final NamespaceRepository namespaceRepository;
private final NamespaceService namespaceService;
private final SkillLifecycleProjectionService skillLifecycleProjectionService;
private final RbacService rbacService;
public SkillSearchAppService(
SearchQueryService searchQueryService,
SkillRepository skillRepository,
NamespaceRepository namespaceRepository,
NamespaceService namespaceService,
SkillLifecycleProjectionService skillLifecycleProjectionService) {
SkillLifecycleProjectionService skillLifecycleProjectionService,
RbacService rbacService) {
this.searchQueryService = searchQueryService;
this.skillRepository = skillRepository;
this.namespaceRepository = namespaceRepository;
this.namespaceService = namespaceService;
this.skillLifecycleProjectionService = skillLifecycleProjectionService;
this.rbacService = rbacService;
}
public record SearchResponse(
@ -93,21 +96,36 @@ public class SkillSearchAppService {
}
private SearchVisibilityScope buildVisibilityScope(String userId, Map<Long, NamespaceRole> userNsRoles) {
if (userId == null || userNsRoles == null) {
if (userId == null) {
return SearchVisibilityScope.anonymous();
}
Set<Long> memberNamespaceIds = userNsRoles.keySet();
Set<Long> adminNamespaceIds = userNsRoles.entrySet().stream()
Map<Long, NamespaceRole> normalizedRoles = userNsRoles != null ? userNsRoles : Map.of();
Set<Long> memberNamespaceIds = normalizedRoles.keySet();
Set<Long> adminNamespaceIds = normalizedRoles.entrySet().stream()
.filter(e -> e.getValue() == NamespaceRole.ADMIN)
.map(Map.Entry::getKey)
.collect(java.util.stream.Collectors.toSet());
adminNamespaceIds.addAll(userNsRoles.entrySet().stream()
adminNamespaceIds.addAll(normalizedRoles.entrySet().stream()
.filter(e -> e.getValue() == NamespaceRole.OWNER)
.map(Map.Entry::getKey)
.toList());
return new SearchVisibilityScope(userId, memberNamespaceIds, adminNamespaceIds);
Set<String> platformRoles = rbacService.getUserRoleCodes(userId);
return new SearchVisibilityScope(
userId,
memberNamespaceIds,
adminNamespaceIds,
hasPlatformWideReadAccess(platformRoles)
);
}
private boolean hasPlatformWideReadAccess(Set<String> platformRoles) {
if (platformRoles == null || platformRoles.isEmpty()) {
return false;
}
return platformRoles.contains("SUPER_ADMIN");
}
private SearchResponse searchVisibleSkills(

View file

@ -1,5 +1,6 @@
package com.iflytek.skillhub.service;
import com.iflytek.skillhub.auth.rbac.RbacService;
import com.iflytek.skillhub.domain.namespace.Namespace;
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
@ -7,21 +8,23 @@ 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.VisibilityChecker;
import com.iflytek.skillhub.domain.skill.SkillVersionRepository;
import com.iflytek.skillhub.domain.skill.SkillVisibility;
import com.iflytek.skillhub.domain.skill.service.SkillLifecycleProjectionService;
import com.iflytek.skillhub.search.SearchQuery;
import com.iflytek.skillhub.search.SearchQueryService;
import com.iflytek.skillhub.search.SearchResult;
import org.mockito.ArgumentCaptor;
import com.iflytek.skillhub.search.SearchVisibilityScope;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.ArgumentCaptor;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ -49,6 +52,9 @@ class SkillSearchAppServiceTest {
@Mock
private NamespaceService namespaceService;
@Mock
private RbacService rbacService;
private SkillSearchAppService service;
@BeforeEach
@ -58,7 +64,8 @@ class SkillSearchAppServiceTest {
skillRepository,
namespaceRepository,
namespaceService,
new SkillLifecycleProjectionService(skillVersionRepository)
new SkillLifecycleProjectionService(skillVersionRepository),
rbacService
);
}
@ -190,6 +197,40 @@ class SkillSearchAppServiceTest {
assertEquals(List.of("code-generation", "official"), captor.getValue().labelSlugs());
}
@Test
void search_shouldIncludeMemberNamespacesInVisibilityScope() {
when(searchQueryService.search(any()))
.thenReturn(new SearchResult(List.of(), 0, 0, 20));
when(rbacService.getUserRoleCodes("user-9")).thenReturn(Set.of("USER"));
service.search("skill", null, "newest", 0, 20, "user-9", Map.of(7L, NamespaceRole.MEMBER));
ArgumentCaptor<SearchQuery> captor = ArgumentCaptor.forClass(SearchQuery.class);
verify(searchQueryService).search(captor.capture());
SearchVisibilityScope scope = captor.getValue().visibilityScope();
assertEquals("user-9", scope.userId());
assertEquals(Set.of(7L), scope.memberNamespaceIds());
assertEquals(Set.of(), scope.adminNamespaceIds());
assertEquals(false, scope.platformWideAccess());
}
@Test
void search_shouldGrantPlatformWideAccessToSuperAdmin() {
when(searchQueryService.search(any()))
.thenReturn(new SearchResult(List.of(), 0, 0, 20));
when(rbacService.getUserRoleCodes("admin-1")).thenReturn(Set.of("SUPER_ADMIN", "USER"));
service.search("skill", null, "newest", 0, 20, "admin-1", Map.of());
ArgumentCaptor<SearchQuery> captor = ArgumentCaptor.forClass(SearchQuery.class);
verify(searchQueryService).search(captor.capture());
SearchVisibilityScope scope = captor.getValue().visibilityScope();
assertEquals("admin-1", scope.userId());
assertEquals(true, scope.platformWideAccess());
}
private void setField(Object target, String fieldName, Object value) {
try {
java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName);

View file

@ -8,9 +8,17 @@ import java.util.Set;
public record SearchVisibilityScope(
String userId,
Set<Long> memberNamespaceIds,
Set<Long> adminNamespaceIds
Set<Long> adminNamespaceIds,
boolean platformWideAccess
) {
public SearchVisibilityScope(
String userId,
Set<Long> memberNamespaceIds,
Set<Long> adminNamespaceIds) {
this(userId, memberNamespaceIds, adminNamespaceIds, false);
}
public static SearchVisibilityScope anonymous() {
return new SearchVisibilityScope(null, Set.of(), Set.of());
return new SearchVisibilityScope(null, Set.of(), Set.of(), false);
}
}

View file

@ -105,6 +105,7 @@ public class PostgresFullTextQueryService implements SearchQueryService {
Set<Long> adminNamespaceIds = query.visibilityScope().adminNamespaceIds().isEmpty()
? Set.of(-1L)
: query.visibilityScope().adminNamespaceIds();
boolean platformWideAccess = query.visibilityScope().platformWideAccess();
StringBuilder sql = new StringBuilder();
sql.append("SELECT d.skill_id ");
@ -117,7 +118,9 @@ public class PostgresFullTextQueryService implements SearchQueryService {
sql.append("AND (d.visibility = 'PUBLIC' ");
if (query.visibilityScope().userId() != null) {
sql.append("OR (d.visibility = 'NAMESPACE_ONLY' AND d.namespace_id IN :memberNamespaceIds) ");
sql.append("OR (d.visibility = 'NAMESPACE_ONLY' AND :platformWideAccess = TRUE) ");
sql.append("OR (d.visibility = 'PRIVATE' AND (d.namespace_id IN :adminNamespaceIds OR d.owner_id = :userId)) ");
sql.append("OR (d.visibility = 'PRIVATE' AND :platformWideAccess = TRUE) ");
}
sql.append(") ");
@ -128,6 +131,7 @@ public class PostgresFullTextQueryService implements SearchQueryService {
sql.append("AND (n.status <> 'ARCHIVED' ");
if (query.visibilityScope().userId() != null) {
sql.append("OR d.namespace_id IN :memberNamespaceIds ");
sql.append("OR :platformWideAccess = TRUE ");
}
sql.append(") ");
@ -192,6 +196,7 @@ public class PostgresFullTextQueryService implements SearchQueryService {
if (query.visibilityScope().userId() != null) {
nativeQuery.setParameter("memberNamespaceIds", memberNamespaceIds);
nativeQuery.setParameter("adminNamespaceIds", adminNamespaceIds);
nativeQuery.setParameter("platformWideAccess", platformWideAccess);
nativeQuery.setParameter("userId", query.visibilityScope().userId());
}
@ -238,6 +243,7 @@ public class PostgresFullTextQueryService implements SearchQueryService {
if (query.visibilityScope().userId() != null) {
countQuery.setParameter("memberNamespaceIds", memberNamespaceIds);
countQuery.setParameter("adminNamespaceIds", adminNamespaceIds);
countQuery.setParameter("platformWideAccess", platformWideAccess);
countQuery.setParameter("userId", query.visibilityScope().userId());
}

View file

@ -392,6 +392,40 @@ class PostgresFullTextQueryServiceTest {
assertThat(sqlCaptor.getAllValues().getFirst()).contains("OR d.namespace_id IN :memberNamespaceIds");
}
@Test
void platformWideAccessShouldBypassNamespaceVisibilityRestrictions() {
EntityManager entityManager = mock(EntityManager.class);
Query nativeQuery = mock(Query.class);
Query countQuery = mock(Query.class);
when(entityManager.createNativeQuery(anyString()))
.thenReturn(nativeQuery)
.thenReturn(countQuery);
when(nativeQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(nativeQuery);
when(countQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(countQuery);
when(nativeQuery.getResultList()).thenReturn(List.of());
when(countQuery.getSingleResult()).thenReturn(0L);
PostgresFullTextQueryService service = new PostgresFullTextQueryService(entityManager);
service.search(new SearchQuery(
null,
null,
new SearchVisibilityScope("admin-1", Set.of(), Set.of(), true),
"newest",
0,
12
));
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
verify(entityManager, org.mockito.Mockito.times(2)).createNativeQuery(sqlCaptor.capture());
assertThat(sqlCaptor.getAllValues().getFirst())
.contains("OR (d.visibility = 'NAMESPACE_ONLY' AND :platformWideAccess = TRUE)")
.contains("OR (d.visibility = 'PRIVATE' AND :platformWideAccess = TRUE)")
.contains("OR :platformWideAccess = TRUE");
verify(nativeQuery).setParameter("platformWideAccess", true);
verify(countQuery).setParameter("platformWideAccess", true);
}
@Test
void maliciousKeywordShouldBeBoundAsParameterInsteadOfInlinedIntoSql() {
EntityManager entityManager = mock(EntityManager.class);