PadMessageHandler: Allow `handleMessageSecurity` to grant one-time write access

pull/5336/head
Richard Hansen 2021-12-20 17:55:00 -05:00
parent 31b025bd9d
commit 02a56dc58c
5 changed files with 94 additions and 35 deletions

View File

@ -35,6 +35,8 @@
* The `handleMessageSecurity` and `handleMessage` server-side hooks have a new
`sessionInfo` context property that includes the user's author ID, the pad ID,
and whether the user only has read-only access.
* The `handleMessageSecurity` server-side hook can now be used to grant write
access for the current message only.
### Compatibility changes
@ -43,6 +45,8 @@
* The `client` context property for the `handleMessageSecurity` and
`handleMessage` server-side hooks is deprecated; use the `socket` context
property instead.
* Returning `true` from a `handleMessageSecurity` hook function is deprecated;
return `'permitOnce'` instead.
* Changes to the `src/static/js/Changeset.js` library:
* The following attribute processing functions are deprecated (use the new
attribute APIs instead):

View File

@ -616,12 +616,15 @@ temporary write access to a pad.
Supported return values:
* `undefined`: No change in access status.
* `true`: Override the user's read-only access for all `COLLABROOM` messages
from the same socket.io connection (including the current message, if
applicable) until the client's next `CLIENT_READY` message. Has no effect if
the user already has write access to the pad. Read-only access is reset
**after** each `CLIENT_READY` message, so returning `true` has no effect for
`CLIENT_READY` messages.
* `'permitOnce'`: Override the user's read-only access for the current
`COLLABROOM` message only. Has no effect if the current message is not a
`COLLABROOM` message, or if the user already has write access to the pad.
* `true`: (**Deprecated**; return `'permitOnce'` instead.) Override the user's
read-only access for all `COLLABROOM` messages from the same socket.io
connection (including the current message, if applicable) until the client's
next `CLIENT_READY` message. Has no effect if the user already has write
access to the pad. Read-only access is reset **after** each `CLIENT_READY`
message, so returning `true` has no effect for `CLIENT_READY` messages.
Context properties:
@ -639,9 +642,9 @@ Example:
```javascript
exports.handleMessageSecurity = async (hookName, context) => {
const {message, sessionInfo: {readOnly}, socket} = context;
const {message, sessionInfo: {readOnly}} = context;
if (!readOnly || message.type !== 'COLLABROOM') return;
if (shouldGrantWriteAccess(message, socket)) return true;
if (await messageIsBenign(message)) return 'permitOnce';
};
```

View File

@ -276,6 +276,7 @@ exports.handleMessage = async (socket, message) => {
thisSession.author = authorID;
// Allow plugins to bypass the readonly message blocker
let readOnly = thisSession.readonly;
const context = {
message,
sessionInfo: {
@ -291,8 +292,21 @@ exports.handleMessage = async (socket, message) => {
return this.socket;
},
};
if ((await hooks.aCallAll('handleMessageSecurity', context)).some((w) => w === true)) {
for (const res of await hooks.aCallAll('handleMessageSecurity', context)) {
switch (res) {
case true:
padutils.warnDeprecated(
'returning `true` from a `handleMessageSecurity` hook function is deprecated; ' +
'return "permitOnce" instead');
thisSession.readonly = false;
// Fall through:
case 'permitOnce':
readOnly = false;
break;
default:
messageLogger.warn(
'Ignoring unsupported return value from handleMessageSecurity hook function:', res);
}
}
// Call handleMessage hook. If a plugin returns null, the message will be dropped.
@ -312,7 +326,7 @@ exports.handleMessage = async (socket, message) => {
} else if (message.type === 'CHANGESET_REQ') {
await handleChangesetRequest(socket, message);
} else if (message.type === 'COLLABROOM') {
if (thisSession.readonly) {
if (readOnly) {
messageLogger.warn('Dropped message, COLLABROOM for readonly pad');
} else if (message.data.type === 'USER_CHANGES') {
stats.counter('pendingEdits').inc();

View File

@ -172,14 +172,14 @@ exports.connect = async (res = null) => {
* @param {string} padId - Which pad to join.
* @returns The CLIENT_VARS message from the server.
*/
exports.handshake = async (socket, padId) => {
exports.handshake = async (socket, padId, token = 't.12345') => {
logger.debug('sending CLIENT_READY...');
socket.send({
component: 'pad',
type: 'CLIENT_READY',
padId,
sessionID: null,
token: 't.12345',
token,
});
logger.debug('waiting for CLIENT_VARS response...');
const msg = await exports.waitForSocketEvent(socket, 'message');

View File

@ -1,103 +1,141 @@
'use strict';
const AttributePool = require('../../../static/js/AttributePool');
const assert = require('assert').strict;
const common = require('../common');
const padManager = require('../../../node/db/PadManager');
const plugins = require('../../../static/js/pluginfw/plugin_defs');
const readOnlyManager = require('../../../node/db/ReadOnlyManager');
describe(__filename, function () {
let agent;
let pad;
let padId;
let roPadId;
let rev;
let socket;
let roSocket;
const backups = {};
before(async function () {
agent = await common.init();
});
beforeEach(async function () {
backups.hooks = {handleMessageSecurity: plugins.hooks.handleMessageSecurity};
plugins.hooks.handleMessageSecurity = [];
padId = common.randomString();
assert(!await padManager.doesPadExist(padId));
pad = await padManager.getPad(padId, '');
pad = await padManager.getPad(padId, 'dummy text');
await pad.setText('\n'); // Make sure the pad is created.
assert.equal(pad.text(), '\n');
const res = await agent.get(`/p/${padId}`).expect(200);
let res = await agent.get(`/p/${padId}`).expect(200);
socket = await common.connect(res);
const {type, data: clientVars} = await common.handshake(socket, padId);
assert.equal(type, 'CLIENT_VARS');
rev = clientVars.collab_client_vars.rev;
roPadId = await readOnlyManager.getReadOnlyId(padId);
res = await agent.get(`/p/${roPadId}`).expect(200);
roSocket = await common.connect(res);
await common.handshake(roSocket, roPadId, `t.${common.randomString(8)}`);
});
afterEach(async function () {
Object.assign(plugins.hooks, backups.hooks);
if (socket != null) socket.close();
socket = null;
if (roSocket != null) roSocket.close();
roSocket = null;
if (pad != null) await pad.remove();
pad = null;
});
describe('USER_CHANGES', function () {
const sendUserChanges =
async (changeset) => await common.sendUserChanges(socket, {baseRev: rev, changeset});
const assertAccepted = async (wantRev) => {
async (socket, cs) => await common.sendUserChanges(socket, {baseRev: rev, changeset: cs});
const assertAccepted = async (socket, wantRev) => {
await common.waitForAcceptCommit(socket, wantRev);
rev = wantRev;
};
const assertRejected = async () => {
const assertRejected = async (socket) => {
const msg = await common.waitForSocketEvent(socket, 'message');
assert.deepEqual(msg, {disconnect: 'badChangeset'});
};
it('changes are applied', async function () {
await Promise.all([
assertAccepted(rev + 1),
sendUserChanges('Z:1>5+5$hello'),
assertAccepted(socket, rev + 1),
sendUserChanges(socket, 'Z:1>5+5$hello'),
]);
assert.equal(pad.text(), 'hello\n');
});
it('bad changeset is rejected', async function () {
await Promise.all([
assertRejected(),
sendUserChanges('this is not a valid changeset'),
assertRejected(socket),
sendUserChanges(socket, 'this is not a valid changeset'),
]);
});
it('retransmission is accepted, has no effect', async function () {
const cs = 'Z:1>5+5$hello';
await Promise.all([
assertAccepted(rev + 1),
sendUserChanges(cs),
assertAccepted(socket, rev + 1),
sendUserChanges(socket, cs),
]);
--rev;
await Promise.all([
assertAccepted(rev + 1),
sendUserChanges(cs),
assertAccepted(socket, rev + 1),
sendUserChanges(socket, cs),
]);
assert.equal(pad.text(), 'hello\n');
});
it('identity changeset is accepted, has no effect', async function () {
await Promise.all([
assertAccepted(rev + 1),
sendUserChanges('Z:1>5+5$hello'),
assertAccepted(socket, rev + 1),
sendUserChanges(socket, 'Z:1>5+5$hello'),
]);
await Promise.all([
assertAccepted(rev),
sendUserChanges('Z:6>0$'),
assertAccepted(socket, rev),
sendUserChanges(socket, 'Z:6>0$'),
]);
assert.equal(pad.text(), 'hello\n');
});
it('non-identity changeset with no net change is accepted, has no effect', async function () {
await Promise.all([
assertAccepted(rev + 1),
sendUserChanges('Z:1>5+5$hello'),
assertAccepted(socket, rev + 1),
sendUserChanges(socket, 'Z:1>5+5$hello'),
]);
await Promise.all([
assertAccepted(rev),
sendUserChanges('Z:6>0-5+5$hello'),
assertAccepted(socket, rev),
sendUserChanges(socket, 'Z:6>0-5+5$hello'),
]);
assert.equal(pad.text(), 'hello\n');
});
it('handleMessageSecurity can grant one-time write access', async function () {
const cs = 'Z:1>5+5$hello';
// First try to send a change and verify that it was dropped.
await sendUserChanges(roSocket, cs);
// sendUserChanges() waits for message ack, so if the message was accepted then head should
// have already incremented by the time we get here.
assert.equal(pad.head, rev); // Not incremented.
// Now allow the change.
plugins.hooks.handleMessageSecurity.push({hook_fn: () => 'permitOnce'});
await Promise.all([
assertAccepted(roSocket, rev + 1),
sendUserChanges(roSocket, cs),
]);
assert.equal(pad.text(), 'hello\n');
// The next change should be dropped.
plugins.hooks.handleMessageSecurity = [];
await sendUserChanges(roSocket, 'Z:6>6=5+6$ world');
assert.equal(pad.head, rev); // Not incremented.
assert.equal(pad.text(), 'hello\n');
});
});
});