fix(auth): handle blank device verification URI configuration (#1034)

* fix(auth): resolve blank device verification URI safely Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(deploy): align device URI preflight validation Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
2026-11-14 09:01:16 +00:00 · 2026-10-29 17:14:41 +08:00 · 2026-10-29 17:14:41 +08:00 · 22d8451ec8
commit 22d8451ec8
parent d09bb7d20c
11 changed files with 257 additions and 11 deletions
--- a/charts/skillhub/README.md
+++ b/charts/skillhub/README.md
@ -48,7 +48,8 @@ helm -n skillhub upgrade -i skillhub ./charts/skillhub \
 ```

 未显式设置 `deviceAuthVerificationUri` 时，Chart 使用
-`<publicBaseUrl>/cli/auth`。所有 values 会先经过 `values.schema.json` 和跨字段校验，
+`<publicBaseUrl>/device`，保留部署子路径。`/cli/auth` 是旧版 loopback 回调入口，
+需要客户端提供回调参数，不能作为设备码授权地址。所有 values 会先经过 `values.schema.json` 和跨字段校验，
 无效的组件、Ingress、HPA 与存储组合会在安装前失败。

 首次安装可通过 `auth.initialSettings.passwordLoginEnabled` 和
--- 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 }}
--- 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 \
--- 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
--- 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` 默认对应
+  `<publicBaseUrl>/device`，也可显式覆盖。
+
+`/cli/auth` 是旧版浏览器 loopback 回调入口，需要客户端附带回调参数；它不是输入设备码的页面。
+不要将设备授权地址设成这个旧入口。此次配置保护不改变旧客户端回调流程。
+
 ## 1 运行模型

 按用途选择运行方式；以下重点介绍源码开发和单机交付，Kubernetes 另见[部署指南](skillhub/guide/kubernetes.md)：
--- 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'
--- 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"
--- 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}
--- 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
@ -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<String, Object> 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<String, Object> variables, Consumer<AssertableApplicationContext> assertions)
+            throws IOException {
+        var sources = new YamlPropertySourceLoader().load("application", new ClassPathResource("application.yml"));
+        RedisTemplate<String, Object> 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);
+    }
+}
--- 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<String, Object> 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;
    }

    /**
--- 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", "");
    }

    /**