GTM-93: add desktop login attribution callback#12983
Draft
benceruleanlu wants to merge 1 commit into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughA new desktop login bridge module is added that validates loopback callback URLs in route query parameters and POSTs Firebase credentials to a local desktop application upon successful cloud authentication. This is wired into cloud route guards, the login view, and the post-auth redirect composable. A new PostHog identity queue module is introduced to support deferred user identification. ChangesDesktop Login Bridge
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note over CloudLoginView,LoopbackCallback: Desktop Login Completion Flow
end
participant CloudLoginView
participant usePostAuthRedirect
participant desktopLoginBridge
participant posthogIdentity
participant LoopbackCallback
CloudLoginView->>CloudLoginView: watch(authStore.isInitialized, authStore.currentUser)
CloudLoginView->>usePostAuthRedirect: onAuthSuccess()
usePostAuthRedirect->>desktopLoginBridge: completeDesktopLoginIfNeeded(query, currentUser)
desktopLoginBridge->>posthogIdentity: identifyPostHogUser(user.uid)
desktopLoginBridge->>desktopLoginBridge: getFirebaseConfig() → apiKey
desktopLoginBridge->>LoopbackCallback: POST /callback { state, apiKey, user }
alt success
LoopbackCallback-->>desktopLoginBridge: 200 OK
desktopLoginBridge-->>usePostAuthRedirect: true
usePostAuthRedirect-->>CloudLoginView: return early (no further redirect)
else failure
LoopbackCallback-->>desktopLoginBridge: non-OK or missing apiKey
desktopLoginBridge-->>usePostAuthRedirect: throws error
usePostAuthRedirect-->>CloudLoginView: set authError + show error toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ❌ 1669 passed, 1 failed · 1 flaky❌ Failed Tests📊 Browser Reports
📦 Bundle: 7.45 MB gzip 🔴 +2.26 kBDetailsSummary
Category Glance App Entry Points — 45.2 kB (baseline 45.8 kB) • 🟢 -634 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • 🔴 +47 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 95.8 kB (baseline 95.3 kB) • 🔴 +536 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed Panels & Settings — 524 kB (baseline 523 kB) • 🔴 +1.41 kBConfiguration panels, inspectors, and settings screens
Status: 25 added / 24 removed User & Accounts — 22.9 kB (baseline 19.9 kB) • 🔴 +2.98 kBAuthentication, profile, and account management bundles
Status: 10 added / 9 removed Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 5 added / 5 removed UI Components — 57.2 kB (baseline 57.2 kB) • ⚪ 0 BReusable component library chunks
Status: 13 added / 13 removed Data & Services — 266 kB (baseline 266 kB) • 🔴 +64 BStores, services, APIs, and repositories
Status: 15 added / 15 removed Utilities & Hooks — 3.31 MB (baseline 3.31 MB) • 🟢 -980 BHelpers, composables, and utility bundles
Status: 30 added / 30 removed Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • 🔴 +1.06 kBExternal libraries and shared vendor chunks
Status: 13 added / 12 removed / 4 unchanged Other — 10.4 MB (baseline 10.4 MB) • 🔴 +373 BBundles that do not match a named category
Status: 145 added / 145 removed / 3 unchanged ⚡ Performance
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/platform/cloud/onboarding/desktopLoginBridge.ts (2)
16-29: 💤 Low valueConsider IPv6 loopback support.
The current loopback validation only accepts
localhostand127.0.0.1. If desktop applications bind to IPv6, they might use::1(IPv6 loopback). Consider adding it to the allowlist or documenting why IPv6 is intentionally excluded.Optional: Add IPv6 loopback support
function parseLoopbackCallback(rawUrl: string): URL | null { let url: URL try { url = new URL(rawUrl) } catch { return null } if (url.protocol !== 'http:') return null - if (!['localhost', '127.0.0.1'].includes(url.hostname)) return null + if (!['localhost', '127.0.0.1', '::1'].includes(url.hostname)) return null if (url.pathname !== '/callback') return null return url }🤖 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 `@src/platform/cloud/onboarding/desktopLoginBridge.ts` around lines 16 - 29, The parseLoopbackCallback function currently only validates IPv4 loopback addresses (localhost and 127.0.0.1) but does not account for IPv6 loopback addresses. Update the hostname validation check in parseLoopbackCallback where the includes method checks for valid hostnames to also include the IPv6 loopback address '::1' in addition to the existing IPv4 addresses.
65-84: ⚡ Quick winConsider adding a fetch timeout.
The
fetchcall to the desktop loopback URL has no timeout. If the desktop application crashes or becomes unresponsive, the POST could hang indefinitely, blocking the browser login flow.Add AbortSignal with timeout
+ const abortController = new AbortController() + const timeoutId = setTimeout(() => abortController.abort(), 10000) // 10s + const response = await fetch(request.callbackUrl.href, { method: 'POST', mode: 'cors', credentials: 'omit', + signal: abortController.signal, headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ state: request.state, apiKey: firebaseConfig.apiKey, user: user.toJSON() }) }) + clearTimeout(timeoutId)🤖 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 `@src/platform/cloud/onboarding/desktopLoginBridge.ts` around lines 65 - 84, The fetch call to request.callbackUrl.href lacks timeout protection, which can cause the browser login flow to hang indefinitely if the desktop application becomes unresponsive. Add an AbortController with a timeout mechanism to the fetch request. Create the AbortController, set a timeout that aborts the request after a reasonable duration (e.g., 30 seconds), and pass the abort signal in the fetch options alongside the existing method, mode, credentials, headers, and body properties. This ensures the POST request will be cancelled if the desktop application does not respond within the specified timeframe.
🤖 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 `@src/platform/cloud/onboarding/desktopLoginBridge.ts`:
- Around line 16-29: The parseLoopbackCallback function currently only validates
IPv4 loopback addresses (localhost and 127.0.0.1) but does not account for IPv6
loopback addresses. Update the hostname validation check in
parseLoopbackCallback where the includes method checks for valid hostnames to
also include the IPv6 loopback address '::1' in addition to the existing IPv4
addresses.
- Around line 65-84: The fetch call to request.callbackUrl.href lacks timeout
protection, which can cause the browser login flow to hang indefinitely if the
desktop application becomes unresponsive. Add an AbortController with a timeout
mechanism to the fetch request. Create the AbortController, set a timeout that
aborts the request after a reasonable duration (e.g., 30 seconds), and pass the
abort signal in the fetch options alongside the existing method, mode,
credentials, headers, and body properties. This ensures the POST request will be
cancelled if the desktop application does not respond within the specified
timeframe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 457bdadc-b145-492b-b535-7965628ef251
📒 Files selected for processing (8)
src/platform/cloud/onboarding/CloudLoginView.vuesrc/platform/cloud/onboarding/composables/usePostAuthRedirect.tssrc/platform/cloud/onboarding/desktopLoginBridge.test.tssrc/platform/cloud/onboarding/desktopLoginBridge.tssrc/platform/cloud/onboarding/onboardingCloudRoutes.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.tssrc/platform/telemetry/providers/cloud/posthogIdentity.test.tssrc/platform/telemetry/providers/cloud/posthogIdentity.ts
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #12983 +/- ##
==========================================
- Coverage 76.37% 76.37% -0.01%
==========================================
Files 1569 1571 +2
Lines 102782 102859 +77
Branches 31977 31995 +18
==========================================
+ Hits 78499 78554 +55
- Misses 23452 23471 +19
- Partials 831 834 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
can use local storage and have a generalized client anon id. need to send that in identify calls i believe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Links
Test plan