diff --git a/js/app.js b/js/app.js index 9acbf248..f9803039 100644 --- a/js/app.js +++ b/js/app.js @@ -1264,7 +1264,9 @@ class Application { // encrypted); safe no-op when nothing is cached or for NIP-07/46. try { if (typeof nostrClient.restoreUnlockedSigner === "function") { - await nostrClient.restoreUnlockedSigner(normalizedSaved); + await nostrClient.restoreUnlockedSigner(normalizedSaved, { + validator: this.buildAccessControlValidator(), + }); } } catch (error) { devLogger.warn( @@ -2186,7 +2188,9 @@ class Application { !isSignerCapable(resolveSignerCapabilities(getActiveSigner()), need) ) { try { - const restored = await nostrClient.restoreUnlockedSigner(normalized); + const restored = await nostrClient.restoreUnlockedSigner(normalized, { + validator: this.buildAccessControlValidator(), + }); if ( restored?.restored && isSignerCapable(resolveSignerCapabilities(getActiveSigner()), need) @@ -2260,6 +2264,22 @@ class Application { return { ok: true, unlocked: true }; } + // Access-control gate passed into keep-unlocked signer restores so a + // since-blocked account's cached key is forgotten instead of restoring. + buildAccessControlValidator() { + return (pk) => { + if ( + accessControl && + typeof accessControl.canAccess === "function" && + !accessControl.canAccess(pk) + ) { + const error = new Error("Access restricted for this account."); + error.code = "access-denied"; + throw error; + } + }; + } + // True when the active (or given) account currently has a cached "keep // unlocked" key — i.e. there is something to lock (TODO #51). isSessionKeptUnlocked(pubkey) { diff --git a/js/app/authSessionCoordinator.js b/js/app/authSessionCoordinator.js index af2d4902..ea3059e3 100644 --- a/js/app/authSessionCoordinator.js +++ b/js/app/authSessionCoordinator.js @@ -999,15 +999,14 @@ export function createAuthSessionCoordinator(deps) { clearWatchHistoryConversationKeyCache(); clearWatchHistoryDecryptedChunkCache(); - // Forget any cached "keep unlocked" key for the account being logged out, so - // logout actually re-locks it (TODO #51). + // Forget the cached "keep unlocked" key so logout re-locks (TODO #51). + // MUST read previousPubkey: authService.logout already nulled the active + // pubkey, so detail.pubkey/this.pubkey are empty here. try { - const loggedOutPubkey = detail?.pubkey || this.pubkey; - if ( - loggedOutPubkey && - typeof nostrClient.forgetUnlockedSigner === "function" - ) { - nostrClient.forgetUnlockedSigner(loggedOutPubkey); + const loggedOutPubkey = + detail?.previousPubkey || detail?.pubkey || this.pubkey; + if (loggedOutPubkey) { + nostrClient.forgetUnlockedSigner?.(loggedOutPubkey); } } catch (error) { devLogger.warn("Failed to forget cached unlock during logout:", error); @@ -1712,14 +1711,14 @@ export function createAuthSessionCoordinator(deps) { } } - // Removing a saved profile must also forget its cached "keep unlocked" key. + // A removed profile must not leave keys on the device: forget its cached + // "keep unlocked" key + stored credentials (nsec key, NIP-46 session). try { - if (typeof nostrClient.forgetUnlockedSigner === "function") { - nostrClient.forgetUnlockedSigner(normalizedTarget); - } + nostrClient.forgetUnlockedSigner?.(normalizedTarget); + nostrClient.forgetStoredAccount?.(normalizedTarget); } catch (error) { devLogger.warn( - "[Application] Failed to forget cached unlock for removed profile:", + "[Application] Failed to forget credentials for removed profile:", error, ); } diff --git a/js/nostr/client.js b/js/nostr/client.js index 4b75b729..41f72c4b 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -2539,9 +2539,10 @@ export class NostrClient { try { opts.validator(adapter.pubkey); } catch (error) { - // Don't leave a since-blocked key persisted on this device. + // Don't leave the since-blocked key persisted on this device. Targeted: + // only THIS account's entry — the no-arg clear wiped every saved key. try { - clearStoredSessionActorEntry(); + clearStoredSessionActorEntry(adapter.pubkey); } catch (cleanupError) { devLogger.warn( "[nostr] Failed to clear stored session after access denial:", @@ -2738,8 +2739,11 @@ export class NostrClient { // Restore an nsec signer from the "keep unlocked" cache without a passphrase // (TODO #51). Reads the cached decrypted key for `targetPubkey`, verifies it // derives to that pubkey (defense-in-depth; a mismatch forgets the cache), and - // registers it app-wide. Returns { restored: boolean }. - async restoreUnlockedSigner(targetPubkey) { + // registers it app-wide. Pass a `validator` (access-control gate, like + // unlockStoredSessionActor's) where available — a since-blocked account's + // cached key is then forgotten instead of silently restoring its signer. + // Returns { restored: boolean }. + async restoreUnlockedSigner(targetPubkey, { validator } = {}) { const pubkey = typeof targetPubkey === "string" && HEX64_REGEX.test(targetPubkey.trim().toLowerCase()) ? targetPubkey.trim().toLowerCase() @@ -2771,6 +2775,17 @@ export class NostrClient { return { restored: false, reason: "pubkey-mismatch" }; } + // Access-control gate (mirrors unlockStoredSessionActor): a since-blocked + // account must not silently restore its signer — forget its cached key. + if (typeof validator === "function") { + try { + validator(pubkey); + } catch (error) { + forgetUnlockedKey(pubkey); + return { restored: false, reason: "access-denied", error }; + } + } + try { const adapter = await this.registerPrivateKeySigner({ privateKey: privateKeyHex, @@ -3064,6 +3079,28 @@ export class NostrClient { return this.signerManager.clearStoredSessionActor(); } + // Forget ONE removed account's stored credentials (remembered nsec key + + // NIP-46 remote-signer session) without touching the active session. Used by + // the profile switcher's remove-account flow so a removed profile doesn't + // leave its keys on the device. + forgetStoredAccount(pubkey) { + const normalized = + typeof pubkey === "string" ? pubkey.trim().toLowerCase() : ""; + if (!normalized) { + return; + } + try { + this.signerManager.forgetStoredSessionActor(normalized); + } catch (error) { + devLogger.warn("[nostr] Failed to forget stored nsec key:", error); + } + try { + clearStoredNip46Session(normalized); + } catch (error) { + devLogger.warn("[nostr] Failed to forget stored NIP-46 session:", error); + } + } + /** * Logs out the current user and clears all session state. * diff --git a/js/nostr/managers/SignerManager.js b/js/nostr/managers/SignerManager.js index 43a819f9..e4e9b521 100644 --- a/js/nostr/managers/SignerManager.js +++ b/js/nostr/managers/SignerManager.js @@ -203,12 +203,36 @@ export class SignerManager { return newActor.pubkey; } - clearStoredSessionActor() { - clearStoredSessionActorEntry(); + // Forget a stored (remembered) key. Pass the account's pubkey — the no-arg + // form only clears the legacy v1 slot and must never be used to log out one + // account (it used to wipe EVERY saved account's key). + clearStoredSessionActor(pubkey) { + clearStoredSessionActorEntry(pubkey); this.sessionActor = null; this.lockedSessionActor = null; } + // Forget ONE account's remembered key WITHOUT touching the current session's + // in-memory actors (used when removing a non-active saved profile). Only drops + // the in-memory references if they belong to the removed account. + forgetStoredSessionActor(pubkey) { + const target = + typeof pubkey === "string" ? pubkey.trim().toLowerCase() : ""; + if (!target) { + return; + } + clearStoredSessionActorEntry(target); + const matches = (actor) => + typeof actor?.pubkey === "string" && + actor.pubkey.trim().toLowerCase() === target; + if (matches(this.lockedSessionActor)) { + this.lockedSessionActor = null; + } + if (matches(this.sessionActor) && !this.sessionActor?.privateKey) { + this.sessionActor = null; + } + } + async ensureActiveSignerForPubkey(pubkey) { const normalizedPubkey = typeof pubkey === "string" && pubkey.trim() @@ -1083,8 +1107,15 @@ export class SignerManager { previousSessionActor.persisted !== true; if (shouldClearStoredSession) { - this.lockedSessionActor = null; - this.clearStoredSessionActor(); + // Forget ONLY the departing account's remembered key. The no-arg clear + // used to wipe the whole per-account map, so logging out one non-persisted + // nsec login destroyed every OTHER saved account's key. Skip entirely when + // we can't name the account — never wipe-all implicitly. + const clearTarget = previousSessionActor.pubkey || previousPubkey || ""; + if (clearTarget) { + this.lockedSessionActor = null; + this.clearStoredSessionActor(clearTarget); + } } if ( diff --git a/js/nostr/sessionActor.js b/js/nostr/sessionActor.js index 18b98b99..1cba12bd 100644 --- a/js/nostr/sessionActor.js +++ b/js/nostr/sessionActor.js @@ -677,19 +677,19 @@ export function clearStoredSessionActor(pubkey) { return; } - clearStoredSessionActorIndexedDb(); - + // No target pubkey: clear ONLY the legacy v1 "last saved" slot. Wiping the + // per-account v2 map here deleted EVERY saved account's remembered nsec key — + // the no-arg callers were logout/teardown paths for ONE account, so logging + // out (or a blocked-key cleanup) silently destroyed the other saved accounts' + // keys and broke switching back to them. Same bug class as the NIP-46 + // session-map wipe. A specific account is forgotten via + // clearStoredSessionActor(pubkey), which remains targeted. if (typeof localStorage !== "undefined") { try { localStorage.removeItem(SESSION_ACTOR_STORAGE_KEY); } catch (error) { devLogger.warn("[nostr] Failed to clear stored session actor:", error); } - try { - localStorage.removeItem(SESSION_ACTORS_MAP_STORAGE_KEY); - } catch (error) { - devLogger.warn("[nostr] Failed to clear session actor map:", error); - } } } diff --git a/tests/session-actor-multi-account.test.mjs b/tests/session-actor-multi-account.test.mjs index a54cf89c..6b0eec7e 100644 --- a/tests/session-actor-multi-account.test.mjs +++ b/tests/session-actor-multi-account.test.mjs @@ -4,7 +4,7 @@ // key survived, so switching among saved nsec accounts silently failed. // // test_integrity_note: -// change_type: ["new_tests"] +// change_type: ["new_tests", "spec_correction"] // scenarios: // - id: SCN-nsec-multi-account-storage // given: "two nsec accounts each persist their encrypted key" @@ -83,15 +83,27 @@ test("no-arg read returns the last-saved account as the default", () => { assert.equal(readStoredSessionActorEntry().privateKeyEncrypted, "cipher-B"); }); -test("no-arg clear wipes every stored account (logout)", () => { +// spec_correction: the previous test asserted "no-arg clear wipes every stored +// account (logout)". That encoded a bug: the no-arg clear is reached from +// per-account paths (SignerManager.logout's non-persisted-nsec cleanup, the +// blocked-key cleanup in registerPrivateKeySigner), so logging out ONE account +// silently destroyed every OTHER saved account's remembered key and broke +// switching back to them — the same bug class as the NIP-46 session-map wipe. +// Correct behavior: no-arg clears only the legacy v1 "last saved" slot; the +// per-account v2 map survives. Forgetting a specific account uses the targeted +// clearStoredSessionActor(pubkey) (tested above). +test("no-arg clear drops the legacy v1 slot but KEEPS per-account saved keys", () => { persistSessionActor(actor(PUB_A, "cipher-A")); persistSessionActor(actor(PUB_B, "cipher-B")); - clearStoredSessionActor(); + clearStoredSessionActor(); // e.g. one account's logout cleanup - assert.deepEqual(listStoredSessionActorPubkeys(), []); - assert.equal(readStoredSessionActorEntry(PUB_A), null); - assert.equal(readStoredSessionActorEntry(), null); + // Both accounts remain switchable — their remembered keys survive. + assert.deepEqual(listStoredSessionActorPubkeys().sort(), [PUB_A, PUB_B].sort()); + assert.equal(readStoredSessionActorEntry(PUB_A).privateKeyEncrypted, "cipher-A"); + assert.equal(readStoredSessionActorEntry(PUB_B).privateKeyEncrypted, "cipher-B"); + // The legacy single-slot default is gone. + assert.equal(localStorage.getItem(SESSION_ACTOR_STORAGE_KEY), null); }); test("a legacy v1 single-slot entry migrates into the per-pubkey map", () => {