feat(auth): loopback OAuth redirect with deep-link fallback#2511
Conversation
Adds an RFC 8252 loopback HTTP listener as the preferred desktop OAuth
redirect target, with the existing `openhuman://` deep link as fallback.
The Tauri shell binds `127.0.0.1:53824/auth` on demand per login click,
generates a state nonce, and emits a `loopback-oauth-callback` event when
the browser hits it; the frontend funnels the callback URL through the
same `handleDeepLinkUrls` handler used by the deep-link path so token
exchange and CoreStateProvider commit logic stays in one place. If bind
fails (port in use, not in Tauri, etc.) we transparently fall back to
the legacy `openhuman://` redirect — no regression.
Backend dependency: `${backendUrl}/auth/<provider>/login` must accept a
`redirectUri` query param and echo `state` back on the callback for the
loopback path to actually fire.
|
Warning Review limit reached
Your plan currently allows 3 reviews/hour. Refill in 8 minutes and 15 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Tauri Rust module that spawns a one-shot loopback HTTP listener for OAuth callbacks, a TypeScript wrapper exposing await/cancel handles, integrates loopback-based callbacks into the OAuth component on desktop, and exports the deep-link handler for routing received callbacks. ChangesLoopback OAuth Desktop Callback Flow
Sequence DiagramsequenceDiagram
participant Browser
participant LoopbackListener as LoopbackListener(Rust)
participant TauriEvent as TauriEventEmitter
participant Frontend
Browser->>LoopbackListener: GET 127.0.0.1:port/auth?state=...&code=...
LoopbackListener->>LoopbackListener: parse request, validate state
LoopbackListener->>Browser: 200 "Signed in" HTML
LoopbackListener->>TauriEvent: emit loopback-oauth-callback (callback URL)
TauriEvent->>Frontend: frontend receives callback URL
Frontend->>Frontend: convert to openhuman:// deep-link and route
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/utils/loopbackOauthListener.ts (1)
67-83: ⚡ Quick winUse the frontend debug logger instead of
console.warnhere.This helper adds new frontend logging via
console.warn, which drifts from the repo’s namespaceddebugpattern and makes these messages harder to correlate with the rest of the auth flow.🔧 Example cleanup
+import debug from 'debug'; import { invoke } from '`@tauri-apps/api/core`'; import { listen, type UnlistenFn } from '`@tauri-apps/api/event`'; import { isTauri } from './tauriCommands/common'; +const warnLog = debug('oauth:loopback:warn'); + ... try { result = await invoke<StartResult>('start_loopback_oauth_listener', { port, timeoutSecs }); } catch (err) { - console.warn('[loopback-oauth] start failed, falling back to deep link', err); + warnLog('start failed, falling back to deep link %o', err); return null; } ... const stop = async () => { try { await invoke('stop_loopback_oauth_listener'); } catch (err) { - console.warn('[loopback-oauth] stop failed', err); + warnLog('stop failed %o', err); } };As per coding guidelines, "Use namespaced
debugfunction in TypeScript frontend with dev-only detail logging."🤖 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 `@app/src/utils/loopbackOauthListener.ts` around lines 67 - 83, Replace the two console.warn calls with the repo’s namespaced frontend debug logger: import and instantiate the debug logger for this module (e.g. const debug = createDebug('frontend:loopback-oauth') or the project’s standard debug factory), then call debug('[loopback-oauth] start failed, falling back to deep link', err) in the catch around invoke<StartResult>('start_loopback_oauth_listener', ...) and debug('[loopback-oauth] stop failed', err) in the catch inside stop(); keep the existing behavior/return values and do not change invoke, StartResult, appendState or stop signatures.
🤖 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 `@app/src-tauri/src/loopback_oauth.rs`:
- Line 39: The current static ACTIVE_LISTENER races: a stale task can
unconditionally clear the global sender and remove a newer listener; change
ACTIVE_LISTENER to store a (listener_id: u64, sender: oneshot::Sender<()>) tuple
(e.g. Mutex<Option<(u64, oneshot::Sender<()>)>>) and have
start_loopback_oauth_listener generate/increment a new id when installing a
sender, have the exiting/cancel path and stop_loopback_oauth_listener compare
the id they think they own against the id currently stored and only clear/remove
the entry when the ids match; update all code that reads/writes ACTIVE_LISTENER
(creation in start_loopback_oauth_listener, cancellation in
stop_loopback_oauth_listener, and the task-exit path around Line 152) to use and
check the id so a stale task cannot wipe out a newer sender.
- Around line 240-241: The emitted callback URL omits the bound port so the
frontend rewrite doesn't match; change the construction of callback_url (used in
app.emit(LOOPBACK_CALLBACK_EVENT, CallbackPayload { url: callback_url })) to
include the listener's actual port (e.g., format!("http://127.0.0.1:{}{}", port,
target) or by calling listener.local_addr().port()) so the emitted URL is
"http://127.0.0.1:<port><target>" and will correctly be rewritten to the
openhuman:// scheme.
---
Nitpick comments:
In `@app/src/utils/loopbackOauthListener.ts`:
- Around line 67-83: Replace the two console.warn calls with the repo’s
namespaced frontend debug logger: import and instantiate the debug logger for
this module (e.g. const debug = createDebug('frontend:loopback-oauth') or the
project’s standard debug factory), then call debug('[loopback-oauth] start
failed, falling back to deep link', err) in the catch around
invoke<StartResult>('start_loopback_oauth_listener', ...) and
debug('[loopback-oauth] stop failed', err) in the catch inside stop(); keep the
existing behavior/return values and do not change invoke, StartResult,
appendState or stop signatures.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1712395d-dd12-4744-8913-d55abf3d5269
📒 Files selected for processing (6)
app/src-tauri/src/lib.rsapp/src-tauri/src/loopback_oauth.rsapp/src/components/oauth/OAuthProviderButton.tsxapp/src/utils/__tests__/loopbackOauthListener.test.tsapp/src/utils/desktopDeepLinkListener.tsapp/src/utils/loopbackOauthListener.ts
- Tag ACTIVE_LISTENER with a monotonic id so a superseded task can't wipe out the newer sender installed by the start that cancelled it. - Emit the callback URL with the bound port. The frontend rewrite matches /^https?:\/\/127\.0\.0\.1:\d+\/auth/, so omitting the port prevented the loopback callback from being rewritten to openhuman:// and stalled the desktop auth flow.
Bring diff-coverage above the 80% gate by exercising the previously untested branches in OAuthProviderButton's loopback wiring and loopbackOauthListener's stop/listen/timeout paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/utils/__tests__/loopbackOauthListener.test.ts`:
- Around line 83-95: The test "cancel swallows stop_loopback_oauth_listener
failure" creates a console.warn spy but restores it only at the end, risking
leakage if an assertion throws; update the test around
startLoopbackOauthListener/handle.cancel to ensure the spy is always restored by
wrapping the assertions in a try/finally (or using afterEach) so
warn.mockRestore() runs regardless of failures, referencing the test, the
console.warn spy (warn), startLoopbackOauthListener(), and handle.cancel() to
locate the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7d17e25-7c4d-4fac-afd2-133bc8a3ceff
📒 Files selected for processing (2)
app/src/components/oauth/__tests__/OAuthProviderButton.test.tsxapp/src/utils/__tests__/loopbackOauthListener.test.ts
Lines 529-550 duplicated the MCP server translation block already present at lines 211-235, breaking tsc (TS1117) which cascaded into Type Check TypeScript and the E2E build step. Keep the earlier (more complete) block; drop the trailing duplicates.
Summary
http://127.0.0.1:53824/auth) as the preferred desktop OAuth redirect target, with the existingopenhuman://deep link as automatic fallback.start_loopback_oauth_listener/stop_loopback_oauth_listenerspawn a one-shot listener per login click, validate a state nonce, emit aloopback-oauth-callbackevent, and tear themselves down.OAuthProviderButtontries loopback first; on bind failure (port in use, not in Tauri) it transparently falls back to the legacy deep-link path — no regression.Problem
Desktop deep links (
openhuman://auth) are unpredictable on Linux/Windows and rely on single-instance forwarding through a named pipe (#1130). RFC 8252 recommends loopback HTTP redirects for native apps because they're robust across platforms, work with system browsers, and don't require OS-level URL-scheme registration. We had no loopback option, so any deep-link failure (xdg-utils missing, brokenms-registration, etc.) stranded users at the system browser with no path back into the app.Solution
app/src-tauri/src/loopback_oauth.rs— a minimal tokio TCP accept loop (no axum/hyper added). Binds127.0.0.1:53824on demand, generates a 32-char hex state nonce, accepts loopback-only connections, validates state on/auth?...&state=, emits the callback URL to the frontend, and shuts down. 300 s timeout, supersedes any prior listener on a newstart.app/src/utils/loopbackOauthListener.tsreturnsnullon bind failure so callers can fall back; exposesredirectUri(state pre-appended),awaitCallback(), andcancel().desktopDeepLinkListener.tsexportshandleDeepLinkUrlsso the loopback path reuses the same auth/token-exchange/CoreStateProvider commit logic — token handling stays in one place.OAuthProviderButton.tsxrequests a loopback handle before opening the browser, appendsredirectUri=<loopback>to the backend login URL, and feeds the eventual callback throughhandleDeepLinkUrlsas a syntheticopenhuman://auth?….Design choices (clarified up front):
redirect_uriallowlisting is typically static.Submission Checklist
Impact
${backendUrl}/auth/<provider>/loginmust accept aredirectUriquery param and echostateback on the redirect for the loopback path to actually fire. Until that's wired backend-side, every login transparently falls back to the existingopenhuman://authdeep link.127.0.0.1peers; state nonce guards against a hostile page on the same loopback origin faking a callback.tokio,rand,hex,serde.Pre-push hook note: the local pre-push hook applied two whitespace-only formatting auto-fixes (one in
loopback_oauth.rs, one inloopbackOauthListener.ts) — committed aschore: apply auto-fixes. No--no-verifywas used.Related
redirectUriquery param + echostateon auth login redirectAI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check(pre-push hook auto-applied two whitespace fixes, then passed)pnpm typecheckcargo test --lib loopback_oauth(4 passing),pnpm test(3116 passing, 3 skipped — new loopbackOauthListener.test.ts included),pnpm test:e2e login-flow.spec.ts(12/12),pnpm test:e2e smoke.spec.ts(3/3 + 1 skipped)Validation Blocked
Behavior Changes
openhuman://deep link when loopback bind fails.redirectUriquery param; current behavior is unchanged.Parity Contract
nulland the original deep-link URL is sent verbatim.openhuman://auth?…and dispatched through the samehandleDeepLinkUrlshandler used by the deep-link plugin, so token consume + CoreStateProvider commit logic is identical.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Improvements
Tests