mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-24 00:55:35 +00:00
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>
This commit is contained in:
parent
75c7f9a880
commit
629c1ced55
3 changed files with 82 additions and 2 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue