diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractor.java index a2ad65e9..ba7fded0 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractor.java @@ -41,13 +41,13 @@ public class FeishuClaimsExtractor implements OAuthClaimsExtractor { // in real time, so they carry no verification signal; keep emailVerified false. boolean emailVerified = false; + // name -> en_name and stop, matching the GitHub and GitLab extractors. Falling back to the + // subject would write it into UserAccount.displayName and into UserActivatedEvent, pushing + // the external subject somewhere event consumers may log it. String username = (String) attrs.get("name"); if (username == null || username.isBlank()) { username = (String) attrs.get("en_name"); } - if (username == null || username.isBlank()) { - username = "feishu-" + subject; - } return new OAuthClaims( FeishuOAuth2UserService.PROVIDER, 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 d866ac41..c5e4bcb6 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 @@ -2,12 +2,14 @@ package com.iflytek.skillhub.auth.oauth; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.io.InputStream; import java.time.Duration; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpRequestFactory; @@ -35,6 +37,11 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(5); private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); + /** A Feishu user_info payload is well under 1 KB; this only needs to stop an unbounded body. */ + private static final int MAX_RESPONSE_BYTES = 64 * 1024; + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + /** * Uses an external-service client that is intentionally not customized with application * tracing. Trace context must not be propagated to the external Feishu service. @@ -61,6 +68,19 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { return factory; } + /** + * Reads at most {@link #MAX_RESPONSE_BYTES} before parsing, so a misconfigured or hostile + * {@code OAUTH2_FEISHU_BASE_URI} cannot stream an unbounded body into the parser. Reading one + * byte past the cap is what distinguishes an oversized payload from one that exactly fills it. + */ + private static FeishuUserResponse readBounded(InputStream body) throws IOException { + byte[] bytes = body.readNBytes(MAX_RESPONSE_BYTES + 1); + if (bytes.length > MAX_RESPONSE_BYTES) { + throw new IOException("Feishu user info response exceeds " + MAX_RESPONSE_BYTES + " bytes"); + } + return OBJECT_MAPPER.readValue(bytes, FeishuUserResponse.class); + } + @Override public String getProvider() { return PROVIDER; @@ -76,8 +96,7 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { response = restClient.get() .uri(userInfoUri) .header(HttpHeaders.AUTHORIZATION, "Bearer " + userRequest.getAccessToken().getTokenValue()) - .retrieve() - .body(new ParameterizedTypeReference() {}); + .exchange((request, clientResponse) -> readBounded(clientResponse.getBody())); } catch (Exception e) { // 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. @@ -117,7 +136,6 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { putIfPresent(attributes, "avatar_url", data.avatarUrl()); putIfPresent(attributes, "email", data.email()); putIfPresent(attributes, "enterprise_email", data.enterpriseEmail()); - putIfPresent(attributes, "mobile", data.mobile()); if (!attributes.containsKey(userNameAttributeName)) { throw new OAuth2AuthenticationException( new OAuth2Error("feishu_userinfo_error", "Feishu user info missing " + userNameAttributeName, null) @@ -143,7 +161,6 @@ public class FeishuOAuth2UserService implements ProviderOAuth2UserService { @JsonProperty("en_name") String enName, @JsonProperty("avatar_url") String avatarUrl, @JsonProperty("email") String email, - @JsonProperty("enterprise_email") String enterpriseEmail, - @JsonProperty("mobile") String mobile + @JsonProperty("enterprise_email") String enterpriseEmail ) {} } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java index 3b4d265e..25e74cc9 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java @@ -79,23 +79,6 @@ public class OAuthLoginFlowService { this.remoteIdentityIo = remoteIdentityIo; } - OAuthLoginFlowService(List extractorList, - AccessPolicy accessPolicy, - IdentityBindingService identityBindingService, - LegacyPlatformIdentityCore identityCore, - OAuth2UserService delegate, - RemoteIdentityIoExecutor remoteIdentityIo) { - this( - extractorList, - List.of(), - accessPolicy, - identityBindingService, - identityCore, - delegate, - remoteIdentityIo - ); - } - OAuthLoginFlowService(List extractorList, AccessPolicy accessPolicy, IdentityBindingService identityBindingService, diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractorTest.java index 9b70fd72..d7715045 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractorTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/FeishuClaimsExtractorTest.java @@ -40,7 +40,7 @@ class FeishuClaimsExtractorTest { } @Test - void extract_allowsNullEmailAndFallsBackUsername() { + void extract_allowsNullEmailAndLeavesDisplayNameUnsetWhenFeishuSendsNoName() { Map attrs = new HashMap<>(Map.of("open_id", "ou_456")); OAuthClaims claims = extractor.extract(userRequest(), user(attrs)); @@ -48,7 +48,9 @@ class FeishuClaimsExtractorTest { assertThat(claims.subject()).isEqualTo("ou_456"); assertThat(claims.email()).isNull(); assertThat(claims.emailVerified()).isFalse(); - assertThat(claims.providerLogin()).isEqualTo("feishu-ou_456"); + // Must not synthesize "feishu-": providerLogin is written to displayName and into + // UserActivatedEvent, so a synthesized value would carry the subject into event consumers. + assertThat(claims.providerLogin()).isNull(); } @Test @@ -77,16 +79,6 @@ class FeishuClaimsExtractorTest { .hasMessageContaining("open_id"); } - @Test - void extract_rejectsMissingOpenIdWithoutFabricatingASubject() { - Map attrs = new HashMap<>(); - attrs.put("name", "张三"); - - assertThatThrownBy(() -> extractor.extract(userRequest(), permissiveUser(attrs))) - .isInstanceOf(OAuth2AuthenticationException.class) - .hasMessageContaining("open_id"); - } - /** An {@link OAuth2User} that does not enforce the name attribute, unlike DefaultOAuth2User. */ private OAuth2User permissiveUser(Map attrs) { return new OAuth2User() { 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 ab9c5ff1..39367a3f 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 @@ -81,6 +81,26 @@ class FeishuOAuth2UserServiceTest { server.verify(); } + @Test + void loadUser_rejectsOversizedResponseBody() { + RestClient.Builder restClientBuilder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(restClientBuilder).build(); + // 64 KB cap; pad a structurally valid envelope past it so the size check fires, not the parser. + String padding = "x".repeat(70 * 1024); + server.expect(requestTo("https://open.feishu.cn/open-apis/authen/v1/user_info")) + .andRespond(withSuccess( + "{\"code\":0,\"msg\":\"" + padding + "\",\"data\":{\"open_id\":\"ou_123\"}}", + MediaType.APPLICATION_JSON + )); + FeishuOAuth2UserService service = new FeishuOAuth2UserService(restClientBuilder); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()) + .isEqualTo("feishu_userinfo_error")); + server.verify(); + } + @Test void loadUser_errorDescriptionDoesNotEchoUpstreamTextOrToken() { RestClient.Builder restClientBuilder = RestClient.builder(); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java index e9046ef7..7ece104d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java @@ -74,6 +74,7 @@ class OAuthLoginFlowServiceTest { }; OAuthLoginFlowService service = new OAuthLoginFlowService( List.of(extractor), + List.of(), accessPolicy, identityBindingService, identityCore,