From 629c1ced55b2d6b7624ac45f250904acecffb1cd Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:21:10 +0800 Subject: [PATCH] fix(auth): bound the DingTalk token response and log a rejected login Two gaps from reviewing this batch against the Feishu adapter it mirrors. The token exchange had no response size limit while the userinfo call did, so the same hostile or misconfigured endpoint was bounded on one call and unbounded on the other. Adds the same 64 KB cap through a RestTemplate interceptor, which keeps the existing tests working against an injected template. buildRestTemplate becomes package-visible so one test can exercise the production template, cap included; removing the interceptor makes that test fail. A missing unionId threw without logging, unlike the equivalent Feishu branch. This is a reachable failure -- DingTalk omits unionId for some app configurations -- and an operator seeing every login rejected needs to know why. Logs the claim name only, which says nothing about the user. Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../auth/oauth/DingTalkOAuth2UserService.java | 7 +++ .../oauth/DingTalkTokenResponseClient.java | 55 ++++++++++++++++++- .../DingTalkTokenResponseClientTest.java | 22 ++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) 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 7b8331a2..66c29036 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 @@ -138,6 +138,13 @@ public class DingTalkOAuth2UserService implements ProviderOAuth2UserService { attributes.put("avatar_url", avatar); } if (!attributes.containsKey(DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME)) { + // A reachable failure: DingTalk omits unionId for some app configurations, and the + // operator needs to see why every login is being rejected. The claim name is a + // constant, so this records nothing about the user. + log.warn( + "DingTalk user info response omitted {}; login rejected", + DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME + ); throw new OAuth2AuthenticationException( new OAuth2Error( "dingtalk_userinfo_error", 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 bfb79978..36fac975 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 @@ -2,9 +2,14 @@ package com.iflytek.skillhub.auth.oauth; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.time.Duration; import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; @@ -51,11 +56,57 @@ public class DingTalkTokenResponseClient implements ProviderTokenResponseClient return DingTalkOAuth2Constants.REGISTRATION_ID; } - private static RestTemplate buildRestTemplate() { + /** A DingTalk token payload is a few hundred bytes; this only stops an unbounded body. */ + private static final int MAX_RESPONSE_BYTES = 64 * 1024; + + /** Package-visible so a test can exercise the production template, size cap included. */ + static RestTemplate buildRestTemplate() { var factory = new SimpleClientHttpRequestFactory(); factory.setConnectTimeout(Duration.ofSeconds(5)); factory.setReadTimeout(Duration.ofSeconds(10)); - return new RestTemplate(factory); + RestTemplate template = new RestTemplate(factory); + // The timeouts bound how long the exchange may take; this bounds how much it may return, so + // a misconfigured or hostile token endpoint cannot stream an unbounded body into the parser. + // The userinfo client applies the same cap. + template.getInterceptors().add((request, body, execution) -> { + ClientHttpResponse response = execution.execute(request, body); + byte[] bytes = response.getBody().readNBytes(MAX_RESPONSE_BYTES + 1); + if (bytes.length > MAX_RESPONSE_BYTES) { + throw new IOException("DingTalk token response exceeds " + MAX_RESPONSE_BYTES + " bytes"); + } + return new BoundedClientHttpResponse(response, bytes); + }); + return template; + } + + /** Replays the already-read, size-checked body so the converters can still parse it. */ + private record BoundedClientHttpResponse(ClientHttpResponse delegate, byte[] body) + implements ClientHttpResponse { + + @Override + public HttpStatusCode getStatusCode() throws IOException { + return delegate.getStatusCode(); + } + + @Override + public String getStatusText() throws IOException { + return delegate.getStatusText(); + } + + @Override + public void close() { + delegate.close(); + } + + @Override + public InputStream getBody() { + return new ByteArrayInputStream(body); + } + + @Override + public HttpHeaders getHeaders() { + return delegate.getHeaders(); + } } @Override diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java index ac15a660..fa127ec2 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java @@ -198,4 +198,26 @@ class DingTalkTokenResponseClientTest { new OAuth2AuthorizationExchange(authRequest, authResponse) ); } + + @Test + void getTokenResponse_rejectsOversizedResponseBody() { + // Uses the production template so the size-cap interceptor is in play; the tests above + // inject a bare RestTemplate and therefore cannot reach it. + RestTemplate productionTemplate = DingTalkTokenResponseClient.buildRestTemplate(); + MockRestServiceServer server = MockRestServiceServer.createServer(productionTemplate); + // 64 KB cap; pad a structurally valid token payload past it so the size check fires. + String padding = "x".repeat(70 * 1024); + server.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + "{\"accessToken\":\"" + padding + "\",\"expireIn\":7200}", + MediaType.APPLICATION_JSON + )); + DingTalkTokenResponseClient boundedClient = new DingTalkTokenResponseClient(productionTemplate); + + assertThatThrownBy(() -> boundedClient.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()) + .isEqualTo("token_exchange_io_error")); + server.verify(); + } }