diff --git a/charts/skillhub/tests/configuration-contracts.sh b/charts/skillhub/tests/configuration-contracts.sh index a2c628ab..d255db0a 100755 --- a/charts/skillhub/tests/configuration-contracts.sh +++ b/charts/skillhub/tests/configuration-contracts.sh @@ -74,6 +74,17 @@ render stable "$CHART_DIR" "${stable_args[@]}" >"$TMP_DIR/stable-a.yaml" render stable "$CHART_DIR" "${stable_args[@]}" >"$TMP_DIR/stable-b.yaml" cmp "$TMP_DIR/stable-a.yaml" "$TMP_DIR/stable-b.yaml" +render dingtalk "$CHART_DIR" "${stable_args[@]}" \ + --set-string secrets.oauth2DingtalkClientId=ding-test \ + --set-string secrets.oauth2DingtalkClientSecret=dingtalk-test-secret \ + >"$TMP_DIR/dingtalk.yaml" +grep -Fq 'oauth2-dingtalk-client-id: "ding-test"' "$TMP_DIR/dingtalk.yaml" \ + || fail "Helm must render the configured DingTalk client id" +grep -Fq 'oauth2-dingtalk-client-secret: "dingtalk-test-secret"' "$TMP_DIR/dingtalk.yaml" \ + || fail "Helm must render the configured DingTalk client secret" +grep -Fq 'name: OAUTH2_DINGTALK_CLIENT_ID' "$TMP_DIR/dingtalk.yaml" \ + || fail "server deployment must inject the DingTalk client id" + render private-registry "$CHART_DIR" \ --set server.dependencyWait.image.registry=registry.example.com \ --set server.dependencyWait.image.repository=library/busybox \ diff --git a/charts/skillhub/values.schema.json b/charts/skillhub/values.schema.json index e7bb6b47..6d3bf510 100644 --- a/charts/skillhub/values.schema.json +++ b/charts/skillhub/values.schema.json @@ -177,6 +177,8 @@ "oauth2GithubClientSecret": { "type": "string" }, "oauth2FeishuClientId": { "type": "string" }, "oauth2FeishuClientSecret": { "type": "string" }, + "oauth2DingtalkClientId": { "type": "string" }, + "oauth2DingtalkClientSecret": { "type": "string" }, "scannerLlmApiKey": { "type": "string" }, "scannerLlmBaseUrl": { "type": "string" }, "scannerLlmModel": { "type": "string" } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java index c87a9ecc..4686250f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java @@ -61,7 +61,7 @@ class ProviderStrategyWiringTest { // standard OAuth2 behaviour its endpoints reject. assertThat(tokenResponseClients) .extracting(ProviderTokenResponseClient::getProvider) - .contains(DingTalkOAuth2Constants.REGISTRATION_ID); + .contains(DingTalkOAuth2Constants.REGISTRATION_ID, "feishu"); assertThat(authorizationCustomizers) .extracting(ProviderAuthorizationRequestCustomizer::getProvider) .contains(DingTalkOAuth2Constants.REGISTRATION_ID); 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 66c29036..cb031985 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 @@ -90,7 +90,16 @@ public class DingTalkOAuth2UserService implements ProviderOAuth2UserService { DingTalkOAuth2Constants.ACCESS_TOKEN_HEADER, userRequest.getAccessToken().getTokenValue() ) - .exchange((request, clientResponse) -> readBounded(clientResponse.getBody())); + .exchange((request, clientResponse) -> { + if (!clientResponse.getStatusCode().is2xxSuccessful()) { + log.warn( + "DingTalk user info returned HTTP {}; response body omitted", + clientResponse.getStatusCode().value()); + throw new IOException( + "DingTalk user info returned HTTP " + clientResponse.getStatusCode().value()); + } + return readBounded(clientResponse.getBody()); + }); } catch (Exception e) { // Exception class only: the message can quote the request URI, which holds the token. log.warn("DingTalk user info request failed with {}", e.getClass().getSimpleName()); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java index 36fac975..cea2f825 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java @@ -72,6 +72,7 @@ public class DingTalkTokenResponseClient implements ProviderTokenResponseClient ClientHttpResponse response = execution.execute(request, body); byte[] bytes = response.getBody().readNBytes(MAX_RESPONSE_BYTES + 1); if (bytes.length > MAX_RESPONSE_BYTES) { + response.close(); throw new IOException("DingTalk token response exceeds " + MAX_RESPONSE_BYTES + " bytes"); } return new BoundedClientHttpResponse(response, bytes); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java index 401fa98e..5a03639a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuOAuth2AccessTokenResponseClient.java @@ -33,7 +33,7 @@ import org.springframework.web.client.RestClient; */ @Component public class FeishuOAuth2AccessTokenResponseClient - implements OAuth2AccessTokenResponseClient { + implements ProviderTokenResponseClient { private static final Logger log = LoggerFactory.getLogger(FeishuOAuth2AccessTokenResponseClient.class); private static final String FEISHU_PROVIDER = "feishu"; @@ -78,6 +78,11 @@ public class FeishuOAuth2AccessTokenResponseClient this.protocolVersion = normalizeProtocolVersion(protocolVersion); } + @Override + public String getProvider() { + return FEISHU_PROVIDER; + } + @Override public OAuth2AccessTokenResponse getTokenResponse( OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest) { 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 50c0ef3e..800fe73f 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 @@ -5,10 +5,12 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; 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.withStatus; import java.time.Instant; import org.junit.jupiter.api.Test; import org.springframework.http.MediaType; +import org.springframework.http.HttpStatus; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; import org.springframework.security.oauth2.core.AuthorizationGrantType; @@ -99,6 +101,26 @@ class DingTalkOAuth2UserServiceTest { server.verify(); } + @Test + void loadUser_rejectsNonSuccessfulHttpStatusWithoutExposingBody() { + RestClient.Builder builder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(builder).build(); + server.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withStatus(HttpStatus.FORBIDDEN) + .body("access denied for token-123") + .contentType(MediaType.APPLICATION_JSON)); + DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> { + var error = ((OAuth2AuthenticationException) ex).getError(); + assertThat(error.getErrorCode()).isEqualTo("dingtalk_userinfo_error"); + assertThat(error.getDescription()).doesNotContain("token-123", "access denied"); + }); + server.verify(); + } + @Test void loadUser_errorDescriptionDoesNotEchoUpstreamTextOrToken() { RestClient.Builder builder = RestClient.builder();