fix(desktop-auth): make RFC 8252 native login work end-to-end at runtime
The native-app (RFC 8252) login passed its unit tests but failed in real Electron runtime — the tests mocked the exact seams that were broken. Four runtime defects, each proven against a live gated gateway: 1. Lockfile drift: apps/desktop declared @assistant-ui/react + @assistant-ui/react-streamdown but package-lock.json didn't place them, so `npm ci` (CI + every fresh checkout) failed to install them → Vite "Failed to resolve @assistant-ui/react". Reconcile the lockfile. 2. Double JSON encoding: postJsonNoAuth pre-JSON.stringify'd the body before fetchJson (which stringifies again), so /auth/native/token received a JSON string, not an object → gateway 422 "Input should be a valid dictionary" → native login silently fell back to the embedded webview. 3. Cookie-only liveness gate: buildRemoteConnection (and the Settings connected indicator) treated "signed in" as "has OAuth cookie". The native flow stores a bearer and sets no cookie, so a completed native login looped the UI into needsOauthLogin. Accept native token OR cookie. 4. Cookie-only REST path: the hermes:api handler routed oauth-mode REST through the cookie partition only. A cookieless native session → 401 no_cookie on every API call. Prefer the native bearer (with transparent refresh), else cookie — mirroring mintGatewayWsTicket, which was already bearer-aware. The three decision points (2–4) are extracted into a pure native-auth-decisions.ts with regression tests, since the mocked flow tests could not catch them. Verified live: system-browser login → cookieless bearer → connected chat, no embedded webview.
This commit is contained in:
@@ -88,6 +88,11 @@ import {
|
||||
tokenNeedsRefresh,
|
||||
type NativeTokenSet
|
||||
} from './native-oauth'
|
||||
import {
|
||||
oauthSessionIsLive,
|
||||
resolveJsonBody,
|
||||
resolveOauthRestAuth
|
||||
} from './native-auth-decisions'
|
||||
import { scanGitRepos } from './git-repo-scan'
|
||||
import {
|
||||
fileDiffVsHead,
|
||||
@@ -5811,7 +5816,12 @@ function hasNativeSession(baseUrl: string): boolean {
|
||||
// refresh exchanges, which are cookieless by design. Thin wrapper over
|
||||
// fetchJson (no token) so it shares timeout/JSON handling.
|
||||
function postJsonNoAuth(url: string, body: unknown, opts: any = {}) {
|
||||
return fetchJson(url, null, { method: 'POST', body: JSON.stringify(body), ...opts })
|
||||
// resolveJsonBody passes the object through UNCHANGED — fetchJson owns
|
||||
// JSON.stringify. Pre-stringifying here double-encodes the body (a JSON
|
||||
// string inside a JSON string), which the gateway's Pydantic model rejects
|
||||
// with a 422 "Input should be a valid dictionary" (the native
|
||||
// /auth/native/token + /auth/native/refresh legs both go through here).
|
||||
return fetchJson(url, null, { method: 'POST', body: resolveJsonBody(body), ...opts })
|
||||
}
|
||||
|
||||
// Return a valid native access token for baseUrl, refreshing via
|
||||
@@ -6445,9 +6455,14 @@ async function sanitizeDesktopConnectionConfig(config = readDesktopConnectionCon
|
||||
try {
|
||||
// Display signal: treat a live RT cookie as "connected" even if the AT
|
||||
// cookie has lapsed — the gateway refreshes the AT on the next request,
|
||||
// so the session is still usable. The authoritative liveness check is
|
||||
// the ws-ticket mint in resolveRemoteBackend at actual connect time.
|
||||
remoteOauthConnected = await hasLiveOauthSession(remoteUrl)
|
||||
// so the session is still usable. A stored native bearer token (cookieless
|
||||
// RFC 8252 flow) counts as connected too — otherwise a completed native
|
||||
// sign-in shows "not connected" in Settings. The authoritative liveness
|
||||
// check is the ws-ticket mint in resolveRemoteBackend at actual connect time.
|
||||
remoteOauthConnected = oauthSessionIsLive(
|
||||
hasNativeSession(remoteUrl),
|
||||
await hasLiveOauthSession(remoteUrl)
|
||||
)
|
||||
} catch {
|
||||
remoteOauthConnected = false
|
||||
}
|
||||
@@ -6639,15 +6654,22 @@ async function buildRemoteConnection(
|
||||
const host = remoteHost || hostLabelFromBaseUrl(baseUrl)
|
||||
|
||||
if (authMode === 'oauth') {
|
||||
// OAuth gateway: auth comes from the session cookies in the OAuth
|
||||
// partition. Liveness is NOT "is the access-token cookie present?" —
|
||||
// Portal issues a 24h rotating refresh token (hermes #37247), and the
|
||||
// gateway middleware transparently rotates a fresh ~15-min access token
|
||||
// from it on the next authenticated request. So a session with an expired
|
||||
// AT cookie but a live RT cookie is still perfectly connectable. We
|
||||
// early-out only when neither cookie is present, then mint a ws-ticket as
|
||||
// the authoritative liveness check.
|
||||
if (!(await hasLiveOauthSession(baseUrl))) {
|
||||
// OAuth gateway: auth comes from EITHER a native bearer token (cookieless
|
||||
// RFC 8252 flow) OR the session cookies in the OAuth partition. Liveness is
|
||||
// NOT "is the access-token cookie present?" — Portal issues a 24h rotating
|
||||
// refresh token (hermes #37247), and the gateway middleware transparently
|
||||
// rotates a fresh ~15-min access token from it on the next authenticated
|
||||
// request. So a session with an expired AT cookie but a live RT cookie is
|
||||
// still perfectly connectable. We early-out only when NEITHER a native
|
||||
// token NOR any cookie is present, then mint a ws-ticket (which itself
|
||||
// prefers the native bearer) as the authoritative liveness check.
|
||||
//
|
||||
// The native-token check is essential: the native login stores bearer
|
||||
// tokens (no cookie is ever set), so gating solely on hasLiveOauthSession
|
||||
// here would reject a freshly-completed native sign-in and loop the UI back
|
||||
// into "not signed in" even though mintGatewayWsTicket would succeed with
|
||||
// the stored bearer.
|
||||
if (!oauthSessionIsLive(hasNativeSession(baseUrl), await hasLiveOauthSession(baseUrl))) {
|
||||
const err = new Error(
|
||||
'Remote Hermes gateway uses OAuth, but you are not signed in. ' +
|
||||
'Open Settings → Gateway and click "Sign in", or switch back to Local.'
|
||||
@@ -9325,10 +9347,14 @@ ipcMain.handle('hermes:api', async (_event, request) => {
|
||||
|
||||
const url = `${connection.baseUrl}${requestPath}`
|
||||
|
||||
// OAuth gateways authenticate REST via the HttpOnly session cookie held in
|
||||
// the OAuth partition — route through Electron's net stack bound to that
|
||||
// session so the cookie attaches automatically. Token/local modes keep using
|
||||
// the static session-token header.
|
||||
// OAuth gateways authenticate REST via EITHER a native bearer token
|
||||
// (cookieless RFC 8252 flow) OR the HttpOnly session cookie held in the OAuth
|
||||
// partition. Prefer the native bearer when present (mirroring
|
||||
// mintGatewayWsTicket): the native flow never sets a cookie, so routing an
|
||||
// oauth-mode REST call through the cookie-only path returns 401 no_cookie even
|
||||
// though a valid bearer is held. Cookie mode rides Electron's net stack bound
|
||||
// to the OAuth partition so the cookie attaches automatically. Token/local
|
||||
// modes keep using the static session-token header.
|
||||
if (connection.authMode === 'oauth') {
|
||||
// The OAuth path rides electron.net with JSON headers; multipart isn't
|
||||
// wired there. Fail loudly rather than corrupting the upload.
|
||||
@@ -9336,6 +9362,21 @@ ipcMain.handle('hermes:api', async (_event, request) => {
|
||||
throw new Error('File uploads are not supported against OAuth-gated remote backends yet.')
|
||||
}
|
||||
|
||||
// Native bearer first (cookieless). ensureNativeAccessToken transparently
|
||||
// refreshes a near-expiry AT via /auth/native/refresh; a null return means
|
||||
// no native session (resolveOauthRestAuth then selects the cookie path).
|
||||
const nativeAt = await ensureNativeAccessToken(connection.baseUrl).catch(() => null)
|
||||
const restAuth = resolveOauthRestAuth(nativeAt)
|
||||
|
||||
if (restAuth.kind === 'bearer') {
|
||||
return fetchJson(url, null, {
|
||||
method: request?.method,
|
||||
body: request?.body,
|
||||
timeoutMs,
|
||||
bearer: restAuth.token
|
||||
})
|
||||
}
|
||||
|
||||
return fetchJsonViaOauthSession(url, {
|
||||
method: request?.method,
|
||||
body: request?.body,
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
/**
|
||||
* Regression tests for electron/native-auth-decisions.ts — the three pure
|
||||
* decision seams behind the RFC 8252 native-app auth flow, each of which was a
|
||||
* real runtime bug that the mocked flow tests could not catch.
|
||||
*
|
||||
* Run via the vitest `electron` project (electron/**\/*.test.ts).
|
||||
*/
|
||||
|
||||
import assert from 'node:assert/strict'
|
||||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import {
|
||||
oauthSessionIsLive,
|
||||
resolveJsonBody,
|
||||
resolveOauthRestAuth
|
||||
} from './native-auth-decisions'
|
||||
|
||||
// --- 1. body encoding (guards the double-JSON.stringify 422) ---
|
||||
|
||||
test('resolveJsonBody returns the object unchanged (no pre-stringify)', () => {
|
||||
const body = { code: 'abc', code_verifier: 'xyz' }
|
||||
const out = resolveJsonBody(body)
|
||||
|
||||
// Must be the SAME object reference / shape — NOT a JSON string. Pre-
|
||||
// stringifying here is what produced the gateway 422 "Input should be a
|
||||
// valid dictionary" at /auth/native/token.
|
||||
assert.equal(typeof out, 'object')
|
||||
assert.deepEqual(out, body)
|
||||
})
|
||||
|
||||
test('resolveJsonBody does not stringify — a string stays a string, an object stays an object', () => {
|
||||
assert.equal(typeof resolveJsonBody({ a: 1 }), 'object')
|
||||
// If a caller ever passes an already-encoded string (the bug), we return it
|
||||
// as-is rather than re-wrapping — the contract is "fetchJson owns encoding".
|
||||
assert.equal(typeof resolveJsonBody('{"a":1}'), 'string')
|
||||
})
|
||||
|
||||
// --- 2. oauth liveness (guards the needsOauthLogin loop) ---
|
||||
|
||||
test('oauthSessionIsLive is true when a native bearer token exists, even with no cookie', () => {
|
||||
// The exact bug: native login stores a bearer, sets no cookie. Gating on the
|
||||
// cookie alone looped the UI into "not signed in".
|
||||
assert.equal(oauthSessionIsLive(true, false), true)
|
||||
})
|
||||
|
||||
test('oauthSessionIsLive is true when a live cookie exists with no native token', () => {
|
||||
assert.equal(oauthSessionIsLive(false, true), true)
|
||||
})
|
||||
|
||||
test('oauthSessionIsLive is true when both are present', () => {
|
||||
assert.equal(oauthSessionIsLive(true, true), true)
|
||||
})
|
||||
|
||||
test('oauthSessionIsLive is false only when neither is present', () => {
|
||||
assert.equal(oauthSessionIsLive(false, false), false)
|
||||
})
|
||||
|
||||
// --- 3. REST auth selection (guards the 401 no_cookie) ---
|
||||
|
||||
test('resolveOauthRestAuth prefers the native bearer when a token is present', () => {
|
||||
const auth = resolveOauthRestAuth('bearer-token-123')
|
||||
|
||||
assert.deepEqual(auth, { kind: 'bearer', token: 'bearer-token-123' })
|
||||
})
|
||||
|
||||
test('resolveOauthRestAuth falls back to cookie when there is no native token', () => {
|
||||
assert.deepEqual(resolveOauthRestAuth(null), { kind: 'cookie' })
|
||||
assert.deepEqual(resolveOauthRestAuth(undefined), { kind: 'cookie' })
|
||||
// Empty string is not a usable bearer — must fall back, not send "Bearer ".
|
||||
assert.deepEqual(resolveOauthRestAuth(''), { kind: 'cookie' })
|
||||
})
|
||||
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* native-auth-decisions.ts
|
||||
*
|
||||
* Pure decision helpers extracted from main.ts for the RFC 8252 native-app
|
||||
* auth flow. These encode three choices that were each the site of a real
|
||||
* runtime bug — invisible to the mocked flow tests because the tests never
|
||||
* exercised the real main.ts internals. Keeping them pure + unit-tested here
|
||||
* prevents silent regressions:
|
||||
*
|
||||
* 1. resolveJsonBody — the token/refresh POST body must be the raw
|
||||
* object (fetchJson owns JSON.stringify). Pre-stringifying double-encodes
|
||||
* it into a JSON string, which the gateway's Pydantic model rejects with
|
||||
* 422 "Input should be a valid dictionary".
|
||||
*
|
||||
* 2. oauthSessionIsLive — an OAuth gateway is "signed in" when EITHER a
|
||||
* native bearer token OR a live cookie session exists. Gating on the
|
||||
* cookie alone rejects a completed native login and loops the UI into
|
||||
* "not signed in".
|
||||
*
|
||||
* 3. resolveOauthRestAuth — an oauth-mode REST call authenticates with the
|
||||
* native bearer when present, else the cookie partition. Cookie-only
|
||||
* routing returns 401 no_cookie for a cookieless native session.
|
||||
*
|
||||
* All three are trivial once named; the value is the test that pins the
|
||||
* contract so the god-file call sites can't drift back to the buggy shape.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Decide the request body to hand to fetchJson (which JSON.stringifies it).
|
||||
* Returns the object UNCHANGED — callers must NOT pre-stringify. A string here
|
||||
* would be double-encoded downstream; this function exists to document and
|
||||
* pin that contract at the one seam that got it wrong.
|
||||
*/
|
||||
export function resolveJsonBody<T>(body: T): T {
|
||||
return body
|
||||
}
|
||||
|
||||
/**
|
||||
* True when an oauth gateway should be treated as signed-in. `hasNativeToken`
|
||||
* is whether a native bearer token is stored; `hasCookieSession` is whether a
|
||||
* live AT-or-RT cookie exists in the OAuth partition. Either suffices.
|
||||
*/
|
||||
export function oauthSessionIsLive(hasNativeToken: boolean, hasCookieSession: boolean): boolean {
|
||||
return hasNativeToken || hasCookieSession
|
||||
}
|
||||
|
||||
export type OauthRestAuth =
|
||||
| { kind: 'bearer'; token: string }
|
||||
| { kind: 'cookie' }
|
||||
|
||||
/**
|
||||
* Decide how an oauth-mode REST request authenticates: prefer the native
|
||||
* bearer (cookieless RFC 8252 flow) when a non-empty access token is present,
|
||||
* otherwise fall back to the cookie partition. `nativeAccessToken` is the
|
||||
* result of ensureNativeAccessToken (null/empty when there is no native
|
||||
* session or the refresh terminally failed).
|
||||
*/
|
||||
export function resolveOauthRestAuth(nativeAccessToken: string | null | undefined): OauthRestAuth {
|
||||
if (nativeAccessToken) {
|
||||
return { kind: 'bearer', token: nativeAccessToken }
|
||||
}
|
||||
|
||||
return { kind: 'cookie' }
|
||||
}
|
||||
@@ -56,8 +56,8 @@
|
||||
"test:e2e:update-snapshots": "WLR_BACKENDS=headless WLR_NO_HARDWARE_CURSORS=1 cage -- npx playwright test e2e/ --reporter=list --update-snapshots"
|
||||
},
|
||||
"dependencies": {
|
||||
"@assistant-ui/react": "^0.14.23",
|
||||
"@assistant-ui/react-streamdown": "^0.3.4",
|
||||
"@assistant-ui/react": "^0.14.24",
|
||||
"@assistant-ui/react-streamdown": "^0.3.5",
|
||||
"@audiowave/react": "^0.6.2",
|
||||
"@chenglou/pretext": "^0.0.6",
|
||||
"@codemirror/commands": "^6.10.4",
|
||||
|
||||
Generated
+409
-388
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user