mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-24 00:55:35 +00:00
fix(auth): diagnose DingTalk userinfo failures
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
ca4de37d08
commit
36f5f06d9c
5 changed files with 147 additions and 4 deletions
|
|
@ -58,7 +58,7 @@ spring:
|
|||
scope:
|
||||
- read:user
|
||||
- user:email
|
||||
redirect-uri: "${OAUTH2_DINGTALK_REDIRECT_URI:{baseUrl}/login/oauth2/code/{registrationId}}"
|
||||
redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
|
||||
client-name: GitHub
|
||||
authorization-grant-type: authorization_code
|
||||
gitlab:
|
||||
|
|
@ -93,7 +93,7 @@ spring:
|
|||
# description; "none" would additionally make Spring apply PKCE, and the DingTalk token
|
||||
# request sends no code_verifier to match the challenge.
|
||||
client-authentication-method: client_secret_post
|
||||
redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
|
||||
redirect-uri: "${OAUTH2_DINGTALK_REDIRECT_URI:{baseUrl}/login/oauth2/code/{registrationId}}"
|
||||
client-name: ${OAUTH2_DINGTALK_DISPLAY_NAME:钉钉}
|
||||
provider:
|
||||
github:
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthoriza
|
|||
String authorizationRequestUri = UriComponentsBuilder
|
||||
.fromUriString(builder.build().getAuthorizationRequestUri())
|
||||
.replaceQueryParam("scope", DingTalkOAuth2Constants.AUTHORIZATION_SCOPE)
|
||||
.replaceQueryParam("prompt", "consent")
|
||||
.build(true)
|
||||
.toUriString();
|
||||
builder.authorizationRequestUri(authorizationRequestUri);
|
||||
|
|
|
|||
|
|
@ -1,12 +1,17 @@
|
|||
package com.iflytek.skillhub.auth.oauth;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.databind.JsonNode;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.time.Duration;
|
||||
import java.util.Collections;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
|
|
@ -92,9 +97,13 @@ public class DingTalkOAuth2UserService implements ProviderOAuth2UserService {
|
|||
)
|
||||
.exchange((request, clientResponse) -> {
|
||||
if (!clientResponse.getStatusCode().is2xxSuccessful()) {
|
||||
SafeErrorSummary summary = readSafeErrorSummary(clientResponse.getBody());
|
||||
log.warn(
|
||||
"DingTalk user info returned HTTP {}; response body omitted",
|
||||
clientResponse.getStatusCode().value());
|
||||
"DingTalk user info returned HTTP {}; code={}, requiredScopes={}, requestId={}",
|
||||
clientResponse.getStatusCode().value(),
|
||||
summary.code(),
|
||||
summary.requiredScopes(),
|
||||
summary.requestId());
|
||||
throw new IOException(
|
||||
"DingTalk user info returned HTTP " + clientResponse.getStatusCode().value());
|
||||
}
|
||||
|
|
@ -130,6 +139,88 @@ public class DingTalkOAuth2UserService implements ProviderOAuth2UserService {
|
|||
});
|
||||
}
|
||||
|
||||
/** Extracts provider diagnostics without logging tokens, messages, or the upstream body. */
|
||||
private static SafeErrorSummary readSafeErrorSummary(InputStream body) {
|
||||
try {
|
||||
byte[] bytes = body.readNBytes(MAX_RESPONSE_BYTES + 1);
|
||||
if (bytes.length > MAX_RESPONSE_BYTES) {
|
||||
return SafeErrorSummary.UNKNOWN;
|
||||
}
|
||||
JsonNode root = OBJECT_MAPPER.readTree(bytes);
|
||||
if (root == null) {
|
||||
return SafeErrorSummary.UNKNOWN;
|
||||
}
|
||||
String code = text(findNode(root, Set.of("code")));
|
||||
String requestId = text(findNode(root, Set.of("requestid")));
|
||||
JsonNode data = findNode(root, Set.of("data"));
|
||||
if (data != null && data.isTextual()) {
|
||||
try {
|
||||
JsonNode nested = OBJECT_MAPPER.readTree(data.asText());
|
||||
if (nested != null) {
|
||||
root = nested;
|
||||
}
|
||||
} catch (Exception ignored) {
|
||||
// Keep the outer diagnostic fields when Data is not JSON.
|
||||
}
|
||||
}
|
||||
code = valueOrUnknown(code);
|
||||
requestId = valueOrUnknown(requestId != null ? requestId : text(findNode(root, Set.of("requestid"))));
|
||||
JsonNode scopes = findNode(root, Set.of("requiredscopes"));
|
||||
String requiredScopes = scopes != null && scopes.isArray()
|
||||
? String.join(",", textValues(scopes))
|
||||
: "-";
|
||||
return new SafeErrorSummary(code, requiredScopes, requestId);
|
||||
} catch (Exception ignored) {
|
||||
return SafeErrorSummary.UNKNOWN;
|
||||
}
|
||||
}
|
||||
|
||||
private static JsonNode findNode(JsonNode node, Set<String> names) {
|
||||
if (node.isObject()) {
|
||||
Iterator<Map.Entry<String, JsonNode>> fields = node.fields();
|
||||
while (fields.hasNext()) {
|
||||
Map.Entry<String, JsonNode> field = fields.next();
|
||||
if (names.contains(field.getKey().toLowerCase())) {
|
||||
return field.getValue();
|
||||
}
|
||||
JsonNode nested = findNode(field.getValue(), names);
|
||||
if (nested != null) {
|
||||
return nested;
|
||||
}
|
||||
}
|
||||
} else if (node.isArray()) {
|
||||
for (JsonNode child : node) {
|
||||
JsonNode nested = findNode(child, names);
|
||||
if (nested != null) {
|
||||
return nested;
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private static List<String> textValues(JsonNode array) {
|
||||
List<String> values = new ArrayList<>();
|
||||
array.forEach(value -> {
|
||||
if (value.isTextual() && !value.asText().isBlank()) {
|
||||
values.add(value.asText());
|
||||
}
|
||||
});
|
||||
return values;
|
||||
}
|
||||
|
||||
private static String text(JsonNode node) {
|
||||
return node != null && node.isValueNode() ? node.asText() : null;
|
||||
}
|
||||
|
||||
private static String valueOrUnknown(String value) {
|
||||
return value == null || value.isBlank() ? "-" : value;
|
||||
}
|
||||
|
||||
private record SafeErrorSummary(String code, String requiredScopes, String requestId) {
|
||||
private static final SafeErrorSummary UNKNOWN = new SafeErrorSummary("-", "-", "-");
|
||||
}
|
||||
|
||||
/**
|
||||
* Copies through only the attributes the platform consumes, and aliases DingTalk's
|
||||
* {@code avatarUrl} to the {@code avatar_url} key the identity core reads. Attributes the
|
||||
|
|
|
|||
|
|
@ -7,7 +7,12 @@ import static org.springframework.test.web.client.match.MockRestRequestMatchers.
|
|||
import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
|
||||
import static org.springframework.test.web.client.response.MockRestResponseCreators.withStatus;
|
||||
|
||||
import ch.qos.logback.classic.Level;
|
||||
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.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.HttpStatus;
|
||||
|
|
@ -20,9 +25,21 @@ import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
|
|||
import org.springframework.security.oauth2.core.user.OAuth2User;
|
||||
import org.springframework.test.web.client.MockRestServiceServer;
|
||||
import org.springframework.web.client.RestClient;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
class DingTalkOAuth2UserServiceTest {
|
||||
|
||||
private final Logger logger = (Logger) LoggerFactory.getLogger(DingTalkOAuth2UserService.class);
|
||||
private ListAppender<ILoggingEvent> appender;
|
||||
|
||||
@AfterEach
|
||||
void tearDown() {
|
||||
if (appender != null) {
|
||||
logger.detachAppender(appender);
|
||||
appender.stop();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void loadUser_sendsCustomTokenHeaderAndNormalizesAttributes() {
|
||||
RestClient.Builder builder = RestClient.builder();
|
||||
|
|
@ -121,6 +138,39 @@ class DingTalkOAuth2UserServiceTest {
|
|||
server.verify();
|
||||
}
|
||||
|
||||
@Test
|
||||
void loadUser_logsSafeProviderDiagnosticsWithoutUpstreamMessageOrToken() {
|
||||
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("{\"Code\":\"Forbidden.AccessDenied.AccessTokenPermissionDenied\","
|
||||
+ "\"Data\":\"{\\\"AccessDeniedDetail\\\":{\\\"requiredScopes\\\":[\\\"Contact.User.Read\\\"]},"
|
||||
+ "\\\"RequestId\\\":\\\"req-123\\\"}\","
|
||||
+ "\"Message\":\"secret upstream message token-123\"}")
|
||||
.contentType(MediaType.APPLICATION_JSON));
|
||||
attachAppender();
|
||||
DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder);
|
||||
|
||||
assertThatThrownBy(() -> service.loadUser(userRequest()))
|
||||
.isInstanceOf(OAuth2AuthenticationException.class);
|
||||
|
||||
assertThat(appender.list).extracting(ILoggingEvent::getFormattedMessage)
|
||||
.anySatisfy(message -> assertThat(message)
|
||||
.contains("code=Forbidden.AccessDenied.AccessTokenPermissionDenied")
|
||||
.contains("requiredScopes=Contact.User.Read")
|
||||
.contains("requestId=req-123")
|
||||
.doesNotContain("secret upstream message", "token-123"));
|
||||
server.verify();
|
||||
}
|
||||
|
||||
private void attachAppender() {
|
||||
logger.setLevel(Level.INFO);
|
||||
appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
}
|
||||
|
||||
@Test
|
||||
void loadUser_errorDescriptionDoesNotEchoUpstreamTextOrToken() {
|
||||
RestClient.Builder builder = RestClient.builder();
|
||||
|
|
|
|||
|
|
@ -99,6 +99,7 @@ class OAuth2AuthorizationRequestResolverTest {
|
|||
assertThat(authorizationRequest).isNotNull();
|
||||
// DingTalk's authorize endpoint requires scope=openid on the wire.
|
||||
assertThat(authorizationRequest.getAuthorizationRequestUri()).contains("scope=openid");
|
||||
assertThat(authorizationRequest.getAuthorizationRequestUri()).contains("prompt=consent");
|
||||
|
||||
// But getScopes() must stay empty. OAuth2LoginAuthenticationProvider.authenticate returns
|
||||
// null when the authorization request's scopes contain "openid", which hands the callback to
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue