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] 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: + * + *

*/ @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(); }