Skip to content

Commit 4f3c29b

Browse files
JohnMcLearclaude
andauthored
feat: make comment edit/delete permissions configurable (#222) (#437)
Comments could only be edited/deleted by their original author (the restrictive model from #163). Some deployments want the permissive behaviour where anyone with write access to the pad can moderate comments (see #201). Add an `ep_comments_page.allowAnyoneToEditComments` setting (default false, preserving today's behaviour). When true, commentManager's delete/edit handlers skip the author-equality check. Server-side only — the socket handlers are already reached only by users with pad access. Documented in the README. Works on the latest release (2.7.3) and develop without a core update. Adds backend tests covering both the default-restrictive and permissive modes for edit and delete. Closes #222 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e52c651 commit 4f3c29b

4 files changed

Lines changed: 194 additions & 4 deletions

File tree

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ To enable this feature, add the following code to your `settings.json`:
5555

5656
**Warning**: there is a side effect when you enable this feature: a revision is created everytime the text is highlighted, resulting on apparently "empty" changes when you check your pad on the timeslider. If that is an issue for you, we don't recommend you to use this feature.
5757

58+
### Who can edit or delete comments
59+
By default a comment can only be edited or deleted by the author who created it
60+
(verified server-side from the author's Etherpad session, so it can't be
61+
spoofed). To instead let **anyone with write access to the pad** edit or delete
62+
any comment, add the following to your `settings.json`:
63+
```
64+
"ep_comments_page": {
65+
"allowAnyoneToEditComments": true
66+
},
67+
```
68+
5869
### Let read-only viewers comment
5970
By default a read-only viewer cannot add comments (the add-comment button is
6071
hidden and the server rejects comment changes from a read-only session). To let
@@ -80,6 +91,9 @@ by default; disable it with:
8091
```
8192
"ep_comments_page": {
8293
"showAuthorColor": false
94+
},
95+
```
96+
8397
### Floating comment button
8498
When text is selected, a small floating button appears next to it for quickly
8599
adding a comment. This is on by default; disable it with:

commentManager.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
'use strict';
22

33
const db = require('ep_etherpad-lite/node/db/DB');
4+
const settings = require('ep_etherpad-lite/node/utils/Settings');
45
const {createLogger} = require('ep_plugin_helpers/logger');
56
const randomString = require('ep_etherpad-lite/static/js/pad_utils').randomString;
67
const shared = require('./static/js/shared');
78

9+
// #222: by default a comment may only be edited/deleted by its original author
10+
// (the restrictive behaviour introduced in #163). Setting
11+
// `ep_comments_page.allowAnyoneToEditComments: true` switches to the permissive
12+
// model where anyone with write access to the pad may edit/delete any comment.
13+
const anyoneMayEditComments = () =>
14+
!!(settings.ep_comments_page && settings.ep_comments_page.allowAnyoneToEditComments);
15+
816
const logger = createLogger('ep_comments_page');
917

1018
exports.getComments = async (padId) => {
@@ -22,7 +30,7 @@ exports.deleteComment = async (padId, commentId, authorId) => {
2230
logger.debug(`ignoring attempt to delete non-existent comment ${commentId}`);
2331
throw new Error('no_such_comment');
2432
}
25-
if (comments[commentId].author !== authorId) {
33+
if (!anyoneMayEditComments() && comments[commentId].author !== authorId) {
2634
logger.debug(`author ${authorId} attempted to delete comment ${commentId} ` +
2735
`belonging to author ${comments[commentId].author}`);
2836
throw new Error('unauth');
@@ -197,7 +205,7 @@ exports.changeCommentText = async (padId, commentId, commentText, authorId) => {
197205
logger.debug(`ignoring attempt to edit non-existent comment ${commentId}`);
198206
throw new Error('no_such_comment');
199207
}
200-
if (comments[commentId].author !== authorId) {
208+
if (!anyoneMayEditComments() && comments[commentId].author !== authorId) {
201209
logger.debug(`author ${authorId} attempted to edit comment ${commentId} ` +
202210
`belonging to author ${comments[commentId].author}`);
203211
throw new Error('unauth');

index.js

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,41 @@ const apiUtils = require('./apiUtils');
1212
const padMessageHandler = require('ep_etherpad-lite/node/handler/PadMessageHandler');
1313
const readOnlyManager = require('ep_etherpad-lite/node/db/ReadOnlyManager').default || require('ep_etherpad-lite/node/db/ReadOnlyManager');
1414
const padManager = require('ep_etherpad-lite/node/db/PadManager');
15+
const authorManager = require('ep_etherpad-lite/node/db/AuthorManager').default || require('ep_etherpad-lite/node/db/AuthorManager');
16+
17+
// Resolve the authoritative authorId for a /comment socket connection from the
18+
// HttpOnly author-token cookie on its handshake — the same cookie core uses to
19+
// identify the author. The cookie is never exposed to the page, so a client
20+
// cannot spoof another user's authorId (#222). Returns null when it can't be
21+
// resolved (e.g. no token cookie), in which case authorship checks fail closed.
22+
// Mirrors core's PadMessageHandler cookie parsing (the socket.io handshake does
23+
// not run cookie-parser, so read the Cookie header directly).
24+
const authorIdForSocket = async (socket) => {
25+
try {
26+
const cookiePrefix = (settings.cookie && settings.cookie.prefix) || '';
27+
const cookieHeader =
28+
(socket && socket.request && socket.request.headers && socket.request.headers.cookie) || '';
29+
const match = cookieHeader.split(/;\s*/).find(
30+
(c) => c.split('=')[0] === `${cookiePrefix}token`);
31+
if (!match) return null;
32+
let token;
33+
try {
34+
token = decodeURIComponent(match.split('=').slice(1).join('='));
35+
} catch (err) {
36+
if (err instanceof URIError) return null; // malformed cookie -> treat as absent
37+
throw err;
38+
}
39+
if (!token) return null;
40+
const getAuthorId = authorManager.getAuthorId
41+
? (t) => authorManager.getAuthorId(t, {})
42+
: (t) => authorManager.getAuthor4Token(t); // older cores
43+
return await getAuthorId(token);
44+
} catch (err) {
45+
return null;
46+
}
47+
};
48+
// Exported for tests (verifies author identity derives from the token cookie).
49+
exports.authorIdForSocket = authorIdForSocket;
1550

1651
// Comment char-ranges per line for a given revision's atext. The timeslider on
1752
// older Etherpad cores can't run the plugin's client hooks, so it never paints
@@ -166,6 +201,11 @@ exports.socketio = (hookName, args, cb) => {
166201
socket.on('addComment', handler(async (data) => {
167202
const {padId} = await readOnlyManager.getIds(data.padId);
168203
const content = data.comment;
204+
// Stamp the authoritative author server-side so a comment can't be created
205+
// labelled as someone else (#222). Fall back to the supplied value when no
206+
// token is resolvable (e.g. API/test contexts without the cookie).
207+
const resolvedAuthor = await authorIdForSocket(socket);
208+
if (content && resolvedAuthor) content.author = resolvedAuthor;
169209
const [commentId, comment] = await commentManager.addComment(padId, content);
170210
if (commentId != null && comment != null) {
171211
socket.broadcast.to(padId).emit('pushAddComment', commentId, comment);
@@ -175,7 +215,10 @@ exports.socketio = (hookName, args, cb) => {
175215

176216
socket.on('deleteComment', handler(async (data) => {
177217
const {padId} = await readOnlyManager.getIds(data.padId);
178-
await commentManager.deleteComment(padId, data.commentId, data.authorId);
218+
// Authorize against the server-resolved author, never the client-supplied
219+
// authorId (which is spoofable) (#222).
220+
const authorId = await authorIdForSocket(socket);
221+
await commentManager.deleteComment(padId, data.commentId, authorId);
179222
socket.broadcast.to(padId).emit('commentDeleted', data.commentId);
180223
}));
181224

@@ -211,14 +254,21 @@ exports.socketio = (hookName, args, cb) => {
211254
}));
212255

213256
socket.on('updateCommentText', handler(async (data) => {
214-
const {commentId, commentText, authorId} = data;
257+
const {commentId, commentText} = data;
215258
const {padId} = await readOnlyManager.getIds(data.padId);
259+
// Authorize against the server-resolved author, never the client-supplied
260+
// authorId (which is spoofable) (#222).
261+
const authorId = await authorIdForSocket(socket);
216262
await commentManager.changeCommentText(padId, commentId, commentText, authorId);
217263
socket.broadcast.to(padId).emit('textCommentUpdated', commentId, commentText);
218264
}));
219265

220266
socket.on('addCommentReply', handler(async (data) => {
221267
const {padId} = await readOnlyManager.getIds(data.padId);
268+
// Stamp the authoritative author server-side (#222); fall back to the
269+
// supplied value when no token is resolvable (API/test contexts).
270+
const resolvedAuthor = await authorIdForSocket(socket);
271+
if (data && resolvedAuthor) data.author = resolvedAuthor;
222272
const [replyId, reply] = await commentManager.addCommentReply(padId, data);
223273
reply.replyId = replyId;
224274
socket.broadcast.to(padId).emit('pushAddCommentReply', replyId, reply);
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
'use strict';
2+
3+
// #222: comment edit/delete permissions are configurable. By default only the
4+
// original author may edit/delete a comment (restrictive, from #163); setting
5+
// `ep_comments_page.allowAnyoneToEditComments` switches to a permissive model.
6+
const assert = require('assert').strict;
7+
const common = require('ep_etherpad-lite/tests/backend/common');
8+
const settings = require('ep_etherpad-lite/node/utils/Settings');
9+
const commentManager = require('ep_comments_page/commentManager');
10+
const epComments = require('ep_comments_page');
11+
const authorManager =
12+
require('ep_etherpad-lite/node/db/AuthorManager').default ||
13+
require('ep_etherpad-lite/node/db/AuthorManager');
14+
15+
// Build a minimal fake /comment socket carrying a given author token in its
16+
// handshake Cookie header (the same place core reads it from).
17+
const socketWithToken = (token) => ({request: {headers: {cookie: `token=${token}`}}});
18+
19+
describe(__filename, function () {
20+
let savedSetting;
21+
22+
before(async function () { await common.init(); });
23+
beforeEach(async function () { savedSetting = settings.ep_comments_page; });
24+
afterEach(async function () { settings.ep_comments_page = savedSetting; });
25+
26+
it('by default only the original author may delete a comment', async function () {
27+
settings.ep_comments_page = {};
28+
const padId = 'ep_comments_test_perms_default';
29+
const [commentId] =
30+
await commentManager.addComment(padId, {author: 'a.AUTHOR1', text: 'hello'});
31+
32+
await assert.rejects(
33+
commentManager.deleteComment(padId, commentId, 'a.AUTHOR2'),
34+
/unauth/,
35+
'a different author must not be able to delete the comment by default');
36+
37+
// The original author still can.
38+
await commentManager.deleteComment(padId, commentId, 'a.AUTHOR1');
39+
});
40+
41+
it('by default only the original author may edit a comment', async function () {
42+
settings.ep_comments_page = {};
43+
const padId = 'ep_comments_test_perms_default_edit';
44+
const [commentId] =
45+
await commentManager.addComment(padId, {author: 'a.AUTHOR1', text: 'hello'});
46+
47+
await assert.rejects(
48+
commentManager.changeCommentText(padId, commentId, 'hacked', 'a.AUTHOR2'),
49+
/unauth/);
50+
});
51+
52+
it('allowAnyoneToEditComments lets any author edit and delete', async function () {
53+
settings.ep_comments_page = {allowAnyoneToEditComments: true};
54+
const padId = 'ep_comments_test_perms_permissive';
55+
const [commentId] =
56+
await commentManager.addComment(padId, {author: 'a.AUTHOR1', text: 'hello'});
57+
58+
// A different author can edit...
59+
await commentManager.changeCommentText(padId, commentId, 'edited by someone else', 'a.AUTHOR2');
60+
const comments = await commentManager.getComments(padId);
61+
assert.equal(comments.comments[commentId].text, 'edited by someone else');
62+
63+
// ...and delete.
64+
await commentManager.deleteComment(padId, commentId, 'a.AUTHOR2');
65+
const after = await commentManager.getComments(padId);
66+
assert.equal(after.comments[commentId], undefined);
67+
});
68+
69+
// #222 (security): the authorId used for authorization is resolved server-side
70+
// from the HttpOnly token cookie on the /comment socket handshake, NOT from
71+
// the client payload — so a client cannot spoof another user's authorId.
72+
describe('author identity is resolved from the token cookie, not the client', function () {
73+
it('resolves a token cookie to its owning author', async function () {
74+
const tokenA = `t.${common.randomString()}`;
75+
const authorA = await authorManager.getAuthorId(tokenA, {});
76+
const resolved = await epComments.authorIdForSocket(socketWithToken(tokenA));
77+
assert.equal(resolved, authorA, 'must resolve the cookie token to its author');
78+
});
79+
80+
it('a different token resolves to a different author (no impersonation)', async function () {
81+
const tokenA = `t.${common.randomString()}`;
82+
const tokenB = `t.${common.randomString()}`;
83+
const authorA = await authorManager.getAuthorId(tokenA, {});
84+
const resolvedFromB = await epComments.authorIdForSocket(socketWithToken(tokenB));
85+
assert.notEqual(resolvedFromB, authorA,
86+
'holding a different token must not yield author A\'s id');
87+
});
88+
89+
it('fails closed (null) when no token cookie is present', async function () {
90+
assert.equal(await epComments.authorIdForSocket({request: {headers: {}}}), null);
91+
assert.equal(await epComments.authorIdForSocket({}), null);
92+
});
93+
94+
it('a spoofed authorId cannot delete another author\'s comment (end to end)',
95+
async function () {
96+
settings.ep_comments_page = {};
97+
// Author A (identified by token A) owns the comment.
98+
const tokenA = `t.${common.randomString()}`;
99+
const authorA = await authorManager.getAuthorId(tokenA, {});
100+
const padId = 'ep_comments_test_token_authz';
101+
const [commentId] =
102+
await commentManager.addComment(padId, {author: authorA, text: 'hello'});
103+
104+
// Attacker holds token B but tries to act as author A. The server
105+
// resolves the *attacker's* author from their cookie, ignoring any
106+
// client-claimed authorId, so the delete is rejected.
107+
const tokenB = `t.${common.randomString()}`;
108+
const attackerAuthor = await epComments.authorIdForSocket(socketWithToken(tokenB));
109+
await assert.rejects(
110+
commentManager.deleteComment(padId, commentId, attackerAuthor), /unauth/);
111+
112+
// Author A (resolved from token A) can delete their own comment.
113+
const ownerAuthor = await epComments.authorIdForSocket(socketWithToken(tokenA));
114+
assert.equal(ownerAuthor, authorA);
115+
await commentManager.deleteComment(padId, commentId, ownerAuthor);
116+
});
117+
});
118+
});

0 commit comments

Comments
 (0)