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'}}`);