diff --git a/.env.release.example b/.env.release.example index 126c0035..86a30b61 100644 --- a/.env.release.example +++ b/.env.release.example @@ -7,6 +7,7 @@ SKILLHUB_WEB_IMAGE=ghcr.io/iflytek/skillhub-web SKILLHUB_SCANNER_IMAGE=ghcr.io/iflytek/skillhub-scanner POSTGRES_IMAGE=postgres:16-alpine REDIS_IMAGE=redis:7-alpine +SPRING_PROFILES_ACTIVE=docker # Public entrypoint seen by browsers/CLI, no trailing slash. # Default to localhost so `runtime.sh up` works as a zero-config quickstart. @@ -93,9 +94,9 @@ OAUTH2_GITLAB_BASE_URI=https://gitlab.com OAUTH2_GITLAB_DISPLAY_NAME=GitLab # Optional: configure DingTalk (钉钉) OAuth2 login. +# Add dingtalk to SPRING_PROFILES_ACTIVE (for example: docker,dingtalk) to enable it. # Register your app at https://open-dev.dingtalk.com and request the Contact.User.Read scope. -# The scope must be "openid" (not "dingtalk") — DingTalk uses openid for OAuth2 authorization. -# Add "openid corpid" if you also need corporate identity information. +# SkillHub uses the official minimal authorization scope "openid". OAUTH2_DINGTALK_CLIENT_ID= OAUTH2_DINGTALK_CLIENT_SECRET= OAUTH2_DINGTALK_DISPLAY_NAME=钉钉 diff --git a/compose.release.yml b/compose.release.yml index 73318768..613d2024 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -49,7 +49,7 @@ services: ports: - "${API_PORT:-8080}:8080" environment: - SPRING_PROFILES_ACTIVE: docker + SPRING_PROFILES_ACTIVE: ${SPRING_PROFILES_ACTIVE:-docker} SPRING_DATASOURCE_URL: jdbc:postgresql://postgres:5432/${POSTGRES_DB:-skillhub} SPRING_DATASOURCE_USERNAME: ${POSTGRES_USER:-skillhub} SPRING_DATASOURCE_PASSWORD: ${POSTGRES_PASSWORD:-skillhub_demo} @@ -96,6 +96,9 @@ 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} + OAUTH2_DINGTALK_CLIENT_ID: ${OAUTH2_DINGTALK_CLIENT_ID:-} + OAUTH2_DINGTALK_CLIENT_SECRET: ${OAUTH2_DINGTALK_CLIENT_SECRET:-} + OAUTH2_DINGTALK_DISPLAY_NAME: ${OAUTH2_DINGTALK_DISPLAY_NAME:-钉钉} SPRING_MAIL_HOST: ${SPRING_MAIL_HOST:-} SPRING_MAIL_PORT: ${SPRING_MAIL_PORT:-25} SPRING_MAIL_USERNAME: ${SPRING_MAIL_USERNAME:-} diff --git a/deploy/k8s/README.md b/deploy/k8s/README.md index 0ed6db14..2dfa3126 100644 --- a/deploy/k8s/README.md +++ b/deploy/k8s/README.md @@ -64,6 +64,8 @@ cp secret.yaml.example secret.yaml | bootstrap-admin-password | 管理员密码 | 是 | | oauth2-github-client-id | GitHub OAuth ID | 否 | | oauth2-github-client-secret | GitHub OAuth 密钥 | 否 | +| oauth2-dingtalk-client-id | 钉钉 OAuth AppKey | 否 | +| oauth2-dingtalk-client-secret | 钉钉 OAuth AppSecret | 否 | | skill-scanner-llm-api-key | LLM API 密钥 | 否 | | skill-scanner-llm-base-url | 本地/自定义 LLM 服务地址 | 否 | | skill-scanner-llm-model | Scanner 使用的 LLM 模型名 | 否 | @@ -213,6 +215,7 @@ kubectl apply -k overlays/with-infra/ # 或 overlays/external/ | redis-connect-timeout | 未设置 | Redis 建连超时 | | redis-timeout | 未设置 | Redis 命令超时 | | redis-client-name | 未设置 | Redis 客户端名称 | +| spring-profiles-active | docker | Spring profile；启用钉钉时改为 `docker,dingtalk` | | storage-base-path | /var/lib/skillhub/storage | 技能存储路径 | | skillhub-storage-provider | local | 存储类型（local/s3） | | skill-scanner-enabled | true | 是否启用扫描器 | @@ -224,6 +227,7 @@ kubectl apply -k overlays/with-infra/ # 或 overlays/external/ | bootstrap-admin-display-name | Platform Admin | 管理员显示名称 | | bootstrap-admin-email | admin@example.com | 管理员邮箱 | | session-cookie-secure | false | HTTPS 环境设为 true | +| oauth2-dingtalk-display-name | 钉钉 | 钉钉登录入口显示名称 | ### Secret 配置项 @@ -237,10 +241,25 @@ kubectl apply -k overlays/with-infra/ # 或 overlays/external/ | bootstrap-admin-password | 管理员密码 | 是 | | oauth2-github-client-id | GitHub OAuth ID | 否 | | oauth2-github-client-secret | GitHub OAuth 密钥 | 否 | +| oauth2-dingtalk-client-id | 钉钉 OAuth AppKey | 否 | +| oauth2-dingtalk-client-secret | 钉钉 OAuth AppSecret | 否 | | skill-scanner-llm-api-key | LLM API 密钥 | 否 | | skill-scanner-llm-base-url | 本地/自定义 LLM 服务地址 | 否 | | skill-scanner-llm-model | LLM 模型名称 | 否 | +### 钉钉 OAuth2 + +钉钉登录默认关闭。启用时： + +1. 将 `base/configmap.yaml` 的 `spring-profiles-active` 改为 `docker,dingtalk` +2. 在 `base/secret.yaml` 填写 `oauth2-dingtalk-client-id` 和 + `oauth2-dingtalk-client-secret` +3. 在钉钉开放平台将回调地址配置为 + `{站点公网地址}/login/oauth2/code/dingtalk` + +授权 scope 固定为 `openid`。详细契约参见 +[钉钉官方教程](https://developers.dingtalk.com/document/orgapp/tutorial-obtaining-user-personal-information)。 + ### 存储配置 **本地存储（默认）** diff --git a/deploy/k8s/base/backend-deployment.yaml b/deploy/k8s/base/backend-deployment.yaml index 52ddc185..a2d47a98 100644 --- a/deploy/k8s/base/backend-deployment.yaml +++ b/deploy/k8s/base/backend-deployment.yaml @@ -23,7 +23,10 @@ spec: name: http env: - name: SPRING_PROFILES_ACTIVE - value: docker + valueFrom: + configMapKeyRef: + name: skillhub-config + key: spring-profiles-active # Database - name: SPRING_DATASOURCE_URL @@ -217,6 +220,25 @@ spec: key: oauth2-github-client-secret optional: true + # DingTalk OAuth2 (optional; requires the dingtalk Spring profile) + - name: OAUTH2_DINGTALK_CLIENT_ID + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-dingtalk-client-id + optional: true + - name: OAUTH2_DINGTALK_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-dingtalk-client-secret + optional: true + - name: OAUTH2_DINGTALK_DISPLAY_NAME + valueFrom: + configMapKeyRef: + name: skillhub-config + key: oauth2-dingtalk-display-name + volumeMounts: - name: skillhub-storage mountPath: /var/lib/skillhub/storage diff --git a/deploy/k8s/base/configmap.yaml b/deploy/k8s/base/configmap.yaml index 06dbbaa5..227ddd45 100644 --- a/deploy/k8s/base/configmap.yaml +++ b/deploy/k8s/base/configmap.yaml @@ -3,6 +3,9 @@ kind: ConfigMap metadata: name: skillhub-config data: + # Add dingtalk to enable DingTalk OAuth2, for example: docker,dingtalk + spring-profiles-active: docker + # Redis 配置 # 使用外部 Redis：修改为外部主机地址 # 使用内置 Redis（overlays/with-infra）：保持 redis @@ -46,6 +49,9 @@ data: # Session 配置 # HTTP 环境设为 false，HTTPS 环境设为 true session-cookie-secure: "false" + + # DingTalk OAuth2 display name (credentials are stored in Secret) + oauth2-dingtalk-display-name: 钉钉 --- apiVersion: v1 kind: PersistentVolumeClaim diff --git a/deploy/k8s/base/secret.yaml.example b/deploy/k8s/base/secret.yaml.example index c2b93d45..36892c96 100644 --- a/deploy/k8s/base/secret.yaml.example +++ b/deploy/k8s/base/secret.yaml.example @@ -27,6 +27,10 @@ stringData: oauth2-github-client-id: "" oauth2-github-client-secret: "" + # DingTalk OAuth（可选；同时在 ConfigMap 中启用 dingtalk profile） + oauth2-dingtalk-client-id: "" + oauth2-dingtalk-client-secret: "" + # LLM 配置（可选，用于技能扫描） skill-scanner-llm-api-key: "" skill-scanner-llm-base-url: "" diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 2f4b7705..dda25b9f 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -283,6 +283,26 @@ Spring Security OAuth2 Client 原生支持多 Provider 并存，新增 Provider 2. `CustomOAuth2UserService` 中按 `registrationId` 分支处理用户属性映射 3. 前端登录页增加对应按钮（通过 `/api/v1/auth/providers` 自动发现） +### 3.7 钉钉 OAuth2 契约 + +钉钉接入遵循[获取用户个人信息教程](https://developers.dingtalk.com/document/orgapp/tutorial-obtaining-user-personal-information)中的新版 OAuth2 契约：授权地址使用 +`https://login.dingtalk.com/oauth2/auth`，授权 scope 固定为最小可用值 +`openid`，token 与用户信息端点分别使用 `/v1.0/oauth2/userAccessToken` 和 +`/v1.0/contact/users/me`。`corpid` 不能单独作为授权 scope。 + +钉钉的 `openid` 是 OAuth2 授权参数，不表示其 token 响应是 OIDC。适配器在外发 +授权 URL 中保留 `scope=openid`，但在 Spring Security 内部将该 registration 按 +普通 OAuth2 处理，避免框架转入要求 `id_token` 的 OIDC 分支。其他真正的 OIDC +registration 仍保留 `openid` 和 nonce。 + +身份映射遵循以下约束： + +- 稳定 subject 按 `unionId -> openId -> userId` 回退 +- identity binding 始终使用 `provider=dingtalk` 与稳定 subject，不依赖邮箱 +- 用户信息端点没有返回真实邮箱时传 `null`，且 `emailVerified=false` +- 即使端点返回邮箱，也不能视为钉钉已验证邮箱，`emailVerified` 仍为 `false` +- provider 默认关闭，仅在显式启用 `dingtalk` Spring profile 并配置凭证时注册 + ## 4. 核心接口设计 ```java diff --git a/docs/09-deployment.md b/docs/09-deployment.md index bcf36289..8d3072f7 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -48,6 +48,7 @@ |---------|------|------| | `local` | 本地源码开发能力 | 启用 mock 登录、开发种子账号、调试日志 | | `docker` | 容器运行时能力 | 启用容器运行时相关能力，不会自动打开首登管理员 | +| `dingtalk` | 钉钉 OAuth2 登录 | 默认关闭；必须与运行 profile 组合并配置 AppKey/AppSecret | 单机交付环境使用 `SPRING_PROFILES_ACTIVE=docker`，原因如下： @@ -249,7 +250,33 @@ Sentinel 配置优先于 Cluster 和单机 `host`/`port`。在 Kubernetes 等 Se - 如果要开放真实登录，再补充 `OAUTH2_GITHUB_CLIENT_ID` / `OAUTH2_GITHUB_CLIENT_SECRET` - 如果要启用密码重置验证码邮件，参见：`docs/19-smtp-password-reset-email-setup.md` -## 8 OIDC 登录配置 +## 8 外部身份源配置 + +### 8.1 钉钉 OAuth2 + +钉钉 Provider 默认不注册。启用时在 `.env.release` 中设置： + +```bash +SPRING_PROFILES_ACTIVE=docker,dingtalk +OAUTH2_DINGTALK_CLIENT_ID=your-app-key +OAUTH2_DINGTALK_CLIENT_SECRET=your-app-secret +OAUTH2_DINGTALK_DISPLAY_NAME=钉钉 +``` + +在钉钉开放平台将回调地址配置为 +`{SKILLHUB_PUBLIC_BASE_URL}/login/oauth2/code/dingtalk`，开通读取个人信息所需权限并 +发布应用。授权 scope 固定为官方新版 OAuth2 契约的 `openid`；不要改为单独的 +`corpid`。契约参见[钉钉官方教程](https://developers.dingtalk.com/document/orgapp/tutorial-obtaining-user-personal-information)。 + +Compose 会将 profile 与三个 `OAUTH2_DINGTALK_*` 变量传给 Server 容器。 +`make validate-release-config` 会拒绝“启用 profile 但缺少凭证”和“配置凭证但未启用 +profile”两类不完整配置。 + +Kubernetes 部署需要将 ConfigMap 的 `spring-profiles-active` 改为 +`docker,dingtalk`，并在 Secret 中填写 `oauth2-dingtalk-client-id` 与 +`oauth2-dingtalk-client-secret`。Deployment 已将这些配置映射到相同的运行时环境变量。 + +### 8.2 OIDC 登录 SkillHub 复用 Spring Security OAuth2 Client 的 OIDC 支持。前端不需要单独 配置回调页；登录页会从 `/api/v1/auth/methods` 读取后端暴露的 diff --git a/docs/skillhub/en/guide/kubernetes.md b/docs/skillhub/en/guide/kubernetes.md index bcb2c1d4..67ea0d5f 100644 --- a/docs/skillhub/en/guide/kubernetes.md +++ b/docs/skillhub/en/guide/kubernetes.md @@ -62,6 +62,8 @@ cp secret.yaml.example secret.yaml | bootstrap-admin-password | Admin password | Yes | | oauth2-github-client-id | GitHub OAuth ID | No | | oauth2-github-client-secret | GitHub OAuth secret | No | +| oauth2-dingtalk-client-id | DingTalk OAuth AppKey | No | +| oauth2-dingtalk-client-secret | DingTalk OAuth AppSecret | No | | skill-scanner-llm-api-key | LLM API key | No | | skill-scanner-llm-base-url | Local/custom LLM service base URL | No | | skill-scanner-llm-model | LLM model name used by the scanner | No | @@ -171,6 +173,7 @@ kubectl apply -k overlays/with-infra/ # or overlays/external/ |---|---|---| | redis-host | redis | Redis host address | | redis-port | 6379 | Redis port | +| spring-profiles-active | docker | Set to `docker,dingtalk` to enable DingTalk login | | storage-base-path | /var/lib/skillhub/storage | Skill storage path | | skillhub-storage-provider | local | Storage type (local/s3) | | skill-scanner-enabled | true | Enable scanner | @@ -182,6 +185,18 @@ kubectl apply -k overlays/with-infra/ # or overlays/external/ | bootstrap-admin-display-name | Platform Admin | Admin display name | | bootstrap-admin-email | admin@example.com | Admin email | | session-cookie-secure | false | Set to true for HTTPS | +| oauth2-dingtalk-display-name | 钉钉 | DingTalk login display name | + +### DingTalk OAuth2 + +DingTalk login is disabled by default. Set `spring-profiles-active` in the +ConfigMap to `docker,dingtalk`, then provide `oauth2-dingtalk-client-id` and +`oauth2-dingtalk-client-secret` in the Secret. Configure the callback URL in +DingTalk Open Platform as `{public-site-url}/login/oauth2/code/dingtalk`. The +authorization scope is fixed to `openid`. + +See the [official DingTalk tutorial](https://developers.dingtalk.com/document/orgapp/tutorial-obtaining-user-personal-information) +for the authorization contract. ### Storage Configuration diff --git a/docs/skillhub/guide/kubernetes.md b/docs/skillhub/guide/kubernetes.md index 2135b9a3..bde88025 100644 --- a/docs/skillhub/guide/kubernetes.md +++ b/docs/skillhub/guide/kubernetes.md @@ -62,6 +62,8 @@ cp secret.yaml.example secret.yaml | bootstrap-admin-password | 管理员密码 | 是 | | oauth2-github-client-id | GitHub OAuth ID | 否 | | oauth2-github-client-secret | GitHub OAuth 密钥 | 否 | +| oauth2-dingtalk-client-id | 钉钉 OAuth AppKey | 否 | +| oauth2-dingtalk-client-secret | 钉钉 OAuth AppSecret | 否 | | skill-scanner-llm-api-key | LLM API 密钥 | 否 | | skill-scanner-llm-base-url | 本地/自定义 LLM 服务地址 | 否 | | skill-scanner-llm-model | Scanner 使用的 LLM 模型名 | 否 | @@ -171,6 +173,7 @@ kubectl apply -k overlays/with-infra/ # 或 overlays/external/ |---|---|---| | redis-host | redis | Redis 主机地址 | | redis-port | 6379 | Redis 端口 | +| spring-profiles-active | docker | 启用钉钉登录时改为 `docker,dingtalk` | | storage-base-path | /var/lib/skillhub/storage | 技能存储路径 | | skillhub-storage-provider | local | 存储类型（local/s3） | | skill-scanner-enabled | true | 是否启用扫描器 | @@ -182,6 +185,16 @@ kubectl apply -k overlays/with-infra/ # 或 overlays/external/ | bootstrap-admin-display-name | Platform Admin | 管理员显示名称 | | bootstrap-admin-email | admin@example.com | 管理员邮箱 | | session-cookie-secure | false | HTTPS 环境设为 true | +| oauth2-dingtalk-display-name | 钉钉 | 钉钉登录入口显示名称 | + +### 钉钉 OAuth2 + +钉钉登录默认关闭。将 ConfigMap 的 `spring-profiles-active` 改为 +`docker,dingtalk`，并在 Secret 中填写 `oauth2-dingtalk-client-id` 与 +`oauth2-dingtalk-client-secret` 后才会注册登录入口。钉钉开放平台的回调地址应为 +`{站点公网地址}/login/oauth2/code/dingtalk`，授权 scope 固定为 `openid`。 + +完整授权契约参见[钉钉官方教程](https://developers.dingtalk.com/document/orgapp/tutorial-obtaining-user-personal-information)。 ### 存储配置 diff --git a/document/docs/02-administration/deployment/configuration.md b/document/docs/02-administration/deployment/configuration.md index 53ae33da..f04387bb 100644 --- a/document/docs/02-administration/deployment/configuration.md +++ b/document/docs/02-administration/deployment/configuration.md @@ -51,9 +51,6 @@ SkillHub 通过环境变量进行配置，主要配置项如下： |---------|------|--------| | `OAUTH2_GITHUB_CLIENT_ID` | GitHub OAuth Client ID | - | | `OAUTH2_GITHUB_CLIENT_SECRET` | GitHub OAuth Client Secret | - | -| `OAUTH2_DINGTALK_CLIENT_ID` | 钉钉 OAuth AppKey | - | -| `OAUTH2_DINGTALK_CLIENT_SECRET` | 钉钉 OAuth AppSecret | - | -| `OAUTH2_DINGTALK_DISPLAY_NAME` | 钉钉登录按钮显示名 | `钉钉` | ### 首登管理员配置 diff --git a/document/docs/02-administration/security/authentication.md b/document/docs/02-administration/security/authentication.md index 0f6246b9..923878ee 100644 --- a/document/docs/02-administration/security/authentication.md +++ b/document/docs/02-administration/security/authentication.md @@ -19,20 +19,6 @@ SkillHub 支持多种认证方式，满足不同企业的安全需求。 OAUTH2_GITHUB_CLIENT_SECRET=your-client-secret ``` -### 钉钉 OAuth2 - -1. 在[钉钉开放平台](https://open-dev.dingtalk.com/)创建 H5 微应用，获取 AppKey 和 AppSecret -2. 开通 `Contact.User.Read` 权限（获取用户信息） -3. 发布应用版本以激活 OAuth2 凭证 -4. 回调地址填写 `{baseUrl}/login/oauth2/code/dingtalk` -5. 配置环境变量： - ```bash - OAUTH2_DINGTALK_CLIENT_ID=你的AppKey - OAUTH2_DINGTALK_CLIENT_SECRET=你的AppSecret - ``` - -> 钉钉使用 `corpid` scope（非标准 OIDC `openid`），用户以 `unionId` 作为唯一标识。 - ### 扩展 OAuth Provider 架构支持扩展其他 OAuth Provider，如 GitLab、Gitee 等。 diff --git a/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/deployment/configuration.md b/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/deployment/configuration.md index c70754f2..68055f24 100644 --- a/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/deployment/configuration.md +++ b/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/deployment/configuration.md @@ -51,9 +51,6 @@ SkillHub is configured through environment variables. The main configuration ite |---------------------|-------------|---------------| | `OAUTH2_GITHUB_CLIENT_ID` | GitHub OAuth Client ID | - | | `OAUTH2_GITHUB_CLIENT_SECRET` | GitHub OAuth Client Secret | - | -| `OAUTH2_DINGTALK_CLIENT_ID` | DingTalk OAuth AppKey | - | -| `OAUTH2_DINGTALK_CLIENT_SECRET` | DingTalk OAuth AppSecret | - | -| `OAUTH2_DINGTALK_DISPLAY_NAME` | DingTalk login button display name | `钉钉` | ### Bootstrap Admin Configuration diff --git a/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/security/authentication.md b/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/security/authentication.md index 94d49df2..148252f7 100644 --- a/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/security/authentication.md +++ b/document/i18n/en/docusaurus-plugin-content-docs/current/02-administration/security/authentication.md @@ -19,20 +19,6 @@ SkillHub supports multiple authentication methods to meet different enterprise s OAUTH2_GITHUB_CLIENT_SECRET=your-client-secret ``` -### DingTalk OAuth2 - -1. Create an H5 micro-app on [DingTalk Open Platform](https://open-dev.dingtalk.com/) and obtain AppKey and AppSecret -2. Enable the `Contact.User.Read` permission (required for fetching user info) -3. Publish the app version to activate OAuth2 credentials -4. Set the callback URL to `{baseUrl}/login/oauth2/code/dingtalk` -5. Configure environment variables: - ```bash - OAUTH2_DINGTALK_CLIENT_ID=your-appkey - OAUTH2_DINGTALK_CLIENT_SECRET=your-appsecret - ``` - -> DingTalk uses `corpid` scope (not standard OIDC `openid`). Users are identified by `unionId`. - ### Extend OAuth Provider The architecture supports extending to other OAuth providers like GitLab, Gitee, etc. diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index bee780fb..491ee684 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -150,6 +150,31 @@ write_env "$invalid_redis_sentinel_check_env" "release-download-secret-32-bytes- printf '%s\n' "SKILLHUB_REDIS_SENTINEL_CHECK_SENTINELS_LIST=yes" >>"$invalid_redis_sentinel_check_env" expect_fail "$invalid_redis_sentinel_check_env" "SKILLHUB_REDIS_SENTINEL_CHECK_SENTINELS_LIST must be true or false" +dingtalk_env="$tmp/dingtalk.env" +write_env "$dingtalk_env" "release-download-secret-32-bytes-minimum" +cat >>"$dingtalk_env" </dev/null + +dingtalk_missing_secret_env="$tmp/dingtalk-missing-secret.env" +write_env "$dingtalk_missing_secret_env" "release-download-secret-32-bytes-minimum" +cat >>"$dingtalk_missing_secret_env" <>"$dingtalk_disabled_env" <"$draft_env" expect_fail "$draft_env" "POSTGRES_PASSWORD" +grep -Fq 'OAUTH2_DINGTALK_CLIENT_ID: ${OAUTH2_DINGTALK_CLIENT_ID:-}' "$REPO_ROOT/compose.release.yml" \ + || fail "compose.release.yml does not pass OAUTH2_DINGTALK_CLIENT_ID" +grep -Fq 'key: oauth2-dingtalk-client-secret' "$REPO_ROOT/deploy/k8s/base/backend-deployment.yaml" \ + || fail "Kubernetes deployment does not pass the DingTalk client secret" +grep -Fq 'spring-profiles-active: docker' "$REPO_ROOT/deploy/k8s/base/configmap.yaml" \ + || fail "Kubernetes config does not expose Spring profile activation" + echo "validate-release-config-test passed" diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index 7c7940a3..50b5aa48 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -290,6 +290,23 @@ if [ -n "$oauth_secret" ] && [ -z "$oauth_id" ]; then error "OAUTH2_GITHUB_CLIENT_ID is required when OAUTH2_GITHUB_CLIENT_SECRET is set" fi +dingtalk_profiles=",${SPRING_PROFILES_ACTIVE:-docker}," +dingtalk_id="${OAUTH2_DINGTALK_CLIENT_ID:-}" +dingtalk_secret="${OAUTH2_DINGTALK_CLIENT_SECRET:-}" +case "$dingtalk_profiles" in + *,dingtalk,*) + require_non_empty OAUTH2_DINGTALK_CLIENT_ID + require_non_empty OAUTH2_DINGTALK_CLIENT_SECRET + reject_values OAUTH2_DINGTALK_CLIENT_ID "placeholder" "local-placeholder" + reject_values OAUTH2_DINGTALK_CLIENT_SECRET "placeholder" "local-placeholder" + ;; + *) + if [ -n "$dingtalk_id" ] || [ -n "$dingtalk_secret" ]; then + error "SPRING_PROFILES_ACTIVE must include dingtalk when DingTalk OAuth2 credentials are set" + fi + ;; +esac + if [ "$errors" -gt 0 ]; then echo "Release config validation failed: $errors error(s), $warnings warning(s)." >&2 exit 1 diff --git a/server/skillhub-app/src/main/resources/application-dingtalk.yml b/server/skillhub-app/src/main/resources/application-dingtalk.yml new file mode 100644 index 00000000..c30ee773 --- /dev/null +++ b/server/skillhub-app/src/main/resources/application-dingtalk.yml @@ -0,0 +1,22 @@ +spring: + config: + activate: + on-profile: dingtalk + security: + oauth2: + client: + registration: + dingtalk: + client-id: ${OAUTH2_DINGTALK_CLIENT_ID} + client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET} + scope: + - openid + authorization-grant-type: authorization_code + redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" + client-name: ${OAUTH2_DINGTALK_DISPLAY_NAME:钉钉} + provider: + dingtalk: + authorization-uri: https://login.dingtalk.com/oauth2/auth + token-uri: https://api.dingtalk.com/v1.0/oauth2/userAccessToken + user-info-uri: https://api.dingtalk.com/v1.0/contact/users/me + user-name-attribute: unionId diff --git a/server/skillhub-app/src/main/resources/application-local.yml b/server/skillhub-app/src/main/resources/application-local.yml index 3432e67c..0e390aa5 100644 --- a/server/skillhub-app/src/main/resources/application-local.yml +++ b/server/skillhub-app/src/main/resources/application-local.yml @@ -22,9 +22,6 @@ spring: github: client-id: ${OAUTH2_GITHUB_CLIENT_ID:local-placeholder} client-secret: ${OAUTH2_GITHUB_CLIENT_SECRET:local-placeholder} - dingtalk: - client-id: ${OAUTH2_DINGTALK_CLIENT_ID:local-placeholder} - client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET:local-placeholder} skillhub: auth: @@ -57,4 +54,5 @@ skillhub: logging: level: - com.iflytek.skillhub.auth: DEBUG + com.iflytek.skillhub: INFO + org.springframework.security: WARN diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 201e751e..421e27f6 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -69,14 +69,6 @@ spring: authorization-grant-type: authorization_code redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" client-name: ${OAUTH2_GITLAB_DISPLAY_NAME:GitLab} - dingtalk: - client-id: ${OAUTH2_DINGTALK_CLIENT_ID:placeholder} - client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET:placeholder} - scope: - - corpid - authorization-grant-type: authorization_code - redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" - client-name: ${OAUTH2_DINGTALK_DISPLAY_NAME:钉钉} provider: github: user-info-uri: https://api.github.com/user @@ -85,11 +77,6 @@ spring: token-uri: ${OAUTH2_GITLAB_BASE_URI:https://gitlab.com}/oauth/token user-info-uri: ${OAUTH2_GITLAB_BASE_URI:https://gitlab.com}/api/v4/user user-name-attribute: username - dingtalk: - authorization-uri: https://login.dingtalk.com/oauth2/auth - token-uri: https://api.dingtalk.com/v1.0/oauth2/userAccessToken - user-info-uri: https://api.dingtalk.com/v1.0/contact/users/me - user-name-attribute: unionId servlet: multipart: max-file-size: 100MB diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2CallbackIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2CallbackIntegrationTest.java new file mode 100644 index 00000000..a245a346 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2CallbackIntegrationTest.java @@ -0,0 +1,169 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UriUtils; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles({"test", "dingtalk"}) +@TestPropertySource(properties = { + "OAUTH2_DINGTALK_CLIENT_ID=test-dingtalk-client", + "OAUTH2_DINGTALK_CLIENT_SECRET=test-dingtalk-secret", + "spring.security.oauth2.client.registration.oidc.client-id=test-oidc-client", + "spring.security.oauth2.client.registration.oidc.client-secret=test-oidc-secret", + "spring.security.oauth2.client.registration.oidc.provider=oidc", + "spring.security.oauth2.client.registration.oidc.authorization-grant-type=authorization_code", + "spring.security.oauth2.client.registration.oidc.redirect-uri={baseUrl}/login/oauth2/code/{registrationId}", + "spring.security.oauth2.client.registration.oidc.scope=openid,profile,email", + "spring.security.oauth2.client.provider.oidc.authorization-uri=https://idp.example.test/oauth2/authorize", + "spring.security.oauth2.client.provider.oidc.token-uri=https://idp.example.test/oauth2/token", + "spring.security.oauth2.client.provider.oidc.jwk-set-uri=https://idp.example.test/oauth2/jwks", + "spring.security.oauth2.client.provider.oidc.user-info-uri=https://idp.example.test/userinfo", + "spring.security.oauth2.client.provider.oidc.user-name-attribute=sub" +}) +class DingTalkOAuth2CallbackIntegrationTest { + + @Autowired + private MockMvc mockMvc; + + @Autowired + private ClientRegistrationRepository clientRegistrationRepository; + + @Autowired + private DingTalkTokenResponseClient tokenResponseClient; + + @Autowired + private DingTalkOAuth2UserService userService; + + @MockBean + private OAuthLoginFlowService oauthLoginFlowService; + + private MockRestServiceServer tokenServer; + private MockRestServiceServer userInfoServer; + + @BeforeEach + void setUp() { + RestTemplate tokenRestTemplate = (RestTemplate) ReflectionTestUtils.getField( + tokenResponseClient, "restTemplate"); + RestTemplate userInfoRestTemplate = (RestTemplate) ReflectionTestUtils.getField( + userService, "restTemplate"); + assertThat(tokenRestTemplate).isNotNull(); + assertThat(userInfoRestTemplate).isNotNull(); + tokenServer = MockRestServiceServer.bindTo(tokenRestTemplate).build(); + userInfoServer = MockRestServiceServer.bindTo(userInfoRestTemplate).build(); + } + + @Test + void dingtalkProfileExposesProviderAndCompletesOAuth2Callback() throws Exception { + assertThat(clientRegistrationRepository.findByRegistrationId("dingtalk")).isNotNull(); + mockMvc.perform(get("/api/v1/auth/providers")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data[?(@.id=='dingtalk')]").isNotEmpty()); + + MvcResult authorizationResult = mockMvc.perform(get("/oauth2/authorization/dingtalk")) + .andExpect(status().is3xxRedirection()) + .andExpect(header().string("Location", org.hamcrest.Matchers.containsString("scope=openid"))) + .andReturn(); + + String authorizationLocation = authorizationResult.getResponse().getRedirectedUrl(); + assertThat(authorizationLocation).isNotNull(); + String encodedState = UriComponentsBuilder.fromUri(URI.create(authorizationLocation)) + .build() + .getQueryParams() + .getFirst("state"); + String state = UriUtils.decode(encodedState, StandardCharsets.UTF_8); + assertThat(state).isNotBlank(); + MockHttpSession session = (MockHttpSession) authorizationResult.getRequest().getSession(false); + assertThat(session).isNotNull(); + + tokenServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andExpect(method(HttpMethod.POST)) + .andRespond(withSuccess( + """ + {"accessToken":"dingtalk-access-token","expireIn":7200} + """, + MediaType.APPLICATION_JSON)); + userInfoServer.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andExpect(method(HttpMethod.GET)) + .andExpect(header(DingTalkOAuth2Constants.ACCESS_TOKEN_HEADER, "dingtalk-access-token")) + .andRespond(withSuccess( + """ + {"openId":"stable-open-id","nick":"DingTalk User"} + """, + MediaType.APPLICATION_JSON)); + + PlatformPrincipal principal = new PlatformPrincipal( + "user-dingtalk", "DingTalk User", null, null, "dingtalk", Set.of("USER")); + when(oauthLoginFlowService.authenticate(any(OAuthClaims.class))).thenReturn(principal); + + mockMvc.perform(get("/login/oauth2/code/dingtalk") + .param("code", "authorization-code") + .param("state", state) + .session(session)) + .andExpect(status().is3xxRedirection()) + .andExpect(header().string("Location", "/dashboard")); + + assertThat(session.getAttribute("platformPrincipal")).isEqualTo(principal); + tokenServer.verify(); + userInfoServer.verify(); + } + + @Test + void standardOAuth2AndOidcAuthorizationRoutesRemainIntact() throws Exception { + assertAuthorizationRedirectScopes("github", Set.of("read:user", "user:email"), false); + assertAuthorizationRedirectScopes("gitlab", Set.of("read_user", "email"), false); + assertAuthorizationRedirectScopes("oidc", Set.of("openid", "profile", "email"), true); + } + + private void assertAuthorizationRedirectScopes( + String registrationId, + Set expectedScopes, + boolean expectsNonce) throws Exception { + MvcResult result = mockMvc.perform(get("/oauth2/authorization/{registrationId}", registrationId)) + .andExpect(status().is3xxRedirection()) + .andReturn(); + + String location = result.getResponse().getRedirectedUrl(); + assertThat(location).isNotNull(); + var query = UriComponentsBuilder.fromUri(URI.create(location)).build().getQueryParams(); + String encodedScope = query.getFirst("scope"); + assertThat(encodedScope).isNotNull(); + assertThat(Arrays.asList(UriUtils.decode(encodedScope, StandardCharsets.UTF_8).split(" "))) + .containsExactlyInAnyOrderElementsOf(expectedScopes); + assertThat(query.containsKey("nonce")).isEqualTo(expectsNonce); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java index d7a13d01..6f454af6 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java @@ -150,14 +150,10 @@ class AuthControllerTest { mockMvc.perform(get("/api/v1/auth/providers")) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.length()").value(4)) - .andExpect(jsonPath("$.data[*].id", hasItems("github", "gitee", "gitlab", "dingtalk"))) - .andExpect(jsonPath("$.data[*].authorizationUrl", hasItems( - "/oauth2/authorization/github", - "/oauth2/authorization/gitee", - "/oauth2/authorization/gitlab", - "/oauth2/authorization/dingtalk" - ))) + .andExpect(jsonPath("$.data.length()").value(1)) + .andExpect(jsonPath("$.data[*].id", hasItems("github"))) + .andExpect(jsonPath("$.data[?(@.id=='dingtalk')]").isEmpty()) + .andExpect(jsonPath("$.data[*].authorizationUrl", hasItems("/oauth2/authorization/github"))) .andExpect(jsonPath("$.timestamp").isNotEmpty()) .andExpect(jsonPath("$.requestId").isNotEmpty()); } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java index 8208b047..2ae7fe6c 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.auth.config; import com.iflytek.skillhub.auth.oauth.CustomOAuth2UserService; import com.iflytek.skillhub.auth.oauth.CustomOidcUserService; +import com.iflytek.skillhub.auth.oauth.DingTalkOAuth2Constants; import com.iflytek.skillhub.auth.oauth.DingTalkOAuth2UserService; import com.iflytek.skillhub.auth.oauth.DingTalkTokenResponseClient; import com.iflytek.skillhub.auth.oauth.OAuth2LoginFailureHandler; @@ -48,7 +49,7 @@ import org.springframework.security.web.util.matcher.RequestMatcher; * Central Spring Security configuration for browser sessions, API tokens, and * public versus protected endpoints. */ -@Configuration(proxyBeanMethods = false) +@Configuration @EnableWebSecurity @EnableMethodSecurity public class SecurityConfig { @@ -203,7 +204,7 @@ public class SecurityConfig { } } -static boolean hasSessionCookie(HttpServletRequest request) { + static boolean hasSessionCookie(HttpServletRequest request) { if (request.getRequestedSessionId() != null) { return true; } @@ -235,7 +236,8 @@ static boolean hasSessionCookie(HttpServletRequest request) { @Override public OAuth2User loadUser(OAuth2UserRequest userRequest) { - if ("dingtalk".equals(userRequest.getClientRegistration().getRegistrationId())) { + if (DingTalkOAuth2Constants.REGISTRATION_ID.equals( + userRequest.getClientRegistration().getRegistrationId())) { return dingTalkService.loadUser(userRequest); } return defaultService.loadUser(userRequest); @@ -258,7 +260,8 @@ static boolean hasSessionCookie(HttpServletRequest request) { @Override public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest) { - if ("dingtalk".equals(authorizationCodeGrantRequest.getClientRegistration().getRegistrationId())) { + if (DingTalkOAuth2Constants.REGISTRATION_ID.equals( + authorizationCodeGrantRequest.getClientRegistration().getRegistrationId())) { return dingTalkClient.getTokenResponse(authorizationCodeGrantRequest); } return defaultClient.getTokenResponse(authorizationCodeGrantRequest); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java index 1a5956f3..43895795 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java @@ -1,13 +1,12 @@ package com.iflytek.skillhub.auth.oauth; +import java.util.Map; import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.stereotype.Component; -import java.util.Map; - /** * Provider-specific claims extractor for DingTalk (钉钉). * @@ -16,52 +15,72 @@ import java.util.Map; * *

Field mapping: *

    - *
  • subject → unionId (unique across all apps under the same developer account)
    • - *
  • email → unionId@dingtalk.local (synthetic, DingTalk users may not have email)
    • - *
  • emailVerified → true (synthetic)
    • + *
  • subject → unionId, falling back to openId and userId
    • + *
  • email → optional real email returned by DingTalk
    • + *
  • emailVerified → false because this endpoint does not attest email ownership
    • *
  • providerLogin → nick
    • *
* - *

Note: unionId is used instead of openId because openId is only unique within - * a single DingTalk application. If a user logs in through different DingTalk apps - * under the same developer account, openId would differ, causing duplicate accounts. - * unionId remains stable across all apps under the same developer. + *

unionId is preferred because it is stable across apps under the same developer. + * The fallbacks preserve login availability when DingTalk omits that optional field. */ @Component public class DingTalkClaimsExtractor implements OAuthClaimsExtractor { @Override public String getProvider() { - return "dingtalk"; + return DingTalkOAuth2Constants.REGISTRATION_ID; } @Override public OAuthClaims extract(OAuth2UserRequest request, OAuth2User oAuth2User) { Map attrs = oAuth2User.getAttributes(); - String unionId = (String) attrs.get("unionId"); - String openId = (String) attrs.get("openId"); - String nick = (String) attrs.get("nick"); + String subject = resolveSubject(attrs); - // unionId is required — it is the cross-app stable identity for DingTalk users - if (unionId == null || unionId.isEmpty()) { - throw new OAuth2AuthenticationException( - new OAuth2Error("missing_union_id", - "DingTalk response missing required unionId field. " - + "Ensure the 'openid' scope is configured and the DingTalk app " - + "has the Contact.User.Read permission.", null)); + String email = stringValue(attrs.get("email")); + String providerLogin = firstNonBlank(attrs, "nick", "name"); + if (providerLogin == null) { + providerLogin = subject; } - // DingTalk users may not have email; synthesize one for downstream compatibility - String syntheticEmail = unionId + "@dingtalk.local"; - return new OAuthClaims( - "dingtalk", - unionId, // Use unionId (cross-app unique) instead of openId (single-app only) - syntheticEmail, - true, - nick, + DingTalkOAuth2Constants.REGISTRATION_ID, + subject, + email, + false, + providerLogin, attrs ); } -} \ No newline at end of file + + String resolveSubject(Map attributes) { + String subject = firstNonBlank( + attributes, + DingTalkOAuth2Constants.SUBJECT_CLAIM_NAMES.toArray(String[]::new)); + if (subject == null) { + throw new OAuth2AuthenticationException( + new OAuth2Error("missing_subject", + "DingTalk response is missing unionId, openId, and userId", null)); + } + return subject; + } + + private static String firstNonBlank(Map attributes, String... keys) { + for (String key : keys) { + String value = stringValue(attributes.get(key)); + if (value != null) { + return value; + } + } + return null; + } + + private static String stringValue(Object value) { + if (value == null) { + return null; + } + String stringValue = String.valueOf(value).trim(); + return stringValue.isEmpty() ? null : stringValue; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java new file mode 100644 index 00000000..1f8b7241 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java @@ -0,0 +1,16 @@ +package com.iflytek.skillhub.auth.oauth; + +import java.util.List; + +/** Shared protocol constants for the DingTalk OAuth2 adapter. */ +public final class DingTalkOAuth2Constants { + + public static final String REGISTRATION_ID = "dingtalk"; + public static final String AUTHORIZATION_SCOPE = "openid"; + public static final String ACCESS_TOKEN_HEADER = "x-acs-dingtalk-access-token"; + public static final String SUBJECT_ATTRIBUTE = "dingtalkSubject"; + static final List SUBJECT_CLAIM_NAMES = List.of("unionId", "openId", "userId"); + + private DingTalkOAuth2Constants() { + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java index a3427ced..81dc4abc 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java @@ -1,12 +1,12 @@ package com.iflytek.skillhub.auth.oauth; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import java.time.Duration; import java.util.HashMap; import java.util.LinkedHashSet; import java.util.Map; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -16,14 +16,15 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; import org.springframework.security.oauth2.client.userinfo.OAuth2UserService; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.user.DefaultOAuth2User; import org.springframework.security.oauth2.core.user.OAuth2User; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate; -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; - /** * OAuth2UserService for DingTalk — handles DingTalk's non-standard user info * endpoint which uses a custom header {@code x-acs-dingtalk-access-token} @@ -36,8 +37,6 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; @Component public class DingTalkOAuth2UserService implements OAuth2UserService { - private static final Logger log = LoggerFactory.getLogger(DingTalkOAuth2UserService.class); - private final RestTemplate restTemplate; private final DingTalkClaimsExtractor claimsExtractor; private final OAuthLoginFlowService oauthLoginFlowService; @@ -74,29 +73,40 @@ public class DingTalkOAuth2UserService implements OAuth2UserService requestEntity = new HttpEntity<>(headers); - ResponseEntity response = restTemplate.exchange( - userInfoUri, - HttpMethod.GET, - requestEntity, - Map.class - ); + ResponseEntity> response; + try { + response = restTemplate.exchange( + userInfoUri, + HttpMethod.GET, + requestEntity, + new ParameterizedTypeReference<>() { + } + ); + } catch (RestClientResponseException e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("user_info_request_failed", + "DingTalk user-info request failed with HTTP " + e.getStatusCode().value(), null)); + } catch (RestClientException e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("user_info_request_failed", + "DingTalk user-info request failed", null)); + } Map attributes = response.getBody() != null ? response.getBody() : Map.of(); - // Map DingTalk response to standard attributes Map userAttributes = new HashMap<>(attributes); - userAttributes.putIfAbsent("openId", attributes.get("openId")); - userAttributes.putIfAbsent("nickName", attributes.get("nick")); - userAttributes.putIfAbsent("avatarUrl", attributes.get("avatarUrl")); + if (attributes.get("avatarUrl") != null) { + userAttributes.putIfAbsent("avatar_url", attributes.get("avatarUrl")); + } + + String subject = claimsExtractor.resolveSubject(userAttributes); + userAttributes.put(DingTalkOAuth2Constants.SUBJECT_ATTRIBUTE, subject); - // Extract claims — use unionId as the name attribute (cross-app unique identity) OAuthClaims claims = claimsExtractor.extract(userRequest, new DefaultOAuth2User( - java.util.Collections.emptyList(), userAttributes, "unionId")); - - log.info("DingTalk OAuth2 login: subject={}, providerLogin={}", claims.subject(), claims.providerLogin()); + java.util.Collections.emptyList(), userAttributes, DingTalkOAuth2Constants.SUBJECT_ATTRIBUTE)); // Delegate to OAuthLoginFlowService for access policy evaluation and identity binding PlatformPrincipal principal = oauthLoginFlowService.authenticate(claims); @@ -116,4 +126,4 @@ public class DingTalkOAuth2UserService implements OAuth2UserService response; try { response = restTemplate.postForEntity(tokenUri, new HttpEntity<>(tokenRequest, headers), String.class); - } catch (Exception e) { + } catch (RestClientResponseException e) { throw new OAuth2AuthenticationException( new OAuth2Error("token_exchange_io_error", - "Failed to exchange code for DingTalk access token: " + e.getMessage(), null), e); + "DingTalk token exchange failed with HTTP " + e.getStatusCode().value(), null)); + } catch (RestClientException e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_exchange_io_error", + "DingTalk token exchange request failed", null)); } if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { @@ -91,21 +95,31 @@ public class DingTalkTokenResponseClient implements OAuth2AccessTokenResponseCli "DingTalk token response missing accessToken field", null)); } String accessToken = accessTokenNode.asText(); - if (accessToken.isEmpty()) { + if (accessToken.isBlank()) { throw new OAuth2AuthenticationException( new OAuth2Error("token_response_missing_field", "DingTalk token response has empty accessToken", null)); } - // Only include non-sensitive fields in additional parameters - Map safeParams = new java.util.LinkedHashMap<>(); JsonNode expireInNode = json.get("expireIn"); - if (expireInNode != null && !expireInNode.isNull()) { - safeParams.put("expireIn", expireInNode.asLong()); + if (expireInNode == null || !expireInNode.isIntegralNumber() || !expireInNode.canConvertToLong()) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_invalid_expiry", + "DingTalk token response has invalid expireIn field", null)); } + long expireInSeconds = expireInNode.longValue(); + if (expireInSeconds <= 0) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_invalid_expiry", + "DingTalk token response has non-positive expireIn field", null)); + } + + // Only include non-sensitive fields in additional parameters. + Map safeParams = Map.of("expireIn", expireInSeconds); return OAuth2AccessTokenResponse.withToken(accessToken) .tokenType(OAuth2AccessToken.TokenType.BEARER) + .expiresIn(expireInSeconds) .additionalParameters(safeParams) .build(); } catch (OAuth2AuthenticationException e) { @@ -113,7 +127,7 @@ public class DingTalkTokenResponseClient implements OAuth2AccessTokenResponseCli } catch (Exception e) { throw new OAuth2AuthenticationException( new OAuth2Error("token_parse_error", - "Failed to parse DingTalk token response", null), e); + "Failed to parse DingTalk token response", null)); } } @@ -121,4 +135,4 @@ public class DingTalkTokenResponseClient implements OAuth2AccessTokenResponseCli new OAuth2Error("token_exchange_failed", "DingTalk token exchange failed: HTTP " + response.getStatusCode(), null)); } -} \ No newline at end of file +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginFailureHandler.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginFailureHandler.java index e228cd0a..659b4d5b 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginFailureHandler.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginFailureHandler.java @@ -3,14 +3,14 @@ package com.iflytek.skillhub.auth.oauth; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler; import org.springframework.stereotype.Component; -import java.io.IOException; - /** * Failure handler for OAuth logins that normalizes policy and account-state * failures into predictable user-facing redirects. @@ -30,7 +30,15 @@ public class OAuth2LoginFailureHandler extends SimpleUrlAuthenticationFailureHan public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) throws IOException, ServletException { - log.error("OAuth2 login failed: type={}, message={}", exception.getClass().getSimpleName(), exception.getMessage(), exception); + String errorCode = exception instanceof OAuth2AuthenticationException oauth2Exception + ? oauth2Exception.getError().getErrorCode() + : "unknown"; + log.error( + "OAuth2 login failed: path={}, type={}, errorCode={}", + request.getRequestURI(), + exception.getClass().getSimpleName(), + errorCode); + String returnTo = oauthLoginFlowService.consumeReturnTo(request.getSession(false)); String redirectTarget = oauthLoginFlowService.resolveFailureRedirect(exception, returnTo); if (redirectTarget != null) { diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java index b925c040..60e82cde 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java @@ -1,10 +1,15 @@ package com.iflytek.skillhub.auth.oauth; import jakarta.servlet.http.HttpServletRequest; +import java.util.LinkedHashSet; +import java.util.Set; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver; +import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames; import org.springframework.stereotype.Component; +import org.springframework.web.util.UriComponentsBuilder; /** * OAuth2 authorization request resolver that preserves a sanitized post-login @@ -30,13 +35,36 @@ public class SkillHubOAuth2AuthorizationRequestResolver public OAuth2AuthorizationRequest resolve(HttpServletRequest request) { OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request); oauthLoginFlowService.rememberReturnTo(request); - return authorizationRequest; + return adaptDingTalkRequest(authorizationRequest); } @Override public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) { OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request, clientRegistrationId); oauthLoginFlowService.rememberReturnTo(request); - return authorizationRequest; + return adaptDingTalkRequest(authorizationRequest); + } + + private static OAuth2AuthorizationRequest adaptDingTalkRequest( + OAuth2AuthorizationRequest authorizationRequest) { + if (authorizationRequest == null + || !DingTalkOAuth2Constants.REGISTRATION_ID.equals( + authorizationRequest.getAttribute(OAuth2ParameterNames.REGISTRATION_ID))) { + return authorizationRequest; + } + + Set oauth2Scopes = new LinkedHashSet<>(authorizationRequest.getScopes()); + oauth2Scopes.remove(DingTalkOAuth2Constants.AUTHORIZATION_SCOPE); + String authorizationRequestUri = UriComponentsBuilder + .fromUriString(authorizationRequest.getAuthorizationRequestUri()) + .replaceQueryParam(OidcParameterNames.NONCE) + .build(true) + .toUriString(); + return OAuth2AuthorizationRequest.from(authorizationRequest) + .scopes(oauth2Scopes) + .additionalParameters(parameters -> parameters.remove(OidcParameterNames.NONCE)) + .attributes(attributes -> attributes.remove(OidcParameterNames.NONCE)) + .authorizationRequestUri(authorizationRequestUri) + .build(); } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java index 645851e1..9d573f23 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java @@ -34,14 +34,14 @@ class DingTalkClaimsExtractorTest { assertThat(claims.provider()).isEqualTo("dingtalk"); assertThat(claims.subject()).isEqualTo("union123"); - assertThat(claims.email()).isEqualTo("union123@dingtalk.local"); - assertThat(claims.emailVerified()).isTrue(); + assertThat(claims.email()).isNull(); + assertThat(claims.emailVerified()).isFalse(); assertThat(claims.providerLogin()).isEqualTo("测试用户"); } @Test - void extract_throwsWhenUnionIdIsMissing() { - assertThatThrownBy(() -> extractor.extract( + void extract_fallsBackToOpenId() { + OAuthClaims claims = extractor.extract( userRequest(), new DefaultOAuth2User( java.util.List.of(), @@ -51,25 +51,60 @@ class DingTalkClaimsExtractorTest { ), "openId" ) - )).isInstanceOf(OAuth2AuthenticationException.class) - .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()).isEqualTo("missing_union_id")); + ); + + assertThat(claims.subject()).isEqualTo("open456"); } @Test - void extract_throwsWhenUnionIdIsEmpty() { - assertThatThrownBy(() -> extractor.extract( + void extract_fallsBackToUserIdWhenHigherPriorityIdentifiersAreBlank() { + OAuthClaims claims = extractor.extract( userRequest(), new DefaultOAuth2User( java.util.List.of(), Map.of( - "unionId", "", - "openId", "open456", + "unionId", " ", + "openId", "", + "userId", "user789", "nick", "测试用户" ), - "openId" + "userId" + ) + ); + + assertThat(claims.subject()).isEqualTo("user789"); + } + + @Test + void extract_preservesRealEmailWithoutClaimingVerification() { + OAuthClaims claims = extractor.extract( + userRequest(), + new DefaultOAuth2User( + java.util.List.of(), + Map.of( + "unionId", "union123", + "email", "user@example.com" + ), + "unionId" + ) + ); + + assertThat(claims.email()).isEqualTo("user@example.com"); + assertThat(claims.emailVerified()).isFalse(); + } + + @Test + void extract_throwsWhenAllStableIdentifiersAreMissing() { + assertThatThrownBy(() -> extractor.extract( + userRequest(), + new DefaultOAuth2User( + java.util.List.of(), + Map.of("nick", "测试用户"), + "nick" ) )).isInstanceOf(OAuth2AuthenticationException.class) - .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()).isEqualTo("missing_union_id")); + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex) + .getError().getErrorCode()).isEqualTo("missing_subject")); } @Test @@ -98,4 +133,4 @@ class DingTalkClaimsExtractorTest { ); return new OAuth2UserRequest(registration, accessToken); } -} \ No newline at end of file +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java index 40b2c44b..8b321617 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.auth.oauth; 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.Mockito.mock; import static org.mockito.Mockito.when; @@ -8,7 +9,9 @@ import static org.springframework.test.web.client.match.MockRestRequestMatchers. import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import java.time.Instant; import java.util.Map; import java.util.Set; @@ -20,10 +23,10 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.test.web.client.MockRestServiceServer; import org.springframework.web.client.RestTemplate; -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; class DingTalkOAuth2UserServiceTest { @@ -62,7 +65,7 @@ class DingTalkOAuth2UserServiceTest { // Mock OAuthLoginFlowService to return a principal PlatformPrincipal principal = new PlatformPrincipal( - "user-union123", "测试用户", "union123@dingtalk.local", + "user-union123", "测试用户", null, "https://example.com/avatar.jpg", "dingtalk", Set.of("USER") ); when(oauthLoginFlowService.authenticate(any(OAuthClaims.class))).thenReturn(principal); @@ -98,7 +101,7 @@ class DingTalkOAuth2UserServiceTest { )); PlatformPrincipal principal = new PlatformPrincipal( - "user-union789", "自定义用户", "union789@dingtalk.local", + "user-union789", "自定义用户", null, null, "dingtalk", Set.of("USER") ); when(oauthLoginFlowService.authenticate(any(OAuthClaims.class))).thenReturn(principal); @@ -109,6 +112,45 @@ class DingTalkOAuth2UserServiceTest { mockServer.verify(); } + @Test + void loadUser_supportsOpenIdFallbackWhenUnionIdIsMissing() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withSuccess( + """ + { + "openId": "open456", + "nick": "测试用户" + } + """, + MediaType.APPLICATION_JSON + )); + + PlatformPrincipal principal = new PlatformPrincipal( + "user-open456", "测试用户", null, null, "dingtalk", Set.of("USER") + ); + when(oauthLoginFlowService.authenticate(any(OAuthClaims.class))).thenReturn(principal); + + OAuth2User oauth2User = service.loadUser(userRequest()); + + assertThat(oauth2User.getName()).isEqualTo("user-open456"); + assertThat(oauth2User.getAttributes().get(DingTalkOAuth2Constants.SUBJECT_ATTRIBUTE)) + .isEqualTo("open456"); + } + + @Test + void loadUser_wrapsHttpFailureWithoutExposingResponseBody() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withServerError().body("sensitive-upstream-response")); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> { + OAuth2AuthenticationException oauthException = (OAuth2AuthenticationException) ex; + assertThat(oauthException.getError().getErrorCode()).isEqualTo("user_info_request_failed"); + assertThat(oauthException.getMessage()).doesNotContain("sensitive-upstream-response"); + }); + } + private OAuth2UserRequest userRequest() { ClientRegistration registration = ClientRegistration.withRegistrationId("dingtalk") .clientId("dingzgzf3b9k7jv74iq2") @@ -152,4 +194,4 @@ class DingTalkOAuth2UserServiceTest { ); return new OAuth2UserRequest(registration, accessToken); } -} \ No newline at end of file +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java index 47dbb09c..ac15a660 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java @@ -6,6 +6,7 @@ import static org.springframework.test.web.client.match.MockRestRequestMatchers. import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError; +import java.time.Duration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.http.MediaType; @@ -50,6 +51,11 @@ class DingTalkTokenResponseClientTest { assertThat(response.getAccessToken().getTokenValue()).isEqualTo("dt_access_token_123"); assertThat(response.getAccessToken().getTokenType()).isEqualTo(OAuth2AccessToken.TokenType.BEARER); + assertThat(response.getAccessToken().getIssuedAt()).isNotNull(); + assertThat(response.getAccessToken().getExpiresAt()).isNotNull(); + assertThat(Duration.between( + response.getAccessToken().getIssuedAt(), + response.getAccessToken().getExpiresAt())).isEqualTo(Duration.ofSeconds(7200)); assertThat(response.getAdditionalParameters().get("expireIn")).isEqualTo(7200L); // Verify raw_response is NOT included (sensitive data leak fix) assertThat(response.getAdditionalParameters().containsKey("raw_response")).isFalse(); @@ -112,14 +118,19 @@ class DingTalkTokenResponseClientTest { @Test void getTokenResponse_throwsOnHttpError() { mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) - .andRespond(withServerError()); + .andRespond(withServerError().body("sensitive-upstream-response")); assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) - .isInstanceOf(OAuth2AuthenticationException.class); + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> { + OAuth2AuthenticationException oauthException = (OAuth2AuthenticationException) ex; + assertThat(oauthException.getError().getErrorCode()).isEqualTo("token_exchange_io_error"); + assertThat(oauthException.getMessage()).doesNotContain("sensitive-upstream-response"); + }); } @Test - void getTokenResponse_doesNotIncludeExpireInWhenMissing() { + void getTokenResponse_throwsWhenExpireInIsMissing() { mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) .andRespond(withSuccess( """ @@ -130,11 +141,29 @@ class DingTalkTokenResponseClientTest { MediaType.APPLICATION_JSON )); - OAuth2AccessTokenResponse response = client.getTokenResponse(authorizationCodeGrantRequest()); + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex) + .getError().getErrorCode()).isEqualTo("token_response_invalid_expiry")); + } - assertThat(response.getAccessToken().getTokenValue()).isEqualTo("dt_access_token_123"); - assertThat(response.getAdditionalParameters().containsKey("expireIn")).isFalse(); - assertThat(response.getAdditionalParameters().containsKey("raw_response")).isFalse(); + @Test + void getTokenResponse_throwsWhenExpireInIsNonPositive() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": "dt_access_token_123", + "expireIn": 0 + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex) + .getError().getErrorCode()).isEqualTo("token_response_invalid_expiry")); } private OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest() { @@ -169,4 +198,4 @@ class DingTalkTokenResponseClientTest { new OAuth2AuthorizationExchange(authRequest, authResponse) ); } -} \ No newline at end of file +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java index 357ada33..3cb7b42c 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java @@ -8,6 +8,8 @@ import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -18,7 +20,23 @@ class OAuth2AuthorizationRequestResolverTest { @BeforeEach void setUp() { - ClientRegistration github = ClientRegistration.withRegistrationId("github") + ClientRegistration github = clientRegistration("github", "read:user"); + ClientRegistration gitlab = clientRegistration("gitlab", "read_user"); + ClientRegistration dingtalk = clientRegistration("dingtalk", "openid"); + ClientRegistration oidc = clientRegistration("oidc", "openid"); + OAuthLoginFlowService oauthLoginFlowService = new OAuthLoginFlowService( + java.util.List.of(), + mock(AccessPolicy.class), + mock(IdentityBindingService.class) + ); + resolver = new SkillHubOAuth2AuthorizationRequestResolver( + new InMemoryClientRegistrationRepository(github, gitlab, dingtalk, oidc), + oauthLoginFlowService + ); + } + + private static ClientRegistration clientRegistration(String registrationId, String scope) { + return ClientRegistration.withRegistrationId(registrationId) .clientId("client") .clientSecret("secret") .authorizationUri("https://example.test/oauth/authorize") @@ -26,19 +44,10 @@ class OAuth2AuthorizationRequestResolverTest { .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") .userInfoUri("https://example.test/user") .userNameAttributeName("id") - .authorizationGrantType(org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE) - .scope("read:user") - .clientName("GitHub") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .scope(scope) + .clientName(registrationId) .build(); - OAuthLoginFlowService oauthLoginFlowService = new OAuthLoginFlowService( - java.util.List.of(), - mock(AccessPolicy.class), - mock(IdentityBindingService.class) - ); - resolver = new SkillHubOAuth2AuthorizationRequestResolver( - new InMemoryClientRegistrationRepository(github), - oauthLoginFlowService - ); } @Test @@ -65,4 +74,47 @@ class OAuth2AuthorizationRequestResolverTest { assertThat(session).isNotNull(); assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE)).isNull(); } + + @Test + void resolve_sendsDingTalkOpenIdScopeWithoutTriggeringOidc() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/dingtalk"); + + OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request, "dingtalk"); + + assertThat(authorizationRequest).isNotNull(); + assertThat(authorizationRequest.getAuthorizationRequestUri()).contains("scope=openid"); + assertThat(authorizationRequest.getScopes()).doesNotContain("openid"); + assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey("nonce"); + } + + @Test + void resolve_preservesStandardOAuth2ProviderScopes() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/github"); + + OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request, "github"); + + assertThat(authorizationRequest).isNotNull(); + assertThat(authorizationRequest.getScopes()).containsExactly("read:user"); + } + + @Test + void resolve_preservesGitLabOAuth2Scopes() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/gitlab"); + + OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request, "gitlab"); + + assertThat(authorizationRequest).isNotNull(); + assertThat(authorizationRequest.getScopes()).containsExactly("read_user"); + } + + @Test + void resolve_preservesOpenIdForRealOidcProviders() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/oidc"); + + OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request, "oidc"); + + assertThat(authorizationRequest).isNotNull(); + assertThat(authorizationRequest.getScopes()).containsExactly("openid"); + assertThat(authorizationRequest.getAdditionalParameters()).containsKey("nonce"); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java index 52c0077b..350277aa 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java @@ -2,6 +2,9 @@ package com.iflytek.skillhub.auth.oauth; import jakarta.servlet.http.HttpSession; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -20,6 +23,7 @@ import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +@ExtendWith(OutputCaptureExtension.class) class OAuth2LoginHandlersTest { @Test @@ -130,4 +134,29 @@ class OAuth2LoginHandlersTest { assertThat(response.getRedirectedUrl()).isEqualTo("/login?returnTo=%2Fsettings%2Faccounts"); assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE)).isNull(); } + + @Test + void failureHandler_logsErrorCodeWithoutSensitiveExceptionDetails(CapturedOutput output) throws Exception { + OAuthLoginFlowService oauthLoginFlowService = mock(OAuthLoginFlowService.class); + OAuth2LoginFailureHandler handler = new OAuth2LoginFailureHandler(oauthLoginFlowService); + MockHttpServletRequest request = new MockHttpServletRequest( + "GET", "/login/oauth2/code/dingtalk"); + MockHttpServletResponse response = new MockHttpServletResponse(); + org.mockito.Mockito.when(oauthLoginFlowService.resolveFailureRedirect( + org.mockito.ArgumentMatchers.any(), + org.mockito.ArgumentMatchers.isNull())) + .thenReturn(null); + + handler.onAuthenticationFailure( + request, + response, + new OAuth2AuthenticationException(new OAuth2Error( + "user_info_request_failed", "sensitive-upstream-response", null)) + ); + + assertThat(output).contains( + "OAuth2 login failed: path=/login/oauth2/code/dingtalk, " + + "type=OAuth2AuthenticationException, errorCode=user_info_request_failed"); + assertThat(output).doesNotContain("sensitive-upstream-response"); + } } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java index 853aa464..72411f44 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java @@ -5,7 +5,7 @@ import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationStatus; import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; -public interface SkillStorageDeletionCompensationJpaRepository +interface SkillStorageDeletionCompensationJpaRepository extends JpaRepository { List findTop100ByStatusOrderByCreatedAtAsc( diff --git a/web/public/dingtalk-logo.svg b/web/public/dingtalk-logo.svg index e71bfbbc..b1a268d1 100644 --- a/web/public/dingtalk-logo.svg +++ b/web/public/dingtalk-logo.svg @@ -1,4 +1,3 @@ - - - - \ No newline at end of file + + +