From 59161df063729543088750e7904b2d6002056d11 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Thu, 30 Oct 2025 11:39:09 -0400 Subject: [PATCH] Require active signer for comment publishing --- js/nostr/commentEvents.js | 101 +++++++++++---------------- js/ui/videoModalCommentController.js | 19 ++++- tests/nostr/comment-events.test.mjs | 91 +++++++++--------------- tests/video-modal-comments.test.mjs | 101 +++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 121 deletions(-) diff --git a/js/nostr/commentEvents.js b/js/nostr/commentEvents.js index 19580dc6..b4499249 100644 --- a/js/nostr/commentEvents.js +++ b/js/nostr/commentEvents.js @@ -1023,7 +1023,6 @@ export async function publishComment( { resolveActiveSigner, shouldRequestExtensionPermissions, - signEventWithPrivateKey, DEFAULT_NIP07_PERMISSION_METHODS, } = {}, ) { @@ -1036,9 +1035,12 @@ export async function publishComment( return { ok: false, error: "invalid-target" }; } - const actorPubkey = await client.ensureSessionActor(); + const actorPubkey = + typeof client?.pubkey === "string" && client.pubkey.trim() + ? client.pubkey.trim() + : ""; if (!actorPubkey) { - return { ok: false, error: "missing-actor" }; + return { ok: false, error: "auth-required" }; } const createdAt = @@ -1097,71 +1099,46 @@ export async function publishComment( let signedEvent = null; - const normalizedActor = - typeof actorPubkey === "string" ? actorPubkey.toLowerCase() : ""; - const normalizedLogged = - typeof client.pubkey === "string" ? client.pubkey.toLowerCase() : ""; - const resolveSignerFn = typeof resolveActiveSigner === "function" ? resolveActiveSigner : null; const signer = resolveSignerFn ? resolveSignerFn(actorPubkey) : null; - const canUseActiveSigner = - normalizedActor && - normalizedActor === normalizedLogged && - signer && - typeof signer.signEvent === "function"; - - if (canUseActiveSigner) { - let permissionResult = { ok: true }; - const shouldRequestPermissions = - typeof shouldRequestExtensionPermissions === "function" - ? shouldRequestExtensionPermissions(signer) - : false; - - if (shouldRequestPermissions) { - permissionResult = await client.ensureExtensionPermissions( - DEFAULT_NIP07_PERMISSION_METHODS, - ); - } - - if (permissionResult.ok) { - try { - signedEvent = await signer.signEvent(event); - } catch (error) { - userLogger.warn( - "[nostr] Failed to sign comment event with active signer:", - error, - ); - return { ok: false, error: "signing-failed", details: error }; - } - } else { - userLogger.warn( - "[nostr] Active signer permissions missing; signing comment with session key.", - permissionResult.error, - ); - } + if (!signer || typeof signer.signEvent !== "function") { + return { ok: false, error: "auth-required" }; } - if (!signedEvent) { - if (typeof signEventWithPrivateKey !== "function") { - return { ok: false, error: "signing-unavailable" }; - } - try { - if (!client.sessionActor || client.sessionActor.pubkey !== actorPubkey) { - await client.ensureSessionActor(true); - } - if (!client.sessionActor || client.sessionActor.pubkey !== actorPubkey) { - throw new Error("session-actor-mismatch"); - } - signedEvent = signEventWithPrivateKey( - event, - client.sessionActor.privateKey, - ); - } catch (error) { - userLogger.warn("[nostr] Failed to sign comment event with session key:", error); - return { ok: false, error: "signing-failed", details: error }; - } + let permissionResult = { ok: true }; + const shouldRequestPermissions = + typeof shouldRequestExtensionPermissions === "function" + ? shouldRequestExtensionPermissions(signer) + : false; + + if (shouldRequestPermissions) { + permissionResult = await client.ensureExtensionPermissions( + DEFAULT_NIP07_PERMISSION_METHODS, + ); + } + + if (!permissionResult.ok) { + userLogger.warn( + "[nostr] Active signer permissions missing; comment publish requires login.", + permissionResult.error, + ); + return { + ok: false, + error: "auth-required", + details: permissionResult, + }; + } + + try { + signedEvent = await signer.signEvent(event); + } catch (error) { + userLogger.warn( + "[nostr] Failed to sign comment event with active signer:", + error, + ); + return { ok: false, error: "signing-failed", details: error }; } const relayList = sanitizeRelayList(options.relays, client.relays); diff --git a/js/ui/videoModalCommentController.js b/js/ui/videoModalCommentController.js index 38d8662a..143e9a5b 100644 --- a/js/ui/videoModalCommentController.js +++ b/js/ui/videoModalCommentController.js @@ -628,7 +628,7 @@ export default class VideoModalCommentController { try { const result = await publishPromise; if (!result?.ok || !result.event) { - throw result?.error || new Error("publish-failed"); + throw result || { error: "publish-failed" }; } const event = this.enrichCommentEvent(result.event); @@ -650,6 +650,23 @@ export default class VideoModalCommentController { this.applyCommentComposerAuthState(); this.videoModal.setCommentStatus?.("Comment posted."); } catch (error) { + const authRequired = + (error && typeof error === "object" && error.error === "auth-required") || + error === "auth-required"; + + if (authRequired) { + this.videoModal.setCommentComposerState?.({ + disabled: true, + reason: "login-required", + }); + this.handleVideoModalCommentLoginRequired(detail); + this.videoModal.setCommentComposerState?.({ + disabled: true, + reason: "login-required", + }); + return; + } + devLogger.warn("[comment] Failed to publish comment:", error); this.videoModal.setCommentComposerState?.({ disabled: false, diff --git a/tests/nostr/comment-events.test.mjs b/tests/nostr/comment-events.test.mjs index 46c6c390..b8d47b96 100644 --- a/tests/nostr/comment-events.test.mjs +++ b/tests/nostr/comment-events.test.mjs @@ -10,9 +10,9 @@ import { function createMockClient({ actorPubkey = "actor-pubkey", - sessionPrivateKey = "session-private", relays = ["wss://primary"], publishResults = [], + extensionPermissions = { ok: true }, } = {}) { const publishCalls = []; const pool = { @@ -34,29 +34,14 @@ function createMockClient({ sub: () => ({ on() {}, unsub() {} }), }; - const ensureSessionActorCalls = []; const client = { pool, relays, pubkey: actorPubkey, - sessionActor: { - pubkey: actorPubkey, - privateKey: sessionPrivateKey, - }, - ensureSessionActor: async (forceRefresh = false) => { - ensureSessionActorCalls.push(forceRefresh); - if (forceRefresh && !client.sessionActor) { - client.sessionActor = { - pubkey: actorPubkey, - privateKey: sessionPrivateKey, - }; - } - return actorPubkey; - }, - ensureExtensionPermissions: async () => ({ ok: true }), + ensureExtensionPermissions: async () => extensionPermissions, }; - return { client, pool, publishCalls, ensureSessionActorCalls }; + return { client, pool, publishCalls }; } function buildCommentEventTags(event) { @@ -83,8 +68,6 @@ test("publishComment prefers active signer when available", async () => { }, }; - let privateKeySignCalls = 0; - const result = await publishComment( client, { @@ -102,10 +85,6 @@ test("publishComment prefers active signer when available", async () => { { resolveActiveSigner: () => signer, shouldRequestExtensionPermissions: () => true, - signEventWithPrivateKey: () => { - privateKeySignCalls += 1; - return null; - }, DEFAULT_NIP07_PERMISSION_METHODS: ["signEvent"], }, ); @@ -127,7 +106,6 @@ test("publishComment prefers active signer when available", async () => { "publish should target the caller supplied relays", ); - assert.equal(privateKeySignCalls, 0, "session key signing should be skipped when active signer succeeds"); assert.equal(signerCalls.length, 1, "active signer should sign the event once"); const eventTags = buildCommentEventTags(signerCalls[0]); @@ -146,20 +124,12 @@ test("publishComment prefers active signer when available", async () => { ); }); -test("publishComment falls back to session signer when active signer is missing", async () => { - const { - client, - publishCalls, - ensureSessionActorCalls, - } = createMockClient(); +test("publishComment rejects when active signer is unavailable", async () => { + const { client, publishCalls } = createMockClient(); - client.sessionActor = null; - - let privateKeySignCalls = 0; - const signEventWithPrivateKey = (event, key) => { - privateKeySignCalls += 1; - assert.equal(key, "session-private", "session key should be used when rehydrated"); - return { ...event, id: "signed-by-session" }; + client.sessionActor = { + pubkey: client.pubkey, + privateKey: "session-private", }; const result = await publishComment( @@ -172,28 +142,26 @@ test("publishComment falls back to session signer when active signer is missing" { resolveActiveSigner: () => null, shouldRequestExtensionPermissions: () => false, - signEventWithPrivateKey, DEFAULT_NIP07_PERMISSION_METHODS: [], }, ); - assert.equal(result.ok, true, "publish should succeed with session signer"); - assert.equal(privateKeySignCalls, 1, "session signing should occur exactly once"); - assert.equal( - ensureSessionActorCalls.includes(true), - true, - "ensureSessionActor should be invoked with force refresh", - ); - assert.equal(publishCalls.length, 1, "publish should still occur once"); + assert.equal(result.ok, false, "publish should fail without an active signer"); + assert.equal(result.error, "auth-required", "missing signer should yield auth-required"); + assert.equal(publishCalls.length, 0, "publish should not be attempted without a signer"); }); test("publishComment accepts legacy targets with only an event id", async () => { const { client, publishCalls } = createMockClient(); let signCalls = 0; - const signEventWithPrivateKey = (event) => { - signCalls += 1; - return { ...event, id: `signed-${signCalls}` }; + const signer = { + type: "extension", + pubkey: client.pubkey, + signEvent: (event) => { + signCalls += 1; + return { ...event, id: `signed-${signCalls}` }; + }, }; const result = await publishComment( @@ -204,15 +172,14 @@ test("publishComment accepts legacy targets with only an event id", async () => }, { content: "Legacy comment" }, { - resolveActiveSigner: () => null, + resolveActiveSigner: () => signer, shouldRequestExtensionPermissions: () => false, - signEventWithPrivateKey, DEFAULT_NIP07_PERMISSION_METHODS: [], }, ); assert.equal(result.ok, true, "publishing should succeed without a definition address"); - assert.equal(signCalls, 1, "session signer should be used to sign the event"); + assert.equal(signCalls, 1, "active signer should be used to sign the event"); assert.equal(publishCalls.length, 1, "event should be published exactly once"); const publishedEvent = publishCalls[0]?.event; @@ -249,14 +216,19 @@ test("publishComment derives root and parent metadata from parent comment tags", ], }; + const signer = { + type: "extension", + pubkey: client.pubkey, + signEvent: (event) => ({ ...event, id: "signed" }), + }; + const result = await publishComment( client, { parentComment: parentEvent }, { content: "Reply" }, { - resolveActiveSigner: () => null, + resolveActiveSigner: () => signer, shouldRequestExtensionPermissions: () => false, - signEventWithPrivateKey: (event) => ({ ...event, id: "signed" }), DEFAULT_NIP07_PERMISSION_METHODS: [], }, ); @@ -277,7 +249,11 @@ test("publishComment derives root and parent metadata from parent comment tags", test("publishComment emits only video event #e tag when address and parent are absent", async () => { const { client, publishCalls } = createMockClient(); - const signEventWithPrivateKey = (event) => ({ ...event, id: "solo" }); + const signer = { + type: "extension", + pubkey: client.pubkey, + signEvent: (event) => ({ ...event, id: "solo" }), + }; const result = await publishComment( client, @@ -286,9 +262,8 @@ test("publishComment emits only video event #e tag when address and parent are a }, { content: "Solo" }, { - resolveActiveSigner: () => null, + resolveActiveSigner: () => signer, shouldRequestExtensionPermissions: () => false, - signEventWithPrivateKey, DEFAULT_NIP07_PERMISSION_METHODS: [], }, ); diff --git a/tests/video-modal-comments.test.mjs b/tests/video-modal-comments.test.mjs index 58b22ff5..3eb22737 100644 --- a/tests/video-modal-comments.test.mjs +++ b/tests/video-modal-comments.test.mjs @@ -331,6 +331,107 @@ test( }, ); +test( + "VideoModalCommentController prompts login when publish requires authentication", + async () => { + const composerStates = []; + const loginLifecycle = []; + const errorMessages = []; + + const loginModal = { + openModal: ({ triggerElement } = {}) => { + loginLifecycle.push({ type: "openModal", triggerElement }); + return true; + }, + }; + + const controller = new VideoModalCommentController({ + commentThreadService: { + setCallbacks: () => {}, + processIncomingEvent: () => {}, + }, + videoModal: { + setCommentComposerState: (state) => { + composerStates.push({ ...state }); + }, + resetCommentComposer: () => { + throw new Error("composer should not reset when auth is required"); + }, + setCommentStatus: () => {}, + appendComment: () => {}, + }, + auth: { + isLoggedIn: () => true, + initializeLoginModalController: ({ logIfMissing }) => { + loginLifecycle.push({ type: "initialize", logIfMissing }); + }, + getLoginModalController: () => loginModal, + requestLogin: ({ allowAccountSelection }) => { + loginLifecycle.push({ type: "request", allowAccountSelection }); + return Promise.resolve(true); + }, + }, + callbacks: { + showError: (message) => { + errorMessages.push(message); + }, + showStatus: () => {}, + muteAuthor: () => Promise.resolve(), + shouldHideAuthor: () => false, + }, + services: { + publishComment: async () => ({ ok: false, error: "auth-required" }), + }, + utils: { + normalizeHexPubkey: (value) => value?.toLowerCase?.() || value, + }, + }); + + controller.currentVideo = { + id: "video-auth", + pubkey: "AUTHORPK", + enableComments: true, + kind: 30078, + tags: [], + }; + controller.modalCommentState.videoEventId = "video-auth"; + controller.modalCommentState.videoKind = "30078"; + controller.modalCommentState.videoAuthorPubkey = "AUTHORPK"; + + const triggerElement = { id: "comment-trigger" }; + + await controller.handleVideoModalCommentSubmit({ + text: "Needs auth", + triggerElement, + }); + + const lastComposerState = composerStates[composerStates.length - 1]; + assert.deepStrictEqual( + lastComposerState, + { disabled: true, reason: "login-required" }, + "composer should remain disabled while prompting for login", + ); + assert.equal(errorMessages.length, 0, "auth-required should not surface as an error"); + + assert.equal( + loginLifecycle.some((step) => step.type === "initialize"), + true, + "login modal controller should be initialized", + ); + assert.equal( + loginLifecycle.some((step) => step.type === "openModal"), + true, + "login modal should attempt to open", + ); + const modalOpen = loginLifecycle.find((step) => step.type === "openModal"); + assert.equal( + modalOpen?.triggerElement, + triggerElement, + "trigger element should propagate to login modal", + ); + }, +); + test( "VideoModalCommentController includes parent metadata when replying", async () => {