mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-07 02:57:51 +00:00
fix: restrict skill hiding to super admins
This commit is contained in:
parent
beff78512a
commit
b5bbb09c73
7 changed files with 91 additions and 18 deletions
|
|
@ -29,7 +29,7 @@ public class AdminSkillController extends BaseApiController {
|
|||
}
|
||||
|
||||
@PostMapping("/{skillId}/hide")
|
||||
@PreAuthorize("hasAnyRole('SKILL_ADMIN', 'SUPER_ADMIN')")
|
||||
@PreAuthorize("hasRole('SUPER_ADMIN')")
|
||||
public ApiResponse<AdminSkillMutationResponse> hideSkill(@PathVariable Long skillId,
|
||||
@RequestBody(required = false) AdminSkillActionRequest request,
|
||||
@AuthenticationPrincipal PlatformPrincipal principal,
|
||||
|
|
@ -45,7 +45,7 @@ public class AdminSkillController extends BaseApiController {
|
|||
}
|
||||
|
||||
@PostMapping("/{skillId}/unhide")
|
||||
@PreAuthorize("hasAnyRole('SKILL_ADMIN', 'SUPER_ADMIN')")
|
||||
@PreAuthorize("hasRole('SUPER_ADMIN')")
|
||||
public ApiResponse<AdminSkillMutationResponse> unhideSkill(@PathVariable Long skillId,
|
||||
@AuthenticationPrincipal PlatformPrincipal principal,
|
||||
HttpServletRequest httpRequest) {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
|
|||
import com.iflytek.skillhub.controller.BaseApiController;
|
||||
import com.iflytek.skillhub.domain.report.SkillReportDisposition;
|
||||
import com.iflytek.skillhub.domain.report.SkillReportService;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
|
||||
import com.iflytek.skillhub.dto.AdminSkillReportActionRequest;
|
||||
import com.iflytek.skillhub.dto.AdminSkillReportSummaryResponse;
|
||||
import com.iflytek.skillhub.dto.ApiResponse;
|
||||
|
|
@ -52,12 +53,16 @@ public class AdminSkillReportController extends BaseApiController {
|
|||
@RequestBody(required = false) AdminSkillReportActionRequest request,
|
||||
@AuthenticationPrincipal PlatformPrincipal principal,
|
||||
HttpServletRequest httpRequest) {
|
||||
SkillReportDisposition disposition = request != null && request.disposition() != null
|
||||
? SkillReportDisposition.valueOf(request.disposition().trim().toUpperCase())
|
||||
: SkillReportDisposition.RESOLVE_ONLY;
|
||||
if (disposition == SkillReportDisposition.RESOLVE_AND_HIDE && !principal.platformRoles().contains("SUPER_ADMIN")) {
|
||||
throw new DomainForbiddenException("error.skill.lifecycle.noPermission");
|
||||
}
|
||||
var report = skillReportService.resolveReport(
|
||||
reportId,
|
||||
principal.userId(),
|
||||
request != null && request.disposition() != null
|
||||
? SkillReportDisposition.valueOf(request.disposition().trim().toUpperCase())
|
||||
: SkillReportDisposition.RESOLVE_ONLY,
|
||||
disposition,
|
||||
request != null ? request.comment() : null,
|
||||
httpRequest.getRemoteAddr(),
|
||||
httpRequest.getHeader("User-Agent")
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ 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.VisibilityChecker;
|
||||
import com.iflytek.skillhub.domain.skill.SkillVersion;
|
||||
import com.iflytek.skillhub.domain.skill.SkillVersionRepository;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
|
||||
|
|
@ -31,18 +32,21 @@ public class SkillSearchAppService {
|
|||
private final NamespaceRepository namespaceRepository;
|
||||
private final SkillVersionRepository skillVersionRepository;
|
||||
private final NamespaceService namespaceService;
|
||||
private final VisibilityChecker visibilityChecker;
|
||||
|
||||
public SkillSearchAppService(
|
||||
SearchQueryService searchQueryService,
|
||||
SkillRepository skillRepository,
|
||||
NamespaceRepository namespaceRepository,
|
||||
SkillVersionRepository skillVersionRepository,
|
||||
NamespaceService namespaceService) {
|
||||
NamespaceService namespaceService,
|
||||
VisibilityChecker visibilityChecker) {
|
||||
this.searchQueryService = searchQueryService;
|
||||
this.skillRepository = skillRepository;
|
||||
this.namespaceRepository = namespaceRepository;
|
||||
this.skillVersionRepository = skillVersionRepository;
|
||||
this.namespaceService = namespaceService;
|
||||
this.visibilityChecker = visibilityChecker;
|
||||
}
|
||||
|
||||
public record SearchResponse(
|
||||
|
|
@ -171,6 +175,7 @@ public class SkillSearchAppService {
|
|||
return skillIds.stream()
|
||||
.map(skillsById::get)
|
||||
.filter(java.util.Objects::nonNull)
|
||||
.filter(skill -> visibilityChecker.canAccess(skill, userId, userNsRoles != null ? userNsRoles : Map.of()))
|
||||
.filter(skill -> namespaceVisible(skill.getNamespaceId(), namespacesById, userId, userNsRoles))
|
||||
.map(skill -> toSummaryResponse(skill, versionsById, namespaceSlugsById))
|
||||
.toList();
|
||||
|
|
|
|||
|
|
@ -51,8 +51,8 @@ class AdminSkillControllerTest {
|
|||
given(skillGovernanceService.hideSkill(org.mockito.ArgumentMatchers.eq(10L), org.mockito.ArgumentMatchers.eq("admin"), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.eq("policy")))
|
||||
.willReturn(skill);
|
||||
|
||||
PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SKILL_ADMIN"));
|
||||
var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN")));
|
||||
PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SUPER_ADMIN"));
|
||||
var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")));
|
||||
|
||||
mockMvc.perform(post("/api/v1/admin/skills/10/hide")
|
||||
.with(authentication(auth))
|
||||
|
|
@ -100,4 +100,18 @@ class AdminSkillControllerTest {
|
|||
.andExpect(status().isForbidden())
|
||||
.andExpect(jsonPath("$.code").value(403));
|
||||
}
|
||||
|
||||
@Test
|
||||
void hideSkill_withSkillAdminRole_returns403() throws Exception {
|
||||
PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SKILL_ADMIN"));
|
||||
var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN")));
|
||||
|
||||
mockMvc.perform(post("/api/v1/admin/skills/10/hide")
|
||||
.with(authentication(auth))
|
||||
.with(csrf())
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"reason\":\"policy\"}"))
|
||||
.andExpect(status().isForbidden())
|
||||
.andExpect(jsonPath("$.code").value(403));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -112,6 +112,17 @@ class AdminSkillReportControllerTest {
|
|||
.andExpect(jsonPath("$.data.status").value("RESOLVED"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void resolveReport_withHideDispositionAndSkillAdmin_returns403() throws Exception {
|
||||
mockMvc.perform(post("/api/v1/admin/skill-reports/99/resolve")
|
||||
.with(authentication(adminAuth()))
|
||||
.with(csrf())
|
||||
.contentType(APPLICATION_JSON)
|
||||
.content("{\"comment\":\"handled\",\"disposition\":\"RESOLVE_AND_HIDE\"}"))
|
||||
.andExpect(status().isForbidden())
|
||||
.andExpect(jsonPath("$.code").value(403));
|
||||
}
|
||||
|
||||
@Test
|
||||
void listReports_withAuditorRole_returns403() throws Exception {
|
||||
PlatformPrincipal principal = new PlatformPrincipal(
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ 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.search.SearchQueryService;
|
||||
|
|
@ -47,7 +48,14 @@ class SkillSearchAppServiceTest {
|
|||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
service = new SkillSearchAppService(searchQueryService, skillRepository, namespaceRepository, skillVersionRepository, namespaceService);
|
||||
service = new SkillSearchAppService(
|
||||
searchQueryService,
|
||||
skillRepository,
|
||||
namespaceRepository,
|
||||
skillVersionRepository,
|
||||
namespaceService,
|
||||
new VisibilityChecker()
|
||||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -114,6 +122,33 @@ class SkillSearchAppServiceTest {
|
|||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
void search_shouldExcludeHiddenSkillsForRegularUsers() {
|
||||
Skill visibleSkill = new Skill(1L, "visible-skill", "owner-1", SkillVisibility.PUBLIC);
|
||||
setField(visibleSkill, "id", 10L);
|
||||
visibleSkill.setLatestVersionId(101L);
|
||||
|
||||
Skill hiddenSkill = new Skill(1L, "hidden-skill", "owner-2", SkillVisibility.PUBLIC);
|
||||
setField(hiddenSkill, "id", 11L);
|
||||
hiddenSkill.setLatestVersionId(102L);
|
||||
hiddenSkill.setHidden(true);
|
||||
|
||||
Namespace namespace = new Namespace("team-a", "Team A", "owner-1");
|
||||
setField(namespace, "id", 1L);
|
||||
namespace.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(visibleSkill, hiddenSkill));
|
||||
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
|
||||
|
||||
SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 20, "user-9", Map.of());
|
||||
|
||||
assertEquals(1, response.items().size());
|
||||
assertEquals("visible-skill", response.items().getFirst().slug());
|
||||
assertEquals(1, response.total());
|
||||
}
|
||||
|
||||
private void setField(Object target, String fieldName, Object value) {
|
||||
try {
|
||||
java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName);
|
||||
|
|
|
|||
|
|
@ -107,6 +107,7 @@ export function SkillDetailPage() {
|
|||
const { data: diffSourceReadme } = useSkillReadme(namespace, slug, diffSourceVersion ?? undefined, diffSourceDocumentationPath)
|
||||
const { data: diffCompareReadme } = useSkillReadme(namespace, slug, diffCompareVersion ?? undefined, diffCompareDocumentationPath)
|
||||
const governanceVisible = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN')
|
||||
const canHideSkill = hasRole('SUPER_ADMIN')
|
||||
const isPendingPreview = skill?.viewingVersionStatus === 'PENDING_REVIEW'
|
||||
const canInteract = skill?.canInteract ?? true
|
||||
const canReport = skill?.canReport ?? true
|
||||
|
|
@ -736,15 +737,17 @@ export function SkillDetailPage() {
|
|||
<Card className="p-5 space-y-3">
|
||||
<div className="text-sm font-semibold font-heading text-foreground">{t('skillDetail.governance')}</div>
|
||||
<div className="flex flex-col gap-3">
|
||||
{!skill.hidden ? (
|
||||
<Button variant="outline" onClick={() => hideMutation.mutate()} disabled={hideMutation.isPending}>
|
||||
{hideMutation.isPending ? t('skillDetail.processing') : t('skillDetail.hideSkill')}
|
||||
</Button>
|
||||
) : (
|
||||
<Button variant="outline" onClick={() => unhideMutation.mutate()} disabled={unhideMutation.isPending}>
|
||||
{unhideMutation.isPending ? t('skillDetail.processing') : t('skillDetail.unhideSkill')}
|
||||
</Button>
|
||||
)}
|
||||
{canHideSkill ? (
|
||||
!skill.hidden ? (
|
||||
<Button variant="outline" onClick={() => hideMutation.mutate()} disabled={hideMutation.isPending}>
|
||||
{hideMutation.isPending ? t('skillDetail.processing') : t('skillDetail.hideSkill')}
|
||||
</Button>
|
||||
) : (
|
||||
<Button variant="outline" onClick={() => unhideMutation.mutate()} disabled={unhideMutation.isPending}>
|
||||
{unhideMutation.isPending ? t('skillDetail.processing') : t('skillDetail.unhideSkill')}
|
||||
</Button>
|
||||
)
|
||||
) : null}
|
||||
{selectedVersionEntry && (
|
||||
<Button variant="destructive" onClick={() => yankMutation.mutate()} disabled={yankMutation.isPending}>
|
||||
{yankMutation.isPending ? t('skillDetail.processing') : t('skillDetail.yankVersion')}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue