diff --git a/src/node/db/API.ts b/src/node/db/API.ts index bdc659b05d3..b79975b6c69 100644 --- a/src/node/db/API.ts +++ b/src/node/db/API.ts @@ -551,10 +551,11 @@ exports.createPad = async (padID: string, text: string, authorId = '') => { // create pad await getPadSafe(padID, false, text, authorId); - // When requireAuthentication is on, every creator has a stable identity, so - // the cookie/identity path covers recovery and the one-time token is just - // an extra surface to leak. - const deletionToken = settings.requireAuthentication + // No recovery token when it cannot help: requireAuthentication gives every + // creator a stable identity, and allowPadDeletionByAllUsers lets anyone delete + // the pad with no token at all (issue #7926). Either way the token is just an + // extra surface to leak. + const deletionToken = settings.requireAuthentication || settings.allowPadDeletionByAllUsers ? null : await padDeletionManager.createDeletionTokenIfAbsent(padID); return {deletionToken}; diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index 0bf559dfdd8..ab813d737b1 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -1287,15 +1287,31 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => { // once. Readonly sessions never see it. const isCreator = !sessionInfo.readonly && sessionInfo.author === await pad.getRevisionAuthor(0); - // Skip token issuance — and so the client never shows the "Save your pad - // deletion token" modal (issue #7926) — when the token cannot help: - // - requireAuthentication: every creator already has a stable identity, so - // the cookie/identity path is sufficient. + // The deletion token is a recovery handle for the one class of creator that + // can otherwise lose the ability to delete their pad: a user whose creator + // status lives only in a per-browser author-token cookie. It is pointless — + // and the "Save your pad deletion token" modal only overwhelms users who + // will never need it (issue #7926) — when either of these holds: + // // - allowPadDeletionByAllUsers: anyone can delete the pad with no token at - // all (see handlePadDelete's flagOk branch), so a recovery token is noise - // and the modal only overwhelms users who will never need it. + // all (see handlePadDelete's flagOk branch). + // - the creator has a *durable* identity: authenticated (req.session.user + // with a username) AND the deployment maps that identity to a stable + // authorID via a getAuthorId hook. Only then does `isCreator` + // (author === revision-0 author) survive a cookie clear or a different + // device, so the creator path replaces the token on any device. + // + // Note we deliberately do NOT treat requireAuthentication alone as durable: + // without a getAuthorId hook the authorID still comes from the per-browser + // token cookie (AuthorManager.getAuthorId -> getAuthor4Token), so an + // authenticated user on a second device is NOT the creator and would be + // stranded if we also withheld the token. The getAuthorId hook is the + // documented way (doc/api/hooks_server-side) to pin authorID to username. + const hasGetAuthorIdHook = (plugins.hooks.getAuthorId || []).length > 0; + const hasDurableIdentity = hasGetAuthorIdHook && !!(user && user.username); + const canDeleteWithoutToken = settings.allowPadDeletionByAllUsers || hasDurableIdentity; const padDeletionToken = - isCreator && !settings.requireAuthentication && !settings.allowPadDeletionByAllUsers + isCreator && !canDeleteWithoutToken ? await padDeletionManager.createDeletionTokenIfAbsent(sessionInfo.padId) : null; @@ -1314,6 +1330,11 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => { enablePadWideSettings: settings.enablePadWideSettings, enablePluginPadOptions: settings.enablePluginPadOptions, padDeletionToken, + // Drives the deletion-button label/visibility in pad settings: when the + // user can already delete without a token the recovery-token disclosure is + // redundant, so the client labels the action "Delete Pad" instead of + // "Delete with token" (issue #7926). See showDeletionTokenModalIfPresent. + canDeleteWithoutToken, // Allow-listed copy — settings.privacyBanner could carry extra nested // keys from a hand-edited settings.json; sending those by reference // would leak them to every browser. See getPublicPrivacyBanner(). diff --git a/src/static/js/pad_editor.ts b/src/static/js/pad_editor.ts index 96db7879a28..0382a813a00 100644 --- a/src/static/js/pad_editor.ts +++ b/src/static/js/pad_editor.ts @@ -149,6 +149,16 @@ const padeditor = (() => { } }); + // The recovery-token disclosure (#delete-pad-with-token) is rendered for + // every session because a token may now be issued under requireAuthentication + // too (when no getAuthorId hook pins a durable authorID — issue #7926). + // Show it only when the user actually needs a token: when they can already + // delete without one (everyone may delete, or they have a durable + // authenticated identity) the plain "Delete Pad" button suffices, so hide + // the disclosure and all its token wording entirely. + $('#delete-pad-with-token').prop( + 'hidden', !!(window as any).clientVars?.canDeleteWithoutToken); + // delete pad using a recovery token (second device / no creator cookie) $('#delete-pad-token-submit').on('click', () => { const token = String($('#delete-pad-token-input').val() || '').trim(); diff --git a/src/templates/pad.html b/src/templates/pad.html index 6bbc359c6ef..40c7ad0a943 100644 --- a/src/templates/pad.html +++ b/src/templates/pad.html @@ -387,15 +387,17 @@
<% } %> - <% if (!settings.requireAuthentication) { %> -