feat(pad): suppress deletion token for durable identities + relabel recovery action (#7926) (#7930)

* feat(pad): suppress deletion token for durable identities; relabel recovery action (#7926)

Builds on the allowPadDeletionByAllUsers suppression with the rest of the
ideas discussed on the issue.

Server (handleClientReady):
- A creator's deletion token is now also withheld when they have a *durable*
  identity: authenticated (req.session.user with a username) AND the deployment
  pins that identity to a stable authorID via a getAuthorId hook. Only then does
  the creator path (author === revision-0 author) survive a cookie clear or a
  different device, making the recovery token redundant.
- This deliberately tightens the previous `requireAuthentication => always
  suppress` rule: without a getAuthorId hook the authorID still comes from the
  per-browser token cookie, so an authenticated user on a second device is NOT
  the creator. Withholding the token there would strand them, so they now keep
  getting one. SSO deployments using the documented getAuthorId pattern get the
  clean no-modal experience.
- New `canDeleteWithoutToken` clientVar (allowPadDeletionByAllUsers OR durable
  identity) drives the client label.

Client (pad_editor.ts):
- When canDeleteWithoutToken, the recovery-token disclosure summary is labelled
  plainly "Delete Pad" (reusing the already-translated pad.settings.deletePad
  key) instead of the jargon "Delete with token".

Tests (socketio.ts): anonymous -> token; allowPadDeletionByAllUsers -> none;
authenticated without a getAuthorId hook -> token; authenticated with one ->
none. Verified in a browser for both label/modal outcomes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(pad): render recovery disclosure for all sessions; hide it (not relabel) when no token is needed

Addresses Qodo review on #7930:

1. Token UI fully suppressed when not needed (was: only the summary relabelled).
   When canDeleteWithoutToken (allowPadDeletionByAllUsers or a durable identity),
   pad_editor.ts now hides the whole #delete-pad-with-token disclosure — label,
   token field and submit — so no deletion-token wording remains. With the plain
   "Delete Pad" button present this is also the cleaner UX.

2. Recovery disclosure no longer gated on !requireAuthentication. Because a token
   can now be issued under requireAuthentication when the deployment lacks a
   durable getAuthorId mapping, the template must render the recovery form there
   too — otherwise an authenticated creator gets a token with no UI to enter it
   on another device. It is rendered hidden by default and shown by the client
   only when canDeleteWithoutToken is false.

3. API.createPad aligned: also returns null deletionToken under
   allowPadDeletionByAllUsers, matching the socket/UI path.

Tests: deletePad.ts gains a createPad-under-allowPadDeletionByAllUsers case.
Verified live in Chromium: allowAll -> disclosure hidden, no modal; default
anonymous -> disclosure visible, modal shown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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};

src/node/handler/PadMessageHandler.ts

@@ -1298,15 +1298,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;
 
@@ -1325,6 +1341,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().

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();

src/templates/pad.html

@@ -394,15 +394,17 @@
               <button class="btn btn-danger" data-l10n-id="pad.settings.deletePad" id="delete-pad">Delete pad</button>
             </div><% } %>
           </div>
-          <% if (!settings.requireAuthentication) { %>
-          <details id="delete-pad-with-token">
+          <!-- Visibility is controlled at runtime in pad_editor.ts: hidden when
+               clientVars.canDeleteWithoutToken (issue #7926). Rendered for every
+               session (incl. requireAuthentication) because a recovery token may
+               be issued whenever the creator lacks a durable cross-device identity. -->
+          <details id="delete-pad-with-token" hidden>
             <summary data-l10n-id="pad.deletionToken.deleteWithToken">Delete with token</summary>
             <label for="delete-pad-token-input" data-l10n-id="pad.deletionToken.tokenFieldLabel">Pad deletion token</label>
             <input type="password" id="delete-pad-token-input" autocomplete="off" spellcheck="false">
             <button id="delete-pad-token-submit" type="button" class="btn btn-danger"
                     data-l10n-id="pad.settings.deletePad">Delete pad</button>
           </details>
-          <% } %>
         <h2 data-l10n-id="pad.settings.about">About</h2>
           <span data-l10n-id="pad.settings.poweredBy">Powered by</span>
           <a href="https://etherpad.org" target="_blank" referrerpolicy="no-referrer" rel="noopener">Etherpad</a>

src/tests/backend/specs/api/deletePad.ts

@@ -80,6 +80,17 @@ describe(__filename, function () {
     await callApi('deletePad', {padID: padId});
   });
 
+  it('createPad returns null deletionToken when allowPadDeletionByAllUsers is on', async function () {
+    // Anyone can delete the pad with no token at all, so the recovery token is
+    // pointless — matches the socket/UI path (issue #7926).
+    settings.allowPadDeletionByAllUsers = true;
+    const padId = makeId();
+    const res = await callApi('createPad', {padID: padId});
+    assert.equal(res.body.code, 0, JSON.stringify(res.body));
+    assert.equal(res.body.data.deletionToken, null);
+    await callApi('deletePad', {padID: padId});
+  });
+
   it('JWT admin call (no deletionToken) still works — admins stay trusted', async function () {
     const padId = makeId();
     await callApi('createPad', {padID: padId});

src/tests/backend/specs/socketio.ts

@@ -493,6 +493,8 @@ describe(__filename, function () {
   });
 
   describe('Pad deletion token issuance (#7926)', function () {
+    let getAuthorIdBackup: any;
+
     const removeIfExists = async (padId: string) => {
       if (await padManager.doesPadExist(padId)) {
         const p = await padManager.getPad(padId);
@@ -500,14 +502,32 @@ describe(__filename, function () {
       }
     };
 
+    // A getAuthorId hook that pins authorID to the authenticated username — the
+    // documented way (doc/api/hooks_server-side) to give a user a stable
+    // identity across cookie clears and devices. Its mere presence is what makes
+    // an authenticated session "durable" for token-suppression purposes.
+    const installStableIdentityHook = () => {
+      plugins.hooks.getAuthorId = [{hook_fn: async (hookName: string, context: any) => {
+        const username = context.user && context.user.username;
+        if (!username) return;
+        context.dbKey = `username=${username}`;
+        return '';
+      }}];
+    };
+
     beforeEach(async function () {
       // @ts-ignore - public setting toggled per test
       settings.allowPadDeletionByAllUsers = false;
+      // The outer harness only backs up preAuthorize/authenticate/authorize, so
+      // manage getAuthorId ourselves to avoid leaking it into later specs.
+      getAuthorIdBackup = plugins.hooks.getAuthorId;
+      plugins.hooks.getAuthorId = [];
       await removeIfExists('pad');
     });
     afterEach(async function () {
       if (socket) socket.close();
       socket = null;
+      plugins.hooks.getAuthorId = getAuthorIdBackup;
       await removeIfExists('pad');
     });
 
@@ -519,6 +539,7 @@ describe(__filename, function () {
       assert.equal(typeof cv.data.padDeletionToken, 'string',
           'creator should get a token so the client can show the save-token modal');
       assert.ok(cv.data.padDeletionToken.length >= 32);
+      assert.equal(cv.data.canDeleteWithoutToken, false);
     });
 
     it('no token (and so no modal) when allowPadDeletionByAllUsers is true', async function () {
@@ -532,7 +553,33 @@ describe(__filename, function () {
       // client, so the "Save your pad deletion token" modal never appears. Anyone
       // can already delete the pad without a token in this configuration.
       assert.equal(cv.data.padDeletionToken, null);
+      assert.equal(cv.data.canDeleteWithoutToken, true);
     });
+
+    it('authenticated creator WITHOUT a getAuthorId hook still gets a token', async function () {
+      // requireAuthentication alone is NOT durable: the authorID still comes from
+      // the per-browser token cookie, so this user would be stranded on a second
+      // device if the token were withheld. They must keep getting one.
+      settings.requireAuthentication = true;
+      const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
+      socket = await common.connect(res);
+      const cv: any = await common.handshake(socket, 'pad');
+      assert.equal(cv.type, 'CLIENT_VARS');
+      assert.equal(typeof cv.data.padDeletionToken, 'string');
+      assert.equal(cv.data.canDeleteWithoutToken, false);
+    });
+
+    it('authenticated creator WITH a getAuthorId hook gets no token (durable identity)',
+        async function () {
+          settings.requireAuthentication = true;
+          installStableIdentityHook();
+          const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
+          socket = await common.connect(res);
+          const cv: any = await common.handshake(socket, 'pad');
+          assert.equal(cv.type, 'CLIENT_VARS');
+          assert.equal(cv.data.padDeletionToken, null);
+          assert.equal(cv.data.canDeleteWithoutToken, true);
+        });
   });
 
   describe('SocketIORouter.js', function () {