diff --git a/.gitignore b/.gitignore index c4c2a57b..f854fd8d 100644 --- a/.gitignore +++ b/.gitignore @@ -67,4 +67,5 @@ __pycache__/ # Superpowers (AI planning artifacts) docs/superpowers/ -CLAUDE.md \ No newline at end of file +docs/review/ +CLAUDE.md diff --git a/scripts/check-openapi-generated.sh b/scripts/check-openapi-generated.sh index 2cc21ef5..c111ab0f 100755 --- a/scripts/check-openapi-generated.sh +++ b/scripts/check-openapi-generated.sh @@ -6,7 +6,18 @@ ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" SERVER_DIR="$ROOT_DIR/server" WEB_DIR="$ROOT_DIR/web" API_LOG="${TMPDIR:-/tmp}/skillhub-openapi-check.log" +BUILD_LOG="${TMPDIR:-/tmp}/skillhub-openapi-build.log" SERVER_PID="" +OPENAPI_URL="http://127.0.0.1:8080/v3/api-docs" + +print_log_tail() { + local log_file="$1" + + if [[ -f "$log_file" ]]; then + echo "--- Last 50 lines of $log_file ---" >&2 + tail -n 50 "$log_file" >&2 || true + fi +} cleanup() { if [[ -n "$SERVER_PID" ]] && kill -0 "$SERVER_PID" 2>/dev/null; then @@ -21,6 +32,15 @@ trap cleanup EXIT cd "$ROOT_DIR" docker compose up -d --wait postgres redis +( + cd "$SERVER_DIR" + ./mvnw -pl skillhub-app -am -DskipTests install +) >"$BUILD_LOG" 2>&1 || { + echo "Failed to prepare backend modules. See $BUILD_LOG" >&2 + print_log_tail "$BUILD_LOG" + exit 1 +} + ( cd "$SERVER_DIR" SPRING_PROFILES_ACTIVE=local ./mvnw -pl skillhub-app spring-boot:run @@ -28,14 +48,22 @@ docker compose up -d --wait postgres redis SERVER_PID=$! for _ in $(seq 1 90); do - if curl -fsS "http://127.0.0.1:8080/v3/api-docs" >/dev/null 2>&1; then + if curl -fsS "$OPENAPI_URL" >/dev/null 2>&1; then break fi + + if ! kill -0 "$SERVER_PID" 2>/dev/null; then + echo "Backend exited before exposing /v3/api-docs. See $API_LOG" >&2 + print_log_tail "$API_LOG" + exit 1 + fi + sleep 2 done -if ! curl -fsS "http://127.0.0.1:8080/v3/api-docs" >/dev/null 2>&1; then +if ! curl -fsS "$OPENAPI_URL" >/dev/null 2>&1; then echo "Backend did not expose /v3/api-docs. See $API_LOG" >&2 + print_log_tail "$API_LOG" exit 1 fi diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java index a131e068..b25e6c5b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java @@ -2,28 +2,29 @@ package com.iflytek.skillhub.compat; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.compat.dto.ClawHubPublishResponse; +import com.iflytek.skillhub.compat.dto.ClawHubSkillItem; +import com.iflytek.skillhub.compat.dto.ClawHubResolveResponse; +import com.iflytek.skillhub.compat.dto.ClawHubSearchResponse; +import com.iflytek.skillhub.compat.dto.ClawHubWhoamiResponse; import com.iflytek.skillhub.controller.support.ZipPackageExtractor; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; -import com.iflytek.skillhub.compat.dto.ClawHubResolveResponse; -import com.iflytek.skillhub.compat.dto.ClawHubSearchResponse; -import com.iflytek.skillhub.compat.dto.ClawHubSkillItem; -import com.iflytek.skillhub.compat.dto.ClawHubWhoamiResponse; import com.iflytek.skillhub.service.SkillSearchAppService; -import java.io.IOException; -import org.springframework.security.core.annotation.AuthenticationPrincipal; +import jakarta.servlet.http.HttpServletRequest; +import org.slf4j.MDC; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; +import java.io.IOException; import java.util.List; import java.util.Map; -import org.slf4j.MDC; @RestController @RequestMapping("/api/compat/v1") @@ -51,18 +52,31 @@ public class ClawHubCompatController { } @GetMapping("/search") - public ClawHubSearchResponse search(@RequestParam String q, - @RequestAttribute(value = "userId", required = false) String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { - var result = skillSearchAppService.search(q, null, "relevance", 0, 20, userId, userNsRoles != null ? userNsRoles : Map.of()); - return new ClawHubSearchResponse(result.items().stream() - .map(item -> new ClawHubSkillItem( - mapper.toCanonical(item.namespace(), item.slug()), - item.summary(), - item.latestVersion(), - item.starCount() - )) - .toList()); + public ClawHubSearchResponse search( + @RequestParam String q, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int limit, + @RequestAttribute(value = "userId", required = false) String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + SkillSearchAppService.SearchResponse response = skillSearchAppService.search( + q, + null, + q == null || q.isBlank() ? "newest" : "relevance", + page, + limit, + userId, + userNsRoles + ); + + List items = response.items().stream() + .map(item -> new ClawHubSkillItem( + mapper.toCanonical(item.namespace(), item.slug()), + item.summary(), + item.latestVersion(), + item.starCount())) + .toList(); + + return new ClawHubSearchResponse(items); } @GetMapping("/resolve/{canonicalSlug}") @@ -72,14 +86,14 @@ public class ClawHubCompatController { @RequestAttribute(value = "userId", required = false) String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { SkillCoordinate coord = mapper.fromCanonical(canonicalSlug); - var resolved = skillQueryService.resolveVersion( - coord.namespace(), - coord.slug(), - "latest".equals(version) ? null : version, - "latest".equals(version) ? "latest" : null, - null, - userId, - userNsRoles != null ? userNsRoles : Map.of() + SkillQueryService.ResolvedVersionDTO resolved = skillQueryService.resolveVersion( + coord.namespace(), + coord.slug(), + "latest".equals(version) ? null : version, + "latest".equals(version) ? "latest" : null, + null, + userId, + userNsRoles != null ? userNsRoles : Map.of() ); return new ClawHubResolveResponse( canonicalSlug, @@ -93,39 +107,39 @@ public class ClawHubCompatController { @RequestParam(defaultValue = "latest") String version) { SkillCoordinate coord = mapper.fromCanonical(canonicalSlug); String location = "latest".equals(version) - ? "/api/v1/skills/" + coord.namespace() + "/" + coord.slug() + "/download" - : "/api/v1/skills/" + coord.namespace() + "/" + coord.slug() + "/versions/" + version + "/download"; + ? "/api/v1/skills/" + coord.namespace() + "/" + coord.slug() + "/download" + : "/api/v1/skills/" + coord.namespace() + "/" + coord.slug() + "/versions/" + version + "/download"; return ResponseEntity.status(HttpStatus.FOUND) - .header(HttpHeaders.LOCATION, location) - .build(); + .header(HttpHeaders.LOCATION, location) + .build(); } @PostMapping("/publish") public ClawHubPublishResponse publish(@RequestParam("file") MultipartFile file, @RequestParam("namespace") String namespace, @AuthenticationPrincipal PlatformPrincipal principal, - jakarta.servlet.http.HttpServletRequest request) throws IOException { - var result = skillPublishService.publishFromEntries( - namespace, - zipPackageExtractor.extract(file), - principal.userId(), - SkillVisibility.PUBLIC, - principal.platformRoles() + HttpServletRequest request) throws IOException { + SkillPublishService.PublishResult result = skillPublishService.publishFromEntries( + namespace, + zipPackageExtractor.extract(file), + principal.userId(), + SkillVisibility.PUBLIC, + principal.platformRoles() ); auditLogService.record( - principal.userId(), - "COMPAT_PUBLISH", - "SKILL_VERSION", - result.version().getId(), - MDC.get("requestId"), - request.getRemoteAddr(), - request.getHeader("User-Agent"), - "{\"namespace\":\"" + namespace + "\"}" + principal.userId(), + "COMPAT_PUBLISH", + "SKILL_VERSION", + result.version().getId(), + MDC.get("requestId"), + request.getRemoteAddr(), + request.getHeader("User-Agent"), + "{\"namespace\":\"" + namespace + "\"}" ); return new ClawHubPublishResponse( - mapper.toCanonical(namespace, result.slug()), - result.version().getVersion(), - result.version().getStatus().name() + mapper.toCanonical(namespace, result.slug()), + result.version().getVersion(), + result.version().getStatus().name() ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/CliController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/CliController.java index d5b031b1..13d8071a 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/CliController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/CliController.java @@ -1,39 +1,41 @@ package com.iflytek.skillhub.controller; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; import com.iflytek.skillhub.domain.skill.validation.SkillPackageValidator; import com.iflytek.skillhub.domain.skill.validation.ValidationResult; +import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.CliWhoamiResponse; import com.iflytek.skillhub.dto.ResolveVersionResponse; import com.iflytek.skillhub.dto.SkillCheckResponse; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import org.springframework.security.core.annotation.AuthenticationPrincipal; import com.iflytek.skillhub.exception.UnauthorizedException; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; import java.io.IOException; -import java.util.ArrayList; import java.util.List; -import java.util.zip.ZipEntry; -import java.util.zip.ZipInputStream; +import java.util.Map; @RestController @RequestMapping("/api/v1/cli") public class CliController extends BaseApiController { private final SkillPackageValidator skillPackageValidator; + private final SkillPackageArchiveExtractor skillPackageArchiveExtractor; private final SkillQueryService skillQueryService; public CliController(ApiResponseFactory responseFactory, SkillPackageValidator skillPackageValidator, + SkillPackageArchiveExtractor skillPackageArchiveExtractor, SkillQueryService skillQueryService) { super(responseFactory); this.skillPackageValidator = skillPackageValidator; + this.skillPackageArchiveExtractor = skillPackageArchiveExtractor; this.skillQueryService = skillQueryService; } @@ -48,7 +50,18 @@ public class CliController extends BaseApiController { @PostMapping("/check") public ApiResponse check(@RequestParam("file") MultipartFile file) throws IOException { - List entries = extractZipEntries(file); + List entries; + try { + entries = skillPackageArchiveExtractor.extract(file); + } catch (IllegalArgumentException e) { + SkillCheckResponse response = new SkillCheckResponse( + false, + List.of(e.getMessage()), + 0, + 0L + ); + return ok("response.success.validated", response); + } ValidationResult result = skillPackageValidator.validate(entries); SkillCheckResponse response = new SkillCheckResponse( @@ -68,56 +81,25 @@ public class CliController extends BaseApiController { @RequestParam(required = false) String tag, @RequestParam(required = false) String hash, @RequestAttribute(value = "userId", required = false) String userId, - @RequestAttribute(value = "userNsRoles", required = false) java.util.Map userNsRoles) { + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { SkillQueryService.ResolvedVersionDTO resolved = skillQueryService.resolveVersion( - namespace, - slug, - version, - tag, - hash, - userId, - userNsRoles != null ? userNsRoles : java.util.Map.of() + namespace, + slug, + version, + tag, + hash, + userId, + userNsRoles != null ? userNsRoles : Map.of() ); return ok("response.success.read", new ResolveVersionResponse( - resolved.skillId(), - resolved.namespace(), - resolved.slug(), - resolved.version(), - resolved.versionId(), - resolved.fingerprint(), - resolved.matched(), - resolved.downloadUrl() + resolved.skillId(), + resolved.namespace(), + resolved.slug(), + resolved.version(), + resolved.versionId(), + resolved.fingerprint(), + resolved.matched(), + resolved.downloadUrl() )); } - - private List extractZipEntries(MultipartFile file) throws IOException { - List entries = new ArrayList<>(); - - try (ZipInputStream zis = new ZipInputStream(file.getInputStream())) { - ZipEntry zipEntry; - while ((zipEntry = zis.getNextEntry()) != null) { - if (!zipEntry.isDirectory()) { - byte[] content = zis.readAllBytes(); - entries.add(new PackageEntry( - zipEntry.getName(), - content, - content.length, - determineContentType(zipEntry.getName()) - )); - } - zis.closeEntry(); - } - } - - return entries; - } - - private String determineContentType(String filename) { - if (filename.endsWith(".py")) return "text/x-python"; - if (filename.endsWith(".json")) return "application/json"; - if (filename.endsWith(".yaml") || filename.endsWith(".yml")) return "application/x-yaml"; - if (filename.endsWith(".txt")) return "text/plain"; - if (filename.endsWith(".md")) return "text/markdown"; - return "application/octet-stream"; - } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/TokenController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/TokenController.java index d1b8420b..cb2af098 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/TokenController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/TokenController.java @@ -4,11 +4,11 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.token.ApiTokenService; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; -import com.iflytek.skillhub.dto.MessageResponse; import com.iflytek.skillhub.dto.TokenCreateRequest; import com.iflytek.skillhub.dto.TokenCreateResponse; import com.iflytek.skillhub.dto.TokenSummaryResponse; import jakarta.validation.Valid; +import org.springframework.http.ResponseEntity; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.*; @@ -59,10 +59,10 @@ public class TokenController extends BaseApiController { } @DeleteMapping("/{id}") - public ApiResponse revoke( + public ResponseEntity revoke( @AuthenticationPrincipal PlatformPrincipal principal, @PathVariable Long id) { apiTokenService.revokeToken(id, principal.userId()); - return ok("response.success.revoked", new MessageResponse("Token revoked")); + return ResponseEntity.noContent().build(); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AuditLogController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AuditLogController.java index 32ec0079..21fa7405 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AuditLogController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AuditLogController.java @@ -5,7 +5,7 @@ import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.AuditLogItemResponse; import com.iflytek.skillhub.dto.PageResponse; -import com.iflytek.skillhub.domain.audit.AuditLogQueryService; +import com.iflytek.skillhub.service.AdminAuditLogAppService; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.*; @@ -13,12 +13,12 @@ import org.springframework.web.bind.annotation.*; @RequestMapping("/api/v1/admin/audit-logs") public class AuditLogController extends BaseApiController { - private final AuditLogQueryService auditLogQueryService; + private final AdminAuditLogAppService adminAuditLogAppService; - public AuditLogController(ApiResponseFactory responseFactory, - AuditLogQueryService auditLogQueryService) { + public AuditLogController(AdminAuditLogAppService adminAuditLogAppService, + ApiResponseFactory responseFactory) { super(responseFactory); - this.auditLogQueryService = auditLogQueryService; + this.adminAuditLogAppService = adminAuditLogAppService; } @GetMapping @@ -28,16 +28,6 @@ public class AuditLogController extends BaseApiController { @RequestParam(defaultValue = "20") int size, @RequestParam(required = false) String userId, @RequestParam(required = false) String action) { - var logs = auditLogQueryService.list(page, size, userId, action) - .map(log -> new AuditLogItemResponse( - String.valueOf(log.getId()), - log.getActorUserId(), - log.getAction(), - log.getTargetType(), - log.getTargetId() != null ? String.valueOf(log.getTargetId()) : "", - log.getCreatedAt(), - log.getClientIp() - )); - return ok("response.success.read", PageResponse.from(logs)); + return ok("response.success.read", adminAuditLogAppService.listAuditLogs(page, size, userId, action)); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java index 1d6363b4..850ecc26 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java @@ -1,7 +1,7 @@ package com.iflytek.skillhub.controller.admin; -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.AdminUserMutationResponse; import com.iflytek.skillhub.dto.AdminUserRoleUpdateRequest; import com.iflytek.skillhub.dto.AdminUserStatusUpdateRequest; @@ -9,7 +9,7 @@ import com.iflytek.skillhub.dto.AdminUserSummaryResponse; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.PageResponse; -import com.iflytek.skillhub.service.AdminUserManagementService; +import com.iflytek.skillhub.service.AdminUserAppService; import jakarta.validation.Valid; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; @@ -19,12 +19,12 @@ import org.springframework.web.bind.annotation.*; @RequestMapping("/api/v1/admin/users") public class UserManagementController extends BaseApiController { - private final AdminUserManagementService adminUserManagementService; + private final AdminUserAppService adminUserAppService; - public UserManagementController(ApiResponseFactory responseFactory, - AdminUserManagementService adminUserManagementService) { + public UserManagementController(AdminUserAppService adminUserAppService, + ApiResponseFactory responseFactory) { super(responseFactory); - this.adminUserManagementService = adminUserManagementService; + this.adminUserAppService = adminUserAppService; } @GetMapping @@ -34,17 +34,17 @@ public class UserManagementController extends BaseApiController { @RequestParam(required = false) String status, @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "20") int size) { - return ok("response.success.read", adminUserManagementService.listUsers(search, status, page, size)); + return ok("response.success.read", adminUserAppService.listUsers(search, status, page, size)); } @PutMapping("/{userId}/role") @PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')") public ApiResponse updateUserRole( @PathVariable String userId, - @Valid @RequestBody AdminUserRoleUpdateRequest request, - @AuthenticationPrincipal PlatformPrincipal principal) { - AdminUserSummaryResponse user = adminUserManagementService.updateUserRole(userId, request.role(), principal); - return ok("response.success.updated", new AdminUserMutationResponse(user.userId(), request.role(), user.status())); + @AuthenticationPrincipal PlatformPrincipal principal, + @Valid @RequestBody AdminUserRoleUpdateRequest request) { + return ok("response.success.updated", + adminUserAppService.updateUserRole(userId, request.role(), principal.platformRoles())); } @PutMapping("/{userId}/status") @@ -52,28 +52,24 @@ public class UserManagementController extends BaseApiController { public ApiResponse updateUserStatus( @PathVariable String userId, @Valid @RequestBody AdminUserStatusUpdateRequest request) { - AdminUserSummaryResponse user = adminUserManagementService.updateUserStatus(userId, request.status()); - return ok("response.success.updated", new AdminUserMutationResponse(user.userId(), null, user.status())); + return ok("response.success.updated", adminUserAppService.updateUserStatus(userId, request.status())); } @PostMapping("/{userId}/approve") @PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')") public ApiResponse approveUser(@PathVariable String userId) { - AdminUserSummaryResponse user = adminUserManagementService.approveUser(userId); - return ok("response.success.updated", new AdminUserMutationResponse(user.userId(), null, user.status())); + return ok("response.success.updated", adminUserAppService.updateUserStatus(userId, "ACTIVE")); } @PostMapping("/{userId}/disable") @PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')") public ApiResponse disableUser(@PathVariable String userId) { - AdminUserSummaryResponse user = adminUserManagementService.disableUser(userId); - return ok("response.success.updated", new AdminUserMutationResponse(user.userId(), null, user.status())); + return ok("response.success.updated", adminUserAppService.updateUserStatus(userId, "DISABLED")); } @PostMapping("/{userId}/enable") @PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')") public ApiResponse enableUser(@PathVariable String userId) { - AdminUserSummaryResponse user = adminUserManagementService.enableUser(userId); - return ok("response.success.updated", new AdminUserMutationResponse(user.userId(), null, user.status())); + return ok("response.success.updated", adminUserAppService.updateUserStatus(userId, "ACTIVE")); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/cli/CliPublishController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/cli/CliPublishController.java index bde961f5..73cbd1fa 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/cli/CliPublishController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/cli/CliPublishController.java @@ -2,8 +2,9 @@ package com.iflytek.skillhub.controller.cli; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.controller.BaseApiController; -import com.iflytek.skillhub.controller.support.ZipPackageExtractor; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; @@ -26,18 +27,18 @@ import java.util.List; public class CliPublishController extends BaseApiController { private final SkillPublishService skillPublishService; - private final ZipPackageExtractor zipPackageExtractor; + private final SkillPackageArchiveExtractor skillPackageArchiveExtractor; private final SkillHubMetrics skillHubMetrics; private final AuditLogService auditLogService; public CliPublishController(SkillPublishService skillPublishService, - ZipPackageExtractor zipPackageExtractor, + SkillPackageArchiveExtractor skillPackageArchiveExtractor, ApiResponseFactory responseFactory, SkillHubMetrics skillHubMetrics, AuditLogService auditLogService) { super(responseFactory); this.skillPublishService = skillPublishService; - this.zipPackageExtractor = zipPackageExtractor; + this.skillPackageArchiveExtractor = skillPackageArchiveExtractor; this.skillHubMetrics = skillHubMetrics; this.auditLogService = auditLogService; } @@ -53,7 +54,12 @@ public class CliPublishController extends BaseApiController { SkillVisibility skillVisibility = SkillVisibility.valueOf(visibility.toUpperCase()); - List entries = zipPackageExtractor.extract(file); + List entries; + try { + entries = skillPackageArchiveExtractor.extract(file); + } catch (IllegalArgumentException e) { + throw new DomainBadRequestException("error.skill.publish.package.invalid", e.getMessage()); + } SkillPublishService.PublishResult publishResult = skillPublishService.publishFromEntries( namespace, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java index f7e541cf..14628e63 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java @@ -18,15 +18,27 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; -import com.iflytek.skillhub.dto.*; +import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.PromotionActionRequest; +import com.iflytek.skillhub.dto.PromotionRequestDto; +import com.iflytek.skillhub.dto.PromotionResponseDto; import jakarta.servlet.http.HttpServletRequest; +import org.slf4j.MDC; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; import java.util.Map; import java.util.Set; -import org.slf4j.MDC; @RestController @RequestMapping("/api/v1/promotions") @@ -62,56 +74,57 @@ public class PromotionController extends BaseApiController { } @PostMapping - public ApiResponse submitPromotion( - @RequestBody PromotionRequestDto request, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, - HttpServletRequest httpRequest) { + public ApiResponse submitPromotion(@RequestBody PromotionRequestDto request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { PromotionRequest promotion = promotionService.submitPromotion( - request.sourceSkillId(), request.sourceVersionId(), - request.targetNamespaceId(), userId, + request.sourceSkillId(), + request.sourceVersionId(), + request.targetNamespaceId(), + userId, userNsRoles != null ? userNsRoles : Map.of(), - rbacService.getUserRoleCodes(userId)); - recordAudit("PROMOTION_SUBMIT", userId, promotion.getId(), httpRequest, - "{\"sourceSkillId\":" + request.sourceSkillId() + ",\"sourceVersionId\":" + request.sourceVersionId() + "}"); + rbacService.getUserRoleCodes(userId) + ); + recordAudit( + "PROMOTION_SUBMIT", + userId, + promotion.getId(), + httpRequest, + "{\"sourceSkillId\":" + request.sourceSkillId() + ",\"sourceVersionId\":" + request.sourceVersionId() + "}" + ); return ok("response.success.created", toResponse(promotion)); } @PostMapping("/{id}/approve") - public ApiResponse approvePromotion( - @PathVariable Long id, - @RequestBody(required = false) PromotionActionRequest request, - @RequestAttribute("userId") String userId, - HttpServletRequest httpRequest) { + public ApiResponse approvePromotion(@PathVariable Long id, + @RequestBody(required = false) PromotionActionRequest request, + @RequestAttribute("userId") String userId, + HttpServletRequest httpRequest) { String comment = request != null ? request.comment() : null; - Set platformRoles = rbacService.getUserRoleCodes(userId); - PromotionRequest promotion = promotionService.approvePromotion(id, userId, comment, platformRoles); + PromotionRequest promotion = promotionService.approvePromotion(id, userId, comment, rbacService.getUserRoleCodes(userId)); recordAudit("PROMOTION_APPROVE", userId, promotion.getId(), httpRequest, detailWithComment(comment)); return ok("response.success.updated", toResponse(promotion)); } @PostMapping("/{id}/reject") - public ApiResponse rejectPromotion( - @PathVariable Long id, - @RequestBody(required = false) PromotionActionRequest request, - @RequestAttribute("userId") String userId, - HttpServletRequest httpRequest) { + public ApiResponse rejectPromotion(@PathVariable Long id, + @RequestBody(required = false) PromotionActionRequest request, + @RequestAttribute("userId") String userId, + HttpServletRequest httpRequest) { String comment = request != null ? request.comment() : null; - Set platformRoles = rbacService.getUserRoleCodes(userId); - PromotionRequest promotion = promotionService.rejectPromotion(id, userId, comment, platformRoles); + PromotionRequest promotion = promotionService.rejectPromotion(id, userId, comment, rbacService.getUserRoleCodes(userId)); recordAudit("PROMOTION_REJECT", userId, promotion.getId(), httpRequest, detailWithComment(comment)); return ok("response.success.updated", toResponse(promotion)); } @GetMapping - public ApiResponse> listPromotions( - @RequestParam(defaultValue = "PENDING") String status, - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId) { + public ApiResponse> listPromotions(@RequestParam(defaultValue = "PENDING") String status, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId) { Set platformRoles = rbacService.getUserRoleCodes(userId); - boolean hasAdminRole = platformRoles.contains("SKILL_ADMIN") || platformRoles.contains("SUPER_ADMIN"); - if (!hasAdminRole) { + if (!platformRoles.contains("SKILL_ADMIN") && !platformRoles.contains("SUPER_ADMIN")) { throw new DomainForbiddenException("promotion.no_permission"); } ReviewTaskStatus reviewStatus = ReviewTaskStatus.valueOf(status.toUpperCase()); @@ -120,13 +133,11 @@ public class PromotionController extends BaseApiController { } @GetMapping("/pending") - public ApiResponse> listPendingPromotions( - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId) { + public ApiResponse> listPendingPromotions(@RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId) { Set platformRoles = rbacService.getUserRoleCodes(userId); - boolean hasAdminRole = platformRoles.contains("SKILL_ADMIN") || platformRoles.contains("SUPER_ADMIN"); - if (!hasAdminRole) { + if (!platformRoles.contains("SKILL_ADMIN") && !platformRoles.contains("SUPER_ADMIN")) { throw new DomainForbiddenException("promotion.no_permission"); } Page requests = promotionRequestRepository.findByStatus( @@ -145,40 +156,39 @@ public class PromotionController extends BaseApiController { return ok("response.success.read", toResponse(promotion)); } - private PromotionResponseDto toResponse(PromotionRequest req) { - Skill sourceSkill = skillRepository.findById(req.getSourceSkillId()) - .orElseThrow(() -> new DomainNotFoundException("skill.not_found", req.getSourceSkillId())); - SkillVersion sourceVersion = skillVersionRepository.findById(req.getSourceVersionId()) - .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", req.getSourceVersionId())); - Namespace sourceNs = namespaceRepository.findById(sourceSkill.getNamespaceId()) + private PromotionResponseDto toResponse(PromotionRequest request) { + Skill sourceSkill = skillRepository.findById(request.getSourceSkillId()) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", request.getSourceSkillId())); + SkillVersion sourceVersion = skillVersionRepository.findById(request.getSourceVersionId()) + .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", request.getSourceVersionId())); + Namespace sourceNamespace = namespaceRepository.findById(sourceSkill.getNamespaceId()) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", sourceSkill.getNamespaceId())); - Namespace targetNs = namespaceRepository.findById(req.getTargetNamespaceId()) - .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", req.getTargetNamespaceId())); + Namespace targetNamespace = namespaceRepository.findById(request.getTargetNamespaceId()) + .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", request.getTargetNamespaceId())); - String submittedByName = userAccountRepository.findById(req.getSubmittedBy()) - .map(UserAccount::getDisplayName).orElse(null); - - String reviewedByName = req.getReviewedBy() != null - ? userAccountRepository.findById(req.getReviewedBy()) - .map(UserAccount::getDisplayName).orElse(null) + String submittedByName = userAccountRepository.findById(request.getSubmittedBy()) + .map(UserAccount::getDisplayName) + .orElse(null); + String reviewedByName = request.getReviewedBy() != null + ? userAccountRepository.findById(request.getReviewedBy()).map(UserAccount::getDisplayName).orElse(null) : null; return new PromotionResponseDto( - req.getId(), - req.getSourceSkillId(), - sourceNs.getSlug(), + request.getId(), + request.getSourceSkillId(), + sourceNamespace.getSlug(), sourceSkill.getSlug(), sourceVersion.getVersion(), - targetNs.getSlug(), - req.getTargetSkillId(), - req.getStatus().name(), - req.getSubmittedBy(), + targetNamespace.getSlug(), + request.getTargetSkillId(), + request.getStatus().name(), + request.getSubmittedBy(), submittedByName, - req.getReviewedBy(), + request.getReviewedBy(), reviewedByName, - req.getReviewComment(), - req.getSubmittedAt(), - req.getReviewedAt() + request.getReviewComment(), + request.getSubmittedAt(), + request.getReviewedAt() ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java index 6e766132..61a13e40 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java @@ -18,16 +18,28 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; -import com.iflytek.skillhub.dto.*; +import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.ReviewActionRequest; +import com.iflytek.skillhub.dto.ReviewTaskRequest; +import com.iflytek.skillhub.dto.ReviewTaskResponse; import jakarta.servlet.http.HttpServletRequest; +import org.slf4j.MDC; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; import java.util.Map; import java.util.Set; -import org.slf4j.MDC; @RestController @RequestMapping("/api/v1/reviews") @@ -63,11 +75,10 @@ public class ReviewController extends BaseApiController { } @PostMapping - public ApiResponse submitReview( - @RequestBody ReviewTaskRequest request, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, - HttpServletRequest httpRequest) { + public ApiResponse submitReview(@RequestBody ReviewTaskRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { ReviewTask task = reviewService.submitReview( request.skillVersionId(), userId, @@ -79,54 +90,59 @@ public class ReviewController extends BaseApiController { } @PostMapping("/{id}/approve") - public ApiResponse approveReview( - @PathVariable Long id, - @RequestBody(required = false) ReviewActionRequest request, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, - HttpServletRequest httpRequest) { + public ApiResponse approveReview(@PathVariable Long id, + @RequestBody(required = false) ReviewActionRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { String comment = request != null ? request.comment() : null; - Set platformRoles = rbacService.getUserRoleCodes(userId); - ReviewTask task = reviewService.approveReview(id, userId, comment, - userNsRoles != null ? userNsRoles : Map.of(), platformRoles); + ReviewTask task = reviewService.approveReview( + id, + userId, + comment, + userNsRoles != null ? userNsRoles : Map.of(), + rbacService.getUserRoleCodes(userId) + ); recordAudit("REVIEW_APPROVE", userId, task.getId(), httpRequest, detailWithComment(comment)); return ok("response.success.updated", toResponse(task)); } @PostMapping("/{id}/reject") - public ApiResponse rejectReview( - @PathVariable Long id, - @RequestBody(required = false) ReviewActionRequest request, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, - HttpServletRequest httpRequest) { + public ApiResponse rejectReview(@PathVariable Long id, + @RequestBody(required = false) ReviewActionRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { String comment = request != null ? request.comment() : null; - Set platformRoles = rbacService.getUserRoleCodes(userId); - ReviewTask task = reviewService.rejectReview(id, userId, comment, - userNsRoles != null ? userNsRoles : Map.of(), platformRoles); + ReviewTask task = reviewService.rejectReview( + id, + userId, + comment, + userNsRoles != null ? userNsRoles : Map.of(), + rbacService.getUserRoleCodes(userId) + ); recordAudit("REVIEW_REJECT", userId, task.getId(), httpRequest, detailWithComment(comment)); return ok("response.success.updated", toResponse(task)); } @PostMapping("/{id}/withdraw") - public ApiResponse withdrawReview( - @PathVariable Long id, - @RequestAttribute("userId") String userId, - HttpServletRequest httpRequest) { - ReviewTask task = reviewTaskRepository.findById(id).orElseThrow(); + public ApiResponse withdrawReview(@PathVariable Long id, + @RequestAttribute("userId") String userId, + HttpServletRequest httpRequest) { + ReviewTask task = reviewTaskRepository.findById(id) + .orElseThrow(() -> new DomainNotFoundException("review_task.not_found", id)); reviewService.withdrawReview(task.getSkillVersionId(), userId); recordAudit("REVIEW_WITHDRAW", userId, id, httpRequest, "{\"skillVersionId\":" + task.getSkillVersionId() + "}"); return ok("response.success.updated", null); } @GetMapping - public ApiResponse> listReviews( - @RequestParam String status, - @RequestParam(required = false) Long namespaceId, - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + public ApiResponse> listReviews(@RequestParam String status, + @RequestParam(required = false) Long namespaceId, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { ReviewTaskStatus reviewStatus = ReviewTaskStatus.valueOf(status.toUpperCase()); Map namespaceRoles = userNsRoles != null ? userNsRoles : Map.of(); @@ -135,7 +151,12 @@ public class ReviewController extends BaseApiController { Namespace namespace = namespaceRepository.findById(namespaceId) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", namespaceId)); ReviewTask probe = new ReviewTask(0L, namespaceId, userId); - if (!reviewService.canReviewNamespace(probe, userId, namespace.getType(), namespaceRoles, rbacService.getUserRoleCodes(userId))) { + if (!reviewService.canReviewNamespace( + probe, + userId, + namespace.getType(), + namespaceRoles, + rbacService.getUserRoleCodes(userId))) { throw new DomainForbiddenException("review.no_permission"); } tasks = reviewTaskRepository.findByNamespaceIdAndStatus(namespaceId, reviewStatus, PageRequest.of(page, size)); @@ -147,34 +168,40 @@ public class ReviewController extends BaseApiController { .filter(task -> canViewReview(task, userId, namespaceRoles)) .map(this::toResponse) .toList(); - Page responsePage = new PageImpl<>(visibleItems, tasks.getPageable(), visibleItems.size()); - return ok("response.success.read", PageResponse.from(responsePage)); + + return ok( + "response.success.read", + PageResponse.from(new PageImpl<>(visibleItems, tasks.getPageable(), visibleItems.size())) + ); } @GetMapping("/pending") - public ApiResponse> listPendingReviews( - @RequestParam Long namespaceId, - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + public ApiResponse> listPendingReviews(@RequestParam Long namespaceId, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { Namespace namespace = namespaceRepository.findById(namespaceId) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", namespaceId)); - ReviewTask probe = new ReviewTask(0L, namespaceId, "probe"); - if (!reviewService.canReviewNamespace(probe, userId, namespace.getType(), - userNsRoles != null ? userNsRoles : Map.of(), rbacService.getUserRoleCodes(userId))) { + ReviewTask probe = new ReviewTask(0L, namespaceId, userId); + if (!reviewService.canReviewNamespace( + probe, + userId, + namespace.getType(), + userNsRoles != null ? userNsRoles : Map.of(), + rbacService.getUserRoleCodes(userId))) { throw new DomainForbiddenException("review.no_permission"); } + Page tasks = reviewTaskRepository.findByNamespaceIdAndStatus( namespaceId, ReviewTaskStatus.PENDING, PageRequest.of(page, size)); return ok("response.success.read", PageResponse.from(tasks.map(this::toResponse))); } @GetMapping("/my-submissions") - public ApiResponse> listMySubmissions( - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "20") int size, - @RequestAttribute("userId") String userId) { + public ApiResponse> listMySubmissions(@RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId) { Page tasks = reviewTaskRepository.findBySubmittedByAndStatus( userId, ReviewTaskStatus.PENDING, PageRequest.of(page, size)); return ok("response.success.read", PageResponse.from(tasks.map(this::toResponse))); @@ -188,35 +215,38 @@ public class ReviewController extends BaseApiController { .orElseThrow(() -> new DomainNotFoundException("review_task.not_found", id)); Namespace namespace = namespaceRepository.findById(task.getNamespaceId()) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", task.getNamespaceId())); - if (!reviewService.canViewReview(task, userId, namespace.getType(), - userNsRoles != null ? userNsRoles : Map.of(), rbacService.getUserRoleCodes(userId))) { + if (!reviewService.canViewReview( + task, + userId, + namespace.getType(), + userNsRoles != null ? userNsRoles : Map.of(), + rbacService.getUserRoleCodes(userId))) { throw new DomainForbiddenException("review.no_permission"); } return ok("response.success.read", toResponse(task)); } private ReviewTaskResponse toResponse(ReviewTask task) { - SkillVersion sv = skillVersionRepository.findById(task.getSkillVersionId()) + SkillVersion skillVersion = skillVersionRepository.findById(task.getSkillVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", task.getSkillVersionId())); - Skill skill = skillRepository.findById(sv.getSkillId()) - .orElseThrow(() -> new DomainNotFoundException("skill.not_found", sv.getSkillId())); - Namespace ns = namespaceRepository.findById(skill.getNamespaceId()) + Skill skill = skillRepository.findById(skillVersion.getSkillId()) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); + Namespace namespace = namespaceRepository.findById(skill.getNamespaceId()) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", skill.getNamespaceId())); String submittedByName = userAccountRepository.findById(task.getSubmittedBy()) - .map(UserAccount::getDisplayName).orElse(null); - + .map(UserAccount::getDisplayName) + .orElse(null); String reviewedByName = task.getReviewedBy() != null - ? userAccountRepository.findById(task.getReviewedBy()) - .map(UserAccount::getDisplayName).orElse(null) + ? userAccountRepository.findById(task.getReviewedBy()).map(UserAccount::getDisplayName).orElse(null) : null; return new ReviewTaskResponse( task.getId(), task.getSkillVersionId(), - ns.getSlug(), + namespace.getSlug(), skill.getSlug(), - sv.getVersion(), + skillVersion.getVersion(), task.getStatus().name(), task.getSubmittedBy(), submittedByName, @@ -231,7 +261,13 @@ public class ReviewController extends BaseApiController { private boolean canViewReview(ReviewTask task, String userId, Map namespaceRoles) { Namespace namespace = namespaceRepository.findById(task.getNamespaceId()) .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", task.getNamespaceId())); - return reviewService.canViewReview(task, userId, namespace.getType(), namespaceRoles, rbacService.getUserRoleCodes(userId)); + return reviewService.canViewReview( + task, + userId, + namespace.getType(), + namespaceRoles, + rbacService.getUserRoleCodes(userId) + ); } private void recordAudit(String action, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java index 898b9beb..b7faaeae 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java @@ -2,7 +2,8 @@ package com.iflytek.skillhub.controller.portal; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.controller.BaseApiController; -import com.iflytek.skillhub.controller.support.ZipPackageExtractor; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; @@ -23,16 +24,16 @@ import java.util.List; public class SkillPublishController extends BaseApiController { private final SkillPublishService skillPublishService; - private final ZipPackageExtractor zipPackageExtractor; + private final SkillPackageArchiveExtractor skillPackageArchiveExtractor; private final SkillHubMetrics skillHubMetrics; public SkillPublishController(SkillPublishService skillPublishService, - ZipPackageExtractor zipPackageExtractor, + SkillPackageArchiveExtractor skillPackageArchiveExtractor, ApiResponseFactory responseFactory, SkillHubMetrics skillHubMetrics) { super(responseFactory); this.skillPublishService = skillPublishService; - this.zipPackageExtractor = zipPackageExtractor; + this.skillPackageArchiveExtractor = skillPackageArchiveExtractor; this.skillHubMetrics = skillHubMetrics; } @@ -46,7 +47,12 @@ public class SkillPublishController extends BaseApiController { SkillVisibility skillVisibility = SkillVisibility.valueOf(visibility.toUpperCase()); - List entries = zipPackageExtractor.extract(file); + List entries; + try { + entries = skillPackageArchiveExtractor.extract(file); + } catch (IllegalArgumentException e) { + throw new DomainBadRequestException("error.skill.publish.package.invalid", e.getMessage()); + } SkillPublishService.PublishResult publishResult = skillPublishService.publishFromEntries( namespace, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java new file mode 100644 index 00000000..b9193b8e --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java @@ -0,0 +1,92 @@ +package com.iflytek.skillhub.controller.support; + +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; +import org.springframework.stereotype.Component; +import org.springframework.web.multipart.MultipartFile; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; + +@Component +public class SkillPackageArchiveExtractor { + + public List extract(MultipartFile file) throws IOException { + if (file.getSize() > SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE) { + throw new IllegalArgumentException( + "Package too large: " + file.getSize() + " bytes (max: " + + SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE + ")" + ); + } + + List entries = new ArrayList<>(); + long totalSize = 0; + + try (ZipInputStream zis = new ZipInputStream(file.getInputStream())) { + ZipEntry zipEntry; + while ((zipEntry = zis.getNextEntry()) != null) { + if (zipEntry.isDirectory()) { + zis.closeEntry(); + continue; + } + + if (entries.size() >= SkillPackagePolicy.MAX_FILE_COUNT) { + throw new IllegalArgumentException( + "Too many files: more than " + SkillPackagePolicy.MAX_FILE_COUNT + ); + } + + String normalizedPath = SkillPackagePolicy.normalizeEntryPath(zipEntry.getName()); + byte[] content = readEntry(zis, normalizedPath); + totalSize += content.length; + if (totalSize > SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE) { + throw new IllegalArgumentException( + "Package too large: " + totalSize + " bytes (max: " + + SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE + ")" + ); + } + + entries.add(new PackageEntry( + normalizedPath, + content, + content.length, + determineContentType(normalizedPath) + )); + zis.closeEntry(); + } + } + + return entries; + } + + private byte[] readEntry(ZipInputStream zis, String path) throws IOException { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + long totalRead = 0; + int read; + while ((read = zis.read(buffer)) != -1) { + totalRead += read; + if (totalRead > SkillPackagePolicy.MAX_SINGLE_FILE_SIZE) { + throw new IllegalArgumentException( + "File too large: " + path + " (" + totalRead + " bytes, max: " + + SkillPackagePolicy.MAX_SINGLE_FILE_SIZE + ")" + ); + } + outputStream.write(buffer, 0, read); + } + return outputStream.toByteArray(); + } + + private String determineContentType(String filename) { + if (filename.endsWith(".py")) return "text/x-python"; + if (filename.endsWith(".json")) return "application/json"; + if (filename.endsWith(".yaml") || filename.endsWith(".yml")) return "application/x-yaml"; + if (filename.endsWith(".txt")) return "text/plain"; + if (filename.endsWith(".md")) return "text/markdown"; + return "application/octet-stream"; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AdminUserSummaryResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AdminUserSummaryResponse.java index 3412fb1a..3cdbbfd2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AdminUserSummaryResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AdminUserSummaryResponse.java @@ -4,11 +4,11 @@ import java.time.LocalDateTime; import java.util.List; public record AdminUserSummaryResponse( - String userId, + String id, String username, String email, - List platformRoles, String status, + List platformRoles, LocalDateTime createdAt ) { } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuditLogItemResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuditLogItemResponse.java index cd285c8a..699c1b19 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuditLogItemResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuditLogItemResponse.java @@ -3,12 +3,12 @@ package com.iflytek.skillhub.dto; import java.time.Instant; public record AuditLogItemResponse( - String id, - String userId, + Long id, String action, - String resourceType, - String resourceId, - Instant timestamp, - String ipAddress + String userId, + String username, + String details, + String ipAddress, + Instant timestamp ) { } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/AdminUserSearchRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/AdminUserSearchRepository.java new file mode 100644 index 00000000..c68bb7ed --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/AdminUserSearchRepository.java @@ -0,0 +1,74 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserStatus; +import jakarta.persistence.EntityManager; +import jakarta.persistence.TypedQuery; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Predicate; +import jakarta.persistence.criteria.Root; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.Pageable; +import org.springframework.stereotype.Repository; +import org.springframework.util.StringUtils; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +@Repository +public class AdminUserSearchRepository { + + private final EntityManager entityManager; + + public AdminUserSearchRepository(EntityManager entityManager) { + this.entityManager = entityManager; + } + + public Page search(String search, UserStatus status, Pageable pageable) { + CriteriaBuilder builder = entityManager.getCriteriaBuilder(); + + CriteriaQuery query = builder.createQuery(UserAccount.class); + Root root = query.from(UserAccount.class); + List predicates = buildPredicates(search, status, builder, root); + query.select(root) + .where(predicates.toArray(Predicate[]::new)) + .orderBy(builder.desc(root.get("createdAt"))); + + TypedQuery typedQuery = entityManager.createQuery(query); + typedQuery.setFirstResult((int) pageable.getOffset()); + typedQuery.setMaxResults(pageable.getPageSize()); + List users = typedQuery.getResultList(); + + CriteriaQuery countQuery = builder.createQuery(Long.class); + Root countRoot = countQuery.from(UserAccount.class); + List countPredicates = buildPredicates(search, status, builder, countRoot); + countQuery.select(builder.count(countRoot)) + .where(countPredicates.toArray(Predicate[]::new)); + long total = entityManager.createQuery(countQuery).getSingleResult(); + + return new PageImpl<>(users, pageable, total); + } + + private List buildPredicates( + String search, + UserStatus status, + CriteriaBuilder builder, + Root root) { + List predicates = new ArrayList<>(); + if (StringUtils.hasText(search)) { + String normalized = "%" + search.trim().toLowerCase(Locale.ROOT) + "%"; + predicates.add(builder.or( + builder.like(builder.lower(root.get("id")), normalized), + builder.like(builder.lower(root.get("displayName")), normalized), + builder.like(builder.lower(root.get("email")), normalized) + )); + } + if (status != null) { + predicates.add(builder.equal(root.get("status"), status)); + } + return predicates; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java new file mode 100644 index 00000000..d1673617 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminAuditLogAppService.java @@ -0,0 +1,97 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.dto.AuditLogItemResponse; +import com.iflytek.skillhub.dto.PageResponse; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.util.StringUtils; + +import java.sql.Timestamp; +import java.time.Instant; +import java.util.List; + +@Service +public class AdminAuditLogAppService { + + private final NamedParameterJdbcTemplate namedParameterJdbcTemplate; + + public AdminAuditLogAppService(NamedParameterJdbcTemplate namedParameterJdbcTemplate) { + this.namedParameterJdbcTemplate = namedParameterJdbcTemplate; + } + + @Transactional(readOnly = true) + public PageResponse listAuditLogs(int page, int size, String userId, String action) { + MapSqlParameterSource parameters = new MapSqlParameterSource() + .addValue("limit", size) + .addValue("offset", Math.max(page, 0) * size); + + String whereClause = buildWhereClause(parameters, userId, action); + Long total = namedParameterJdbcTemplate.queryForObject( + "SELECT COUNT(*) FROM audit_log al" + whereClause, + parameters, + Long.class + ); + + List items = namedParameterJdbcTemplate.query( + """ + SELECT al.id, + al.action, + al.actor_user_id, + ua.display_name, + al.detail_json, + al.target_type, + al.target_id, + al.client_ip, + al.created_at + FROM audit_log al + LEFT JOIN user_account ua ON ua.id = al.actor_user_id + """ + whereClause + """ + ORDER BY al.created_at DESC + LIMIT :limit OFFSET :offset + """, + parameters, + (rs, rowNum) -> new AuditLogItemResponse( + rs.getLong("id"), + rs.getString("action"), + rs.getString("actor_user_id"), + rs.getString("display_name"), + renderDetails( + rs.getString("detail_json"), + rs.getString("target_type"), + rs.getObject("target_id")), + rs.getString("client_ip"), + toInstant(rs.getTimestamp("created_at"))) + ); + + return new PageResponse<>(items, total == null ? 0 : total, page, size); + } + + private String buildWhereClause(MapSqlParameterSource parameters, String userId, String action) { + StringBuilder clause = new StringBuilder(" WHERE 1 = 1"); + if (StringUtils.hasText(userId)) { + clause.append(" AND al.actor_user_id = :userId"); + parameters.addValue("userId", userId.trim()); + } + if (StringUtils.hasText(action)) { + clause.append(" AND al.action = :action"); + parameters.addValue("action", action.trim()); + } + return clause.toString(); + } + + private String renderDetails(String detailJson, String targetType, Object targetId) { + if (StringUtils.hasText(detailJson)) { + return detailJson; + } + if (!StringUtils.hasText(targetType) && targetId == null) { + return null; + } + return targetType + ":" + targetId; + } + + private Instant toInstant(Timestamp timestamp) { + return timestamp == null ? null : timestamp.toInstant(); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java new file mode 100644 index 00000000..5d7a140d --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java @@ -0,0 +1,145 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.entity.Role; +import com.iflytek.skillhub.auth.entity.UserRoleBinding; +import com.iflytek.skillhub.auth.repository.RoleRepository; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import com.iflytek.skillhub.dto.AdminUserMutationResponse; +import com.iflytek.skillhub.dto.AdminUserSummaryResponse; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.repository.AdminUserSearchRepository; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.util.StringUtils; + +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +@Service +public class AdminUserAppService { + + private static final Set MANAGEABLE_STATUSES = Set.of(UserStatus.ACTIVE, UserStatus.DISABLED); + + private final AdminUserSearchRepository adminUserSearchRepository; + private final UserAccountRepository userAccountRepository; + private final UserRoleBindingRepository userRoleBindingRepository; + private final RoleRepository roleRepository; + + public AdminUserAppService( + AdminUserSearchRepository adminUserSearchRepository, + UserAccountRepository userAccountRepository, + UserRoleBindingRepository userRoleBindingRepository, + RoleRepository roleRepository) { + this.adminUserSearchRepository = adminUserSearchRepository; + this.userAccountRepository = userAccountRepository; + this.userRoleBindingRepository = userRoleBindingRepository; + this.roleRepository = roleRepository; + } + + @Transactional(readOnly = true) + public PageResponse listUsers(String search, String status, int page, int size) { + Pageable pageable = PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "createdAt")); + Page result = adminUserSearchRepository.search( + search, + StringUtils.hasText(status) ? parseStatus(status) : null, + pageable + ); + Map> rolesByUserId = loadRolesByUserId( + result.getContent().stream().map(UserAccount::getId).toList()); + + List items = result.getContent().stream() + .map(user -> new AdminUserSummaryResponse( + user.getId(), + user.getDisplayName(), + user.getEmail(), + user.getStatus().name(), + rolesByUserId.getOrDefault(user.getId(), List.of()), + user.getCreatedAt())) + .toList(); + + return new PageResponse<>(items, result.getTotalElements(), result.getNumber(), result.getSize()); + } + + @Transactional + public AdminUserMutationResponse updateUserRole(String userId, String roleCode, Set actorPlatformRoles) { + UserAccount user = loadUser(userId); + String normalizedRoleCode = normalizeRoleCode(roleCode); + + if ("SUPER_ADMIN".equals(normalizedRoleCode) + && (actorPlatformRoles == null || !actorPlatformRoles.contains("SUPER_ADMIN"))) { + throw new DomainForbiddenException("error.admin.user.role.superAdmin.assignDenied"); + } + + userRoleBindingRepository.deleteByUserId(user.getId()); + + if (!"USER".equals(normalizedRoleCode)) { + Role role = roleRepository.findByCode(normalizedRoleCode) + .orElseThrow(() -> new DomainBadRequestException("error.admin.user.role.invalid", roleCode)); + userRoleBindingRepository.save(new UserRoleBinding(user.getId(), role)); + } + + return new AdminUserMutationResponse(user.getId(), normalizedRoleCode, user.getStatus().name()); + } + + @Transactional + public AdminUserMutationResponse updateUserStatus(String userId, String status) { + UserAccount user = loadUser(userId); + UserStatus nextStatus = parseManageableStatus(status); + user.setStatus(nextStatus); + userAccountRepository.save(user); + return new AdminUserMutationResponse(user.getId(), null, nextStatus.name()); + } + + private UserStatus parseManageableStatus(String status) { + UserStatus parsedStatus = parseStatus(status); + if (!MANAGEABLE_STATUSES.contains(parsedStatus)) { + throw new DomainBadRequestException("error.admin.user.status.unsupported"); + } + return parsedStatus; + } + + private UserStatus parseStatus(String status) { + try { + return UserStatus.valueOf(status.trim().toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException ex) { + throw new DomainBadRequestException("error.admin.user.status.invalid", status); + } + } + + private String normalizeRoleCode(String roleCode) { + if (!StringUtils.hasText(roleCode)) { + throw new DomainBadRequestException("error.admin.user.role.invalid", roleCode); + } + return roleCode.trim().toUpperCase(Locale.ROOT); + } + + private Map> loadRolesByUserId(List userIds) { + if (userIds.isEmpty()) { + return Map.of(); + } + return userRoleBindingRepository.findByUserIdIn(userIds).stream() + .collect(Collectors.groupingBy( + UserRoleBinding::getUserId, + Collectors.mapping(binding -> binding.getRole().getCode(), + Collectors.collectingAndThen(Collectors.toList(), + roles -> roles.stream().sorted().toList())))); + } + + private UserAccount loadUser(String userId) { + return userAccountRepository.findById(userId) + .orElseThrow(() -> new DomainNotFoundException("error.admin.user.notFound", userId)); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserManagementService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserManagementService.java index 7be258b7..09c12260 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserManagementService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserManagementService.java @@ -110,8 +110,8 @@ public class AdminUserManagementService { user.getId(), user.getDisplayName(), user.getEmail(), - List.copyOf(roles), user.getStatus().name(), + List.copyOf(roles), user.getCreatedAt() ); } diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index f672ff01..8ab6ad5e 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -56,12 +56,6 @@ error.skill.publish.package.invalid=Package validation failed: {0} error.skill.publish.skillMd.notFound=SKILL.md not found error.skill.publish.precheck.failed=Pre-publish validation failed: {0} error.skill.notFound=Skill not found: {0} -skill.not_found=Skill not found: {0} -skill_version.not_found=Skill version not found: {0} -namespace.not_found=Namespace not found: {0} -promotion.not_found=Promotion request not found: {0} -review_task.not_found=Review task not found: {0} -review_task.not_found_for_version=Review task not found for skill version: {0} error.skill.access.denied=Access denied to skill: {0} error.skill.status.notActive=Skill is not active error.skill.version.exists=Version already exists: {0} @@ -82,33 +76,8 @@ error.deviceAuth.userCode.invalid=Invalid or expired user code error.deviceAuth.deviceCode.expired=Device code expired error.deviceAuth.deviceCode.invalid=Device code expired or invalid error.deviceAuth.deviceCode.used=Device code has already been used -error.auth.local.username.invalid=Username must be 3-64 characters and contain only letters, numbers, or underscores -error.auth.local.username.exists=Username already exists -error.auth.local.email.exists=Email already exists -error.auth.local.invalidCredentials=Invalid username or password -error.auth.local.accountDisabled=Account has been disabled -error.auth.local.accountPending=Account is pending approval -error.auth.local.accountMerged=Account has been merged into another account -error.auth.local.locked=Account is locked. Try again in {0} minute(s) -error.auth.local.notEnabled=Password login is not enabled for this account -error.auth.local.password.tooShort=Password must be at least 8 characters -error.auth.local.password.tooLong=Password must not exceed 128 characters -error.auth.local.password.tooWeak=Password must contain at least three character types -error.auth.merge.identifierRequired=Secondary account identifier is required -error.auth.merge.identifierInvalid=Secondary account identifier is invalid -error.auth.merge.primaryNotFound=Primary account not found -error.auth.merge.primaryNotActive=Primary account must be active -error.auth.merge.secondaryNotFound=Secondary account not found -error.auth.merge.secondaryNotActive=Secondary account must be active -error.auth.merge.sameAccount=Cannot merge the current account into itself -error.auth.merge.pendingExists=A pending merge request already exists for this secondary account -error.auth.merge.localCredentialConflict=Both accounts already have local credentials -error.auth.merge.requestNotFound=Merge request not found -error.auth.merge.requestNotPending=Merge request is not pending -error.auth.merge.requestNotVerified=Merge request is not verified -error.auth.merge.tokenExpired=Merge verification token has expired -error.auth.merge.invalidToken=Invalid merge verification token -error.admin.role.assign_super_admin_forbidden=Only SUPER_ADMIN can assign the SUPER_ADMIN role -error.role.notFound=Role not found: {0} -error.user.notFound=User not found: {0} -error.user.status.invalid=Invalid user status: {0} +error.admin.user.notFound=User not found: {0} +error.admin.user.role.invalid=Invalid role: {0} +error.admin.user.role.superAdmin.assignDenied=Only SUPER_ADMIN can assign SUPER_ADMIN role +error.admin.user.status.invalid=Invalid user status: {0} +error.admin.user.status.unsupported=Only ACTIVE or DISABLED status can be managed here diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index e2a62d34..9480c9df 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -51,12 +51,6 @@ error.skill.publish.package.invalid=技能包校验失败：{0} error.skill.publish.skillMd.notFound=未找到 SKILL.md error.skill.publish.precheck.failed=预发布校验失败：{0} error.skill.notFound=未找到技能：{0} -skill.not_found=未找到技能：{0} -skill_version.not_found=未找到技能版本：{0} -namespace.not_found=未找到命名空间：{0} -promotion.not_found=未找到推广申请：{0} -review_task.not_found=未找到审核任务：{0} -review_task.not_found_for_version=未找到该技能版本对应的审核任务：{0} error.skill.access.denied=没有权限访问技能：{0} error.skill.status.notActive=技能未处于 ACTIVE 状态 error.skill.version.exists=版本已存在：{0} @@ -77,33 +71,8 @@ error.deviceAuth.userCode.invalid=无效或已过期的用户验证码 error.deviceAuth.deviceCode.expired=设备验证码已过期 error.deviceAuth.deviceCode.invalid=设备验证码无效或已过期 error.deviceAuth.deviceCode.used=设备验证码已被使用 -error.auth.local.username.invalid=用户名长度必须为 3 到 64 个字符，且只能包含字母、数字或下划线 -error.auth.local.username.exists=用户名已存在 -error.auth.local.email.exists=邮箱已存在 -error.auth.local.invalidCredentials=用户名或密码错误 -error.auth.local.accountDisabled=账号已被禁用 -error.auth.local.accountPending=账号仍在审核中 -error.auth.local.accountMerged=账号已合并到其他账号 -error.auth.local.locked=账号已锁定，请 {0} 分钟后重试 -error.auth.local.notEnabled=当前账号未启用密码登录 -error.auth.local.password.tooShort=密码长度至少为 8 位 -error.auth.local.password.tooLong=密码长度不能超过 128 位 -error.auth.local.password.tooWeak=密码至少需要包含三种字符类型 -error.auth.merge.identifierRequired=待合并账号标识不能为空 -error.auth.merge.identifierInvalid=待合并账号标识格式不正确 -error.auth.merge.primaryNotFound=未找到主账号 -error.auth.merge.primaryNotActive=主账号必须处于激活状态 -error.auth.merge.secondaryNotFound=未找到待合并账号 -error.auth.merge.secondaryNotActive=待合并账号必须处于激活状态 -error.auth.merge.sameAccount=不能将当前账号合并到自己 -error.auth.merge.pendingExists=该待合并账号已有进行中的合并请求 -error.auth.merge.localCredentialConflict=两个账号都已启用本地密码登录，无法自动合并 -error.auth.merge.requestNotFound=未找到合并请求 -error.auth.merge.requestNotPending=该合并请求不处于待验证状态 -error.auth.merge.requestNotVerified=该合并请求尚未完成验证 -error.auth.merge.tokenExpired=合并验证 token 已过期 -error.auth.merge.invalidToken=合并验证 token 无效 -error.admin.role.assign_super_admin_forbidden=只有 SUPER_ADMIN 才能分配 SUPER_ADMIN 角色 -error.role.notFound=角色不存在：{0} -error.user.notFound=用户不存在：{0} -error.user.status.invalid=非法的用户状态：{0} +error.admin.user.notFound=鐢ㄦ埛涓嶅瓨鍦細{0} +error.admin.user.role.invalid=鏃犳晥鐨勮鑹诧細{0} +error.admin.user.role.superAdmin.assignDenied=鍙湁 SUPER_ADMIN 鍙互鍒嗛厤 SUPER_ADMIN 瑙掕壊 +error.admin.user.status.invalid=鏃犳晥鐨勭敤鎴风姸鎬侊細{0} +error.admin.user.status.unsupported=杩欓噷鍙厑璁告寜 ACTIVE 鎴?DISABLED 绠＄悊鐢ㄦ埛鐘舵€? diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java index 2d53d01a..df671106 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java @@ -3,40 +3,28 @@ package com.iflytek.skillhub.compat; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.auth.device.DeviceAuthService; -import com.iflytek.skillhub.domain.audit.AuditLogService; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.skill.SkillVersion; -import com.iflytek.skillhub.domain.skill.SkillVersionStatus; -import com.iflytek.skillhub.domain.skill.SkillVisibility; -import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.service.SkillSearchAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.mock.web.MockMultipartFile; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; -import java.io.ByteArrayOutputStream; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; +import java.math.BigDecimal; +import java.time.LocalDateTime; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; @SpringBootTest @@ -58,44 +46,43 @@ class ClawHubCompatControllerTest { @MockBean private SkillQueryService skillQueryService; - @MockBean - private SkillPublishService skillPublishService; - @MockBean - private AuditLogService auditLogService; @Test - void search_returns_200() throws Exception { - given(skillSearchAppService.search("test", null, "relevance", 0, 20, null, Map.of())) - .willReturn(new SkillSearchAppService.SearchResponse(List.of(), 0, 0, 20)); + void search_returns_mapped_results() throws Exception { + when(skillSearchAppService.search("test", null, "relevance", 0, 20, null, null)) + .thenReturn(new SkillSearchAppService.SearchResponse( + List.of(new SkillSummaryResponse( + 1L, + "my-skill", + "My Skill", + "test summary", + 10L, + 5, + BigDecimal.valueOf(4.5), + 2, + "1.2.0", + "global", + LocalDateTime.of(2026, 3, 13, 9, 0))), + 1, + 0, + 20 + )); mockMvc.perform(get("/api/compat/v1/search") .param("q", "test")) .andExpect(status().isOk()) .andExpect(jsonPath("$.items").isArray()) - .andExpect(jsonPath("$.items").isEmpty()); + .andExpect(jsonPath("$.items[0].canonicalSlug").value("my-skill")) + .andExpect(jsonPath("$.items[0].description").value("test summary")) + .andExpect(jsonPath("$.items[0].latestVersion").value("1.2.0")) + .andExpect(jsonPath("$.items[0].starCount").value(5)); } @Test void resolve_returns_correct_downloadUrl() throws Exception { - given(skillQueryService.resolveVersion( - eq("global"), - eq("my-skill"), - isNull(), - eq("latest"), - isNull(), - isNull(), - eq(Map.of()))) - .willReturn(new SkillQueryService.ResolvedVersionDTO( - 1L, - "global", - "my-skill", - "latest", - 1L, - "sha256:test", - true, - "/api/v1/skills/global/my-skill/download" - )); - + when(skillQueryService.resolveVersion("global", "my-skill", null, "latest", null, null, java.util.Map.of())) + .thenReturn(new SkillQueryService.ResolvedVersionDTO( + 1L, "global", "my-skill", "latest", 2L, "sha", true, "/api/v1/skills/global/my-skill/download")); mockMvc.perform(get("/api/compat/v1/resolve/my-skill")) .andExpect(status().isOk()) .andExpect(jsonPath("$.canonicalSlug").value("my-skill")) @@ -105,25 +92,9 @@ class ClawHubCompatControllerTest { @Test void resolve_with_namespace_returns_correct_downloadUrl() throws Exception { - given(skillQueryService.resolveVersion( - eq("team-ai"), - eq("my-skill"), - isNull(), - eq("latest"), - isNull(), - isNull(), - eq(Map.of()))) - .willReturn(new SkillQueryService.ResolvedVersionDTO( - 1L, - "team-ai", - "my-skill", - "latest", - 1L, - "sha256:test", - true, - "/api/v1/skills/team-ai/my-skill/download" - )); - + when(skillQueryService.resolveVersion("team-ai", "my-skill", null, "latest", null, null, java.util.Map.of())) + .thenReturn(new SkillQueryService.ResolvedVersionDTO( + 1L, "team-ai", "my-skill", "latest", 2L, "sha", true, "/api/v1/skills/team-ai/my-skill/download")); mockMvc.perform(get("/api/compat/v1/resolve/team-ai--my-skill")) .andExpect(status().isOk()) .andExpect(jsonPath("$.canonicalSlug").value("team-ai--my-skill")) @@ -133,25 +104,9 @@ class ClawHubCompatControllerTest { @Test void resolve_with_version_returns_specified_version() throws Exception { - given(skillQueryService.resolveVersion( - eq("global"), - eq("my-skill"), - eq("1.0.0"), - isNull(), - isNull(), - isNull(), - eq(Map.of()))) - .willReturn(new SkillQueryService.ResolvedVersionDTO( - 1L, - "global", - "my-skill", - "1.0.0", - 2L, - "sha256:test", - true, - "/api/v1/skills/global/my-skill/download" - )); - + when(skillQueryService.resolveVersion("global", "my-skill", "1.0.0", null, null, null, java.util.Map.of())) + .thenReturn(new SkillQueryService.ResolvedVersionDTO( + 1L, "global", "my-skill", "1.0.0", 2L, "sha", true, "/api/v1/skills/global/my-skill/download")); mockMvc.perform(get("/api/compat/v1/resolve/my-skill") .param("version", "1.0.0")) .andExpect(status().isOk()) @@ -184,62 +139,4 @@ class ClawHubCompatControllerTest { .andExpect(jsonPath("$.displayName").value("tester")) .andExpect(jsonPath("$.email").value("tester@example.com")); } - - @Test - void publish_passesSuperAdminRolesToDomainService() throws Exception { - SkillVersion version = new SkillVersion(1L, "1.0.0", "user-42"); - version.setStatus(SkillVersionStatus.PUBLISHED); - - given(skillPublishService.publishFromEntries( - eq("global"), - anyList(), - eq("user-42"), - eq(SkillVisibility.PUBLIC), - eq(Set.of("SUPER_ADMIN")))) - .willReturn(new SkillPublishService.PublishResult(1L, "demo-skill", version)); - - PlatformPrincipal principal = new PlatformPrincipal( - "user-42", - "tester", - "tester@example.com", - "", - "github", - Set.of("SUPER_ADMIN") - ); - var auth = new UsernamePasswordAuthenticationToken( - principal, - null, - List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) - ); - - MockMultipartFile file = new MockMultipartFile( - "file", - "skill.zip", - "application/zip", - createValidSkillZip() - ); - - mockMvc.perform(multipart("/api/compat/v1/publish") - .file(file) - .param("namespace", "global") - .with(authentication(auth)) - .with(csrf())) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.status").value("PUBLISHED")); - } - - private byte[] createValidSkillZip() throws Exception { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try (ZipOutputStream zos = new ZipOutputStream(baos)) { - zos.putNextEntry(new ZipEntry("SKILL.md")); - zos.write(""" - --- - name: test-skill - version: 1.0.0 - --- - """.getBytes()); - zos.closeEntry(); - } - return baos.toByteArray(); - } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/CliControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/CliControllerTest.java index 33811b4e..58d721ba 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/CliControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/CliControllerTest.java @@ -3,12 +3,6 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.skill.SkillVersion; -import com.iflytek.skillhub.domain.skill.SkillVersionStatus; -import com.iflytek.skillhub.domain.skill.SkillVisibility; -import com.iflytek.skillhub.domain.skill.service.SkillPublishService; -import com.iflytek.skillhub.domain.audit.AuditLogService; -import com.iflytek.skillhub.metrics.SkillHubMetrics; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -26,11 +20,8 @@ import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; -import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -49,12 +40,6 @@ class CliControllerTest { @MockBean private DeviceAuthService deviceAuthService; - @MockBean - private SkillPublishService skillPublishService; - @MockBean - private SkillHubMetrics skillHubMetrics; - @MockBean - private AuditLogService auditLogService; @Test void whoamiShouldReturnUnauthorizedForAnonymousRequest() throws Exception { @@ -148,50 +133,22 @@ class CliControllerTest { } @Test - void publishShouldPassPlatformRolesToDomainService() throws Exception { - SkillVersion version = new SkillVersion(12L, "1.0.0", "user-7"); - version.setStatus(SkillVersionStatus.PUBLISHED); - version.setFileCount(1); - version.setTotalSize(128L); - - given(skillPublishService.publishFromEntries( - eq("global"), - anyList(), - eq("user-7"), - eq(SkillVisibility.PUBLIC), - eq(Set.of("SUPER_ADMIN")))) - .willReturn(new SkillPublishService.PublishResult(12L, "demo-skill", version)); - - PlatformPrincipal principal = new PlatformPrincipal( - "user-7", - "cli-user", - "cli@example.com", - "", - "api_token", - Set.of("SUPER_ADMIN") - ); - - var auth = new UsernamePasswordAuthenticationToken( - principal, - null, - List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) - ); - + void checkShouldReturnInvalidForPathTraversalEntry() throws Exception { + byte[] zipBytes = createZipWithUnsafePath(); MockMultipartFile file = new MockMultipartFile( "file", "skill.zip", "application/zip", - createValidSkillZip() + zipBytes ); - mockMvc.perform(multipart("/api/v1/cli/publish") - .file(file) - .param("namespace", "global") - .param("visibility", "PUBLIC") - .with(authentication(auth)) - .with(csrf())) + mockMvc.perform(multipart("/api/v1/cli/check").file(file)) .andExpect(status().isOk()) - .andExpect(jsonPath("$.data.status").value("PUBLISHED")); + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.valid").value(false)) + .andExpect(jsonPath("$.data.errors[0]").value(org.hamcrest.Matchers.containsString("escapes package root"))) + .andExpect(jsonPath("$.data.fileCount").value(0)) + .andExpect(jsonPath("$.data.totalSize").value(0)); } private byte[] createValidSkillZip() throws Exception { @@ -253,4 +210,15 @@ class CliControllerTest { } return baos.toByteArray(); } + + private byte[] createZipWithUnsafePath() throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + ZipEntry unsafeEntry = new ZipEntry("../secrets.txt"); + zos.putNextEntry(unsafeEntry); + zos.write("hidden".getBytes()); + zos.closeEntry(); + } + return baos.toByteArray(); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DeviceAuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DeviceAuthControllerTest.java index 75631a60..f1ede80c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DeviceAuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DeviceAuthControllerTest.java @@ -70,4 +70,20 @@ class DeviceAuthControllerTest { .andExpect(jsonPath("$.data.accessToken").isEmpty()) .andExpect(jsonPath("$.data.tokenType").isEmpty()); } + + @Test + void pollToken_returns_access_token_when_authorized() throws Exception { + DeviceTokenResponse response = DeviceTokenResponse.success("sk_device_flow_token"); + + given(deviceAuthService.pollToken("device_abc123")).willReturn(response); + + mockMvc.perform(post("/api/v1/cli/auth/device/token") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"deviceCode\": \"device_abc123\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.accessToken").value("sk_device_flow_token")) + .andExpect(jsonPath("$.data.tokenType").value("Bearer")) + .andExpect(jsonPath("$.data.error").isEmpty()); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java new file mode 100644 index 00000000..b4cad51a --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/PromotionPortalControllerTest.java @@ -0,0 +1,206 @@ +package com.iflytek.skillhub.controller; + +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.PromotionRequest; +import com.iflytek.skillhub.domain.review.PromotionRequestRepository; +import com.iflytek.skillhub.domain.review.PromotionService; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.RequestPostProcessor; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class PromotionPortalControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private PromotionService promotionService; + + @MockBean + private PromotionRequestRepository promotionRequestRepository; + + @MockBean + private SkillRepository skillRepository; + + @MockBean + private SkillVersionRepository skillVersionRepository; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private com.iflytek.skillhub.domain.namespace.NamespaceRepository namespaceRepository; + + @MockBean + private UserAccountRepository userAccountRepository; + + @MockBean + private RbacService rbacService; + + @MockBean + private ReviewPermissionChecker permissionChecker; + + @MockBean + private AuditLogService auditLogService; + + @Test + void submitPromotion_passesNamespaceRolesToService() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(5L, "user-1", NamespaceRole.ADMIN))); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(promotionService.submitPromotion(10L, 20L, 30L, "user-1", Map.of(5L, NamespaceRole.ADMIN), Set.of())) + .willReturn(request); + stubPromotionResponse(request); + + mockMvc.perform(post("/api/v1/promotions") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"sourceSkillId\":10,\"sourceVersionId\":20,\"targetNamespaceId\":30}") + .with(csrf()) + .with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(1L)); + } + + @Test + void listPendingPromotions_forbidsRegularUser() throws Exception { + stubNamespaceRoles("user-1", List.of()); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canListPendingPromotions(Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/promotions/pending").with(auth("user-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(promotionRequestRepository, never()).findByStatus(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + } + + @Test + void getPromotionDetail_allowsSubmitter() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-1", List.of()); + given(promotionRequestRepository.findById(1L)).willReturn(Optional.of(request)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(promotionService.canViewPromotion(request, "user-1", Set.of())).willReturn(true); + stubPromotionResponse(request); + + mockMvc.perform(get("/api/v1/promotions/1").with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.submittedBy").value("user-1")); + } + + @Test + void getPromotionDetail_forbidsUnrelatedUser() throws Exception { + PromotionRequest request = createPromotionRequest(1L, "user-1"); + stubNamespaceRoles("user-9", List.of()); + given(promotionRequestRepository.findById(1L)).willReturn(Optional.of(request)); + given(rbacService.getUserRoleCodes("user-9")).willReturn(Set.of()); + given(promotionService.canViewPromotion(request, "user-9", Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/promotions/1").with(auth("user-9"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + private void stubPromotionResponse(PromotionRequest request) { + Skill skill = new Skill(5L, "skill-a", request.getSubmittedBy(), SkillVisibility.PUBLIC); + setField(skill, "id", request.getSourceSkillId()); + SkillVersion version = new SkillVersion(request.getSourceSkillId(), "1.0.0", request.getSubmittedBy()); + setField(version, "id", request.getSourceVersionId()); + Namespace sourceNamespace = new Namespace("team-a", "Team A", "owner-1"); + setField(sourceNamespace, "id", 5L); + Namespace targetNamespace = new Namespace("global", "Global", "owner-2"); + setField(targetNamespace, "id", request.getTargetNamespaceId()); + UserAccount submitter = new UserAccount(request.getSubmittedBy(), "Submitter", "submitter@example.com", ""); + + given(skillRepository.findById(request.getSourceSkillId())).willReturn(Optional.of(skill)); + given(skillVersionRepository.findById(request.getSourceVersionId())).willReturn(Optional.of(version)); + given(namespaceRepository.findById(5L)).willReturn(Optional.of(sourceNamespace)); + given(namespaceRepository.findById(request.getTargetNamespaceId())).willReturn(Optional.of(targetNamespace)); + given(userAccountRepository.findById(request.getSubmittedBy())).willReturn(Optional.of(submitter)); + } + + private void stubNamespaceRoles(String userId, List members) { + given(namespaceMemberRepository.findByUserId(userId)).willReturn(members); + } + + private RequestPostProcessor auth(String userId) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of() + ); + UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_USER")) + ); + return authentication(authenticationToken); + } + + private PromotionRequest createPromotionRequest(Long id, String submittedBy) { + PromotionRequest request = new PromotionRequest(10L, 20L, 30L, submittedBy); + setField(request, "id", id); + setField(request, "status", ReviewTaskStatus.PENDING); + return request; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java new file mode 100644 index 00000000..77581ab4 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java @@ -0,0 +1,218 @@ +package com.iflytek.skillhub.controller; + +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewPermissionChecker; +import com.iflytek.skillhub.domain.review.ReviewService; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.RequestPostProcessor; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class ReviewPortalControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private ReviewService reviewService; + + @MockBean + private ReviewTaskRepository reviewTaskRepository; + + @MockBean + private SkillRepository skillRepository; + + @MockBean + private SkillVersionRepository skillVersionRepository; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private com.iflytek.skillhub.domain.namespace.NamespaceRepository namespaceRepository; + + @MockBean + private UserAccountRepository userAccountRepository; + + @MockBean + private RbacService rbacService; + + @MockBean + private ReviewPermissionChecker permissionChecker; + + @MockBean + private AuditLogService auditLogService; + + @Test + void submitReview_passesNamespaceRolesToService() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(20L, "user-1", NamespaceRole.MEMBER))); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(reviewService.submitReview(100L, "user-1", Map.of(20L, NamespaceRole.MEMBER), Set.of())).willReturn(task); + stubReviewResponse(task); + + mockMvc.perform(post("/api/v1/reviews") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"skillVersionId\":100}") + .with(csrf()) + .with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(1L)); + } + + @Test + void listPendingReviews_forbidsNamespaceMember() throws Exception { + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-1", List.of(new NamespaceMember(20L, "user-1", NamespaceRole.MEMBER))); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(permissionChecker.canManageNamespaceReviews( + 20L, + namespace.getType(), + Map.of(20L, NamespaceRole.MEMBER), + Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/pending") + .param("namespaceId", "20") + .with(auth("user-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(reviewTaskRepository, never()).findByNamespaceIdAndStatus(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + } + + @Test + void getReviewDetail_allowsSubmitter() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-1", List.of()); + given(reviewTaskRepository.findById(1L)).willReturn(Optional.of(task)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-1")).willReturn(Set.of()); + given(reviewService.canViewReview(task, "user-1", namespace.getType(), Map.of(), Set.of())).willReturn(true); + stubReviewResponse(task); + + mockMvc.perform(get("/api/v1/reviews/1").with(auth("user-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.submittedBy").value("user-1")); + } + + @Test + void getReviewDetail_forbidsUnrelatedUser() throws Exception { + ReviewTask task = createReviewTask(1L, 20L, "user-1"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("user-9", List.of()); + given(reviewTaskRepository.findById(1L)).willReturn(Optional.of(task)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(rbacService.getUserRoleCodes("user-9")).willReturn(Set.of()); + given(reviewService.canViewReview(task, "user-9", namespace.getType(), Map.of(), Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/1").with(auth("user-9"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + private void stubReviewResponse(ReviewTask task) { + SkillVersion version = new SkillVersion(30L, "1.0.0", task.getSubmittedBy()); + setField(version, "id", task.getSkillVersionId()); + Skill skill = new Skill(task.getNamespaceId(), "skill-a", task.getSubmittedBy(), SkillVisibility.PUBLIC); + setField(skill, "id", 30L); + UserAccount submitter = new UserAccount(task.getSubmittedBy(), "Submitter", "submitter@example.com", ""); + + given(skillVersionRepository.findById(task.getSkillVersionId())).willReturn(Optional.of(version)); + given(skillRepository.findById(30L)).willReturn(Optional.of(skill)); + given(namespaceRepository.findById(task.getNamespaceId())).willReturn(Optional.of(createNamespace(task.getNamespaceId(), "team-a"))); + given(userAccountRepository.findById(task.getSubmittedBy())).willReturn(Optional.of(submitter)); + } + + private void stubNamespaceRoles(String userId, List members) { + given(namespaceMemberRepository.findByUserId(userId)).willReturn(members); + } + + private RequestPostProcessor auth(String userId) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of() + ); + UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_USER")) + ); + return authentication(authenticationToken); + } + + private ReviewTask createReviewTask(Long id, Long namespaceId, String submittedBy) { + ReviewTask task = new ReviewTask(100L, namespaceId, submittedBy); + setField(task, "id", id); + setField(task, "status", ReviewTaskStatus.PENDING); + return task; + } + + private Namespace createNamespace(Long id, String slug) { + Namespace namespace = new Namespace(slug, "Team", "owner-1"); + setField(namespace, "id", id); + return namespace; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillRatingControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillRatingControllerTest.java index 7e6e1cdf..7043ac04 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillRatingControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillRatingControllerTest.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; -import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.social.SkillRatingService; import org.junit.jupiter.api.Test; @@ -110,31 +109,8 @@ class SkillRatingControllerTest { } @Test - void get_user_rating_missing_skill_returns_404_envelope() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal( - "user-42", - "tester", - "tester@example.com", - "https://example.com/avatar.png", - "github", - Set.of("SUPER_ADMIN") - ); - var auth = new UsernamePasswordAuthenticationToken( - principal, - null, - List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) - ); - - when(skillRatingService.getUserRating(eq(999L), eq("user-42"))) - .thenThrow(new DomainNotFoundException("skill.not_found", 999L)); - - mockMvc.perform(get("/api/v1/skills/999/rating") - .with(authentication(auth)) - .with(csrf())) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.code").value(404)) - .andExpect(jsonPath("$.msg").value("Skill not found: 999")) - .andExpect(jsonPath("$.timestamp").isNotEmpty()) - .andExpect(jsonPath("$.requestId").isNotEmpty()); + void get_user_rating_unauthenticated_returns_401() throws Exception { + mockMvc.perform(get("/api/v1/skills/10/rating")) + .andExpect(status().isUnauthorized()); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillStarControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillStarControllerTest.java index 7397ec1c..2912fa7f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillStarControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillStarControllerTest.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; -import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.social.SkillStarService; import org.junit.jupiter.api.Test; @@ -129,31 +128,8 @@ class SkillStarControllerTest { } @Test - void check_starred_missing_skill_returns_404_envelope() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal( - "user-42", - "tester", - "tester@example.com", - "https://example.com/avatar.png", - "github", - Set.of("SUPER_ADMIN") - ); - var auth = new UsernamePasswordAuthenticationToken( - principal, - null, - List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) - ); - - when(skillStarService.isStarred(eq(999L), eq("user-42"))) - .thenThrow(new DomainNotFoundException("skill.not_found", 999L)); - - mockMvc.perform(get("/api/v1/skills/999/star") - .with(authentication(auth)) - .with(csrf())) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.code").value(404)) - .andExpect(jsonPath("$.msg").value("Skill not found: 999")) - .andExpect(jsonPath("$.timestamp").isNotEmpty()) - .andExpect(jsonPath("$.requestId").isNotEmpty()); + void check_starred_unauthenticated_returns_401() throws Exception { + mockMvc.perform(get("/api/v1/skills/10/star")) + .andExpect(status().isUnauthorized()); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillTagControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillTagControllerTest.java new file mode 100644 index 00000000..8c664b4c --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillTagControllerTest.java @@ -0,0 +1,50 @@ +package com.iflytek.skillhub.controller; + +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.skill.SkillTag; +import com.iflytek.skillhub.domain.skill.service.SkillTagService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +import java.util.List; +import java.util.Map; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class SkillTagControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private SkillTagService skillTagService; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @Test + void list_tags_is_public() throws Exception { + when(skillTagService.listTags(eq("team"), eq("demo"), isNull(), eq(Map.of()))) + .thenReturn(List.of(new SkillTag(1L, "latest", 2L, "user-1"))); + + mockMvc.perform(get("/api/v1/skills/team/demo/tags")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data[0].tagName").value("latest")) + .andExpect(jsonPath("$.timestamp").isNotEmpty()) + .andExpect(jsonPath("$.requestId").isNotEmpty()); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/TokenControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/TokenControllerTest.java new file mode 100644 index 00000000..52b98554 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/TokenControllerTest.java @@ -0,0 +1,64 @@ +package com.iflytek.skillhub.controller; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +import java.util.List; +import java.util.Set; + +import static org.mockito.Mockito.verify; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class TokenControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private ApiTokenService apiTokenService; + + @Test + void revoke_returns204NoContent() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", "tester", "tester@example.com", "", "github", Set.of("USER") + ); + var auth = new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER")) + ); + + mockMvc.perform(delete("/api/v1/tokens/7") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isNoContent()) + .andExpect(content().string("")); + + verify(apiTokenService).revokeToken(7L, "user-42"); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AuditLogControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AuditLogControllerTest.java index 286201bc..d255616d 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AuditLogControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AuditLogControllerTest.java @@ -3,27 +3,26 @@ package com.iflytek.skillhub.controller.admin; import com.iflytek.skillhub.TestRedisConfig; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.device.DeviceAuthService; -import com.iflytek.skillhub.domain.audit.AuditLog; -import com.iflytek.skillhub.domain.audit.AuditLogQueryService; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.dto.AuditLogItemResponse; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.service.AdminAuditLogAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; -import org.springframework.data.domain.PageImpl; -import org.springframework.data.domain.PageRequest; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; -import java.time.Instant; import java.util.List; import java.util.Set; +import java.time.Instant; -import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -45,7 +44,7 @@ class AuditLogControllerTest { private DeviceAuthService deviceAuthService; @MockBean - private AuditLogQueryService auditLogQueryService; + private AdminAuditLogAppService adminAuditLogAppService; @Test void listAuditLogs_unauthenticated_returns401() throws Exception { @@ -55,15 +54,6 @@ class AuditLogControllerTest { @Test void listAuditLogs_withAuditorRole_returns200() throws Exception { - AuditLog log1 = new AuditLog("user-1", "CREATE_SKILL", "SKILL", 123L, null, "192.168.1.1", "", null); - AuditLog log2 = new AuditLog("user-2", "UPDATE_NAMESPACE", "NAMESPACE", 456L, null, "192.168.1.2", "", null); - org.springframework.test.util.ReflectionTestUtils.setField(log1, "id", 1L); - org.springframework.test.util.ReflectionTestUtils.setField(log2, "id", 2L); - org.springframework.test.util.ReflectionTestUtils.setField(log1, "createdAt", Instant.now()); - org.springframework.test.util.ReflectionTestUtils.setField(log2, "createdAt", Instant.now()); - given(auditLogQueryService.list(0, 20, null, null)) - .willReturn(new PageImpl<>(List.of(log1, log2), PageRequest.of(0, 20), 2)); - PlatformPrincipal principal = new PlatformPrincipal( "user-50", "auditor", "auditor@example.com", "", "github", Set.of("AUDITOR") ); @@ -71,17 +61,31 @@ class AuditLogControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_AUDITOR")) ); + when(adminAuditLogAppService.listAuditLogs(0, 20, null, null)) + .thenReturn(new PageResponse<>( + List.of(new AuditLogItemResponse( + 1L, + "USER_STATUS_CHANGE", + "user-1", + "alice", + "{\"status\":\"DISABLED\"}", + "127.0.0.1", + Instant.parse("2026-03-13T01:00:00Z"))), + 1, + 0, + 20)); + mockMvc.perform(get("/api/v1/admin/audit-logs").with(authentication(auth))) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.items").isArray()) - .andExpect(jsonPath("$.data.total").value(2)); + .andExpect(jsonPath("$.data.total").value(1)) + .andExpect(jsonPath("$.data.items[0].username").value("alice")) + .andExpect(jsonPath("$.data.items[0].details").value("{\"status\":\"DISABLED\"}")); } @Test void listAuditLogs_withSuperAdminRole_returns200() throws Exception { - given(auditLogQueryService.list(0, 20, null, null)) - .willReturn(new PageImpl<>(List.of(), PageRequest.of(0, 20), 0)); PlatformPrincipal principal = new PlatformPrincipal( "user-99", "superadmin", "super@example.com", "", "github", Set.of("SUPER_ADMIN") ); @@ -89,6 +93,9 @@ class AuditLogControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) ); + when(adminAuditLogAppService.listAuditLogs(0, 20, null, null)) + .thenReturn(new PageResponse<>(List.of(), 0, 0, 20)); + mockMvc.perform(get("/api/v1/admin/audit-logs").with(authentication(auth))) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.items").isArray()); @@ -96,8 +103,6 @@ class AuditLogControllerTest { @Test void listAuditLogs_withFilters_returns200() throws Exception { - given(auditLogQueryService.list(0, 20, "user-1", "CREATE_SKILL")) - .willReturn(new PageImpl<>(List.of(), PageRequest.of(0, 20), 0)); PlatformPrincipal principal = new PlatformPrincipal( "user-50", "auditor", "auditor@example.com", "", "github", Set.of("AUDITOR") ); @@ -105,6 +110,9 @@ class AuditLogControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_AUDITOR")) ); + when(adminAuditLogAppService.listAuditLogs(0, 20, "user-1", "CREATE_SKILL")) + .thenReturn(new PageResponse<>(List.of(), 0, 0, 20)); + mockMvc.perform(get("/api/v1/admin/audit-logs") .param("userId", "user-1") .param("action", "CREATE_SKILL") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java index 3ec1746d..8379a594 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java @@ -4,9 +4,10 @@ import com.iflytek.skillhub.TestRedisConfig; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.dto.AdminUserMutationResponse; import com.iflytek.skillhub.dto.AdminUserSummaryResponse; import com.iflytek.skillhub.dto.PageResponse; -import com.iflytek.skillhub.service.AdminUserManagementService; +import com.iflytek.skillhub.service.AdminUserAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -22,7 +23,6 @@ import java.util.List; import java.util.Set; import java.time.LocalDateTime; -import static org.mockito.BDDMockito.given; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -30,6 +30,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.http.MediaType.APPLICATION_JSON; +import static org.mockito.Mockito.when; @SpringBootTest @AutoConfigureMockMvc @@ -47,7 +48,7 @@ class UserManagementControllerTest { private DeviceAuthService deviceAuthService; @MockBean - private AdminUserManagementService adminUserManagementService; + private AdminUserAppService adminUserAppService; @Test void listUsers_unauthenticated_returns401() throws Exception { @@ -57,17 +58,6 @@ class UserManagementControllerTest { @Test void listUsers_withUserAdminRole_returns200() throws Exception { - given(adminUserManagementService.listUsers(null, null, 0, 20)) - .willReturn(new PageResponse<>( - List.of( - new AdminUserSummaryResponse("user-1", "alice", "alice@example.com", List.of("USER"), "ACTIVE", LocalDateTime.parse("2026-03-12T12:00:00")), - new AdminUserSummaryResponse("user-2", "bob", "bob@example.com", List.of("USER_ADMIN"), "PENDING", LocalDateTime.parse("2026-03-12T13:00:00")) - ), - 2, - 0, - 20 - )); - PlatformPrincipal principal = new PlatformPrincipal( "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") ); @@ -75,23 +65,31 @@ class UserManagementControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) ); + when(adminUserAppService.listUsers(null, null, 0, 20)) + .thenReturn(new PageResponse<>( + List.of(new AdminUserSummaryResponse( + "user-1", + "alice", + "alice@example.com", + "ACTIVE", + List.of("AUDITOR"), + LocalDateTime.of(2026, 3, 13, 9, 0))), + 1, + 0, + 20)); + mockMvc.perform(get("/api/v1/admin/users").with(authentication(auth))) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.items").isArray()) - .andExpect(jsonPath("$.data.total").value(2)); + .andExpect(jsonPath("$.data.total").value(1)) + .andExpect(jsonPath("$.data.items[0].id").value("user-1")) + .andExpect(jsonPath("$.data.items[0].email").value("alice@example.com")) + .andExpect(jsonPath("$.data.items[0].platformRoles[0]").value("AUDITOR")); } @Test void listUsers_withSuperAdminRole_returns200() throws Exception { - given(adminUserManagementService.listUsers(null, null, 0, 20)) - .willReturn(new PageResponse<>( - List.of(new AdminUserSummaryResponse("user-99", "superadmin", "super@example.com", List.of("SUPER_ADMIN"), "ACTIVE", LocalDateTime.parse("2026-03-12T14:00:00"))), - 1, - 0, - 20 - )); - PlatformPrincipal principal = new PlatformPrincipal( "user-99", "superadmin", "super@example.com", "", "github", Set.of("SUPER_ADMIN") ); @@ -99,6 +97,9 @@ class UserManagementControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) ); + when(adminUserAppService.listUsers(null, null, 0, 20)) + .thenReturn(new PageResponse<>(List.of(), 0, 0, 20)); + mockMvc.perform(get("/api/v1/admin/users").with(authentication(auth))) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.items").isArray()); @@ -106,9 +107,6 @@ class UserManagementControllerTest { @Test void updateUserRole_withUserAdminRole_returns200() throws Exception { - given(adminUserManagementService.updateUserRole(org.mockito.ArgumentMatchers.eq("user-123"), org.mockito.ArgumentMatchers.eq("USER_ADMIN"), org.mockito.ArgumentMatchers.any())) - .willReturn(new AdminUserSummaryResponse("user-123", "target", "target@example.com", List.of("USER_ADMIN"), "ACTIVE", LocalDateTime.parse("2026-03-12T15:00:00"))); - PlatformPrincipal principal = new PlatformPrincipal( "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") ); @@ -116,7 +114,10 @@ class UserManagementControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) ); - String requestBody = "{\"role\":\"USER_ADMIN\"}"; + String requestBody = "{\"role\":\"MODERATOR\"}"; + + when(adminUserAppService.updateUserRole("user-123", "MODERATOR", Set.of("USER_ADMIN"))) + .thenReturn(new AdminUserMutationResponse("user-123", "MODERATOR", "ACTIVE")); mockMvc.perform(put("/api/v1/admin/users/user-123/role") .with(authentication(auth)) @@ -126,14 +127,12 @@ class UserManagementControllerTest { .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.userId").value("user-123")) - .andExpect(jsonPath("$.data.role").value("USER_ADMIN")); + .andExpect(jsonPath("$.data.role").value("MODERATOR")) + .andExpect(jsonPath("$.data.status").value("ACTIVE")); } @Test void updateUserStatus_withUserAdminRole_returns200() throws Exception { - given(adminUserManagementService.updateUserStatus("user-123", "DISABLED")) - .willReturn(new AdminUserSummaryResponse("user-123", "target", "target@example.com", List.of("USER"), "DISABLED", LocalDateTime.parse("2026-03-12T16:00:00"))); - PlatformPrincipal principal = new PlatformPrincipal( "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") ); @@ -143,6 +142,9 @@ class UserManagementControllerTest { String requestBody = "{\"status\":\"DISABLED\"}"; + when(adminUserAppService.updateUserStatus("user-123", "DISABLED")) + .thenReturn(new AdminUserMutationResponse("user-123", null, "DISABLED")); + mockMvc.perform(put("/api/v1/admin/users/user-123/status") .with(authentication(auth)) .with(csrf()) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java new file mode 100644 index 00000000..d5464a60 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java @@ -0,0 +1,61 @@ +package com.iflytek.skillhub.controller.support; + +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockMultipartFile; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SkillPackageArchiveExtractorTest { + + private final SkillPackageArchiveExtractor extractor = new SkillPackageArchiveExtractor(); + + @Test + void shouldRejectPathTraversalEntry() throws Exception { + MockMultipartFile file = new MockMultipartFile( + "file", + "skill.zip", + "application/zip", + createZip("../secrets.txt", "hidden") + ); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> extractor.extract(file)); + + assertTrue(error.getMessage().contains("escapes package root")); + } + + @Test + void shouldRejectOversizedZipEntry() throws Exception { + byte[] content = new byte[1024 * 1024 + 1]; + MockMultipartFile file = new MockMultipartFile( + "file", + "skill.zip", + "application/zip", + createZip("large.txt", content) + ); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> extractor.extract(file)); + + assertTrue(error.getMessage().contains("File too large: large.txt")); + } + + private byte[] createZip(String entryName, String content) throws Exception { + return createZip(entryName, content.getBytes(StandardCharsets.UTF_8)); + } + + private byte[] createZip(String entryName, byte[] content) throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(baos)) { + ZipEntry entry = new ZipEntry(entryName); + zos.putNextEntry(entry); + zos.write(content); + zos.closeEntry(); + } + return baos.toByteArray(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java new file mode 100644 index 00000000..5f2d64cc --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminAuditLogAppServiceTest.java @@ -0,0 +1,44 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.dto.AuditLogItemResponse; +import com.iflytek.skillhub.dto.PageResponse; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; + +import java.time.Instant; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +class AdminAuditLogAppServiceTest { + + private final NamedParameterJdbcTemplate jdbcTemplate = mock(NamedParameterJdbcTemplate.class); + private final AdminAuditLogAppService service = new AdminAuditLogAppService(jdbcTemplate); + + @Test + void listAuditLogs_returnsJdbcBackedPage() { + when(jdbcTemplate.queryForObject(contains("COUNT(*)"), any(MapSqlParameterSource.class), eq(Long.class))) + .thenReturn(1L); + when(jdbcTemplate.query(contains("FROM audit_log"), any(MapSqlParameterSource.class), any(RowMapper.class))) + .thenReturn(List.of(new AuditLogItemResponse( + 1L, + "USER_STATUS_CHANGE", + "user-1", + "alice", + "{\"status\":\"DISABLED\"}", + "127.0.0.1", + Instant.parse("2026-03-13T01:00:00Z") + ))); + + PageResponse response = service.listAuditLogs(0, 20, "user-1", "USER_STATUS_CHANGE"); + + assertThat(response.total()).isEqualTo(1); + assertThat(response.items()).hasSize(1); + verify(jdbcTemplate).queryForObject(contains("al.actor_user_id = :userId"), any(MapSqlParameterSource.class), eq(Long.class)); + verify(jdbcTemplate).query(contains("al.action = :action"), any(MapSqlParameterSource.class), any(RowMapper.class)); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java new file mode 100644 index 00000000..03b7b853 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java @@ -0,0 +1,147 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.entity.Role; +import com.iflytek.skillhub.auth.entity.UserRoleBinding; +import com.iflytek.skillhub.auth.repository.RoleRepository; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.repository.AdminUserSearchRepository; +import org.junit.jupiter.api.Test; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Sort; +import org.springframework.test.util.ReflectionTestUtils; + +import java.time.LocalDateTime; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; + +class AdminUserAppServiceTest { + + private final AdminUserSearchRepository adminUserSearchRepository = mock(AdminUserSearchRepository.class); + private final UserRoleBindingRepository userRoleBindingRepository = mock(UserRoleBindingRepository.class); + private final RoleRepository roleRepository = mock(RoleRepository.class); + private final UserAccountRepository userAccountRepository = mock(UserAccountRepository.class); + private final AdminUserAppService service = new AdminUserAppService( + adminUserSearchRepository, + userAccountRepository, + userRoleBindingRepository, + roleRepository + ); + + @Test + void listUsers_returnsPagedUsersFromRepository() { + UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE); + PageRequest pageable = PageRequest.of(0, 20, Sort.by(Sort.Direction.DESC, "createdAt")); + when(adminUserSearchRepository.search("ali", UserStatus.ACTIVE, pageable)) + .thenReturn(new PageImpl<>(List.of(user), pageable, 1)); + when(userRoleBindingRepository.findByUserIdIn(List.of("user-1"))) + .thenReturn(List.of(new UserRoleBinding("user-1", role("AUDITOR")))); + + PageResponse response = service.listUsers("ali", "ACTIVE", 0, 20); + + assertThat(response.total()).isEqualTo(1); + assertThat(response.items()).hasSize(1); + assertThat(response.items().get(0)).extracting("id", "username", "email", "status") + .containsExactly("user-1", "alice", "alice@example.com", "ACTIVE"); + assertThat(response.items().get(0)).extracting("platformRoles") + .isEqualTo(List.of("AUDITOR")); + } + + @Test + void listUsers_withInvalidStatus_throwsBadRequest() { + assertThrows(DomainBadRequestException.class, () -> service.listUsers(null, "BANNED", 0, 20)); + } + + @Test + void updateUserRole_nonSuperAdminCannotAssignSuperAdmin() { + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE))); + + assertThrows(DomainForbiddenException.class, + () -> service.updateUserRole("user-1", "SUPER_ADMIN", Set.of("USER_ADMIN"))); + } + + @Test + void updateUserRole_replacesExistingBindings() { + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE))); + when(roleRepository.findByCode("AUDITOR")).thenReturn(Optional.of(role("AUDITOR"))); + + var response = service.updateUserRole("user-1", "AUDITOR", Set.of("SUPER_ADMIN")); + + verify(userRoleBindingRepository).deleteByUserId("user-1"); + verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); + assertThat(response.userId()).isEqualTo("user-1"); + assertThat(response.role()).isEqualTo("AUDITOR"); + assertThat(response.status()).isEqualTo("ACTIVE"); + } + + @Test + void updateUserRole_userPseudoRoleClearsBindingsWithoutSavingNewRole() { + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE))); + + var response = service.updateUserRole("user-1", "USER", Set.of("SUPER_ADMIN")); + + verify(userRoleBindingRepository).deleteByUserId("user-1"); + verify(userRoleBindingRepository, never()).save(any(UserRoleBinding.class)); + assertThat(response.role()).isEqualTo("USER"); + } + + @Test + void updateUserStatus_rejectsUnsupportedStatuses() { + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE))); + + assertThrows(DomainBadRequestException.class, () -> service.updateUserStatus("user-1", "MERGED")); + } + + @Test + void updateUserStatus_updatesPersistedStatus() { + UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.ACTIVE); + when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user)); + when(userAccountRepository.save(user)).thenReturn(user); + + var response = service.updateUserStatus("user-1", "DISABLED"); + + verify(userAccountRepository).save(user); + assertThat(user.getStatus()).isEqualTo(UserStatus.DISABLED); + assertThat(response.status()).isEqualTo("DISABLED"); + } + + @Test + void updateUserStatus_withUnknownUser_throwsNotFound() { + when(userAccountRepository.findById("missing")).thenReturn(Optional.empty()); + + assertThrows(DomainNotFoundException.class, () -> service.updateUserStatus("missing", "DISABLED")); + } + + private UserAccount user(String id, String displayName, String email, UserStatus status) { + UserAccount user = new UserAccount(id, displayName, email, null); + user.setStatus(status); + ReflectionTestUtils.setField(user, "createdAt", LocalDateTime.of(2026, 3, 13, 9, 0)); + ReflectionTestUtils.setField(user, "updatedAt", LocalDateTime.of(2026, 3, 13, 9, 0)); + return user; + } + + private Role role(String code) { + Role role = new Role(); + ReflectionTestUtils.setField(role, "code", code); + ReflectionTestUtils.setField(role, "name", code); + return role; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java index ba1f128e..889cd1cb 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java @@ -6,6 +6,7 @@ import com.iflytek.skillhub.auth.oauth.OAuth2LoginSuccessHandler; import com.iflytek.skillhub.auth.oauth.SkillHubOAuth2AuthorizationRequestResolver; import com.iflytek.skillhub.auth.mock.MockAuthFilter; import com.iflytek.skillhub.auth.token.ApiTokenAuthenticationFilter; +import com.iflytek.skillhub.auth.token.ApiTokenScopeFilter; import org.springframework.beans.factory.ObjectProvider; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -35,6 +36,7 @@ public class SecurityConfig { private final OAuth2LoginSuccessHandler successHandler; private final OAuth2LoginFailureHandler failureHandler; private final ApiTokenAuthenticationFilter apiTokenAuthenticationFilter; + private final ApiTokenScopeFilter apiTokenScopeFilter; private final AuthenticationEntryPoint apiAuthenticationEntryPoint; private final AccessDeniedHandler apiAccessDeniedHandler; private final ObjectProvider mockAuthFilterProvider; @@ -44,6 +46,7 @@ public class SecurityConfig { OAuth2LoginSuccessHandler successHandler, OAuth2LoginFailureHandler failureHandler, ApiTokenAuthenticationFilter apiTokenAuthenticationFilter, + ApiTokenScopeFilter apiTokenScopeFilter, AuthenticationEntryPoint apiAuthenticationEntryPoint, AccessDeniedHandler apiAccessDeniedHandler, ObjectProvider mockAuthFilterProvider) { @@ -52,6 +55,7 @@ public class SecurityConfig { this.successHandler = successHandler; this.failureHandler = failureHandler; this.apiTokenAuthenticationFilter = apiTokenAuthenticationFilter; + this.apiTokenScopeFilter = apiTokenScopeFilter; this.apiAuthenticationEntryPoint = apiAuthenticationEntryPoint; this.apiAccessDeniedHandler = apiAccessDeniedHandler; this.mockAuthFilterProvider = mockAuthFilterProvider; @@ -88,7 +92,9 @@ public class SecurityConfig { "/api/compat/v1/resolve/**", "/api/compat/v1/download/**" ).permitAll() - .requestMatchers(HttpMethod.GET, + .requestMatchers(HttpMethod.GET, "/api/v1/skills/*/star", "/api/v1/skills/*/rating").authenticated() + .requestMatchers( + HttpMethod.GET, "/api/v1/skills", "/api/v1/skills/*/*", "/api/v1/skills/*/*/versions", @@ -134,7 +140,8 @@ public class SecurityConfig { .invalidateHttpSession(true) .deleteCookies("SESSION") ) - .addFilterBefore(apiTokenAuthenticationFilter, UsernamePasswordAuthenticationFilter.class); + .addFilterBefore(apiTokenAuthenticationFilter, UsernamePasswordAuthenticationFilter.class) + .addFilterAfter(apiTokenScopeFilter, ApiTokenAuthenticationFilter.class); MockAuthFilter mockAuthFilter = mockAuthFilterProvider.getIfAvailable(); if (mockAuthFilter != null) { diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java index 2e3db564..7a60e0df 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java @@ -1,15 +1,11 @@ package com.iflytek.skillhub.auth.device; -import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.auth.token.ApiTokenService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.data.redis.core.RedisOperations; import org.springframework.data.redis.core.RedisTemplate; -import org.springframework.data.redis.core.SessionCallback; -import org.springframework.data.redis.core.ValueOperations; import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; import java.security.SecureRandom; import java.util.Base64; @@ -19,32 +15,27 @@ import java.util.concurrent.TimeUnit; public class DeviceAuthService { private static final String DEVICE_CODE_PREFIX = "device:code:"; + private static final String DEVICE_CLAIM_PREFIX = "device:claim:"; private static final String USER_CODE_PREFIX = "device:usercode:"; private static final int EXPIRES_IN_SECONDS = 900; private static final int POLL_INTERVAL_SECONDS = 5; private static final String USER_CODE_CHARS = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; + private static final long PENDING_CODE_TTL_MINUTES = EXPIRES_IN_SECONDS / 60L; + private static final long USED_CODE_TTL_MINUTES = 1L; + private static final String CLI_DEVICE_TOKEN_NAME = "CLI Device Flow"; + private static final String CLI_DEVICE_SCOPE_JSON = "[\"skill:read\",\"skill:publish\"]"; private final RedisTemplate redisTemplate; - private final String verificationUri; - private final ObjectMapper objectMapper; private final ApiTokenService apiTokenService; + private final String verificationUri; private final SecureRandom random = new SecureRandom(); - @Autowired public DeviceAuthService(RedisTemplate redisTemplate, ApiTokenService apiTokenService, @Value("${skillhub.device-auth.verification-uri:/device}") String verificationUri) { - this(redisTemplate, apiTokenService, verificationUri, new ObjectMapper()); - } - - public DeviceAuthService(RedisTemplate redisTemplate, - ApiTokenService apiTokenService, - String verificationUri, - ObjectMapper objectMapper) { this.redisTemplate = redisTemplate; - this.verificationUri = verificationUri; - this.objectMapper = objectMapper; this.apiTokenService = apiTokenService; + this.verificationUri = verificationUri; } public DeviceCodeResponse generateDeviceCode() { @@ -54,9 +45,9 @@ public class DeviceAuthService { DeviceCodeData data = new DeviceCodeData(deviceCode, userCode, DeviceCodeStatus.PENDING, null); redisTemplate.opsForValue().set( - DEVICE_CODE_PREFIX + deviceCode, data, EXPIRES_IN_SECONDS / 60, TimeUnit.MINUTES); + DEVICE_CODE_PREFIX + deviceCode, data, PENDING_CODE_TTL_MINUTES, TimeUnit.MINUTES); redisTemplate.opsForValue().set( - USER_CODE_PREFIX + userCode, deviceCode, EXPIRES_IN_SECONDS / 60, TimeUnit.MINUTES); + USER_CODE_PREFIX + userCode, deviceCode, PENDING_CODE_TTL_MINUTES, TimeUnit.MINUTES); return new DeviceCodeResponse(deviceCode, userCode, verificationUri, EXPIRES_IN_SECONDS, POLL_INTERVAL_SECONDS); } @@ -67,60 +58,76 @@ public class DeviceAuthService { throw new DomainBadRequestException("error.deviceAuth.userCode.invalid"); } - DeviceCodeData data = readDeviceCodeData(deviceCode); + DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); if (data == null) { throw new DomainBadRequestException("error.deviceAuth.deviceCode.expired"); } - data.setStatus(DeviceCodeStatus.AUTHORIZED); - data.setUserId(userId); - redisTemplate.opsForValue().set( - DEVICE_CODE_PREFIX + deviceCode, data, EXPIRES_IN_SECONDS / 60, TimeUnit.MINUTES); + switch (data.getStatus()) { + case PENDING -> { + data.setStatus(DeviceCodeStatus.AUTHORIZED); + data.setUserId(userId); + redisTemplate.opsForValue().set( + DEVICE_CODE_PREFIX + deviceCode, data, PENDING_CODE_TTL_MINUTES, TimeUnit.MINUTES); + } + case AUTHORIZED -> { + if (!userId.equals(data.getUserId())) { + throw new DomainBadRequestException("error.deviceAuth.deviceCode.alreadyAuthorized"); + } + } + case USED -> throw new DomainBadRequestException("error.deviceAuth.deviceCode.used"); + } } public DeviceTokenResponse pollToken(String deviceCode) { - String key = DEVICE_CODE_PREFIX + deviceCode; - DeviceCodeData consumed = redisTemplate.execute(new SessionCallback<>() { - @Override - public DeviceCodeData execute(RedisOperations operations) { - while (true) { - operations.watch(key); - DeviceCodeData data = readDeviceCodeData(operations.opsForValue(), deviceCode); + DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); - if (data == null) { - operations.unwatch(); - throw new DomainBadRequestException("error.deviceAuth.deviceCode.invalid"); - } - - switch (data.getStatus()) { - case PENDING -> { - operations.unwatch(); - return null; - } - case USED -> { - operations.unwatch(); - throw new DomainBadRequestException("error.deviceAuth.deviceCode.used"); - } - case AUTHORIZED -> { - data.setStatus(DeviceCodeStatus.USED); - operations.multi(); - operations.opsForValue().set(key, data, 1, TimeUnit.MINUTES); - if (operations.exec() != null) { - return data; - } - } - } - } - } - }); - - if (consumed == null) { - return DeviceTokenResponse.pending(); + if (data == null) { + throw new DomainBadRequestException("error.deviceAuth.deviceCode.invalid"); } - String token = apiTokenService.createToken( - consumed.getUserId(), "device-auth", "[]").rawToken(); - return DeviceTokenResponse.success(token); + return switch (data.getStatus()) { + case PENDING -> DeviceTokenResponse.pending(); + case AUTHORIZED -> redeemAuthorizedDeviceCode(deviceCode, data); + case USED -> throw new DomainBadRequestException("error.deviceAuth.deviceCode.used"); + }; + } + + private DeviceTokenResponse redeemAuthorizedDeviceCode(String deviceCode, DeviceCodeData data) { + boolean claimed = Boolean.TRUE.equals(redisTemplate.opsForValue().setIfAbsent( + DEVICE_CLAIM_PREFIX + deviceCode, + "claimed", + USED_CODE_TTL_MINUTES, + TimeUnit.MINUTES + )); + if (!claimed) { + throw new DomainBadRequestException("error.deviceAuth.deviceCode.used"); + } + + try { + if (!StringUtils.hasText(data.getUserId())) { + throw new DomainBadRequestException("error.deviceAuth.deviceCode.invalid"); + } + + String token = apiTokenService.createToken( + data.getUserId(), + CLI_DEVICE_TOKEN_NAME, + CLI_DEVICE_SCOPE_JSON + ).rawToken(); + + data.setStatus(DeviceCodeStatus.USED); + redisTemplate.opsForValue().set( + DEVICE_CODE_PREFIX + deviceCode, + data, + USED_CODE_TTL_MINUTES, + TimeUnit.MINUTES + ); + redisTemplate.delete(USER_CODE_PREFIX + data.getUserCode()); + return DeviceTokenResponse.success(token); + } catch (RuntimeException ex) { + redisTemplate.delete(DEVICE_CLAIM_PREFIX + deviceCode); + throw ex; + } } private String generateRandomDeviceCode() { @@ -137,19 +144,4 @@ public class DeviceAuthService { } return code.toString(); } - - private DeviceCodeData readDeviceCodeData(String deviceCode) { - return readDeviceCodeData(redisTemplate.opsForValue(), deviceCode); - } - - private DeviceCodeData readDeviceCodeData(ValueOperations valueOperations, String deviceCode) { - Object raw = valueOperations.get(DEVICE_CODE_PREFIX + deviceCode); - if (raw == null) { - return null; - } - if (raw instanceof DeviceCodeData data) { - return data; - } - return objectMapper.convertValue(raw, DeviceCodeData.class); - } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/UserRoleBindingRepository.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/UserRoleBindingRepository.java index ceb753c3..84e5bd37 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/UserRoleBindingRepository.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/repository/UserRoleBindingRepository.java @@ -1,9 +1,10 @@ package com.iflytek.skillhub.auth.repository; import com.iflytek.skillhub.auth.entity.UserRoleBinding; -import java.util.Collection; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.stereotype.Repository; + +import java.util.Collection; import java.util.List; @Repository diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java index 2a09417f..01632e9b 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java @@ -16,6 +16,9 @@ import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -28,13 +31,16 @@ public class ApiTokenAuthenticationFilter extends OncePerRequestFilter { private final ApiTokenService apiTokenService; private final UserAccountRepository userRepo; private final UserRoleBindingRepository roleBindingRepo; + private final ApiTokenScopeService apiTokenScopeService; public ApiTokenAuthenticationFilter(ApiTokenService apiTokenService, - UserAccountRepository userRepo, - UserRoleBindingRepository roleBindingRepo) { + UserAccountRepository userRepo, + UserRoleBindingRepository roleBindingRepo, + ApiTokenScopeService apiTokenScopeService) { this.apiTokenService = apiTokenService; this.userRepo = userRepo; this.roleBindingRepo = roleBindingRepo; + this.apiTokenScopeService = apiTokenScopeService; } @Override @@ -44,20 +50,28 @@ public class ApiTokenAuthenticationFilter extends OncePerRequestFilter { if (authHeader != null && authHeader.startsWith(BEARER_PREFIX)) { String rawToken = authHeader.substring(BEARER_PREFIX.length()); apiTokenService.validateToken(rawToken).ifPresent(token -> { - apiTokenService.touchLastUsed(token); userRepo.findById(token.getUserId()).ifPresent(user -> { + if (!user.isActive()) { + return; + } Set roles = roleBindingRepo.findByUserId(user.getId()).stream() .map(rb -> rb.getRole().getCode()) .collect(Collectors.toSet()); + Set scopes = apiTokenScopeService.parseScopes(token.getScopeJson()); PlatformPrincipal principal = new PlatformPrincipal( user.getId(), user.getDisplayName(), user.getEmail(), user.getAvatarUrl(), "api_token", roles ); - var authorities = roles.stream() + List authorities = new ArrayList<>(); + authorities.addAll(roles.stream() .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) - .toList(); + .toList()); + authorities.addAll(scopes.stream() + .map(scope -> new SimpleGrantedAuthority("SCOPE_" + scope)) + .toList()); var auth = new UsernamePasswordAuthenticationToken(principal, null, authorities); SecurityContextHolder.getContext().setAuthentication(auth); + apiTokenService.touchLastUsed(token); }); }); } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java new file mode 100644 index 00000000..c4ff48a5 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java @@ -0,0 +1,81 @@ +package com.iflytek.skillhub.auth.token; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.access.AccessDeniedHandler; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; +import java.util.Set; +import java.util.stream.Collectors; + +@Component +public class ApiTokenScopeFilter extends OncePerRequestFilter { + + private final ApiTokenScopeService apiTokenScopeService; + private final AccessDeniedHandler accessDeniedHandler; + + public ApiTokenScopeFilter(ApiTokenScopeService apiTokenScopeService, + AccessDeniedHandler accessDeniedHandler) { + this.apiTokenScopeService = apiTokenScopeService; + this.accessDeniedHandler = accessDeniedHandler; + } + + @Override + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (!isApiTokenAuthentication(authentication)) { + filterChain.doFilter(request, response); + return; + } + + Set tokenScopes = authentication.getAuthorities().stream() + .map(GrantedAuthority::getAuthority) + .filter(authority -> authority.startsWith("SCOPE_")) + .map(authority -> authority.substring("SCOPE_".length())) + .collect(Collectors.toSet()); + + ApiTokenScopeService.AuthorizationDecision decision = apiTokenScopeService.authorize( + request.getMethod(), + request.getRequestURI(), + tokenScopes + ); + + if (decision.allowed()) { + filterChain.doFilter(request, response); + return; + } + + accessDeniedHandler.handle( + request, + response, + new AccessDeniedException(decision.message()) + ); + } + + @Override + protected boolean shouldNotFilter(HttpServletRequest request) { + String path = request.getRequestURI(); + return path == null || (!path.startsWith("/api/v1/") && !path.startsWith("/api/compat/")); + } + + private boolean isApiTokenAuthentication(Authentication authentication) { + if (authentication == null || !authentication.isAuthenticated()) { + return false; + } + + Object principal = authentication.getPrincipal(); + return principal instanceof PlatformPrincipal platformPrincipal + && "api_token".equals(platformPrincipal.oauthProvider()); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeService.java new file mode 100644 index 00000000..09445f69 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeService.java @@ -0,0 +1,132 @@ +package com.iflytek.skillhub.auth.token; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.springframework.stereotype.Service; +import org.springframework.util.AntPathMatcher; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +@Service +public class ApiTokenScopeService { + + private static final TypeReference> STRING_LIST = new TypeReference<>() { + }; + + private static final List UNSCOPED_ALLOWED_RULES = List.of( + ScopeRule.allow(null, "/api/v1/health"), + ScopeRule.allow(null, "/api/v1/auth/providers"), + ScopeRule.allow(null, "/api/v1/auth/me"), + ScopeRule.allow(null, "/api/v1/cli/auth/device/**"), + ScopeRule.allow(null, "/api/v1/cli/check"), + ScopeRule.allow("GET", "/api/v1/cli/whoami"), + ScopeRule.allow("GET", "/api/v1/skills"), + ScopeRule.allow("GET", "/api/v1/skills/**"), + ScopeRule.allow("GET", "/api/v1/namespaces"), + ScopeRule.allow("GET", "/api/v1/namespaces/*"), + ScopeRule.allow("GET", "/api/compat/v1/search"), + ScopeRule.allow("GET", "/api/compat/v1/resolve/**"), + ScopeRule.allow("GET", "/api/compat/v1/whoami"), + ScopeRule.allow(null, "/.well-known/**"), + ScopeRule.allow(null, "/actuator/health"), + ScopeRule.allow(null, "/v3/api-docs/**"), + ScopeRule.allow(null, "/swagger-ui/**") + ); + + private static final List REQUIRED_SCOPE_RULES = List.of( + ScopeRule.require(null, "/api/v1/tokens", "token:manage"), + ScopeRule.require(null, "/api/v1/tokens/**", "token:manage"), + ScopeRule.require("POST", "/api/v1/skills/*/publish", "skill:publish"), + ScopeRule.require("POST", "/api/v1/cli/publish", "skill:publish"), + ScopeRule.require("POST", "/api/compat/v1/publish", "skill:publish") + ); + + private final ObjectMapper objectMapper; + private final AntPathMatcher pathMatcher = new AntPathMatcher(); + + public ApiTokenScopeService(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + public Set parseScopes(String scopeJson) { + if (scopeJson == null || scopeJson.isBlank()) { + return Set.of(); + } + + try { + List scopes = objectMapper.readValue(scopeJson, STRING_LIST); + Set normalized = new LinkedHashSet<>(); + for (String scope : scopes) { + if (scope != null) { + String trimmed = scope.trim(); + if (!trimmed.isEmpty()) { + normalized.add(trimmed); + } + } + } + return Set.copyOf(normalized); + } catch (Exception e) { + return Set.of(); + } + } + + public AuthorizationDecision authorize(String method, String path, Set tokenScopes) { + if (!isApiPath(path)) { + return AuthorizationDecision.allow(); + } + + for (ScopeRule rule : UNSCOPED_ALLOWED_RULES) { + if (rule.matches(method, path, pathMatcher)) { + return AuthorizationDecision.allow(); + } + } + + for (ScopeRule rule : REQUIRED_SCOPE_RULES) { + if (rule.matches(method, path, pathMatcher)) { + if (tokenScopes.contains(rule.requiredScope())) { + return AuthorizationDecision.allow(); + } + return AuthorizationDecision.missingScope(rule.requiredScope()); + } + } + + return AuthorizationDecision.unsupported(path); + } + + private boolean isApiPath(String path) { + return path != null && (path.startsWith("/api/v1/") || path.startsWith("/api/compat/")); + } + + public record AuthorizationDecision(boolean allowed, String requiredScope, String message) { + public static AuthorizationDecision allow() { + return new AuthorizationDecision(true, null, null); + } + + public static AuthorizationDecision missingScope(String requiredScope) { + return new AuthorizationDecision(false, requiredScope, "Missing API token scope: " + requiredScope); + } + + public static AuthorizationDecision unsupported(String path) { + return new AuthorizationDecision(false, null, "API token cannot access endpoint: " + path); + } + } + + private record ScopeRule(String method, String pattern, String requiredScope) { + static ScopeRule allow(String method, String pattern) { + return new ScopeRule(method, pattern, null); + } + + static ScopeRule require(String method, String pattern, String requiredScope) { + return new ScopeRule(method, pattern, requiredScope); + } + + boolean matches(String requestMethod, String requestPath, AntPathMatcher matcher) { + if (method != null && !method.equalsIgnoreCase(requestMethod)) { + return false; + } + return matcher.match(pattern, requestPath); + } + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java index e3118d65..ac15521a 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java @@ -1,21 +1,17 @@ package com.iflytek.skillhub.auth.device; -import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.auth.entity.ApiToken; import com.iflytek.skillhub.auth.token.ApiTokenService; -import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.data.redis.core.RedisOperations; -import org.springframework.data.redis.core.SessionCallback; import org.springframework.data.redis.core.RedisTemplate; import org.springframework.data.redis.core.ValueOperations; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; @@ -31,8 +27,6 @@ class DeviceAuthServiceTest { @Mock private ValueOperations valueOperations; - @Mock - private RedisOperations redisOperations; @Mock private ApiTokenService apiTokenService; @@ -41,11 +35,8 @@ class DeviceAuthServiceTest { @BeforeEach void setUp() { - lenient().when(redisTemplate.opsForValue()).thenReturn(valueOperations); - lenient().when(redisOperations.opsForValue()).thenReturn(valueOperations); - lenient().when(redisTemplate.execute(any(SessionCallback.class))) - .thenAnswer(invocation -> invocation.>getArgument(0).execute(redisOperations)); - service = new DeviceAuthService(redisTemplate, apiTokenService, "https://skillhub.example.com/device", new ObjectMapper()); + when(redisTemplate.opsForValue()).thenReturn(valueOperations); + service = new DeviceAuthService(redisTemplate, apiTokenService, "https://skillhub.example.com/device"); } @Test @@ -91,6 +82,41 @@ class DeviceAuthServiceTest { assertThat(response.accessToken()).isNull(); } + @Test + void pollToken_returns_access_token_when_authorized() { + // Given + DeviceCodeData data = new DeviceCodeData("device123", "ABCD-1234", DeviceCodeStatus.AUTHORIZED, "42"); + when(valueOperations.get("device:code:device123")).thenReturn(data); + when(valueOperations.setIfAbsent("device:claim:device123", "claimed", 1L, TimeUnit.MINUTES)).thenReturn(true); + when(apiTokenService.createToken("42", "CLI Device Flow", "[\"skill:read\",\"skill:publish\"]")) + .thenReturn(new ApiTokenService.TokenCreateResult("sk_cli_token", mock(ApiToken.class))); + + // When + DeviceTokenResponse response = service.pollToken("device123"); + + // Then + assertThat(response.accessToken()).isEqualTo("sk_cli_token"); + assertThat(response.tokenType()).isEqualTo("Bearer"); + assertThat(response.error()).isNull(); + assertThat(data.getStatus()).isEqualTo(DeviceCodeStatus.USED); + verify(valueOperations).set("device:code:device123", data, 1L, TimeUnit.MINUTES); + verify(redisTemplate).delete("device:usercode:ABCD-1234"); + } + + @Test + void pollToken_rejects_second_exchange_attempt() { + // Given + DeviceCodeData data = new DeviceCodeData("device123", "ABCD-1234", DeviceCodeStatus.AUTHORIZED, "42"); + when(valueOperations.get("device:code:device123")).thenReturn(data); + when(valueOperations.setIfAbsent("device:claim:device123", "claimed", 1L, TimeUnit.MINUTES)).thenReturn(false); + + // When / Then + assertThatThrownBy(() -> service.pollToken("device123")) + .isInstanceOf(DomainBadRequestException.class) + .hasMessageContaining("error.deviceAuth.deviceCode.used"); + verify(apiTokenService, never()).createToken(anyString(), anyString(), anyString()); + } + @Test void pollToken_returns_error_when_expired() { // Given @@ -119,62 +145,15 @@ class DeviceAuthServiceTest { } @Test - void authorizeDeviceCode_accepts_linked_hash_map_from_redis_serializer() { - Map redisValue = new HashMap<>(); - redisValue.put("deviceCode", "device123"); - redisValue.put("userCode", "ABCD-1234"); - redisValue.put("status", "PENDING"); - redisValue.put("userId", null); - when(valueOperations.get("device:usercode:ABCD-1234")).thenReturn("device123"); - when(valueOperations.get("device:code:device123")).thenReturn(redisValue); - - service.authorizeDeviceCode("ABCD-1234", "42"); - - ArgumentCaptor captor = ArgumentCaptor.forClass(DeviceCodeData.class); - verify(valueOperations).set(eq("device:code:device123"), captor.capture(), eq(15L), eq(TimeUnit.MINUTES)); - assertThat(captor.getValue().getStatus()).isEqualTo(DeviceCodeStatus.AUTHORIZED); - assertThat(captor.getValue().getUserId()).isEqualTo("42"); - } - - @Test - void pollToken_accepts_linked_hash_map_from_redis_serializer() { - Map redisValue = new HashMap<>(); - redisValue.put("deviceCode", "device123"); - redisValue.put("userCode", "ABCD-1234"); - redisValue.put("status", "AUTHORIZED"); - redisValue.put("userId", "42"); - when(valueOperations.get("device:code:device123")).thenReturn(redisValue); - when(redisOperations.exec()).thenReturn(java.util.List.of("OK")); - when(apiTokenService.createToken("42", "device-auth", "[]")) - .thenReturn(new ApiTokenService.TokenCreateResult("sk_device_token", null)); - - DeviceTokenResponse response = service.pollToken("device123"); - - assertThat(response.error()).isNull(); - assertThat(response.accessToken()).isEqualTo("sk_device_token"); - assertThat(response.tokenType()).isEqualTo("Bearer"); - verify(redisOperations).watch("device:code:device123"); - verify(redisOperations).multi(); - verify(valueOperations).set(eq("device:code:device123"), any(DeviceCodeData.class), eq(1L), eq(TimeUnit.MINUTES)); - verify(redisOperations).exec(); - } - - @Test - void pollToken_returns_access_token_when_authorized() { + void authorizeDeviceCode_rejects_different_user_after_authorization() { + // Given DeviceCodeData data = new DeviceCodeData("device123", "ABCD-1234", DeviceCodeStatus.AUTHORIZED, "42"); + when(valueOperations.get("device:usercode:ABCD-1234")).thenReturn("device123"); when(valueOperations.get("device:code:device123")).thenReturn(data); - when(redisOperations.exec()).thenReturn(java.util.List.of("OK")); - when(apiTokenService.createToken("42", "device-auth", "[]")) - .thenReturn(new ApiTokenService.TokenCreateResult("sk_device_token", null)); - DeviceTokenResponse response = service.pollToken("device123"); - - assertThat(response.error()).isNull(); - assertThat(response.accessToken()).isEqualTo("sk_device_token"); - assertThat(response.tokenType()).isEqualTo("Bearer"); - verify(redisOperations).watch("device:code:device123"); - verify(redisOperations).multi(); - verify(valueOperations).set(eq("device:code:device123"), eq(data), eq(1L), eq(TimeUnit.MINUTES)); - verify(redisOperations).exec(); + // When / Then + assertThatThrownBy(() -> service.authorizeDeviceCode("ABCD-1234", "99")) + .isInstanceOf(DomainBadRequestException.class) + .hasMessageContaining("error.deviceAuth.deviceCode.alreadyAuthorized"); } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java new file mode 100644 index 00000000..a981ffd1 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java @@ -0,0 +1,95 @@ +package com.iflytek.skillhub.auth.token; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.entity.ApiToken; +import com.iflytek.skillhub.auth.entity.Role; +import com.iflytek.skillhub.auth.entity.UserRoleBinding; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.core.context.SecurityContextHolder; +import java.util.List; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class ApiTokenAuthenticationFilterTest { + + private final ApiTokenService apiTokenService = mock(ApiTokenService.class); + private final UserAccountRepository userAccountRepository = mock(UserAccountRepository.class); + private final UserRoleBindingRepository roleBindingRepository = mock(UserRoleBindingRepository.class); + private final ApiTokenScopeService scopeService = new ApiTokenScopeService(new ObjectMapper()); + private final ApiTokenAuthenticationFilter filter = new ApiTokenAuthenticationFilter( + apiTokenService, + userAccountRepository, + roleBindingRepository, + scopeService + ); + + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + + @Test + void shouldPopulateRoleAndScopeAuthoritiesForActiveUser() throws Exception { + ApiToken token = new ApiToken("user-1", "cli", "sk_test", "hash", "[\"skill:publish\",\"token:manage\"]"); + UserAccount user = new UserAccount("user-1", "Alice", "alice@example.com", ""); + UserRoleBinding binding = mock(UserRoleBinding.class); + Role role = mock(Role.class); + + when(apiTokenService.validateToken("raw-token")).thenReturn(Optional.of(token)); + when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user)); + when(roleBindingRepository.findByUserId("user-1")).thenReturn(List.of(binding)); + when(binding.getRole()).thenReturn(role); + when(role.getCode()).thenReturn("SKILL_ADMIN"); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/api/v1/cli/whoami"); + request.addHeader("Authorization", "Bearer raw-token"); + + filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + + var authentication = SecurityContextHolder.getContext().getAuthentication(); + assertNotNull(authentication); + assertTrue(authentication.getAuthorities().stream() + .anyMatch(authority -> authority.getAuthority().equals("ROLE_SKILL_ADMIN"))); + assertTrue(authentication.getAuthorities().stream() + .anyMatch(authority -> authority.getAuthority().equals("SCOPE_skill:publish"))); + assertTrue(authentication.getAuthorities().stream() + .anyMatch(authority -> authority.getAuthority().equals("SCOPE_token:manage"))); + verify(apiTokenService).touchLastUsed(token); + } + + @Test + void shouldRejectDisabledUsers() throws Exception { + ApiToken token = new ApiToken("user-2", "cli", "sk_test", "hash", "[\"skill:publish\"]"); + UserAccount user = new UserAccount("user-2", "Bob", "bob@example.com", ""); + user.setStatus(UserStatus.DISABLED); + + when(apiTokenService.validateToken("raw-token")).thenReturn(Optional.of(token)); + when(userAccountRepository.findById("user-2")).thenReturn(Optional.of(user)); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/api/v1/cli/publish"); + request.addHeader("Authorization", "Bearer raw-token"); + + filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + + assertNull(SecurityContextHolder.getContext().getAuthentication()); + verify(apiTokenService, never()).touchLastUsed(token); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java new file mode 100644 index 00000000..fd50db75 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java @@ -0,0 +1,102 @@ +package com.iflytek.skillhub.auth.token; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.access.AccessDeniedHandler; + +import java.util.List; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +class ApiTokenScopeFilterTest { + + private final ApiTokenScopeService scopeService = new ApiTokenScopeService(new ObjectMapper()); + + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + + @Test + void shouldDenyApiTokenWithoutRequiredScope() throws Exception { + AccessDeniedHandler handler = (request, response, accessDeniedException) -> { + response.sendError(HttpServletResponse.SC_FORBIDDEN, accessDeniedException.getMessage()); + }; + ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); + + PlatformPrincipal principal = new PlatformPrincipal( + "user-1", + "Alice", + "alice@example.com", + "", + "api_token", + Set.of("SKILL_ADMIN") + ); + var authentication = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of( + new SimpleGrantedAuthority("ROLE_SKILL_ADMIN"), + new SimpleGrantedAuthority("SCOPE_skill:read") + ) + ); + SecurityContextHolder.getContext().setAuthentication(authentication); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/cli/publish"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus()); + assertTrue(response.getErrorMessage().contains("Missing API token scope: skill:publish")); + verify(chain, never()).doFilter(request, response); + } + + @Test + void shouldAllowSessionAuthRequestsWithoutScopeChecks() throws Exception { + AccessDeniedHandler handler = mock(AccessDeniedHandler.class); + ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); + + PlatformPrincipal principal = new PlatformPrincipal( + "user-2", + "Carol", + "carol@example.com", + "", + "github", + Set.of("SUPER_ADMIN") + ); + var authentication = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) + ); + SecurityContextHolder.getContext().setAuthentication(authentication); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/admin/users/user-2/status"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + verify(chain).doFilter(request, response); + verify(handler, never()).handle(eq(request), eq(response), any()); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java new file mode 100644 index 00000000..70c73b2d --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java @@ -0,0 +1,77 @@ +package com.iflytek.skillhub.auth.token; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; + +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ApiTokenScopeServiceTest { + + private final ApiTokenScopeService scopeService = new ApiTokenScopeService(new ObjectMapper()); + + @Test + void parseScopesShouldNormalizeJsonArray() { + Set scopes = scopeService.parseScopes("[\"skill:read\", \"skill:publish\", \"skill:read\", \" \"]"); + + assertEquals(Set.of("skill:read", "skill:publish"), scopes); + } + + @Test + void authorizeShouldAllowCliWhoamiWithoutScope() { + ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( + "GET", + "/api/v1/cli/whoami", + Set.of() + ); + + assertTrue(decision.allowed()); + } + + @Test + void authorizeShouldRequirePublishScopeForPortalPublish() { + ApiTokenScopeService.AuthorizationDecision denied = scopeService.authorize( + "POST", + "/api/v1/skills/team-a/publish", + Set.of("skill:read") + ); + + assertFalse(denied.allowed()); + assertEquals("skill:publish", denied.requiredScope()); + + ApiTokenScopeService.AuthorizationDecision allowed = scopeService.authorize( + "POST", + "/api/v1/skills/team-a/publish", + Set.of("skill:publish") + ); + + assertTrue(allowed.allowed()); + } + + @Test + void authorizeShouldRequireTokenManageScopeForTokenEndpoints() { + ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( + "GET", + "/api/v1/tokens", + Set.of("skill:publish") + ); + + assertFalse(decision.allowed()); + assertEquals("token:manage", decision.requiredScope()); + } + + @Test + void authorizeShouldDenyUnsupportedAuthenticatedEndpoints() { + ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( + "GET", + "/api/v1/me/skills", + Set.of("skill:read", "skill:publish") + ); + + assertFalse(decision.allowed()); + assertEquals("API token cannot access endpoint: /api/v1/me/skills", decision.message()); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java index 40aff667..aaf3d705 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java @@ -9,15 +9,14 @@ import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.*; -import jakarta.persistence.EntityManager; import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.time.Instant; import java.time.LocalDateTime; import java.util.ConcurrentModificationException; import java.util.List; +import java.util.Map; import java.util.Set; @Service @@ -30,7 +29,6 @@ public class PromotionService { private final NamespaceRepository namespaceRepository; private final ReviewPermissionChecker permissionChecker; private final ApplicationEventPublisher eventPublisher; - private final EntityManager entityManager; public PromotionService(PromotionRequestRepository promotionRequestRepository, SkillRepository skillRepository, @@ -38,8 +36,7 @@ public class PromotionService { SkillFileRepository skillFileRepository, NamespaceRepository namespaceRepository, ReviewPermissionChecker permissionChecker, - ApplicationEventPublisher eventPublisher, - EntityManager entityManager) { + ApplicationEventPublisher eventPublisher) { this.promotionRequestRepository = promotionRequestRepository; this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; @@ -47,13 +44,12 @@ public class PromotionService { this.namespaceRepository = namespaceRepository; this.permissionChecker = permissionChecker; this.eventPublisher = eventPublisher; - this.entityManager = entityManager; } @Transactional public PromotionRequest submitPromotion(Long sourceSkillId, Long sourceVersionId, Long targetNamespaceId, String userId, - java.util.Map userNamespaceRoles, + Map userNamespaceRoles, Set platformRoles) { Skill sourceSkill = skillRepository.findById(sourceSkillId) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", sourceSkillId)); @@ -89,6 +85,44 @@ public class PromotionService { return promotionRequestRepository.save(request); } + @Transactional + public PromotionRequest submitPromotion(Long sourceSkillId, Long sourceVersionId, + Long targetNamespaceId, String userId, + Map userNamespaceRoles) { + Skill sourceSkill = skillRepository.findById(sourceSkillId) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", sourceSkillId)); + + SkillVersion sourceVersion = skillVersionRepository.findById(sourceVersionId) + .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", sourceVersionId)); + + if (!sourceVersion.getSkillId().equals(sourceSkillId)) { + throw new DomainBadRequestException("promotion.version_skill_mismatch", sourceVersionId, sourceSkillId); + } + + if (sourceVersion.getStatus() != SkillVersionStatus.PUBLISHED) { + throw new DomainBadRequestException("promotion.version_not_published", sourceVersionId); + } + + if (!permissionChecker.canSubmitPromotion(sourceSkill, userId, userNamespaceRoles)) { + throw new DomainForbiddenException("promotion.submit.no_permission"); + } + + Namespace targetNamespace = namespaceRepository.findById(targetNamespaceId) + .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", targetNamespaceId)); + + if (targetNamespace.getType() != NamespaceType.GLOBAL) { + throw new DomainBadRequestException("promotion.target_not_global", targetNamespaceId); + } + + promotionRequestRepository.findBySourceVersionIdAndStatus(sourceVersionId, ReviewTaskStatus.PENDING) + .ifPresent(existing -> { + throw new DomainBadRequestException("promotion.duplicate_pending", sourceVersionId); + }); + + PromotionRequest request = new PromotionRequest(sourceSkillId, sourceVersionId, targetNamespaceId, userId); + return promotionRequestRepository.save(request); + } + @Transactional public PromotionRequest approvePromotion(Long promotionId, String reviewerId, String comment, Set platformRoles) { @@ -108,11 +142,6 @@ public class PromotionService { if (updated == 0) { throw new ConcurrentModificationException("Promotion request was modified concurrently"); } - entityManager.detach(request); - request.setStatus(ReviewTaskStatus.APPROVED); - request.setReviewedBy(reviewerId); - request.setReviewComment(comment); - request.setReviewedAt(Instant.now()); Skill sourceSkill = skillRepository.findById(request.getSourceSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", request.getSourceSkillId())); @@ -155,12 +184,9 @@ public class PromotionService { .toList(); skillFileRepository.saveAll(copiedFiles); - int targetUpdated = promotionRequestRepository.updateStatusWithVersion( - promotionId, ReviewTaskStatus.APPROVED, reviewerId, comment, newSkill.getId(), request.getVersion() + 1); - if (targetUpdated == 0) { - throw new ConcurrentModificationException("Promotion request target skill was modified concurrently"); - } + // Update promotion request with target skill id request.setTargetSkillId(newSkill.getId()); + promotionRequestRepository.save(request); eventPublisher.publishEvent(new SkillPublishedEvent( newSkill.getId(), newVersion.getId(), reviewerId)); @@ -187,12 +213,8 @@ public class PromotionService { if (updated == 0) { throw new ConcurrentModificationException("Promotion request was modified concurrently"); } - entityManager.detach(request); - request.setStatus(ReviewTaskStatus.REJECTED); - request.setReviewedBy(reviewerId); - request.setReviewComment(comment); - request.setReviewedAt(Instant.now()); - return request; + + return promotionRequestRepository.findById(promotionId).orElse(request); } public boolean canViewPromotion(PromotionRequest request, String userId, Set platformRoles) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java index ffd44a05..c3cb3665 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewPermissionChecker.java @@ -11,27 +11,22 @@ import java.util.Set; @Component public class ReviewPermissionChecker { - /** - * Check if a user can review a ReviewTask. - * - * @param task the review task - * @param userId the reviewer's user ID - * @param namespaceType the type of the namespace - * @param userNamespaceRoles user's roles keyed by namespace ID - * @param platformRoles user's platform-level roles - * @return true if the user is allowed to review - */ + public boolean canSubmitReview(Long namespaceId, + Map userNamespaceRoles) { + NamespaceRole role = userNamespaceRoles.get(namespaceId); + return role == NamespaceRole.OWNER + || role == NamespaceRole.ADMIN + || role == NamespaceRole.MEMBER; + } + public boolean canReview(ReviewTask task, String userId, NamespaceType namespaceType, Map userNamespaceRoles, Set platformRoles) { - // Admins can review their own submissions if (task.getSubmittedBy().equals(userId)) { - return platformRoles.contains("SKILL_ADMIN") - || platformRoles.contains("SUPER_ADMIN"); + return false; } - return canReviewNamespace(task.getNamespaceId(), namespaceType, userNamespaceRoles, platformRoles); } @@ -42,9 +37,7 @@ public class ReviewPermissionChecker { if (skill.getOwnerId().equals(userId)) { return true; } - - if (platformRoles.contains("SKILL_ADMIN") - || platformRoles.contains("SUPER_ADMIN")) { + if (hasPlatformReviewRole(platformRoles)) { return true; } @@ -67,23 +60,45 @@ public class ReviewPermissionChecker { NamespaceType namespaceType, Map userNamespaceRoles, Set platformRoles) { - if (platformRoles.contains("SKILL_ADMIN") - || platformRoles.contains("SUPER_ADMIN")) { + if (hasPlatformReviewRole(platformRoles)) { return true; } - if (namespaceType == NamespaceType.GLOBAL) { return false; } NamespaceRole role = userNamespaceRoles.get(namespaceId); - return role == NamespaceRole.ADMIN || role == NamespaceRole.OWNER; + return role == NamespaceRole.OWNER || role == NamespaceRole.ADMIN; + } + + public boolean canManageNamespaceReviews(Long namespaceId, + NamespaceType namespaceType, + Map userNamespaceRoles, + Set platformRoles) { + return canReviewNamespace(namespaceId, namespaceType, userNamespaceRoles, platformRoles); + } + + public boolean canReadReview(ReviewTask task, + String userId, + NamespaceType namespaceType, + Map userNamespaceRoles, + Set platformRoles) { + return canViewReview(task, userId, namespaceType, userNamespaceRoles, platformRoles); + } + + public boolean canSubmitPromotion(Skill sourceSkill, + String userId, + Map userNamespaceRoles, + Set platformRoles) { + return canSubmitForReview(sourceSkill, userId, userNamespaceRoles, platformRoles); + } + + public boolean canSubmitPromotion(Skill sourceSkill, + String userId, + Map userNamespaceRoles) { + return canSubmitPromotion(sourceSkill, userId, userNamespaceRoles, Set.of()); } - /** - * Check if a user can review a PromotionRequest. - * Only SKILL_ADMIN or SUPER_ADMIN, and not own. - */ public boolean canReviewPromotion( PromotionRequest request, String userId, @@ -91,15 +106,7 @@ public class ReviewPermissionChecker { if (request.getSubmittedBy().equals(userId)) { return false; } - return platformRoles.contains("SKILL_ADMIN") - || platformRoles.contains("SUPER_ADMIN"); - } - - public boolean canSubmitPromotion(Skill skill, - String userId, - Map userNamespaceRoles, - Set platformRoles) { - return canSubmitForReview(skill, userId, userNamespaceRoles, platformRoles); + return hasPlatformReviewRole(platformRoles); } public boolean canViewPromotion(PromotionRequest request, @@ -110,4 +117,19 @@ public class ReviewPermissionChecker { } return canReviewPromotion(request, userId, platformRoles); } + + public boolean canListPendingPromotions(Set platformRoles) { + return hasPlatformReviewRole(platformRoles); + } + + public boolean canReadPromotion(PromotionRequest request, + String userId, + Set platformRoles) { + return canViewPromotion(request, userId, platformRoles); + } + + private boolean hasPlatformReviewRole(Set platformRoles) { + return platformRoles.contains("SKILL_ADMIN") + || platformRoles.contains("SUPER_ADMIN"); + } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index 9207b8cb..4d654eea 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.review; +import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; @@ -12,13 +13,12 @@ import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; -import jakarta.persistence.EntityManager; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; import org.springframework.context.ApplicationEventPublisher; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.time.Instant; import java.time.LocalDateTime; import java.util.ConcurrentModificationException; import java.util.Map; @@ -33,7 +33,7 @@ public class ReviewService { private final NamespaceRepository namespaceRepository; private final ReviewPermissionChecker permissionChecker; private final ApplicationEventPublisher eventPublisher; - private final EntityManager entityManager; + private final ObjectMapper objectMapper; public ReviewService(ReviewTaskRepository reviewTaskRepository, SkillVersionRepository skillVersionRepository, @@ -41,14 +41,14 @@ public class ReviewService { NamespaceRepository namespaceRepository, ReviewPermissionChecker permissionChecker, ApplicationEventPublisher eventPublisher, - EntityManager entityManager) { + ObjectMapper objectMapper) { this.reviewTaskRepository = reviewTaskRepository; this.skillVersionRepository = skillVersionRepository; this.skillRepository = skillRepository; this.namespaceRepository = namespaceRepository; this.permissionChecker = permissionChecker; this.eventPublisher = eventPublisher; - this.entityManager = entityManager; + this.objectMapper = objectMapper; } @Transactional @@ -58,6 +58,7 @@ public class ReviewService { Set platformRoles) { SkillVersion skillVersion = skillVersionRepository.findById(skillVersionId) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", skillVersionId)); + Skill skill = skillRepository.findById(skillVersion.getSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); @@ -80,6 +81,35 @@ public class ReviewService { } } + @Transactional + public ReviewTask submitReview(Long skillVersionId, + String userId, + Map userNamespaceRoles) { + SkillVersion skillVersion = skillVersionRepository.findById(skillVersionId) + .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", skillVersionId)); + + Skill skill = skillRepository.findById(skillVersion.getSkillId()) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); + + if (skillVersion.getStatus() != SkillVersionStatus.DRAFT) { + throw new DomainBadRequestException("review.submit.not_draft", skillVersionId); + } + + if (!permissionChecker.canSubmitReview(skill.getNamespaceId(), userNamespaceRoles)) { + throw new DomainForbiddenException("review.submit.no_permission"); + } + + skillVersion.setStatus(SkillVersionStatus.PENDING_REVIEW); + skillVersionRepository.save(skillVersion); + + ReviewTask task = new ReviewTask(skillVersionId, skill.getNamespaceId(), userId); + try { + return reviewTaskRepository.save(task); + } catch (DataIntegrityViolationException e) { + throw new DomainBadRequestException("review.submit.duplicate", skillVersionId); + } + } + @Transactional public ReviewTask approveReview(Long reviewTaskId, String reviewerId, String comment, Map userNamespaceRoles, @@ -104,11 +134,6 @@ public class ReviewService { if (updated == 0) { throw new ConcurrentModificationException("Review task was modified concurrently"); } - entityManager.detach(task); - task.setStatus(ReviewTaskStatus.APPROVED); - task.setReviewedBy(reviewerId); - task.setReviewComment(comment); - task.setReviewedAt(Instant.now()); SkillVersion skillVersion = skillVersionRepository.findById(task.getSkillVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", task.getSkillVersionId())); @@ -119,12 +144,15 @@ public class ReviewService { Skill skill = skillRepository.findById(skillVersion.getSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); skill.setLatestVersionId(skillVersion.getId()); + applyPublishedMetadata(skill, skillVersion); + skill.setUpdatedBy(reviewerId); skillRepository.save(skill); eventPublisher.publishEvent(new SkillPublishedEvent( skill.getId(), skillVersion.getId(), reviewerId)); - return task; + // Reload to return updated state + return reviewTaskRepository.findById(reviewTaskId).orElse(task); } @Transactional @@ -151,18 +179,13 @@ public class ReviewService { if (updated == 0) { throw new ConcurrentModificationException("Review task was modified concurrently"); } - entityManager.detach(task); - task.setStatus(ReviewTaskStatus.REJECTED); - task.setReviewedBy(reviewerId); - task.setReviewComment(comment); - task.setReviewedAt(Instant.now()); SkillVersion skillVersion = skillVersionRepository.findById(task.getSkillVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", task.getSkillVersionId())); skillVersion.setStatus(SkillVersionStatus.REJECTED); skillVersionRepository.save(skillVersion); - return task; + return reviewTaskRepository.findById(reviewTaskId).orElse(task); } @Transactional @@ -198,4 +221,19 @@ public class ReviewService { Set platformRoles) { return permissionChecker.canViewReview(task, userId, namespaceType, userNamespaceRoles, platformRoles); } + + private void applyPublishedMetadata(Skill skill, SkillVersion skillVersion) { + String metadataJson = skillVersion.getParsedMetadataJson(); + if (metadataJson == null || metadataJson.isBlank()) { + return; + } + + try { + SkillMetadata metadata = objectMapper.readValue(metadataJson, SkillMetadata.class); + skill.setDisplayName(metadata.name()); + skill.setSummary(metadata.description()); + } catch (Exception e) { + throw new IllegalStateException("Failed to deserialize skill metadata", e); + } + } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java index a65ad7d0..a6589562 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java @@ -57,6 +57,10 @@ public class SkillTagService { return tags; } + public List listTags(String namespaceSlug, String skillSlug) { + return listTags(namespaceSlug, skillSlug, null, java.util.Map.of()); + } + @Transactional public SkillTag createOrMoveTag( String namespaceSlug, diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java new file mode 100644 index 00000000..3c0e5498 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackagePolicy.java @@ -0,0 +1,56 @@ +package com.iflytek.skillhub.domain.skill.validation; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Set; + +public final class SkillPackagePolicy { + + public static final int MAX_FILE_COUNT = 100; + public static final long MAX_SINGLE_FILE_SIZE = 1024 * 1024; // 1MB + public static final long MAX_TOTAL_PACKAGE_SIZE = 10 * 1024 * 1024; // 10MB + public static final String SKILL_MD_PATH = "SKILL.md"; + public static final Set ALLOWED_EXTENSIONS = Set.of( + ".md", ".txt", ".json", ".yaml", ".yml", + ".js", ".ts", ".py", ".sh", + ".png", ".jpg", ".svg" + ); + + private SkillPackagePolicy() { + } + + public static String normalizeEntryPath(String rawPath) { + if (rawPath == null) { + throw new IllegalArgumentException("Package entry path is missing"); + } + + String sanitized = rawPath.replace('\\', '/').trim(); + if (sanitized.isEmpty()) { + throw new IllegalArgumentException("Package entry path is empty"); + } + if (sanitized.startsWith("/") || sanitized.startsWith("\\")) { + throw new IllegalArgumentException("Package entry path must be relative: " + rawPath); + } + if (sanitized.contains(":")) { + throw new IllegalArgumentException("Package entry path contains an invalid drive or scheme prefix: " + rawPath); + } + + Path normalized = Paths.get(sanitized).normalize(); + String canonical = normalized.toString().replace('\\', '/'); + if (normalized.isAbsolute() || canonical.isBlank()) { + throw new IllegalArgumentException("Package entry path is invalid: " + rawPath); + } + if (canonical.equals(".") || canonical.equals("..") || canonical.startsWith("../")) { + throw new IllegalArgumentException("Package entry path escapes package root: " + rawPath); + } + if (!sanitized.equals(canonical)) { + throw new IllegalArgumentException("Package entry path must be normalized: " + rawPath); + } + + return canonical; + } + + public static boolean hasAllowedExtension(String path) { + return ALLOWED_EXTENSIONS.stream().anyMatch(path::endsWith); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java index 4f11b546..142a8b2c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java @@ -3,8 +3,6 @@ package com.iflytek.skillhub.domain.skill.validation; import com.iflytek.skillhub.domain.shared.exception.LocalizedDomainException; import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; -import java.nio.file.InvalidPathException; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -12,13 +10,6 @@ import java.util.Set; public class SkillPackageValidator { - private static final String SKILL_MD_PATH = "SKILL.md"; - private static final Set DEFAULT_ALLOWED_EXTENSIONS = Set.of( - ".md", ".txt", ".json", ".yaml", ".yml", - ".js", ".ts", ".py", ".sh", - ".png", ".jpg", ".svg" - ); - private final SkillMetadataParser metadataParser; private final int maxFileCount; private final long maxSingleFileSize; @@ -26,7 +17,13 @@ public class SkillPackageValidator { private final Set allowedExtensions; public SkillPackageValidator(SkillMetadataParser metadataParser) { - this(metadataParser, 100, 1024 * 1024, 10 * 1024 * 1024, DEFAULT_ALLOWED_EXTENSIONS); + this( + metadataParser, + SkillPackagePolicy.MAX_FILE_COUNT, + SkillPackagePolicy.MAX_SINGLE_FILE_SIZE, + SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE, + SkillPackagePolicy.ALLOWED_EXTENSIONS + ); } public SkillPackageValidator(SkillMetadataParser metadataParser, @@ -45,33 +42,38 @@ public class SkillPackageValidator { public ValidationResult validate(List entries) { List errors = new ArrayList<>(); - Set seenPaths = new HashSet<>(); + Set normalizedPaths = new HashSet<>(); + PackageEntry skillMd = null; - // 1. Check file count - if (entries.size() > maxFileCount) { - errors.add("Too many files: " + entries.size() + " (max: " + maxFileCount + ")"); - } - - // 2. Validate paths and duplicates for (PackageEntry entry : entries) { - String normalizedPath = validateAndNormalizePath(entry.path(), errors); - if (normalizedPath != null && !seenPaths.add(normalizedPath)) { - errors.add("Duplicate file path: " + normalizedPath); + String normalizedPath; + try { + normalizedPath = SkillPackagePolicy.normalizeEntryPath(entry.path()); + } catch (IllegalArgumentException e) { + errors.add(e.getMessage()); + continue; + } + + if (!normalizedPaths.add(normalizedPath)) { + errors.add("Duplicate package entry path: " + normalizedPath); + } + + if (!hasAllowedExtension(normalizedPath)) { + errors.add("Disallowed file extension: " + normalizedPath); + } + + if (SkillPackagePolicy.SKILL_MD_PATH.equals(normalizedPath) && skillMd == null) { + skillMd = entry; } } - // 3. Check SKILL.md exists at root - PackageEntry skillMd = entries.stream() - .filter(e -> e.path().equals(SKILL_MD_PATH)) - .findFirst() - .orElse(null); - + // 1. Check SKILL.md exists at root if (skillMd == null) { errors.add("Missing required file: SKILL.md at root"); return ValidationResult.fail(errors); } - // 4. Validate frontmatter + // 2. Validate frontmatter try { String content = new String(skillMd.content()); metadataParser.parse(content); @@ -82,23 +84,19 @@ public class SkillPackageValidator { errors.add("Invalid SKILL.md frontmatter: " + detail); } - // 5. Check file extensions - for (PackageEntry entry : entries) { - String path = entry.path().toLowerCase(); - boolean hasAllowedExtension = allowedExtensions.stream().anyMatch(path::endsWith); - if (!hasAllowedExtension) { - errors.add("Disallowed file extension: " + path); - } + // 3. Check file count + if (entries.size() > maxFileCount) { + errors.add("Too many files: " + entries.size() + " (max: " + maxFileCount + ")"); } - // 6. Check single file size + // 4. Check single file size for (PackageEntry entry : entries) { if (entry.size() > maxSingleFileSize) { errors.add("File too large: " + entry.path() + " (" + entry.size() + " bytes, max: " + maxSingleFileSize + ")"); } } - // 7. Check total package size + // 5. Check total package size long totalSize = entries.stream().mapToLong(PackageEntry::size).sum(); if (totalSize > maxTotalPackageSize) { errors.add("Package too large: " + totalSize + " bytes (max: " + maxTotalPackageSize + ")"); @@ -107,35 +105,7 @@ public class SkillPackageValidator { return errors.isEmpty() ? ValidationResult.pass() : ValidationResult.fail(errors); } - private String validateAndNormalizePath(String path, List errors) { - if (path == null || path.isBlank()) { - errors.add("Package entry path must not be blank"); - return null; - } - if (path.contains("\\")) { - errors.add("Package entry must use '/' separators: " + path); - return null; - } - if (path.startsWith("/") || path.contains("//")) { - errors.add("Unsafe file path: " + path); - return null; - } - - try { - Path normalized = Path.of(path).normalize(); - String normalizedPath = normalized.toString().replace('\\', '/'); - if (normalized.isAbsolute() - || normalizedPath.isBlank() - || normalizedPath.equals(".") - || normalizedPath.equals("..") - || normalizedPath.startsWith("../")) { - errors.add("Unsafe file path: " + path); - return null; - } - return normalizedPath; - } catch (InvalidPathException ex) { - errors.add("Invalid file path: " + path); - return null; - } + private boolean hasAllowedExtension(String normalizedPath) { + return allowedExtensions.stream().anyMatch(normalizedPath::endsWith); } } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java index 71677aa3..b1a8ea76 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java @@ -2,7 +2,6 @@ package com.iflytek.skillhub.domain.review; import com.iflytek.skillhub.domain.event.SkillPublishedEvent; 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.NamespaceType; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; @@ -17,10 +16,10 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; -import jakarta.persistence.EntityManager; import java.util.*; +import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -35,7 +34,6 @@ class PromotionServiceTest { @Mock private NamespaceRepository namespaceRepository; @Mock private ReviewPermissionChecker permissionChecker; @Mock private ApplicationEventPublisher eventPublisher; - @Mock private EntityManager entityManager; private PromotionService promotionService; @@ -52,7 +50,7 @@ class PromotionServiceTest { void setUp() { promotionService = new PromotionService( promotionRequestRepository, skillRepository, skillVersionRepository, - skillFileRepository, namespaceRepository, permissionChecker, eventPublisher, entityManager); + skillFileRepository, namespaceRepository, permissionChecker, eventPublisher); } private static void setField(Object target, String fieldName, Object value) { @@ -123,7 +121,7 @@ class PromotionServiceTest { when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); - when(permissionChecker.canSubmitPromotion(eq(sourceSkill), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(globalNs)); when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) .thenReturn(Optional.empty()); @@ -135,8 +133,7 @@ class PromotionServiceTest { }); PromotionRequest result = promotionService.submitPromotion( - SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, - Map.of(5L, NamespaceRole.OWNER), Set.of()); + SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of()); assertNotNull(result); assertEquals(SOURCE_SKILL_ID, result.getSourceSkillId()); @@ -151,7 +148,7 @@ class PromotionServiceTest { when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -160,7 +157,7 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -173,7 +170,7 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sv)); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test @@ -186,42 +183,99 @@ class PromotionServiceTest { when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sv)); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenTargetNamespaceNotFound() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); - when(permissionChecker.canSubmitPromotion(any(), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenTargetNamespaceNotGlobal() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); - when(permissionChecker.canSubmitPromotion(any(), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(createTeamNamespace())); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenDuplicatePendingExists() { - when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill())); + Skill sourceSkill = createSourceSkill(); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion())); - when(permissionChecker.canSubmitPromotion(any(), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitPromotion(sourceSkill, USER_ID, Map.of())).thenReturn(true); when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(createGlobalNamespace())); when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) .thenReturn(Optional.of(createPendingPromotion())); assertThrows(DomainBadRequestException.class, - () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of(), Set.of())); + () -> promotionService.submitPromotion(SOURCE_SKILL_ID, SOURCE_VERSION_ID, TARGET_NAMESPACE_ID, USER_ID, Map.of())); + } + + @Test + void shouldThrowWhenSubmitterIsNotOwnerOrNamespaceAdmin() { + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(permissionChecker.canSubmitPromotion( + sourceSkill, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.MEMBER))) + .thenReturn(false); + + assertThrows(DomainForbiddenException.class, + () -> promotionService.submitPromotion( + SOURCE_SKILL_ID, + SOURCE_VERSION_ID, + TARGET_NAMESPACE_ID, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.MEMBER) + )); + verify(promotionRequestRepository, never()).save(any(PromotionRequest.class)); + } + + @Test + void shouldAllowNamespaceAdminToSubmitPromotionForForeignSkill() { + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + Namespace globalNs = createGlobalNamespace(); + + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(permissionChecker.canSubmitPromotion( + sourceSkill, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.ADMIN))) + .thenReturn(true); + when(namespaceRepository.findById(TARGET_NAMESPACE_ID)).thenReturn(Optional.of(globalNs)); + when(promotionRequestRepository.findBySourceVersionIdAndStatus(SOURCE_VERSION_ID, ReviewTaskStatus.PENDING)) + .thenReturn(Optional.empty()); + when(promotionRequestRepository.save(any(PromotionRequest.class))) + .thenAnswer(inv -> inv.getArgument(0)); + + PromotionRequest result = promotionService.submitPromotion( + SOURCE_SKILL_ID, + SOURCE_VERSION_ID, + TARGET_NAMESPACE_ID, + "user-999", + Map.of(sourceSkill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.ADMIN) + ); + + assertNotNull(result); } } @@ -240,9 +294,6 @@ class PromotionServiceTest { when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "LGTM", null, pr.getVersion())) .thenReturn(1); - when(promotionRequestRepository.updateStatusWithVersion( - PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "LGTM", NEW_SKILL_ID, pr.getVersion() + 1)) - .thenReturn(1); when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); when(skillRepository.save(any(Skill.class))).thenAnswer(inv -> { @@ -302,9 +353,8 @@ class PromotionServiceTest { assertEquals(NEW_VERSION_ID, event.versionId()); assertEquals(REVIEWER_ID, event.publisherId()); - verify(entityManager).detach(pr); - verify(promotionRequestRepository).updateStatusWithVersion( - PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "LGTM", NEW_SKILL_ID, pr.getVersion() + 1); + // Verify targetSkillId updated on promotion request + verify(promotionRequestRepository).save(pr); assertEquals(NEW_SKILL_ID, pr.getTargetSkillId()); } @@ -357,9 +407,6 @@ class PromotionServiceTest { when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion(any(), any(), any(), any(), any(), any())).thenReturn(1); - when(promotionRequestRepository.updateStatusWithVersion( - PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", NEW_SKILL_ID, pr.getVersion() + 1)) - .thenReturn(1); when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); when(skillRepository.save(any(Skill.class))).thenAnswer(inv -> { @@ -396,15 +443,14 @@ class PromotionServiceTest { when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.REJECTED, REVIEWER_ID, "Not ready", null, pr.getVersion())) .thenReturn(1); + when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + PromotionRequest result = promotionService.rejectPromotion( PROMOTION_ID, REVIEWER_ID, "Not ready", Set.of("SKILL_ADMIN")); assertNotNull(result); - assertEquals(ReviewTaskStatus.REJECTED, result.getStatus()); - assertEquals(REVIEWER_ID, result.getReviewedBy()); verify(promotionRequestRepository).updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.REJECTED, REVIEWER_ID, "Not ready", null, pr.getVersion()); - verify(entityManager).detach(pr); verify(eventPublisher, never()).publishEvent(any()); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java index ce6c094a..6ba7a4de 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewPermissionCheckerTest.java @@ -2,6 +2,8 @@ package com.iflytek.skillhub.domain.review; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; import org.junit.jupiter.api.Test; import java.util.Map; @@ -24,18 +26,18 @@ class ReviewPermissionCheckerTest { } @Test - void skillAdminCanReviewOwnSubmission() { + void skillAdminCannotReviewOwnSubmission() { String userId = "user-1"; ReviewTask task = new ReviewTask(1L, 10L, userId); - assertTrue(checker.canReview(task, userId, + assertFalse(checker.canReview(task, userId, NamespaceType.TEAM, Map.of(), Set.of("SKILL_ADMIN"))); } @Test - void superAdminCanReviewOwnSubmission() { + void superAdminCannotReviewOwnSubmission() { String userId = "user-1"; ReviewTask task = new ReviewTask(1L, 10L, userId); - assertTrue(checker.canReview(task, userId, + assertFalse(checker.canReview(task, userId, NamespaceType.TEAM, Map.of(), Set.of("SUPER_ADMIN"))); } @@ -105,6 +107,53 @@ class ReviewPermissionCheckerTest { // --- canReviewPromotion tests --- + @Test + void memberCanSubmitReview() { + assertTrue(checker.canSubmitReview(10L, Map.of(10L, NamespaceRole.MEMBER))); + } + + @Test + void outsiderCannotSubmitReview() { + assertFalse(checker.canSubmitReview(10L, Map.of())); + } + + @Test + void teamAdminCanManagePendingReviewList() { + assertTrue(checker.canManageNamespaceReviews( + 10L, NamespaceType.TEAM, Map.of(10L, NamespaceRole.ADMIN), Set.of())); + } + + @Test + void submitterCanReadOwnReview() { + ReviewTask task = new ReviewTask(1L, 10L, "user-1"); + assertTrue(checker.canReadReview(task, "user-1", + NamespaceType.TEAM, Map.of(), Set.of())); + } + + @Test + void ownerCanSubmitPromotion() { + Skill sourceSkill = new Skill(10L, "skill-a", "user-1", SkillVisibility.PUBLIC); + assertTrue(checker.canSubmitPromotion(sourceSkill, "user-1", Map.of())); + } + + @Test + void teamAdminCanSubmitPromotionForForeignSkill() { + Skill sourceSkill = new Skill(10L, "skill-a", "user-2", SkillVisibility.PUBLIC); + assertTrue(checker.canSubmitPromotion(sourceSkill, "user-1", + Map.of(10L, NamespaceRole.ADMIN))); + } + + @Test + void submitterCanReadOwnPromotion() { + PromotionRequest req = new PromotionRequest(1L, 1L, 1L, "user-1"); + assertTrue(checker.canReadPromotion(req, "user-1", Set.of())); + } + + @Test + void skillAdminCanListPendingPromotions() { + assertTrue(checker.canListPendingPromotions(Set.of("SKILL_ADMIN"))); + } + @Test void skillAdminCanReviewPromotion() { PromotionRequest req = new PromotionRequest(1L, 1L, 1L, "user-2"); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java index 786f6af2..fd6815ae 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.review; +import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.domain.event.SkillPublishedEvent; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; @@ -13,7 +14,7 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; import com.iflytek.skillhub.domain.skill.SkillVisibility; -import jakarta.persistence.EntityManager; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -42,7 +43,6 @@ class ReviewServiceTest { @Mock private NamespaceRepository namespaceRepository; @Mock private ReviewPermissionChecker permissionChecker; @Mock private ApplicationEventPublisher eventPublisher; - @Mock private EntityManager entityManager; private ReviewService reviewService; @@ -52,12 +52,14 @@ class ReviewServiceTest { private static final String REVIEWER_ID = "user-200"; private static final Long REVIEW_TASK_ID = 1L; private static final Long SKILL_ID = 30L; + private ObjectMapper objectMapper; @BeforeEach void setUp() { + objectMapper = new ObjectMapper(); reviewService = new ReviewService( reviewTaskRepository, skillVersionRepository, skillRepository, - namespaceRepository, permissionChecker, eventPublisher, entityManager); + namespaceRepository, permissionChecker, eventPublisher, objectMapper); } private SkillVersion createDraftSkillVersion() { @@ -109,11 +111,17 @@ class ReviewServiceTest { Skill skill = createSkill(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); - when(permissionChecker.canSubmitForReview(eq(skill), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitReview( + NAMESPACE_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))).thenReturn(true); ReviewTask savedTask = createPendingReviewTask(); when(reviewTaskRepository.save(any(ReviewTask.class))).thenReturn(savedTask); - ReviewTask result = reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(), Set.of()); + ReviewTask result = reviewService.submitReview( + SKILL_VERSION_ID, + USER_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER) + ); assertNotNull(result); assertEquals(SkillVersionStatus.PENDING_REVIEW, sv.getStatus()); @@ -126,19 +134,17 @@ class ReviewServiceTest { when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.empty()); assertThrows(DomainNotFoundException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(), Set.of())); + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of())); } @Test void shouldThrowWhenStatusNotDraft() { SkillVersion sv = createPendingReviewSkillVersion(); - Skill skill = createSkill(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); - when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); - when(permissionChecker.canSubmitForReview(eq(skill), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(createSkill())); assertThrows(DomainBadRequestException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(), Set.of())); + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))); } @Test @@ -147,12 +153,31 @@ class ReviewServiceTest { Skill skill = createSkill(); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); - when(permissionChecker.canSubmitForReview(eq(skill), eq(USER_ID), anyMap(), anySet())).thenReturn(true); + when(permissionChecker.canSubmitReview( + NAMESPACE_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER))).thenReturn(true); when(reviewTaskRepository.save(any(ReviewTask.class))) .thenThrow(new DataIntegrityViolationException("duplicate")); assertThrows(DomainBadRequestException.class, - () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of(), Set.of())); + () -> reviewService.submitReview( + SKILL_VERSION_ID, + USER_ID, + Map.of(NAMESPACE_ID, NamespaceRole.MEMBER) + )); + } + + @Test + void shouldThrowWhenSubmitterLacksNamespaceMembership() { + SkillVersion sv = createDraftSkillVersion(); + Skill skill = createSkill(); + when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(permissionChecker.canSubmitReview(NAMESPACE_ID, Map.of())).thenReturn(false); + + assertThrows(DomainForbiddenException.class, + () -> reviewService.submitReview(SKILL_VERSION_ID, USER_ID, Map.of())); + verify(reviewTaskRepository, never()).save(any(ReviewTask.class)); } } @@ -165,6 +190,12 @@ class ReviewServiceTest { Namespace ns = createTeamNamespace(); SkillVersion sv = createPendingReviewSkillVersion(); Skill skill = createSkill(); + skill.setDisplayName("Published Name"); + skill.setSummary("Published Summary"); + skill.setUpdatedBy("previous-reviewer"); + assertDoesNotThrow(() -> sv.setParsedMetadataJson(objectMapper.writeValueAsString( + new SkillMetadata("Approved Name", "Approved Summary", "1.0.0", "Body", Map.of()) + ))); when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(ns)); @@ -175,19 +206,19 @@ class ReviewServiceTest { .thenReturn(1); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); ReviewTask result = reviewService.approveReview( REVIEW_TASK_ID, REVIEWER_ID, "LGTM", Map.of(NAMESPACE_ID, NamespaceRole.ADMIN), Set.of()); assertNotNull(result); - assertEquals(ReviewTaskStatus.APPROVED, result.getStatus()); - assertEquals(REVIEWER_ID, result.getReviewedBy()); - assertEquals("LGTM", result.getReviewComment()); - assertNotNull(result.getReviewedAt()); assertEquals(SkillVersionStatus.PUBLISHED, sv.getStatus()); assertNotNull(sv.getPublishedAt()); assertEquals(SKILL_VERSION_ID, skill.getLatestVersionId()); + assertEquals("Approved Name", skill.getDisplayName()); + assertEquals("Approved Summary", skill.getSummary()); + assertEquals(REVIEWER_ID, skill.getUpdatedBy()); verify(eventPublisher).publishEvent(any(SkillPublishedEvent.class)); } @@ -204,6 +235,7 @@ class ReviewServiceTest { when(reviewTaskRepository.updateStatusWithVersion(any(), any(), any(), any(), any())).thenReturn(1); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); reviewService.approveReview(REVIEW_TASK_ID, REVIEWER_ID, "ok", Map.of(NAMESPACE_ID, NamespaceRole.ADMIN), Set.of()); @@ -275,16 +307,13 @@ class ReviewServiceTest { when(permissionChecker.canReview(any(), any(), any(), anyMap(), anySet())).thenReturn(true); when(reviewTaskRepository.updateStatusWithVersion(any(), any(), any(), any(), any())).thenReturn(1); when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); ReviewTask result = reviewService.rejectReview( REVIEW_TASK_ID, REVIEWER_ID, "needs work", Map.of(NAMESPACE_ID, NamespaceRole.ADMIN), Set.of()); assertNotNull(result); - assertEquals(ReviewTaskStatus.REJECTED, result.getStatus()); - assertEquals(REVIEWER_ID, result.getReviewedBy()); - assertEquals("needs work", result.getReviewComment()); - assertNotNull(result.getReviewedAt()); assertEquals(SkillVersionStatus.REJECTED, sv.getStatus()); verify(skillVersionRepository).save(sv); verify(eventPublisher, never()).publishEvent(any(SkillPublishedEvent.class)); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java index bd0a6542..c36f4f42 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java @@ -176,4 +176,49 @@ class SkillPackageValidatorTest { assertFalse(result.passed()); assertTrue(result.errors().stream().anyMatch(e -> e.contains("Package too large"))); } + + @Test + void testPathTraversalEntryRejected() { + String skillMdContent = """ + --- + name: test-skill + description: A test skill + version: 1.0.0 + --- + Body + """; + + List entries = List.of( + new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"), + new PackageEntry("../secrets.txt", "hidden".getBytes(), 6, "text/plain") + ); + + ValidationResult result = validator.validate(entries); + + assertFalse(result.passed()); + assertTrue(result.errors().stream().anyMatch(e -> e.contains("escapes package root"))); + } + + @Test + void testDuplicateNormalizedPathRejected() { + String skillMdContent = """ + --- + name: test-skill + description: A test skill + version: 1.0.0 + --- + Body + """; + + List entries = List.of( + new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"), + new PackageEntry("docs\\guide.md", "first".getBytes(), 5, "text/markdown"), + new PackageEntry("docs/guide.md", "second".getBytes(), 6, "text/markdown") + ); + + ValidationResult result = validator.validate(entries); + + assertFalse(result.passed()); + assertTrue(result.errors().stream().anyMatch(e -> e.contains("Duplicate package entry path: docs/guide.md"))); + } } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/UserAccountJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/UserAccountJpaRepository.java index d5a907be..8fbb45cc 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/UserAccountJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/UserAccountJpaRepository.java @@ -6,13 +6,14 @@ import com.iflytek.skillhub.domain.user.UserStatus; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; @Repository public interface UserAccountJpaRepository - extends JpaRepository, UserAccountRepository { + extends JpaRepository, JpaSpecificationExecutor, UserAccountRepository { @Override @Query(""" diff --git a/server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java b/server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java index 34064c88..fc03d788 100644 --- a/server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java +++ b/server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java @@ -66,7 +66,7 @@ public class LocalFileStorageService implements ObjectStorageService { private Path resolve(String key) { Path resolved = basePath.resolve(key).normalize(); if (!resolved.startsWith(basePath)) { - throw new IllegalArgumentException("Resolved path escapes storage base path: " + key); + throw new IllegalArgumentException("Invalid storage key: " + key); } return resolved; } diff --git a/server/skillhub-storage/src/test/java/com/iflytek/skillhub/storage/LocalFileStorageServiceTest.java b/server/skillhub-storage/src/test/java/com/iflytek/skillhub/storage/LocalFileStorageServiceTest.java index 088845ce..1c29af9f 100644 --- a/server/skillhub-storage/src/test/java/com/iflytek/skillhub/storage/LocalFileStorageServiceTest.java +++ b/server/skillhub-storage/src/test/java/com/iflytek/skillhub/storage/LocalFileStorageServiceTest.java @@ -1,24 +1,110 @@ package com.iflytek.skillhub.storage; -import static org.assertj.core.api.Assertions.assertThat; - -import java.nio.file.Files; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + class LocalFileStorageServiceTest { @TempDir - java.nio.file.Path tempDir; + Path tempDir; + + private LocalFileStorageService storageService; + + @BeforeEach + void setUp() { + StorageProperties props = new StorageProperties(); + props.getLocal().setBasePath(tempDir.toString()); + storageService = new LocalFileStorageService(props); + } @Test - void generatePresignedUrl_returnsNullForLocalStorage() throws Exception { + void shouldPutAndGetObject() throws Exception { + String key = "skills/1/1/SKILL.md"; + byte[] content = "# Hello".getBytes(StandardCharsets.UTF_8); + storageService.putObject(key, new ByteArrayInputStream(content), content.length, "text/markdown"); + try (InputStream result = storageService.getObject(key)) { + assertArrayEquals(content, result.readAllBytes()); + } + } + + @Test + void shouldCheckExistence() { + assertFalse(storageService.exists("test/exists.txt")); + byte[] content = "data".getBytes(StandardCharsets.UTF_8); + storageService.putObject("test/exists.txt", new ByteArrayInputStream(content), content.length, "text/plain"); + assertTrue(storageService.exists("test/exists.txt")); + } + + @Test + void shouldDeleteObject() { + byte[] content = "data".getBytes(StandardCharsets.UTF_8); + storageService.putObject("test/delete.txt", new ByteArrayInputStream(content), content.length, "text/plain"); + assertTrue(storageService.exists("test/delete.txt")); + storageService.deleteObject("test/delete.txt"); + assertFalse(storageService.exists("test/delete.txt")); + } + + @Test + void shouldDeleteMultipleObjects() { + byte[] content = "data".getBytes(StandardCharsets.UTF_8); + storageService.putObject("a/1.txt", new ByteArrayInputStream(content), content.length, "text/plain"); + storageService.putObject("a/2.txt", new ByteArrayInputStream(content), content.length, "text/plain"); + storageService.deleteObjects(List.of("a/1.txt", "a/2.txt")); + assertFalse(storageService.exists("a/1.txt")); + assertFalse(storageService.exists("a/2.txt")); + } + + @Test + void shouldGetMetadata() { + byte[] content = "hello world".getBytes(StandardCharsets.UTF_8); + storageService.putObject("test/meta.txt", new ByteArrayInputStream(content), content.length, "text/plain"); + ObjectMetadata metadata = storageService.getMetadata("test/meta.txt"); + assertEquals(content.length, metadata.size()); + assertNotNull(metadata.lastModified()); + } + + @Test + void shouldRejectPathTraversalKeys() { + byte[] content = "data".getBytes(StandardCharsets.UTF_8); + + IllegalArgumentException putError = assertThrows( + IllegalArgumentException.class, + () -> storageService.putObject("../escape.txt", new ByteArrayInputStream(content), content.length, "text/plain") + ); + assertEquals("Invalid storage key: ../escape.txt", putError.getMessage()); + + IllegalArgumentException getError = assertThrows( + IllegalArgumentException.class, + () -> storageService.getObject("..\\escape.txt") + ); + assertEquals("Invalid storage key: ..\\escape.txt", getError.getMessage()); + } + + @Test + void generatePresignedUrlReturnsNullForLocalStorage() throws Exception { StorageProperties properties = new StorageProperties(); properties.getLocal().setBasePath(tempDir.toString()); Files.createDirectories(tempDir); LocalFileStorageService service = new LocalFileStorageService(properties); - assertThat(service.generatePresignedUrl("packages/demo.zip", java.time.Duration.ofMinutes(10))).isNull(); + assertThat(service.generatePresignedUrl("packages/demo.zip", Duration.ofMinutes(10))).isNull(); } } diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 946155c7..56e82e46 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -1,44 +1,77 @@ -import { lazy, type ComponentType } from 'react' +import { lazy, Suspense, type ComponentType } from 'react' import { createRouter, createRoute, createRootRoute, redirect } from '@tanstack/react-router' import { Layout } from './layout' import { getCurrentUser } from '@/api/client' -function lazyRouteComponent>( +function createLazyRouteComponent>( importer: () => Promise, exportName: keyof TModule, ) { const LazyComponent = lazy(async () => { const module = await importer() - return { default: module[exportName] as ComponentType } + return { default: module[exportName] as ComponentType } }) - return LazyComponent + return function LazyRouteComponent(props: Record) { + return ( + + Loading... + + } + > + + + ) + } } -const HomePage = lazyRouteComponent(() => import('@/pages/home'), 'HomePage') -const LoginPage = lazyRouteComponent(() => import('@/pages/login'), 'LoginPage') -const RegisterPage = lazyRouteComponent(() => import('@/pages/register'), 'RegisterPage') -const PrivacyPolicyPage = lazyRouteComponent(() => import('@/pages/privacy'), 'PrivacyPolicyPage') -const SearchPage = lazyRouteComponent(() => import('@/pages/search'), 'SearchPage') -const TermsOfServicePage = lazyRouteComponent(() => import('@/pages/terms'), 'TermsOfServicePage') -const NamespacePage = lazyRouteComponent(() => import('@/pages/namespace'), 'NamespacePage') -const SkillDetailPage = lazyRouteComponent(() => import('@/pages/skill-detail'), 'SkillDetailPage') -const DashboardPage = lazyRouteComponent(() => import('@/pages/dashboard'), 'DashboardPage') -const MySkillsPage = lazyRouteComponent(() => import('@/pages/dashboard/my-skills'), 'MySkillsPage') -const PublishPage = lazyRouteComponent(() => import('@/pages/dashboard/publish'), 'PublishPage') -const MyNamespacesPage = lazyRouteComponent(() => import('@/pages/dashboard/my-namespaces'), 'MyNamespacesPage') -const NamespaceMembersPage = lazyRouteComponent(() => import('@/pages/dashboard/namespace-members'), 'NamespaceMembersPage') -const NamespaceReviewsPage = lazyRouteComponent(() => import('@/pages/dashboard/namespace-reviews'), 'NamespaceReviewsPage') -const ReviewsPage = lazyRouteComponent(() => import('@/pages/dashboard/reviews'), 'ReviewsPage') -const ReviewDetailPage = lazyRouteComponent(() => import('@/pages/dashboard/review-detail'), 'ReviewDetailPage') -const PromotionsPage = lazyRouteComponent(() => import('@/pages/dashboard/promotions'), 'PromotionsPage') -const MyStarsPage = lazyRouteComponent(() => import('@/pages/dashboard/stars'), 'MyStarsPage') -const TokensPage = lazyRouteComponent(() => import('@/pages/dashboard/tokens'), 'TokensPage') -const DeviceAuthPage = lazyRouteComponent(() => import('@/pages/device'), 'DeviceAuthPage') -const SecuritySettingsPage = lazyRouteComponent(() => import('@/pages/settings/security'), 'SecuritySettingsPage') -const AccountSettingsPage = lazyRouteComponent(() => import('@/pages/settings/accounts'), 'AccountSettingsPage') -const AdminUsersPage = lazyRouteComponent(() => import('@/pages/admin/users'), 'AdminUsersPage') -const AuditLogPage = lazyRouteComponent(() => import('@/pages/admin/audit-log'), 'AuditLogPage') +const HomePage = createLazyRouteComponent(() => import('@/pages/home'), 'HomePage') +const LoginPage = createLazyRouteComponent(() => import('@/pages/login'), 'LoginPage') +const RegisterPage = createLazyRouteComponent(() => import('@/pages/register'), 'RegisterPage') +const PrivacyPolicyPage = createLazyRouteComponent(() => import('@/pages/privacy'), 'PrivacyPolicyPage') +const SearchPage = createLazyRouteComponent(() => import('@/pages/search'), 'SearchPage') +const TermsOfServicePage = createLazyRouteComponent(() => import('@/pages/terms'), 'TermsOfServicePage') +const NamespacePage = createLazyRouteComponent(() => import('@/pages/namespace'), 'NamespacePage') +const SkillDetailPage = createLazyRouteComponent(() => import('@/pages/skill-detail'), 'SkillDetailPage') +const DashboardPage = createLazyRouteComponent(() => import('@/pages/dashboard'), 'DashboardPage') +const MySkillsPage = createLazyRouteComponent(() => import('@/pages/dashboard/my-skills'), 'MySkillsPage') +const PublishPage = createLazyRouteComponent(() => import('@/pages/dashboard/publish'), 'PublishPage') +const MyNamespacesPage = createLazyRouteComponent( + () => import('@/pages/dashboard/my-namespaces'), + 'MyNamespacesPage', +) +const NamespaceMembersPage = createLazyRouteComponent( + () => import('@/pages/dashboard/namespace-members'), + 'NamespaceMembersPage', +) +const NamespaceReviewsPage = createLazyRouteComponent( + () => import('@/pages/dashboard/namespace-reviews'), + 'NamespaceReviewsPage', +) +const ReviewsPage = createLazyRouteComponent(() => import('@/pages/dashboard/reviews'), 'ReviewsPage') +const ReviewDetailPage = createLazyRouteComponent( + () => import('@/pages/dashboard/review-detail'), + 'ReviewDetailPage', +) +const PromotionsPage = createLazyRouteComponent( + () => import('@/pages/dashboard/promotions'), + 'PromotionsPage', +) +const MyStarsPage = createLazyRouteComponent(() => import('@/pages/dashboard/stars'), 'MyStarsPage') +const TokensPage = createLazyRouteComponent(() => import('@/pages/dashboard/tokens'), 'TokensPage') +const DeviceAuthPage = createLazyRouteComponent(() => import('@/pages/device'), 'DeviceAuthPage') +const SecuritySettingsPage = createLazyRouteComponent( + () => import('@/pages/settings/security'), + 'SecuritySettingsPage', +) +const AccountSettingsPage = createLazyRouteComponent( + () => import('@/pages/settings/accounts'), + 'AccountSettingsPage', +) +const AdminUsersPage = createLazyRouteComponent(() => import('@/pages/admin/users'), 'AdminUsersPage') +const AuditLogPage = createLazyRouteComponent(() => import('@/pages/admin/audit-log'), 'AuditLogPage') const rootRoute = createRootRoute({ component: Layout, diff --git a/web/src/features/skill/markdown-renderer.tsx b/web/src/features/skill/markdown-renderer.tsx index 53227547..6b164983 100644 --- a/web/src/features/skill/markdown-renderer.tsx +++ b/web/src/features/skill/markdown-renderer.tsx @@ -2,51 +2,21 @@ import ReactMarkdown from 'react-markdown' import rehypeHighlight from 'rehype-highlight' import rehypeSanitize from 'rehype-sanitize' import remarkGfm from 'remark-gfm' -import { cn } from '@/shared/lib/utils' interface MarkdownRendererProps { content: string className?: string } -function stripFrontmatter(content: string) { - return content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n?/, '') -} - export function MarkdownRenderer({ content, className }: MarkdownRendererProps) { - const markdown = stripFrontmatter(content).trim() + const containerClassName = [className, 'prose prose-sm max-w-none dark:prose-invert'] + .filter(Boolean) + .join(' ') return ( -
- - {markdown} +
+ + {content}
)