This repository was archived by the owner on Jun 1, 2026. It is now read-only.
fix(auth): use password-first remote login#33
Merged
Conversation
Replace remote custom auth header setup with username/password plus conditional OTP verification across desktop, mobile, and TUI. Centralize Basic/OTP header handling and pass verified OTP tokens through PTY websocket connections. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: codeplane-agent[bot] <287208015+codeplane-agent[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the remote instance setup flow across Desktop, Mobile, and TUI to use a consistent username/password-first login with conditional OTP verification (via /global/auth and /global/auth/verify), backed by new shared auth-header helpers. It also propagates verified OTP tokens into the app’s PTY WebSocket connection parameters and extends Desktop E2E coverage for OTP-gated setup.
Changes:
- Added shared remote auth utilities (
composeRemoteAuthHeaders,splitRemoteAuthHeaders,/global/authprobe +/global/auth/verifyOTP exchange) with unit tests. - Reworked Desktop/Mobile/TUI setup UIs and platform IPC/API plumbing to use password + conditional OTP verification instead of custom header / browser-sign-in flows.
- Added Desktop E2E coverage for OTP-gated remote setup and ensured OTP tokens are passed to PTY WebSocket connections.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/remote-auth.ts | Introduces shared Basic/OTP header composition + /global/auth probe + OTP verification helpers. |
| packages/shared/test/remote-auth.test.ts | Adds unit tests for header composition/splitting and auth/verify request behavior. |
| packages/mobile/src/screens/setup.tsx | Wires new authStatus / verifyOtp instance APIs into the instance form. |
| packages/mobile/src/platform/api.ts | Adds Capacitor-native fetch wrapper and exposes authStatus/verifyOtp on the instances API. |
| packages/mobile/src/components/instance-list.tsx | Updates UI chip wording from headers-based auth to “Login saved”. |
| packages/mobile/src/components/instance-form.tsx | Replaces free-form headers input with username/password + conditional OTP UX and save-time auth resolution. |
| packages/desktop/src/setup/app.tsx | Replaces advanced auth/header UI with login + conditional OTP and uses auth-status/verify IPC for setup. |
| packages/desktop/src/main/preload.ts | Updates preload API surface: removes browser sign-in IPC and adds auth-status + verify-otp IPC. |
| packages/desktop/src/main/main.ts | Implements new IPC handlers calling shared remote-auth helpers using session-aware fetch. |
| packages/desktop/e2e/desktop.spec.ts | Updates existing setup assertions and adds OTP-gated remote setup E2E coverage. |
| packages/codeplane/src/tui/i18n.ts | Updates TUI copy to reflect password-first login + OTP flow and related errors. |
| packages/codeplane/src/tui/boot/remote-form.tsx | Reworks TUI remote form to use username/password + conditional OTP (removes browser sign-in + custom headers). |
| packages/app/src/components/terminal.tsx | Propagates otp_token to PTY WebSocket connect URLs when present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1264
to
+1266
| setOtpVisible(true) | ||
| if (!otpCode().trim()) return { ok: false, message: "Enter the one-time code for this server." } | ||
| const verified = await api.instances.verifyOtp({ |
Comment on lines
+123
to
+125
| setDraft({ ...draft(), otpVisible: true }) | ||
| if (!current.otpCode.trim()) return { ok: false as const, message: "Enter the one-time code for this server." } | ||
| const verified = await props.verifyOtp({ |
Comment on lines
+228
to
+232
| setOtpVisible(true) | ||
| if (!otpCode().trim()) { | ||
| setFocused("otp") | ||
| return { ok: false, message: tuiT("boot.remote.authOtpRequired") } | ||
| } |
Comment on lines
199
to
211
| <input | ||
| id="cp-mobile-url" | ||
| class="mobile-input" | ||
| type="url" | ||
| inputmode="url" | ||
| autocapitalize="none" | ||
| autocorrect="off" | ||
| spellcheck={false} | ||
| required | ||
| placeholder="https://codeplane.example.com" | ||
| value={draft().url} | ||
| onInput={(e) => setDraft({ ...draft(), url: e.currentTarget.value })} | ||
| onInput={(e) => setDraft({ ...draft(), url: e.currentTarget.value, otpToken: "", otpCode: "", otpVisible: false })} | ||
| /> |
Comment on lines
+233
to
+244
| <input | ||
| id="cp-mobile-username" | ||
| class="mobile-input" | ||
| type="text" | ||
| autocapitalize="none" | ||
| autocorrect="off" | ||
| spellcheck={false} | ||
| placeholder={"Authorization: Bearer ...\nCookie: session=..."} | ||
| value={draft().headersText} | ||
| onInput={(e) => setDraft({ ...draft(), headersText: e.currentTarget.value })} | ||
| autocomplete="username" | ||
| placeholder="codeplane" | ||
| value={draft().username} | ||
| onInput={(e) => setDraft({ ...draft(), username: e.currentTarget.value, otpToken: "", otpCode: "" })} | ||
| /> |
Comment on lines
+251
to
262
| <input | ||
| id="cp-mobile-password" | ||
| class="mobile-input" | ||
| type="password" | ||
| autocapitalize="none" | ||
| autocorrect="off" | ||
| spellcheck={false} | ||
| autocomplete="current-password" | ||
| placeholder="Password" | ||
| value={draft().password} | ||
| onInput={(e) => setDraft({ ...draft(), password: e.currentTarget.value, otpToken: "", otpCode: "" })} | ||
| /> |
Comment on lines
+274
to
+286
| <input | ||
| id="cp-mobile-otp" | ||
| class="mobile-input" | ||
| type="text" | ||
| inputmode="numeric" | ||
| autocapitalize="none" | ||
| autocorrect="off" | ||
| spellcheck={false} | ||
| autocomplete="one-time-code" | ||
| placeholder={draft().otpToken ? "OTP verified" : "123456"} | ||
| value={draft().otpCode} | ||
| onInput={(e) => setDraft({ ...draft(), otpCode: e.currentTarget.value })} | ||
| /> |
Comment on lines
+251
to
+268
| const capacitorFetch: typeof fetch = async (input, init) => { | ||
| const headers: Record<string, string> = {} | ||
| new Headers(init?.headers).forEach((value, key) => { | ||
| headers[key] = value | ||
| }) | ||
| const response = await CapacitorHttp.request({ | ||
| method: init?.method ?? "GET", | ||
| url: typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url, | ||
| headers, | ||
| data: typeof init?.body === "string" ? init.body : undefined, | ||
| connectTimeout: 8000, | ||
| readTimeout: 8000, | ||
| }) | ||
| return new Response(typeof response.data === "string" ? response.data : JSON.stringify(response.data ?? null), { | ||
| status: response.status, | ||
| headers: response.headers, | ||
| }) | ||
| } |
Comment on lines
+256
to
+263
| const response = await CapacitorHttp.request({ | ||
| method: init?.method ?? "GET", | ||
| url: typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url, | ||
| headers, | ||
| data: typeof init?.body === "string" ? init.body : undefined, | ||
| connectTimeout: 8000, | ||
| readTimeout: 8000, | ||
| }) |
Comment on lines
+2568
to
+2570
| const ses = ensureSession(input) | ||
| const nativeFetch = "fetch" in ses && typeof ses.fetch === "function" ? ses.fetch.bind(ses) : fetch | ||
| return checkRemoteAuth({ url: target.toString(), headers: input.headers }, nativeFetch, { timeoutMs: 8000 }) |
Collaborator
Author
Triage — Keep OpenConfirmed real defects in this PR. Copilot review flagged them; no past agent commentary here. Defects found
Why not closeAll five are real auth/mobile bugs affecting correctness, security, and UX. Keep open until addressed. |
Collaborator
Author
Review: password-first remote loginStatus: Merge candidate All CI checks are green. The password-first + OTP flow replaces the broken browser-sign-in approach cleanly. Shared remote-auth.ts module with proper helpers. Backend endpoints correctly structured. Desktop E2E tests added. Note: Removes the browser-sign-in flow entirely — worth noting in release notes so users who relied on copying auth headers from DevTools are aware. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
/global/authplus/global/auth/verifywithout letting saved OTP tokens reveal the OTP field.Verification
bun --cwd packages/shared test test/remote-auth.test.tsbun --cwd packages/desktop typecheckbun --cwd packages/mobile typecheckbun --cwd packages/codeplane typecheckbun turbo typecheckbun lintbun run buildinpackages/desktop$DISPLAYand noxvfb-run, so Electron exits before tests run.