Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/node/handler/PadMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,12 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
const hasGetAuthorIdHook = (plugins.hooks.getAuthorId || []).length > 0;
const hasDurableIdentity = hasGetAuthorIdHook && !!(user && user.username);
const canDeleteWithoutToken = settings.allowPadDeletionByAllUsers || hasDurableIdentity;
// Whether this session may delete the pad with no token at all: the creator
// on this device (creator-cookie still present), or any user when the
// instance opted everyone in. Drives the plain "Delete pad" button, which is
// independent of enablePadWideSettings (issue #7959) — deletion is not a
// pad-wide setting and must stay reachable when that section is disabled.
const canDeletePad = isCreator || settings.allowPadDeletionByAllUsers;
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
const padDeletionToken =
isCreator && !canDeleteWithoutToken
? await padDeletionManager.createDeletionTokenIfAbsent(sessionInfo.padId)
Expand All @@ -1346,6 +1352,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
// redundant, so the client labels the action "Delete Pad" instead of
// "Delete with token" (issue #7926). See showDeletionTokenModalIfPresent.
canDeleteWithoutToken,
canDeletePad,
// 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().
Expand Down
7 changes: 7 additions & 0 deletions src/static/js/pad_editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,13 @@ const padeditor = (() => {
$('#delete-pad-with-token').prop(
'hidden', !!(window as any).clientVars?.canDeleteWithoutToken);

// The plain "Delete pad" button is shown whenever this session can delete
// without a token (creator on this device, or allowPadDeletionByAllUsers).
// It is independent of pad-wide settings so it stays reachable when that
// section is disabled (issue #7959).
$('#delete-pad').prop(
'hidden', !(window as any).clientVars?.canDeletePad);

// 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();
Expand Down
14 changes: 9 additions & 5 deletions src/templates/pad.html
Original file line number Diff line number Diff line change
Expand Up @@ -391,13 +391,17 @@ <h2 data-l10n-id="pad.settings.padSettings"></h2>
</p>
<% e.end_block(); %>
</div>
<button class="btn btn-danger" data-l10n-id="pad.settings.deletePad" id="delete-pad">Delete pad</button>
</div><% } %>
</div>
<!-- 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. -->
<!-- Pad deletion is independent of pad-wide settings (issue #7959):
both controls are always rendered and pad_editor.ts shows exactly
the one that applies. #delete-pad (token-less) is shown when
clientVars.canDeletePad — the creator on this device, or any user
when allowPadDeletionByAllUsers is on. The recovery-token
disclosure is shown when clientVars.canDeleteWithoutToken is false
(issue #7926): a creator on a second device, or under
requireAuthentication without a durable cross-device identity. -->
<button class="btn btn-danger" data-l10n-id="pad.settings.deletePad" id="delete-pad" hidden>Delete pad</button>
<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>
Expand Down
44 changes: 44 additions & 0 deletions src/tests/backend/specs/padDeletionUiPlacement.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';

import {MapArrayType} from '../../../node/types/MapType';
import settings from '../../../node/utils/Settings';

const assert = require('assert').strict;
const common = require('../common');

// Regression coverage for issue #7959. The token-less "Delete pad" button
// (#delete-pad) used to be nested inside the `enablePadWideSettings`-gated
// pad-settings section, so disabling pad-wide settings removed the only way to
// delete a pad without a recovery token. Pad deletion is unrelated to pad-wide
// settings, so the button must be rendered regardless of that flag (its
// visibility is then driven at runtime by clientVars.canDeletePad).
describe(__filename, function () {
this.timeout(30000);
let agent: any;
const backup: MapArrayType<any> = {};

before(async function () { agent = await common.init(); });

beforeEach(async function () {
backup.enablePadWideSettings = settings.enablePadWideSettings;
});

afterEach(async function () {
settings.enablePadWideSettings = backup.enablePadWideSettings;
});

const hasDeletePadButton = (html: string): boolean =>
/id="delete-pad"/.test(html);

it('renders the Delete pad button with pad-wide settings enabled', async function () {
settings.enablePadWideSettings = true;
const res = await agent.get('/p/deleteUiPlacementOn').expect(200);
assert.equal(hasDeletePadButton(res.text), true);
});

it('renders the Delete pad button with pad-wide settings disabled (#7959)', async function () {
settings.enablePadWideSettings = false;
const res = await agent.get('/p/deleteUiPlacementOff').expect(200);
assert.equal(hasDeletePadButton(res.text), true);
});
});
45 changes: 45 additions & 0 deletions src/tests/backend/specs/socketio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,9 @@ describe(__filename, function () {
'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);
// The creator can always delete without a token on this device, so the
// plain "Delete pad" button is offered (issue #7959).
assert.equal(cv.data.canDeletePad, true);
});

it('no token (and so no modal) when allowPadDeletionByAllUsers is true', async function () {
Expand All @@ -554,8 +557,48 @@ describe(__filename, function () {
// can already delete the pad without a token in this configuration.
assert.equal(cv.data.padDeletionToken, null);
assert.equal(cv.data.canDeleteWithoutToken, true);
assert.equal(cv.data.canDeletePad, true);
});

it('non-creator gets canDeletePad=false by default, true under allowPadDeletionByAllUsers (#7959)',
async function () {
const supertest = require('supertest');
// The creator (default cookie jar) establishes the pad's rev-0 author.
const resCreator = await agent.get('/p/pad').expect(200);
socket = await common.connect(resCreator);
const cvCreator: any = await common.handshake(socket, 'pad');
assert.equal(cvCreator.data.canDeletePad, true, 'creator can always delete');

// A different browser (separate cookie jar) is NOT the creator, so with
// allowPadDeletionByAllUsers off it must not be offered the token-less
// Delete pad button.
const otherBrowser = supertest(common.baseUrl);
const resOther = await otherBrowser.get('/p/pad').expect(200);
const otherSocket = await common.connect(resOther);
try {
const cvOther: any = await common.handshake(otherSocket, 'pad');
assert.equal(cvOther.data.canDeletePad, false,
'non-creator must not see Delete pad by default');
} finally {
otherSocket.close();
}

// With everyone opted in, the same non-creator CAN delete, so the
// button must be offered — independent of enablePadWideSettings (#7959).
// @ts-ignore - public setting toggled per test
settings.allowPadDeletionByAllUsers = true;
const otherBrowser2 = supertest(common.baseUrl);
const resOther2 = await otherBrowser2.get('/p/pad').expect(200);
const otherSocket2 = await common.connect(resOther2);
try {
const cvOther2: any = await common.handshake(otherSocket2, 'pad');
assert.equal(cvOther2.data.canDeletePad, true,
'allowPadDeletionByAllUsers must offer Delete pad to everyone');
} finally {
otherSocket2.close();
}
});

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
Expand All @@ -567,6 +610,7 @@ describe(__filename, function () {
assert.equal(cv.type, 'CLIENT_VARS');
assert.equal(typeof cv.data.padDeletionToken, 'string');
assert.equal(cv.data.canDeleteWithoutToken, false);
assert.equal(cv.data.canDeletePad, true);
});

it('authenticated creator WITH a getAuthorId hook gets no token (durable identity)',
Expand All @@ -579,6 +623,7 @@ describe(__filename, function () {
assert.equal(cv.type, 'CLIENT_VARS');
assert.equal(cv.data.padDeletionToken, null);
assert.equal(cv.data.canDeleteWithoutToken, true);
assert.equal(cv.data.canDeletePad, true);
});
});

Expand Down
5 changes: 4 additions & 1 deletion src/tests/frontend-new/specs/pad_settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {goToNewPad, goToPad, sendChatMessage, showChat} from "../helper/padHelpe
import {showSettings} from "../helper/settingsHelper";

test.describe('creator-owned pad settings', () => {
test('shows pad settings only to the creator and keeps delete pad there', async ({page, browser}) => {
test('shows pad settings only to the creator; delete pad is creator-gated but separate', async ({page, browser}) => {
const padId = await goToNewPad(page);

const context2 = await browser.newContext();
Expand All @@ -19,6 +19,9 @@ test.describe('creator-owned pad settings', () => {
await expect(page.locator('#pad-settings-section')).toBeVisible();
await expect(page.locator('#delete-pad')).toBeVisible();
await expect(page.locator('#padsettings-enforcecheck')).toBeVisible();
// The delete-pad button is no longer nested inside the pad-wide settings
// section: deletion is independent of enablePadWideSettings (issue #7959).
await expect(page.locator('#pad-settings-section #delete-pad')).toHaveCount(0);

await expect(page2.locator('#user-settings-section > h2')).toHaveText('User Settings');
await expect(page2.locator('#theme-toggle-row')).toBeVisible();
Expand Down
Loading