Add validation for API key IP and Referrer restrictions in DevPortal#1393
Add validation for API key IP and Referrer restrictions in DevPortal#1393Gayuth-W wants to merge 9 commits into
Conversation
- Add isValidPermittedIPList and isValidPermittedRefererList for validating DevPortal API key Security Restriction inputs, with rules derived from what the gateway accepts (ApiKeyAuthenticatorUtils#validateAPIKeyRestrictions): - IP: comma-separated IPv4/IPv6 addresses or CIDR ranges, using the same address patterns as the Admin Portal blacklist form - Referrer: comma-separated URL/host/wildcard patterns; the gateway matches entries literally with '*' wildcards, so each entry must be non-empty, whitespace-free, and contain one of '.', ':', '/', '*'
- Reject invalid Security Restriction values before generating an API key, in both the per-API key dialog (ApiKeyGenerate/ApiKeyListing) and the legacy application key dialog (LegacyApiKeys). Previously any string was accepted and a key was generated with a restriction that could never match. - Show an inline field error alongside the existing alert toast, clear it when the value or restriction type changes, and update the field placeholders to document the accepted formats.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded shared validation for IP/CIDR and referrer restrictions during standard and legacy API key generation. Invalid values now stop generation, show localized alerts, and display inline field errors. Placeholders and unit tests cover supported list, CIDR, IPv6, and wildcard formats. ChangesAPI key restriction validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant APIKeyForm
participant constraintValidator
participant Alert
APIKeyForm->>constraintValidator: validate selected IP or referrer restriction
constraintValidator-->>APIKeyForm: return localized validation result
APIKeyForm->>Alert: show error for invalid input
APIKeyForm-->>APIKeyForm: render inline validation feedback
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
… into fix/validate-ip
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx (1)
238-241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrim the restriction value for consistency.
In
ApiKeyGenerate.jsx,.trim()is applied to the restriction value when constructing the finalrestrictionspayload. Consider applying.trim()here as well to maintain consistency between the standard and legacy key generation flows, preventing strings with leading or trailing whitespaces from being sent to the backend.♻️ Proposed refactor
const restrictions = { - permittedIP: restrictionType === 'ip' ? restrictionValue : '', - permittedReferer: restrictionType === 'referrer' ? restrictionValue : '', + permittedIP: restrictionType === 'ip' ? restrictionValue.trim() : '', + permittedReferer: restrictionType === 'referrer' ? restrictionValue.trim() : '', };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx` around lines 238 - 241, Update the restrictions construction in the legacy key generation flow to trim restrictionValue before assigning permittedIP or permittedReferer, matching the behavior in ApiKeyGenerate.jsx and ensuring surrounding whitespace is excluded from the backend payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx`:
- Around line 238-241: Update the restrictions construction in the legacy key
generation flow to trim restrictionValue before assigning permittedIP or
permittedReferer, matching the behavior in ApiKeyGenerate.jsx and ensuring
surrounding whitespace is excluded from the backend payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 051c3c9f-2007-456e-8342-aedc7c45e17e
📒 Files selected for processing (4)
portals/devportal/src/main/webapp/site/public/locales/en.jsonportals/devportal/src/main/webapp/source/src/app/components/Apis/Details/APIKeys/ApiKeyGenerate.jsxportals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsxportals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/constraintValidator.js
- Shorten the comment blocks in constraintValidator and add a doc comment to validateRestrictionOrAlert
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx (2)
242-245: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueTrim restriction values for consistency.
In
ApiKeyGenerate.jsx,restrictionValueis trimmed before being added to therestrictionsobject. Consider trimming it here as well to ensure consistent behavior across both generation flows and to prevent trailing whitespace from being sent in the API payload.♻️ Proposed refactor
const restrictions = { - permittedIP: restrictionType === 'ip' ? restrictionValue : '', - permittedReferer: restrictionType === 'referrer' ? restrictionValue : '', + permittedIP: restrictionType === 'ip' ? restrictionValue.trim() : '', + permittedReferer: restrictionType === 'referrer' ? restrictionValue.trim() : '', };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx` around lines 242 - 245, Update the restrictions construction in the legacy API key generation flow to trim restrictionValue before assigning it to permittedIP or permittedReferer, matching the behavior in ApiKeyGenerate.jsx and ensuring whitespace is excluded from the API payload.
69-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deduplicating
validateRestrictionOrAlert.This helper function appears identical to the one implemented in
ApiKeyGenerate.jsx. Consider moving it to a shared utility file (or refactoring the components to just callvalidateRestrictionValueand handle the UI state themselves) to avoid code duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx` around lines 69 - 77, Deduplicate validateRestrictionOrAlert by extracting the shared validation-and-alert behavior into a common utility, or by having both components call validateRestrictionValue while handling their own UI state consistently. Update the implementations in LegacyApiKeys.jsx and ApiKeyGenerate.jsx to use the shared approach, preserving setRestrictionError, Alert.error, and boolean return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Apis/Details/APIKeys/ApiKeyGenerate.jsx`:
- Line 172: Update the submission flow around validateRestrictionOrAlert to
clear the existing restrictionError before validation or immediately after
successful validation, while preserving the early return for invalid
restrictions so stale inline errors disappear once corrected.
---
Nitpick comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsx`:
- Around line 242-245: Update the restrictions construction in the legacy API
key generation flow to trim restrictionValue before assigning it to permittedIP
or permittedReferer, matching the behavior in ApiKeyGenerate.jsx and ensuring
whitespace is excluded from the API payload.
- Around line 69-77: Deduplicate validateRestrictionOrAlert by extracting the
shared validation-and-alert behavior into a common utility, or by having both
components call validateRestrictionValue while handling their own UI state
consistently. Update the implementations in LegacyApiKeys.jsx and
ApiKeyGenerate.jsx to use the shared approach, preserving setRestrictionError,
Alert.error, and boolean return behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 177d151b-888e-4d9c-8d21-2b91c7231f86
📒 Files selected for processing (3)
portals/devportal/src/main/webapp/source/src/app/components/Apis/Details/APIKeys/ApiKeyGenerate.jsxportals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/LegacyApiKeys.jsxportals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/constraintValidator.js
🚧 Files skipped from review as they are similar to previous changes (1)
- portals/devportal/src/main/webapp/source/src/app/components/Shared/AppsAndKeys/constraintValidator.js
|
1 participant
Purpose
The DevPortal API key generation dialogs accept any string as a Security Restriction value - a key generates successfully with an IP or Referrer restriction that can never match, silently producing an unusable or falsely-restricted key.
Approach
Added
isValidPermittedIPList/isValidPermittedRefererListto the existingShared/AppsAndKeys/constraintValidator.jsand wired them into both key generation dialogs (ApiKeyGenerate/ApiKeyListingandLegacyApiKeys) with an inline field error alongside the existing toast. Rules mirror what the gateway accepts (ApiKeyAuthenticatorUtils#validateAPIKeyRestrictions):. : / *Covered by unit tests (
npx jest: 74/74 passing); verified manually on a local APIM 4.7.0 instance.Fixes wso2/api-manager#5093