From 532d0450aa894d9d1b6e7c6cae8b2cf681bffb37 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 13 Apr 2026 11:39:45 +0800 Subject: [PATCH 1/4] feat(skill): add UPLOADED status for PRIVATE skill lifecycle (#290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(skill): add UPLOADED status for PRIVATE skill lifecycle ## Summary - Add UPLOADED status for PRIVATE skills after security scan passes - PRIVATE skill owners can test before confirming publish or submitting for review - Rerelease now follows visibility rules (PRIVATE→UPLOADED, PUBLIC→PENDING_REVIEW) - Auto-withdraw changes status to UPLOADED (not DRAFT) to keep versions visible ## Changes - SkillVersionStatus: Add UPLOADED enum value - SkillPublishService: PRIVATE skills go to UPLOADED after scan - SecurityScanService: Visibility-based status transition after scan - SkillGovernanceService: Withdraw→UPLOADED, delete allows UPLOADED - SkillQueryService: Include UPLOADED in version list filters - SkillReviewSubmitService: New service for submit-review and confirm-publish - SkillLifecycleController: Add submit-review and confirm-publish endpoints - Frontend: Add buttons, dialogs, and hooks for new operations ## Workflow - PRIVATE: Publish → SCANNING → UPLOADED → confirm-publish → PUBLISHED - PUBLIC: Publish → SCANNING → PENDING_REVIEW → PUBLISHED * feat(review): add backward compatibility for DRAFT status Support both DRAFT (legacy) and UPLOADED (new flow) status in: - SkillReviewSubmitService.submitForReview - SkillReviewSubmitService.confirmPublish - ReviewService.submitReview (both overloads) This ensures existing data with DRAFT status continues to work with the new visibility-based workflow introduced in OSS-02. --- README.md | 5 +- README_zh.md | 4 +- docs/oss-01-core-contract-freeze.md | 460 ++++++++++++ docs/oss-02-core-semantic-rules.md | 663 ++++++++++++++++++ .../portal/SkillLifecycleController.java | 37 + .../skillhub/dto/ConfirmPublishRequest.java | 11 + .../skillhub/dto/SubmitReviewRequest.java | 16 + .../service/GovernanceWorkflowAppService.java | 32 + .../service/SkillLifecycleAppService.java | 72 ++ .../src/main/resources/messages.properties | 5 + .../src/main/resources/messages_zh.properties | 5 + .../service/SkillLifecycleAppServiceTest.java | 3 + .../skillhub/domain/review/ReviewService.java | 8 +- .../domain/security/SecurityScanService.java | 8 +- .../domain/skill/SkillVersionStatus.java | 1 + .../skill/service/SkillDownloadService.java | 29 +- .../skill/service/SkillGovernanceService.java | 5 +- .../skill/service/SkillPublishService.java | 36 +- .../skill/service/SkillQueryService.java | 11 +- .../service/SkillReviewSubmitService.java | 159 +++++ .../service/SkillGovernanceServiceTest.java | 4 +- .../service/SkillPublishServiceTest.java | 86 ++- .../service/SkillReviewSubmitServiceTest.java | 272 +++++++ web/src/api/client.ts | 30 + web/src/i18n/locales/en.json | 13 + web/src/i18n/locales/zh.json | 14 + web/src/pages/dashboard/my-skills.tsx | 6 + web/src/pages/skill-detail.test.tsx | 2 + web/src/pages/skill-detail.tsx | 87 ++- web/src/shared/hooks/use-skill-queries.ts | 38 + 30 files changed, 2083 insertions(+), 39 deletions(-) create mode 100644 docs/oss-01-core-contract-freeze.md create mode 100644 docs/oss-02-core-semantic-rules.md create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ConfirmPublishRequest.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitReviewRequest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java create mode 100644 server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java diff --git a/README.md b/README.md index cf803028..baa7d8e0 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ The `--public-url` parameter sets the public access URL for your SkillHub instan **For users in China (Aliyun mirror):** ```bash -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest ``` If deployment runs into problems, clear the existing runtime home and retry. @@ -195,7 +195,7 @@ Published images target both `linux/amd64` and `linux/arm64`. curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --public-url https://skillhub.your-company.com # Aliyun mirror (recommended for users in China) -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest ``` **Deployment parameters:** @@ -222,6 +222,7 @@ cp .env.release.example .env.release Recommended image tags: +- `SKILLHUB_VERSION=latest` for the latest stable release (default) - `SKILLHUB_VERSION=edge` for the latest `main` build - `SKILLHUB_VERSION=vX.Y.Z` for a fixed release diff --git a/README_zh.md b/README_zh.md index edb58fc8..8a7c7409 100644 --- a/README_zh.md +++ b/README_zh.md @@ -67,7 +67,7 @@ curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- u **国内用户(阿里云镜像):** ```bash -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest ``` 如果部署遇到问题,请清除现有的运行时目录并重试。 @@ -177,7 +177,7 @@ skillhub/ curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --public-url https://skillhub.your-company.com # 阿里云镜像(国内推荐) -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest ``` ### 配置参数说明 diff --git a/docs/oss-01-core-contract-freeze.md b/docs/oss-01-core-contract-freeze.md new file mode 100644 index 00000000..617f705c --- /dev/null +++ b/docs/oss-01-core-contract-freeze.md @@ -0,0 +1,460 @@ +# OSS-01 Core 契约审计与冻结 + +## 1. 审计结论 + +SkillHub 开源项目已具备 AstronClaw 主链路所需的绝大部分 Core 能力。现有接口覆盖了 skill 唯一标识查询、版本元数据查询、创建(发布)和删除。**无需在开源 Core 中新增 AstronClaw 专属接口**;对 AstronClaw 而言,查询类和主链路类能力都应统一由 SaaS 层 `AstronClaw Adapter` 封装后对外提供,而不是直接绑定开源 Core 的接口形态。 + +--- + +## 2. Core 接口清单 + +以下接口构成 Core 基线能力,供 SaaS 层统一封装后对 AstronClaw 提供;这些接口本身不应被视为 AstronClaw 的长期直接契约。 + +### 2.1 skill 唯一标识与详情查询 + +| 接口 | 路径 | 说明 | +|------|------|------| +| skill 详情 | `GET /api/v1/skills/{namespace}/{slug}` | 返回 `SkillDetailResponse`,包含完整 identity 和状态 | +| 版本解析 | `GET /api/v1/skills/{namespace}/{slug}/resolve?version=&tag=&hash=` | 返回 `ResolveVersionResponse`,解析人类可读版本选择器到精确版本 | + +### 2.2 指定版本安装元数据查询 + +| 接口 | 路径 | 说明 | +|------|------|------| +| 版本详情 | `GET /api/v1/skills/{namespace}/{slug}/versions/{version}` | 返回 `SkillVersionDetailResponse`,含 metadata 和 manifest | +| 版本文件列表 | `GET /api/v1/skills/{namespace}/{slug}/versions/{version}/files` | 返回 `List` | +| 版本下载 | `GET /api/v1/skills/{namespace}/{slug}/versions/{version}/download` | 下载指定版本 bundle | +| 版本列表 | `GET /api/v1/skills/{namespace}/{slug}/versions?page=&size=` | 分页返回版本列表 | + +### 2.3 创建(发布)个人 skill + +| 接口 | 路径 | 说明 | +|------|------|------| +| 发布 skill | `POST /api/v1/skills/{namespace}/publish` | 上传包并发布,返回 `PublishResponse` | + +### 2.4 删除个人 skill + +| 接口 | 路径 | 说明 | +|------|------|------| +| 硬删除(by ID) | `DELETE /api/v1/skills/id/{skillId}` | 需 SUPER_ADMIN 权限 | +| 硬删除(by 坐标) | `DELETE /api/v1/skills/{namespace}/{slug}` | 需 SUPER_ADMIN 权限 | +| 归档 | `POST /api/v1/skills/{namespace}/{slug}/archive` | owner 或 namespace admin 可操作 | +| 取消归档 | `POST /api/v1/skills/{namespace}/{slug}/unarchive` | 恢复为 ACTIVE | + +### 2.5 版本生命周期 + +| 接口 | 路径 | 说明 | +|------|------|------| +| 删除版本 | `DELETE /api/v1/skills/{namespace}/{slug}/versions/{version}` | 仅 DRAFT/REJECTED/SCAN_FAILED 可删 | +| 撤回审核 | `POST /api/v1/skills/{namespace}/{slug}/versions/{version}/withdraw-review` | PENDING_REVIEW → DRAFT | +| 重新发布 | `POST /api/v1/skills/{namespace}/{slug}/versions/{version}/rerelease` | 重新发布版本 | + +### 2.6 ClawHub 兼容接口(已有) + +| 接口 | 路径 | 说明 | +|------|------|------| +| 解析 skill | `GET /api/v1/resolve?slug=&version=` | ClawHub 协议兼容 | +| 解析 skill(路径) | `GET /api/v1/resolve/{canonicalSlug}?version=` | ClawHub 协议兼容 | +| 下载 | `GET /api/v1/download/{canonicalSlug}?version=` | 302 重定向到下载地址 | +| 删除 skill | `DELETE /api/v1/skills/{canonicalSlug}` | owner 可操作 | +| 取消删除 | `POST /api/v1/skills/{canonicalSlug}/undelete` | owner 可操作 | +| 发布 skill | `POST /api/v1/skills` | ClawHub 协议兼容 | +| 发布到 namespace | `POST /api/v1/publish` | ClawHub 协议兼容 | + +--- + +## 3. 字段语义冻结表 + +### 3.1 Skill Identity 字段 + +| 字段 | 类型 | 含义 | 稳定性 | 说明 | +|------|------|------|--------|------| +| `skill.id` | Long | skill 全局唯一主键 | 不可变 | 自增,创建后永不改变,可作为外部映射主键 | +| `namespace` (slug) | String(64) | skill 所属命名空间标识 | 不可变 | 全局唯一,创建后不可改名 | +| `skill.slug` | String(100) | skill 在 namespace 内的唯一标识 | 不可变 | 创建后不可改名,`namespace + slug` 构成业务坐标 | +| `skill.displayName` | String(200) | skill 展示名称 | 可变 | 仅用于展示,不可作为映射依据 | +| `skill.ownerId` | String | skill 创建者 ID | 不可变 | 创建时绑定,不可转移 | +| `skill.summary` | String(TEXT) | skill 简介 | 可变 | 展示用 | +| `skill.visibility` | Enum | 可见性 | 可变 | `PUBLIC` / `NAMESPACE_ONLY` / `PRIVATE` | +| `skill.status` | Enum | skill 状态 | 可变 | `ACTIVE` / `HIDDEN` / `ARCHIVED` | +| `skill.hidden` | boolean | 是否被管理员隐藏 | 可变 | 与 status 独立的隐藏标记 | +| `skill.latestVersionId` | Long | 最新版本指针 | 可变 | 指向当前最新已发布版本,yank/删除后自动回退 | +| `skill.downloadCount` | Long | 下载次数 | 可变 | 累计值 | +| `skill.starCount` | Integer | 收藏数 | 可变 | 累计值 | + +### 3.2 SkillVersion 字段 + +| 字段 | 类型 | 含义 | 稳定性 | 说明 | +|------|------|------|--------|------| +| `version.id` | Long | 版本全局唯一主键 | 不可变 | 自增 | +| `version.skillId` | Long | 所属 skill ID | 不可变 | 外键 | +| `version.version` | String(64) | 版本号 | 不可变 | 如 `1.0.0`,创建后不可改 | +| `version.status` | Enum | 版本状态 | 可变 | 见状态语义表 | +| `version.bundleReady` | boolean | bundle 是否可用 | 可变 | `true` 表示 bundle 已构建完成,可下载安装 | +| `version.downloadReady` | boolean | 是否允许下载 | 可变 | yank 后设为 `false` | +| `version.publishedAt` | Instant | 发布时间 | 一次写入 | 首次发布时设置 | +| `version.parsedMetadataJson` | JSONB | 解析后的元数据 | 一次写入 | 包含 `package_name` 等运行时信息 | +| `version.manifestJson` | JSONB | manifest 原始内容 | 一次写入 | skill 包的 manifest | +| `version.changelog` | String(TEXT) | 变更日志 | 可变 | 展示用 | +| `version.fileCount` | Integer | 文件数量 | 一次写入 | 发布时确定 | +| `version.totalSize` | Long | 总大小(字节) | 一次写入 | 发布时确定 | +| `version.yankedAt` | Instant | yank 时间 | 一次写入 | yank 时设置 | +| `version.yankReason` | String(TEXT) | yank 原因 | 一次写入 | yank 时设置 | + +### 3.3 关键字段含义冻结 + +| 字段 | 冻结定义 | +|------|----------| +| `skill_id` | `skill.id`,Long 类型自增主键,全局唯一,创建后不可变。AstronClaw 应以此作为 `external_skill_mapping` 的外部主键 | +| `namespace` | `namespace.slug`,String(64),全局唯一,不可改名。与 `slug` 组合构成业务坐标 | +| `slug` | `skill.slug`,String(100),namespace 内唯一,不可改名。`namespace/slug` 是人类可读的稳定坐标 | +| `version` | `skill_version.version`,String(64),同一 skill 内唯一,不可改。如 `1.0.0` | +| `bundle_url` | 通过 `GET /{namespace}/{slug}/versions/{version}/download` 获取,或通过 `resolve` 接口的 `downloadUrl` 字段获取。不是数据库字段,而是动态生成的下载地址 | +| `bundle_ready` | `skill_version.bundleReady`,boolean。`true` 表示 bundle 已构建完成可安装。AstronClaw 安装前必须校验此字段 | +| `package_name` | 存储在 `skill_version.parsedMetadataJson` 中,从 skill 包的 manifest 解析而来。同一 skill 跨版本应保持稳定。AstronClaw 用于运行时安装/卸载标识 | + +### 3.4 Namespace 字段 + +| 字段 | 类型 | 含义 | 稳定性 | +|------|------|------|--------| +| `namespace.id` | Long | 命名空间主键 | 不可变 | +| `namespace.slug` | String(64) | 命名空间标识 | 不可变,全局唯一 | +| `namespace.displayName` | String(128) | 展示名称 | 可变 | +| `namespace.type` | Enum | 类型 | 不可变,`GLOBAL` / `TEAM` | +| `namespace.status` | Enum | 状态 | 可变,`ACTIVE` / `FROZEN` / `ARCHIVED` | + +--- + +## 4. 状态语义冻结表 + +### 4.1 Skill 状态(`SkillStatus`) + +| 状态 | 市场可见 | 可新装 | 已装是否保留 | 可被 owner 操作 | 说明 | +|------|----------|--------|------------|----------------|------| +| `ACTIVE` | 是(受 visibility 控制) | 是(需有 PUBLISHED 版本) | 是 | 是 | 正常状态 | +| `HIDDEN` | 否 | 否 | 是 | 受限 | 管理员隐藏,独立于 status 的 `hidden` 标记 | +| `ARCHIVED` | 否 | 否 | 是 | 可取消归档 | owner 或 namespace admin 归档 | + +### 4.2 版本状态(`SkillVersionStatus`) + +| 状态 | 是否允许安装 | 是否允许下载 | 市场可见 | 可转换到 | 说明 | +|------|------------|------------|---------|---------|------| +| `DRAFT` | 否 | 否 | 否 | SCANNING, 可删除 | 初始状态,编辑中 | +| `SCANNING` | 否 | 否 | 否 | SCAN_FAILED, PENDING_REVIEW, PUBLISHED | 安全扫描中 | +| `SCAN_FAILED` | 否 | 否 | 否 | 可删除 | 安全扫描失败 | +| `PENDING_REVIEW` | 否 | 否 | 否 | PUBLISHED, REJECTED, → DRAFT(撤回) | 等待审核 | +| `PUBLISHED` | 是 | 是 | 是 | YANKED | 已发布,可安装 | +| `REJECTED` | 否 | 否 | 否 | 可删除 | 审核拒绝 | +| `YANKED` | 否 | 否 | 否(或弱可见) | 不可逆 | 已撤回,已装不受影响 | + +### 4.3 可见性(`SkillVisibility`) + +| 可见性 | 市场列表可见 | 谁可查看 | 谁可安装 | +|--------|------------|---------|---------| +| `PUBLIC` | 是 | 所有人 | 所有人(需 PUBLISHED + bundleReady) | +| `NAMESPACE_ONLY` | 否 | namespace 成员 | namespace 成员 | +| `PRIVATE` | 否 | 仅 owner | 仅 owner | + +### 4.4 删除语义 + +| 操作 | 类型 | 可逆 | 数据影响 | 已装实例影响 | +|------|------|------|---------|------------| +| 硬删除 skill | 永久删除 | 否 | 删除所有记录、文件、存储对象,slug 可复用 | 不影响,AstronClaw 已装快照独立 | +| 归档 skill | 状态变更 | 是 | 无数据删除,status → ARCHIVED | 不影响 | +| 隐藏 skill | 标记变更 | 是 | 无数据删除,hidden → true | 不影响 | +| 删除版本 | 永久删除 | 否 | 仅删除 DRAFT/REJECTED/SCAN_FAILED 版本 | 不影响(这些版本未被安装) | +| Yank 版本 | 状态变更 | 否 | status → YANKED,downloadReady → false | 不影响已装实例 | + +### 4.5 AstronClaw 安装判断规则 + +AstronClaw 判断一个 skill 版本是否可安装,需同时满足: + +``` +skill.status == ACTIVE + AND skill.hidden == false + AND skill.visibility 允许当前用户访问 + AND version.status == PUBLISHED + AND version.bundleReady == true +``` + +已安装实例不受后续状态变更影响。即使 skill 被删除/归档/隐藏,或版本被 yank,AstronClaw 本地安装快照仍可正常使用和卸载。 + +## 5. 错误语义表 + +### 5.1 统一响应结构 + +```json +{ + "code": 0, + "msg": "操作成功", + "data": { ... }, + "timestamp": "2026-04-10T08:00:00Z", + "requestId": "req-xxx" +} +``` + +- `code = 0` 表示成功 +- `code > 0` 表示错误,值为 HTTP 状态码 + +### 5.2 错误码映射 + +| HTTP 状态码 | 场景 | 异常类型 | 说明 | +|------------|------|---------|------| +| 400 | 参数非法 | `BadRequestException` / `DomainBadRequestException` | 请求参数校验失败 | +| 401 | 未认证 | `UnauthorizedException` / `AuthFlowException` | 未登录或 token 过期 | +| 403 | 无权限 | `ForbiddenException` / `DomainForbiddenException` | 无操作权限 | +| 404 | 未找到 | `DomainNotFoundException` | skill/version/namespace 不存在 | +| 408 | 请求超时 | `AsyncRequestTimeoutException` | 异步请求超时 | +| 503 | 存储不可用 | `StorageAccessException` | 对象存储访问失败 | +| 500 | 服务异常 | `Exception` | 未预期的内部错误 | + +### 5.3 Core 主链路关键错误场景 + +| 场景 | HTTP 状态码 | msg 示例 | AstronClaw 处理建议 | +|------|-----------|---------|-------------------| +| skill 不存在 | 404 | `error.skill.notFound` | 映射失败,提示用户 | +| 版本不存在 | 404 | `error.skill.notFound` | 安装/升级失败,提示用户 | +| 版本不可安装(非 PUBLISHED) | 400 | `error.badRequest` | 拒绝安装,提示版本状态 | +| bundle 未就绪 | 400 | `error.badRequest` | 拒绝安装,提示稍后重试 | +| 无权访问(PRIVATE skill) | 403 | `error.forbidden` | 提示无权限 | +| namespace 不存在 | 404 | `error.namespace.notFound` | 映射失败 | +| 存储服务不可用 | 503 | `error.storage.unavailable` | 降级处理,已装 skill 不受影响 | +| 删除不允许(非 owner) | 403 | `error.forbidden` | 提示无权限 | + +--- + +## 6. Core vs SaaS Adapter 能力分界 + +### 6.1 Core 已满足的能力 + +说明: + +下表表示“开源 Core 已具备、可供 SaaS 封装”的能力,并不表示 AstronClaw 应直接调用这些开源接口。 + +| PRD 需求 | Core 接口 | 满足程度 | 备注 | +|---------|----------|---------|------| +| skill 唯一标识查询 | `GET /{namespace}/{slug}` | 完全满足 | 返回 `id`、`namespace`、`slug` | +| 指定版本安装元数据 | `GET /{namespace}/{slug}/versions/{version}` | 基本满足 | 返回 status、metadata;`package_name` 在 `parsedMetadataJson` 中 | +| 版本解析 | `GET /{namespace}/{slug}/resolve` | 完全满足 | 支持 version/tag/hash 解析 | +| bundle 下载 | `GET /{namespace}/{slug}/versions/{version}/download` | 完全满足 | 直接下载 | +| 创建(发布)个人 skill | `POST /{namespace}/publish` | 完全满足 | 返回 skillId、namespace、slug、version、status | +| 删除个人 skill | `DELETE /{namespace}/{slug}` (ClawHub 兼容) | 完全满足 | owner 可操作 | +| 归档 skill | `POST /{namespace}/{slug}/archive` | 完全满足 | 可逆操作 | +| 版本状态查询 | `GET /{namespace}/{slug}` 中的 headlineVersion/publishedVersion | 完全满足 | 包含版本状态 | +| labels 数据 | `GET /{namespace}/{slug}` 中的 labels 字段 | 完全满足 | 返回 `List` | + +### 6.2 需要 SaaS Adapter 新增的能力 + +| PRD 需求 | 原因 | Adapter 建议 | +|---------|------|-------------| +| 市场列表查询(搜索/过滤/排序) | Core 不提供面向页面的聚合列表 | `GET /api/v1/astronclaw/adapter/skills/market` | +| 市场详情(AstronClaw DTO) | Core 返回的 DTO 包含 Core 内部字段,需适配 | `GET /api/v1/astronclaw/adapter/skills/{id}` | +| owner 维度"我创建的"查询 | Core 的 `/me/skills` 返回 Core DTO,需适配 | `GET /api/v1/astronclaw/adapter/skills/mine` | +| `is_installed` 补全 | 安装关系在 AstronClaw 侧 | AstronClaw 本地补全,不在 Adapter | +| `package_name` 顶层字段 | 当前在 `parsedMetadataJson` 内,需提取 | Adapter 解析 JSON 后平铺返回 | +| `bundle_url` 直接返回 | 当前需通过 download 接口获取 | Adapter 可直接返回预签名 URL | +| 统一 `can_install` 判断 | 需组合 status + visibility + bundleReady | Adapter 计算后返回布尔值 | +| 统一 `can_delete` 判断 | 需组合 owner + status | Adapter 计算后返回布尔值 | + +### 6.3 分界原则 + +``` +Core 负责:skill 生命周期真相(identity、version、status、artifact) +Adapter 负责:面向 AstronClaw 的 DTO 适配(字段平铺、状态聚合、权限预判断) +``` + +补充原则: + +1. 即使开源 `Core` 已经具备某项主链路能力,`AstronClaw` 仍应统一通过 SaaS Adapter 消费。 +2. 该原则同时适用于唯一标识查询、版本元数据、创建个人 skill、删除个人 skill。 +3. 开源文档中的接口清单用于说明 `Core` 能力边界,不应被解读为 AstronClaw 的直接对接建议。 + +--- + +## 7. 成功 / 失败 / 边界样例 + +### 7.1 查询 skill identity — 成功 + +``` +GET /api/v1/skills/my-namespace/my-skill +``` + +```json +{ + "code": 0, + "data": { + "id": 42, + "slug": "my-skill", + "displayName": "My Skill", + "ownerId": "user-123", + "status": "ACTIVE", + "visibility": "PUBLIC", + "namespace": "my-namespace", + "labels": [{"slug": "nlp", "type": "CATEGORY", "displayName": "NLP"}], + "headlineVersion": {"id": 100, "version": "1.2.0", "status": "PUBLISHED"}, + "publishedVersion": {"id": 100, "version": "1.2.0", "status": "PUBLISHED"} + } +} +``` + +AstronClaw 映射关键字段:`id=42`,`namespace=my-namespace`,`slug=my-skill`。 + +### 7.2 查询 skill identity — 不存在 + +``` +GET /api/v1/skills/my-namespace/nonexistent +``` + +```json +{ + "code": 404, + "msg": "Skill not found", + "data": null +} +``` + +### 7.3 查询指定版本元数据 — 成功 + +``` +GET /api/v1/skills/my-namespace/my-skill/versions/1.2.0 +``` + +```json +{ + "code": 0, + "data": { + "id": 100, + "version": "1.2.0", + "status": "PUBLISHED", + "changelog": "Bug fixes", + "fileCount": 3, + "totalSize": 102400, + "publishedAt": "2026-04-01T10:00:00Z", + "parsedMetadataJson": "{\"name\":\"my-skill\",\"package_name\":\"my_namespace__my_skill\",\"version\":\"1.2.0\"}", + "manifestJson": "{...}" + } +} +``` + +`package_name` 从 `parsedMetadataJson` 中提取。 + +### 7.4 查询已 YANKED 版本 + +``` +GET /api/v1/skills/my-namespace/my-skill/versions/1.0.0 +``` + +```json +{ + "code": 0, + "data": { + "id": 98, + "version": "1.0.0", + "status": "YANKED", + "publishedAt": "2026-03-01T10:00:00Z" + } +} +``` + +AstronClaw 判断 `status != PUBLISHED`,拒绝新安装。已装实例不受影响。 + +### 7.5 发布(创建)个人 skill — 成功 + +``` +POST /api/v1/skills/my-namespace/publish +Content-Type: multipart/form-data +file: +visibility: PRIVATE +``` + +```json +{ + "code": 0, + "data": { + "skillId": 43, + "namespace": "my-namespace", + "slug": "new-skill", + "version": "0.1.0", + "status": "DRAFT", + "fileCount": 2, + "totalSize": 51200 + } +} +``` + +### 7.6 删除个人 skill — 成功 + +``` +DELETE /api/v1/skills/my-namespace/my-skill +``` + +```json +{ + "code": 0, + "data": { + "ok": true + } +} +``` + +### 7.7 删除个人 skill — 无权限 + +``` +DELETE /api/v1/skills/other-namespace/other-skill +``` + +```json +{ + "code": 403, + "msg": "Forbidden", + "data": null +} +``` + +### 7.8 边界:skill 已归档后查询 + +``` +GET /api/v1/skills/my-namespace/archived-skill +``` + +```json +{ + "code": 0, + "data": { + "id": 44, + "slug": "archived-skill", + "status": "ARCHIVED", + "visibility": "PUBLIC" + } +} +``` + +skill 仍可查询,但 AstronClaw 应根据 `status=ARCHIVED` 判断不可新装。 + +--- + +## 8. 遗留问题与建议 + +### 8.1 `package_name` 提取 + +当前 `package_name` 嵌套在 `parsedMetadataJson` JSONB 字段中,不是顶层字段。 + +建议:SaaS Adapter 在返回 AstronClaw DTO 时,解析 JSON 并将 `package_name` 提取为顶层字段。Core 不需要改动。 + +### 8.2 `bundle_url` 获取方式 + +当前没有直接返回 `bundle_url` 的字段,需通过 download 接口获取。`ResolveVersionResponse` 中有 `downloadUrl` 字段。 + +建议:SaaS Adapter 可通过 `resolve` 接口获取 `downloadUrl`,或直接生成预签名 URL 返回给 AstronClaw。 + +### 8.3 删除接口权限 + +当前 `DELETE /api/v1/skills/{namespace}/{slug}`(portal 路径)需要 SUPER_ADMIN 权限。ClawHub 兼容接口 `DELETE /api/v1/skills/{canonicalSlug}` 允许 owner 操作。 + +建议:SaaS Adapter 应统一封装 owner 可操作的删除接口,对 AstronClaw 暴露稳定契约;AstronClaw 不直接依赖开源删除接口路径。 + +### 8.4 `hidden` 与 `status` 的关系 + +当前 `hidden` 是独立于 `status` 的布尔标记(管理员操作),而 `HIDDEN` 是 `SkillStatus` 枚举值之一但实际代码中 skill 的 status 枚举包含 `ACTIVE`、`HIDDEN`、`ARCHIVED`。 + +建议:SaaS Adapter 统一为 AstronClaw 提供一个 `is_visible` 聚合字段,屏蔽内部 hidden 标记与 status 的复杂关系。 diff --git a/docs/oss-02-core-semantic-rules.md b/docs/oss-02-core-semantic-rules.md new file mode 100644 index 00000000..cd256d05 --- /dev/null +++ b/docs/oss-02-core-semantic-rules.md @@ -0,0 +1,663 @@ +# OSS-02 Core 语义规则收口 + +## 1. 文档目标 + +本文档固化 SkillHub Core 的运行时语义规则,确保开源版与 SaaS 版对删除、YANKED、同名冲突、package_name 等规则口径一致,避免 AstronClaw 接入后出现状态漂移。本文定义的是可由 SaaS 统一封装并对 AstronClaw 提供的 `Core` 规则基线,不表示 AstronClaw 直接对接这些开源接口。 + +--- + +## 2. 变更概要 + +### 2.1 新增功能 + +| 功能 | 说明 | +|------|------| +| UPLOADED 状态 | 新增版本状态,表示"已上传,未提交审核" | +| PRIVATE skill 自动发布 | PRIVATE skill 发布后进入 UPLOADED 状态,不自动进入审核 | +| 提交审核接口 | 新增 `POST /{namespace}/{slug}/submit-review`,允许 UPLOADED 状态的版本提交审核 | +| 撤回审核后进入 UPLOADED | 撤回审核后版本状态变为 UPLOADED,而不是 DRAFT | + +### 2.2 状态机变更 + +**变更前**: +``` +DRAFT → SCANNING → PENDING_REVIEW → PUBLISHED + ↓ ↓ + REJECTED YANKED +``` + +**变更后**: +``` +DRAFT → SCANNING → UPLOADED → PENDING_REVIEW → PUBLISHED + ↓ ↓ ↓ ↓ + SCAN_FAILED (可删除) REJECTED YANKED + ↓ ↓ + (可删除) (可删除) +``` + +### 2.3 权限模型变更 + +**核心原则**:权限只和 status 相关,visibility 只影响状态流转。 + +--- + +## 3. 版本状态定义 + +### 3.1 状态枚举 + +```java +public enum SkillVersionStatus { + DRAFT, // 草稿,编辑中 + SCANNING, // 安全扫描中 + SCAN_FAILED, // 扫描失败 + UPLOADED, // 已上传,未提交审核(新增) + PENDING_REVIEW, // 等待审核 + PUBLISHED, // 已发布 + REJECTED, // 审核拒绝 + YANKED // 已撤回 +} +``` + +### 3.2 状态语义 + +| 状态 | 含义 | 文件状态 | 可下载 | 可编辑 | 有检测报告 | +|------|------|---------|-------|-------|----------| +| DRAFT | 草稿,编辑中 | 可能不完整 | 否 | 是 | 否 | +| SCANNING | 安全扫描中 | 完整 | 否 | 否 | 否 | +| SCAN_FAILED | 扫描失败 | 完整 | 否 | 是 | 是(失败) | +| UPLOADED | 已上传,扫描通过 | 完整 | owner | 否 | 是 | +| PENDING_REVIEW | 审核中 | 完整 | owner | 否 | 是 | +| PUBLISHED | 已发布 | 完整 | 看 visibility | 否 | 是 | +| REJECTED | 审核拒绝 | 完整 | 否 | 是 | 是 | +| YANKED | 已撤回 | 完整 | 否 | 否 | 是 | + +--- + +## 4. 发布流程设计 + +### 4.1 发布路径 + +| visibility | 发布后初始状态 | 是否创建审核任务 | +|------------|--------------|----------------| +| PRIVATE | UPLOADED | 否 | +| NAMESPACE_ONLY | PENDING_REVIEW | 是 | +| PUBLIC | PENDING_REVIEW | 是 | + +### 4.2 PRIVATE skill 完整生命周期 + +``` +用户发布 PRIVATE skill + ↓ +状态:SCANNING(安全扫描中) + ↓ +扫描通过 + ↓ +状态:UPLOADED +visibility:PRIVATE + ↓ +owner 可下载/安装/测试 +市场不可见 +管理员可见(用于审计) +已有检测报告 + ↓ +owner 测试满意,确认发布(confirm-publish) + ↓ +状态:PUBLISHED +visibility:PRIVATE(正式私有版本) + ↓ +owner 可下载/安装 +市场不可见 + ↓ +用户想公开,提交审核 + ↓ +状态:PENDING_REVIEW +requestedVisibility:PUBLIC + ↓ +owner 仍可下载/测试 + ↓ +审核通过 + ↓ +状态:PUBLISHED +visibility:PUBLIC(不再是 PRIVATE) + ↓ +市场可见,所有人可下载 +``` + +### 4.3 PUBLIC/NAMESPACE_ONLY skill 生命周期 + +``` +用户发布 PUBLIC/NAMESPACE_ONLY skill + ↓ +状态:PENDING_REVIEW + ↓ +owner 可下载/测试 + ↓ +审核通过 + ↓ +状态:PUBLISHED +visibility:PUBLIC 或 NAMESPACE_ONLY + ↓ +市场可见(受 visibility 控制) +``` + +--- + +## 5. 权限矩阵 + +### 5.1 status 决定下载权限 + +| status | 市场可见 | 可下载 | +|--------|---------|-------| +| DRAFT | 否 | 否 | +| SCANNING | 否 | 否 | +| SCAN_FAILED | 否 | 否 | +| UPLOADED | 否 | owner | +| PENDING_REVIEW | 否 | owner | +| PUBLISHED | 看 visibility | 看 visibility | +| REJECTED | 否 | 否 | +| YANKED | 否 | 否 | + +### 5.2 PUBLISHED 状态下,visibility 决定可见性 + +| visibility | 市场可见 | 可下载 | +|------------|---------|-------| +| PUBLIC | 是 | 所有人 | +| NAMESPACE_ONLY | 命名空间内 | 命名空间成员 | +| PRIVATE | 否 | owner | + +### 5.3 AstronClaw 安装判断规则 + +``` +可安装 = + skill.status == ACTIVE + AND skill.hidden == false + AND 存在至少一个可下载版本 + AND 该版本 bundleReady == true + +可下载版本判断: + - UPLOADED/PENDING_REVIEW:仅 owner + - PUBLISHED:按 visibility 规则 +``` + +--- + +## 6. 状态流转详细设计 + +### 6.1 状态转换表 + +| 当前状态 | 操作 | 目标状态 | 说明 | +|---------|------|---------|------| +| DRAFT | 上传包 | SCANNING | 开始安全扫描 | +| SCANNING | 扫描通过 | UPLOADED 或 PENDING_REVIEW | 看 visibility | +| SCANNING | 扫描失败 | SCAN_FAILED | - | +| SCAN_FAILED | 重新上传 | SCANNING | - | +| UPLOADED | 提交审核 | PENDING_REVIEW | 新增操作 | +| UPLOADED | 确认发布 | PUBLISHED | PRIVATE skill 正式发布,不触发新扫描 | +| UPLOADED | 重新上传 | SCANNING | 允许重新上传 | +| UPLOADED | 删除 | (删除) | 允许删除,未正式发布 | +| PENDING_REVIEW | 审核通过 | PUBLISHED | - | +| PENDING_REVIEW | 审核拒绝 | REJECTED | - | +| PENDING_REVIEW | 撤回审核 | UPLOADED | 变更:原为 DRAFT | +| PUBLISHED | Yank | YANKED | - | +| REJECTED | 重新上传 | SCANNING | - | + +### 6.2 状态机图 + +``` + ┌─────────────────────────────────────────┐ + │ 上传包 │ + └─────────────────────────────────────────┘ + ↓ + ┌───────────────┐ + │ SCANNING │ + └───────────────┘ + / \ + 扫描通过 / \ 扫描失败 + / \ + ┌────────────────────────┐ ┌───────────────┐ + │ visibility=PRIVATE │ │ SCAN_FAILED │ + │ → UPLOADED │ └───────────────┘ + │ visibility=PUBLIC/ │ │ + │ NAMESPACE_ONLY │ │ 重新上传 + │ → PENDING_REVIEW │ ↓ + └────────────────────────┘ ┌───────────────┐ + │ │ SCANNING │ + ↓ └───────────────┘ + ┌────────────────────────┐ + │ UPLOADED │◄────────────────────────┐ + │ (PRIVATE skill 专属) │ │ + │ 已有检测报告 │ │ + └────────────────────────┘ │ + / \ │ + 确认发布 / \ 提交审核 │ + (不触发新扫描) / \ │ + / \ │ + ↓ ↓ │ + ┌───────────────────┐ ┌───────────────────┐ │ + │ PUBLISHED │ │ PENDING_REVIEW │ │ + │ visibility=PRIVATE│ └───────────────────┘ │ + └───────────────────┘ │ │ + │ │ │ + │ 提交审核 │ 审核通过 │ + ↓ ↓ │ + ┌───────────────────┐ ┌───────────────────┐ │ + │ PENDING_REVIEW │ │ PUBLISHED │ │ + └───────────────────┘ │ visibility=PUBLIC │ │ + │ │ 或 NAMESPACE_ONLY │ │ + │ └───────────────────┘ │ + │ 撤回审核 │ │ + └──────────────────────┘ │ + (进入 UPLOADED) │ + │ + ┌───────────────────┐ │ + │ REJECTED │────────────────────────────────────────┘ + └───────────────────┘ 重新上传 + │ + │ 删除 + ↓ + (删除) +``` + +--- + +## 7. 新增接口设计 + +说明: + +以下接口属于开源 `Core` 为 SaaS 提供的基础状态机能力。对 `AstronClaw` 而言,后续仍应统一通过 `SkillHub SaaS` 的 `AstronClaw Adapter` 消费这些能力,而不是直接绑定这些开源接口路径。 + +### 7.1 提交审核接口 + +**接口**:`POST /api/v1/skills/{namespace}/{slug}/submit-review` + +**请求参数**: +```json +{ + "version": "1.0.0", + "targetVisibility": "PUBLIC" +} +``` + +**前置条件**: +- 版本状态为 UPLOADED +- 操作者为 skill owner 或 namespace ADMIN/OWNER + +**执行效果**: +- 版本状态 → PENDING_REVIEW +- `requestedVisibility` 设为目标可见性 +- 创建审核任务 + +**响应**: +```json +{ + "code": 0, + "data": { + "versionId": 100, + "status": "PENDING_REVIEW", + "requestedVisibility": "PUBLIC" + } +} +``` + +### 7.2 确认发布接口(PRIVATE skill) + +**接口**:`POST /api/v1/skills/{namespace}/{slug}/confirm-publish` + +**请求参数**: +```json +{ + "version": "1.0.0" +} +``` + +**前置条件**: +- 版本状态为 UPLOADED +- skill.visibility = PRIVATE +- 操作者为 skill owner + +**执行效果**: +- 版本状态 → PUBLISHED +- visibility 保持 PRIVATE +- **不触发新的扫描**,复用 UPLOADED 时的扫描结果 +- 未来可扩展:加入"发布扫描"功能 + +**响应**: +```json +{ + "code": 0, + "data": { + "skillId": 42, + "versionId": 100, + "status": "PUBLISHED", + "visibility": "PRIVATE" + } +} +``` + +--- + +## 8. 删除 / 隐藏 / 归档 / YANKED 语义规则 + +### 8.1 操作语义总表 + +| 操作 | 触发方式 | 可逆 | 市场可见 | 可新装 | 已装保留 | 可卸载 | slug 可复用 | +|------|---------|------|---------|-------|---------|-------|-----------| +| **硬删除 skill** | owner 或 SUPER_ADMIN | 否 | 否 | 否 | 是 | 是 | 是 | +| **归档 skill** | owner / namespace admin | 是 | 否 | 否 | 是 | 是 | 否 | +| **隐藏 skill** | 管理员 | 是 | 否 | 否 | 是 | 是 | 否 | +| **Yank 版本** | owner / namespace admin | 否 | 否 | 否 | 是 | 是 | N/A | + +### 8.2 Yank 版本 + +**定义**:YANK 是"撤回已发布版本"的操作,用于将一个已发布的版本从可用状态移除。 + +**触发条件**: +- owner 或 namespace ADMIN/OWNER 对 PUBLISHED 状态的版本执行 yank + +**执行效果**: +- `version.status` → `YANKED`(不可逆,无 un-yank 操作) +- `version.downloadReady` → `false` +- 记录 `yankedAt`、`yankedBy`、`yankReason` +- 如果该版本是 `skill.latestVersionId` 指向的版本: + - 自动回退到上一个 PUBLISHED 版本 + - 如果没有其他 PUBLISHED 版本,`latestVersionId` → `null` + +**对 AstronClaw 的影响**: +- 已安装实例不受影响 +- 无法新装该版本 +- 升级场景:目标版本被 yank → 升级失败 + +对接原则: +- 上述语义应由 SaaS Adapter 原样继承并稳定对外提供 +- AstronClaw 通过 Adapter 感知这些状态,不直接绑定开源返回形态 + +**补救方式**: +- 不能 un-yank +- 只能发布新版本(rerelease 或重新上传) + +--- + +## 9. 同名冲突规则 + +### 9.1 唯一性约束 + +数据库约束:`UNIQUE(namespace_id, slug, owner_id)` + +含义: +- 同一 namespace 下,不同 owner 可以有相同 slug +- 同一 namespace 下,同一 owner 只能有一个相同 slug 的 skill + +### 9.2 冲突规则设计原则 + +**核心原则**:只有 PUBLISHED 状态才会阻塞同名发布,但区分 visibility。 + +| 对方状态 | 我发布同名 PRIVATE | 我发布同名 PUBLIC | 说明 | +|---------|-------------------|------------------|------| +| UPLOADED | ✅ 允许 | ✅ 允许 | 多个 UPLOADED 可共存 | +| PENDING_REVIEW | ✅ 允许 | ✅ 允许 | 还未正式发布 | +| PRIVATE + PUBLISHED | ❌ 拒绝 | ❌ 拒绝 | 只允许一个正式私有版本 | +| PUBLIC + PUBLISHED | ❌ 拒绝 | ❌ 拒绝 | 市场已占用 | + +### 9.3 冲突规则表(详细) + +| 场景 | 是否允许 | 说明 | +|------|---------|------| +| 同 namespace,同 slug,同 owner | 允许(复用) | 新版本挂到已有 skill 下 | +| 同 namespace,同 slug,不同 owner,对方只有 UPLOADED | 允许 | 多个 UPLOADED 可共存测试 | +| 同 namespace,同 slug,不同 owner,对方只有 PENDING_REVIEW | 允许 | 还未正式发布 | +| 同 namespace,同 slug,不同 owner,对方有 PRIVATE + PUBLISHED | 拒绝 | 只允许一个正式私有版本 | +| 同 namespace,同 slug,不同 owner,对方有 PUBLIC/NAMESPACE_ONLY + PUBLISHED | 拒绝 | 市场已占用 | +| 不同 namespace,同 slug | 允许 | namespace 隔离 | + +### 9.4 完整流程示例 + +``` +用户 A 发布 PRIVATE `ns/my-skill` + ↓ +状态:UPLOADED + ↓ +用户 B 发布 PRIVATE `ns/my-skill` + ↓ +状态:UPLOADED ✅ 允许(多个 UPLOADED 可共存) + ↓ +用户 A 确认发布 → PRIVATE + PUBLISHED ✅ 允许 + ↓ +用户 B 确认发布 → ❌ 被拒绝 + ↓ +错误信息:error.skill.publish.nameConflict.private + ↓ +用户 B 可以: + 1. 改名发布 + 2. 等用户 A 删除/归档后再发布 + 3. 提交审核变成 PUBLIC(如果 A 是 PRIVATE) +``` + +### 9.5 代码改动 + +**文件**:`SkillPublishService.java` + +```java +// 冲突检查逻辑(第 230-242 行) +for (Skill existing : existingSkills) { + if (!existing.getOwnerId().equals(publisherId)) { + // 检查是否有 PUBLISHED 版本 + boolean hasPublished = !skillVersionRepository + .findBySkillIdAndStatus(existing.getId(), SkillVersionStatus.PUBLISHED) + .isEmpty(); + + if (hasPublished) { + // PUBLISHED 版本存在,无论 visibility 如何都拒绝 + // 因为只允许一个 PRIVATE + PUBLISHED 或 PUBLIC + PUBLISHED + if (existing.getVisibility() == SkillVisibility.PRIVATE) { + throw new DomainBadRequestException("error.skill.publish.nameConflict.private", skillSlug); + } else { + throw new DomainBadRequestException("error.skill.publish.nameConflict", skillSlug); + } + } + } +} +``` + +### 9.6 错误信息 + +| 错误码 | 说明 | +|-------|------| +| `error.skill.publish.nameConflict` | 已有同名 PUBLIC/NAMESPACE_ONLY skill 发布 | +| `error.skill.publish.nameConflict.private` | 已有同名 PRIVATE skill 正式发布 | + +--- + +## 10. package_name / runtime 规则 + +### 10.1 当前实现 + +- `package_name` 不是 Core 的结构化字段 +- 存储在 `skill_version.parsedMetadataJson` JSONB 字段中 +- 由 skill 作者在 SKILL.md frontmatter 中定义 + +### 10.2 SaaS Adapter 职责 + +- 从 `parsedMetadataJson` 中提取 `package_name` +- 作为顶层字段返回给 AstronClaw +- 可选:检查跨 skill 的 package_name 唯一性 +- 统一封装 `submit-review`、`confirm-publish`、删除、查询等 Core 能力,对 AstronClaw 暴露稳定接口 + +### 10.3 规则建议 + +| 规则 | 建议 | +|------|------| +| 格式 | 建议使用 `namespace__slug` 格式,避免冲突 | +| 跨版本稳定性 | 同一 skill 跨版本应保持 package_name 一致 | +| 唯一性 | SaaS Adapter 可检查并警告冲突,但不强制阻止 | + +--- + +## 11. 代码改动清单 + +说明: + +以下改动属于开源 `Core` 的规则实现,用于给 SaaS 封装层提供稳定能力基线;不等同于直接向 AstronClaw 暴露这些开源接口。 + +### 11.1 枚举新增 + +**文件**:`SkillVersionStatus.java` + +```java +public enum SkillVersionStatus { + DRAFT, + SCANNING, + SCAN_FAILED, + UPLOADED, // 新增 + PENDING_REVIEW, + PUBLISHED, + REJECTED, + YANKED +} +``` + +### 11.2 发布逻辑改动 + +**文件**:`SkillPublishService.java` + +```java +// 第 279-285 行,改为 +if (visibility == SkillVisibility.PRIVATE) { + version.setStatus(SkillVersionStatus.UPLOADED); + version.setPublishedAt(currentTime()); + // 不创建审核任务 +} else if (autoPublish) { + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setPublishedAt(currentTime()); +} else { + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + // 创建审核任务 +} +``` + +### 11.3 撤回审核改动 + +**文件**:`SkillGovernanceService.java` + +```java +// withdrawPendingVersion 方法,改为 +skillVersion.setStatus(SkillVersionStatus.UPLOADED); // 原为 DRAFT +``` + +### 11.4 下载权限改动 + +**文件**:`SkillDownloadService.java`、`SkillQueryService.java` + +```java +// UPLOADED 和 PENDING_REVIEW 状态允许 owner 下载 +private boolean canDownload(SkillVersion version, Skill skill, String currentUserId) { + return switch (version.getStatus()) { + case UPLOADED, PENDING_REVIEW -> skill.getOwnerId().equals(currentUserId); + case PUBLISHED -> true; // 按 visibility 判断 + default -> false; + }; +} +``` + +### 11.5 新增服务 + +**文件**:`SkillReviewSubmitService.java`(新增) + +- 实现 UPLOADED 版本提交审核逻辑 + +### 11.6 新增控制器 + +**文件**:`SkillReviewSubmitController.java`(新增) + +- 暴露 `POST /{namespace}/{slug}/submit-review` 接口 +- 暴露 `POST /{namespace}/{slug}/confirm-publish` 接口 + +### 11.7 管理员可见性 + +**文件**:`VisibilityChecker.java` + +- SUPER_ADMIN 可以看到所有 skill,包括 UPLOADED 状态 + +### 11.8 数据库迁移 + +**文件**:新增迁移脚本 + +- 更新 `skill_version_status` 枚举类型,添加 UPLOADED 值 + +--- + +## 12. 阻塞上线条件 + +| 问题 | 严重程度 | 状态 | +|------|---------|------| +| 新增 UPLOADED 状态 | 高 | 待实现 | +| PRIVATE skill 发布逻辑改动 | 高 | 待实现 | +| 提交审核接口 | 高 | 待实现 | +| 撤回审核后进入 UPLOADED | 中 | 待实现 | +| 同名冲突检查补全 | 中 | 待实现 | +| 管理员可见 UPLOADED skill | 低 | 待实现 | +| package_name 唯一性检查 | 低 | 可选 | + +--- + +## 13. 对老版本的影响 + +### 13.1 数据兼容性 + +| 影响点 | 分析 | 需要处理 | +|--------|------|---------| +| 老版本数据 | 不受影响,状态不变 | 否 | +| 数据库枚举 | 需添加 UPLOADED 值 | 是 | +| API 兼容性 | 新接口是新增,不影响老接口 | 否 | + +### 13.2 状态流转影响 + +| 场景 | 老逻辑 | 新逻辑 | 影响 | +|------|--------|--------|------| +| 老版本撤回审核 | PENDING_REVIEW → DRAFT | PENDING_REVIEW → UPLOADED | 前端需适配新状态 | +| 老版本删除 | DRAFT/REJECTED/SCAN_FAILED 可删 | UPLOADED 也可删 | 需更新代码判断 | + +### 13.3 代码改动点 + +**文件**:`SkillGovernanceService.java` + +**1. 删除版本逻辑**(第163-166行): +```java +// 原代码 +if (version.getStatus() != SkillVersionStatus.DRAFT + && version.getStatus() != SkillVersionStatus.REJECTED + && version.getStatus() != SkillVersionStatus.SCAN_FAILED) { + throw new DomainBadRequestException("error.skill.version.delete.unsupported", version.getVersion()); +} + +// 改为:允许删除 UPLOADED 状态 +if (version.getStatus() != SkillVersionStatus.DRAFT + && version.getStatus() != SkillVersionStatus.REJECTED + && version.getStatus() != SkillVersionStatus.SCAN_FAILED + && version.getStatus() != SkillVersionStatus.UPLOADED) { + throw new DomainBadRequestException("error.skill.version.delete.unsupported", version.getVersion()); +} +``` + +**2. 撤回审核逻辑**(第245行): +```java +// 原代码 +version.setStatus(SkillVersionStatus.DRAFT); + +// 改为 +version.setStatus(SkillVersionStatus.UPLOADED); +``` + +### 13.4 前端适配 + +| 状态 | 前端展示建议 | +|------|-------------| +| UPLOADED | "已上传" 或 "待确认" | +| 可删除状态 | DRAFT、SCAN_FAILED、REJECTED、UPLOADED | +| 可编辑状态 | DRAFT、SCAN_FAILED、REJECTED | + +### 13.5 迁移策略 + +1. **数据库迁移**:添加 UPLOADED 枚举值 +2. **代码部署**:先部署后端,再部署前端 +3. **老数据处理**:无需处理,老版本状态保持不变 +4. **回滚方案**:如需回滚,UPLOADED 状态的版本按 DRAFT 处理 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java index 16b7d2e4..b590fa22 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java @@ -5,8 +5,10 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.dto.AdminSkillActionRequest; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.ConfirmPublishRequest; import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; import com.iflytek.skillhub.dto.SkillVersionRereleaseRequest; +import com.iflytek.skillhub.dto.SubmitReviewRequest; import com.iflytek.skillhub.service.AuditRequestContext; import com.iflytek.skillhub.service.GovernanceWorkflowAppService; import jakarta.validation.Valid; @@ -118,4 +120,39 @@ public class SkillLifecycleController extends BaseApiController { userNsRoles, AuditRequestContext.from(httpRequest))); } + + @PostMapping("/{namespace}/{slug}/submit-review") + public ApiResponse submitForReview(@PathVariable String namespace, + @PathVariable String slug, + @Valid @RequestBody SubmitReviewRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { + return ok("response.success.updated", + governanceWorkflowAppService.submitForReview( + namespace, + slug, + request.version(), + request.targetVisibility(), + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); + } + + @PostMapping("/{namespace}/{slug}/confirm-publish") + public ApiResponse confirmPublish(@PathVariable String namespace, + @PathVariable String slug, + @Valid @RequestBody ConfirmPublishRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest httpRequest) { + return ok("response.success.updated", + governanceWorkflowAppService.confirmPublish( + namespace, + slug, + request.version(), + userId, + userNsRoles, + AuditRequestContext.from(httpRequest))); + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ConfirmPublishRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ConfirmPublishRequest.java new file mode 100644 index 00000000..cfcb2991 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ConfirmPublishRequest.java @@ -0,0 +1,11 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.NotBlank; + +/** + * Request to confirm publish for a PRIVATE skill version. + */ +public record ConfirmPublishRequest( + @NotBlank(message = "Version is required") + String version +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitReviewRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitReviewRequest.java new file mode 100644 index 00000000..817e9c97 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitReviewRequest.java @@ -0,0 +1,16 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; + +/** + * Request to submit a skill version for review. + */ +public record SubmitReviewRequest( + @NotBlank(message = "Version is required") + String version, + + @NotBlank(message = "Target visibility is required") + @Pattern(regexp = "PUBLIC|NAMESPACE_ONLY", message = "Target visibility must be PUBLIC or NAMESPACE_ONLY") + String targetVisibility +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java index 4432d060..6cca3952 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java @@ -254,4 +254,36 @@ public class GovernanceWorkflowAppService { AuditRequestContext auditContext) { return namespacePortalCommandAppService.restoreNamespace(slug, userId, auditContext); } + + public SkillLifecycleMutationResponse submitForReview(String namespace, + String slug, + String version, + String targetVisibility, + String userId, + Map userNsRoles, + AuditRequestContext auditContext) { + return skillLifecycleAppService.submitForReview( + namespace, + slug, + version, + targetVisibility, + userId, + userNsRoles, + auditContext); + } + + public SkillLifecycleMutationResponse confirmPublish(String namespace, + String slug, + String version, + String userId, + Map userNsRoles, + AuditRequestContext auditContext) { + return skillLifecycleAppService.confirmPublish( + namespace, + slug, + version, + userId, + userNsRoles, + auditContext); + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java index fda05a72..5670ebef 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java @@ -11,6 +11,7 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillReviewSubmitService; import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.AdminSkillActionRequest; import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; @@ -31,6 +32,7 @@ public class SkillLifecycleAppService { private final SkillGovernanceService skillGovernanceService; private final ReviewService reviewService; private final SkillPublishService skillPublishService; + private final SkillReviewSubmitService skillReviewSubmitService; private final AuditLogService auditLogService; private final SkillSlugResolutionService skillSlugResolutionService; @@ -39,6 +41,7 @@ public class SkillLifecycleAppService { SkillGovernanceService skillGovernanceService, ReviewService reviewService, SkillPublishService skillPublishService, + SkillReviewSubmitService skillReviewSubmitService, AuditLogService auditLogService, SkillSlugResolutionService skillSlugResolutionService) { this.namespaceRepository = namespaceRepository; @@ -46,6 +49,7 @@ public class SkillLifecycleAppService { this.skillGovernanceService = skillGovernanceService; this.reviewService = reviewService; this.skillPublishService = skillPublishService; + this.skillReviewSubmitService = skillReviewSubmitService; this.auditLogService = auditLogService; this.skillSlugResolutionService = skillSlugResolutionService; } @@ -171,6 +175,74 @@ public class SkillLifecycleAppService { ); } + @Transactional + public SkillLifecycleMutationResponse submitForReview(String namespace, + String slug, + String version, + String targetVisibility, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + SkillVersion skillVersion = findVersion(skill.getId(), version); + skillReviewSubmitService.submitForReview( + skill.getId(), + skillVersion.getId(), + com.iflytek.skillhub.domain.skill.SkillVisibility.valueOf(targetVisibility), + userId, + normalizeRoles(userNamespaceRoles) + ); + auditLogService.record( + userId, + "SUBMIT_REVIEW", + "SKILL_VERSION", + skillVersion.getId(), + null, + auditContext.clientIp(), + auditContext.userAgent(), + "{\"version\":\"" + version.replace("\"", "\\\"") + "\",\"targetVisibility\":\"" + targetVisibility + "\"}" + ); + return new SkillLifecycleMutationResponse( + skill.getId(), + skillVersion.getId(), + "SUBMIT_REVIEW", + "PENDING_REVIEW" + ); + } + + @Transactional + public SkillLifecycleMutationResponse confirmPublish(String namespace, + String slug, + String version, + String userId, + Map userNamespaceRoles, + AuditRequestContext auditContext) { + Skill skill = findSkill(namespace, slug, userId); + SkillVersion skillVersion = findVersion(skill.getId(), version); + skillReviewSubmitService.confirmPublish( + skill.getId(), + skillVersion.getId(), + userId, + normalizeRoles(userNamespaceRoles) + ); + auditLogService.record( + userId, + "CONFIRM_PUBLISH", + "SKILL_VERSION", + skillVersion.getId(), + null, + auditContext.clientIp(), + auditContext.userAgent(), + "{\"version\":\"" + version.replace("\"", "\\\"") + "\"}" + ); + return new SkillLifecycleMutationResponse( + skill.getId(), + skillVersion.getId(), + "CONFIRM_PUBLISH", + "PUBLISHED" + ); + } + private Skill findSkill(String namespaceSlug, String skillSlug, String currentUserId) { String cleanNamespace = namespaceSlug.startsWith("@") ? namespaceSlug.substring(1) : namespaceSlug; Namespace namespace = namespaceRepository.findBySlug(cleanNamespace) diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index eba859f5..6e754186 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -133,7 +133,12 @@ error.admin.user.role.superAdmin.assignDenied=Only SUPER_ADMIN can assign SUPER_ error.admin.user.status.invalid=Invalid user status: {0} error.admin.user.status.unsupported=Only ACTIVE or DISABLED status can be managed here error.skill.publish.nameConflict=A published skill with name ''{0}'' already exists in this namespace +error.skill.publish.nameConflict.private=A private skill with name ''{0}'' has already been published in this namespace error.skill.approve.nameConflict=Cannot approve: a published skill with name ''{0}'' already exists in this namespace +error.skill.version.submit.notUploaded=Version ''{0}'' is not in UPLOADED status and cannot be submitted for review +error.skill.version.confirm.notUploaded=Version ''{0}'' is not in UPLOADED status and cannot be confirmed +error.skill.confirm.notPrivate=Only PRIVATE skills can use confirm-publish +error.skill.version.notDownloadable=Version ''{0}'' is not available for download # Profile update error.profile.displayName.length=Display name must be between 2 and 32 characters diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index d220e409..541770db 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -133,7 +133,12 @@ error.admin.user.role.superAdmin.assignDenied=只有 SUPER_ADMIN 可以分配 SU error.admin.user.status.invalid=无效的用户状态:{0} error.admin.user.status.unsupported=这里只允许管理 ACTIVE 或 DISABLED 状态的用户 error.skill.publish.nameConflict=该命名空间下已存在名为"{0}"的已发布技能,无法提交 +error.skill.publish.nameConflict.private=该命名空间下已存在名为"{0}"的已发布私有技能,无法提交 error.skill.approve.nameConflict=无法通过审核:该命名空间下已存在名为"{0}"的已发布技能 +error.skill.version.submit.notUploaded=版本"{0}"不在 UPLOADED 状态,无法提交审核 +error.skill.version.confirm.notUploaded=版本"{0}"不在 UPLOADED 状态,无法确认发布 +error.skill.confirm.notPrivate=只有 PRIVATE 技能可以使用确认发布功能 +error.skill.version.notDownloadable=版本"{0}"不可下载 # 用户资料修改 error.profile.displayName.length=昵称长度需在 2-32 个字符之间 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java index 53215011..e2c10146 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java @@ -18,6 +18,7 @@ import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillReviewSubmitService; import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.AdminSkillActionRequest; import org.junit.jupiter.api.Test; @@ -33,6 +34,7 @@ class SkillLifecycleAppServiceTest { private final SkillGovernanceService skillGovernanceService = mock(SkillGovernanceService.class); private final ReviewService reviewService = mock(ReviewService.class); private final SkillPublishService skillPublishService = mock(SkillPublishService.class); + private final SkillReviewSubmitService skillReviewSubmitService = mock(SkillReviewSubmitService.class); private final AuditLogService auditLogService = mock(AuditLogService.class); private final SkillSlugResolutionService skillSlugResolutionService = mock(SkillSlugResolutionService.class); private final SkillLifecycleAppService service = new SkillLifecycleAppService( @@ -41,6 +43,7 @@ class SkillLifecycleAppServiceTest { skillGovernanceService, reviewService, skillPublishService, + skillReviewSubmitService, auditLogService, skillSlugResolutionService ); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index bd31218a..56ea7884 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -101,7 +101,9 @@ public class ReviewService { throw new DomainForbiddenException("review.submit.no_permission"); } - if (skillVersion.getStatus() != SkillVersionStatus.DRAFT) { + // Support both DRAFT (legacy) and UPLOADED (new flow) status + if (skillVersion.getStatus() != SkillVersionStatus.DRAFT + && skillVersion.getStatus() != SkillVersionStatus.UPLOADED) { throw new DomainBadRequestException("review.submit.not_draft", skillVersionId); } @@ -137,7 +139,9 @@ public class ReviewService { .orElseThrow(() -> new DomainNotFoundException("namespace.not_found", skill.getNamespaceId())); assertNamespaceActive(namespace); - if (skillVersion.getStatus() != SkillVersionStatus.DRAFT) { + // Support both DRAFT (legacy) and UPLOADED (new flow) status + if (skillVersion.getStatus() != SkillVersionStatus.DRAFT + && skillVersion.getStatus() != SkillVersionStatus.UPLOADED) { throw new DomainBadRequestException("review.submit.not_draft", skillVersionId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java index 195510a5..fb24451f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.domain.security; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; @@ -105,7 +106,12 @@ public class SecurityScanService { audit.setScannedAt(Instant.now(Clock.systemUTC())); auditRepository.save(audit); - version.setStatus(SkillVersionStatus.PENDING_REVIEW); + // Set status based on requestedVisibility + if (version.getRequestedVisibility() == SkillVisibility.PRIVATE) { + version.setStatus(SkillVersionStatus.UPLOADED); + } else { + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + } skillVersionRepository.save(version); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatus.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatus.java index 78fa2bb1..21978985 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatus.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatus.java @@ -4,6 +4,7 @@ public enum SkillVersionStatus { DRAFT, SCANNING, SCAN_FAILED, + UPLOADED, PENDING_REVIEW, PUBLISHED, REJECTED, diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java index 259d9e18..3bb194ff 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java @@ -164,12 +164,15 @@ public class SkillDownloadService { private DownloadResult downloadVersion(Skill skill, SkillVersion version) { assertPublishedAccessible(skill); - assertPublishedVersion(version); + assertDownloadableVersion(skill, version); DownloadResult result = buildDownloadResult(skill, version); - skillRepository.incrementDownloadCount(skill.getId()); - skillVersionStatsRepository.incrementDownloadCount(version.getId(), skill.getId()); - eventPublisher.publishEvent(new SkillDownloadedEvent(skill.getId(), version.getId())); + // Only increment download count for PUBLISHED versions + if (version.getStatus() == SkillVersionStatus.PUBLISHED) { + skillRepository.incrementDownloadCount(skill.getId()); + skillVersionStatsRepository.incrementDownloadCount(version.getId(), skill.getId()); + eventPublisher.publishEvent(new SkillDownloadedEvent(skill.getId(), version.getId())); + } return result; } @@ -292,9 +295,21 @@ public class SkillDownloadService { } } - private void assertPublishedVersion(SkillVersion version) { - if (version.getStatus() != SkillVersionStatus.PUBLISHED) { - throw new DomainBadRequestException("error.skill.version.notPublished", version.getVersion()); + /** + * Asserts that the version can be downloaded. + * - PUBLISHED: anyone with skill access can download + * - UPLOADED/PENDING_REVIEW: only skill owner can download + */ + private void assertDownloadableVersion(Skill skill, SkillVersion version) { + switch (version.getStatus()) { + case PUBLISHED -> { + // Anyone with skill access can download published versions + } + case UPLOADED, PENDING_REVIEW -> { + // Only owner can download UPLOADED/PENDING_REVIEW versions + // Note: This check is already done in assertCanDownload via visibilityChecker + } + default -> throw new DomainBadRequestException("error.skill.version.notDownloadable", version.getVersion()); } } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 9f0be2bf..0dfcb5f3 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -162,7 +162,8 @@ public class SkillGovernanceService { assertCanManageLifecycle(skill, actorUserId, userNamespaceRoles); if (version.getStatus() != SkillVersionStatus.DRAFT && version.getStatus() != SkillVersionStatus.REJECTED - && version.getStatus() != SkillVersionStatus.SCAN_FAILED) { + && version.getStatus() != SkillVersionStatus.SCAN_FAILED + && version.getStatus() != SkillVersionStatus.UPLOADED) { throw new DomainBadRequestException("error.skill.version.delete.unsupported", version.getVersion()); } @@ -242,7 +243,7 @@ public class SkillGovernanceService { if (version.getStatus() != SkillVersionStatus.PENDING_REVIEW) { throw new DomainBadRequestException("review.withdraw.not_pending", version.getId()); } - version.setStatus(SkillVersionStatus.DRAFT); + version.setStatus(SkillVersionStatus.UPLOADED); SkillVersion savedVersion = skillVersionRepository.save(version); skill.setUpdatedBy(actorUserId); skillRepository.save(skill); 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 1afb2a1c..07ca9735 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 @@ -172,14 +172,17 @@ public class SkillPublishService { List entries = rebuildEntriesForRerelease(skillId, publishedVersion.getId(), targetVersion); + // Rerelease follows the same visibility-based workflow as normal publish: + // - PRIVATE skills go to UPLOADED status + // - PUBLIC/NAMESPACE_ONLY skills go to PENDING_REVIEW (or UPLOADED after scan) return publishFromEntriesInternal( resolveNamespaceSlug(skill.getNamespaceId()), entries, publisherId, skill.getVisibility(), Set.of(), - true, - true, + false, // confirmWarnings=false: no warnings to confirm for rerelease + false, // forceAutoPublish=false: respect visibility rules true ); } @@ -250,13 +253,19 @@ public class SkillPublishService { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespace.getId(), skillSlug); // Check if any other owner's skill has published versions + // Only PUBLISHED status blocks same-name publishing (UPLOADED/PENDING_REVIEW allowed) for (Skill existing : existingSkills) { if (!existing.getOwnerId().equals(publisherId)) { boolean hasPublished = !skillVersionRepository .findBySkillIdAndStatus(existing.getId(), SkillVersionStatus.PUBLISHED) .isEmpty(); if (hasPublished) { - throw new DomainBadRequestException("error.skill.publish.nameConflict", skillSlug); + // Distinguish between PRIVATE and PUBLIC/NAMESPACE_ONLY conflicts + if (existing.getVisibility() == SkillVisibility.PRIVATE) { + throw new DomainBadRequestException("error.skill.publish.nameConflict.private", skillSlug); + } else { + throw new DomainBadRequestException("error.skill.publish.nameConflict", skillSlug); + } } } } @@ -274,12 +283,13 @@ public class SkillPublishService { } // 6c. Auto-withdraw pending review versions + // When publishing a new version, existing PENDING_REVIEW versions are withdrawn to UPLOADED status List pendingVersions = skillVersionRepository .findBySkillIdAndStatus(skill.getId(), SkillVersionStatus.PENDING_REVIEW); for (SkillVersion pending : pendingVersions) { reviewTaskRepository.findBySkillVersionIdAndStatus(pending.getId(), ReviewTaskStatus.PENDING) .ifPresent(reviewTaskRepository::delete); - pending.setStatus(SkillVersionStatus.DRAFT); + pending.setStatus(SkillVersionStatus.UPLOADED); skillVersionRepository.save(pending); } @@ -300,6 +310,10 @@ public class SkillPublishService { if (autoPublish) { version.setStatus(SkillVersionStatus.PUBLISHED); version.setPublishedAt(currentTime()); + } else if (visibility == SkillVisibility.PRIVATE) { + // PRIVATE skill goes to UPLOADED status, no review task created + version.setStatus(SkillVersionStatus.UPLOADED); + version.setPublishedAt(currentTime()); } else { version.setStatus(SkillVersionStatus.PENDING_REVIEW); } @@ -376,7 +390,8 @@ public class SkillPublishService { version.setDownloadReady(!skillFiles.isEmpty()); skillVersionRepository.save(version); - if (!autoPublish) { + // Create review task for PUBLIC/NAMESPACE_ONLY (not PRIVATE) + if (!autoPublish && visibility != SkillVisibility.PRIVATE) { ReviewTask reviewTask = new ReviewTask(version.getId(), namespace.getId(), publisherId); ReviewTask savedReviewTask = reviewTaskRepository.save(reviewTask); eventPublisher.publishEvent(new ReviewSubmittedEvent( @@ -386,15 +401,18 @@ public class SkillPublishService { savedReviewTask.getSubmittedBy(), savedReviewTask.getNamespaceId() )); - if (securityScanService.isEnabled()) { - securityScanService.triggerScan(version.getId(), entries, publisherId); - } + } + + // Trigger security scan for all non-autoPublish versions + if (!autoPublish && securityScanService.isEnabled()) { + securityScanService.triggerScan(version.getId(), entries, publisherId); } // 12. Update skill metadata and move the published pointer for auto-publish flows skill.setDisplayName(metadata.name()); skill.setSummary(metadata.description()); - if (autoPublish) { + if (autoPublish || visibility == SkillVisibility.PRIVATE) { + // Update latestVersionId for autoPublish or PRIVATE skill (UPLOADED status) skill.setLatestVersionId(version.getId()); skill.setVisibility(visibility); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java index 376f43b3..0a64260c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java @@ -333,6 +333,7 @@ public class SkillQueryService { visibleVersions = skillVersionRepository.findBySkillId(skill.getId()).stream() .filter(version -> version.getStatus() == SkillVersionStatus.PUBLISHED || version.getStatus() == SkillVersionStatus.PENDING_REVIEW + || version.getStatus() == SkillVersionStatus.UPLOADED || version.getStatus() == SkillVersionStatus.DRAFT || version.getStatus() == SkillVersionStatus.REJECTED || version.getStatus() == SkillVersionStatus.YANKED @@ -382,6 +383,7 @@ public class SkillQueryService { List versions = skillVersionRepository.findBySkillId(skill.getId()).stream() .filter(version -> version.getStatus() == SkillVersionStatus.PUBLISHED || version.getStatus() == SkillVersionStatus.PENDING_REVIEW + || version.getStatus() == SkillVersionStatus.UPLOADED || version.getStatus() == SkillVersionStatus.DRAFT || version.getStatus() == SkillVersionStatus.REJECTED || version.getStatus() == SkillVersionStatus.YANKED @@ -689,15 +691,18 @@ public class SkillQueryService { if (status == SkillVersionStatus.SCAN_FAILED) { return 1; } - if (status == SkillVersionStatus.REJECTED) { + if (status == SkillVersionStatus.UPLOADED) { return 2; } - if (status == SkillVersionStatus.PENDING_REVIEW) { + if (status == SkillVersionStatus.REJECTED) { return 3; } - if (status == SkillVersionStatus.DRAFT) { + if (status == SkillVersionStatus.PENDING_REVIEW) { return 4; } + if (status == SkillVersionStatus.DRAFT) { + return 5; + } if (status == SkillVersionStatus.YANKED) { return 5; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java new file mode 100644 index 00000000..df8e9fd1 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java @@ -0,0 +1,159 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.*; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Clock; +import java.time.Instant; +import java.util.Map; + +/** + * Service for submitting skill versions for review and confirming private publishes. + * + *

This service handles two key workflows for UPLOADED skill versions: + *

    + *
  • submitForReview: Transitions an UPLOADED version to PENDING_REVIEW status, + * creating a review task for PUBLIC/NAMESPACE_ONLY visibility changes.
  • + *
  • confirmPublish: Transitions an UPLOADED version directly to PUBLISHED status + * for PRIVATE skills without requiring review.
  • + *
+ * + * @see SkillVersionStatus#UPLOADED + * @see SkillVisibility#PRIVATE + */ +@Service +public class SkillReviewSubmitService { + + private final SkillRepository skillRepository; + private final SkillVersionRepository skillVersionRepository; + private final ReviewTaskRepository reviewTaskRepository; + private final NamespaceMemberRepository namespaceMemberRepository; + private final ApplicationEventPublisher eventPublisher; + private final Clock clock; + + public SkillReviewSubmitService( + SkillRepository skillRepository, + SkillVersionRepository skillVersionRepository, + ReviewTaskRepository reviewTaskRepository, + NamespaceMemberRepository namespaceMemberRepository, + ApplicationEventPublisher eventPublisher, + Clock clock) { + this.skillRepository = skillRepository; + this.skillVersionRepository = skillVersionRepository; + this.reviewTaskRepository = reviewTaskRepository; + this.namespaceMemberRepository = namespaceMemberRepository; + this.eventPublisher = eventPublisher; + this.clock = clock; + } + + /** + * Submit an UPLOADED or DRAFT version for review. + * Transitions version status from UPLOADED/DRAFT to PENDING_REVIEW. + * + *

Supports both UPLOADED (new flow) and DRAFT (legacy compatibility) status. + * + * @param skillId the skill ID + * @param versionId the version ID + * @param targetVisibility the target visibility after approval + * @param actorUserId the user performing the action + * @param userNamespaceRoles user's namespace roles + */ + @Transactional + public void submitForReview(Long skillId, Long versionId, SkillVisibility targetVisibility, + String actorUserId, Map userNamespaceRoles) { + Skill skill = skillRepository.findById(skillId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", skillId)); + SkillVersion version = skillVersionRepository.findById(versionId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); + + // Validate ownership + assertCanManageLifecycle(skill, actorUserId, userNamespaceRoles); + + // Validate version status - support both UPLOADED (new) and DRAFT (legacy) + if (version.getStatus() != SkillVersionStatus.UPLOADED + && version.getStatus() != SkillVersionStatus.DRAFT) { + throw new DomainBadRequestException("error.skill.version.submit.notUploaded", version.getVersion()); + } + + // Validate version belongs to skill + if (!version.getSkillId().equals(skillId)) { + throw new DomainBadRequestException("error.skill.version.mismatch"); + } + + // Update version + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + version.setRequestedVisibility(targetVisibility); + skillVersionRepository.save(version); + + // Create review task + ReviewTask reviewTask = new ReviewTask(versionId, skill.getNamespaceId(), actorUserId); + reviewTaskRepository.save(reviewTask); + } + + /** + * Confirm publish for a PRIVATE skill version. + * Transitions version status from UPLOADED/DRAFT to PUBLISHED without review. + * + *

Supports both UPLOADED (new flow) and DRAFT (legacy compatibility) status. + * + * @param skillId the skill ID + * @param versionId the version ID + * @param actorUserId the user performing the action + * @param userNamespaceRoles user's namespace roles + */ + @Transactional + public void confirmPublish(Long skillId, Long versionId, String actorUserId, + Map userNamespaceRoles) { + Skill skill = skillRepository.findById(skillId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", skillId)); + SkillVersion version = skillVersionRepository.findById(versionId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); + + // Validate ownership + assertCanManageLifecycle(skill, actorUserId, userNamespaceRoles); + + // Validate skill visibility is PRIVATE + if (skill.getVisibility() != SkillVisibility.PRIVATE) { + throw new DomainBadRequestException("error.skill.confirm.notPrivate"); + } + + // Validate version status - support both UPLOADED (new) and DRAFT (legacy) + if (version.getStatus() != SkillVersionStatus.UPLOADED + && version.getStatus() != SkillVersionStatus.DRAFT) { + throw new DomainBadRequestException("error.skill.version.confirm.notUploaded", version.getVersion()); + } + + // Validate version belongs to skill + if (!version.getSkillId().equals(skillId)) { + throw new DomainBadRequestException("error.skill.version.mismatch"); + } + + // Update version to PUBLISHED + version.setStatus(SkillVersionStatus.PUBLISHED); + version.setPublishedAt(Instant.now(clock)); + skillVersionRepository.save(version); + + // Update skill's latest version + skill.setLatestVersionId(versionId); + skill.setUpdatedBy(actorUserId); + skillRepository.save(skill); + } + + private void assertCanManageLifecycle(Skill skill, String actorUserId, Map userNamespaceRoles) { + NamespaceRole namespaceRole = userNamespaceRoles.get(skill.getNamespaceId()); + boolean canManage = skill.getOwnerId().equals(actorUserId) + || namespaceRole == NamespaceRole.ADMIN + || namespaceRole == NamespaceRole.OWNER; + if (!canManage) { + throw new DomainForbiddenException("error.skill.lifecycle.noPermission"); + } + } +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java index f2b254fa..b6f3faff 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java @@ -156,7 +156,7 @@ class SkillGovernanceServiceTest { } @Test - void withdrawPendingVersion_demotesVersionToDraft() { + void withdrawPendingVersion_demotesVersionToUploaded() { Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); setField(skill, "id", 1L); SkillVersion version = new SkillVersion(1L, "1.0.0", "owner"); @@ -167,7 +167,7 @@ class SkillGovernanceServiceTest { SkillVersion result = service.withdrawPendingVersion(skill, version, "owner"); - assertThat(result.getStatus()).isEqualTo(SkillVersionStatus.DRAFT); + assertThat(result.getStatus()).isEqualTo(SkillVersionStatus.UPLOADED); verify(skillVersionRepository).save(version); verify(skillRepository).save(skill); verify(objectStorageService, never()).deleteObject(any()); 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 7e2ff7c1..ae091889 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 @@ -796,7 +796,7 @@ class SkillPublishServiceTest { } @Test - void testRereleasePublishedVersion_ShouldCloneFilesAndAutoPublish() throws Exception { + void testRereleasePublishedVersion_ShouldCloneFilesAndSubmitForReview() throws Exception { String publisherId = "user-100"; Skill skill = new Skill(1L, "demo-skill", publisherId, SkillVisibility.PUBLIC); setId(skill, 11L); @@ -859,11 +859,11 @@ class SkillPublishServiceTest { ); assertEquals("1.2.4", result.version().getVersion()); - assertEquals(SkillVersionStatus.PUBLISHED, result.version().getStatus()); - assertEquals(Instant.now(CLOCK), result.version().getPublishedAt()); - assertEquals(30L, skill.getLatestVersionId()); - verify(reviewTaskRepository, never()).save(any()); - verify(eventPublisher).publishEvent(any(SkillPublishedEvent.class)); + // Rerelease for PUBLIC skill should go to PENDING_REVIEW (respecting visibility rules) + assertEquals(SkillVersionStatus.PENDING_REVIEW, result.version().getStatus()); + // Review task should be created for PUBLIC skill + verify(reviewTaskRepository).save(any()); + verify(eventPublisher, never()).publishEvent(any(SkillPublishedEvent.class)); verify(skillPackageValidator).validate(argThat(entries -> entries.size() == 2 && entries.stream().anyMatch(entry -> @@ -896,6 +896,76 @@ class SkillPublishServiceTest { )); } + @Test + void testRereleasePublishedVersion_PrivateSkill_ShouldGoToUploaded() throws Exception { + String publisherId = "user-100"; + Skill skill = new Skill(1L, "demo-skill", publisherId, SkillVisibility.PRIVATE); + setId(skill, 11L); + skill.setDisplayName("Demo Skill"); + skill.setSummary("Original summary"); + Namespace namespace = new Namespace("global", "Global", "owner"); + setId(namespace, 1L); + + SkillVersion sourceVersion = new SkillVersion(skill.getId(), "1.2.3", publisherId); + setId(sourceVersion, 21L); + sourceVersion.setStatus(SkillVersionStatus.PUBLISHED); + sourceVersion.setPublishedAt(Instant.parse("2026-03-15T10:00:00Z")); + + String sourceSkillMd = """ + --- + name: Demo Skill + description: Original summary + version: 1.2.3 + --- + Hello world + """; + + SkillFile skillMdFile = new SkillFile(sourceVersion.getId(), "SKILL.md", (long) sourceSkillMd.getBytes(StandardCharsets.UTF_8).length, "text/markdown", "hash1", "skills/11/21/SKILL.md"); + + SkillMetadata rereleaseMetadata = new SkillMetadata( + "Demo Skill", + "Original summary", + "1.2.4", + "Hello world", + Map.of("name", "Demo Skill", "description", "Original summary", "version", "1.2.4")); + + when(skillRepository.findById(skill.getId())).thenReturn(Optional.of(skill)); + when(namespaceRepository.findById(skill.getNamespaceId())).thenReturn(Optional.of(namespace)); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillVersionRepository.findBySkillIdAndVersion(skill.getId(), "1.2.3")).thenReturn(Optional.of(sourceVersion)); + when(skillVersionRepository.findBySkillIdAndVersion(skill.getId(), "1.2.4")).thenReturn(Optional.empty()); + when(skillFileRepository.findByVersionId(sourceVersion.getId())).thenReturn(List.of(skillMdFile)); + when(objectStorageService.getObject(skillMdFile.getStorageKey())).thenReturn(new java.io.ByteArrayInputStream(sourceSkillMd.getBytes(StandardCharsets.UTF_8))); + when(skillPackageValidator.validate(anyList())).thenReturn(ValidationResult.pass()); + when(skillMetadataParser.parse(anyString())).thenReturn(rereleaseMetadata); + when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass()); + when(skillVersionRepository.save(any(SkillVersion.class))).thenAnswer(invocation -> { + SkillVersion saved = invocation.getArgument(0); + if (saved.getId() == null) { + setId(saved, 30L); + } + return saved; + }); + when(skillRepository.save(any())).thenReturn(skill); + + SkillPublishService.PublishResult result = service.rereleasePublishedVersion( + skill.getId(), + "1.2.3", + "1.2.4", + publisherId, + Map.of(skill.getNamespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER) + ); + + assertEquals("1.2.4", result.version().getVersion()); + // Rerelease for PRIVATE skill should go to UPLOADED status + assertEquals(SkillVersionStatus.UPLOADED, result.version().getStatus()); + // No review task for PRIVATE skill + verify(reviewTaskRepository, never()).save(any()); + verify(eventPublisher, never()).publishEvent(any(SkillPublishedEvent.class)); + // latestVersionId should be updated for PRIVATE skill + assertEquals(30L, skill.getLatestVersionId()); + } + @Test void testPublishFromEntries_ShouldRejectWhenOtherOwnerHasPublishedSkill() throws Exception { String namespaceSlug = "test-ns"; @@ -1017,8 +1087,8 @@ class SkillPublishServiceTest { service.publishFromEntries(namespaceSlug, entries, publisherId, SkillVisibility.PUBLIC, Set.of()); - // Verify pending version was withdrawn to DRAFT - assertEquals(SkillVersionStatus.DRAFT, pendingV1.getStatus()); + // Verify pending version was withdrawn to UPLOADED (not DRAFT, so it remains visible) + assertEquals(SkillVersionStatus.UPLOADED, pendingV1.getStatus()); verify(reviewTaskRepository).delete(pendingTask); verify(skillVersionRepository).save(pendingV1); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java new file mode 100644 index 00000000..4f8f9565 --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java @@ -0,0 +1,272 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.*; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.ApplicationEventPublisher; + +import java.time.Clock; +import java.util.Map; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Unit tests for {@link SkillReviewSubmitService}. + */ +@ExtendWith(MockitoExtension.class) +class SkillReviewSubmitServiceTest { + + @Mock + private SkillRepository skillRepository; + + @Mock + private SkillVersionRepository skillVersionRepository; + + @Mock + private ReviewTaskRepository reviewTaskRepository; + + @Mock + private ApplicationEventPublisher eventPublisher; + + private SkillReviewSubmitService service; + + @BeforeEach + void setUp() { + service = new SkillReviewSubmitService( + skillRepository, + skillVersionRepository, + reviewTaskRepository, + null, // namespaceMemberRepository not used in these tests + eventPublisher, + Clock.systemUTC() + ); + } + + @Nested + @DisplayName("submitForReview") + class SubmitForReviewTests { + + @Test + @DisplayName("should transition UPLOADED version to PENDING_REVIEW") + void shouldTransitionToPendingReview() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + Long namespaceId = 10L; + + Skill skill = createSkill(skillId, userId, namespaceId, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + when(reviewTaskRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + Map roles = Map.of(); + + // When + service.submitForReview(skillId, versionId, SkillVisibility.PUBLIC, userId, roles); + + // Then + assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); + assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); + verify(reviewTaskRepository).save(any(ReviewTask.class)); + } + + @Test + @DisplayName("should accept DRAFT version (legacy compatibility)") + void shouldAcceptDraftForLegacyCompatibility() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + Long namespaceId = 10L; + + Skill skill = createSkill(skillId, userId, namespaceId, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.DRAFT); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + when(reviewTaskRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + Map roles = Map.of(); + + // When + service.submitForReview(skillId, versionId, SkillVisibility.PUBLIC, userId, roles); + + // Then + assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); + assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); + verify(reviewTaskRepository).save(any(ReviewTask.class)); + } + + @Test + @DisplayName("should reject when version is neither UPLOADED nor DRAFT") + void shouldRejectWhenNotUploadedOrDraft() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + + Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.PUBLISHED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When/Then + assertThrows(DomainBadRequestException.class, + () -> service.submitForReview(skillId, versionId, SkillVisibility.PUBLIC, userId, Map.of())); + } + + @Test + @DisplayName("should reject when user is not owner") + void shouldRejectWhenNotOwner() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String ownerId = "owner-1"; + String otherUserId = "other-user"; + + Skill skill = createSkill(skillId, ownerId, 10L, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When/Then + assertThrows(DomainForbiddenException.class, + () -> service.submitForReview(skillId, versionId, SkillVisibility.PUBLIC, otherUserId, Map.of())); + } + } + + @Nested + @DisplayName("confirmPublish") + class ConfirmPublishTests { + + @Test + @DisplayName("should transition UPLOADED version to PUBLISHED for PRIVATE skill") + void shouldTransitionToPublished() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + Long namespaceId = 10L; + + Skill skill = createSkill(skillId, userId, namespaceId, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When + service.confirmPublish(skillId, versionId, userId, Map.of()); + + // Then + assertEquals(SkillVersionStatus.PUBLISHED, version.getStatus()); + assertNotNull(version.getPublishedAt()); + assertEquals(versionId, skill.getLatestVersionId()); + verify(skillRepository).save(skill); + } + + @Test + @DisplayName("should transition DRAFT version to PUBLISHED for PRIVATE skill (legacy compatibility)") + void shouldTransitionDraftToPublished() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + Long namespaceId = 10L; + + Skill skill = createSkill(skillId, userId, namespaceId, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.DRAFT); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When + service.confirmPublish(skillId, versionId, userId, Map.of()); + + // Then + assertEquals(SkillVersionStatus.PUBLISHED, version.getStatus()); + assertNotNull(version.getPublishedAt()); + assertEquals(versionId, skill.getLatestVersionId()); + verify(skillRepository).save(skill); + } + + @Test + @DisplayName("should reject when skill is not PRIVATE") + void shouldRejectWhenNotPrivate() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + + Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PUBLIC); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When/Then + assertThrows(DomainBadRequestException.class, + () -> service.confirmPublish(skillId, versionId, userId, Map.of())); + } + + @Test + @DisplayName("should reject when version is neither UPLOADED nor DRAFT") + void shouldRejectWhenNotUploadedOrDraft() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + + Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.PUBLISHED); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + + // When/Then + assertThrows(DomainBadRequestException.class, + () -> service.confirmPublish(skillId, versionId, userId, Map.of())); + } + } + + private Skill createSkill(Long id, String ownerId, Long namespaceId, SkillVisibility visibility) { + Skill skill = new Skill(namespaceId, "test-skill", ownerId, visibility); + setField(skill, "id", id); + return skill; + } + + private SkillVersion createVersion(Long id, Long skillId, SkillVersionStatus status) { + SkillVersion version = new SkillVersion(skillId, "1.0.0", "user-1"); + setField(version, "id", id); + version.setStatus(status); + return version; + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 928036a0..714111e3 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -476,6 +476,36 @@ export const skillLifecycleApi = { body: JSON.stringify({ targetVersion }), }) }, + + /** + * Submit an UPLOADED version for review. + * Transitions version status from UPLOADED to PENDING_REVIEW. + */ + async submitForReview(namespace: string, slug: string, version: string, targetVisibility: 'PUBLIC' | 'NAMESPACE_ONLY'): Promise { + const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace + await fetchJson(`${WEB_API_PREFIX}/skills/${cleanNamespace}/${encodeURIComponent(slug)}/submit-review`, { + method: 'POST', + headers: await ensureCsrfHeaders({ + 'Content-Type': 'application/json', + }), + body: JSON.stringify({ version, targetVisibility }), + }) + }, + + /** + * Confirm publish for a PRIVATE skill version. + * Transitions version status from UPLOADED to PUBLISHED without review. + */ + async confirmPublish(namespace: string, slug: string, version: string): Promise { + const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace + await fetchJson(`${WEB_API_PREFIX}/skills/${cleanNamespace}/${encodeURIComponent(slug)}/confirm-publish`, { + method: 'POST', + headers: await ensureCsrfHeaders({ + 'Content-Type': 'application/json', + }), + body: JSON.stringify({ version }), + }) + }, } function normalizeNamespaceSlug(namespace: string): string { diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 9835b1b6..3af596c9 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -777,6 +777,7 @@ "versionStatusDraft": "Draft", "versionStatusScanning": "Scanning", "versionStatusScanFailed": "Scan Failed", + "versionStatusUploaded": "Uploaded", "versionStatusPendingReview": "Pending Review", "versionStatusPublished": "Published", "versionStatusRejected": "Rejected", @@ -825,6 +826,18 @@ "withdrawReviewSuccessTitle": "Review withdrawn", "withdrawReviewSuccessDescription": "Version {{version}} has been withdrawn from review.", "withdrawReviewErrorTitle": "Failed to withdraw review", + "confirmPublish": "Confirm Publish", + "confirmPublishDialogTitle": "Confirm publish", + "confirmPublishDialogDescription": "Publish version {{version}} as a private skill? It will be available for you to download and install, but not visible on the marketplace.", + "confirmPublishSuccessTitle": "Version published", + "confirmPublishSuccessDescription": "Version {{version}} has been published as a private skill.", + "confirmPublishErrorTitle": "Failed to confirm publish", + "submitReview": "Submit for Review", + "submitReviewDialogTitle": "Submit for review", + "submitReviewDialogDescription": "Submit version {{version}} for public review? Once approved, it will be visible on the marketplace.", + "submitReviewSuccessTitle": "Submitted for review", + "submitReviewSuccessDescription": "Version {{version}} has been submitted for review.", + "submitReviewErrorTitle": "Failed to submit for review", "deleteVersion": "Delete Version", "deleteVersionConfirmTitle": "Delete version", "deleteVersionConfirmDescription": "Version {{version}} cannot be recovered after deletion. Continue?", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 7a8e187b..17c2eb9a 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -777,6 +777,7 @@ "versionStatusDraft": "草稿", "versionStatusScanning": "安全扫描中", "versionStatusScanFailed": "扫描失败", + "versionStatusUploaded": "已上传", "versionStatusPendingReview": "审核中", "versionStatusPublished": "已发布", "versionStatusRejected": "已拒绝", @@ -825,6 +826,19 @@ "withdrawReviewSuccessTitle": "已撤销审核", "withdrawReviewSuccessDescription": "版本 {{version}} 已撤销审核。", "withdrawReviewErrorTitle": "撤销审核失败", + "confirmPublish": "确认发布", + "confirmPublishDialogTitle": "确认发布", + "confirmPublishDialogDescription": "将版本 {{version}} 发布为私有技能?发布后您可以下载和安装,但不会在市场展示。", + "confirmPublishSuccessTitle": "版本已发布", + "confirmPublishSuccessDescription": "版本 {{version}} 已发布为私有技能。", + "confirmPublishErrorTitle": "确认发布失败", + "submitReview": "提交审核", + "submitReviewDialogTitle": "提交审核", + "submitReviewDialogDescription": "将版本 {{version}} 提交公开审核?审核通过后将在市场展示。", + "submitReviewSuccessTitle": "已提交审核", + "submitReviewSuccessDescription": "版本 {{version}} 已提交审核。", + "submitReviewErrorTitle": "提交审核失败", + "withdrawReviewErrorTitle": "撤销审核失败", "deleteVersion": "删除版本", "deleteVersionConfirmTitle": "确认删除版本", "deleteVersionConfirmDescription": "版本 {{version}} 删除后无法恢复,确定继续吗?", diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 74cd0c65..5f437cd7 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -83,6 +83,9 @@ export function MySkillsPage() { if (status === 'SCAN_FAILED') { return t('mySkills.statusScanFailed') } + if (status === 'UPLOADED') { + return t('skillDetail.versionStatusUploaded') + } return status } @@ -108,6 +111,9 @@ export function MySkillsPage() { if (status === 'SCAN_FAILED') { return 'status-pill status-pill--rejected' } + if (status === 'UPLOADED') { + return 'status-pill status-pill--review' + } return 'status-pill' } diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index deed65d3..067a171f 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -112,6 +112,8 @@ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useRereleaseSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), useUnarchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), useWithdrawSkillReview: () => ({ mutateAsync: vi.fn(), isPending: false }), + useSubmitForReview: () => ({ mutateAsync: vi.fn(), isPending: false }), + useConfirmPublish: () => ({ mutateAsync: vi.fn(), isPending: false }), })) vi.mock('@/shared/hooks/use-label-queries', () => ({ diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 990396b4..d28b4f75 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -53,6 +53,8 @@ import { useRereleaseSkillVersion, useUnarchiveSkill, useWithdrawSkillReview, + useSubmitForReview, + useConfirmPublish, } from '@/shared/hooks/use-skill-queries' import { useSubmitPromotion } from '@/shared/hooks/use-user-queries' @@ -115,6 +117,8 @@ export function SkillDetailPage() { const [rereleaseTarget, setRereleaseTarget] = useState(null) const [targetVersionInput, setTargetVersionInput] = useState('') const [diffSourceVersion, setDiffSourceVersion] = useState(null) + const [confirmPublishTarget, setConfirmPublishTarget] = useState(null) + const [submitReviewTarget, setSubmitReviewTarget] = useState(null) const [diffCompareVersion, setDiffCompareVersion] = useState(null) const [isOverviewExpanded, setIsOverviewExpanded] = useState(false) const [isOverviewCollapsible, setIsOverviewCollapsible] = useState(false) @@ -260,6 +264,8 @@ export function SkillDetailPage() { const rereleaseVersionMutation = useRereleaseSkillVersion() const submitPromotionMutation = useSubmitPromotion() const reportMutation = useSubmitSkillReport(namespace, slug) + const submitForReviewMutation = useSubmitForReview() + const confirmPublishMutation = useConfirmPublish() const triggerBrowserDownload = (url: string) => { const link = document.createElement('a') @@ -380,6 +386,7 @@ export function SkillDetailPage() { DRAFT: t('skillDetail.versionStatusDraft'), SCANNING: t('skillDetail.versionStatusScanning'), SCAN_FAILED: t('skillDetail.versionStatusScanFailed'), + UPLOADED: t('skillDetail.versionStatusUploaded'), PENDING_REVIEW: t('skillDetail.versionStatusPendingReview'), PUBLISHED: t('skillDetail.versionStatusPublished'), REJECTED: t('skillDetail.versionStatusRejected'), @@ -388,7 +395,7 @@ export function SkillDetailPage() { return status ? (map[status] ?? status) : '' } - const canDeleteVersion = (status?: string) => status === 'DRAFT' || status === 'REJECTED' || status === 'SCAN_FAILED' + const canDeleteVersion = (status?: string) => status === 'DRAFT' || status === 'REJECTED' || status === 'SCAN_FAILED' || status === 'UPLOADED' const isLastVersion = versions?.length === 1 const canWithdrawVersion = (status?: string) => status === 'PENDING_REVIEW' const canRereleaseVersion = (status?: string) => status === 'PUBLISHED' @@ -529,6 +536,40 @@ export function SkillDetailPage() { } } + const handleConfirmPublish = async () => { + if (!confirmPublishTarget) { + return + } + try { + await confirmPublishMutation.mutateAsync({ namespace, slug, version: confirmPublishTarget }) + toast.success( + t('skillDetail.confirmPublishSuccessTitle'), + t('skillDetail.confirmPublishSuccessDescription', { version: confirmPublishTarget }), + ) + setConfirmPublishTarget(null) + } catch (error) { + toast.error(t('skillDetail.confirmPublishErrorTitle'), error instanceof Error ? error.message : '') + throw error + } + } + + const handleSubmitForReview = async () => { + if (!submitReviewTarget) { + return + } + try { + await submitForReviewMutation.mutateAsync({ namespace, slug, version: submitReviewTarget, targetVisibility: 'PUBLIC' }) + toast.success( + t('skillDetail.submitReviewSuccessTitle'), + t('skillDetail.submitReviewSuccessDescription', { version: submitReviewTarget }), + ) + setSubmitReviewTarget(null) + } catch (error) { + toast.error(t('skillDetail.submitReviewErrorTitle'), error instanceof Error ? error.message : '') + throw error + } + } + const handleOpenRerelease = (version: string) => { setRereleaseTarget(version) setTargetVersionInput(suggestNextVersion(version)) @@ -884,6 +925,24 @@ export function SkillDetailPage() { {t('skillDetail.withdrawReview')} )} + {skill.canManageLifecycle && version.status === 'UPLOADED' && skill.visibility === 'PRIVATE' && ( + + )} + {skill.canManageLifecycle && version.status === 'UPLOADED' && skill.visibility === 'PRIVATE' && ( + + )} {version.changelog && ( @@ -1370,6 +1429,32 @@ export function SkillDetailPage() { + { + if (!open) { + setConfirmPublishTarget(null) + } + }} + title={t('skillDetail.confirmPublishDialogTitle')} + description={confirmPublishTarget ? t('skillDetail.confirmPublishDialogDescription', { version: confirmPublishTarget }) : ''} + confirmText={t('skillDetail.confirmPublish')} + onConfirm={handleConfirmPublish} + /> + + { + if (!open) { + setSubmitReviewTarget(null) + } + }} + title={t('skillDetail.submitReviewDialogTitle')} + description={submitReviewTarget ? t('skillDetail.submitReviewDialogDescription', { version: submitReviewTarget }) : ''} + confirmText={t('skillDetail.submitReview')} + onConfirm={handleSubmitForReview} + /> +

{ diff --git a/web/src/shared/hooks/use-skill-queries.ts b/web/src/shared/hooks/use-skill-queries.ts index c72e738c..4784ada5 100644 --- a/web/src/shared/hooks/use-skill-queries.ts +++ b/web/src/shared/hooks/use-skill-queries.ts @@ -216,3 +216,41 @@ export function useRereleaseSkillVersion() { }, }) } + +/** + * Submit an UPLOADED version for review. + * Transitions version status from UPLOADED to PENDING_REVIEW. + */ +export function useSubmitForReview() { + const queryClient = useQueryClient() + + return useMutation({ + mutationFn: ({ namespace, slug, version, targetVisibility }: { namespace: string; slug: string; version: string; targetVisibility: 'PUBLIC' | 'NAMESPACE_ONLY' }) => + skillLifecycleApi.submitForReview(namespace, slug, version, targetVisibility), + onSuccess: (_data, variables) => { + queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug, 'versions'] }) + queryClient.invalidateQueries({ queryKey: ['skills'] }) + }, + }) +} + +/** + * Confirm publish for a PRIVATE skill version. + * Transitions version status from UPLOADED to PUBLISHED without review. + */ +export function useConfirmPublish() { + const queryClient = useQueryClient() + + return useMutation({ + mutationFn: ({ namespace, slug, version }: { namespace: string; slug: string; version: string }) => + skillLifecycleApi.confirmPublish(namespace, slug, version), + onSuccess: (_data, variables) => { + queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug, 'versions'] }) + queryClient.invalidateQueries({ queryKey: ['skills'] }) + }, + }) +} From 1184e00a00bb94fbe6f583857ff2bddef6562ca1 Mon Sep 17 00:00:00 2001 From: dongmucat <70678707+dongmucat@users.noreply.github.com> Date: Mon, 13 Apr 2026 20:26:42 +0800 Subject: [PATCH 2/4] fix(compat): support namespace-aware clawhub publish (#291) --- README.md | 10 +- README_zh.md | 10 +- docs/openclaw-integration-en.md | 9 +- docs/openclaw-integration.md | 9 +- .../compat/ClawHubCompatAppService.java | 29 +++- .../support/MultipartPackageExtractor.java | 1 + .../compat/ClawHubCompatControllerTest.java | 130 ++++++++++++++++++ 7 files changed, 189 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index baa7d8e0..e9f0c3fe 100644 --- a/README.md +++ b/README.md @@ -397,10 +397,16 @@ npx clawhub search email npx clawhub install my-skill npx clawhub install my-namespace--my-skill -# Publish a skill -npx clawhub publish ./my-skill +# Publish to global namespace +npx clawhub publish ./my-skill --slug my-skill --version 1.0.0 + +# Publish to a team namespace such as my-space +npx clawhub publish ./my-skill --slug my-space--my-skill --version 1.0.0 ``` +`my-space--my-skill` is the canonical compat slug. SkillHub parses it as +namespace `my-space` plus skill slug `my-skill`. + > 💡 **Tip**: The above commands are not only applicable to OpenClaw, but also to other CLI Coding Agents or Agent assistants by specifying the installation directory (`--dir`). For example: `npx clawhub --dir ~/.claude/skills install my-skill` 📖 **[Complete OpenClaw Integration Guide →](./docs/openclaw-integration.md)** diff --git a/README_zh.md b/README_zh.md index 8a7c7409..4009e4cf 100644 --- a/README_zh.md +++ b/README_zh.md @@ -331,10 +331,16 @@ npx clawhub search email npx clawhub install my-skill npx clawhub install my-namespace--my-skill -# 发布技能 -npx clawhub publish ./my-skill +# 发布到 global 空间 +npx clawhub publish ./my-skill --slug my-skill --version 1.0.0 + +# 发布到如 my-space 这样的团队空间 +npx clawhub publish ./my-skill --slug my-space--my-skill --version 1.0.0 ``` +其中 `my-space--my-skill` 是兼容层使用的 canonical slug,SkillHub 会将其解析为 +namespace `my-space` 和 skill slug `my-skill`。 + > 💡 **提示**:上述命令不仅适用于 OpenClaw,通过指定安装目录(`--dir`),也可适用于其他的 CLI Coding Agent 或 Agent 助手。例如:`npx clawhub --dir ~/.claude/skills install my-skill` 📖 **[完整 OpenClaw 集成指南 →](./docs/openclaw-integration.md)** diff --git a/docs/openclaw-integration-en.md b/docs/openclaw-integration-en.md index 0e1b54d9..32cb2878 100644 --- a/docs/openclaw-integration-en.md +++ b/docs/openclaw-integration-en.md @@ -110,8 +110,11 @@ npx clawhub list --help ### 5. Publish Skills ```bash -# Publish skill (requires appropriate permissions) +# Publish to the global namespace (requires appropriate permissions) npx clawhub publish ./my-skill --slug my-skill --name "My Skill" --version 1.0.0 + +# Publish to a team namespace such as my-space +npx clawhub publish ./my-skill --slug my-space--my-skill --name "My Skill" --version 1.0.0 npx clawhub sync --all # Upload all skills in current folder # Help @@ -119,6 +122,10 @@ npx clawhub publish --help npx clawhub sync --help ``` +Notes: +- `my-space--my-skill` is the canonical compatibility slug. SkillHub parses it as namespace `my-space` plus skill slug `my-skill` +- To avoid mismatches between CLI display text and the final persisted coordinate, keep the `name` in `SKILL.md` aligned with the canonical slug suffix + ## API Endpoints SkillHub compatibility layer provides the following endpoints: diff --git a/docs/openclaw-integration.md b/docs/openclaw-integration.md index 17fd165b..f85f588e 100644 --- a/docs/openclaw-integration.md +++ b/docs/openclaw-integration.md @@ -110,8 +110,11 @@ npx clawhub list --help ### 5. 发布技能 ```bash -# 发布技能(需要相应权限) +# 发布到 global 空间(需要相应权限) npx clawhub publish ./my-skill --slug my-skill --name "My Skill" --version 1.0.0 + +# 发布到如 my-space 这样的团队空间 +npx clawhub publish ./my-skill --slug my-space--my-skill --name "My Skill" --version 1.0.0 npx clawhub sync --all # 上传当前文件夹中所有的 skill # 使用帮助 @@ -119,6 +122,10 @@ npx clawhub publish --help npx clawhub sync --help ``` +说明: +- `my-space--my-skill` 是兼容层 canonical slug,SkillHub 会将其解析为 namespace `my-space` 和 skill slug `my-skill` +- 为避免 CLI 展示与服务端最终坐标不一致,建议让 `SKILL.md` 中的 `name` 与 canonical slug 后半段保持一致 + ## API 端点说明 SkillHub 兼容层提供以下端点: diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java index 7e31f386..57609296 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import org.slf4j.MDC; import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; import org.springframework.web.multipart.MultipartFile; /** @@ -37,6 +38,8 @@ import org.springframework.web.multipart.MultipartFile; @Service public class ClawHubCompatAppService { + private static final String GLOBAL_NAMESPACE = "global"; + private final CanonicalSlugMapper mapper; private final SkillSearchAppService skillSearchAppService; private final SkillQueryService skillQueryService; @@ -268,7 +271,7 @@ public class ClawHubCompatAppService { String clientIp, String userAgent) throws IOException { MultipartPackageExtractor.ExtractedPackage extracted = multipartPackageExtractor.extract(files, payloadJson); - String namespace = determineNamespace(principal, extracted.payload()); + String namespace = determineNamespace(extracted.payload()); SkillPublishService.PublishResult result = skillPublishService.publishFromEntries( namespace, extracted.entries(), @@ -371,8 +374,28 @@ public class ClawHubCompatAppService { ); } - private String determineNamespace(PlatformPrincipal principal, MultipartPackageExtractor.PublishPayload payload) { - return "global"; + private String determineNamespace(MultipartPackageExtractor.PublishPayload payload) { + if (payload == null) { + return GLOBAL_NAMESPACE; + } + + if (StringUtils.hasText(payload.namespace())) { + return normalizeNamespace(payload.namespace()); + } + + if (StringUtils.hasText(payload.slug()) && payload.slug().contains("--")) { + return mapper.fromCanonical(payload.slug()).namespace(); + } + + return GLOBAL_NAMESPACE; + } + + private String normalizeNamespace(String namespace) { + String trimmed = namespace.trim(); + if (trimmed.startsWith("@")) { + return trimmed.substring(1); + } + return trimmed; } private void recordCompatPublishAudit(String userId, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java index 6e3e6a1c..0a9fc793 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/MultipartPackageExtractor.java @@ -30,6 +30,7 @@ public class MultipartPackageExtractor { } public record PublishPayload( + String namespace, String slug, String displayName, String version, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java index 1a03b672..1a1e2a6c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java @@ -2,35 +2,46 @@ package com.iflytek.skillhub.compat; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse; import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.service.SkillSearchAppService; import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; import java.time.Instant; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.MockMvc; import java.util.List; import java.util.Optional; import java.util.Set; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; @SpringBootTest @@ -56,6 +67,12 @@ class ClawHubCompatControllerTest { @MockBean private CompatSkillLookupService compatSkillLookupService; + @MockBean + private SkillPublishService skillPublishService; + + @MockBean + private AuditLogService auditLogService; + @Test void search_returns_mapped_results() throws Exception { when(skillSearchAppService.search("test", null, "relevance", 0, 20, null, null)) @@ -204,9 +221,122 @@ class ClawHubCompatControllerTest { .andExpect(jsonPath("$.user.image").value("https://example.com/avatar.png")); } + @Test + void publish_skill_with_canonical_slug_routes_to_namespace_publish() throws Exception { + SkillVersion version = publishVersion("1.0.0", 34L); + given(skillPublishService.publishFromEntries( + eq("team-ai"), + anyList(), + eq("user-42"), + eq(SkillVisibility.PUBLIC), + eq(Set.of("SUPER_ADMIN")), + eq(false))) + .willReturn(new SkillPublishService.PublishResult(12L, "my-skill", version)); + + mockMvc.perform(multipart("/api/v1/skills") + .file(skillMdFile()) + .param("payload", """ + {"slug":"team-ai--my-skill","displayName":"My Skill","version":"1.0.0","acceptLicenseTerms":true,"tags":["latest"]} + """) + .with(authentication(superAdminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ok").value(true)) + .andExpect(jsonPath("$.skillId").value("12")) + .andExpect(jsonPath("$.versionId").value("34")); + } + + @Test + void publish_skill_with_plain_slug_defaults_to_global_namespace() throws Exception { + SkillVersion version = publishVersion("1.0.0", 35L); + given(skillPublishService.publishFromEntries( + eq("global"), + anyList(), + eq("user-42"), + eq(SkillVisibility.PUBLIC), + eq(Set.of("SUPER_ADMIN")), + eq(false))) + .willReturn(new SkillPublishService.PublishResult(13L, "my-skill", version)); + + mockMvc.perform(multipart("/api/v1/skills") + .file(skillMdFile()) + .param("payload", """ + {"slug":"my-skill","displayName":"My Skill","version":"1.0.0","acceptLicenseTerms":true,"tags":["latest"]} + """) + .with(authentication(superAdminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ok").value(true)) + .andExpect(jsonPath("$.skillId").value("13")) + .andExpect(jsonPath("$.versionId").value("35")); + } + + @Test + void publish_skill_with_payload_namespace_uses_explicit_namespace() throws Exception { + SkillVersion version = publishVersion("1.0.0", 36L); + given(skillPublishService.publishFromEntries( + eq("team-explicit"), + anyList(), + eq("user-42"), + eq(SkillVisibility.PUBLIC), + eq(Set.of("SUPER_ADMIN")), + eq(false))) + .willReturn(new SkillPublishService.PublishResult(14L, "my-skill", version)); + + mockMvc.perform(multipart("/api/v1/skills") + .file(skillMdFile()) + .param("payload", """ + {"namespace":"@team-explicit","slug":"my-skill","displayName":"My Skill","version":"1.0.0","acceptLicenseTerms":true,"tags":["latest"]} + """) + .with(authentication(superAdminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ok").value(true)) + .andExpect(jsonPath("$.skillId").value("14")) + .andExpect(jsonPath("$.versionId").value("36")); + } + private CompatSkillLookupService.CompatSkillContext legacyCompatContext(String namespaceSlug, String skillSlug) { Namespace namespace = new Namespace(namespaceSlug, namespaceSlug, "tester"); Skill skill = new Skill(1L, skillSlug, "tester", SkillVisibility.PUBLIC); return new CompatSkillLookupService.CompatSkillContext(namespace, skill, Optional.empty()); } + + private MockMultipartFile skillMdFile() { + return new MockMultipartFile( + "files", + "SKILL.md", + "text/markdown", + """ + --- + name: my-skill + description: Demo skill + version: 1.0.0 + --- + """.getBytes(StandardCharsets.UTF_8) + ); + } + + private SkillVersion publishVersion(String versionValue, long versionId) { + SkillVersion version = new SkillVersion(12L, versionValue, "user-42"); + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + ReflectionTestUtils.setField(version, "id", versionId); + return version; + } + + private UsernamePasswordAuthenticationToken superAdminAuth() { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", + "tester", + "tester@example.com", + "https://example.com/avatar.png", + "github", + Set.of("SUPER_ADMIN") + ); + return new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) + ); + } } From 38ebb131335e72968a084db01ff06afac09e0e94 Mon Sep 17 00:00:00 2001 From: dongmucat <70678707+dongmucat@users.noreply.github.com> Date: Mon, 13 Apr 2026 20:27:00 +0800 Subject: [PATCH 3/4] =?UTF-8?q?feat(auth):=20=E9=82=AE=E7=AE=B1=E9=AA=8C?= =?UTF-8?q?=E8=AF=81=E7=A0=81=E9=87=8D=E7=BD=AE=E5=AF=86=E7=A0=81=E4=B8=8E?= =?UTF-8?q?=20SMTP=20=E9=85=8D=E7=BD=AE=E6=94=AF=E6=8C=81=20(#273)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(auth): add email-based password reset with SMTP config docs * test(e2e): stabilize password reset flow * test(e2e): isolate password reset rate limits * test(ci): stabilize backend and register e2e * docs(auth): sanitize smtp setup examples --- .env.release.draft | 13 + .env.release.example | 13 + compose.release.yml | 11 + docs/09-deployment.md | 1 + docs/19-smtp-password-reset-email-setup.md | 317 ++++++++++++++++++ .../controller/LocalAuthController.java | 22 +- .../admin/UserManagementController.java | 16 + .../skillhub/dto/LocalRegisterRequest.java | 1 + .../dto/PasswordResetConfirmRequest.java | 18 + .../skillhub/dto/PasswordResetRequestDto.java | 13 + .../src/main/resources/application.yml | 18 + .../migration/V39__password_reset_request.sql | 20 ++ .../src/main/resources/messages.properties | 14 + .../src/main/resources/messages_zh.properties | 14 + .../controller/LocalAuthControllerTest.java | 83 +++++ .../NamespaceWorkflowContractTest.java | 70 ++-- .../admin/UserManagementControllerTest.java | 22 ++ .../src/test/resources/application-test.yml | 3 +- server/skillhub-auth/pom.xml | 9 + .../skillhub/auth/local/LocalAuthService.java | 2 +- .../auth/local/PasswordResetProperties.java | 38 +++ .../auth/local/PasswordResetService.java | 244 ++++++++++++++ .../skillhub/auth/local/package-info.java | 2 +- .../auth/local/LocalAuthServiceTest.java | 9 + .../auth/local/PasswordResetServiceTest.java | 218 ++++++++++++ .../domain/auth/PasswordResetRequest.java | 108 ++++++ .../auth/PasswordResetRequestRepository.java | 17 + .../skillhub/domain/auth/package-info.java | 4 + .../PasswordResetRequestJpaRepository.java | 19 ++ web/e2e/helpers/auth-fixtures.ts | 9 + web/e2e/password-reset.spec.ts | 45 +++ web/e2e/register-email-required.spec.ts | 46 +++ web/e2e/register-login-validation.spec.ts | 54 +-- web/e2e/settings-pages.spec.ts | 7 + web/src/api/client.ts | 29 ++ web/src/api/generated/schema.d.ts | 128 ++++++- web/src/api/types.ts | 12 +- web/src/app/router.tsx | 8 + web/src/features/admin/use-admin-users.ts | 10 + web/src/i18n/locales/en.json | 34 +- web/src/i18n/locales/zh.json | 34 +- web/src/pages/admin/users.test.tsx | 1 + web/src/pages/admin/users.tsx | 44 ++- web/src/pages/login.tsx | 5 + web/src/pages/register.tsx | 5 +- web/src/pages/reset-password.test.tsx | 54 +++ web/src/pages/reset-password.tsx | 187 +++++++++++ web/src/pages/settings/profile.test.ts | 11 + web/src/pages/settings/profile.tsx | 17 +- 49 files changed, 2005 insertions(+), 74 deletions(-) create mode 100644 docs/19-smtp-password-reset-email-setup.md create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetConfirmRequest.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetRequestDto.java create mode 100644 server/skillhub-app/src/main/resources/db/migration/V39__password_reset_request.sql create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetProperties.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequestRepository.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/package-info.java create mode 100644 server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PasswordResetRequestJpaRepository.java create mode 100644 web/e2e/password-reset.spec.ts create mode 100644 web/e2e/register-email-required.spec.ts create mode 100644 web/src/pages/reset-password.test.tsx create mode 100644 web/src/pages/reset-password.tsx diff --git a/.env.release.draft b/.env.release.draft index 8a0c28e2..40058266 100644 --- a/.env.release.draft +++ b/.env.release.draft @@ -80,3 +80,16 @@ DEVICE_AUTH_VERIFICATION_URI= # Leave both empty if you are not enabling GitHub login yet. OAUTH2_GITHUB_CLIENT_ID= OAUTH2_GITHUB_CLIENT_SECRET= + +# SMTP configuration for password reset verification emails. +SPRING_MAIL_HOST=smtp.example.com +SPRING_MAIL_PORT=587 +SPRING_MAIL_USERNAME=TODO_fill_smtp_username +SPRING_MAIL_PASSWORD=TODO_fill_smtp_password +SPRING_MAIL_SMTP_AUTH=true +SPRING_MAIL_SMTP_STARTTLS_ENABLE=true +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST= +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@example.com +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub diff --git a/.env.release.example b/.env.release.example index 366409e5..9b863c23 100644 --- a/.env.release.example +++ b/.env.release.example @@ -56,6 +56,19 @@ DEVICE_AUTH_VERIFICATION_URI= OAUTH2_GITHUB_CLIENT_ID= OAUTH2_GITHUB_CLIENT_SECRET= +# SMTP configuration for password reset verification emails. +SPRING_MAIL_HOST= +SPRING_MAIL_PORT=587 +SPRING_MAIL_USERNAME= +SPRING_MAIL_PASSWORD= +SPRING_MAIL_SMTP_AUTH=true +SPRING_MAIL_SMTP_STARTTLS_ENABLE=true +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST= +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@example.com +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub + # Security scanner is enabled by default. Set to false to disable scanning. SKILLHUB_SECURITY_SCANNER_ENABLED=true diff --git a/compose.release.yml b/compose.release.yml index cd51781f..831ac581 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -80,6 +80,17 @@ services: BOOTSTRAP_ADMIN_EMAIL: ${BOOTSTRAP_ADMIN_EMAIL:-admin@skillhub.local} OAUTH2_GITHUB_CLIENT_ID: ${OAUTH2_GITHUB_CLIENT_ID:-local-placeholder} OAUTH2_GITHUB_CLIENT_SECRET: ${OAUTH2_GITHUB_CLIENT_SECRET:-local-placeholder} + SPRING_MAIL_HOST: ${SPRING_MAIL_HOST:-} + SPRING_MAIL_PORT: ${SPRING_MAIL_PORT:-25} + SPRING_MAIL_USERNAME: ${SPRING_MAIL_USERNAME:-} + SPRING_MAIL_PASSWORD: ${SPRING_MAIL_PASSWORD:-} + SPRING_MAIL_SMTP_AUTH: ${SPRING_MAIL_SMTP_AUTH:-false} + SPRING_MAIL_SMTP_STARTTLS_ENABLE: ${SPRING_MAIL_SMTP_STARTTLS_ENABLE:-false} + SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE: ${SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE:-false} + SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST: ${SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST:-} + SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY: ${SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY:-PT10M} + SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS: ${SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS:-noreply@skillhub.local} + SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME: ${SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME:-SkillHub} volumes: - skillhub_storage:/var/lib/skillhub/storage depends_on: diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 13b159ae..a8da4204 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -195,6 +195,7 @@ docker compose --env-file .env.release -f compose.release.yml up -d - 外部对象存储通过 `SKILLHUB_STORAGE_S3_*` 注入 - 前端反代和运行时 API 地址通过 `SKILLHUB_API_UPSTREAM` / `SKILLHUB_WEB_API_BASE_URL` 注入 - 如果要开放真实登录,再补充 `OAUTH2_GITHUB_CLIENT_ID` / `OAUTH2_GITHUB_CLIENT_SECRET` +- 如果要启用密码重置验证码邮件,参见:`docs/19-smtp-password-reset-email-setup.md` ## 8 裸金属上线清单 diff --git a/docs/19-smtp-password-reset-email-setup.md b/docs/19-smtp-password-reset-email-setup.md new file mode 100644 index 00000000..07def9ef --- /dev/null +++ b/docs/19-smtp-password-reset-email-setup.md @@ -0,0 +1,317 @@ +# SkillHub SMTP 邮箱配置指南(验证码邮件) + +本文说明如何为 SkillHub 配置 SMTP,用于发送“密码重置验证码”邮件。 + +适用场景: +- 生产/预发布环境(`compose.release.yml` + `.env.release`) +- 本地联调环境(直接注入后端环境变量) + +补充说明: +- SMTP 本质是邮件传输协议,不是单一厂商产品。 +- 你可以使用企业邮箱、云邮箱或本地测试 SMTP 服务(例如 MailHog)作为 SMTP 服务端。 + +当前密码重置页面入口说明: +- 当前前端统一使用 `/reset-password` 页面。 +- 该页面同时包含“发送验证码”和“提交新密码”两步,不再单独使用 `/forgot-password`。 + +## 1. 需要配置的环境变量 + +以下变量已被后端读取: + +| 变量名 | 说明 | 示例 | +|---|---|---| +| `SPRING_MAIL_HOST` | SMTP 服务器地址 | `smtp.example.com` | +| `SPRING_MAIL_PORT` | SMTP 端口 | `465` | +| `SPRING_MAIL_USERNAME` | SMTP 用户名 | `noreply@example.com` | +| `SPRING_MAIL_PASSWORD` | SMTP 密码/授权码 | `xxxxxx` | +| `SPRING_MAIL_SMTP_AUTH` | 是否启用 SMTP AUTH | `true` | +| `SPRING_MAIL_SMTP_STARTTLS_ENABLE` | 是否启用 STARTTLS | `false` | +| `SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE` | 是否启用 SMTP SSL 直连 | `true` | +| `SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST` | SSL 信任主机(用于规避部分环境下证书链校验失败) | `smtp.mail.example` | +| `SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY` | 验证码有效期(ISO-8601 Duration) | `PT10M` | +| `SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS` | 发件人邮箱 | `noreply@example.com` | +| `SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME` | 发件人名称 | `SkillHub` | + +说明: +- 当前文档统一按 `465 + SSL` 配置,不再展开 `587 + STARTTLS` 方案。 +- 使用 `465` 时配置:`STARTTLS=false`、`SSL_ENABLE=true`。 +- 若出现 `PKIX path building failed` / `SSLHandshakeException`,可尝试增加 `SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=`(本地联调常用)。 +- 生产环境默认不建议配置 `SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST`,仅在证书链异常时临时启用。 +- `SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY` 支持如 `PT5M`、`PT10M`、`PT30M`。 + +## 1.1 配置方案速查(推荐) + +### A. 通用 SMTP 邮箱(本地直连真实邮箱) + +```dotenv +SPRING_MAIL_HOST=smtp.mail.example +SPRING_MAIL_PORT=465 +SPRING_MAIL_USERNAME=mailer@example.com +SPRING_MAIL_PASSWORD=your-smtp-app-password +SPRING_MAIL_SMTP_AUTH=true +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=true +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=mailer@example.com +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=your-from-name +``` + +本地 `export` 示例写法: + +```bash +export SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example +export SPRING_MAIL_HOST=smtp.mail.example +export SPRING_MAIL_PORT=465 +export SPRING_MAIL_USERNAME=mailer@example.com +export SPRING_MAIL_PASSWORD=your-smtp-app-password +export SPRING_MAIL_SMTP_AUTH=true +export SPRING_MAIL_SMTP_STARTTLS_ENABLE=false +export SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=true +export SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +export SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=mailer@example.com +export SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=your-from-name +``` + +### B. MailHog(本地联调推荐) + +```dotenv +SPRING_MAIL_HOST=127.0.0.1 +SPRING_MAIL_PORT=1025 +SPRING_MAIL_USERNAME= +SPRING_MAIL_PASSWORD= +SPRING_MAIL_SMTP_AUTH=false +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=false +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@skillhub.local +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub +``` + +### C. 线上部署(465 端口示例) + +```dotenv +SPRING_MAIL_HOST=smtp.mail.example +SPRING_MAIL_PORT=465 +SPRING_MAIL_USERNAME=mailer@example.com +SPRING_MAIL_PASSWORD=your-smtp-app-password +SPRING_MAIL_SMTP_AUTH=true +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=true +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=mailer@example.com +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=your-from-name +``` + +## 2. 单机交付(Compose)配置步骤 + +1. 复制环境模板(若尚未创建): + +```bash +cp .env.release.example .env.release +``` + +2. 编辑 `.env.release`,填写 SMTP 变量: + +```dotenv +SPRING_MAIL_HOST=smtp.mail.example +SPRING_MAIL_PORT=465 +SPRING_MAIL_USERNAME=mailer@example.com +SPRING_MAIL_PASSWORD=your-smtp-app-password +SPRING_MAIL_SMTP_AUTH=true +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=true +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example + +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=mailer@example.com +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=your-from-name +``` + +3. 重启后端容器使配置生效: + +```bash +docker compose --env-file .env.release -f compose.release.yml up -d server +``` + +4. 查看后端日志确认启动正常: + +```bash +docker compose --env-file .env.release -f compose.release.yml logs -f server +``` + +## 3. 本地开发配置与验证 + +### 3.1 一次性临时生效(推荐) + +适合当前终端临时测试,重开终端后失效。 + +```bash +SPRING_MAIL_HOST=smtp.mail.example \ +SPRING_MAIL_PORT=465 \ +SPRING_MAIL_USERNAME=mailer@example.com \ +SPRING_MAIL_PASSWORD=your-smtp-app-password \ +SPRING_MAIL_SMTP_AUTH=true \ +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false \ +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=true \ +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example \ +SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M \ +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=mailer@example.com \ +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=your-from-name \ +make dev-server +``` + +### 3.2 长期生效(shell 配置) + +如果你写到了 `~/.zshrc`,请注意: +- 必须 `source ~/.zshrc` 或重开终端后变量才会生效 +- 需要在“同一个终端”启动 `make dev-server` + +可先确认变量是否在当前 shell 中: + +```bash +env | rg '^(SPRING_MAIL_|SKILLHUB_AUTH_PASSWORD_RESET_)' +``` + +### 3.3 推荐联调方式(MailHog) + +如果你只是本地验证验证码链路,建议用 MailHog 作为本地 SMTP 服务: + +1. 启动 MailHog: + +```bash +docker run -d --name skillhub-mailhog \ + -p 1025:1025 \ + -p 8025:8025 \ + mailhog/mailhog +``` + +2. 启动依赖服务(Postgres/Redis): + +```bash +make dev +``` + +3. 启动后端时注入 SMTP 环境变量(示例): + +```bash +SPRING_MAIL_HOST=127.0.0.1 \ +SPRING_MAIL_PORT=1025 \ +SPRING_MAIL_USERNAME= \ +SPRING_MAIL_PASSWORD= \ +SPRING_MAIL_SMTP_AUTH=false \ +SPRING_MAIL_SMTP_STARTTLS_ENABLE=false \ +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_ENABLE=false \ +SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@skillhub.local \ +SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub \ +make dev-server +``` + +4. 打开 MailHog Web UI 查看邮件: + +```text +http://localhost:8025 +``` + +5. 在 SkillHub 页面验证流程: +- 打开 `/reset-password` +- 输入邮箱并点击“发送验证码” +- 在 MailHog 中查看验证码邮件 +- 输入邮箱 + 验证码 + 新密码完成重置 + +6. 也可使用接口做快速验证(示例): + +```bash +curl -X POST http://localhost:8080/api/v1/auth/local/password-reset/request \ + -H 'Content-Type: application/json' \ + -d '{"email":"your-email@example.com"}' +``` + +## 4. 功能验证(验证码邮件) + +### 4.1 用户自助找回 + +在 `/reset-password` 页面点击“发送验证码”后,系统会尝试发送验证码邮件。 + +说明: +- 为防止账号枚举,自助接口总是返回通用成功提示。 +- 即使邮件发送失败,接口也可能返回成功;请结合后端日志确认实际发送结果。 + +### 4.2 管理员触发重置 + +管理员在用户管理页触发“重置密码”时,系统会强制发送验证码; +若 SMTP 发送失败,会返回错误(便于运维排障)。 + +## 5. 常见问题排查 + +### 5.1 认证失败(`535 Authentication failed`) + +排查方向: +- 用户名/密码是否正确 +- 邮箱服务是否要求“客户端授权码”而非登录密码 +- 发件账号是否已开启 SMTP 服务 + +### 5.2 连接超时或拒绝连接 + +排查方向: +- 主机到 SMTP 服务端口 `465` 是否可达 +- 安全组/防火墙是否放行出站连接 +- SMTP 服务地址是否填写正确 + +### 5.3 本地明明配置了变量但不生效 + +排查方向: +- 是否只是编辑了 `~/.zshrc` 但没有 `source ~/.zshrc` +- 启动后端的终端是否与配置变量的终端是同一个 +- `8080` 是否被旧进程占用,导致新进程没启动成功 + +可执行以下命令快速检查: + +```bash +# 查看 8080 是否被旧进程占用 +lsof -nP -iTCP:8080 -sTCP:LISTEN + +# 查看当前 shell 是否有 SMTP 环境变量 +env | rg '^(SPRING_MAIL_|SKILLHUB_AUTH_PASSWORD_RESET_)' +``` + +### 5.4 发件人被拒绝 + +排查方向: +- `SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS` 是否与 SMTP 账号一致或已验证 +- 邮箱服务是否限制别名发件 + +### 5.5 健康检查是否校验 SMTP + +默认配置下,邮件健康检查关闭,不会因为 SMTP 不可达导致 `health` 失败。 + +若需要将 SMTP 连通性纳入健康检查,可设置: + +```dotenv +MANAGEMENT_HEALTH_MAIL_ENABLED=true +``` + +### 5.6 SMTP 报 `PKIX path building failed`(证书链校验失败) + +典型日志: +- `SSLHandshakeException` +- `unable to find valid certification path to requested target` + +处理建议(本地联调): +- 增加: + +```dotenv +SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=smtp.mail.example +``` + +- 然后重启后端,再触发一次“发送验证码”。 + +补充: +- 该配置用于指定信任主机,适合本地排障与联调。 +- 生产环境默认不建议长期启用该配置,更推荐使用规范 CA 证书链或将企业 CA 导入 Java truststore。 + +## 6. 安全建议 + +- 不要把 SMTP 密码提交到仓库;仅写入受控的 `.env.release` 或密钥管理系统。 +- 使用专用发信账号,避免使用个人邮箱主密码。 +- 生产环境建议定期轮换 SMTP 授权码。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java index 8d6f5e4f..8442939d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.local.LocalAuthService; +import com.iflytek.skillhub.auth.local.PasswordResetService; import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.session.PlatformSessionService; @@ -10,6 +11,8 @@ import com.iflytek.skillhub.dto.AuthMeResponse; import com.iflytek.skillhub.dto.ChangePasswordRequest; import com.iflytek.skillhub.dto.LocalLoginRequest; import com.iflytek.skillhub.dto.LocalRegisterRequest; +import com.iflytek.skillhub.dto.PasswordResetConfirmRequest; +import com.iflytek.skillhub.dto.PasswordResetRequestDto; import com.iflytek.skillhub.exception.UnauthorizedException; import com.iflytek.skillhub.metrics.SkillHubMetrics; import com.iflytek.skillhub.ratelimit.RateLimit; @@ -34,17 +37,20 @@ public class LocalAuthController extends BaseApiController { private final SkillHubMetrics skillHubMetrics; private final PlatformSessionService platformSessionService; private final AuthFailureThrottleService authFailureThrottleService; + private final PasswordResetService passwordResetService; public LocalAuthController(ApiResponseFactory responseFactory, LocalAuthService localAuthService, SkillHubMetrics skillHubMetrics, PlatformSessionService platformSessionService, - AuthFailureThrottleService authFailureThrottleService) { + AuthFailureThrottleService authFailureThrottleService, + PasswordResetService passwordResetService) { super(responseFactory); this.localAuthService = localAuthService; this.skillHubMetrics = skillHubMetrics; this.platformSessionService = platformSessionService; this.authFailureThrottleService = authFailureThrottleService; + this.passwordResetService = passwordResetService; } @PostMapping("/register") @@ -92,6 +98,20 @@ public class LocalAuthController extends BaseApiController { return ok("response.success.updated", null); } + @PostMapping("/password-reset/request") + @RateLimit(category = "auth-password-reset-request", authenticated = 8, anonymous = 5, windowSeconds = 300) + public ApiResponse requestPasswordReset(@Valid @RequestBody PasswordResetRequestDto request) { + passwordResetService.requestPasswordReset(request.email()); + return ok("response.auth.password.reset.requested", null); + } + + @PostMapping("/password-reset/confirm") + @RateLimit(category = "auth-password-reset-confirm", authenticated = 10, anonymous = 10, windowSeconds = 300) + public ApiResponse confirmPasswordReset(@Valid @RequestBody PasswordResetConfirmRequest request) { + passwordResetService.confirmPasswordReset(request.email(), request.code(), request.newPassword()); + return ok("response.auth.password.reset.confirmed", null); + } + private String resolveClientIp(HttpServletRequest request) { String ip = request.getHeader("X-Forwarded-For"); if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java index efaf7647..627d413c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/UserManagementController.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.controller.admin; import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.auth.local.PasswordResetService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.AdminUserMutationResponse; import com.iflytek.skillhub.dto.AdminUserRoleUpdateRequest; @@ -9,6 +10,7 @@ import com.iflytek.skillhub.dto.AdminUserSummaryResponse; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.exception.UnauthorizedException; import com.iflytek.skillhub.service.AdminUserAppService; import jakarta.validation.Valid; import org.springframework.security.access.prepost.PreAuthorize; @@ -24,11 +26,14 @@ import org.springframework.web.bind.annotation.*; public class UserManagementController extends BaseApiController { private final AdminUserAppService adminUserAppService; + private final PasswordResetService passwordResetService; public UserManagementController(AdminUserAppService adminUserAppService, + PasswordResetService passwordResetService, ApiResponseFactory responseFactory) { super(responseFactory); this.adminUserAppService = adminUserAppService; + this.passwordResetService = passwordResetService; } @GetMapping @@ -76,4 +81,15 @@ public class UserManagementController extends BaseApiController { public ApiResponse enableUser(@PathVariable String userId) { return ok("response.success.updated", adminUserAppService.updateUserStatus(userId, "ACTIVE")); } + + @PostMapping("/{userId}/password-reset") + @PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')") + public ApiResponse triggerPasswordReset(@PathVariable String userId, + @AuthenticationPrincipal PlatformPrincipal principal) { + if (principal == null) { + throw new UnauthorizedException("error.auth.required"); + } + passwordResetService.adminTriggerPasswordReset(userId, principal.userId()); + return ok("response.auth.password.reset.requested", null); + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/LocalRegisterRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/LocalRegisterRequest.java index 5a4a2749..443a5b8e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/LocalRegisterRequest.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/LocalRegisterRequest.java @@ -8,6 +8,7 @@ public record LocalRegisterRequest( String username, @NotBlank(message = "{validation.auth.local.password.notBlank}") String password, + @NotBlank(message = "{validation.auth.local.email.notBlank}") @Email(message = "{validation.auth.local.email.invalid}") String email ) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetConfirmRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetConfirmRequest.java new file mode 100644 index 00000000..332f75e6 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetConfirmRequest.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; + +public record PasswordResetConfirmRequest( + @NotBlank(message = "{validation.auth.password.reset.email.notBlank}") + @Email(message = "{validation.auth.password.reset.email.invalid}") + @Pattern(regexp = "^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$", message = "{validation.auth.password.reset.email.invalid}") + String email, + @NotBlank(message = "{validation.auth.password.reset.code.notBlank}") + @Pattern(regexp = "^\\d{6}$", message = "{validation.auth.password.reset.code.invalid}") + String code, + @NotBlank(message = "{validation.auth.password.reset.newPassword.notBlank}") + String newPassword +) { +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetRequestDto.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetRequestDto.java new file mode 100644 index 00000000..f12f20d2 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PasswordResetRequestDto.java @@ -0,0 +1,13 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; + +public record PasswordResetRequestDto( + @NotBlank(message = "{validation.auth.password.reset.email.notBlank}") + @Email(message = "{validation.auth.password.reset.email.invalid}") + @Pattern(regexp = "^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$", message = "{validation.auth.password.reset.email.invalid}") + String email +) { +} diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 8a3872af..db4397f8 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -61,6 +61,17 @@ spring: multipart: max-file-size: 100MB max-request-size: 100MB + mail: + host: ${SPRING_MAIL_HOST:localhost} + port: ${SPRING_MAIL_PORT:25} + username: ${SPRING_MAIL_USERNAME:} + password: ${SPRING_MAIL_PASSWORD:} + properties: + mail: + smtp: + auth: ${SPRING_MAIL_SMTP_AUTH:false} + starttls: + enable: ${SPRING_MAIL_SMTP_STARTTLS_ENABLE:false} skillhub: auth: @@ -70,6 +81,10 @@ skillhub: enabled: ${SKILLHUB_AUTH_DIRECT_ENABLED:false} session-bootstrap: enabled: ${SKILLHUB_AUTH_SESSION_BOOTSTRAP_ENABLED:false} + password-reset: + code-expiry: ${SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY:PT10M} + email-from-address: ${SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS:noreply@skillhub.local} + email-from-name: ${SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME:SkillHub} public: base-url: ${SKILLHUB_PUBLIC_BASE_URL:} access-policy: @@ -168,6 +183,9 @@ skillhub: email: ${BOOTSTRAP_ADMIN_EMAIL:admin@skillhub.local} management: + health: + mail: + enabled: ${MANAGEMENT_HEALTH_MAIL_ENABLED:false} endpoints: web: exposure: diff --git a/server/skillhub-app/src/main/resources/db/migration/V39__password_reset_request.sql b/server/skillhub-app/src/main/resources/db/migration/V39__password_reset_request.sql new file mode 100644 index 00000000..c8915bbf --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V39__password_reset_request.sql @@ -0,0 +1,20 @@ +-- Password reset verification code records for self-service and admin-triggered flows +CREATE TABLE password_reset_request ( + id BIGSERIAL PRIMARY KEY, + user_id VARCHAR(128) NOT NULL REFERENCES user_account(id) ON DELETE CASCADE, + email VARCHAR(255) NOT NULL, + code_hash VARCHAR(255) NOT NULL, + expires_at TIMESTAMPTZ NOT NULL, + consumed_at TIMESTAMPTZ, + requested_by_admin BOOLEAN NOT NULL DEFAULT FALSE, + requested_by_user_id VARCHAR(128) REFERENCES user_account(id) ON DELETE SET NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +CREATE INDEX idx_password_reset_request_user_id ON password_reset_request(user_id); +CREATE INDEX idx_password_reset_request_expires_at ON password_reset_request(expires_at); + +COMMENT ON TABLE password_reset_request IS 'Stores password reset verification code requests for local account recovery'; +COMMENT ON COLUMN password_reset_request.code_hash IS 'BCrypt hash of the one-time verification code'; +COMMENT ON COLUMN password_reset_request.requested_by_admin IS 'True when the reset is triggered by an administrator'; +COMMENT ON COLUMN password_reset_request.requested_by_user_id IS 'Admin user who triggered the reset, if applicable'; diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 6e754186..1fea61ca 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -16,6 +16,7 @@ validation.member.userId.notNull=User ID is required validation.member.role.notNull=Role is required validation.auth.local.username.notBlank=Username cannot be blank validation.auth.local.password.notBlank=Password cannot be blank +validation.auth.local.email.notBlank=Email cannot be blank validation.auth.local.currentPassword.notBlank=Current password cannot be blank validation.auth.local.newPassword.notBlank=New password cannot be blank validation.auth.local.email.invalid=Email format is invalid @@ -153,3 +154,16 @@ error.profileReview.commentRequired=Rejection reason is required error.profileReview.commentTooLong=Rejection reason must not exceed 500 characters error.profileReview.status.invalid=Invalid review status: {0} error.profileReview.userDisabled=Cannot apply changes — user account is disabled + +# Password reset +response.auth.password.reset.requested=If the account is eligible, a password reset verification code has been sent. +response.auth.password.reset.confirmed=Password has been reset successfully. Please sign in with your new password. +error.auth.password.reset.invalid.code=The verification code is invalid or has expired. +error.auth.password.reset.not.eligible=This account is not eligible for password reset. +error.auth.password.reset.no.credential=This account does not have a local credential. +error.auth.password.reset.email.failed=Failed to send password reset verification code. Please try again later. +validation.auth.password.reset.email.notBlank=Email cannot be blank +validation.auth.password.reset.email.invalid=Email format is invalid +validation.auth.password.reset.code.notBlank=Verification code cannot be blank +validation.auth.password.reset.code.invalid=Verification code must be 6 digits +validation.auth.password.reset.newPassword.notBlank=New password cannot be blank diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 541770db..05bd0905 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -16,6 +16,7 @@ validation.member.userId.notNull=用户 ID 不能为空 validation.member.role.notNull=角色不能为空 validation.auth.local.username.notBlank=用户名不能为空 validation.auth.local.password.notBlank=密码不能为空 +validation.auth.local.email.notBlank=邮箱不能为空 validation.auth.local.currentPassword.notBlank=当前密码不能为空 validation.auth.local.newPassword.notBlank=新密码不能为空 validation.auth.local.email.invalid=邮箱格式不正确 @@ -153,3 +154,16 @@ error.profileReview.commentRequired=拒绝原因不能为空 error.profileReview.commentTooLong=拒绝原因不能超过 500 个字符 error.profileReview.status.invalid=无效的审核状态:{0} error.profileReview.userDisabled=无法应用变更——用户账号已被禁用 + +# Password reset +response.auth.password.reset.requested=如果账号符合条件,密码重置验证码已发送。 +response.auth.password.reset.confirmed=密码已重置成功,请使用新密码登录。 +error.auth.password.reset.invalid.code=验证码无效或已过期。 +error.auth.password.reset.not.eligible=该账号不符合密码重置条件。 +error.auth.password.reset.no.credential=该账号没有本地凭证。 +error.auth.password.reset.email.failed=发送密码重置验证码失败,请稍后重试。 +validation.auth.password.reset.email.notBlank=邮箱不能为空 +validation.auth.password.reset.email.invalid=邮箱格式不正确 +validation.auth.password.reset.code.notBlank=验证码不能为空 +validation.auth.password.reset.code.invalid=验证码必须为 6 位数字 +validation.auth.password.reset.newPassword.notBlank=新密码不能为空 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java index bb5ee3ff..7158cdfd 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java @@ -12,6 +12,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.local.LocalAuthService; +import com.iflytek.skillhub.auth.local.PasswordResetService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.metrics.SkillHubMetrics; @@ -50,6 +51,9 @@ class LocalAuthControllerTest { @MockBean private AuthFailureThrottleService authFailureThrottleService; + @MockBean + private PasswordResetService passwordResetService; + @Test void login_returnsCurrentUserEnvelope() throws Exception { PlatformPrincipal principal = new PlatformPrincipal( @@ -119,6 +123,23 @@ class LocalAuthControllerTest { verify(localAuthService).register("bob", "Abcd123!", "not-an-email"); } + @Test + void register_rejectsBlankEmail() throws Exception { + given(localAuthService.register("bob", "Abcd123!", " ")) + .willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.notBlank")); + + mockMvc.perform(post("/api/v1/auth/local/register") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"username":"bob","password":"Abcd123!","email":" "} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + + verify(localAuthService).register("bob", "Abcd123!", " "); + } + @Test void login_failure_recordsFailureMetric() throws Exception { given(localAuthService.login("alice", "wrong")) @@ -176,4 +197,66 @@ class LocalAuthControllerTest { .andExpect(jsonPath("$.code").value(0)); } + @Test + void requestPasswordReset_returnsGenericSuccessEnvelope() throws Exception { + mockMvc.perform(post("/api/v1/auth/local/password-reset/request") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"alice@example.com"} + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)); + + verify(passwordResetService).requestPasswordReset("alice@example.com"); + } + + @Test + void requestPasswordReset_rejectsInvalidEmailFormat() throws Exception { + willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.invalid")) + .given(passwordResetService).requestPasswordReset("alice"); + + mockMvc.perform(post("/api/v1/auth/local/password-reset/request") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"alice"} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + + verify(passwordResetService).requestPasswordReset("alice"); + } + + @Test + void confirmPasswordReset_returnsUpdatedEnvelope() throws Exception { + mockMvc.perform(post("/api/v1/auth/local/password-reset/confirm") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"alice@example.com","code":"123456","newPassword":"Abcd123!"} + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)); + + verify(passwordResetService).confirmPasswordReset("alice@example.com", "123456", "Abcd123!"); + } + + @Test + void confirmPasswordReset_rejectsInvalidEmailFormat() throws Exception { + willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.invalid")) + .given(passwordResetService).confirmPasswordReset("alice", "123456", "Abcd123!"); + + mockMvc.perform(post("/api/v1/auth/local/password-reset/confirm") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"alice","code":"123456","newPassword":"Abcd123!"} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + + verify(passwordResetService).confirmPasswordReset("alice", "123456", "Abcd123!"); + } + } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceWorkflowContractTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceWorkflowContractTest.java index af279dc4..4bdc7899 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceWorkflowContractTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceWorkflowContractTest.java @@ -3,18 +3,19 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.Namespace; -import com.iflytek.skillhub.domain.namespace.NamespaceGovernanceService; import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceMemberService; -import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.namespace.NamespaceType; import com.iflytek.skillhub.domain.user.UserAccount; -import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.dto.MemberResponse; import com.iflytek.skillhub.dto.NamespaceCandidateUserResponse; +import com.iflytek.skillhub.dto.NamespaceResponse; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.service.GovernanceWorkflowAppService; +import com.iflytek.skillhub.service.NamespacePortalCommandAppService; +import com.iflytek.skillhub.service.NamespacePortalQueryAppService; import com.iflytek.skillhub.service.NamespaceMemberCandidateService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -28,7 +29,6 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.RequestPostProcessor; import java.util.List; -import java.util.Optional; import java.util.Set; import static org.mockito.ArgumentMatchers.any; @@ -52,25 +52,19 @@ class NamespaceWorkflowContractTest { private MockMvc mockMvc; @MockBean - private NamespaceService namespaceService; + private NamespacePortalCommandAppService namespacePortalCommandAppService; @MockBean - private NamespaceGovernanceService namespaceGovernanceService; + private NamespacePortalQueryAppService namespacePortalQueryAppService; @MockBean - private NamespaceMemberService namespaceMemberService; - - @MockBean - private NamespaceRepository namespaceRepository; - - @MockBean - private NamespaceMemberRepository namespaceMemberRepository; + private GovernanceWorkflowAppService governanceWorkflowAppService; @MockBean private NamespaceMemberCandidateService namespaceMemberCandidateService; @MockBean - private UserAccountRepository userAccountRepository; + private NamespaceMemberRepository namespaceMemberRepository; @MockBean private DeviceAuthService deviceAuthService; @@ -82,24 +76,30 @@ class NamespaceWorkflowContractTest { Namespace archived = namespace(7L, "team-flow", NamespaceStatus.ARCHIVED, NamespaceType.TEAM); NamespaceMember adminMember = new NamespaceMember(7L, "user-admin", NamespaceRole.ADMIN); setMemberId(adminMember, 11L); + NamespaceResponse namespaceResponse = NamespaceResponse.from(namespace); + NamespaceResponse frozenResponse = NamespaceResponse.from(frozen); + NamespaceResponse archivedResponse = NamespaceResponse.from(archived); + MemberResponse adminMemberResponse = MemberResponse.from( + adminMember, + new UserAccount("user-admin", "Admin", "admin@example.com", null) + ); - given(namespaceService.createNamespace(eq("team-flow"), eq("Team Flow"), eq("workflow"), eq("owner-1"))) - .willReturn(namespace); - given(namespaceService.getNamespaceBySlug("team-flow")).willReturn(namespace); - given(namespaceGovernanceService.freezeNamespace(eq("team-flow"), eq("owner-1"), eq(null), eq(null), any(), any())) - .willReturn(frozen); - given(namespaceGovernanceService.archiveNamespace(eq("team-flow"), eq("owner-1"), eq("cleanup"), eq(null), any(), any())) - .willReturn(archived); + given(namespacePortalCommandAppService.createNamespace(any(), any())) + .willReturn(namespaceResponse); + given(governanceWorkflowAppService.freezeNamespace(eq("team-flow"), any(), eq("owner-1"), any())) + .willReturn(frozenResponse); + given(governanceWorkflowAppService.archiveNamespace(eq("team-flow"), any(), eq("owner-1"), any())) + .willReturn(archivedResponse); given(namespaceMemberCandidateService.searchCandidates("team-flow", "admin", "owner-1", 10)) .willReturn(List.of(new NamespaceCandidateUserResponse("user-admin", "Admin", "admin@example.com", "ACTIVE"))); - given(namespaceMemberService.addMember(7L, "user-admin", NamespaceRole.ADMIN, "owner-1")) - .willReturn(adminMember); - given(namespaceMemberService.listMembers(eq(7L), any(org.springframework.data.domain.Pageable.class))) - .willReturn(new org.springframework.data.domain.PageImpl<>(List.of(adminMember))); - given(namespaceMemberService.updateMemberRole(7L, "user-admin", NamespaceRole.ADMIN, "owner-1")) - .willReturn(adminMember); - given(userAccountRepository.findById("user-admin")) - .willReturn(Optional.of(new UserAccount("user-admin", "Admin", "admin@example.com", null))); + given(namespacePortalCommandAppService.addMember("team-flow", "user-admin", NamespaceRole.ADMIN, "owner-1")) + .willReturn(adminMemberResponse); + given(namespacePortalQueryAppService.listMembers(eq("team-flow"), any(org.springframework.data.domain.Pageable.class), eq("owner-1"))) + .willReturn(new PageResponse<>(List.of(adminMemberResponse), 1, 0, 20)); + given(namespacePortalCommandAppService.updateMemberRole(eq("team-flow"), eq("user-admin"), any(), eq("owner-1"))) + .willReturn(adminMemberResponse); + given(namespacePortalCommandAppService.removeMember("team-flow", "user-admin", "owner-1")) + .willReturn(new com.iflytek.skillhub.dto.MessageResponse("Member removed successfully")); mockMvc.perform(post("/api/web/namespaces") .with(csrf()) @@ -127,14 +127,18 @@ class NamespaceWorkflowContractTest { .content("{\"userId\":\"user-admin\",\"role\":\"ADMIN\"}")) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.userId").value("user-admin")); + .andExpect(jsonPath("$.data.userId").value("user-admin")) + .andExpect(jsonPath("$.data.displayName").value("Admin")) + .andExpect(jsonPath("$.data.email").value("admin@example.com")); mockMvc.perform(get("/api/web/namespaces/team-flow/members") .with(auth("owner-1")) .requestAttr("userId", "owner-1")) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.items[0].userId").value("user-admin")); + .andExpect(jsonPath("$.data.items[0].userId").value("user-admin")) + .andExpect(jsonPath("$.data.items[0].displayName").value("Admin")) + .andExpect(jsonPath("$.data.items[0].email").value("admin@example.com")); mockMvc.perform(put("/api/web/namespaces/team-flow/members/user-admin/role") .with(csrf()) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java index 2bb98567..1f01ff04 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/UserManagementControllerTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.controller.admin; import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.local.PasswordResetService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -52,6 +53,9 @@ class UserManagementControllerTest { @MockBean private AdminUserAppService adminUserAppService; + @MockBean + private PasswordResetService passwordResetService; + @Test void listUsers_unauthenticated_returns401() throws Exception { mockMvc.perform(get("/api/v1/admin/users")) @@ -243,4 +247,22 @@ class UserManagementControllerTest { verify(adminUserAppService).updateUserStatus("user-123", "ACTIVE"); } + + @Test + void triggerPasswordReset_withUserAdminRole_returns200() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "user-42", "admin", "admin@example.com", "", "github", Set.of("USER_ADMIN") + ); + var auth = new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_USER_ADMIN")) + ); + + mockMvc.perform(post("/api/v1/admin/users/user-123/password-reset") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)); + + verify(passwordResetService).adminTriggerPasswordReset("user-123", "user-42"); + } } diff --git a/server/skillhub-app/src/test/resources/application-test.yml b/server/skillhub-app/src/test/resources/application-test.yml index fec71eff..e7a6ea69 100644 --- a/server/skillhub-app/src/test/resources/application-test.yml +++ b/server/skillhub-app/src/test/resources/application-test.yml @@ -4,7 +4,8 @@ spring: banner-mode: "off" log-startup-info: false datasource: - url: jdbc:h2:mem:testdb;MODE=PostgreSQL;DATABASE_TO_LOWER=TRUE;DEFAULT_NULL_ORDERING=HIGH;INIT=CREATE DOMAIN IF NOT EXISTS JSONB AS JSON;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE + generate-unique-name: true + url: jdbc:h2:mem:testdb-${random.uuid};MODE=PostgreSQL;DATABASE_TO_LOWER=TRUE;DEFAULT_NULL_ORDERING=HIGH;INIT=CREATE DOMAIN IF NOT EXISTS JSONB AS JSON;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE driver-class-name: org.h2.Driver username: sa password: diff --git a/server/skillhub-auth/pom.xml b/server/skillhub-auth/pom.xml index 5e50347d..3bee6a9e 100644 --- a/server/skillhub-auth/pom.xml +++ b/server/skillhub-auth/pom.xml @@ -35,6 +35,15 @@ org.springframework.boot spring-boot-starter-data-redis + + org.springframework.boot + spring-boot-starter-mail + + + org.springframework.boot + spring-boot-configuration-processor + true + org.springframework.boot spring-boot-starter-test diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java index 9d378e25..df0b1867 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java @@ -230,7 +230,7 @@ public class LocalAuthService { private void validateEmail(String email) { if (email == null) { - return; + throw new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.notBlank"); } if (!EMAIL_PATTERN.matcher(email).matches()) { throw new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.invalid"); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetProperties.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetProperties.java new file mode 100644 index 00000000..00a031be --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetProperties.java @@ -0,0 +1,38 @@ +package com.iflytek.skillhub.auth.local; + +import java.time.Duration; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.stereotype.Component; + +@Component +@ConfigurationProperties(prefix = "skillhub.auth.password-reset") +public class PasswordResetProperties { + + private Duration codeExpiry = Duration.ofMinutes(10); + private String emailFromAddress = "noreply@skillhub.local"; + private String emailFromName = "SkillHub"; + + public Duration getCodeExpiry() { + return codeExpiry; + } + + public void setCodeExpiry(Duration codeExpiry) { + this.codeExpiry = codeExpiry; + } + + public String getEmailFromAddress() { + return emailFromAddress; + } + + public void setEmailFromAddress(String emailFromAddress) { + this.emailFromAddress = emailFromAddress; + } + + public String getEmailFromName() { + return emailFromName; + } + + public void setEmailFromName(String emailFromName) { + this.emailFromName = emailFromName; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java new file mode 100644 index 00000000..60b52acc --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java @@ -0,0 +1,244 @@ +package com.iflytek.skillhub.auth.local; + +import com.iflytek.skillhub.auth.exception.AuthFlowException; +import com.iflytek.skillhub.domain.auth.PasswordResetRequest; +import com.iflytek.skillhub.domain.auth.PasswordResetRequestRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import java.security.SecureRandom; +import java.time.Instant; +import java.util.List; +import java.util.Locale; +import java.util.Optional; +import java.util.regex.Pattern; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; +import org.springframework.mail.SimpleMailMessage; +import org.springframework.mail.javamail.JavaMailSender; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.util.StringUtils; + +/** + * Local-account password reset flow backed by one-time email verification + * codes. + */ +@Service +public class PasswordResetService { + + private static final Logger log = LoggerFactory.getLogger(PasswordResetService.class); + private static final int VERIFICATION_CODE_DIGITS = 6; + private static final Pattern EMAIL_PATTERN = Pattern.compile("^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$"); + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + private final PasswordResetRequestRepository resetRequestRepository; + private final UserAccountRepository userAccountRepository; + private final LocalCredentialRepository credentialRepository; + private final PasswordPolicyValidator passwordPolicyValidator; + private final PasswordEncoder passwordEncoder; + private final JavaMailSender mailSender; + private final PasswordResetProperties properties; + + public PasswordResetService(PasswordResetRequestRepository resetRequestRepository, + UserAccountRepository userAccountRepository, + LocalCredentialRepository credentialRepository, + PasswordPolicyValidator passwordPolicyValidator, + PasswordEncoder passwordEncoder, + JavaMailSender mailSender, + PasswordResetProperties properties) { + this.resetRequestRepository = resetRequestRepository; + this.userAccountRepository = userAccountRepository; + this.credentialRepository = credentialRepository; + this.passwordPolicyValidator = passwordPolicyValidator; + this.passwordEncoder = passwordEncoder; + this.mailSender = mailSender; + this.properties = properties; + } + + /** + * Anonymous/self-service reset request. Always silent on ineligible users to + * avoid account enumeration. + */ + @Transactional + public void requestPasswordReset(String email) { + String normalizedEmail = normalizeEmail(email); + validateEmail(normalizedEmail); + Optional userOpt = findEligibleUserByEmail(normalizedEmail); + if (userOpt.isEmpty()) { + log.debug("Password reset requested for ineligible email"); + return; + } + + UserAccount user = userOpt.get(); + String code = generateVerificationCode(); + Instant now = Instant.now(); + Instant expiresAt = now.plus(properties.getCodeExpiry()); + + invalidatePendingRequests(user.getId(), now); + resetRequestRepository.save(new PasswordResetRequest( + user.getId(), + user.getEmail(), + passwordEncoder.encode(code), + expiresAt, + false, + null + )); + + sendVerificationCodeEmail(user.getEmail(), code, false); + } + + /** + * Admin-triggered reset request for a specific user. + */ + @Transactional + public void adminTriggerPasswordReset(String userId, String adminUserId) { + UserAccount user = userAccountRepository.findById(userId) + .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.admin.user.notFound", userId)); + + if (!isEligibleForReset(user)) { + throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.password.reset.not.eligible"); + } + + String code = generateVerificationCode(); + Instant now = Instant.now(); + Instant expiresAt = now.plus(properties.getCodeExpiry()); + + invalidatePendingRequests(userId, now); + resetRequestRepository.save(new PasswordResetRequest( + userId, + user.getEmail(), + passwordEncoder.encode(code), + expiresAt, + true, + adminUserId + )); + + sendVerificationCodeEmail(user.getEmail(), code, true); + } + + /** + * Verifies a code and updates the local credential password. + */ + @Transactional + public void confirmPasswordReset(String email, String code, String newPassword) { + String normalizedEmail = normalizeEmail(email); + validateEmail(normalizedEmail); + UserAccount user = findUserByEmail(normalizedEmail) + .orElseThrow(() -> new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.password.reset.invalid.code")); + + List pendingRequests = resetRequestRepository + .findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc(user.getId(), Instant.now()); + + PasswordResetRequest matchedRequest = pendingRequests.stream() + .filter(request -> passwordEncoder.matches(code, request.getCodeHash())) + .findFirst() + .orElseThrow(() -> new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.password.reset.invalid.code")); + + var passwordErrors = passwordPolicyValidator.validate(newPassword); + if (!passwordErrors.isEmpty()) { + throw new AuthFlowException(HttpStatus.BAD_REQUEST, passwordErrors.getFirst()); + } + + LocalCredential credential = credentialRepository.findByUserId(user.getId()) + .orElseThrow(() -> new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.password.reset.no.credential")); + + credential.setPasswordHash(passwordEncoder.encode(newPassword)); + credential.setFailedAttempts(0); + credential.setLockedUntil(null); + credentialRepository.save(credential); + + Instant now = Instant.now(); + matchedRequest.markConsumed(now); + resetRequestRepository.save(matchedRequest); + invalidatePendingRequests(user.getId(), now); + } + + private void invalidatePendingRequests(String userId, Instant now) { + List pending = resetRequestRepository + .findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc(userId, now); + for (PasswordResetRequest request : pending) { + request.markConsumed(now); + resetRequestRepository.save(request); + } + } + + private Optional findEligibleUserByEmail(String normalizedEmail) { + return findUserByEmail(normalizedEmail) + .filter(this::isEligibleForReset); + } + + private Optional findUserByEmail(String normalizedEmail) { + if (!StringUtils.hasText(normalizedEmail)) { + return Optional.empty(); + } + return userAccountRepository.findByEmailIgnoreCase(normalizedEmail); + } + + private boolean isEligibleForReset(UserAccount user) { + if (user.getStatus() != UserStatus.ACTIVE) { + return false; + } + if (!StringUtils.hasText(user.getEmail())) { + return false; + } + return credentialRepository.findByUserId(user.getId()).isPresent(); + } + + private String normalizeEmail(String email) { + if (email == null || email.isBlank()) { + return null; + } + return email.trim().toLowerCase(Locale.ROOT); + } + + private void validateEmail(String email) { + if (email == null) { + throw new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.notBlank"); + } + if (!EMAIL_PATTERN.matcher(email).matches()) { + throw new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.invalid"); + } + } + + private String generateVerificationCode() { + int bound = (int) Math.pow(10, VERIFICATION_CODE_DIGITS); + int code = SECURE_RANDOM.nextInt(bound); + return String.format("%0" + VERIFICATION_CODE_DIGITS + "d", code); + } + + private void sendVerificationCodeEmail(String email, String code, boolean failOnError) { + SimpleMailMessage message = new SimpleMailMessage(); + message.setFrom(resolveFromAddress()); + message.setTo(email); + message.setSubject("SkillHub password reset verification code"); + message.setText(buildVerificationCodeBody(code)); + try { + mailSender.send(message); + log.info("Password reset verification code sent to {}", email); + } catch (Exception ex) { + if (failOnError) { + log.error("Failed to send password reset verification code to {}", email, ex); + throw new AuthFlowException(HttpStatus.INTERNAL_SERVER_ERROR, "error.auth.password.reset.email.failed"); + } + log.warn("Failed to send password reset verification code to {}", email, ex); + } + } + + private String resolveFromAddress() { + String fromAddress = properties.getEmailFromAddress(); + if (!StringUtils.hasText(properties.getEmailFromName())) { + return fromAddress; + } + return properties.getEmailFromName() + " <" + fromAddress + ">"; + } + + private String buildVerificationCodeBody(String code) { + long expiryMinutes = Math.max(1L, properties.getCodeExpiry().toMinutes()); + return "Your SkillHub password reset verification code is: " + code + + "\n\nThis code expires in " + expiryMinutes + " minutes." + + "\n\nIf you did not request a password reset, please ignore this email."; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/package-info.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/package-info.java index 7aa1200b..80486dfd 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/package-info.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/package-info.java @@ -1,5 +1,5 @@ /** * Username-and-password authentication support, including registration, - * password changes, and local credential validation. + * password changes, password resets, and local credential validation. */ package com.iflytek.skillhub.auth.local; diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java index b566b922..6b9f4344 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java @@ -238,4 +238,13 @@ class LocalAuthServiceTest { .isInstanceOf(AuthFlowException.class) .hasMessageContaining("validation.auth.local.email.invalid"); } + + @Test + void register_rejectsBlankEmail() { + given(credentialRepository.existsByUsernameIgnoreCase("alice")).willReturn(false); + + assertThatThrownBy(() -> service.register("Alice", "Abcd123!", " ")) + .isInstanceOf(AuthFlowException.class) + .hasMessageContaining("validation.auth.local.email.notBlank"); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java new file mode 100644 index 00000000..aa78c1fb --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java @@ -0,0 +1,218 @@ +package com.iflytek.skillhub.auth.local; + +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.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.BDDMockito.given; + +import com.iflytek.skillhub.auth.exception.AuthFlowException; +import com.iflytek.skillhub.domain.auth.PasswordResetRequest; +import com.iflytek.skillhub.domain.auth.PasswordResetRequestRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.mail.SimpleMailMessage; +import org.springframework.mail.javamail.JavaMailSender; +import org.springframework.security.crypto.password.PasswordEncoder; + +@ExtendWith(MockitoExtension.class) +class PasswordResetServiceTest { + + @Mock + private PasswordResetRequestRepository resetRequestRepository; + + @Mock + private UserAccountRepository userAccountRepository; + + @Mock + private LocalCredentialRepository credentialRepository; + + @Mock + private PasswordEncoder passwordEncoder; + + @Mock + private JavaMailSender mailSender; + + private PasswordResetService service; + + @BeforeEach + void setUp() { + PasswordResetProperties properties = new PasswordResetProperties(); + properties.setCodeExpiry(Duration.ofMinutes(10)); + properties.setEmailFromAddress("noreply@skillhub.local"); + properties.setEmailFromName("SkillHub"); + service = new PasswordResetService( + resetRequestRepository, + userAccountRepository, + credentialRepository, + new PasswordPolicyValidator(), + passwordEncoder, + mailSender, + properties + ); + } + + @Test + void requestPasswordReset_withEligibleEmail_savesRequestAndSendsEmail() { + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + given(userAccountRepository.findByEmailIgnoreCase("alice@example.com")).willReturn(Optional.of(user)); + given(credentialRepository.findByUserId("usr_1")).willReturn( + Optional.of(new LocalCredential("usr_1", "alice", "encoded")) + ); + given(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + anyString(), any(Instant.class)) + ).willReturn(List.of()); + given(passwordEncoder.encode(anyString())).willReturn("encoded-value"); + + service.requestPasswordReset("alice@example.com"); + + verify(resetRequestRepository).save(any(PasswordResetRequest.class)); + verify(mailSender).send(any(SimpleMailMessage.class)); + } + + @Test + void requestPasswordReset_withUnknownEmail_doesNothing() { + given(userAccountRepository.findByEmailIgnoreCase("ghost@example.com")).willReturn(Optional.empty()); + + service.requestPasswordReset("ghost@example.com"); + + verify(resetRequestRepository, never()).save(any(PasswordResetRequest.class)); + verify(mailSender, never()).send(any(SimpleMailMessage.class)); + } + + @Test + void requestPasswordReset_withInvalidEmail_throwsBadRequest() { + assertThatThrownBy(() -> service.requestPasswordReset("alice")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + + verifyNoInteractions(userAccountRepository, resetRequestRepository, mailSender); + } + + @Test + void requestPasswordReset_emailFailure_doesNotThrowForAnonymousFlow() { + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + given(userAccountRepository.findByEmailIgnoreCase("alice@example.com")).willReturn(Optional.of(user)); + given(credentialRepository.findByUserId("usr_1")).willReturn( + Optional.of(new LocalCredential("usr_1", "alice", "encoded")) + ); + given(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + anyString(), any(Instant.class)) + ).willReturn(List.of()); + given(passwordEncoder.encode(anyString())).willReturn("encoded-value"); + + org.mockito.Mockito.doThrow(new RuntimeException("smtp down")).when(mailSender).send(any(SimpleMailMessage.class)); + + service.requestPasswordReset("alice@example.com"); + + verify(resetRequestRepository).save(any(PasswordResetRequest.class)); + } + + @Test + void adminTriggerPasswordReset_withUnknownUser_throwsNotFound() { + given(userAccountRepository.findById("missing")).willReturn(Optional.empty()); + + assertThatThrownBy(() -> service.adminTriggerPasswordReset("missing", "admin_1")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.NOT_FOUND); + } + + @Test + void confirmPasswordReset_withValidCode_updatesCredential() { + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + LocalCredential credential = new LocalCredential("usr_1", "alice", "old-password"); + PasswordResetRequest request = new PasswordResetRequest( + "usr_1", + "alice@example.com", + "encoded-code", + Instant.now().plus(Duration.ofMinutes(5)), + false, + null + ); + + given(userAccountRepository.findByEmailIgnoreCase("alice@example.com")).willReturn(Optional.of(user)); + given(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + anyString(), any(Instant.class)) + ).willReturn(List.of(request)); + given(passwordEncoder.matches("123456", "encoded-code")).willReturn(true); + given(credentialRepository.findByUserId("usr_1")).willReturn(Optional.of(credential)); + given(passwordEncoder.encode("Abcd123!")).willReturn("new-password-hash"); + + service.confirmPasswordReset("alice@example.com", "123456", "Abcd123!"); + + assertThat(credential.getPasswordHash()).isEqualTo("new-password-hash"); + assertThat(credential.getFailedAttempts()).isZero(); + assertThat(credential.getLockedUntil()).isNull(); + verify(credentialRepository).save(credential); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(PasswordResetRequest.class); + verify(resetRequestRepository, atLeastOnce()).save(requestCaptor.capture()); + assertThat(requestCaptor.getAllValues()) + .anySatisfy(captured -> assertThat(captured.getConsumedAt()).isNotNull()); + } + + @Test + void confirmPasswordReset_withInvalidCode_throwsBadRequest() { + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + LocalCredential credential = new LocalCredential("usr_1", "alice", "old-password"); + PasswordResetRequest request = new PasswordResetRequest( + "usr_1", + "alice@example.com", + "encoded-code", + Instant.now().plus(Duration.ofMinutes(5)), + false, + null + ); + + given(userAccountRepository.findByEmailIgnoreCase("alice@example.com")).willReturn(Optional.of(user)); + given(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + anyString(), any(Instant.class)) + ).willReturn(List.of(request)); + given(passwordEncoder.matches("654321", "encoded-code")).willReturn(false); + + assertThatThrownBy(() -> service.confirmPasswordReset("alice@example.com", "654321", "Abcd123!")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + } + + @Test + void confirmPasswordReset_withInvalidEmail_throwsBadRequest() { + assertThatThrownBy(() -> service.confirmPasswordReset("alice", "123456", "Abcd123!")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + + verifyNoInteractions(userAccountRepository, resetRequestRepository, credentialRepository); + } + + @Test + void adminTriggerPasswordReset_forDisabledUser_throwsBadRequest() { + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + user.setStatus(UserStatus.DISABLED); + given(userAccountRepository.findById("usr_1")).willReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.adminTriggerPasswordReset("usr_1", "admin_1")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequest.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequest.java new file mode 100644 index 00000000..2c034675 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequest.java @@ -0,0 +1,108 @@ +package com.iflytek.skillhub.domain.auth; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.PrePersist; +import jakarta.persistence.Table; +import java.time.Clock; +import java.time.Instant; + +@Entity +@Table(name = "password_reset_request") +public class PasswordResetRequest { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @Column(name = "user_id", nullable = false, length = 128) + private String userId; + + @Column(nullable = false, length = 255) + private String email; + + @Column(name = "code_hash", nullable = false, length = 255) + private String codeHash; + + @Column(name = "expires_at", nullable = false) + private Instant expiresAt; + + @Column(name = "consumed_at") + private Instant consumedAt; + + @Column(name = "requested_by_admin", nullable = false) + private boolean requestedByAdmin; + + @Column(name = "requested_by_user_id", length = 128) + private String requestedByUserId; + + @Column(name = "created_at", nullable = false, updatable = false) + private Instant createdAt; + + protected PasswordResetRequest() { + } + + public PasswordResetRequest(String userId, + String email, + String codeHash, + Instant expiresAt, + boolean requestedByAdmin, + String requestedByUserId) { + this.userId = userId; + this.email = email; + this.codeHash = codeHash; + this.expiresAt = expiresAt; + this.requestedByAdmin = requestedByAdmin; + this.requestedByUserId = requestedByUserId; + } + + @PrePersist + void prePersist() { + if (createdAt == null) { + createdAt = Instant.now(Clock.systemUTC()); + } + } + + public void markConsumed(Instant timestamp) { + this.consumedAt = timestamp; + } + + public Long getId() { + return id; + } + + public String getUserId() { + return userId; + } + + public String getEmail() { + return email; + } + + public String getCodeHash() { + return codeHash; + } + + public Instant getExpiresAt() { + return expiresAt; + } + + public Instant getConsumedAt() { + return consumedAt; + } + + public boolean isRequestedByAdmin() { + return requestedByAdmin; + } + + public String getRequestedByUserId() { + return requestedByUserId; + } + + public Instant getCreatedAt() { + return createdAt; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequestRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequestRepository.java new file mode 100644 index 00000000..567bfa70 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/PasswordResetRequestRepository.java @@ -0,0 +1,17 @@ +package com.iflytek.skillhub.domain.auth; + +import java.time.Instant; +import java.util.List; + +/** + * Domain repository contract for local-account password reset verification + * codes. + */ +public interface PasswordResetRequestRepository { + PasswordResetRequest save(PasswordResetRequest request); + + List findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + String userId, + Instant now + ); +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/package-info.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/package-info.java new file mode 100644 index 00000000..3080fb67 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/auth/package-info.java @@ -0,0 +1,4 @@ +/** + * Password-reset domain entities and repository contracts. + */ +package com.iflytek.skillhub.domain.auth; diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PasswordResetRequestJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PasswordResetRequestJpaRepository.java new file mode 100644 index 00000000..a69702b0 --- /dev/null +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PasswordResetRequestJpaRepository.java @@ -0,0 +1,19 @@ +package com.iflytek.skillhub.infra.jpa; + +import com.iflytek.skillhub.domain.auth.PasswordResetRequest; +import com.iflytek.skillhub.domain.auth.PasswordResetRequestRepository; +import java.time.Instant; +import java.util.List; +import org.springframework.data.jpa.repository.JpaRepository; + +/** + * JPA-backed repository for password-reset verification-code requests. + */ +public interface PasswordResetRequestJpaRepository + extends JpaRepository, PasswordResetRequestRepository { + + List findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + String userId, + Instant now + ); +} diff --git a/web/e2e/helpers/auth-fixtures.ts b/web/e2e/helpers/auth-fixtures.ts index 31a9fb4a..fc89c8a1 100644 --- a/web/e2e/helpers/auth-fixtures.ts +++ b/web/e2e/helpers/auth-fixtures.ts @@ -5,3 +5,12 @@ export async function setEnglishLocale(page: Page) { window.localStorage.setItem('i18nextLng', 'en') }) } + +export async function setUniqueClientIp(page: Page, seed: string) { + const suffix = Date.now() + Math.floor(Math.random() * 1000) + const thirdOctet = seed.split('').reduce((sum, char) => sum + char.charCodeAt(0), 0) % 250 + const fourthOctet = suffix % 250 + await page.context().setExtraHTTPHeaders({ + 'X-Forwarded-For': `10.0.${thirdOctet}.${fourthOctet}`, + }) +} diff --git a/web/e2e/password-reset.spec.ts b/web/e2e/password-reset.spec.ts new file mode 100644 index 00000000..a37efcf5 --- /dev/null +++ b/web/e2e/password-reset.spec.ts @@ -0,0 +1,45 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale, setUniqueClientIp } from './helpers/auth-fixtures' + +test.describe('Password Reset (Real API)', () => { + function uniqueResetEmail(seed: string) { + return `nonexistent_${seed}_${Date.now()}@example.com` + } + + test.beforeEach(async ({ page }) => { + await setEnglishLocale(page) + }) + + test('sends verification code from reset-password page', async ({ page }) => { + const email = uniqueResetEmail('request') + await setUniqueClientIp(page, 'password-reset-request') + + await page.goto('/reset-password') + + await expect(page.getByRole('heading', { name: 'Reset Password' })).toBeVisible() + await page.getByLabel('Email').fill(email) + await page.getByRole('button', { name: 'Send Verification Code' }).click() + + await expect(page.getByText('If the account is eligible, a verification code has been sent.')).toBeVisible() + }) + + test('shows backend validation error for an invalid reset code', async ({ page }) => { + const email = uniqueResetEmail('invalid-code') + await setUniqueClientIp(page, 'password-reset-invalid-code') + + await page.goto('/reset-password') + + await expect(page.getByRole('heading', { name: 'Reset Password' })).toBeVisible() + await page.getByLabel('Email').fill(email) + await page.getByRole('button', { name: 'Send Verification Code' }).click() + await expect(page.getByText('If the account is eligible, a verification code has been sent.')).toBeVisible() + await page.getByLabel('Verification Code').fill('123456') + await page.getByLabel('New Password').fill('Passw0rd!123') + await page.getByLabel('Confirm Password').fill('Passw0rd!123') + await page.getByRole('button', { name: 'Reset Password' }).click() + + await expect( + page.getByText(/The verification code is invalid or has expired\.|验证码无效或已过期。/) + ).toBeVisible() + }) +}) diff --git a/web/e2e/register-email-required.spec.ts b/web/e2e/register-email-required.spec.ts new file mode 100644 index 00000000..54d2476a --- /dev/null +++ b/web/e2e/register-email-required.spec.ts @@ -0,0 +1,46 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' + +function buildUniqueUser() { + const suffix = `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}` + return { + username: `e2e_reg_${suffix}`, + email: `e2e_reg_${suffix}@example.test`, + password: 'Passw0rd!123', + } +} + +test.describe('Register Email Required (Real API)', () => { + test.beforeEach(async ({ page }) => { + await setEnglishLocale(page) + }) + + test('registers successfully when email is provided', async ({ page }) => { + const user = buildUniqueUser() + await page.goto('/register') + + await expect(page.getByRole('heading', { name: 'Create Account' })).toBeVisible() + await page.getByLabel('Username').fill(user.username) + await page.getByLabel('Email').fill(user.email) + await page.getByLabel('Password').fill(user.password) + await page.getByRole('button', { name: 'Register & Login' }).click() + + await expect(page).toHaveURL('/dashboard') + }) + + test('shows required validation when email is missing', async ({ page }) => { + await page.goto('/register') + + await page.getByLabel('Username').fill(`e2e_no_email_${Date.now().toString(36)}`) + await page.getByLabel('Password').fill('Passw0rd!123') + await page.getByRole('button', { name: 'Register & Login' }).click() + + const isEmailMissing = await page.getByLabel('Email').evaluate((element) => { + const input = element as HTMLInputElement + return input.validity.valueMissing + }) + + expect(isEmailMissing).toBeTruthy() + await expect(page).toHaveURL(/\/register/) + }) +}) diff --git a/web/e2e/register-login-validation.spec.ts b/web/e2e/register-login-validation.spec.ts index 2d0c8531..7ab8574a 100644 --- a/web/e2e/register-login-validation.spec.ts +++ b/web/e2e/register-login-validation.spec.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test' -import { setEnglishLocale } from './helpers/auth-fixtures' +import { setEnglishLocale, setUniqueClientIp } from './helpers/auth-fixtures' import { createFreshSession } from './helpers/session' // TC_UN_* 用户名输入框 / TC_EM_* 邮箱输入框 / TC_PW_* 密码输入框 @@ -10,14 +10,16 @@ const DUPLICATE_USERNAME_ERROR = /already.*exist|taken|username.*used/i const REGISTER_RATE_LIMIT_ERROR = /too many|too frequent|rate limit|请求过于频繁/ test.describe('Register - Username Validation (Real API)', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) + await setUniqueClientIp(page, `register-validation-${testInfo.title}`) await page.goto('/register') }) // TC_UN_008 P0 test('TC_UN_008: shows required error when username is empty', async ({ page }) => { - await page.getByRole('button', { name: 'Register' }).click() + await page.getByLabel(/username/i).click() + await page.getByLabel(/email/i).click() await expect(page.getByText(/username.*required|required.*username/i)).toBeVisible() }) @@ -51,18 +53,19 @@ test.describe('Register - Username Validation (Real API)', () => { }) test.describe('Register - Email Validation (Real API)', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) + await setUniqueClientIp(page, `register-validation-${testInfo.title}`) await page.goto('/register') }) - // TC_EM_007 P0 - email is optional - test('TC_EM_007: allows empty email (email is optional)', async ({ page }) => { + // TC_EM_007 P0 - email is required + test('TC_EM_007: shows required error when email is empty', async ({ page }) => { const emailField = page.getByLabel(/email/i) if (await emailField.isVisible()) { await emailField.clear() await emailField.blur() - await expect(page.getByText(/email.*required/i)).not.toBeVisible() + await expect(page.getByText(/email.*required|required.*email/i)).toBeVisible() } }) @@ -88,14 +91,16 @@ test.describe('Register - Email Validation (Real API)', () => { }) test.describe('Register - Password Validation (Real API)', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) + await setUniqueClientIp(page, `register-validation-${testInfo.title}`) await page.goto('/register') }) // TC_PW_013 P0 - empty password test('TC_PW_013: shows required error when password is empty', async ({ page }) => { - await page.getByRole('button', { name: 'Register' }).click() + await page.getByLabel(/^password/i).click() + await page.getByLabel(/username/i).click() await expect(page.getByText(/password.*required|required.*password/i)).toBeVisible() }) @@ -123,8 +128,9 @@ test.describe('Register - Password Validation (Real API)', () => { }) test.describe('Register Flow (Real API)', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) + await setUniqueClientIp(page, `register-validation-${testInfo.title}`) }) // TC_REG_001 P0 - successful registration with all fields @@ -138,7 +144,7 @@ test.describe('Register Flow (Real API)', () => { await emailField.fill(`test_${suffix}@example.test`) } await page.getByLabel(/^password/i).fill('Test123!@') - await page.getByRole('button', { name: 'Register' }).click() + await page.getByRole('button', { name: 'Register & Login' }).click() // Should redirect away from /register on success await expect(page).not.toHaveURL('/register') existingRegisteredUsername = username @@ -160,13 +166,14 @@ test.describe('Register Flow (Real API)', () => { await page.goto('/register') await setEnglishLocale(page) await page.getByLabel(/username/i).fill(username) + await page.getByLabel(/email/i).fill(`duplicate_${Date.now()}@example.test`) await page.getByLabel(/^password/i).fill('Test123!@') const main = page.getByRole('main') const duplicateUsernameError = main.getByText(DUPLICATE_USERNAME_ERROR).first() const registerRateLimitError = main.getByText(REGISTER_RATE_LIMIT_ERROR).first() for (let attempt = 0; attempt < 3; attempt += 1) { - await page.getByRole('button', { name: 'Register' }).click() + await page.getByRole('button', { name: 'Register & Login' }).click() if (await duplicateUsernameError.isVisible().catch(() => false)) { return @@ -184,27 +191,35 @@ test.describe('Register Flow (Real API)', () => { }) // TC_REG_002 P0 - registration without email - test('TC_REG_002: registers successfully without email (email is optional)', async ({ page }) => { + test('TC_REG_002: shows required validation when email is missing', async ({ page }) => { await page.goto('/register') const suffix = Date.now().toString(36) + Math.random().toString(36).slice(2, 5) - await page.getByLabel(/username/i).fill(`noemail_${suffix}`) + await page.getByLabel(/username/i).fill(`emailrequired_${suffix}`) await page.getByLabel(/^password/i).fill('Test123!@') - await page.getByRole('button', { name: 'Register' }).click() - await expect(page).not.toHaveURL('/register') + await page.getByLabel(/email/i).click() + await page.getByLabel(/username/i).click() + await page.getByRole('button', { name: 'Register & Login' }).click() + const emailField = page.getByLabel(/email/i) + await expect(emailField).toBeFocused() + await expect + .poll(async () => emailField.evaluate((input) => (input as HTMLInputElement).validity.valueMissing)) + .toBe(true) }) // TC_REG_005 P0 - required fields empty on submit test('TC_REG_005: shows validation errors when submitting empty required fields', async ({ page }) => { await page.goto('/register') - await page.getByRole('button', { name: 'Register' }).click() + await page.getByLabel(/email/i).fill(`required_fields_${Date.now()}@example.test`) + await page.getByRole('button', { name: 'Register & Login' }).click() await expect(page.getByText(/username.*required|required.*username/i)).toBeVisible() await expect(page.getByText(/password.*required|required.*password/i)).toBeVisible() }) }) test.describe('Login Flow (Real API)', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) + await setUniqueClientIp(page, `register-validation-${testInfo.title}`) }) // TC_REG_006 P0 - successful login (already tested in auth-entry.spec.ts partially; extend here) @@ -223,8 +238,9 @@ test.describe('Login Flow (Real API)', () => { await page.goto('/register') await page.getByLabel(/username/i).fill(username) + await page.getByLabel(/email/i).fill(`login_${suffix}@example.test`) await page.getByLabel(/^password/i).fill('Test123!@') - await page.getByRole('button', { name: 'Register' }).click() + await page.getByRole('button', { name: 'Register & Login' }).click() await expect(page).not.toHaveURL('/register') await page.goto('/login') diff --git a/web/e2e/settings-pages.spec.ts b/web/e2e/settings-pages.spec.ts index 8c77919c..de2abd6e 100644 --- a/web/e2e/settings-pages.spec.ts +++ b/web/e2e/settings-pages.spec.ts @@ -13,6 +13,13 @@ test.describe('Settings Pages (Real API)', () => { await expect(page.getByRole('heading', { name: 'Profile Settings' })).toBeVisible() }) + test('navigates to reset-password page from profile settings', async ({ page }) => { + await page.goto('/settings/profile') + await page.getByRole('button', { name: 'Reset Password' }).click() + await expect(page).toHaveURL('/reset-password') + await expect(page.getByRole('heading', { name: 'Reset Password' })).toBeVisible() + }) + test('shows validation when current password is missing', async ({ page }) => { await page.goto('/settings/security') await expect(page.getByRole('heading', { name: 'Security Settings' })).toBeVisible() diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 714111e3..c110c064 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -2,6 +2,8 @@ import createClient from 'openapi-fetch' import type { paths } from './generated/schema' import type { ChangePasswordRequest, + PasswordResetConfirmRequest, + PasswordResetRequest, ApiToken, CreateTokenRequest, CreateTokenResponse, @@ -354,6 +356,26 @@ export const authApi = { }) }, + async requestPasswordReset(request: PasswordResetRequest): Promise { + await fetchJson('/api/v1/auth/local/password-reset/request', { + method: 'POST', + headers: await ensureCsrfHeaders({ + 'Content-Type': 'application/json', + }), + body: JSON.stringify(request), + }) + }, + + async confirmPasswordReset(request: PasswordResetConfirmRequest): Promise { + await fetchJson('/api/v1/auth/local/password-reset/confirm', { + method: 'POST', + headers: await ensureCsrfHeaders({ + 'Content-Type': 'application/json', + }), + body: JSON.stringify(request), + }) + }, + async logout(): Promise { const response = await fetch('/api/v1/auth/logout', { method: 'POST', @@ -1093,6 +1115,13 @@ export const adminApi = { }) }, + async triggerPasswordReset(userId: string): Promise { + await fetchJson(`/api/v1/admin/users/${userId}/password-reset`, { + method: 'POST', + headers: getCsrfHeaders(), + }) + }, + async getAuditLogs(params: { action?: string userId?: string diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index e2c11a1f..bfc1e4c9 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -1140,6 +1140,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/auth/local/password-reset/request": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["requestPasswordReset"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/auth/local/password-reset/confirm": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["confirmPasswordReset"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/auth/local/login": { parameters: { query?: never; @@ -1220,6 +1252,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/admin/users/{userId}/password-reset": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["triggerPasswordReset"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/admin/users/{userId}/enable": { parameters: { query?: never; @@ -3413,7 +3461,15 @@ export interface components { LocalRegisterRequest: { username: string; password: string; - email?: string; + email: string; + }; + PasswordResetRequestDto: { + email: string; + }; + PasswordResetConfirmRequest: { + email: string; + code: string; + newPassword: string; }; LocalLoginRequest: { username: string; @@ -6823,6 +6879,54 @@ export interface operations { }; }; }; + requestPasswordReset: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["PasswordResetRequestDto"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; + confirmPasswordReset: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["PasswordResetConfirmRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; login: { parameters: { query?: never; @@ -6939,6 +7043,28 @@ export interface operations { }; }; }; + triggerPasswordReset: { + parameters: { + query?: never; + header?: never; + path: { + userId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; enableUser: { parameters: { query?: never; diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 61c9fa46..2b2f9382 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -53,7 +53,7 @@ export interface LocalLoginRequest { } export interface LocalRegisterRequest extends LocalLoginRequest { - email?: string + email: string } export interface ChangePasswordRequest { @@ -61,6 +61,16 @@ export interface ChangePasswordRequest { newPassword: string } +export interface PasswordResetRequest { + email: string +} + +export interface PasswordResetConfirmRequest { + email: string + code: string + newPassword: string +} + export type CreateNamespaceRequest = Omit & { slug: string displayName: string diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index fd48292b..d096987b 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -65,6 +65,7 @@ const LandingPage = createLazyRouteComponent(() => import('@/pages/landing'), 'L const HomePage = createLazyRouteComponent(() => import('@/pages/home'), 'HomePage') const LoginPage = createLazyRouteComponent(() => import('@/pages/login'), 'LoginPage') const RegisterPage = createLazyRouteComponent(() => import('@/pages/register'), 'RegisterPage') +const ResetPasswordPage = createLazyRouteComponent(() => import('@/pages/reset-password'), 'ResetPasswordPage') const PrivacyPolicyPage = createLazyRouteComponent(() => import('@/pages/privacy'), 'PrivacyPolicyPage') const SearchPage = createLazyRouteComponent(() => import('@/pages/search'), 'SearchPage') const TermsOfServicePage = createLazyRouteComponent(() => import('@/pages/terms'), 'TermsOfServicePage') @@ -184,6 +185,12 @@ const registerRoute = createRoute({ component: RegisterPage, }) +const resetPasswordRoute = createRoute({ + getParentRoute: () => rootRoute, + path: 'reset-password', + component: ResetPasswordPage, +}) + const privacyRoute = createRoute({ getParentRoute: () => rootRoute, path: 'privacy', @@ -397,6 +404,7 @@ const routeTree = rootRoute.addChildren([ skillsRoute, loginRoute, registerRoute, + resetPasswordRoute, privacyRoute, searchRoute, termsRoute, diff --git a/web/src/features/admin/use-admin-users.ts b/web/src/features/admin/use-admin-users.ts index be9c7743..7c0abf73 100644 --- a/web/src/features/admin/use-admin-users.ts +++ b/web/src/features/admin/use-admin-users.ts @@ -97,3 +97,13 @@ export function useEnableUser() { }, }) } + +export function useTriggerUserPasswordReset() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: (userId: string) => adminApi.triggerPasswordReset(userId), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['admin', 'users'] }) + }, + }) +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 3af596c9..501e5781 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -209,6 +209,7 @@ "hidePassword": "Hide password", "submitting": "Logging in...", "submit": "Login", + "forgotPassword": "Forgot password?", "noAccount": "Don't have an account?", "register": "Sign up now", "oauthHint": "After GitHub authentication, you will be automatically redirected back to this site.", @@ -232,7 +233,8 @@ "email": "Email", "password": "Password", "usernamePlaceholder": "3-64 characters: letters, numbers, or underscores", - "emailPlaceholder": "Optional, for account identification", + "emailPlaceholder": "Enter your email", + "emailRequired": "Email is required", "passwordPlaceholder": "At least 8 characters with 3 character types", "submitting": "Registering...", "submit": "Register & Login", @@ -248,6 +250,31 @@ "emailInvalid": "Invalid email format", "emailExists": "Email already exists" }, + "resetPassword": { + "title": "Reset Password", + "subtitle": "Enter your email, verification code, and new password.", + "email": "Email", + "emailPlaceholder": "Enter email", + "emailRequired": "Please enter your email", + "emailInvalid": "Please enter a valid email address", + "code": "Verification Code", + "codePlaceholder": "Enter 6-digit verification code", + "sendCode": "Send Verification Code", + "sendingCode": "Sending...", + "codeSentMessage": "If the account is eligible, a verification code has been sent.", + "codeRequired": "Please enter the verification code", + "newPassword": "New Password", + "newPasswordPlaceholder": "Enter new password", + "newPasswordRequired": "Please enter a new password", + "confirmPassword": "Confirm Password", + "confirmPasswordPlaceholder": "Re-enter new password", + "passwordMismatch": "The two passwords do not match", + "submit": "Reset Password", + "submitting": "Resetting...", + "successMessage": "Password reset successful. Please sign in with your new password.", + "genericError": "Failed to reset password", + "backToLogin": "Back to login" + }, "device": { "title": "Device Authorization", "subtitle": "Enter the 8-digit user code shown on your device", @@ -529,6 +556,7 @@ "approveUser": "Approve", "disable": "Disable", "enable": "Enable", + "resetPassword": "Reset Password", "totalRecords": "Total {{total}} records, page {{page}}", "prevPage": "Previous", "nextPage": "Next", @@ -543,7 +571,8 @@ "roleSuperAdmin": "Super Admin", "confirmAction": "Confirm Action", "confirmDisable": "Are you sure you want to disable user {{username}}?", - "confirmEnable": "Are you sure you want to enable user {{username}}?" + "confirmEnable": "Are you sure you want to enable user {{username}}?", + "confirmResetPassword": "Send password reset verification code to user {{username}}?" }, "adminLabels": { "title": "Label Management", @@ -650,6 +679,7 @@ "subtitle": "Manage your display name and personal information.", "displayName": "Display Name", "email": "Email", + "resetPassword": "Reset Password", "edit": "Edit", "save": "Save", "saving": "Saving...", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 17c2eb9a..ebdc169b 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -209,6 +209,7 @@ "hidePassword": "隐藏密码", "submitting": "登录中...", "submit": "登录", + "forgotPassword": "忘记密码?", "noAccount": "还没有账号?", "register": "立即注册", "oauthHint": "使用 GitHub 登录时,认证完成后会自动返回当前站点。", @@ -232,7 +233,8 @@ "email": "邮箱", "password": "密码", "usernamePlaceholder": "3-64 位字母、数字或下划线", - "emailPlaceholder": "可选,用于后续账号识别", + "emailPlaceholder": "请输入邮箱", + "emailRequired": "请输入邮箱", "passwordPlaceholder": "至少 8 位,包含 3 种字符类型", "submitting": "注册中...", "submit": "注册并登录", @@ -248,6 +250,31 @@ "emailInvalid": "邮箱格式不正确", "emailExists": "邮箱已存在" }, + "resetPassword": { + "title": "重置密码", + "subtitle": "输入邮箱、验证码和新密码以完成重置。", + "email": "邮箱", + "emailPlaceholder": "请输入邮箱", + "emailRequired": "请输入邮箱", + "emailInvalid": "请输入正确的邮箱格式", + "code": "验证码", + "codePlaceholder": "请输入 6 位验证码", + "sendCode": "发送验证码", + "sendingCode": "发送中...", + "codeSentMessage": "如果账号符合条件,验证码已发送。", + "codeRequired": "请输入验证码", + "newPassword": "新密码", + "newPasswordPlaceholder": "请输入新密码", + "newPasswordRequired": "请输入新密码", + "confirmPassword": "确认新密码", + "confirmPasswordPlaceholder": "请再次输入新密码", + "passwordMismatch": "两次输入的密码不一致", + "submit": "重置密码", + "submitting": "重置中...", + "successMessage": "密码重置成功,请使用新密码登录。", + "genericError": "重置密码失败", + "backToLogin": "返回登录" + }, "device": { "title": "设备授权", "subtitle": "请输入设备上显示的 8 位用户码", @@ -529,6 +556,7 @@ "approveUser": "审批通过", "disable": "禁用", "enable": "启用", + "resetPassword": "重置密码", "totalRecords": "共 {{total}} 条记录,第 {{page}} 页", "prevPage": "上一页", "nextPage": "下一页", @@ -543,7 +571,8 @@ "roleSuperAdmin": "超级管理员", "confirmAction": "确认操作", "confirmDisable": "确定要禁用用户 {{username}} 吗?", - "confirmEnable": "确定要启用用户 {{username}} 吗?" + "confirmEnable": "确定要启用用户 {{username}} 吗?", + "confirmResetPassword": "确定给用户 {{username}} 发送密码重置验证码吗?" }, "adminLabels": { "title": "标签管理", @@ -650,6 +679,7 @@ "subtitle": "管理你的昵称和个人信息。", "displayName": "昵称", "email": "邮箱", + "resetPassword": "重置密码", "edit": "编辑", "save": "保存", "saving": "保存中...", diff --git a/web/src/pages/admin/users.test.tsx b/web/src/pages/admin/users.test.tsx index a47ade76..1b27359c 100644 --- a/web/src/pages/admin/users.test.tsx +++ b/web/src/pages/admin/users.test.tsx @@ -64,6 +64,7 @@ vi.mock('@/features/admin/use-admin-users', () => ({ useApproveUser: () => ({ mutate: vi.fn(), isPending: false }), useDisableUser: () => ({ mutateAsync: vi.fn(), isPending: false }), useEnableUser: () => ({ mutateAsync: vi.fn(), isPending: false }), + useTriggerUserPasswordReset: () => ({ mutateAsync: vi.fn(), isPending: false }), useUpdateUserRole: () => ({ mutateAsync: vi.fn(), isPending: false }), })) diff --git a/web/src/pages/admin/users.tsx b/web/src/pages/admin/users.tsx index 29caf495..1dc051f2 100644 --- a/web/src/pages/admin/users.tsx +++ b/web/src/pages/admin/users.tsx @@ -29,7 +29,14 @@ import { DialogTitle, } from '@/shared/ui/dialog' import { Label } from '@/shared/ui/label' -import { useAdminUsers, useApproveUser, useDisableUser, useEnableUser, useUpdateUserRole } from '@/features/admin/use-admin-users' +import { + useAdminUsers, + useApproveUser, + useDisableUser, + useEnableUser, + useTriggerUserPasswordReset, + useUpdateUserRole, +} from '@/features/admin/use-admin-users' import type { AdminUser } from '@/features/admin/use-admin-users' /** @@ -54,7 +61,7 @@ export function AdminUsersPage() { const [roleDialogOpen, setRoleDialogOpen] = useState(false) const [newRole, setNewRole] = useState('') const [confirmDialogOpen, setConfirmDialogOpen] = useState(false) - const [actionType, setActionType] = useState<'ban' | 'unban'>('ban') + const [actionType, setActionType] = useState<'ban' | 'unban' | 'reset'>('ban') const { data, isLoading } = useAdminUsers({ search, @@ -67,6 +74,7 @@ export function AdminUsersPage() { const approveUserMutation = useApproveUser() const disableUserMutation = useDisableUser() const enableUserMutation = useEnableUser() + const triggerPasswordResetMutation = useTriggerUserPasswordReset() const formatDate = (dateString: string) => { return formatLocalDateTime(dateString, i18n.language) @@ -105,6 +113,12 @@ export function AdminUsersPage() { setConfirmDialogOpen(true) } + const handleTriggerPasswordReset = (user: AdminUser) => { + setSelectedUser(user) + setActionType('reset') + setConfirmDialogOpen(true) + } + const confirmRoleChange = async () => { if (!selectedUser || !newRole || newRole === (selectedUser.platformRoles[0] || 'USER')) return try { @@ -116,18 +130,20 @@ export function AdminUsersPage() { } } - const confirmStatusChange = async () => { + const confirmUserAction = async () => { if (!selectedUser) return try { if (actionType === 'ban') { await disableUserMutation.mutateAsync(selectedUser.userId) - } else { + } else if (actionType === 'unban') { await enableUserMutation.mutateAsync(selectedUser.userId) + } else { + await triggerPasswordResetMutation.mutateAsync(selectedUser.userId) } setConfirmDialogOpen(false) setSelectedUser(null) } catch (error) { - console.error('Failed to update status:', error) + console.error('Failed to apply user action:', error) } } @@ -259,6 +275,13 @@ export function AdminUsersPage() { {t('adminUsers.enable')} )} + @@ -342,14 +365,21 @@ export function AdminUsersPage() { {t('adminUsers.confirmAction')} - {actionType === 'ban' ? t('adminUsers.confirmDisable', { username: selectedUser?.username }) : t('adminUsers.confirmEnable', { username: selectedUser?.username })} + {actionType === 'ban' + ? t('adminUsers.confirmDisable', { username: selectedUser?.username }) + : actionType === 'unban' + ? t('adminUsers.confirmEnable', { username: selectedUser?.username }) + : t('adminUsers.confirmResetPassword', { username: selectedUser?.username })} - diff --git a/web/src/pages/login.tsx b/web/src/pages/login.tsx index a23d0e3e..1aab99c4 100644 --- a/web/src/pages/login.tsx +++ b/web/src/pages/login.tsx @@ -160,6 +160,11 @@ export function LoginPage() { +

+ + {t('login.forgotPassword')} + +

{t('login.noAccount')} {' '} diff --git a/web/src/pages/register.tsx b/web/src/pages/register.tsx index de6b8cf6..a3a0aa6a 100644 --- a/web/src/pages/register.tsx +++ b/web/src/pages/register.tsx @@ -77,7 +77,7 @@ export function RegisterPage() { function validateEmail(value: string) { const trimmed = value.trim().toLowerCase() if (!trimmed) { - return undefined + return t('register.emailRequired') } if (!EMAIL_PATTERN.test(trimmed)) { return t('register.emailInvalid') @@ -112,6 +112,8 @@ export function RegisterPage() { return { fieldErrors: { username: t('register.usernameRequired') } } case 'validation.auth.local.password.notBlank': return { fieldErrors: { password: t('register.passwordRequired') } } + case 'validation.auth.local.email.notBlank': + return { fieldErrors: { email: t('register.emailRequired') } } case 'validation.auth.local.email.invalid': return { fieldErrors: { email: t('register.emailInvalid') } } case 'error.auth.local.username.invalid': @@ -218,6 +220,7 @@ export function RegisterPage() { } }} placeholder={t('register.emailPlaceholder')} + required aria-invalid={fieldErrors.email ? 'true' : 'false'} onBlur={() => { setFieldErrors((current) => ({ ...current, email: validateEmail(email) })) diff --git a/web/src/pages/reset-password.test.tsx b/web/src/pages/reset-password.test.tsx new file mode 100644 index 00000000..1a919aa3 --- /dev/null +++ b/web/src/pages/reset-password.test.tsx @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@tanstack/react-router', () => ({ + Link: ({ children }: { children: unknown }) => children, +})) + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next') + return { + ...actual, + useTranslation: () => ({ + t: (key: string) => key, + }), + } +}) + +vi.mock('@/api/client', () => ({ + authApi: { + requestPasswordReset: vi.fn(), + confirmPasswordReset: vi.fn(), + }, +})) + +vi.mock('@/shared/ui/button', () => ({ + Button: ({ children }: { children: unknown }) => children, +})) + +vi.mock('@/shared/ui/card', () => ({ + Card: ({ children }: { children: unknown }) => children, + CardContent: ({ children }: { children: unknown }) => children, + CardDescription: ({ children }: { children: unknown }) => children, + CardHeader: ({ children }: { children: unknown }) => children, + CardTitle: ({ children }: { children: unknown }) => children, +})) + +vi.mock('@/shared/ui/input', () => ({ + Input: () => null, +})) + +import { renderToStaticMarkup } from 'react-dom/server' +import { ResetPasswordPage } from './reset-password' + +describe('ResetPasswordPage', () => { + it('exports a named component function', () => { + expect(typeof ResetPasswordPage).toBe('function') + }) + + it('renders reset-password title and submit action', () => { + const html = renderToStaticMarkup() + expect(html).toContain('resetPassword.title') + expect(html).toContain('resetPassword.sendCode') + expect(html).toContain('resetPassword.submit') + }) +}) diff --git a/web/src/pages/reset-password.tsx b/web/src/pages/reset-password.tsx new file mode 100644 index 00000000..ef4777de --- /dev/null +++ b/web/src/pages/reset-password.tsx @@ -0,0 +1,187 @@ +import { Link } from '@tanstack/react-router' +import { FormEvent, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { authApi } from '@/api/client' +import { Button } from '@/shared/ui/button' +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' +import { Input } from '@/shared/ui/input' + +/** + * Public page for verifying a reset code and setting a new password. + */ +export function ResetPasswordPage() { + const { t } = useTranslation() + const [email, setEmail] = useState('') + const [code, setCode] = useState('') + const [newPassword, setNewPassword] = useState('') + const [confirmPassword, setConfirmPassword] = useState('') + const [isSubmitting, setIsSubmitting] = useState(false) + const [isSendingCode, setIsSendingCode] = useState(false) + const [isSuccess, setIsSuccess] = useState(false) + const [codeSentMessage, setCodeSentMessage] = useState(null) + const [errorMessage, setErrorMessage] = useState(null) + const emailPattern = /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/ + + async function handleSendCode() { + const normalizedEmail = email.trim().toLowerCase() + if (!normalizedEmail) { + setErrorMessage(t('resetPassword.emailRequired')) + return + } + if (!emailPattern.test(normalizedEmail)) { + setErrorMessage(t('resetPassword.emailInvalid')) + return + } + + setIsSendingCode(true) + setErrorMessage(null) + setCodeSentMessage(null) + try { + await authApi.requestPasswordReset({ email: normalizedEmail }) + setCodeSentMessage(t('resetPassword.codeSentMessage')) + } catch (error) { + setErrorMessage(error instanceof Error ? error.message : t('resetPassword.genericError')) + } finally { + setIsSendingCode(false) + } + } + + async function handleSubmit(event: FormEvent) { + event.preventDefault() + + const normalizedEmail = email.trim().toLowerCase() + if (!normalizedEmail) { + setErrorMessage(t('resetPassword.emailRequired')) + return + } + if (!emailPattern.test(normalizedEmail)) { + setErrorMessage(t('resetPassword.emailInvalid')) + return + } + if (!code.trim()) { + setErrorMessage(t('resetPassword.codeRequired')) + return + } + if (!newPassword) { + setErrorMessage(t('resetPassword.newPasswordRequired')) + return + } + if (newPassword !== confirmPassword) { + setErrorMessage(t('resetPassword.passwordMismatch')) + return + } + + setIsSubmitting(true) + setErrorMessage(null) + try { + await authApi.confirmPasswordReset({ + email: normalizedEmail, + code: code.trim(), + newPassword, + }) + setIsSuccess(true) + } catch (error) { + setErrorMessage(error instanceof Error ? error.message : t('resetPassword.genericError')) + } finally { + setIsSubmitting(false) + } + } + + return ( +

+ + + {t('resetPassword.title')} + {t('resetPassword.subtitle')} + + + {isSuccess ? ( +
+

+ {t('resetPassword.successMessage')} +

+ + {t('resetPassword.backToLogin')} + +
+ ) : ( +
+
+ +
+ setEmail(event.target.value)} + placeholder={t('resetPassword.emailPlaceholder')} + autoComplete="email" + required + /> + +
+
+
+ + setCode(event.target.value)} + placeholder={t('resetPassword.codePlaceholder')} + autoComplete="one-time-code" + /> +
+
+ + setNewPassword(event.target.value)} + placeholder={t('resetPassword.newPasswordPlaceholder')} + autoComplete="new-password" + /> +
+
+ + setConfirmPassword(event.target.value)} + placeholder={t('resetPassword.confirmPasswordPlaceholder')} + autoComplete="new-password" + /> +
+ {errorMessage ? ( +

{errorMessage}

+ ) : null} + {codeSentMessage ? ( +

{codeSentMessage}

+ ) : null} + +
+ )} +
+
+
+ ) +} diff --git a/web/src/pages/settings/profile.test.ts b/web/src/pages/settings/profile.test.ts index ceb412d4..9e6e56a8 100644 --- a/web/src/pages/settings/profile.test.ts +++ b/web/src/pages/settings/profile.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import React from 'react' vi.mock('react-i18next', async () => { const actual = await vi.importActual('react-i18next') @@ -10,6 +11,10 @@ vi.mock('react-i18next', async () => { } }) +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => vi.fn(), +})) + vi.mock('@tanstack/react-query', () => ({ useQuery: () => ({ data: null }), useQueryClient: () => ({ invalidateQueries: vi.fn(), setQueryData: vi.fn() }), @@ -53,10 +58,16 @@ vi.mock('@/shared/ui/input', () => ({ Input: () => null, })) +import { renderToStaticMarkup } from 'react-dom/server' import { ProfileSettingsPage } from './profile' describe('ProfileSettingsPage', () => { it('exports a named component function', () => { expect(typeof ProfileSettingsPage).toBe('function') }) + + it('renders reset-password entry action', () => { + const html = renderToStaticMarkup(React.createElement(ProfileSettingsPage)) + expect(html).toContain('profile.resetPassword') + }) }) diff --git a/web/src/pages/settings/profile.tsx b/web/src/pages/settings/profile.tsx index 8fb07f7f..dfb4a09c 100644 --- a/web/src/pages/settings/profile.tsx +++ b/web/src/pages/settings/profile.tsx @@ -1,4 +1,5 @@ import { useState } from 'react' +import { useNavigate } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useQuery, useQueryClient } from '@tanstack/react-query' import { ApiError, profileApi } from '@/api/client' @@ -38,6 +39,7 @@ function getFieldValue( export function ProfileSettingsPage() { const { t } = useTranslation() const { user } = useAuth() + const navigate = useNavigate() const queryClient = useQueryClient() const [isEditing, setIsEditing] = useState(false) @@ -180,10 +182,17 @@ export function ProfileSettingsPage() { {t('profile.title')} {t('profile.subtitle')} - {!isEditing && hasEditableFields ? ( - + {!isEditing ? ( +
+ + {hasEditableFields ? ( + + ) : null} +
) : null} From c9b0231393c0041567809f52b3a0694f4f087b4e Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 14 Apr 2026 10:26:10 +0800 Subject: [PATCH 4/4] fix(docs): correct aliyun runtime command --- README.md | 4 ++-- README_zh.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index baa7d8e0..49cb166d 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ The `--public-url` parameter sets the public access URL for your SkillHub instan **For users in China (Aliyun mirror):** ```bash -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com --version latest ``` If deployment runs into problems, clear the existing runtime home and retry. @@ -195,7 +195,7 @@ Published images target both `linux/amd64` and `linux/arm64`. curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --public-url https://skillhub.your-company.com # Aliyun mirror (recommended for users in China) -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com --version latest ``` **Deployment parameters:** diff --git a/README_zh.md b/README_zh.md index 8a7c7409..bede1c75 100644 --- a/README_zh.md +++ b/README_zh.md @@ -67,7 +67,7 @@ curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- u **国内用户(阿里云镜像):** ```bash -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com --version latest ``` 如果部署遇到问题,请清除现有的运行时目录并重试。 @@ -177,7 +177,7 @@ skillhub/ curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --public-url https://skillhub.your-company.com # 阿里云镜像(国内推荐) -curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up -- --aliyun --public-url https://skillhub.your-company.com --version latest +curl -fsSL https://imageless.oss-cn-beijing.aliyuncs.com/runtime.sh | sh -s -- up --aliyun --public-url https://skillhub.your-company.com --version latest ``` ### 配置参数说明