From bfbd252ea2f59672a78f704e21289c06a4b319d9 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 15:36:37 +0200 Subject: [PATCH] fix(stalker): retain abandoned auth fences --- .changes/stalker-smart-discovery.md | 10 +-- CLAUDE.md | 2 +- ...playlist-connection-editor.service.spec.ts | 31 +++++++ ...lker-playlist-connection-editor.service.ts | 32 ++++++- docs/architecture/stalker-portal.md | 14 ++- .../stalker-portal-discovery.service.spec.ts | 87 ++++++++++++++++--- .../lib/stalker-portal-discovery.service.ts | 30 +++++-- 7 files changed, 176 insertions(+), 30 deletions(-) diff --git a/.changes/stalker-smart-discovery.md b/.changes/stalker-smart-discovery.md index 8d54f604b..356e131eb 100644 --- a/.changes/stalker-smart-discovery.md +++ b/.changes/stalker-smart-discovery.md @@ -3,8 +3,8 @@ type: feature area: stalker --- -Stalker setup now accepts a host or `/c` address, discovers the working API -endpoint and authentication mode, and rechecks connection details edited -later. Canceled or failed edits leave the saved playlist and active session -unchanged. Completed edits reject requests still holding the previous portal -configuration and discard their late responses. +Stalker setup now accepts hosts or `/c`, discovers the working API endpoint and +authentication mode, and rechecks edited connection details. Canceled or failed +edits leave saved and active sessions unchanged. Completed edits reject old +configuration requests and late responses. Timed-out authentication stays +fenced until its transport settles. diff --git a/CLAUDE.md b/CLAUDE.md index a84e4d5a5..046b116ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1263,7 +1263,7 @@ engine` (restart required) or - Portal mode (full vs. simple) follows OBSERVED behavior, never a URL substring. The single predicate is `isFullStalkerPortalPlaylist()` / `isFullStalkerPortalUrl()` in `@iptvnator/shared/interfaces` (`stalker-portal-mode.util.ts`): the persisted `Playlist.isFullStalkerPortal` flag is authoritative and the URL shape is a fallback for legacy rows only. Three diverging copies of this rule used to exist and shipped broken configurations (#850/#686/#755) — never re-implement it. A token-enforcing `portal.php` panel is a full portal; a `server/load.php` endpoint that answers without a token is a simple one. - Import requires an explicit HTTP(S) scheme but accepts a bare host, `/c`, or a concrete `.php` address. It probes candidates in order (a pasted `.php` endpoint first, then `/portal.php` → `/server/load.php` → `/stalker_portal/server/load.php`) and classifies each by behavior — a token-less `itv/get_genres` returning data proves a token-free panel; the plain-text auth failure proves a full portal, confirmed by a real handshake + `get_profile`. `StalkerPortalDiscoveryService` (`libs/portal/stalker/data-access`) persists and displays the proven endpoint and mode. An unreachable panel-style import remains allowed with a warning; a bare host falls back to `/portal.php`, while canonical-shaped unreachable addresses still abort. -- The playlist-info Edit dialog loads the complete persisted Stalker row before enabling the form, because Electron's startup metadata projection omits payload-only serial/device/signature/mode fields; a summarized row must never render and then persist an empty portal identity. It preserves an unchanged connection byte-for-byte and skips discovery. Changing URL, MAC, credentials, serial, device IDs or signatures blocks duplicate saves, disables dialog closure for the validation window, and runs the existing discovery service through the app-provided `STALKER_PLAYLIST_CONNECTION_EDITOR` token, keeping Stalker data-access out of `playlist-shared-ui`. Before discovery it reserves the playlist ID; a second Edit cannot replace that owner. The reservation blocks every new authentication (including fingerprint-equivalent URL edits) and repair, drains existing work, and rechecks ownership after every asynchronous drain/rebase; failure releases it without changing the saved or runtime connection. If navigation or another owner closes/destroys the dialog while discovery is in flight, the UI discards a later successful result through `discardResolvedConnection()` and releases both reservations without persisting it. Success uses one awaited write to atomically replace endpoint, mode, normalized identity and session metadata, then feeds its complete merged row into the state-only NgRx update and active `StalkerStore`/session/watchdog replacement before another same-route request can use the old connection. This preserves playback headers and other metadata absent from the form. Runtime configuration authority covers the observed full/simple mode as well as the session fingerprint, and both authenticated and direct simple requests cross its guard before dispatch and after transport, so a same-endpoint mode change rejects stale snapshots and completed responses in either direction. A changed authority may rebase only when the persisted row proves that it owns the same playlist ID, keeping delete/restore and backup merge usable. The transient `PlaylistMetaUpdate.stalkerSessionPatch` preserves on absence, clears on `null`, and fully replaces from an object before storage; it is projected onto existing flat playlist fields and never changes the DB or backup shape. +- The playlist-info Edit dialog loads the complete persisted Stalker row before enabling the form, because Electron's startup metadata projection omits payload-only serial/device/signature/mode fields; a summarized row must never render and then persist an empty portal identity. It preserves an unchanged connection byte-for-byte and skips discovery. Changing URL, MAC, credentials, serial, device IDs or signatures blocks duplicate saves, disables dialog closure for the validation window, and runs the existing discovery service through the app-provided `STALKER_PLAYLIST_CONNECTION_EDITOR` token, keeping Stalker data-access out of `playlist-shared-ui`. Before discovery it reserves the playlist ID; a second Edit cannot replace that owner. The reservation blocks every new authentication (including fingerprint-equivalent URL edits) and repair, drains existing work, and rechecks ownership after every asynchronous drain/rebase; ordinary failure releases it without changing the saved or runtime connection. If discovery returns after its bounded drain while an abandoned authentication is still on the wire, that result carries its settlement promise and both reservations remain installed until it resolves, so catalog, watchdog, repair, or retry authentication cannot race a late `get_profile`. If navigation or another owner closes/destroys the dialog while discovery is in flight, the UI discards a later successful result through `discardResolvedConnection()` and releases both reservations without persisting it. Success uses one awaited write to atomically replace endpoint, mode, normalized identity and session metadata, then feeds its complete merged row into the state-only NgRx update and active `StalkerStore`/session/watchdog replacement before another same-route request can use the old connection. This preserves playback headers and other metadata absent from the form. Runtime configuration authority covers the observed full/simple mode as well as the session fingerprint, and both authenticated and direct simple requests cross its guard before dispatch and after transport, so a same-endpoint mode change rejects stale snapshots and completed responses in either direction. A changed authority may rebase only when the persisted row proves that it owns the same playlist ID, keeping delete/restore and backup merge usable. The transient `PlaylistMetaUpdate.stalkerSessionPatch` preserves on absence, clears on `null`, and fully replaces from an object before storage; it is projected onto existing flat playlist fields and never changes the DB or backup shape. - `executeStalkerRequest()` (`stores/utils/stalker-request.utils.ts`) is the choke point for catalog, content and playback requests: mode routing, the in-session repair override, and retry-once all live there. Four callers are deliberately outside it because they run below or before the thing it routes on — `StalkerAuthApi` (handshake/`get_profile`/`do_auth`, which the full-portal branch is built from; routing them back would recurse), `StalkerPortalDiscoveryService` (probes precede the mode they determine), `StalkerAccountInfoService.fetchViaProfile()`, and `StreamResolverService` for a collection item with no playlist row. They are exempt from the routing, not from the repair it hooks, but only `fetchViaProfile()` wires `StalkerPortalRepairService` itself: discovery is what repair _drives_, the row-less resolver branch has no playlist to repair, and the auth layer needs nothing — a terminal handshake failure propagates out of the full-portal branch into whichever `executeStalkerRequest()` call triggered the authentication, which is why terminal handshake failures are a repair trigger. Anything new that is not auth or discovery belongs on `executeStalkerRequest()`. Existing playlists are repaired LAZILY (`StalkerPortalRepairService`) — only after a request fails with a shape a wrong endpoint/mode produces, at most once per source configuration per playlist per session, persisted through the atomic `PlaylistsService.transformPlaylistMeta`. Before an unrecorded repair calls discovery, it verifies that the persisted row still owns the failing source, so a late pre-Edit request cannot authenticate against the old portal after Edit commits and invalidate the newly saved token. There is deliberately **no eager one-shot migration**: a portal that works is never re-probed. - Explicit Edit advances the repair generation before installing its resolved session. A lazy repair that started earlier is discarded even if it had already verified its row, so it cannot restore an older endpoint, mode or token after Edit. - Both transports build the wire format from the same shared builders in `@iptvnator/shared/interfaces` — `buildStalkerRequestUrl()`, `buildStalkerIdentityRequestContext()`, `encodeStalkerCmdValue()` — so the Electron and PWA legs cannot drift. The mock's `/stalker` mirror shares the identity builder only — it dispatches in-process, so there is no portal URL to build and it mirrors the `JsHttpRequest` default by hand. Never fork any of them. diff --git a/apps/web/src/app/services/stalker-playlist-connection-editor.service.spec.ts b/apps/web/src/app/services/stalker-playlist-connection-editor.service.spec.ts index 0e70ef381..fa4d89a61 100644 --- a/apps/web/src/app/services/stalker-playlist-connection-editor.service.spec.ts +++ b/apps/web/src/app/services/stalker-playlist-connection-editor.service.spec.ts @@ -209,6 +209,37 @@ describe('AppStalkerPlaylistConnectionEditorService', () => { }); }); + it('keeps an abandoned authentication fenced until it settles', async () => { + let settleAuthentication: () => void = () => undefined; + const abandonedAuthenticationSettled = new Promise((resolve) => { + settleAuthentication = resolve; + }); + discovery.discover.mockResolvedValue({ + status: 'auth-rejected', + portalUrl: 'https://portal.example.com/server/load.php', + abandonedInFlight: true, + abandonedAuthenticationSettled, + }); + + await expect(service.resolveConnection(draft)).resolves.toMatchObject({ + status: STALKER_PLAYLIST_CONNECTION_EDITOR_STATUS.AUTH_REJECTED, + }); + + expect(stalkerSession.cancelEditDiscovery).not.toHaveBeenCalled(); + expect(portalRepair.releasePlaylistEdit).not.toHaveBeenCalled(); + + settleAuthentication(); + await abandonedAuthenticationSettled; + await Promise.resolve(); + + expect(stalkerSession.cancelEditDiscovery).toHaveBeenCalledWith( + expect.objectContaining({ playlistId: draft._id }) + ); + expect(portalRepair.releasePlaylistEdit).toHaveBeenCalledWith( + draft._id + ); + }); + it('returns a dedicated error when no endpoint can be reached', async () => { discovery.discover.mockResolvedValue({ status: 'unreachable' }); diff --git a/apps/web/src/app/services/stalker-playlist-connection-editor.service.ts b/apps/web/src/app/services/stalker-playlist-connection-editor.service.ts index 24887d76f..45d4d6ad2 100644 --- a/apps/web/src/app/services/stalker-playlist-connection-editor.service.ts +++ b/apps/web/src/app/services/stalker-playlist-connection-editor.service.ts @@ -136,7 +136,15 @@ export class AppStalkerPlaylistConnectionEditorService implements StalkerPlaylis } if (outcome.status === 'auth-rejected') { - this.releaseEditFence(playlist._id, fence); + if (outcome.abandonedInFlight) { + this.releaseEditFenceAfterAuthenticationSettles( + playlist._id, + fence, + outcome.abandonedAuthenticationSettled + ); + } else { + this.releaseEditFence(playlist._id, fence); + } return { status: STALKER_PLAYLIST_CONNECTION_EDITOR_STATUS.AUTH_REJECTED, message: this.buildAuthErrorMessage(outcome.error), @@ -230,6 +238,28 @@ export class AppStalkerPlaylistConnectionEditorService implements StalkerPlaylis this.portalRepair.releasePlaylistEdit(playlistId); } + private releaseEditFenceAfterAuthenticationSettles( + playlistId: string, + fence: StalkerEditFence, + authenticationSettled: Promise | undefined + ): void { + // Fail closed if a producer ever reports an abandoned request without + // its lifetime. Releasing here would let a fresh session authenticate + // while the old get_profile can still land and invalidate its token. + if (!authenticationSettled) { + return; + } + + void authenticationSettled.then(() => { + // The dialog may have been closed or a later owner may already + // have retired this exact reservation. Never release a different + // edit's counted repair fence from this late continuation. + if (this.editFences.get(playlistId) === fence) { + this.releaseEditFence(playlistId, fence); + } + }); + } + private buildAuthErrorMessage(error: unknown): string { const portalError = asStalkerPortalError(error); const headline = this.translate.instant( diff --git a/docs/architecture/stalker-portal.md b/docs/architecture/stalker-portal.md index 40c3cb6e3..d4092b71b 100644 --- a/docs/architecture/stalker-portal.md +++ b/docs/architecture/stalker-portal.md @@ -211,8 +211,10 @@ discovery starts, Edit reserves the playlist ID; an overlapping Edit cannot replace that owner. The reservation blocks every new authentication (including a URL edit with the same normalized fingerprint) and repair, and drains any work already in flight. Ownership is rechecked after -every asynchronous drain or authority rebase; failure releases that reservation -with the previous runtime untouched. +every asynchronous drain or authority rebase. Ordinary failure releases that +reservation with the previous runtime untouched; if a bounded discovery result +still has an abandoned authentication on the wire, Edit keeps both reservations +until the transport operation actually settles. Success atomically replaces the endpoint, mode and normalized identity together with session metadata: simple mode clears token/fingerprint/watchdog/account state, while full mode replaces it @@ -753,11 +755,15 @@ plus the 15 s drain is one no transport can recall — the PWA `fetch()` takes n signal at all, and the Electron main process runs its HTTP request to completion. Advancing anyway would stake the next candidate's freshly issued session on that request never landing. So the rejection carries -`abandonedInFlight` and the candidate loop returns instead of probing on, +`abandonedInFlight` plus a settlement promise and the candidate loop returns +instead of probing on, preferring an honest "could not confirm this portal" the user can retry over a session that looks established and dies later. It costs nothing in the normal case: an aborted attempt settles as soon as its in-flight request errors out, -so the drain returns at once and the loop continues. +so the drain returns at once and the loop continues. Edit may return that +bounded error to the dialog, but its session and repair fences remain installed +until the settlement promise resolves; catalog, watchdog, repair and retry +authentication therefore cannot race the abandoned `get_profile`. The budget itself covers the longest real flow: a status-2 portal costs four sequential requests (handshake, profile, `do_auth`, profile retry) and the 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 a03d54a1d..32964a386 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 @@ -27,9 +27,7 @@ describe('StalkerPortalDiscoveryService', () => { const { url } = payload as { url: string }; const handler = handlers[url]; if (!handler) { - return Promise.reject( - new Error(`unexpected probe for ${url}`) - ); + return Promise.reject(new Error(`unexpected probe for ${url}`)); } if ('reject' in handler) { return Promise.reject(handler.reject); @@ -180,7 +178,10 @@ describe('StalkerPortalDiscoveryService', () => { // for portals that previously authenticated directly. mockProbes({ 'http://gated.example/portal.php': { - reject: { message: 'HTTP Error 401: Unauthorized', status: 401 }, + reject: { + message: 'HTTP Error 401: Unauthorized', + status: 401, + }, }, }); authenticate.mockResolvedValue({ token: 'TOKEN4' }); @@ -214,7 +215,9 @@ describe('StalkerPortalDiscoveryService', () => { reject: { message: 'HTTP Error 404: Not Found', status: 404 }, }, }); - authenticate.mockRejectedValue(new Error('Handshake failed: No token received')); + authenticate.mockRejectedValue( + new Error('Handshake failed: No token received') + ); const outcome = await service.discover('http://gated.example/c', MAC); @@ -370,10 +373,7 @@ describe('StalkerPortalDiscoveryService', () => { } ); - const discovery = service.discover( - 'http://slow.example/c', - MAC - ); + 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(); @@ -454,6 +454,47 @@ describe('StalkerPortalDiscoveryService', () => { } }); + it('preserves a later abandoned attempt over an earlier ordinary rejection', async () => { + jest.useFakeTimers(); + try { + mockProbes({ + 'http://mixed.example/portal.php': { + resolve: 'Authorization failed.', + }, + 'http://mixed.example/server/load.php': { + resolve: 'Authorization failed.', + }, + }); + authenticate + .mockRejectedValueOnce(new Error('first refused')) + .mockImplementationOnce(() => new Promise(() => undefined)); + + const discovery = service.discover('http://mixed.example/c', MAC); + for (let i = 0; i < 20; i += 1) { + await Promise.resolve(); + } + expect(authenticate).toHaveBeenCalledTimes(2); + + jest.advanceTimersByTime(65_001); + for (let i = 0; i < 20; i += 1) { + await Promise.resolve(); + } + jest.advanceTimersByTime(15_001); + for (let i = 0; i < 20; i += 1) { + await Promise.resolve(); + } + + await expect(discovery).resolves.toMatchObject({ + status: 'auth-rejected', + portalUrl: 'http://mixed.example/server/load.php', + abandonedInFlight: true, + abandonedAuthenticationSettled: expect.any(Promise), + }); + } finally { + jest.useRealTimers(); + } + }); + it('stops discovery when the drain deadline expires with the attempt still live', async () => { jest.useFakeTimers(); try { @@ -471,9 +512,15 @@ describe('StalkerPortalDiscoveryService', () => { }, }); - // Never settles — not even after the drain. + let settleAuthentication: () => void = () => undefined; authenticate - .mockImplementationOnce(() => new Promise(() => undefined)) + .mockImplementationOnce( + () => + new Promise((resolve) => { + settleAuthentication = () => + resolve({ token: 'ABANDONED' }); + }) + ) .mockResolvedValue({ token: 'SECOND' }); const discovery = service.discover('http://hung.example/c', MAC); @@ -490,12 +537,28 @@ describe('StalkerPortalDiscoveryService', () => { await Promise.resolve(); } - await expect(discovery).resolves.toMatchObject({ + const outcome = await discovery; + expect(outcome).toMatchObject({ status: 'auth-rejected', abandonedInFlight: true, }); // The second candidate was never authenticated. expect(authenticate).toHaveBeenCalledTimes(1); + + if (outcome.status !== 'auth-rejected') { + throw new Error('Expected an authentication rejection'); + } + expect(outcome.abandonedAuthenticationSettled).toBeDefined(); + let abandonedSettled = false; + void outcome.abandonedAuthenticationSettled?.then(() => { + abandonedSettled = true; + }); + await Promise.resolve(); + expect(abandonedSettled).toBe(false); + + settleAuthentication(); + await outcome.abandonedAuthenticationSettled; + expect(abandonedSettled).toBe(true); } 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 009c1c8e5..d3903e685 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 @@ -55,6 +55,13 @@ export interface StalkerPortalDiscoveryRejection { * invalidate the session a later candidate had just established. */ abandonedInFlight?: boolean; + /** + * Resolves once the abandoned authentication has actually left the + * transport. Callers that reserve the playlist during discovery must + * keep that reservation until this resolves: returning the user-facing + * rejection is bounded, but the request on the wire is not. + */ + abandonedAuthenticationSettled?: Promise; } /** No candidate answered like a Stalker portal (host down or not a portal). */ @@ -167,7 +174,10 @@ export class StalkerPortalDiscoveryService { return outcome; } if (outcome.abandonedInFlight) { - return authRejection ?? outcome; + // This attempt's transport lifetime must reach the + // caller. Returning an earlier ordinary refusal would + // hide the live request and let Edit release its fence. + return outcome; } authRejection = authRejection ?? outcome; continue; @@ -217,7 +227,7 @@ export class StalkerPortalDiscoveryService { // An attempt still on the wire outranks further probing: // see `abandonedInFlight`. if (outcome.abandonedInFlight) { - return authRejection ?? outcome; + return outcome; } // The endpoint is real but refused our credentials; // remember the first such endpoint in case no later @@ -294,11 +304,12 @@ export class StalkerPortalDiscoveryService { // would stake a working candidate's session on a request nobody // can recall. const DRAINED = Symbol('drained'); + const abandonedAuthenticationSettled = pending.then( + () => undefined, + () => undefined + ); const outcome = await Promise.race([ - pending.then( - () => DRAINED, - () => DRAINED - ), + abandonedAuthenticationSettled.then(() => DRAINED), new Promise((resolve) => setTimeout(resolve, ABANDONED_DRAIN_MS) ), @@ -312,7 +323,12 @@ export class StalkerPortalDiscoveryService { status: 'auth-rejected', portalUrl: candidate, error, - ...(outcome === DRAINED ? {} : { abandonedInFlight: true }), + ...(outcome === DRAINED + ? {} + : { + abandonedInFlight: true, + abandonedAuthenticationSettled, + }), }; } }