fix(desktop): fail remote test when OAuth ws-ticket mint fails
Youssef's review caught a residual false-positive: resolveTestWsUrl swallowed an OAuth ticket-mint failure and returned null, so the caller skipped the WS probe and reported the remote test as reachable. But the real boot path (resolveRemoteBackend) treats a mint failure as a hard 'session expired' auth error and refuses to connect — so an expired OAuth session passed the test then failed boot, the exact false-positive this PR exists to kill. Extract resolveTestWsUrl into the electron-free connection-config.cjs (injectable mintTicket) so it's unit-testable, and make OAuth mint failure throw an actionable needsOauthLogin error instead of skipping. Adds the three cases Youssef requested plus a mintTicket-required guard.
This commit is contained in:
@ -65,6 +65,59 @@ function buildGatewayWsUrlWithTicket(baseUrl, ticket) {
|
||||
return `${wsScheme}://${parsed.host}${prefix}/api/ws?ticket=${encodeURIComponent(ticket)}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the WS URL the renderer would connect with, so the connection test can
|
||||
* exercise the same transport the app actually uses.
|
||||
*
|
||||
* The OAuth ticket-minter is injected (`mintTicket(baseUrl) -> Promise<ticket>`)
|
||||
* so this stays electron-free and unit-testable; main.cjs passes the real
|
||||
* `mintGatewayWsTicket`.
|
||||
*
|
||||
* Return semantics:
|
||||
* - token mode + token → ws(s)://…/api/ws?token=…
|
||||
* - token mode, no token → null (genuine skip; nothing to authenticate with)
|
||||
* - oauth, mint ok → ws(s)://…/api/ws?ticket=…
|
||||
* - oauth, mint fails → THROWS (NOT a skip)
|
||||
*
|
||||
* The oauth-mint-failure throw is the important case: the real boot path
|
||||
* (resolveRemoteBackend in main.cjs) treats a mint failure as a hard
|
||||
* "session expired" auth error and refuses to connect. Swallowing it here
|
||||
* would re-introduce the exact false-positive this test exists to catch —
|
||||
* HTTP /api/status passes, the test reports "reachable", then the renderer
|
||||
* can't authenticate /api/ws and boot dies with "Could not connect".
|
||||
*
|
||||
* @param {string} baseUrl
|
||||
* @param {'token'|'oauth'} authMode
|
||||
* @param {string|null} token
|
||||
* @param {{ mintTicket: (baseUrl: string) => Promise<string> }} deps
|
||||
* @returns {Promise<string|null>}
|
||||
*/
|
||||
async function resolveTestWsUrl(baseUrl, authMode, token, deps = {}) {
|
||||
if (authMode === 'oauth') {
|
||||
const mintTicket = deps.mintTicket
|
||||
if (typeof mintTicket !== 'function') {
|
||||
throw new Error('resolveTestWsUrl: a mintTicket function is required in OAuth mode.')
|
||||
}
|
||||
let ticket
|
||||
try {
|
||||
ticket = await mintTicket(baseUrl)
|
||||
} catch (error) {
|
||||
const err = new Error(
|
||||
'Reached the gateway over HTTP, but could not mint a WebSocket ticket for the OAuth session ' +
|
||||
'(it may have expired). Open Settings → Gateway and sign in again.'
|
||||
)
|
||||
err.needsOauthLogin = true
|
||||
err.cause = error
|
||||
throw err
|
||||
}
|
||||
return buildGatewayWsUrlWithTicket(baseUrl, ticket)
|
||||
}
|
||||
if (!token) {
|
||||
return null
|
||||
}
|
||||
return buildGatewayWsUrl(baseUrl, token)
|
||||
}
|
||||
|
||||
function tokenPreview(value) {
|
||||
const raw = String(value || '')
|
||||
|
||||
@ -114,5 +167,6 @@ module.exports = {
|
||||
cookiesHaveSession,
|
||||
normalizeRemoteBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveTestWsUrl,
|
||||
tokenPreview
|
||||
}
|
||||
|
||||
@ -21,6 +21,7 @@ const {
|
||||
cookiesHaveSession,
|
||||
normalizeRemoteBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveTestWsUrl,
|
||||
tokenPreview
|
||||
} = require('./connection-config.cjs')
|
||||
|
||||
@ -159,3 +160,52 @@ test('tokenPreview returns set for short tokens', () => {
|
||||
test('tokenPreview returns a masked suffix for long tokens', () => {
|
||||
assert.equal(tokenPreview('abcdefghijklmnop'), '...klmnop')
|
||||
})
|
||||
|
||||
// --- resolveTestWsUrl ---
|
||||
//
|
||||
// The "Test remote" button must exercise the same WS transport the app uses,
|
||||
// and must FAIL (not skip) when an OAuth session can't mint a ws-ticket — that
|
||||
// is the exact false-positive PR #39098 set out to eliminate.
|
||||
|
||||
test('resolveTestWsUrl (token mode) builds a ?token= URL the WS probe can use', async () => {
|
||||
const url = await resolveTestWsUrl('https://gw.example.com', 'token', 'tok123')
|
||||
assert.equal(url, 'wss://gw.example.com/api/ws?token=tok123')
|
||||
})
|
||||
|
||||
test('resolveTestWsUrl (token mode, no token) returns null — genuine skip', async () => {
|
||||
assert.equal(await resolveTestWsUrl('https://gw.example.com', 'token', null), null)
|
||||
})
|
||||
|
||||
test('resolveTestWsUrl (oauth, mint ok) builds a ?ticket= URL', async () => {
|
||||
const url = await resolveTestWsUrl('https://gw.example.com', 'oauth', null, {
|
||||
mintTicket: async () => 'tkt-9'
|
||||
})
|
||||
assert.equal(url, 'wss://gw.example.com/api/ws?ticket=tkt-9')
|
||||
})
|
||||
|
||||
test('resolveTestWsUrl (oauth, mint FAILS) throws — must NOT skip WS validation', async () => {
|
||||
await assert.rejects(
|
||||
() =>
|
||||
resolveTestWsUrl('https://gw.example.com', 'oauth', null, {
|
||||
mintTicket: async () => {
|
||||
throw new Error('401 ticket mint failed')
|
||||
}
|
||||
}),
|
||||
err => {
|
||||
// Actionable, points the user at re-auth, and preserves the cause + flag
|
||||
// the boot overlay uses to offer a sign-in prompt.
|
||||
assert.match(err.message, /WebSocket ticket/i)
|
||||
assert.match(err.message, /sign in again/i)
|
||||
assert.equal(err.needsOauthLogin, true)
|
||||
assert.ok(err.cause instanceof Error)
|
||||
return true
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('resolveTestWsUrl (oauth) requires a mintTicket function', async () => {
|
||||
await assert.rejects(
|
||||
() => resolveTestWsUrl('https://gw.example.com', 'oauth', null),
|
||||
/mintTicket function is required/
|
||||
)
|
||||
})
|
||||
|
||||
@ -35,6 +35,7 @@ const {
|
||||
cookiesHaveSession,
|
||||
normalizeRemoteBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveTestWsUrl,
|
||||
tokenPreview
|
||||
} = require('./connection-config.cjs')
|
||||
const {
|
||||
@ -3770,7 +3771,7 @@ async function testDesktopConnectionConfig(input = {}) {
|
||||
// false-positive "reachable" while the real boot still failed with "Could not
|
||||
// connect to Hermes gateway". Mirror the renderer's connect here so the test
|
||||
// reflects the full path the app actually uses.
|
||||
const wsUrl = await resolveTestWsUrl(baseUrl, authMode, token)
|
||||
const wsUrl = await resolveTestWsUrl(baseUrl, authMode, token, { mintTicket: mintGatewayWsTicket })
|
||||
// Skip the WS leg only when the runtime genuinely lacks a WebSocket (so an
|
||||
// older Electron/Node never fails the test spuriously); Electron's main
|
||||
// process ships a global WebSocket on every supported version.
|
||||
@ -3791,26 +3792,6 @@ async function testDesktopConnectionConfig(input = {}) {
|
||||
}
|
||||
}
|
||||
|
||||
// Build the WS URL the renderer would connect with, so the connection test can
|
||||
// exercise the same transport. OAuth gateways need a freshly minted single-use
|
||||
// ticket; token/local gateways carry a long-lived token in the query string. A
|
||||
// null return means we can't form a credentialed URL (e.g. OAuth without a live
|
||||
// session) and the WS leg of the test is skipped rather than failing spuriously.
|
||||
async function resolveTestWsUrl(baseUrl, authMode, token) {
|
||||
if (authMode === 'oauth') {
|
||||
try {
|
||||
const ticket = await mintGatewayWsTicket(baseUrl)
|
||||
return buildGatewayWsUrlWithTicket(baseUrl, ticket)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
if (!token) {
|
||||
return null
|
||||
}
|
||||
return buildGatewayWsUrl(baseUrl, token)
|
||||
}
|
||||
|
||||
function resetBootProgressForReconnect() {
|
||||
updateBootProgress(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user