From b8838c1c0967c4d17a2cfd437537c1da5a585441 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 20:26:39 +0200 Subject: [PATCH] fix(server-storage): no token for an unverified instance, sign-in remembers its server - ServerStorageStatusService.accessToken() returns the stored token only while server mode runs on an instance this start verified. The API client, the HttpClient interceptor and PwaService's raw fetch calls use it, so after an offline start from a remembered instance no request (including the /instance reachability check) carries the token to whatever answers at that address, and a 401 then does not clear it. - Sign-in remembers the server along with the token; when storage refuses either, it reports SIGN_IN_STORAGE instead of reloading into a start that would drop the token. Co-Authored-By: Claude Opus 5.5 --- .../server-sign-in.component.spec.ts | 54 ++++++++++++++++++- .../server-sign-in.component.ts | 12 ++++- .../server-storage/server-storage-start.ts | 16 +++++- apps/web/src/app/services/pwa.service.ts | 3 +- docs/architecture/pwa-self-hosted.md | 13 +++-- .../src/lib/server-storage-status.service.ts | 23 +++++++- .../server-access-token.interceptor.spec.ts | 29 ++++++++++ .../server-access-token.interceptor.ts | 7 +-- .../server-storage/server-api-client.spec.ts | 25 +++++++++ .../lib/server-storage/server-api-client.ts | 17 ++++-- .../server-storage/server-storage-session.ts | 10 +++- 11 files changed, 186 insertions(+), 23 deletions(-) diff --git a/apps/web/src/app/server-storage/server-sign-in.component.spec.ts b/apps/web/src/app/server-storage/server-sign-in.component.spec.ts index 8c055861a..82a28c023 100644 --- a/apps/web/src/app/server-storage/server-sign-in.component.spec.ts +++ b/apps/web/src/app/server-storage/server-sign-in.component.spec.ts @@ -1,9 +1,11 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { TranslateModule } from '@ngx-translate/core'; import { + SERVER_STORAGE_CONFIG, ServerApiError, ServerStorageSession, } from '@iptvnator/services/server-storage'; +import { REMEMBERED_SERVER_KEY } from './server-storage-start'; import { ServerSignInComponent } from './server-sign-in.component'; const TOKEN_KEY = 'iptvnator-server:access-token'; @@ -16,7 +18,22 @@ describe('ServerSignInComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ imports: [ServerSignInComponent, TranslateModule.forRoot()], - providers: [{ provide: ServerStorageSession, useValue: session }], + providers: [ + { provide: ServerStorageSession, useValue: session }, + { + provide: SERVER_STORAGE_CONFIG, + useValue: { + baseUrl: '/api', + instance: { + service: 'iptvnator', + apiVersion: 1, + instanceId: 'i-1', + storage: 'server', + auth: 'token', + }, + }, + }, + ], }).compileComponents(); fixture = TestBed.createComponent(ServerSignInComponent); component = fixture.componentInstance; @@ -100,4 +117,39 @@ describe('ServerSignInComponent', () => { expect(session.fetchStatus).not.toHaveBeenCalled(); expect(feedbackText()).toBe('SERVER_STORAGE.SIGN_IN_STORAGE'); }); + + it('remembers the server the token was accepted for', async () => { + session.fetchStatus.mockResolvedValue({ playlists: 0 }); + jest.spyOn(component, 'openWorkspace').mockImplementation( + () => undefined + ); + + component.edit('right'); + await component.submit(new Event('submit')); + + expect( + JSON.parse(localStorage.getItem(REMEMBERED_SERVER_KEY) ?? '{}') + ).toMatchObject({ baseUrl: '/api', instance: { instanceId: 'i-1' } }); + }); + + it('signs in only when the server can be remembered too', async () => { + const refuse = jest + .spyOn(Storage.prototype, 'setItem') + .mockImplementation((key: string) => { + if (key === REMEMBERED_SERVER_KEY) { + throw new DOMException('full', 'QuotaExceededError'); + } + }); + try { + component.edit('right'); + await component.submit(new Event('submit')); + fixture.detectChanges(); + } finally { + refuse.mockRestore(); + } + + expect(session.fetchStatus).not.toHaveBeenCalled(); + expect(localStorage.getItem(TOKEN_KEY)).toBeNull(); + expect(feedbackText()).toBe('SERVER_STORAGE.SIGN_IN_STORAGE'); + }); }); diff --git a/apps/web/src/app/server-storage/server-sign-in.component.ts b/apps/web/src/app/server-storage/server-sign-in.component.ts index 9b20057fc..d05134778 100644 --- a/apps/web/src/app/server-storage/server-sign-in.component.ts +++ b/apps/web/src/app/server-storage/server-sign-in.component.ts @@ -14,9 +14,11 @@ import { writeServerAccessToken, } from '@iptvnator/services'; import { + SERVER_STORAGE_CONFIG, ServerApiError, ServerStorageSession, } from '@iptvnator/services/server-storage'; +import { rememberServer } from './server-storage-start'; import { PasswordVisibilityToggleDirective } from '@iptvnator/ui/components/password-visibility-toggle'; export type ServerSignInFeedback = 'rejected' | 'unreachable' | 'storage'; @@ -37,6 +39,7 @@ export type ServerSignInFeedback = 'rejected' | 'unreachable' | 'storage'; }) export class ServerSignInComponent { private readonly session = inject(ServerStorageSession); + private readonly config = inject(SERVER_STORAGE_CONFIG); readonly token = signal(''); readonly busy = signal(false); @@ -53,8 +56,13 @@ export class ServerSignInComponent { if (!token || this.busy()) { return; } - // Every request and the reload read the token from storage. - if (!writeServerAccessToken(token)) { + // Every request and the reload read the token from storage, and the + // next start keeps it only for the server it remembers. + if ( + !writeServerAccessToken(token) || + !rememberServer(this.config.baseUrl, this.config.instance) + ) { + clearServerAccessToken(); this.feedback.set('storage'); return; } 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 f9f7a5ad6..51c682f9b 100644 --- a/apps/web/src/app/server-storage/server-storage-start.ts +++ b/apps/web/src/app/server-storage/server-storage-start.ts @@ -95,14 +95,28 @@ function read( } } +/** + * Remembers the server a token was accepted for; false when storage + * refused it (the next start would take the token for another server's). + */ +export function rememberServer( + baseUrl: string, + instance: ServerInstanceInfo, + storage: Pick | null = safeLocalStorage() +): boolean { + return write(storage, { baseUrl, instance }); +} + function write( storage: Pick | null, value: RememberedServer -): void { +): boolean { try { storage?.setItem(REMEMBERED_SERVER_KEY, JSON.stringify(value)); + return !!storage; } catch { // Without storage an offline start falls back to browser mode. + return false; } } diff --git a/apps/web/src/app/services/pwa.service.ts b/apps/web/src/app/services/pwa.service.ts index 62ec56287..426589160 100644 --- a/apps/web/src/app/services/pwa.service.ts +++ b/apps/web/src/app/services/pwa.service.ts @@ -16,7 +16,6 @@ import { import { DataService, isServerAccessChallenge, - readServerAccessToken, ServerStorageStatusService, } from '@iptvnator/services'; import { @@ -810,7 +809,7 @@ export class PwaService extends DataService { * backend: a token kept from another backend must not reach this one. */ private serverAccessToken(): string | null { - return this.serverStatus.isActive() ? readServerAccessToken() : null; + return this.serverStatus.accessToken(); } /** A token changed on the server shows up on any backend call. */ diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 737aae50c..bfa93ce24 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -186,8 +186,9 @@ instance under `iptvnator-server:instance`. When the probe gets no answer restarts, or a refusal other than 404, such as a proxy's 429), a browser that remembers this backend still starts in server mode, offline, and reloads once the server answers; it never falls back to an empty browser library. Until that reload it sends no writes -(`holdWrites`): the address may lead to another database by then, which -the reload's probe finds out. An answer without server storage, or a 404 from +(`holdWrites`) and no token anywhere (`ServerStorageStatusService.accessToken()` +is null for an unverified instance): the address may lead to another +server or database by then, which the reload's probe finds out. An answer without server storage, or a 404 from an older backend, forgets it (`server-storage-start.ts`). Everything else of the client lives behind the `@iptvnator/services/server-storage` sub-entry, so other builds never load it; `ServerStorageStatusService` and the token @@ -299,9 +300,11 @@ settings and guards read them in every build. 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 only while - server mode runs. The browser forgets the token when the backend answers - without server storage, or as another server or database (backend URL or - instance id) than the one it remembers. Both turn + server mode runs on a verified instance. The browser forgets the token + when the backend answers without server storage, or as another server or + database (backend URL or instance id) than the one it remembers; sign-in + therefore remembers the server along with the token, or reports storage + trouble. 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-status.service.ts b/libs/services/src/lib/server-storage-status.service.ts index 0d0c37411..a670dd369 100644 --- a/libs/services/src/lib/server-storage-status.service.ts +++ b/libs/services/src/lib/server-storage-status.service.ts @@ -21,18 +21,39 @@ export class ServerStorageStatusService { private readonly instanceSignal = signal(null); private readonly lastSuccessSignal = signal(null); private retryHandler: (() => void) | null = null; + private verified = true; readonly state = this.stateSignal.asReadonly(); readonly instance = this.instanceSignal.asReadonly(); readonly lastSuccessAt = this.lastSuccessSignal.asReadonly(); readonly isActive = computed(() => this.stateSignal() !== 'inactive'); - activate(instance: ServerInstanceInfo, retry: () => void): void { + /** + * `verified` is false for a start from a remembered instance that did + * not answer: the address may lead to another server by now. + */ + activate( + instance: ServerInstanceInfo, + retry: () => void, + verified = true + ): void { this.instanceSignal.set(instance); this.retryHandler = retry; + this.verified = verified; this.stateSignal.set('online'); } + /** + * The stored token, only while server mode runs on an instance this + * start verified: a token must never reach a server it was not given + * for. + */ + accessToken(): string | null { + return this.isActive() && this.verified + ? readServerAccessToken() + : null; + } + reportSuccess(): void { // A late answer to a request sent with the old token must not undo // sign-out; only a reload after a new token leaves this state. diff --git a/libs/services/src/lib/server-storage/server-access-token.interceptor.spec.ts b/libs/services/src/lib/server-storage/server-access-token.interceptor.spec.ts index 2f9a15fa4..152990e43 100644 --- a/libs/services/src/lib/server-storage/server-access-token.interceptor.spec.ts +++ b/libs/services/src/lib/server-storage/server-access-token.interceptor.spec.ts @@ -39,7 +39,21 @@ describe('ServerAccessTokenInterceptor', () => { afterEach(() => localStorage.clear()); + const activate = (verified = true) => + TestBed.inject(ServerStorageStatusService).activate( + { + service: 'iptvnator', + apiVersion: 1, + instanceId: 'i', + storage: 'server', + auth: 'token', + }, + () => undefined, + verified + ); + it('adds the token to backend calls only', () => { + activate(); const http = TestBed.inject(HttpClient); const controller = TestBed.inject(HttpTestingController); @@ -58,6 +72,21 @@ describe('ServerAccessTokenInterceptor', () => { ).toBe(false); }); + it('withholds the token from an instance not checked yet', () => { + activate(false); + const http = TestBed.inject(HttpClient); + const controller = TestBed.inject(HttpTestingController); + + http.get('/api/v1/instance').subscribe(); + + expect( + controller + .expectOne('/api/v1/instance') + .request.headers.has('Authorization') + ).toBe(false); + controller.verify(); + }); + it('asks for a new token when the backend refuses the stored one', () => { const status = TestBed.inject(ServerStorageStatusService); status.activate( diff --git a/libs/services/src/lib/server-storage/server-access-token.interceptor.ts b/libs/services/src/lib/server-storage/server-access-token.interceptor.ts index 098136417..63ef61fc5 100644 --- a/libs/services/src/lib/server-storage/server-access-token.interceptor.ts +++ b/libs/services/src/lib/server-storage/server-access-token.interceptor.ts @@ -7,10 +7,7 @@ import { } from '@angular/common/http'; import { inject, Injectable } from '@angular/core'; import { Observable, tap } from 'rxjs'; -import { - isServerAccessChallenge, - readServerAccessToken, -} from '../server-access-token'; +import { isServerAccessChallenge } from '../server-access-token'; import { ServerStorageStatusService } from '../server-storage-status.service'; import { SERVER_STORAGE_CONFIG } from './server-storage.config'; @@ -31,7 +28,7 @@ export class ServerAccessTokenInterceptor implements HttpInterceptor { if (!isBackendUrl(request.url, this.baseUrl)) { return next.handle(request); } - const token = readServerAccessToken(); + const token = this.status.accessToken(); const sent = token ? request.clone({ setHeaders: { Authorization: `Bearer ${token}` }, diff --git a/libs/services/src/lib/server-storage/server-api-client.spec.ts b/libs/services/src/lib/server-storage/server-api-client.spec.ts index bbb5c339d..96a6fea9a 100644 --- a/libs/services/src/lib/server-storage/server-api-client.spec.ts +++ b/libs/services/src/lib/server-storage/server-api-client.spec.ts @@ -204,6 +204,31 @@ describe('ServerApiClient', () => { expect(status.reportUnauthorized).not.toHaveBeenCalled(); }); + it('sends no token to an instance not checked yet and keeps it on a 401', async () => { + localStorage.setItem(SERVER_ACCESS_TOKEN_STORAGE_KEY, 'kept'); + const fetchFn = jest + .fn() + .mockResolvedValue(response(401, { code: 'unauthorized' })); + const unchecked = new ServerApiClient({ + baseUrl: '/api', + status, + fetchFn: fetchFn as unknown as typeof fetch, + accessToken: () => null, + }); + + await expect(unchecked.getJson('/instance')).rejects.toBeInstanceOf( + ServerApiError + ); + + expect(fetchFn.mock.calls[0][1].headers).not.toHaveProperty( + 'Authorization' + ); + expect(localStorage.getItem(SERVER_ACCESS_TOKEN_STORAGE_KEY)).toBe( + 'kept' + ); + expect(status.reportUnauthorized).not.toHaveBeenCalled(); + }); + it('passes 304 through and maps error bodies', async () => { const fetchFn = jest .fn() diff --git a/libs/services/src/lib/server-storage/server-api-client.ts b/libs/services/src/lib/server-storage/server-api-client.ts index 507e60da8..f7e64d44b 100644 --- a/libs/services/src/lib/server-storage/server-api-client.ts +++ b/libs/services/src/lib/server-storage/server-api-client.ts @@ -6,7 +6,6 @@ import type { import { clearServerAccessToken, readServerAccessToken, - serverAuthorizationHeaders, } from '../server-access-token'; import type { ServerStorageStatusService } from '../server-storage-status.service'; @@ -54,6 +53,8 @@ export interface ServerApiClientOptions { * the time it does. */ readonly holdWrites?: boolean; + /** The token to send; the stored one by default. */ + readonly accessToken?: () => string | null; } /** `fetch` for `/v1`: token, timeout, error mapping, connection state. */ @@ -64,13 +65,17 @@ export class ServerApiClient { this.root = `${options.baseUrl.replace(/\/+$/, '')}/v1`; } + private token(): string | null { + return (this.options.accessToken ?? readServerAccessToken)(); + } + url(path: string): string { return `${this.root}${path}`; } async request(path: string, init: RequestInit = {}): Promise { const isWrite = (init.method ?? 'GET') !== 'GET'; - const token = readServerAccessToken(); + const token = this.token(); let response: Response; try { if (isWrite && this.options.holdWrites) { @@ -80,7 +85,7 @@ export class ServerApiClient { ...init, cache: 'no-store', headers: { - ...serverAuthorizationHeaders(), + ...bearer(token), ...(init.headers as Record | undefined), }, signal: init.signal ?? this.timeoutSignal(), @@ -192,7 +197,7 @@ export class ServerApiClient { method, keepalive: true, headers: { - ...serverAuthorizationHeaders(), + ...bearer(this.token()), ...headers, ...(body === undefined ? {} @@ -248,3 +253,7 @@ async function readErrorBody( return { message: response.statusText }; } } + +function bearer(token: string | null): Record { + return token ? { Authorization: `Bearer ${token}` } : {}; +} diff --git a/libs/services/src/lib/server-storage/server-storage-session.ts b/libs/services/src/lib/server-storage/server-storage-session.ts index 257c936c4..f694048d6 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.ts @@ -45,8 +45,10 @@ export class ServerStorageSession { status: this.status, onWriteFailure: (error) => this.announceWriteFailure(error), // A remembered instance may answer as another database: writes - // wait for the reload that checks which one it is. + // wait for the reload that checks which one it is, and the token + // goes out only once it is checked. holdWrites: this.config.startedOffline, + accessToken: () => this.status.accessToken(), }); readonly documentStore = new ServerPlaylistDocumentStore( this.api, @@ -75,7 +77,11 @@ export class ServerStorageSession { /** App initializer: must run before the first playlist read. */ async start(): Promise { this.backend.useServerStore(this.documentStore); - this.status.activate(this.config.instance, () => void this.refresh()); + this.status.activate( + this.config.instance, + () => void this.refresh(), + !this.config.startedOffline + ); // Also while sign-in is pending: this page may already hold another // page load's writes, so it must mark itself closed on page hide. this.listen();