security: Enable authorize plugins to grant read-only access

temp-remove-safari
Richard Hansen 2020-09-19 15:30:04 -04:00 committed by John McLear
parent 505d67ed1c
commit 180983736d
5 changed files with 41 additions and 5 deletions

View File

@ -292,6 +292,7 @@ You can pass the following values to the provided callback:
downgraded to modify-only if `settings.editOnly` is true.) downgraded to modify-only if `settings.editOnly` is true.)
* `['modify']` will grant access to modify but not create the pad if the * `['modify']` will grant access to modify but not create the pad if the
request is for a pad, otherwise access is simply granted. request is for a pad, otherwise access is simply granted.
* `['readOnly']` will grant read-only access.
* `[false]` will deny access. * `[false]` will deny access.
* `[]` or `undefined` will defer the authorization decision to the next * `[]` or `undefined` will defer the authorization decision to the next
authorization plugin (if any, otherwise deny). authorization plugin (if any, otherwise deny).

View File

@ -39,6 +39,7 @@ var remoteAddress = require("../utils/RemoteAddress").remoteAddress;
const assert = require('assert').strict; const assert = require('assert').strict;
const nodeify = require("nodeify"); const nodeify = require("nodeify");
const { RateLimiterMemory } = require('rate-limiter-flexible'); const { RateLimiterMemory } = require('rate-limiter-flexible');
const webaccess = require('../hooks/express/webaccess');
const rateLimiter = new RateLimiterMemory({ const rateLimiter = new RateLimiterMemory({
points: settings.commitRateLimiting.points, points: settings.commitRateLimiting.points,
@ -224,7 +225,6 @@ exports.handleMessage = async function(client, message)
padId = await readOnlyManager.getPadId(padId); padId = await readOnlyManager.getPadId(padId);
} }
// FIXME: Allow to override readwrite access with readonly
const {session: {user} = {}} = client.client.request; const {session: {user} = {}} = client.client.request;
const {accessStatus, authorID} = const {accessStatus, authorID} =
await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password, user); 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 // Save in sessioninfos that this session belonges to this pad
sessionInfo.padId = padIds.padId; sessionInfo.padId = padIds.padId;
sessionInfo.readOnlyPadId = padIds.readOnlyPadId; 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 // Log creation/(re-)entering of a pad
let ip = remoteAddress[client.id]; let ip = remoteAddress[client.id];
@ -1112,7 +1113,7 @@ async function handleClientReady(client, message, authorID)
"chatHead": pad.chatHead, "chatHead": pad.chatHead,
"numConnectedUsers": roomClients.length, "numConnectedUsers": roomClients.length,
"readOnlyId": padIds.readOnlyPadId, "readOnlyId": padIds.readOnlyPadId,
"readonly": padIds.readonly, "readonly": sessionInfo.readonly,
"serverTimestamp": Date.now(), "serverTimestamp": Date.now(),
"userId": authorID, "userId": authorID,
"abiwordAvailable": settings.abiwordAvailable(), "abiwordAvailable": settings.abiwordAvailable(),

View File

@ -3,6 +3,7 @@ var eejs = require('ep_etherpad-lite/node/eejs');
var toolbar = require("ep_etherpad-lite/node/utils/toolbar"); var toolbar = require("ep_etherpad-lite/node/utils/toolbar");
var hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks'); var hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks');
var settings = require('../../utils/Settings'); var settings = require('../../utils/Settings');
const webaccess = require('./webaccess');
exports.expressCreateServer = function (hook_name, args, cb) { exports.expressCreateServer = function (hook_name, args, cb) {
// expose current stats // expose current stats
@ -42,7 +43,8 @@ exports.expressCreateServer = function (hook_name, args, cb) {
args.app.get('/p/:pad', function(req, res, next) args.app.get('/p/:pad', function(req, res, next)
{ {
// The below might break for pads being rewritten // 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", { hooks.callAll("padInitToolbar", {
toolbar: toolbar, toolbar: toolbar,

View File

@ -1,3 +1,4 @@
const assert = require('assert').strict;
const express = require('express'); const express = require('express');
const log4js = require('log4js'); const log4js = require('log4js');
const httpLogger = log4js.getLogger('http'); const httpLogger = log4js.getLogger('http');
@ -15,6 +16,7 @@ exports.normalizeAuthzLevel = (level) => {
switch (level) { switch (level) {
case true: case true:
return 'create'; return 'create';
case 'readOnly':
case 'modify': case 'modify':
case 'create': case 'create':
return level; return level;
@ -24,6 +26,16 @@ exports.normalizeAuthzLevel = (level) => {
return false; 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. // Exported so that tests can set this to 0 to avoid unnecessary test slowness.
exports.authnFailureDelayMs = 1000; exports.authnFailureDelayMs = 1000;

View File

@ -252,7 +252,7 @@ describe('socket.io access checks', function() {
assert.equal(clientVars.data.readonly, false); assert.equal(clientVars.data.readonly, false);
}); });
it("level='modify' -> can modify", async () => { it("level='modify' -> can modify", async () => {
const pad = await padManager.getPad('pad'); // Create the pad. await padManager.getPad('pad'); // Create the pad.
authorize = () => 'modify'; authorize = () => 'modify';
settings.requireAuthentication = true; settings.requireAuthentication = true;
settings.requireAuthorization = true; settings.requireAuthorization = true;
@ -282,4 +282,24 @@ describe('socket.io access checks', function() {
const message = await handshake(socket, 'pad'); const message = await handshake(socket, 'pad');
assert.equal(message.accessStatus, 'deny'); 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);
});
}); });