feat: add dingding login #6567
feat: add dingding login #6567
Conversation
ming4762
requested review from
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
|
WalkthroughThis change introduces DingTalk (钉钉) login support as a new third-party authentication method. It adds a reusable DingTalk login Vue component, updates configuration, localization, and type definitions, and integrates the new login option into the third-party login UI. Supporting utilities for dynamic script loading and related tests are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DingdingLoginComponent
participant loadScript
participant DingTalkScript
participant DingTalkOAuth
participant Modal
User->>DingdingLoginComponent: Clicks login button
alt QR code login
DingdingLoginComponent->>loadScript: loadScript(dingtalk_url)
loadScript-->>DingdingLoginComponent: Promise resolved
DingdingLoginComponent->>DingTalkScript: Initialize QR code widget
DingTalkScript-->>Modal: Render QR code
User->>DingTalkScript: Scan QR code
DingTalkScript-->>DingdingLoginComponent: onSuccess callback
DingdingLoginComponent->>User: Redirect to DingTalk callback URL
else Redirect login
DingdingLoginComponent->>DingTalkOAuth: Redirect to DingTalk OAuth URL
User->>DingTalkOAuth: Login via DingTalk
DingTalkOAuth-->>User: Redirects back to app with code
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/@core/base/shared/src/utils/resources.ts (1)
1-4: Consider using English for JSDoc comments for consistency.The JSDoc documentation is in Chinese while the codebase appears to use English elsewhere. Consider translating for consistency.
-/** - * 加载js文件 - * @param src js文件地址 - */ +/** + * Load a JavaScript file dynamically + * @param src - The URL of the JavaScript file to load + * @returns Promise that resolves when the script is loaded + */playground/.env.development (1)
18-20: Environment variables follow correct patterns for DingTalk OAuth.The new environment variables use the appropriate
VITE_GLOB_AUTH_prefix and will be correctly exposed to the client-side for OAuth flows.Consider using English placeholder text for consistency:
-# 钉钉登录配置 -VITE_GLOB_AUTH_DINGDING_CLIENT_ID=应用的clientId -VITE_GLOB_AUTH_DINGDING_CORP_ID=应用的corpId +# DingTalk login configuration +VITE_GLOB_AUTH_DINGDING_CLIENT_ID=your_client_id +VITE_GLOB_AUTH_DINGDING_CORP_ID=your_corp_idpackages/effects/common-ui/src/ui/authentication/dingding-login.vue (1)
107-113: Consider using more specific selectors instead of !important.The styling correctly removes padding for QR code display, but the
!importantdeclaration suggests style conflicts. Consider using more specific selectors for better maintainability..dingding-qrcode-login-modal { - .relative { + .relative > * { padding: 0 !important; } }Or investigate if the padding can be controlled through modal props instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/@core/base/shared/src/utils/__tests__/resources.test.ts(1 hunks)packages/@core/base/shared/src/utils/index.ts(1 hunks)packages/@core/base/shared/src/utils/resources.ts(1 hunks)packages/effects/common-ui/src/ui/authentication/dingding-login.vue(1 hunks)packages/effects/common-ui/src/ui/authentication/third-party-login.vue(2 hunks)packages/effects/hooks/src/use-app-config.ts(1 hunks)packages/icons/src/iconify/index.ts(1 hunks)packages/locales/src/langs/en-US/authentication.json(1 hunks)packages/locales/src/langs/zh-CN/authentication.json(1 hunks)packages/types/global.d.ts(1 hunks)playground/.env.development(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/icons/src/iconify/index.ts (1)
packages/@core/base/icons/src/create-icon.ts (1)
createIconifyIcon(14-14)
packages/@core/base/shared/src/utils/__tests__/resources.test.ts (1)
packages/@core/base/shared/src/utils/resources.ts (1)
loadScript(21-21)
packages/effects/hooks/src/use-app-config.ts (1)
packages/types/global.d.ts (1)
ApplicationConfig(23-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (27)
packages/icons/src/iconify/index.ts (1)
15-15: Verify DingTalk icon loading
TheRiDingdingexport points tori:dingding-fill. Our checks show:
- SVG endpoint (…/ri:dingding-fill.svg) returns HTTP 200
- JSON metadata endpoint (…/ri:dingding-fill.json) returns HTTP 404
Please confirm that
createIconifyIconwill load the SVG directly (and not rely on the missing JSON metadata) and test in the running app to ensure the DingTalk icon renders correctly.packages/@core/base/shared/src/utils/resources.ts (1)
5-19: Approve script loading utility – no SSR guard required based on current usage
- Verified with ripgrep:
loadScriptis only imported in browser-only Vue components (e.g.,dingding-login.vue), with no SSR or Node.js contexts detected.- Since this utility isn’t used server-side today, an explicit browser check isn’t necessary. You can add one later if it’s ever consumed in SSR code.
- All other aspects (duplicate prevention, promise API, error handling, DOM cleanup) look solid—great work!
packages/@core/base/shared/src/utils/index.ts (1)
10-10: LGTM! Export addition follows established patterns.The new resources module export is correctly placed in alphabetical order and follows the same pattern as other module exports.
packages/locales/src/langs/en-US/authentication.json (1)
39-43: Well-structured localization entries for third-party login options.The new localization keys follow a consistent naming pattern and provide clear, appropriate English labels for the social login buttons.
packages/locales/src/langs/zh-CN/authentication.json (1)
39-43: LGTM! Chinese translations for third-party login options look good.The new localization keys follow the existing naming conventions and provide appropriate Chinese translations for the third-party login buttons.
packages/types/global.d.ts (3)
12-14: Good addition of DingTalk environment variables to the config interface.The new properties follow the existing naming convention and are appropriately typed as strings.
16-21: Well-designed AuthConfig interface with good extensibility.The optional
dingdingproperty allows for flexible configuration, and the interface structure makes it easy to add other authentication providers in the future.
25-25: Appropriate integration of auth configuration into ApplicationConfig.The addition of the
authproperty maintains consistency with the existing interface structure.packages/effects/hooks/src/use-app-config.ts (3)
18-22: Clean destructuring of the new DingTalk configuration variables.The extraction follows the existing pattern and maintains code consistency.
24-27: Good initialization of the auth configuration object.Starting with an empty auth object provides a clean foundation for conditional configuration additions.
28-33: Excellent conditional logic for DingTalk configuration.The check ensures that both
corpIdandclientIdare present before adding the DingTalk configuration, preventing incomplete auth setups that could cause runtime issues.packages/effects/common-ui/src/ui/authentication/third-party-login.vue (5)
2-2: Good integration of the app configuration hook.Importing
useAppConfigallows the component to access DingTalk authentication configuration dynamically.
8-8: Appropriate import of the DingdingLogin component.The import follows Vue component naming conventions.
14-16: Clean extraction of DingTalk configuration.The destructuring pattern cleanly extracts the DingTalk auth config for conditional rendering.
30-36: Great UX improvement with localized tooltips.Adding tooltips with localized text improves user experience by clearly indicating what each login button does.
58-63: Excellent conditional rendering and prop binding.The
v-ifdirective ensures the DingdingLogin component only renders when configuration is available, and the prop binding correctly passes the required credentials.packages/@core/base/shared/src/utils/__tests__/resources.test.ts (5)
8-12: Good test setup with proper environment isolation.The
beforeEachhook ensures each test starts with a clean DOM state, preventing test interference.
14-28: Comprehensive test for successful script loading.The test properly verifies DOM manipulation and promise resolution by simulating the load event.
30-45: Excellent test for duplicate script prevention.This test ensures the utility doesn't create duplicate script tags, which is crucial for performance and avoiding conflicts.
47-59: Good error handling test with appropriate error message.The test verifies that loading failures are properly handled and provides meaningful error messages.
61-81: Thorough test for concurrent call handling.This test ensures the utility handles multiple simultaneous requests for the same script correctly, which is important for real-world usage scenarios.
packages/effects/common-ui/src/ui/authentication/dingding-login.vue (6)
1-9: LGTM! Clean imports and proper Vue 3 setup.The imports are well-organized and follow proper TypeScript conventions. Good use of Vue 3 composition API with script setup syntax.
11-22: Well-defined Props interface with good documentation.The TypeScript interface is properly structured with clear property definitions. The Chinese comments provide helpful context for the redirect URI usage.
24-32: Modal configuration is appropriate for QR code display.The fixed dimensions and callback-based initialization ensure the QR code widget loads properly when the modal opens.
34-40: Good fallback logic for redirect URI.The function properly handles both explicit redirect URI and automatic fallback to current location.
81-89: URL construction needs parameter consistency check.The redirect URL construction should use consistent parameter naming. Verify that
corpid(line 87) matches DingTalk's API requirements, as the QR code login usescorpId(line 67).Consider extracting URL construction to a helper function for better maintainability:
+const buildOAuthUrl = () => { + const { clientId, corpId } = props; + const params = new URLSearchParams({ + redirect_uri: getRedirectUri(), + response_type: 'code', + client_id: clientId, + scope: 'openid', + corpid: corpId, // Verify this parameter name + prompt: 'consent' + }); + return `https://login.dingtalk.com/oauth2/auth?${params}`; +}; const handleLogin = () => { const { isQrCode } = props; if (isQrCode) { modalApi.open(); } else { - window.location.href = `https://login.dingtalk.com/oauth2/auth?redirect_uri=${encodeURIComponent(getRedirectUri())}&response_type=code&client_id=${clientId}&scope=openid&corpid=${corpId}&prompt=consent`; + window.location.href = buildOAuthUrl(); } };
92-105: Clean template structure with proper component usage.The template follows Vue 3 best practices with appropriate use of Vben UI components. The tooltip configuration and modal integration are well-implemented.
| const handleQrCodeLogin = async () => { | ||
| const { clientId, corpId } = props; | ||
| if (!(window as any).DTFrameLogin) { | ||
| // 二维码登录 加载资源 | ||
| await loadScript( | ||
| 'https://g.alicdn.com/dingding/h5-dingtalk-login/0.21.0/ddlogin.js', | ||
| ); | ||
| } | ||
| (window as any).DTFrameLogin( | ||
| { | ||
| id: 'dingding_qrcode_login_element', | ||
| width: 300, | ||
| height: 300, | ||
| }, | ||
| { | ||
| // 注意：redirect_uri 需为完整URL，扫码后钉钉会带code跳转到这里 | ||
| redirect_uri: encodeURIComponent(getRedirectUri()), | ||
| client_id: clientId, | ||
| scope: 'openid corpid', | ||
| response_type: 'code', | ||
| state: '1', | ||
| prompt: 'consent', | ||
| corpId, | ||
| }, | ||
| (loginResult: any) => { | ||
| const { redirectUrl } = loginResult; | ||
| // 这里可以直接进行重定向 | ||
| window.location.href = redirectUrl; | ||
| }, | ||
| (errorMsg: string) => { | ||
| // 这里一般需要展示登录失败的具体原因 | ||
| alert(`Login Error: ${errorMsg}`); | ||
| }, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify parameter consistency and improve error handling.
The QR code login implementation is solid, but there are a few areas for improvement:
- Parameter naming inconsistency: Line 67 uses
corpIdwhile line 87 usescorpid- ensure consistency with DingTalk API requirements. - Consider adding loading state: Users don't see feedback during script loading.
- Error handling: The alert for login errors could be more user-friendly.
🏁 Script executed:
#!/bin/bash
# Verify DingTalk API parameter naming consistency
rg -A 5 -B 5 "corpid|corpId" --type vueLength of output: 69
🏁 Script executed:
#!/bin/bash
# Search for `corpId` and `corpid` usage across all .vue files to verify naming consistency
rg -n 'corpId|corpid' -g '*.vue'Length of output: 935
Update DingTalk QR Code Login Implementation
The corpId and corpid usages are correct (camel-case prop for the JS SDK vs. lowercase query param and scope name). Focus on the remaining improvements:
• Remove the parameter-name consistency suggestion—these keys serve different, correct purposes.
• Add a loading indicator/state while ddlogin.js is being fetched.
• Replace the alert in the error callback with a more user-friendly UI message (e.g., inline error banner or toast).
🤖 Prompt for AI Agents
In packages/effects/common-ui/src/ui/authentication/dingding-login.vue between
lines 45 and 79, add a loading state to show a loading indicator while the
ddlogin.js script is being fetched asynchronously. Remove any changes suggesting
renaming corpId or corpid as they are correctly used. Replace the alert in the
error callback with a user-friendly UI message such as an inline error banner or
toast notification to display login errors more gracefully.
2 participants
Summary by CodeRabbit