mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-11 19:46:16 -08:00
fix(stalker): abandon a timed-out portal authentication before advancing
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
28429580a5
commit
2cf0ff8214
5 files changed
+174
-9
No files matched your search
@@ -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
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<StalkerAuthenticationResult> {
|
||||
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,
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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<T>(promise: Promise<T>, timeoutMs: number): Promise<T> {
|
||||
function withTimeout<T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
onTimeout?: () => void
|
||||
): Promise<T> {
|
||||
return new Promise<T>((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'}}`);
|
||||
|
||||
Reference in new issue
Block a user