fix(clerk-js): Update inCrossOriginIframe to handle nested cross origin iframes
#7212
Conversation
…igin iframes This handles the following dom structure ``` top (replit.com) - iframe1 (foo.janeway.replit.dev/__iframe.html) - iframe2(foo.janeway.replit.dev; Clerk loaded here) ```
🦋 Changeset detectedLatest commit: 36c9a25 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe pull request adds a changeset entry and updates the Changes
Sequence DiagramsequenceDiagram
participant Code as Runtime Code
participant Util as inCrossOriginIframe()
Util->>Util: Check inIframe()
alt Not in iframe
Util-->>Code: return false
else In iframe
Util->>Util: Attempt access to<br/>window.top.location.href
alt Access succeeds<br/>(same-origin ancestor)
Util-->>Code: return false
else SecurityError thrown<br/>(cross-origin ancestor)
Util-->>Code: return true
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
thiskevinwang
changed the title
inCrossOriginIframe to handle nested cross origin iframesinCrossOriginIframe to handle nested cross origin iframes
|
!snapshot |
|
Hey @thiskevinwang - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/cute-apes-watch.md (1)
5-5: Fix hyphenation in compound adjective.The phrase should use "cross-origin" (hyphenated) when modifying "iframes" as a compound adjective.
Apply this diff:
-fix(clerk-js): update inCrossOriginIframe to handle nested cross origin iframes +fix(clerk-js): update inCrossOriginIframe to handle nested cross-origin iframes
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/cute-apes-watch.md(1 hunks)packages/clerk-js/src/utils/runtime.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/cute-apes-watch.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...CrossOriginIframe to handle nested cross origin iframes
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (1)
packages/clerk-js/src/utils/runtime.ts (1)
24-38: The web search returned general information about unit testing in TypeScript, but not specific to this codebase. Let me search for test files within the utils directory structure directly.Perfect! The web search confirms that reading from window.top.location.href throws a SecurityError if the iframe is cross-origin, which validates the implementation approach in the code. Now let me check for test files specifically for the runtime utilities.
Based on the comprehensive verification I've conducted:
Implementation correctness verified: The web search confirms that attempting to read
window.top.location.hrefthrows a SecurityError in cross-origin iframe contexts, which is the exact mechanism this code relies upon.Test coverage identified: The initial shell script showed that
inCrossOriginIframeis already being tested, but only as a mocked dependency in cookie-handling tests (inclientUat.test.tsandsession.test.ts), not as dedicated unit tests for the function itself.No dedicated unit tests found: The shell output shows the function is imported and mocked but not tested directly for its internal logic.
The original review comment was asking for test verification but made specific requests. Since the implementation itself is correct and uses a well-established technique validated by browser security policies, I can now provide my final response.
Excellent implementation of nested cross-origin iframe detection!
The code correctly uses the SecurityError thrown when accessing
window.top.location.hrefto detect cross-origin iframes at any nesting level. This approach is sound and aligns with browser same-origin policies.The existing integration tests in
packages/clerk-js/src/core/auth/cookies/__tests__/already verify the function's behavior by mocking it with both true and false return values, validating that cookies are set correctly in both same-origin and cross-origin contexts.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
Description
I've observed the
__clerk_db_jwtcookie to fail to be set on the Replit primary site, but successfully set when visiting the preview URL directly.This aims to fix the former flow by improving our cross origin frame detection to support the following DOM structure
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit