feat(desktop-login): /desktop-login route for desktop OAuth handshake#18
Merged
Conversation
Phase 1.f.desktop.1b — bridges the WaveFlow desktop's planned
tiny_http loopback listener (mirroring the existing
`commands::spotify` pattern) to Better Auth's JWT minting:
1. Desktop binds 127.0.0.1:PORT/cb, generates a random `state`, opens
the browser to `/desktop-login?cb=…&state=…`.
2. `resolveDesktopLogin` server fn validates `cb` (loopback only),
checks for a Better Auth session, and mints a fresh JWT via
`auth.api.getToken`.
3. Browser is server-side-redirected (302) to `<cb>?token=…&state=…`.
Desktop listener validates `state` and stores the JWT.
Security boundary lives in `parseLoopback`:
- `protocol === 'http:'` (loopback doesn't get TLS, the desktop
listener is plain).
- `hostname ∈ { '127.0.0.1', 'localhost', '[::1]' }` — no external
hosts. A malicious link with `cb=http://attacker.com:49388/cb`
would let a phishing site exfiltrate the JWT, so the validator
rejects everything that isn't unambiguously loopback.
- `port ∈ [1024, 65535]` — non-privileged.
- Case-insensitive hostname comparison.
Sign-in route gains a `continue` search param so the no-session path
can resume the OAuth flow after login. `safeContinueTarget`
whitelists `/desktop-login` to keep it from becoming an
open-redirect.
Live tests: 13 cases on `parseLoopback` covering every accept +
reject branch (privileged ports, IP-confusing unicode hostnames,
non-http schemes, external hosts, etc.).
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughImplémentation d’un pont OAuth-like desktop : parseLoopback sécurisé + tests, server-fn resolveDesktopLogin (session→token→redirect/state mapping), route client /desktop-login avec beforeLoad pour redirections/états, et sécurisation du paramètre continue dans /sign-in avec tests. ChangesDesktop Login Flow
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant Route as /desktop-login beforeLoad
participant ServerFn as resolveDesktopLogin
participant Auth as auth.api
User->>Browser: ouvre /desktop-login?cb=...&state=...
Browser->>Route: beforeLoad
Route->>ServerFn: resolveDesktopLogin(cb,state)
ServerFn->>Auth: getSession()
Auth-->>ServerFn: session / APIError(UNAUTHORIZED)
alt session ok
ServerFn->>Auth: getToken()
Auth-->>ServerFn: token / APIError(UNAUTHORIZED) / error
alt token ok
ServerFn-->>Route: { status: 'redirect', href }
Route-->>Browser: throw redirect(href)
else token fail
ServerFn-->>Route: { status: 'mint-failed' }
Route->>Browser: render error UI
end
else no session
ServerFn-->>Route: { status: 'no-session' }
Route-->>Browser: throw redirect('/sign-in?continue=...')
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
scope: routes
size: m
type: feat
and removed
type: feat
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/routes/sign-in.tsx`:
- Around line 20-24: La fonction safeContinueTarget accepte des entrées comme
"/desktop-login/../admin" qui passent le startsWith mais redirigent hors de la
zone voulue ; update safeContinueTarget to first parse/normalize the incoming
raw value into a canonical pathname (e.g., with URL or path.posix.normalize) and
then validate that the normalized pathname startsWith "/desktop-login" and does
not contain ".." or resolve outside the allowed prefix (also reject absolute
URLs or host components); return "/" for any invalid or unsafe result. Ensure
you modify the safeContinueTarget implementation to perform normalization before
the startsWith check and keep the same return behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6aafcedb-76dd-4432-beb5-0370c2bd89da
📒 Files selected for processing (4)
src/routes/desktop-login.tsxsrc/routes/sign-in.tsxsrc/server-fns/desktop-login.test.tssrc/server-fns/desktop-login.ts
Merged
6 tasks
@coderabbitai on PR #18 surfaced two findings: 1. `safeContinueTarget` accepted `/desktop-login/../admin` and similar path-traversal payloads because `startsWith` ran on the raw input. The browser would normalise the URL after navigation and land the user on `/admin`, defeating the prefix gate. Fix: parse against a dummy base, reject any value whose normalised `origin` isn't the base (catches absolute / protocol-relative URLs), then check the normalised pathname. Adds 18 test cases covering path traversal, trailing-slash edge cases, host-injection variants (`https://attacker.com`, `//attacker.com`, `javascript:`, `data:` schemes), and the legitimate happy path. 2. `bun run format:check` failed on `desktop-login.tsx` (one stray line over `printWidth`). Ran `prettier --write` across the three files this PR touches — no behaviour change, just formatting. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
InstaZDLL
added a commit
to InstaZDLL/WaveFlow
that referenced
this pull request
May 31, 2026
…p.1b) (#190) * feat(server-auth): oauth-loopback browser handshake (Phase 1.f.desktop.1b) Replaces the manual-paste sign-in flow with a proper local-loopback OAuth-style handshake, mirroring the existing `commands::spotify` pattern. The companion `/desktop-login` route lives on [waveflow-web PR #18](InstaZDLL/waveflow-web#18). ## Backend - New `app_setting['app.waveflow_web_url']` for the waveflow-web URL (separate from the server URL — most deployments will host the API and the web on different domains). Same trim-and-validate gate as the server URL. - `ServerStatus` gains a `web_url` field. - New Tauri commands: - `server_set_web_url` - `server_begin_loopback_login` — generates a 256-bit `state` (anti-replay), spawns the `tiny_http` listener on `127.0.0.1:49388` in a blocking task with a 3-minute timeout, opens the browser to `<web-url>/desktop-login?cb=http://127.0.0.1:49388/wf/callback&state=…`, awaits the callback, validates `state`, persists the JWT via the existing `write_token` path, and returns the fresh `ServerStatus`. - `wait_for_callback` renders three confirmation pages depending on outcome (success / cancelled / state mismatch) so the user knows whether to close the tab or retry. - `random_state` reuses the same UUID-pair pattern Spotify uses for its PKCE verifier — 256 bits of entropy, URL-safe. ## Frontend - Settings → Intégrations → Compte serveur card now exposes: - Server URL field (existing) - **Web URL field** (new — required for the OAuth button) - **Sign in with browser** primary button (disabled until web URL set), with a `loginInProgress` state during the round-trip - Manual JWT paste fallback (kept for users who can't run the handshake — restricted networks, etc.) ## i18n - New keys `settings.serverAccount.{webUrlLabel, webUrlPlaceholder, signInWithBrowser, signInWithBrowserHint, loginInProgress}` propagated to all 17 locales per the CLAUDE.md convention. Brand tokens verbatim (WaveFlow, JWT, WaveFlow Web). ## Verified - `cargo check --workspace` - `cargo clippy --workspace --all-targets -- -D warnings` - `cargo fmt -p waveflow --check` - `bun run typecheck` - `bun run lint` Signed-off-by: InstaZDLL <github.105mh@8shield.net> * fix(server-auth): reject token+error mix in oauth callback @coderabbitai on PR #190 flagged that the success match arm accepted a token even when the callback also carried an `error` claim. Spec- wise the web side never sends both, but the explicit `None` guard makes the fall-through to the error arm authoritative — a future protocol change can't silently smuggle a token past the validation. Signed-off-by: InstaZDLL <github.105mh@8shield.net> --------- Signed-off-by: InstaZDLL <github.105mh@8shield.net>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/routes/sign-in.test.ts`:
- Around line 20-38: The test is missing a rejection case for the sibling-prefix
attack noted in sign-in.tsx; update the it.each list in
src/routes/sign-in.test.ts to include '/desktop-login-evil' (and any similar
sibling-prefix strings) so safeContinueTarget is asserted to return '/' for that
input, ensuring safeContinueTarget continues to block paths that start with the
trusted prefix plus extra characters (refer to safeContinueTarget and the
'/desktop-login' check in sign-in.tsx).
In `@src/routes/sign-in.tsx`:
- Line 44: La validation actuelle utilise
parsed.pathname.startsWith('/desktop-login') et accepte des frères non désirés
comme '/desktop-login-evil'; remplacez cette garde pour n'autoriser que la route
exacte ou son sous-arbre en changeant la condition sur parsed.pathname pour
vérifier soit exact ('/desktop-login') soit un préfixe suivi d'un slash (par ex.
startsWith('/desktop-login/')), et renvoyer '/' sinon; localisez la ligne
contenant parsed.pathname.startsWith('/desktop-login') et remplacez-la par la
condition composée (exact ou sous-arbre).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0776448e-6a9a-44a9-a164-18f48cd3a430
📒 Files selected for processing (3)
src/routes/desktop-login.tsxsrc/routes/sign-in.test.tssrc/routes/sign-in.tsx
@coderabbitai on PR #18 flagged that startsWith('/desktop-login') accepts sibling paths like /desktop-login-evil and /desktop-loginXYZ. Origin is already pinned to localhost so it isn't an open-redirect, but the prefix gate's intent is "the route or its subtree", and the sibling-prefix slip contradicts that. Anchor on either exact match or a trailing slash; pin the regression with two new test cases. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
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
1 participant
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.
Companion web-side PR for Phase 1.f.desktop.1b on the WaveFlow desktop repo — adds the route the desktop's planned loopback listener (mirroring `commands::spotify`) hits to swap a Better Auth session for a fresh waveflow-server JWT.
Summary
Flow
```
/desktop-login?cb=http://127.0.0.1:49388/cb&state=…
```
Security: `parseLoopback`
Every JWT leak vector goes through this validator. Rules:
Test cases in `desktop-login.test.ts` cover every rejection branch — privileged ports, port 0, port > 65535, missing port, public IPs, link-local addresses (169.254.169.254 cloud metadata), `https://`, `file://`, `javascript:`, unicode-confusing hostnames like `127.0.0.1.attacker.com`. 13 cases total, all green.
Test plan
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests