From cf715a75735e42961233749ddbd1f00fbae18b03 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 19:54:40 +0200 Subject: [PATCH] fix(server-storage): one registry key per page load, server token only in server mode - The unconfirmed-writes registry stores each page load's record under its own key (iptvnator-server:unconfirmed::), listed through a new keys() on the user-data storage adapter, so tabs never overwrite each other's records. Saved writes without a fresh record, also when the record was lost, are adopted as a closed tab's (#1920). - PwaService sends the stored server token on its raw fetch calls only while server mode runs, and a backend that answers without server storage makes the browser forget the token. Co-Authored-By: Claude Opus 5.5 --- .../server-storage-start.spec.ts | 10 ++ .../server-storage/server-storage-start.ts | 3 + apps/web/src/app/services/pwa.service.spec.ts | 61 ++++++- apps/web/src/app/services/pwa.service.ts | 21 ++- docs/architecture/pwa-self-hosted.md | 16 +- .../server-unsent-writes.spec.ts | 1 + .../server-storage/server-unsent-writes.ts | 151 +++++++++++------- .../server-user-data-storage.spec.ts | 11 +- .../server-user-data-sync.spec.ts | 53 +++++- libs/services/src/lib/user-data-storage.ts | 18 ++- 10 files changed, 263 insertions(+), 82 deletions(-) diff --git a/apps/web/src/app/server-storage/server-storage-start.spec.ts b/apps/web/src/app/server-storage/server-storage-start.spec.ts index 025a47a69..e3a12e07f 100644 --- a/apps/web/src/app/server-storage/server-storage-start.spec.ts +++ b/apps/web/src/app/server-storage/server-storage-start.spec.ts @@ -45,6 +45,16 @@ describe('resolveServerStorageStart', () => { ).toBeNull(); }); + it('forgets the token of a server that no longer offers storage', () => { + localStorage.setItem('iptvnator-server:access-token', 'old'); + + resolveServerStorageStart({ kind: 'none' }, '/api'); + + expect( + localStorage.getItem('iptvnator-server:access-token') + ).toBeNull(); + }); + it('uses browser mode for an unknown or different server', () => { expect( resolveServerStorageStart({ kind: 'unreachable' }, '/api') diff --git a/apps/web/src/app/server-storage/server-storage-start.ts b/apps/web/src/app/server-storage/server-storage-start.ts index 5064c2df1..b892c8015 100644 --- a/apps/web/src/app/server-storage/server-storage-start.ts +++ b/apps/web/src/app/server-storage/server-storage-start.ts @@ -1,3 +1,4 @@ +import { clearServerAccessToken } from '@iptvnator/services'; import type { ServerInstanceInfo } from '@iptvnator/shared/interfaces'; import type { ServerInstanceProbe } from './server-instance-probe'; @@ -36,6 +37,8 @@ export function resolveServerStorageStart( return { instance: probe.instance, startedOffline: false }; case 'none': remove(storage); + // The token belonged to that server; another must not see it. + clearServerAccessToken(); return null; case 'unreachable': { const remembered = read(storage); diff --git a/apps/web/src/app/services/pwa.service.spec.ts b/apps/web/src/app/services/pwa.service.spec.ts index 0238e94ae..b381bde46 100644 --- a/apps/web/src/app/services/pwa.service.spec.ts +++ b/apps/web/src/app/services/pwa.service.spec.ts @@ -20,7 +20,10 @@ import { getStalkerRequestErrorStatus, isStalkerProbeTimeout, } from '@iptvnator/portal/stalker/data-access'; -import { ServerStorageStatusService } from '@iptvnator/services'; +import { + SERVER_ACCESS_TOKEN_STORAGE_KEY, + ServerStorageStatusService, +} from '@iptvnator/services'; import { PwaService } from './pwa.service'; describe('PwaService', () => { @@ -280,6 +283,62 @@ describe('PwaService', () => { globalThis.fetch = originalFetch; }); + it('sends a stored server token only while server mode runs', async () => { + localStorage.setItem(SERVER_ACCESS_TOKEN_STORAGE_KEY, 'kept-token'); + const originalFetch = globalThis.fetch; + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + headers: new Headers(), + json: async () => ({ js: [] }), + } as unknown as Response); + globalThis.fetch = fetchMock as unknown as typeof fetch; + const call = async () => { + const outcome = ( + service.sendIpcEvent(STALKER_REQUEST, { + url: 'http://portal.example/portal.php', + macAddress: '00:1A:79:AA:BB:CC', + params: { action: 'get_genres' }, + silent: true, + }) as Promise + ).catch((error: unknown) => error); + await Promise.resolve(); + for (const request of http.match((candidate) => + candidate.url.endsWith('/provider-targets') + )) { + request.flush({ targetId: 'target-1' }); + } + await outcome; + }; + + try { + // A token kept from another backend: server mode is not active. + await call(); + expect(fetchMock.mock.calls[0][1].headers).not.toHaveProperty( + 'Authorization' + ); + + TestBed.inject(ServerStorageStatusService).activate( + { + service: 'iptvnator', + apiVersion: 1, + instanceId: 'i', + storage: 'server', + auth: 'token', + }, + () => undefined + ); + await call(); + expect(fetchMock.mock.calls[1][1].headers).toMatchObject({ + Authorization: 'Bearer kept-token', + }); + } finally { + globalThis.fetch = originalFetch; + localStorage.removeItem(SERVER_ACCESS_TOKEN_STORAGE_KEY); + } + }); + it('asks for a new server token when the access guard refuses a Stalker call', async () => { const status = TestBed.inject(ServerStorageStatusService); status.activate( diff --git a/apps/web/src/app/services/pwa.service.ts b/apps/web/src/app/services/pwa.service.ts index 2fca37d6d..62ec56287 100644 --- a/apps/web/src/app/services/pwa.service.ts +++ b/apps/web/src/app/services/pwa.service.ts @@ -17,7 +17,6 @@ import { DataService, isServerAccessChallenge, readServerAccessToken, - serverAuthorizationHeaders, ServerStorageStatusService, } from '@iptvnator/services'; import { @@ -202,14 +201,14 @@ export class PwaService extends DataService { ); try { - const token = readServerAccessToken(); + const token = this.serverAccessToken(); const response = await fetch( `${this.corsProxyUrl}/connectivity-guard/reset`, { body: JSON.stringify({ url: payload.url }), headers: { 'content-type': 'application/json', - ...serverAuthorizationHeaders(), + ...bearer(token), }, method: 'POST', signal: controller.signal, @@ -649,9 +648,9 @@ export class PwaService extends DataService { logPortalDebugRequest(context); // Make the fetch request - const accessToken = readServerAccessToken(); + const accessToken = this.serverAccessToken(); const response = await fetch(requestUrl, { - headers: serverAuthorizationHeaders(), + headers: bearer(accessToken), }); this.noteServerAccess(response, accessToken); @@ -806,6 +805,14 @@ export class PwaService extends DataService { return 'pwa'; } + /** + * The stored server token, only while server mode runs for this + * backend: a token kept from another backend must not reach this one. + */ + private serverAccessToken(): string | null { + return this.serverStatus.isActive() ? readServerAccessToken() : null; + } + /** A token changed on the server shows up on any backend call. */ private noteServerAccess(response: Response, token: string | null): void { if ( @@ -819,3 +826,7 @@ export class PwaService extends DataService { } } } + +function bearer(token: string | null): Record { + return token ? { Authorization: `Bearer ${token}` } : {}; +} diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index d631958fd..1f77be69d 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -268,11 +268,13 @@ settings and guards read them in every build. arrives (`applyEdit`): a new device or a stale cache adds its favorite instead of replacing the server's list. On this tab's recovered unsent write they are kept as their own steps after it. They are saved per page load too - (`iptvnator-server:deferred-edits::`). A shared - registry in `localStorage` (`iptvnator-server:unconfirmed:`, - one record per page load, renewed every 30 s while it has unsent writes, - stale after ten minutes) lets a re-read in one tab keep the cache values - another tab is still sending. A value too big for `localStorage`, or any + (`iptvnator-server:deferred-edits::`). Records in + `localStorage` (`iptvnator-server:unconfirmed::`, one + key per page load so tabs never overwrite each other's, renewed every + 30 s while it has unsent writes, stale after ten minutes) let a re-read + in one tab keep the cache values another tab is still sending; saved + writes without a fresh record, also when the record was lost, count as a + closed tab's (the storage adapter lists its keys for this). A value too big for `localStorage`, or any value when the browser refuses storage access, stays readable from memory, so callers and the diff baseline agree. A re-read updates the cache; views already open show another device's changes on @@ -295,7 +297,9 @@ settings and guards read them in every build. before it stores it under `iptvnator-server:access-token` and reloads (a browser that refuses to store it is told so instead). `ServerAccessTokenInterceptor` adds the header to `HttpClient` calls to the - backend, and `PwaService` adds it to its two raw `fetch` calls. Both turn + backend, and `PwaService` adds it to its two raw `fetch` calls only while + server mode runs (a backend that answers without server storage also + makes the browser forget the token). Both turn the guard's 401 (`isServerAccessChallenge`) into `reportAccessDenied()`, so a token changed on the server sends an open page back to sign-in; a late 401 for a token replaced since (a new sign-in) changes nothing. diff --git a/libs/services/src/lib/server-storage/server-unsent-writes.spec.ts b/libs/services/src/lib/server-storage/server-unsent-writes.spec.ts index e2a36206a..ebb997939 100644 --- a/libs/services/src/lib/server-storage/server-unsent-writes.spec.ts +++ b/libs/services/src/lib/server-storage/server-unsent-writes.spec.ts @@ -10,6 +10,7 @@ function memoryStorage() { getItem: (key: string) => values.get(key) ?? null, setItem: (key: string, value: string) => void values.set(key, value), removeItem: (key: string) => void values.delete(key), + keys: () => [...values.keys()], }; } diff --git a/libs/services/src/lib/server-storage/server-unsent-writes.ts b/libs/services/src/lib/server-storage/server-unsent-writes.ts index efa86f423..edf9a6351 100644 --- a/libs/services/src/lib/server-storage/server-unsent-writes.ts +++ b/libs/services/src/lib/server-storage/server-unsent-writes.ts @@ -22,14 +22,15 @@ const writesKey = (instanceId: string, page: string) => `${UNSENT_WRITES_KEY}:${instanceId}:${page}`; /** * Which entries each page load has not had confirmed yet, so a re-read in - * one tab keeps the cache values another tab is still sending. A live tab - * renews its record; one older than `UNCONFIRMED_TTL_MS` belongs to a tab - * that is gone. The records also list whose saved writes exist. + * one tab keeps the cache values another tab is still sending. Each page + * load writes only its own key, so tabs never overwrite each other's + * record. A live tab renews it; one older than `UNCONFIRMED_TTL_MS` + * belongs to a tab that is gone. */ export const UNCONFIRMED_KEY = 'iptvnator-server:unconfirmed'; export const UNCONFIRMED_TTL_MS = 10 * 60 * 1000; -const unconfirmedKey = (instanceId: string) => - `${UNCONFIRMED_KEY}:${instanceId}`; +const recordKey = (instanceId: string, page: string) => + `${UNCONFIRMED_KEY}:${instanceId}:${page}`; /** Edits made before the first read of a visit, kept as before → after. */ export const DEFERRED_EDITS_KEY = 'iptvnator-server:deferred-edits'; @@ -51,11 +52,11 @@ export function claimCache(local: UserDataStorage, instanceId: string): void { local.removeItem(`iptvnator-server:${namespace}`); } if (owner) { - for (const page of Object.keys(readRegistry(local, owner))) { - local.removeItem(writesKey(owner, page)); - local.removeItem(deferredKey(owner, page)); + for (const prefix of PAGE_PREFIXES) { + for (const page of pagesUnder(local, `${prefix}:${owner}:`)) { + local.removeItem(`${prefix}:${owner}:${page}`); + } } - local.removeItem(unconfirmedKey(owner)); } local.setItem(CACHE_OWNER_KEY, instanceId); } catch { @@ -118,20 +119,30 @@ export function swapPage( } } -/** Page loads whose record went stale: closed tabs nobody sends for. */ +/** + * Page loads with saved writes or a record but no fresh record: closed + * tabs nobody sends for (also when their record was lost). + */ export function orphanedPages( local: UserDataStorage, instanceId: string, page: string, now = Date.now() ): string[] { - return Object.entries(readRegistry(local, instanceId)) - .filter( - ([owner, record]) => - owner !== page && - !(now - Number(record?.at) < UNCONFIRMED_TTL_MS) - ) - .map(([owner]) => owner); + const registry = readRegistry(local, instanceId); + const pages = new Set(); + for (const prefix of PAGE_PREFIXES) { + for (const owner of pagesUnder(local, `${prefix}:${instanceId}:`)) { + pages.add(owner); + } + } + return [...pages].filter((owner) => { + const record = registry[owner]; + return ( + owner !== page && + !(record && now - Number(record.at) < UNCONFIRMED_TTL_MS) + ); + }); } /** Forgets a page load's saved writes, early edits and record. */ @@ -293,10 +304,54 @@ export function saveDeferredEdits( } /** `closed`: the page unloaded (not into the back-forward cache). */ -type UnconfirmedRegistry = Record< - string, - { at: number; keys: string[]; closed?: boolean } ->; +interface PageRecord { + readonly at: number; + readonly keys: string[]; + readonly closed?: boolean; +} + +/** Key prefixes of what a page load keeps, each followed by `::`. */ +const PAGE_PREFIXES = [UNCONFIRMED_KEY, UNSENT_WRITES_KEY, DEFERRED_EDITS_KEY]; + +/** The page loads of keys that start with `prefix`. */ +function pagesUnder(local: UserDataStorage, prefix: string): string[] { + try { + return (local.keys?.() ?? []) + .filter((key) => key.startsWith(prefix)) + .map((key) => key.slice(prefix.length)); + } catch { + return []; + } +} + +function readRecord( + local: UserDataStorage, + instanceId: string, + page: string +): PageRecord | null { + try { + const parsed = JSON.parse( + local.getItem(recordKey(instanceId, page)) ?? 'null' + ) as PageRecord | null; + return parsed && Array.isArray(parsed.keys) ? parsed : null; + } catch { + return null; + } +} + +function readRegistry( + local: UserDataStorage, + instanceId: string +): Record { + const registry: Record = Object.create(null); + for (const page of pagesUnder(local, `${UNCONFIRMED_KEY}:${instanceId}:`)) { + const record = readRecord(local, instanceId, page); + if (record) { + registry[page] = record; + } + } + return registry; +} /** Marks a page load's record as unloaded: a reload may take it over. */ export function markClosed( @@ -304,13 +359,15 @@ export function markClosed( instanceId: string, page: string ): void { - const registry = readRegistry(local, instanceId); - if (!registry[page]) { + const record = readRecord(local, instanceId, page); + if (!record) { return; } - registry[page] = { ...registry[page], closed: true }; try { - local.setItem(unconfirmedKey(instanceId), JSON.stringify(registry)); + local.setItem( + recordKey(instanceId, page), + JSON.stringify({ ...record, closed: true }) + ); } catch { // Storage full: the record goes stale instead. } @@ -327,7 +384,7 @@ export function pageIsLive( page: string, now = Date.now() ): boolean { - const record = readRegistry(local, instanceId)[page]; + const record = readRecord(local, instanceId, page); return ( !!record && !record.closed && @@ -335,23 +392,7 @@ export function pageIsLive( ); } -function readRegistry( - local: UserDataStorage, - instanceId: string -): UnconfirmedRegistry { - try { - const parsed: unknown = JSON.parse( - local.getItem(unconfirmedKey(instanceId)) ?? '{}' - ); - return parsed && typeof parsed === 'object' && !Array.isArray(parsed) - ? (parsed as UnconfirmedRegistry) - : {}; - } catch { - return {}; - } -} - -/** Replaces only this tab's record. */ +/** Writes only this page load's record (none without keys). */ export function markUnconfirmed( local: UserDataStorage, instanceId: string, @@ -359,17 +400,15 @@ export function markUnconfirmed( keys: readonly string[], now = Date.now() ): void { - const registry = readRegistry(local, instanceId); - if (keys.length === 0) { - if (!(tabId in registry)) { - return; - } - delete registry[tabId]; - } else { - registry[tabId] = { at: now, keys: [...keys] }; - } try { - local.setItem(unconfirmedKey(instanceId), JSON.stringify(registry)); + if (keys.length === 0) { + local.removeItem(recordKey(instanceId, tabId)); + } else { + local.setItem( + recordKey(instanceId, tabId), + JSON.stringify({ at: now, keys: [...keys] }) + ); + } } catch { // Storage full: re-reads in other tabs lose this protection only. } @@ -386,11 +425,7 @@ export function othersUnconfirmed( for (const [owner, record] of Object.entries( readRegistry(local, instanceId) )) { - if ( - owner !== tabId && - Array.isArray(record?.keys) && - now - Number(record.at) < UNCONFIRMED_TTL_MS - ) { + if (owner !== tabId && now - Number(record.at) < UNCONFIRMED_TTL_MS) { for (const key of record.keys) { keys.add(String(key)); } diff --git a/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts b/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts index 0f4e1ffb5..35bdcbfc4 100644 --- a/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-storage.spec.ts @@ -8,6 +8,7 @@ function memoryStorage() { getItem: (key: string) => values.get(key) ?? null, setItem: (key: string, value: string) => void values.set(key, value), removeItem: (key: string) => void values.delete(key), + keys: () => [...values.keys()], }; } @@ -536,8 +537,8 @@ describe('ServerUserDataStorage', () => { local.setItem('iptvnator-server:cache-owner', 'old-db'); local.setItem('iptvnator-server:xtream-favorites', '{"p1":["a"]}'); local.setItem( - 'iptvnator-server:unconfirmed:old-db', - JSON.stringify({ tab: { at: 0, keys: ['xtream-favorites|p1'] } }) + 'iptvnator-server:unconfirmed:old-db:tab', + JSON.stringify({ at: 0, keys: ['xtream-favorites|p1'] }) ); local.setItem( 'iptvnator-server:unsent-writes:old-db:tab', @@ -562,9 +563,9 @@ describe('ServerUserDataStorage', () => { false ); expect(savedWrites(local, 'old-db')).toEqual([]); - expect(local.values.has('iptvnator-server:unconfirmed:old-db')).toBe( - false - ); + expect( + local.values.has('iptvnator-server:unconfirmed:old-db:tab') + ).toBe(false); expect(local.values.get('iptvnator-server:cache-owner')).toBe('new-db'); }); diff --git a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts index 521821207..70598b017 100644 --- a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts @@ -9,6 +9,7 @@ function memoryStorage() { getItem: (key: string) => values.get(key) ?? null, setItem: (key: string, value: string) => void values.set(key, value), removeItem: (key: string) => void values.delete(key), + keys: () => [...values.keys()], }; } @@ -523,12 +524,14 @@ describe('ServerUserDataStorage with other devices', () => { closed.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); await closed.flush(); // The tab is gone: its record stops being renewed. - const registry = 'iptvnator-server:unconfirmed:default'; - const records = JSON.parse(shared.values.get(registry) ?? '{}'); - for (const record of Object.values(records)) { - (record as { at: number }).at = 0; + for (const [key, value] of shared.values) { + if (key.startsWith('iptvnator-server:unconfirmed:default:')) { + shared.setItem( + key, + JSON.stringify({ ...JSON.parse(value), at: 0 }) + ); + } } - shared.setItem(registry, JSON.stringify(records)); const next = new ServerUserDataStorage( server.api as never, @@ -1280,4 +1283,44 @@ describe('ServerUserDataStorage with other devices', () => { JSON.parse('{"__proto__":["a","c"],"p2":["b"]}') ); }); + + it("sends a closed tab's saved writes whose record was lost", async () => { + const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); + const shared = memoryStorage(); + const closed = new ServerUserDataStorage( + { + ...server.api, + sendJson: jest + .fn() + .mockRejectedValue( + new ServerApiError(0, 'network', 'offline') + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await closed.hydrate(); + closed.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); + await closed.flush(); + // Another tab's save once replaced the shared record: now it is gone. + for (const key of [...shared.values.keys()]) { + if (key.startsWith('iptvnator-server:unconfirmed:')) { + shared.removeItem(key); + } + } + + const next = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await next.hydrate(); + await next.flush(); + + expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'X']); + }); }); diff --git a/libs/services/src/lib/user-data-storage.ts b/libs/services/src/lib/user-data-storage.ts index a291edf74..e1f931d17 100644 --- a/libs/services/src/lib/user-data-storage.ts +++ b/libs/services/src/lib/user-data-storage.ts @@ -1,10 +1,13 @@ import { InjectionToken } from '@angular/core'; -/** The slice of the Web Storage API that PWA user-data stores rely on. */ +/** + * The slice of the Web Storage API that PWA user-data stores rely on, plus + * the stored keys where the store can list them. + */ export type UserDataStorage = Pick< Storage, 'getItem' | 'setItem' | 'removeItem' ->; +> & { keys?(): string[] }; /** * `localStorage` behind an adapter that reads the global only inside each @@ -17,6 +20,17 @@ export function createBrowserUserDataStorage(): UserDataStorage { getItem: (key) => globalThis.localStorage.getItem(key), setItem: (key, value) => globalThis.localStorage.setItem(key, value), removeItem: (key) => globalThis.localStorage.removeItem(key), + keys: () => { + const storage = globalThis.localStorage; + const keys: string[] = []; + for (let index = 0; index < storage.length; index += 1) { + const key = storage.key(index); + if (key !== null) { + keys.push(key); + } + } + return keys; + }, }; }