feat: Add support for whitelisted parent frame origins - Cashout#575
feat: Add support for whitelisted parent frame origins - Cashout#575Hugo0 merged 4 commits intopeanutprotocol:mainfrom
Conversation
|
@ardaerturk is attempting to deploy a commit to the squirrellabs Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@ardaerturk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 43 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)
WalkthroughThe changes focus on enhancing iframe domain handling in the cashout utility module. A new constant Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Utils as Cashout Utils
participant Window as Browser Window
App->>Utils: Call convertPersonaUrl()
Utils->>Window: Check isInAllowedFrame()
Window-->>Utils: Return iframe status
alt Is in Allowed Frame
Utils->>Utils: Use referrer origin
else Not in Allowed Frame
Utils->>Utils: Use current window origin
end
Utils->>App: Return constructed URL
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (1)
src/utils/cashout.utils.ts (1)
8-8: Consider enhancing domain whitelist security and maintainability.
- Make the array immutable to prevent runtime modifications
- Consider moving to a configuration file for better maintainability
-const ALLOWED_PARENT_DOMAINS = ['intersend.io', 'app.intersend.io']; +const ALLOWED_PARENT_DOMAINS = Object.freeze(['intersend.io', 'app.intersend.io']) as ReadonlyArray<string>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/cashout.utils.ts(1 hunks)
🔇 Additional comments (1)
src/utils/cashout.utils.ts (1)
8-42: Verify iframe integration changes across the codebase.
Let's verify the impact of these changes:
- Usage of convertPersonaUrl function
- Other potential iframe integrations that might need similar protection
✅ Verification successful
Changes to iframe integration are properly scoped and secure
The changes to convertPersonaUrl function are properly integrated and the iframe security measures are well implemented:
- The
convertPersonaUrlfunction is only used in the KYC flow withinKYCComponent - The
IframeWrappercomponent already implements proper security measures:- Uses appropriate sandbox attributes
- Has proper origin checks for postMessage
- Implements proper iframe cleanup
The new domain allowlist and frame detection in cashout.utils.ts adds an extra layer of security without conflicting with existing implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other iframe integrations that might need protection
echo "Checking for iframe-related code..."
rg -l "(?i)(iframe|frame|parent\.|top\.|postMessage)" --type ts --type tsx
echo "Checking for Persona Bridge widget usage..."
rg -l "bridge\.withpersona\.com" --type ts --type tsx
echo "Checking for convertPersonaUrl usage..."
rg -l "convertPersonaUrl" --type ts --type tsx
Length of output: 530
Script:
#!/bin/bash
# Check for other iframe integrations that might need protection
echo "Checking for iframe-related code..."
rg -l "(?i)(iframe|frame|parent\.|top\.|postMessage)" -t typescript -t javascriptreact -t typescriptreact
echo "Checking for Persona Bridge widget usage..."
rg -l "bridge\.withpersona\.com" -t typescript -t javascriptreact -t typescriptreact
echo "Checking for convertPersonaUrl usage..."
rg -l "convertPersonaUrl" -t typescript -t javascriptreact -t typescriptreact
# Let's also check the content of files that use convertPersonaUrl
echo "Checking convertPersonaUrl implementation details..."
rg "convertPersonaUrl" -t typescript -t javascriptreact -t typescriptreact -C 3
Length of output: 874
Script:
#!/bin/bash
# Check for other iframe integrations that might need protection
echo "Checking for iframe-related code..."
rg -l "(?i)(iframe|frame|parent\.|top\.|postMessage)"
echo "Checking for Persona Bridge widget usage..."
rg -l "bridge\.withpersona\.com"
echo "Checking for convertPersonaUrl usage..."
rg -l "convertPersonaUrl"
# Let's also check the content of files that use convertPersonaUrl
echo "Checking convertPersonaUrl implementation details..."
rg "convertPersonaUrl" -C 3
Length of output: 2241
Script:
#!/bin/bash
# Let's check the IframeWrapper component since it might be relevant
echo "Checking IframeWrapper implementation..."
cat src/components/Global/IframeWrapper/index.tsx
# Let's also check how KYCComponent uses the iframe
echo "Checking KYCComponent implementation..."
cat src/components/Global/KYCComponent/index.tsx
Length of output: 26743
src/utils/cashout.utils.ts
Outdated
| // Use parent frame origin if in allowed iframe, otherwise use current origin | ||
| const origin = encodeURIComponent( | ||
| isInAllowedFrame() | ||
| ? new URL(document.referrer).origin | ||
| : window.location.origin | ||
| ); | ||
|
|
||
| return `https://bridge.withpersona.com/widget?environment=production&inquiry-template-id=${templateId}&fields[iqt_token=${iqtToken}&iframe-origin=${origin}&redirect-uri=${origin}&fields[developer_id]=${developerId}&reference-id=${referenceId}`; |
There was a problem hiding this comment.
Fix security vulnerabilities in URL construction.
The current implementation has several issues:
- Missing input validation
- Inconsistent parameter encoding
- Malformed query parameter for
iqt_token
Apply this diff to fix the issues:
// Use parent frame origin if in allowed iframe, otherwise use current origin
const origin = encodeURIComponent(
isInAllowedFrame()
? new URL(document.referrer).origin
: window.location.origin
);
- return `https://bridge.withpersona.com/widget?environment=production&inquiry-template-id=${templateId}&fields[iqt_token=${iqtToken}&iframe-origin=${origin}&redirect-uri=${origin}&fields[developer_id]=${developerId}&reference-id=${referenceId}`;
+ // Validate required parameters
+ if (!templateId || !iqtToken || !developerId || !referenceId) {
+ throw new Error('Missing required parameters for Persona URL');
+ }
+
+ const params = new URLSearchParams({
+ 'environment': 'production',
+ 'inquiry-template-id': templateId,
+ 'fields[iqt_token]': iqtToken,
+ 'iframe-origin': origin,
+ 'redirect-uri': origin,
+ 'fields[developer_id]': developerId,
+ 'reference-id': referenceId
+ });
+
+ return `https://bridge.withpersona.com/widget?${params.toString()}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Use parent frame origin if in allowed iframe, otherwise use current origin | |
| const origin = encodeURIComponent( | |
| isInAllowedFrame() | |
| ? new URL(document.referrer).origin | |
| : window.location.origin | |
| ); | |
| return `https://bridge.withpersona.com/widget?environment=production&inquiry-template-id=${templateId}&fields[iqt_token=${iqtToken}&iframe-origin=${origin}&redirect-uri=${origin}&fields[developer_id]=${developerId}&reference-id=${referenceId}`; | |
| // Use parent frame origin if in allowed iframe, otherwise use current origin | |
| const origin = encodeURIComponent( | |
| isInAllowedFrame() | |
| ? new URL(document.referrer).origin | |
| : window.location.origin | |
| ); | |
| // Validate required parameters | |
| if (!templateId || !iqtToken || !developerId || !referenceId) { | |
| throw new Error('Missing required parameters for Persona URL'); | |
| } | |
| const params = new URLSearchParams({ | |
| 'environment': 'production', | |
| 'inquiry-template-id': templateId, | |
| 'fields[iqt_token]': iqtToken, | |
| 'iframe-origin': origin, | |
| 'redirect-uri': origin, | |
| 'fields[developer_id]': developerId, | |
| 'reference-id': referenceId | |
| }); | |
| return `https://bridge.withpersona.com/widget?${params.toString()}`; |
src/utils/cashout.utils.ts
Outdated
| const isInAllowedFrame = (): boolean => { | ||
| if (window.location === window.parent.location) return false; | ||
|
|
||
| // Check ancestor origins (modern browsers) | ||
| if (window.location.ancestorOrigins?.length) { | ||
| return ALLOWED_PARENT_DOMAINS.some(domain => | ||
| window.location.ancestorOrigins[0].includes(domain) | ||
| ); | ||
| } | ||
|
|
||
| // Fallback to referrer check | ||
| return ALLOWED_PARENT_DOMAINS.some(domain => | ||
| document.referrer.includes(domain) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Fix security vulnerability in domain validation.
The current domain validation using includes() is vulnerable to domain spoofing. For example, 'malicious-intersend.io' would pass the check.
Apply this diff to implement strict domain matching:
const isInAllowedFrame = (): boolean => {
if (window.location === window.parent.location) return false;
// Check ancestor origins (modern browsers)
if (window.location.ancestorOrigins?.length) {
- return ALLOWED_PARENT_DOMAINS.some(domain =>
- window.location.ancestorOrigins[0].includes(domain)
- );
+ const parentOrigin = window.location.ancestorOrigins[0];
+ return ALLOWED_PARENT_DOMAINS.some(domain => {
+ try {
+ const parentDomain = new URL(parentOrigin).hostname;
+ return parentDomain === domain;
+ } catch {
+ return false;
+ }
+ });
}
// Fallback to referrer check
- return ALLOWED_PARENT_DOMAINS.some(domain =>
- document.referrer.includes(domain)
- );
+ try {
+ const referrerDomain = new URL(document.referrer).hostname;
+ return ALLOWED_PARENT_DOMAINS.some(domain => domain === referrerDomain);
+ } catch {
+ return false;
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isInAllowedFrame = (): boolean => { | |
| if (window.location === window.parent.location) return false; | |
| // Check ancestor origins (modern browsers) | |
| if (window.location.ancestorOrigins?.length) { | |
| return ALLOWED_PARENT_DOMAINS.some(domain => | |
| window.location.ancestorOrigins[0].includes(domain) | |
| ); | |
| } | |
| // Fallback to referrer check | |
| return ALLOWED_PARENT_DOMAINS.some(domain => | |
| document.referrer.includes(domain) | |
| ); | |
| }; | |
| const isInAllowedFrame = (): boolean => { | |
| if (window.location === window.parent.location) return false; | |
| // Check ancestor origins (modern browsers) | |
| if (window.location.ancestorOrigins?.length) { | |
| const parentOrigin = window.location.ancestorOrigins[0]; | |
| return ALLOWED_PARENT_DOMAINS.some(domain => { | |
| try { | |
| const parentDomain = new URL(parentOrigin).hostname; | |
| return parentDomain === domain; | |
| } catch { | |
| return false; | |
| } | |
| }); | |
| } | |
| // Fallback to referrer check | |
| try { | |
| const referrerDomain = new URL(document.referrer).hostname; | |
| return ALLOWED_PARENT_DOMAINS.some(domain => domain === referrerDomain); | |
| } catch { | |
| return false; | |
| } | |
| }; |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2 participants
Description
This PR adds support for handling iframe integration with whitelisted parent domains. When the application is embedded within an allowed parent frame, it will properly use the parent's origin for Bridge widget integration.
Changes
ALLOWED_PARENT_DOMAINSconstant to maintain a list of trusted parent domainsisInAllowedFramehelper function to safely detect if running within an allowed iframeconvertPersonaUrlto use parent origin when appropriateSummary by CodeRabbit