feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 - #4112
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughAdds a boolean SMTP option Changes
Sequence DiagramsequenceDiagram
participant UI as Settings UI
participant Config as Backend Config
participant Logic as getSMTPAuth()
participant Auth as AuthMechanism
participant SMTP as SMTP Server
UI->>Config: Toggle/Save SMTPForceAuthLogin
Config->>Logic: Provide SMTPForceAuthLogin, SMTPAccount, SMTPServer, SMTPToken
Logic->>Logic: shouldUseSMTPLoginAuth()? (force flag, isOutlookServer, server list)
alt Use AUTH LOGIN
Logic->>Auth: Create LoginAuth(account, token)
else Use PLAIN
Logic->>Auth: Create PlainAuth(account, token, server)
end
Logic->>SMTP: Send email using computed Auth
SMTP-->>Logic: Response / auth result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/email.go`:
- Around line 90-92: The SSL (SSL/465) send path currently leaves auth as
smtp.PlainAuth so SMTPForceAuthLogin is ignored; update the SSL branch where
auth is set (the branch that currently uses smtp.PlainAuth for SSL) to respect
the same logic as the non-SSL branch by calling shouldUseSMTPLoginAuth() (or
checking SMTPForceAuthLogin) and assigning auth = LoginAuth(SMTPAccount,
SMTPToken) when appropriate before calling smtp.SendMail; ensure the SSL path
uses the same auth variable (and passes it to smtp.SendMail) so forced AUTH
LOGIN behavior applies to both SSL and non-SSL flows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8aed63f-1d3f-448f-9818-4e06765b283c
⛔ Files ignored due to path filters (1)
web/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
common/constants.gocommon/email.gocommon/email_auth_selection_test.gomodel/option.gomodel/option_smtp_auth_test.goweb/src/components/settings/SystemSetting.jsx
|
感谢您的pr,我们发现email_auth_selection_test.go这个测试文件并不需要提交上来,您可以去掉测试文件后重新提交吗 |
已经移除 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
common/email.go (1)
22-24:⚠️ Potential issue | 🔴 Critical
SMTPForceAuthLoginis used at Line 23 but appears undeclared (compile blocker).
shouldUseSMTPLoginAuth()referencesSMTPForceAuthLogin, but in the providedcommon/constants.goSMTP globals (Lines 80-85), this symbol is missing. This will fail withundefined: SMTPForceAuthLogin.🔧 Minimal fix
--- a/common/constants.go +++ b/common/constants.go @@ var SMTPToken = "" +var SMTPForceAuthLogin = false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/email.go` around lines 22 - 24, The function shouldUseSMTPLoginAuth references an undeclared symbol SMTPForceAuthLogin; add a package-level declaration for SMTPForceAuthLogin (e.g., var SMTPForceAuthLogin bool = false) to the SMTP globals in common/constants.go, or if an existing flag like SMTPForceAuth was intended, replace SMTPForceAuthLogin with that symbol in shouldUseSMTPLoginAuth; ensure the chosen symbol is exported/defined in the same package so the code compiles.
🧹 Nitpick comments (1)
common/email.go (1)
97-100: Remove redundant conditional at Line 97.
else if shouldUseSMTPLoginAuth()andelseexecute the exact samesmtp.SendMail(...)call, so this branch is dead logic after introducinggetSMTPAuth().♻️ Simplification
- } else if shouldUseSMTPLoginAuth() { - err = smtp.SendMail(addr, auth, SMTPFrom, to, mail) - } else { + } else { err = smtp.SendMail(addr, auth, SMTPFrom, to, mail) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/email.go` around lines 97 - 100, The conditional branch checking shouldUseSMTPLoginAuth() is redundant because both branches call smtp.SendMail(addr, auth, SMTPFrom, to, mail); remove the else-if/else branching and replace it with a single smtp.SendMail(...) invocation (assigning to err) after obtaining auth via getSMTPAuth() so the logic in the function that calls smtp.SendMail is simplified and uses the same variables (addr, auth, SMTPFrom, to, mail).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/email.go`:
- Around line 22-27: The SMTPForceAuthLogin flag is used by
shouldUseSMTPLoginAuth but isn't persisted/loaded from options; add
SMTPForceAuthLogin to the option mapping in model/option.go so admin/UI changes
persist and load correctly — update the options map/struct entry for
SMTPForceAuthLogin with its default value and proper key, and ensure it is
included in the load/save/encode paths the file uses (the same pattern used for
other SMTP_* keys) so shouldUseSMTPLoginAuth reads the persisted setting.
---
Duplicate comments:
In `@common/email.go`:
- Around line 22-24: The function shouldUseSMTPLoginAuth references an
undeclared symbol SMTPForceAuthLogin; add a package-level declaration for
SMTPForceAuthLogin (e.g., var SMTPForceAuthLogin bool = false) to the SMTP
globals in common/constants.go, or if an existing flag like SMTPForceAuth was
intended, replace SMTPForceAuthLogin with that symbol in shouldUseSMTPLoginAuth;
ensure the chosen symbol is exported/defined in the same package so the code
compiles.
---
Nitpick comments:
In `@common/email.go`:
- Around line 97-100: The conditional branch checking shouldUseSMTPLoginAuth()
is redundant because both branches call smtp.SendMail(addr, auth, SMTPFrom, to,
mail); remove the else-if/else branching and replace it with a single
smtp.SendMail(...) invocation (assigning to err) after obtaining auth via
getSMTPAuth() so the logic in the function that calls smtp.SendMail is
simplified and uses the same variables (addr, auth, SMTPFrom, to, mail).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aedf1c76-ae1c-4492-982b-e6e8cd4eba4c
⛔ Files ignored due to path filters (1)
web/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
common/constants.gocommon/email.gomodel/option.gomodel/option_smtp_auth_test.goweb/src/components/settings/SystemSetting.jsx
✅ Files skipped from review due to trivial changes (1)
- model/option_smtp_auth_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/components/settings/SystemSetting.jsx
- common/constants.go
- model/option.go
- Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox
* feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 * fix: 修复通过 SSL 发送邮件时绕过 AUTH LOGIN 的问题 * fix: remove redundant branch, delete test file, add i18n translations - Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox (cherry picked from commit a18ea3cc167ec69be05052fc8f35b74d974a4856)
* feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 * fix: 修复通过 SSL 发送邮件时绕过 AUTH LOGIN 的问题 * fix: remove redundant branch, delete test file, add i18n translations - Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox (cherry picked from commit a18ea3c)
* feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 * fix: 修复通过 SSL 发送邮件时绕过 AUTH LOGIN 的问题 * fix: remove redundant branch, delete test file, add i18n translations - Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox
* feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 * fix: 修复通过 SSL 发送邮件时绕过 AUTH LOGIN 的问题 * fix: remove redundant branch, delete test file, add i18n translations - Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox
* feat: 支持强制使用 AUTH LOGIN 以解决 outlook 等邮箱的发件问题 * fix: 修复通过 SSL 发送邮件时绕过 AUTH LOGIN 的问题 * fix: remove redundant branch, delete test file, add i18n translations - Remove redundant else-if branch in SendEmail since auth is already computed via getSMTPAuth() - Delete option_smtp_auth_test.go as requested - Add i18n translations for '强制使用 AUTH LOGIN' checkbox
Summary by CodeRabbit
New Features
Internationalization