Skip to content

fix(pad): suppress deletion-token modal when allowPadDeletionByAllUsers is on (#7926) #7929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohnMcLear wants to merge 1 commit into
base: develop
Choose a base branch
from
+53 −4
Open

fix(pad): suppress deletion-token modal when allowPadDeletionByAllUsers is on (#7926) #7929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions src/node/handler/PadMessageHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,9 +1287,15 @@ 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 when requireAuthentication is on: every creator has a
// stable identity so the cookie/identity path is sufficient.
const padDeletionToken = isCreator && !settings.requireAuthentication
// 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.
// - 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.
const padDeletionToken =
isCreator && !settings.requireAuthentication && !settings.allowPadDeletionByAllUsers
? await padDeletionManager.createDeletionTokenIfAbsent(sessionInfo.padId)
: null;
Comment on lines +1297 to 1300

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. delete-pad-with-token still rendered 📎 Requirement gap ≡ Correctness

When allowPadDeletionByAllUsers is true, the UI still includes deletion-token-related
controls/text (the "Delete with token" section) even though the token is unnecessary. This violates
the requirement to fully suppress deletion token UI in this configuration, not just the one-time
modal.
Agent Prompt 
## Issue description
The PR suppresses issuance of `clientVars.padDeletionToken` when `settings.allowPadDeletionByAllUsers` is enabled, which prevents the one-time modal. However, the settings UI still renders the token-related delete controls ("Delete with token" section), which violates the requirement that deletion-token UI should not appear at all when `allowPadDeletionByAllUsers` is `true`.

## Issue Context
The token-related controls are rendered in `pad.html` under a condition that only checks `!settings.requireAuthentication`, so they remain present even when `allowPadDeletionByAllUsers` is enabled.

## Fix Focus Areas
- src/templates/pad.html[390-398]
- src/node/handler/PadMessageHandler.ts[1297-1300]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@JohnMcLear JohnMcLear Jun 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this is a deliberate scoping decision, not an oversight. This PR is intentionally minimal: it fixes the issue's primary complaint (the overwhelming "Save your pad deletion token" modal) with an unconditionally-safe server change.

The remaining deletion-token controls (relabeling the "Delete with token" disclosure to "Delete Pad" when no token is needed) are handled in the stacked follow-up #7930, which adds a canDeleteWithoutToken clientVar and switches the disclosure label accordingly. The maintainer (@JohnMcLear) explicitly asked to split this into two PRs — modal suppression first, the UI/label bits second.

Note we keep the disclosure present rather than removing it: with enablePadWideSettings: false it can be the only delete affordance, so hiding it outright would be unsafe. #7930 relabels instead.


Expand Down
45 changes: 44 additions & 1 deletion src/tests/backend/specs/socketio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe(__filename, function () {
plugins.hooks[hookName] = [];
}
backups.settings = {};
for (const setting of ['editOnly', 'requireAuthentication', 'requireAuthorization', 'users', 'enablePadWideSettings']) {
for (const setting of ['editOnly', 'requireAuthentication', 'requireAuthorization', 'users', 'enablePadWideSettings', 'allowPadDeletionByAllUsers']) {
// @ts-ignore
backups.settings[setting] = settings[setting];
}
Expand Down Expand Up @@ -492,6 +492,49 @@ describe(__filename, function () {
});
});

describe('Pad deletion token issuance (#7926)', function () {
const removeIfExists = async (padId: string) => {
if (await padManager.doesPadExist(padId)) {
const p = await padManager.getPad(padId);
await p.remove();
}
};

beforeEach(async function () {
// @ts-ignore - public setting toggled per test
settings.allowPadDeletionByAllUsers = false;
await removeIfExists('pad');
});
afterEach(async function () {
if (socket) socket.close();
socket = null;
await removeIfExists('pad');
});

it('anonymous creator receives a deletion token by default', async function () {
const res = await agent.get('/p/pad').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',
'creator should get a token so the client can show the save-token modal');
assert.ok(cv.data.padDeletionToken.length >= 32);
});

it('no token (and so no modal) when allowPadDeletionByAllUsers is true', async function () {
// @ts-ignore - public setting
settings.allowPadDeletionByAllUsers = true;
const res = await agent.get('/p/pad').expect(200);
socket = await common.connect(res);
const cv: any = await common.handshake(socket, 'pad');
assert.equal(cv.type, 'CLIENT_VARS');
// A null token means showDeletionTokenModalIfPresent() returns early on the
// 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);
});
});

describe('SocketIORouter.js', function () {
const Module = class {
setSocketIO(io:any) {}
Expand Down
Loading