diff --git a/docs/17-backend-annotation-findings.md b/docs/17-backend-annotation-findings.md index b4f88640..30986a43 100644 --- a/docs/17-backend-annotation-findings.md +++ b/docs/17-backend-annotation-findings.md @@ -2,8 +2,31 @@ This document records architecture and structure issues that became consistently visible while enriching backend comments. The goal is to preserve concrete observations discovered during code reading, not to propose a full redesign. +## Status Update (2026-03-19) + +This document was re-checked after the refactor branch work for findings 1, 2, and 4. + +- Finding 1 is now handled in code. +- Finding 2 is partially handled in code. +- Finding 4 is now handled in code. + +Validation completed on the standard regression path: + +- `make test` +- backend Maven tests: `208` passed +- frontend Vitest tests: `61` passed + +Double-check notes: + +- The admin-user refactor removed an overlapping, unused application service rather than changing the controller-facing workflow owner. +- The namespace and skill-lifecycle refactors moved orchestration out of controllers, but preserved the same downstream domain-service calls, request parameters, audit fields, response message keys, and mutation response shapes. +- The security refactor centralized route metadata into one registry, but preserved the same route authorization rules, API-token scope behavior, and CSRF-ignore behavior. +- `AuthContextFilter` is now scoped to API paths when projecting request attributes. This narrows unnecessary work on non-API requests, but it does not change existing business behavior because `userId` and `userNsRoles` consumers are API-side controllers and interceptors. + ## 1. Admin user management is split across overlapping application services +Status: handled on branch `docs/backend-annotation-findings-discussion` + Observed files: - `server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java` @@ -19,8 +42,16 @@ Suggested direction: - Either consolidate them into one application service, or split them with an explicit boundary such as query vs. command, or account governance vs. account operations. +Current state: + +- `AdminUserManagementService` has been removed. +- `UserManagementController` continues to use `AdminUserAppService` as the single application-service entry point. +- Behavior review found no business-logic drift here because the deleted service had no active controller call path. + ## 2. Several controllers still perform orchestration that belongs in application services +Status: partially handled on branch `docs/backend-annotation-findings-discussion` + Observed files: - `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java` @@ -39,6 +70,13 @@ Suggested direction: - Move multi-step orchestration into dedicated application services and keep controllers focused on transport concerns. +Current state: + +- `NamespaceController` has been slimmed down by moving orchestration into `NamespacePortalQueryAppService` and `NamespacePortalCommandAppService`. +- `SkillLifecycleController` has been slimmed down by moving orchestration into `SkillLifecycleAppService`. +- This branch preserved the original domain-service calls and response contracts for the refactored endpoints. +- `ReviewController`, `PromotionController`, and `ClawHubCompatController` still exhibit the same structural issue and remain future work. + ## 3. Compatibility endpoints are tightly coupled to canonical domain and repository internals Observed files: @@ -58,6 +96,8 @@ Suggested direction: ## 4. Security route policy is spread across configuration and implementation classes +Status: handled on branch `docs/backend-annotation-findings-discussion` + Observed files: - `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java` @@ -73,6 +113,12 @@ Suggested direction: - Centralize route policy metadata or at least define one authoritative mapping between path patterns, authentication modes, and scope requirements. +Current state: + +- Route metadata is now centralized in `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java`. +- `SecurityConfig`, `ApiTokenScopeService`, and `AuthContextFilter` now depend on that shared registry instead of maintaining separate route lists. +- Double-check review confirmed that the refactor preserved the previous access model while removing duplication. + ## 5. Governance behavior is distributed across multiple services without one clear workflow owner Observed files: diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java index 84e2d297..c6f5b267 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java @@ -2,19 +2,29 @@ package com.iflytek.skillhub.controller.portal; import com.iflytek.skillhub.controller.BaseApiController; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; -import com.iflytek.skillhub.domain.namespace.*; -import com.iflytek.skillhub.dto.*; -import com.iflytek.skillhub.exception.ForbiddenException; -import com.iflytek.skillhub.exception.UnauthorizedException; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.MemberRequest; +import com.iflytek.skillhub.dto.MemberResponse; +import com.iflytek.skillhub.dto.MessageResponse; +import com.iflytek.skillhub.dto.MyNamespaceResponse; +import com.iflytek.skillhub.dto.NamespaceCandidateUserResponse; +import com.iflytek.skillhub.dto.NamespaceLifecycleRequest; +import com.iflytek.skillhub.dto.NamespaceRequest; +import com.iflytek.skillhub.dto.NamespaceResponse; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.UpdateMemberRoleRequest; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.NamespacePortalCommandAppService; +import com.iflytek.skillhub.service.NamespacePortalQueryAppService; import com.iflytek.skillhub.service.NamespaceMemberCandidateService; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; -import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.*; -import java.util.Comparator; import java.util.List; import java.util.Map; @@ -26,84 +36,46 @@ import java.util.Map; @RequestMapping({"/api/v1", "/api/web"}) public class NamespaceController extends BaseApiController { - private final NamespaceService namespaceService; - private final NamespaceMemberService namespaceMemberService; - private final NamespaceRepository namespaceRepository; - private final NamespaceGovernanceService namespaceGovernanceService; - private final NamespaceAccessPolicy namespaceAccessPolicy; + private final NamespacePortalQueryAppService namespacePortalQueryAppService; + private final NamespacePortalCommandAppService namespacePortalCommandAppService; private final NamespaceMemberCandidateService namespaceMemberCandidateService; - public NamespaceController(NamespaceService namespaceService, - NamespaceMemberService namespaceMemberService, - NamespaceRepository namespaceRepository, - NamespaceGovernanceService namespaceGovernanceService, - NamespaceAccessPolicy namespaceAccessPolicy, - NamespaceMemberCandidateService namespaceMemberCandidateService, - ApiResponseFactory responseFactory) { + public NamespaceController(NamespacePortalQueryAppService namespacePortalQueryAppService, + NamespacePortalCommandAppService namespacePortalCommandAppService, + NamespaceMemberCandidateService namespaceMemberCandidateService, + ApiResponseFactory responseFactory) { super(responseFactory); - this.namespaceService = namespaceService; - this.namespaceMemberService = namespaceMemberService; - this.namespaceRepository = namespaceRepository; - this.namespaceGovernanceService = namespaceGovernanceService; - this.namespaceAccessPolicy = namespaceAccessPolicy; + this.namespacePortalQueryAppService = namespacePortalQueryAppService; + this.namespacePortalCommandAppService = namespacePortalCommandAppService; this.namespaceMemberCandidateService = namespaceMemberCandidateService; } @GetMapping("/namespaces") public ApiResponse> listNamespaces(Pageable pageable) { - Page namespaces = namespaceRepository.findByStatus(NamespaceStatus.ACTIVE, pageable); - PageResponse response = PageResponse.from(namespaces.map(NamespaceResponse::from)); - return ok("response.success.read", response); + return ok("response.success.read", namespacePortalQueryAppService.listNamespaces(pageable)); } @GetMapping("/me/namespaces") public ApiResponse> listMyNamespaces( @RequestAttribute("userId") String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { - Map namespaceRoles = userNsRoles != null ? userNsRoles : Map.of(); - if (namespaceRoles.isEmpty()) { - return ok("response.success.read", List.of()); - } - - List response = namespaceRepository.findByIdIn(namespaceRoles.keySet().stream().toList()).stream() - .sorted(Comparator.comparing(Namespace::getSlug)) - .map(namespace -> MyNamespaceResponse.from(namespace, namespaceRoles.get(namespace.getId()), namespaceAccessPolicy)) - .toList(); - - return ok("response.success.read", response); + return ok("response.success.read", namespacePortalQueryAppService.listMyNamespaces(userNsRoles)); } @GetMapping("/namespaces/{slug}") public ApiResponse getNamespace(@PathVariable String slug, @RequestAttribute(value = "userId", required = false) String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { - Namespace namespace = namespaceService.getNamespaceBySlugForRead(slug, userId, userNsRoles != null ? userNsRoles : Map.of()); - return ok("response.success.read", NamespaceResponse.from(namespace)); + return ok("response.success.read", + namespacePortalQueryAppService.getNamespace(slug, userId, userNsRoles)); } @PostMapping("/namespaces") public ApiResponse createNamespace( @Valid @RequestBody NamespaceRequest request, @AuthenticationPrincipal PlatformPrincipal principal) { - if (principal == null) { - throw new UnauthorizedException("error.auth.required"); - } - if (!canCreateNamespace(principal)) { - throw new ForbiddenException("error.namespace.create.platformAdminRequired"); - } - - Namespace namespace = namespaceService.createNamespace( - request.slug(), - request.displayName(), - request.description(), - principal.userId() - ); - return ok("response.success.created", NamespaceResponse.from(namespace)); - } - - private boolean canCreateNamespace(PlatformPrincipal principal) { - return principal.platformRoles().contains("SKILL_ADMIN") - || principal.platformRoles().contains("SUPER_ADMIN"); + return ok("response.success.created", + namespacePortalCommandAppService.createNamespace(request, principal)); } @PutMapping("/namespaces/{slug}") @@ -111,15 +83,8 @@ public class NamespaceController extends BaseApiController { @PathVariable String slug, @RequestBody NamespaceRequest request, @RequestAttribute("userId") String userId) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); - Namespace updated = namespaceService.updateNamespace( - namespace.getId(), - request.displayName(), - request.description(), - null, - userId - ); - return ok("response.success.updated", NamespaceResponse.from(updated)); + return ok("response.success.updated", + namespacePortalCommandAppService.updateNamespace(slug, request, userId)); } @PostMapping("/namespaces/{slug}/freeze") @@ -127,29 +92,23 @@ public class NamespaceController extends BaseApiController { @RequestBody(required = false) NamespaceLifecycleRequest request, @RequestAttribute("userId") String userId, HttpServletRequest httpRequest) { - Namespace namespace = namespaceGovernanceService.freezeNamespace( - slug, - userId, - request != null ? request.reason() : null, - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.updated", NamespaceResponse.from(namespace)); + return ok("response.success.updated", + namespacePortalCommandAppService.freezeNamespace( + slug, + request, + userId, + AuditRequestContext.from(httpRequest))); } @PostMapping("/namespaces/{slug}/unfreeze") public ApiResponse unfreezeNamespace(@PathVariable String slug, @RequestAttribute("userId") String userId, HttpServletRequest httpRequest) { - Namespace namespace = namespaceGovernanceService.unfreezeNamespace( - slug, - userId, - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.updated", NamespaceResponse.from(namespace)); + return ok("response.success.updated", + namespacePortalCommandAppService.unfreezeNamespace( + slug, + userId, + AuditRequestContext.from(httpRequest))); } @PostMapping("/namespaces/{slug}/archive") @@ -157,40 +116,31 @@ public class NamespaceController extends BaseApiController { @RequestBody(required = false) NamespaceLifecycleRequest request, @RequestAttribute("userId") String userId, HttpServletRequest httpRequest) { - Namespace namespace = namespaceGovernanceService.archiveNamespace( - slug, - userId, - request != null ? request.reason() : null, - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.updated", NamespaceResponse.from(namespace)); + return ok("response.success.updated", + namespacePortalCommandAppService.archiveNamespace( + slug, + request, + userId, + AuditRequestContext.from(httpRequest))); } @PostMapping("/namespaces/{slug}/restore") public ApiResponse restoreNamespace(@PathVariable String slug, @RequestAttribute("userId") String userId, HttpServletRequest httpRequest) { - Namespace namespace = namespaceGovernanceService.restoreNamespace( - slug, - userId, - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.updated", NamespaceResponse.from(namespace)); + return ok("response.success.updated", + namespacePortalCommandAppService.restoreNamespace( + slug, + userId, + AuditRequestContext.from(httpRequest))); } @GetMapping("/namespaces/{slug}/members") public ApiResponse> listMembers(@PathVariable String slug, Pageable pageable, @RequestAttribute("userId") String userId) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); - namespaceService.assertMember(namespace.getId(), userId); - Page members = namespaceMemberService.listMembers(namespace.getId(), pageable); - PageResponse response = PageResponse.from(members.map(MemberResponse::from)); - return ok("response.success.read", response); + return ok("response.success.read", + namespacePortalQueryAppService.listMembers(slug, pageable, userId)); } @GetMapping("/namespaces/{slug}/member-candidates") @@ -207,14 +157,8 @@ public class NamespaceController extends BaseApiController { @PathVariable String slug, @Valid @RequestBody MemberRequest request, @RequestAttribute("userId") String userId) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); - NamespaceMember member = namespaceMemberService.addMember( - namespace.getId(), - request.userId(), - request.role(), - userId - ); - return ok("response.success.created", MemberResponse.from(member)); + return ok("response.success.created", + namespacePortalCommandAppService.addMember(slug, request.userId(), request.role(), userId)); } @DeleteMapping("/namespaces/{slug}/members/{userId}") @@ -222,9 +166,8 @@ public class NamespaceController extends BaseApiController { @PathVariable String slug, @PathVariable("userId") String memberUserId, @RequestAttribute("userId") String operatorUserId) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); - namespaceMemberService.removeMember(namespace.getId(), memberUserId, operatorUserId); - return ok("response.success.deleted", new MessageResponse("Member removed successfully")); + return ok("response.success.deleted", + namespacePortalCommandAppService.removeMember(slug, memberUserId, operatorUserId)); } @PutMapping("/namespaces/{slug}/members/{userId}/role") @@ -233,13 +176,7 @@ public class NamespaceController extends BaseApiController { @PathVariable String userId, @Valid @RequestBody UpdateMemberRoleRequest request, @RequestAttribute("userId") String operatorUserId) { - Namespace namespace = namespaceService.getNamespaceBySlug(slug); - NamespaceMember member = namespaceMemberService.updateMemberRole( - namespace.getId(), - userId, - request.role(), - operatorUserId - ); - return ok("response.success.updated", MemberResponse.from(member)); + return ok("response.success.updated", + namespacePortalCommandAppService.updateMemberRole(slug, userId, request, operatorUserId)); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java index 5bee8282..4f07ffd8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java @@ -1,23 +1,14 @@ package com.iflytek.skillhub.controller.portal; import com.iflytek.skillhub.controller.BaseApiController; -import com.iflytek.skillhub.domain.audit.AuditLogService; -import com.iflytek.skillhub.domain.namespace.Namespace; -import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.review.ReviewService; -import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; -import com.iflytek.skillhub.domain.skill.Skill; -import com.iflytek.skillhub.domain.skill.SkillVersion; -import com.iflytek.skillhub.domain.skill.SkillVersionRepository; -import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; -import com.iflytek.skillhub.domain.skill.service.SkillPublishService; -import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.AdminSkillActionRequest; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; import com.iflytek.skillhub.dto.SkillVersionRereleaseRequest; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillLifecycleAppService; import jakarta.validation.Valid; import jakarta.servlet.http.HttpServletRequest; import java.util.Map; @@ -37,30 +28,12 @@ import org.springframework.web.bind.annotation.RestController; @RequestMapping({"/api/v1/skills", "/api/web/skills"}) public class SkillLifecycleController extends BaseApiController { - private final NamespaceRepository namespaceRepository; - private final SkillVersionRepository skillVersionRepository; - private final SkillGovernanceService skillGovernanceService; - private final ReviewService reviewService; - private final SkillPublishService skillPublishService; - private final AuditLogService auditLogService; - private final SkillSlugResolutionService skillSlugResolutionService; + private final SkillLifecycleAppService skillLifecycleAppService; - public SkillLifecycleController(NamespaceRepository namespaceRepository, - SkillVersionRepository skillVersionRepository, - SkillGovernanceService skillGovernanceService, - ReviewService reviewService, - SkillPublishService skillPublishService, - AuditLogService auditLogService, - SkillSlugResolutionService skillSlugResolutionService, + public SkillLifecycleController(SkillLifecycleAppService skillLifecycleAppService, ApiResponseFactory responseFactory) { super(responseFactory); - this.namespaceRepository = namespaceRepository; - this.skillVersionRepository = skillVersionRepository; - this.skillGovernanceService = skillGovernanceService; - this.reviewService = reviewService; - this.skillPublishService = skillPublishService; - this.auditLogService = auditLogService; - this.skillSlugResolutionService = skillSlugResolutionService; + this.skillLifecycleAppService = skillLifecycleAppService; } @PostMapping("/{namespace}/{slug}/archive") @@ -70,18 +43,14 @@ public class SkillLifecycleController extends BaseApiController { @RequestAttribute("userId") String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, HttpServletRequest httpRequest) { - Skill skill = findSkill(namespace, slug, userId); - Skill archived = skillGovernanceService.archiveSkill( - skill.getId(), - userId, - userNsRoles != null ? userNsRoles : Map.of(), - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent"), - request != null ? request.reason() : null - ); - return ok("response.success.updated", - new SkillLifecycleMutationResponse(archived.getId(), null, "ARCHIVE", archived.getStatus().name())); + skillLifecycleAppService.archiveSkill( + namespace, + slug, + request, + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); } @PostMapping("/{namespace}/{slug}/unarchive") @@ -90,17 +59,13 @@ public class SkillLifecycleController extends BaseApiController { @RequestAttribute("userId") String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, HttpServletRequest httpRequest) { - Skill skill = findSkill(namespace, slug, userId); - Skill restored = skillGovernanceService.unarchiveSkill( - skill.getId(), - userId, - userNsRoles != null ? userNsRoles : Map.of(), - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.updated", - new SkillLifecycleMutationResponse(restored.getId(), null, "UNARCHIVE", restored.getStatus().name())); + skillLifecycleAppService.unarchiveSkill( + namespace, + slug, + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); } @DeleteMapping("/{namespace}/{slug}/versions/{version}") @@ -110,20 +75,14 @@ public class SkillLifecycleController extends BaseApiController { @RequestAttribute("userId") String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, HttpServletRequest httpRequest) { - Skill skill = findSkill(namespace, slug, userId); - SkillVersion skillVersion = skillVersionRepository.findBySkillIdAndVersion(skill.getId(), version) - .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); - skillGovernanceService.deleteVersion( - skill, - skillVersion, - userId, - userNsRoles != null ? userNsRoles : Map.of(), - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent") - ); - return ok("response.success.deleted", - new SkillLifecycleMutationResponse(skill.getId(), skillVersion.getId(), "DELETE_VERSION", version)); + skillLifecycleAppService.deleteVersion( + namespace, + slug, + version, + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); } @PostMapping("/{namespace}/{slug}/versions/{version}/withdraw-review") @@ -132,23 +91,13 @@ public class SkillLifecycleController extends BaseApiController { @PathVariable String version, @RequestAttribute("userId") String userId, HttpServletRequest httpRequest) { - Skill skill = findSkill(namespace, slug, userId); - SkillVersion skillVersion = skillVersionRepository.findBySkillIdAndVersion(skill.getId(), version) - .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); - SkillVersion withdrawnVersion = reviewService.withdrawReview(skillVersion.getId(), userId); - auditLogService.record( - userId, - "REVIEW_WITHDRAW", - "SKILL_VERSION", - skillVersion.getId(), - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent"), - "{\"version\":\"" + version.replace("\"", "\\\"") + "\"}" - ); - return ok("response.success.updated", - new SkillLifecycleMutationResponse(skill.getId(), skillVersion.getId(), "WITHDRAW_REVIEW", withdrawnVersion.getStatus().name())); + skillLifecycleAppService.withdrawReview( + namespace, + slug, + version, + userId, + AuditRequestContext.from(httpRequest))); } @PostMapping("/{namespace}/{slug}/versions/{version}/rerelease") @@ -159,44 +108,14 @@ public class SkillLifecycleController extends BaseApiController { @RequestAttribute("userId") String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, HttpServletRequest httpRequest) { - Skill skill = findSkill(namespace, slug, userId); - SkillVersion skillVersion = skillVersionRepository.findBySkillIdAndVersion(skill.getId(), version) - .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); - SkillPublishService.PublishResult result = skillPublishService.rereleasePublishedVersion( - skill.getId(), - skillVersion.getVersion(), - request.targetVersion().trim(), - userId, - userNsRoles != null ? userNsRoles : Map.of() - ); - auditLogService.record( - userId, - "RERELEASE_SKILL_VERSION", - "SKILL_VERSION", - skillVersion.getId(), - null, - httpRequest.getRemoteAddr(), - httpRequest.getHeader("User-Agent"), - "{\"sourceVersion\":\"" + version.replace("\"", "\\\"") - + "\",\"targetVersion\":\"" + request.targetVersion().trim().replace("\"", "\\\"") + "\"}" - ); - return ok("response.success.updated", - new SkillLifecycleMutationResponse(result.skillId(), result.version().getId(), "RERELEASE_VERSION", result.version().getStatus().name())); - } - - private Skill findSkill(String namespaceSlug, String skillSlug, String currentUserId) { - String cleanNamespace = namespaceSlug.startsWith("@") ? namespaceSlug.substring(1) : namespaceSlug; - Namespace namespace = namespaceRepository.findBySlug(cleanNamespace) - .orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", cleanNamespace)); - return resolveVisibleSkill(namespace.getId(), skillSlug, currentUserId); - } - - private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - return skillSlugResolutionService.resolve( - namespaceId, - slug, - currentUserId, - SkillSlugResolutionService.Preference.CURRENT_USER); + skillLifecycleAppService.rereleaseVersion( + namespace, + slug, + version, + request, + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java index 8c776c77..366038ee 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.filter; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -34,17 +35,20 @@ public class AuthContextFilter extends OncePerRequestFilter { private final ApiResponseFactory apiResponseFactory; private final ObjectMapper objectMapper; private final boolean enforceActiveUserCheck; + private final RouteSecurityPolicyRegistry routeSecurityPolicyRegistry; public AuthContextFilter(NamespaceMemberRepository namespaceMemberRepository, UserAccountRepository userAccountRepository, ApiResponseFactory apiResponseFactory, ObjectMapper objectMapper, - @Value("${skillhub.auth.enforce-active-user-check:true}") boolean enforceActiveUserCheck) { + @Value("${skillhub.auth.enforce-active-user-check:true}") boolean enforceActiveUserCheck, + RouteSecurityPolicyRegistry routeSecurityPolicyRegistry) { this.namespaceMemberRepository = namespaceMemberRepository; this.userAccountRepository = userAccountRepository; this.apiResponseFactory = apiResponseFactory; this.objectMapper = objectMapper; this.enforceActiveUserCheck = enforceActiveUserCheck; + this.routeSecurityPolicyRegistry = routeSecurityPolicyRegistry; } @Override @@ -52,6 +56,10 @@ public class AuthContextFilter extends OncePerRequestFilter { HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { + if (!routeSecurityPolicyRegistry.shouldProjectRequestContext(request.getRequestURI())) { + filterChain.doFilter(request, response); + return; + } PlatformPrincipal principal = resolvePrincipal(request); if (principal != null) { if (isInactiveUser(principal.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 deleted file mode 100644 index 6109efeb..00000000 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserManagementService.java +++ /dev/null @@ -1,157 +0,0 @@ -package com.iflytek.skillhub.service; - -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; -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.AdminUserSummaryResponse; -import com.iflytek.skillhub.dto.PageResponse; -import java.util.Comparator; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.TreeSet; -import java.util.Set; -import org.springframework.data.domain.Page; -import org.springframework.data.domain.PageImpl; -import org.springframework.data.domain.PageRequest; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; - -/** - * Alternative user-management aggregation service that combines user records - * with role bindings for management-oriented views. - */ -@Service -public class AdminUserManagementService { - - private final UserAccountRepository userAccountRepository; - private final UserRoleBindingRepository userRoleBindingRepository; - private final RoleRepository roleRepository; - - public AdminUserManagementService(UserAccountRepository userAccountRepository, - UserRoleBindingRepository userRoleBindingRepository, - RoleRepository roleRepository) { - this.userAccountRepository = userAccountRepository; - this.userRoleBindingRepository = userRoleBindingRepository; - this.roleRepository = roleRepository; - } - - @Transactional(readOnly = true) - public PageResponse listUsers(String keyword, String status, int page, int size) { - UserStatus userStatus = parseStatus(status); - Page users = userAccountRepository.search(normalize(keyword), userStatus, PageRequest.of(page, size)); - List items = users.getContent().stream() - .map(this::toSummary) - .toList(); - return PageResponse.from(new PageImpl<>(items, users.getPageable(), users.getTotalElements())); - } - - @Transactional - public AdminUserSummaryResponse updateUserRole(String userId, String roleCode, PlatformPrincipal principal) { - UserAccount user = loadUser(userId); - if (principal != null - && !principal.platformRoles().contains("SUPER_ADMIN") - && "SUPER_ADMIN".equalsIgnoreCase(roleCode)) { - throw new DomainForbiddenException("error.admin.role.assign_super_admin_forbidden"); - } - Role role = roleRepository.findByCode(roleCode) - .orElseThrow(() -> new DomainBadRequestException("error.role.notFound", roleCode)); - - List existing = userRoleBindingRepository.findByUserId(userId); - boolean alreadyAssigned = existing.stream().anyMatch(binding -> binding.getRole().getCode().equals(roleCode)); - if (!alreadyAssigned) { - userRoleBindingRepository.save(new UserRoleBinding(userId, role)); - } - return toSummary(user); - } - - @Transactional - public AdminUserSummaryResponse approveUser(String userId) { - UserAccount user = loadUser(userId); - user.setStatus(UserStatus.ACTIVE); - return toSummary(userAccountRepository.save(user)); - } - - @Transactional - public AdminUserSummaryResponse updateUserStatus(String userId, String status) { - UserAccount user = loadUser(userId); - user.setStatus(parseRequiredStatus(status)); - return toSummary(userAccountRepository.save(user)); - } - - @Transactional - public AdminUserSummaryResponse disableUser(String userId) { - UserAccount user = loadUser(userId); - user.setStatus(UserStatus.DISABLED); - return toSummary(userAccountRepository.save(user)); - } - - @Transactional - public AdminUserSummaryResponse enableUser(String userId) { - UserAccount user = loadUser(userId); - user.setStatus(UserStatus.ACTIVE); - return toSummary(userAccountRepository.save(user)); - } - - private UserAccount loadUser(String userId) { - return userAccountRepository.findById(userId) - .orElseThrow(() -> new DomainNotFoundException("error.user.notFound", userId)); - } - - private AdminUserSummaryResponse toSummary(UserAccount user) { - Set roles = new LinkedHashSet<>(); - userRoleBindingRepository.findByUserId(user.getId()).stream() - .map(binding -> binding.getRole().getCode()) - .sorted(Comparator.naturalOrder()) - .forEach(roles::add); - roles = new LinkedHashSet<>(withDefaultUserRole(roles)); - return new AdminUserSummaryResponse( - user.getId(), - user.getDisplayName(), - user.getEmail(), - user.getStatus().name(), - List.copyOf(roles), - user.getCreatedAt() - ); - } - - private String normalize(String keyword) { - if (keyword == null || keyword.isBlank()) { - return null; - } - return keyword.trim(); - } - - private Set withDefaultUserRole(Set roles) { - Set resolvedRoles = new TreeSet<>(); - if (roles != null) { - resolvedRoles.addAll(roles); - } - if (resolvedRoles.isEmpty()) { - resolvedRoles.add("USER"); - } - return Set.copyOf(resolvedRoles); - } - - private UserStatus parseStatus(String status) { - if (status == null || status.isBlank()) { - return null; - } - return parseRequiredStatus(status); - } - - private UserStatus parseRequiredStatus(String status) { - try { - return UserStatus.valueOf(status.trim().toUpperCase()); - } catch (IllegalArgumentException ex) { - throw new DomainBadRequestException("error.user.status.invalid", status); - } - } -} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuditRequestContext.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuditRequestContext.java new file mode 100644 index 00000000..23772575 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuditRequestContext.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.service; + +import jakarta.servlet.http.HttpServletRequest; + +/** + * Transport-level audit fields extracted from the current HTTP request. + */ +public record AuditRequestContext( + String clientIp, + String userAgent +) { + public static AuditRequestContext from(HttpServletRequest request) { + return new AuditRequestContext( + request != null ? request.getRemoteAddr() : null, + request != null ? request.getHeader("User-Agent") : null + ); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalCommandAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalCommandAppService.java new file mode 100644 index 00000000..92c1340b --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalCommandAppService.java @@ -0,0 +1,167 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceGovernanceService; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceService; +import com.iflytek.skillhub.dto.MemberResponse; +import com.iflytek.skillhub.dto.MessageResponse; +import com.iflytek.skillhub.dto.NamespaceLifecycleRequest; +import com.iflytek.skillhub.dto.NamespaceRequest; +import com.iflytek.skillhub.dto.NamespaceResponse; +import com.iflytek.skillhub.dto.UpdateMemberRoleRequest; +import com.iflytek.skillhub.exception.ForbiddenException; +import com.iflytek.skillhub.exception.UnauthorizedException; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Command-facing namespace application service for portal endpoints. + */ +@Service +public class NamespacePortalCommandAppService { + + private final NamespaceService namespaceService; + private final NamespaceRepository namespaceRepository; + private final NamespaceGovernanceService namespaceGovernanceService; + private final NamespaceMemberService namespaceMemberService; + + public NamespacePortalCommandAppService(NamespaceService namespaceService, + NamespaceRepository namespaceRepository, + NamespaceGovernanceService namespaceGovernanceService, + NamespaceMemberService namespaceMemberService) { + this.namespaceService = namespaceService; + this.namespaceRepository = namespaceRepository; + this.namespaceGovernanceService = namespaceGovernanceService; + this.namespaceMemberService = namespaceMemberService; + } + + @Transactional + public NamespaceResponse createNamespace(NamespaceRequest request, PlatformPrincipal principal) { + if (principal == null) { + throw new UnauthorizedException("error.auth.required"); + } + if (!canCreateNamespace(principal)) { + throw new ForbiddenException("error.namespace.create.platformAdminRequired"); + } + + Namespace namespace = namespaceService.createNamespace( + request.slug(), + request.displayName(), + request.description(), + principal.userId() + ); + return NamespaceResponse.from(namespace); + } + + @Transactional + public NamespaceResponse updateNamespace(String slug, NamespaceRequest request, String userId) { + Namespace namespace = namespaceService.getNamespaceBySlug(slug); + Namespace updated = namespaceService.updateNamespace( + namespace.getId(), + request.displayName(), + request.description(), + null, + userId + ); + return NamespaceResponse.from(updated); + } + + @Transactional + public NamespaceResponse freezeNamespace(String slug, + NamespaceLifecycleRequest request, + String userId, + AuditRequestContext auditContext) { + Namespace namespace = namespaceGovernanceService.freezeNamespace( + slug, + userId, + request != null ? request.reason() : null, + null, + auditContext.clientIp(), + auditContext.userAgent() + ); + return NamespaceResponse.from(namespace); + } + + @Transactional + public NamespaceResponse unfreezeNamespace(String slug, String userId, AuditRequestContext auditContext) { + Namespace namespace = namespaceGovernanceService.unfreezeNamespace( + slug, + userId, + null, + auditContext.clientIp(), + auditContext.userAgent() + ); + return NamespaceResponse.from(namespace); + } + + @Transactional + public NamespaceResponse archiveNamespace(String slug, + NamespaceLifecycleRequest request, + String userId, + AuditRequestContext auditContext) { + Namespace namespace = namespaceGovernanceService.archiveNamespace( + slug, + userId, + request != null ? request.reason() : null, + null, + auditContext.clientIp(), + auditContext.userAgent() + ); + return NamespaceResponse.from(namespace); + } + + @Transactional + public NamespaceResponse restoreNamespace(String slug, String userId, AuditRequestContext auditContext) { + Namespace namespace = namespaceGovernanceService.restoreNamespace( + slug, + userId, + null, + auditContext.clientIp(), + auditContext.userAgent() + ); + return NamespaceResponse.from(namespace); + } + + @Transactional + public MemberResponse addMember(String slug, String memberUserId, com.iflytek.skillhub.domain.namespace.NamespaceRole role, String operatorUserId) { + Namespace namespace = namespaceService.getNamespaceBySlug(slug); + NamespaceMember member = namespaceMemberService.addMember( + namespace.getId(), + memberUserId, + role, + operatorUserId + ); + return MemberResponse.from(member); + } + + @Transactional + public MessageResponse removeMember(String slug, String memberUserId, String operatorUserId) { + Namespace namespace = namespaceService.getNamespaceBySlug(slug); + namespaceMemberService.removeMember(namespace.getId(), memberUserId, operatorUserId); + return new MessageResponse("Member removed successfully"); + } + + @Transactional + public MemberResponse updateMemberRole(String slug, + String userId, + UpdateMemberRoleRequest request, + String operatorUserId) { + Namespace namespace = namespaceService.getNamespaceBySlug(slug); + NamespaceMember member = namespaceMemberService.updateMemberRole( + namespace.getId(), + userId, + request.role(), + operatorUserId + ); + return MemberResponse.from(member); + } + + private boolean canCreateNamespace(PlatformPrincipal principal) { + return principal.platformRoles().contains("SKILL_ADMIN") + || principal.platformRoles().contains("SUPER_ADMIN"); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java new file mode 100644 index 00000000..17dd9b99 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java @@ -0,0 +1,83 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceAccessPolicy; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceService; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.dto.MemberResponse; +import com.iflytek.skillhub.dto.MyNamespaceResponse; +import com.iflytek.skillhub.dto.NamespaceResponse; +import com.iflytek.skillhub.dto.PageResponse; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Query-facing namespace application service that keeps controller methods + * thin while preserving current response contracts. + */ +@Service +public class NamespacePortalQueryAppService { + + private final NamespaceRepository namespaceRepository; + private final NamespaceService namespaceService; + private final NamespaceMemberService namespaceMemberService; + private final NamespaceAccessPolicy namespaceAccessPolicy; + + public NamespacePortalQueryAppService(NamespaceRepository namespaceRepository, + NamespaceService namespaceService, + NamespaceMemberService namespaceMemberService, + NamespaceAccessPolicy namespaceAccessPolicy) { + this.namespaceRepository = namespaceRepository; + this.namespaceService = namespaceService; + this.namespaceMemberService = namespaceMemberService; + this.namespaceAccessPolicy = namespaceAccessPolicy; + } + + @Transactional(readOnly = true) + public PageResponse listNamespaces(Pageable pageable) { + Page namespaces = namespaceRepository.findByStatus(NamespaceStatus.ACTIVE, pageable); + return PageResponse.from(namespaces.map(NamespaceResponse::from)); + } + + @Transactional(readOnly = true) + public List listMyNamespaces(Map userNamespaceRoles) { + Map namespaceRoles = userNamespaceRoles != null ? userNamespaceRoles : Map.of(); + if (namespaceRoles.isEmpty()) { + return List.of(); + } + + return namespaceRepository.findByIdIn(namespaceRoles.keySet().stream().toList()).stream() + .sorted(Comparator.comparing(Namespace::getSlug)) + .map(namespace -> MyNamespaceResponse.from( + namespace, + namespaceRoles.get(namespace.getId()), + namespaceAccessPolicy)) + .toList(); + } + + @Transactional(readOnly = true) + public NamespaceResponse getNamespace(String slug, String userId, Map userNamespaceRoles) { + Namespace namespace = namespaceService.getNamespaceBySlugForRead( + slug, + userId, + userNamespaceRoles != null ? userNamespaceRoles : Map.of()); + return NamespaceResponse.from(namespace); + } + + @Transactional(readOnly = true) + public PageResponse listMembers(String slug, Pageable pageable, String userId) { + Namespace namespace = namespaceService.getNamespaceBySlug(slug); + namespaceService.assertMember(namespace.getId(), userId); + Page members = namespaceMemberService.listMembers(namespace.getId(), pageable); + return PageResponse.from(members.map(MemberResponse::from)); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java new file mode 100644 index 00000000..db687186 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java @@ -0,0 +1,193 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; +import com.iflytek.skillhub.dto.AdminSkillActionRequest; +import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; +import com.iflytek.skillhub.dto.SkillVersionRereleaseRequest; +import java.util.Map; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Orchestrates skill lifecycle mutations so controllers only handle transport + * concerns and envelope assembly. + */ +@Service +public class SkillLifecycleAppService { + + private final NamespaceRepository namespaceRepository; + private final SkillVersionRepository skillVersionRepository; + private final SkillGovernanceService skillGovernanceService; + private final ReviewService reviewService; + private final SkillPublishService skillPublishService; + private final AuditLogService auditLogService; + private final SkillSlugResolutionService skillSlugResolutionService; + + public SkillLifecycleAppService(NamespaceRepository namespaceRepository, + SkillVersionRepository skillVersionRepository, + SkillGovernanceService skillGovernanceService, + ReviewService reviewService, + SkillPublishService skillPublishService, + AuditLogService auditLogService, + SkillSlugResolutionService skillSlugResolutionService) { + this.namespaceRepository = namespaceRepository; + this.skillVersionRepository = skillVersionRepository; + this.skillGovernanceService = skillGovernanceService; + this.reviewService = reviewService; + this.skillPublishService = skillPublishService; + this.auditLogService = auditLogService; + this.skillSlugResolutionService = skillSlugResolutionService; + } + + @Transactional + public SkillLifecycleMutationResponse archiveSkill(String namespace, + String slug, + AdminSkillActionRequest request, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + Skill archived = skillGovernanceService.archiveSkill( + skill.getId(), + userId, + normalizeRoles(userNamespaceRoles), + auditContext.clientIp(), + auditContext.userAgent(), + request != null ? request.reason() : null + ); + return new SkillLifecycleMutationResponse(archived.getId(), null, "ARCHIVE", archived.getStatus().name()); + } + + @Transactional + public SkillLifecycleMutationResponse unarchiveSkill(String namespace, + String slug, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + Skill restored = skillGovernanceService.unarchiveSkill( + skill.getId(), + userId, + normalizeRoles(userNamespaceRoles), + auditContext.clientIp(), + auditContext.userAgent() + ); + return new SkillLifecycleMutationResponse(restored.getId(), null, "UNARCHIVE", restored.getStatus().name()); + } + + @Transactional + public SkillLifecycleMutationResponse deleteVersion(String namespace, + String slug, + String version, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + SkillVersion skillVersion = findVersion(skill.getId(), version); + skillGovernanceService.deleteVersion( + skill, + skillVersion, + userId, + normalizeRoles(userNamespaceRoles), + auditContext.clientIp(), + auditContext.userAgent() + ); + return new SkillLifecycleMutationResponse(skill.getId(), skillVersion.getId(), "DELETE_VERSION", version); + } + + @Transactional + public SkillLifecycleMutationResponse withdrawReview(String namespace, + String slug, + String version, + String userId, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + SkillVersion skillVersion = findVersion(skill.getId(), version); + SkillVersion withdrawnVersion = reviewService.withdrawReview(skillVersion.getId(), userId); + auditLogService.record( + userId, + "REVIEW_WITHDRAW", + "SKILL_VERSION", + skillVersion.getId(), + null, + auditContext.clientIp(), + auditContext.userAgent(), + "{\"version\":\"" + version.replace("\"", "\\\"") + "\"}" + ); + return new SkillLifecycleMutationResponse( + skill.getId(), + skillVersion.getId(), + "WITHDRAW_REVIEW", + withdrawnVersion.getStatus().name() + ); + } + + @Transactional + public SkillLifecycleMutationResponse rereleaseVersion(String namespace, + String slug, + String version, + SkillVersionRereleaseRequest request, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + SkillVersion skillVersion = findVersion(skill.getId(), version); + String targetVersion = request.targetVersion().trim(); + SkillPublishService.PublishResult result = skillPublishService.rereleasePublishedVersion( + skill.getId(), + skillVersion.getVersion(), + targetVersion, + userId, + normalizeRoles(userNamespaceRoles) + ); + auditLogService.record( + userId, + "RERELEASE_SKILL_VERSION", + "SKILL_VERSION", + skillVersion.getId(), + null, + auditContext.clientIp(), + auditContext.userAgent(), + "{\"sourceVersion\":\"" + version.replace("\"", "\\\"") + + "\",\"targetVersion\":\"" + targetVersion.replace("\"", "\\\"") + "\"}" + ); + return new SkillLifecycleMutationResponse( + result.skillId(), + result.version().getId(), + "RERELEASE_VERSION", + result.version().getStatus().name() + ); + } + + private Skill findSkill(String namespaceSlug, String skillSlug, String currentUserId) { + String cleanNamespace = namespaceSlug.startsWith("@") ? namespaceSlug.substring(1) : namespaceSlug; + Namespace namespace = namespaceRepository.findBySlug(cleanNamespace) + .orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", cleanNamespace)); + return skillSlugResolutionService.resolve( + namespace.getId(), + skillSlug, + currentUserId, + SkillSlugResolutionService.Preference.CURRENT_USER + ); + } + + private SkillVersion findVersion(Long skillId, String version) { + return skillVersionRepository.findBySkillIdAndVersion(skillId, version) + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); + } + + private Map normalizeRoles(Map userNamespaceRoles) { + return userNamespaceRoles != null ? userNamespaceRoles : Map.of(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java index 5cc73856..76663e99 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java @@ -4,7 +4,9 @@ import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceGovernanceService; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; @@ -54,6 +56,9 @@ class NamespacePortalControllerTest { @MockBean private NamespaceGovernanceService namespaceGovernanceService; + @MockBean + private NamespaceMemberService namespaceMemberService; + @MockBean private com.iflytek.skillhub.domain.namespace.NamespaceRepository namespaceRepository; @@ -116,6 +121,33 @@ class NamespacePortalControllerTest { .andExpect(jsonPath("$.data.status").value("ARCHIVED")); } + @Test + void updateNamespace_returnsUpdatedNamespace() throws Exception { + Namespace existing = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); + Namespace updated = new Namespace("team-a", "Team A+", "owner-1"); + setField(updated, "id", 1L); + updated.setStatus(NamespaceStatus.ACTIVE); + updated.setType(NamespaceType.TEAM); + updated.setDescription("Updated description"); + given(namespaceService.getNamespaceBySlug("team-a")).willReturn(existing); + given(namespaceService.updateNamespace(1L, "Team A+", "Updated description", null, "owner-1")) + .willReturn(updated); + + mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put("/api/v1/namespaces/team-a") + .with(csrf()) + .with(auth("owner-1")) + .requestAttr("userId", "owner-1") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"slug":"team-a","displayName":"Team A+","description":"Updated description"} + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.slug").value("team-a")) + .andExpect(jsonPath("$.data.displayName").value("Team A+")) + .andExpect(jsonPath("$.data.description").value("Updated description")); + } + @Test void listMembers_forNonMember_returns403() throws Exception { Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); @@ -152,6 +184,64 @@ class NamespacePortalControllerTest { .andExpect(jsonPath("$.data[0].displayName").value("alice")); } + @Test + void addMember_returnsCreatedMember() throws Exception { + Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); + NamespaceMember member = new NamespaceMember(1L, "user-2", NamespaceRole.ADMIN); + given(namespaceService.getNamespaceBySlug("team-a")).willReturn(namespace); + given(namespaceMemberService.addMember(1L, "user-2", NamespaceRole.ADMIN, "owner-1")) + .willReturn(member); + + mockMvc.perform(post("/api/v1/namespaces/team-a/members") + .with(csrf()) + .with(auth("owner-1")) + .requestAttr("userId", "owner-1") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"userId":"user-2","role":"ADMIN"} + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.userId").value("user-2")) + .andExpect(jsonPath("$.data.role").value("ADMIN")); + } + + @Test + void removeMember_returnsSuccessMessage() throws Exception { + Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); + given(namespaceService.getNamespaceBySlug("team-a")).willReturn(namespace); + + mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete("/api/v1/namespaces/team-a/members/user-2") + .with(csrf()) + .with(auth("owner-1")) + .requestAttr("userId", "owner-1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.message").value("Member removed successfully")); + } + + @Test + void updateMemberRole_returnsUpdatedMember() throws Exception { + Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); + NamespaceMember member = new NamespaceMember(1L, "user-2", NamespaceRole.OWNER); + given(namespaceService.getNamespaceBySlug("team-a")).willReturn(namespace); + given(namespaceMemberService.updateMemberRole(1L, "user-2", NamespaceRole.OWNER, "owner-1")) + .willReturn(member); + + mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put("/api/v1/namespaces/team-a/members/user-2/role") + .with(csrf()) + .with(auth("owner-1")) + .requestAttr("userId", "owner-1") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"role":"OWNER"} + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.userId").value("user-2")) + .andExpect(jsonPath("$.data.role").value("OWNER")); + } + @Test void createNamespace_requiresPlatformAdminRole() throws Exception { mockMvc.perform(post("/api/v1/namespaces") 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 822a06f5..2bb98567 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 @@ -26,10 +26,12 @@ import java.util.Set; 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.request.MockMvcRequestBuilders.put; 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.verify; import static org.mockito.Mockito.when; @SpringBootTest @@ -172,4 +174,73 @@ class UserManagementControllerTest { .andExpect(jsonPath("$.data.userId").value("user-123")) .andExpect(jsonPath("$.data.status").value("DISABLED")); } + + @Test + void approveUser_delegatesToActiveStatusMutation() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") + ); + var auth = new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) + ); + + when(adminUserAppService.updateUserStatus("user-123", "ACTIVE")) + .thenReturn(new AdminUserMutationResponse("user-123", null, "ACTIVE")); + + mockMvc.perform(post("/api/v1/admin/users/user-123/approve") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.userId").value("user-123")) + .andExpect(jsonPath("$.data.status").value("ACTIVE")); + + verify(adminUserAppService).updateUserStatus("user-123", "ACTIVE"); + } + + @Test + void disableUser_delegatesToDisabledStatusMutation() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") + ); + var auth = new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) + ); + + when(adminUserAppService.updateUserStatus("user-123", "DISABLED")) + .thenReturn(new AdminUserMutationResponse("user-123", null, "DISABLED")); + + mockMvc.perform(post("/api/v1/admin/users/user-123/disable") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.userId").value("user-123")) + .andExpect(jsonPath("$.data.status").value("DISABLED")); + + verify(adminUserAppService).updateUserStatus("user-123", "DISABLED"); + } + + @Test + void enableUser_delegatesToActiveStatusMutation() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") + ); + var auth = new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) + ); + + when(adminUserAppService.updateUserStatus("user-123", "ACTIVE")) + .thenReturn(new AdminUserMutationResponse("user-123", null, "ACTIVE")); + + mockMvc.perform(post("/api/v1/admin/users/user-123/enable") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.userId").value("user-123")) + .andExpect(jsonPath("$.data.status").value("ACTIVE")); + + verify(adminUserAppService).updateUserStatus("user-123", "ACTIVE"); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java index bd7ef931..acf5a24f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java @@ -4,6 +4,7 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verify; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -229,6 +230,78 @@ class SkillLifecycleControllerTest { .andExpect(jsonPath("$.data.status").value("PUBLISHED")); } + @Test + void archiveSkill_acceptsAtPrefixedNamespaceSlug() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner"); + setNamespaceId(namespace, 1L); + Skill skill = new Skill(1L, "demo-skill", "owner", SkillVisibility.PUBLIC); + setSkillId(skill, 1L); + + given(namespaceRepository.findBySlug("global")).willReturn(java.util.Optional.of(namespace)); + given(skillSlugResolutionService.resolve(1L, "demo-skill", "usr_1", SkillSlugResolutionService.Preference.CURRENT_USER)) + .willReturn(skill); + given(skillGovernanceService.archiveSkill(eq(1L), eq("usr_1"), anyMap(), nullable(String.class), nullable(String.class), eq("cleanup"))) + .willReturn(skillWithStatus(skill, com.iflytek.skillhub.domain.skill.SkillStatus.ARCHIVED)); + + mockMvc.perform(post("/api/web/skills/@global/demo-skill/archive") + .requestAttr("userId", "usr_1") + .requestAttr("userNsRoles", java.util.Map.of(1L, NamespaceRole.ADMIN)) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reason\":\"cleanup\"}") + .with(user("usr_1")) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.skillId").value(1)) + .andExpect(jsonPath("$.data.action").value("ARCHIVE")) + .andExpect(jsonPath("$.data.status").value("ARCHIVED")); + } + + @Test + void rereleaseVersion_trimsTargetVersionBeforeDelegating() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner"); + setNamespaceId(namespace, 1L); + Skill skill = new Skill(1L, "demo-skill", "owner", SkillVisibility.PUBLIC); + setSkillId(skill, 1L); + SkillVersion newVersion = new SkillVersion(1L, "1.2.4", "owner"); + setSkillVersionId(newVersion, 3L); + newVersion.setStatus(SkillVersionStatus.PUBLISHED); + + given(namespaceRepository.findBySlug("global")).willReturn(java.util.Optional.of(namespace)); + given(skillSlugResolutionService.resolve(1L, "demo-skill", "usr_1", SkillSlugResolutionService.Preference.CURRENT_USER)) + .willReturn(skill); + SkillVersion sourceVersion = new SkillVersion(1L, "1.2.3", "owner"); + setSkillVersionId(sourceVersion, 2L); + sourceVersion.setStatus(SkillVersionStatus.PUBLISHED); + given(skillVersionRepository.findBySkillIdAndVersion(1L, "1.2.3")).willReturn(java.util.Optional.of(sourceVersion)); + given(skillPublishService.rereleasePublishedVersion( + eq(1L), + eq("1.2.3"), + eq("1.2.4"), + eq("usr_1"), + anyMap())) + .willReturn(new SkillPublishService.PublishResult(1L, "demo-skill", newVersion)); + + mockMvc.perform(post("/api/web/skills/global/demo-skill/versions/1.2.3/rerelease") + .requestAttr("userId", "usr_1") + .requestAttr("userNsRoles", java.util.Map.of(1L, NamespaceRole.ADMIN)) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"targetVersion\":\" 1.2.4 \"}") + .with(user("usr_1")) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.versionId").value(3)) + .andExpect(jsonPath("$.data.action").value("RERELEASE_VERSION")); + + verify(skillPublishService).rereleasePublishedVersion( + eq(1L), + eq("1.2.3"), + eq("1.2.4"), + eq("usr_1"), + anyMap()); + } + private Skill skillWithStatus(Skill skill, com.iflytek.skillhub.domain.skill.SkillStatus status) { skill.setStatus(status); return skill; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java index 80f761ea..6bf56337 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.filter; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -32,6 +33,7 @@ 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.same; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,7 +53,8 @@ class AuthContextFilterTest { userAccountRepository, apiResponseFactory, new ObjectMapper().registerModule(new JavaTimeModule()), - true + true, + new RouteSecurityPolicyRegistry() ); } @@ -67,6 +70,7 @@ class AuthContextFilterTest { user.setStatus(UserStatus.DISABLED); MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/api/v1/auth/me"); MockHttpSession session = (MockHttpSession) request.getSession(true); session.setAttribute("platformPrincipal", principal); SecurityContextHolder.getContext().setAuthentication( @@ -95,6 +99,7 @@ class AuthContextFilterTest { NamespaceMember member = new NamespaceMember(9L, "user-2", NamespaceRole.ADMIN); MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/api/v1/auth/me"); request.getSession(true).setAttribute("platformPrincipal", principal); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, List.of()) @@ -112,4 +117,20 @@ class AuthContextFilterTest { assertEquals(NamespaceRole.ADMIN, ((java.util.Map) request.getAttribute("userNsRoles")).get(9L)); verify(filterChain).doFilter(request, response); } + + @Test + void anonymousRequest_shouldPassThroughWithoutLoadingUserContext() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/assets/app.js"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + filter.doFilter(request, response, filterChain); + + assertNull(request.getAttribute("userId")); + assertNull(request.getAttribute("userNsRoles")); + verify(filterChain).doFilter(same(request), same(response)); + verify(userAccountRepository, never()).findById(org.mockito.ArgumentMatchers.anyString()); + verify(namespaceMemberRepository, never()).findByUserId(org.mockito.ArgumentMatchers.anyString()); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalCommandAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalCommandAppServiceTest.java new file mode 100644 index 00000000..80a76c15 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalCommandAppServiceTest.java @@ -0,0 +1,74 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceGovernanceService; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceService; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.dto.NamespaceLifecycleRequest; +import com.iflytek.skillhub.dto.NamespaceRequest; +import com.iflytek.skillhub.exception.ForbiddenException; +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; + +import java.util.Set; + +class NamespacePortalCommandAppServiceTest { + + private final NamespaceService namespaceService = mock(NamespaceService.class); + private final NamespaceRepository namespaceRepository = mock(NamespaceRepository.class); + private final NamespaceGovernanceService namespaceGovernanceService = mock(NamespaceGovernanceService.class); + private final NamespaceMemberService namespaceMemberService = mock(NamespaceMemberService.class); + private final NamespacePortalCommandAppService service = new NamespacePortalCommandAppService( + namespaceService, + namespaceRepository, + namespaceGovernanceService, + namespaceMemberService + ); + + @Test + void createNamespace_requiresPlatformAdminRole() { + NamespaceRequest request = new NamespaceRequest("team-alpha", "Team Alpha", null); + PlatformPrincipal principal = new PlatformPrincipal( + "user-1", "user-1", "user-1@example.com", "", "github", Set.of("USER") + ); + + assertThrows(ForbiddenException.class, () -> service.createNamespace(request, principal)); + } + + @Test + void freezeNamespace_mapsAuditContextAndReturnsResponse() { + Namespace namespace = namespace(7L, "team-alpha"); + namespace.setStatus(NamespaceStatus.FROZEN); + when(namespaceGovernanceService.freezeNamespace("team-alpha", "owner-1", "cleanup", null, "127.0.0.1", "JUnit")) + .thenReturn(namespace); + + var response = service.freezeNamespace( + "team-alpha", + new NamespaceLifecycleRequest("cleanup"), + "owner-1", + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(response.id()).isEqualTo(7L); + assertThat(response.slug()).isEqualTo("team-alpha"); + assertThat(response.status()).isEqualTo(NamespaceStatus.FROZEN); + verify(namespaceGovernanceService).freezeNamespace("team-alpha", "owner-1", "cleanup", null, "127.0.0.1", "JUnit"); + } + + private Namespace namespace(Long id, String slug) { + Namespace namespace = new Namespace(slug, "Team Alpha", "owner-1"); + ReflectionTestUtils.setField(namespace, "id", id); + namespace.setType(NamespaceType.TEAM); + return namespace; + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java new file mode 100644 index 00000000..61e6a458 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java @@ -0,0 +1,70 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceAccessPolicy; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceService; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; + +import java.util.List; +import java.util.Map; + +class NamespacePortalQueryAppServiceTest { + + private final NamespaceRepository namespaceRepository = mock(NamespaceRepository.class); + private final NamespaceService namespaceService = mock(NamespaceService.class); + private final NamespaceMemberService namespaceMemberService = mock(NamespaceMemberService.class); + private final NamespaceAccessPolicy namespaceAccessPolicy = mock(NamespaceAccessPolicy.class); + private final NamespacePortalQueryAppService service = new NamespacePortalQueryAppService( + namespaceRepository, + namespaceService, + namespaceMemberService, + namespaceAccessPolicy + ); + + @Test + void listMyNamespaces_sortsBySlugAndProjectsRoleCapabilities() { + Namespace zeta = namespace(2L, "zeta"); + Namespace alpha = namespace(1L, "alpha"); + when(namespaceRepository.findByIdIn(anyList())).thenReturn(List.of(zeta, alpha)); + when(namespaceAccessPolicy.isImmutable(alpha)).thenReturn(false); + when(namespaceAccessPolicy.canFreeze(alpha, NamespaceRole.OWNER)).thenReturn(true); + when(namespaceAccessPolicy.canUnfreeze(alpha, NamespaceRole.OWNER)).thenReturn(false); + when(namespaceAccessPolicy.canArchive(alpha, NamespaceRole.OWNER)).thenReturn(true); + when(namespaceAccessPolicy.canRestore(alpha, NamespaceRole.OWNER)).thenReturn(false); + when(namespaceAccessPolicy.isImmutable(zeta)).thenReturn(false); + when(namespaceAccessPolicy.canFreeze(zeta, NamespaceRole.ADMIN)).thenReturn(true); + when(namespaceAccessPolicy.canUnfreeze(zeta, NamespaceRole.ADMIN)).thenReturn(false); + when(namespaceAccessPolicy.canArchive(zeta, NamespaceRole.ADMIN)).thenReturn(true); + when(namespaceAccessPolicy.canRestore(zeta, NamespaceRole.ADMIN)).thenReturn(false); + + var response = service.listMyNamespaces(Map.of( + 2L, NamespaceRole.ADMIN, + 1L, NamespaceRole.OWNER + )); + + assertThat(response).hasSize(2); + assertThat(response.get(0).slug()).isEqualTo("alpha"); + assertThat(response.get(0).currentUserRole()).isEqualTo(NamespaceRole.OWNER); + assertThat(response.get(1).slug()).isEqualTo("zeta"); + assertThat(response.get(1).currentUserRole()).isEqualTo(NamespaceRole.ADMIN); + } + + private Namespace namespace(Long id, String slug) { + Namespace namespace = new Namespace(slug, slug, "owner-1"); + ReflectionTestUtils.setField(namespace, "id", id); + namespace.setStatus(NamespaceStatus.ACTIVE); + namespace.setType(NamespaceType.TEAM); + return namespace; + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java new file mode 100644 index 00000000..53215011 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java @@ -0,0 +1,76 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewService; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; +import com.iflytek.skillhub.dto.AdminSkillActionRequest; +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; + +import java.util.Map; +import java.util.Optional; + +class SkillLifecycleAppServiceTest { + + private final NamespaceRepository namespaceRepository = mock(NamespaceRepository.class); + private final SkillVersionRepository skillVersionRepository = mock(SkillVersionRepository.class); + private final SkillGovernanceService skillGovernanceService = mock(SkillGovernanceService.class); + private final ReviewService reviewService = mock(ReviewService.class); + private final SkillPublishService skillPublishService = mock(SkillPublishService.class); + private final AuditLogService auditLogService = mock(AuditLogService.class); + private final SkillSlugResolutionService skillSlugResolutionService = mock(SkillSlugResolutionService.class); + private final SkillLifecycleAppService service = new SkillLifecycleAppService( + namespaceRepository, + skillVersionRepository, + skillGovernanceService, + reviewService, + skillPublishService, + auditLogService, + skillSlugResolutionService + ); + + @Test + void archiveSkill_resolvesNamespaceAndDelegatesLifecycleMutation() { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + ReflectionTestUtils.setField(namespace, "id", 7L); + Skill skill = new Skill(7L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(skill, "id", 11L); + skill.setStatus(com.iflytek.skillhub.domain.skill.SkillStatus.ARCHIVED); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillSlugResolutionService.resolve(7L, "demo-skill", "owner-1", SkillSlugResolutionService.Preference.CURRENT_USER)) + .thenReturn(skill); + when(skillGovernanceService.archiveSkill(eq(11L), eq("owner-1"), anyMap(), nullable(String.class), nullable(String.class), eq("cleanup"))) + .thenReturn(skill); + + var response = service.archiveSkill( + "global", + "demo-skill", + new AdminSkillActionRequest("cleanup"), + "owner-1", + Map.of(7L, NamespaceRole.OWNER), + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(response.skillId()).isEqualTo(11L); + assertThat(response.action()).isEqualTo("ARCHIVE"); + assertThat(response.status()).isEqualTo("ARCHIVED"); + verify(skillGovernanceService).archiveSkill(11L, "owner-1", Map.of(7L, NamespaceRole.OWNER), "127.0.0.1", "JUnit", "cleanup"); + } +} 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 6958b6db..276826ec 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 @@ -5,17 +5,18 @@ import com.iflytek.skillhub.auth.oauth.OAuth2LoginFailureHandler; 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.policy.RouteSecurityPolicyRegistry; 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; import org.springframework.http.HttpStatus; -import org.springframework.http.HttpMethod; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configurers.AuthorizeHttpRequestsConfigurer; import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.http.SessionCreationPolicy; @@ -58,6 +59,7 @@ public class SecurityConfig { private final AuthenticationEntryPoint apiAuthenticationEntryPoint; private final AccessDeniedHandler apiAccessDeniedHandler; private final ObjectProvider mockAuthFilterProvider; + private final RouteSecurityPolicyRegistry routeSecurityPolicyRegistry; public SecurityConfig(CustomOAuth2UserService customOAuth2UserService, SkillHubOAuth2AuthorizationRequestResolver authorizationRequestResolver, @@ -67,7 +69,8 @@ public class SecurityConfig { ApiTokenScopeFilter apiTokenScopeFilter, AuthenticationEntryPoint apiAuthenticationEntryPoint, AccessDeniedHandler apiAccessDeniedHandler, - ObjectProvider mockAuthFilterProvider) { + ObjectProvider mockAuthFilterProvider, + RouteSecurityPolicyRegistry routeSecurityPolicyRegistry) { this.customOAuth2UserService = customOAuth2UserService; this.authorizationRequestResolver = authorizationRequestResolver; this.successHandler = successHandler; @@ -77,6 +80,7 @@ public class SecurityConfig { this.apiAuthenticationEntryPoint = apiAuthenticationEntryPoint; this.apiAccessDeniedHandler = apiAccessDeniedHandler; this.mockAuthFilterProvider = mockAuthFilterProvider; + this.routeSecurityPolicyRegistry = routeSecurityPolicyRegistry; } /** @@ -93,15 +97,7 @@ public class SecurityConfig { RequestMatcher csrfIgnoreMatcher = request -> { String path = request.getRequestURI(); String authorization = request.getHeader("Authorization"); - if (authorization != null && authorization.startsWith("Bearer ")) { - return true; - } - if (path == null) { - return false; - } - return path.startsWith("/api/") - || path.equals("/api/v1/publish") - || path.startsWith("/api/v1/auth/device/"); + return routeSecurityPolicyRegistry.shouldIgnoreCsrf(path, authorization); }; http @@ -110,75 +106,10 @@ public class SecurityConfig { .csrfTokenRequestHandler(csrfHandler) .ignoringRequestMatchers(csrfIgnoreMatcher) ) - .authorizeHttpRequests(auth -> auth - .requestMatchers( - "/api/v1/health", - "/api/v1/search", - "/api/v1/resolve/**", - "/api/v1/download/**", - "/api/v1/auth/providers", - "/api/v1/auth/methods", - "/api/v1/auth/me", - "/api/v1/auth/session/bootstrap", - "/api/v1/auth/direct/login", - "/api/v1/auth/local/**", - "/api/v1/auth/device/**", - "/api/v1/check", - "/actuator/health", - "/v3/api-docs/**", - "/swagger-ui/**", - "/.well-known/**", - "/api/v1/search", - "/api/v1/resolve/**", - "/api/v1/download/**" - ).permitAll() - .requestMatchers("/actuator/prometheus").hasAnyRole("SUPER_ADMIN", "AUDITOR") - .requestMatchers( - HttpMethod.GET, - "/api/v1/skills/*/star", - "/api/v1/skills/*/rating", - "/api/web/skills/*/star", - "/api/web/skills/*/rating" - ).authenticated() - .requestMatchers( - HttpMethod.GET, - "/api/v1/skills", - "/api/v1/skills/*/*", - "/api/v1/skills/*/*/versions", - "/api/v1/skills/*/*/versions/*", - "/api/v1/skills/*/*/download", - "/api/v1/skills/*/*/versions/*/download", - "/api/v1/skills/*/*/versions/*/files", - "/api/v1/skills/*/*/versions/*/file", - "/api/v1/skills/*/*/resolve", - "/api/v1/skills/*/*/tags", - "/api/v1/skills/*/*/tags/*/download", - "/api/v1/skills/*/*/tags/*/files", - "/api/v1/skills/*/*/tags/*/file", - "/api/web/skills", - "/api/web/skills/*/*", - "/api/web/skills/*/*/versions", - "/api/web/skills/*/*/versions/*", - "/api/web/skills/*/*/download", - "/api/web/skills/*/*/versions/*/download", - "/api/web/skills/*/*/versions/*/files", - "/api/web/skills/*/*/versions/*/file", - "/api/web/skills/*/*/resolve", - "/api/web/skills/*/*/tags", - "/api/web/skills/*/*/tags/*/download", - "/api/web/skills/*/*/tags/*/files", - "/api/web/skills/*/*/tags/*/file" - ).permitAll() - .requestMatchers( - HttpMethod.GET, - "/api/v1/namespaces", - "/api/v1/namespaces/*", - "/api/web/namespaces", - "/api/web/namespaces/*" - ).permitAll() - .requestMatchers("/api/v1/admin/**").authenticated() - .anyRequest().authenticated() - ) + .authorizeHttpRequests(auth -> { + configureRoutePolicies(auth); + auth.anyRequest().authenticated(); + }) .oauth2Login(oauth2 -> oauth2 .authorizationEndpoint(endpoint -> endpoint.authorizationRequestResolver(authorizationRequestResolver)) .userInfoEndpoint(userInfo -> userInfo.userService(customOAuth2UserService)) @@ -229,4 +160,14 @@ public class SecurityConfig { public PasswordEncoder passwordEncoder() { return new BCryptPasswordEncoder(12); } + + private void configureRoutePolicies(AuthorizeHttpRequestsConfigurer.AuthorizationManagerRequestMatcherRegistry auth) { + for (RouteSecurityPolicyRegistry.RouteAuthorizationPolicy policy : routeSecurityPolicyRegistry.authorizationPolicies()) { + switch (policy.accessLevel()) { + case PERMIT_ALL -> auth.requestMatchers(policy.toRequestMatcher()).permitAll(); + case AUTHENTICATED -> auth.requestMatchers(policy.toRequestMatcher()).authenticated(); + case ROLE_PROTECTED -> auth.requestMatchers(policy.toRequestMatcher()).hasAnyRole(policy.roles()); + } + } + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java new file mode 100644 index 00000000..297de089 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -0,0 +1,205 @@ +package com.iflytek.skillhub.auth.policy; + +import java.util.List; +import java.util.Set; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.stereotype.Component; +import org.springframework.util.AntPathMatcher; + +/** + * Authoritative route-policy catalog shared by web security configuration, + * API-token scope checks, and request-context projection. + */ +@Component +public class RouteSecurityPolicyRegistry { + + private static final List AUTHORIZATION_POLICIES = List.of( + RouteAuthorizationPolicy.permitAll(null, "/api/v1/health"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/search"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/resolve/**"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/download/**"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/providers"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/methods"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/me"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/session/bootstrap"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/direct/login"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/local/**"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/auth/device/**"), + RouteAuthorizationPolicy.permitAll(null, "/api/v1/check"), + RouteAuthorizationPolicy.permitAll(null, "/actuator/health"), + RouteAuthorizationPolicy.permitAll(null, "/v3/api-docs/**"), + RouteAuthorizationPolicy.permitAll(null, "/swagger-ui/**"), + RouteAuthorizationPolicy.permitAll(null, "/.well-known/**"), + RouteAuthorizationPolicy.roles(null, "/actuator/prometheus", "SUPER_ADMIN", "AUDITOR"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/rating"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/rating"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions/*/files"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions/*/file"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/resolve"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/tags"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/tags/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/tags/*/files"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/tags/*/file"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions/*/files"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions/*/file"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/resolve"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/download"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/files"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/file"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/namespaces"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/namespaces/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/namespaces"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/namespaces/*"), + RouteAuthorizationPolicy.authenticated(null, "/api/v1/admin/**") + ); + + private static final List API_TOKEN_POLICIES = List.of( + ApiTokenPolicy.allow(null, "/api/v1/health"), + ApiTokenPolicy.allow(null, "/api/v1/auth/providers"), + ApiTokenPolicy.allow(null, "/api/v1/auth/me"), + ApiTokenPolicy.allow(null, "/api/v1/auth/device/**"), + ApiTokenPolicy.allow(null, "/api/v1/check"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/whoami"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/search"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/skills"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/skills/**"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills/**"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces/*"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/namespaces"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/namespaces/*"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/resolve/**"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/download"), + ApiTokenPolicy.allow(null, "/.well-known/**"), + ApiTokenPolicy.allow(null, "/actuator/health"), + ApiTokenPolicy.allow(null, "/v3/api-docs/**"), + ApiTokenPolicy.allow(null, "/swagger-ui/**"), + ApiTokenPolicy.require(null, "/api/v1/tokens", "token:manage"), + ApiTokenPolicy.require(null, "/api/v1/tokens/**", "token:manage"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills", "skill:publish"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills/*/publish", "skill:publish"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/web/skills/*/publish", "skill:publish"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/publish", "skill:publish") + ); + + private final AntPathMatcher pathMatcher = new AntPathMatcher(); + + public List authorizationPolicies() { + return AUTHORIZATION_POLICIES; + } + + public ApiTokenAuthorizationDecision authorizeApiToken(String method, String path, Set tokenScopes) { + if (!isApiPath(path)) { + return ApiTokenAuthorizationDecision.allow(); + } + + for (ApiTokenPolicy policy : API_TOKEN_POLICIES) { + if (!policy.matches(method, path, pathMatcher)) { + continue; + } + if (policy.requiredScope() == null || tokenScopes.contains(policy.requiredScope())) { + return ApiTokenAuthorizationDecision.allow(); + } + return ApiTokenAuthorizationDecision.missingScope(policy.requiredScope()); + } + + return ApiTokenAuthorizationDecision.unsupported(path); + } + + public boolean shouldIgnoreCsrf(String path, String authorizationHeader) { + if (authorizationHeader != null && authorizationHeader.startsWith("Bearer ")) { + return true; + } + if (path == null) { + return false; + } + return path.startsWith("/api/") + || path.equals("/api/v1/publish") + || path.startsWith("/api/v1/auth/device/"); + } + + public boolean shouldProjectRequestContext(String path) { + return path != null && (path.startsWith("/api/v1/") + || path.startsWith("/api/web/") + || path.startsWith("/api/")); + } + + private boolean isApiPath(String path) { + return shouldProjectRequestContext(path); + } + + public record ApiTokenAuthorizationDecision(boolean allowed, String requiredScope, String message) { + public static ApiTokenAuthorizationDecision allow() { + return new ApiTokenAuthorizationDecision(true, null, null); + } + + public static ApiTokenAuthorizationDecision missingScope(String requiredScope) { + return new ApiTokenAuthorizationDecision(false, requiredScope, "Missing API token scope: " + requiredScope); + } + + public static ApiTokenAuthorizationDecision unsupported(String path) { + return new ApiTokenAuthorizationDecision(false, null, "API token cannot access endpoint: " + path); + } + } + + public enum AccessLevel { + PERMIT_ALL, + AUTHENTICATED, + ROLE_PROTECTED + } + + public record RouteAuthorizationPolicy(HttpMethod method, String pattern, AccessLevel accessLevel, String[] roles) { + public static RouteAuthorizationPolicy permitAll(HttpMethod method, String pattern) { + return new RouteAuthorizationPolicy(method, pattern, AccessLevel.PERMIT_ALL, new String[0]); + } + + public static RouteAuthorizationPolicy authenticated(HttpMethod method, String pattern) { + return new RouteAuthorizationPolicy(method, pattern, AccessLevel.AUTHENTICATED, new String[0]); + } + + public static RouteAuthorizationPolicy roles(HttpMethod method, String pattern, String... roles) { + return new RouteAuthorizationPolicy(method, pattern, AccessLevel.ROLE_PROTECTED, roles); + } + + public RequestMatcher toRequestMatcher() { + return method == null + ? new AntPathRequestMatcher(pattern) + : new AntPathRequestMatcher(pattern, method.name()); + } + } + + private record ApiTokenPolicy(HttpMethod method, String pattern, String requiredScope) { + static ApiTokenPolicy allow(HttpMethod method, String pattern) { + return new ApiTokenPolicy(method, pattern, null); + } + + static ApiTokenPolicy require(HttpMethod method, String pattern, String requiredScope) { + return new ApiTokenPolicy(method, pattern, requiredScope); + } + + boolean matches(String requestMethod, String requestPath, AntPathMatcher matcher) { + if (method != null && (requestMethod == null || !method.name().equalsIgnoreCase(requestMethod))) { + return false; + } + return matcher.match(pattern, requestPath); + } + } +} 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 index cb868267..29eb6993 100644 --- 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 @@ -2,8 +2,8 @@ package com.iflytek.skillhub.auth.token; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import org.springframework.stereotype.Service; -import org.springframework.util.AntPathMatcher; import java.util.LinkedHashSet; import java.util.List; @@ -19,47 +19,13 @@ 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/auth/device/**"), - ScopeRule.allow(null, "/api/v1/check"), - ScopeRule.allow("GET", "/api/v1/whoami"), - ScopeRule.allow("GET", "/api/v1/search"), - ScopeRule.allow("GET", "/api/v1/skills"), - ScopeRule.allow("GET", "/api/v1/skills/**"), - ScopeRule.allow("GET", "/api/web/skills"), - ScopeRule.allow("GET", "/api/web/skills/**"), - ScopeRule.allow("GET", "/api/v1/namespaces"), - ScopeRule.allow("GET", "/api/v1/namespaces/*"), - ScopeRule.allow("GET", "/api/web/namespaces"), - ScopeRule.allow("GET", "/api/web/namespaces/*"), - ScopeRule.allow("GET", "/api/v1/search"), - ScopeRule.allow("GET", "/api/v1/resolve/**"), - ScopeRule.allow("GET", "/api/v1/whoami"), - ScopeRule.allow("GET", "/api/v1/download"), - 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", "skill:publish"), - ScopeRule.require("POST", "/api/v1/skills/*/publish", "skill:publish"), - ScopeRule.require("POST", "/api/web/skills/*/publish", "skill:publish"), - ScopeRule.require("POST", "/api/v1/publish", "skill:publish"), - ScopeRule.require("POST", "/api/v1/publish", "skill:publish") - ); - private final ObjectMapper objectMapper; - private final AntPathMatcher pathMatcher = new AntPathMatcher(); + private final RouteSecurityPolicyRegistry routeSecurityPolicyRegistry; - public ApiTokenScopeService(ObjectMapper objectMapper) { + public ApiTokenScopeService(ObjectMapper objectMapper, + RouteSecurityPolicyRegistry routeSecurityPolicyRegistry) { this.objectMapper = objectMapper; + this.routeSecurityPolicyRegistry = routeSecurityPolicyRegistry; } public Set parseScopes(String scopeJson) { @@ -85,32 +51,17 @@ public class ApiTokenScopeService { } public AuthorizationDecision authorize(String method, String path, Set tokenScopes) { - if (!isApiPath(path)) { + RouteSecurityPolicyRegistry.ApiTokenAuthorizationDecision decision = + routeSecurityPolicyRegistry.authorizeApiToken(method, path, tokenScopes); + if (decision.allowed()) { return AuthorizationDecision.allow(); } - - for (ScopeRule rule : UNSCOPED_ALLOWED_RULES) { - if (rule.matches(method, path, pathMatcher)) { - return AuthorizationDecision.allow(); - } + if (decision.requiredScope() != null) { + return AuthorizationDecision.missingScope(decision.requiredScope()); } - - 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/web/") || path.startsWith("/api/")); - } - public record AuthorizationDecision(boolean allowed, String requiredScope, String message) { public static AuthorizationDecision allow() { return new AuthorizationDecision(true, null, null); @@ -124,21 +75,4 @@ public class ApiTokenScopeService { 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/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java new file mode 100644 index 00000000..e97961f0 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -0,0 +1,36 @@ +package com.iflytek.skillhub.auth.policy; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Set; +import org.junit.jupiter.api.Test; + +class RouteSecurityPolicyRegistryTest { + + private final RouteSecurityPolicyRegistry registry = new RouteSecurityPolicyRegistry(); + + @Test + void authorizeApiToken_requiresPublishScopeForPublishEndpoints() { + var denied = registry.authorizeApiToken("POST", "/api/web/skills/global/publish", Set.of("skill:read")); + var allowed = registry.authorizeApiToken("POST", "/api/web/skills/global/publish", Set.of("skill:publish")); + + assertFalse(denied.allowed()); + assertEquals("skill:publish", denied.requiredScope()); + assertTrue(allowed.allowed()); + } + + @Test + void shouldIgnoreCsrf_forBearerAndApiPaths() { + assertTrue(registry.shouldIgnoreCsrf("/api/v1/admin/users", null)); + assertTrue(registry.shouldIgnoreCsrf("/not-api", "Bearer token")); + assertFalse(registry.shouldIgnoreCsrf("/ui/settings", null)); + } + + @Test + void shouldProjectRequestContext_onlyForApiRoutes() { + assertTrue(registry.shouldProjectRequestContext("/api/web/namespaces/team-a")); + assertFalse(registry.shouldProjectRequestContext("/assets/index.css")); + } +} 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 index f40e542c..e82f7a1a 100644 --- 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 @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.token; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import com.iflytek.skillhub.auth.entity.ApiToken; import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; @@ -31,7 +32,8 @@ 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 ApiTokenScopeService scopeService = + new ApiTokenScopeService(new ObjectMapper(), new RouteSecurityPolicyRegistry()); private final ApiTokenAuthenticationFilter filter = new ApiTokenAuthenticationFilter( apiTokenService, userAccountRepository, 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 index 735fddfc..9156eba8 100644 --- 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 @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.token; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import jakarta.servlet.FilterChain; import jakarta.servlet.http.HttpServletRequest; @@ -27,7 +28,8 @@ import static org.mockito.Mockito.verify; class ApiTokenScopeFilterTest { - private final ApiTokenScopeService scopeService = new ApiTokenScopeService(new ObjectMapper()); + private final ApiTokenScopeService scopeService = + new ApiTokenScopeService(new ObjectMapper(), new RouteSecurityPolicyRegistry()); @AfterEach void clearSecurityContext() { 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 index 002ec8a7..cc877bf3 100644 --- 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 @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.token; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import org.junit.jupiter.api.Test; import java.util.Set; @@ -11,7 +12,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; class ApiTokenScopeServiceTest { - private final ApiTokenScopeService scopeService = new ApiTokenScopeService(new ObjectMapper()); + private final ApiTokenScopeService scopeService = + new ApiTokenScopeService(new ObjectMapper(), new RouteSecurityPolicyRegistry()); @Test void parseScopesShouldNormalizeJsonArray() { @@ -74,4 +76,26 @@ class ApiTokenScopeServiceTest { assertFalse(decision.allowed()); assertEquals("API token cannot access endpoint: /api/v1/me/skills", decision.message()); } + + @Test + void authorizeShouldAllowPublicNamespaceReadWithoutScope() { + ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( + "GET", + "/api/v1/namespaces/team-a", + Set.of() + ); + + assertTrue(decision.allowed()); + } + + @Test + void authorizeShouldPermitAuthMeWithoutScope() { + ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( + "GET", + "/api/v1/auth/me", + Set.of() + ); + + assertTrue(decision.allowed()); + } }