feat(auth): enhance IP restriction handling with CIDR support - #2445
Conversation
WalkthroughCentralized IP utilities and added CIDR-aware matching; removed duplicate IsIP. Token model now returns an IP slice. Auth middleware validates parsed client IP and enforces CIDR-based allowlist checks. SSRF protection delegates to the new CIDR helper. UI labels and translations updated to indicate CIDR support and gateway guidance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
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)
🧰 Additional context used🧬 Code graph analysis (1)middleware/auth.go (2)
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
🧹 Nitpick comments (4)
common/ip.go (1)
33-50: CIDR + single‑IP matching logic is correct; consider minor naming cleanup
IsIpInCIDRListcorrectly:
- treats each item as CIDR first and falls back to exact IP match on parse failure,
- safely ignores malformed entries.
You might optionally rename it to
IsIPInCIDRListfor consistency withIsIPand to match Go’s usual acronym casing, but this is non‑blocking.model/token.go (1)
37-57: GetIpLimits behavior is sound; small cleanups are optionalThe method:
- Safely handles
nil/ emptyAllowIps,- Normalizes and filters out empty entries,
- Leaves actual validation to the shared CIDR helper.
If you want to simplify later, you could drop the global
ReplaceAll(*token.AllowIps, " ", "")and just trim per line, but current logic is functionally fine.web/src/i18n/locales/fr.json (1)
1765-1765: Minor French style & capitalization improvement for the warningCurrent text is correct but can read a bit more natural in French and normalize “Nginx/CDN” capitalization.
- "请勿过度信任此功能,IP可能被伪造,请配合nginx和cdn等网关使用": "Ne faites pas trop confiance à cette fonctionnalité, l'IP peut être usurpée, veuillez l'utiliser en conjonction avec des passerelles telles que nginx et cdn", + "请勿过度信任此功能,IP可能被伪造,请配合nginx和cdn等网关使用": "Ne faites pas une confiance excessive à cette fonctionnalité : l’adresse IP peut être usurpée. Utilisez‑la conjointement avec une passerelle (reverse proxy) comme Nginx ou un CDN.",web/src/i18n/locales/vi.json (1)
1990-1990: Optional improvement to wording and gateway terminologyThe translation is understandable and accurate. To improve clarity and match common terminology, you could name reverse proxies/CDN explicitly and capitalize them.
- "请勿过度信任此功能,IP可能被伪造,请配合nginx和cdn等网关使用": "Đừng quá tin tưởng tính năng này, IP có thể bị giả mạo, vui lòng sử dụng cùng với nginx và các cổng khác như cdn", + "请勿过度信任此功能,IP可能被伪造,请配合nginx和cdn等网关使用": "Đừng quá tin tưởng tính năng này, IP có thể bị giả mạo, vui lòng sử dụng cùng với các gateway/ngược proxy như Nginx hoặc một CDN.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
common/ip.go(2 hunks)common/ssrf_protection.go(1 hunks)common/utils.go(0 hunks)middleware/auth.go(2 hunks)model/token.go(1 hunks)web/src/components/table/tokens/modals/EditTokenModal.jsx(1 hunks)web/src/i18n/locales/en.json(2 hunks)web/src/i18n/locales/fr.json(2 hunks)web/src/i18n/locales/ja.json(2 hunks)web/src/i18n/locales/ru.json(2 hunks)web/src/i18n/locales/vi.json(2 hunks)web/src/i18n/locales/zh.json(2 hunks)
💤 Files with no reviewable changes (1)
- common/utils.go
🧰 Additional context used
🧬 Code graph analysis (1)
middleware/auth.go (2)
logger/logger.go (1)
LogDebug(69-76)common/ip.go (2)
ParseIP(10-12)IsIpInCIDRList(33-51)
🔇 Additional comments (9)
common/ip.go (1)
5-12: Centralized IP parse/validate helpers look goodWrapping
net.ParseIPinIsIP/ParseIPis straightforward and keeps IP handling in one place; API and behavior are clear and correct.common/ssrf_protection.go (1)
184-190: Delegating IP matching toIsIpInCIDRListis a clean refactorReusing
IsIpInCIDRListkeeps SSRF IP allow/deny logic centralized while preserving empty‑list semantics; no issues spotted.web/src/i18n/locales/ru.json (1)
104-104: RU translations correctly reflect CIDR support and gateway guidanceThe updated Russian strings for the IP whitelist label and the spoofing warning accurately capture “CIDR expressions” support and recommend using nginx/CDN gateways; looks consistent with other locales.
Also applies to: 1776-1776
web/src/i18n/locales/ja.json (1)
85-85: JA translations for CIDR label and IP spoofing warning look accurate
IPホワイトリスト(CIDR表記に対応)and the updated caution about spoofable IPs + nginx/CDN gateways read naturally in Japanese and match the intended meaning.Also applies to: 1672-1672
web/src/i18n/locales/zh.json (1)
98-98: Localization updates properly reflect CIDR support and security guidance.The Chinese locale updates clearly communicate CIDR notation support and include important security warnings about IP spoofing with gateway integration recommendations.
Also applies to: 1743-1743
web/src/components/table/tokens/modals/EditTokenModal.jsx (1)
560-560: Translation key references are properly implemented using react-i18next.The form field label and helper text correctly use the
t()function to reference the localized strings. The changes maintain consistency with other form fields in the component and properly expose CIDR support information to users.Please verify that all supported locale files (French, Japanese, Russian, Vietnamese, etc.) have been updated with the corresponding translations for these two keys:
"IP白名单(支持CIDR表达式)""请勿过度信任此功能,IP可能被伪造,请配合nginx和cdn等网关使用"The enriched summary indicates these updates were made across multiple locales, but this review can only confirm the Chinese and English translations provided.
Also applies to: 564-564
web/src/i18n/locales/en.json (1)
100-100: English translations accurately convey CIDR support messaging and security guidance.The English translations properly localize both the label update and the expanded security guidance. The messaging is clear, professional, and maintains consistency with the original Chinese intent while using natural English phrasing.
Also applies to: 1755-1755
web/src/i18n/locales/fr.json (1)
102-102: CIDR label translation looks goodThe wording is clear, technically accurate, and consistent with the Chinese source; no changes needed.
web/src/i18n/locales/vi.json (1)
85-85: CIDR label translation is accurateThe Vietnamese string correctly conveys IP allowlist semantics and CIDR support; no adjustments needed.
…itelist-cidr feat(auth): enhance IP restriction handling with CIDR support
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.