From 2cf0ff8214357f02d99b5bbf7d60de91173b8c7c Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 2 Aug 2026 19:59:32 +0200 Subject: [PATCH] fix(stalker): abandon a timed-out portal authentication before advancing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1356. Discovery wrapped `authenticate()` in a timeout that rejected only the wrapper — the underlying flow kept running, so a late `get_profile` could land after a healthy candidate had already authenticated. `get_profile` is what adopts a token for the MAC portal-side, so the abandoned attempt invalidated the working candidate's token and discovery reported a live portal as refused. `authenticate()` now takes an `AbortSignal` and checks it before every portal call; discovery gives each confirmation its own controller and aborts it when the attempt exceeds its budget. The issue proposed threading the signal through IPC to axios. That does not address this harm: once a request is on the wire the server processes it whether or not the client tears the socket down, so only NOT SENDING the request prevents the adoption — which is what the between-calls check guarantees. Cancellation is therefore cooperative by design. Co-Authored-By: Claude Fable 5 --- docs/architecture/stalker-portal.md | 19 +++++++ .../src/lib/stalker-auth.api.spec.ts | 53 +++++++++++++++++++ .../data-access/src/lib/stalker-auth.api.ts | 32 +++++++++++ .../stalker-portal-discovery.service.spec.ts | 52 +++++++++++++++++- .../lib/stalker-portal-discovery.service.ts | 27 +++++++--- 5 files changed, 174 insertions(+), 9 deletions(-) diff --git a/docs/architecture/stalker-portal.md b/docs/architecture/stalker-portal.md index f21a6c360..caf93f13f 100644 --- a/docs/architecture/stalker-portal.md +++ b/docs/architecture/stalker-portal.md @@ -336,6 +336,25 @@ dialog shows them in its failure snackbar (with kind-specific i18n headlines, guidance) when category loading failed with a portal refusal (`stalkerCategoryErrorDescription` in `workspace-context-panel.component.ts`). +### Abandoning an authentication + +`authenticate()` takes an optional `AbortSignal` and checks it before every +portal call. Endpoint discovery gives each confirmation attempt its own +controller and aborts it when the attempt exceeds its budget, before moving to +the next candidate. + +This matters because `get_profile` — not the handshake — is what adopts a +token for the MAC portal-side. Without the check, a timed-out attempt could +still send its `get_profile` after a later candidate had authenticated, +invalidating that healthy candidate's token and making discovery report a +working portal as refused. + +Cancellation is deliberately cooperative rather than a socket-level abort +threaded to axios: once a request is on the wire the server processes it +regardless of what the client does, so tearing the socket down would not +prevent the adoption. Only not sending the request does, which is exactly what +the between-calls check guarantees. + ### Watchdog The portal expects `get_events` every `watchdog_timeout` seconds — **120 by diff --git a/libs/portal/stalker/data-access/src/lib/stalker-auth.api.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-auth.api.spec.ts index cc4c6654b..aebbea760 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-auth.api.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-auth.api.spec.ts @@ -284,6 +284,59 @@ describe('StalkerAuthApi', () => { ).rejects.toMatchObject({ kind: 'blocked' }); }); + it('never sends get_profile once the caller abandoned the attempt', async () => { + // The harm this prevents: `get_profile` is what adopts a token for + // the MAC portal-side, so an abandoned discovery attempt reaching it + // after another candidate authenticated would invalidate that + // healthy candidate's token. + const abandon = new AbortController(); + sendIpcEvent.mockImplementationOnce(async () => { + abandon.abort(); + return { js: { token: 'TOKEN-1', random: 'r1' } }; + }); + + await expect( + api.authenticate(portalUrl, macAddress, {}, { + signal: abandon.signal, + }) + ).rejects.toMatchObject({ name: 'StalkerAuthAbortedError' }); + + expect(callsByAction('handshake')).toHaveLength(1); + expect(callsByAction('get_profile')).toHaveLength(0); + }); + + it('does not even handshake when already abandoned', async () => { + const abandon = new AbortController(); + abandon.abort(); + + await expect( + api.authenticate(portalUrl, macAddress, {}, { + signal: abandon.signal, + }) + ).rejects.toMatchObject({ name: 'StalkerAuthAbortedError' }); + + expect(sendIpcEvent).not.toHaveBeenCalled(); + }); + + it('stops the status-2 login flow when abandoned mid-way', async () => { + const abandon = new AbortController(); + sendIpcEvent + .mockResolvedValueOnce({ js: { token: 'TOKEN-1', random: 'r1' } }) + .mockImplementationOnce(async () => { + abandon.abort(); + return { js: { status: 2 } }; + }); + + await expect( + api.authenticate(portalUrl, macAddress, {}, { + credentials: { username: 'user', password: 'secret' }, + signal: abandon.signal, + }) + ).rejects.toMatchObject({ name: 'StalkerAuthAbortedError' }); + + expect(callsByAction('do_auth')).toHaveLength(0); + }); + it('classifies a transport auth-failure marker during get_profile', async () => { sendIpcEvent .mockResolvedValueOnce({ diff --git a/libs/portal/stalker/data-access/src/lib/stalker-auth.api.ts b/libs/portal/stalker/data-access/src/lib/stalker-auth.api.ts index 51981bfec..e263bcb72 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-auth.api.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-auth.api.ts @@ -65,6 +65,18 @@ export interface StalkerAuthenticateOptions { * unchanged — an unchanged token is already an adopted session. */ skipProfileWhenReused?: boolean; + /** + * Abandons the flow between portal calls. + * + * This is what a timed-out discovery attempt needs: `get_profile` is the + * call that adopts a token for the MAC portal-side, so an abandoned + * attempt reaching it AFTER another candidate authenticated would + * invalidate that healthy candidate's token. Checking the signal before + * each call stops the request from being sent, which is the only thing + * that prevents it — a request already on the wire is processed by the + * server whether or not the client tears the socket down. + */ + signal?: AbortSignal; } export interface StalkerAuthenticationResult { @@ -135,6 +147,20 @@ function toFiniteNumber(value: unknown): number | undefined { return undefined; } +/** Raised when a caller abandoned the flow between portal calls. */ +export class StalkerAuthAbortedError extends Error { + constructor() { + super('Stalker authentication was aborted'); + this.name = 'StalkerAuthAbortedError'; + } +} + +function assertNotAborted(signal: AbortSignal | undefined): void { + if (signal?.aborted) { + throw new StalkerAuthAbortedError(); + } +} + /** * Throws when a response is one of the portal's plain-text auth-failure * bodies (already classified by the transport, or still raw on legacy paths). @@ -356,6 +382,7 @@ export class StalkerAuthApi { ): Promise { const normalizedIdentity = normalizeStalkerPortalIdentity(identity); + assertNotAborted(options.signal); const handshake = await this.performHandshake( portalUrl, macAddress, @@ -378,6 +405,9 @@ export class StalkerAuthApi { return { token: handshake.token, reusedStoredToken: true }; } + // The abandoned-attempt guard that matters: this is the call that + // adopts the token for the MAC portal-side. + assertNotAborted(options.signal); const profile = await this.getProfile( portalUrl, macAddress, @@ -436,6 +466,7 @@ export class StalkerAuthApi { throw new StalkerPortalError('login-required'); } + assertNotAborted(options.signal); const accepted = await this.doAuth( portalUrl, macAddress, @@ -447,6 +478,7 @@ export class StalkerAuthApi { throw new StalkerPortalError('login-rejected'); } + assertNotAborted(options.signal); const retried = await this.getProfile( portalUrl, macAddress, diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts index 7dfa44c85..29907b88d 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts @@ -104,7 +104,9 @@ describe('StalkerPortalDiscoveryService', () => { { serialNumber: 'SN1' }, // Import credentials ride along for portals that answer // get_profile with status 2 (login/password required). - { credentials: undefined } + // A per-attempt abort signal: a timed-out authentication must + // not send its get_profile behind the next candidate's back. + { credentials: undefined, signal: expect.any(AbortSignal) } ); }); @@ -194,7 +196,9 @@ describe('StalkerPortalDiscoveryService', () => { 'http://gated.example/portal.php', MAC, {}, - { credentials: undefined } + // A per-attempt abort signal: a timed-out authentication must + // not send its get_profile behind the next candidate's back. + { credentials: undefined, signal: expect.any(AbortSignal) } ); }); @@ -347,4 +351,48 @@ describe('StalkerPortalDiscoveryService', () => { isFullStalkerPortal: false, }); }); + + it('aborts a timed-out authentication before advancing to the next candidate', async () => { + jest.useFakeTimers(); + try { + mockProbes({ + 'http://slow.example/portal.php': { + resolve: 'Authorization failed.', + }, + }); + + let captured: AbortSignal | undefined; + authenticate.mockImplementation( + (_url, _mac, _identity, options) => { + captured = options?.signal; + // Hangs past the auth budget, like the real symptom. + return new Promise(() => undefined); + } + ); + + const discovery = service.discover( + 'http://slow.example/c', + MAC + ); + // Let the probe resolve so the auth attempt actually starts. + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + jest.advanceTimersByTime(45_000); + await Promise.resolve(); + + // The abandoned attempt is cancelled, so its get_profile never + // goes out to adopt the MAC's token behind a later candidate. + expect(captured?.aborted).toBe(true); + // Outcome shape is unchanged: a timed-out confirmation is still + // reported as a refusal of that endpoint. + await expect(discovery).resolves.toMatchObject({ + status: 'auth-rejected', + portalUrl: 'http://slow.example/portal.php', + }); + } finally { + jest.useRealTimers(); + } + }); }); diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts index 723587159..7baf5c2de 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts @@ -62,12 +62,20 @@ const PROBE_TIMEOUT_MS = 20_000; /** authenticate() is two sequential requests; give it a matching budget. */ const AUTH_TIMEOUT_MS = 45_000; -function withTimeout(promise: Promise, timeoutMs: number): Promise { +function withTimeout( + promise: Promise, + timeoutMs: number, + onTimeout?: () => void +): Promise { return new Promise((resolve, reject) => { - const timer = setTimeout( - () => reject(new Error('Stalker portal probe timed out')), - timeoutMs - ); + const timer = setTimeout(() => { + // Abandon the underlying operation BEFORE advancing: the timer + // only rejects this wrapper, and a late `get_profile` would adopt + // the MAC's token portal-side, invalidating whatever the next + // candidate just negotiated. + onTimeout?.(); + reject(new Error('Stalker portal probe timed out')); + }, timeoutMs); promise.then( (value) => { clearTimeout(timer); @@ -200,15 +208,20 @@ export class StalkerPortalDiscoveryService { ): Promise< StalkerPortalEndpointResolution | StalkerPortalDiscoveryRejection > { + // Cooperative cancellation: `authenticate()` checks this before each + // portal call, so a timed-out attempt never sends the `get_profile` + // that would adopt the MAC's token behind the next candidate's back. + const abandon = new AbortController(); try { const auth = await withTimeout( this.stalkerSession.authenticate( candidate, macAddress, identity, - { credentials } + { credentials, signal: abandon.signal } ), - AUTH_TIMEOUT_MS + AUTH_TIMEOUT_MS, + () => abandon.abort() ); // A handshake can hand out a token whose `get_profile` still // answers a structured denial (`{js:{error:'Invalid token'}}`);