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();