Skip to content

Commit 096e2f7

Browse files
committed
fix session arch for desktop
1 parent 1e210f4 commit 096e2f7

30 files changed

Lines changed: 957 additions & 133 deletions

apps/desktop/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,12 @@ Everything is bundled by esbuild into `dist/main.cjs` + `dist/preload.cjs` — i
4848
- The app loads the hosted origin top-level; better-auth session cookies live in a persistent partition (`persist:sim`, per-origin for self-hosts). Email/password and verified-lenient providers (GitHub) sign in fully in-window.
4949
- **Google / Microsoft / SSO cannot OAuth inside an embedded browser** (`disallowed_useragent`; UA spoofing is fingerprint-defeated — do not ship it). Navigation to those hosts from an auth surface is intercepted and rerouted through the **system-browser handoff**:
5050
1. App starts a one-shot `127.0.0.1` loopback listener on an ephemeral port and opens `<origin>/desktop/auth?state=<random>&port=<port>` in the browser. The state is single-use, in-memory (the app is always running when the callback returns, so nothing is persisted), and constant-time compared.
51-
2. `apps/sim/app/desktop/auth/page.tsx` requires a browser session (redirects through `/login?callbackUrl=…`), mints a better-auth one-time token, and **redirects straight to the loopback** (`http://127.0.0.1:<port>/auth/callback?token=…&state=…`, RFC 8252 §7.3). This is the single hand-back channel — a deterministic server redirect, no OS scheme registration, no client-side step, works identically in dev and packaged builds. Interception of loopback is mitigated the way PKCE mitigates it: the token is single-use, short-TTL, and bound to a 128-bit state the app compares in constant time. (RFC 8252's *most*-preferred callback is claimed-`https` / macOS universal links, which bind the OS to a verified app identity — a future hardening step that needs an associated-domains entitlement + `apple-app-site-association` on the origin.)
51+
2. `apps/sim/app/desktop/auth/page.tsx` requires a browser session (redirects through `/login?callbackUrl=…`) and renders a **Continue** gesture gate; only that click mints a one-time token (`POST /api/desktop/auth/handoff`) and sends the browser to the loopback (`http://127.0.0.1:<port>/auth/callback?token=…&state=…`, RFC 8252 §7.3). The gate is the security boundary — state and port are attacker-choosable in a crafted link, so a bare GET must never mint. The loopback is the single hand-back channel: no OS scheme registration, works identically in dev and packaged builds. Interception of loopback is mitigated the way PKCE mitigates it: the token is single-use, short-TTL, and bound to a 128-bit state the app compares in constant time. (RFC 8252's *most*-preferred callback is claimed-`https` / macOS universal links, which bind the OS to a verified app identity — a future hardening step that needs an associated-domains entitlement + `apple-app-site-association` on the origin.)
5252
3. The loopback fires the callback in the main process: the app validates the state, then a renderer in the app partition POSTs the token to `/api/auth/one-time-token/verify` (same-origin ⇒ trustedOrigins/CSRF pass; better-auth sets the session cookie and burns the token) and loads `/workspace`.
53+
- **The app gets its own session, never the browser's.** The token is minted by `POST /api/desktop/auth/handoff` (`apps/sim/lib/auth/desktop-handoff.ts`), which creates a *new* session row for the user and points the one-time token at that. Better-auth's own `generateOneTimeToken` binds to the *calling* session, so using it directly made the app and the authorizing browser share one row: signing out of either deleted the row the other was still presenting, and — because the session cookie cache is not revalidated against the database — the survivor kept looking signed in while every database-backed session resolution failed (the socket handshake logged `Session not found` and 401-looped). A session per device is what better-auth's own device authorization grant does and what RFC 8252 assumes: sign-out, revocation, and expiry apply to one surface at a time. **Do not "simplify" this back to `generateOneTimeToken`.**
5354
- Integration connects are **same-window redirects** (`client.oauth2.link`), not popups. Unknown provider hosts stay in-window (lenient default); Google/Microsoft connects get a native dialog offering to finish in the browser — the browser is signed in after the login handoff, tokens land server-side, the app just refreshes.
5455
- The MCP OAuth popup (`mcp-oauth-*`) is allowed as a same-partition child so `window.opener.postMessage` keeps working.
55-
- Sign-out clears cookies/localStorage/IndexedDB/cache/service workers plus any pending handoff state. Two signals trigger it: the `/login?fromLogout=true` navigation (fast path) **and** deletion of the better-auth session cookie confirmed by a `get-session` probe (robust backstop — catches every sign-out path, not just the settings one, and rotation can't cause a false teardown). API 401s (probe-confirmed) surface a native re-auth prompt.
56+
- Sign-out **revokes server-side first** (`POST /api/auth/sign-out` from the app-origin renderer — a main-process `session.fetch` would be rejected by better-auth's origin check), then clears cookies/localStorage/IndexedDB/cache/service workers plus any pending handoff state. The revoke is not optional: since the app owns its own session row, clearing the partition alone would strand a live 30-day credential that nothing can revoke. Two signals trigger teardown: the `/login?fromLogout=true` navigation (fast path) **and** deletion of the better-auth session cookie confirmed by a `get-session` probe (robust backstop — catches every sign-out path, not just the settings one, and rotation can't cause a false teardown). API 401s (probe-confirmed) surface a native re-auth prompt.
5657

5758
Deviations from the original plan doc (deliberate):
5859
- **One hand-back channel, not two.** The plan proposed a `sim://` deep link with a loopback fallback; that was collapsed to loopback-only. The app is always running when the callback returns (it started the loopback), so the custom scheme added complexity and a dev-only failure mode without buying anything — loopback works identically everywhere. No `sim://` scheme is registered.
@@ -77,6 +78,7 @@ A thin shell over a hosted web app unavoidably knows a few of the web app's conv
7778
| `navigation.ts` `mcp-oauth-*` frame name | `hooks/queries/mcp.ts` opens `mcp-oauth-${id}` | renames the popup frame | MCP popup treated as generic → opener lost, flow hangs | String contract; add a shared constant if it churns |
7879
| `window.ts` theme probe | `document.documentElement.classList.contains('dark')` (next-themes `attribute='class'`) | drops the `dark` class convention | pre-paint background may flash once | Cosmetic only; self-corrects on next load |
7980
| `handoff.ts` redeem | `POST /api/auth/one-time-token/verify` sets the cookie | better-auth changes the endpoint | handoff sign-in fails | better-auth built-in endpoint; pinned by the `better-auth` version |
81+
| `apps/sim/lib/auth/desktop-handoff.ts` mint | better-auth stores one-time tokens as `verification` rows keyed `one-time-token:<token>`, unhashed (`storeToken: 'plain'` default) | better-auth renames the namespace or hashes by default | every redeem fails with `Invalid token` | Single constant in one module; the covering test asserts the identifier shape. Revisit on better-auth major |
8082

8183
Overall this is **within normal thin-wrapper coupling** — every item is either backstopped (sign-out), cosmetic (theme), or a stable library/route contract. The only one that genuinely can't self-heal without a release is the IdP host list, which is inherent to the "pin Chromium, ship a binary" model and is managed by the upgrade program.
8284

apps/desktop/src/main/handoff.test.ts

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ function makeDeps(overrides: Partial<HandoffManagerDeps> = {}): HandoffManagerDe
2525
origin: () => 'https://sim.ai',
2626
openExternal: vi.fn(async () => true),
2727
events: makeEvents(),
28+
currentUserId: vi.fn(async () => 'user-1'),
2829
...overrides,
2930
}
3031
}
@@ -105,12 +106,17 @@ describe('createHandoffManager', () => {
105106
expect(badToken.status).toBe(400)
106107
expect(received).toHaveLength(0)
107108

108-
const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`)
109-
expect(ok.status).toBe(200)
109+
const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`, {
110+
redirect: 'manual',
111+
})
112+
// Hands the browser back to a real app page rather than serving HTML from
113+
// the main process, so the closing screen matches the rest of Sim.
114+
expect(ok.status).toBe(302)
115+
expect(ok.headers.get('location')).toBe('https://sim.ai/desktop/done?kind=auth')
110116
expect(received).toEqual([{ token: VALID_TOKEN, state }])
111117

112118
await expect(
113-
fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`)
119+
fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`, { redirect: 'manual' })
114120
).rejects.toThrow()
115121
})
116122

@@ -135,8 +141,10 @@ describe('createHandoffManager', () => {
135141
expect(wrongState.status).toBe(403)
136142
expect(received).toHaveLength(0)
137143

138-
const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`)
139-
expect(ok.status).toBe(200)
144+
const ok = await fetch(`${base}/auth/callback?token=${VALID_TOKEN}&state=${state}`, {
145+
redirect: 'manual',
146+
})
147+
expect(ok.status).toBe(302)
140148
expect(received).toEqual([{ token: VALID_TOKEN, state }])
141149
})
142150

@@ -212,8 +220,11 @@ describe('createHandoffManager', () => {
212220
expect(badError.status).toBe(400)
213221
expect(received).toHaveLength(0)
214222

215-
const ok = await fetch(`${base}/connect/callback?state=${state}&error=oauth_failed`)
216-
expect(ok.status).toBe(200)
223+
const ok = await fetch(`${base}/connect/callback?state=${state}&error=oauth_failed`, {
224+
redirect: 'manual',
225+
})
226+
expect(ok.status).toBe(302)
227+
expect(ok.headers.get('location')).toBe('https://sim.ai/desktop/done?kind=connect')
217228
expect(received).toEqual([{ state, error: 'oauth_failed' }])
218229
expect(manager.consume(state, 'connect')).toBe(true)
219230
})
@@ -229,3 +240,33 @@ describe('createHandoffManager', () => {
229240
expect(manager.consume(state, 'connect')).toBe(true)
230241
})
231242
})
243+
244+
describe('connect handoff account pinning', () => {
245+
it('pins the connect flow to the account the app is signed in as', async () => {
246+
// The OAuth flow runs in the browser under the BROWSER's session, which is
247+
// a different row from the app's — without this the credential would attach
248+
// to whichever account the browser happens to be signed into.
249+
const deps = makeDeps({ currentUserId: vi.fn(async () => 'desktop-user') })
250+
const manager = createHandoffManager(deps, makeCallbacks())
251+
252+
expect(await manager.beginConnect('google-email')).toBe(true)
253+
254+
const landing = new URL(vi.mocked(deps.openExternal).mock.calls[0][0])
255+
expect(landing.pathname).toBe('/desktop/connect')
256+
expect(landing.searchParams.get('user')).toBe('desktop-user')
257+
manager.clear()
258+
})
259+
260+
it('omits the pin when the app account cannot be read', async () => {
261+
// Offline or signed out: fall back to the page's own login redirect rather
262+
// than blocking a connect on a failed probe.
263+
const deps = makeDeps({ currentUserId: vi.fn(async () => null) })
264+
const manager = createHandoffManager(deps, makeCallbacks())
265+
266+
expect(await manager.beginConnect('google-email')).toBe(true)
267+
268+
const landing = new URL(vi.mocked(deps.openExternal).mock.calls[0][0])
269+
expect(landing.searchParams.has('user')).toBe(false)
270+
manager.clear()
271+
})
272+
})

apps/desktop/src/main/handoff.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ const ERROR_SLUG_PATTERN = /^[A-Za-z0-9_-]{1,64}$/
2525
// stay valid.
2626
const HANDOFF_TTL_MS = 30 * 60 * 1000
2727

28-
function responsePage(message: string): string {
29-
return `<!doctype html>
30-
<html><head><meta charset="utf-8"><title>Sim</title></head>
31-
<body style="font-family:-apple-system,sans-serif;display:flex;align-items:center;justify-content:center;height:100vh;margin:0">
32-
<p>${message}</p>
33-
</body></html>`
34-
}
28+
/**
29+
* Where the browser lands once the loopback has taken the callback. Redirecting
30+
* to a real app page — rather than serving HTML from here — keeps the closing
31+
* screen on Sim's design system instead of a page hand-rolled in the main
32+
* process. Purely informational: the handoff has already completed by then.
33+
*/
34+
const DONE_PATH = '/desktop/done'
3535

3636
export type HandoffKind = 'login' | 'connect'
3737

@@ -54,6 +54,11 @@ export interface HandoffManagerDeps {
5454
origin: () => string
5555
openExternal: (url: string) => Promise<boolean>
5656
events: EventRecorder
57+
/**
58+
* The account the app is signed in as, used to pin an OAuth connect to it.
59+
* See {@link HandoffManager.beginConnect}.
60+
*/
61+
currentUserId: () => Promise<string | null>
5762
now?: () => number
5863
}
5964

@@ -104,12 +109,12 @@ export function createHandoffManager(
104109
* is one new row.
105110
*/
106111
interface LoopbackRoute {
107-
html: string
112+
kind: HandoffKind
108113
parse: (url: URL) => { state: string; dispatch: () => void } | null
109114
}
110115
const routes: Record<string, LoopbackRoute> = {
111116
[CALLBACK_PATH]: {
112-
html: responsePage('You’re signed in — return to the Sim app. You can close this tab.'),
117+
kind: 'login',
113118
parse: (url) => {
114119
const token = url.searchParams.get('token') ?? ''
115120
const state = url.searchParams.get('state') ?? ''
@@ -120,7 +125,7 @@ export function createHandoffManager(
120125
},
121126
},
122127
[CONNECT_CALLBACK_PATH]: {
123-
html: responsePage('Connection finished — return to the Sim app. You can close this tab.'),
128+
kind: 'connect',
124129
parse: (url) => {
125130
const state = url.searchParams.get('state') ?? ''
126131
const error = url.searchParams.get('error')
@@ -181,7 +186,9 @@ export function createHandoffManager(
181186
response.writeHead(403, { 'Content-Type': 'text/plain' }).end('Forbidden')
182187
return
183188
}
184-
response.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }).end(route.html)
189+
const done = new URL(DONE_PATH, deps.origin())
190+
done.searchParams.set('kind', route.kind === 'login' ? 'auth' : 'connect')
191+
response.writeHead(302, { Location: done.toString() }).end()
185192
stopLoopback()
186193
callback.dispatch()
187194
})
@@ -239,13 +246,21 @@ export function createHandoffManager(
239246
begin() {
240247
return beginFlow('login', '/desktop/auth', {})
241248
},
242-
beginConnect(providerId: string, scope: ConnectScope = {}) {
249+
async beginConnect(providerId: string, scope: ConnectScope = {}) {
243250
if (!PROVIDER_ID_PATTERN.test(providerId)) {
244251
logger.warn('Rejected connect handoff for invalid providerId')
245-
return Promise.resolve(false)
252+
return false
246253
}
254+
// The whole OAuth flow runs in the browser, under whatever account the
255+
// browser is signed into — which is no longer guaranteed to be this app's
256+
// account. Pin the flow to the app's user so a mismatch is refused instead
257+
// of quietly attaching the credential to the wrong account. Omitted when
258+
// unknown (offline, signed out): the page then falls back to its normal
259+
// login redirect rather than blocking a connect on a failed probe.
260+
const userId = await deps.currentUserId()
247261
return beginFlow('connect', '/desktop/connect', {
248262
provider: providerId,
263+
...(userId ? { user: userId } : {}),
249264
...(scope.workspaceId ? { workspaceId: scope.workspaceId } : {}),
250265
...(scope.credentialId ? { credentialId: scope.credentialId } : {}),
251266
})

apps/desktop/src/main/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
createSessionLifecycleCoordinator,
4343
decideStartRoute,
4444
handleConnectIntercept,
45+
readSessionUserId,
4546
resolveStartRoute,
4647
} from '@/main/session-lifecycle'
4748
import { attachTelemetryPolicy } from '@/main/telemetry-policy'
@@ -133,6 +134,7 @@ function main(): void {
133134
origin: appOrigin,
134135
openExternal: (url) => openExternalSafe(url, allowHttpLocalhost()),
135136
events,
137+
currentUserId: () => readSessionUserId(ensureAppSession(), appOrigin()),
136138
},
137139
{
138140
onLogin: (callback) => void authFlow.handleCallback(callback),
@@ -200,6 +202,7 @@ function main(): void {
200202
getWindows,
201203
clearHandoffState: async () => {
202204
handoff.clear()
205+
tray?.clearRecentChats()
203206
await localFilesystem.forgetAll()
204207
},
205208
clearBrowserProfile: clearAgentBrowserProfile,

0 commit comments

Comments
 (0)