feat: 关联 discord 账号 - #1706
Conversation
WalkthroughAdds Discord OAuth: backend handlers for login and binding, Discord settings, user model DiscordId, frontend OAuth flows/buttons/routes, system settings UI, and API/docs updates to expose /api/oauth/discord and related status flags. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User (Browser)
participant FE as Frontend
participant BE as Backend (Gin)
participant D as Discord OAuth
participant DB as Database
rect rgb(220,240,255)
note over U,BE: Discord OAuth Login / Registration
U->>FE: Click "Continue with Discord"
FE->>FE: getOAuthState()
FE->>D: Redirect to Discord OAuth (client_id, redirect_uri, state)
D->>U: Authorize & redirect back (/oauth/discord?code&state)
U->>FE: Browser hits /oauth/discord
FE->>BE: GET /api/oauth/discord?code=...
BE->>D: Exchange code -> token
D-->>BE: access_token
BE->>D: Fetch user info (identify, openid)
D-->>BE: discord user (id, username)
alt DiscordId exists
BE->>DB: Lookup user by discord_id
DB-->>BE: user
BE->>BE: validate status -> login
else New user
BE->>DB: ensure discord_id not taken
BE->>DB: create user with discord_id
end
BE-->>FE: JSON success / user info
FE->>U: Complete login / redirect
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
|
代码看着没什么问题，但是还有最后一个地方需要处理，新的设置项的读取和写入需要模仿 @settings 下的内容来操作，这个弄完就应当可以合并了 |
这个具体在哪里，我在仓库里搜索 是指这个吗 new-api/setting/system_setting/oidc.go Lines 1 to 25 in efb8f1f |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web/src/components/auth/RegisterForm.jsx (1)
31-32: Discord OAuth handler and loading state are wired correctly; optional alignment with terms gatingThe new
onDiscordOAuthClickedimport,discordLoadingstate, andhandleDiscordClickimplementation are consistent with existing OAuth handlers (GitHub, LinuxDO, OIDC) and the helper signature. Given thatLoginForm.jsx’shandleDiscordClickenforces the 用户协议/隐私政策 checkbox before redirect, consider reusing the same(hasUserAgreement || hasPrivacyPolicy) && !agreedToTermsguard here if you want Discord-based registration to obey the same terms requirement.Also applies to: 55-79, 270-277
controller/discord.go (1)
36-100: Discord token & userinfo fetch helper is sound; optional logging enhancementThe code correctly validates
code, uses client credentials, applies a 5s timeout, checks for a missingaccess_token, and then fetches/users/@mewith proper authorization and status checks. For easier debugging of misconfigurations, you might additionally log the token endpoint’s non-200 status and (sanitized) body before returning the generic “获取 Token 失败/用户信息失败” errors, but this is not required for correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
README.en.md(1 hunks)README.md(1 hunks)common/constants.go(2 hunks)controller/discord.go(1 hunks)controller/misc.go(1 hunks)controller/option.go(1 hunks)controller/user.go(1 hunks)docs/api/web_api.md(1 hunks)model/option.go(4 hunks)model/user.go(3 hunks)router/api-router.go(1 hunks)web/src/App.jsx(1 hunks)web/src/components/auth/LoginForm.jsx(7 hunks)web/src/components/auth/RegisterForm.jsx(7 hunks)web/src/components/settings/SystemSetting.jsx(5 hunks)web/src/components/settings/personal/cards/AccountManagement.jsx(2 hunks)web/src/components/table/users/modals/EditUserModal.jsx(2 hunks)web/src/helpers/api.js(1 hunks)web/src/i18n/locales/zh.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
router/api-router.go (2)
middleware/rate-limit.go (1)
CriticalRateLimit(104-109)controller/discord.go (1)
DiscordOAuth(102-179)
controller/option.go (1)
common/constants.go (1)
DiscordClientId(86-86)
model/user.go (1)
model/main.go (1)
DB(64-64)
model/option.go (1)
common/constants.go (4)
OptionMap(37-37)DiscordOAuthEnabled(47-47)DiscordClientId(86-86)DiscordClientSecret(87-87)
web/src/components/settings/personal/cards/AccountManagement.jsx (2)
web/src/components/settings/PersonalSetting.jsx (1)
status(61-61)web/src/helpers/api.js (1)
onDiscordOAuthClicked(234-243)
web/src/components/auth/RegisterForm.jsx (2)
web/src/components/auth/LoginForm.jsx (3)
discordLoading(78-78)handleDiscordClick(305-317)status(105-108)web/src/helpers/api.js (1)
onDiscordOAuthClicked(234-243)
controller/misc.go (1)
common/constants.go (2)
DiscordOAuthEnabled(47-47)DiscordClientId(86-86)
web/src/components/auth/LoginForm.jsx (3)
web/src/components/auth/RegisterForm.jsx (6)
discordLoading(77-77)handleDiscordClick(270-277)hasUserAgreement(89-89)hasPrivacyPolicy(90-90)agreedToTerms(88-88)status(103-106)web/src/helpers/utils.jsx (1)
showInfo(161-163)web/src/helpers/api.js (1)
onDiscordOAuthClicked(234-243)
controller/discord.go (5)
common/constants.go (5)
DiscordClientId(86-86)DiscordClientSecret(87-87)DiscordOAuthEnabled(47-47)RegisterEnabled(52-52)UserStatusEnabled(179-179)setting/system_setting/system_setting_old.go (1)
ServerAddress(3-3)common/sys_log.go (2)
SysLog(11-14)SysError(16-19)common/gin.go (1)
ApiError(104-109)model/user.go (3)
User(20-50)IsDiscordIdAlreadyTaken(590-592)GetMaxUserId(182-186)
web/src/App.jsx (2)
web/src/components/common/ui/Loading.jsx (1)
Loading(23-29)web/src/components/auth/OAuth2Callback.jsx (1)
OAuth2Callback(33-93)
🔇 Additional comments (29)
controller/user.go (1)
456-456: LGTM: Discord ID field properly exposed in user self-information.The addition of
discord_idto the GetSelf response is consistent with the existing OAuth provider fields (github_id, oidc_id, wechat_id, telegram_id) and correctly exposes the Discord account binding information to the frontend.web/src/components/table/users/modals/EditUserModal.jsx (1)
75-75: LGTM: Discord ID field properly integrated into user edit modal.The discord_id field has been correctly added to:
- Initial form values (line 75) for proper form initialization
- Binding information display list (line 336) following the established pattern for third-party OAuth accounts
The placement and implementation are consistent with other OAuth provider integrations.
Also applies to: 336-336
model/user.go (1)
30-30: LGTM: Discord ID field and lookup method properly implemented.The DiscordId field definition (line 30) and FillUserByDiscordId method (lines 543-549) follow the established patterns for other OAuth providers, with proper:
- JSON and GORM tags with indexing
- Error handling for empty IDs
- Database query implementation
Also applies to: 543-549
docs/api/web_api.md (1)
45-45: LGTM: API documentation properly updated.The Discord OAuth endpoint documentation has been correctly added to the OAuth / Third-party login section, following the same format as existing OAuth endpoints (GitHub, OIDC, LinuxDO, etc.).
web/src/i18n/locales/zh.json (1)
260-260: LGTM: Chinese translation properly added.The Discord login translation entry follows the established pattern for other OAuth providers (GitHub, LinuxDO, OIDC) and is correctly placed in the localization file.
controller/misc.go (1)
55-56: LGTM: Discord OAuth status fields properly exposed.The discord_oauth and discord_client_id fields have been correctly added to the GetStatus endpoint response, following the same pattern as the GitHub OAuth integration (lines 53-54). This enables the frontend to check Discord OAuth availability and configuration.
Note: Based on the PR review comment from seefs001, there may be additional work needed to handle Discord settings through the @settings system. Please verify that
common.DiscordOAuthEnabledandcommon.DiscordClientIdare properly synchronized with the settings/options persistence layer, similar to how other OAuth providers handle their configuration.Based on learnings
router/api-router.go (1)
33-33: LGTM: Discord OAuth route properly registered.The Discord OAuth endpoint has been correctly added with:
- Appropriate CriticalRateLimit middleware (consistent with other OAuth routes)
- Proper route placement in the OAuth section
- Standard RESTful pattern matching existing OAuth implementations
README.md (1)
196-196: LGTM: Documentation properly updated with Discord OAuth feature.The Discord authorization login feature has been correctly added to the "授权与安全" (Authorization and Security) section of the README, maintaining consistency with other OAuth provider listings.
README.en.md (1)
196-196: LGTM - Documentation update for Discord OAuthThe Discord authorization login entry is properly added to the Authorization and Security section, consistent with other OAuth provider entries.
web/src/App.jsx (1)
195-202: LGTM - Discord OAuth route follows established patternThe new Discord OAuth callback route is correctly implemented, matching the structure of existing OAuth providers (GitHub, OIDC, LinuxDO) with proper Suspense wrapping and error handling.
common/constants.go (2)
47-47: LGTM - Discord OAuth constants properly definedThe Discord OAuth configuration constants follow the established pattern of other OAuth providers (GitHub, LinuxDO) with appropriate default values.
86-87: LGTM - Discord credentials constantsClient ID and Secret constants are properly declared, consistent with the GitHub OAuth implementation pattern.
web/src/components/settings/personal/cards/AccountManagement.jsx (1)
251-290: LGTM - Discord account binding UI properly implementedThe Discord binding card follows the exact pattern of existing OAuth provider bindings (GitHub, LinuxDO), with proper state management, conditional rendering, and consistent UI layout.
model/option.go (4)
41-41: LGTM - Discord OAuth enabled flag initializationThe DiscordOAuthEnabled option is properly initialized in the option map, following the same pattern as GitHubOAuthEnabled.
99-100: LGTM - Discord credentials initializationDiscord client ID and secret are properly initialized with empty defaults in the option map, consistent with GitHub OAuth credentials.
230-231: LGTM - Discord enabled flag update logicThe update logic for DiscordOAuthEnabled properly converts the string value to boolean and assigns it to the global variable, following the established pattern.
368-371: LGTM - Discord credentials update logicThe update cases for DiscordClientId and DiscordClientSecret properly assign values to the corresponding global variables, consistent with GitHub OAuth handling.
controller/option.go (1)
74-81: LGTM - Discord OAuth validation follows established patternThe validation check for Discord OAuth credentials mirrors the GitHub OAuth validation, preventing enablement without proper configuration. The Chinese error message is consistent with other validation messages in this file.
web/src/components/auth/LoginForm.jsx (4)
304-317: LGTM - Discord login handler properly implementedThe handleDiscordClick function correctly implements user agreement validation and loading state management, following the same pattern as GitHub OAuth.
493-504: LGTM - Discord login button consistent with other OAuth providersThe Discord OAuth button is properly rendered with:
- Conditional display based on
status.discord_oauth- Consistent styling with other OAuth buttons
- Proper icon integration
- Loading state handling
- Localized button text
748-748: LGTM - Discord included in OAuth options checkDiscord is properly included in the conditional logic that determines whether to show the "other login options" button.
884-884: LGTM - Discord included in form display logicDiscord is correctly included in the conditional that decides whether to show OAuth options or the email login form.
web/src/helpers/api.js (1)
234-243: Original review comment is incorrectDiscord DOES support
openidas a valid OAuth 2.0 scope. The claim that "Discord doesn't implement OpenID Connect" is inaccurate. The current implementation usingscope = 'identify+openid'is valid and no changes are needed.Likely an incorrect or invalid review comment.
web/src/components/auth/RegisterForm.jsx (2)
392-403: Discord OAuth button integration looks correctThe new Discord button is properly gated on
status.discord_oauth, usesSiDiscordwith appropriate sizing, binds tohandleDiscordClick, and reflectsdiscordLoadingfor UX consistency with other providers.
618-623: Gating conditions correctly include Discord as an OAuth optionAdding
status.discord_oauthinto both the “其他注册选项” condition and the top-level decision between email form vs. OAuth options ensures that Discord-only setups behave correctly (i.e., the alternate options button shows and the OAuth panel is used when Discord is the sole provider).Also applies to: 713-719
controller/discord.go (1)
102-179: DiscordOAuth flow correctly handles state, toggle flags, and auto-registration
DiscordOAuthproperly validates thestateagainst the session, branches to binding when a session user exists, respectsDiscordOAuthEnabledandRegisterEnabled, reusesIsDiscordIdAlreadyTaken/FillUserByDiscordIdfor existing accounts, and only auto-creates a user when allowed, with sensible fallbacks forUsernameandDisplayName. The post-login status check againstUserStatusEnabledbeforesetupLoginis also consistent with the rest of the system.web/src/components/settings/SystemSetting.jsx (3)
49-58: Discord settings are correctly added to inputs and option normalizationThe additions of
DiscordOAuthEnabled,DiscordClientId, andDiscordClientSecrettoinputsandoriginInputs, plus inclusion ofDiscordOAuthEnabledin the booleantoBooleanswitch, mirror how other auth toggles are handled. This satisfies the requirement that new settings follow the existing@settingsread/normalize pattern.Also applies to: 173-191
480-499: submitDiscordOAuth mirrors existing provider submit patterns
submitDiscordOAuthfollows the same approach assubmitGitHubOAuth/submitLinuxDOOAuth: only pushesDiscordClientIdwhen changed, and only sendsDiscordClientSecretwhen it has changed and is non-empty, avoiding accidental secret clearing and unnecessary writes. Using the sharedupdateOptionshelper keeps behavior consistent with other providers.
1042-1050: Discord OAuth toggle and configuration UI are consistent with other providersThe new
DiscordOAuthEnabledcheckbox in the 登录/注册 section and the dedicated “配置 Discord OAuth” card align with the existing GitHub/Linux DO patterns. The banner text correctly guides admins to use${ServerAddress}and${ServerAddress}/oauth/discordfor Homepage and callback URLs, matching the backend’sredirect_uriconstruction. This should make the new provider straightforward to operate.Also applies to: 1447-1477
| func DiscordBind(c *gin.Context) { | ||
| if !common.DiscordOAuthEnabled { | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": false, | ||
| "message": "管理员未开启通过 Discord 登录以及注册", | ||
| }) | ||
| return | ||
| } | ||
| code := c.Query("code") | ||
| discordUser, err := getDiscordUserInfoByCode(code) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| user := model.User{ | ||
| DiscordId: discordUser.UID, | ||
| } | ||
| if model.IsDiscordIdAlreadyTaken(user.DiscordId) { | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": false, | ||
| "message": "该 Discord 账户已被绑定", | ||
| }) | ||
| return | ||
| } | ||
| session := sessions.Default(c) | ||
| id := session.Get("id") | ||
| // id := c.GetInt("id") // critical bug! | ||
| user.Id = id.(int) | ||
| err = user.FillUserById() | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| user.DiscordId = discordUser.UID | ||
| err = user.Update(false) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "bind", | ||
| }) | ||
| } |
There was a problem hiding this comment.
Guard against panics when reading user id from session in DiscordBind
DiscordBind assumes session.Get("id") is always a non-nil int and immediately does id.(int). If the route is ever hit without a fully populated session (e.g., expired session, misconfigured middleware, or different type stored), this will panic and 500 the request. The commented // id := c.GetInt("id") // critical bug! line is also stale and confusing.
Safely handle missing/invalid session ids and remove the dead comment, for example:
- session := sessions.Default(c)
- id := session.Get("id")
- // id := c.GetInt("id") // critical bug!
- user.Id = id.(int)
+ session := sessions.Default(c)
+ idVal := session.Get("id")
+ id, ok := idVal.(int)
+ if !ok {
+ c.JSON(http.StatusOK, gin.H{
+ "success": false,
+ "message": "用户未登录或会话无效",
+ })
+ return
+ }
+ user.Id = idThis avoids a runtime panic and returns a controlled JSON error instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func DiscordBind(c *gin.Context) { | |
| if !common.DiscordOAuthEnabled { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "管理员未开启通过 Discord 登录以及注册", | |
| }) | |
| return | |
| } | |
| code := c.Query("code") | |
| discordUser, err := getDiscordUserInfoByCode(code) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| user := model.User{ | |
| DiscordId: discordUser.UID, | |
| } | |
| if model.IsDiscordIdAlreadyTaken(user.DiscordId) { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "该 Discord 账户已被绑定", | |
| }) | |
| return | |
| } | |
| session := sessions.Default(c) | |
| id := session.Get("id") | |
| // id := c.GetInt("id") // critical bug! | |
| user.Id = id.(int) | |
| err = user.FillUserById() | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| user.DiscordId = discordUser.UID | |
| err = user.Update(false) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "message": "bind", | |
| }) | |
| } | |
| func DiscordBind(c *gin.Context) { | |
| if !common.DiscordOAuthEnabled { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "管理员未开启通过 Discord 登录以及注册", | |
| }) | |
| return | |
| } | |
| code := c.Query("code") | |
| discordUser, err := getDiscordUserInfoByCode(code) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| user := model.User{ | |
| DiscordId: discordUser.UID, | |
| } | |
| if model.IsDiscordIdAlreadyTaken(user.DiscordId) { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "该 Discord 账户已被绑定", | |
| }) | |
| return | |
| } | |
| session := sessions.Default(c) | |
| idVal := session.Get("id") | |
| id, ok := idVal.(int) | |
| if !ok { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "用户未登录或会话无效", | |
| }) | |
| return | |
| } | |
| user.Id = id | |
| err = user.FillUserById() | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| user.DiscordId = discordUser.UID | |
| err = user.Update(false) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "message": "bind", | |
| }) | |
| } |
🤖 Prompt for AI Agents
In controller/discord.go around lines 181-224, DiscordBind currently does an
unchecked id.(int) on session.Get("id") and has a stale commented line; change
it to safely read and validate the session id (remove the dead comment):
retrieve session.Get("id"), check for nil, perform a safe type assertion (or use
a type switch) to extract an int, and if missing/invalid return a controlled
JSON error (or common.ApiError) instead of panicking; only proceed to
FillUserById/Update when you have a valid int user id.
对的，需要按照这个加一个 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
setting/system_setting/discord.go (1)
5-9: Consider following Go naming conventions for acronyms.The field name
ClientIdshould beClientIDto follow Go's naming convention where acronyms are capitalized (e.g.,ID,URL,HTTP). Same applies to the JSON tag which should remainclient_id.type DiscordSettings struct { Enabled bool `json:"enabled"` - ClientId string `json:"client_id"` + ClientID string `json:"client_id"` ClientSecret string `json:"client_secret"` }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
controller/discord.go(1 hunks)controller/misc.go(1 hunks)controller/option.go(1 hunks)setting/system_setting/discord.go(1 hunks)web/src/components/settings/SystemSetting.jsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- controller/option.go
- controller/discord.go
- controller/misc.go
🧰 Additional context used
🧬 Code graph analysis (2)
setting/system_setting/discord.go (1)
setting/config/config.go (1)
GlobalConfig(19-19)
web/src/components/settings/SystemSetting.jsx (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (2)
originInputs(130-166)inputs(171-171)
🔇 Additional comments (7)
setting/system_setting/discord.go (2)
19-21: Verify thread safety of returning pointer to package-level variable.Returning a direct pointer to the package-level
defaultDiscordSettingsallows callers to mutate the shared state, which could lead to race conditions if accessed concurrently. Consider whether the config manager provides synchronization, or if a copy should be returned instead.If thread safety is a concern, you might consider returning a copy:
func GetDiscordSettings() DiscordSettings { return defaultDiscordSettings }Or ensuring all access is synchronized through the config manager.
14-17: Implementation follows the requested pattern correctly.The Discord settings registration follows the same pattern as other OAuth providers (like OIDC) in the codebase, which addresses the reviewer's feedback about following the
@settingsimplementation pattern.web/src/components/settings/SystemSetting.jsx (5)
55-57: State initialization follows consistent naming pattern.The Discord state fields use dot notation (
discord.enabled,discord.client_id,discord.client_secret), which is consistent with the OIDC implementation and appropriate for namespaced settings.
185-185: Boolean option handling is correct.The
discord.enabledfield is properly added to the boolean normalization switch case, ensuring the value from the API is correctly converted to a boolean. This matches the pattern used for other OAuth providers.
480-499: Submit function implementation looks good.The
submitDiscordOAuthfunction correctly follows the established pattern used bysubmitGitHubOAuthand other OAuth providers:
- Only submits changed fields
- Protects the secret field by only updating when non-empty
- Uses the existing
updateOptionshelper
1042-1050: Discord login checkbox is properly implemented.The Discord enable checkbox is correctly integrated into the login/registration configuration section, following the same pattern as other OAuth providers (GitHub, OIDC, etc.).
1447-1477: Discord OAuth configuration UI is well-implemented.The Discord OAuth configuration card follows the established pattern:
- Provides helpful setup instructions in the Banner
- Uses proper field notation for the Form inputs
- Correctly marks the secret field as password type
- Wired to the appropriate submit handler
The implementation is consistent with other OAuth provider configurations (GitHub, OIDC).
feat: 关联 discord 账号
2 participants
PR 类型
PR 是否包含破坏性更新？
PR 描述
为 discord 添加 oauth 关联
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.