From 42da467336f79e6f7e78d2dd2aeb6d4786f60aa7 Mon Sep 17 00:00:00 2001 From: wurongjie Date: Thu, 9 Apr 2026 13:53:52 +0800 Subject: [PATCH 01/25] fix(nginx): use X-Forwarded-Proto header for proper protocol forwarding Replace $scheme with $http_x_forwarded_proto in proxy headers to correctly forward the original client protocol when behind a reverse proxy or load balancer. This fixes OAuth2 authentication issues where redirects would use the wrong protocol scheme. --- web/nginx.conf.template | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/web/nginx.conf.template b/web/nginx.conf.template index fe0300b6..be2a51a2 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -10,6 +10,11 @@ server { gzip_types text/plain text/css application/json application/javascript text/xml; gzip_min_length 1000; + set $proxy_x_forwarded_proto $scheme; + if ($http_x_forwarded_proto) { + set $proxy_x_forwarded_proto $http_x_forwarded_proto; + } + location / { try_files $uri $uri/ /index.html; } @@ -19,27 +24,27 @@ server { proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /oauth2/ { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /login/oauth2/ { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /.well-known/ { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; - proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /assets/ { From f5f42454f796a0751dd6b33407be9307ed7a86a5 Mon Sep 17 00:00:00 2001 From: yaffir <97219715+yaffir@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:31:48 +0800 Subject: [PATCH 02/25] =?UTF-8?q?=E8=87=AA=E5=8A=A8=E5=B1=8F=E8=94=BD=20pl?= =?UTF-8?q?aceholder=20OAuth=20provider?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a method to validate OAuth provider configurations based on client ID. Signed-off-by: yaffir <97219715+yaffir@users.noreply.github.com> --- .../skillhub/service/AuthMethodCatalog.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java index 84324f18..cc927801 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java @@ -43,6 +43,7 @@ public class AuthMethodCatalog { public List listOAuthProviders(String returnTo) { String sanitizedReturnTo = OAuthLoginRedirectSupport.sanitizeReturnTo(returnTo); return new ArrayList<>(oAuth2ClientProperties.getRegistration().entrySet().stream() + .filter(entry -> isValidOAuthProvider(entry.getValue())) .sorted(Comparator.comparing(entry -> entry.getKey())) .map(entry -> new AuthProviderResponse( entry.getKey(), @@ -54,6 +55,19 @@ public class AuthMethodCatalog { .toList()); } + /** + * Check if an OAuth provider has valid configuration (non-empty client-id that is not a placeholder). + */ + private boolean isValidOAuthProvider(OAuth2ClientProperties.Registration registration) { + String clientId = registration.getClientId(); + if (clientId == null || clientId.isBlank()) { + return false; + } + // Filter out placeholder values used in dev/test configs + String lowerClientId = clientId.toLowerCase(); + return !lowerClientId.contains("placeholder") && !lowerClientId.contains("local-placeholder"); + } + public List listMethods(String returnTo) { String sanitizedReturnTo = OAuthLoginRedirectSupport.sanitizeReturnTo(returnTo); List methods = new ArrayList<>(); @@ -67,6 +81,7 @@ public class AuthMethodCatalog { )); oAuth2ClientProperties.getRegistration().entrySet().stream() + .filter(entry -> isValidOAuthProvider(entry.getValue())) .sorted(Comparator.comparing(entry -> entry.getKey())) .forEach(entry -> methods.add(new AuthMethodResponse( "oauth-" + entry.getKey(), From 74bad000e3a2d0e069228e9f4a766d1e24433357 Mon Sep 17 00:00:00 2001 From: shychee Date: Tue, 21 Jul 2026 15:58:32 +0800 Subject: [PATCH 03/25] fix(search): rebuild search index asynchronously after label change Attaching or detaching a skill label triggers a search index rebuild via an afterCommit callback. Because LabelSearchSyncService.rebuildSkill ran synchronously on the request thread, the @Transactional index write executed inside the already-committed transaction-synchronization phase and was silently dropped -- the search document was never written, so label keywords never became searchable. Move rebuildSkill onto the skillhubEventExecutor with @Async (matching the existing rebuildSkills batch path) so the rebuild runs on a fresh thread and transaction. Add an integration test that fails on the old synchronous path and passes with the async fix. Signed-off-by: shychee --- .../service/LabelSearchSyncService.java | 7 +- .../LabelSearchSyncIntegrationTest.java | 137 ++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelSearchSyncService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelSearchSyncService.java index 4920402d..0dc9ce41 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelSearchSyncService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelSearchSyncService.java @@ -20,8 +20,13 @@ public class LabelSearchSyncService { this.searchRebuildService = searchRebuildService; } + @Async("skillhubEventExecutor") public void rebuildSkill(Long skillId) { - searchRebuildService.rebuildBySkill(skillId); + try { + searchRebuildService.rebuildBySkill(skillId); + } catch (RuntimeException ex) { + log.error("Failed to rebuild search document for skill {}", skillId, ex); + } } @Async("skillhubEventExecutor") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java new file mode 100644 index 00000000..c7b9dd3a --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java @@ -0,0 +1,137 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.domain.label.LabelDefinition; +import com.iflytek.skillhub.domain.label.LabelDefinitionRepository; +import com.iflytek.skillhub.domain.label.LabelTranslation; +import com.iflytek.skillhub.domain.label.LabelTranslationRepository; +import com.iflytek.skillhub.domain.label.LabelType; +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.NamespaceType; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentEntity; +import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentJpaRepository; +import com.iflytek.skillhub.search.SearchEmbeddingService; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.ActiveProfiles; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; + +/** + * Reproduces the bug where attaching a skill label does not update the search + * index. The label keyword should appear in the rebuilt search document after + * {@code attachLabel} commits. + * + *

With the upstream (synchronous) {@code LabelSearchSyncService.rebuildSkill}, + * the rebuild runs inside the {@code afterCommit} callback on the request thread, + * where the {@code @Transactional index()} write does not persist — so the keyword + * never lands in the index and this test fails. Adding {@code @Async} moves the + * rebuild to a fresh thread/transaction and the keyword appears. + */ +@SpringBootTest(classes = SkillhubApplication.class) +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class LabelSearchSyncIntegrationTest { + + @Autowired + private SkillLabelAppService skillLabelAppService; + + @Autowired + private NamespaceRepository namespaceRepository; + + @Autowired + private SkillRepository skillRepository; + + @Autowired + private LabelDefinitionRepository labelDefinitionRepository; + + @Autowired + private LabelTranslationRepository labelTranslationRepository; + + @Autowired + private SkillSearchDocumentJpaRepository skillSearchDocumentJpaRepository; + + @MockBean + private SearchEmbeddingService searchEmbeddingService; + + @BeforeEach + void setUp() { + when(searchEmbeddingService.embed(anyString())).thenReturn(""); + when(searchEmbeddingService.similarity(anyString(), anyString())).thenReturn(0.0d); + } + + @Test + void attachingLabel_updatesSearchIndexWithLabelKeyword() throws Exception { + String suffix = UUID.randomUUID().toString().substring(0, 8); + String ownerId = "owner-" + suffix; + // ASCII display name so the tokenizer keeps it as a single searchable token. + String labelDisplayName = "MachineLearning" + suffix; + String labelSlug = "ml-" + suffix; + + Namespace namespace = new Namespace("ns-" + suffix, "NS " + suffix, ownerId); + namespace.setType(NamespaceType.GLOBAL); + namespace = namespaceRepository.save(namespace); + + Skill skill = new Skill(namespace.getId(), "skill-" + suffix, ownerId, SkillVisibility.PUBLIC); + skill.setDisplayName("Skill " + suffix); + skill.setSummary("A skill used to reproduce the label search sync bug."); + skill.setCreatedBy(ownerId); + skill.setUpdatedBy(ownerId); + skill = skillRepository.save(skill); + skillRepository.flush(); + + LabelDefinition label = labelDefinitionRepository.save( + new LabelDefinition(labelSlug, LabelType.RECOMMENDED, true, 0, ownerId)); + labelTranslationRepository.saveAll(List.of( + new LabelTranslation(label.getId(), "en", labelDisplayName))); + labelTranslationRepository.flush(); + + // Baseline: nothing indexed yet. + assertThat(skillSearchDocumentJpaRepository.findBySkillId(skill.getId())).isEmpty(); + + // Act: attach the label as the skill owner (passes resolve + permission checks). + Map ownerRoles = Map.of(namespace.getId(), NamespaceRole.OWNER); + skillLabelAppService.attachLabel( + namespace.getSlug(), + skill.getSlug(), + labelSlug, + ownerId, + ownerRoles, + new AuditRequestContext("127.0.0.1", "junit")); + + // Assert: the rebuilt search document must contain the label keyword. + SkillSearchDocumentEntity indexed = awaitIndexedDocument(skill.getId()); + assertThat(indexed.getKeywords()) + .as("label keyword should be indexed after attachLabel commits") + .contains(labelDisplayName); + } + + private SkillSearchDocumentEntity awaitIndexedDocument(Long skillId) throws InterruptedException { + Instant deadline = Instant.now().plus(Duration.ofSeconds(15)); + Optional indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId); + while (indexed.isEmpty() && Instant.now().isBefore(deadline)) { + Thread.sleep(100L); + indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId); + } + return indexed.orElseThrow( + () -> new AssertionError("Expected search document for skill " + skillId)); + } +} From de033da537c599d9aa65a5622a016d9718b91659 Mon Sep 17 00:00:00 2001 From: shychee Date: Wed, 22 Jul 2026 18:37:28 +0800 Subject: [PATCH 04/25] fix(search): make index writes REQUIRES_NEW to survive async caller-runs fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @Async rebuildSkill fix relied on a fresh thread giving a clean transaction boundary. But skillhubEventExecutor uses CallerRunsPolicy: under saturation the rejected task runs on the caller (request) thread, back inside the afterCommit synchronization phase — the original failure context where the @Transactional index write is silently dropped. Mark SearchIndexService.index as REQUIRES_NEW so it always suspends any lingering post-commit synchronization and commits in its own transaction, independent of whether the async dispatch actually happened. Add regression tests: detach removes the label keyword, and a synchronous rebuild inside the afterCommit phase still persists the document (fails without REQUIRES_NEW). Signed-off-by: shychee --- .../LabelSearchSyncIntegrationTest.java | 132 ++++++++++++++++++ .../PostgresFullTextIndexService.java | 3 +- 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java index c7b9dd3a..257d33d2 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java @@ -17,6 +17,7 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentEntity; import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentJpaRepository; import com.iflytek.skillhub.search.SearchEmbeddingService; +import com.iflytek.skillhub.search.SearchRebuildService; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -30,6 +31,9 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; import org.springframework.test.context.ActiveProfiles; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.support.TransactionTemplate; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyString; @@ -69,6 +73,12 @@ class LabelSearchSyncIntegrationTest { @Autowired private SkillSearchDocumentJpaRepository skillSearchDocumentJpaRepository; + @Autowired + private SearchRebuildService searchRebuildService; + + @Autowired + private TransactionTemplate transactionTemplate; + @MockBean private SearchEmbeddingService searchEmbeddingService; @@ -124,6 +134,65 @@ class LabelSearchSyncIntegrationTest { .contains(labelDisplayName); } + @Test + void detachingLabel_removesKeywordFromSearchIndex() throws Exception { + Fixture f = createFixture(); + + skillLabelAppService.attachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + SkillSearchDocumentEntity afterAttach = awaitIndexedDocument(f.skillId); + assertThat(afterAttach.getKeywords()) + .as("precondition: label keyword indexed after attach") + .contains(f.labelDisplayName); + + // Act: detach the same label. + skillLabelAppService.detachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + + // Assert: the rebuilt document must no longer contain the label keyword. + awaitKeywordAbsent(f.skillId, f.labelDisplayName); + } + + /** + * Guards against the {@code CallerRunsPolicy} regression: when the executor is + * saturated, {@code rebuildSkill} runs synchronously on the request thread inside + * the {@code afterCommit} phase — the exact context where the index write used to be + * dropped. This exercises that path directly (no async hop) and asserts the document + * is still persisted, proving the fix relies on {@code REQUIRES_NEW}, not on the + * executor having spare capacity. + */ + @Test + void syncRebuildInAfterCommitPhase_persistsIndex() throws Exception { + Fixture f = createFixture(); + + // Establish the skill-label association and a baseline index via the normal path. + skillLabelAppService.attachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + awaitIndexedDocument(f.skillId); + + // Clear the index so we can observe the synchronous rebuild in isolation. + transactionTemplate.executeWithoutResult( + status -> skillSearchDocumentJpaRepository.deleteBySkillId(f.skillId)); + assertThat(skillSearchDocumentJpaRepository.findBySkillId(f.skillId)).isEmpty(); + + // Rebuild synchronously on the caller thread, inside a post-commit synchronization + // (mirrors the CallerRuns fallback from afterCommit(() -> rebuildSkill(...))). + transactionTemplate.executeWithoutResult(status -> + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + searchRebuildService.rebuildBySkill(f.skillId); + } + })); + + SkillSearchDocumentEntity indexed = skillSearchDocumentJpaRepository.findBySkillId(f.skillId) + .orElseThrow(() -> new AssertionError( + "synchronous rebuild in afterCommit phase must persist the index document")); + assertThat(indexed.getKeywords()) + .as("label keyword must be indexed even on the synchronous caller-runs path") + .contains(f.labelDisplayName); + } + private SkillSearchDocumentEntity awaitIndexedDocument(Long skillId) throws InterruptedException { Instant deadline = Instant.now().plus(Duration.ofSeconds(15)); Optional indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId); @@ -134,4 +203,67 @@ class LabelSearchSyncIntegrationTest { return indexed.orElseThrow( () -> new AssertionError("Expected search document for skill " + skillId)); } + + private void awaitKeywordAbsent(Long skillId, String keyword) throws InterruptedException { + Instant deadline = Instant.now().plus(Duration.ofSeconds(15)); + while (Instant.now().isBefore(deadline)) { + Optional indexed = + skillSearchDocumentJpaRepository.findBySkillId(skillId); + if (indexed.isPresent() && !indexed.get().getKeywords().contains(keyword)) { + return; + } + Thread.sleep(100L); + } + String keywords = skillSearchDocumentJpaRepository.findBySkillId(skillId) + .map(SkillSearchDocumentEntity::getKeywords) + .orElse(""); + throw new AssertionError( + "Expected keyword '" + keyword + "' to be removed from index for skill " + + skillId + " but keywords were: " + keywords); + } + + private AuditRequestContext auditContext() { + return new AuditRequestContext("127.0.0.1", "junit"); + } + + private Fixture createFixture() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + String ownerId = "owner-" + suffix; + // ASCII display name so the tokenizer keeps it as a single searchable token. + String labelDisplayName = "MachineLearning" + suffix; + String labelSlug = "ml-" + suffix; + + Namespace namespace = new Namespace("ns-" + suffix, "NS " + suffix, ownerId); + namespace.setType(NamespaceType.GLOBAL); + namespace = namespaceRepository.save(namespace); + + Skill skill = new Skill(namespace.getId(), "skill-" + suffix, ownerId, SkillVisibility.PUBLIC); + skill.setDisplayName("Skill " + suffix); + skill.setSummary("A skill used to reproduce the label search sync bug."); + skill.setCreatedBy(ownerId); + skill.setUpdatedBy(ownerId); + skill = skillRepository.save(skill); + skillRepository.flush(); + + LabelDefinition label = labelDefinitionRepository.save( + new LabelDefinition(labelSlug, LabelType.RECOMMENDED, true, 0, ownerId)); + labelTranslationRepository.saveAll(List.of( + new LabelTranslation(label.getId(), "en", labelDisplayName))); + labelTranslationRepository.flush(); + + return new Fixture( + namespace.getSlug(), skill.getSlug(), skill.getId(), + labelSlug, labelDisplayName, ownerId, + Map.of(namespace.getId(), NamespaceRole.OWNER)); + } + + private record Fixture( + String namespaceSlug, + String skillSlug, + Long skillId, + String labelSlug, + String labelDisplayName, + String ownerId, + Map ownerRoles) { + } } diff --git a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java index bac3cce1..304b5ac0 100644 --- a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java +++ b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java @@ -6,6 +6,7 @@ import com.iflytek.skillhub.search.SearchEmbeddingService; import com.iflytek.skillhub.search.SearchIndexService; import com.iflytek.skillhub.search.SkillSearchDocument; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import java.util.List; @@ -32,7 +33,7 @@ public class PostgresFullTextIndexService implements SearchIndexService { } @Override - @Transactional + @Transactional(propagation = Propagation.REQUIRES_NEW) public void index(SkillSearchDocument document) { SkillSearchDocument normalizedDocument = normalize(document); Optional existing = repository.findBySkillId(document.skillId()); From 55a739a1bf68f7cf38224eb6ad79e4a045e5982e Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:15:52 +0800 Subject: [PATCH 05/25] docs(integrations): add HarnessClaw Engine skill guide HarnessClaw Engine loads skills from SKILL.md files with YAML frontmatter and parameter substitution, so SkillHub packages install into it directly via the CLI --dir option, the same way Hermes Agent does. Signed-off-by: FenjuFu --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index 2b068ab9..fe0fb4b9 100644 --- a/README.md +++ b/README.md @@ -442,6 +442,13 @@ namespace `my-space` plus skill slug `my-skill`. 📖 **[Complete Hermes Agent Integration Guide →](./docs/hermes-integration-en.md)** +### [HarnessClaw Engine](https://github.com/harnessclaw/harnessclaw-engine) + +[HarnessClaw Engine](https://github.com/harnessclaw/harnessclaw-engine) is a Go LLM programming assistant engine that exposes its capabilities over WebSocket. It loads skills from `SKILL.md` files with YAML frontmatter and parameter substitution, scanning each configured directory for `skill-name/SKILL.md` (default `~/.harnessclaw/workspace/skills/`, with earlier directories taking priority on name conflicts). Install a SkillHub package straight into that directory with the CLI's `--dir` option, no registry adapter required: + +```bash +npx clawhub --dir ~/.harnessclaw/workspace/skills install my-skill +``` ### [AstronClaw](https://agent.xfyun.cn/astron-claw) [AstronClaw](https://agent.xfyun.cn/astron-claw) is a cloud AI assistant built on OpenClaw's core capabilities, providing 24/7 online service through enterprise platforms like WeChat Work, DingTalk, and Feishu. It features a built-in skill system with over 130 official skills. You can connect it to a self-hosted SkillHub registry to enable one-click skill installation, search repository, dialogue-based automatic installation, and even custom skills management within your organization. From 3d6c7db040e94c8eebcc275a37a60b0a6529a1bf Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:15:57 +0800 Subject: [PATCH 06/25] docs(integrations): add HarnessClaw Engine skill guide HarnessClaw Engine loads skills from SKILL.md files with YAML frontmatter and parameter substitution, so SkillHub packages install into it directly via the CLI --dir option, the same way Hermes Agent does. Signed-off-by: FenjuFu --- README_zh.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README_zh.md b/README_zh.md index 42fb8225..3cfc79d6 100644 --- a/README_zh.md +++ b/README_zh.md @@ -376,6 +376,13 @@ namespace `my-space` 和 skill slug `my-skill`。 📖 **[完整 Hermes Agent 集成指南 →](./docs/hermes-integration.md)** +### [HarnessClaw Engine](https://github.com/harnessclaw/harnessclaw-engine) + +[HarnessClaw Engine](https://github.com/harnessclaw/harnessclaw-engine) 是基于 Go 的 LLM 编程助手引擎，通过 WebSocket 协议对外提供能力。它从 `SKILL.md` 文件加载技能，支持 YAML frontmatter 与参数替换，并按配置顺序扫描各目录下的 `skill-name/SKILL.md`（默认 `~/.harnessclaw/workspace/skills/`，靠前的目录在重名时优先）。通过 SkillHub CLI 的 `--dir` 参数即可把技能包直接安装到该目录，无需新增 registry 适配器： + +```bash +npx clawhub --dir ~/.harnessclaw/workspace/skills install my-skill +``` ### [AstronClaw](https://agent.xfyun.cn/astron-claw) [AstronClaw](https://agent.xfyun.cn/astron-claw) 是基于 OpenClaw 核心能力打造的云端 AI 助手，提供全天候在线服务，随时随地通过企业微信、钉钉、飞书等渠道提供服务。它内置了丰富的技能系统，您可以将其连接到自托管的 SkillHub 注册中心，支持技能市场一键安装、仓库搜索、对话自动安装，甚至管理和分发组织内部的自定义私有技能。 From 5b9fc1627718f2981f3e18b3299357e2218f4f81 Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:07:52 +0800 Subject: [PATCH 07/25] docs(readme): add star/watch buttons and guidance to first screen The badge row had no star or watch affordance. Adds social-style badges and a one-line note under the intro explaining why starring matters and how to watch releases only, in both language versions. --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 2b068ab9..38611044 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,9 @@ [![Java](https://img.shields.io/badge/java-21-ED8B00?logo=openjdk&logoColor=white)](https://openjdk.org/projects/jdk/21/) [![React](https://img.shields.io/badge/react-19-61DAFB?logo=react&logoColor=black)](https://react.dev) +[![GitHub Stars](https://img.shields.io/github/stars/iflytek/skillhub?style=social)](https://github.com/iflytek/skillhub/stargazers) +[![GitHub Watchers](https://img.shields.io/github/watchers/iflytek/skillhub?style=social)](https://github.com/iflytek/skillhub/watchers) +

@@ -35,6 +38,8 @@ it to a namespace, and let others find it through search or install it via CLI. Built for on-premise deployment behind your firewall, with the same polish you'd expect from a public registry. +> ⭐ If SkillHub fits your team, **star** the repo to help other teams find it, and **Watch → Custom → Releases** to get notified when a new version ships. + ## Documentation - 📖 **[User Guide](https://iflytek.github.io/skillhub/)** — Skill publishing, search, CLI usage and other user guides From 8dd0598acb1cea007cb67085cefd0f911af6392b Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:07:57 +0800 Subject: [PATCH 08/25] docs(readme): add star/watch buttons and guidance to first screen The badge row had no star or watch affordance. Adds social-style badges and a one-line note under the intro explaining why starring matters and how to watch releases only, in both language versions. --- README_zh.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README_zh.md b/README_zh.md index 42fb8225..fa233133 100644 --- a/README_zh.md +++ b/README_zh.md @@ -14,6 +14,9 @@ [![Java](https://img.shields.io/badge/java-21-ED8B00?logo=openjdk&logoColor=white)](https://openjdk.org/projects/jdk/21/) [![React](https://img.shields.io/badge/react-19-61DAFB?logo=react&logoColor=black)](https://react.dev) +[![GitHub Stars](https://img.shields.io/github/stars/iflytek/skillhub?style=social)](https://github.com/iflytek/skillhub/stargazers) +[![GitHub Watchers](https://img.shields.io/github/watchers/iflytek/skillhub?style=social)](https://github.com/iflytek/skillhub/watchers) +
--- @@ -24,6 +27,8 @@ SkillHub 是一个自托管平台，为团队提供私有的、受治理的智能体技能共享空间。发布技能包，推送到命名空间，让其他人通过搜索发现或通过 CLI 安装。专为防火墙后的本地部署而构建，提供与公共注册中心相同的精致体验。 +> ⭐ 如果 SkillHub 适合你的团队，欢迎 **Star** 本仓库帮助更多团队发现它；点 **Watch → Custom → Releases** 可在新版本发布时收到通知。 + ## 文档 - 📖 **[用户指南](https://iflytek.github.io/skillhub/)** — 技能发布、搜索、CLI 使用等用户操作指南 From 03085f19b59096e0dfe5b93d10bcc97fe51c9b90 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 11:08:37 +0800 Subject: [PATCH 09/25] docs(auth): define revoked token validation design (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- ...6-07-28-revoked-token-validation-design.md | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md diff --git a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md new file mode 100644 index 00000000..a50ac5ce --- /dev/null +++ b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md @@ -0,0 +1,150 @@ +# Revoked API Token Validation Design + +## Goal + +Prove and preserve fail-closed API-token behavior across the CLI API using a +real persisted token lifecycle. Invalid Bearer credentials must return HTTP +401 before endpoint business logic runs, while requests without an +`Authorization` header retain the existing anonymous-public-read contract and +valid credentials without sufficient authorization continue to return HTTP +403. + +## Scope + +This change covers the following CLI routes: + +- `GET /api/cli/v1/auth/whoami` +- `GET /api/cli/v1/skills/search` +- `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` +- `GET /api/cli/v1/skills/{namespace}/{slug}/download` +- `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` + +It also covers the authenticated-versus-forbidden boundary on an existing +scope-protected CLI route. It does not add endpoints, change response fields, +change token storage, add a database migration, or change anonymous resource +visibility rules. + +## Current-State Finding + +The fail-closed implementation from closed PR #511 was later included in the +single replacement PR #523 and is present in both v0.2.14 and current `main`. +`ApiTokenAuthenticationFilter` already validates Bearer credentials before +business logic and rejects empty, malformed, unknown, expired, revoked, +missing-user, and disabled-user credentials through the configured +`AuthenticationEntryPoint`. + +The verified repository gap is regression coverage, not a demonstrated +production-code gap. Existing tests separately prove token lifecycle +validation and invalid-Bearer filtering, but they do not exercise persisted +token creation, revocation, and all affected CLI endpoints in one integrated +matrix. The CLI API table in `docs/03-authentication-design.md` also retains +legacy paths, and there is no dedicated OpenAPI 3.0 authentication contract in +`docs/api/`. + +## Architecture + +`ApiTokenAuthenticationFilter` remains the single Bearer-authentication entry +point. Controllers must not duplicate token parsing or lifecycle checks. + +The regression test will boot the Spring application with MockMvc, real +`ApiTokenService`, real `ApiTokenRepository`, and real user persistence. CLI +endpoint business services may be mocked only to make successful public-read +responses deterministic; authentication and token lifecycle components remain +real. This isolates the contract boundary under test: a rejected credential +must stop in the security chain before controller business logic executes. + +Production authentication code will be changed only when a new regression +test fails for the expected behavioral reason. Any fix must be the smallest +change at the shared authentication or token-validation source of the failure. +Endpoint-specific authentication patches and unrelated refactoring are out of +scope. + +## Persisted Token Lifecycle + +The test fixture creates an active user and issues a token through +`ApiTokenService`, retaining only the raw token returned at creation time. +Lifecycle transitions use production persistence paths: + +1. Call an affected endpoint with the valid raw token and confirm successful + authentication. +2. Revoke the token through `ApiTokenService.revokeToken`. +3. Call every affected endpoint with the same raw token. +4. Assert HTTP 401 and confirm protected endpoint business logic was not + reached. + +Expired-token coverage persists a token with an expiration timestamp earlier +than the service clock, then validates it through the same filter and +repository path. Unknown and malformed tokens exercise the same HTTP security +chain without creating a token row. + +## Behavioral Matrix + +| Credential state | `whoami` | Public `search` | Public `resolve` | Public `download` | Meaning | +|---|---:|---:|---:|---:|---| +| No `Authorization` header | 401 | Existing anonymous result | Existing anonymous result | Existing anonymous result | Anonymous access is preserved only where already public | +| Valid active token | 200 | Authenticated result | Authenticated result | Authenticated result | Principal and roles/scopes are projected | +| Revoked token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Expired token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Unknown token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Malformed or empty Bearer | 401 | 401 | 401 | 401 | Authentication attempt is rejected before validation/business logic | +| Valid token lacking required authorization | N/A | N/A | 403 for a restricted resource or protected CLI action | 403 for a restricted resource or protected CLI action | Authenticated-but-forbidden remains distinct from invalid credentials | + +The test may use the existing scope-protected delete route to make the 403 +boundary deterministic without changing resource visibility or constructing a +private namespace scenario unrelated to token validation. + +## Error Handling and Security + +- Invalid Bearer credentials return the existing structured HTTP 401 response + through `ApiAuthenticationEntryPoint`. +- Valid credentials that fail scope or resource authorization return the + existing structured HTTP 403 response through the access-denied path. +- Responses must not reveal whether a token is unknown, expired, or revoked. +- Tests, logs, documentation, and commits must not contain real secrets. Test + credentials are generated locally and exist only in the in-memory test + database. +- Token material must never be logged. + +## Documentation + +Two documentation updates are required: + +1. Update `docs/03-authentication-design.md` so the CLI API section uses the + current `/api/cli/v1/...` routes and explicitly states the 401/403 and + anonymous-access boundary. +2. Add `docs/api/authentication.openapi.yaml` using OpenAPI 3.0. The document + must define Bearer authentication, all affected paths, query/path + parameters, success schemas, the common response envelope, HTTP 401 and 403 + responses, examples, and the rule that absent credentials are allowed only + on existing public-read routes. + +No controller signature or response schema changes are planned. Therefore the +generated `web/src/api/generated/schema.d.ts` should remain unchanged; if a +production fix unexpectedly changes a controller contract, `make generate-api` +becomes mandatory and the generated diff must be committed. + +## Verification + +Verification proceeds in this order: + +1. Run the new focused persisted-token matrix and record whether it fails or + passes on unmodified `main` behavior. +2. If it fails, preserve the failure output as reproduction evidence, apply one + minimal shared fix, and rerun the focused matrix. +3. Run auth-module and affected app integration tests. +4. Run `make test-backend-app`. +5. Run `make typecheck-web` and `make lint-web` as repository pre-PR gates. +6. Run `make staging` for containerized regression and smoke coverage. +7. Run `git diff --check` and confirm no generated OpenAPI type drift when no + controller contract changed. +8. Perform structured security and code review before opening the single final + pull request. + +## Delivery Constraints + +- Work only on `fix/auth-revoked-token-validation`. +- Keep PR #511 closed and use it only as historical reference. +- Create exactly one final pull request for GitHub issue #605. +- GitHub-facing text must not contain a Multica issue identifier. +- Do not merge `main`; merging remains the responsibility of an explicitly + authorized human owner. From 6567c19664f988a4fb7b218b9bbd33938134137a Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 11:20:08 +0800 Subject: [PATCH 10/25] docs(auth): tighten runtime validation gates (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- ...6-07-28-revoked-token-validation-design.md | 161 +++++++++++++++--- 1 file changed, 135 insertions(+), 26 deletions(-) diff --git a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md index a50ac5ce..43fc316a 100644 --- a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md +++ b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md @@ -19,10 +19,11 @@ This change covers the following CLI routes: - `GET /api/cli/v1/skills/{namespace}/{slug}/download` - `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` -It also covers the authenticated-versus-forbidden boundary on an existing -scope-protected CLI route. It does not add endpoints, change response fields, -change token storage, add a database migration, or change anonymous resource -visibility rules. +It also covers the authenticated-versus-forbidden boundary on the affected +restricted read routes. An existing scope-protected CLI route may provide +supplementary scope-filter evidence only. This change does not add endpoints, +change response fields, change token storage, add a database migration, or +change anonymous resource visibility rules. ## Current-State Finding @@ -41,6 +42,61 @@ matrix. The CLI API table in `docs/03-authentication-design.md` also retains legacy paths, and there is no dedicated OpenAPI 3.0 authentication contract in `docs/api/`. +The reported v0.2.14 runtime behavior still contradicts the source and test +evidence. Source equality alone does not establish which artifact or replica +served the reported requests. The defect therefore remains open until the +release artifact and affected runtime are identified and the same token +lifecycle is replayed against that identified runtime. + +## Release Artifact and Runtime Identity Gate + +Runtime verification is a required investigation track, not an optional +deployment check. Before interpreting a runtime result, record all of the +following for every server replica that may receive the request: + +1. The configured deployment version and resolved image reference from the + runtime environment and `docker compose config --images`. +2. The running container's image ID and registry `RepoDigest` from + `docker inspect` / `docker image inspect`. +3. The OCI `org.opencontainers.image.revision` and + `org.opencontainers.image.version` labels. The publish workflow generates + these labels and also publishes a `sha-` tag, so the revision can + be mapped back to a repository commit. +4. The externally observed application URL, health result, deployment profile, + and request IDs for the authentication probes. + +If the revision label is absent, the image digest must be mapped to the +corresponding publish-images workflow output or registry manifest. A mutable +tag such as `latest` or `v0.2.14` is not sufficient identity evidence by +itself. If neither a revision nor a digest-to-build mapping can be obtained, +the source/runtime contradiction is unresolved and the defect cannot be +closed. + +Using a dedicated test user and non-production token, replay one lifecycle +against the identified running image: + +1. Issue the token and call every matrix endpoint while it is valid. +2. Revoke that same token through the normal product flow and verify its + persisted `revoked_at` value without exposing the raw token. +3. Reuse the same raw token against every matrix endpoint and capture status, + response envelope, request ID, timestamp, and serving replica when + available. +4. Repeat or pin requests per replica when a load balancer can route to mixed + versions, and compare the image digest/revision of each replica. + +If production mutation is not authorized, run the exact identified digest in +an approved isolated environment with equivalent auth/proxy configuration and +record that limitation. This does not by itself close the original field +report: an authorized runtime replay or owner-provided equivalent evidence is +still required. + +The contradiction is closed only when source commit, published image digest, +running instance identity, and replay result form one consistent chain. A +mismatched digest indicates deployment drift; identical application images +with divergent behavior require investigation of proxy header forwarding, +mixed replicas, session/cookie contamination, and request routing before any +source-code conclusion is accepted. + ## Architecture `ApiTokenAuthenticationFilter` remains the single Bearer-authentication entry @@ -53,6 +109,14 @@ responses deterministic; authentication and token lifecycle components remain real. This isolates the contract boundary under test: a rejected credential must stop in the security chain before controller business logic executes. +The restricted-read authorization test is separate and must not mock the +permission decision. It will persist a PRIVATE or NAMESPACE_ONLY skill owned by +another user, authenticate a valid outsider token with no qualifying namespace +role, and exercise the real `CliSkillAppService` plus domain query/download +authorization path. At least `resolve`, latest download, and versioned download +must return HTTP 403. A DELETE request with a missing token scope may supplement +this check, but cannot replace any affected read-path assertion. + Production authentication code will be changed only when a new regression test fails for the expected behavioral reason. Any fix must be the smallest change at the shared authentication or token-validation source of the failure. @@ -79,19 +143,31 @@ chain without creating a token row. ## Behavioral Matrix -| Credential state | `whoami` | Public `search` | Public `resolve` | Public `download` | Meaning | -|---|---:|---:|---:|---:|---| -| No `Authorization` header | 401 | Existing anonymous result | Existing anonymous result | Existing anonymous result | Anonymous access is preserved only where already public | -| Valid active token | 200 | Authenticated result | Authenticated result | Authenticated result | Principal and roles/scopes are projected | -| Revoked token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Expired token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Unknown token | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Malformed or empty Bearer | 401 | 401 | 401 | 401 | Authentication attempt is rejected before validation/business logic | -| Valid token lacking required authorization | N/A | N/A | 403 for a restricted resource or protected CLI action | 403 for a restricted resource or protected CLI action | Authenticated-but-forbidden remains distinct from invalid credentials | +The authentication rows use deterministic public fixtures. Latest and +versioned downloads are independent endpoints and must have independent test +arguments and assertions for every credential state. -The test may use the existing scope-protected delete route to make the 403 -boundary deterministic without changing resource visibility or constructing a -private namespace scenario unrelated to token validation. +| Credential state | `whoami` | Public `search` | Public `resolve` | Public latest download | Public versioned download | Meaning | +|---|---:|---:|---:|---:|---:|---| +| No `Authorization` header | 401 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Anonymous access is preserved only where already public | +| Valid active token | 200 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Principal and roles/scopes are projected | +| Revoked token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Expired token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Unknown token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | +| Empty Bearer credential | 401 | 401 | 401 | 401 | 401 | Empty authentication attempt is rejected before business logic | +| Malformed Bearer credential | 401 | 401 | 401 | 401 | 401 | Malformed authentication attempt is rejected before business logic | + +The authorization row uses a persisted PRIVATE or NAMESPACE_ONLY fixture and +the real read-authorization path: + +| Valid credential, insufficient resource permission | `whoami` | `search` | Restricted `resolve` | Restricted latest download | Restricted versioned download | +|---|---:|---:|---:|---:|---:| +| Outsider token with no qualifying namespace role | 200 | 200 with restricted skill omitted | 403 | 403 | 403 | + +The same fixture must also prove that an authorized owner or qualifying +namespace member can reach the restricted read path, so a 403 cannot be caused +by an invalid fixture. Missing-scope DELETE coverage is optional supplementary +evidence for the API-token scope filter only. ## Error Handling and Security @@ -123,22 +199,52 @@ generated `web/src/api/generated/schema.d.ts` should remain unchanged; if a production fix unexpectedly changes a controller contract, `make generate-api` becomes mandatory and the generated diff must be committed. +## Implementation Plan Requirements + +The detailed implementation plan must preserve the following independent +steps rather than collapsing them into one generic download case: + +1. Create the real persisted token/user fixture and public endpoint stubs used + by the authentication matrix. +2. Exercise `whoami`, `search`, and `resolve` for every credential state. +3. Exercise latest download for every credential state. +4. Exercise versioned download for every credential state. +5. Persist a restricted skill plus authorized and unauthorized users, then use + the real read-authorization path to prove 403 for restricted `resolve`, + latest download, and versioned download and success for an authorized user. +6. Update the authentication design and OpenAPI contract. +7. Identify the published/running image and replay the valid-to-revoked token + lifecycle against that exact digest, or record the external access blocker + without treating the field contradiction as resolved. + +Each endpoint/state step must state its own expected status and test command. +The plan may share fixture helpers, but it must not share one assertion in a +way that can skip either download route. + ## Verification Verification proceeds in this order: 1. Run the new focused persisted-token matrix and record whether it fails or - passes on unmodified `main` behavior. -2. If it fails, preserve the failure output as reproduction evidence, apply one - minimal shared fix, and rerun the focused matrix. -3. Run auth-module and affected app integration tests. -4. Run `make test-backend-app`. -5. Run `make typecheck-web` and `make lint-web` as repository pre-PR gates. -6. Run `make staging` for containerized regression and smoke coverage. -7. Run `git diff --check` and confirm no generated OpenAPI type drift when no + passes on unmodified `main` behavior, with separate results for latest and + versioned download. +2. Run the persisted restricted-resource checks through real query/download + authorization and record outsider 403 plus authorized-user success. +3. If an authentication row fails, preserve the failure output as reproduction + evidence, apply one minimal shared fix, and rerun the focused matrix. +4. Run auth-module and affected app integration tests. +5. Run `make test-backend-app`. +6. Run `make typecheck-web` and `make lint-web` as repository pre-PR gates. +7. Run `make staging` for containerized regression and smoke coverage. +8. Run `git diff --check` and confirm no generated OpenAPI type drift when no controller contract changed. -8. Perform structured security and code review before opening the single final - pull request. +9. Record the release tag, build revision, image reference, immutable digest, + and every serving replica's running image identity. +10. Replay the same valid-to-revoked token lifecycle against the identified + runtime and record endpoint-level status, request ID, and replica evidence, + keeping latest and versioned download results separate. +11. Perform structured security and code review before opening the single final + pull request. ## Delivery Constraints @@ -146,5 +252,8 @@ Verification proceeds in this order: - Keep PR #511 closed and use it only as historical reference. - Create exactly one final pull request for GitHub issue #605. - GitHub-facing text must not contain a Multica issue identifier. +- Do not mark the defect resolved or eligible for closure while the reported + runtime behavior and the identified artifact/runtime replay remain + contradictory or incomplete. - Do not merge `main`; merging remains the responsibility of an explicitly authorized human owner. From e5b843967804d1698f2635cc1d0aea86f5afbc1b Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 11:37:07 +0800 Subject: [PATCH 11/25] docs(auth): plan revoked token regression coverage (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- .../2026-07-28-revoked-token-validation.md | 1081 +++++++++++++++++ 1 file changed, 1081 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-28-revoked-token-validation.md diff --git a/docs/superpowers/plans/2026-07-28-revoked-token-validation.md b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md new file mode 100644 index 00000000..d17b2d88 --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md @@ -0,0 +1,1081 @@ +# Revoked API Token Validation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Lock the CLI API's fail-closed Bearer behavior with persisted token lifecycle tests, prove 401/403 semantics on every affected read endpoint, publish the authentication OpenAPI contract, and reconcile source behavior with the actual runtime artifact. + +**Architecture:** Keep `ApiTokenAuthenticationFilter` as the sole Bearer authentication entry point. Use one Spring Boot/MockMvc class with real token and user persistence plus deterministic controller-service stubs for the credential-state matrix, and a second Spring Boot/MockMvc class with real query/download authorization and a persisted PRIVATE skill for resource-level 403 checks. Production authentication code remains unchanged unless the unmodified-source matrix reproduces a failure; any such failure stops this plan for systematic root-cause analysis before a minimal fix is planned. + +**Tech Stack:** Java 21, Spring Boot 3.2, Spring Security, Spring Data JPA/H2, MockMvc, JUnit 5 parameterized tests, Mockito, OpenAPI 3.0 YAML, Docker/OCI image inspection. + +--- + +## File Map + +- Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java`: persisted valid/revoked/expired/unknown/empty/malformed credential matrix for each CLI endpoint. +- Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java`: real PRIVATE-skill read authorization through resolve, latest download, and versioned download. +- Modify `docs/03-authentication-design.md`: current CLI route table and explicit anonymous/401/403 rules. +- Create `docs/api/authentication.openapi.yaml`: OpenAPI 3.0 contract for whoami, search, resolve, latest download, and versioned download. +- Do not modify `server/skillhub-auth/src/main/**` unless Task 5 records a failing unmodified-source assertion and a separate systematic-debugging plan amendment identifies the root cause. + +### Task 1: Persisted credential fixture and whoami/search/resolve matrix + +**Files:** +- Create: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java` + +- [ ] **Step 1: Create the integration test fixture and endpoint tests** + +Create the class with real `ApiTokenService`, `ApiTokenRepository`, and `UserAccountRepository`; mock only `CliSkillAppService` so successful public reads are deterministic. Add independent anonymous, valid, and parameterized invalid-state methods for whoami, search, and resolve: + +```java +package com.iflytek.skillhub.controller.cli; + +import com.iflytek.skillhub.auth.entity.ApiToken; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.repository.ApiTokenRepository; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.dto.cli.CliResolveResponse; +import com.iflytek.skillhub.service.cli.CliSkillAppService; +import java.io.ByteArrayInputStream; +import java.time.Clock; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +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.ResponseEntity; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.clearInvocations; +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; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class CliTokenLifecycleSecurityIntegrationTest { + + private enum InvalidCredentialState { + REVOKED, + EXPIRED, + UNKNOWN, + EMPTY, + MALFORMED + } + + @Autowired MockMvc mockMvc; + @Autowired ApiTokenService apiTokenService; + @Autowired ApiTokenRepository apiTokenRepository; + @Autowired UserAccountRepository userAccountRepository; + @Autowired Clock clock; + @MockBean CliSkillAppService cliSkillAppService; + + private String userId; + + @BeforeEach + void setUp() { + userId = "token-matrix-" + UUID.randomUUID(); + userAccountRepository.save(new UserAccount( + userId, "Token Matrix", userId + "@example.com", "")); + given(cliSkillAppService.search(any(), anyInt(), any(), any())) + .willReturn(new CliSkillAppService.CliSearchResult(List.of(), 0, 20)); + given(cliSkillAppService.resolve(anyString(), anyString(), any(), any(), any())) + .willReturn(new CliResolveResponse( + "global", "demo", "1.0.0", 1L, "sha256:empty", + "/api/v1/skills/global/demo/versions/1.0.0/download")); + given(cliSkillAppService.downloadLatest(anyString(), anyString(), any())) + .willAnswer(ignored -> downloadResponse()); + given(cliSkillAppService.downloadVersion(anyString(), anyString(), anyString(), any())) + .willAnswer(ignored -> downloadResponse()); + } + + @Test + void whoamiWithoutAuthorizationReturns401() throws Exception { + mockMvc.perform(get("/api/cli/v1/auth/whoami")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + + @Test + void whoamiWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/auth/whoami"), token)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.handle").value(userId)); + } + + @ParameterizedTest(name = "whoami rejects {0}") + @EnumSource(InvalidCredentialState.class) + void whoamiRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/auth/whoami"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void searchWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20")) + .andExpect(status().isOk()); + } + + @Test + void searchWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "search rejects {0}") + @EnumSource(InvalidCredentialState.class) + void searchRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void resolveWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/resolve")) + .andExpect(status().isOk()); + } + + @Test + void resolveWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/skills/global/demo/resolve"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "resolve rejects {0}") + @EnumSource(InvalidCredentialState.class) + void resolveRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/resolve"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + private MockHttpServletRequestBuilder withInvalidBearer( + MockHttpServletRequestBuilder request, + InvalidCredentialState state) { + return request + .header(HttpHeaders.AUTHORIZATION, authorizationHeader(state)) + .with(authentication(sessionAuthentication())); + } + + private MockHttpServletRequestBuilder withBearer( + MockHttpServletRequestBuilder request, + String rawToken) { + return request.header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken); + } + + private String authorizationHeader(InvalidCredentialState state) { + return switch (state) { + case REVOKED -> { + ApiTokenService.TokenCreateResult result = createToken(); + apiTokenService.revokeToken(result.entity().getId(), userId); + yield "Bearer " + result.rawToken(); + } + case EXPIRED -> { + ApiTokenService.TokenCreateResult result = createToken(); + ApiToken token = result.entity(); + token.setExpiresAt(Instant.now(clock).minusSeconds(1)); + apiTokenRepository.saveAndFlush(token); + yield "Bearer " + result.rawToken(); + } + case UNKNOWN -> "Bearer sk_unknown_" + UUID.randomUUID(); + case EMPTY -> "Bearer "; + case MALFORMED -> "Bearer"; + }; + } + + private String createActiveToken() { + return createToken().rawToken(); + } + + private ApiTokenService.TokenCreateResult createToken() { + return apiTokenService.createToken( + userId, "matrix-" + UUID.randomUUID(), "[\"skill:read\"]"); + } + + private UsernamePasswordAuthenticationToken sessionAuthentication() { + PlatformPrincipal principal = new PlatformPrincipal( + userId, "Session User", userId + "@example.com", "", "session", Set.of("USER")); + return new UsernamePasswordAuthenticationToken(principal, null, List.of()); + } + + private ResponseEntity downloadResponse() { + return ResponseEntity.ok(new InputStreamResource( + new ByteArrayInputStream("zip".getBytes(java.nio.charset.StandardCharsets.UTF_8)))); + } +} +``` + +- [ ] **Step 2: Apply a reversible fail-open mutation before the first test run** + +Temporarily change both rejection branches in `ApiTokenAuthenticationFilter.doFilterInternal` so malformed and invalid credentials continue down the chain. Do not stage or commit this mutation: + +```java +if (rawToken == null) { + filterChain.doFilter(request, response); + return; +} + +var token = apiTokenService.validateToken(rawToken); +if (token.isEmpty()) { + filterChain.doFilter(request, response); + return; +} +``` + +- [ ] **Step 3: Run whoami RED verification** + +Run: + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest#whoamiRejectsInvalidBearer \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: FAIL for the invalid-state invocations because the pre-authenticated session reaches whoami and returns 200 instead of 401. + +- [ ] **Step 4: Run search RED verification** + +Run the same Maven command with `#searchRejectsInvalidBearer`. + +Expected: FAIL with expected 401 but actual 200 for revoked, expired, unknown, empty, and malformed Bearer credentials. + +- [ ] **Step 5: Run resolve RED verification** + +Run the same Maven command with `#resolveRejectsInvalidBearer`. + +Expected: FAIL with expected 401 but actual 200 for all invalid credential states. + +- [ ] **Step 6: Restore the two original reject branches** + +Restore exactly: + +```java +if (rawToken == null) { + rejectBearer(request, response); + return; +} + +var token = apiTokenService.validateToken(rawToken); +if (token.isEmpty()) { + rejectBearer(request, response); + return; +} +``` + +Confirm `git diff -- server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java` is empty. + +- [ ] **Step 7: Run whoami/search/resolve GREEN commands independently** + +Run three Maven commands, one for each of: + +```text +CliTokenLifecycleSecurityIntegrationTest#whoamiRejectsInvalidBearer +CliTokenLifecycleSecurityIntegrationTest#searchRejectsInvalidBearer +CliTokenLifecycleSecurityIntegrationTest#resolveRejectsInvalidBearer +``` + +Expected: each command reports all parameterized invocations PASS, with no production authentication diff. + +### Task 2: Latest download matrix + +**Files:** +- Modify: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java` + +- [ ] **Step 1: Add independent latest-download methods** + +Insert before the helper methods: + +```java +@Test +void latestDownloadWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/download")) + .andExpect(status().isOk()); +} + +@Test +void latestDownloadWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/skills/global/demo/download"), token)) + .andExpect(status().isOk()); +} + +@ParameterizedTest(name = "latest download rejects {0}") +@EnumSource(InvalidCredentialState.class) +void latestDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/download"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); +} +``` + +- [ ] **Step 2: Reapply the reversible fail-open mutation and run latest-download RED** + +Run: + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest#latestDownloadRejectsInvalidBearer \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: FAIL with expected 401 but actual 200 for every invalid state. + +- [ ] **Step 3: Restore the original reject branches and run latest-download GREEN** + +Run the same command after restoring the filter. + +Expected: all five invalid-state invocations PASS. Then run independent anonymous and valid methods with `#latestDownloadWithoutAuthorizationReturns200` and `#latestDownloadWithValidPersistedTokenReturns200`; both PASS. + +### Task 3: Versioned download matrix + +**Files:** +- Modify: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java` + +- [ ] **Step 1: Add independent versioned-download methods** + +Insert before the helper methods: + +```java +@Test +void versionedDownloadWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/versions/1.0.0/download")) + .andExpect(status().isOk()); +} + +@Test +void versionedDownloadWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), token)) + .andExpect(status().isOk()); +} + +@ParameterizedTest(name = "versioned download rejects {0}") +@EnumSource(InvalidCredentialState.class) +void versionedDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); +} +``` + +- [ ] **Step 2: Reapply the reversible fail-open mutation and run versioned-download RED** + +Run the focused method command for `#versionedDownloadRejectsInvalidBearer`. + +Expected: FAIL with expected 401 but actual 200 for every invalid state. + +- [ ] **Step 3: Restore the filter and run versioned-download GREEN independently** + +Run focused commands for the invalid, anonymous, and valid versioned-download methods. + +Expected: all commands PASS and the filter source has no diff. + +- [ ] **Step 4: Run the complete persisted credential matrix** + +Run: + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: PASS for all five endpoints and all absent, valid, revoked, expired, unknown, empty, and malformed credential cases. + +- [ ] **Step 5: Commit the credential matrix** + +```bash +git add server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java +git commit -s -m "test(auth): cover persisted CLI token states (#605)" +``` + +### Task 4: Real restricted-read 403 boundary + +**Files:** +- Create: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java` + +- [ ] **Step 1: Create a persisted PRIVATE skill fixture and real authorization tests** + +```java +package com.iflytek.skillhub.controller.cli; + +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +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.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import java.time.Instant; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +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.http.HttpHeaders; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class CliRestrictedReadAuthorizationIntegrationTest { + + @Autowired MockMvc mockMvc; + @Autowired ApiTokenService apiTokenService; + @Autowired UserAccountRepository userAccountRepository; + @Autowired NamespaceRepository namespaceRepository; + @Autowired SkillRepository skillRepository; + @Autowired SkillVersionRepository skillVersionRepository; + + private String namespaceSlug; + private String skillSlug; + private String version; + private String ownerToken; + private String outsiderToken; + + @BeforeEach + void setUp() { + String suffix = UUID.randomUUID().toString().replace("-", ""); + String ownerId = "private-owner-" + suffix; + String outsiderId = "private-outsider-" + suffix; + namespaceSlug = "private-ns-" + suffix; + skillSlug = "private-skill-" + suffix; + version = "1.0.0"; + + userAccountRepository.save(new UserAccount(ownerId, "Owner", ownerId + "@example.com", "")); + userAccountRepository.save(new UserAccount( + outsiderId, "Outsider", outsiderId + "@example.com", "")); + ownerToken = apiTokenService.createToken( + ownerId, "owner-token-" + suffix, "[\"skill:read\"]").rawToken(); + outsiderToken = apiTokenService.createToken( + outsiderId, "outsider-token-" + suffix, "[\"skill:read\"]").rawToken(); + + Namespace namespace = namespaceRepository.save(new Namespace(namespaceSlug, "Private NS", ownerId)); + Skill skill = skillRepository.save(new Skill( + namespace.getId(), skillSlug, ownerId, SkillVisibility.PRIVATE)); + SkillVersion published = new SkillVersion(skill.getId(), version, ownerId); + published.setStatus(SkillVersionStatus.PUBLISHED); + published.setPublishedAt(Instant.parse("2026-07-28T00:00:00Z")); + published.setDownloadReady(true); + published = skillVersionRepository.save(published); + skill.setLatestVersionId(published.getId()); + skillRepository.save(skill); + skillRepository.flush(); + skillVersionRepository.flush(); + } + + @Test + void outsiderCannotResolvePrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void outsiderCannotDownloadLatestPrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/download", namespaceSlug, skillSlug), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void outsiderCannotDownloadVersionedPrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download", + namespaceSlug, skillSlug, version), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void ownerCanResolvePrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), + ownerToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.slug").value(skillSlug)); + } + + private MockHttpServletRequestBuilder withBearer( + MockHttpServletRequestBuilder request, + String rawToken) { + return request.header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken); + } +} +``` + +- [ ] **Step 2: Apply a reversible authorization mutation before the first run** + +Temporarily change only the PRIVATE arm in `VisibilityChecker.canAccess`: + +```java +case PRIVATE -> true; +``` + +Do not stage or commit this mutation. + +- [ ] **Step 3: Run three independent restricted-read RED commands** + +Run the focused Maven command separately for: + +```text +CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotResolvePrivateSkill +CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotDownloadLatestPrivateSkill +CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotDownloadVersionedPrivateSkill +``` + +Expected: each command FAILS because the outsider no longer receives 403. Resolve reaches 200; downloads proceed past authorization and return a non-403 response. + +- [ ] **Step 4: Restore PRIVATE authorization and run GREEN commands** + +Restore: + +```java +case PRIVATE -> isOwner(skill, currentUserId) || isAdminOrAbove(roles.get(skill.getNamespaceId())); +``` + +Confirm `git diff -- server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java` is empty. Run all four test methods independently. + +Expected: outsider resolve/latest/versioned each PASS with 403; owner resolve PASS with 200. + +- [ ] **Step 5: Run the existing search-visibility boundary tests** + +Run the search authorization checks independently as a supplementary 200-with-omission boundary: + +```bash +cd server +./mvnw -pl skillhub-app -am \ + -Dtest='PostgresFullTextQueryServiceTest#anonymousSearchSqlShouldOnlyReadPublicActiveVisibleNonArchivedSkills' \ + -Dsurefire.failIfNoSpecifiedTests=false test +./mvnw -pl skillhub-app -am \ + -Dtest='SkillSearchAppServiceTest#search_shouldIncludeMemberNamespacesInVisibilityScope' \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: both commands PASS. Record search as a successful response whose result set omits inaccessible PRIVATE skills; it is not a substitute for the real resolve/download 403 assertions above. + +- [ ] **Step 6: Commit the restricted-read tests** + +```bash +git add server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java +git commit -s -m "test(auth): cover restricted CLI read authorization (#605)" +``` + +### Task 5: Production-code decision gate + +**Files:** +- Inspect only: `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java` +- Inspect only: `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenService.java` + +- [ ] **Step 1: Confirm unmodified-source results and production diff** + +Run both new classes without any mutation, then run: + +```bash +git diff --exit-code origin/main -- \ + server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilter.java \ + server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenService.java +``` + +Expected: both classes PASS and the production authentication diff is empty. Record the outcome as “current source matrix passes; no production authentication change justified.” + +- [ ] **Step 2: Stop for systematic debugging if the expected result is false** + +If any unmodified-source assertion fails, stop execution before editing production code. Preserve the failing command and output, invoke `superpowers:systematic-debugging`, trace the request through token persistence, security chains, filters, and endpoint service boundaries, then amend this plan with the confirmed minimal change. Do not continue to documentation with a speculative fix. + +### Task 6: Authentication and OpenAPI documentation + +**Files:** +- Modify: `docs/03-authentication-design.md` +- Create: `docs/api/authentication.openapi.yaml` + +- [ ] **Step 1: Replace the CLI API table with current paths and semantics** + +Use this content in section 10.3: + +```markdown +### 10.3 CLI API + +| 接口 | 凭证规则 | 授权与错误语义 | +|------|---------|---------------| +| `GET /api/cli/v1/auth/whoami` | 必须提供有效 Bearer Token | 缺失、未知、过期、撤销或 malformed token 返回 401 | +| `GET /api/cli/v1/skills/search` | 可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；坏凭证返回 401，不得降级匿名 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | 可匿名读取公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | + +公共读接口仅在完全缺少 `Authorization` 头时允许匿名访问。请求一旦携带 Bearer 凭证，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 +``` + +- [ ] **Step 2: Create the complete OpenAPI 3.0 document** + +Create `docs/api/authentication.openapi.yaml` with `openapi: 3.0.3`, a `bearerAuth` HTTP bearer security scheme, all five paths, and these exact contract rules: + +```yaml +openapi: 3.0.3 +info: + title: SkillHub CLI Authentication API + version: 1.0.0 + description: >- + Authentication contract for CLI identity and public skill reads. Public read + operations permit a request with no Authorization header, but any supplied + Bearer credential must be valid; malformed, unknown, expired, or revoked + credentials return HTTP 401 and never fall back to anonymous access. +servers: + - url: / +tags: + - name: CLI Authentication + - name: CLI Skills +paths: + /api/cli/v1/auth/whoami: + get: + tags: [CLI Authentication] + summary: Return the current CLI identity + operationId: cliWhoAmI + security: + - bearerAuth: [] + responses: + '200': + description: Authenticated CLI identity + content: + application/json: + schema: + $ref: '#/components/schemas/CliWhoAmIEnvelope' + '401': + $ref: '#/components/responses/Unauthorized' + /api/cli/v1/skills/search: + get: + tags: [CLI Skills] + summary: Search CLI-installable skills + operationId: cliSearchSkills + description: No Authorization header uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + security: + - {} + - bearerAuth: [] + parameters: + - name: q + in: query + required: false + schema: {type: string} + example: pdf + description: Optional search text. + - name: limit + in: query + required: false + schema: {type: integer, format: int32, default: 20} + example: 20 + description: Maximum number of results. + responses: + '200': + description: Search result + content: + application/json: + schema: + $ref: '#/components/schemas/CliSearchEnvelope' + '401': + $ref: '#/components/responses/Unauthorized' + /api/cli/v1/skills/{namespace}/{slug}/resolve: + get: + tags: [CLI Skills] + summary: Resolve a skill version + operationId: cliResolveSkill + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + - name: version + in: query + required: false + schema: {type: string} + example: 1.0.0 + description: Optional exact version; omitted resolves latest. + responses: + '200': + description: Resolved version + content: + application/json: + schema: + $ref: '#/components/schemas/CliResolveEnvelope' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + /api/cli/v1/skills/{namespace}/{slug}/download: + get: + tags: [CLI Skills] + summary: Download the latest installable skill version + operationId: cliDownloadLatestSkill + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + responses: + '200': + $ref: '#/components/responses/Download' + '302': + $ref: '#/components/responses/DownloadRedirect' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '503': + $ref: '#/components/responses/StorageUnavailable' + /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download: + get: + tags: [CLI Skills] + summary: Download an exact installable skill version + operationId: cliDownloadSkillVersion + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + - $ref: '#/components/parameters/Version' + responses: + '200': + $ref: '#/components/responses/Download' + '302': + $ref: '#/components/responses/DownloadRedirect' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '503': + $ref: '#/components/responses/StorageUnavailable' +components: + securitySchemes: + bearerAuth: + type: http + scheme: bearer + bearerFormat: SkillHub API token + description: API token issued by SkillHub. Invalid lifecycle states all return the same 401 response. + parameters: + Namespace: + name: namespace + in: path + required: true + schema: {type: string} + example: global + description: Namespace slug. + Slug: + name: slug + in: path + required: true + schema: {type: string} + example: pdf-parser + description: Skill slug. + Version: + name: version + in: path + required: true + schema: {type: string} + example: 1.0.0 + description: Exact semantic version. + responses: + Download: + description: ZIP package stream + headers: + Content-Disposition: + schema: {type: string} + description: Attachment filename. + content: + application/zip: + schema: {type: string, format: binary} + DownloadRedirect: + description: Redirect to a presigned object-storage URL + headers: + Location: + schema: {type: string, format: uri} + BadRequest: + description: Namespace, skill, or version cannot be resolved. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + Unauthorized: + description: Bearer credential is missing where required, malformed, unknown, expired, revoked, or belongs to an unavailable user. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + example: + code: 401 + msg: Authentication required + data: null + timestamp: '2026-07-28T00:00:00Z' + requestId: req-123 + Forbidden: + description: Credential is valid but token scope or resource permission is insufficient. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + example: + code: 403 + msg: Forbidden + data: null + timestamp: '2026-07-28T00:00:00Z' + requestId: req-123 + StorageUnavailable: + description: Object storage is unavailable. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + schemas: + Envelope: + type: object + required: [code, msg, timestamp] + properties: + code: {type: integer, format: int32} + msg: {type: string} + data: {type: object, nullable: true} + timestamp: {type: string, format: date-time} + requestId: {type: string, nullable: true} + ErrorEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: {type: object, nullable: true, example: null} + CliWhoAmIEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliWhoAmI' + CliWhoAmI: + type: object + required: [handle, displayName, email] + properties: + handle: {type: string, example: user-123} + displayName: {type: string, example: CLI User} + email: {type: string, format: email, example: cli@example.com} + CliSearchEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliSearchResult' + CliSearchResult: + type: object + required: [items, total, limit] + properties: + items: + type: array + items: {$ref: '#/components/schemas/CliSearchItem'} + total: {type: integer, format: int64, example: 1} + limit: {type: integer, format: int32, example: 20} + CliSearchItem: + type: object + required: [namespace, slug, latestVersion] + properties: + namespace: {type: string, example: global} + slug: {type: string, example: pdf-parser} + latestVersion: {type: string, example: 1.2.0} + summary: {type: string, nullable: true, example: Parse PDF files} + CliResolveEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliResolveResult' + CliResolveResult: + type: object + required: [namespace, slug, version, versionId, fingerprint, downloadUrl] + properties: + namespace: {type: string, example: global} + slug: {type: string, example: pdf-parser} + version: {type: string, example: 1.2.0} + versionId: {type: integer, format: int64, example: 42} + fingerprint: {type: string, example: 'sha256:abc123'} + downloadUrl: {type: string, example: /api/v1/skills/global/pdf-parser/versions/1.2.0/download} +``` + +- [ ] **Step 3: Validate documentation formatting and contract paths** + +Run: + +```bash +ruby -e 'require "yaml"; YAML.load_file("docs/api/authentication.openapi.yaml"); puts "OpenAPI YAML OK"' +rg -n '/api/cli/v1/(auth/whoami|skills)' docs/03-authentication-design.md docs/api/authentication.openapi.yaml +git diff --check +``` + +Expected: YAML parser prints `OpenAPI YAML OK`, all five current paths are found, and `git diff --check` exits 0. + +- [ ] **Step 4: Commit authentication documentation** + +```bash +git add docs/03-authentication-design.md docs/api/authentication.openapi.yaml +git commit -s -m "docs(auth): document CLI token failure semantics (#605)" +``` + +### Task 7: Release artifact and runtime identity evidence + +**Files:** +- No repository file changes; evidence belongs in the active issue comment because runtime URLs, replica identities, and operational details may not be suitable for the public repository. + +- [ ] **Step 1: Resolve the published v0.2.14 server digest and revision** + +Run: + +```bash +docker buildx imagetools inspect ghcr.io/iflytek/skillhub-server:v0.2.14 +docker buildx imagetools inspect ghcr.io/iflytek/skillhub-server:sha-982258d +``` + +Expected: record the immutable manifest digest and confirm whether the release tag and SHA tag resolve to the same manifest. If registry access is denied, capture the denial and escalate access to the human owner. + +- [ ] **Step 2: Inspect every affected runtime replica when access is provided** + +On the runtime host, from the release compose directory, run: + +```bash +docker compose -f compose.release.yml config --images +SERVER_CONTAINER_IDS="$(docker compose -f compose.release.yml ps -q server)" +docker inspect --format '{{.Name}} {{.Config.Image}} {{.Image}} {{index .Config.Labels "org.opencontainers.image.revision"}} {{index .Config.Labels "org.opencontainers.image.version"}}' ${SERVER_CONTAINER_IDS} +for container_id in ${SERVER_CONTAINER_IDS}; do + image_id="$(docker inspect --format '{{.Image}}' "${container_id}")" + docker image inspect --format '{{json .RepoDigests}}' "${image_id}" +done +``` + +Expected: record configured version, resolved image reference, image ID, OCI revision/version, and immutable RepoDigest for every replica. A mutable tag alone is not a pass. + +- [ ] **Step 3: Replay one token lifecycle against the identified runtime** + +Using an authorized dedicated test account, create one token through the normal product flow, verify all five endpoint results while valid, revoke the same token, verify its database `revoked_at` through an authorized operational read, then repeat all five requests with the same raw token. Record HTTP status, response `requestId`, timestamp, and serving replica separately for whoami, search, resolve, latest download, and versioned download. Never paste the raw token into comments or logs. + +Expected after revocation: 401 on every endpoint. If behavior differs, preserve the exact digest/replica/request evidence and continue systematic root-cause investigation; do not claim the defect is fixed or closable. + +- [ ] **Step 4: Escalate missing runtime authority explicitly** + +If no affected runtime URL, host/replica access, or authorization to create/revoke a test token is available, explicitly escalate to the human owner in the active issue. Name the missing authority and request the exact evidence still required: deployed version, immutable server digest or build SHA, all replica identities, and same-token valid-to-revoked replay. State that repository tests do not close the field contradiction and therefore cannot justify closing the defect. + +### Task 8: Quality gates and implementation review handoff + +**Files:** +- Verify all changed files; do not create a PR in this stage. + +- [ ] **Step 1: Run both focused integration classes** + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest,CliRestrictedReadAuthorizationIntegrationTest \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: PASS, with latest and versioned download reported as distinct methods. + +- [ ] **Step 2: Run the complete backend gate** + +```bash +make test-backend-app +``` + +Expected: `BUILD SUCCESS`, zero failures, zero errors. + +- [ ] **Step 3: Run repository web gates required before delivery** + +```bash +make typecheck-web +make lint-web +``` + +Expected: zero TypeScript errors and zero ESLint errors/warnings. + +- [ ] **Step 4: Run containerized staging regression** + +```bash +make staging +``` + +Expected: backend/frontend images build, services become healthy, and smoke tests pass. Tear down with `make staging-down` after collecting evidence. + +- [ ] **Step 5: Verify scope, formatting, and commit hygiene** + +```bash +git diff --check origin/main...HEAD +git diff --name-only origin/main...HEAD +git status --short --branch +git log --format='%h %s%n%b' origin/main..HEAD +``` + +Expected: only the approved spec/plan, two test classes, authentication design, and OpenAPI document are changed; no production authentication source is changed when the matrix passes; all commits are signed off and reference GitHub issue #605 without any Multica identifier. + +- [ ] **Step 6: Route to tester and reviewer quality gates** + +Provide the branch, focused commands, complete matrix result, 403 fixture result, docs path, runtime identity/replay evidence or explicit external blocker, and full gate output to the project tester. After tester passes, request structured reviewer/security review. Address any findings on the same branch and rerun affected gates. + +- [ ] **Step 7: Report completion without creating a PR** + +Post the implementation result to the active issue thread. Include commit SHAs, endpoint-by-state matrix, RED mutation evidence, GREEN results, quality gates, OpenAPI path, production-code decision, and runtime identity/replay status. Do not create a PR, do not change issue status, and do not merge `main` during this stage. From 83b621880e3ee190f9776ec90420465671ab999a Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 12:03:24 +0800 Subject: [PATCH 12/25] test(auth): cover persisted CLI token states (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- ...TokenLifecycleSecurityIntegrationTest.java | 257 ++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java new file mode 100644 index 00000000..c02ebb4e --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java @@ -0,0 +1,257 @@ +package com.iflytek.skillhub.controller.cli; + +import com.iflytek.skillhub.auth.entity.ApiToken; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.repository.ApiTokenRepository; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.dto.cli.CliResolveResponse; +import com.iflytek.skillhub.service.cli.CliSkillAppService; +import java.io.ByteArrayInputStream; +import java.time.Clock; +import java.time.Instant; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +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.ResponseEntity; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.verifyNoInteractions; +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; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class CliTokenLifecycleSecurityIntegrationTest { + + private enum InvalidCredentialState { + REVOKED, + EXPIRED, + UNKNOWN, + EMPTY, + MALFORMED + } + + @Autowired MockMvc mockMvc; + @Autowired ApiTokenService apiTokenService; + @Autowired ApiTokenRepository apiTokenRepository; + @Autowired UserAccountRepository userAccountRepository; + @Autowired Clock clock; + @MockBean CliSkillAppService cliSkillAppService; + + private String userId; + + @BeforeEach + void setUp() { + userId = "token-matrix-" + UUID.randomUUID(); + userAccountRepository.save(new UserAccount( + userId, "Token Matrix", userId + "@example.com", "")); + given(cliSkillAppService.search(any(), anyInt(), any(), any())) + .willReturn(new CliSkillAppService.CliSearchResult(List.of(), 0, 20)); + given(cliSkillAppService.resolve(anyString(), anyString(), any(), any(), any())) + .willReturn(new CliResolveResponse( + "global", "demo", "1.0.0", 1L, "sha256:empty", + "/api/v1/skills/global/demo/versions/1.0.0/download")); + given(cliSkillAppService.downloadLatest(anyString(), anyString(), any())) + .willAnswer(ignored -> downloadResponse()); + given(cliSkillAppService.downloadVersion(anyString(), anyString(), anyString(), any())) + .willAnswer(ignored -> downloadResponse()); + } + + @Test + void whoamiWithoutAuthorizationReturns401() throws Exception { + mockMvc.perform(get("/api/cli/v1/auth/whoami")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + + @Test + void whoamiWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/auth/whoami"), token)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.handle").value(userId)); + } + + @ParameterizedTest(name = "whoami rejects {0}") + @EnumSource(InvalidCredentialState.class) + void whoamiRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/auth/whoami"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void searchWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20")) + .andExpect(status().isOk()); + } + + @Test + void searchWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "search rejects {0}") + @EnumSource(InvalidCredentialState.class) + void searchRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void resolveWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/resolve")) + .andExpect(status().isOk()); + } + + @Test + void resolveWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/skills/global/demo/resolve"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "resolve rejects {0}") + @EnumSource(InvalidCredentialState.class) + void resolveRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/resolve"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void latestDownloadWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/download")) + .andExpect(status().isOk()); + } + + @Test + void latestDownloadWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer(get("/api/cli/v1/skills/global/demo/download"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "latest download rejects {0}") + @EnumSource(InvalidCredentialState.class) + void latestDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/download"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + @Test + void versionedDownloadWithoutAuthorizationReturns200() throws Exception { + mockMvc.perform(get("/api/cli/v1/skills/global/demo/versions/1.0.0/download")) + .andExpect(status().isOk()); + } + + @Test + void versionedDownloadWithValidPersistedTokenReturns200() throws Exception { + String token = createActiveToken(); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), token)) + .andExpect(status().isOk()); + } + + @ParameterizedTest(name = "versioned download rejects {0}") + @EnumSource(InvalidCredentialState.class) + void versionedDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { + clearInvocations(cliSkillAppService); + mockMvc.perform(withInvalidBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), state)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + verifyNoInteractions(cliSkillAppService); + } + + private MockHttpServletRequestBuilder withInvalidBearer( + MockHttpServletRequestBuilder request, + InvalidCredentialState state) { + return request + .header(HttpHeaders.AUTHORIZATION, authorizationHeader(state)) + .with(authentication(sessionAuthentication())); + } + + private MockHttpServletRequestBuilder withBearer( + MockHttpServletRequestBuilder request, + String rawToken) { + return request.header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken); + } + + private String authorizationHeader(InvalidCredentialState state) { + return switch (state) { + case REVOKED -> { + ApiTokenService.TokenCreateResult result = createToken(); + apiTokenService.revokeToken(result.entity().getId(), userId); + yield "Bearer " + result.rawToken(); + } + case EXPIRED -> { + ApiTokenService.TokenCreateResult result = createToken(); + ApiToken token = result.entity(); + token.setExpiresAt(Instant.now(clock).minusSeconds(1)); + apiTokenRepository.saveAndFlush(token); + yield "Bearer " + result.rawToken(); + } + case UNKNOWN -> "Bearer sk_unknown_" + UUID.randomUUID(); + case EMPTY -> "Bearer "; + case MALFORMED -> "Bearer"; + }; + } + + private String createActiveToken() { + return createToken().rawToken(); + } + + private ApiTokenService.TokenCreateResult createToken() { + return apiTokenService.createToken( + userId, "matrix-" + UUID.randomUUID(), "[\"skill:read\"]"); + } + + private UsernamePasswordAuthenticationToken sessionAuthentication() { + PlatformPrincipal principal = new PlatformPrincipal( + userId, "Session User", userId + "@example.com", "", "session", Set.of("USER")); + return new UsernamePasswordAuthenticationToken(principal, null, List.of()); + } + + private ResponseEntity downloadResponse() { + return ResponseEntity.ok(new InputStreamResource( + new ByteArrayInputStream("zip".getBytes(java.nio.charset.StandardCharsets.UTF_8)))); + } +} From 52843c8020da52be4fc9f20464f1c9a3296f69cc Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 12:16:28 +0800 Subject: [PATCH 13/25] fix(test): assert CLI download media type (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- .../CliTokenLifecycleSecurityIntegrationTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java index c02ebb4e..04b19de5 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java @@ -24,6 +24,7 @@ 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.http.ResponseEntity; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.test.context.ActiveProfiles; @@ -38,6 +39,7 @@ import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.verifyNoInteractions; 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.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -163,7 +165,8 @@ class CliTokenLifecycleSecurityIntegrationTest { void latestDownloadWithValidPersistedTokenReturns200() throws Exception { String token = createActiveToken(); mockMvc.perform(withBearer(get("/api/cli/v1/skills/global/demo/download"), token)) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(content().contentType("application/zip")); } @ParameterizedTest(name = "latest download rejects {0}") @@ -187,7 +190,8 @@ class CliTokenLifecycleSecurityIntegrationTest { String token = createActiveToken(); mockMvc.perform(withBearer( get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), token)) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(content().contentType("application/zip")); } @ParameterizedTest(name = "versioned download rejects {0}") @@ -251,7 +255,9 @@ class CliTokenLifecycleSecurityIntegrationTest { } private ResponseEntity downloadResponse() { - return ResponseEntity.ok(new InputStreamResource( - new ByteArrayInputStream("zip".getBytes(java.nio.charset.StandardCharsets.UTF_8)))); + return ResponseEntity.ok() + .contentType(MediaType.parseMediaType("application/zip")) + .body(new InputStreamResource( + new ByteArrayInputStream("zip".getBytes(java.nio.charset.StandardCharsets.UTF_8)))); } } From 06cecd4237c57e9738b16db37ac25ab1d46a2a40 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 12:51:09 +0800 Subject: [PATCH 14/25] test(auth): cover restricted CLI read authorization (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- ...ictedReadAuthorizationIntegrationTest.java | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java new file mode 100644 index 00000000..0424e373 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java @@ -0,0 +1,123 @@ +package com.iflytek.skillhub.controller.cli; + +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +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.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import java.time.Instant; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +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.http.HttpHeaders; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class CliRestrictedReadAuthorizationIntegrationTest { + + @Autowired MockMvc mockMvc; + @Autowired ApiTokenService apiTokenService; + @Autowired UserAccountRepository userAccountRepository; + @Autowired NamespaceRepository namespaceRepository; + @Autowired SkillRepository skillRepository; + @Autowired SkillVersionRepository skillVersionRepository; + + private String namespaceSlug; + private String skillSlug; + private String version; + private String ownerToken; + private String outsiderToken; + + @BeforeEach + void setUp() { + String suffix = UUID.randomUUID().toString().replace("-", ""); + String ownerId = "private-owner-" + suffix; + String outsiderId = "private-outsider-" + suffix; + namespaceSlug = "private-ns-" + suffix; + skillSlug = "private-skill-" + suffix; + version = "1.0.0"; + + userAccountRepository.save(new UserAccount( + ownerId, "Private Skill Owner", ownerId + "@example.com", "")); + userAccountRepository.save(new UserAccount( + outsiderId, "Private Skill Outsider", outsiderId + "@example.com", "")); + ownerToken = apiTokenService.createToken( + ownerId, "owner-token-" + suffix, "[\"skill:read\"]").rawToken(); + outsiderToken = apiTokenService.createToken( + outsiderId, "outsider-token-" + suffix, "[\"skill:read\"]").rawToken(); + + Namespace namespace = namespaceRepository.save( + new Namespace(namespaceSlug, "Private Namespace", ownerId)); + Skill skill = skillRepository.save(new Skill( + namespace.getId(), skillSlug, ownerId, SkillVisibility.PRIVATE)); + SkillVersion published = new SkillVersion(skill.getId(), version, ownerId); + published.setStatus(SkillVersionStatus.PUBLISHED); + published.setPublishedAt(Instant.parse("2026-07-28T00:00:00Z")); + published.setDownloadReady(true); + published = skillVersionRepository.save(published); + skill.setLatestVersionId(published.getId()); + skillRepository.save(skill); + skillRepository.flush(); + skillVersionRepository.flush(); + } + + @Test + void outsiderCannotResolvePrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void outsiderCannotDownloadLatestPrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/download", namespaceSlug, skillSlug), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void outsiderCannotDownloadVersionedPrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download", + namespaceSlug, skillSlug, version), + outsiderToken)) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + + @Test + void ownerCanResolvePrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), + ownerToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.slug").value(skillSlug)); + } + + private MockHttpServletRequestBuilder withBearer( + MockHttpServletRequestBuilder request, + String rawToken) { + return request.header(HttpHeaders.AUTHORIZATION, "Bearer " + rawToken); + } +} From 5805e0f1d3431b671ffb9a9240499d59df2359fc Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 12:55:35 +0800 Subject: [PATCH 15/25] docs(auth): document CLI token failure semantics (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- docs/03-authentication-design.md | 13 +- docs/api/authentication.openapi.yaml | 288 +++++++++++++++++++++++++++ 2 files changed, 297 insertions(+), 4 deletions(-) create mode 100644 docs/api/authentication.openapi.yaml diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index b4fb0cd1..5c70b51f 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -621,10 +621,15 @@ window.location.href = '/oauth2/authorization/github' ### 10.3 CLI API -| 接口 | 所需凭证 | 额外判定 | -|------|---------|---------| -| `GET /api/v1/whoami` | 任意有效 Bearer Token | 无 | -| `POST /api/v1/publish` | Bearer Token + `skill:publish` | 普通用户要求目标 namespace 成员；`SUPER_ADMIN` 可绕过 | +| 接口 | 凭证规则 | 授权与错误语义 | +|------|---------|---------------| +| `GET /api/cli/v1/auth/whoami` | 必须提供有效 Bearer Token | 缺失、未知、过期、撤销或 malformed token 返回 401 | +| `GET /api/cli/v1/skills/search` | 可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；坏凭证返回 401，不得降级匿名 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | 可匿名读取公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | + +公共读接口仅在完全缺少 `Authorization` 头时允许匿名访问。请求一旦携带 Bearer 凭证，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 ### 10.4 Admin API diff --git a/docs/api/authentication.openapi.yaml b/docs/api/authentication.openapi.yaml new file mode 100644 index 00000000..97f170c5 --- /dev/null +++ b/docs/api/authentication.openapi.yaml @@ -0,0 +1,288 @@ +openapi: 3.0.3 +info: + title: SkillHub CLI Authentication API + version: 1.0.0 + description: >- + Authentication contract for CLI identity and public skill reads. Public read + operations permit a request with no Authorization header, but any supplied + Bearer credential must be valid; malformed, unknown, expired, or revoked + credentials return HTTP 401 and never fall back to anonymous access. +servers: + - url: / +tags: + - name: CLI Authentication + - name: CLI Skills +paths: + /api/cli/v1/auth/whoami: + get: + tags: [CLI Authentication] + summary: Return the current CLI identity + operationId: cliWhoAmI + security: + - bearerAuth: [] + responses: + '200': + description: Authenticated CLI identity + content: + application/json: + schema: + $ref: '#/components/schemas/CliWhoAmIEnvelope' + '401': + $ref: '#/components/responses/Unauthorized' + /api/cli/v1/skills/search: + get: + tags: [CLI Skills] + summary: Search CLI-installable skills + operationId: cliSearchSkills + description: No Authorization header uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + security: + - {} + - bearerAuth: [] + parameters: + - name: q + in: query + required: false + schema: {type: string} + example: pdf + description: Optional search text. + - name: limit + in: query + required: false + schema: {type: integer, format: int32, default: 20} + example: 20 + description: Maximum number of results. + responses: + '200': + description: Search result + content: + application/json: + schema: + $ref: '#/components/schemas/CliSearchEnvelope' + '401': + $ref: '#/components/responses/Unauthorized' + /api/cli/v1/skills/{namespace}/{slug}/resolve: + get: + tags: [CLI Skills] + summary: Resolve a skill version + operationId: cliResolveSkill + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + - name: version + in: query + required: false + schema: {type: string} + example: 1.0.0 + description: Optional exact version; omitted resolves latest. + responses: + '200': + description: Resolved version + content: + application/json: + schema: + $ref: '#/components/schemas/CliResolveEnvelope' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + /api/cli/v1/skills/{namespace}/{slug}/download: + get: + tags: [CLI Skills] + summary: Download the latest installable skill version + operationId: cliDownloadLatestSkill + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + responses: + '200': + $ref: '#/components/responses/Download' + '302': + $ref: '#/components/responses/DownloadRedirect' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '503': + $ref: '#/components/responses/StorageUnavailable' + /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download: + get: + tags: [CLI Skills] + summary: Download an exact installable skill version + operationId: cliDownloadSkillVersion + security: + - {} + - bearerAuth: [] + parameters: + - $ref: '#/components/parameters/Namespace' + - $ref: '#/components/parameters/Slug' + - $ref: '#/components/parameters/Version' + responses: + '200': + $ref: '#/components/responses/Download' + '302': + $ref: '#/components/responses/DownloadRedirect' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '503': + $ref: '#/components/responses/StorageUnavailable' +components: + securitySchemes: + bearerAuth: + type: http + scheme: bearer + bearerFormat: SkillHub API token + description: API token issued by SkillHub. Invalid lifecycle states all return the same 401 response. + parameters: + Namespace: + name: namespace + in: path + required: true + schema: {type: string} + example: global + description: Namespace slug. + Slug: + name: slug + in: path + required: true + schema: {type: string} + example: pdf-parser + description: Skill slug. + Version: + name: version + in: path + required: true + schema: {type: string} + example: 1.0.0 + description: Exact semantic version. + responses: + Download: + description: ZIP package stream + headers: + Content-Disposition: + schema: {type: string} + description: Attachment filename. + content: + application/zip: + schema: {type: string, format: binary} + DownloadRedirect: + description: Redirect to a presigned object-storage URL + headers: + Location: + schema: {type: string, format: uri} + BadRequest: + description: Namespace, skill, or version cannot be resolved. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + Unauthorized: + description: Bearer credential is missing where required, malformed, unknown, expired, revoked, or belongs to an unavailable user. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + example: + code: 401 + msg: Authentication required + data: null + timestamp: '2026-07-28T00:00:00Z' + requestId: req-123 + Forbidden: + description: Credential is valid but token scope or resource permission is insufficient. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + example: + code: 403 + msg: Forbidden + data: null + timestamp: '2026-07-28T00:00:00Z' + requestId: req-123 + StorageUnavailable: + description: Object storage is unavailable. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} + schemas: + Envelope: + type: object + required: [code, msg, timestamp] + properties: + code: {type: integer, format: int32} + msg: {type: string} + data: {type: object, nullable: true} + timestamp: {type: string, format: date-time} + requestId: {type: string, nullable: true} + ErrorEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: {type: object, nullable: true, example: null} + CliWhoAmIEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliWhoAmI' + CliWhoAmI: + type: object + required: [handle, displayName, email] + properties: + handle: {type: string, example: user-123} + displayName: {type: string, example: CLI User} + email: {type: string, format: email, example: cli@example.com} + CliSearchEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliSearchResult' + CliSearchResult: + type: object + required: [items, total, limit] + properties: + items: + type: array + items: {$ref: '#/components/schemas/CliSearchItem'} + total: {type: integer, format: int64, example: 1} + limit: {type: integer, format: int32, example: 20} + CliSearchItem: + type: object + required: [namespace, slug, latestVersion] + properties: + namespace: {type: string, example: global} + slug: {type: string, example: pdf-parser} + latestVersion: {type: string, example: 1.2.0} + summary: {type: string, nullable: true, example: Parse PDF files} + CliResolveEnvelope: + allOf: + - $ref: '#/components/schemas/Envelope' + - type: object + properties: + data: + $ref: '#/components/schemas/CliResolveResult' + CliResolveResult: + type: object + required: [namespace, slug, version, versionId, fingerprint, downloadUrl] + properties: + namespace: {type: string, example: global} + slug: {type: string, example: pdf-parser} + version: {type: string, example: 1.2.0} + versionId: {type: integer, format: int64, example: 42} + fingerprint: {type: string, example: 'sha256:abc123'} + downloadUrl: {type: string, example: /api/v1/skills/global/pdf-parser/versions/1.2.0/download} From e5f0cc140a94484ee07ecd7eb728a1a84a81af35 Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:49:38 +0800 Subject: [PATCH 16/25] docs(faq): add community-sourced deployment and operations Q&A (#593) * docs(faq): add community-sourced deployment and operations Q&A Adds entries collected from real user-support threads to the reference FAQ (both zh and en): - 502 on auth APIs while the page loads, traced to server startup failure on the SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET placeholder guard - config changes not taking effect (.env.release.example vs .env.release, restart vs recreate) - built-in skill sync failure in offline environments - upgrade path with Flyway auto-migration and volume retention - external dependencies and the lack of MySQL support - granting SUPER_ADMIN to an OAuth account via the bootstrap admin - telling CLI and server versions apart - installing skills into a target directory on an intranet Signed-off-by: FenjuFu * docs(faq): move entries to the published docs source and fix inaccuracies Move the new FAQ entries from document/ (a generated tree that the docs build does not read) to docs/skillhub/, which is what make docs-build and the Pages deploy actually publish. Also address review feedback: - drop the SKILLHUB_BUILTIN_SKILLS_ENABLED tip; compose.release.yml does not pass that variable through, so setting it has no effect - correct the dependency list: object storage defaults to local, S3 is recommended for production - soften the 502 wording, since upstream/DNS/network can also cause it - state the 32-character minimum for the cookie secret - give a real bulk-install example and qualify v0.2.12 as a server version - drop entries already covered by existing upgrade/MySQL/version questions Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> * docs(faq): correct deployment and admin guidance Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * docs(faq): fix remaining recreate guidance Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * docs(faq): clarify bulk install paths Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: FenjuFu Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/skillhub/en/faq.md | 83 +++++++++++++++++++++++++++++++++++++++-- docs/skillhub/faq.md | 83 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 160 insertions(+), 6 deletions(-) diff --git a/docs/skillhub/en/faq.md b/docs/skillhub/en/faq.md index 9cf6d556..7cc4500f 100644 --- a/docs/skillhub/en/faq.md +++ b/docs/skillhub/en/faq.md @@ -156,10 +156,18 @@ A: This is most commonly seen with **manual deployment** (caused by API errors o ## Q: How do I change the admin password? Why don't my config changes take effect? -A: Environment variables are read at container startup, so you must restart the containers after changing them. +A: Environment variables are injected when a container is created, so you must recreate the containers after changing them; `restart` alone does not re-inject environment variables. 1. Edit `/tmp/skillhub-runtime/.env.release` in the runtime directory (refer to [.env.release.example](https://github.com/iflytek/skillhub/blob/main/.env.release.example)). -2. Restart the relevant containers. +2. Recreate the relevant containers: + + ```bash + docker compose \ + --env-file /tmp/skillhub-runtime/.env.release \ + -f /tmp/skillhub-runtime/compose.release.yml \ + up -d --force-recreate + ``` + 3. If the password was already persisted to the database and the change still doesn't take effect, you may need to clear the corresponding data and re-initialize. ## Q: Is an email verification code required to change / reset a password? @@ -219,7 +227,7 @@ A: The default limit is **100 files** (this is separate from the 100MB size limi SKILLHUB_PUBLISH_MAX_FILE_COUNT=500 ``` -Restart the containers for the change to take effect. Note that `compose.release.yml` must also reference this variable; older versions (e.g. v0.2.6) may hard-code the value, so upgrading to the latest version is recommended. +Recreate the containers for the change to take effect; `restart` alone does not re-inject environment variables. Note that `compose.release.yml` must also reference this variable; older versions (e.g. v0.2.6) may hard-code the value, so upgrading to the latest version is recommended. ## Q: Is there a server version requirement for using the CLI (publish / download, etc.)? @@ -246,6 +254,75 @@ docker image inspect ghcr.io/iflytek/skillhub-server:latest --format '{{index .C - Check the CLI version: `skillhub version`. - For customization (e.g. changing the logo), it is recommended to fork the latest code, modify it, and build your own Docker image. +## Q: The page loads, but the login / register APIs return 502? + +A: The page is served by the `web` container, while login, register and other APIs are proxied by `web` to `server` (default `SKILLHUB_API_UPSTREAM=http://server:8080`). When the page works but the API returns 502, check whether `server` started correctly first; a wrong upstream, DNS, or container-network problem can also produce a 502. + +Troubleshooting order: + +```bash +# 1. Check whether server is running +docker compose --env-file .env.release -f compose.release.yml ps + +# 2. Look at the first error in the server startup log +docker compose --env-file .env.release -f compose.release.yml logs server | head -50 +``` + +One common startup failure is: + +``` +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET must not use the default placeholder +``` + +This means `server` still reads the placeholder from the template. Replace it in `.env.release` with your own random string (**at least 32 characters**) and recreate the containers: + +```bash +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET= +``` + +Running `make validate-release-config` before startup validates `.env.release` and surfaces placeholders and missing values early. + +## Q: Why doesn't my configuration change take effect? + +A: Two common causes: + +1. **Edited the wrong file**: `.env.release.example` is only a template; Compose reads the file passed via `--env-file`, i.e. `.env.release`. Run `cp .env.release.example .env.release` first, then edit `.env.release`. +2. **Restarted instead of recreated**: environment variables are injected when the container is created, and `restart` does not re-inject them. Recreate the containers after a config change: + +```bash +docker compose --env-file .env.release -f compose.release.yml up -d --force-recreate +``` + +## Q: What external dependencies does SkillHub require at runtime? + +A: PostgreSQL and Redis are required. Object storage supports both `local` and S3, controlled by `SKILLHUB_STORAGE_PROVIDER`. `.env.release.example` explicitly selects `local`, but if the variable is completely unset when using `compose.release.yml`, the Compose fallback is `s3`. Set it explicitly; S3 is recommended for production (configured via `SKILLHUB_STORAGE_S3_*`). Only PostgreSQL is supported as the database — MySQL is not. + +The release Compose file already bundles PostgreSQL and Redis, bound to `127.0.0.1` by default. + +## Q: How does an account created through OAuth (GitHub / GitLab, etc.) get admin rights? + +A: The first OAuth login creates a regular user. An existing `SUPER_ADMIN` (for example the bootstrap admin created during initialization) has to promote it from the admin console. + +A `USER_ADMIN` can manage user status and assign platform roles other than `SUPER_ADMIN`, but cannot grant `SUPER_ADMIN` to any account or change the role of an existing `SUPER_ADMIN`. Only a `SUPER_ADMIN` can perform those two operations. + +## Q: How do I install multiple skills in bulk? + +A: The CLI `install` command handles one skill at a time. Both examples below use `--dir` to install the skills under the same target root; each skill is placed in `$target_dir//`: + +```bash +target_dir=/opt/skillhub-skills + +# install one by one +for skill in skill-a skill-b skill-c; do + skillhub install "$skill" --dir "$target_dir" +done + +# or read from a manifest file (one skill name per line) +xargs -a skills.txt -I {} skillhub install "{}" --dir "$target_dir" +``` + +Since **SkillHub Server v0.2.12**, public skills support anonymous search and install. Note that an invalid bearer token now fails the command instead of falling back to anonymous access — update or remove the stale credential in that case. + ## Q: What should I do if I encounter issues? A: You can get help through the following channels: diff --git a/docs/skillhub/faq.md b/docs/skillhub/faq.md index 2aac0a54..b16ecd81 100644 --- a/docs/skillhub/faq.md +++ b/docs/skillhub/faq.md @@ -156,10 +156,18 @@ A: 该现象多见于「手动部署」场景（接口异常或初始化未完 ## Q: 如何修改 admin 密码？修改配置后不生效？ -A: 环境变量在容器启动时读取，修改后必须重启容器才会生效。 +A: 环境变量在容器创建时注入，修改后必须重新创建容器才会生效；仅执行 `restart` 不会重新注入环境变量。 1. 修改运行时目录下的 `/tmp/skillhub-runtime/.env.release`（参考仓库 [.env.release.example](https://github.com/iflytek/skillhub/blob/main/.env.release.example)）。 -2. 重启相关容器。 +2. 重新创建相关容器： + + ```bash + docker compose \ + --env-file /tmp/skillhub-runtime/.env.release \ + -f /tmp/skillhub-runtime/compose.release.yml \ + up -d --force-recreate + ``` + 3. 若此前密码已写入数据库导致仍不生效，可能需要清理对应数据后重新初始化。 ## Q: 修改 / 找回密码必须使用邮箱验证码吗？ @@ -219,7 +227,7 @@ A: 默认上限为 **100 个文件**（这与 100MB 的大小限制是两回事 SKILLHUB_PUBLISH_MAX_FILE_COUNT=500 ``` -修改后需重启容器生效。注意 `compose.release.yml` 中也需引用该变量；较旧版本（如 v0.2.6）可能将该值写死，建议升级到最新版本。 +修改后需重新创建容器才会生效；仅执行 `restart` 不会重新注入环境变量。注意 `compose.release.yml` 中也需引用该变量；较旧版本（如 v0.2.6）可能将该值写死，建议升级到最新版本。 ## Q: 使用 CLI（发布 / 下载等）对服务端版本有要求吗？ @@ -246,6 +254,75 @@ docker image inspect ghcr.io/iflytek/skillhub-server:latest --format '{{index .C - 查看 CLI 版本：`skillhub version`。 - 如需定制（如修改 logo 等），建议基于最新代码进行二次开发并自行构建 docker 镜像。 +## Q: 页面能打开，但登录 / 注册接口返回 502？ + +A: 页面由 `web` 容器提供，登录、注册等接口由 `web` 转发给 `server`（默认 `SKILLHUB_API_UPSTREAM=http://server:8080`）。出现「页面正常但 API 502」时，通常先检查 `server` 是否正常启动；upstream 配置、DNS 或容器网络异常也可能返回 502。 + +排查顺序： + +```bash +# 1. 看 server 是否处于运行状态 +docker compose --env-file .env.release -f compose.release.yml ps + +# 2. 看 server 启动日志中的第一条错误 +docker compose --env-file .env.release -f compose.release.yml logs server | head -50 +``` + +一条常见的启动失败日志是： + +``` +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET must not use the default placeholder +``` + +说明 `server` 读到的仍是模板里的占位值。在 `.env.release` 中改成自己的随机字符串（**至少 32 个字符**）后重建容器即可： + +```bash +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=<替换成你自己的随机字符串，至少 32 个字符> +``` + +启动前可以先执行 `make validate-release-config`，它会校验 `.env.release`，提前暴露这类占位值和缺失项。 + +## Q: 改了配置为什么不生效？ + +A: 两个高频原因： + +1. **改错了文件**：`.env.release.example` 只是模板，Compose 实际读取的是 `--env-file` 指定的 `.env.release`。请先 `cp .env.release.example .env.release`，然后修改 `.env.release`。 +2. **只重启没重建**：环境变量在容器创建时注入，`restart` 不会重新注入。改完配置需要重建容器： + +```bash +docker compose --env-file .env.release -f compose.release.yml up -d --force-recreate +``` + +## Q: SkillHub 运行时需要哪些外部依赖？ + +A: 必需 PostgreSQL 和 Redis；对象存储支持 `local` 与 S3 两种模式，由 `SKILLHUB_STORAGE_PROVIDER` 控制。`.env.release.example` 显式配置为 `local`，但如果使用 `compose.release.yml` 时完全没有设置该变量，Compose 的回退值是 `s3`。建议始终显式设置；生产环境推荐使用 S3（通过 `SKILLHUB_STORAGE_S3_*` 配置）。数据库仅支持 PostgreSQL，暂不支持 MySQL。 + +发布版 Compose 已内置 PostgreSQL 与 Redis，默认只绑定在 `127.0.0.1`。 + +## Q: 通过 OAuth（GitHub / GitLab 等）登录的账号，如何取得管理员权限？ + +A: OAuth 首次登录创建的是普通用户。需要由已有的 `SUPER_ADMIN`（例如初始化时的 bootstrap admin）在后台将其提升为管理员。 + +`USER_ADMIN` 可以管理用户状态，并分配除 `SUPER_ADMIN` 之外的平台角色；但不能向任何账号授予 `SUPER_ADMIN`，也不能修改已有 `SUPER_ADMIN` 账号的角色。这两类操作只有 `SUPER_ADMIN` 可以执行。 + +## Q: 如何批量安装多个技能包？ + +A: CLI 的 `install` 一次处理一个技能包。下面两个示例都通过 `--dir` 将技能批量安装到同一个目标根目录；每个技能实际位于 `$target_dir//`： + +```bash +target_dir=/opt/skillhub-skills + +# 逐个安装 +for skill in skill-a skill-b skill-c; do + skillhub install "$skill" --dir "$target_dir" +done + +# 或从清单文件读取（每行一个技能名） +xargs -a skills.txt -I {} skillhub install "{}" --dir "$target_dir" +``` + +自 **SkillHub Server v0.2.12** 起，公开技能支持匿名搜索与安装；如果配置了无效的 Bearer Token，命令会直接失败而不再回退匿名访问，遇到这种情况请更新凭据或先移除无效 Token。 + ## Q: 遇到问题怎么办？ A: 可以通过以下方式获取帮助： From 726eeac8b24dc85a6e98276738d211ef92551d58 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 13:52:26 +0800 Subject: [PATCH 17/25] test(auth): cover token replay and private search (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- ...ictedReadAuthorizationIntegrationTest.java | 29 +++++++++ ...TokenLifecycleSecurityIntegrationTest.java | 62 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java index 0424e373..5a29cffa 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java @@ -11,6 +11,8 @@ import com.iflytek.skillhub.domain.skill.SkillVersionStatus; 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.infra.jpa.SkillSearchDocumentEntity; +import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentJpaRepository; import java.time.Instant; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; @@ -23,6 +25,9 @@ import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; 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; @@ -38,6 +43,7 @@ class CliRestrictedReadAuthorizationIntegrationTest { @Autowired NamespaceRepository namespaceRepository; @Autowired SkillRepository skillRepository; @Autowired SkillVersionRepository skillVersionRepository; + @Autowired SkillSearchDocumentJpaRepository skillSearchDocumentRepository; private String namespaceSlug; private String skillSlug; @@ -76,6 +82,29 @@ class CliRestrictedReadAuthorizationIntegrationTest { skillRepository.save(skill); skillRepository.flush(); skillVersionRepository.flush(); + skillSearchDocumentRepository.saveAndFlush(new SkillSearchDocumentEntity( + skill.getId(), + namespace.getId(), + namespaceSlug, + ownerId, + skillSlug, + "Private skill search fixture", + "private", + skillSlug, + "", + SkillVisibility.PRIVATE.name(), + skill.getStatus().name())); + } + + @Test + void outsiderSearchOmitsPersistedPrivateSkill() throws Exception { + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/search").param("limit", "20"), + outsiderToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", aMapWithSize(5))) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.items[*].slug", not(hasItem(skillSlug)))); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java index 04b19de5..e351a7e5 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java @@ -29,8 +29,11 @@ import org.springframework.http.ResponseEntity; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; @@ -205,6 +208,65 @@ class CliTokenLifecycleSecurityIntegrationTest { verifyNoInteractions(cliSkillAppService); } + @Test + void sameRawTokenIsRejectedByAllEndpointsAfterValidUseAndRevocation() throws Exception { + ApiTokenService.TokenCreateResult token = createToken(); + String rawToken = token.rawToken(); + + assertSuccessEnvelope(withBearer(get("/api/cli/v1/auth/whoami"), rawToken)) + .andExpect(jsonPath("$.data.handle").value(userId)); + assertSuccessEnvelope(withBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), + rawToken)); + assertSuccessEnvelope(withBearer( + get("/api/cli/v1/skills/global/demo/resolve"), rawToken)); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/global/demo/download"), rawToken)) + .andExpect(status().isOk()) + .andExpect(content().contentType("application/zip")); + mockMvc.perform(withBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), rawToken)) + .andExpect(status().isOk()) + .andExpect(content().contentType("application/zip")); + + apiTokenService.revokeToken(token.entity().getId(), userId); + clearInvocations(cliSkillAppService); + + assertUnauthorizedEnvelope(withBearer(get("/api/cli/v1/auth/whoami"), rawToken)); + assertUnauthorizedEnvelope(withBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), + rawToken)); + assertUnauthorizedEnvelope(withBearer( + get("/api/cli/v1/skills/global/demo/resolve"), rawToken)); + assertUnauthorizedEnvelope(withBearer( + get("/api/cli/v1/skills/global/demo/download"), rawToken)); + assertUnauthorizedEnvelope(withBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), rawToken)); + verifyNoInteractions(cliSkillAppService); + } + + private ResultActions assertSuccessEnvelope(MockHttpServletRequestBuilder request) throws Exception { + return mockMvc.perform(request) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", aMapWithSize(5))) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.msg").isString()) + .andExpect(jsonPath("$.data").exists()) + .andExpect(jsonPath("$.timestamp").isString()) + .andExpect(jsonPath("$.requestId").isString()); + } + + private void assertUnauthorizedEnvelope(MockHttpServletRequestBuilder request) throws Exception { + mockMvc.perform(request) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$", aMapWithSize(5))) + .andExpect(jsonPath("$.code").value(401)) + .andExpect(jsonPath("$.msg").isString()) + .andExpect(jsonPath("$.data").value(nullValue())) + .andExpect(jsonPath("$.timestamp").isString()) + .andExpect(jsonPath("$.requestId").isString()); + } + private MockHttpServletRequestBuilder withInvalidBearer( MockHttpServletRequestBuilder request, InvalidCredentialState state) { From 8163a48e9e489f1c6f7c3e854274bb8c7f50997e Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 13:52:42 +0800 Subject: [PATCH 18/25] docs(auth): align Bearer-only response contract (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- docs/03-authentication-design.md | 4 +- docs/api/authentication.openapi.yaml | 17 +++-- .../2026-07-28-revoked-token-validation.md | 71 ++++++++++++++----- ...6-07-28-revoked-token-validation-design.md | 14 ++-- 4 files changed, 75 insertions(+), 31 deletions(-) diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 5c70b51f..e28f10af 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -377,7 +377,7 @@ API Token 仍保留，但定位从“CLI 唯一认证方式”调整为“平台 - 用途：自动化脚本、兼容层调用、手工 Token 管理、后续系统集成 - 存储：只存 SHA-256 哈希，明文只展示一次 - 校验：从 `Authorization: Bearer ` 提取 → 哈希比对 → 加载关联用户 → 检查用户状态 -- 失败闭合：公共读接口只有在缺少 `Authorization` 头时才按匿名访问处理；只要出现 Bearer 凭证，空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 401，不能回退为匿名访问 +- 失败闭合：共享认证过滤器只识别 Bearer scheme；公共读接口在未提供可识别的 Bearer 凭证时按匿名访问处理（包括缺少 `Authorization` 头，以及 Basic 或其他非 Bearer scheme）。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 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 的绑定。 @@ -629,7 +629,7 @@ window.location.href = '/oauth2/authorization/github' | `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | | `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -公共读接口仅在完全缺少 `Authorization` 头时允许匿名访问。请求一旦携带 Bearer 凭证，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 +共享 API token 过滤器只识别 Bearer scheme。公共读接口在未提供可识别的 Bearer 凭证时允许匿名访问：这既包括完全缺少 `Authorization` 头，也包括 Basic 或其他非 Bearer scheme；`whoami` 因自身要求认证，在这些情况下仍返回 401。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401，不能降级为匿名身份。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 ### 10.4 Admin API diff --git a/docs/api/authentication.openapi.yaml b/docs/api/authentication.openapi.yaml index 97f170c5..4fafac22 100644 --- a/docs/api/authentication.openapi.yaml +++ b/docs/api/authentication.openapi.yaml @@ -4,9 +4,11 @@ info: version: 1.0.0 description: >- Authentication contract for CLI identity and public skill reads. Public read - operations permit a request with no Authorization header, but any supplied - Bearer credential must be valid; malformed, unknown, expired, or revoked - credentials return HTTP 401 and never fall back to anonymous access. + operations treat a request with no recognized Bearer credential as anonymous, + including an absent Authorization header or an unsupported scheme such as + Basic. Once the Bearer scheme is used, the credential must be valid; + malformed, unknown, expired, or revoked Bearer credentials return HTTP 401 + and never fall back to anonymous access. servers: - url: / tags: @@ -34,7 +36,7 @@ paths: tags: [CLI Skills] summary: Search CLI-installable skills operationId: cliSearchSkills - description: No Authorization header uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. security: - {} - bearerAuth: [] @@ -65,6 +67,7 @@ paths: tags: [CLI Skills] summary: Resolve a skill version operationId: cliResolveSkill + description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. security: - {} - bearerAuth: [] @@ -95,6 +98,7 @@ paths: tags: [CLI Skills] summary: Download the latest installable skill version operationId: cliDownloadLatestSkill + description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. security: - {} - bearerAuth: [] @@ -119,6 +123,7 @@ paths: tags: [CLI Skills] summary: Download an exact installable skill version operationId: cliDownloadSkillVersion + description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. security: - {} - bearerAuth: [] @@ -218,13 +223,13 @@ components: schemas: Envelope: type: object - required: [code, msg, timestamp] + required: [code, msg, data, timestamp, requestId] properties: code: {type: integer, format: int32} msg: {type: string} data: {type: object, nullable: true} timestamp: {type: string, format: date-time} - requestId: {type: string, nullable: true} + requestId: {type: string, example: req-123} ErrorEnvelope: allOf: - $ref: '#/components/schemas/Envelope' diff --git a/docs/superpowers/plans/2026-07-28-revoked-token-validation.md b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md index d17b2d88..8deb0553 100644 --- a/docs/superpowers/plans/2026-07-28-revoked-token-validation.md +++ b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md @@ -13,7 +13,7 @@ ## File Map - Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java`: persisted valid/revoked/expired/unknown/empty/malformed credential matrix for each CLI endpoint. -- Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java`: real PRIVATE-skill read authorization through resolve, latest download, and versioned download. +- Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java`: real PRIVATE-skill search omission and read authorization through resolve, latest download, and versioned download. - Modify `docs/03-authentication-design.md`: current CLI route table and explicit anonymous/401/403 rules. - Create `docs/api/authentication.openapi.yaml`: OpenAPI 3.0 contract for whoami, search, resolve, latest download, and versioned download. - Do not modify `server/skillhub-auth/src/main/**` unless Task 5 records a failing unmodified-source assertion and a separate systematic-debugging plan amendment identifies the root cause. @@ -412,7 +412,34 @@ Run focused commands for the invalid, anonymous, and valid versioned-download me Expected: all commands PASS and the filter source has no diff. -- [ ] **Step 4: Run the complete persisted credential matrix** +- [ ] **Step 4: Add and prove the same-token valid-to-revoked replay** + +Create one token through `ApiTokenService`, retain its raw value, and use that +same value successfully against whoami, search, resolve, latest download, and +versioned download. Revoke the persisted token through +`ApiTokenService.revokeToken`, clear prior business-service invocations, then +replay the exact same raw value against all five endpoints. Each replay must +return 401 and the mocked business service must receive no post-revocation +interaction. + +For the three valid JSON responses and all five revoked error responses, assert +that the outer JSON object contains exactly `code`, `msg`, `data`, `timestamp`, +and `requestId`; successful downloads remain binary-stream exceptions. + +Apply the reversible invalid-token fail-open mutation and run: + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest#sameRawTokenIsRejectedByAllEndpointsAfterValidUseAndRevocation \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected RED: at least one public read replay returns 200 instead of 401. +Restore the filter, confirm its production diff is empty, and rerun the same +command. Expected GREEN: one test passes with all five valid calls and all five +revoked replays exercised. + +- [ ] **Step 5: Run the complete persisted credential matrix** Run: @@ -422,9 +449,11 @@ cd server && ./mvnw -pl skillhub-app -am \ -Dsurefire.failIfNoSpecifiedTests=false test ``` -Expected: PASS for all five endpoints and all absent, valid, revoked, expired, unknown, empty, and malformed credential cases. +Expected: PASS for all five endpoints and all absent, valid, revoked, expired, +unknown, empty, and malformed credential cases, plus the same-token lifecycle +replay. -- [ ] **Step 5: Commit the credential matrix** +- [ ] **Step 6: Commit the credential matrix** ```bash git add server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java @@ -596,21 +625,25 @@ Confirm `git diff -- server/skillhub-domain/src/main/java/com/iflytek/skillhub/d Expected: outsider resolve/latest/versioned each PASS with 403; owner resolve PASS with 200. -- [ ] **Step 5: Run the existing search-visibility boundary tests** +- [ ] **Step 5: Persist and verify the PRIVATE search-visibility boundary** -Run the search authorization checks independently as a supplementary 200-with-omission boundary: +Persist a `SkillSearchDocumentEntity` for the same PRIVATE fixture, call the CLI +search endpoint with the valid outsider token through the real +`CliSkillAppService` and `SearchQueryService`, and assert HTTP 200 with the +fixture slug omitted. Run it independently: ```bash cd server ./mvnw -pl skillhub-app -am \ - -Dtest='PostgresFullTextQueryServiceTest#anonymousSearchSqlShouldOnlyReadPublicActiveVisibleNonArchivedSkills' \ - -Dsurefire.failIfNoSpecifiedTests=false test -./mvnw -pl skillhub-app -am \ - -Dtest='SkillSearchAppServiceTest#search_shouldIncludeMemberNamespacesInVisibilityScope' \ + -Dtest='CliRestrictedReadAuthorizationIntegrationTest#outsiderSearchOmitsPersistedPrivateSkill' \ -Dsurefire.failIfNoSpecifiedTests=false test ``` -Expected: both commands PASS. Record search as a successful response whose result set omits inaccessible PRIVATE skills; it is not a substitute for the real resolve/download 403 assertions above. +Before the GREEN run, temporarily include PRIVATE documents in the search +adapter's visibility predicate and confirm the test fails because the fixture +slug appears. Restore the production predicate and confirm the command passes. +The search omission is not a substitute for the real resolve/download 403 +assertions above. - [ ] **Step 6: Commit the restricted-read tests** @@ -662,7 +695,7 @@ Use this content in section 10.3: | `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | | `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -公共读接口仅在完全缺少 `Authorization` 头时允许匿名访问。请求一旦携带 Bearer 凭证，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 +共享 API token 过滤器只识别 Bearer scheme。公共读接口在未提供可识别的 Bearer 凭证时允许匿名访问：这既包括完全缺少 `Authorization` 头，也包括 Basic 或其他非 Bearer scheme；`whoami` 因自身要求认证，在这些情况下仍返回 401。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401，不能降级为匿名身份。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 ``` - [ ] **Step 2: Create the complete OpenAPI 3.0 document** @@ -676,9 +709,11 @@ info: version: 1.0.0 description: >- Authentication contract for CLI identity and public skill reads. Public read - operations permit a request with no Authorization header, but any supplied - Bearer credential must be valid; malformed, unknown, expired, or revoked - credentials return HTTP 401 and never fall back to anonymous access. + operations treat a request with no recognized Bearer credential as anonymous, + including an absent Authorization header or an unsupported scheme such as + Basic. Once the Bearer scheme is used, the credential must be valid; + malformed, unknown, expired, or revoked Bearer credentials return HTTP 401 + and never fall back to anonymous access. servers: - url: / tags: @@ -706,7 +741,7 @@ paths: tags: [CLI Skills] summary: Search CLI-installable skills operationId: cliSearchSkills - description: No Authorization header uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. security: - {} - bearerAuth: [] @@ -890,13 +925,13 @@ components: schemas: Envelope: type: object - required: [code, msg, timestamp] + required: [code, msg, data, timestamp, requestId] properties: code: {type: integer, format: int32} msg: {type: string} data: {type: object, nullable: true} timestamp: {type: string, format: date-time} - requestId: {type: string, nullable: true} + requestId: {type: string, example: req-123} ErrorEnvelope: allOf: - $ref: '#/components/schemas/Envelope' diff --git a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md index 43fc316a..e0116d9a 100644 --- a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md +++ b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md @@ -4,10 +4,11 @@ Prove and preserve fail-closed API-token behavior across the CLI API using a real persisted token lifecycle. Invalid Bearer credentials must return HTTP -401 before endpoint business logic runs, while requests without an -`Authorization` header retain the existing anonymous-public-read contract and -valid credentials without sufficient authorization continue to return HTTP -403. +401 before endpoint business logic runs, while requests without a recognized +Bearer credential retain the existing anonymous-public-read contract. This +includes an absent `Authorization` header and unsupported schemes such as +Basic. Valid credentials without sufficient authorization continue to return +HTTP 403. ## Scope @@ -100,7 +101,9 @@ source-code conclusion is accepted. ## Architecture `ApiTokenAuthenticationFilter` remains the single Bearer-authentication entry -point. Controllers must not duplicate token parsing or lifecycle checks. +point. It ignores Basic and other non-Bearer schemes, which therefore reach +public read routes as anonymous requests; controllers must not duplicate token +parsing or lifecycle checks. The regression test will boot the Spring application with MockMvc, real `ApiTokenService`, real `ApiTokenRepository`, and real user persistence. CLI @@ -150,6 +153,7 @@ arguments and assertions for every credential state. | Credential state | `whoami` | Public `search` | Public `resolve` | Public latest download | Public versioned download | Meaning | |---|---:|---:|---:|---:|---:|---| | No `Authorization` header | 401 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Anonymous access is preserved only where already public | +| Basic or another non-Bearer scheme | 401 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Unsupported schemes are not treated as API-token attempts | | Valid active token | 200 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Principal and roles/scopes are projected | | Revoked token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | | Expired token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | From 4fdc7e3dc5c63b786d45ed1004b79c0450c5add1 Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:57:35 +0800 Subject: [PATCH 19/25] fix(publish): delete review tasks of any status when replacing a version (#601) * fix(publish): delete review tasks of any status when replacing a version Re-uploading a rejected version under the same version number returned HTTP 500. deleteReplaceableVersionArtifacts only removed a PENDING review task, but a rejected version owns a REJECTED one; that row kept a foreign key on the skill_version, so the subsequent delete hit a constraint violation that surfaced as a 500. Delete every review task attached to the version instead. Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> * test(publish): drop the spring-test dependency from the new test skillhub-domain has no spring-test on its test classpath, so ReflectionTestUtils does not resolve there. Use plain JDK reflection for setting the generated id and invoking the private method. Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> * fix(publish): constrain rejected version replacement Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(publish): verify replaced review is deleted Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(e2e): use generated API response types Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skill/service/SkillPublishService.java | 14 ++- .../service/SkillPublishServiceTest.java | 62 ++++++++++++-- web/e2e/helpers/test-data-builder.ts | 61 ++++++++++--- web/e2e/rejected-version-republish.spec.ts | 85 +++++++++++++++++++ 4 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 web/e2e/rejected-version-republish.spec.ts diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index c204306a..610c8203 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -62,6 +62,12 @@ public class SkillPublishService { private static final DateTimeFormatter AUTO_VERSION_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd.HHmmss").withZone(ZoneId.systemDefault()); + private static final Set REPLACEABLE_VERSION_STATUSES = Set.of( + SkillVersionStatus.DRAFT, + SkillVersionStatus.SCAN_FAILED, + SkillVersionStatus.UPLOADED, + SkillVersionStatus.REJECTED + ); private static final Logger log = LoggerFactory.getLogger(SkillPublishService.class); public record PublishResult( @@ -566,7 +572,7 @@ public class SkillPublishService { } private void deleteReplaceableVersionArtifacts(Skill skill, SkillVersion version, String namespaceSlug) { - if (version.getStatus() == SkillVersionStatus.PUBLISHED) { + if (!REPLACEABLE_VERSION_STATUSES.contains(version.getStatus())) { throw new DomainBadRequestException("error.skill.version.exists", version.getVersion()); } @@ -577,8 +583,10 @@ public class SkillPublishService { skillRepository.flush(); } - reviewTaskRepository.findBySkillVersionIdAndStatus(version.getId(), ReviewTaskStatus.PENDING) - .ifPresent(reviewTaskRepository::delete); + // Every review task referencing this version has to go, not just a PENDING one: + // a rejected version still owns a REJECTED task whose foreign key blocks the + // skill_version delete below, which surfaces to the caller as an HTTP 500. + reviewTaskRepository.deleteBySkillVersionIdIn(List.of(version.getId())); List files = skillFileRepository.findByVersionId(version.getId()); List storageKeys = new ArrayList<>(); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index 73f118fd..a75f971f 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -260,7 +260,7 @@ class SkillPublishServiceTest { } @Test - void testPublishFromEntries_ShouldReplaceDraftVersionWithSameVersion() throws Exception { + void testPublishFromEntries_ShouldReplaceRejectedVersionWithSameVersion() throws Exception { String namespaceSlug = "test-ns"; String publisherId = "user-100"; String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.0.0\n---\nBody"; @@ -275,9 +275,9 @@ class SkillPublishServiceTest { Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); setId(skill, 1L); - SkillVersion draftVersion = new SkillVersion(1L, "1.0.0", publisherId); - draftVersion.setStatus(SkillVersionStatus.DRAFT); - setId(draftVersion, 8L); + SkillVersion rejectedVersion = new SkillVersion(1L, "1.0.0", publisherId); + rejectedVersion.setStatus(SkillVersionStatus.REJECTED); + setId(rejectedVersion, 8L); SkillFile oldFile = new SkillFile(8L, "SKILL.md", (long) skillMdContent.length(), "text/markdown", "abc", "skills/1/8/SKILL.md"); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); @@ -288,7 +288,7 @@ class SkillPublishServiceTest { when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(List.of(skill)); when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("test-skill"), eq(publisherId))).thenReturn(Optional.of(skill)); when(skillVersionRepository.findBySkillIdAndStatus(1L, SkillVersionStatus.PENDING_REVIEW)).thenReturn(List.of()); - when(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).thenReturn(Optional.of(draftVersion)); + when(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).thenReturn(Optional.of(rejectedVersion)); when(skillFileRepository.findByVersionId(8L)).thenReturn(List.of(oldFile)); when(skillVersionRepository.save(any(SkillVersion.class))).thenAnswer(invocation -> { SkillVersion saved = invocation.getArgument(0); @@ -309,10 +309,60 @@ class SkillPublishServiceTest { assertEquals("1.0.0", result.version().getVersion()); assertEquals(SkillVersionStatus.PENDING_REVIEW, result.version().getStatus()); + verify(reviewTaskRepository).deleteBySkillVersionIdIn(List.of(8L)); verify(skillFileRepository).deleteByVersionId(8L); - verify(skillVersionRepository).delete(draftVersion); + verify(skillVersionRepository).delete(rejectedVersion); verify(skillVersionRepository).flush(); verify(objectStorageService).deleteObjects(List.of("skills/1/8/SKILL.md", "packages/1/8/bundle.zip")); + + ArgumentCaptor reviewTaskCaptor = ArgumentCaptor.forClass(ReviewTask.class); + verify(reviewTaskRepository).save(reviewTaskCaptor.capture()); + assertEquals(result.version().getId(), reviewTaskCaptor.getValue().getSkillVersionId()); + assertEquals(publisherId, reviewTaskCaptor.getValue().getSubmittedBy()); + } + + @Test + void testPublishFromEntries_ShouldRejectReplacementOfYankedVersion() throws Exception { + String namespaceSlug = "test-ns"; + String publisherId = "user-100"; + String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.0.0\n---\nBody"; + + PackageEntry skillMd = new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"); + List entries = List.of(skillMd); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + NamespaceMember member = mock(NamespaceMember.class); + SkillMetadata metadata = new SkillMetadata("test-skill", "Test", "1.0.0", "Body", Map.of()); + + Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); + setId(skill, 1L); + SkillVersion yankedVersion = new SkillVersion(1L, "1.0.0", publisherId); + yankedVersion.setStatus(SkillVersionStatus.YANKED); + setId(yankedVersion, 8L); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(any(), eq(publisherId))).thenReturn(Optional.of(member)); + when(skillPackageValidator.validate(entries)).thenReturn(ValidationResult.pass()); + when(skillMetadataParser.parse(skillMdContent)).thenReturn(metadata); + when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass()); + when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(List.of(skill)); + when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("test-skill"), eq(publisherId))).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).thenReturn(Optional.of(yankedVersion)); + + DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> + service.publishFromEntries( + namespaceSlug, + entries, + publisherId, + SkillVisibility.PUBLIC, + Set.of() + )); + + assertEquals("error.skill.version.exists", exception.messageCode()); + verify(reviewTaskRepository, never()).deleteBySkillVersionIdIn(anyList()); + verify(skillVersionRepository, never()).delete(any()); + verify(skillFileRepository, never()).deleteByVersionId(any()); } @Test diff --git a/web/e2e/helpers/test-data-builder.ts b/web/e2e/helpers/test-data-builder.ts index f255ea4a..71cd9ebd 100644 --- a/web/e2e/helpers/test-data-builder.ts +++ b/web/e2e/helpers/test-data-builder.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { execFileSync } from 'node:child_process' import path from 'node:path' import type { APIRequestContext, Page, TestInfo } from '@playwright/test' +import type { components } from '../../src/api/generated/schema' import { csrfHeaders } from './csrf' type CleanupTask = () => Promise @@ -32,14 +33,9 @@ export interface SeededReviewData { skill: SeededSkill } -interface ReviewTaskSummary { - id: number - namespace: string - skillSlug: string - status: string - submittedBy: string - version: string -} +type ReviewTaskResponse = components['schemas']['ReviewTaskResponse'] +type SkillVersionResponse = components['schemas']['SkillVersionResponse'] +type SkillVersionStatus = NonNullable interface NamespaceCandidate { userId: string @@ -463,19 +459,17 @@ export class E2eTestDataBuilder { async waitForPendingReview(namespaceSlug: string, skillSlug: string, version: string): Promise { for (let attempt = 0; attempt < 20; attempt += 1) { try { - const page = await parseEnvelope<{ - items: ReviewTaskSummary[] - }>( + const page = await parseEnvelope( await this.request.get('/api/web/reviews?status=PENDING&page=0&size=100&sortDirection=DESC'), ) - const matched = page.items.find((item) => + const matched = page.items?.find((item) => item.namespace === namespaceSlug && item.skillSlug === skillSlug && item.version === version && item.status === 'PENDING', ) - if (matched) { + if (matched?.id != null) { return matched.id } } catch { @@ -488,6 +482,38 @@ export class E2eTestDataBuilder { throw new Error(`Timed out waiting for pending review ${namespaceSlug}/${skillSlug}@${version}`) } + async waitForVersionStatus( + namespaceSlug: string, + skillSlug: string, + version: string, + expectedStatus: SkillVersionStatus, + ): Promise { + for (let attempt = 0; attempt < 60; attempt += 1) { + try { + const page = await parseEnvelope( + await this.request.get( + `/api/web/skills/${encodeURIComponent(namespaceSlug)}/${encodeURIComponent(skillSlug)}/versions?page=0&size=100`, + ), + ) + + const matched = page.items?.find((item) => + item.version === version && item.status === expectedStatus, + ) + if (matched?.id != null) { + return matched.id + } + } catch { + // Security scanning and version projection can complete asynchronously. + } + + await new Promise((resolve) => setTimeout(resolve, 1_000)) + } + + throw new Error( + `Timed out waiting for ${namespaceSlug}/${skillSlug}@${version} to reach ${expectedStatus}`, + ) + } + async approveReview(reviewTaskId: number, comment = 'Approved by Playwright E2E'): Promise { let lastError: unknown for (let attempt = 0; attempt < 60; attempt += 1) { @@ -512,6 +538,15 @@ export class E2eTestDataBuilder { throw lastError instanceof Error ? lastError : new Error('approveReview timed out') } + async rejectReview(reviewTaskId: number, comment = 'Rejected by Playwright E2E'): Promise { + await parseEnvelope( + await this.request.post(`/api/web/reviews/${reviewTaskId}/reject`, { + data: { comment }, + headers: await csrfHeaders(this.page), + }), + ) + } + async searchNamespaceMemberCandidates(slug: string, search: string): Promise { const query = new URLSearchParams({ search }) return parseEnvelope( diff --git a/web/e2e/rejected-version-republish.spec.ts b/web/e2e/rejected-version-republish.spec.ts new file mode 100644 index 00000000..cbbcada4 --- /dev/null +++ b/web/e2e/rejected-version-republish.spec.ts @@ -0,0 +1,85 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' +import { loginWithCredentials, registerSession } from './helpers/session' +import { E2eTestDataBuilder } from './helpers/test-data-builder' + +function getOptionalEnv(name: string): string | undefined { + const value = process.env[name]?.trim() + return value ? value : undefined +} + +function adminCredentials() { + return { + username: getOptionalEnv('E2E_ADMIN_USERNAME') ?? getOptionalEnv('BOOTSTRAP_ADMIN_USERNAME') ?? 'admin', + password: getOptionalEnv('E2E_ADMIN_PASSWORD') ?? getOptionalEnv('BOOTSTRAP_ADMIN_PASSWORD') ?? 'ChangeMe!2026', + } +} + +test.describe('Rejected version replacement (Real API)', () => { + test.describe.configure({ timeout: 150_000 }) + + test.beforeEach(async ({ page }, testInfo) => { + await setEnglishLocale(page) + await registerSession(page, testInfo) + }) + + test('re-publishes the same version after rejection', async ({ page, browser }, testInfo) => { + const publisherBuilder = new E2eTestDataBuilder(page, testInfo) + await publisherBuilder.init() + + const adminContext = await browser.newContext() + const adminPage = await adminContext.newPage() + const adminBuilder = new E2eTestDataBuilder(adminPage, testInfo) + await loginWithCredentials(adminPage, adminCredentials(), testInfo) + await adminBuilder.init() + + try { + const namespace = await publisherBuilder.ensureWritableNamespace() + const skillName = `replace-rejected-${Date.now().toString(36)}` + const firstPublish = await publisherBuilder.publishSkill(namespace.slug, { + name: skillName, + version: '1.0.0', + }) + const rejectedReviewId = await adminBuilder.waitForPendingReview( + namespace.slug, + firstPublish.slug, + firstPublish.version, + ) + await publisherBuilder.waitForVersionStatus( + namespace.slug, + firstPublish.slug, + firstPublish.version, + 'PENDING_REVIEW', + ) + await adminBuilder.rejectReview(rejectedReviewId) + + const replacement = await publisherBuilder.publishSkill(namespace.slug, { + name: skillName, + description: 'Replacement after review rejection', + version: '1.0.0', + }) + const replacementReviewId = await adminBuilder.waitForPendingReview( + namespace.slug, + replacement.slug, + replacement.version, + ) + await publisherBuilder.waitForVersionStatus( + namespace.slug, + replacement.slug, + replacement.version, + 'PENDING_REVIEW', + ) + + expect(replacement.skillId).toBe(firstPublish.skillId) + expect(replacement.version).toBe(firstPublish.version) + expect(replacementReviewId).not.toBe(rejectedReviewId) + + const replacedReviewResponse = await adminPage.request.get(`/api/web/reviews/${rejectedReviewId}`) + expect(replacedReviewResponse.status()).toBe(404) + } finally { + await adminBuilder.cleanup() + await adminContext.close() + await publisherBuilder.cleanup() + } + }) +}) From 5012b31af2d042ca18df2e3ec128f706ecd43e26 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 28 Jul 2026 15:22:11 +0800 Subject: [PATCH 20/25] test(auth): cover CLI session fallback (#605) Signed-off-by: dongmucat <1127093059@qq.com> --- docs/03-authentication-design.md | 14 +- docs/api/authentication.openapi.yaml | 38 ++-- .../2026-07-28-revoked-token-validation.md | 166 +++++++++++++--- ...6-07-28-revoked-token-validation-design.md | 64 +++--- ...ictedReadAuthorizationIntegrationTest.java | 71 +++++-- ...TokenLifecycleSecurityIntegrationTest.java | 183 ++++++++++++++++-- 6 files changed, 432 insertions(+), 104 deletions(-) diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index e28f10af..7cac3b82 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -377,7 +377,7 @@ API Token 仍保留，但定位从“CLI 唯一认证方式”调整为“平台 - 用途：自动化脚本、兼容层调用、手工 Token 管理、后续系统集成 - 存储：只存 SHA-256 哈希，明文只展示一次 - 校验：从 `Authorization: Bearer ` 提取 → 哈希比对 → 加载关联用户 → 检查用户状态 -- 失败闭合：共享认证过滤器只识别 Bearer scheme；公共读接口在未提供可识别的 Bearer 凭证时按匿名访问处理（包括缺少 `Authorization` 头，以及 Basic 或其他非 Bearer scheme）。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 401，不能回退为匿名访问 +- 失败闭合与身份优先级：共享认证过滤器只识别 Bearer scheme。有效 Bearer 覆盖已加载的 Web Session 身份；Bearer 为空、格式错误、未知、过期、已吊销、用户缺失或用户禁用时立即返回 401，即使存在有效 Session 也不得回退。缺少 `Authorization` 头或使用 Basic/其他非 Bearer scheme 时保留有效 Session；若无 Session，公共读接口按匿名访问，`whoami` 返回 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 的绑定。 @@ -623,13 +623,13 @@ window.location.href = '/oauth2/authorization/github' | 接口 | 凭证规则 | 授权与错误语义 | |------|---------|---------------| -| `GET /api/cli/v1/auth/whoami` | 必须提供有效 Bearer Token | 缺失、未知、过期、撤销或 malformed token 返回 401 | -| `GET /api/cli/v1/skills/search` | 可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；坏凭证返回 401，不得降级匿名 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | 可匿名读取公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/auth/whoami` | 有效 Web Session 或有效 Bearer Token | 无有效身份返回 401；坏 Bearer 即使存在 Session 也返回 401 | +| `GET /api/cli/v1/skills/search` | Session 可用；无 Session 时可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；有效 Bearer 覆盖 Session；坏 Bearer 返回 401，不得降级 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | Session 可用；无 Session 时可匿名读取公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | Session 可用；无 Session 时可匿名下载公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | Session 可用；无 Session 时可匿名下载公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | -共享 API token 过滤器只识别 Bearer scheme。公共读接口在未提供可识别的 Bearer 凭证时允许匿名访问：这既包括完全缺少 `Authorization` 头，也包括 Basic 或其他非 Bearer scheme；`whoami` 因自身要求认证，在这些情况下仍返回 401。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401，不能降级为匿名身份。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 +Spring Security 先加载 Web Session 身份，共享 API token 过滤器随后只处理 Bearer scheme。有效 Bearer 会覆盖 Session，确保请求使用 token 的用户、角色与 scope；Bearer 为空、格式错误、未知、过期、已撤销、用户缺失或用户禁用时，过滤器清除当前身份并立即返回 401，不能回退到 Session 或匿名身份。完全缺少 `Authorization` 头或使用 Basic/其他非 Bearer scheme 时，过滤器不改变已有 Session；如果 Session 也不存在，公共读接口按匿名身份执行，而 `whoami` 返回 401。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。`whoami.email` 字段始终存在，但没有可用邮箱时值为 `null`。 ### 10.4 Admin API diff --git a/docs/api/authentication.openapi.yaml b/docs/api/authentication.openapi.yaml index 4fafac22..e50788ff 100644 --- a/docs/api/authentication.openapi.yaml +++ b/docs/api/authentication.openapi.yaml @@ -3,12 +3,13 @@ info: title: SkillHub CLI Authentication API version: 1.0.0 description: >- - Authentication contract for CLI identity and public skill reads. Public read - operations treat a request with no recognized Bearer credential as anonymous, - including an absent Authorization header or an unsupported scheme such as - Basic. Once the Bearer scheme is used, the credential must be valid; - malformed, unknown, expired, or revoked Bearer credentials return HTTP 401 - and never fall back to anonymous access. + Authentication contract for CLI identity and public skill reads. A valid + Bearer credential overrides a Web Session identity. Once the Bearer scheme + is used, the credential must be valid: empty, malformed, unknown, expired, + or revoked Bearer credentials return HTTP 401 and never fall back to the + Session or anonymous access. An absent Authorization header or an + unsupported scheme such as Basic preserves a valid Web Session. Without a + Session, public reads use anonymous visibility and whoami returns HTTP 401. servers: - url: / tags: @@ -20,8 +21,10 @@ paths: tags: [CLI Authentication] summary: Return the current CLI identity operationId: cliWhoAmI + description: Requires a valid Bearer credential or Web Session. Bearer takes priority over Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session, but returns 401 when no Session exists. security: - bearerAuth: [] + - sessionAuth: [] responses: '200': description: Authenticated CLI identity @@ -36,9 +39,10 @@ paths: tags: [CLI Skills] summary: Search CLI-installable skills operationId: cliSearchSkills - description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: Valid Bearer overrides Web Session. Invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session; without Session, the request uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - name: q @@ -67,9 +71,10 @@ paths: tags: [CLI Skills] summary: Resolve a skill version operationId: cliResolveSkill - description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: Valid Bearer overrides Web Session. Invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session; without Session, the request uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -98,9 +103,10 @@ paths: tags: [CLI Skills] summary: Download the latest installable skill version operationId: cliDownloadLatestSkill - description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: Valid Bearer overrides Web Session. Invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session; without Session, the request uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -123,9 +129,10 @@ paths: tags: [CLI Skills] summary: Download an exact installable skill version operationId: cliDownloadSkillVersion - description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: Valid Bearer overrides Web Session. Invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session; without Session, the request uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -150,7 +157,12 @@ components: type: http scheme: bearer bearerFormat: SkillHub API token - description: API token issued by SkillHub. Invalid lifecycle states all return the same 401 response. + description: API token issued by SkillHub. A valid token overrides Web Session; invalid lifecycle states all return the same 401 response without Session fallback. + sessionAuth: + type: apiKey + in: cookie + name: SESSION + description: Spring Session browser identity. It is preserved when Authorization is absent or uses a non-Bearer scheme, and is overridden by a valid Bearer token. parameters: Namespace: name: namespace @@ -194,7 +206,7 @@ components: application/json: schema: {$ref: '#/components/schemas/ErrorEnvelope'} Unauthorized: - description: Bearer credential is missing where required, malformed, unknown, expired, revoked, or belongs to an unavailable user. + description: No valid supported identity is present where required, or the Bearer credential is empty, malformed, unknown, expired, revoked, or belongs to an unavailable user. Invalid Bearer never falls back to Web Session. content: application/json: schema: {$ref: '#/components/schemas/ErrorEnvelope'} @@ -249,7 +261,7 @@ components: properties: handle: {type: string, example: user-123} displayName: {type: string, example: CLI User} - email: {type: string, format: email, example: cli@example.com} + email: {type: string, format: email, nullable: true, example: cli@example.com, description: Email address when available; the required field is null when the account has no email.} CliSearchEnvelope: allOf: - $ref: '#/components/schemas/Envelope' diff --git a/docs/superpowers/plans/2026-07-28-revoked-token-validation.md b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md index 8deb0553..2888470d 100644 --- a/docs/superpowers/plans/2026-07-28-revoked-token-validation.md +++ b/docs/superpowers/plans/2026-07-28-revoked-token-validation.md @@ -2,9 +2,9 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Lock the CLI API's fail-closed Bearer behavior with persisted token lifecycle tests, prove 401/403 semantics on every affected read endpoint, publish the authentication OpenAPI contract, and reconcile source behavior with the actual runtime artifact. +**Goal:** Lock the CLI API's fail-closed Bearer behavior and Web Session fallback with persisted lifecycle and mixed-credential tests, prove 401/403 semantics on every affected read endpoint, publish the authentication OpenAPI contract, and reconcile source behavior with the actual runtime artifact. -**Architecture:** Keep `ApiTokenAuthenticationFilter` as the sole Bearer authentication entry point. Use one Spring Boot/MockMvc class with real token and user persistence plus deterministic controller-service stubs for the credential-state matrix, and a second Spring Boot/MockMvc class with real query/download authorization and a persisted PRIVATE skill for resource-level 403 checks. Production authentication code remains unchanged unless the unmodified-source matrix reproduces a failure; any such failure stops this plan for systematic root-cause analysis before a minimal fix is planned. +**Architecture:** Keep `ApiTokenAuthenticationFilter` as the sole Bearer authentication entry point while preserving Spring Security's existing Web Session identity. Valid Bearer replaces Session; invalid Bearer fails closed without Session fallback; absent or non-Bearer Authorization preserves Session and otherwise leaves public reads anonymous. Use one Spring Boot/MockMvc class with real token and user persistence plus deterministic controller-service stubs for the credential-state matrix, and a second Spring Boot/MockMvc class with real query/download authorization plus persisted PRIVATE and matching PUBLIC skills for authorization checks. Production authentication code remains unchanged unless the unmodified-source matrix reproduces a failure; any such failure stops this plan for systematic root-cause analysis before a minimal fix is planned. **Tech Stack:** Java 21, Spring Boot 3.2, Spring Security, Spring Data JPA/H2, MockMvc, JUnit 5 parameterized tests, Mockito, OpenAPI 3.0 YAML, Docker/OCI image inspection. @@ -14,7 +14,7 @@ - Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java`: persisted valid/revoked/expired/unknown/empty/malformed credential matrix for each CLI endpoint. - Create `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java`: real PRIVATE-skill search omission and read authorization through resolve, latest download, and versioned download. -- Modify `docs/03-authentication-design.md`: current CLI route table and explicit anonymous/401/403 rules. +- Modify `docs/03-authentication-design.md`: current CLI route table, Web Session/Bearer priority, and explicit anonymous/401/403 rules. - Create `docs/api/authentication.openapi.yaml`: OpenAPI 3.0 contract for whoami, search, resolve, latest download, and versioned download. - Do not modify `server/skillhub-auth/src/main/**` unless Task 5 records a failing unmodified-source assertion and a separate systematic-debugging plan amendment identifies the root cause. @@ -689,13 +689,13 @@ Use this content in section 10.3: | 接口 | 凭证规则 | 授权与错误语义 | |------|---------|---------------| -| `GET /api/cli/v1/auth/whoami` | 必须提供有效 Bearer Token | 缺失、未知、过期、撤销或 malformed token 返回 401 | -| `GET /api/cli/v1/skills/search` | 可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；坏凭证返回 401，不得降级匿名 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | 可匿名读取公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | -| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | 可匿名下载公开资源；提供 Bearer 时必须有效 | 坏凭证返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/auth/whoami` | 有效 Web Session 或有效 Bearer Token | 无有效身份返回 401；坏 Bearer 即使存在 Session 也返回 401 | +| `GET /api/cli/v1/skills/search` | Session 可用；无 Session 时可匿名；提供 Bearer 时必须有效 | 匿名仅返回公开可安装 skill；有效 Bearer 覆盖 Session；坏 Bearer 返回 401，不得降级 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/resolve` | Session 可用；无 Session 时可匿名读取公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/download` | Session 可用；无 Session 时可匿名下载公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | +| `GET /api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download` | Session 可用；无 Session 时可匿名下载公开资源；提供 Bearer 时必须有效 | 有效 Bearer 覆盖 Session；坏 Bearer 返回 401；有效身份无资源权限返回 403 | -共享 API token 过滤器只识别 Bearer scheme。公共读接口在未提供可识别的 Bearer 凭证时允许匿名访问：这既包括完全缺少 `Authorization` 头，也包括 Basic 或其他非 Bearer scheme；`whoami` 因自身要求认证，在这些情况下仍返回 401。请求一旦使用 Bearer scheme，空值、格式错误、未知、过期、已撤销、用户缺失或用户禁用均由共享认证过滤器返回 401，不能降级为匿名身份。身份已验证但 token scope 或资源可见性不足时返回 403；服务端不向客户端区分 token 不存在、过期或已撤销。 +Spring Security 先加载 Web Session 身份，共享 API token 过滤器随后只处理 Bearer scheme。有效 Bearer 覆盖 Session；坏 Bearer 清除当前身份并立即返回 401，不回退 Session 或匿名。没有 Authorization 或使用 Basic/其他非 Bearer scheme 时保留 Session；如果 Session 也不存在，公共读匿名而 `whoami` 返回 401。身份已验证但 token scope 或资源权限不足时返回 403。`whoami.email` 字段始终存在，没有邮箱时为 `null`。 ``` - [ ] **Step 2: Create the complete OpenAPI 3.0 document** @@ -708,12 +708,11 @@ info: title: SkillHub CLI Authentication API version: 1.0.0 description: >- - Authentication contract for CLI identity and public skill reads. Public read - operations treat a request with no recognized Bearer credential as anonymous, - including an absent Authorization header or an unsupported scheme such as - Basic. Once the Bearer scheme is used, the credential must be valid; - malformed, unknown, expired, or revoked Bearer credentials return HTTP 401 - and never fall back to anonymous access. + Authentication contract for CLI identity and public skill reads. Valid + Bearer overrides Web Session. Invalid Bearer returns HTTP 401 without + Session fallback. An absent Authorization header or unsupported scheme such + as Basic preserves Session; without Session, public reads are anonymous and + whoami returns HTTP 401. servers: - url: / tags: @@ -725,8 +724,10 @@ paths: tags: [CLI Authentication] summary: Return the current CLI identity operationId: cliWhoAmI + description: Valid Bearer overrides Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer Authorization header preserves Session. security: - bearerAuth: [] + - sessionAuth: [] responses: '200': description: Authenticated CLI identity @@ -741,9 +742,10 @@ paths: tags: [CLI Skills] summary: Search CLI-installable skills operationId: cliSearchSkills - description: An absent Bearer credential, including an unsupported non-Bearer Authorization scheme, uses anonymous public visibility. A supplied invalid Bearer credential returns 401. + description: Valid Bearer overrides Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer header preserves Session, otherwise this route uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - name: q @@ -772,8 +774,10 @@ paths: tags: [CLI Skills] summary: Resolve a skill version operationId: cliResolveSkill + description: Valid Bearer overrides Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer header preserves Session, otherwise this route uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -802,8 +806,10 @@ paths: tags: [CLI Skills] summary: Download the latest installable skill version operationId: cliDownloadLatestSkill + description: Valid Bearer overrides Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer header preserves Session, otherwise this route uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -826,8 +832,10 @@ paths: tags: [CLI Skills] summary: Download an exact installable skill version operationId: cliDownloadSkillVersion + description: Valid Bearer overrides Session; invalid Bearer returns 401 without Session fallback. An absent or non-Bearer header preserves Session, otherwise this route uses anonymous public visibility. security: - {} + - sessionAuth: [] - bearerAuth: [] parameters: - $ref: '#/components/parameters/Namespace' @@ -852,7 +860,12 @@ components: type: http scheme: bearer bearerFormat: SkillHub API token - description: API token issued by SkillHub. Invalid lifecycle states all return the same 401 response. + description: API token issued by SkillHub. Valid Bearer overrides Session; invalid lifecycle states return the same 401 response without Session fallback. + sessionAuth: + type: apiKey + in: cookie + name: SESSION + description: Spring Session browser identity, preserved when Authorization is absent or uses a non-Bearer scheme. parameters: Namespace: name: namespace @@ -896,7 +909,7 @@ components: application/json: schema: {$ref: '#/components/schemas/ErrorEnvelope'} Unauthorized: - description: Bearer credential is missing where required, malformed, unknown, expired, revoked, or belongs to an unavailable user. + description: No valid supported identity is present where required, or the Bearer credential is invalid. Invalid Bearer never falls back to Web Session. content: application/json: schema: {$ref: '#/components/schemas/ErrorEnvelope'} @@ -951,7 +964,7 @@ components: properties: handle: {type: string, example: user-123} displayName: {type: string, example: CLI User} - email: {type: string, format: email, example: cli@example.com} + email: {type: string, format: email, nullable: true, example: cli@example.com} CliSearchEnvelope: allOf: - $ref: '#/components/schemas/Envelope' @@ -1056,7 +1069,111 @@ Expected after revocation: 401 on every endpoint. If behavior differs, preserve If no affected runtime URL, host/replica access, or authorization to create/revoke a test token is available, explicitly escalate to the human owner in the active issue. Name the missing authority and request the exact evidence still required: deployed version, immutable server digest or build SHA, all replica identities, and same-token valid-to-revoked replay. State that repository tests do not close the field contradiction and therefore cannot justify closing the defect. -### Task 8: Quality gates and implementation review handoff +### Task 8: Preserve Web Session fallback and harden the reviewed contracts + +**Files:** +- Modify: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java` +- Modify: `server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java` +- Modify: `docs/03-authentication-design.md` +- Modify: `docs/api/authentication.openapi.yaml` +- Modify: `docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md` + +- [ ] **Step 1: Add the five-endpoint Web Session and mixed-credential matrix** + +Add independent arguments for `whoami`, search, resolve, latest download, and +versioned download. For each endpoint exercise Session-only, Session + Basic, +Basic-only, and Session + valid Bearer. Persist distinct Session and token +users, assert Session identity is retained when Bearer is absent or the scheme +is Basic, assert public reads are anonymous for Basic-only, and assert valid +Bearer identity replaces Session identity. Existing revoked, expired, unknown, +empty, and malformed Bearer cases must attach a real mock HTTP Session and +continue to return the fixed five-field 401 envelope before controller service +logic runs. + +Run a reversible filter mutation that prevents valid Bearer replacement of an +existing Session principal, then run: + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest#sessionAndAuthorizationSchemeMatrix \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected RED: Session + valid Bearer exposes the Session user instead of the +token user. Restore production source immediately and rerun the same command. +Expected GREEN: all 20 endpoint/credential arguments pass without a production +source diff. + +- [ ] **Step 2: Lock the nullable whoami email contract** + +Persist an active user whose email is `null`, issue its token through +`ApiTokenService`, call `GET /api/cli/v1/auth/whoami`, and assert the `email` +key is present with a JSON null value inside the standard five-field envelope. + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliTokenLifecycleSecurityIntegrationTest#whoamiReturnsNullEmailForPersistedUserWithoutEmail \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: PASS against existing production behavior; this is a response-shape +characterization test. Update `CliWhoAmI.email` in OpenAPI to remain required +while becoming `nullable: true`. + +- [ ] **Step 3: Make PRIVATE search omission a positive and negative proof** + +Use a unique numeric `skillSlug` as `q`, persist an installable PUBLIC skill +whose search document contains the same keyword, and keep the existing +installable PRIVATE skill. Assert the PUBLIC slug is returned and the PRIVATE +slug is omitted for the outsider token. + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliRestrictedReadAuthorizationIntegrationTest#outsiderSearchReturnsMatchingPublicSkillAndOmitsPrivateSkill \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected RED before the PUBLIC fixture is persisted: the expected PUBLIC slug +is absent. Expected GREEN after the fixture is added: the same non-empty result +contains PUBLIC and omits PRIVATE. + +- [ ] **Step 4: Assert the fixed five-field 403 envelope on every restricted read** + +Replace status/code-only assertions for restricted resolve, latest download, +and versioned download with a shared assertion for exactly `code`, `msg`, +`data`, `timestamp`, and `requestId`; require `code=403`, `data=null`, and +string timestamps/request IDs. Keep the three routes as separate test methods. + +```bash +cd server && ./mvnw -pl skillhub-app -am \ + -Dtest=CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotResolvePrivateSkill,CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotDownloadLatestPrivateSkill,CliRestrictedReadAuthorizationIntegrationTest#outsiderCannotDownloadVersionedPrivateSkill \ + -Dsurefire.failIfNoSpecifiedTests=false test +``` + +Expected: all three pass through the real access-denied path. + +- [ ] **Step 5: Align authentication design and OpenAPI priority rules** + +Document these exact rules: valid Bearer overrides Web Session; any Bearer +attempt that is empty, malformed, unknown, expired, revoked, or tied to an +unavailable user returns 401 without Session fallback; no Authorization header +or a non-Bearer scheme preserves a valid Session; without a Session, public +reads use anonymous visibility and `whoami` returns 401. Add cookie +`sessionAuth` to OpenAPI and list it as an alternative on all five operations. +OpenAPI descriptions must state the precedence because security alternatives +cannot encode it alone. + +- [ ] **Step 6: Confirm the review correction did not change production auth** + +```bash +git diff --name-only origin/main...HEAD +git diff --exit-code origin/main...HEAD -- server/skillhub-auth/src/main server/skillhub-app/src/main +``` + +Expected: only tests and documentation changed; the production-code diff +command exits 0. + +### Task 9: Quality gates and implementation review handoff **Files:** - Verify all changed files; do not create a PR in this stage. @@ -1111,6 +1228,11 @@ Expected: only the approved spec/plan, two test classes, authentication design, Provide the branch, focused commands, complete matrix result, 403 fixture result, docs path, runtime identity/replay evidence or explicit external blocker, and full gate output to the project tester. After tester passes, request structured reviewer/security review. Address any findings on the same branch and rerun affected gates. -- [ ] **Step 7: Report completion without creating a PR** +- [ ] **Step 7: Update the existing single PR and report completion** -Post the implementation result to the active issue thread. Include commit SHAs, endpoint-by-state matrix, RED mutation evidence, GREEN results, quality gates, OpenAPI path, production-code decision, and runtime identity/replay status. Do not create a PR, do not change issue status, and do not merge `main` during this stage. +Commit and push to the existing `fix/auth-revoked-token-validation` branch so +PR #609 updates in place. Post the implementation result to the active issue +thread. Include commit SHAs, endpoint-by-state matrix, RED mutation evidence, +GREEN results, quality gates, OpenAPI path, production-code decision, and +runtime identity/replay status. Do not create a second PR, do not change issue +status, and do not merge `main` during this stage. diff --git a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md index e0116d9a..1b6d6488 100644 --- a/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md +++ b/docs/superpowers/specs/2026-07-28-revoked-token-validation-design.md @@ -4,11 +4,12 @@ Prove and preserve fail-closed API-token behavior across the CLI API using a real persisted token lifecycle. Invalid Bearer credentials must return HTTP -401 before endpoint business logic runs, while requests without a recognized -Bearer credential retain the existing anonymous-public-read contract. This -includes an absent `Authorization` header and unsupported schemes such as -Basic. Valid credentials without sufficient authorization continue to return -HTTP 403. +401 before endpoint business logic runs, including when a valid Web Session is +also present. A valid Bearer credential overrides the Session identity. When +Bearer is absent or the Authorization scheme is unsupported, the existing Web +Session identity is preserved; without a valid Session, public reads remain +anonymous and `whoami` returns 401. Valid credentials without sufficient +authorization continue to return HTTP 403. ## Scope @@ -23,8 +24,10 @@ This change covers the following CLI routes: It also covers the authenticated-versus-forbidden boundary on the affected restricted read routes. An existing scope-protected CLI route may provide supplementary scope-filter evidence only. This change does not add endpoints, -change response fields, change token storage, add a database migration, or -change anonymous resource visibility rules. +change runtime response fields, change token storage, add a database migration, +or change anonymous resource visibility rules. The OpenAPI correction marks +the already-nullable `whoami.email` value accurately without changing its JSON +field presence. ## Current-State Finding @@ -101,9 +104,13 @@ source-code conclusion is accepted. ## Architecture `ApiTokenAuthenticationFilter` remains the single Bearer-authentication entry -point. It ignores Basic and other non-Bearer schemes, which therefore reach -public read routes as anonymous requests; controllers must not duplicate token -parsing or lifecycle checks. +point. Spring Security loads an existing Web Session identity before the token +filter runs. A valid Bearer token replaces that identity; an invalid, empty, or +malformed Bearer attempt clears it and returns 401. The filter ignores Basic +and other non-Bearer schemes, preserving the loaded Session identity. If no +Session exists, those schemes reach public reads anonymously and `whoami` +returns 401. Controllers must not duplicate token parsing, Session resolution, +or lifecycle checks. The regression test will boot the Spring application with MockMvc, real `ApiTokenService`, real `ApiTokenRepository`, and real user persistence. CLI @@ -154,12 +161,15 @@ arguments and assertions for every credential state. |---|---:|---:|---:|---:|---:|---| | No `Authorization` header | 401 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Anonymous access is preserved only where already public | | Basic or another non-Bearer scheme | 401 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Unsupported schemes are not treated as API-token attempts | +| Valid Web Session, no `Authorization` header | 200 as Session user | 200 as Session user | 200 as Session user | Existing 200/302 as Session user | Existing 200/302 as Session user | Existing browser identity is preserved | +| Valid Web Session + Basic | 200 as Session user | 200 as Session user | 200 as Session user | Existing 200/302 as Session user | Existing 200/302 as Session user | Non-Bearer schemes do not erase Session identity | | Valid active token | 200 | 200 | 200 | Existing 200/302 success | Existing 200/302 success | Principal and roles/scopes are projected | -| Revoked token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Expired token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Unknown token | 401 | 401 | 401 | 401 | 401 | Credential cannot degrade to anonymous | -| Empty Bearer credential | 401 | 401 | 401 | 401 | 401 | Empty authentication attempt is rejected before business logic | -| Malformed Bearer credential | 401 | 401 | 401 | 401 | 401 | Malformed authentication attempt is rejected before business logic | +| Valid Web Session + valid active token | 200 as token user | 200 as token user | 200 as token user | Existing 200/302 as token user | Existing 200/302 as token user | Bearer identity overrides Session identity | +| Valid Web Session + revoked token | 401 | 401 | 401 | 401 | 401 | Credential cannot fall back to Session or anonymous | +| Valid Web Session + expired token | 401 | 401 | 401 | 401 | 401 | Credential cannot fall back to Session or anonymous | +| Valid Web Session + unknown token | 401 | 401 | 401 | 401 | 401 | Credential cannot fall back to Session or anonymous | +| Valid Web Session + empty Bearer credential | 401 | 401 | 401 | 401 | 401 | Empty authentication attempt is rejected before business logic | +| Valid Web Session + malformed Bearer credential | 401 | 401 | 401 | 401 | 401 | Malformed authentication attempt is rejected before business logic | The authorization row uses a persisted PRIVATE or NAMESPACE_ONLY fixture and the real read-authorization path: @@ -190,13 +200,14 @@ evidence for the API-token scope filter only. Two documentation updates are required: 1. Update `docs/03-authentication-design.md` so the CLI API section uses the - current `/api/cli/v1/...` routes and explicitly states the 401/403 and - anonymous-access boundary. + current `/api/cli/v1/...` routes and explicitly states Bearer-over-Session + priority, Session fallback, and the anonymous/401/403 boundary. 2. Add `docs/api/authentication.openapi.yaml` using OpenAPI 3.0. The document - must define Bearer authentication, all affected paths, query/path - parameters, success schemas, the common response envelope, HTTP 401 and 403 - responses, examples, and the rule that absent credentials are allowed only - on existing public-read routes. + must define Bearer and Web Session authentication, all affected paths, + query/path parameters, success schemas, the common response envelope, HTTP + 401 and 403 responses, examples, credential priority, and the rule that + requests without either identity are allowed only on existing public-read + routes. `CliWhoAmI.email` remains required but is nullable. No controller signature or response schema changes are planned. Therefore the generated `web/src/api/generated/schema.d.ts` should remain unchanged; if a @@ -217,7 +228,12 @@ steps rather than collapsing them into one generic download case: the real read-authorization path to prove 403 for restricted `resolve`, latest download, and versioned download and success for an authorized user. 6. Update the authentication design and OpenAPI contract. -7. Identify the published/running image and replay the valid-to-revoked token +7. Exercise Session-only, Session + Basic, Basic-only, and Session + valid or + invalid Bearer independently on all five endpoints; latest and versioned + download remain separate cases. +8. Prove PRIVATE search omission with a non-empty same-keyword PUBLIC result + and assert the fixed five-field 403 envelope on each restricted read. +9. Identify the published/running image and replay the valid-to-revoked token lifecycle against that exact digest, or record the external access blocker without treating the field contradiction as resolved. @@ -247,8 +263,8 @@ Verification proceeds in this order: 10. Replay the same valid-to-revoked token lifecycle against the identified runtime and record endpoint-level status, request ID, and replica evidence, keeping latest and versioned download results separate. -11. Perform structured security and code review before opening the single final - pull request. +11. Perform structured security and code review before updating the existing + single final pull request. ## Delivery Constraints diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java index 5a29cffa..8f4498ea 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliRestrictedReadAuthorizationIntegrationTest.java @@ -28,6 +28,7 @@ import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilde import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; 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; @@ -47,6 +48,7 @@ class CliRestrictedReadAuthorizationIntegrationTest { private String namespaceSlug; private String skillSlug; + private String publicSkillSlug; private String version; private String ownerToken; private String outsiderToken; @@ -57,7 +59,8 @@ class CliRestrictedReadAuthorizationIntegrationTest { String ownerId = "private-owner-" + suffix; String outsiderId = "private-outsider-" + suffix; namespaceSlug = "private-ns-" + suffix; - skillSlug = "private-skill-" + suffix; + skillSlug = Long.toUnsignedString(UUID.randomUUID().getMostSignificantBits()); + publicSkillSlug = "public-skill-" + suffix; version = "1.0.0"; userAccountRepository.save(new UserAccount( @@ -94,45 +97,77 @@ class CliRestrictedReadAuthorizationIntegrationTest { "", SkillVisibility.PRIVATE.name(), skill.getStatus().name())); + + Skill publicSkill = skillRepository.save(new Skill( + namespace.getId(), publicSkillSlug, ownerId, SkillVisibility.PUBLIC)); + SkillVersion publicPublished = new SkillVersion(publicSkill.getId(), version, ownerId); + publicPublished.setStatus(SkillVersionStatus.PUBLISHED); + publicPublished.setPublishedAt(Instant.parse("2026-07-28T00:00:00Z")); + publicPublished.setDownloadReady(true); + publicPublished = skillVersionRepository.save(publicPublished); + publicSkill.setLatestVersionId(publicPublished.getId()); + skillRepository.save(publicSkill); + skillRepository.flush(); + skillVersionRepository.flush(); + skillSearchDocumentRepository.saveAndFlush(new SkillSearchDocumentEntity( + publicSkill.getId(), + namespace.getId(), + namespaceSlug, + ownerId, + skillSlug, + "Public match for " + publicSkillSlug, + "public", + skillSlug, + "", + SkillVisibility.PUBLIC.name(), + publicSkill.getStatus().name())); } @Test - void outsiderSearchOmitsPersistedPrivateSkill() throws Exception { + void outsiderSearchReturnsMatchingPublicSkillAndOmitsPrivateSkill() throws Exception { mockMvc.perform(withBearer( - get("/api/cli/v1/skills/search").param("limit", "20"), + get("/api/cli/v1/skills/search") + .param("q", skillSlug) + .param("limit", "20"), outsiderToken)) .andExpect(status().isOk()) .andExpect(jsonPath("$", aMapWithSize(5))) .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.items[*].slug", hasItem(publicSkillSlug))) .andExpect(jsonPath("$.data.items[*].slug", not(hasItem(skillSlug)))); } @Test void outsiderCannotResolvePrivateSkill() throws Exception { - mockMvc.perform(withBearer( - get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), - outsiderToken)) - .andExpect(status().isForbidden()) - .andExpect(jsonPath("$.code").value(403)); + assertForbiddenEnvelope(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/resolve", namespaceSlug, skillSlug), + outsiderToken)); } @Test void outsiderCannotDownloadLatestPrivateSkill() throws Exception { - mockMvc.perform(withBearer( - get("/api/cli/v1/skills/{namespace}/{slug}/download", namespaceSlug, skillSlug), - outsiderToken)) - .andExpect(status().isForbidden()) - .andExpect(jsonPath("$.code").value(403)); + assertForbiddenEnvelope(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/download", namespaceSlug, skillSlug), + outsiderToken)); } @Test void outsiderCannotDownloadVersionedPrivateSkill() throws Exception { - mockMvc.perform(withBearer( - get("/api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download", - namespaceSlug, skillSlug, version), - outsiderToken)) + assertForbiddenEnvelope(withBearer( + get("/api/cli/v1/skills/{namespace}/{slug}/versions/{version}/download", + namespaceSlug, skillSlug, version), + outsiderToken)); + } + + private void assertForbiddenEnvelope(MockHttpServletRequestBuilder request) throws Exception { + mockMvc.perform(request) .andExpect(status().isForbidden()) - .andExpect(jsonPath("$.code").value(403)); + .andExpect(jsonPath("$", aMapWithSize(5))) + .andExpect(jsonPath("$.code").value(403)) + .andExpect(jsonPath("$.msg").isString()) + .andExpect(jsonPath("$.data").value(nullValue())) + .andExpect(jsonPath("$.timestamp").isString()) + .andExpect(jsonPath("$.requestId").isString()); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java index e351a7e5..1783bf71 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/cli/CliTokenLifecycleSecurityIntegrationTest.java @@ -8,16 +8,21 @@ import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.dto.cli.CliResolveResponse; import com.iflytek.skillhub.service.cli.CliSkillAppService; +import jakarta.servlet.http.HttpServletRequest; import java.io.ByteArrayInputStream; import java.time.Clock; import java.time.Instant; import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; @@ -27,20 +32,26 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; +import org.springframework.mock.web.MockHttpSession; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -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.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -59,6 +70,21 @@ class CliTokenLifecycleSecurityIntegrationTest { MALFORMED } + private enum EndpointCase { + WHOAMI, + SEARCH, + RESOLVE, + LATEST_DOWNLOAD, + VERSIONED_DOWNLOAD + } + + private enum MixedCredentialState { + SESSION_ONLY, + SESSION_BASIC, + BASIC_ONLY, + SESSION_VALID_BEARER + } + @Autowired MockMvc mockMvc; @Autowired ApiTokenService apiTokenService; @Autowired ApiTokenRepository apiTokenRepository; @@ -67,12 +93,16 @@ class CliTokenLifecycleSecurityIntegrationTest { @MockBean CliSkillAppService cliSkillAppService; private String userId; + private String sessionUserId; @BeforeEach void setUp() { userId = "token-matrix-" + UUID.randomUUID(); + sessionUserId = "session-matrix-" + UUID.randomUUID(); userAccountRepository.save(new UserAccount( userId, "Token Matrix", userId + "@example.com", "")); + userAccountRepository.save(new UserAccount( + sessionUserId, "Session Matrix", sessionUserId + "@example.com", "")); given(cliSkillAppService.search(any(), anyInt(), any(), any())) .willReturn(new CliSkillAppService.CliSearchResult(List.of(), 0, 20)); given(cliSkillAppService.resolve(anyString(), anyString(), any(), any(), any())) @@ -100,13 +130,54 @@ class CliTokenLifecycleSecurityIntegrationTest { .andExpect(jsonPath("$.data.handle").value(userId)); } + @ParameterizedTest(name = "{0} with {1}") + @MethodSource("mixedCredentialMatrix") + void sessionAndAuthorizationSchemeMatrix( + EndpointCase endpoint, + MixedCredentialState credentialState) throws Exception { + clearInvocations(cliSkillAppService); + String expectedUserId = expectedUserId(credentialState); + MockHttpServletRequestBuilder request = withCredentials(requestFor(endpoint), credentialState); + + if (endpoint == EndpointCase.WHOAMI) { + if (credentialState == MixedCredentialState.BASIC_ONLY) { + assertUnauthorizedEnvelope(request); + } else { + assertSuccessEnvelope(request) + .andExpect(jsonPath("$.data.handle").value(expectedUserId)); + } + verifyNoInteractions(cliSkillAppService); + return; + } + + ResultActions result = mockMvc.perform(request).andExpect(status().isOk()); + if (endpoint == EndpointCase.LATEST_DOWNLOAD + || endpoint == EndpointCase.VERSIONED_DOWNLOAD) { + result.andExpect(content().contentType("application/zip")); + } else { + result.andExpect(jsonPath("$", aMapWithSize(5))) + .andExpect(jsonPath("$.code").value(0)); + } + assertProjectedUser(endpoint, expectedUserId); + } + + @Test + void whoamiReturnsNullEmailForPersistedUserWithoutEmail() throws Exception { + String noEmailUserId = "token-no-email-" + UUID.randomUUID(); + userAccountRepository.save(new UserAccount(noEmailUserId, "No Email User", null, "")); + String rawToken = apiTokenService.createToken( + noEmailUserId, "no-email-" + UUID.randomUUID(), "[\"skill:read\"]").rawToken(); + + assertSuccessEnvelope(withBearer(get("/api/cli/v1/auth/whoami"), rawToken)) + .andExpect(jsonPath("$.data", hasKey("email"))) + .andExpect(jsonPath("$.data.email").value(nullValue())); + } + @ParameterizedTest(name = "whoami rejects {0}") @EnumSource(InvalidCredentialState.class) void whoamiRejectsInvalidBearer(InvalidCredentialState state) throws Exception { clearInvocations(cliSkillAppService); - mockMvc.perform(withInvalidBearer(get("/api/cli/v1/auth/whoami"), state)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.code").value(401)); + assertUnauthorizedEnvelope(withInvalidBearer(get("/api/cli/v1/auth/whoami"), state)); verifyNoInteractions(cliSkillAppService); } @@ -128,10 +199,8 @@ class CliTokenLifecycleSecurityIntegrationTest { @EnumSource(InvalidCredentialState.class) void searchRejectsInvalidBearer(InvalidCredentialState state) throws Exception { clearInvocations(cliSkillAppService); - mockMvc.perform(withInvalidBearer( - get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), state)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.code").value(401)); + assertUnauthorizedEnvelope(withInvalidBearer( + get("/api/cli/v1/skills/search").param("q", "demo").param("limit", "20"), state)); verifyNoInteractions(cliSkillAppService); } @@ -152,9 +221,8 @@ class CliTokenLifecycleSecurityIntegrationTest { @EnumSource(InvalidCredentialState.class) void resolveRejectsInvalidBearer(InvalidCredentialState state) throws Exception { clearInvocations(cliSkillAppService); - mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/resolve"), state)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.code").value(401)); + assertUnauthorizedEnvelope(withInvalidBearer( + get("/api/cli/v1/skills/global/demo/resolve"), state)); verifyNoInteractions(cliSkillAppService); } @@ -176,9 +244,8 @@ class CliTokenLifecycleSecurityIntegrationTest { @EnumSource(InvalidCredentialState.class) void latestDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { clearInvocations(cliSkillAppService); - mockMvc.perform(withInvalidBearer(get("/api/cli/v1/skills/global/demo/download"), state)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.code").value(401)); + assertUnauthorizedEnvelope(withInvalidBearer( + get("/api/cli/v1/skills/global/demo/download"), state)); verifyNoInteractions(cliSkillAppService); } @@ -201,10 +268,8 @@ class CliTokenLifecycleSecurityIntegrationTest { @EnumSource(InvalidCredentialState.class) void versionedDownloadRejectsInvalidBearer(InvalidCredentialState state) throws Exception { clearInvocations(cliSkillAppService); - mockMvc.perform(withInvalidBearer( - get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), state)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.code").value(401)); + assertUnauthorizedEnvelope(withInvalidBearer( + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"), state)); verifyNoInteractions(cliSkillAppService); } @@ -272,7 +337,70 @@ class CliTokenLifecycleSecurityIntegrationTest { InvalidCredentialState state) { return request .header(HttpHeaders.AUTHORIZATION, authorizationHeader(state)) - .with(authentication(sessionAuthentication())); + .session(session()); + } + + private static Stream mixedCredentialMatrix() { + return Stream.of(EndpointCase.values()) + .flatMap(endpoint -> Stream.of(MixedCredentialState.values()) + .map(state -> Arguments.of(endpoint, state))); + } + + private MockHttpServletRequestBuilder requestFor(EndpointCase endpoint) { + return switch (endpoint) { + case WHOAMI -> get("/api/cli/v1/auth/whoami"); + case SEARCH -> get("/api/cli/v1/skills/search") + .param("q", "demo") + .param("limit", "20"); + case RESOLVE -> get("/api/cli/v1/skills/global/demo/resolve"); + case LATEST_DOWNLOAD -> get("/api/cli/v1/skills/global/demo/download"); + case VERSIONED_DOWNLOAD -> + get("/api/cli/v1/skills/global/demo/versions/1.0.0/download"); + }; + } + + private MockHttpServletRequestBuilder withCredentials( + MockHttpServletRequestBuilder request, + MixedCredentialState state) { + return switch (state) { + case SESSION_ONLY -> request.session(session()); + case SESSION_BASIC -> request.session(session()) + .header(HttpHeaders.AUTHORIZATION, "Basic dGVzdDp0ZXN0"); + case BASIC_ONLY -> request.header(HttpHeaders.AUTHORIZATION, "Basic dGVzdDp0ZXN0"); + case SESSION_VALID_BEARER -> withBearer(request.session(session()), createActiveToken()); + }; + } + + private String expectedUserId(MixedCredentialState state) { + return switch (state) { + case SESSION_ONLY, SESSION_BASIC -> sessionUserId; + case BASIC_ONLY -> null; + case SESSION_VALID_BEARER -> userId; + }; + } + + private void assertProjectedUser(EndpointCase endpoint, String expectedUserId) { + if (endpoint == EndpointCase.SEARCH) { + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(String.class); + verify(cliSkillAppService).search(any(), anyInt(), userCaptor.capture(), any()); + assertEquals(expectedUserId, userCaptor.getValue()); + return; + } + if (endpoint == EndpointCase.RESOLVE) { + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(String.class); + verify(cliSkillAppService).resolve(anyString(), anyString(), any(), userCaptor.capture(), any()); + assertEquals(expectedUserId, userCaptor.getValue()); + return; + } + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(HttpServletRequest.class); + if (endpoint == EndpointCase.LATEST_DOWNLOAD) { + verify(cliSkillAppService).downloadLatest(anyString(), anyString(), requestCaptor.capture()); + } else { + verify(cliSkillAppService).downloadVersion( + anyString(), anyString(), anyString(), requestCaptor.capture()); + } + assertEquals(expectedUserId, requestCaptor.getValue().getAttribute("userId")); } private MockHttpServletRequestBuilder withBearer( @@ -310,9 +438,24 @@ class CliTokenLifecycleSecurityIntegrationTest { userId, "matrix-" + UUID.randomUUID(), "[\"skill:read\"]"); } + private MockHttpSession session() { + SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); + securityContext.setAuthentication(sessionAuthentication()); + MockHttpSession session = new MockHttpSession(); + session.setAttribute( + HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, + securityContext); + return session; + } + private UsernamePasswordAuthenticationToken sessionAuthentication() { PlatformPrincipal principal = new PlatformPrincipal( - userId, "Session User", userId + "@example.com", "", "session", Set.of("USER")); + sessionUserId, + "Session User", + sessionUserId + "@example.com", + "", + "session", + Set.of("USER")); return new UsernamePasswordAuthenticationToken(principal, null, List.of()); } From 8435ee1ab16501a70849104c83c8826489fd77bc Mon Sep 17 00:00:00 2001 From: Gal Eyal Date: Mon, 27 Jul 2026 21:47:59 +0300 Subject: [PATCH 21/25] fix(auth): read device-code state via ObjectMapper conversion, not cast The shared RedisTemplate uses GenericJackson2JsonRedisSerializer with the application ObjectMapper, which embeds no type information, so stored DeviceCodeData deserializes as a LinkedHashMap. The typed casts in pollToken and authorizeDeviceCode then throw ClassCastException on every call, making the whole device authorization flow unusable (every poll returns 500). Convert the raw value with ObjectMapper.convertValue instead of casting; this reads both the current untyped map format and any typed format, so no stored-data migration is needed. Adds bean setters to DeviceCodeData for map conversion and regression tests that feed the service exactly what Redis returns in production (untyped maps). Fixes #604 Co-Authored-By: Claude Fable 5 Signed-off-by: Gal Eyal --- .../auth/device/DeviceAuthService.java | 19 +++- .../skillhub/auth/device/DeviceCodeData.java | 2 + .../auth/device/DeviceAuthServiceTest.java | 106 ++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java index e838c9a8..5061f854 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.auth.device; +import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.auth.token.ApiTokenService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import org.springframework.beans.factory.annotation.Value; @@ -33,14 +34,17 @@ public class DeviceAuthService { private final RedisTemplate redisTemplate; private final ApiTokenService apiTokenService; + private final ObjectMapper objectMapper; private final String verificationUri; private final SecureRandom random = new SecureRandom(); public DeviceAuthService(RedisTemplate redisTemplate, ApiTokenService apiTokenService, + ObjectMapper objectMapper, @Value("${skillhub.device-auth.verification-uri:/cli/auth}") String verificationUri) { this.redisTemplate = redisTemplate; this.apiTokenService = apiTokenService; + this.objectMapper = objectMapper; this.verificationUri = verificationUri; } @@ -71,7 +75,7 @@ public class DeviceAuthService { throw new DomainBadRequestException("error.deviceAuth.userCode.invalid"); } - DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + DeviceCodeData data = readDeviceCodeData(deviceCode); if (data == null) { throw new DomainBadRequestException("error.deviceAuth.deviceCode.expired"); } @@ -97,7 +101,7 @@ public class DeviceAuthService { * into an API token exactly once. */ public DeviceTokenResponse pollToken(String deviceCode) { - DeviceCodeData data = (DeviceCodeData) redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + DeviceCodeData data = readDeviceCodeData(deviceCode); if (data == null) { throw new DomainBadRequestException("error.deviceAuth.deviceCode.invalid"); @@ -147,6 +151,17 @@ public class DeviceAuthService { } } + /** + * Reads device-code state from Redis. The shared template's JSON value + * serializer carries no type information, so values deserialize as plain + * maps; convert explicitly instead of casting (a direct cast throws + * {@code ClassCastException} on every read). + */ + private DeviceCodeData readDeviceCodeData(String deviceCode) { + Object raw = redisTemplate.opsForValue().get(DEVICE_CODE_PREFIX + deviceCode); + return raw == null ? null : objectMapper.convertValue(raw, DeviceCodeData.class); + } + private String generateRandomDeviceCode() { byte[] bytes = new byte[32]; random.nextBytes(bytes); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java index 015896b7..7c44a22d 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceCodeData.java @@ -19,7 +19,9 @@ public class DeviceCodeData implements Serializable { } public String getDeviceCode() { return deviceCode; } + public void setDeviceCode(String deviceCode) { this.deviceCode = deviceCode; } public String getUserCode() { return userCode; } + public void setUserCode(String userCode) { this.userCode = userCode; } public DeviceCodeStatus getStatus() { return status; } public void setStatus(DeviceCodeStatus status) { this.status = status; } public String getUserId() { return userId; } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java new file mode 100644 index 00000000..fca992b2 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java @@ -0,0 +1,106 @@ +package com.iflytek.skillhub.auth.device; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.core.ValueOperations; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DeviceAuthServiceTest { + + private static final String DEVICE_CODE = "device-code-1"; + private static final String USER_CODE = "ABCD-2345"; + + @Mock + private RedisTemplate redisTemplate; + + @Mock + private ValueOperations valueOperations; + + @Mock + private ApiTokenService apiTokenService; + + private DeviceAuthService service; + + @BeforeEach + void setUp() { + lenient().when(redisTemplate.opsForValue()).thenReturn(valueOperations); + service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/cli/auth"); + } + + /** + * The shared RedisTemplate's JSON serializer keeps no type information, so + * stored DeviceCodeData comes back as a plain map. A typed cast used to + * throw ClassCastException on every poll; the service must convert instead. + */ + private static Map storedDeviceCode(DeviceCodeStatus status, String userId) { + Map raw = new LinkedHashMap<>(); + raw.put("deviceCode", DEVICE_CODE); + raw.put("userCode", USER_CODE); + raw.put("status", status.name()); + raw.put("userId", userId); + return raw; + } + + @Test + void pollTokenReturnsPendingWhenRedisValueIsUntypedMap() { + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.PENDING, null)); + + DeviceTokenResponse response = service.pollToken(DEVICE_CODE); + + assertThat(response.error()).isEqualTo("authorization_pending"); + } + + @Test + void pollTokenRedeemsAuthorizedCodeFromUntypedMap() { + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.AUTHORIZED, "usr_1")); + when(valueOperations.setIfAbsent(eq("device:claim:" + DEVICE_CODE), any(), anyLong(), any())) + .thenReturn(Boolean.TRUE); + when(apiTokenService.rotateToken(eq("usr_1"), any(), any())) + .thenReturn(new ApiTokenService.TokenCreateResult("sk_test_token", null)); + + DeviceTokenResponse response = service.pollToken(DEVICE_CODE); + + assertThat(response.accessToken()).isEqualTo("sk_test_token"); + } + + @Test + void pollTokenRejectsUnknownDeviceCode() { + when(valueOperations.get("device:code:" + DEVICE_CODE)).thenReturn(null); + + assertThatThrownBy(() -> service.pollToken(DEVICE_CODE)) + .isInstanceOf(DomainBadRequestException.class); + } + + @Test + void authorizeDeviceCodeMarksPendingCodeFromUntypedMap() { + when(valueOperations.get("device:usercode:" + USER_CODE)).thenReturn(DEVICE_CODE); + when(valueOperations.get("device:code:" + DEVICE_CODE)) + .thenReturn(storedDeviceCode(DeviceCodeStatus.PENDING, null)); + + service.authorizeDeviceCode(USER_CODE, "usr_1"); + + verify(valueOperations).set(startsWith("device:code:"), any(DeviceCodeData.class), anyLong(), any()); + } +} From 1d679c526ab498cba1d56a761d20d7028195e8d2 Mon Sep 17 00:00:00 2001 From: "1664940968@qq.com" <1664940968@qq.com> Date: Tue, 28 Jul 2026 16:36:47 +0800 Subject: [PATCH 22/25] fix(auth): recover login page from stale lazy-loaded chunks after logout (#560) * fix(auth): recover from stale login chunks after logout * fix(auth): prevent repeated stale chunk reloads Signed-off-by: ylhu16 --------- Signed-off-by: ylhu16 Co-authored-by: ylhu16 --- web/src/app/router.tsx | 11 ++- .../lib/dynamic-import-recovery.test.ts | 87 +++++++++++++++++++ web/src/shared/lib/dynamic-import-recovery.ts | 48 ++++++++++ 3 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 web/src/shared/lib/dynamic-import-recovery.test.ts create mode 100644 web/src/shared/lib/dynamic-import-recovery.ts diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index c9b3bac8..ec9749a5 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -4,6 +4,7 @@ import { Layout } from './layout' import { getCurrentUser } from '@/api/client' import { RoleGuard } from '@/shared/components/role-guard' import { createRequireAuth } from '@/shared/lib/auth-route' +import { clearDynamicImportReloadGuard, recoverFromDynamicImportError } from '@/shared/lib/dynamic-import-recovery' import { normalizeSearchQuery } from '@/shared/lib/search-query' /** @@ -25,7 +26,15 @@ function createLazyRouteComponent>( // Lazy route modules are wrapped in a uniform suspense fallback so route transitions behave // consistently across public and dashboard pages. const LazyComponent = lazy(async () => { - const module = await importer() + const module = await importer().catch((error) => { + if (recoverFromDynamicImportError(error)) { + return new Promise(() => {}) + } + throw error + }) + // Router resolution can finish before React.lazy imports the route module. Only clear the + // one-time reload guard after the chunk itself has loaded successfully. + clearDynamicImportReloadGuard() return { default: module[exportName] as ComponentType> } }) diff --git a/web/src/shared/lib/dynamic-import-recovery.test.ts b/web/src/shared/lib/dynamic-import-recovery.test.ts new file mode 100644 index 00000000..8cba7774 --- /dev/null +++ b/web/src/shared/lib/dynamic-import-recovery.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + clearDynamicImportReloadGuard, + isDynamicImportFetchError, + recoverFromDynamicImportError, +} from './dynamic-import-recovery' + +const values = new Map() +const reload = vi.fn() +const sessionStorage = { + get length() { + return values.size + }, + clear: vi.fn(() => values.clear()), + getItem: vi.fn((key: string) => values.get(key) ?? null), + key: vi.fn((index: number) => Array.from(values.keys())[index] ?? null), + removeItem: vi.fn((key: string) => values.delete(key)), + setItem: vi.fn((key: string, value: string) => values.set(key, value)), +} satisfies Storage + +describe('dynamic import recovery', () => { + beforeEach(() => { + values.clear() + reload.mockClear() + vi.stubGlobal('window', { + location: { reload }, + sessionStorage, + }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it.each([ + 'Failed to fetch dynamically imported module: /assets/login.js', + 'error loading dynamically imported module: /assets/login.js', + 'Importing a module script failed', + 'ChunkLoadError: Loading chunk 42 failed', + ])('recognizes a stale dynamic import error: %s', (message) => { + expect(isDynamicImportFetchError(new Error(message))).toBe(true) + }) + + it('ignores unrelated errors', () => { + expect(isDynamicImportFetchError(new Error('Request failed with status 500'))).toBe(false) + }) + + it('recognizes errors whose name is ChunkLoadError', () => { + const error = new Error('Loading chunk 42 failed') + error.name = 'ChunkLoadError' + + expect(isDynamicImportFetchError(error)).toBe(true) + }) + + it('reloads only once while the recovery guard is active', () => { + const error = new Error('Failed to fetch dynamically imported module') + + expect(recoverFromDynamicImportError(error)).toBe(true) + expect(recoverFromDynamicImportError(error)).toBe(false) + expect(recoverFromDynamicImportError(error)).toBe(false) + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('allows recovery again after a dynamic import succeeds', () => { + const error = new Error('Failed to fetch dynamically imported module') + + expect(recoverFromDynamicImportError(error)).toBe(true) + clearDynamicImportReloadGuard() + expect(recoverFromDynamicImportError(error)).toBe(true) + expect(reload).toHaveBeenCalledTimes(2) + }) + + it('does not mask the original import error when session storage is unavailable', () => { + vi.stubGlobal('window', { + location: { reload }, + get sessionStorage() { + throw new DOMException('Access denied', 'SecurityError') + }, + }) + + const error = new Error('Failed to fetch dynamically imported module') + + expect(recoverFromDynamicImportError(error)).toBe(false) + expect(() => clearDynamicImportReloadGuard()).not.toThrow() + expect(reload).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/shared/lib/dynamic-import-recovery.ts b/web/src/shared/lib/dynamic-import-recovery.ts new file mode 100644 index 00000000..27c04264 --- /dev/null +++ b/web/src/shared/lib/dynamic-import-recovery.ts @@ -0,0 +1,48 @@ +const RELOAD_GUARD_KEY = 'skillhub:dynamic-import-reload' + +function resolveErrorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message + } + return String(error ?? '') +} + +export function isDynamicImportFetchError(error: unknown): boolean { + const message = resolveErrorMessage(error) + return (error instanceof Error && error.name === 'ChunkLoadError') + || message.includes('Failed to fetch dynamically imported module') + || message.includes('error loading dynamically imported module') + || message.includes('Importing a module script failed') + || message.includes('ChunkLoadError') +} + +export function recoverFromDynamicImportError(error: unknown): boolean { + if (typeof window === 'undefined' || !isDynamicImportFetchError(error)) { + return false + } + + let sessionStorage: Storage + try { + sessionStorage = window.sessionStorage + if (sessionStorage.getItem(RELOAD_GUARD_KEY) === '1') { + return false + } + sessionStorage.setItem(RELOAD_GUARD_KEY, '1') + } catch { + return false + } + + window.location.reload() + return true +} + +export function clearDynamicImportReloadGuard(): void { + if (typeof window === 'undefined') { + return + } + try { + window.sessionStorage.removeItem(RELOAD_GUARD_KEY) + } catch { + // Session storage can be unavailable in restricted browsing contexts. + } +} From d977ea9dc41cd226985fee3374a39e669a32fc42 Mon Sep 17 00:00:00 2001 From: gale-popai Date: Tue, 28 Jul 2026 12:42:20 +0300 Subject: [PATCH 23/25] fix(api): tell callers why a request was forbidden (#610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(api): tell callers why a request was forbidden The scope filter already computes an exact reason ("Missing API token scope: skill:delete", "API token cannot access endpoint: /x") and the access-denied handler discarded it, returning a bare "Forbidden" for every case: missing scope, endpoint closed to API tokens, and paths that simply don't exist. Clients cannot tell those apart, so they guess — the published CLI reports every 403 as "token may lack required scope", which sent us debugging token scopes for an hour when the real causes were a revoked token and a mistyped namespace path. The reason now rides in the response via a new error.forbidden.detail message (en + zh), and is logged alongside the exception type. Signed-off-by: Gal Eyal Co-Authored-By: Claude Fable 5 * fix(api): safely expose API token denial reasons Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Co-authored-by: Claude Fable 5 Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- cli/src/clients/skillhub-client.ts | 32 +++- cli/src/shared/output.ts | 3 + cli/test/helpers/fake-registry.ts | 8 +- cli/test/integration/publish-dry-run.test.ts | 1 + cli/test/unit/clients/skillhub-client.test.ts | 48 +++++- cli/test/unit/shared/output.test.ts | 2 + docs/03-authentication-design.md | 1 + docs/skillhub/en/guide/cli.md | 2 + docs/skillhub/guide/cli.md | 2 + .../security/ApiAccessDeniedHandler.java | 18 ++- .../src/main/resources/messages.properties | 2 + .../src/main/resources/messages_zh.properties | 2 + .../security/ApiAccessDeniedHandlerTest.java | 137 ++++++++++++++++++ .../token/ApiTokenAccessDeniedException.java | 42 ++++++ .../auth/token/ApiTokenScopeFilter.java | 10 +- .../auth/token/ApiTokenScopeFilterTest.java | 10 ++ 16 files changed, 302 insertions(+), 18 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAccessDeniedException.java diff --git a/cli/src/clients/skillhub-client.ts b/cli/src/clients/skillhub-client.ts index 14e14ec3..8d008483 100644 --- a/cli/src/clients/skillhub-client.ts +++ b/cli/src/clients/skillhub-client.ts @@ -52,6 +52,11 @@ export interface DryRunResponse { resolvedVersion: string | null } +interface ErrorEnvelope { + msg?: unknown + requestId?: unknown +} + export class SkillHubClient { constructor( readonly registry: string, @@ -88,9 +93,12 @@ export class SkillHubClient { } catch { throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' }) } - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' }) } + if (response.status === 403) { + throw await this.createAccessDeniedError(response) + } if (response.status === 404) { throw new CliError('skill or version not found', EXIT.generic, { registry: this.registry }) } @@ -155,7 +163,7 @@ export class SkillHubClient { throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' }) } if (response.status === 403) { - throw new CliError('access denied — token may lack required scope', EXIT.auth, { registry: this.registry, next: 'regenerate token with required scopes or run `skillhub login`' }) + throw await this.createAccessDeniedError(response) } if (response.status === 404) { throw new CliError('resource not found', EXIT.generic, { registry: this.registry }) @@ -172,6 +180,26 @@ export class SkillHubClient { return body.data as T } + private async createAccessDeniedError(response: Response): Promise { + const error = await this.readErrorEnvelope(response) + return new CliError(error.message ?? 'access denied', EXIT.auth, { + registry: this.registry, + ...(error.requestId ? { requestId: error.requestId } : {}) + }) + } + + private async readErrorEnvelope(response: Response): Promise<{ message?: string; requestId?: string }> { + try { + const body = await response.json() as ErrorEnvelope + return { + ...(typeof body.msg === 'string' && body.msg.trim() ? { message: body.msg } : {}), + ...(typeof body.requestId === 'string' && body.requestId.trim() ? { requestId: body.requestId } : {}) + } + } catch { + return {} + } + } + private headers(): HeadersInit { return this.token ? { Authorization: `Bearer ${this.token}` } : {} } diff --git a/cli/src/shared/output.ts b/cli/src/shared/output.ts index 9b2eafd9..977116bc 100644 --- a/cli/src/shared/output.ts +++ b/cli/src/shared/output.ts @@ -30,6 +30,9 @@ export function renderError(error: unknown, json: boolean): string { if (typeof cliError.details.path === 'string') { lines.push(`Context: path ${cliError.details.path}`) } + if (typeof cliError.details.requestId === 'string') { + lines.push(`Request ID: ${cliError.details.requestId}`) + } if (typeof cliError.details.next === 'string') { lines.push(`Next: ${cliError.details.next}`) } diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index a3ea9ef8..4fde3a95 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -22,7 +22,7 @@ export function createFakeRegistry(handlers: Record) { /** * Controls how a specific endpoint behaves when a failure is injected: * 'auth' => 401 { code: 401, message: 'unauthorized' } - * 'forbidden' => 403 { code: 403, message: 'forbidden' } + * 'forbidden' => 403 with a standard SkillHub error envelope * 'not_found' => 404 { code: 404, message: 'not found' } * 'server_error' => 500 { code: 500, message: 'internal error' } * 'network' => handler throws, causing fetch() to reject with a TypeError @@ -34,7 +34,11 @@ function failureResponse(mode: FailureMode): Response { case 'auth': return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) case 'forbidden': - return Response.json({ code: 403, message: 'forbidden' }, { status: 403 }) + return Response.json({ + code: 403, + msg: 'API token is missing required scope: skill:publish', + requestId: 'req-test-forbidden' + }, { status: 403 }) case 'not_found': return Response.json({ code: 404, message: 'not found' }, { status: 404 }) case 'server_error': diff --git a/cli/test/integration/publish-dry-run.test.ts b/cli/test/integration/publish-dry-run.test.ts index deabb7b2..456572d1 100644 --- a/cli/test/integration/publish-dry-run.test.ts +++ b/cli/test/integration/publish-dry-run.test.ts @@ -172,5 +172,6 @@ describe('publish --dry-run', () => { expect(result.exitCode).toBe(2) expect(result.stderr).toContain('scope') + expect(result.stderr).toContain('Request ID: req-test-forbidden') }) }) diff --git a/cli/test/unit/clients/skillhub-client.test.ts b/cli/test/unit/clients/skillhub-client.test.ts index c07083c2..e545e99f 100644 --- a/cli/test/unit/clients/skillhub-client.test.ts +++ b/cli/test/unit/clients/skillhub-client.test.ts @@ -37,12 +37,21 @@ describe('SkillHubClient', () => { }) test('download() throws auth error on 403', async () => { - const fetchImpl = (async () => new Response(null, { status: 403 })) as unknown as typeof fetch + const fetchImpl = (async () => Response.json({ + code: 403, + msg: 'API token is missing required scope: skill:read', + requestId: 'req-download' + }, { status: 403 })) as unknown as typeof fetch const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) - const err = expect(client.download('ns', 'slug')).rejects - await err.toBeInstanceOf(CliError) - await err.toHaveProperty('message', 'authentication failed') - await err.toHaveProperty('exitCode', EXIT.auth) + + await expect(client.download('ns', 'slug')).rejects.toMatchObject({ + message: 'API token is missing required scope: skill:read', + exitCode: EXIT.auth, + details: { + registry: 'http://registry.test', + requestId: 'req-download' + } + }) }) test('download() throws not-found error on 404', async () => { @@ -159,6 +168,35 @@ describe('SkillHubClient', () => { // --- handleJsonResponse() non-2xx classification --- + test('whoami() surfaces server reason and request ID on 403', async () => { + const fetchImpl = (async () => Response.json({ + code: 403, + msg: 'API token cannot access endpoint: /api/cli/v1/whoami', + requestId: 'req-610' + }, { status: 403 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + + await expect(client.whoami()).rejects.toMatchObject({ + message: 'API token cannot access endpoint: /api/cli/v1/whoami', + exitCode: EXIT.auth, + details: { + registry: 'http://registry.test', + requestId: 'req-610' + } + }) + }) + + test('whoami() falls back to generic access denied when 403 body is invalid', async () => { + const fetchImpl = (async () => new Response('not-json', { status: 403 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + + await expect(client.whoami()).rejects.toMatchObject({ + message: 'access denied', + exitCode: EXIT.auth, + details: { registry: 'http://registry.test' } + }) + }) + test('whoami() throws generic error on 500', async () => { const fetchImpl = (async () => new Response(null, { status: 500 })) as unknown as typeof fetch const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) diff --git a/cli/test/unit/shared/output.test.ts b/cli/test/unit/shared/output.test.ts index 8d051172..d71bcce0 100644 --- a/cli/test/unit/shared/output.test.ts +++ b/cli/test/unit/shared/output.test.ts @@ -16,11 +16,13 @@ describe('renderError', () => { test('renders human error without stack trace', () => { const error = new CliError('registry unreachable', 3, { registry: 'https://registry.example.com', + requestId: 'req-610', next: 'check network or pass --registry' }) expect(renderError(error, false)).toBe([ 'Error: registry unreachable', 'Context: registry https://registry.example.com', + 'Request ID: req-610', 'Next: check network or pass --registry' ].join('\n')) }) diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index b4fb0cd1..2be902e8 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -379,6 +379,7 @@ API Token 仍保留，但定位从“CLI 唯一认证方式”调整为“平台 - 校验：从 `Authorization: Bearer ` 提取 → 哈希比对 → 加载关联用户 → 检查用户状态 - 失败闭合：公共读接口只有在缺少 `Authorization` 头时才按匿名访问处理；只要出现 Bearer 凭证，空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 401，不能回退为匿名访问 - 作用域：`skill:read`, `skill:publish`, `skill:delete`, `token:manage` +- 拒绝原因：API Token 缺少作用域或不能访问某个接口时，403 响应返回本地化的安全原因和 `requestId`；其他授权失败仍返回通用信息，避免暴露内部异常 > **一期作用域说明（非最小权限）**：一期 Token 作用域为粗粒度动作级别，不与 namespace 绑定。Token 继承用户的全部权限——如果用户是某个 namespace 的 MEMBER，则该用户的任何 Token（只要包含 `skill:publish` scope）都可以向该 namespace 发布技能。这是有意的一期简化，不满足最小权限原则。后续版本计划引入 namespace 级别的 Token 作用域限定（如 `namespace:ai-team:skill:publish`），或通过 `api_token_scope` 子表实现 Token 与 namespace 的绑定。 diff --git a/docs/skillhub/en/guide/cli.md b/docs/skillhub/en/guide/cli.md index 6cbc779c..e4fa2842 100644 --- a/docs/skillhub/en/guide/cli.md +++ b/docs/skillhub/en/guide/cli.md @@ -83,6 +83,8 @@ skillhub login --token sk_xxx --registry https://skillhub.example.com `login` validates the token, stores it in `~/.skillhub/credentials.json`, and writes the registry to `~/.skillhub/config.json`. +When an API-token request is denied, the CLI shows the safe reason returned by the server and its `Request ID`. Use that ID to correlate the failure with server logs. Other authorization failures continue to use a generic message. + ### Check Current Identity ```bash diff --git a/docs/skillhub/guide/cli.md b/docs/skillhub/guide/cli.md index 22162d56..dfadf6d3 100644 --- a/docs/skillhub/guide/cli.md +++ b/docs/skillhub/guide/cli.md @@ -83,6 +83,8 @@ skillhub login --token sk_xxx --registry https://skillhub.example.com `login` 会验证 token 有效性，然后将 token 存储到 `~/.skillhub/credentials.json`，同时将 registry 写入 `~/.skillhub/config.json`。 +API Token 请求被拒绝时，CLI 会显示服务端返回的具体原因和 `Request ID`。排查问题时可使用该 ID 对照服务端日志；非 API Token 的授权失败仍只显示通用信息。 + ### 查看当前身份 ```bash diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java index 81cebbde..2c930aa6 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.security; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.token.ApiTokenAccessDeniedException; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import jakarta.servlet.http.HttpServletRequest; @@ -38,14 +39,25 @@ public class ApiAccessDeniedHandler implements AccessDeniedHandler { public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException accessDeniedException) throws IOException { + ApiTokenAccessDeniedException apiTokenException = + accessDeniedException instanceof ApiTokenAccessDeniedException typedException + ? typedException + : null; logger.info( - "Forbidden API request [requestId={}, method={}, path={}, reason={}]", + "Forbidden API request [requestId={}, method={}, path={}, reason={}, detail={}]", MDC.get("requestId"), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), - accessDeniedException.getClass().getSimpleName() + accessDeniedException.getClass().getSimpleName(), + apiTokenException != null ? apiTokenException.getMessage() : null ); - ApiResponse body = apiResponseFactory.error(403, "error.forbidden"); + ApiResponse body = apiTokenException != null + ? apiResponseFactory.error( + 403, + apiTokenException.getMessageCode(), + apiTokenException.getMessageArgs() + ) + : apiResponseFactory.error(403, "error.forbidden"); response.setStatus(HttpServletResponse.SC_FORBIDDEN); response.setContentType(MediaType.APPLICATION_JSON_VALUE); objectMapper.writeValue(response.getOutputStream(), body); diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 19189122..79195af7 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -48,6 +48,8 @@ error.auth.sessionBootstrap.providerUnsupported=Unsupported session bootstrap pr error.auth.sessionBootstrap.notAuthenticated=No authenticated external session found error.badRequest=Invalid request error.forbidden=Forbidden +error.apiToken.scope.missing=API token is missing required scope: {0} +error.apiToken.endpoint.unsupported=API token cannot access endpoint: {0} error.request.timeout=Request timed out error.rateLimit.exceeded=Rate limit exceeded error.storage.unavailable=Object storage is temporarily unavailable. Please try again later. diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 6885dfd3..d7b6b11b 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -48,6 +48,8 @@ error.auth.sessionBootstrap.providerUnsupported=不支持的会话引导提供 error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会话 error.badRequest=请求参数不合法 error.forbidden=没有权限执行该操作 +error.apiToken.scope.missing=API 令牌缺少所需权限范围：{0} +error.apiToken.endpoint.unsupported=API 令牌无法访问接口：{0} error.request.timeout=请求超时 error.rateLimit.exceeded=请求过于频繁，请稍后再试 error.storage.unavailable=对象存储暂时不可用，请稍后再试 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java new file mode 100644 index 00000000..db8982b7 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java @@ -0,0 +1,137 @@ +package com.iflytek.skillhub.security; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.token.ApiTokenScopeFilter; +import com.iflytek.skillhub.auth.token.ApiTokenScopeService; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import jakarta.servlet.FilterChain; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; +import org.springframework.context.i18n.LocaleContextHolder; +import org.springframework.context.support.ResourceBundleMessageSource; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; + +class ApiAccessDeniedHandlerTest { + + private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules(); + private ApiAccessDeniedHandler handler; + + @BeforeEach + void setUp() { + ResourceBundleMessageSource messageSource = new ResourceBundleMessageSource(); + messageSource.setBasename("messages"); + messageSource.setDefaultEncoding("UTF-8"); + ApiResponseFactory responseFactory = new ApiResponseFactory( + messageSource, + Clock.fixed(Instant.parse("2026-07-28T00:00:00Z"), ZoneOffset.UTC) + ); + handler = new ApiAccessDeniedHandler( + objectMapper, + responseFactory, + new SensitiveLogSanitizer() + ); + MDC.put("requestId", "req-610"); + LocaleContextHolder.setLocale(Locale.ENGLISH); + } + + @AfterEach + void tearDown() { + MDC.clear(); + LocaleContextHolder.resetLocaleContext(); + SecurityContextHolder.clearContext(); + } + + @Test + void shouldExposeLocalizedApiTokenScopeReasonAndRequestId() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/publish"); + MockHttpServletResponse response = new MockHttpServletResponse(); + ApiTokenScopeService scopeService = + new ApiTokenScopeService(objectMapper, new RouteSecurityPolicyRegistry()); + ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); + PlatformPrincipal principal = new PlatformPrincipal( + "user-1", + "Alice", + "alice@example.com", + "", + "api_token", + Set.of("USER") + ); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("SCOPE_skill:read")) + ) + ); + FilterChain chain = (servletRequest, servletResponse) -> { + throw new AssertionError("Denied request must not continue"); + }; + + filter.doFilter(request, response, chain); + + JsonNode body = objectMapper.readTree(response.getContentAsByteArray()); + assertThat(response.getStatus()).isEqualTo(403); + assertThat(body.path("msg").asText()) + .isEqualTo("API token is missing required scope: skill:publish"); + assertThat(body.path("requestId").asText()).isEqualTo("req-610"); + } + + @Test + void shouldTranslateSafeApiTokenReason() throws Exception { + LocaleContextHolder.setLocale(Locale.SIMPLIFIED_CHINESE); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/whoami"); + MockHttpServletResponse response = new MockHttpServletResponse(); + ApiTokenScopeService scopeService = + new ApiTokenScopeService(objectMapper, new RouteSecurityPolicyRegistry()); + ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); + PlatformPrincipal principal = new PlatformPrincipal( + "user-1", + "Alice", + "alice@example.com", + "", + "api_token", + Set.of("USER") + ); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(principal, null, List.of()) + ); + + filter.doFilter(request, response, (servletRequest, servletResponse) -> { + throw new AssertionError("Denied request must not continue"); + }); + + JsonNode body = objectMapper.readTree(response.getContentAsByteArray()); + assertThat(body.path("msg").asText()) + .isEqualTo("API 令牌无法访问接口：/api/cli/v1/whoami"); + } + + @Test + void shouldHideGenericAccessDeniedExceptionMessage() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/admin"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + handler.handle(request, response, new AccessDeniedException("internal authorization detail")); + + JsonNode body = objectMapper.readTree(response.getContentAsByteArray()); + assertThat(body.path("msg").asText()).isEqualTo("Forbidden"); + assertThat(response.getContentAsString()).doesNotContain("internal authorization detail"); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAccessDeniedException.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAccessDeniedException.java new file mode 100644 index 00000000..62646a53 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenAccessDeniedException.java @@ -0,0 +1,42 @@ +package com.iflytek.skillhub.auth.token; + +import org.springframework.security.access.AccessDeniedException; + +/** + * Marks an API-token authorization failure whose structured reason is safe to expose to clients. + */ +public final class ApiTokenAccessDeniedException extends AccessDeniedException { + + private final String messageCode; + private final Object[] messageArgs; + + private ApiTokenAccessDeniedException(String logMessage, String messageCode, Object... messageArgs) { + super(logMessage); + this.messageCode = messageCode; + this.messageArgs = messageArgs.clone(); + } + + static ApiTokenAccessDeniedException missingScope(String requiredScope) { + return new ApiTokenAccessDeniedException( + "Missing API token scope: " + requiredScope, + "error.apiToken.scope.missing", + requiredScope + ); + } + + static ApiTokenAccessDeniedException unsupportedEndpoint(String path) { + return new ApiTokenAccessDeniedException( + "API token cannot access endpoint: " + path, + "error.apiToken.endpoint.unsupported", + path + ); + } + + public String getMessageCode() { + return messageCode; + } + + public Object[] getMessageArgs() { + return messageArgs.clone(); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java index 97145f5d..5182ce7f 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilter.java @@ -5,7 +5,6 @@ import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; @@ -59,11 +58,10 @@ public class ApiTokenScopeFilter extends OncePerRequestFilter { return; } - accessDeniedHandler.handle( - request, - response, - new AccessDeniedException(decision.message()) - ); + ApiTokenAccessDeniedException exception = decision.requiredScope() != null + ? ApiTokenAccessDeniedException.missingScope(decision.requiredScope()) + : ApiTokenAccessDeniedException.unsupportedEndpoint(request.getRequestURI()); + accessDeniedHandler.handle(request, response, exception); } @Override diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java index 788e0291..085016f4 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterTest.java @@ -17,8 +17,10 @@ import org.springframework.security.web.access.AccessDeniedHandler; import java.util.List; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -38,7 +40,9 @@ class ApiTokenScopeFilterTest { @Test void shouldDenyApiTokenWithoutRequiredScope() throws Exception { + AtomicReference deniedException = new AtomicReference<>(); AccessDeniedHandler handler = (request, response, accessDeniedException) -> { + deniedException.set(accessDeniedException); response.sendError(HttpServletResponse.SC_FORBIDDEN, accessDeniedException.getMessage()); }; ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); @@ -69,6 +73,12 @@ class ApiTokenScopeFilterTest { assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus()); assertTrue(response.getErrorMessage().contains("Missing API token scope: skill:publish")); + ApiTokenAccessDeniedException exception = assertInstanceOf( + ApiTokenAccessDeniedException.class, + deniedException.get() + ); + assertEquals("error.apiToken.scope.missing", exception.getMessageCode()); + assertEquals("skill:publish", exception.getMessageArgs()[0]); verify(chain, never()).doFilter(request, response); } From e4fb26d4ba7067201ae6d76ea3392a902e8d175b Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:03:50 +0800 Subject: [PATCH 24/25] fix(nginx): trust forwarded proto only when configured Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.draft | 3 + .env.release.example | 3 + .github/workflows/pr-scripts.yml | 3 + compose.release.yml | 1 + docs/09-deployment.md | 3 + scripts/tests/nginx-forwarded-proto-test.sh | 101 ++++++++++++++++++ scripts/tests/validate-release-config-test.sh | 6 ++ scripts/tests/workflow-security-test.sh | 6 ++ scripts/validate-release-config.sh | 1 + web/Dockerfile | 1 + web/nginx.conf.template | 14 ++- 11 files changed, 140 insertions(+), 2 deletions(-) create mode 100755 scripts/tests/nginx-forwarded-proto-test.sh diff --git a/.env.release.draft b/.env.release.draft index c8f0872b..417aab30 100644 --- a/.env.release.draft +++ b/.env.release.draft @@ -18,6 +18,9 @@ SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com # Usually keep empty when web and api are served from the same domain. SKILLHUB_WEB_API_BASE_URL= SKILLHUB_API_UPSTREAM=http://server:8080 +# Enable only when a trusted TLS-terminating proxy replaces X-Forwarded-Proto +# and the web container cannot be reached directly. +SKILLHUB_TRUST_FORWARDED_PROTO=false # Keep database and redis local-only on the host unless you explicitly need remote access. POSTGRES_BIND_ADDRESS=127.0.0.1 diff --git a/.env.release.example b/.env.release.example index 2d30c7bc..d038d6e1 100644 --- a/.env.release.example +++ b/.env.release.example @@ -15,6 +15,9 @@ SKILLHUB_PUBLIC_BASE_URL=http://localhost # Frontend usually keeps this empty and proxies to the backend through nginx. SKILLHUB_WEB_API_BASE_URL= SKILLHUB_API_UPSTREAM=http://server:8080 +# Keep false for direct exposure. Enable only behind a trusted proxy that replaces +# X-Forwarded-Proto and blocks direct access to the web container. +SKILLHUB_TRUST_FORWARDED_PROTO=false POSTGRES_BIND_ADDRESS=127.0.0.1 POSTGRES_PORT=5432 diff --git a/.github/workflows/pr-scripts.yml b/.github/workflows/pr-scripts.yml index e521eb9e..082ce102 100644 --- a/.github/workflows/pr-scripts.yml +++ b/.github/workflows/pr-scripts.yml @@ -8,6 +8,8 @@ on: - '.env.release.draft' - 'compose.release.yml' - 'Makefile' + - 'web/Dockerfile' + - 'web/nginx.conf.template' - '.github/workflows/pr-cli.yml' - '.github/workflows/pr-e2e.yml' - '.github/workflows/pr-tests.yml' @@ -33,5 +35,6 @@ jobs: - run: bash scripts/tests/publish-cli-test.sh - run: bash scripts/tests/runtime-secret-test.sh - run: bash scripts/tests/validate-release-config-test.sh + - run: bash scripts/tests/nginx-forwarded-proto-test.sh - run: bash scripts/tests/dev-web-host-test.sh - run: bash scripts/tests/workflow-security-test.sh diff --git a/compose.release.yml b/compose.release.yml index 5ed7086e..69c07496 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -116,6 +116,7 @@ services: - "${WEB_PORT:-80}:80" environment: SKILLHUB_API_UPSTREAM: ${SKILLHUB_API_UPSTREAM:-http://server:8080} + SKILLHUB_TRUST_FORWARDED_PROTO: ${SKILLHUB_TRUST_FORWARDED_PROTO:-false} SKILLHUB_WEB_API_BASE_URL: ${SKILLHUB_WEB_API_BASE_URL:-} SKILLHUB_PUBLIC_BASE_URL: ${SKILLHUB_PUBLIC_BASE_URL:-} SKILLHUB_WEB_AUTH_DIRECT_ENABLED: ${SKILLHUB_WEB_AUTH_DIRECT_ENABLED:-false} diff --git a/docs/09-deployment.md b/docs/09-deployment.md index fe631d80..48c5d9d0 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -194,6 +194,9 @@ docker compose --env-file .env.release -f compose.release.yml up -d - 推荐将敏感变量放入 CI/CD Secret 或主机上的受控 `.env.release` - 外部对象存储通过 `SKILLHUB_STORAGE_S3_*` 注入 - 前端反代和运行时 API 地址通过 `SKILLHUB_API_UPSTREAM` / `SKILLHUB_WEB_API_BASE_URL` 注入 +- `SKILLHUB_TRUST_FORWARDED_PROTO` 默认保持 `false`。只有 Web 容器仅能经由可信 + TLS 终止代理访问，且该代理会覆盖客户端传入的 `X-Forwarded-Proto` 时才设为 + `true`；否则客户端可伪造协议并影响 OAuth 回调、重定向和安全 Cookie 判断 - 如果要开放真实登录，再补充 `OAUTH2_GITHUB_CLIENT_ID` / `OAUTH2_GITHUB_CLIENT_SECRET` - 如果要启用密码重置验证码邮件，参见：`docs/19-smtp-password-reset-email-setup.md` diff --git a/scripts/tests/nginx-forwarded-proto-test.sh b/scripts/tests/nginx-forwarded-proto-test.sh new file mode 100755 index 00000000..01be85c3 --- /dev/null +++ b/scripts/tests/nginx-forwarded-proto-test.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +TEMPLATE="$REPO_ROOT/web/nginx.conf.template" +NGINX_IMAGE="${NGINX_TEST_IMAGE:-nginx:alpine}" +TEST_ID="skillhub-nginx-forwarded-proto-$$" +NETWORK="${TEST_ID}-network" +BACKEND="${TEST_ID}-backend" +DEFAULT_PROXY="${TEST_ID}-default" +TRUSTED_PROXY="${TEST_ID}-trusted" +TMP_DIR="$(mktemp -d)" +CONTAINERS=() + +cleanup() { + if ((${#CONTAINERS[@]} > 0)); then + docker rm -f "${CONTAINERS[@]}" >/dev/null 2>&1 || true + fi + docker network rm "$NETWORK" >/dev/null 2>&1 || true + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +wait_for_nginx() { + local container="$1" + local attempt + for attempt in {1..30}; do + if docker exec "$container" wget -qO- http://127.0.0.1/nginx-health >/dev/null 2>&1; then + return 0 + fi + sleep 0.2 + done + docker logs "$container" >&2 || true + fail "$container did not become healthy" +} + +start_proxy() { + local container="$1" + local trust_forwarded_proto="$2" + docker run --detach \ + --name "$container" \ + --network "$NETWORK" \ + --env "SKILLHUB_API_UPSTREAM=http://$BACKEND:8080" \ + --env "SKILLHUB_TRUST_FORWARDED_PROTO=$trust_forwarded_proto" \ + --volume "$TEMPLATE:/etc/nginx/templates/default.conf.template:ro" \ + "$NGINX_IMAGE" >/dev/null + CONTAINERS+=("$container") + wait_for_nginx "$container" +} + +assert_proto() { + local container="$1" + local expected="$2" + local header="${3:-}" + local path="${4:-/api/proto}" + local actual + if [[ -n "$header" ]]; then + actual="$(docker exec "$container" wget -qO- \ + --header="X-Forwarded-Proto: $header" \ + "http://127.0.0.1$path")" + else + actual="$(docker exec "$container" wget -qO- "http://127.0.0.1$path")" + fi + [[ "$actual" == "$expected" ]] \ + || fail "$container forwarded proto '$actual', expected '$expected' for $path with header '${header:-}'" +} + +cat >"$TMP_DIR/backend.conf" <<'EOF' +server { + listen 8080; + location / { + default_type text/plain; + return 200 $http_x_forwarded_proto; + } +} +EOF + +docker network create "$NETWORK" >/dev/null +docker run --detach \ + --name "$BACKEND" \ + --network "$NETWORK" \ + --volume "$TMP_DIR/backend.conf:/etc/nginx/conf.d/default.conf:ro" \ + "$NGINX_IMAGE" >/dev/null +CONTAINERS+=("$BACKEND") + +start_proxy "$DEFAULT_PROXY" false +start_proxy "$TRUSTED_PROXY" true + +for path in /api/proto /oauth2/proto /login/oauth2/proto /.well-known/proto; do + assert_proto "$DEFAULT_PROXY" http https "$path" + assert_proto "$TRUSTED_PROXY" https https "$path" +done +assert_proto "$TRUSTED_PROXY" http +assert_proto "$TRUSTED_PROXY" http "https,http" + +echo "nginx-forwarded-proto-test passed" diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index fd76ddfc..d94ed62c 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -36,6 +36,7 @@ POSTGRES_USER=skillhub POSTGRES_PASSWORD=strong-postgres-password SESSION_COOKIE_SECURE=true BOOTSTRAP_ADMIN_ENABLED=false +SKILLHUB_TRUST_FORWARDED_PROTO=false SKILLHUB_STORAGE_PROVIDER=s3 SKILLHUB_STORAGE_S3_ENDPOINT=https://storage.example.com SKILLHUB_STORAGE_S3_BUCKET=skillhub @@ -80,6 +81,11 @@ short_env="$tmp/short.env" write_env "$short_env" "too-short" expect_fail "$short_env" "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET must be at least 32 characters" +invalid_forwarded_proto_env="$tmp/invalid-forwarded-proto.env" +write_env "$invalid_forwarded_proto_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' "SKILLHUB_TRUST_FORWARDED_PROTO=yes" >>"$invalid_forwarded_proto_env" +expect_fail "$invalid_forwarded_proto_env" "SKILLHUB_TRUST_FORWARDED_PROTO must be true or false" + draft_env="$tmp/draft.env" while IFS= read -r line || [[ -n "$line" ]]; do case "$line" in diff --git a/scripts/tests/workflow-security-test.sh b/scripts/tests/workflow-security-test.sh index 9a1688ef..ec1f70ce 100755 --- a/scripts/tests/workflow-security-test.sh +++ b/scripts/tests/workflow-security-test.sh @@ -64,8 +64,14 @@ grep -Fq '.env.release.draft' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run when release env draft changes" grep -Fq 'compose.release.yml' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run when release compose changes" +grep -Fq 'web/Dockerfile' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run when the web image changes" +grep -Fq 'web/nginx.conf.template' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run when the nginx template changes" grep -Fq 'bash scripts/tests/validate-release-config-test.sh' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run validate-release-config-test" +grep -Fq 'bash scripts/tests/nginx-forwarded-proto-test.sh' "$PR_SCRIPTS_WORKFLOW" \ + || fail "pr-scripts must run nginx-forwarded-proto-test" grep -Fq 'bash scripts/tests/runtime-secret-test.sh' "$PR_SCRIPTS_WORKFLOW" \ || fail "pr-scripts must run runtime-secret-test" grep -Fq 'bash scripts/tests/dev-web-host-test.sh' "$PR_SCRIPTS_WORKFLOW" \ diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index 03e42bd8..27e9d042 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -151,6 +151,7 @@ reject_patterns SPRING_MAIL_PASSWORD "TODO_*" "todo_*" "replace*" validate_boolean SESSION_COOKIE_SECURE validate_boolean BOOTSTRAP_ADMIN_ENABLED +validate_boolean SKILLHUB_TRUST_FORWARDED_PROTO validate_boolean SKILLHUB_STORAGE_S3_FORCE_PATH_STYLE validate_boolean SKILLHUB_STORAGE_S3_AUTO_CREATE_BUCKET diff --git a/web/Dockerfile b/web/Dockerfile index e301f7c8..2ed67ae0 100644 --- a/web/Dockerfile +++ b/web/Dockerfile @@ -7,6 +7,7 @@ COPY . . RUN pnpm build FROM nginx:alpine +ENV SKILLHUB_TRUST_FORWARDED_PROTO=false COPY --from=build /app/dist /usr/share/nginx/html COPY --from=build /app/src/docs/skill.md.template /usr/share/nginx/html/registry/skill.md.template COPY nginx.conf.template /etc/nginx/templates/default.conf.template diff --git a/web/nginx.conf.template b/web/nginx.conf.template index be2a51a2..25db2869 100644 --- a/web/nginx.conf.template +++ b/web/nginx.conf.template @@ -10,9 +10,15 @@ server { gzip_types text/plain text/css application/json application/javascript text/xml; gzip_min_length 1000; + # Ignore client-supplied forwarded proto by default. Operators may explicitly trust a + # sanitizing upstream proxy; only canonical http/https values are then accepted. set $proxy_x_forwarded_proto $scheme; - if ($http_x_forwarded_proto) { - set $proxy_x_forwarded_proto $http_x_forwarded_proto; + set $forwarded_proto_source "${SKILLHUB_TRUST_FORWARDED_PROTO}:$http_x_forwarded_proto"; + if ($forwarded_proto_source ~* "^true:https$") { + set $proxy_x_forwarded_proto https; + } + if ($forwarded_proto_source ~* "^true:http$") { + set $proxy_x_forwarded_proto http; } location / { @@ -31,6 +37,7 @@ server { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } @@ -38,12 +45,15 @@ server { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } location /.well-known/ { proxy_pass ${SKILLHUB_API_UPSTREAM}; proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; } From 155ab8f6d5091dc406a4bba504e46c7a87d75095 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:16:45 +0800 Subject: [PATCH 25/25] fix(auth): hide placeholder OAuth providers Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skillhub/service/AuthMethodCatalog.java | 7 ++-- .../controller/AuthControllerTest.java | 16 ++++------ .../service/AuthMethodCatalogTest.java | 32 +++++++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java index cc927801..8c63cb9d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMethodCatalog.java @@ -12,6 +12,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Locale; import org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientProperties; import org.springframework.stereotype.Service; @@ -56,16 +57,14 @@ public class AuthMethodCatalog { } /** - * Check if an OAuth provider has valid configuration (non-empty client-id that is not a placeholder). + * Checks whether an OAuth provider has a non-empty, non-placeholder client ID. */ private boolean isValidOAuthProvider(OAuth2ClientProperties.Registration registration) { String clientId = registration.getClientId(); if (clientId == null || clientId.isBlank()) { return false; } - // Filter out placeholder values used in dev/test configs - String lowerClientId = clientId.toLowerCase(); - return !lowerClientId.contains("placeholder") && !lowerClientId.contains("local-placeholder"); + return !clientId.toLowerCase(Locale.ROOT).contains("placeholder"); } public List listMethods(String returnTo) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java index d79d368a..8e4118c6 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java @@ -150,13 +150,9 @@ class AuthControllerTest { mockMvc.perform(get("/api/v1/auth/providers")) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.length()").value(3)) - .andExpect(jsonPath("$.data[*].id", hasItems("github", "gitee", "gitlab"))) - .andExpect(jsonPath("$.data[*].authorizationUrl", hasItems( - "/oauth2/authorization/github", - "/oauth2/authorization/gitee", - "/oauth2/authorization/gitlab" - ))) + .andExpect(jsonPath("$.data.length()").value(1)) + .andExpect(jsonPath("$.data[*].id", hasItems("github"))) + .andExpect(jsonPath("$.data[*].authorizationUrl", hasItems("/oauth2/authorization/github"))) .andExpect(jsonPath("$.timestamp").isNotEmpty()) .andExpect(jsonPath("$.requestId").isNotEmpty()); } @@ -167,8 +163,7 @@ class AuthControllerTest { .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data[*].authorizationUrl", hasItems( - "/oauth2/authorization/github?returnTo=%2Fdashboard%2Fpublish", - "/oauth2/authorization/gitee?returnTo=%2Fdashboard%2Fpublish" + "/oauth2/authorization/github?returnTo=%2Fdashboard%2Fpublish" ))); } @@ -177,7 +172,8 @@ class AuthControllerTest { mockMvc.perform(get("/api/v1/auth/methods").param("returnTo", "/dashboard/publish")) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data[*].id", hasItems("local-password", "oauth-github", "oauth-gitee"))) + .andExpect(jsonPath("$.data.length()").value(2)) + .andExpect(jsonPath("$.data[*].id", hasItems("local-password", "oauth-github"))) .andExpect(jsonPath("$.data[?(@.id=='local-password')].methodType").value(hasItems("PASSWORD"))) .andExpect(jsonPath("$.data[?(@.id=='oauth-github')].actionUrl") .value(hasItems("/oauth2/authorization/github?returnTo=%2Fdashboard%2Fpublish"))); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AuthMethodCatalogTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AuthMethodCatalogTest.java index 35ca8d75..e9ef372f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AuthMethodCatalogTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AuthMethodCatalogTest.java @@ -16,6 +16,31 @@ import org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2Clien class AuthMethodCatalogTest { + @Test + void catalogsShouldHideEmptyAndPlaceholderOAuthProviders() { + OAuth2ClientProperties oauthProperties = new OAuth2ClientProperties(); + oauthProperties.getRegistration().put("valid", registration("production-client", "Valid")); + oauthProperties.getRegistration().put("missing", registration(null, "Missing")); + oauthProperties.getRegistration().put("blank", registration(" ", "Blank")); + oauthProperties.getRegistration().put("placeholder", registration("PLACEHOLDER", "Placeholder")); + oauthProperties.getRegistration().put("local", registration("local-placeholder", "Local")); + + AuthMethodCatalog catalog = new AuthMethodCatalog( + oauthProperties, + new DirectAuthProperties(), + new AuthSessionBootstrapProperties(), + List.of(), + List.of() + ); + + assertThat(catalog.listOAuthProviders(null)) + .extracting(provider -> provider.id()) + .containsExactly("valid"); + assertThat(catalog.listMethods(null)) + .extracting(method -> method.id()) + .containsExactly("local-password", "oauth-valid"); + } + @Test void listMethodsShouldUseProviderDisplayNamesForCompatibleAuthMethods() { OAuth2ClientProperties oauthProperties = new OAuth2ClientProperties(); @@ -122,4 +147,11 @@ class AuthMethodCatalogTest { "bootstrap-private-sso:private-sso" ); } + + private static OAuth2ClientProperties.Registration registration(String clientId, String clientName) { + OAuth2ClientProperties.Registration registration = new OAuth2ClientProperties.Registration(); + registration.setClientId(clientId); + registration.setClientName(clientName); + return registration; + } }