From dd82c8a62c3466558cc9dc736e30683f2a1cd927 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:04:52 +0800 Subject: [PATCH] feat(deploy): wire Feishu credentials into the release surfaces .env.release.example advertised OAUTH2_FEISHU_* knobs that no deployment path could actually deliver. compose.release.yml has no env_file, so every variable must be listed explicitly, and the Helm chart and k8s base only mapped the GitHub secret keys. Setting the documented variables therefore did nothing. Adds Feishu to compose.release.yml, the Helm secret template and values, the k8s deployment and its secret example. GitLab had the identical gap, so it is wired at the same time rather than leaving the example file half true. validate-release-config.sh only checked that GitHub's id and secret appear together. A half-configured provider renders a login button whose exchange then fails, so the check now loops over all three providers. Its test gained both-directions cases per provider plus a fully configured pass; reverting the loop to GitHub-only makes them fail. Also adds the provider's only failure log. Nothing downstream records a Feishu userinfo failure -- OAuth2LoginFailureHandler does not log either -- so the previous code was silent on error. Logs the exception class and Feishu's own error code, never the upstream msg, which can quote the access token; a test asserts the code is present and the token is not. Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- charts/skillhub/templates/secret.yaml | 8 ++++ .../skillhub/templates/server-deployment.yaml | 14 +++++++ charts/skillhub/values.yaml | 2 + compose.release.yml | 9 +++++ deploy/k8s/base/backend-deployment.yaml | 14 +++++++ deploy/k8s/base/secret.yaml.example | 4 ++ scripts/tests/validate-release-config-test.sh | 23 +++++++++++ scripts/validate-release-config.sh | 18 +++++---- .../auth/oauth/FeishuOAuth2UserService.java | 14 ++++++- .../oauth/FeishuOAuth2UserServiceTest.java | 40 +++++++++++++++++++ 10 files changed, 137 insertions(+), 9 deletions(-) diff --git a/charts/skillhub/templates/secret.yaml b/charts/skillhub/templates/secret.yaml index 8e28c911..d00c41a4 100644 --- a/charts/skillhub/templates/secret.yaml +++ b/charts/skillhub/templates/secret.yaml @@ -59,6 +59,14 @@ stringData: oauth2-github-client-secret: {{ .Values.secrets.oauth2GithubClientSecret | quote }} {{- end }} + # OAuth2 Feishu (optional) + {{- if .Values.secrets.oauth2FeishuClientId }} + oauth2-feishu-client-id: {{ .Values.secrets.oauth2FeishuClientId | quote }} + {{- end }} + {{- if .Values.secrets.oauth2FeishuClientSecret }} + oauth2-feishu-client-secret: {{ .Values.secrets.oauth2FeishuClientSecret | quote }} + {{- end }} + # Scanner LLM 配置 (optional) {{- if .Values.secrets.scannerLlmApiKey }} skill-scanner-llm-api-key: {{ .Values.secrets.scannerLlmApiKey | quote }} diff --git a/charts/skillhub/templates/server-deployment.yaml b/charts/skillhub/templates/server-deployment.yaml index a6e4e788..dc741030 100644 --- a/charts/skillhub/templates/server-deployment.yaml +++ b/charts/skillhub/templates/server-deployment.yaml @@ -355,6 +355,20 @@ spec: key: oauth2-github-client-secret optional: true + # OAuth2 Feishu (optional) + - name: OAUTH2_FEISHU_CLIENT_ID + valueFrom: + secretKeyRef: + name: {{ include "skillhub.secretName" . }} + key: oauth2-feishu-client-id + optional: true + - name: OAUTH2_FEISHU_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: {{ include "skillhub.secretName" . }} + key: oauth2-feishu-client-secret + optional: true + {{- if .Values.server.javaOpts }} - name: JAVA_OPTS value: {{ .Values.server.javaOpts }} diff --git a/charts/skillhub/values.yaml b/charts/skillhub/values.yaml index 092e73e4..fe289836 100644 --- a/charts/skillhub/values.yaml +++ b/charts/skillhub/values.yaml @@ -93,6 +93,8 @@ secrets: downloadAnonCookieSecret: "" oauth2GithubClientId: "" oauth2GithubClientSecret: "" + oauth2FeishuClientId: "" + oauth2FeishuClientSecret: "" scannerLlmApiKey: "" scannerLlmBaseUrl: "" scannerLlmModel: "" diff --git a/compose.release.yml b/compose.release.yml index c707db08..6d772039 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -115,6 +115,15 @@ 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_GITLAB_CLIENT_ID: ${OAUTH2_GITLAB_CLIENT_ID:-local-placeholder} + OAUTH2_GITLAB_CLIENT_SECRET: ${OAUTH2_GITLAB_CLIENT_SECRET:-local-placeholder} + OAUTH2_GITLAB_BASE_URI: ${OAUTH2_GITLAB_BASE_URI:-https://gitlab.com} + OAUTH2_GITLAB_DISPLAY_NAME: ${OAUTH2_GITLAB_DISPLAY_NAME:-GitLab} + OAUTH2_FEISHU_CLIENT_ID: ${OAUTH2_FEISHU_CLIENT_ID:-local-placeholder} + OAUTH2_FEISHU_CLIENT_SECRET: ${OAUTH2_FEISHU_CLIENT_SECRET:-local-placeholder} + OAUTH2_FEISHU_AUTHORIZE_URI: ${OAUTH2_FEISHU_AUTHORIZE_URI:-https://accounts.feishu.cn} + OAUTH2_FEISHU_BASE_URI: ${OAUTH2_FEISHU_BASE_URI:-https://open.feishu.cn} + OAUTH2_FEISHU_DISPLAY_NAME: ${OAUTH2_FEISHU_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/base/backend-deployment.yaml b/deploy/k8s/base/backend-deployment.yaml index 816ffed3..5f11534f 100644 --- a/deploy/k8s/base/backend-deployment.yaml +++ b/deploy/k8s/base/backend-deployment.yaml @@ -227,6 +227,20 @@ spec: key: oauth2-github-client-secret optional: true + # OAuth2 Feishu (optional) + - name: OAUTH2_FEISHU_CLIENT_ID + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-feishu-client-id + optional: true + - name: OAUTH2_FEISHU_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-feishu-client-secret + optional: true + volumeMounts: - name: skillhub-storage mountPath: /var/lib/skillhub/storage diff --git a/deploy/k8s/base/secret.yaml.example b/deploy/k8s/base/secret.yaml.example index c2b93d45..f9c119d6 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: "" + # 飞书 OAuth（可选，用于飞书登录；留空则登录页不展示该入口） + oauth2-feishu-client-id: "" + oauth2-feishu-client-secret: "" + # LLM 配置（可选，用于技能扫描） skill-scanner-llm-api-key: "" skill-scanner-llm-base-url: "" diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index 1ec54043..d26b0145 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -253,6 +253,29 @@ 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" +# An OAuth client id without its secret (or vice versa) leaves the provider half-configured: +# the login button renders but the exchange fails. Checked for every supported provider. +for provider in GITHUB GITLAB FEISHU; do + missing_oauth_secret_env="$tmp/missing-oauth-secret.env" + write_env "$missing_oauth_secret_env" "release-download-secret-32-bytes-minimum" + printf 'OAUTH2_%s_CLIENT_ID=real-client-id\n' "$provider" >>"$missing_oauth_secret_env" + expect_fail "$missing_oauth_secret_env" "OAUTH2_${provider}_CLIENT_SECRET is required" + + missing_oauth_id_env="$tmp/missing-oauth-id.env" + write_env "$missing_oauth_id_env" "release-download-secret-32-bytes-minimum" + printf 'OAUTH2_%s_CLIENT_SECRET=real-client-secret\n' "$provider" >>"$missing_oauth_id_env" + expect_fail "$missing_oauth_id_env" "OAUTH2_${provider}_CLIENT_ID is required" +done + +# A fully configured provider pair must pass. +valid_oauth_env="$tmp/valid-oauth.env" +write_env "$valid_oauth_env" "release-download-secret-32-bytes-minimum" +cat >>"$valid_oauth_env" <<'EOF' +OAUTH2_FEISHU_CLIENT_ID=cli_release_example +OAUTH2_FEISHU_CLIENT_SECRET=release-feishu-secret +EOF +"$SCRIPT" "$valid_oauth_env" >/dev/null + draft_env="$tmp/draft.env" while IFS= read -r line || [[ -n "$line" ]]; do case "$line" in diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index eaaca6b1..9a64729f 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -380,14 +380,16 @@ if [ "${REDIS_BIND_ADDRESS:-127.0.0.1}" != "127.0.0.1" ]; then warn "REDIS_BIND_ADDRESS is not 127.0.0.1; confirm Redis exposure is intended" fi -oauth_id="${OAUTH2_GITHUB_CLIENT_ID:-}" -oauth_secret="${OAUTH2_GITHUB_CLIENT_SECRET:-}" -if [ -n "$oauth_id" ] && [ -z "$oauth_secret" ]; then - error "OAUTH2_GITHUB_CLIENT_SECRET is required when OAUTH2_GITHUB_CLIENT_ID is set" -fi -if [ -n "$oauth_secret" ] && [ -z "$oauth_id" ]; then - error "OAUTH2_GITHUB_CLIENT_ID is required when OAUTH2_GITHUB_CLIENT_SECRET is set" -fi +for provider in GITHUB GITLAB FEISHU; do + eval "oauth_id=\"\${OAUTH2_${provider}_CLIENT_ID:-}\"" + eval "oauth_secret=\"\${OAUTH2_${provider}_CLIENT_SECRET:-}\"" + if [ -n "$oauth_id" ] && [ -z "$oauth_secret" ]; then + error "OAUTH2_${provider}_CLIENT_SECRET is required when OAUTH2_${provider}_CLIENT_ID is set" + fi + if [ -n "$oauth_secret" ] && [ -z "$oauth_id" ]; then + error "OAUTH2_${provider}_CLIENT_ID is required when OAUTH2_${provider}_CLIENT_SECRET is set" + fi +done if [ "$errors" -gt 0 ]; then echo "Release config validation failed: $errors error(s), $warnings warning(s)." >&2 diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserService.java index c5e4bcb6..10d4e44a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserService.java @@ -9,6 +9,8 @@ import java.time.Duration; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; @@ -30,6 +32,8 @@ import org.springframework.web.client.RestClient; @Component public class FeishuOAuth2UserService implements ProviderOAuth2UserService { + private static final Logger log = LoggerFactory.getLogger(FeishuOAuth2UserService.class); + static final String PROVIDER = "feishu"; private final RestClient restClient; @@ -98,8 +102,11 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { .header(HttpHeaders.AUTHORIZATION, "Bearer " + userRequest.getAccessToken().getTokenValue()) .exchange((request, clientResponse) -> readBounded(clientResponse.getBody())); } catch (Exception e) { + // Exception class only: the message can quote the request URI, which holds the token. + // Nothing downstream logs this failure, so without this line it would be silent. + log.warn("Feishu user info request failed with {}", e.getClass().getSimpleName()); // The cause carries the detail for operators; the OAuth2Error description stays generic - // because an upstream message can quote the request URI, which holds the access token. + // for the same reason the log line is. throw new OAuth2AuthenticationException( new OAuth2Error("feishu_userinfo_error", "Failed to load Feishu user info", null), e @@ -107,6 +114,11 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { } if (response == null || response.code() != 0 || response.data() == null) { + // Feishu's own error code is safe to record; its msg text is not. + log.warn( + "Feishu user info returned error code {}", + response == null ? "none" : response.code() + ); throw new OAuth2AuthenticationException( new OAuth2Error( "feishu_userinfo_error", diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserServiceTest.java index 39367a3f..e5d62e6d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2UserServiceTest.java @@ -6,8 +6,12 @@ import static org.springframework.test.web.client.match.MockRestRequestMatchers. import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import java.time.Instant; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.security.oauth2.client.registration.ClientRegistration; @@ -101,6 +105,42 @@ class FeishuOAuth2UserServiceTest { server.verify(); } + @Test + void loadUser_logsErrorCodeButNeverUpstreamTextOrToken() { + RestClient.Builder restClientBuilder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(restClientBuilder).build(); + server.expect(requestTo("https://open.feishu.cn/open-apis/authen/v1/user_info")) + .andRespond(withSuccess( + """ + {"code": 99991663, "msg": "token token-123 rejected for cli_test123"} + """, + MediaType.APPLICATION_JSON + )); + FeishuOAuth2UserService service = new FeishuOAuth2UserService(restClientBuilder); + + ListAppender appender = new ListAppender<>(); + Logger logger = (Logger) LoggerFactory.getLogger(FeishuOAuth2UserService.class); + appender.start(); + logger.addAppender(appender); + try { + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class); + } finally { + logger.detachAppender(appender); + appender.stop(); + } + + String logged = appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .collect(java.util.stream.Collectors.joining("\n")); + // A failure must leave an operator-facing record... + assertThat(logged).contains("99991663"); + // ...but the upstream msg can quote the access token, so it must never be logged. + assertThat(logged).doesNotContain("token-123"); + assertThat(logged).doesNotContain("rejected"); + server.verify(); + } + @Test void loadUser_errorDescriptionDoesNotEchoUpstreamTextOrToken() { RestClient.Builder restClientBuilder = RestClient.builder();