diff --git a/charts/skillhub/README.md b/charts/skillhub/README.md index e7eb76fe6..640aa0795 100644 --- a/charts/skillhub/README.md +++ b/charts/skillhub/README.md @@ -48,7 +48,8 @@ helm -n skillhub upgrade -i skillhub ./charts/skillhub \ ``` 未显式设置 `deviceAuthVerificationUri` 时，Chart 使用 -`/cli/auth`。所有 values 会先经过 `values.schema.json` 和跨字段校验， +`/device`，保留部署子路径。`/cli/auth` 是旧版 loopback 回调入口， +需要客户端提供回调参数，不能作为设备码授权地址。所有 values 会先经过 `values.schema.json` 和跨字段校验， 无效的组件、Ingress、HPA 与存储组合会在安装前失败。 首次安装可通过 `auth.initialSettings.passwordLoginEnabled` 和 diff --git a/charts/skillhub/templates/configmap.yaml b/charts/skillhub/templates/configmap.yaml index 3130fb623..409b70209 100644 --- a/charts/skillhub/templates/configmap.yaml +++ b/charts/skillhub/templates/configmap.yaml @@ -49,7 +49,7 @@ data: public-base-url: {{ .Values.publicBaseUrl | quote }} {{- $deviceAuthVerificationUri := .Values.deviceAuthVerificationUri }} {{- if and (not $deviceAuthVerificationUri) .Values.publicBaseUrl }} - {{- $deviceAuthVerificationUri = printf "%s/cli/auth" (trimSuffix "/" .Values.publicBaseUrl) }} + {{- $deviceAuthVerificationUri = printf "%s/device" (trimSuffix "/" .Values.publicBaseUrl) }} {{- end }} device-auth-verification-uri: {{ $deviceAuthVerificationUri | quote }} auth-direct-enabled: {{ .Values.auth.direct.enabled | quote }} diff --git a/charts/skillhub/tests/configuration-contracts.sh b/charts/skillhub/tests/configuration-contracts.sh index 311989971..b0bfd0b7a 100755 --- a/charts/skillhub/tests/configuration-contracts.sh +++ b/charts/skillhub/tests/configuration-contracts.sh @@ -297,7 +297,18 @@ grep -Fq 'bootstrap-admin-display-name: "Ops: Admin"' "$TMP_DIR/special.yaml" render device "$CHART_DIR" \ --set publicBaseUrl=https://skills.example.com \ --show-only templates/configmap.yaml >"$TMP_DIR/device.yaml" -grep -Fq 'device-auth-verification-uri: "https://skills.example.com/cli/auth"' "$TMP_DIR/device.yaml" +grep -Fq 'device-auth-verification-uri: "https://skills.example.com/device"' "$TMP_DIR/device.yaml" + +render device-subpath "$CHART_DIR" \ + --set publicBaseUrl=https://skills.example.com/portal/ \ + --show-only templates/configmap.yaml >"$TMP_DIR/device-subpath.yaml" +grep -Fq 'device-auth-verification-uri: "https://skills.example.com/portal/device"' "$TMP_DIR/device-subpath.yaml" + +render device-override "$CHART_DIR" \ + --set publicBaseUrl=https://skills.example.com \ + --set-string 'deviceAuthVerificationUri=https://login.example.com/custom/device?source=cli' \ + --show-only templates/configmap.yaml >"$TMP_DIR/device-override.yaml" +grep -Fq 'device-auth-verification-uri: "https://login.example.com/custom/device?source=cli"' "$TMP_DIR/device-override.yaml" render tls "$CHART_DIR" \ --set ingress.enabled=true \ diff --git a/compose.release.yml b/compose.release.yml index 876655749..bd8ca4997 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -90,7 +90,7 @@ services: SKILLHUB_SUITE_REVIEW_WRITES_ENABLED: ${SKILLHUB_SUITE_REVIEW_WRITES_ENABLED:-true} SESSION_COOKIE_SECURE: ${SESSION_COOKIE_SECURE:-false} SKILLHUB_PUBLIC_BASE_URL: ${SKILLHUB_PUBLIC_BASE_URL:-} - DEVICE_AUTH_VERIFICATION_URI: ${DEVICE_AUTH_VERIFICATION_URI:-/device} + DEVICE_AUTH_VERIFICATION_URI: ${DEVICE_AUTH_VERIFICATION_URI:-} SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET:?required} SKILLHUB_STORAGE_PROVIDER: ${SKILLHUB_STORAGE_PROVIDER:-s3} STORAGE_BASE_PATH: /var/lib/skillhub/storage diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 9299b9bff..af6c3462b 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -2,6 +2,26 @@ 对象存储可选部署、凭证和迁移说明见[对象存储手册](object-storage.md)。 +### CLI 设备授权地址 + +`skillhub login --no-open --registry https://skills.example.com` 使用设备码授权： +CLI 请求 `/api/v1/auth/device/code`，用户打开返回的 `/device` 页面并登录、确认授权， +CLI 再轮询兑换 Token。完成后用 `skillhub whoami --registry https://skills.example.com` +确认登录身份；不要分享设备码、用户码或 Token。 + +- `DEVICE_AUTH_VERIFICATION_URI` 未设置、为空或仅含空白时，服务端使用 + `SKILLHUB_PUBLIC_BASE_URL` 加 `/device`，保留子路径并移除末尾重复斜杠。 + 例如 `https://skills.example.com/portal/` 对应 `https://skills.example.com/portal/device`。 +- 若公开地址也未设置，保留兼容的 `/device` 相对地址，由 CLI 按 registry 的源解析。 + 正式部署、子路径部署或 Web/API 分离部署应设置正确的 Web 公开地址。 +- 显式指定有效的 HTTP(S) 地址或根相对路径时原样使用，支持自定义页面和查询参数。 + 非法地址会阻止服务启动，错误提示只包含配置项名称，不回显配置值。修改后需重启服务。 +- Compose 的空配置交由后端处理；Helm 的 `deviceAuthVerificationUri` 默认对应 + `/device`，也可显式覆盖。 + +`/cli/auth` 是旧版浏览器 loopback 回调入口，需要客户端附带回调参数；它不是输入设备码的页面。 +不要将设备授权地址设成这个旧入口。此次配置保护不改变旧客户端回调流程。 + ## 1 运行模型 按用途选择运行方式；以下重点介绍源码开发和单机交付，Kubernetes 另见[部署指南](skillhub/guide/kubernetes.md)： diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index 0d7426ebe..062f74995 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -95,6 +95,39 @@ SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com \ grep -Fq 'https://skillhub.example.com/login/oauth2/code/feishu' "$compose_default_redirect" \ || fail "compose must derive the default Feishu redirect URI from SKILLHUB_PUBLIC_BASE_URL" +# Preserve empty device overrides so the backend derives /device from the public +# URL, including a deployment sub-path, rather than forcing a root-level route. +for device_override in '' ' ' 'https://login.example.com/custom/device'; do + DEVICE_AUTH_VERIFICATION_URI="$device_override" \ + SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=release-download-secret-32-bytes-minimum \ + SKILLHUB_PUBLIC_BASE_URL=https://skillhub.example.com/portal \ + docker compose -f "$REPO_ROOT/compose.release.yml" config --format json \ + >"$tmp/compose-device.json" + python3 - "$tmp/compose-device.json" "$device_override" <<'PY' +import json +import sys +with open(sys.argv[1]) as source: + environment = json.load(source)["services"]["server"]["environment"] +assert environment["DEVICE_AUTH_VERIFICATION_URI"] == sys.argv[2] +assert environment["SKILLHUB_PUBLIC_BASE_URL"] == "https://skillhub.example.com/portal" +PY +done + +for device_override in '' ' ' '/portal/device' 'https://login.example.com/custom/device?source=cli#confirm'; do + device_env="$tmp/device-valid.env" + write_env "$device_env" "release-download-secret-32-bytes-minimum" + printf 'DEVICE_AUTH_VERIFICATION_URI=%s\n' "$device_override" >>"$device_env" + "$SCRIPT" "$device_env" >/dev/null +done + +for device_override in 'javascript:alert(1)' '//login.example.com/device' 'https:///device' \ + 'https://user:secret@example.com/device' 'https://skills.example.com/invalid path'; do + device_env="$tmp/device-invalid.env" + write_env "$device_env" "release-download-secret-32-bytes-minimum" + printf 'DEVICE_AUTH_VERIFICATION_URI=%s\n' "$device_override" >>"$device_env" + expect_fail "$device_env" "DEVICE_AUTH_VERIFICATION_URI" +done + valid_feishu_env="$tmp/valid-feishu.env" write_env "$valid_feishu_env" "release-download-secret-32-bytes-minimum" cat >>"$valid_feishu_env" <<'EOF' diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index f84cbb4e1..71d3f7b76 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -98,6 +98,38 @@ validate_url() { esac } +validate_device_auth_verification_uri() { + device_uri=${DEVICE_AUTH_VERIFICATION_URI:-} + # Unlike a base URL, this is a complete destination. Blank means derive it + # in the backend; custom destinations may include query/fragment components. + if [ -z "$(printf '%s' "$device_uri" | tr -d '[:space:]')" ]; then + return 0 + fi + case "$device_uri" in + *[[:space:]]*|*'\'*) + error "DEVICE_AUTH_VERIFICATION_URI must be a valid HTTP(S) URL or root-relative path" + return 0 + ;; + //*) + error "DEVICE_AUTH_VERIFICATION_URI must not be a scheme-relative URL" + return 0 + ;; + /*) return 0 ;; + esac + case "$device_uri" in + [hH][tT][tT][pP]://*|[hH][tT][tT][pP][sS]://*) + device_authority=${device_uri#*://} + device_authority=${device_authority%%[/?#]*} + case "$device_authority" in + ""|:*|*@*) + error "DEVICE_AUTH_VERIFICATION_URI must include a host without credentials" + ;; + esac + ;; + *) error "DEVICE_AUTH_VERIFICATION_URI must be an HTTP(S) URL or root-relative path" ;; + esac +} + validate_web_api_base_url() { var_name="$1" eval "var_value=\${$var_name:-}" @@ -369,9 +401,7 @@ validate_web_base_path_format validate_public_base_path_alignment validate_api_base_path_alignment -if [ -n "${DEVICE_AUTH_VERIFICATION_URI:-}" ]; then - validate_url DEVICE_AUTH_VERIFICATION_URI -fi +validate_device_auth_verification_uri if [ "${SESSION_COOKIE_SECURE:-true}" != "true" ]; then warn "SESSION_COOKIE_SECURE is not true; only acceptable behind plain HTTP during temporary local verification" diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 3b2f58f82..c0d01b7db 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -277,7 +277,7 @@ skillhub: editable: false requires-review: false device-auth: - verification-uri: ${DEVICE_AUTH_VERIFICATION_URI:${skillhub.public.base-url:}/device} + verification-uri: ${DEVICE_AUTH_VERIFICATION_URI:} security: scanner: enabled: ${SKILLHUB_SECURITY_SCANNER_ENABLED:true} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/config/DeviceAuthConfigurationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/config/DeviceAuthConfigurationTest.java new file mode 100644 index 000000000..a70cc51ea --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/config/DeviceAuthConfigurationTest.java @@ -0,0 +1,114 @@ +package com.iflytek.skillhub.config; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import java.io.IOException; +import java.util.Map; +import java.util.function.Consumer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.boot.env.YamlPropertySourceLoader; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.env.SystemEnvironmentPropertySource; +import org.springframework.core.io.ClassPathResource; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.core.ValueOperations; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class DeviceAuthConfigurationTest { + + @Test + void unsetOverrideUsesPublicBaseUrl() throws IOException { + assertVerificationUri(Map.of("SKILLHUB_PUBLIC_BASE_URL", "https://skills.example.com"), + "https://skills.example.com/device"); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t\n"}) + void blankOverrideUsesPublicBaseUrl(String override) throws IOException { + assertVerificationUri(Map.of("SKILLHUB_PUBLIC_BASE_URL", "https://skills.example.com/portal/", + "DEVICE_AUTH_VERIFICATION_URI", override), + "https://skills.example.com/portal/device"); + } + + @Test + void absentPublicUrlKeepsSameOriginDefault() throws IOException { + assertVerificationUri(Map.of(), "/device"); + assertVerificationUri(Map.of("DEVICE_AUTH_VERIFICATION_URI", " ", + "SKILLHUB_PUBLIC_BASE_URL", " "), "/device"); + } + + @ParameterizedTest + @ValueSource(strings = {"https://login.example.com/custom/verify?source=cli#confirm", "/portal/device"}) + void validOverrideIsPreserved(String override) throws IOException { + assertVerificationUri(Map.of("SKILLHUB_PUBLIC_BASE_URL", "https://skills.example.com", + "DEVICE_AUTH_VERIFICATION_URI", override), override); + } + + @ParameterizedTest + @ValueSource(strings = {"javascript:alert(1)", "//other.example.com/device", "https:///device", + "https://user:secret@example.com/device", "https://skills.example.com/invalid path", + "https://skills.example.com:65536/device"}) + void invalidOverrideFailsContextWithoutEchoingItsValue(String override) throws IOException { + run(Map.of("DEVICE_AUTH_VERIFICATION_URI", override), context -> { + assertThat(context).hasFailed(); + assertThat(context.getStartupFailure()).hasRootCauseInstanceOf(IllegalArgumentException.class); + assertThat(context.getStartupFailure()).hasStackTraceContaining("skillhub.device-auth.verification-uri") + .hasStackTraceContaining("HTTP(S)") + .hasStackTraceContaining("root-relative"); + assertThat(context.getStartupFailure().getMessage()).doesNotContain(override); + }); + } + + @ParameterizedTest + @ValueSource(strings = {"not-a-url", "https:///portal", "https://skills.example.com/?query=value", + "https://skills.example.com/#fragment"}) + void invalidPublicUrlFailsInsteadOfIssuingBrokenCodes(String baseUrl) throws IOException { + run(Map.of("SKILLHUB_PUBLIC_BASE_URL", baseUrl, "DEVICE_AUTH_VERIFICATION_URI", ""), context -> { + assertThat(context).hasFailed(); + assertThat(context.getStartupFailure()).hasRootCauseInstanceOf(IllegalArgumentException.class) + .hasStackTraceContaining("skillhub.public.base-url"); + assertThat(context.getStartupFailure().getMessage()).doesNotContain(baseUrl); + }); + } + + private void assertVerificationUri(Map variables, String expected) throws IOException { + run(variables, context -> { + assertThat(context).hasNotFailed(); + var response = context.getBean(DeviceAuthService.class).generateDeviceCode(); + assertThat(response.verificationUri()).isEqualTo(expected); + assertThat(response.deviceCode()).isNotBlank(); + assertThat(response.userCode()).matches("[A-Z2-9]{4}-[A-Z2-9]{4}"); + assertThat(response.expiresIn()).isEqualTo(900); + assertThat(response.interval()).isEqualTo(5); + }); + } + + @SuppressWarnings("unchecked") + private void run(Map variables, Consumer assertions) + throws IOException { + var sources = new YamlPropertySourceLoader().load("application", new ClassPathResource("application.yml")); + RedisTemplate redis = mock(RedisTemplate.class); + when(redis.opsForValue()).thenReturn(mock(ValueOperations.class)); + new ApplicationContextRunner() + .withInitializer(context -> { + var environment = context.getEnvironment(); + environment.getPropertySources().remove(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME); + environment.getPropertySources().remove(StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME); + environment.getPropertySources().addFirst(new SystemEnvironmentPropertySource("test-env", variables)); + sources.forEach(environment.getPropertySources()::addLast); + }) + .withBean(RedisTemplate.class, () -> redis) + .withBean(ApiTokenService.class, () -> mock(ApiTokenService.class)) + .withBean(ObjectMapper.class, ObjectMapper::new) + .withUserConfiguration(DeviceAuthService.class) + .run(assertions::accept); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java index d926c8620..ed05be880 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java @@ -9,6 +9,7 @@ import org.springframework.data.redis.core.RedisTemplate; import org.springframework.stereotype.Service; import org.springframework.util.StringUtils; +import java.net.URI; import java.security.SecureRandom; import java.util.Base64; import java.util.concurrent.TimeUnit; @@ -41,11 +42,47 @@ public class DeviceAuthService { public DeviceAuthService(RedisTemplate redisTemplate, ApiTokenService apiTokenService, ObjectMapper objectMapper, - @Value("${skillhub.device-auth.verification-uri:/device}") String verificationUri) { + @Value("${skillhub.device-auth.verification-uri:}") String verificationUri, + @Value("${skillhub.public.base-url:}") String publicBaseUrl) { this.redisTemplate = redisTemplate; this.apiTokenService = apiTokenService; this.objectMapper = objectMapper; - this.verificationUri = verificationUri; + this.verificationUri = resolveVerificationUri(verificationUri, publicBaseUrl); + } + + private static String resolveVerificationUri(String configuredUri, String publicBaseUrl) { + if (StringUtils.hasText(configuredUri)) { + validateUri(configuredUri, "skillhub.device-auth.verification-uri", true); + return configuredUri; + } + if (!StringUtils.hasText(publicBaseUrl)) { + // The CLI resolves this existing same-origin default against its registry URL. + return "/device"; + } + URI base = validateUri(publicBaseUrl, "skillhub.public.base-url", false); + if (base.getRawQuery() != null || base.getRawFragment() != null) { + throw new IllegalArgumentException("skillhub.public.base-url must not contain a query or fragment"); + } + return publicBaseUrl.replaceAll("/+$", "") + "/device"; + } + + private static URI validateUri(String value, String property, boolean allowRelative) { + String error = property + " must be an HTTP(S) URL" + (allowRelative ? " or a root-relative path" : ""); + URI uri; + try { + uri = URI.create(value); + } catch (IllegalArgumentException ex) { + // Do not attach the parser exception: it includes the configured value. + throw new IllegalArgumentException(error); + } + boolean absolute = ("http".equalsIgnoreCase(uri.getScheme()) || "https".equalsIgnoreCase(uri.getScheme())) + && StringUtils.hasText(uri.getHost()) && uri.getRawUserInfo() == null && uri.getPort() <= 65535; + boolean relative = allowRelative && !uri.isAbsolute() && uri.getRawAuthority() == null + && value.startsWith("/") && !value.startsWith("//"); + if (!absolute && !relative) { + throw new IllegalArgumentException(error); + } + return uri; } /** diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java index 99563a72a..b2b40e8fa 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java @@ -44,7 +44,7 @@ class DeviceAuthServiceTest { @BeforeEach void setUp() { lenient().when(redisTemplate.opsForValue()).thenReturn(valueOperations); - service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/device"); + service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/device", ""); } /**