mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-10 22:41:02 +00:00
fix: harden hidden skill visibility and local dev restart flow (#62)
* fix: hide hidden skills from regular viewers * fix: avoid dashboard preview crash after registration * fix: restrict skill hiding to super admins * chore: remove dev process script * fix: hide hidden skills from slug resolution
This commit is contained in:
parent
a569707710
commit
b0e19af3ed
15 changed files with 286 additions and 170 deletions
2
Makefile
2
Makefile
|
|
@ -10,7 +10,7 @@ DEV_API_URL := http://localhost:8080
|
|||
STAGING_API_URL := http://localhost:8080
|
||||
STAGING_WEB_URL := http://localhost
|
||||
STAGING_SERVER_IMAGE := skillhub-server:staging
|
||||
DEV_PROCESS := python3 scripts/dev_process.py
|
||||
DEV_PROCESS := bash scripts/dev-process.sh
|
||||
DEV_SERVER_PREPARE := true
|
||||
DEV_SERVER_CMD := ./scripts/run-dev-app.sh
|
||||
BACKEND_TEST_JAVA_OPTIONS ?= -XX:+EnableDynamicAgentLoading
|
||||
|
|
|
|||
156
scripts/dev-process.sh
Executable file
156
scripts/dev-process.sh
Executable file
|
|
@ -0,0 +1,156 @@
|
|||
#!/usr/bin/env bash
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
usage() {
|
||||
echo "Usage:" >&2
|
||||
echo " $0 status --pid-file <file>" >&2
|
||||
echo " $0 stop --pid-file <file>" >&2
|
||||
echo " $0 start --pid-file <file> --log-file <file> --cwd <dir> -- <command...>" >&2
|
||||
exit 2
|
||||
}
|
||||
|
||||
require_value() {
|
||||
local flag="$1"
|
||||
local value="${2:-}"
|
||||
if [[ -z "$value" ]]; then
|
||||
echo "Missing value for $flag" >&2
|
||||
usage
|
||||
fi
|
||||
}
|
||||
|
||||
resolve_path() {
|
||||
local path="$1"
|
||||
if [[ "$path" = /* ]]; then
|
||||
printf '%s\n' "$path"
|
||||
else
|
||||
printf '%s/%s\n' "$(pwd)" "$path"
|
||||
fi
|
||||
}
|
||||
|
||||
is_running() {
|
||||
local pid_file="$1"
|
||||
[[ -f "$pid_file" ]] || return 1
|
||||
|
||||
local pid
|
||||
pid="$(cat "$pid_file" 2>/dev/null || true)"
|
||||
[[ "$pid" =~ ^[0-9]+$ ]] || return 1
|
||||
|
||||
if kill -0 "$pid" 2>/dev/null; then
|
||||
return 0
|
||||
fi
|
||||
|
||||
rm -f "$pid_file"
|
||||
return 1
|
||||
}
|
||||
|
||||
wait_for_exit() {
|
||||
local pid="$1"
|
||||
for _ in $(seq 1 50); do
|
||||
if ! kill -0 "$pid" 2>/dev/null; then
|
||||
return 0
|
||||
fi
|
||||
sleep 0.1
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
cmd="${1:-}"
|
||||
[[ -n "$cmd" ]] || usage
|
||||
shift || true
|
||||
|
||||
pid_file=""
|
||||
log_file=""
|
||||
cwd=""
|
||||
|
||||
case "$cmd" in
|
||||
status|stop)
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
--pid-file)
|
||||
require_value "$1" "${2:-}"
|
||||
pid_file="$2"
|
||||
shift 2
|
||||
;;
|
||||
*)
|
||||
usage
|
||||
;;
|
||||
esac
|
||||
done
|
||||
[[ -n "$pid_file" ]] || usage
|
||||
;;
|
||||
start)
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
--pid-file)
|
||||
require_value "$1" "${2:-}"
|
||||
pid_file="$2"
|
||||
shift 2
|
||||
;;
|
||||
--log-file)
|
||||
require_value "$1" "${2:-}"
|
||||
log_file="$2"
|
||||
shift 2
|
||||
;;
|
||||
--cwd)
|
||||
require_value "$1" "${2:-}"
|
||||
cwd="$2"
|
||||
shift 2
|
||||
;;
|
||||
--)
|
||||
shift
|
||||
break
|
||||
;;
|
||||
*)
|
||||
usage
|
||||
;;
|
||||
esac
|
||||
done
|
||||
[[ -n "$pid_file" && -n "$log_file" && -n "$cwd" && $# -gt 0 ]] || usage
|
||||
;;
|
||||
*)
|
||||
usage
|
||||
;;
|
||||
esac
|
||||
|
||||
case "$cmd" in
|
||||
status)
|
||||
is_running "$pid_file"
|
||||
;;
|
||||
stop)
|
||||
if ! is_running "$pid_file"; then
|
||||
rm -f "$pid_file"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
pid="$(cat "$pid_file")"
|
||||
kill "$pid" 2>/dev/null || true
|
||||
if ! wait_for_exit "$pid"; then
|
||||
kill -9 "$pid" 2>/dev/null || true
|
||||
wait_for_exit "$pid" || true
|
||||
fi
|
||||
rm -f "$pid_file"
|
||||
;;
|
||||
start)
|
||||
pid_file="$(resolve_path "$pid_file")"
|
||||
log_file="$(resolve_path "$log_file")"
|
||||
cwd="$(resolve_path "$cwd")"
|
||||
mkdir -p "$(dirname "$pid_file")" "$(dirname "$log_file")"
|
||||
if is_running "$pid_file"; then
|
||||
echo "Process already running with PID $(cat "$pid_file")" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
(
|
||||
cd "$cwd"
|
||||
if command -v setsid >/dev/null 2>&1; then
|
||||
setsid "$@" >>"$log_file" 2>&1 < /dev/null &
|
||||
else
|
||||
nohup "$@" >>"$log_file" 2>&1 < /dev/null &
|
||||
fi
|
||||
child_pid=$!
|
||||
disown "$child_pid" 2>/dev/null || true
|
||||
echo "$child_pid" >"$pid_file"
|
||||
)
|
||||
;;
|
||||
esac
|
||||
|
|
@ -1,144 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import signal
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
|
||||
def is_running(pid: int) -> bool:
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def read_pid(pid_file: Path) -> Optional[int]:
|
||||
if not pid_file.exists():
|
||||
return None
|
||||
content = pid_file.read_text(encoding="utf-8").strip()
|
||||
if not content:
|
||||
return None
|
||||
try:
|
||||
return int(content)
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def write_pid(pid_file: Path, pid: int) -> None:
|
||||
pid_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
pid_file.write_text(f"{pid}\n", encoding="utf-8")
|
||||
|
||||
|
||||
def start_process(args: argparse.Namespace) -> int:
|
||||
pid_file = Path(args.pid_file)
|
||||
log_file = Path(args.log_file)
|
||||
cwd = Path(args.cwd)
|
||||
existing_pid = read_pid(pid_file)
|
||||
if existing_pid and is_running(existing_pid):
|
||||
print(existing_pid)
|
||||
return 0
|
||||
|
||||
pid_file.unlink(missing_ok=True)
|
||||
log_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
command = list(args.command)
|
||||
if command and command[0] == "--":
|
||||
command = command[1:]
|
||||
|
||||
with log_file.open("ab") as log_handle, open(os.devnull, "rb") as devnull:
|
||||
process = subprocess.Popen(
|
||||
command,
|
||||
cwd=cwd,
|
||||
stdin=devnull,
|
||||
stdout=log_handle,
|
||||
stderr=subprocess.STDOUT,
|
||||
start_new_session=True,
|
||||
)
|
||||
|
||||
time.sleep(0.2)
|
||||
if process.poll() is not None:
|
||||
pid_file.unlink(missing_ok=True)
|
||||
return process.returncode or 1
|
||||
|
||||
write_pid(pid_file, process.pid)
|
||||
print(process.pid)
|
||||
return 0
|
||||
|
||||
|
||||
def stop_process(args: argparse.Namespace) -> int:
|
||||
pid_file = Path(args.pid_file)
|
||||
pid = read_pid(pid_file)
|
||||
if not pid:
|
||||
return 0
|
||||
|
||||
if not is_running(pid):
|
||||
pid_file.unlink(missing_ok=True)
|
||||
return 0
|
||||
|
||||
os.kill(pid, signal.SIGTERM)
|
||||
deadline = time.time() + args.timeout
|
||||
while time.time() < deadline:
|
||||
if not is_running(pid):
|
||||
pid_file.unlink(missing_ok=True)
|
||||
return 0
|
||||
time.sleep(0.2)
|
||||
|
||||
os.kill(pid, signal.SIGKILL)
|
||||
pid_file.unlink(missing_ok=True)
|
||||
return 0
|
||||
|
||||
|
||||
def status_process(args: argparse.Namespace) -> int:
|
||||
pid_file = Path(args.pid_file)
|
||||
pid = read_pid(pid_file)
|
||||
if not pid:
|
||||
return 1
|
||||
if not is_running(pid):
|
||||
pid_file.unlink(missing_ok=True)
|
||||
return 1
|
||||
print(pid)
|
||||
return 0
|
||||
|
||||
|
||||
def build_parser() -> argparse.ArgumentParser:
|
||||
parser = argparse.ArgumentParser(description="Manage detached dev processes.")
|
||||
subparsers = parser.add_subparsers(dest="action", required=True)
|
||||
|
||||
start_parser = subparsers.add_parser("start")
|
||||
start_parser.add_argument("--pid-file", required=True)
|
||||
start_parser.add_argument("--log-file", required=True)
|
||||
start_parser.add_argument("--cwd", required=True)
|
||||
start_parser.add_argument("command", nargs=argparse.REMAINDER)
|
||||
|
||||
stop_parser = subparsers.add_parser("stop")
|
||||
stop_parser.add_argument("--pid-file", required=True)
|
||||
stop_parser.add_argument("--timeout", type=float, default=10.0)
|
||||
|
||||
status_parser = subparsers.add_parser("status")
|
||||
status_parser.add_argument("--pid-file", required=True)
|
||||
return parser
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = build_parser()
|
||||
args = parser.parse_args()
|
||||
|
||||
if args.action == "start":
|
||||
if not args.command:
|
||||
parser.error("start requires a command")
|
||||
return start_process(args)
|
||||
if args.action == "stop":
|
||||
return stop_process(args)
|
||||
if args.action == "status":
|
||||
return status_process(args)
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ class AdminSkillReportControllerTest {
|
|||
report.setStatus(com.iflytek.skillhub.domain.report.SkillReportStatus.RESOLVED);
|
||||
when(skillReportService.resolveReport(
|
||||
org.mockito.ArgumentMatchers.eq(99L),
|
||||
org.mockito.ArgumentMatchers.eq("admin"),
|
||||
org.mockito.ArgumentMatchers.eq("super-admin"),
|
||||
org.mockito.ArgumentMatchers.eq(SkillReportDisposition.RESOLVE_AND_HIDE),
|
||||
org.mockito.ArgumentMatchers.eq("handled"),
|
||||
org.mockito.ArgumentMatchers.any(),
|
||||
|
|
@ -103,7 +103,7 @@ class AdminSkillReportControllerTest {
|
|||
.thenReturn(report);
|
||||
|
||||
mockMvc.perform(post("/api/v1/admin/skill-reports/99/resolve")
|
||||
.with(authentication(adminAuth()))
|
||||
.with(authentication(superAdminAuth()))
|
||||
.with(csrf())
|
||||
.contentType(APPLICATION_JSON)
|
||||
.content("{\"comment\":\"handled\",\"disposition\":\"RESOLVE_AND_HIDE\"}"))
|
||||
|
|
@ -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(
|
||||
|
|
@ -136,4 +147,13 @@ class AdminSkillReportControllerTest {
|
|||
principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN"))
|
||||
);
|
||||
}
|
||||
|
||||
private UsernamePasswordAuthenticationToken superAdminAuth() {
|
||||
PlatformPrincipal principal = new PlatformPrincipal(
|
||||
"super-admin", "super-admin", "admin@example.com", "", "github", Set.of("SUPER_ADMIN")
|
||||
);
|
||||
return new UsernamePasswordAuthenticationToken(
|
||||
principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -74,8 +82,10 @@ class SkillSearchAppServiceTest {
|
|||
void search_shouldFillVisiblePageAcrossArchivedNamespaceResults() {
|
||||
Skill archivedSkill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC);
|
||||
setField(archivedSkill, "id", 10L);
|
||||
archivedSkill.setLatestVersionId(110L);
|
||||
Skill visibleSkill = new Skill(2L, "visible-skill", "owner-1", SkillVisibility.PUBLIC);
|
||||
setField(visibleSkill, "id", 11L);
|
||||
visibleSkill.setLatestVersionId(111L);
|
||||
|
||||
Namespace archivedNamespace = new Namespace("archived-team", "Archived Team", "owner-1");
|
||||
setField(archivedNamespace, "id", 1L);
|
||||
|
|
@ -114,6 +124,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);
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ public class SkillSlugResolutionService {
|
|||
? Optional.empty()
|
||||
: skills.stream().filter(skill -> currentUserId.equals(skill.getOwnerId())).findFirst();
|
||||
Optional<Skill> publishedSkill = skills.stream()
|
||||
.filter(skill -> skill.getLatestVersionId() != null)
|
||||
.filter(skill -> skill.getLatestVersionId() != null && !skill.isHidden())
|
||||
.findFirst();
|
||||
|
||||
if (preference == Preference.CURRENT_USER) {
|
||||
|
|
|
|||
|
|
@ -58,6 +58,16 @@ class SkillSlugResolutionServiceTest {
|
|||
service.resolve(1L, "demo", null, SkillSlugResolutionService.Preference.CURRENT_USER));
|
||||
}
|
||||
|
||||
@Test
|
||||
void throwsWhenOnlyPublishedSkillIsHiddenFromCurrentUser() throws Exception {
|
||||
Skill hiddenPublishedSkill = createSkill(4L, "demo", "user-2", 44L);
|
||||
hiddenPublishedSkill.setHidden(true);
|
||||
when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of(hiddenPublishedSkill));
|
||||
|
||||
assertThrows(DomainBadRequestException.class, () ->
|
||||
service.resolve(1L, "demo", "user-9", SkillSlugResolutionService.Preference.CURRENT_USER));
|
||||
}
|
||||
|
||||
private Skill createSkill(Long id, String slug, String ownerId, Long latestVersionId) throws Exception {
|
||||
Skill skill = new Skill(1L, slug, ownerId, SkillVisibility.PUBLIC);
|
||||
Field idField = Skill.class.getDeclaredField("id");
|
||||
|
|
|
|||
|
|
@ -25,4 +25,12 @@ describe('limitPreviewItems', () => {
|
|||
remainingCount: 1,
|
||||
})
|
||||
})
|
||||
|
||||
it('returns an empty preview instead of throwing when input is not an array', () => {
|
||||
expect(limitPreviewItems({ items: ['a', 'b'] } as never, 3)).toEqual({
|
||||
items: [],
|
||||
hasMore: false,
|
||||
remainingCount: 0,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,10 +1,11 @@
|
|||
export function limitPreviewItems<T>(items: T[], limit: number): {
|
||||
export function limitPreviewItems<T>(items: T[] | null | undefined | unknown, limit: number): {
|
||||
items: T[]
|
||||
hasMore: boolean
|
||||
remainingCount: number
|
||||
} {
|
||||
const visibleItems = items.slice(0, limit)
|
||||
const remainingCount = Math.max(items.length - visibleItems.length, 0)
|
||||
const normalizedItems: T[] = Array.isArray(items) ? (items as T[]) : []
|
||||
const visibleItems = normalizedItems.slice(0, limit)
|
||||
const remainingCount = Math.max(normalizedItems.length - visibleItems.length, 0)
|
||||
|
||||
return {
|
||||
items: visibleItems,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { Link } from '@tanstack/react-router'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import { useAuth } from '@/features/auth/use-auth'
|
||||
import type { SkillSummary } from '@/api/types'
|
||||
import { useMySkills } from '@/shared/hooks/use-skill-queries'
|
||||
import { TokenList } from '@/features/token/token-list'
|
||||
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card'
|
||||
|
|
@ -14,7 +15,7 @@ export function DashboardPage() {
|
|||
const { user, hasRole } = useAuth()
|
||||
const governanceVisible = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN')
|
||||
const { data: skillPage, isLoading: isLoadingSkills } = useMySkills({ page: 0, size: skillPreviewPageSize })
|
||||
const skillPreview = limitPreviewItems(skillPage?.items ?? [], DASHBOARD_PREVIEW_LIMIT)
|
||||
const skillPreview = limitPreviewItems<SkillSummary>(skillPage?.items ?? [], DASHBOARD_PREVIEW_LIMIT)
|
||||
|
||||
return (
|
||||
<div className="space-y-8 animate-fade-up">
|
||||
|
|
|
|||
|
|
@ -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