diff --git a/src/auth/refresh-wiring.ts b/src/auth/refresh-wiring.ts index e962ad2..750c6f8 100644 --- a/src/auth/refresh-wiring.ts +++ b/src/auth/refresh-wiring.ts @@ -18,7 +18,9 @@ export interface RefreshWiring { /** * Decides the session-auth merge and refresh-retry wiring shared by both * cli-builder.ts client-construction sites (the createCli routine-runtime path - * and the run() path). Kept pure so both sites stay in lockstep (#135). + * and the run() path). Shared by both sites so they stay in lockstep (#135); + * the only side effect is a diagnostic console.warn on a suspicious sessionAuth + * block (see below). * * `options.refreshOn` (from CliOptions / .apijack/settings.json) takes * precedence over `sessionAuth.refreshOn`, so a project can opt a custom @@ -37,9 +39,20 @@ export function resolveRefreshWiring( const refreshOn = options.refreshOn ?? rawSessionAuth?.refreshOn; if (rawSessionAuth && !mergedSessionAuth) { - console.warn( - '[apijack] sessionAuth is set but missing session.endpoint — SessionAuthStrategy will not be used.', + // An endpoint-less block that carries nothing but `refreshOn` is a deliberate, + // supported config (#135) — refreshOn survives the narrowing above precisely so + // this works without a session.endpoint. Only warn when there's something else + // in the block, which is the signature of a typo'd handshake key (e.g. `sessions:` + // instead of `session:`) rather than an intentional refresh-only block. + const foundKeys = Object.keys(rawSessionAuth).filter( + key => key !== 'refreshOn' && (rawSessionAuth as Record)[key] !== undefined, ); + + if (foundKeys.length > 0) { + console.warn( + `[apijack] sessionAuth is set but missing session.endpoint — SessionAuthStrategy will not be used. Found: ${foundKeys.join(', ')}.`, + ); + } } return { mergedSessionAuth, refreshOn }; diff --git a/tests/auth/refresh-wiring.test.ts b/tests/auth/refresh-wiring.test.ts index 0f0c6d7..48d2f10 100644 --- a/tests/auth/refresh-wiring.test.ts +++ b/tests/auth/refresh-wiring.test.ts @@ -50,17 +50,12 @@ describe('resolveRefreshWiring', () => { // Not expressible through the SessionAuthConfig type from a fully-typed // caller, but envConfig.sessionAuth is only a Partial — // a JS/dynamic caller could still hand cli-builder a refreshOn-only block. - const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}); const refreshOnlySessionAuth = { refreshOn: [401] } as unknown as SessionAuthConfig; - try { - const result = resolveRefreshWiring({ sessionAuth: refreshOnlySessionAuth }, undefined); - expect(result.mergedSessionAuth).toBeUndefined(); - // refreshOn still surfaces from the raw (unguarded) merge. - expect(result.refreshOn).toEqual([401]); - } finally { - warnSpy.mockRestore(); - } + const result = resolveRefreshWiring({ sessionAuth: refreshOnlySessionAuth }, undefined); + expect(result.mergedSessionAuth).toBeUndefined(); + // refreshOn still surfaces from the raw (unguarded) merge. + expect(result.refreshOn).toEqual([401]); }); test('does not mutate inputs', () => { @@ -73,14 +68,75 @@ describe('resolveRefreshWiring', () => { }); describe('missing session.endpoint diagnostic warning', () => { - test('warns when sessionAuth is set but has no session.endpoint', () => { + test('is silent for a deliberate refreshOn-only sessionAuth block (#135, #148)', () => { const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}); const refreshOnlySessionAuth = { refreshOn: [401] } as unknown as SessionAuthConfig; try { - resolveRefreshWiring({ sessionAuth: refreshOnlySessionAuth }, undefined); + const result = resolveRefreshWiring({ sessionAuth: refreshOnlySessionAuth }, undefined); + expect(warnSpy).not.toHaveBeenCalled(); + // refreshOn behavior from #135 is unchanged: it still surfaces from the + // raw (unguarded) merge even though mergedSessionAuth stays undefined. + expect(result.mergedSessionAuth).toBeUndefined(); + expect(result.refreshOn).toEqual([401]); + } finally { + warnSpy.mockRestore(); + } + }); + + test('warns and names onChallenge when a refreshOn-only block also carries a defined onChallenge (intentional, #148)', () => { + // bin/apijack.ts and src/run-routine.ts both inject `onChallenge` from + // .apijack/auth.ts into the sessionAuth object before it reaches here, so a + // project with a deliberate refreshOn-only block AND a custom onChallenge + // export ends up with foundKeys === ['onChallenge']. That's NOT a leak of the + // bug this file just fixed: onChallenge is only ever consumed by + // SessionAuthStrategy, which is never constructed without a session.endpoint, + // so in this exact config the hook is genuinely dead code — the warning is + // pointing at a real mistake (an onChallenge that can never fire), not at the + // supported refreshOn-only pattern. + const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}); + const refreshOnlyWithChallenge = { + refreshOn: [401], + onChallenge: async () => {}, + } as unknown as SessionAuthConfig; + + try { + const result = resolveRefreshWiring({ sessionAuth: refreshOnlyWithChallenge }, undefined); + expect(warnSpy).toHaveBeenCalledTimes(1); + const message = warnSpy.mock.calls[0]![0] as string; + expect(message).toContain('session.endpoint'); + expect(message).toContain('onChallenge'); + expect(message).not.toContain('refreshOn'); + // refreshOn behavior from #135 is unchanged even in this configuration. + expect(result.mergedSessionAuth).toBeUndefined(); + expect(result.refreshOn).toEqual([401]); + } finally { + warnSpy.mockRestore(); + } + }); + + test('warns and names the found keys for a block with a typo\'d handshake key', () => { + const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}); + // `sessions:` instead of `session:` — the block has handshake-shaped keys + // but no reachable session.endpoint, which is the case the warning targets. + const typoSessionAuth = { + sessions: { endpoint: '/session' }, + cookies: { extract: ['SESSION'], applyTo: ['POST'] }, + refreshOn: [401], + } as unknown as SessionAuthConfig; + + try { + const result = resolveRefreshWiring({ sessionAuth: typoSessionAuth }, undefined); expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy.mock.calls[0]![0]).toContain('session.endpoint'); + const message = warnSpy.mock.calls[0]![0] as string; + expect(message).toContain('session.endpoint'); + expect(message).toContain('sessions'); + expect(message).toContain('cookies'); + expect(message).not.toContain('refreshOn'); + // refreshOn behavior from #135 is unchanged: still surfaces even though + // mergedSessionAuth stays undefined for the typo'd block. + expect(result.mergedSessionAuth).toBeUndefined(); + expect(result.refreshOn).toEqual([401]); } finally { warnSpy.mockRestore(); }