From b0c96ecee312fbd9d489bad0e5e9f26c57f4ac2f Mon Sep 17 00:00:00 2001 From: StephenWithPH Date: Fri, 17 Jul 2026 12:58:37 -0700 Subject: [PATCH 1/2] ERA-13733: route step-up 401s to interactive recovery over the websocket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a resp_authorization 401, read the RFC 9470 step-up challenge the server surfaces in status.www_authenticate (ERA-13732). When present, route to the shared unit's interactive step-up (recoverAuth({ stepUp: true, challenge })), which redirects rather than silent-renewing — a same-ACR silent token cannot satisfy the MFA challenge. Step-up bypasses the once-per-cycle authRetried guard so a renewed-but-MFA-stale token escalates instead of signing out. Ordinary 401s keep the ERA-13685 silent-renew path unchanged. Reuses the parsers, mode-keyed single-flight, and interactive stepUp primitive from ERA-13405, so a concurrent REST + socket step-up coalesces to one step-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/withSocketConnection/index.test.js | 68 ++++++++++++++++++- .../useRealTimeImplementation/index.js | 9 ++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/withSocketConnection/index.test.js b/src/withSocketConnection/index.test.js index 78cab51d8..24867d436 100644 --- a/src/withSocketConnection/index.test.js +++ b/src/withSocketConnection/index.test.js @@ -6,7 +6,11 @@ import { recoverAuth } from '../utils/auth-recovery'; import { clearAuth } from '../ducks/auth'; jest.mock('socket.io-client', () => ({ __esModule: true, default: jest.fn() })); -jest.mock('../utils/auth-recovery', () => ({ recoverAuth: jest.fn() })); +// Real parsers; only recoverAuth is stubbed. +jest.mock('../utils/auth-recovery', () => ({ + ...jest.requireActual('../utils/auth-recovery'), + recoverAuth: jest.fn(), +})); jest.mock('../ducks/auth', () => ({ clearAuth: jest.fn() })); // The socket handler dispatches through the imported store singleton; stub it so the // test controls dispatch and importing the hook doesn't build the real reducer tree. @@ -143,4 +147,66 @@ describe('websocket auth recovery (resp_authorization)', () => { expect(recoverAuth).toHaveBeenCalledTimes(2); expect(clearAuth).not.toHaveBeenCalled(); }); + + test('a 401 carrying the RFC 9470 step-up challenge routes to interactive step-up, not silent-renew', async () => { + // Step-up never settles (redirect); assert the routing decision, not a completion. + recoverAuth.mockReturnValue(new Promise(() => {})); + const challenge = 'Bearer error="insufficient_user_authentication", acr_values="http://schemas.openid.net/pape/policies/2007/06/multi-factor", max_age="31536000"'; + + const { socket, store } = bindSocket(); + await act(async () => { + socket.handlers.resp_authorization({ status: { code: 401, www_authenticate: challenge } }); + await Promise.resolve(); + }); + + expect(recoverAuth).toHaveBeenCalledWith({ + stepUp: true, + challenge: { + error: 'insufficient_user_authentication', + acrValues: 'http://schemas.openid.net/pape/policies/2007/06/multi-factor', + maxAge: '31536000', + }, + }); + expect(clearAuth).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalledWith(CLEAR_AUTH_ACTION); + }); + + test('a 401 with an ordinary (non-step-up) challenge still takes the silent-renew path', async () => { + recoverAuth.mockResolvedValue('fresh.token'); + + const { socket } = bindSocket(); + await act(async () => { + await socket.handlers.resp_authorization({ + status: { code: 401, www_authenticate: 'Bearer error="invalid_token"' }, + }); + }); + + expect(recoverAuth).toHaveBeenCalledWith(undefined); + expect(clearAuth).not.toHaveBeenCalled(); + }); + + test('escalates to step-up (not sign-out) when a silently-renewed socket then 401s with a step-up challenge', async () => { + const { socket, store } = bindSocket(); + + // Ordinary 401 → silent renew → re-authorize (sets the once-per-cycle guard). + recoverAuth.mockResolvedValueOnce('renewed.token'); + await act(async () => { + await socket.handlers.resp_authorization({ status: { code: 401 } }); + }); + + // The re-authorize 401s with a step-up challenge (guard already set); step-up bypasses it. + recoverAuth.mockReturnValue(new Promise(() => {})); + const challenge = 'Bearer error="insufficient_user_authentication", acr_values="urn:mfa", max_age="3600"'; + await act(async () => { + socket.handlers.resp_authorization({ status: { code: 401, www_authenticate: challenge } }); + await Promise.resolve(); + }); + + expect(recoverAuth).toHaveBeenLastCalledWith({ + stepUp: true, + challenge: { error: 'insufficient_user_authentication', acrValues: 'urn:mfa', maxAge: '3600' }, + }); + expect(clearAuth).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalledWith(CLEAR_AUTH_ACTION); + }); }); diff --git a/src/withSocketConnection/useRealTimeImplementation/index.js b/src/withSocketConnection/useRealTimeImplementation/index.js index 175649deb..e673064d7 100644 --- a/src/withSocketConnection/useRealTimeImplementation/index.js +++ b/src/withSocketConnection/useRealTimeImplementation/index.js @@ -4,7 +4,7 @@ import { DAS_HOST } from '../../constants'; import { resetSocketStateTracking } from './helpers'; import { SOCKET_HEALTHY_STATUS } from '../../ducks/system-status'; import { clearAuth } from '../../ducks/auth'; -import { recoverAuth } from '../../utils/auth-recovery'; +import { isStepUpChallenge, parseAuthChallenge, recoverAuth } from '../../utils/auth-recovery'; import { events } from './config'; import { calcEventFilterForRequest } from '../../utils/event-filter'; import { calcPatrolFilterForRequest } from '../../utils/patrol-filter'; @@ -73,10 +73,13 @@ const useRealTimeImplementation = () => { socket.on('resp_authorization', async (msg) => { const { status } = msg; if (status.code === 401) { - if (!authRetried) { + // Step-up bypasses the once-per-cycle guard: it redirects rather than looping. + const challenge = status.www_authenticate; + const stepUp = isStepUpChallenge(challenge); + if (stepUp || !authRetried) { authRetried = true; try { - await recoverAuth(); + await recoverAuth(stepUp ? { stepUp: true, challenge: parseAuthChallenge(challenge) } : undefined); console.log('realtime: token renewed; re-authorizing'); return authorize(); } catch (renewalError) { From 889d4970d3b84a3e1ba2e506101c92b95d7296c0 Mon Sep 17 00:00:00 2001 From: StephenWithPH Date: Fri, 17 Jul 2026 13:10:50 -0700 Subject: [PATCH 2/2] ERA-13733: bound the step-up redirect handoff so a failed redirect signs out instead of hanging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stepUp primitive resolves loginWithRedirect and then stays pending until the page unloads, so recoverAuth's inFlight.stepUp latch relied on unload to clear. If loginWithRedirect ever resolves without navigating, the latch stuck forever and every later step-up 401 hung with no sign-out fallback (raised in review on the ERA-13405 PR). Time-box the step-up flight with a generous redirect-handoff watchdog. A successful redirect unloads well before it fires, so it only trips when the redirect never navigated — converting an indefinite hang into a recoverable sign-out and releasing the latch so a later step-up retries. Applies to both the REST interceptor and the socket handler. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/utils/auth-recovery.js | 18 ++++++++++-------- src/utils/auth-recovery.test.js | 20 ++++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/utils/auth-recovery.js b/src/utils/auth-recovery.js index 5ce20bf71..2602e564d 100644 --- a/src/utils/auth-recovery.js +++ b/src/utils/auth-recovery.js @@ -27,15 +27,15 @@ export const parseAuthChallenge = (challenge) => { export const isStepUpChallenge = (challenge) => parseAuthChallenge(challenge)?.error === STEP_UP_ERROR; -// A stalled silent renewal (e.g. a network black-hole) must not hang recovery for both -// transports, so it is time-boxed. Interactive step-up is deliberately NOT bounded — it -// waits on the user completing MFA, which can take far longer than any network timeout. +// Time-boxed so a stalled recovery can't hang forever. Step-up's longer bound only trips +// if the redirect never navigated (a real redirect unloads the page first). const SILENT_RENEW_TIMEOUT_MS = 30_000; +const STEP_UP_REDIRECT_TIMEOUT_MS = 60_000; -const withTimeout = async (promise, ms) => { +const withTimeout = async (promise, ms, label = 'recovery') => { let timeoutId; const timeout = new Promise((_, reject) => { - timeoutId = setTimeout(() => reject(new Error('auth-recovery: renewal timed out')), ms); + timeoutId = setTimeout(() => reject(new Error(`auth-recovery: ${label} timed out`)), ms); }); try { return await Promise.race([promise, timeout]); @@ -65,9 +65,11 @@ export const recoverAuth = ({ stepUp = false, challenge = null } = {}) => { throw new Error(`auth-recovery: no ${stepUp ? 'stepUp' : 'silentRenew'} primitive registered`); } - const accessToken = stepUp - ? await recover(challenge) - : await withTimeout(recover(challenge), SILENT_RENEW_TIMEOUT_MS); + const accessToken = await withTimeout( + recover(challenge), + stepUp ? STEP_UP_REDIRECT_TIMEOUT_MS : SILENT_RENEW_TIMEOUT_MS, + stepUp ? 'step-up redirect' : 'silent renewal', + ); if (!accessToken) { throw new Error('auth-recovery: renewal returned no token'); } diff --git a/src/utils/auth-recovery.test.js b/src/utils/auth-recovery.test.js index e6af8ca16..8ae5e9e0f 100644 --- a/src/utils/auth-recovery.test.js +++ b/src/utils/auth-recovery.test.js @@ -98,7 +98,7 @@ describe('auth-recovery', () => { const silentRenew = jest.fn(() => new Promise(() => {})); registerAuthRecovery({ silentRenew }); - const rejection = expect(recoverAuth()).rejects.toThrow(/timed out/); + const rejection = expect(recoverAuth()).rejects.toThrow(/silent renewal timed out/); await jest.advanceTimersByTimeAsync(30_000); await rejection; @@ -112,18 +112,22 @@ describe('auth-recovery', () => { } }); - test('does not time out an interactive step-up (it waits on the user)', async () => { + test('times out a step-up whose redirect never navigates, clearing in-flight so a later step-up retries', async () => { jest.useFakeTimers(); try { - const stepUp = jest.fn(() => new Promise((resolve) => { - setTimeout(() => resolve('stepped.token'), 90_000); // past the 30s silent-renewal timeout - })); + // A redirect that resolves but never navigates: pending forever. + const stepUp = jest.fn(() => new Promise(() => {})); registerAuthRecovery({ stepUp }); - const pending = recoverAuth({ stepUp: true }); + const rejection = expect(recoverAuth({ stepUp: true, challenge: {} })).rejects.toThrow(/step-up redirect timed out/); await jest.advanceTimersByTimeAsync(60_000); - await jest.advanceTimersByTimeAsync(30_000); - await expect(pending).resolves.toBe('stepped.token'); + await rejection; + + expect(store.dispatch).not.toHaveBeenCalled(); + + stepUp.mockImplementationOnce(() => Promise.resolve('stepped.token')); + await expect(recoverAuth({ stepUp: true, challenge: {} })).resolves.toBe('stepped.token'); + expect(stepUp).toHaveBeenCalledTimes(2); } finally { jest.useRealTimers(); }