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