PR: Add workflow to check for allowlist#16
Conversation
A workflow was added as an example of checking against an allowlist for IP addresses to restrict authentication.
|
Warning Rate limit exceeded@ahn-nath has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new post-authentication workflow that enforces an IPv4 allowlist: exposes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AuthSvc as Authentication Service
participant Workflow as PostAuth Allowlist Workflow
participant Validator as IP/Allowlist Validator
participant Logger as Logger
participant Policy as Access Control
AuthSvc->>Workflow: onPostAuthentication(event)
Workflow->>Validator: validateAllowList(allowList)
Validator-->>Workflow: valid / invalid
alt allowlist invalid
Workflow->>Logger: error("Allowlist misconfigured")
Workflow->>Policy: denyAccess("Allowlist misconfigured")
Policy-->>AuthSvc: Deny
else allowlist valid
Workflow->>Workflow: extractClientIp(event.request.ip, testFalsePositive?)
alt IP missing or malformed
Workflow->>Logger: warn("Missing/invalid IP")
Workflow->>Policy: denyAccess("IP invalid or not allowed")
Policy-->>AuthSvc: Deny
else IP parsed
Workflow->>Validator: isValidIpAddress(ip)
Validator-->>Workflow: valid / invalid
alt IP invalid
Workflow->>Logger: warn("Invalid or private IP")
Workflow->>Policy: denyAccess("IP invalid")
Policy-->>AuthSvc: Deny
else IP valid
Workflow->>Validator: isAllowed(ip, allowList)
alt not allowlisted
Workflow->>Policy: denyAccess("IP not allowlisted")
Policy-->>AuthSvc: Deny
else allowlisted
Workflow-->>AuthSvc: Continue (no deny)
end
end
end
end
opt unexpected exception
Workflow->>Logger: error("Exception")
Workflow->>Policy: denyAccess("Unexpected error")
Policy-->>AuthSvc: Deny
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (4)
84-87: Fix typo and standardize error log.Spelling: “Againts” → “Against”.
- console.error(`Check Againts IP Address Workflow Error: ${errorMessage}`, error); + console.error(`Check Against IP Address Workflow Error: ${errorMessage}`, error);
106-106: Add explicit return type for handler.Improves clarity and prevents accidental return type drift.
-export default async function handlePostAuth(event: onPostAuthenticationEvent) { +export default async function handlePostAuth(event: onPostAuthenticationEvent): Promise<void> {
41-44: Externalize configuration (allowList and test toggle).Hardcoding IPs and a test flag in code risks drift and accidental exposure. Move to env/config/bindings with validation and safe defaults.
I can refactor to read from workflow bindings or environment, validate on startup, and surface configuration errors via deployment logs rather than at request time.
127-137: Observability: structured logs and context.Use structured logs (JSON) with fields like workflowId, userId (if available), requestId, ip, decision to improve traceability.
I can wire a small helper
logDecision({ ip, decision, reason })to standardize messages and avoid leaking details to users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts(1 hunks)
🔇 Additional comments (2)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (2)
120-136: Confirm denyAccess contract (return/await semantics).If
denyAccessis async or throws, considerreturn denyAccess(...)orawait denyAccess(...)to guarantee early termination. You currently call thenreturn;.Would you confirm the function type of
denyAccessfrom@kinde/infrastructure? If it returns a Promise, wrapping withreturn await(or justreturn) clarifies intent.
94-102: IPv6 handling (verify requirements).
event.request.ipmay be IPv6 or IPv4‑mapped (e.g.,::ffff:64.227.0.197). Current validator rejects these, potentially denying legitimate users.If IPv6 must be supported now, I can provide a compact IPv6 validator and normalization for IPv4‑mapped IPv6 before allowlist comparison. Preferably, allowlist entries should support both canonical IPv6 and IPv4‑mapped forms.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
📚 Learning: 2025-07-21T05:31:53.656Z
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
Applied to files:
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (5)
85-85: Fix typo in error log message."Againts" should be "Against".
Apply this diff:
- console.error(`Check Againts IP Address Workflow Error: ${errorMessage}`, error); + console.error(`Check Against IP Address Workflow Error: ${errorMessage}`, error);
84-87: Avoid exposing internal error details to end users.The
denyAccesscall includes the rawerrorMessagewhich may leak implementation details. Log technical details for ops but return a generic message to users.Apply this diff:
function handleExceptionError(errorMessage: string, error?: any): void { console.error(`Check Against IP Address Workflow Error: ${errorMessage}`, error); - denyAccess(`Access blocked due to an issue: ${errorMessage}`); + denyAccess('Access denied due to a configuration or validation error.'); }
123-123: Bug: Possible crash when event.request is undefined.
event.request.ip?.split(…)still dereferencesrequestbefore checkingip, which can throw ifrequestis undefined.Apply this diff:
- let ip = event.request.ip?.split(',')[0].trim() ?? 'unknown'; + const rawIp = event.request?.ip ?? ''; + let ip = rawIp.split(',')[0]?.trim() || 'unknown';
131-132: Avoid exposing IPs in end-user messages (privacy/compliance).The actual IP address is included in the user-facing
denyAccessmessage. Log the IP for ops but return a generic message to users.Apply this diff:
- console.warn(`Invalid or private IP address detected: ${ip}. Access denied.`); - denyAccess(`Access denied: Invalid or private IP address.`); + console.warn(`Invalid or private IP address detected: ${ip}. Denying access.`); + denyAccess('Access denied: Invalid IP address.');
140-141: Avoid exposing IPs in end-user messages (privacy/compliance).The actual IP address is included in the user-facing
denyAccessmessage. This may violate privacy policies. Log the IP internally but provide a generic denial message to users.Apply this diff:
- console.warn(`IP address ${ip} is not in the allowlist. Access denied.`); - denyAccess(`Access denied: IP address ${ip} is not in the allowlist.`); + console.warn(`IP not in allowlist: ${ip}. Denying access.`); + denyAccess('Access denied: Your IP is not authorized.');
🧹 Nitpick comments (3)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (3)
45-45: Good fix: testFalsePositive now defaults to false.The previous blocker (testFalsePositive = true) has been resolved. For additional production safety, consider reading from environment variables with explicit guards:
-const testFalsePositive = false; +const testFalsePositive = + process.env.ALLOWLIST_TEST_FALSE_POSITIVE === 'true' && + process.env.NODE_ENV !== 'production';
94-111: Good implementation of private IP range validation.The private and reserved IPv4 range checks requested in previous reviews have been properly implemented. The function now correctly rejects private (RFC1918), loopback, and link-local addresses.
For completeness, consider also rejecting these additional reserved ranges:
if (a === 169 && b === 254) return false; // 169.254.0.0/16 + if (a === 0) return false; // 0.0.0.0/8 + if (a >= 224) return false; // 224.0.0.0/4 (multicast) and above return true;Based on learnings.
124-127: Use console.warn for test override to highlight non-production usage.The test override currently uses
console.log, butconsole.warnwould better signal this is a development-only feature that should not appear in production logs.Apply this diff:
if (testFalsePositive) { ip = '64.227.0.197'; // A known "allowed" IP for testing purposes - console.log('Test false positive is enabled. Overriding IP for testing purposes.'); + console.warn('testFalsePositive is enabled. Overriding client IP for DEV verification ONLY.'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
📚 Learning: 2025-07-21T05:31:53.656Z
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
Applied to files:
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts
🔇 Additional comments (3)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (3)
33-38: LGTM!Imports are appropriate and all are used in the implementation.
49-57: LGTM!Workflow settings are correctly configured for post-authentication with appropriate failure policy and bindings.
66-77: LGTM!Validation logic is sound and provides clear error messages for configuration issues.
We are adding another check, in case the request object comes as undefined.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (5)
20-21: Documentation mentions environment variable but implementation uses hardcoded constant.The documentation references
ALLOWLIST_TEST_FALSE_POSITIVEenvironment variable, but the implementation uses a hardcodedtestFalsePositiveconstant on line 45. This inconsistency has been previously flagged.
85-85: Fix typo in error log message."Againts" should be "Against". This issue was previously flagged.
84-87: Avoid exposing internal error details to end users.The
denyAccesscall includes the rawerrorMessagewhich may leak implementation details. This issue was previously flagged.
132-135: Avoid exposing IPs in end-user messages (privacy/compliance).The IP address is included in the
denyAccessmessage shown to end users. This issue was previously flagged.
141-145: Avoid exposing IPs in end-user messages (privacy/compliance).The IP address is included in the
denyAccessmessage shown to end users. This issue was previously flagged.
🧹 Nitpick comments (2)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (2)
94-111: LGTM! Private IP range validation is now implemented.The function correctly validates IPv4 format and rejects private/reserved ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16). IPv4-only validation is intentional for this infrastructure.
Optional enhancement: Consider adding checks for multicast (224.0.0.0/4), broadcast (255.255.255.255), and 0.0.0.0/8 ranges for completeness.
126-129: Use console.warn for security override notification.The test override bypasses IP validation, which is a security-sensitive operation. Using
console.warninstead ofconsole.logensures better visibility in production logs.Apply this diff:
if (testFalsePositive) { ip = '64.227.0.197'; // A known "allowed" IP for testing purposes - console.log('Test false positive is enabled. Overriding IP for testing purposes.'); + console.warn('Test false positive is enabled. Overriding IP for testing purposes.'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
📚 Learning: 2025-07-21T05:31:53.656Z
Learnt from: alex72508
Repo: kinde-starter-kits/workflow-examples PR: 9
File: postUserAuthentication/checkIPWithAbuseIPDBWorkflow.ts:107-115
Timestamp: 2025-07-21T05:31:53.656Z
Learning: The workflow-examples repository uses IPv4-only validation in IP address checking functions because IPv6 is not supported across all systems in their infrastructure.
Applied to files:
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts
🔇 Additional comments (5)
postUserAuthentication/checkIPAgainstAllowlistWorkflow.ts (5)
33-45: LGTM! Configuration defaults are secure.The imports are correct and the
testFalsePositiveflag now defaults tofalse, addressing the critical security concern from previous reviews. The allowlist structure is clear and ready for production IP addresses.
49-57: LGTM! Workflow settings are correctly configured.The workflow settings appropriately use
WorkflowTrigger.PostAuthenticationwith astopfailure policy, which is the correct choice for a security-critical IP allowlist check.
66-77: LGTM! Allowlist validation is thorough.The function correctly validates both the array structure and each IP address entry. The error messages are descriptive for troubleshooting.
123-124: LGTM! IP extraction is now safe.The optional chaining on
event.request?.ipand safe string handling with fallback to'unknown'correctly addresses the previous crash risk.
150-152: LGTM! Error handling structure is appropriate.The catch block correctly delegates to the centralized error handler, ensuring consistent logging and access denial for unexpected errors.
|
@onderay |
2 participants
Explain your changes
Summary
Adds a Post-Authentication workflow that enforces an IP allowlist. The workflow validates the configured allowlist, extracts and validates the incoming client IP from the authentication event, supports a developer test override mode, and either allows the session to continue or denies access via
denyAccess(...). It includes clear logging and centralized error handling.Files changed
checkIPAgainstAllowlistWorkflow.ts.ts(new workflow implementation)What this implements
Runs on
PostAuthenticationevents (workflow id:"onPostUserAuthentication", name:"checkIPAgainstAllowlist").Primary behavior
allowListis a non-empty array of valid IPv4 addresses (validateAllowList).event.request.ip.'unknown','localhost', and'127.0.0.1'.testFalsePositiveistrue, overrides the IP with a known test IP (dev-only convenience).allowList, denies access by calling platformdenyAccess(...).handleExceptionError(logs error and denies access).Workflow settings
trigger: WorkflowTrigger.PostAuthenticationfailurePolicy: { action: 'stop' }bindings: { "kinde.auth": {} }Logging & errors
In-file configuration
allowListarray andtestFalsePositiveflag are currently defined in-file (clear comments recommend moving to env/config).Why this change
This enforces a simple, deterministic allowlist check as part of post-authentication flows. It's useful when you want to restrict access to a known set of client IP addresses (e.g., corporate networks, specific partners) and fail closed (deny access) when there are configuration problems or runtime errors.
Checklist
Summary by CodeRabbit