From 5c92c9eeede51b64aa026267158f4f54862f12b0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:56:33 +0800 Subject: [PATCH 01/11] feat(auth): let providers override token exchange and authorization params Extends the per-provider strategy pattern from the userinfo step to the two earlier stages of the authorization-code flow, so a provider whose endpoints deviate from the standard contract needs no branch in shared code: - ProviderTokenResponseClient for a non-standard token exchange, dispatched by DispatchingTokenResponseClient because Spring's tokenEndpoint accepts only one client - ProviderAuthorizationRequestCustomizer for authorization parameters, dispatched through the resolver's existing customizer hook Registrations without an override keep the standard Spring behaviour. Together with ProviderOAuth2UserService this covers all three stages where a provider can deviate: authorize, token, userinfo. Account decisions stay outside these hooks, in the unified identity core. Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skillhub/auth/config/SecurityConfig.java | 11 +-- .../oauth/DispatchingTokenResponseClient.java | 48 +++++++++ ...roviderAuthorizationRequestCustomizer.java | 17 ++++ .../oauth/ProviderTokenResponseClient.java | 16 +++ ...HubOAuth2AuthorizationRequestResolver.java | 33 ++++++- .../DispatchingTokenResponseClientTest.java | 99 +++++++++++++++++++ 6 files changed, 216 insertions(+), 8 deletions(-) create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClient.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderAuthorizationRequestCustomizer.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderTokenResponseClient.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClientTest.java diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java index 5880f58a..3039de42 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java @@ -2,7 +2,7 @@ package com.iflytek.skillhub.auth.config; import com.iflytek.skillhub.auth.oauth.CustomOAuth2UserService; import com.iflytek.skillhub.auth.oauth.CustomOidcUserService; -import com.iflytek.skillhub.auth.oauth.FeishuOAuth2AccessTokenResponseClient; +import com.iflytek.skillhub.auth.oauth.DispatchingTokenResponseClient; import com.iflytek.skillhub.auth.oauth.OAuth2LoginFailureHandler; import com.iflytek.skillhub.auth.oauth.OAuth2LoginSuccessHandler; import com.iflytek.skillhub.auth.oauth.SkillHubOAuth2AuthorizationRequestResolver; @@ -62,7 +62,7 @@ public class SecurityConfig { private final CustomOAuth2UserService customOAuth2UserService; private final CustomOidcUserService customOidcUserService; - private final FeishuOAuth2AccessTokenResponseClient feishuOAuth2AccessTokenResponseClient; + private final DispatchingTokenResponseClient tokenResponseClient; private final SkillHubOAuth2AuthorizationRequestResolver authorizationRequestResolver; private final OAuth2LoginSuccessHandler successHandler; private final OAuth2LoginFailureHandler failureHandler; @@ -77,7 +77,7 @@ public class SecurityConfig { public SecurityConfig(CustomOAuth2UserService customOAuth2UserService, CustomOidcUserService customOidcUserService, - FeishuOAuth2AccessTokenResponseClient feishuOAuth2AccessTokenResponseClient, + DispatchingTokenResponseClient tokenResponseClient, SkillHubOAuth2AuthorizationRequestResolver authorizationRequestResolver, OAuth2LoginSuccessHandler successHandler, OAuth2LoginFailureHandler failureHandler, @@ -91,7 +91,7 @@ public class SecurityConfig { @Value("${server.servlet.session.cookie.name:SESSION}") String sessionCookieName) { this.customOAuth2UserService = customOAuth2UserService; this.customOidcUserService = customOidcUserService; - this.feishuOAuth2AccessTokenResponseClient = feishuOAuth2AccessTokenResponseClient; + this.tokenResponseClient = tokenResponseClient; this.authorizationRequestResolver = authorizationRequestResolver; this.successHandler = successHandler; this.failureHandler = failureHandler; @@ -136,8 +136,7 @@ public class SecurityConfig { }) .oauth2Login(oauth2 -> oauth2 .authorizationEndpoint(endpoint -> endpoint.authorizationRequestResolver(authorizationRequestResolver)) - .tokenEndpoint(tokenEndpoint -> tokenEndpoint - .accessTokenResponseClient(feishuOAuth2AccessTokenResponseClient)) + .tokenEndpoint(token -> token.accessTokenResponseClient(tokenResponseClient)) .userInfoEndpoint(userInfo -> userInfo .userService(customOAuth2UserService) .oidcUserService(customOidcUserService)) diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClient.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClient.java new file mode 100644 index 00000000..934e89c7 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClient.java @@ -0,0 +1,48 @@ +package com.iflytek.skillhub.auth.oauth; + +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.oauth2.client.endpoint.DefaultAuthorizationCodeTokenResponseClient; +import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; +import org.springframework.stereotype.Component; + +/** + * Routes the authorization-code token exchange to a {@link ProviderTokenResponseClient} when one + * claims the registration, and to the standard Spring client otherwise. + * + *

Spring's {@code tokenEndpoint} accepts a single client, so per-provider exchange needs one + * dispatcher rather than a branch inside the security configuration. + */ +@Component +public class DispatchingTokenResponseClient + implements OAuth2AccessTokenResponseClient { + + private final Map overrides; + private final OAuth2AccessTokenResponseClient delegate; + + @Autowired + public DispatchingTokenResponseClient(List providerClients) { + this(providerClients, new DefaultAuthorizationCodeTokenResponseClient()); + } + + DispatchingTokenResponseClient( + List providerClients, + OAuth2AccessTokenResponseClient delegate + ) { + this.overrides = providerClients.stream() + .collect(Collectors.toMap(ProviderTokenResponseClient::getProvider, Function.identity())); + this.delegate = delegate; + } + + @Override + public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRequest request) { + String registrationId = request.getClientRegistration().getRegistrationId(); + ProviderTokenResponseClient override = overrides.get(registrationId); + return (override != null ? override : delegate).getTokenResponse(request); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderAuthorizationRequestCustomizer.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderAuthorizationRequestCustomizer.java new file mode 100644 index 00000000..a8504181 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderAuthorizationRequestCustomizer.java @@ -0,0 +1,17 @@ +package com.iflytek.skillhub.auth.oauth; + +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; + +/** + * Strategy interface for provider-specific authorization request tweaks, for providers whose + * authorize endpoint deviates from the standard parameter contract. + * + *

The token and userinfo counterparts are {@link ProviderTokenResponseClient} and + * {@link ProviderOAuth2UserService}. + */ +public interface ProviderAuthorizationRequestCustomizer { + + String getProvider(); + + void customize(OAuth2AuthorizationRequest.Builder builder); +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderTokenResponseClient.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderTokenResponseClient.java new file mode 100644 index 00000000..9dc85aef --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/ProviderTokenResponseClient.java @@ -0,0 +1,16 @@ +package com.iflytek.skillhub.auth.oauth; + +import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; + +/** + * Strategy interface for provider-specific token exchange. Implementations override the default + * exchange for providers whose token endpoints deviate from the standard form-urlencoded contract. + * + *

The userinfo counterpart is {@link ProviderOAuth2UserService}. + */ +public interface ProviderTokenResponseClient + extends OAuth2AccessTokenResponseClient { + + String getProvider(); +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java index df985f8f..04b387d1 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java @@ -1,11 +1,17 @@ package com.iflytek.skillhub.auth.oauth; import jakarta.servlet.http.HttpServletRequest; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.springframework.beans.factory.annotation.Autowired; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.stereotype.Component; /** @@ -21,13 +27,36 @@ public class SkillHubOAuth2AuthorizationRequestResolver private final DefaultOAuth2AuthorizationRequestResolver delegate; private final OAuthLoginFlowService oauthLoginFlowService; - public SkillHubOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository, - OAuthLoginFlowService oauthLoginFlowService) { + SkillHubOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository, + OAuthLoginFlowService oauthLoginFlowService) { + this(clientRegistrationRepository, oauthLoginFlowService, List.of()); + } + + @Autowired + public SkillHubOAuth2AuthorizationRequestResolver( + ClientRegistrationRepository clientRegistrationRepository, + OAuthLoginFlowService oauthLoginFlowService, + List customizers) { this.delegate = new DefaultOAuth2AuthorizationRequestResolver( clientRegistrationRepository, "/oauth2/authorization" ); this.oauthLoginFlowService = oauthLoginFlowService; + Map byProvider = customizers.stream() + .collect(Collectors.toMap( + ProviderAuthorizationRequestCustomizer::getProvider, + Function.identity() + )); + // Spring resolves the registration id into the builder attributes, so one customizer hook + // can dispatch per provider instead of this class knowing about any of them. + this.delegate.setAuthorizationRequestCustomizer(builder -> { + OAuth2AuthorizationRequest probe = builder.build(); + String registrationId = probe.getAttribute(OAuth2ParameterNames.REGISTRATION_ID); + ProviderAuthorizationRequestCustomizer customizer = byProvider.get(registrationId); + if (customizer != null) { + customizer.customize(builder); + } + }); } @Override diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClientTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClientTest.java new file mode 100644 index 00000000..d2c18586 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DispatchingTokenResponseClientTest.java @@ -0,0 +1,99 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationExchange; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse; + +class DispatchingTokenResponseClientTest { + + @Test + void routesToProviderOverrideWhenOneClaimsTheRegistration() { + OAuth2AccessTokenResponse overrideResponse = response("from-override"); + OAuth2AccessTokenResponse defaultResponse = response("from-default"); + DispatchingTokenResponseClient client = new DispatchingTokenResponseClient( + List.of(stubProvider("dingtalk", overrideResponse)), + request -> defaultResponse + ); + + OAuth2AccessTokenResponse result = client.getTokenResponse(grantRequest("dingtalk")); + + assertThat(result.getAccessToken().getTokenValue()).isEqualTo("from-override"); + } + + @Test + void fallsBackToDefaultClientForUnclaimedRegistrations() { + OAuth2AccessTokenResponse overrideResponse = response("from-override"); + OAuth2AccessTokenResponse defaultResponse = response("from-default"); + DispatchingTokenResponseClient client = new DispatchingTokenResponseClient( + List.of(stubProvider("dingtalk", overrideResponse)), + request -> defaultResponse + ); + + // GitHub must keep the standard exchange even while a DingTalk override is registered. + OAuth2AccessTokenResponse result = client.getTokenResponse(grantRequest("github")); + + assertThat(result.getAccessToken().getTokenValue()).isEqualTo("from-default"); + } + + private static ProviderTokenResponseClient stubProvider( + String provider, + OAuth2AccessTokenResponse response + ) { + return new ProviderTokenResponseClient() { + @Override + public String getProvider() { + return provider; + } + + @Override + public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRequest request) { + return response; + } + }; + } + + private static OAuth2AccessTokenResponse response(String tokenValue) { + return OAuth2AccessTokenResponse.withToken(tokenValue) + .tokenType(org.springframework.security.oauth2.core.OAuth2AccessToken.TokenType.BEARER) + .expiresIn(3600) + .build(); + } + + private static OAuth2AuthorizationCodeGrantRequest grantRequest(String registrationId) { + ClientRegistration registration = ClientRegistration.withRegistrationId(registrationId) + .clientId("client") + .clientSecret("secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .authorizationUri("https://provider.example/authorize") + .tokenUri("https://provider.example/token") + .userInfoUri("https://provider.example/me") + .userNameAttributeName("id") + .build(); + OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode() + .authorizationUri("https://provider.example/authorize") + .clientId("client") + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .state("state-1") + .build(); + OAuth2AuthorizationResponse authorizationResponse = OAuth2AuthorizationResponse.success("code-1") + .redirectUri("https://skillhub.example/login/oauth2/code/" + registrationId) + .state("state-1") + .build(); + return new OAuth2AuthorizationCodeGrantRequest( + registration, + new OAuth2AuthorizationExchange(authorizationRequest, authorizationResponse) + ); + } + +} From 96f244b416fa56b077d8612390eb6e410b14ca2c Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:56:58 +0800 Subject: [PATCH 02/11] feat(auth): add DingTalk as a public login provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds DingTalk (钉钉) as a public sign-in option: it authenticates a SkillHub platform account and nothing more. No Organization membership, no directory sync, no Namespace grants. DingTalk deviates from standard OAuth at all three stages, one strategy each: - authorize: its endpoint wants scope=openid, but declaring that scope in configuration makes Spring treat the registration as OIDC and attach a nonce, which DingTalk rejects. The scope is added by DingTalkAuthorizationRequestCustomizer instead, keeping this a plain OAuth2 client. A test asserts the scope is present and the nonce is not. - token: credentials go in a JSON body rather than a form, handled by DingTalkTokenResponseClient. - userinfo: the token travels in x-acs-dingtalk-access-token rather than Authorization: Bearer. Subject and email semantics, which decide whether a login can reach an existing account: - unionId is the only accepted subject. DingTalk also returns openId and userId, but they must not act as fallbacks: openId is scoped per app and userId per organization, so a login falling back to either would bind a different identity than a later login carrying unionId, splitting one person across two platform accounts. - A blank or missing unionId fails the login. - emailVerified is always false. DingTalk returns the email an organization admin recorded without attesting the user controls it. The userinfo service only fetches attributes; account matching, provisioning and session creation stay with the unified identity core. The reference implementation called OAuthLoginFlowService.authenticate() from inside loadUser, which decided the account before the core's gate ran. Operational bounds match the Feishu adapter: connect and read timeouts, a 64 KB response cap, error descriptions and logs carrying only the exception class or provider error code, and no logging in the claims extractor. Unused PII is dropped rather than carried into the principal -- notably mobile and stateCode. Adds ProviderStrategyWiringTest, which loads the real application context. The unit tests call package-visible constructors and so cannot catch Spring wiring faults; a component with two constructors and no @Autowired marker unit-tests green and then fails at startup. That happened during this work. Adapted from the implementation in #467 by @konglong87, re-extracted onto current main with the subject, structure and bounds changes above. Part of R1-A2 (public Provider adapters) per openspec/changes/enterprise-identity-platform/rollout-plan.md. Co-authored-by: konglong87 Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../src/main/resources/application.yml | 17 ++ .../oauth/ProviderStrategyWiringTest.java | 88 ++++++++ ...ingTalkAuthorizationRequestCustomizer.java | 30 +++ .../auth/oauth/DingTalkClaimsExtractor.java | 76 +++++++ .../auth/oauth/DingTalkOAuth2Constants.java | 23 ++ .../auth/oauth/DingTalkOAuth2UserService.java | 162 ++++++++++++++ .../oauth/DingTalkTokenResponseClient.java | 145 +++++++++++++ .../oauth/DingTalkClaimsExtractorTest.java | 122 +++++++++++ .../oauth/DingTalkOAuth2UserServiceTest.java | 140 ++++++++++++ .../DingTalkTokenResponseClientTest.java | 201 ++++++++++++++++++ ...Auth2AuthorizationRequestResolverTest.java | 87 ++++++++ web/public/dingtalk-logo.svg | 3 + 12 files changed, 1094 insertions(+) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java create mode 100644 web/public/dingtalk-logo.svg diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 0328a749..97b724f7 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -80,6 +80,18 @@ spring: client-authentication-method: client_secret_post redirect-uri: "${OAUTH2_FEISHU_REDIRECT_URI:{baseUrl}/login/oauth2/code/{registrationId}}" client-name: ${OAUTH2_FEISHU_DISPLAY_NAME:飞书} + dingtalk: + client-id: ${OAUTH2_DINGTALK_CLIENT_ID:placeholder} + client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET:placeholder} + # No scope is declared on purpose. DingTalk's authorize endpoint wants scope=openid, + # but declaring it here makes Spring treat the registration as OIDC and attach a + # nonce, which DingTalk rejects. DingTalkAuthorizationRequestCustomizer adds the + # scope back to the outgoing URI without turning this into an OIDC flow. + authorization-grant-type: authorization_code + # DingTalk sends credentials in a JSON body, handled by DingTalkTokenResponseClient. + client-authentication-method: none + redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" + client-name: ${OAUTH2_DINGTALK_DISPLAY_NAME:钉钉} provider: github: api-base-url: ${OAUTH2_GITHUB_API_BASE_URL:https://api.github.com} @@ -97,6 +109,11 @@ spring: token-uri: ${OAUTH2_FEISHU_TOKEN_URI:https://accounts.feishu.cn/oauth/v3/token} user-info-uri: ${OAUTH2_FEISHU_USER_INFO_URI:${OAUTH2_FEISHU_BASE_URI:https://open.feishu.cn}/open-apis/authen/v1/user_info} user-name-attribute: open_id + dingtalk: + authorization-uri: ${OAUTH2_DINGTALK_AUTHORIZE_URI:https://login.dingtalk.com}/oauth2/auth + token-uri: ${OAUTH2_DINGTALK_BASE_URI:https://api.dingtalk.com}/v1.0/oauth2/userAccessToken + user-info-uri: ${OAUTH2_DINGTALK_BASE_URI:https://api.dingtalk.com}/v1.0/contact/users/me + user-name-attribute: unionId servlet: multipart: max-file-size: 100MB 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 new file mode 100644 index 00000000..c87a9ecc --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/auth/oauth/ProviderStrategyWiringTest.java @@ -0,0 +1,88 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.oauth.DingTalkOAuth2Constants; +import com.iflytek.skillhub.auth.oauth.DispatchingTokenResponseClient; +import com.iflytek.skillhub.auth.oauth.OAuthClaimsExtractor; +import com.iflytek.skillhub.auth.oauth.ProviderAuthorizationRequestCustomizer; +import com.iflytek.skillhub.auth.oauth.ProviderOAuth2UserService; +import com.iflytek.skillhub.auth.oauth.ProviderTokenResponseClient; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.ActiveProfiles; + +/** + * Loads the real application context to prove the provider strategy beans are constructible. + * + *

The unit tests for these classes call their package-visible constructors directly, so they + * cannot catch Spring wiring faults: a component with two constructors and no {@code @Autowired} + * marker compiles and unit-tests green, then fails at startup with "No default constructor found". + * This test is the guard for that class of failure. + */ +@SpringBootTest +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class ProviderStrategyWiringTest { + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @Autowired + private DispatchingTokenResponseClient dispatchingTokenResponseClient; + + @Autowired + private List tokenResponseClients; + + @Autowired + private List userServices; + + @Autowired + private List authorizationCustomizers; + + @Autowired + private List claimsExtractors; + + @Test + void dispatcherAndEveryProviderStrategyAreConstructible() { + assertThat(dispatchingTokenResponseClient).isNotNull(); + + // DingTalk needs all three strategy hooks; a missing bean would silently fall back to the + // standard OAuth2 behaviour its endpoints reject. + assertThat(tokenResponseClients) + .extracting(ProviderTokenResponseClient::getProvider) + .contains(DingTalkOAuth2Constants.REGISTRATION_ID); + assertThat(authorizationCustomizers) + .extracting(ProviderAuthorizationRequestCustomizer::getProvider) + .contains(DingTalkOAuth2Constants.REGISTRATION_ID); + assertThat(userServices) + .extracting(ProviderOAuth2UserService::getProvider) + .contains(DingTalkOAuth2Constants.REGISTRATION_ID, "feishu"); + assertThat(claimsExtractors) + .extracting(OAuthClaimsExtractor::getProvider) + .contains(DingTalkOAuth2Constants.REGISTRATION_ID, "feishu", "github"); + } + + @Test + void providerKeysAreUniqueSoDispatchMapsCannotCollide() { + // Collectors.toMap in the dispatchers throws on duplicate keys, which would break startup. + assertThat(tokenResponseClients).extracting(ProviderTokenResponseClient::getProvider) + .doesNotHaveDuplicates(); + assertThat(userServices).extracting(ProviderOAuth2UserService::getProvider) + .doesNotHaveDuplicates(); + assertThat(authorizationCustomizers).extracting(ProviderAuthorizationRequestCustomizer::getProvider) + .doesNotHaveDuplicates(); + assertThat(claimsExtractors).extracting(OAuthClaimsExtractor::getProvider) + .doesNotHaveDuplicates(); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java new file mode 100644 index 00000000..aa5494c1 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java @@ -0,0 +1,30 @@ +package com.iflytek.skillhub.auth.oauth; + +import java.util.LinkedHashSet; +import java.util.Set; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.stereotype.Component; + +/** + * Adds the {@code openid} scope DingTalk's authorize endpoint requires. + * + *

The scope cannot simply be declared in {@code application.yml}: Spring Security treats a + * registration carrying {@code openid} as an OIDC client and attaches a {@code nonce} parameter, + * which DingTalk rejects. Adding the scope here keeps the registration a plain OAuth2 client while + * still sending the parameter DingTalk expects. + */ +@Component +public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthorizationRequestCustomizer { + + @Override + public String getProvider() { + return DingTalkOAuth2Constants.REGISTRATION_ID; + } + + @Override + public void customize(OAuth2AuthorizationRequest.Builder builder) { + Set scopes = new LinkedHashSet<>(builder.build().getScopes()); + scopes.add(DingTalkOAuth2Constants.AUTHORIZATION_SCOPE); + builder.scopes(scopes); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java new file mode 100644 index 00000000..a382f075 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractor.java @@ -0,0 +1,76 @@ +package com.iflytek.skillhub.auth.oauth; + +import java.util.Map; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.user.OAuth2User; +import org.springframework.stereotype.Component; + +/** + * Provider-specific claims extractor for DingTalk (钉钉). Attributes are already fetched by + * {@link DingTalkOAuth2UserService}, which reads them from DingTalk's non-standard user info + * endpoint. + * + *

Like the GitHub and Feishu extractors, this class logs nothing: the subject, display name and + * email it handles are exactly the values that must stay out of the logs. + */ +@Component +public class DingTalkClaimsExtractor implements OAuthClaimsExtractor { + + @Override + public String getProvider() { + return DingTalkOAuth2Constants.REGISTRATION_ID; + } + + @Override + public OAuthClaims extract(OAuth2UserRequest request, OAuth2User oAuth2User) { + Map attrs = oAuth2User.getAttributes(); + + String subject = requireText( + attrs.get(DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME), + DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME + ); + + String email = text(attrs.get("email")); + // DingTalk's user info endpoint returns the email recorded by the organization admin and + // does not attest that the user controls it, so it carries no verification signal and + // cannot be used to join an existing account. + boolean emailVerified = false; + + // nick -> name and stop. 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 providerLogin = text(attrs.get("nick")); + if (providerLogin == null) { + providerLogin = text(attrs.get("name")); + } + + return new OAuthClaims( + DingTalkOAuth2Constants.REGISTRATION_ID, + subject, + email, + emailVerified, + providerLogin, + attrs + ); + } + + private static String requireText(Object value, String attribute) { + String text = text(value); + if (text == null) { + throw new OAuth2AuthenticationException( + new OAuth2Error("missing_subject", "DingTalk user info is missing " + attribute, null) + ); + } + return text; + } + + private static String text(Object value) { + if (value == null) { + return null; + } + String text = String.valueOf(value).trim(); + return text.isEmpty() ? null : text; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java new file mode 100644 index 00000000..83026e50 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java @@ -0,0 +1,23 @@ +package com.iflytek.skillhub.auth.oauth; + +/** Shared protocol constants for the DingTalk OAuth2 adapter. */ +public final class DingTalkOAuth2Constants { + + public static final String REGISTRATION_ID = "dingtalk"; + public static final String AUTHORIZATION_SCOPE = "openid"; + public static final String ACCESS_TOKEN_HEADER = "x-acs-dingtalk-access-token"; + + /** + * The only accepted subject claim. DingTalk also returns {@code openId} and {@code userId}, but + * they must not act as fallbacks: {@code openId} is scoped per app and {@code userId} per + * organization, so a login that fell back to either would bind a different identity than a + * later login carrying {@code unionId}, splitting one person across two platform accounts. + * Promoting another claim later needs an explicit alias migration. + */ + static final String SUBJECT_CLAIM_NAME = "unionId"; + + public static final String SUBJECT_ATTRIBUTE = "dingtalkSubject"; + + private DingTalkOAuth2Constants() { + } +} 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 new file mode 100644 index 00000000..7b8331a2 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserService.java @@ -0,0 +1,162 @@ +package com.iflytek.skillhub.auth.oauth; + +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.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.user.DefaultOAuth2User; +import org.springframework.security.oauth2.core.user.OAuth2User; +import org.springframework.stereotype.Component; +import org.springframework.web.client.RestClient; + +/** + * Loads DingTalk (钉钉) user info, which deviates from standard OAuth: the access token travels in + * a custom {@code x-acs-dingtalk-access-token} header rather than {@code Authorization: Bearer}. + * + *

This service only fetches attributes. Account matching, provisioning and session creation + * stay with the unified identity core reached through {@link OAuthLoginFlowService}, so DingTalk + * cannot decide who a login resolves to. + */ +@Component +public class DingTalkOAuth2UserService implements ProviderOAuth2UserService { + + private static final Logger log = LoggerFactory.getLogger(DingTalkOAuth2UserService.class); + + private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(5); + private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); + + /** A DingTalk contact 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(); + + private final RestClient restClient; + + /** + * Uses an external-service client that is intentionally not customized with application + * tracing. Trace context must not be propagated to the external DingTalk service. + */ + @Autowired + public DingTalkOAuth2UserService() { + this(RestClient.builder().requestFactory(defaultRequestFactory())); + } + + public DingTalkOAuth2UserService(RestClient.Builder restClientBuilder) { + this.restClient = restClientBuilder + .defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) + .build(); + } + + /** + * Bounds the userinfo call so an unresponsive DingTalk endpoint cannot hold a login thread. The + * timeouts apply to this provider client only and do not change the shared HTTP defaults. + */ + private static ClientHttpRequestFactory defaultRequestFactory() { + SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(CONNECT_TIMEOUT); + factory.setReadTimeout(READ_TIMEOUT); + return factory; + } + + @Override + public String getProvider() { + return DingTalkOAuth2Constants.REGISTRATION_ID; + } + + @Override + public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { + String userInfoUri = userRequest.getClientRegistration().getProviderDetails() + .getUserInfoEndpoint().getUri(); + + Map payload; + try { + payload = restClient.get() + .uri(userInfoUri) + .header( + DingTalkOAuth2Constants.ACCESS_TOKEN_HEADER, + userRequest.getAccessToken().getTokenValue() + ) + .exchange((request, clientResponse) -> 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()); + throw new OAuth2AuthenticationException( + new OAuth2Error("dingtalk_userinfo_error", "Failed to load DingTalk user info", null), + e + ); + } + + return new DefaultOAuth2User( + Collections.singleton(new SimpleGrantedAuthority("ROLE_USER")), + normalize(payload), + DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME + ); + } + + /** + * Reads at most {@link #MAX_RESPONSE_BYTES} before parsing, so a misconfigured or hostile + * {@code OAUTH2_DINGTALK_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 Map readBounded(InputStream body) throws IOException { + byte[] bytes = body.readNBytes(MAX_RESPONSE_BYTES + 1); + if (bytes.length > MAX_RESPONSE_BYTES) { + throw new IOException("DingTalk user info response exceeds " + MAX_RESPONSE_BYTES + " bytes"); + } + return OBJECT_MAPPER.readValue(bytes, new com.fasterxml.jackson.core.type.TypeReference<>() { + }); + } + + /** + * 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 + * platform does not use -- notably {@code mobile} and {@code stateCode} -- are dropped rather + * than carried into the principal, keeping unused PII out of claims and logs. + */ + private static Map normalize(Map payload) { + Map attributes = new LinkedHashMap<>(); + copyIfPresent(attributes, payload, DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME); + copyIfPresent(attributes, payload, "nick"); + copyIfPresent(attributes, payload, "name"); + copyIfPresent(attributes, payload, "email"); + Object avatar = payload.get("avatarUrl"); + if (avatar != null && !String.valueOf(avatar).isBlank()) { + attributes.put("avatar_url", avatar); + } + if (!attributes.containsKey(DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME)) { + throw new OAuth2AuthenticationException( + new OAuth2Error( + "dingtalk_userinfo_error", + "DingTalk user info missing " + DingTalkOAuth2Constants.SUBJECT_CLAIM_NAME, + null + ) + ); + } + return attributes; + } + + private static void copyIfPresent( + Map target, + Map source, + String key + ) { + Object value = source.get(key); + if (value != null && !String.valueOf(value).isBlank()) { + target.put(key, value); + } + } +} 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 new file mode 100644 index 00000000..bfb79978 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClient.java @@ -0,0 +1,145 @@ +package com.iflytek.skillhub.auth.oauth; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.time.Duration; +import java.util.Map; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; +import org.springframework.stereotype.Component; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestClientResponseException; +import org.springframework.web.client.RestTemplate; + +/** + * Custom token response client for DingTalk (钉钉). + * + *

DingTalk requires a JSON body for token exchange instead of the standard + * form-urlencoded format. This client adapts the request accordingly. + * + *

Request body format: + * 

{ "clientId": "...", "clientSecret": "...", "code": "...", "grantType": "authorization_code" }
+ */ +@Component +public class DingTalkTokenResponseClient implements ProviderTokenResponseClient { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + private final RestTemplate restTemplate; + + @Autowired + public DingTalkTokenResponseClient() { + this.restTemplate = buildRestTemplate(); + } + + /** Package-visible constructor for unit testing with a mock RestTemplate. */ + DingTalkTokenResponseClient(RestTemplate restTemplate) { + this.restTemplate = restTemplate; + } + + @Override + public String getProvider() { + return DingTalkOAuth2Constants.REGISTRATION_ID; + } + + private static RestTemplate buildRestTemplate() { + var factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(Duration.ofSeconds(5)); + factory.setReadTimeout(Duration.ofSeconds(10)); + return new RestTemplate(factory); + } + + @Override + public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest) + throws OAuth2AuthenticationException { + String tokenUri = authorizationCodeGrantRequest.getClientRegistration().getProviderDetails().getTokenUri(); + String clientId = authorizationCodeGrantRequest.getClientRegistration().getClientId(); + String clientSecret = authorizationCodeGrantRequest.getClientRegistration().getClientSecret(); + String code = authorizationCodeGrantRequest.getAuthorizationExchange() + .getAuthorizationResponse() + .getCode(); + + Map tokenRequest = Map.of( + "clientId", clientId, + "clientSecret", clientSecret, + "code", code, + "grantType", "authorization_code" + ); + + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + + ResponseEntity response; + try { + response = restTemplate.postForEntity(tokenUri, new HttpEntity<>(tokenRequest, headers), String.class); + } catch (RestClientResponseException e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_exchange_io_error", + "DingTalk token exchange failed with HTTP " + e.getStatusCode().value(), null)); + } catch (RestClientException e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_exchange_io_error", + "DingTalk token exchange request failed", null)); + } + + if (response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { + try { + JsonNode json = MAPPER.readTree(response.getBody()); + + JsonNode accessTokenNode = json.get("accessToken"); + if (accessTokenNode == null || accessTokenNode.isNull()) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_missing_field", + "DingTalk token response missing accessToken field", null)); + } + String accessToken = accessTokenNode.asText(); + if (accessToken.isBlank()) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_missing_field", + "DingTalk token response has empty accessToken", null)); + } + + JsonNode expireInNode = json.get("expireIn"); + if (expireInNode == null || !expireInNode.isIntegralNumber() || !expireInNode.canConvertToLong()) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_invalid_expiry", + "DingTalk token response has invalid expireIn field", null)); + } + long expireInSeconds = expireInNode.longValue(); + if (expireInSeconds <= 0) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_response_invalid_expiry", + "DingTalk token response has non-positive expireIn field", null)); + } + + // Only include non-sensitive fields in additional parameters. + Map safeParams = Map.of("expireIn", expireInSeconds); + + return OAuth2AccessTokenResponse.withToken(accessToken) + .tokenType(OAuth2AccessToken.TokenType.BEARER) + .expiresIn(expireInSeconds) + .additionalParameters(safeParams) + .build(); + } catch (OAuth2AuthenticationException e) { + throw e; + } catch (Exception e) { + throw new OAuth2AuthenticationException( + new OAuth2Error("token_parse_error", + "Failed to parse DingTalk token response", null)); + } + } + + throw new OAuth2AuthenticationException( + new OAuth2Error("token_exchange_failed", + "DingTalk token exchange failed: HTTP " + response.getStatusCode(), null)); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java new file mode 100644 index 00000000..8bf38b18 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java @@ -0,0 +1,122 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.user.OAuth2User; + +class DingTalkClaimsExtractorTest { + + private final DingTalkClaimsExtractor extractor = new DingTalkClaimsExtractor(); + + @Test + void extract_mapsUnionIdAndNick() { + Map attrs = new HashMap<>(Map.of( + "unionId", "un_123", + "nick", "张三", + "email", "zhangsan@corp.example" + )); + + OAuthClaims claims = extractor.extract(userRequest(), user(attrs)); + + assertThat(claims.provider()).isEqualTo("dingtalk"); + assertThat(claims.subject()).isEqualTo("un_123"); + assertThat(claims.providerLogin()).isEqualTo("张三"); + assertThat(claims.email()).isEqualTo("zhangsan@corp.example"); + // DingTalk's contact endpoint does not attest email ownership. + assertThat(claims.emailVerified()).isFalse(); + } + + @Test + void extract_neverAcceptsOpenIdOrUserIdAsSubject() { + // openId is per-app and userId per-organization. Accepting either as a fallback would bind + // a different identity than a later login carrying unionId, splitting one person across + // two platform accounts. + Map attrs = new HashMap<>(Map.of( + "openId", "op_456", + "userId", "usr_789", + "nick", "张三" + )); + + assertThatThrownBy(() -> extractor.extract(userRequest(), user(attrs))) + .isInstanceOf(OAuth2AuthenticationException.class) + .hasMessageContaining("unionId"); + } + + @Test + void extract_rejectsBlankUnionId() { + Map attrs = new HashMap<>(); + attrs.put("unionId", " "); + attrs.put("nick", "张三"); + + assertThatThrownBy(() -> extractor.extract(userRequest(), user(attrs))) + .isInstanceOf(OAuth2AuthenticationException.class) + .hasMessageContaining("unionId"); + } + + @Test + void extract_fallsBackToNameThenLeavesDisplayNameUnset() { + Map withName = new HashMap<>(Map.of("unionId", "un_1", "name", "Alice")); + assertThat(extractor.extract(userRequest(), user(withName)).providerLogin()).isEqualTo("Alice"); + + // Must not synthesize from the subject: providerLogin is written to displayName and into + // UserActivatedEvent, so a synthesized value would carry the subject to event consumers. + Map bare = new HashMap<>(Map.of("unionId", "un_2")); + OAuthClaims claims = extractor.extract(userRequest(), user(bare)); + assertThat(claims.providerLogin()).isNull(); + assertThat(claims.subject()).isEqualTo("un_2"); + } + + /** Does not enforce the name attribute, unlike DefaultOAuth2User. */ + private OAuth2User user(Map attrs) { + return new OAuth2User() { + @Override + public Map getAttributes() { + return attrs; + } + + @Override + public java.util.Collection + getAuthorities() { + return java.util.List.of(); + } + + @Override + public String getName() { + return String.valueOf(attrs.get("unionId")); + } + }; + } + + private OAuth2UserRequest userRequest() { + ClientRegistration registration = ClientRegistration.withRegistrationId("dingtalk") + .clientId("dingoauth_test") + .clientSecret("client-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .clientAuthenticationMethod(ClientAuthenticationMethod.NONE) + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .authorizationUri("https://login.dingtalk.com/oauth2/auth") + .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") + .userInfoUri("https://api.dingtalk.com/v1.0/contact/users/me") + .userNameAttributeName("unionId") + .clientName("钉钉") + .build(); + OAuth2AccessToken accessToken = new OAuth2AccessToken( + OAuth2AccessToken.TokenType.BEARER, + "token-123", + Instant.now(), + Instant.now().plusSeconds(3600) + ); + return new OAuth2UserRequest(registration, accessToken); + } +} 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 new file mode 100644 index 00000000..e5d7027a --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java @@ -0,0 +1,140 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +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 java.time.Instant; +import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +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; + +class DingTalkOAuth2UserServiceTest { + + @Test + void loadUser_sendsCustomTokenHeaderAndNormalizesAttributes() { + RestClient.Builder builder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(builder).build(); + server.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + // DingTalk reads the token from its own header, not Authorization: Bearer. + .andExpect(header(DingTalkOAuth2Constants.ACCESS_TOKEN_HEADER, "token-123")) + .andRespond(withSuccess( + """ + { + "unionId": "un_123", + "openId": "op_456", + "nick": "张三", + "avatarUrl": "https://avatar.example/z.png", + "email": "zhangsan@corp.example", + "mobile": "13800000000", + "stateCode": "86" + } + """, + MediaType.APPLICATION_JSON + )); + DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder); + + OAuth2User user = service.loadUser(userRequest()); + + assertThat(user.getName()).isEqualTo("un_123"); + assertThat(user.getAttributes()) + .containsEntry("unionId", "un_123") + .containsEntry("nick", "张三") + .containsEntry("email", "zhangsan@corp.example") + // avatarUrl is aliased to the key the identity core reads. + .containsEntry("avatar_url", "https://avatar.example/z.png"); + // Unused PII must not travel into the principal or claims. + assertThat(user.getAttributes()).doesNotContainKeys("mobile", "stateCode", "avatarUrl"); + // openId must not survive as a usable subject candidate. + assertThat(user.getAttributes()).doesNotContainKey("openId"); + server.verify(); + } + + @Test + void loadUser_rejectsResponseWithoutUnionId() { + RestClient.Builder builder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(builder).build(); + server.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withSuccess( + """ + {"openId": "op_456", "nick": "张三"} + """, + MediaType.APPLICATION_JSON + )); + DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .hasMessageContaining("unionId"); + server.verify(); + } + + @Test + void loadUser_rejectsOversizedResponseBody() { + RestClient.Builder builder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(builder).build(); + // 64 KB cap; pad a structurally valid payload past it so the size check fires, not the parser. + String padding = "x".repeat(70 * 1024); + server.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withSuccess( + "{\"unionId\":\"un_123\",\"nick\":\"" + padding + "\"}", + MediaType.APPLICATION_JSON + )); + DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()) + .isEqualTo("dingtalk_userinfo_error")); + server.verify(); + } + + @Test + void loadUser_errorDescriptionDoesNotEchoUpstreamTextOrToken() { + RestClient.Builder builder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(builder).build(); + server.expect(requestTo("https://api.dingtalk.com/v1.0/contact/users/me")) + .andRespond(withSuccess("not json at all: token-123", MediaType.APPLICATION_JSON)); + DingTalkOAuth2UserService service = new DingTalkOAuth2UserService(builder); + + assertThatThrownBy(() -> service.loadUser(userRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> { + String description = ((OAuth2AuthenticationException) ex).getError().getDescription(); + assertThat(description).doesNotContain("token-123"); + }); + server.verify(); + } + + private OAuth2UserRequest userRequest() { + ClientRegistration registration = ClientRegistration.withRegistrationId("dingtalk") + .clientId("dingoauth_test") + .clientSecret("client-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .clientAuthenticationMethod(ClientAuthenticationMethod.NONE) + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .authorizationUri("https://login.dingtalk.com/oauth2/auth") + .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") + .userInfoUri("https://api.dingtalk.com/v1.0/contact/users/me") + .userNameAttributeName("unionId") + .clientName("钉钉") + .build(); + OAuth2AccessToken accessToken = new OAuth2AccessToken( + OAuth2AccessToken.TokenType.BEARER, + "token-123", + Instant.now(), + Instant.now().plusSeconds(3600) + ); + return new OAuth2UserRequest(registration, accessToken); + } +} 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 new file mode 100644 index 00000000..ac15a660 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkTokenResponseClientTest.java @@ -0,0 +1,201 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +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.withServerError; + +import java.time.Duration; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; +import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationExchange; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.web.client.RestTemplate; + +class DingTalkTokenResponseClientTest { + + private DingTalkTokenResponseClient client; + private MockRestServiceServer mockServer; + + @BeforeEach + void setUp() { + RestTemplate restTemplate = new RestTemplate(); + mockServer = MockRestServiceServer.createServer(restTemplate); + client = new DingTalkTokenResponseClient(restTemplate); + } + + @Test + void getTokenResponse_returnsAccessTokenOnSuccess() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": "dt_access_token_123", + "expireIn": 7200 + } + """, + MediaType.APPLICATION_JSON + )); + + OAuth2AccessTokenResponse response = client.getTokenResponse(authorizationCodeGrantRequest()); + + assertThat(response.getAccessToken().getTokenValue()).isEqualTo("dt_access_token_123"); + assertThat(response.getAccessToken().getTokenType()).isEqualTo(OAuth2AccessToken.TokenType.BEARER); + assertThat(response.getAccessToken().getIssuedAt()).isNotNull(); + assertThat(response.getAccessToken().getExpiresAt()).isNotNull(); + assertThat(Duration.between( + response.getAccessToken().getIssuedAt(), + response.getAccessToken().getExpiresAt())).isEqualTo(Duration.ofSeconds(7200)); + assertThat(response.getAdditionalParameters().get("expireIn")).isEqualTo(7200L); + // Verify raw_response is NOT included (sensitive data leak fix) + assertThat(response.getAdditionalParameters().containsKey("raw_response")).isFalse(); + mockServer.verify(); + } + + @Test + void getTokenResponse_throwsWhenAccessTokenFieldMissing() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "expireIn": 7200 + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()).isEqualTo("token_response_missing_field")); + } + + @Test + void getTokenResponse_throwsWhenAccessTokenIsNull() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": null, + "expireIn": 7200 + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()).isEqualTo("token_response_missing_field")); + } + + @Test + void getTokenResponse_throwsWhenAccessTokenIsEmpty() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": "", + "expireIn": 7200 + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex).getError().getErrorCode()).isEqualTo("token_response_missing_field")); + } + + @Test + void getTokenResponse_throwsOnHttpError() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withServerError().body("sensitive-upstream-response")); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> { + OAuth2AuthenticationException oauthException = (OAuth2AuthenticationException) ex; + assertThat(oauthException.getError().getErrorCode()).isEqualTo("token_exchange_io_error"); + assertThat(oauthException.getMessage()).doesNotContain("sensitive-upstream-response"); + }); + } + + @Test + void getTokenResponse_throwsWhenExpireInIsMissing() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": "dt_access_token_123" + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex) + .getError().getErrorCode()).isEqualTo("token_response_invalid_expiry")); + } + + @Test + void getTokenResponse_throwsWhenExpireInIsNonPositive() { + mockServer.expect(requestTo("https://api.dingtalk.com/v1.0/oauth2/userAccessToken")) + .andRespond(withSuccess( + """ + { + "accessToken": "dt_access_token_123", + "expireIn": 0 + } + """, + MediaType.APPLICATION_JSON + )); + + assertThatThrownBy(() -> client.getTokenResponse(authorizationCodeGrantRequest())) + .isInstanceOf(OAuth2AuthenticationException.class) + .satisfies(ex -> assertThat(((OAuth2AuthenticationException) ex) + .getError().getErrorCode()).isEqualTo("token_response_invalid_expiry")); + } + + private OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest() { + ClientRegistration registration = ClientRegistration.withRegistrationId("dingtalk") + .clientId("dingzgzf3b9k7jv74iq2") + .clientSecret("test-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .scope("openid") + .authorizationUri("https://login.dingtalk.com/oauth2/auth") + .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") + .userInfoUri("https://api.dingtalk.com/v1.0/contact/users/me") + .userNameAttributeName("unionId") + .clientName("钉钉") + .build(); + + OAuth2AuthorizationRequest authRequest = OAuth2AuthorizationRequest.authorizationCode() + .clientId(registration.getClientId()) + .authorizationUri(registration.getProviderDetails().getAuthorizationUri()) + .redirectUri(registration.getRedirectUri()) + .scopes(registration.getScopes()) + .state("test-state") + .build(); + + OAuth2AuthorizationResponse authResponse = OAuth2AuthorizationResponse.success("test-code") + .redirectUri(registration.getRedirectUri()) + .state("test-state") + .build(); + + return new OAuth2AuthorizationCodeGrantRequest( + registration, + new OAuth2AuthorizationExchange(authRequest, authResponse) + ); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java index 9cef26b2..49cb92c8 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java @@ -84,4 +84,91 @@ class OAuth2AuthorizationRequestResolverTest { assertThat(session).isNotNull(); assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE)).isNull(); } + + @Test + void resolve_addsDingTalkScopeWithoutTurningTheRequestIntoOidc() { + SkillHubOAuth2AuthorizationRequestResolver dingTalkResolver = resolverFor( + dingTalkRegistration(), + new DingTalkAuthorizationRequestCustomizer() + ); + MockHttpServletRequest request = + new MockHttpServletRequest("GET", "/oauth2/authorization/dingtalk"); + + var authorizationRequest = dingTalkResolver.resolve(request, "dingtalk"); + + assertThat(authorizationRequest).isNotNull(); + // DingTalk's authorize endpoint requires scope=openid... + assertThat(authorizationRequest.getScopes()).contains("openid"); + assertThat(authorizationRequest.getAuthorizationRequestUri()).contains("scope=openid"); + // ...but rejects the nonce Spring attaches when a registration declares openid in config. + // Declaring no scope there and adding it here is what keeps the nonce away. + assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey("nonce"); + assertThat(authorizationRequest.getAttributes()).doesNotContainKey("nonce"); + assertThat(authorizationRequest.getAuthorizationRequestUri()).doesNotContain("nonce="); + } + + @Test + void resolve_leavesOtherProvidersUntouchedWhenADingTalkCustomizerIsRegistered() { + SkillHubOAuth2AuthorizationRequestResolver mixedResolver = resolverFor( + githubRegistration(), + new DingTalkAuthorizationRequestCustomizer() + ); + MockHttpServletRequest request = + new MockHttpServletRequest("GET", "/oauth2/authorization/github"); + + var authorizationRequest = mixedResolver.resolve(request, "github"); + + assertThat(authorizationRequest).isNotNull(); + assertThat(authorizationRequest.getScopes()).containsExactly("read:user"); + } + + private static SkillHubOAuth2AuthorizationRequestResolver resolverFor( + ClientRegistration registration, + ProviderAuthorizationRequestCustomizer customizer + ) { + OAuthLoginFlowService flowService = new OAuthLoginFlowService( + java.util.List.of(), + mock(AccessPolicy.class), + mock(IdentityBindingService.class) + ); + return new SkillHubOAuth2AuthorizationRequestResolver( + new InMemoryClientRegistrationRepository(registration), + flowService, + java.util.List.of(customizer) + ); + } + + private static ClientRegistration githubRegistration() { + return ClientRegistration.withRegistrationId("github") + .clientId("client") + .clientSecret("secret") + .authorizationUri("https://example.test/oauth/authorize") + .tokenUri("https://example.test/oauth/token") + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .userInfoUri("https://example.test/user") + .userNameAttributeName("id") + .authorizationGrantType( + org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE) + .scope("read:user") + .clientName("GitHub") + .build(); + } + + private static ClientRegistration dingTalkRegistration() { + // Mirrors application.yml: no scope declared, so Spring keeps this a plain OAuth2 client. + return ClientRegistration.withRegistrationId("dingtalk") + .clientId("dingoauth_test") + .clientSecret("secret") + .authorizationUri("https://login.dingtalk.com/oauth2/auth") + .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .userInfoUri("https://api.dingtalk.com/v1.0/contact/users/me") + .userNameAttributeName("unionId") + .authorizationGrantType( + org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE) + .clientAuthenticationMethod( + org.springframework.security.oauth2.core.ClientAuthenticationMethod.NONE) + .clientName("钉钉") + .build(); + } } diff --git a/web/public/dingtalk-logo.svg b/web/public/dingtalk-logo.svg new file mode 100644 index 00000000..b1a268d1 --- /dev/null +++ b/web/public/dingtalk-logo.svg @@ -0,0 +1,3 @@ + + + From 75c7f9a88069f8f48fa8a98cd2d1bb008731f182 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:57:11 +0800 Subject: [PATCH 03/11] feat(deploy): wire DingTalk credentials into the release surfaces Adds the DingTalk credentials to every path that actually delivers configuration: compose.release.yml (which has no env_file, so variables must be listed explicitly), the Helm secret template and values, the k8s deployment and its secret example. validate-release-config.sh gains DingTalk in its provider loop, so a half-configured pair is rejected the same way. Documents the three-stage strategy contract in the authentication design: a table mapping each deviation -- authorize parameters, token exchange, userinfo loading -- to its interface and current implementations, plus the rule that a provider must never make account decisions itself. Deployment notes and both FAQs now cover DingTalk, including the shared trap with Feishu: their emails are admin-recorded and never confirmed, so emailVerified is always false and an EMAIL_DOMAIN access policy would reject every login through either provider. Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.example | 10 +++++++ charts/skillhub/templates/secret.yaml | 8 ++++++ .../skillhub/templates/server-deployment.yaml | 14 ++++++++++ charts/skillhub/values.yaml | 2 ++ compose.release.yml | 5 ++++ deploy/k8s/base/backend-deployment.yaml | 14 ++++++++++ deploy/k8s/base/secret.yaml.example | 4 +++ docs/03-authentication-design.md | 28 ++++++++++++++++--- docs/09-deployment.md | 8 ++++-- docs/skillhub/en/faq.md | 2 +- docs/skillhub/faq.md | 2 +- scripts/tests/validate-release-config-test.sh | 2 +- scripts/validate-release-config.sh | 2 +- 13 files changed, 90 insertions(+), 11 deletions(-) diff --git a/.env.release.example b/.env.release.example index 73800c73..add0f570 100644 --- a/.env.release.example +++ b/.env.release.example @@ -138,6 +138,16 @@ OAUTH2_FEISHU_USER_INFO_URI=https://open.feishu.cn/open-apis/authen/v1/user_info OAUTH2_FEISHU_REDIRECT_URI= OAUTH2_FEISHU_DISPLAY_NAME=飞书 +# Optional: DingTalk login as a public sign-in provider. Leaving the client id empty keeps the +# button off the login page. Use the app's AppKey as the client id and AppSecret as the secret. +# Like Feishu, DingTalk returns an organization-recorded email without attesting ownership, so +# emailVerified is always false and the EMAIL_DOMAIN access policy would reject every login. +OAUTH2_DINGTALK_CLIENT_ID= +OAUTH2_DINGTALK_CLIENT_SECRET= +OAUTH2_DINGTALK_AUTHORIZE_URI=https://login.dingtalk.com +OAUTH2_DINGTALK_BASE_URI=https://api.dingtalk.com +OAUTH2_DINGTALK_DISPLAY_NAME=钉钉 + # Optional: OIDC login (e.g. Keycloak, Okta, Azure AD). # Replace "OIDC" in variable names with your registration id (uppercase). # The registration id becomes identity_binding.provider_code — keep it stable. diff --git a/charts/skillhub/templates/secret.yaml b/charts/skillhub/templates/secret.yaml index d00c41a4..523bc386 100644 --- a/charts/skillhub/templates/secret.yaml +++ b/charts/skillhub/templates/secret.yaml @@ -67,6 +67,14 @@ stringData: oauth2-feishu-client-secret: {{ .Values.secrets.oauth2FeishuClientSecret | quote }} {{- end }} + # OAuth2 DingTalk (optional) + {{- if .Values.secrets.oauth2DingtalkClientId }} + oauth2-dingtalk-client-id: {{ .Values.secrets.oauth2DingtalkClientId | quote }} + {{- end }} + {{- if .Values.secrets.oauth2DingtalkClientSecret }} + oauth2-dingtalk-client-secret: {{ .Values.secrets.oauth2DingtalkClientSecret | quote }} + {{- end }} + # Scanner LLM 配置 (optional) {{- if .Values.secrets.scannerLlmApiKey }} skill-scanner-llm-api-key: {{ .Values.secrets.scannerLlmApiKey | quote }} diff --git a/charts/skillhub/templates/server-deployment.yaml b/charts/skillhub/templates/server-deployment.yaml index 7e635fe8..244bc4c9 100644 --- a/charts/skillhub/templates/server-deployment.yaml +++ b/charts/skillhub/templates/server-deployment.yaml @@ -381,6 +381,20 @@ spec: value: {{ . | quote }} {{- end }} + # OAuth2 DingTalk (optional) + - name: OAUTH2_DINGTALK_CLIENT_ID + valueFrom: + secretKeyRef: + name: {{ include "skillhub.secretName" . }} + key: oauth2-dingtalk-client-id + optional: true + - name: OAUTH2_DINGTALK_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: {{ include "skillhub.secretName" . }} + key: oauth2-dingtalk-client-secret + optional: true + {{- if .Values.server.javaOpts }} - name: JAVA_OPTS value: {{ .Values.server.javaOpts }} diff --git a/charts/skillhub/values.yaml b/charts/skillhub/values.yaml index f004f3f0..555ace85 100644 --- a/charts/skillhub/values.yaml +++ b/charts/skillhub/values.yaml @@ -103,6 +103,8 @@ secrets: oauth2GithubClientSecret: "" oauth2FeishuClientId: "" oauth2FeishuClientSecret: "" + oauth2DingtalkClientId: "" + oauth2DingtalkClientSecret: "" scannerLlmApiKey: "" scannerLlmBaseUrl: "" scannerLlmModel: "" diff --git a/compose.release.yml b/compose.release.yml index 6789ddeb..77cc571a 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -128,6 +128,11 @@ services: OAUTH2_FEISHU_USER_INFO_URI: ${OAUTH2_FEISHU_USER_INFO_URI:-${OAUTH2_FEISHU_BASE_URI:-https://open.feishu.cn}/open-apis/authen/v1/user_info} OAUTH2_FEISHU_REDIRECT_URI: ${OAUTH2_FEISHU_REDIRECT_URI:-${SKILLHUB_PUBLIC_BASE_URL:-http://localhost}/login/oauth2/code/feishu} OAUTH2_FEISHU_DISPLAY_NAME: ${OAUTH2_FEISHU_DISPLAY_NAME:-飞书} + OAUTH2_DINGTALK_CLIENT_ID: ${OAUTH2_DINGTALK_CLIENT_ID:-local-placeholder} + OAUTH2_DINGTALK_CLIENT_SECRET: ${OAUTH2_DINGTALK_CLIENT_SECRET:-local-placeholder} + OAUTH2_DINGTALK_AUTHORIZE_URI: ${OAUTH2_DINGTALK_AUTHORIZE_URI:-https://login.dingtalk.com} + OAUTH2_DINGTALK_BASE_URI: ${OAUTH2_DINGTALK_BASE_URI:-https://api.dingtalk.com} + OAUTH2_DINGTALK_DISPLAY_NAME: ${OAUTH2_DINGTALK_DISPLAY_NAME:-钉钉} SPRING_MAIL_HOST: ${SPRING_MAIL_HOST:-} SPRING_MAIL_PORT: ${SPRING_MAIL_PORT:-25} SPRING_MAIL_USERNAME: ${SPRING_MAIL_USERNAME:-} diff --git a/deploy/k8s/base/backend-deployment.yaml b/deploy/k8s/base/backend-deployment.yaml index 52e53b12..01319d16 100644 --- a/deploy/k8s/base/backend-deployment.yaml +++ b/deploy/k8s/base/backend-deployment.yaml @@ -248,6 +248,20 @@ spec: value: "https://accounts.feishu.cn/oauth/v3/token" - name: OAUTH2_FEISHU_USER_INFO_URI value: "https://open.feishu.cn/open-apis/authen/v1/user_info" + + # OAuth2 DingTalk (optional) + - name: OAUTH2_DINGTALK_CLIENT_ID + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-dingtalk-client-id + optional: true + - name: OAUTH2_DINGTALK_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: skillhub-secret + key: oauth2-dingtalk-client-secret + optional: true volumeMounts: - name: skillhub-storage mountPath: /var/lib/skillhub/storage diff --git a/deploy/k8s/base/secret.yaml.example b/deploy/k8s/base/secret.yaml.example index f9c119d6..f983fa19 100644 --- a/deploy/k8s/base/secret.yaml.example +++ b/deploy/k8s/base/secret.yaml.example @@ -31,6 +31,10 @@ stringData: oauth2-feishu-client-id: "" oauth2-feishu-client-secret: "" + # 钉钉 OAuth（可选，用于钉钉登录；留空则登录页不展示该入口） + oauth2-dingtalk-client-id: "" + oauth2-dingtalk-client-secret: "" + # LLM 配置（可选，用于技能扫描） skill-scanner-llm-api-key: "" skill-scanner-llm-base-url: "" diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index ef5aa98a..478e3420 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -282,6 +282,14 @@ spring: # 飞书的 scope 配在开放平台应用上，不在这里传 client-authentication-method: client_secret_post authorization-grant-type: authorization_code + dingtalk: + client-id: ${OAUTH2_DINGTALK_CLIENT_ID} + client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET} + # 故意不声明 scope：钉钉的授权端点要 scope=openid，但在这里声明会让 + # Spring 把该注册当成 OIDC 客户端并附加 nonce，而钉钉不接受 nonce。 + # scope 由 DingTalkAuthorizationRequestCustomizer 在请求阶段补上。 + client-authentication-method: none + authorization-grant-type: authorization_code provider: feishu: # Full endpoints are configurable for Lark, private deployments, and gateways. @@ -300,12 +308,24 @@ Spring Security OAuth2 Client 原生支持多 Provider 并存，新增 Provider 第 2 步是按 Provider 注册一个 Bean，而不是在某个类里按 `registrationId` 分支。 账号匹配、建号、资料权威和账号守卫都在 `OAuthClaims` 之后共享，Provider 自己不做这些决策。 -如果该 Provider 的 userinfo 响应不是标准的扁平结构（例如飞书用 -`{code, msg, data}` 信封，且以 HTTP 200 返回错误），再额外实现一个 -`ProviderOAuth2UserService`：它声明自己负责哪个 `registrationId`， -接管 userinfo 的加载步骤，其余流程不变。该覆盖运行在 +如果该 Provider 的协议有偏离标准之处，按偏离的环节实现对应的策略接口， +每个接口都声明自己负责哪个 `registrationId`，由框架分发，不需要在共享类里写分支： + +| 偏离环节 | 策略接口 | 现有实现 | +|---|---|---| +| 授权请求参数 | `ProviderAuthorizationRequestCustomizer` | 钉钉补 `openid` scope | +| token 交换 | `ProviderTokenResponseClient` | 钉钉用 JSON body 而非表单 | +| userinfo 加载 | `ProviderOAuth2UserService` | 飞书拆信封；钉钉用自定义 token header | + +以 userinfo 为例：飞书用 `{code, msg, data}` 信封且以 HTTP 200 返回错误， +钉钉则把 token 放在 `x-acs-dingtalk-access-token` 而不是 `Authorization: Bearer`。 +两者都只接管加载步骤，其余流程不变。该覆盖运行在 `RemoteIdentityIoExecutor` 边界内，因此 Provider 的 HTTP 调用不会持有数据库事务。 +Provider 的实现**不得**自己做账号决策 —— 不建号、不绑定、不建 session。 +这些一律交给统一身份核心，否则每个 Provider 都会长出一套账号逻辑， +正是统一身份认证要消除的问题。 + Provider 侧还需遵守：subject 必须稳定（不要用可能在两次登录间变化的字段做 fallback，否则同一个人会被拆成两个平台账号）、只有在 Provider 真正证明了邮箱 所有权时才置 `emailVerified=true`、远程调用要有超时与响应大小上限、 diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 76117217..24ca4121 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -314,11 +314,13 @@ services: 兼容回退。`OAUTH2_FEISHU_TOKEN_URI` 必须指向支持 JSON authorization-code exchange 的 endpoint。`OAUTH2_FEISHU_PROTOCOL_VERSION` 只允许 `v2` 或 `v3`， 默认 `v3`，不会自动 fallback。 + - 钉钉：`OAUTH2_DINGTALK_CLIENT_ID` / `OAUTH2_DINGTALK_CLIENT_SECRET` + （分别填应用的 AppKey 与 AppSecret） - 留空即不展示该入口，无需改配置文件。注意：飞书邮箱由企业管理员导入、未经用户 - 确认，因此 `emailVerified` 恒为 false；若在 `application.yml` 中把 + 留空即不展示该入口，无需改配置文件。注意：飞书和钉钉的邮箱都由企业管理员导入、 + 未经用户确认，因此 `emailVerified` 恒为 false；若在 `application.yml` 中把 `skillhub.access-policy.mode` 设为 `EMAIL_DOMAIN`，该策略会拒绝所有未验证邮箱， - 飞书登录将一律失败。启用飞书时请保留默认的 `OPEN` 或改用其他准入模式。 + 这两个入口的登录将一律失败。启用它们时请保留默认的 `OPEN` 或改用其他准入模式。 启用飞书前，使用一个测试租户完成一次真实回调验收。不要把真实 client secret 写入仓库、报告或聊天记录；只在受控的 `.env.release`、CI Secret 或 Kubernetes diff --git a/docs/skillhub/en/faq.md b/docs/skillhub/en/faq.md index c00abdf8..4db5d8bb 100644 --- a/docs/skillhub/en/faq.md +++ b/docs/skillhub/en/faq.md @@ -197,7 +197,7 @@ A: Login entries are config-driven: `/api/v1/auth/methods` only returns registra So there are two ways to hide one: - Leave the matching environment variable unset (for example, omit `OAUTH2_FEISHU_CLIENT_ID`). No config file change needed. -- Or edit `application.yml` and comment out or delete the relevant registration block (`github`, `gitlab`, `feishu`) under `spring.security.oauth2.client.registration`, along with its `provider` section. Spring Boot then won't create that registration at startup. +- Or edit `application.yml` and comment out or delete the relevant registration block (`github`, `gitlab`, `feishu`, `dingtalk`) under `spring.security.oauth2.client.registration`, along with its `provider` section. Spring Boot then won't create that registration at startup. ## Q: Is SkillHub's security scanning (Skill Scanner) developed in-house by iFLYTEK? What license does it use? diff --git a/docs/skillhub/faq.md b/docs/skillhub/faq.md index 75054ca0..e8a094d9 100644 --- a/docs/skillhub/faq.md +++ b/docs/skillhub/faq.md @@ -199,7 +199,7 @@ A: 登录入口是配置驱动的：`/api/v1/auth/methods` 只返回配置了真 - 留空对应的环境变量即可（例如不设置 `OAUTH2_FEISHU_CLIENT_ID`），无需改动配置文件。 - 或修改 `application.yml`，注释/删除 `spring.security.oauth2.client.registration` - 下对应的注册块（`github`、`gitlab`、`feishu`）以及对应的 `provider` 段， + 下对应的注册块（`github`、`gitlab`、`feishu`、`dingtalk`）以及对应的 `provider` 段， Spring Boot 启动时便不会创建该注册。 ## Q: SkillHub 的安全扫描（Skill Scanner）是讯飞自研的吗？使用什么协议？ diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index e18e2648..83e73a70 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -292,7 +292,7 @@ expect_fail "$invalid_redis_sentinel_check_env" "SKILLHUB_REDIS_SENTINEL_CHECK_S # An OAuth client id without its secret (or vice versa) leaves the provider half-configured: # the login button renders but the exchange fails. Checked for every supported provider. -for provider in GITHUB GITLAB FEISHU; do +for provider in GITHUB GITLAB FEISHU DINGTALK; do missing_oauth_secret_env="$tmp/missing-oauth-secret.env" write_env "$missing_oauth_secret_env" "release-download-secret-32-bytes-minimum" printf 'OAUTH2_%s_CLIENT_ID=real-client-id\n' "$provider" >>"$missing_oauth_secret_env" diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index e9a85899..c7189304 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -380,7 +380,7 @@ if [ "${REDIS_BIND_ADDRESS:-127.0.0.1}" != "127.0.0.1" ]; then warn "REDIS_BIND_ADDRESS is not 127.0.0.1; confirm Redis exposure is intended" fi -for provider in GITHUB GITLAB FEISHU; do +for provider in GITHUB GITLAB FEISHU DINGTALK; do eval "oauth_id=\"\${OAUTH2_${provider}_CLIENT_ID:-}\"" eval "oauth_secret=\"\${OAUTH2_${provider}_CLIENT_SECRET:-}\"" if [ -n "$oauth_id" ] && [ -z "$oauth_secret" ]; then 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 04/11] 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(); + } } From b1f1b18737031f2aae291c17160e29d6e649a98d Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:40:25 +0800 Subject: [PATCH 05/11] fix(auth): stop the DingTalk callback being routed to the OIDC provider The DingTalk login could not complete. Adding openid to the authorization request's scope set avoided the nonce at the authorize step but broke the callback: OAuth2LoginAuthenticationProvider.authenticate returns null when getScopes() contains "openid", handing the exchange to OidcAuthorizationCodeAuthenticationProvider, which fails with invalid_id_token because DingTalk returns no id_token. Neither the token client nor the user service was ever reached. spring-security-oauth2-jose is on the runtime classpath, so that provider is registered. The scope now goes onto the outgoing authorization URI directly, leaving getScopes() empty. Both openid-keyed mechanisms are then avoided: no nonce, because the registration still declares no scope in configuration, and no OIDC routing, because the request carries no openid scope. The previous test asserted getScopes() contains "openid" -- the exact state that breaks the callback -- so it locked the bug in. It now asserts the inverse, and restoring the old implementation makes it fail. Also switches the registration from client-authentication-method: none to client-secret-post. "none" made Spring apply PKCE and emit a code_challenge that DingTalkTokenResponseClient cannot answer, since its JSON token request sends no code_verifier. It was also semantically wrong: DingTalk is a confidential client that carries its secret in the request body. Verified against a local staging instance: the authorization URI now carries scope=openid with no nonce and no code_challenge, and a callback with a fake code fails in the token exchange with no OIDC provider involvement in the logs. Drops SUBJECT_ATTRIBUTE, which lost its last reference when the user service stopped pre-resolving the subject. Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../src/main/resources/application.yml | 7 ++-- ...ingTalkAuthorizationRequestCustomizer.java | 32 +++++++++++++------ .../auth/oauth/DingTalkOAuth2Constants.java | 2 -- .../oauth/DingTalkClaimsExtractorTest.java | 2 +- .../oauth/DingTalkOAuth2UserServiceTest.java | 2 +- ...Auth2AuthorizationRequestResolverTest.java | 22 +++++++++---- 6 files changed, 45 insertions(+), 22 deletions(-) diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 97b724f7..6579da34 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -88,8 +88,11 @@ spring: # nonce, which DingTalk rejects. DingTalkAuthorizationRequestCustomizer adds the # scope back to the outgoing URI without turning this into an OIDC flow. authorization-grant-type: authorization_code - # DingTalk sends credentials in a JSON body, handled by DingTalkTokenResponseClient. - client-authentication-method: none + # DingTalk is a confidential client that happens to carry its secret in a JSON body, + # which DingTalkTokenResponseClient builds. client-secret-post is the honest + # 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}" client-name: ${OAUTH2_DINGTALK_DISPLAY_NAME:钉钉} provider: diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java index aa5494c1..53b9b160 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java @@ -1,17 +1,26 @@ package com.iflytek.skillhub.auth.oauth; -import java.util.LinkedHashSet; -import java.util.Set; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.stereotype.Component; +import org.springframework.web.util.UriComponentsBuilder; /** - * Adds the {@code openid} scope DingTalk's authorize endpoint requires. + * Sends the {@code scope=openid} parameter DingTalk's authorize endpoint requires, without letting + * Spring Security classify the login as OIDC. * - *

The scope cannot simply be declared in {@code application.yml}: Spring Security treats a - * registration carrying {@code openid} as an OIDC client and attaches a {@code nonce} parameter, - * which DingTalk rejects. Adding the scope here keeps the registration a plain OAuth2 client while - * still sending the parameter DingTalk expects. + *

Two separate mechanisms keyed off {@code openid} have to be avoided, which is why the scope is + * written onto the URI rather than into the request's scope set: + * + *

    + *
  • A registration declaring {@code openid} in configuration becomes an OIDC client, and + * {@code DefaultOAuth2AuthorizationRequestResolver} attaches a {@code nonce} that DingTalk + * rejects. Hence no scope in {@code application.yml}. + *
  • {@code OAuth2LoginAuthenticationProvider.authenticate} returns null when the authorization + * request's {@code getScopes()} contains {@code openid}, handing the callback to + * {@code OidcAuthorizationCodeAuthenticationProvider}, which then fails with + * {@code invalid_id_token} because DingTalk returns no {@code id_token}. Hence the scope set + * stays empty and only the outgoing URI carries the parameter. + *
*/ @Component public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthorizationRequestCustomizer { @@ -23,8 +32,11 @@ public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthoriza @Override public void customize(OAuth2AuthorizationRequest.Builder builder) { - Set scopes = new LinkedHashSet<>(builder.build().getScopes()); - scopes.add(DingTalkOAuth2Constants.AUTHORIZATION_SCOPE); - builder.scopes(scopes); + String authorizationRequestUri = UriComponentsBuilder + .fromUriString(builder.build().getAuthorizationRequestUri()) + .replaceQueryParam("scope", DingTalkOAuth2Constants.AUTHORIZATION_SCOPE) + .build(true) + .toUriString(); + builder.authorizationRequestUri(authorizationRequestUri); } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java index 83026e50..2617ab70 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java @@ -16,8 +16,6 @@ public final class DingTalkOAuth2Constants { */ static final String SUBJECT_CLAIM_NAME = "unionId"; - public static final String SUBJECT_ATTRIBUTE = "dingtalkSubject"; - private DingTalkOAuth2Constants() { } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java index 8bf38b18..09a8ca93 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java @@ -103,7 +103,7 @@ class DingTalkClaimsExtractorTest { .clientId("dingoauth_test") .clientSecret("client-secret") .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) - .clientAuthenticationMethod(ClientAuthenticationMethod.NONE) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST) .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") .authorizationUri("https://login.dingtalk.com/oauth2/auth") .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") 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 e5d7027a..50c0ef3e 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 @@ -121,7 +121,7 @@ class DingTalkOAuth2UserServiceTest { .clientId("dingoauth_test") .clientSecret("client-secret") .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) - .clientAuthenticationMethod(ClientAuthenticationMethod.NONE) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST) .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") .authorizationUri("https://login.dingtalk.com/oauth2/auth") .tokenUri("https://api.dingtalk.com/v1.0/oauth2/userAccessToken") diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java index 49cb92c8..5eab1475 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java @@ -86,7 +86,7 @@ class OAuth2AuthorizationRequestResolverTest { } @Test - void resolve_addsDingTalkScopeWithoutTurningTheRequestIntoOidc() { + void resolve_sendsDingTalkScopeOnTheUriButKeepsTheRequestNonOidc() { SkillHubOAuth2AuthorizationRequestResolver dingTalkResolver = resolverFor( dingTalkRegistration(), new DingTalkAuthorizationRequestCustomizer() @@ -97,14 +97,24 @@ class OAuth2AuthorizationRequestResolverTest { var authorizationRequest = dingTalkResolver.resolve(request, "dingtalk"); assertThat(authorizationRequest).isNotNull(); - // DingTalk's authorize endpoint requires scope=openid... - assertThat(authorizationRequest.getScopes()).contains("openid"); + // DingTalk's authorize endpoint requires scope=openid on the wire. assertThat(authorizationRequest.getAuthorizationRequestUri()).contains("scope=openid"); - // ...but rejects the nonce Spring attaches when a registration declares openid in config. - // Declaring no scope there and adding it here is what keeps the nonce away. + + // But getScopes() must stay empty. OAuth2LoginAuthenticationProvider.authenticate returns + // null when the authorization request's scopes contain "openid", which hands the callback to + // OidcAuthorizationCodeAuthenticationProvider; that then fails with invalid_id_token because + // DingTalk returns no id_token, and neither the token client nor the user service is reached. + assertThat(authorizationRequest.getScopes()).doesNotContain("openid"); + + // And no nonce: a registration declaring openid in configuration would get one attached, + // which DingTalk also rejects. assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey("nonce"); assertThat(authorizationRequest.getAttributes()).doesNotContainKey("nonce"); assertThat(authorizationRequest.getAuthorizationRequestUri()).doesNotContain("nonce="); + + // client-secret-post rather than none, so Spring does not apply PKCE. The DingTalk token + // request sends no code_verifier, so a challenge on the authorize URI could not be answered. + assertThat(authorizationRequest.getAuthorizationRequestUri()).doesNotContain("code_challenge"); } @Test @@ -167,7 +177,7 @@ class OAuth2AuthorizationRequestResolverTest { .authorizationGrantType( org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE) .clientAuthenticationMethod( - org.springframework.security.oauth2.core.ClientAuthenticationMethod.NONE) + org.springframework.security.oauth2.core.ClientAuthenticationMethod.CLIENT_SECRET_POST) .clientName("钉钉") .build(); } From d92e1f82167388713877777bd05ecfd0e06d4b4d Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:59:37 +0800 Subject: [PATCH 06/11] fix(auth): preserve provider token routing and error bounds Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skillhub/tests/configuration-contracts.sh | 11 ++++++++++ charts/skillhub/values.schema.json | 2 ++ .../oauth/ProviderStrategyWiringTest.java | 2 +- .../auth/oauth/DingTalkOAuth2UserService.java | 11 +++++++++- .../oauth/DingTalkTokenResponseClient.java | 1 + ...FeishuOAuth2AccessTokenResponseClient.java | 7 +++++- .../oauth/DingTalkOAuth2UserServiceTest.java | 22 +++++++++++++++++++ 7 files changed, 53 insertions(+), 3 deletions(-) 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(); From 1297e87c5af567b95c769e56bc4cb7236a86c650 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:02:43 +0800 Subject: [PATCH 07/11] fix(deploy): complete DingTalk runtime configuration Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.example | 2 ++ .../skillhub/templates/server-deployment.yaml | 10 +++++++++ .../skillhub/tests/configuration-contracts.sh | 10 +++++++++ charts/skillhub/values.schema.json | 13 +++++++++++- charts/skillhub/values.yaml | 5 +++++ compose.release.yml | 1 + deploy/k8s/base/backend-deployment.yaml | 6 ++++++ docs/03-authentication-design.md | 4 +++- docs/09-deployment.md | 7 ++++++- scripts/tests/validate-release-config-test.sh | 21 +++++++++++++++++++ scripts/validate-release-config.sh | 9 ++++++++ .../src/main/resources/application.yml | 2 +- 12 files changed, 86 insertions(+), 4 deletions(-) diff --git a/.env.release.example b/.env.release.example index add0f570..ca295b72 100644 --- a/.env.release.example +++ b/.env.release.example @@ -146,6 +146,8 @@ OAUTH2_DINGTALK_CLIENT_ID= OAUTH2_DINGTALK_CLIENT_SECRET= OAUTH2_DINGTALK_AUTHORIZE_URI=https://login.dingtalk.com OAUTH2_DINGTALK_BASE_URI=https://api.dingtalk.com +# Optional; defaults to {baseUrl}/login/oauth2/code/dingtalk. +OAUTH2_DINGTALK_REDIRECT_URI= OAUTH2_DINGTALK_DISPLAY_NAME=钉钉 # Optional: OIDC login (e.g. Keycloak, Okta, Azure AD). diff --git a/charts/skillhub/templates/server-deployment.yaml b/charts/skillhub/templates/server-deployment.yaml index 244bc4c9..c6b99807 100644 --- a/charts/skillhub/templates/server-deployment.yaml +++ b/charts/skillhub/templates/server-deployment.yaml @@ -394,6 +394,16 @@ spec: name: {{ include "skillhub.secretName" . }} key: oauth2-dingtalk-client-secret optional: true + - name: OAUTH2_DINGTALK_AUTHORIZE_URI + value: {{ .Values.oauth2.dingtalk.authorizeBaseUri | quote }} + - name: OAUTH2_DINGTALK_BASE_URI + value: {{ .Values.oauth2.dingtalk.apiBaseUri | quote }} + {{- with .Values.oauth2.dingtalk.redirectUri }} + - name: OAUTH2_DINGTALK_REDIRECT_URI + value: {{ . | quote }} + {{- end }} + - name: OAUTH2_DINGTALK_DISPLAY_NAME + value: {{ .Values.oauth2.dingtalk.displayName | quote }} {{- if .Values.server.javaOpts }} - name: JAVA_OPTS diff --git a/charts/skillhub/tests/configuration-contracts.sh b/charts/skillhub/tests/configuration-contracts.sh index d255db0a..b1dfbbd3 100755 --- a/charts/skillhub/tests/configuration-contracts.sh +++ b/charts/skillhub/tests/configuration-contracts.sh @@ -42,6 +42,9 @@ grep -A1 -F 'name: SKILLHUB_SUITE_REVIEW_WRITES_ENABLED' "$TMP_DIR/default.yaml" if grep -Fq 'name: OAUTH2_FEISHU_REDIRECT_URI' "$TMP_DIR/default.yaml"; then fail "default Helm rendering must omit an empty Feishu redirect URI so Spring can derive baseUrl" fi +if grep -Fq 'name: OAUTH2_DINGTALK_REDIRECT_URI' "$TMP_DIR/default.yaml"; then + fail "default Helm rendering must omit an empty DingTalk redirect URI so Spring can derive baseUrl" +fi render feishu-redirect "$CHART_DIR" \ --set-string oauth2.feishu.redirectUri=https://skills.example.com/login/oauth2/code/feishu \ @@ -50,6 +53,13 @@ grep -A1 -F 'name: OAUTH2_FEISHU_REDIRECT_URI' "$TMP_DIR/feishu-redirect.yaml" \ | grep -Fq 'value: "https://skills.example.com/login/oauth2/code/feishu"' \ || fail "Helm must inject an explicitly configured Feishu redirect URI" +render dingtalk-redirect "$CHART_DIR" \ + --set-string oauth2.dingtalk.redirectUri=https://skills.example.com/login/oauth2/code/dingtalk \ + --show-only templates/server-deployment.yaml >"$TMP_DIR/dingtalk-redirect.yaml" +grep -A1 -F 'name: OAUTH2_DINGTALK_REDIRECT_URI' "$TMP_DIR/dingtalk-redirect.yaml" \ + | grep -Fq 'value: "https://skills.example.com/login/oauth2/code/dingtalk"' \ + || fail "Helm must inject an explicitly configured DingTalk redirect URI" + render suite-review-enabled "$CHART_DIR" \ --set server.suiteReviewWritesEnabled=true \ --show-only templates/server-deployment.yaml >"$TMP_DIR/suite-review-enabled.yaml" diff --git a/charts/skillhub/values.schema.json b/charts/skillhub/values.schema.json index 6d3bf510..83b76ac5 100644 --- a/charts/skillhub/values.schema.json +++ b/charts/skillhub/values.schema.json @@ -37,7 +37,7 @@ "oauth2": { "type": "object", "additionalProperties": false, - "required": ["feishu"], + "required": ["feishu", "dingtalk"], "properties": { "feishu": { "type": "object", @@ -50,6 +50,17 @@ "userInfoUri": { "type": "string", "format": "uri" }, "redirectUri": { "type": "string" } } + }, + "dingtalk": { + "type": "object", + "additionalProperties": false, + "required": ["authorizeBaseUri", "apiBaseUri", "redirectUri", "displayName"], + "properties": { + "authorizeBaseUri": { "type": "string", "format": "uri" }, + "apiBaseUri": { "type": "string", "format": "uri" }, + "redirectUri": { "type": "string" }, + "displayName": { "type": "string", "minLength": 1 } + } } } }, diff --git a/charts/skillhub/values.yaml b/charts/skillhub/values.yaml index 555ace85..5e38e21a 100644 --- a/charts/skillhub/values.yaml +++ b/charts/skillhub/values.yaml @@ -29,6 +29,11 @@ oauth2: tokenUri: https://accounts.feishu.cn/oauth/v3/token userInfoUri: https://open.feishu.cn/open-apis/authen/v1/user_info redirectUri: "" + dingtalk: + authorizeBaseUri: https://login.dingtalk.com + apiBaseUri: https://api.dingtalk.com + redirectUri: "" + displayName: 钉钉 builtinSkills: enabled: true diff --git a/compose.release.yml b/compose.release.yml index 77cc571a..bb55a8f1 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -132,6 +132,7 @@ services: OAUTH2_DINGTALK_CLIENT_SECRET: ${OAUTH2_DINGTALK_CLIENT_SECRET:-local-placeholder} OAUTH2_DINGTALK_AUTHORIZE_URI: ${OAUTH2_DINGTALK_AUTHORIZE_URI:-https://login.dingtalk.com} OAUTH2_DINGTALK_BASE_URI: ${OAUTH2_DINGTALK_BASE_URI:-https://api.dingtalk.com} + OAUTH2_DINGTALK_REDIRECT_URI: ${OAUTH2_DINGTALK_REDIRECT_URI:-${SKILLHUB_PUBLIC_BASE_URL:-http://localhost}/login/oauth2/code/dingtalk} OAUTH2_DINGTALK_DISPLAY_NAME: ${OAUTH2_DINGTALK_DISPLAY_NAME:-钉钉} SPRING_MAIL_HOST: ${SPRING_MAIL_HOST:-} SPRING_MAIL_PORT: ${SPRING_MAIL_PORT:-25} diff --git a/deploy/k8s/base/backend-deployment.yaml b/deploy/k8s/base/backend-deployment.yaml index 01319d16..a7a39648 100644 --- a/deploy/k8s/base/backend-deployment.yaml +++ b/deploy/k8s/base/backend-deployment.yaml @@ -262,6 +262,12 @@ spec: name: skillhub-secret key: oauth2-dingtalk-client-secret optional: true + - name: OAUTH2_DINGTALK_AUTHORIZE_URI + value: "https://login.dingtalk.com" + - name: OAUTH2_DINGTALK_BASE_URI + value: "https://api.dingtalk.com" + - name: OAUTH2_DINGTALK_DISPLAY_NAME + value: "钉钉" volumeMounts: - name: skillhub-storage mountPath: /var/lib/skillhub/storage diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 478e3420..1d264616 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -288,7 +288,9 @@ spring: # 故意不声明 scope：钉钉的授权端点要 scope=openid，但在这里声明会让 # Spring 把该注册当成 OIDC 客户端并附加 nonce，而钉钉不接受 nonce。 # scope 由 DingTalkAuthorizationRequestCustomizer 在请求阶段补上。 - client-authentication-method: none + # 钉钉是 confidential client，只是由自定义 token client 把 secret 放进 JSON body。 + # 不使用 none，避免 Spring 自动添加本实现无法应答的 PKCE challenge。 + client-authentication-method: client_secret_post authorization-grant-type: authorization_code provider: feishu: diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 24ca4121..6b7d30b2 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -315,7 +315,12 @@ services: exchange 的 endpoint。`OAUTH2_FEISHU_PROTOCOL_VERSION` 只允许 `v2` 或 `v3`， 默认 `v3`，不会自动 fallback。 - 钉钉：`OAUTH2_DINGTALK_CLIENT_ID` / `OAUTH2_DINGTALK_CLIENT_SECRET` - （分别填应用的 AppKey 与 AppSecret） + （分别填应用的 AppKey 与 AppSecret）。在钉钉开发者后台登记 + `https://<公网域名>/login/oauth2/code/dingtalk`，并为用户信息接口开通所需权限。 + `OAUTH2_DINGTALK_REDIRECT_URI` 可在动态端口或特殊反向代理场景显式覆盖；Compose + 默认根据 `SKILLHUB_PUBLIC_BASE_URL` 生成回调，Helm/K8s 未设置时由 Spring 使用 + `{baseUrl}`。国际版或网关场景可覆盖 `OAUTH2_DINGTALK_AUTHORIZE_URI` 与 + `OAUTH2_DINGTALK_BASE_URI`。 留空即不展示该入口，无需改配置文件。注意：飞书和钉钉的邮箱都由企业管理员导入、 未经用户确认，因此 `emailVerified` 恒为 false；若在 `application.yml` 中把 diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index 83e73a70..c241e308 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/scripts/tests/validate-release-config-test.sh @@ -107,6 +107,27 @@ write_env "$invalid_feishu_redirect_env" "release-download-secret-32-bytes-minim printf '%s\n' "OAUTH2_FEISHU_REDIRECT_URI=https://skillhub.example.com/login/oauth2/code/feishu?bad=1" >>"$invalid_feishu_redirect_env" expect_fail "$invalid_feishu_redirect_env" "OAUTH2_FEISHU_REDIRECT_URI must not contain a query" +valid_dingtalk_env="$tmp/valid-dingtalk.env" +write_env "$valid_dingtalk_env" "release-download-secret-32-bytes-minimum" +cat >>"$valid_dingtalk_env" <<'EOF' +OAUTH2_DINGTALK_CLIENT_ID=ding-test +OAUTH2_DINGTALK_CLIENT_SECRET=dingtalk-test-secret +OAUTH2_DINGTALK_AUTHORIZE_URI=https://login.dingtalk.com +OAUTH2_DINGTALK_BASE_URI=https://api.dingtalk.com +OAUTH2_DINGTALK_REDIRECT_URI=https://skillhub.example.com/login/oauth2/code/dingtalk +EOF +"$SCRIPT" "$valid_dingtalk_env" >/dev/null + +invalid_dingtalk_base_env="$tmp/invalid-dingtalk-base.env" +write_env "$invalid_dingtalk_base_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' "OAUTH2_DINGTALK_BASE_URI=https://api.dingtalk.com/" >>"$invalid_dingtalk_base_env" +expect_fail "$invalid_dingtalk_base_env" "OAUTH2_DINGTALK_BASE_URI must not have a trailing slash" + +invalid_dingtalk_redirect_env="$tmp/invalid-dingtalk-redirect.env" +write_env "$invalid_dingtalk_redirect_env" "release-download-secret-32-bytes-minimum" +printf '%s\n' "OAUTH2_DINGTALK_REDIRECT_URI=https://skillhub.example.com/callback?bad=1" >>"$invalid_dingtalk_redirect_env" +expect_fail "$invalid_dingtalk_redirect_env" "OAUTH2_DINGTALK_REDIRECT_URI must not contain a query" + disabled_builtin_skills_env="$tmp/disabled-builtin-skills.env" write_env "$disabled_builtin_skills_env" "release-download-secret-32-bytes-minimum" printf '%s\n' "SKILLHUB_BUILTIN_SKILLS_ENABLED=false" >>"$disabled_builtin_skills_env" diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index c7189304..356d241d 100755 --- a/scripts/validate-release-config.sh +++ b/scripts/validate-release-config.sh @@ -406,6 +406,15 @@ for feishu_endpoint in OAUTH2_FEISHU_AUTHORIZATION_URI OAUTH2_FEISHU_TOKEN_URI O fi done +for dingtalk_endpoint in OAUTH2_DINGTALK_AUTHORIZE_URI OAUTH2_DINGTALK_BASE_URI OAUTH2_DINGTALK_REDIRECT_URI; do + eval "dingtalk_endpoint_value=\${$dingtalk_endpoint:-}" + if [ -n "$dingtalk_endpoint_value" ]; then + validate_url "$dingtalk_endpoint" + fi +done +validate_no_trailing_slash OAUTH2_DINGTALK_AUTHORIZE_URI +validate_no_trailing_slash OAUTH2_DINGTALK_BASE_URI + if [ "$errors" -gt 0 ]; then echo "Release config validation failed: $errors error(s), $warnings warning(s)." >&2 exit 1 diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 6579da34..7e580256 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -58,7 +58,7 @@ spring: scope: - read:user - user:email - redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}" + redirect-uri: "${OAUTH2_DINGTALK_REDIRECT_URI:{baseUrl}/login/oauth2/code/{registrationId}}" client-name: GitHub authorization-grant-type: authorization_code gitlab: From ca4de37d0812faa39587a22cc351b96db366c12f Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:08:32 +0800 Subject: [PATCH 08/11] docs(deploy): add DingTalk provider acceptance steps Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/09-deployment.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 6b7d30b2..f13d752f 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -358,6 +358,23 @@ services: 本地 mock 回调只能证明 SkillHub 与协议形状的集成，不能替代上述真实租户验收。 没有可用飞书租户时，应将该项记录为“未验证”，不要宣称 Feishu 登录已通过。 + + 钉钉登录使用同样的验收边界，但协议配置不同：在钉钉开发者后台创建企业内部 + H5 微应用，使用应用的 AppKey/AppSecret，进入“钉钉登录与分享”登记 + `https://<公网域名>/login/oauth2/code/dingtalk`，并开通个人信息读取权限。 + 验收前设置： + + ```dotenv + OAUTH2_DINGTALK_CLIENT_ID= + OAUTH2_DINGTALK_CLIENT_SECRET= + OAUTH2_DINGTALK_REDIRECT_URI=https://<公网域名>/login/oauth2/code/dingtalk + ``` + + 登录请求必须包含 `scope=openid`，但配置文件不能声明 `openid` scope；实现会把 + 它仅写入外发授权 URL，避免 Spring 将回调路由到 OIDC。验收时应确认 token 请求为 + JSON body，userinfo 请求使用 `x-acs-dingtalk-access-token`，重复登录仍绑定同一 + `unionId`，且日志不出现 AppSecret、access token、unionId 或上游错误 body。 + 没有钉钉测试应用凭据时，这些只能标记为“协议测试通过、真实厂商往返未验证”。 - 如果要启用密码重置验证码邮件，参见：`docs/19-smtp-password-reset-email-setup.md` ## 8 OIDC 登录配置 From 36f5f06d9c0a493ee75ad8b572fa5437a93ac74b Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:53:48 +0800 Subject: [PATCH 09/11] fix(auth): diagnose DingTalk userinfo failures Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../src/main/resources/application.yml | 4 +- ...ingTalkAuthorizationRequestCustomizer.java | 1 + .../auth/oauth/DingTalkOAuth2UserService.java | 95 ++++++++++++++++++- .../oauth/DingTalkOAuth2UserServiceTest.java | 50 ++++++++++ ...Auth2AuthorizationRequestResolverTest.java | 1 + 5 files changed, 147 insertions(+), 4 deletions(-) diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 7e580256..6dfbbd42 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -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: diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java index 53b9b160..01ea237a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java @@ -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); 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 cb031985..6528b2ab 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 @@ -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 names) { + if (node.isObject()) { + Iterator> fields = node.fields(); + while (fields.hasNext()) { + Map.Entry 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 textValues(JsonNode array) { + List 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 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 800fe73f..2abc368f 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 @@ -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 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(); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java index 5eab1475..a371cf7d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java @@ -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 From 00033b1b92abcc25573bb4d3d591173a41399114 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:40:23 +0800 Subject: [PATCH 10/11] docs(deploy): document DingTalk egress requirements Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/09-deployment.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/09-deployment.md b/docs/09-deployment.md index f13d752f..e36f665c 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -317,6 +317,10 @@ services: - 钉钉：`OAUTH2_DINGTALK_CLIENT_ID` / `OAUTH2_DINGTALK_CLIENT_SECRET` （分别填应用的 AppKey 与 AppSecret）。在钉钉开发者后台登记 `https://<公网域名>/login/oauth2/code/dingtalk`，并为用户信息接口开通所需权限。 + 同时将钉钉开发者后台的“服务器出口 IP”配置为实际运行 SkillHub 后端并调用 + DingTalk API 的机器公网 IP；仅将回调域名或反向隧道服务器 IP 加入白名单并不能 + 改变本地后端的出站 IP。使用 SSH 反向隧道做本地预览时，应临时加入本机出站 IP， + 或让后端出站流量经过已加入白名单的服务器；生产环境应只配置生产后端的固定出口 IP。 `OAUTH2_DINGTALK_REDIRECT_URI` 可在动态端口或特殊反向代理场景显式覆盖；Compose 默认根据 `SKILLHUB_PUBLIC_BASE_URL` 生成回调，Helm/K8s 未设置时由 Spring 使用 `{baseUrl}`。国际版或网关场景可覆盖 `OAUTH2_DINGTALK_AUTHORIZE_URI` 与 @@ -370,10 +374,11 @@ services: OAUTH2_DINGTALK_REDIRECT_URI=https://<公网域名>/login/oauth2/code/dingtalk ``` - 登录请求必须包含 `scope=openid`，但配置文件不能声明 `openid` scope；实现会把 - 它仅写入外发授权 URL，避免 Spring 将回调路由到 OIDC。验收时应确认 token 请求为 - JSON body，userinfo 请求使用 `x-acs-dingtalk-access-token`，重复登录仍绑定同一 - `unionId`，且日志不出现 AppSecret、access token、unionId 或上游错误 body。 + 登录请求必须包含 `scope=openid` 和 `prompt=consent`，但配置文件不能声明 `openid` + scope；实现会把它们仅写入外发授权 URL，避免 Spring 将回调路由到 OIDC。验收时应 + 确认 token 请求为 JSON body，userinfo 请求使用 `x-acs-dingtalk-access-token`，重复 + 登录仍绑定同一 `unionId`。上游失败时日志只记录 HTTP 状态、错误码、requiredScopes + 和 requestId，不记录 AppSecret、authorization code、access token、unionId 或完整错误正文。 没有钉钉测试应用凭据时，这些只能标记为“协议测试通过、真实厂商往返未验证”。 - 如果要启用密码重置验证码邮件，参见：`docs/19-smtp-password-reset-email-setup.md` From 50e7427596d7464fe1c35fda5599454de476e5a0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:20:24 +0800 Subject: [PATCH 11/11] docs(deploy): complete DingTalk private deployment guide Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.example | 2 + charts/skillhub/README.md | 2 + docs/03-authentication-design.md | 5 +- docs/09-deployment.md | 125 +++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 2 deletions(-) diff --git a/.env.release.example b/.env.release.example index ca295b72..505c594a 100644 --- a/.env.release.example +++ b/.env.release.example @@ -142,6 +142,8 @@ OAUTH2_FEISHU_DISPLAY_NAME=飞书 # button off the login page. Use the app's AppKey as the client id and AppSecret as the secret. # Like Feishu, DingTalk returns an organization-recorded email without attesting ownership, so # emailVerified is always false and the EMAIL_DOMAIN access policy would reject every login. +# The DingTalk console's server egress IP must be the real public IP of the backend calling +# api.dingtalk.com. A reverse tunnel only changes callback ingress and does not change egress. OAUTH2_DINGTALK_CLIENT_ID= OAUTH2_DINGTALK_CLIENT_SECRET= OAUTH2_DINGTALK_AUTHORIZE_URI=https://login.dingtalk.com diff --git a/charts/skillhub/README.md b/charts/skillhub/README.md index 1e8bc1a3..d9ffaa59 100644 --- a/charts/skillhub/README.md +++ b/charts/skillhub/README.md @@ -110,6 +110,8 @@ helm -n skillhub upgrade -i skillhub ./charts/skillhub \ | `skillhub-download-anon-cookie-secret` | 是 | 至少 32 字符的匿名下载 Cookie 签名密钥 | | `oauth2-github-client-id` | 否 | GitHub OAuth2 Client ID | | `oauth2-github-client-secret` | 否 | GitHub OAuth2 Client Secret | +| `oauth2-dingtalk-client-id` | 否 | DingTalk AppKey | +| `oauth2-dingtalk-client-secret` | 否 | DingTalk AppSecret | | `skill-scanner-llm-api-key` | 否 | Scanner LLM API Key | | `skill-scanner-llm-base-url` | 否 | Scanner 自定义 LLM API 地址 | | `skill-scanner-llm-model` | 否 | Scanner LLM 模型名称 | diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 1d264616..19f6140f 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -287,7 +287,8 @@ spring: client-secret: ${OAUTH2_DINGTALK_CLIENT_SECRET} # 故意不声明 scope：钉钉的授权端点要 scope=openid，但在这里声明会让 # Spring 把该注册当成 OIDC 客户端并附加 nonce，而钉钉不接受 nonce。 - # scope 由 DingTalkAuthorizationRequestCustomizer 在请求阶段补上。 + # scope=openid 与 prompt=consent 由 DingTalkAuthorizationRequestCustomizer + # 在请求阶段补上。 # 钉钉是 confidential client，只是由自定义 token client 把 secret 放进 JSON body。 # 不使用 none，避免 Spring 自动添加本实现无法应答的 PKCE challenge。 client-authentication-method: client_secret_post @@ -315,7 +316,7 @@ Spring Security OAuth2 Client 原生支持多 Provider 并存，新增 Provider | 偏离环节 | 策略接口 | 现有实现 | |---|---|---| -| 授权请求参数 | `ProviderAuthorizationRequestCustomizer` | 钉钉补 `openid` scope | +| 授权请求参数 | `ProviderAuthorizationRequestCustomizer` | 钉钉补 `scope=openid` 与 `prompt=consent` | | token 交换 | `ProviderTokenResponseClient` | 钉钉用 JSON body 而非表单 | | userinfo 加载 | `ProviderOAuth2UserService` | 飞书拆信封；钉钉用自定义 token header | diff --git a/docs/09-deployment.md b/docs/09-deployment.md index e36f665c..452f442b 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -380,6 +380,131 @@ services: 登录仍绑定同一 `unionId`。上游失败时日志只记录 HTTP 状态、错误码、requiredScopes 和 requestId，不记录 AppSecret、authorization code、access token、unionId 或完整错误正文。 没有钉钉测试应用凭据时，这些只能标记为“协议测试通过、真实厂商往返未验证”。 + +### 7.1 钉钉配置示例 + +以下示例中的 `AppKey`、`AppSecret`、公网地址和出口 IP 都必须替换为部署环境的真实值。 +不要把 `AppSecret` 提交到 Git、镜像或 HTML 报告。 + +#### Docker Compose release + +在受保护的 `.env.release` 中设置： + +```dotenv +# 浏览器访问地址，不带末尾斜杠 +SKILLHUB_PUBLIC_BASE_URL=https://skills.example.com +SESSION_COOKIE_SECURE=true + +# 钉钉企业内部 H5 微应用 +OAUTH2_DINGTALK_CLIENT_ID=dingxxxxxxxx +OAUTH2_DINGTALK_CLIENT_SECRET=<从密钥管理系统注入> +OAUTH2_DINGTALK_REDIRECT_URI=https://skills.example.com/login/oauth2/code/dingtalk +OAUTH2_DINGTALK_AUTHORIZE_URI=https://login.dingtalk.com +OAUTH2_DINGTALK_BASE_URI=https://api.dingtalk.com +OAUTH2_DINGTALK_DISPLAY_NAME=钉钉 +``` + +启动和检查： + +```bash +make validate-release-config +docker compose --env-file .env.release -f compose.release.yml up -d +curl -fsS http://127.0.0.1:8080/actuator/health +curl -fsS http://127.0.0.1:8080/api/v1/auth/methods +``` + +钉钉后台必须同时配置： + +1. “钉钉登录与分享”回调 URL：与 `OAUTH2_DINGTALK_REDIRECT_URI` 完全一致。 +2. `Contact.User.Read` 个人信息读取权限，并将应用发布到当前版本。 +3. 服务器出口 IP：填写运行 SkillHub 后端并访问 `api.dingtalk.com` 的真实公网出口。 +4. 测试账号必须属于应用所属组织，并在应用可用范围内。 + +#### Helm 私有化部署 + +推荐使用 Kubernetes Secret，不把密钥写入 `values-production.yaml`： + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: skillhub-production-secret + namespace: skillhub +type: Opaque +stringData: + bootstrap-admin-password: "<固定随机密码>" + skillhub-download-anon-cookie-secret: "<至少32字符随机值>" + oauth2-dingtalk-client-id: "dingxxxxxxxx" + oauth2-dingtalk-client-secret: "<从密钥管理系统注入>" +``` + +`values-production.yaml` 只放非敏感配置： + +```yaml +images: + registry: ghcr.io/iflytek + tag: <固定发布版本> + pullPolicy: IfNotPresent +publicBaseUrl: https://skills.example.com +session: + cookieSecure: true +ingress: + enabled: true + className: nginx + hosts: + - host: skills.example.com + paths: + - path: / + pathType: Prefix + tls: + - hosts: + - skills.example.com + secretName: skillhub-tls +oauth2: + dingtalk: + authorizeBaseUri: https://login.dingtalk.com + apiBaseUri: https://api.dingtalk.com + redirectUri: https://skills.example.com/login/oauth2/code/dingtalk + displayName: 钉钉 +``` + +安装或升级： + +```bash +kubectl create namespace skillhub --dry-run=client -o yaml | kubectl apply -f - +kubectl apply -f skillhub-production-secret.yaml +helm upgrade --install skillhub ./charts/skillhub \ + --namespace skillhub \ + -f values-production.yaml \ + --set existingSecret=skillhub-production-secret +``` + +如果使用 Chart 自己创建 Secret，也可以在受保护的 values 文件中设置 +`secrets.oauth2DingtalkClientId` 和 `secrets.oauth2DingtalkClientSecret`；生产环境优先使用 +External Secrets、Sealed Secrets 或其他密钥注入方案。 + +#### 原生 Kubernetes/Kustomize + +在 `deploy/k8s/base/secret.yaml.example` 对应的 Secret 中提供： + +```yaml +stringData: + oauth2-dingtalk-client-id: dingxxxxxxxx + oauth2-dingtalk-client-secret: "<从密钥管理系统注入>" +``` + +再通过环境变量或 overlay 设置公开地址和回调： + +```yaml +env: + - name: SKILLHUB_PUBLIC_BASE_URL + value: https://skills.example.com + - name: OAUTH2_DINGTALK_REDIRECT_URI + value: https://skills.example.com/login/oauth2/code/dingtalk +``` + +Kubernetes 集群节点或出口网关的公网 IP 必须加入钉钉服务器出口 IP 白名单。Ingress 只负责浏览器 +回调可达性，不会替代后端出站 IP 白名单。 - 如果要启用密码重置验证码邮件，参见：`docs/19-smtp-password-reset-email-setup.md` ## 8 OIDC 登录配置