scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-10-05 02:41:49 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fix(auth): stop the DingTalk callback being routed to the OIDC provider

Browse source 
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>
This commit is contained in:
XiaoSeS 2026-09-18 17:40:25 +08:00
parent 629c1ced55
commit b1f1b18737
6 changed files with 45 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
server/skillhub-app/src/main/resources/application.yml
View file

@ -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:

32
server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkAuthorizationRequestCustomizer.java
View file

@ -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.
*
* <p>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.
* <p>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:
*
* <ul>
* <li>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}.
* <li>{@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.
* </ul>
*/
@Component
public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthorizationRequestCustomizer {
@ -23,8 +32,11 @@ public class DingTalkAuthorizationRequestCustomizer implements ProviderAuthoriza
@Override
public void customize(OAuth2AuthorizationRequest.Builder builder) {
Set<String> 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);
}
}

2
server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2Constants.java
View file

@ -16,8 +16,6 @@ public final class DingTalkOAuth2Constants {
*/
static final String SUBJECT_CLAIM_NAME = "unionId";
public static final String SUBJECT_ATTRIBUTE = "dingtalkSubject";
private DingTalkOAuth2Constants() {
}
}

2
server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkClaimsExtractorTest.java
View file

@ -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")

2
server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/DingTalkOAuth2UserServiceTest.java
View file

@ -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")

22
server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java
View file

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