diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 1572bc9b9..3fd1a3e69 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -45,7 +45,12 @@ The PIN is hashed with PBKDF2-SHA256 through WebCrypto (`parental-lock-pin.util.ts`, format `v1$$$`), so Electron and PWA share one implementation. The hash is deliberately not part of `Settings`: settings are logged, backed up and mirrored to the main -process. +process. A hash that could not be READ is not an absent PIN: +`ParentalLockStorageService.readPinHash()` reports the failure as `null` +(an absent PIN is `{hash: null}`), the session then stays locked (`enabled` +treats an unreadable PIN as set while the switch is unknown) and every +PIN-protected step — unlock, change PIN, disable — re-reads it first, so +storage answering later is enough to recover without a restart. The lock store (`ParentalLockStore` in `parental-lock.util.ts`) is keyed by playlist id and holds, per playlist, Xtream `{categoryType, xtreamId}` @@ -269,7 +274,11 @@ on either side. as locked while Electron reads, which filter by the index alone, still serve it); if the rollback write or its re-stamp fails too, the playlist is re-stamped on the next store access, and every launch re-derives the - index from the store for each playlist that has locks — awaited inside + index from the store for each playlist that has locks — which is why a + write that removes a playlist's LAST lock clears the index first and + drops the key afterwards (the reconcile finds playlists only through + their key; an interruption then leaves store-with-lock and index-without, + which the next reconcile repairs toward locked) — awaited inside the store's `load()`, so `readable` (and with it every catalog read the renderer gates) stays false until the index agrees with the store; a re-stamp that keeps failing keeps the session fail-closed. diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts index 3f98a65ce..a2096cd79 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts @@ -90,6 +90,44 @@ describe('ParentalLockLockStore', () => { expect(setCategoryLocks).toHaveBeenCalledWith('pl-1', 'series', []); }); + it('clears the index before the last lock leaves the store', async () => { + await store.load(); + await store.ensureReadable(); + setCategoryLocks.mockClear(); + storage.writeLocks.mockClear(); + const order: string[] = []; + setCategoryLocks.mockImplementation(async () => { + order.push('stamp'); + return true; + }); + storage.writeLocks.mockImplementation(async () => { + order.push('persist'); + return true; + }); + + await expect(store.setXtreamLocks('pl-1', 'live', [])).resolves.toBe( + true + ); + + expect(order).toEqual(['stamp', 'persist']); + expect(setCategoryLocks).toHaveBeenCalledWith('pl-1', 'live', []); + expect(storage.writeLocks).toHaveBeenLastCalledWith({}); + }); + + it('does not drop the key when clearing the index fails', async () => { + await store.load(); + await store.ensureReadable(); + storage.writeLocks.mockClear(); + setCategoryLocks.mockResolvedValue(false); + + await expect(store.setXtreamLocks('pl-1', 'live', [])).resolves.toBe( + false + ); + + expect(storage.writeLocks).not.toHaveBeenCalled(); + expect(store.lockedXtreamIds('pl-1', 'live')).toEqual([7]); + }); + it('rolls the store back when the index re-stamp fails', async () => { await store.load(); setCategoryLocks.mockResolvedValue(false); diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts index 525a4b3d7..a5712d361 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts @@ -183,14 +183,7 @@ export class ParentalLockLockStore { ): Promise { const previous = this.locksFor(playlistId); const next = withXtreamLocks(previous, categoryType, xtreamIds); - if (!(await this.persistPlaylistLocks(playlistId, next))) { - return false; - } - if (await this.stampXtreamLocks(playlistId, [categoryType])) { - return true; - } - await this.rollBack(playlistId, previous, [categoryType]); - return false; + return this.commitLocks(playlistId, previous, next, [categoryType]); } async setStalkerLocks( @@ -223,14 +216,58 @@ export class ParentalLockLockStore { playlistId: string, locks: ParentalLockPlaylistLocks ): Promise { - const previous = this.locksFor(playlistId); - if (!(await this.persistPlaylistLocks(playlistId, locks))) { + return this.commitLocks( + playlistId, + this.locksFor(playlistId), + locks, + XTREAM_CATEGORY_TYPES + ); + } + + /** + * Persists `next` and re-stamps the touched types. Order matters for a + * playlist whose LAST lock goes away: its key leaves the store, and the + * startup reconcile finds playlists only through their key — a crash + * between the two writes would strand stamped rows for good. That case + * clears the index first (from `next`) and persists afterwards, so an + * interruption leaves the store with the lock and the index without it, + * which the next reconcile repairs toward locked. Every other write + * persists first and rolls back on a failed re-stamp. + */ + private async commitLocks( + playlistId: string, + previous: ParentalLockPlaylistLocks, + next: ParentalLockPlaylistLocks, + categoryTypes: readonly ParentalLockXtreamCategoryType[] + ): Promise { + const normalizedNext = normalizeParentalLockPlaylistLocks(next); + if (isParentalLockPlaylistLocksEmpty(normalizedNext)) { + if (!(await this.ensureReadable())) { + return false; + } + if ( + !(await this.stampXtreamLocks( + playlistId, + categoryTypes, + normalizedNext + )) + ) { + return false; + } + if (await this.persistPlaylistLocks(playlistId, normalizedNext)) { + return true; + } + this.markIndexStale(playlistId); + this.revisionState.update((value) => value + 1); return false; } - if (await this.stampXtreamLocks(playlistId, XTREAM_CATEGORY_TYPES)) { + if (!(await this.persistPlaylistLocks(playlistId, next))) { + return false; + } + if (await this.stampXtreamLocks(playlistId, categoryTypes)) { return true; } - await this.rollBack(playlistId, previous, XTREAM_CATEGORY_TYPES); + await this.rollBack(playlistId, previous, categoryTypes); return false; } @@ -266,7 +303,8 @@ export class ParentalLockLockStore { */ async stampXtreamLocks( playlistId: string, - categoryTypes: readonly ParentalLockXtreamCategoryType[] = XTREAM_CATEGORY_TYPES + categoryTypes: readonly ParentalLockXtreamCategoryType[] = XTREAM_CATEGORY_TYPES, + locks: ParentalLockPlaylistLocks = this.locksFor(playlistId) ): Promise { if (!this.runtime.supportsXtreamSqliteDataSource) { return true; @@ -277,7 +315,7 @@ export class ParentalLockLockStore { (await this.databaseService.setCategoryLocks( playlistId, categoryType, - this.lockedXtreamIds(playlistId, categoryType) + lockedXtreamCategoryIds(locks, categoryType) )) && success; } return success; diff --git a/libs/services/src/lib/parental-lock/parental-lock-storage.ts b/libs/services/src/lib/parental-lock/parental-lock-storage.ts index 361df175c..6527a57b8 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-storage.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-storage.ts @@ -26,9 +26,18 @@ export class ParentalLockStorageService { return this.runtime.supportsAppStateStorage; } - async readPinHash(): Promise { - const value = (await this.read(PARENTAL_LOCK_PIN_KEY))?.value; - return value && value.trim() !== '' ? value : null; + /** + * The stored PIN hash (`hash: null` when none was set up), or `null` + * when the read itself failed — the service then retries before every + * PIN-protected operation instead of treating the PIN as absent. + */ + async readPinHash(): Promise<{ hash: string | null } | null> { + const result = await this.read(PARENTAL_LOCK_PIN_KEY); + if (result === null) { + return null; + } + const value = result.value; + return { hash: value && value.trim() !== '' ? value : null }; } async writePinHash(hash: string): Promise { diff --git a/libs/services/src/lib/parental-lock/parental-lock.service.spec.ts b/libs/services/src/lib/parental-lock/parental-lock.service.spec.ts index 49756dd88..bdb497843 100644 --- a/libs/services/src/lib/parental-lock/parental-lock.service.spec.ts +++ b/libs/services/src/lib/parental-lock/parental-lock.service.spec.ts @@ -57,7 +57,7 @@ describe('ParentalLockService', () => { storage = { pinHash: null, locks: {}, - readPinHash: jest.fn(async () => storage.pinHash), + readPinHash: jest.fn(async () => ({ hash: storage.pinHash })), writePinHash: jest.fn(async (hash: string) => { storage.pinHash = hash; return true; @@ -226,6 +226,25 @@ describe('ParentalLockService', () => { expect(updateBridgeSettings).not.toHaveBeenCalled(); }); + it('keeps the session locked on a failed PIN read and retries before unlocking', async () => { + storage.pinHash = await hashParentalLockPin('1234'); + parentalLockEnabled.set(true); + storage.readPinHash.mockResolvedValueOnce(null); + prompt.requestPin.mockImplementation( + async (request: ParentalLockPromptRequest) => + (await request.verify?.('1234')) ? '1234' : null + ); + + const service = await createService(); + + expect(service.active()).toBe(true); + expect(service.hasPin()).toBe(false); + + await expect(service.requestUnlock()).resolves.toBe(true); + expect(service.hasPin()).toBe(true); + expect(service.unlocked()).toBe(true); + }); + it('rolls the relock timeout back when it cannot be persisted', async () => { updateSettings.mockImplementationOnce(async () => { parentalLockRelockMinutes.set(30); diff --git a/libs/services/src/lib/parental-lock/parental-lock.service.ts b/libs/services/src/lib/parental-lock/parental-lock.service.ts index 496484564..ab82a7bf5 100644 --- a/libs/services/src/lib/parental-lock/parental-lock.service.ts +++ b/libs/services/src/lib/parental-lock/parental-lock.service.ts @@ -51,6 +51,8 @@ export class ParentalLockService { private readonly unlockedState = signal(false); private readonly pinHash = signal(null); + /** The PIN hash could not be read; retried before PIN-protected steps. */ + private readonly pinUnreadable = signal(false); private readonly versionState = signal(0); // The main process starts LOCKED whenever the feature is on (mirrored // setting). Reporting our state before settings have loaded would send a @@ -80,7 +82,7 @@ export class ParentalLockService { */ readonly enabled = computed(() => this.switchUnknown() - ? this.pinHash() !== null + ? this.pinHash() !== null || this.pinUnreadable() : this.settingsStore.parentalLockEnabled?.() === true ); /** Feature on and the PIN has been entered this session. */ @@ -89,6 +91,8 @@ export class ParentalLockService { readonly active = computed(() => this.enabled() && !this.unlockedState()); /** A PIN exists; enabling is only possible once this is true. */ readonly hasPin = computed(() => this.pinHash() !== null); + /** The lock store is read and trustworthy; backups must not run without it. */ + readonly locksReadable = computed(() => this.locks.readable()); /** * Locked, and the lock store could not be read: which categories are * locked is unknown, so EVERY category is withheld — the renderer-side @@ -145,12 +149,12 @@ export class ParentalLockService { initialize(): Promise { if (!this.initialization) { this.initialization = (async () => { - const [pinHash] = await Promise.all([ + const [pinRead] = await Promise.all([ this.storage.readPinHash(), this.locks.load(), this.settingsStore.loadSettings(), ]); - this.pinHash.set(pinHash); + this.applyPinRead(pinRead); this.versionState.update((value) => value + 1); this.settingsReady.set(true); })().catch((error) => { @@ -202,6 +206,7 @@ export class ParentalLockService { private async promptForUnlock( options: Pick ): Promise { + await this.ensurePin(); const hash = this.pinHash(); if (!this.prompt || !hash) { return false; @@ -281,6 +286,7 @@ export class ParentalLockService { /** Verifies the current PIN, then replaces it. */ async changePin(): Promise { await this.initialize(); + await this.ensurePin(); if (!this.prompt || !this.hasPin()) { return false; } @@ -317,6 +323,7 @@ export class ParentalLockService { */ private async verifyCurrentPin(): Promise { await this.initialize(); + await this.ensurePin(); const hash = this.pinHash(); if (!this.prompt || !hash) { return false; @@ -470,6 +477,29 @@ export class ParentalLockService { return this.locks.stampXtreamLocks(playlistId, categoryTypes); } + private applyPinRead(read: { hash: string | null } | null): void { + if (read === null) { + console.error('The parental lock PIN could not be read.'); + this.pinUnreadable.set(true); + return; + } + this.pinHash.set(read.hash); + this.pinUnreadable.set(false); + } + + /** + * Retries a PIN read that failed at startup. A failed read is not an + * absent PIN: the session stays locked (`enabled` treats it as set), + * and every PIN-protected step re-reads first so the parent can still + * unlock, change the PIN or switch the feature off once storage answers. + */ + private async ensurePin(): Promise { + if (!this.pinUnreadable()) { + return; + } + this.applyPinRead(await this.storage.readPinHash()); + } + private async storePin(pin: string): Promise { try { const hash = await hashParentalLockPin(pin); @@ -477,6 +507,7 @@ export class ParentalLockService { return false; } this.pinHash.set(hash); + this.pinUnreadable.set(false); return true; } catch (error) { console.error('Failed to store the parental lock PIN.', error); diff --git a/libs/services/src/lib/playlist-backup.service.spec.ts b/libs/services/src/lib/playlist-backup.service.spec.ts index 15e6f8dc7..36e44d8e2 100644 --- a/libs/services/src/lib/playlist-backup.service.spec.ts +++ b/libs/services/src/lib/playlist-backup.service.spec.ts @@ -14,6 +14,23 @@ describe('PlaylistBackupService', () => { localStorage.clear(); }); + it('refuses to export while the parental lock store is not readable', async () => { + const initialize = jest.fn().mockResolvedValue(undefined); + const service = createPlaylistBackupService({ + parentalLock: { + initialize, + locksReadable: jest.fn(() => false), + locksFor: jest.fn(() => ({ xtream: [], stalker: [], m3u: [] })), + replacePlaylistLocks: jest.fn().mockResolvedValue(true), + }, + }); + + await expect(service.exportBackup()).rejects.toThrow( + /parental lock store/ + ); + expect(initialize).toHaveBeenCalled(); + }); + it('exports self-contained M3U data and strips Stalker session fields', async () => { const playlistsService = { addPlaylist: jest.fn((playlist: Playlist) => of(playlist)), diff --git a/libs/services/src/lib/playlist-backup.service.test-helpers.ts b/libs/services/src/lib/playlist-backup.service.test-helpers.ts index e89212f1f..5525ca0cd 100644 --- a/libs/services/src/lib/playlist-backup.service.test-helpers.ts +++ b/libs/services/src/lib/playlist-backup.service.test-helpers.ts @@ -121,6 +121,8 @@ export function createPlaylistBackupService( }, pendingRestoreService, parentalLock: { + initialize: jest.fn().mockResolvedValue(undefined), + locksReadable: jest.fn(() => true), locksFor: jest.fn(() => ({ xtream: [], stalker: [], m3u: [] })), replacePlaylistLocks: jest.fn().mockResolvedValue(true), }, diff --git a/libs/services/src/lib/playlist-backup.service.ts b/libs/services/src/lib/playlist-backup.service.ts index 02631ed0e..238f72095 100644 --- a/libs/services/src/lib/playlist-backup.service.ts +++ b/libs/services/src/lib/playlist-backup.service.ts @@ -76,6 +76,15 @@ export class PlaylistBackupService { private backupImportTail?: Promise; async exportBackup(): Promise { + // Locks are written only when present, and an absent field means + // "no opinion" on restore — so an export must never run against an + // empty in-memory store that merely has not loaded, or could not. + await this.parentalLock.initialize(); + if (!this.parentalLock.locksReadable()) { + throw new Error( + 'The parental lock store could not be read; the backup would omit its locks.' + ); + } const playlists = await firstValueFrom( this.playlistsService.getAllData() );