Skip to content

feat(auth): add DingTalk OAuth2 login support#467

Open
konglong87 wants to merge 7 commits into
iflytek:mainfrom
konglong87:feature/dingtalk-oauth2
Open

feat(auth): add DingTalk OAuth2 login support#467
konglong87 wants to merge 7 commits into
iflytek:mainfrom
konglong87:feature/dingtalk-oauth2

Conversation

@konglong87
Copy link
Copy Markdown

Add DingTalk (钉钉) OAuth2 login support to SkillHub.

DingTalk uses a non-standard OAuth2 flow:

  • Token exchange: JSON body instead of form-urlencoded
  • User info: Custom header x-acs-dingtalk-access-token instead of Authorization: Bearer

This PR leverages the existing OAuthClaimsExtractor strategy pattern, adding three provider-specific components:

┌─────────────────────────────┬────────────────────────────────────────────────────────────────────────┐
│ New file │ Purpose │
├─────────────────────────────┼────────────────────────────────────────────────────────────────────────┤
│ DingTalkClaimsExtractor │ Maps DingTalk fields (openId, nick, unionId) to normalized OAuthClaims │
├─────────────────────────────┼────────────────────────────────────────────────────────────────────────┤
│ DingTalkTokenResponseClient │ Handles DingTalk JSON token exchange │
├─────────────────────────────┼────────────────────────────────────────────────────────────────────────┤
│ DingTalkOAuth2UserService │ Fetches user info via DingTalk custom header │
└─────────────────────────────┴────────────────────────────────────────────────────────────────────────┘

SecurityConfig uses delegating wrappers to route registrationId=dingtalk to these custom components, preserving standard behavior for GitHub, GitLab, and OIDC.

Files changed (7 total)

New (4):

  • DingTalkClaimsExtractor.java
  • DingTalkTokenResponseClient.java
  • DingTalkOAuth2UserService.java
  • dingtalk-logo.svg

Modified (3):

  • SecurityConfig.java — added delegating token/user service routing
  • application.yml — added DingTalk OAuth2 registration (env vars, no hardcoded secrets)
  • .env.release.example — added OAUTH2_DINGTALK_CLIENT_ID/SECRET env vars

Zero-change components

No changes needed to:

  • OAuthLoginFlowService — auto-discovers all OAuthClaimsExtractor beans
  • IdentityBindingService — handles any providerCode generically
  • AuthMethodCatalog — dynamically lists all OAuth registrations
  • LoginButton (frontend) — dynamically renders all OAUTH_REDIRECT methods with provider logos

Configuration

Set these environment variables to enable DingTalk login:
OAUTH2_DINGTALK_CLIENT_ID=
OAUTH2_DINGTALK_CLIENT_SECRET=

Register your app at https://open-dev.dingtalk.com/ and request the Contact.User.Read scope.

Test plan

  • Verify DingTalk OAuth2 login flow works end-to-end
  • Verify existing GitHub/GitLab/OIDC login still works
  • Verify DingTalk login button appears on login page
  • Verify user provisioning via IdentityBindingService.bindOrCreate()
  • Verify access policy evaluation for DingTalk provider

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

DingTalk (钉钉) uses a non-standard OAuth2 flow that requires:
- JSON body for token exchange (instead of form-urlencoded)
- Custom header (x-acs-dingtalk-access-token) for user info requests

This PR integrates DingTalk by leveraging the existing OAuthClaimsExtractor
strategy pattern, adding three provider-specific components:

- DingTalkClaimsExtractor: maps DingTalk user fields to normalized OAuthClaims
- DingTalkTokenResponseClient: handles DingTalk's JSON token exchange
- DingTalkOAuth2UserService: fetches user info via DingTalk's custom header

SecurityConfig uses delegating wrappers to route DingTalk requests to
these custom components while preserving standard behavior for all other
providers (GitHub, GitLab, OIDC).

No changes needed to OAuthLoginFlowService, IdentityBindingService,
AuthMethodCatalog, or frontend LoginButton — all are provider-agnostic.
…ess policy

DingTalkOAuth2UserService was directly calling IdentityBindingService.bindOrCreate(),
bypassing access policy evaluation. Refactored to delegate to
OAuthLoginFlowService.authenticate() for consistent policy + binding,
matching the pattern used by CustomOAuth2UserService.

Also aligned the returned DefaultOAuth2User structure (providerLogin
attribute key, authorities from platformRoles) with CustomOAuth2UserService
so OAuth2LoginSuccessHandler works uniformly across all providers.
@konglong87 konglong87 force-pushed the feature/dingtalk-oauth2 branch from a1dd34c to f7370c8 Compare May 28, 2026 07:04
…ring Data JPA proxy

Package-private JPA repository interfaces cannot be proxied by Spring Data's
JpaRepositoryFactory when using Spring Boot devtools or certain class loader
configurations, causing UnsatisfiedDependencyException at startup.
@konglong87
Copy link
Copy Markdown
Author

I have completed the CLA signing. Please re-run the CLA check, thanks.

@dongmucat
Copy link
Copy Markdown
Collaborator

感谢 @konglong87 的贡献!钉钉登录集成对企业用户很有价值。整体实现思路清晰,正确处理了钉钉的非标准 OAuth2 流程(JSON token 交换 + 自定义 header)。

我们进行了多视角代码审查,发现了一些需要修复的问题。其中有 2 个阻断性问题必须在合并前解决


🚨 必须修复

1. 身份标识选择错误(Critical)

问题DingTalkClaimsExtractor 使用 openId 作为 subject,但 openId 仅在单个钉钉应用内唯一。如果用户通过不同钉钉应用登录,会被识别为不同账号。

修复:使用 unionId(在同一开发者的所有应用中唯一):

// DingTalkClaimsExtractor.java
String unionId = (String) attrs.get("unionId");
String openId = (String) attrs.get("openId");

if (unionId == null || unionId.isEmpty()) {
    throw new OAuth2AuthenticationException(
        new OAuth2Error("missing_union_id", 
            "DingTalk response missing required unionId field", null));
}

return new OAuthClaims(
    "dingtalk",
    unionId,  // 使用 unionId 而非 openId
    syntheticEmail,
    true,
    nick,
    attrs
);

参考:钉钉文档 - unionId 说明


2. Scope 配置错误(Critical)

问题application.ymlscope: dingtalk 不是有效值,钉钉支持的 scope 是 openidopenid corpid

修复

# application.yml
dingtalk:
  scope:
    - openid  # 或 "openid corpid" 如果需要企业信息

同步修改 .env.release.example 中的注释说明。


⚠️ 强烈建议修复

3. RestTemplate 缺少超时配置

两处 new RestTemplate() 未配置超时,钉钉 API 无响应时会导致线程阻塞。建议参考项目中 CasTicketValidator 的做法:

private RestTemplate buildRestTemplate() {
    var factory = new SimpleClientHttpRequestFactory();
    factory.setConnectTimeout(Duration.ofSeconds(5));
    factory.setReadTimeout(Duration.ofSeconds(10));
    return new RestTemplate(factory);
}

4. NPE 风险

DingTalkTokenResponseClient.java:65 行:

// 当前
String accessToken = json.get("accessToken").asText();  // get() 可能返回 null

// 建议
JsonNode accessTokenNode = json.get("accessToken");
if (accessTokenNode == null || accessTokenNode.isNull()) {
    throw new OAuth2AuthenticationException(
        new OAuth2Error("token_response_missing_field", ...));
}
String accessToken = accessTokenNode.asText();

5. 敏感信息泄露

additionalParameters 中的 raw_response 包含完整 token 响应(可能含 refreshToken),建议移除或只保留必要字段(如 expireIn)。


📝 其他建议(可后续 PR)

  • 测试覆盖:3 个新类缺少单元测试,建议补充(可参考 CasTicketValidatorTest 的 mock 模式)
  • 硬编码 URLDingTalkOAuth2UserService 中的 user-info URL 应从 userRequest.getClientRegistration().getProviderDetails().getUserInfoEndpoint().getUri() 读取
  • HTTP 错误处理DingTalkOAuth2UserService.loadUser() 中的 restTemplate.exchange() 应捕获异常并包装为 OAuth2AuthenticationException
  • 不相关变更SkillStorageDeletionCompensationJpaRepository 的可见性修改应独立提交或在 commit message 中说明原因

📚 部署文档补充

建议在文档中说明钉钉应用配置要求:

  1. 需在钉钉开放平台开启 Contact.User.Read 权限
  2. 需发布应用版本才能激活凭证
  3. 回调地址配置为 {baseUrl}/login/oauth2/code/dingtalk

修复前 2 个阻断性问题后我们会立即 review。如有疑问欢迎随时讨论!🙌

1. 身份标识: openId → unionId (跨应用唯一)
   - DingTalkClaimsExtractor: subject使用unionId, 新增null校验
   - DingTalkOAuth2UserService: nameAttribute改为unionId
   - application.yml: user-name-attribute改为unionId

2. Scope配置: dingtalk → openid (钉钉OAuth2正确scope)
   - application.yml: scope改为openid
   - .env.release.example: 新增scope说明注释

3. RestTemplate超时配置 (5s connect + 10s read)
   - DingTalkTokenResponseClient: 防止无限阻塞
   - DingTalkOAuth2UserService: 同步配置

4. NPE风险修复
   - DingTalkTokenResponseClient: JsonNode null检查

5. 敏感信息泄露修复
   - DingTalkTokenResponseClient: 移除raw_response, 只保留expireIn

6. 硬编码URL修复
   - DingTalkOAuth2UserService: userInfoUri从ClientRegistration配置读取

7. 单元测试覆盖 (12个测试全部通过)
   - DingTalkClaimsExtractorTest: 4个
   - DingTalkTokenResponseClientTest: 6个
   - DingTalkOAuth2UserServiceTest: 2个
@konglong87
Copy link
Copy Markdown
Author

already fix,please check it.

感谢 @konglong87 的贡献!钉钉登录集成对企业用户很有价值。整体实现思路清晰,正确处理了钉钉的非标准 OAuth2 流程(JSON token 交换 + 自定义 header)。

我们进行了多视角代码审查,发现了一些需要修复的问题。其中有 2 个阻断性问题必须在合并前解决

🚨 必须修复

1. 身份标识选择错误(Critical)

问题DingTalkClaimsExtractor 使用 openId 作为 subject,但 openId 仅在单个钉钉应用内唯一。如果用户通过不同钉钉应用登录,会被识别为不同账号。

修复:使用 unionId(在同一开发者的所有应用中唯一):

// DingTalkClaimsExtractor.java
String unionId = (String) attrs.get("unionId");
String openId = (String) attrs.get("openId");

if (unionId == null || unionId.isEmpty()) {
    throw new OAuth2AuthenticationException(
        new OAuth2Error("missing_union_id", 
            "DingTalk response missing required unionId field", null));
}

return new OAuthClaims(
    "dingtalk",
    unionId,  // 使用 unionId 而非 openId
    syntheticEmail,
    true,
    nick,
    attrs
);

参考:钉钉文档 - unionId 说明

2. Scope 配置错误(Critical)

问题application.ymlscope: dingtalk 不是有效值,钉钉支持的 scope 是 openidopenid corpid

修复

# application.yml
dingtalk:
  scope:
    - openid  # 或 "openid corpid" 如果需要企业信息

同步修改 .env.release.example 中的注释说明。

⚠️ 强烈建议修复

3. RestTemplate 缺少超时配置

两处 new RestTemplate() 未配置超时,钉钉 API 无响应时会导致线程阻塞。建议参考项目中 CasTicketValidator 的做法:

private RestTemplate buildRestTemplate() {
    var factory = new SimpleClientHttpRequestFactory();
    factory.setConnectTimeout(Duration.ofSeconds(5));
    factory.setReadTimeout(Duration.ofSeconds(10));
    return new RestTemplate(factory);
}

4. NPE 风险

DingTalkTokenResponseClient.java:65 行:

// 当前
String accessToken = json.get("accessToken").asText();  // get() 可能返回 null

// 建议
JsonNode accessTokenNode = json.get("accessToken");
if (accessTokenNode == null || accessTokenNode.isNull()) {
    throw new OAuth2AuthenticationException(
        new OAuth2Error("token_response_missing_field", ...));
}
String accessToken = accessTokenNode.asText();

5. 敏感信息泄露

additionalParameters 中的 raw_response 包含完整 token 响应(可能含 refreshToken),建议移除或只保留必要字段(如 expireIn)。

📝 其他建议(可后续 PR)

  • 测试覆盖:3 个新类缺少单元测试,建议补充(可参考 CasTicketValidatorTest 的 mock 模式)
  • 硬编码 URLDingTalkOAuth2UserService 中的 user-info URL 应从 userRequest.getClientRegistration().getProviderDetails().getUserInfoEndpoint().getUri() 读取
  • HTTP 错误处理DingTalkOAuth2UserService.loadUser() 中的 restTemplate.exchange() 应捕获异常并包装为 OAuth2AuthenticationException
  • 不相关变更SkillStorageDeletionCompensationJpaRepository 的可见性修改应独立提交或在 commit message 中说明原因

📚 部署文档补充

建议在文档中说明钉钉应用配置要求:

  1. 需在钉钉开放平台开启 Contact.User.Read 权限
  2. 需发布应用版本才能激活凭证
  3. 回调地址配置为 {baseUrl}/login/oauth2/code/dingtalk

修复前 2 个阻断性问题后我们会立即 review。如有疑问欢迎随时讨论!🙌

- Change SecurityConfig to @configuration(proxyBeanMethods=false) to avoid
  CGLIB proxy issues with constructor-injected beans
- Add @Autowired to DingTalkOAuth2UserService public constructor so Spring
  resolves the correct constructor when multiple constructors exist
- Change DingTalk scope from openid to corpid: DingTalk does not return
  id_token in its token response, so openid scope causes Spring Security
  to fail with invalid_id_token error. corpid scope works correctly with
  DingTalk's authorization endpoint
- Add error logging to OAuth2LoginFailureHandler for easier debugging
- Add DingTalk client-id/client-secret env vars to application-local.yml
@konglong87
Copy link
Copy Markdown
Author

感谢 review!:

阻断性问题:

  1. ✅ 已修复 — DingTalkClaimsExtractor 使用 unionId 作为 subject,含 null 校验
  2. ⚠️ 需确认 — 当前 scope 为 corpid。我们最初使用 openid,但钉钉授权端点不接受 openid 作为 scope 参数,导致回调 401。改为 corpid 后实测通过,可正常获取 unionId。

强烈建议项:
3. ✅ 已修复 — 两处 RestTemplate 均配置 5s/10s 超时
4. ✅ 已修复 — accessToken 增加 null/isNull/empty 校验
5. ✅ 已修复 — additionalParameters 只保留 expireIn

其他建议:
6. ✅ 已修复 — userInfoUri 从 providerDetails 读取
7. ✅ 已补充 — 部署文档(authentication.md + configuration.md,中英文)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants