feat(channel): 添加2FA验证后查看渠道密钥功能 - #1653
Conversation
- 新增接口通过2FA验证后获取渠道密钥 - 统一实现2FA验证码和备用码的验证逻辑 - 记录用户查看密钥的操作日志 - 编辑渠道弹窗新增查看密钥按钮，触发2FA验证模态框 - 使用TwoFactorAuthModal进行验证码输入及验证 - 验证成功后弹出渠道密钥展示窗口 - 对渠道编辑模态框的状态进行了统一重置优化 - 添加相关国际化文案支持密钥查看功能
WalkthroughAdds a 2FA-gated API endpoint to reveal channel keys (TOTP or backup codes), centralizes 2FA validation, registers a new POST route, and adds frontend pieces: a reusable 2FA modal, a channel key display, EditChannel integration for viewing keys, and related i18n strings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin User
participant UI as Web UI (EditChannelModal)
participant API as API Server
participant Auth as TwoFactor Validator
participant DB as Data Store
Admin->>UI: Click "View key"
UI->>Admin: Show TwoFactorAuthModal
Admin->>UI: Enter code
UI->>API: POST /api/channel/:id/key { code }
API->>Auth: validateTwoFactorAuth(userId, code)
alt code valid (TOTP or backup)
Auth-->>API: validated
API->>DB: Fetch channel including key
DB-->>API: Channel + key
API-->>UI: 200 { key data }
UI->>UI: Render ChannelKeyDisplay
else invalid code
Auth-->>API: invalid
API-->>UI: 4xx error
UI->>Admin: Show error notification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 3
🧹 Nitpick comments (15)
web/src/components/common/modals/TwoFactorAuthModal.jsx (2)
111-118: Improve mobile input UX for 6‑digit TOTP while preserving 8‑digit backupsConsider nudging numeric input on mobile while still allowing 8‑digit backup codes:
- Add inputMode="numeric" and pattern="[0-9]*" to hint numeric keypad for TOTPs; backups remain accepted as digits.
Example:
- <Input + <Input placeholder={placeholder || t('请输入认证器验证码或备用码')} value={code} onChange={onCodeChange} size="large" maxLength={8} onKeyDown={handleKeyDown} autoFocus + inputMode="numeric" + pattern="[0-9]*" />
56-87: Modal accessibility nit: mark decorative SVG as aria-hiddenThe lock icon in the title is decorative. Add aria-hidden="true" to avoid it being read by screen readers.
- <div className="w-8 h-8 rounded-full bg-blue-100 dark:bg-blue-900 flex items-center justify-center mr-3"> - <svg className="w-4 h-4 text-blue-600 dark:text-blue-400" fill="currentColor" viewBox="0 0 20 20"> + <div className="w-8 h-8 rounded-full bg-blue-100 dark:bg-blue-900 flex items-center justify-center mr-3"> + <svg aria-hidden="true" className="w-4 h-4 text-blue-600 dark:text-blue-400" fill="currentColor" viewBox="0 0 20 20"> <path fillRule="evenodd" d="M5 9V7a5 5 0 0110 0v2a2 2 0 012 2v5a2 2 0 01-2 2H5a2 2 0 01-2-2v-5a2 2 0 012-2zm8-2v2H7V7a3 3 0 016 0z" clipRule="evenodd" /> </svg> </div>Repeat for other decorative SVGs within this modal if applicable.
web/src/i18n/locales/en.json (2)
2016-2020: Polish English translations for consistency and clarityMinor phrasing tweaks to sound more natural in English.
- "获取密钥失败": "Failed to get key", + "获取密钥失败": "Failed to retrieve key", "查看密钥": "View key", - "查看渠道密钥": "View channel key", + "查看渠道密钥": "View Channel Key", "渠道密钥信息": "Channel key information", - "密钥获取成功": "Key acquisition successful" + "密钥获取成功": "Key retrieved successfully"
2012-2015: Ensure i18n consistency by reusing existing 2FA keysThe new strings introduced at lines 2013–2015 overlap with existing translations elsewhere in en.json. To reduce duplication and simplify maintenance, please reuse or merge with the following keys:
• Replace the standalone “验证” (line 2013) with the existing “验证身份” key (line 1926).
• Instead of “为了保护账户安全，请验证您的两步验证码。” (line 2014), reuse the copy under key “为了保护您的账户安全，请输入认证器验证码来确认身份” (line 1927).
• For the description “支持6位TOTP验证码或8位备用码” (line 2015), consider whether the existing prompt “请输入验证码或备用码” (line 1968) suffices; if a more instructional tooltip is required, merge both contexts under a single new or existing key.By consolidating these, you’ll avoid near-identical entries and streamline future updates to our 2FA flow.
web/src/components/common/ui/ChannelKeyDisplay.jsx (3)
92-94: Avoid unnecessary re-parsing on re-rendersparseChannelKeys runs on every render; memoize it by keyData to avoid work on large key blobs.
-import React from 'react'; +import React, { useMemo } from 'react'; ... - const parsedKeys = parseChannelKeys(keyData, t); + const parsedKeys = useMemo(() => parseChannelKeys(keyData, t), [keyData, t]);Also applies to: 20-23
151-153: Hardcoded “JSON” tag bypasses localizationYou’re calling t('JSON'), but there’s no explicit key. Either:
- Add "JSON": "JSON" to locales (so other locales can localize if needed), or
- Use a plain string if you don’t plan to translate it.
Example i18n addition:
// en.json + "JSON": "JSON",
95-103: Copy actions: consider trimming trailing whitespace before copyingSome upstream keys may contain accidental trailing newlines. Trimming before copying prevents surprises.
- const handleCopyAll = () => { - copy(keyData); + const handleCopyAll = () => { + copy((keyData || '').trim()); showSuccess(t('所有密钥已复制到剪贴板')); };- const handleCopyKey = (content) => { - copy(content); + const handleCopyKey = (content) => { + copy((content || '').trim()); showSuccess(t('密钥已复制到剪贴板')); };controller/channel.go (3)
445-460: TOTP path ignores validation errors; treat errors explicitly and reduce side channelsYou’re ignoring the error returned by ValidateTOTPAndUpdateUsage. Handle it to avoid masking internal problems and to keep behavior consistent.
- if cleanCode, err := common.ValidateNumericCode(code); err == nil { - if isValid, _ := twoFA.ValidateTOTPAndUpdateUsage(cleanCode); isValid { + if cleanCode, err := common.ValidateNumericCode(code); err == nil { + if isValid, err := twoFA.ValidateTOTPAndUpdateUsage(cleanCode); err == nil && isValid { return true } }
432-438: Audit logging: include minimal context for traceabilityLog already records userId via context; consider adding success/failure outcome and remote IP (if available) to aid incident investigations while still avoiding sensitive data in logs.
Example:
- model.RecordLog(userId, model.LogTypeSystem, fmt.Sprintf("查看渠道密钥信息 (渠道ID: %d)", channelId)) + model.RecordLog(userId, model.LogTypeSystem, fmt.Sprintf("查看渠道密钥信息成功 (渠道ID: %d)", channelId))And on failures (above returns), optionally RecordLog with “失败” if your logging policy allows.
436-443: Success message should be i18n-neutral at API layerThe API currently returns "验证成功" (Chinese). Consider returning a stable, language-neutral message or code and let the frontend localize strings.
- "message": "验证成功", + "message": "", + "code": "ok",Or reuse existing ApiSuccess helper consistently.
web/src/components/table/channels/modals/EditChannelModal.jsx (5)
165-173: Trim redundant 2FA state: remove unusedcodefield and simplify modal visibility flags
twoFAState.codeis never read; the verification flow uses the separateverifyCodestate. Keeping it risks confusion and accidental divergence.- Optional: collapse
showModal/showKeyto a single boolean (e.g.,keyModalVisible) to avoid double-truth sources for visibility.Apply this diff to drop the unused
codeproperty:const [twoFAState, setTwoFAState] = useState({ showModal: false, - code: '', loading: false, showKey: false, keyData: '' });const resetTwoFAState = () => { setTwoFAState({ showModal: false, - code: '', loading: false, showKey: false, keyData: '' }); };If you prefer a single visibility flag, I can follow up with a compact refactor that switches
visiblechecks toBoolean(twoFAState.keyData)and removesshowModal/showKey.Also applies to: 185-193
544-573: Harden 2FA verification: trim/validate input and avoid duplicate/global error handling
- Trim whitespace to tolerate pasted codes with spaces.
- Basic client-side format check reduces unnecessary requests. Assuming backup codes are numeric 8-digits and TOTPs are numeric 6-digits. If your backup codes can include letters, keep only the trim.
- Use
skipErrorHandler: trueto prevent interceptor toasts while you handle errors locally; on failure, prefershowError(error)to leverage centralized mapping inshowError(e.g., 401 redirect, 429 throttling), per helpers/utils.jsx.const handleVerify2FA = async () => { - if (!verifyCode) { + const code = (verifyCode || '').replace(/\s+/g, ''); + if (!code) { showError(t('请输入验证码或备用码')); return; } setVerifyLoading(true); try { - const res = await API.post(`/api/channel/${channelId}/key`, { - code: verifyCode - }); + // 若备份码不严格为数字，请移除下面的正则，仅保留 trim + if (!/^(?:\d{6}|\d{8})$/.test(code)) { + showError(t('验证码格式不正确，请输入6位动态码或8位备用码')); + return; + } + + const res = await API.post( + `/api/channel/${channelId}/key`, + { code }, + { skipErrorHandler: true } + ); if (res.data.success) { // 验证成功，显示密钥 updateTwoFAState({ showModal: true, showKey: true, - keyData: res.data.data.key + keyData: res?.data?.data?.key ?? res?.data?.data ?? '' }); reset2FAVerifyState(); showSuccess(t('验证成功')); } else { - showError(res.data.message); + showError(res?.data?.message || t('验证失败')); } } catch (error) { - showError(t('获取密钥失败')); + showError(error); } finally { setVerifyLoading(false); } };
576-579: Open 2FA modal with a clean slateClear any residual code/loading before showing to avoid edge cases when re-opening quickly.
const handleShow2FAModal = () => { - setShow2FAVerifyModal(true); + setVerifyCode(''); + setVerifyLoading(false); + setShow2FAVerifyModal(true); };
1170-1179: DRY the “查看密钥” trigger buttonThe same button appears 3 times. Extract a tiny local component to reduce duplication and ensure consistent behavior/styles.
Add this inside
EditChannelModal(so it can uset), near other small helpers:const ViewKeyButton = React.memo(({ onClick }) => ( <Button size="small" type="primary" theme="outline" onClick={onClick}> {t('查看密钥')} </Button> ));Then replace the three inline buttons with:
-<Button - size="small" - type="primary" - theme="outline" - onClick={handleShow2FAModal} -> - {t('查看密钥')} -</Button> +<ViewKeyButton onClick={handleShow2FAModal} />Apply the same replacement at Lines 1255-1263 and 1305-1313.
Also applies to: 1255-1263, 1305-1313
1980-2011: Make the key display modal harder to dismiss accidentally (optional) and rely on a single visibility sourceTwo minor hardening tweaks:
- Prevent mask/ESC dismissal to reduce accidental closure while reviewing sensitive info.
- Optional: use a single truth source for visibility (e.g.,
Boolean(twoFAState.keyData)), which also sidestepsshowModal && showKeydivergence.<Modal title={ <div className="flex items-center"> ... </div> } - visible={twoFAState.showModal && twoFAState.showKey} + visible={twoFAState.showModal && twoFAState.showKey} + maskClosable={false} + closeOnEsc={false} onCancel={resetTwoFAState} footer={ <Button type="primary" onClick={resetTwoFAState} > {t('完成')} </Button> } width={700} style={{ maxWidth: '90vw' }} >If you decide to consolidate visibility, change the
visibleprop toBoolean(twoFAState.keyData)and set/clearkeyDataaccordingly on success/cancel.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
controller/channel.go(1 hunks)router/api-router.go(1 hunks)web/src/components/common/modals/TwoFactorAuthModal.jsx(1 hunks)web/src/components/common/ui/ChannelKeyDisplay.jsx(1 hunks)web/src/components/table/channels/modals/EditChannelModal.jsx(8 hunks)web/src/i18n/locales/en.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
web/src/helpers/utils.jsx (2)
showError(113-142)showSuccess(148-150)
🔇 Additional comments (4)
web/src/components/common/ui/ChannelKeyDisplay.jsx (1)
31-72: Robust key parsing with graceful fallback looks goodGood handling of JSON arrays, multiline, and single key inputs with sensible labels and types; the console.warn fallback on JSON parse failure is appropriate.
router/api-router.go (1)
109-137: Route placement and auth look correctThe new POST /api/channel/:id/key lives under AdminAuth within /channel, which aligns with the security posture for viewing secrets.
web/src/components/table/channels/modals/EditChannelModal.jsx (2)
661-685: LGTM: comprehensive modal reset including sensitive stateGood call centralizing all resets, especially clearing
keyDataon close viaresetTwoFAState, which reduces accidental leakage of sensitive info across sessions.
1967-1977: 2FA modal wiring looks correctProps cover loading state, controlled code input, and explicit cancel path that resets state. No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
router/api-router.go (1)
117-117: Resolved: Critical rate limiter added to key-reveal route — LGTMThe sensitive POST /:id/key endpoint is now protected with middleware.CriticalRateLimit() and sits under AdminAuth(). This directly addresses the earlier suggestion to harden the route against brute-force/rapid retries. Good call keeping it POST-only for submitting the 2FA code in the body.
🧹 Nitpick comments (1)
router/api-router.go (1)
117-117: Consider CSRF mitigation for the session-based secret-reveal endpointAlthough your AdminAuth middleware does use cookie-based sessions, you’ve configured the session cookie with
SameSite: http.SameSiteStrictMode, which prevents browsers from sending it on cross-site requests, effectively mitigating CSRF on modern browsers. No additional CSRF middleware or token checks were found.• File to review:
router/api-router.goline 117 (channelRoute.POST("/:id/key", …))
• Session store setup inmain.gouses SameSite Strict (see lines 142–150)
• No explicit CSRF token generation/validation middleware (e.g. gin-contrib/csrf) detectedOptional recommendation:
• If you need to support older browsers lacking SameSite Strict enforcement, or want defense-in-depth, consider adding an explicit CSRF token check before revealing secrets (e.g. double-submit cookie or hidden form field validated on the server). Rate limiting alone is not a substitute for CSRF protection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
router/api-router.go(1 hunks)web/src/components/common/modals/TwoFactorAuthModal.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/common/modals/TwoFactorAuthModal.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
router/api-router.go (1)
controller/channel.go (1)
GetChannelKey(384-443)
2 participants
关闭 #1640
Summary by CodeRabbit