diff --git a/cli/README.md b/cli/README.md index 57a0e4959..6d98df803 100644 --- a/cli/README.md +++ b/cli/README.md @@ -112,6 +112,9 @@ Logout only removes the token for the specified registry, preserving registry co # Keyword search skillhub search pdf +# Search with a one-off token +skillhub search pdf --token sk_xxx + # List all skills (empty query) skillhub search "" --limit 50 @@ -333,7 +336,7 @@ Update mechanism: | `skillhub login --token [--registry ] [--json]` | Save token and registry configuration | | `skillhub logout [--registry ] [--json]` | Remove token for specified registry | | `skillhub whoami [--registry ] [--token ] [--json]` | Validate current token and display user information | -| `skillhub search [--registry ] [--limit ] [--json]` | Search published skills | +| `skillhub search [--registry ] [--token ] [--limit ] [--json]` | Search published skills | | `skillhub install [--scope ] [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--registry ] [--token ] [--json]` | Install a skill | | `skillhub list [--agent ] [--dir ] [--registry ] [--json]` | List installed skills | | `skillhub remove [--agent ] [--all] [--remote] [--hard] [--namespace ] [--registry ] [--token ] [--json]` | Remove a skill | diff --git a/cli/src/commands/help.ts b/cli/src/commands/help.ts index 083d72aae..9b35f3b4b 100644 --- a/cli/src/commands/help.ts +++ b/cli/src/commands/help.ts @@ -28,8 +28,8 @@ export const commands = { }, search: { summary: 'Search published skills', - usage: 'skillhub search [query] [--limit ] [--registry ] [--json]', - examples: ['skillhub search', 'skillhub search pdf'] + usage: 'skillhub search [query] [--limit ] [--registry ] [--token ] [--json]', + examples: ['skillhub search', 'skillhub search pdf', 'skillhub search pdf --token sk_xxx'] }, install: { summary: 'Install a skill locally', diff --git a/cli/src/index.ts b/cli/src/index.ts index 15a7eb84e..512b5b1b2 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -223,9 +223,10 @@ cli cli .command('search [query]', 'Search published skills') .option('--registry ', 'Registry URL') + .option('--token ', 'API token') .option('--limit ', 'Max results', { default: 20 }) .option('--json', 'Output JSON') - .action((query: string | undefined, options: { registry?: string; limit?: number; json?: boolean }) => { + .action((query: string | undefined, options: { registry?: string; token?: string; limit?: number; json?: boolean }) => { return runCommand(() => searchCommand(query ?? '', options), Boolean(options.json)) }) diff --git a/cli/test/integration/install-command.test.ts b/cli/test/integration/install-command.test.ts index 134672f68..a4ca3bd54 100644 --- a/cli/test/integration/install-command.test.ts +++ b/cli/test/integration/install-command.test.ts @@ -218,6 +218,65 @@ describe('install command — P1', () => { expect(result.stderr.toLowerCase()).toMatch(/auth|unauthorized|401/) }) + test('bad token stops on 401 without retrying resolve anonymously', async () => { + const env = await createTempHome() + const installDir = join(env.cwd, 'skills-no-anon-retry') + await mkdir(installDir, { recursive: true }) + + const resolveAuthHeaders: Array = [] + let downloadRequests = 0 + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + const resolveMatch = url.pathname.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/([^/]+)\/resolve$/) + if (resolveMatch) { + const auth = req.headers.get('authorization') + resolveAuthHeaders.push(auth) + if (auth === 'Bearer sk_bad') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ + code: 0, + data: { + namespace: resolveMatch[1], + slug: resolveMatch[2], + version: '1.0.0', + versionId: 1, + fingerprint: 'abc123', + downloadUrl: `${url.protocol}//${url.host}/api/cli/v1/skills/${resolveMatch[1]}/${resolveMatch[2]}/download` + } + }) + } + if (url.pathname.endsWith('/download')) { + downloadRequests += 1 + return new Response(makeSkillZip() as BodyInit, { + status: 200, + headers: { 'Content-Type': 'application/zip' } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registryUrl, '--token', 'sk_bad'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('Error: authentication failed') + expect(result.stderr).toContain(`Context: registry ${registryUrl}`) + expect(result.stderr).toContain('Next:') + expect(resolveAuthHeaders).toEqual(['Bearer sk_bad']) + expect(downloadRequests).toBe(0) + } finally { + server.stop() + } + }) + // ------------------------------------------------------------------------- // P1 — --namespace override // ------------------------------------------------------------------------- diff --git a/cli/test/integration/search-command.test.ts b/cli/test/integration/search-command.test.ts index 1902142c0..f352b158e 100644 --- a/cli/test/integration/search-command.test.ts +++ b/cli/test/integration/search-command.test.ts @@ -10,6 +10,109 @@ afterEach(() => { }) describe('search command', () => { + test('--token sends bearer auth and takes priority over SKILLHUB_TOKEN', async () => { + let capturedAuth = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedAuth = req.headers.get('authorization') ?? '' + return Response.json({ + code: 0, + data: { + items: [{ namespace: 'global', slug: 'pdf-parser', latestVersion: '1.2.0', summary: 'Parse PDFs' }], + total: 1, + limit: 20 + } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const result = await runCli( + ['search', 'pdf', '--registry', `http://localhost:${server.port}`, '--token', 'sk_ok'], + { SKILLHUB_TOKEN: 'sk_bad' } + ) + + expect(result.exitCode).toBe(0) + expect(capturedAuth).toBe('Bearer sk_ok') + expect(result.stdout).toContain('global/pdf-parser') + } finally { + server.stop() + } + }) + + test('bad --token fails with auth output and does not retry anonymously', async () => { + const authHeaders: Array = [] + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + const auth = req.headers.get('authorization') + authHeaders.push(auth) + if (auth === 'Bearer sk_bad') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ + code: 0, + data: { + items: [{ namespace: 'global', slug: 'anonymous-only', latestVersion: '1.0.0', summary: 'anonymous fallback' }], + total: 1, + limit: 20 + } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli(['search', 'pdf', '--registry', registryUrl, '--token', 'sk_bad']) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('Error: authentication failed') + expect(result.stderr).toContain(`Context: registry ${registryUrl}`) + expect(result.stderr).toContain('Next:') + expect(authHeaders).toEqual(['Bearer sk_bad']) + } finally { + server.stop() + } + }) + + test('bad --token returns structured json auth error', async () => { + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli(['search', 'pdf', '--registry', registryUrl, '--token', 'sk_bad', '--json']) + + expect(result.exitCode).toBe(2) + const parsed = JSON.parse(result.stderr) + expect(parsed.ok).toBe(false) + expect(parsed.message).toBe('authentication failed') + expect(parsed.exitCode).toBe(2) + expect(parsed.details.registry).toBe(registryUrl) + expect(typeof parsed.details.next).toBe('string') + expect(parsed.details.next).toContain('skillhub login') + } finally { + server.stop() + } + }) + test('prints compact search table', async () => { registry = await startFakeRegistry({ searchItems: [{ namespace: 'global', slug: 'pdf-parser', latestVersion: '1.2.0', summary: 'Parse PDFs' }] diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index db7c8b22a..7a7e46d63 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -377,6 +377,7 @@ API Token 仍保留，但定位从“CLI 唯一认证方式”调整为“平台 - 用途：自动化脚本、兼容层调用、手工 Token 管理、后续系统集成 - 存储：只存 SHA-256 哈希，明文只展示一次 - 校验：从 `Authorization: Bearer ` 提取 → 哈希比对 → 加载关联用户 → 检查用户状态 +- 失败闭合：公共读接口只有在缺少 `Authorization` 头时才按匿名访问处理；只要出现 Bearer 凭证，空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 401，不能回退为匿名访问 - 作用域：`skill:read`, `skill:publish`, `skill:delete`, `token:manage` > **一期作用域说明（非最小权限）**：一期 Token 作用域为粗粒度动作级别，不与 namespace 绑定。Token 继承用户的全部权限——如果用户是某个 namespace 的 MEMBER，则该用户的任何 Token（只要包含 `skill:publish` scope）都可以向该 namespace 发布技能。这是有意的一期简化，不满足最小权限原则。后续版本计划引入 namespace 级别的 Token 作用域限定（如 `namespace:ai-team:skill:publish`），或通过 `api_token_scope` 子表实现 Token 与 namespace 的绑定。 @@ -604,8 +605,8 @@ window.location.href = '/oauth2/authorization/github' | `GET /api/v1/skills`（搜索） | 仅 `PUBLIC`，且仅搜索 `ACTIVE`、非 hidden、已索引 skill | `PUBLIC + NAMESPACE_ONLY（成员空间）+ PRIVATE（owner/admin）` | `SearchVisibilityScope` + 搜索索引状态 | | `GET /api/v1/skills/{ns}/{slug}` | 仅已发布且可见的 `PUBLIC` skill | 同左，另加 owner 可读未发布 skill、namespace `ADMIN` / `OWNER` 可读 hidden | `visibility + latest_version_id + hidden + namespace 成员关系` | | `GET /api/v1/skills/{ns}/{slug}/versions` | 仅 `PUBLISHED` 版本 | owner / namespace `ADMIN` / `OWNER` 可见全部五种状态 | 同上 + version status 过滤 | -| `GET /api/v1/skills/{ns}/{slug}/download` | 仅全局 namespace 下的 `PUBLIC` skill 支持匿名下载 | 已登录后按 visibility 判定；下载目标版本必须是 `PUBLISHED` | visibility + namespace type + version status | -| `GET /api/v1/skills/{ns}/{slug}/resolve` | 仅全局 namespace 下的 `PUBLIC` skill 可匿名 | 同上 | visibility + namespace type + version status | +| `GET /api/v1/skills/{ns}/{slug}/download` | 仅 `PUBLIC`、`ACTIVE`、非 hidden、命名空间未归档且目标版本可安装的 skill 支持匿名下载 | 已登录后按 visibility 判定；下载目标版本必须可安装 | visibility + namespace status + `SkillInstallability` | +| `GET /api/v1/skills/{ns}/{slug}/resolve` | 仅 `PUBLIC`、`ACTIVE`、非 hidden、命名空间未归档且目标版本可安装的 skill 可匿名 | 同上 | visibility + namespace status + `SkillInstallability` | | `GET /api/v1/namespaces` | 全部 | 全部 | 无限制 | ### 10.2 Authenticated API @@ -654,6 +655,6 @@ window.location.href = '/oauth2/authorization/github' |------|---------|---------| | `GET /api/v1/whoami` | 任意有效 Bearer Token | 无 | | `GET /api/v1/search` | 可选（匿名限 PUBLIC） | `SearchVisibilityScope` | -| `GET /api/v1/resolve` | 可选（匿名仅限全局 namespace 下的 PUBLIC） | visibility + namespace type + version status | -| `GET /api/v1/download/{slug}/{version}` | 可选（匿名仅限全局 namespace 下的 PUBLIC） | visibility + namespace type + version status | +| `GET /api/v1/resolve` | 可选（匿名仅限 `PUBLIC`、`ACTIVE`、非 hidden、命名空间未归档且目标版本可安装） | visibility + namespace status + `SkillInstallability` | +| `GET /api/v1/download/{slug}/{version}` | 可选（匿名仅限 `PUBLIC`、`ACTIVE`、非 hidden、命名空间未归档且目标版本可安装） | visibility + namespace status + `SkillInstallability` | | `POST /api/v1/publish` | Bearer Token + `skill:publish` | 普通用户要求目标 namespace 成员；`SUPER_ADMIN` 可绕过（namespace 由 canonical slug 解析） | diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java index bffa778a9..634106785 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java @@ -85,7 +85,20 @@ public SearchResponse search( SearchVisibilityScope scope = buildVisibilityScope(userId, userNsRoles); - return searchVisibleSkills(keyword, namespaceId, sortBy != null ? sortBy : "newest", page, size, labelSlugs, scope); + return searchVisibleSkills(keyword, namespaceId, sortBy != null ? sortBy : "newest", page, size, labelSlugs, scope, false); + } + + public SearchResponse searchInstallableLatest( + String keyword, + String namespaceSlug, + String sortBy, + int page, + int size, + String userId, + Map userNsRoles) { + Long namespaceId = resolveNamespaceId(namespaceSlug, userId, userNsRoles); + SearchVisibilityScope scope = buildVisibilityScope(userId, userNsRoles); + return searchVisibleSkills(keyword, namespaceId, sortBy != null ? sortBy : "newest", page, size, List.of(), scope, true); } private Long resolveNamespaceId(String namespaceSlug, String userId, Map userNsRoles) { @@ -133,7 +146,8 @@ private SearchResponse searchVisibleSkills( int page, int size, List labelSlugs, - SearchVisibilityScope scope) { + SearchVisibilityScope scope, + boolean requireInstallableLatest) { SearchResult result = searchQueryService.search(new SearchQuery( keyword, namespaceId, @@ -141,7 +155,8 @@ private SearchResponse searchVisibleSkills( sortBy, page, size, - normalizeLabelSlugs(labelSlugs) + normalizeLabelSlugs(labelSlugs), + requireInstallableLatest )); List pageItems = mapVisibleSkillSummaries(result.skillIds()); return new SearchResponse(pageItems, result.total(), page, size); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java index e431eaf33..1fcd2e259 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java @@ -51,7 +51,7 @@ public record CliSearchItem(String namespace, String slug, String latestVersion, public record CliSearchResult(List items, long total, int limit) {} public CliSearchResult search(String q, int limit, String userId, Map userNsRoles) { - SkillSearchAppService.SearchResponse response = skillSearchAppService.search( + SkillSearchAppService.SearchResponse response = skillSearchAppService.searchInstallableLatest( q, null, "newest", 0, limit, userId, userNsRoles ); @@ -59,7 +59,7 @@ public CliSearchResult search(String q, int limit, String userId, Map new CliSearchItem( item.namespace(), item.slug(), - item.publishedVersion() != null ? item.publishedVersion().version() : null, + item.publishedVersion().version(), item.summary() )) .toList(); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliDryRunValidateTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliDryRunValidateTest.java index d82aa32dd..64f8c7c9c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliDryRunValidateTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliDryRunValidateTest.java @@ -1,7 +1,11 @@ package com.iflytek.skillhub.controller.cli; -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.entity.ApiToken; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.auth.token.ApiTokenService; import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.dto.cli.CliDryRunResponse; import com.iflytek.skillhub.service.cli.CliSkillAppService; import org.junit.jupiter.api.Test; @@ -10,18 +14,16 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.mock.web.MockMultipartFile; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import java.util.List; +import java.util.Optional; import java.util.Set; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; -import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -32,18 +34,22 @@ class CliDryRunValidateTest { @Autowired MockMvc mockMvc; @MockBean CliSkillAppService cliSkillAppService; + @MockBean ApiTokenService apiTokenService; + @MockBean UserAccountRepository userAccountRepository; + @MockBean UserRoleBindingRepository userRoleBindingRepository; - private UsernamePasswordAuthenticationToken auth() { - PlatformPrincipal principal = new PlatformPrincipal( - "user-1", "tester", "t@example.com", "", "api_token", Set.of("USER")); - return new UsernamePasswordAuthenticationToken( - principal, null, List.of( - new SimpleGrantedAuthority("ROLE_USER"), - new SimpleGrantedAuthority("SCOPE_skill:publish"))); + private void givenValidPublishToken() { + ApiToken token = new ApiToken("user-1", "cli", "sk_test", "hash", "[\"skill:publish\"]"); + UserAccount user = new UserAccount("user-1", "tester", "t@example.com", ""); + + given(apiTokenService.validateToken("test-token")).willReturn(Optional.of(token)); + given(userAccountRepository.findById("user-1")).willReturn(Optional.of(user)); + given(userRoleBindingRepository.findByUserId("user-1")).willReturn(List.of()); } @Test void validatePublish_returnsValidResult() throws Exception { + givenValidPublishToken(); given(cliSkillAppService.validatePublish( eq("global"), any(), eq("user-1"), eq(SkillVisibility.PUBLIC), eq(Set.of("USER")))) .willReturn(new CliDryRunResponse( @@ -55,8 +61,7 @@ void validatePublish_returnsValidResult() throws Exception { mockMvc.perform(multipart("/api/cli/v1/skills/global/publish/validate") .file(file) - .header("Authorization", "Bearer test-token") - .with(authentication(auth()))) + .header("Authorization", "Bearer test-token")) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.valid").value(true)) .andExpect(jsonPath("$.data.resolvedSlug").value("my-skill")) @@ -65,6 +70,7 @@ void validatePublish_returnsValidResult() throws Exception { @Test void validatePublish_returnsInvalidResult() throws Exception { + givenValidPublishToken(); given(cliSkillAppService.validatePublish( eq("global"), any(), eq("user-1"), eq(SkillVisibility.PUBLIC), eq(Set.of("USER")))) .willReturn(new CliDryRunResponse( @@ -76,8 +82,7 @@ void validatePublish_returnsInvalidResult() throws Exception { mockMvc.perform(multipart("/api/cli/v1/skills/global/publish/validate") .file(file) - .header("Authorization", "Bearer test-token") - .with(authentication(auth()))) + .header("Authorization", "Bearer test-token")) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.valid").value(false)) .andExpect(jsonPath("$.data.errors[0]").value("Missing required file: SKILL.md at root")) @@ -86,6 +91,7 @@ void validatePublish_returnsInvalidResult() throws Exception { @Test void validatePublish_acceptsCustomVisibility() throws Exception { + givenValidPublishToken(); given(cliSkillAppService.validatePublish( eq("global"), any(), eq("user-1"), eq(SkillVisibility.PRIVATE), eq(Set.of("USER")))) .willReturn(new CliDryRunResponse( @@ -97,22 +103,21 @@ void validatePublish_acceptsCustomVisibility() throws Exception { mockMvc.perform(multipart("/api/cli/v1/skills/global/publish/validate") .file(file) .file(new MockMultipartFile("visibility", "", "text/plain", "PRIVATE".getBytes())) - .header("Authorization", "Bearer test-token") - .with(authentication(auth()))) + .header("Authorization", "Bearer test-token")) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.valid").value(true)); } @Test void validatePublish_rejectsInvalidVisibility() throws Exception { + givenValidPublishToken(); MockMultipartFile file = new MockMultipartFile("file", "skill.zip", "application/zip", new byte[]{0x50, 0x4B, 0x03, 0x04}); mockMvc.perform(multipart("/api/cli/v1/skills/global/publish/validate") .file(file) .file(new MockMultipartFile("visibility", "", "text/plain", "BOGUS".getBytes())) - .header("Authorization", "Bearer test-token") - .with(authentication(auth()))) + .header("Authorization", "Bearer test-token")) .andExpect(status().isBadRequest()); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliSkillControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliSkillControllerTest.java index b2421c891..9f6fe695a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliSkillControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliSkillControllerTest.java @@ -1,17 +1,27 @@ package com.iflytek.skillhub.controller.cli; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.entity.ApiToken; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.ratelimit.RateLimit; import com.iflytek.skillhub.service.cli.CliSkillAppService; import jakarta.servlet.http.HttpServletRequest; +import java.io.ByteArrayInputStream; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.core.io.InputStreamResource; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import org.springframework.web.bind.annotation.PostMapping; @@ -19,13 +29,17 @@ import java.lang.reflect.Method; import java.util.List; -import java.util.Set; +import java.util.Map; +import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.BDDMockito.given; -import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -35,7 +49,11 @@ @ActiveProfiles("test") class CliSkillControllerTest { @Autowired MockMvc mockMvc; + @Autowired NamespaceMemberRepository namespaceMemberRepository; @MockBean CliSkillAppService cliSkillAppService; + @MockBean ApiTokenService apiTokenService; + @MockBean UserAccountRepository userAccountRepository; + @MockBean UserRoleBindingRepository userRoleBindingRepository; @Test void downloadRoutesUseDownloadRateLimit() throws Exception { @@ -73,6 +91,47 @@ void searchReturnsCompactCliResponse() throws Exception { .andExpect(jsonPath("$.data.items[0].latestVersion").value("1.2.0")); } + @Test + void searchRejectsInvalidBearerBeforeAnonymousAccess() throws Exception { + givenInvalidBearerToken(); + given(cliSkillAppService.search("pdf", 20, null, null)).willReturn( + new CliSkillAppService.CliSearchResult(List.of(), 0, 20) + ); + + mockMvc.perform(get("/api/cli/v1/skills/search") + .param("q", "pdf") + .param("limit", "20") + .header(HttpHeaders.AUTHORIZATION, "Bearer unknown-token")) + .andExpect(status().isUnauthorized()); + + verifyNoInteractions(cliSkillAppService); + } + + @Test + void searchWithValidBearerProjectsIdentityAndNamespaceRoles() throws Exception { + ApiToken token = new ApiToken("user-cli-token", "cli", "sk_test", "hash", "[]"); + UserAccount user = new UserAccount("user-cli-token", "CLI User", "cli@example.com", ""); + Map nsRoles = Map.of(9L, NamespaceRole.MEMBER); + + given(apiTokenService.validateToken("raw-token")).willReturn(Optional.of(token)); + given(userAccountRepository.findById("user-cli-token")).willReturn(Optional.of(user)); + given(userRoleBindingRepository.findByUserId("user-cli-token")).willReturn(List.of()); + namespaceMemberRepository.save(new NamespaceMember(9L, "user-cli-token", NamespaceRole.MEMBER)); + given(cliSkillAppService.search("private", 20, "user-cli-token", nsRoles)).willReturn( + new CliSkillAppService.CliSearchResult(List.of(), 0, 20) + ); + + mockMvc.perform(get("/api/cli/v1/skills/search") + .param("q", "private") + .param("limit", "20") + .header(HttpHeaders.AUTHORIZATION, "Bearer raw-token")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items").isArray()); + + verify(cliSkillAppService).search("private", 20, "user-cli-token", nsRoles); + verify(apiTokenService).touchLastUsed(token); + } + @Test void resolveReturnsCliResolveResponse() throws Exception { given(cliSkillAppService.resolve("global", "demo", null, null, null)).willReturn( @@ -91,6 +150,47 @@ void resolveReturnsCliResolveResponse() throws Exception { .andExpect(jsonPath("$.data.fingerprint").value("abc123")); } + @Test + void resolveRejectsInvalidBearerBeforeAnonymousAccess() throws Exception { + givenInvalidBearerToken(); + given(cliSkillAppService.resolve("global", "demo", null, null, null)).willReturn( + new com.iflytek.skillhub.dto.cli.CliResolveResponse( + "global", "demo", "2.0.0", 42L, "abc123", + "/api/v1/skills/global/demo/versions/2.0.0/download" + ) + ); + + mockMvc.perform(get("/api/cli/v1/skills/global/demo/resolve") + .header(HttpHeaders.AUTHORIZATION, "Bearer unknown-token")) + .andExpect(status().isUnauthorized()); + + verify(cliSkillAppService, never()).resolve(any(), any(), any(), any(), any()); + } + + @Test + void downloadLatestRejectsInvalidBearerBeforeAnonymousAccess() throws Exception { + givenInvalidBearerToken(); + given(cliSkillAppService.downloadLatest(any(), any(), any())).willReturn(downloadResponse()); + + mockMvc.perform(get("/api/cli/v1/skills/global/demo/download") + .header(HttpHeaders.AUTHORIZATION, "Bearer unknown-token")) + .andExpect(status().isUnauthorized()); + + verify(cliSkillAppService, never()).downloadLatest(any(), any(), any()); + } + + @Test + void downloadVersionRejectsInvalidBearerBeforeAnonymousAccess() throws Exception { + givenInvalidBearerToken(); + given(cliSkillAppService.downloadVersion(any(), any(), any(), any())).willReturn(downloadResponse()); + + mockMvc.perform(get("/api/cli/v1/skills/global/demo/versions/1.0.0/download") + .header(HttpHeaders.AUTHORIZATION, "Bearer unknown-token")) + .andExpect(status().isUnauthorized()); + + verify(cliSkillAppService, never()).downloadVersion(any(), any(), any(), any()); + } + @Test void deleteRequiresAuthentication() throws Exception { mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders @@ -100,13 +200,12 @@ void deleteRequiresAuthentication() throws Exception { @Test void deleteReturnsCliDeleteResponse() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal( - "user-1", "tester", "t@example.com", "", "api_token", Set.of("USER")); - var auth = new UsernamePasswordAuthenticationToken( - principal, null, List.of( - new SimpleGrantedAuthority("ROLE_USER"), - new SimpleGrantedAuthority("SCOPE_skill:delete"))); + ApiToken token = new ApiToken("user-1", "cli", "sk_test", "hash", "[\"skill:delete\"]"); + UserAccount user = new UserAccount("user-1", "tester", "t@example.com", ""); + given(apiTokenService.validateToken("test-token")).willReturn(Optional.of(token)); + given(userAccountRepository.findById("user-1")).willReturn(Optional.of(user)); + given(userRoleBindingRepository.findByUserId("user-1")).willReturn(List.of()); given(cliSkillAppService.deleteRemote( org.mockito.ArgumentMatchers.eq("global"), org.mockito.ArgumentMatchers.eq("demo"), @@ -118,8 +217,7 @@ void deleteReturnsCliDeleteResponse() throws Exception { mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders .delete("/api/cli/v1/skills/global/demo") - .header("Authorization", "Bearer test-token") - .with(authentication(auth))) + .header("Authorization", "Bearer test-token")) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.ok").value(true)) .andExpect(jsonPath("$.data.namespace").value("global")) @@ -132,4 +230,12 @@ private static void assertDownloadRateLimit(RateLimit rateLimit) { assertEquals(120, rateLimit.authenticated()); assertEquals(30, rateLimit.anonymous()); } + + private static ResponseEntity downloadResponse() { + return ResponseEntity.ok(new InputStreamResource(new ByteArrayInputStream("zip".getBytes()))); + } + + private void givenInvalidBearerToken() { + given(apiTokenService.validateToken("unknown-token")).willReturn(Optional.empty()); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java index fdac46f83..3cd408e45 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java @@ -8,7 +8,9 @@ import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillLifecycleProjectionService; import com.iflytek.skillhub.search.SearchQuery; @@ -22,11 +24,13 @@ import org.mockito.ArgumentCaptor; import org.mockito.junit.jupiter.MockitoExtension; +import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; @@ -96,8 +100,6 @@ void search_shouldFillVisiblePageAcrossArchivedNamespaceResults() { when(skillRepository.findByIdIn(List.of(11L))).thenReturn(List.of(visibleSkill)); when(namespaceRepository.findByIdIn(List.of(2L))).thenReturn(List.of(activeNamespace)); when(skillVersionRepository.findByIdIn(List.of(111L))).thenReturn(List.of()); - when(skillVersionRepository.findBySkillIdInAndStatus(List.of(11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of()); SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 1, null, null); @@ -145,8 +147,6 @@ void search_shouldExcludeHiddenSkillsForRegularUsers() { when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(visibleSkill)); when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of()); - when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of()); SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 20, "user-9", Map.of()); @@ -164,6 +164,9 @@ void search_shouldResolvePublishedVersionsInBatch() { setField(second, "id", 11L); second.setLatestVersionId(102L); + SkillVersion firstVersion = publishedVersion(10L, 101L, "1.0.0"); + SkillVersion secondVersion = publishedVersion(11L, 102L, "2.0.0"); + Namespace namespace = new Namespace("team-a", "Team A", "owner-1"); setField(namespace, "id", 1L); namespace.setStatus(NamespaceStatus.ACTIVE); @@ -172,18 +175,112 @@ void search_shouldResolvePublishedVersionsInBatch() { .thenReturn(new SearchResult(List.of(10L, 11L), 2, 0, 20)); when(skillRepository.findByIdIn(List.of(10L, 11L))).thenReturn(List.of(first, second)); when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); - when(skillVersionRepository.findByIdIn(List.of(101L, 102L))).thenReturn(List.of()); - when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L, 11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of()); + when(skillVersionRepository.findByIdIn(List.of(101L, 102L))).thenReturn(List.of(firstVersion, secondVersion)); SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null); assertEquals(2, response.items().size()); + assertEquals("1.0.0", response.items().get(0).publishedVersion().version()); + assertEquals("2.0.0", response.items().get(1).publishedVersion().version()); verify(skillVersionRepository, times(1)).findByIdIn(List.of(101L, 102L)); - verify(skillVersionRepository, times(1)) + verify(skillVersionRepository, times(0)) .findBySkillIdInAndStatus(List.of(10L, 11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED); } + @Test + void search_shouldNotFallbackToOlderPublishedVersionWhenLatestIsMissing() { + Skill skill = new Skill(1L, "missing-latest", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 10L); + + SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0"); + + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setField(namespace, "id", 1L); + namespace.setStatus(NamespaceStatus.ACTIVE); + + when(searchQueryService.search(any())) + .thenReturn(new SearchResult(List.of(10L), 1, 0, 20)); + when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill)); + when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED)) + .thenReturn(List.of(oldInstallable)); + + SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null); + + assertEquals(1, response.items().size()); + assertEquals("missing-latest", response.items().getFirst().slug()); + assertNull(response.items().getFirst().publishedVersion()); + verify(skillVersionRepository, times(0)) + .findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED); + } + + @Test + void search_shouldNotFallbackToOlderPublishedVersionWhenLatestIsYanked() { + Skill skill = new Skill(1L, "yanked-latest", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 10L); + skill.setLatestVersionId(101L); + + SkillVersion latest = publishedVersion(10L, 101L, "1.0.0"); + latest.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0"); + + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setField(namespace, "id", 1L); + namespace.setStatus(NamespaceStatus.ACTIVE); + + when(searchQueryService.search(any())) + .thenReturn(new SearchResult(List.of(10L), 1, 0, 20)); + when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill)); + when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); + when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of(latest)); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED)) + .thenReturn(List.of(oldInstallable)); + + SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null); + + assertEquals(1, response.items().size()); + assertEquals("yanked-latest", response.items().getFirst().slug()); + assertNull(response.items().getFirst().publishedVersion()); + verify(skillVersionRepository, times(0)) + .findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED); + } + + @Test + void search_shouldNotFallbackToOlderPublishedVersionWhenLatestDownloadUnavailable() { + Skill skill = new Skill(1L, "not-ready", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 10L); + skill.setLatestVersionId(101L); + + SkillVersion version = new SkillVersion(10L, "1.0.0", "owner-1"); + setField(version, "id", 101L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(false); + SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0"); + + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setField(namespace, "id", 1L); + namespace.setStatus(NamespaceStatus.ACTIVE); + + when(searchQueryService.search(any())) + .thenReturn(new SearchResult(List.of(10L), 1, 0, 20)); + when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill)); + when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); + when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of(version)); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED)) + .thenReturn(List.of(oldInstallable)); + + SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null); + + assertEquals(1, response.items().size()); + assertEquals("not-ready", response.items().getFirst().slug()); + assertNull(response.items().getFirst().publishedVersion()); + verify(skillVersionRepository, times(0)) + .findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED); + } + @Test void search_shouldNormalizeAndPassLabelSlugs() { when(searchQueryService.search(any())) @@ -240,4 +337,12 @@ private void setField(Object target, String fieldName, Object value) { throw new RuntimeException(e); } } + + private SkillVersion publishedVersion(Long skillId, Long versionId, String versionNumber) { + SkillVersion version = new SkillVersion(skillId, versionNumber, "owner-1"); + setField(version, "id", versionId); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + return version; + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/cli/CliSkillAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/cli/CliSkillAppServiceTest.java index b7fbe1d7f..0c75ca341 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/cli/CliSkillAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/cli/CliSkillAppServiceTest.java @@ -1,9 +1,18 @@ package com.iflytek.skillhub.service.cli; import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceService; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillDownloadService; +import com.iflytek.skillhub.domain.skill.service.SkillLifecycleProjectionService; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.domain.skill.validation.PackageEntry; @@ -15,6 +24,9 @@ import com.iflytek.skillhub.service.AuditRequestContext; import com.iflytek.skillhub.service.SkillDeleteAppService; import com.iflytek.skillhub.service.SkillSearchAppService; +import com.iflytek.skillhub.search.SearchQuery; +import com.iflytek.skillhub.search.SearchQueryService; +import com.iflytek.skillhub.search.SearchResult; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -39,6 +51,11 @@ class CliSkillAppServiceTest { @Mock SkillDownloadService skillDownloadService; @Mock SkillDeleteAppService skillDeleteAppService; @Mock SkillPublishService skillPublishService; + @Mock SkillRepository skillRepository; + @Mock NamespaceRepository namespaceRepository; + @Mock SkillVersionRepository skillVersionRepository; + @Mock NamespaceService namespaceService; + @Mock RbacService rbacService; private CliSkillAppService service; @@ -62,7 +79,7 @@ void search_mapsResultsToCliFormat() { )), 1L, 0, 20 ); - given(skillSearchAppService.search("pdf", null, "newest", 0, 20, null, null)) + given(skillSearchAppService.searchInstallableLatest("pdf", null, "newest", 0, 20, null, null)) .willReturn(searchResponse); var result = service.search("pdf", 20, null, null); @@ -76,6 +93,118 @@ void search_mapsResultsToCliFormat() { assertEquals(20, result.limit()); } + @Test + void search_mapsInstallableSearchTotalFromQueryStage() { + var searchResponse = new SkillSearchAppService.SearchResponse( + List.of( + new SkillSummaryResponse( + 2L, "ready", "Ready", "Installable", + "PUBLIC", "ACTIVE", 0L, 0, BigDecimal.ZERO, 0, + "global", Instant.now(), false, + new SkillLifecycleVersionResponse(2L, "1.0.0", "PUBLISHED"), + new SkillLifecycleVersionResponse(2L, "1.0.0", "PUBLISHED"), + null, "PUBLISHED" + ) + ), + 1L, 0, 20 + ); + given(skillSearchAppService.searchInstallableLatest("demo", null, "newest", 0, 20, null, null)) + .willReturn(searchResponse); + + var result = service.search("demo", 20, null, null); + + assertEquals(1, result.items().size()); + assertEquals("ready", result.items().getFirst().slug()); + assertEquals(1L, result.total()); + } + + @Test + void search_limitOneSkipsUninstallableMatchAndReturnsNextInstallableWithFilteredTotal() { + Skill unavailableFirstMatch = new Skill(1L, "draft-first", "owner-1", SkillVisibility.PUBLIC); + setField(unavailableFirstMatch, "id", 1L); + assertLimitOneSkipsUninstallableFirstMatch(unavailableFirstMatch, List.of()); + } + + @Test + void search_limitOneSkipsYankedLatestMatchAndReturnsNextInstallableWithFilteredTotal() { + Skill unavailableFirstMatch = new Skill(1L, "yanked-first", "owner-1", SkillVisibility.PUBLIC); + setField(unavailableFirstMatch, "id", 1L); + unavailableFirstMatch.setLatestVersionId(10L); + SkillVersion yanked = publishedVersion(1L, 10L, "1.0.0"); + yanked.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + + assertLimitOneSkipsUninstallableFirstMatch(unavailableFirstMatch, List.of(yanked)); + } + + @Test + void search_limitOneSkipsDownloadUnavailableLatestAndReturnsNextInstallableWithFilteredTotal() { + Skill unavailableFirstMatch = new Skill(1L, "not-ready-first", "owner-1", SkillVisibility.PUBLIC); + setField(unavailableFirstMatch, "id", 1L); + unavailableFirstMatch.setLatestVersionId(10L); + SkillVersion notReady = publishedVersion(1L, 10L, "1.0.0"); + notReady.setDownloadReady(false); + + assertLimitOneSkipsUninstallableFirstMatch(unavailableFirstMatch, List.of(notReady)); + } + + private void assertLimitOneSkipsUninstallableFirstMatch( + Skill unavailableFirstMatch, + List unavailableLatestVersions) { + SearchQueryService rankedSearch = query -> requiresInstallableLatest(query) + ? new SearchResult(List.of(2L), 1L, 0, 1) + : new SearchResult(List.of(1L), 2L, 0, 1); + SkillSearchAppService realSearchAppService = new SkillSearchAppService( + rankedSearch, + skillRepository, + namespaceRepository, + namespaceService, + new SkillLifecycleProjectionService(skillVersionRepository), + rbacService + ); + CliSkillAppService realService = new CliSkillAppService( + realSearchAppService, + skillQueryService, + skillDownloadService, + skillDeleteAppService, + skillPublishService + ); + + Skill installableSecondMatch = new Skill(1L, "ready-second", "owner-1", SkillVisibility.PUBLIC); + setField(installableSecondMatch, "id", 2L); + installableSecondMatch.setLatestVersionId(20L); + + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setField(namespace, "id", 1L); + SkillVersion installableVersion = publishedVersion(2L, 20L, "1.0.0"); + + org.mockito.Mockito.lenient() + .when(skillRepository.findByIdIn(List.of(1L))) + .thenReturn(List.of(unavailableFirstMatch)); + org.mockito.Mockito.lenient() + .when(skillRepository.findByIdIn(List.of(2L))) + .thenReturn(List.of(installableSecondMatch)); + org.mockito.Mockito.lenient() + .when(namespaceRepository.findByIdIn(List.of(1L))) + .thenReturn(List.of(namespace)); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findByIdIn(List.of())) + .thenReturn(List.of()); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findByIdIn(List.of(10L))) + .thenReturn(unavailableLatestVersions); + org.mockito.Mockito.lenient() + .when(skillVersionRepository.findByIdIn(List.of(20L))) + .thenReturn(List.of(installableVersion)); + + var result = realService.search("demo", 1, null, null); + + assertEquals(1, result.items().size()); + assertEquals("ready-second", result.items().getFirst().slug()); + assertEquals("1.0.0", result.items().getFirst().latestVersion()); + assertEquals(1L, result.total()); + assertEquals(1, result.limit()); + } + @Test void resolve_delegatesToQueryService() { given(skillQueryService.resolveVersion("global", "demo", "2.0.0", null, null, "user-1", Map.of())) @@ -125,4 +254,30 @@ void publish_delegatesToPublishService() { assertEquals("1.0.0", response.version()); assertEquals("PUBLIC", response.visibility()); } + + private boolean requiresInstallableLatest(SearchQuery query) { + try { + return (boolean) query.getClass().getMethod("requireInstallableLatest").invoke(query); + } catch (ReflectiveOperationException e) { + return false; + } + } + + private SkillVersion publishedVersion(Long skillId, Long versionId, String versionNumber) { + SkillVersion version = new SkillVersion(skillId, versionNumber, "owner-1"); + setField(version, "id", versionId); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + return version; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java index 6f594c1e2..8b24aa867 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java @@ -10,9 +10,12 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; @@ -37,49 +40,74 @@ public class ApiTokenAuthenticationFilter extends OncePerRequestFilter { private final UserAccountRepository userRepo; private final UserRoleBindingRepository roleBindingRepo; private final ApiTokenScopeService apiTokenScopeService; + private final AuthenticationEntryPoint authenticationEntryPoint; + @Autowired public ApiTokenAuthenticationFilter(ApiTokenService apiTokenService, UserAccountRepository userRepo, UserRoleBindingRepository roleBindingRepo, - ApiTokenScopeService apiTokenScopeService) { + ApiTokenScopeService apiTokenScopeService, + AuthenticationEntryPoint authenticationEntryPoint) { this.apiTokenService = apiTokenService; this.userRepo = userRepo; this.roleBindingRepo = roleBindingRepo; this.apiTokenScopeService = apiTokenScopeService; + this.authenticationEntryPoint = authenticationEntryPoint; + } + + ApiTokenAuthenticationFilter(ApiTokenService apiTokenService, + UserAccountRepository userRepo, + UserRoleBindingRepository roleBindingRepo, + ApiTokenScopeService apiTokenScopeService) { + this(apiTokenService, userRepo, roleBindingRepo, apiTokenScopeService, + (request, response, authException) -> + response.sendError(HttpServletResponse.SC_UNAUTHORIZED, authException.getMessage())); } @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { String authHeader = request.getHeader(AUTH_HEADER); - if (authHeader != null && authHeader.startsWith(BEARER_PREFIX)) { - String rawToken = authHeader.substring(BEARER_PREFIX.length()); - apiTokenService.validateToken(rawToken).ifPresent(token -> { - userRepo.findById(token.getUserId()).ifPresent(user -> { - if (!user.isActive()) { - return; - } - Set roles = roleBindingRepo.findByUserId(user.getId()).stream() - .map(rb -> rb.getRole().getCode()) - .collect(Collectors.toSet()); - roles = PlatformRoleDefaults.withDefaultUserRole(roles); - Set scopes = apiTokenScopeService.parseScopes(token.getScopeJson()); - PlatformPrincipal principal = new PlatformPrincipal( - user.getId(), user.getDisplayName(), user.getEmail(), - user.getAvatarUrl(), "api_token", roles - ); - List authorities = new ArrayList<>(); - authorities.addAll(roles.stream() - .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) - .toList()); - authorities.addAll(scopes.stream() - .map(scope -> new SimpleGrantedAuthority("SCOPE_" + scope)) - .toList()); - var auth = new UsernamePasswordAuthenticationToken(principal, null, authorities); - SecurityContextHolder.getContext().setAuthentication(auth); - apiTokenService.touchLastUsed(token); - }); - }); + if (authHeader != null && isBearerAuthorization(authHeader)) { + String rawToken = extractBearerToken(authHeader); + if (rawToken == null) { + rejectBearer(request, response); + return; + } + + var token = apiTokenService.validateToken(rawToken); + if (token.isEmpty()) { + rejectBearer(request, response); + return; + } + + ApiToken apiToken = token.get(); + var user = userRepo.findById(apiToken.getUserId()); + if (user.isEmpty() || !user.get().isActive()) { + rejectBearer(request, response); + return; + } + + UserAccount userAccount = user.get(); + Set roles = roleBindingRepo.findByUserId(userAccount.getId()).stream() + .map(rb -> rb.getRole().getCode()) + .collect(Collectors.toSet()); + roles = PlatformRoleDefaults.withDefaultUserRole(roles); + Set scopes = apiTokenScopeService.parseScopes(apiToken.getScopeJson()); + PlatformPrincipal principal = new PlatformPrincipal( + userAccount.getId(), userAccount.getDisplayName(), userAccount.getEmail(), + userAccount.getAvatarUrl(), "api_token", roles + ); + List authorities = new ArrayList<>(); + authorities.addAll(roles.stream() + .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) + .toList()); + authorities.addAll(scopes.stream() + .map(scope -> new SimpleGrantedAuthority("SCOPE_" + scope)) + .toList()); + var auth = new UsernamePasswordAuthenticationToken(principal, null, authorities); + SecurityContextHolder.getContext().setAuthentication(auth); + apiTokenService.touchLastUsed(apiToken); } filterChain.doFilter(request, response); } @@ -91,4 +119,30 @@ protected boolean shouldNotFilter(HttpServletRequest request) { || path.startsWith("/api/web/") || path.startsWith("/api/cli/")); } + + private boolean isBearerAuthorization(String authHeader) { + if (!authHeader.regionMatches(true, 0, "Bearer", 0, "Bearer".length())) { + return false; + } + return authHeader.length() == "Bearer".length() + || Character.isWhitespace(authHeader.charAt("Bearer".length())); + } + + private String extractBearerToken(String authHeader) { + if (authHeader.length() <= BEARER_PREFIX.length() - 1 + || authHeader.charAt(BEARER_PREFIX.length() - 1) != ' ') { + return null; + } + String rawToken = authHeader.substring(BEARER_PREFIX.length()).trim(); + return rawToken.isEmpty() ? null : rawToken; + } + + private void rejectBearer(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { + SecurityContextHolder.clearContext(); + authenticationEntryPoint.commence( + request, + response, + new BadCredentialsException("Invalid bearer token") + ); + } } 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 e82f7a1ab..d9f030a9d 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 @@ -17,11 +17,13 @@ import org.springframework.security.core.context.SecurityContextHolder; import java.util.List; import java.util.Optional; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -89,12 +91,117 @@ void shouldRejectDisabledUsers() throws Exception { request.setRequestURI("/api/v1/publish"); request.addHeader("Authorization", "Bearer raw-token"); - filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertEquals(MockHttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + assertNull(chain.getRequest()); + verify(apiTokenService, never()).touchLastUsed(token); + } + + @Test + void shouldRejectUnknownBearerTokenOnCliReadRoutes() throws Exception { + when(apiTokenService.validateToken("unknown-token")).thenReturn(Optional.empty()); + + for (String route : cliReadRoutes()) { + SecurityContextHolder.clearContext(); + MockHttpServletRequest request = new MockHttpServletRequest("GET", route); + request.addHeader("Authorization", "Bearer unknown-token"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_UNAUTHORIZED, response.getStatus(), route); + assertNull(SecurityContextHolder.getContext().getAuthentication(), route); + assertNull(chain.getRequest(), route); + } + } + + @Test + void shouldRejectBearerTokenWhenUserIsMissing() throws Exception { + ApiToken token = new ApiToken("missing-user", "cli", "sk_test", "hash", "[]"); + when(apiTokenService.validateToken("raw-token")).thenReturn(Optional.of(token)); + when(userAccountRepository.findById("missing-user")).thenReturn(Optional.empty()); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/skills/search"); + request.addHeader("Authorization", "Bearer raw-token"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertNull(chain.getRequest()); verify(apiTokenService, never()).touchLastUsed(token); } + @Test + void shouldRejectEmptyBearerTokenWithoutValidatingIt() throws Exception { + when(apiTokenService.validateToken("")).thenReturn(Optional.empty()); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/skills/search"); + request.addHeader("Authorization", "Bearer "); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertNull(chain.getRequest()); + verify(apiTokenService, never()).validateToken(any()); + } + + @Test + void shouldRejectMalformedBearerHeaderWithoutValidatingIt() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/skills/search"); + request.addHeader("Authorization", "Bearer"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertNull(chain.getRequest()); + verify(apiTokenService, never()).validateToken(any()); + } + + @Test + void shouldAllowAnonymousCliReadsWhenAuthorizationHeaderIsAbsent() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/skills/search"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_OK, response.getStatus()); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertNotNull(chain.getRequest()); + verify(apiTokenService, never()).validateToken(any()); + } + + @Test + void shouldIgnoreNonBearerAuthorizationHeader() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/skills/search"); + request.addHeader("Authorization", "Basic abc123"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertEquals(MockHttpServletResponse.SC_OK, response.getStatus()); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + assertNotNull(chain.getRequest()); + verify(apiTokenService, never()).validateToken(any()); + } + @Test void shouldAuthenticateBearerTokensForApiWebRequests() throws Exception { ApiToken token = new ApiToken("user-3", "cli", "sk_test", "hash", "[\"skill:publish\"]"); @@ -113,4 +220,13 @@ void shouldAuthenticateBearerTokensForApiWebRequests() throws Exception { assertNotNull(SecurityContextHolder.getContext().getAuthentication()); verify(apiTokenService).touchLastUsed(token); } + + private static List cliReadRoutes() { + return Stream.of( + "/api/cli/v1/skills/search", + "/api/cli/v1/skills/global/demo/resolve", + "/api/cli/v1/skills/global/demo/download", + "/api/cli/v1/skills/global/demo/versions/1.0.0/download" + ).toList(); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenServiceTest.java index 2e4de008f..d9ed2c75e 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenServiceTest.java @@ -10,9 +10,14 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.DataIntegrityViolationException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; +import java.util.HexFormat; +import java.util.Optional; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.assertThat; @@ -124,4 +129,39 @@ void createToken_translatesDatabaseConstraintViolationToDuplicateError() { .isInstanceOf(DomainBadRequestException.class) .hasMessageContaining("error.token.name.duplicate"); } + + @Test + void validateToken_returnsEmptyForUnknownToken() { + when(tokenRepo.findByTokenHash(sha256("missing-token"))).thenReturn(Optional.empty()); + + assertThat(service.validateToken("missing-token")).isEmpty(); + } + + @Test + void validateToken_returnsEmptyForExpiredToken() { + ApiToken token = new ApiToken("user-1", "CLI", "sk_test", sha256("expired-token"), "[]"); + token.setExpiresAt(Instant.parse("2026-03-17T23:59:59Z")); + when(tokenRepo.findByTokenHash(sha256("expired-token"))).thenReturn(Optional.of(token)); + + assertThat(service.validateToken("expired-token")).isEmpty(); + } + + @Test + void validateToken_returnsEmptyForRevokedToken() { + ApiToken token = new ApiToken("user-1", "CLI", "sk_test", sha256("revoked-token"), "[]"); + token.setRevokedAt(Instant.parse("2026-03-17T23:59:59Z")); + when(tokenRepo.findByTokenHash(sha256("revoked-token"))).thenReturn(Optional.of(token)); + + assertThat(service.validateToken("revoked-token")).isEmpty(); + } + + private static String sha256(String input) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8)); + return HexFormat.of().formatHex(hash); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 not available", e); + } + } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillInstallability.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillInstallability.java new file mode 100644 index 000000000..dfeeed1c4 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillInstallability.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.domain.skill; + +/** + * Defines whether a skill version can be installed through public download + * paths. Storage object presence is checked later by the download service so + * fallback bundle behavior stays separate from domain publication state. + */ +public final class SkillInstallability { + private SkillInstallability() { + } + + public static boolean isInstallableVersion(SkillVersion version) { + return version != null + && version.getStatus() == SkillVersionStatus.PUBLISHED + && version.isDownloadReady() + && version.getYankedAt() == null; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java index 65c8e7538..30b52a669 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java @@ -16,19 +16,20 @@ public boolean canAccess(Skill skill, String currentUserId, Map userNamespaceRoles, Set platformRoles) { + Map roles = userNamespaceRoles != null ? userNamespaceRoles : Map.of(); if (isSuperAdmin(platformRoles)) { return true; } if (skill.isHidden()) { - return isOwner(skill, currentUserId) || isAdminOrAbove(userNamespaceRoles.get(skill.getNamespaceId())); + return isOwner(skill, currentUserId) || isAdminOrAbove(roles.get(skill.getNamespaceId())); } if (skill.getLatestVersionId() == null) { return isOwner(skill, currentUserId); } return switch (skill.getVisibility()) { case PUBLIC -> true; - case NAMESPACE_ONLY -> userNamespaceRoles.containsKey(skill.getNamespaceId()); - case PRIVATE -> isOwner(skill, currentUserId) || isAdminOrAbove(userNamespaceRoles.get(skill.getNamespaceId())); + case NAMESPACE_ONLY -> roles.containsKey(skill.getNamespaceId()); + case PRIVATE -> isOwner(skill, currentUserId) || isAdminOrAbove(roles.get(skill.getNamespaceId())); }; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java index 56b69dddf..294563add 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.*; @@ -284,12 +285,20 @@ private void assertCanDownload(Namespace namespace, if (!visibilityChecker.canAccess(skill, currentUserId, userNsRoles)) { throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug()); } + if (namespace.getStatus() == NamespaceStatus.ARCHIVED + && !isNamespaceMember(namespace.getId(), currentUserId, userNsRoles)) { + throw new DomainForbiddenException("error.namespace.archived", namespace.getSlug()); + } } private boolean isAnonymousDownloadAllowed(Skill skill) { return skill.getVisibility() == SkillVisibility.PUBLIC; } + private boolean isNamespaceMember(Long namespaceId, String currentUserId, Map userNsRoles) { + return currentUserId != null && userNsRoles != null && userNsRoles.containsKey(namespaceId); + } + private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { return skillSlugResolutionService.resolve( namespaceId, @@ -306,7 +315,7 @@ private void assertPublishedAccessible(Skill skill) { /** * Asserts that the version can be downloaded. - * - PUBLISHED: anyone with skill access can download + * - PUBLISHED: must be installable before public download * - UPLOADED/PENDING_REVIEW: only skill owner or namespace admin can download */ private void assertDownloadableVersion(Skill skill, @@ -315,7 +324,9 @@ private void assertDownloadableVersion(Skill skill, Map userNsRoles) { switch (version.getStatus()) { case PUBLISHED -> { - // Anyone with skill access can download published versions + if (!SkillInstallability.isInstallableVersion(version)) { + throw new DomainBadRequestException("error.skill.version.notDownloadable", version.getVersion()); + } } case UPLOADED, PENDING_REVIEW -> { if (!canManageSkillDraft(skill, currentUserId, userNsRoles)) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java index 368ae850f..31ff9a6bf 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java @@ -2,6 +2,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillInstallability; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; @@ -88,19 +89,10 @@ public Map projectPublishedSummaries(List skills) { .collect(Collectors.toMap(SkillVersion::getId, Function.identity())); Map publishedBySkillId = new java.util.HashMap<>(); - List unresolvedSkillIds = new java.util.ArrayList<>(); for (Skill skill : skills) { SkillVersion latestVersion = latestVersionsById.get(skill.getLatestVersionId()); - if (latestVersion != null && latestVersion.getStatus() == SkillVersionStatus.PUBLISHED) { + if (SkillInstallability.isInstallableVersion(latestVersion)) { publishedBySkillId.put(skill.getId(), latestVersion); - } else { - unresolvedSkillIds.add(skill.getId()); - } - } - - if (!unresolvedSkillIds.isEmpty()) { - for (SkillVersion version : skillVersionRepository.findBySkillIdInAndStatus(unresolvedSkillIds, SkillVersionStatus.PUBLISHED)) { - publishedBySkillId.merge(version.getSkillId(), version, this::newerVersion); } } @@ -157,10 +149,6 @@ private Comparator versionComparator() { .thenComparing(SkillVersion::getId, Comparator.nullsLast(Comparator.naturalOrder())); } - private SkillVersion newerVersion(SkillVersion left, SkillVersion right) { - return versionComparator().compare(left, right) >= 0 ? left : right; - } - private VersionProjection toProjection(SkillVersion version) { if (version == null) { return null; diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java index 7d966327f..66c5e3191 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java @@ -487,13 +487,7 @@ public Page listVersions(String namespaceSlug, } public boolean isDownloadAvailable(SkillVersion version) { - if (version == null) { - return false; - } - if (version.getStatus() != SkillVersionStatus.PUBLISHED) { - return false; - } - return version.isDownloadReady(); + return SkillInstallability.isInstallableVersion(version); } public ReviewSkillSnapshotDTO getReviewSkillSnapshot(Long skillVersionId) { @@ -565,6 +559,7 @@ public ResolvedVersionDTO resolveVersion( Skill skill = resolveVisibleSkill(namespace.getId(), skillSlug, currentUserId); assertPublishedAccessible(namespace, skill, currentUserId, userNsRoles); SkillVersion resolved = resolveVersionEntity(skill, version, tag, hash); + assertInstallableVersion(resolved, resolved.getVersion()); String fingerprint = computeFingerprint(resolved); Boolean matched = hash == null || hash.isBlank() ? null : Objects.equals(hash, fingerprint); @@ -916,6 +911,12 @@ private void assertPublishedVersion(SkillVersion version, String versionStr) { } } + private void assertInstallableVersion(SkillVersion version, String versionStr) { + if (!SkillInstallability.isInstallableVersion(version)) { + throw new DomainBadRequestException("error.skill.version.notDownloadable", versionStr); + } + } + /** * Checks whether the caller may preview a specific version's files and metadata. * Published versions are visible to everyone; all other statuses are restricted diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java index ce84509df..89a2e4cbf 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java @@ -92,6 +92,7 @@ void testDownloadLatest_Success() throws Exception { SkillVersion version = new SkillVersion(1L, "1.0.0", userId); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); String storageKey = "packages/1/10/bundle.zip"; InputStream content = new ByteArrayInputStream("test".getBytes()); ObjectMetadata metadata = new ObjectMetadata(1000L, "application/zip", Instant.now()); @@ -118,6 +119,137 @@ void testDownloadLatest_Success() throws Exception { verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class)); } + @Test + void testDownloadLatest_ShouldRejectSkillWithoutLatest() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "missing-latest"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.downloadLatest(namespaceSlug, skillSlug, null, Map.of())); + + assertEquals("error.skill.notFound", ex.messageCode()); + assertArrayEquals(new Object[]{skillSlug}, ex.messageArgs()); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + + @Test + void testDownloadLatest_ShouldRejectYankedLatestVersion() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "yanked-latest"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(10L); + + SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + version.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(visibilityChecker.canAccess(skill, null, Map.of())).thenReturn(true); + when(skillVersionRepository.findById(10L)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.downloadLatest(namespaceSlug, skillSlug, null, Map.of())); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{"1.0.0"}, ex.messageArgs()); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + + @Test + void testDownloadLatest_ShouldRejectAnonymousArchivedNamespaceSkill() throws Exception { + String namespaceSlug = "archived"; + String skillSlug = "archived-skill"; + + Namespace namespace = new Namespace(namespaceSlug, "Archived", "owner-1"); + setId(namespace, 1L); + namespace.setStatus(com.iflytek.skillhub.domain.namespace.NamespaceStatus.ARCHIVED); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(10L); + + SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + ObjectMetadata metadata = new ObjectMetadata(1000L, "application/zip", Instant.now()); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(visibilityChecker.canAccess(skill, null, Map.of())).thenReturn(true); + org.mockito.Mockito.lenient().when(skillVersionRepository.findById(10L)).thenReturn(Optional.of(version)); + org.mockito.Mockito.lenient().when(objectStorageService.exists("packages/1/10/bundle.zip")).thenReturn(true); + org.mockito.Mockito.lenient().when(objectStorageService.getMetadata("packages/1/10/bundle.zip")).thenReturn(metadata); + org.mockito.Mockito.lenient().when(objectStorageService.getObject("packages/1/10/bundle.zip")) + .thenReturn(new ByteArrayInputStream("test".getBytes())); + org.mockito.Mockito.lenient() + .when(objectStorageService.generatePresignedUrl(eq("packages/1/10/bundle.zip"), any(), eq("archived-skill-1.0.0.zip"))) + .thenReturn(null); + + assertThrows(com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException.class, () -> + service.downloadLatest(namespaceSlug, skillSlug, null, Map.of())); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + + @Test + void testDownloadLatest_ShouldRejectAnonymousHiddenPrivateAndUnpublishedSkills() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + + Skill hiddenSkill = new Skill(1L, "hidden", "owner-1", SkillVisibility.PUBLIC); + setId(hiddenSkill, 11L); + hiddenSkill.setStatus(SkillStatus.ACTIVE); + hiddenSkill.setLatestVersionId(101L); + hiddenSkill.setHidden(true); + + Skill privateSkill = new Skill(1L, "private", "owner-1", SkillVisibility.PRIVATE); + setId(privateSkill, 12L); + privateSkill.setStatus(SkillStatus.ACTIVE); + privateSkill.setLatestVersionId(102L); + + Skill unpublishedSkill = new Skill(1L, "unpublished", "owner-1", SkillVisibility.PUBLIC); + setId(unpublishedSkill, 13L); + unpublishedSkill.setStatus(SkillStatus.ACTIVE); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "hidden")).thenReturn(List.of(hiddenSkill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "private")).thenReturn(List.of(privateSkill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "unpublished")).thenReturn(List.of(unpublishedSkill)); + + assertThrows(DomainBadRequestException.class, () -> + service.downloadLatest("global", "hidden", null, Map.of())); + assertThrows(com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException.class, () -> + service.downloadLatest("global", "private", null, Map.of())); + assertThrows(DomainBadRequestException.class, () -> + service.downloadLatest("global", "unpublished", null, Map.of())); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + @Test void testDownloadByTag_Success() throws Exception { // Arrange @@ -137,6 +269,7 @@ void testDownloadByTag_Success() throws Exception { SkillVersion version = new SkillVersion(1L, "1.0.0", userId); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); String storageKey = "packages/1/10/bundle.zip"; InputStream content = new ByteArrayInputStream("test".getBytes()); ObjectMetadata metadata = new ObjectMetadata(1000L, "application/zip", Instant.now()); @@ -180,6 +313,7 @@ void testDownloadVersion_WithPresignedUrlStillProvidesStreamFallback() throws Ex SkillVersion version = new SkillVersion(1L, versionStr, userId); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); String storageKey = "packages/1/10/bundle.zip"; InputStream content = new ByteArrayInputStream("test".getBytes()); ObjectMetadata metadata = new ObjectMetadata(1000L, "application/zip", Instant.now()); @@ -232,6 +366,73 @@ void testDownloadVersion_ShouldRejectDraftVersion() throws Exception { verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); } + @Test + void testDownloadVersion_ShouldRejectDownloadUnavailablePublishedVersion() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String versionStr = "1.0.0"; + String userId = "user-100"; + Map userNsRoles = Map.of(1L, NamespaceRole.MEMBER); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, userId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + SkillVersion version = new SkillVersion(1L, versionStr, userId); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(false); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); + when(skillVersionRepository.findBySkillIdAndVersion(1L, versionStr)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.downloadVersion(namespaceSlug, skillSlug, versionStr, userId, userNsRoles)); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{versionStr}, ex.messageArgs()); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + + @Test + void testDownloadVersion_ShouldRejectYankedPublishedVersion() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String versionStr = "1.0.0"; + String userId = "user-100"; + Map userNsRoles = Map.of(1L, NamespaceRole.MEMBER); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, userId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + SkillVersion version = new SkillVersion(1L, versionStr, userId); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + version.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); + when(skillVersionRepository.findBySkillIdAndVersion(1L, versionStr)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.downloadVersion(namespaceSlug, skillSlug, versionStr, userId, userNsRoles)); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{versionStr}, ex.messageArgs()); + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + @Test void testDownloadVersion_ShouldFallbackToBundledFilesWhenBundleIsMissing() throws Exception { String namespaceSlug = "test-ns"; @@ -249,6 +450,7 @@ void testDownloadVersion_ShouldFallbackToBundledFilesWhenBundleIsMissing() throw SkillVersion version = new SkillVersion(1L, versionStr, userId); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); SkillFile file = new SkillFile(10L, "SKILL.md", 4L, "text/markdown", "hash", "skills/1/10/SKILL.md"); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); @@ -297,6 +499,7 @@ void testDownloadVersion_AllowsAnonymousForGlobalPublicSkill() throws Exception SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1"); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(1L, "demo-skill")).thenReturn(List.of(skill)); @@ -332,6 +535,7 @@ void testDownloadVersion_AllowsAnonymousForTeamNamespacePublicSkill() throws Exc SkillVersion version = new SkillVersion(1L, "1.0.0", "owner-1"); setId(version, 10L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); when(namespaceRepository.findBySlug("team-ai")).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(2L, "demo-skill")).thenReturn(List.of(skill)); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java index 7ee683ac7..02cf02f12 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java @@ -27,6 +27,7 @@ import java.io.InputStream; import java.io.UncheckedIOException; import java.lang.reflect.Field; +import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Optional; @@ -439,6 +440,17 @@ void testIsDownloadAvailable_ShouldReturnTrueWhenPublishedVersionHasFiles() thro assertTrue(service.isDownloadAvailable(version)); } + @Test + void testIsDownloadAvailable_ShouldReturnFalseWhenVersionIsYanked() throws Exception { + SkillVersion version = new SkillVersion(1L, "1.0.0", "user-100"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + version.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + + assertFalse(service.isDownloadAvailable(version)); + } + @Test void testIsDownloadAvailable_ShouldNotHitObjectStorageForListSignals() throws Exception { SkillVersion version = new SkillVersion(1L, "1.0.0", "user-100"); @@ -565,9 +577,11 @@ void testResolveVersion_ShouldReturnLatestWhenHashDoesNotMatch() throws Exceptio SkillVersion version100 = new SkillVersion(1L, "1.0.0", "user-100"); setId(version100, 9L); version100.setStatus(SkillVersionStatus.PUBLISHED); + version100.setDownloadReady(true); SkillVersion version110 = new SkillVersion(1L, "1.1.0", "user-100"); setId(version110, 10L); version110.setStatus(SkillVersionStatus.PUBLISHED); + version110.setDownloadReady(true); SkillFile version100File = new SkillFile(9L, "SKILL.md", 10L, "text/markdown", "hash100", "key100"); SkillFile version110File = new SkillFile(10L, "SKILL.md", 10L, "text/markdown", "hash110", "key110"); @@ -611,6 +625,7 @@ void testResolveVersion_ShouldEncodeDownloadUrlPathSegments() throws Exception { SkillVersion version = new SkillVersion(3L, "1.0.0 beta", "user-100"); setId(version, 11L); version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); SkillFile file = new SkillFile(11L, "SKILL.md", 10L, "text/markdown", "hash", "key"); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); @@ -632,6 +647,214 @@ void testResolveVersion_ShouldEncodeDownloadUrlPathSegments() throws Exception { assertEquals("/api/v1/skills/global/smoke-skill-two/versions/1.0.0%20beta/download", result.downloadUrl()); } + @Test + void testResolveVersion_ShouldRejectDownloadUnavailableLatestVersion() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "not-ready"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 3L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(11L); + + SkillVersion version = new SkillVersion(3L, "1.0.0", "owner-1"); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(false); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(skillVersionRepository.findBySkillIdAndStatus(3L, SkillVersionStatus.PUBLISHED)).thenReturn(List.of(version)); + when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion(namespaceSlug, skillSlug, null, null, null, null, Map.of())); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{"1.0.0"}, ex.messageArgs()); + } + + @Test + void testResolveVersion_ShouldRejectSkillWithoutLatest() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "missing-latest"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 3L); + skill.setStatus(SkillStatus.ACTIVE); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion(namespaceSlug, skillSlug, null, null, null, null, Map.of())); + + assertEquals("error.skill.notFound", ex.messageCode()); + assertArrayEquals(new Object[]{skillSlug}, ex.messageArgs()); + } + + @Test + void testResolveVersion_ShouldRejectYankedLatestVersion() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "yanked-latest"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 3L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(11L); + + SkillVersion version = new SkillVersion(3L, "1.0.0", "owner-1"); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(true); + version.setYankedAt(Instant.parse("2026-06-12T00:00:00Z")); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(skillVersionRepository.findBySkillIdAndStatus(3L, SkillVersionStatus.PUBLISHED)).thenReturn(List.of(version)); + when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion(namespaceSlug, skillSlug, null, null, null, null, Map.of())); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{"1.0.0"}, ex.messageArgs()); + } + + @Test + void testResolveVersion_ShouldRejectDownloadUnavailableExplicitVersion() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "explicit-not-ready"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 3L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(11L); + + SkillVersion version = new SkillVersion(3L, "1.0.0", "owner-1"); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(false); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(skillVersionRepository.findBySkillIdAndVersion(3L, "1.0.0")).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion(namespaceSlug, skillSlug, "1.0.0", null, null, null, Map.of())); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{"1.0.0"}, ex.messageArgs()); + } + + @Test + void testResolveVersion_ShouldRejectDownloadUnavailableTaggedVersion() throws Exception { + String namespaceSlug = "global"; + String skillSlug = "tag-not-ready"; + + Namespace namespace = new Namespace(namespaceSlug, "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); + setId(skill, 3L); + skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(11L); + + SkillVersion version = new SkillVersion(3L, "1.0.0", "owner-1"); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setDownloadReady(false); + SkillTag tag = new SkillTag(3L, "stable", 11L, "owner-1"); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(skillTagRepository.findBySkillIdAndTagName(3L, "stable")).thenReturn(Optional.of(tag)); + when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(version)); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion(namespaceSlug, skillSlug, null, "stable", null, null, Map.of())); + + assertEquals("error.skill.version.notDownloadable", ex.messageCode()); + assertArrayEquals(new Object[]{"1.0.0"}, ex.messageArgs()); + } + + @Test + void testResolveVersion_ShouldRejectAnonymousHiddenPrivateArchivedAndUnpublishedSkills() throws Exception { + Namespace activeNamespace = new Namespace("global", "Global", "owner-1"); + setId(activeNamespace, 1L); + Namespace archivedNamespace = new Namespace("archived", "Archived", "owner-1"); + setId(archivedNamespace, 2L); + archivedNamespace.setStatus(NamespaceStatus.ARCHIVED); + + Skill hiddenSkill = new Skill(1L, "hidden", "owner-1", SkillVisibility.PUBLIC); + setId(hiddenSkill, 10L); + hiddenSkill.setStatus(SkillStatus.ACTIVE); + hiddenSkill.setLatestVersionId(101L); + hiddenSkill.setHidden(true); + + Skill privateSkill = new Skill(1L, "private", "owner-1", SkillVisibility.PRIVATE); + setId(privateSkill, 11L); + privateSkill.setStatus(SkillStatus.ACTIVE); + privateSkill.setLatestVersionId(102L); + + Skill archivedSkill = new Skill(2L, "archived", "owner-1", SkillVisibility.PUBLIC); + setId(archivedSkill, 12L); + archivedSkill.setStatus(SkillStatus.ACTIVE); + archivedSkill.setLatestVersionId(103L); + + Skill unpublishedSkill = new Skill(1L, "unpublished", "owner-1", SkillVisibility.PUBLIC); + setId(unpublishedSkill, 13L); + unpublishedSkill.setStatus(SkillStatus.ACTIVE); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(activeNamespace)); + when(namespaceRepository.findBySlug("archived")).thenReturn(Optional.of(archivedNamespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "hidden")).thenReturn(List.of(hiddenSkill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "private")).thenReturn(List.of(privateSkill)); + when(skillRepository.findByNamespaceIdAndSlug(2L, "archived")).thenReturn(List.of(archivedSkill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "unpublished")).thenReturn(List.of(unpublishedSkill)); + + assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion("global", "hidden", null, null, null, null, Map.of())); + assertThrows(DomainForbiddenException.class, () -> + service.resolveVersion("global", "private", null, null, null, null, Map.of())); + assertThrows(DomainForbiddenException.class, () -> + service.resolveVersion("archived", "archived", null, null, null, null, Map.of())); + assertThrows(DomainBadRequestException.class, () -> + service.resolveVersion("global", "unpublished", null, null, null, null, Map.of())); + } + + @Test + void testResolveVersion_ShouldRejectAnonymousPrivateAndNamespaceOnlyWhenRolesAreMissing() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + + Skill privateSkill = new Skill(1L, "private", "owner-1", SkillVisibility.PRIVATE); + setId(privateSkill, 11L); + privateSkill.setStatus(SkillStatus.ACTIVE); + privateSkill.setLatestVersionId(101L); + + Skill namespaceOnlySkill = new Skill(1L, "team-only", "owner-1", SkillVisibility.NAMESPACE_ONLY); + setId(namespaceOnlySkill, 12L); + namespaceOnlySkill.setStatus(SkillStatus.ACTIVE); + namespaceOnlySkill.setLatestVersionId(102L); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "private")).thenReturn(List.of(privateSkill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "team-only")).thenReturn(List.of(namespaceOnlySkill)); + + assertThrows(DomainForbiddenException.class, () -> + service.resolveVersion("global", "private", null, null, null, null, null)); + assertThrows(DomainForbiddenException.class, () -> + service.resolveVersion("global", "team-only", null, null, null, null, null)); + } + @Test void testGetSkillDetail_ShouldFlagLifecyclePermissionForOwner() throws Exception { String namespaceSlug = "test-ns"; diff --git a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/SearchQuery.java b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/SearchQuery.java index 14c2cc4d2..5a54d0a6c 100644 --- a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/SearchQuery.java +++ b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/SearchQuery.java @@ -12,8 +12,20 @@ public record SearchQuery( String sortBy, int page, int size, - List labelSlugs + List labelSlugs, + boolean requireInstallableLatest ) { + public SearchQuery( + String keyword, + Long namespaceId, + SearchVisibilityScope visibilityScope, + String sortBy, + int page, + int size, + List labelSlugs) { + this(keyword, namespaceId, visibilityScope, sortBy, page, size, labelSlugs, false); + } + public SearchQuery( String keyword, Long namespaceId, @@ -21,6 +33,6 @@ public SearchQuery( String sortBy, int page, int size) { - this(keyword, namespaceId, visibilityScope, sortBy, page, size, List.of()); + this(keyword, namespaceId, visibilityScope, sortBy, page, size, List.of(), false); } } diff --git a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryService.java b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryService.java index 2e1ffcb14..640158443 100644 --- a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryService.java +++ b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryService.java @@ -107,6 +107,9 @@ public SearchResult search(SearchQuery query) { sql.append("FROM skill_search_document d "); sql.append("JOIN skill s ON s.id = d.skill_id "); sql.append("JOIN namespace n ON n.id = d.namespace_id "); + if (query.requireInstallableLatest()) { + sql.append("JOIN skill_version latest ON latest.id = s.latest_version_id "); + } sql.append("WHERE 1=1 "); // Visibility filtering @@ -120,6 +123,11 @@ public SearchResult search(SearchQuery query) { sql.append("AND d.status = 'ACTIVE' "); sql.append("AND s.status = 'ACTIVE' "); sql.append("AND s.hidden = FALSE "); + if (query.requireInstallableLatest()) { + sql.append("AND latest.status = 'PUBLISHED' "); + sql.append("AND latest.download_ready = TRUE "); + sql.append("AND latest.yanked_at IS NULL "); + } sql.append("AND (n.status <> 'ARCHIVED' "); if (query.visibilityScope().userId() != null) { sql.append("OR d.namespace_id IN :memberNamespaceIds "); diff --git a/server/skillhub-search/src/test/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryServiceTest.java b/server/skillhub-search/src/test/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryServiceTest.java index f89d05e93..c84c6cbd1 100644 --- a/server/skillhub-search/src/test/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryServiceTest.java +++ b/server/skillhub-search/src/test/java/com/iflytek/skillhub/search/postgres/PostgresFullTextQueryServiceTest.java @@ -363,6 +363,88 @@ void emptyKeywordRelevanceShouldUseStableNewestOrdering() { .contains("ORDER BY s.updated_at DESC, d.skill_id DESC"); } + @Test + void anonymousSearchSqlShouldOnlyReadPublicActiveVisibleNonArchivedSkills() { + EntityManager entityManager = mock(EntityManager.class); + Query nativeQuery = mock(Query.class); + Query countQuery = mock(Query.class); + when(entityManager.createNativeQuery(anyString())) + .thenReturn(nativeQuery) + .thenReturn(countQuery); + when(nativeQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(nativeQuery); + when(countQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(countQuery); + when(nativeQuery.getResultList()).thenReturn(List.of()); + when(countQuery.getSingleResult()).thenReturn(0L); + + PostgresFullTextQueryService service = new PostgresFullTextQueryService(entityManager); + + service.search(new SearchQuery( + null, + null, + new SearchVisibilityScope(null, Set.of(), Set.of()), + "newest", + 0, + 12 + )); + + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(entityManager, org.mockito.Mockito.times(2)).createNativeQuery(sqlCaptor.capture()); + assertThat(sqlCaptor.getAllValues().getFirst()) + .contains("AND (d.visibility = 'PUBLIC' )") + .contains("AND d.status = 'ACTIVE'") + .contains("AND s.status = 'ACTIVE'") + .contains("AND s.hidden = FALSE") + .contains("AND (n.status <> 'ARCHIVED' )") + .doesNotContain("memberNamespaceIds"); + verify(nativeQuery, never()).setParameter(org.mockito.ArgumentMatchers.eq("memberNamespaceIds"), org.mockito.ArgumentMatchers.any()); + verify(countQuery, never()).setParameter(org.mockito.ArgumentMatchers.eq("memberNamespaceIds"), org.mockito.ArgumentMatchers.any()); + } + + @Test + void installableLatestFilterShouldApplyToSearchAndCountQueries() { + EntityManager entityManager = mock(EntityManager.class); + Query nativeQuery = mock(Query.class); + Query countQuery = mock(Query.class); + when(entityManager.createNativeQuery(anyString())) + .thenReturn(nativeQuery) + .thenReturn(countQuery); + when(nativeQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(nativeQuery); + when(countQuery.setParameter(anyString(), org.mockito.ArgumentMatchers.any())).thenReturn(countQuery); + when(nativeQuery.getResultList()).thenReturn(List.of(2L)); + when(countQuery.getSingleResult()).thenReturn(1L); + + PostgresFullTextQueryService service = new PostgresFullTextQueryService(entityManager); + + var result = service.search(new SearchQuery( + "demo", + null, + SearchVisibilityScope.anonymous(), + "newest", + 0, + 1, + List.of(), + true + )); + + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(entityManager, org.mockito.Mockito.times(2)).createNativeQuery(sqlCaptor.capture()); + assertThat(sqlCaptor.getAllValues().getFirst()) + .contains("JOIN skill_version latest ON latest.id = s.latest_version_id") + .contains("AND latest.status = 'PUBLISHED'") + .contains("AND latest.download_ready = TRUE") + .contains("AND latest.yanked_at IS NULL") + .contains("LIMIT :limit OFFSET :offset"); + assertThat(sqlCaptor.getAllValues().get(1)) + .contains("JOIN skill_version latest ON latest.id = s.latest_version_id") + .contains("AND latest.status = 'PUBLISHED'") + .contains("AND latest.download_ready = TRUE") + .contains("AND latest.yanked_at IS NULL") + .doesNotContain("LIMIT :limit") + .doesNotContain("ORDER BY"); + assertThat(result.skillIds()).containsExactly(2L); + assertThat(result.total()).isEqualTo(1L); + } + @Test void authenticatedQueriesShouldAllowArchivedNamespacesForMembers() { EntityManager entityManager = mock(EntityManager.class);