diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 021f633f3..fb7b706df 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -292,6 +292,7 @@ You can pass the following values to the provided callback: downgraded to modify-only if `settings.editOnly` is true.) * `['modify']` will grant access to modify but not create the pad if the request is for a pad, otherwise access is simply granted. +* `['readOnly']` will grant read-only access. * `[false]` will deny access. * `[]` or `undefined` will defer the authorization decision to the next authorization plugin (if any, otherwise deny). diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 56e506023..1094fe837 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -39,6 +39,7 @@ var remoteAddress = require("../utils/RemoteAddress").remoteAddress; const assert = require('assert').strict; const nodeify = require("nodeify"); const { RateLimiterMemory } = require('rate-limiter-flexible'); +const webaccess = require('../hooks/express/webaccess'); const rateLimiter = new RateLimiterMemory({ points: settings.commitRateLimiting.points, @@ -224,7 +225,6 @@ exports.handleMessage = async function(client, message) padId = await readOnlyManager.getPadId(padId); } - // FIXME: Allow to override readwrite access with readonly const {session: {user} = {}} = client.client.request; const {accessStatus, authorID} = await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password, user); @@ -973,7 +973,8 @@ async function handleClientReady(client, message, authorID) // Save in sessioninfos that this session belonges to this pad sessionInfo.padId = padIds.padId; sessionInfo.readOnlyPadId = padIds.readOnlyPadId; - sessionInfo.readonly = padIds.readonly; + sessionInfo.readonly = + padIds.readonly || !webaccess.userCanModify(message.padId, client.client.request); // Log creation/(re-)entering of a pad let ip = remoteAddress[client.id]; @@ -1112,7 +1113,7 @@ async function handleClientReady(client, message, authorID) "chatHead": pad.chatHead, "numConnectedUsers": roomClients.length, "readOnlyId": padIds.readOnlyPadId, - "readonly": padIds.readonly, + "readonly": sessionInfo.readonly, "serverTimestamp": Date.now(), "userId": authorID, "abiwordAvailable": settings.abiwordAvailable(), diff --git a/src/node/hooks/express/specialpages.js b/src/node/hooks/express/specialpages.js index b11f77a00..2b5ea231c 100644 --- a/src/node/hooks/express/specialpages.js +++ b/src/node/hooks/express/specialpages.js @@ -3,6 +3,7 @@ var eejs = require('ep_etherpad-lite/node/eejs'); var toolbar = require("ep_etherpad-lite/node/utils/toolbar"); var hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks'); var settings = require('../../utils/Settings'); +const webaccess = require('./webaccess'); exports.expressCreateServer = function (hook_name, args, cb) { // expose current stats @@ -42,7 +43,8 @@ exports.expressCreateServer = function (hook_name, args, cb) { args.app.get('/p/:pad', function(req, res, next) { // The below might break for pads being rewritten - var isReadOnly = req.url.indexOf("/p/r.") === 0; + const isReadOnly = + req.url.indexOf("/p/r.") === 0 || !webaccess.userCanModify(req.params.pad, req); hooks.callAll("padInitToolbar", { toolbar: toolbar, diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 0f3a01ee7..2be42ceab 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -1,3 +1,4 @@ +const assert = require('assert').strict; const express = require('express'); const log4js = require('log4js'); const httpLogger = log4js.getLogger('http'); @@ -15,6 +16,7 @@ exports.normalizeAuthzLevel = (level) => { switch (level) { case true: return 'create'; + case 'readOnly': case 'modify': case 'create': return level; @@ -24,6 +26,16 @@ exports.normalizeAuthzLevel = (level) => { return false; }; +exports.userCanModify = (padId, req) => { + if (!settings.requireAuthentication) return true; + const {session: {user} = {}} = req; + assert(user); // If authn required and user == null, the request should have already been denied. + assert(user.padAuthorizations); // This is populated even if !settings.requireAuthorization. + const level = exports.normalizeAuthzLevel(user.padAuthorizations[padId]); + assert(level); // If !level, the request should have already been denied. + return level !== 'readOnly'; +}; + // Exported so that tests can set this to 0 to avoid unnecessary test slowness. exports.authnFailureDelayMs = 1000; diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js index 717c4f7b8..b22d8cd5a 100644 --- a/tests/backend/specs/socketio.js +++ b/tests/backend/specs/socketio.js @@ -252,7 +252,7 @@ describe('socket.io access checks', function() { assert.equal(clientVars.data.readonly, false); }); it("level='modify' -> can modify", async () => { - const pad = await padManager.getPad('pad'); // Create the pad. + await padManager.getPad('pad'); // Create the pad. authorize = () => 'modify'; settings.requireAuthentication = true; settings.requireAuthorization = true; @@ -282,4 +282,24 @@ describe('socket.io access checks', function() { const message = await handshake(socket, 'pad'); assert.equal(message.accessStatus, 'deny'); }); + it("level='readOnly' -> unable to create", async () => { + authorize = () => 'readOnly'; + settings.requireAuthentication = true; + settings.requireAuthorization = true; + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + socket = await connect(res); + const message = await handshake(socket, 'pad'); + assert.equal(message.accessStatus, 'deny'); + }); + it("level='readOnly' -> unable to modify", async () => { + await padManager.getPad('pad'); // Create the pad. + authorize = () => 'readOnly'; + settings.requireAuthentication = true; + settings.requireAuthorization = true; + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + socket = await connect(res); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + assert.equal(clientVars.data.readonly, true); + }); });