mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(desktop): name the readiness-probe auth and password-gateway guard decisions
Two pure seams in native-auth-decisions.ts, following that module's existing pattern — the value is the test that pins the contract so the god-file call sites can't drift back to the buggy shape. resolveReadinessProbeAuth decides how the boot readiness probe authenticates, delegating to resolveOauthRestAuth for the oauth case rather than duplicating the bearer-vs-cookie rule. oauthGuardMayHardFail fixes a second way a gated gateway is misread. authModeFromStatus maps the gateway's `auth_required: true` onto 'oauth', but that flag only means the dashboard is GATED — it says nothing about how you authenticate. A gateway whose providers are all username/password can satisfy neither of the pre-flight guard's checks by construction: start_login raises NotImplementedError, /auth/native/authorize rejects password providers, and its cookies come from a plain password-login POST rather than the /auth/callback redirect the OAuth partition is primed for. The guard therefore threw "uses OAuth, but you are not signed in" one line before a mintGatewayWsTicket that would have succeeded against that very partition. The helper returns false only when EVERY advertised provider is password-based; an unknown or empty list keeps the strict guard, so backends predating /api/auth/providers are unaffected.
This commit is contained in:
parent
8d025489cf
commit
cf4fb8993f
2 changed files with 151 additions and 5 deletions
|
|
@ -1,7 +1,7 @@
|
|||
/**
|
||||
* 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.
|
||||
* Regression tests for electron/native-auth-decisions.ts — the 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).
|
||||
*/
|
||||
|
|
@ -10,7 +10,13 @@ import assert from 'node:assert/strict'
|
|||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { oauthSessionIsLive, resolveJsonBody, resolveOauthRestAuth } from './native-auth-decisions'
|
||||
import {
|
||||
oauthGuardMayHardFail,
|
||||
oauthSessionIsLive,
|
||||
resolveJsonBody,
|
||||
resolveOauthRestAuth,
|
||||
resolveReadinessProbeAuth
|
||||
} from './native-auth-decisions'
|
||||
|
||||
// --- 1. body encoding (guards the double-JSON.stringify 422) ---
|
||||
|
||||
|
|
@ -66,3 +72,61 @@ test('resolveOauthRestAuth falls back to cookie when there is no native token',
|
|||
// Empty string is not a usable bearer — must fall back, not send "Bearer ".
|
||||
assert.deepEqual(resolveOauthRestAuth(''), { kind: 'cookie' })
|
||||
})
|
||||
|
||||
// --- 4. readiness-probe auth (guards the credential-free 401 boot loop) ---
|
||||
|
||||
test('resolveReadinessProbeAuth reuses the oauth bearer-vs-cookie choice', () => {
|
||||
assert.deepEqual(resolveReadinessProbeAuth('oauth', 'native-at'), { kind: 'bearer', token: 'native-at' })
|
||||
assert.deepEqual(resolveReadinessProbeAuth('oauth', null), { kind: 'cookie' })
|
||||
assert.deepEqual(resolveReadinessProbeAuth('oauth', ''), { kind: 'cookie' })
|
||||
})
|
||||
|
||||
test('resolveReadinessProbeAuth sends the session token for a token gateway', () => {
|
||||
assert.deepEqual(resolveReadinessProbeAuth('token', null, 'session-token'), {
|
||||
kind: 'token',
|
||||
token: 'session-token'
|
||||
})
|
||||
assert.deepEqual(resolveReadinessProbeAuth('token', null, null), { kind: 'token', token: null })
|
||||
})
|
||||
|
||||
test('resolveReadinessProbeAuth stays public for local and unknown modes', () => {
|
||||
// A loopback backend has no gate; sending credentials it never issued is
|
||||
// meaningless, and an unknown mode must not invent a credential.
|
||||
assert.deepEqual(resolveReadinessProbeAuth('local', 'native-at', 'session-token'), { kind: 'public' })
|
||||
assert.deepEqual(resolveReadinessProbeAuth(undefined, 'native-at', 'session-token'), { kind: 'public' })
|
||||
assert.deepEqual(resolveReadinessProbeAuth('something-new', null, null), { kind: 'public' })
|
||||
})
|
||||
|
||||
// --- 5. oauth guard vs password gateways (guards the false "not signed in") ---
|
||||
|
||||
test('oauthGuardMayHardFail is false only when EVERY provider is password-based', () => {
|
||||
assert.equal(oauthGuardMayHardFail([{ name: 'basic', supportsPassword: true }]), false)
|
||||
assert.equal(
|
||||
oauthGuardMayHardFail([
|
||||
{ name: 'basic', supportsPassword: true },
|
||||
{ name: 'ldap', supportsPassword: true }
|
||||
]),
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
test('oauthGuardMayHardFail keeps the strict guard for oauth and mixed deployments', () => {
|
||||
assert.equal(oauthGuardMayHardFail([{ name: 'nous', supportsPassword: false }]), true)
|
||||
assert.equal(
|
||||
oauthGuardMayHardFail([
|
||||
{ name: 'nous', supportsPassword: false },
|
||||
{ name: 'basic', supportsPassword: true }
|
||||
]),
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
test('oauthGuardMayHardFail keeps the strict guard when the list is unusable', () => {
|
||||
// Backends predating /api/auth/providers, or an unreachable probe, must not
|
||||
// silently weaken the guard.
|
||||
assert.equal(oauthGuardMayHardFail([]), true)
|
||||
assert.equal(oauthGuardMayHardFail(null), true)
|
||||
assert.equal(oauthGuardMayHardFail(undefined), true)
|
||||
assert.equal(oauthGuardMayHardFail('nonsense' as any), true)
|
||||
assert.equal(oauthGuardMayHardFail([{ supportsPassword: true }]), true)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -21,7 +21,17 @@
|
|||
* 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
|
||||
* 4. resolveReadinessProbeAuth — the boot readiness probe must authenticate
|
||||
* the same way the rest of the connection does. A credential-free probe
|
||||
* against a gated gateway 401s forever; worse, it cannot tell a missing
|
||||
* route from a rejected session (see backend-health.ts).
|
||||
*
|
||||
* 5. oauthGuardMayHardFail — `auth_required: true` means "this gateway is
|
||||
* gated", NOT "this gateway speaks OAuth". A password-provider gateway
|
||||
* can satisfy neither the native-bearer nor the OAuth-partition-cookie
|
||||
* check by design, so the pre-flight guard must not hard-fail it.
|
||||
*
|
||||
* All five 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.
|
||||
*/
|
||||
|
||||
|
|
@ -60,3 +70,75 @@ export function resolveOauthRestAuth(nativeAccessToken: string | null | undefine
|
|||
|
||||
return { kind: 'cookie' }
|
||||
}
|
||||
|
||||
export type ReadinessProbeAuth = OauthRestAuth | { kind: 'token'; token: string | null } | { kind: 'public' }
|
||||
|
||||
/**
|
||||
* Decide how the boot readiness probe authenticates.
|
||||
*
|
||||
* The probe must present the SAME credentials the rest of the connection
|
||||
* will use. A credential-free probe against a gated gateway 401s until the
|
||||
* boot deadline even though the session is perfectly valid — and because the
|
||||
* dashboard auth gate runs ahead of the SPA catch-all, an unknown `/api/*`
|
||||
* path answers 401 rather than 404, so the probe also cannot detect a backend
|
||||
* that predates `/api/health`. Sending credentials is what lets a missing
|
||||
* route surface as a real 404 (see `isMissingHealthEndpointError`).
|
||||
*
|
||||
* `oauth` reuses `resolveOauthRestAuth` so the probe and every other oauth
|
||||
* REST call make the identical bearer-vs-cookie choice. `token` presents the
|
||||
* connection's session token. `local` (and anything unrecognized) stays
|
||||
* public: a loopback backend has no gate, and sending credentials it never
|
||||
* issued would be meaningless.
|
||||
*/
|
||||
export function resolveReadinessProbeAuth(
|
||||
authMode: string | null | undefined,
|
||||
nativeAccessToken?: string | null,
|
||||
connectionToken?: string | null
|
||||
): ReadinessProbeAuth {
|
||||
if (authMode === 'oauth') {
|
||||
return resolveOauthRestAuth(nativeAccessToken)
|
||||
}
|
||||
|
||||
if (authMode === 'token') {
|
||||
return { kind: 'token', token: connectionToken ?? null }
|
||||
}
|
||||
|
||||
return { kind: 'public' }
|
||||
}
|
||||
|
||||
export interface AdvertisedAuthProvider {
|
||||
name?: string
|
||||
supportsPassword?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the oauth pre-flight guard may hard-fail a connection for "not
|
||||
* signed in".
|
||||
*
|
||||
* `authModeFromStatus` maps the gateway's `auth_required: true` onto
|
||||
* `'oauth'`, but that flag only means the dashboard is GATED — it says
|
||||
* nothing about how you authenticate. A gateway whose providers are all
|
||||
* username/password cannot satisfy the guard's checks by construction:
|
||||
* `start_login` raises NotImplementedError, `/auth/native/authorize` rejects
|
||||
* password providers, and its cookies are set by a plain password-login POST
|
||||
* rather than the `/auth/callback` redirect the OAuth partition is primed
|
||||
* for. Hard-failing there rejects a live session one line before the
|
||||
* ws-ticket mint that would have succeeded against that very partition.
|
||||
*
|
||||
* Returns false only when EVERY advertised provider is password-based. An
|
||||
* unknown or empty list keeps the strict guard, so backends that predate
|
||||
* `/api/auth/providers` are unaffected.
|
||||
*/
|
||||
export function oauthGuardMayHardFail(providers: AdvertisedAuthProvider[] | null | undefined): boolean {
|
||||
if (!Array.isArray(providers) || providers.length === 0) {
|
||||
return true
|
||||
}
|
||||
|
||||
const named = providers.filter(provider => provider && typeof provider === 'object' && provider.name)
|
||||
|
||||
if (named.length === 0) {
|
||||
return true
|
||||
}
|
||||
|
||||
return !named.every(provider => provider.supportsPassword)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue