From 50db7014af89d2e3c384b0c893eccf80e5c7fd31 Mon Sep 17 00:00:00 2001 From: zakhounet Date: Sun, 26 Jul 2026 14:09:07 +0200 Subject: [PATCH] fix(desktop): authenticate profile session slices against oauth gateways MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two batched `/api/profiles/sessions` dispatches resolved the primary connection and then passed `conn.token` straight to `fetchJson`, skipping the `authMode` branch that `requestJsonForProfile` applies to every other per-profile path. Against an oauth-gated gateway that token is not a valid credential, so both requests 401. Each call site collapses a rejected fetch into `{ sessions: [], total: 0 }`, so the failure surfaces nowhere: the Desktop session sidebar renders empty for the root profile while the sessions are intact server-side and the named profiles list normally. Route both dispatches through `fetchJsonForProfile`, and name the branch they skipped as `resolveProfileRestAuth`. It delegates to `resolveReadinessProbeAuth` and keeps a single deliberate difference: a REST call retains the connection token when `authMode` is not yet known, where the readiness probe treats an unknown gateway as public — probing anonymously is safe, dropping the credential from a real request is not. Unit tests pin both the oauth contract and that divergence. Fixes #67600 --- apps/desktop/electron/main.ts | 43 +++++++++-------- .../electron/native-auth-decisions.test.ts | 47 +++++++++++++++++++ .../desktop/electron/native-auth-decisions.ts | 30 ++++++++++++ 3 files changed, 100 insertions(+), 20 deletions(-) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index f967aa24c3b8..54a9109f389d 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -124,6 +124,7 @@ import { oauthSessionIsLive, resolveJsonBody, resolveOauthRestAuth, + resolveProfileRestAuth, resolveReadinessProbeAuth } from './native-auth-decisions' import { @@ -7374,19 +7375,23 @@ async function requestJsonForProfile(profile: string, path: string, method: stri const url = `${conn.baseUrl}${path}` const opts = { method, body, timeoutMs: DEFAULT_FETCH_TIMEOUT_MS } - if (conn.authMode === 'oauth') { - // Native RFC 8252 flow: authenticate with the bearer token (cookieless) - // when we hold one for this gateway; otherwise use the cookie partition. - const nativeAt = await ensureNativeAccessToken(conn.baseUrl).catch(() => null) + // Native RFC 8252 flow: authenticate with the bearer token (cookieless) when + // we hold one for this gateway; otherwise use the cookie partition. Only a + // non-oauth gateway may use conn.token — see resolveProfileRestAuth. + const nativeAt = + conn.authMode === 'oauth' ? await ensureNativeAccessToken(conn.baseUrl).catch(() => null) : null - if (nativeAt) { - return fetchJson(url, null, { ...opts, bearer: nativeAt }) - } + const auth = resolveProfileRestAuth(conn.authMode, nativeAt, conn.token) + + if (auth.kind === 'bearer') { + return fetchJson(url, null, { ...opts, bearer: auth.token }) + } + if (auth.kind === 'cookie') { return fetchJsonViaOauthSession(url, opts) } - return fetchJson(url, conn.token, opts) + return fetchJson(url, auth.token, opts) } async function probeRemoteAuthMode(rawUrl) { @@ -9555,12 +9560,11 @@ async function fetchProfilesSessionSlice(searchParams, remoteProfiles) { return remoteSessionList(requested, searchParams) } - const primary = await ensureBackend(null) - - return fetchJson(`${primary.baseUrl}/api/profiles/sessions?${searchParams}`, primary.token, { - method: 'GET', - timeoutMs: DEFAULT_FETCH_TIMEOUT_MS - }).catch(() => ({ sessions: [], total: 0, profile_totals: {} })) + return fetchJsonForProfile(null, `/api/profiles/sessions?${searchParams}`).catch(() => ({ + sessions: [], + total: 0, + profile_totals: {} + })) } return mergeRemoteProfileSessions(searchParams, remoteProfiles) @@ -9575,12 +9579,11 @@ async function mergeRemoteProfileSessions(searchParams, remoteProfiles) { const offset = Math.max(0, Number(searchParams.get('offset')) || 0) const order = searchParams.get('order') === 'created' ? 'started_at' : 'last_active' - const primary = await ensureBackend(null) - - const base = (await fetchJson(`${primary.baseUrl}/api/profiles/sessions?${searchParams}`, primary.token, { - method: 'GET', - timeoutMs: DEFAULT_FETCH_TIMEOUT_MS - }).catch(() => ({ sessions: [], total: 0, profile_totals: {} }))) as any + const base = (await fetchJsonForProfile(null, `/api/profiles/sessions?${searchParams}`).catch(() => ({ + sessions: [], + total: 0, + profile_totals: {} + }))) as any // Over-fetch each remote from offset 0 (limit+offset rows) so the merged window // is correct for this page — mirrors the primary's per-profile over-fetch. diff --git a/apps/desktop/electron/native-auth-decisions.test.ts b/apps/desktop/electron/native-auth-decisions.test.ts index d4cfc068cd66..72aedbdedc00 100644 --- a/apps/desktop/electron/native-auth-decisions.test.ts +++ b/apps/desktop/electron/native-auth-decisions.test.ts @@ -15,6 +15,7 @@ import { oauthSessionIsLive, resolveJsonBody, resolveOauthRestAuth, + resolveProfileRestAuth, resolveReadinessProbeAuth } from './native-auth-decisions' @@ -130,3 +131,49 @@ test('oauthGuardMayHardFail keeps the strict guard when the list is unusable', ( assert.equal(oauthGuardMayHardFail('nonsense' as any), true) assert.equal(oauthGuardMayHardFail([{ supportsPassword: true }]), true) }) + +// --- Per-profile REST auth selection (guards the empty sidebar of #67600) --- + +test('resolveProfileRestAuth uses the connection token on a token-auth gateway', () => { + assert.deepEqual(resolveProfileRestAuth('token', null, 'conn-token'), { + kind: 'connection-token', + token: 'conn-token' + }) +}) + +test('resolveProfileRestAuth keeps the connection token when authMode is unknown', () => { + // Deliberate divergence from resolveReadinessProbeAuth, which treats an + // unknown gateway as public: a REST call must not silently drop its + // credential just because the mode has not been resolved yet. + assert.deepEqual(resolveProfileRestAuth('unknown', null, 'conn-token'), { + kind: 'connection-token', + token: 'conn-token' + }) + assert.deepEqual(resolveProfileRestAuth(undefined, null, 'conn-token'), { + kind: 'connection-token', + token: 'conn-token' + }) + assert.deepEqual(resolveProfileRestAuth(null, null, null), { kind: 'connection-token', token: null }) +}) + +test('resolveProfileRestAuth prefers the native bearer on an oauth gateway', () => { + assert.deepEqual(resolveProfileRestAuth('oauth', 'bearer-token-123', 'conn-token'), { + kind: 'bearer', + token: 'bearer-token-123' + }) +}) + +test('resolveProfileRestAuth falls back to the cookie partition on an oauth gateway', () => { + assert.deepEqual(resolveProfileRestAuth('oauth', null, 'conn-token'), { kind: 'cookie' }) +}) + +test('resolveProfileRestAuth never authenticates an oauth gateway with the connection token', () => { + // The #67600 regression: call sites that ignored authMode passed the + // connection token to an oauth-gated gateway, which 401s. Whatever the + // native token state, an oauth gateway must never resolve to that token. + for (const nativeAt of ['bearer-token-123', null, undefined, '']) { + const auth = resolveProfileRestAuth('oauth', nativeAt, 'conn-token') + + assert.notEqual(auth.kind, 'connection-token') + } +}) diff --git a/apps/desktop/electron/native-auth-decisions.ts b/apps/desktop/electron/native-auth-decisions.ts index c0978c3ecddc..c3248ce5abba 100644 --- a/apps/desktop/electron/native-auth-decisions.ts +++ b/apps/desktop/electron/native-auth-decisions.ts @@ -142,3 +142,33 @@ export function oauthGuardMayHardFail(providers: AdvertisedAuthProvider[] | null return !named.every(provider => provider.supportsPassword) } + +export type ProfileRestAuth = OauthRestAuth | { kind: 'connection-token'; token: string | null } + +/** + * Decide how a per-profile REST request authenticates. Same branch as + * resolveReadinessProbeAuth, with one deliberate difference: a REST call keeps + * sending the connection token when authMode is not yet known, where the + * readiness probe treats an unknown gateway as public. Probing anonymously is + * safe; dropping the credential from a real request is not. + * + * This is the seam the /api/profiles/sessions call sites skipped: they resolved + * the connection and passed `conn.token` straight to fetchJson, ignoring + * authMode entirely. Against an oauth gateway that token is not a valid + * credential, so the request 401s — and because those callers collapse a + * rejected fetch into an empty slice, the session sidebar renders empty with no + * error surfaced anywhere (#67600). + */ +export function resolveProfileRestAuth( + authMode: string | null | undefined, + nativeAccessToken: string | null | undefined, + connectionToken: string | null | undefined +): ProfileRestAuth { + const probe = resolveReadinessProbeAuth(authMode, nativeAccessToken, connectionToken) + + if (probe.kind === 'bearer' || probe.kind === 'cookie') { + return probe + } + + return { kind: 'connection-token', token: connectionToken ?? null } +}