libre-service.eu/pad.libre-service.eu-etherpad
libre-service.eu/pad.libre-service.eu-etherpad
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2025-02-01 03:12:42 +01:00
Code Issues Projects Releases Wiki Activity

webaccess: Skip checks if next is called in preAuthenticate

Browse source
This commit is contained in:
Richard Hansen 2021-12-18 00:55:20 -05:00
parent fc498f0ae6
commit 472eddc821
2 changed files with 18 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/node/hooks/express/webaccess.js
View file

@ -106,18 +106,22 @@ const checkAccess = async (req, res, next) => {
// ///////////////////////////////////////////////////////////////////////////////////////////////
let results;
let skip = false;
const preAuthorizeNext = (...args) => { skip = true; next(...args); };
try {
results = await aCallFirst('preAuthorize', {req, res, next},
results = await aCallFirst('preAuthorize', {req, res, next: preAuthorizeNext},
// This predicate will cause aCallFirst to call the hook functions one at a time until one
// of them returns a non-empty list, with an exception: If the request is for an /admin
// page, truthy entries are filtered out before checking to see whether the list is empty.
// This prevents plugin authors from accidentally granting admin privileges to the general
// public.
(r) => (r != null && r.filter((x) => (!requireAdmin || !x)).length > 0));
(r) => (skip || (r != null && r.filter((x) => (!requireAdmin || !x)).length > 0)));
} catch (err) {
httpLogger.error(`Error in preAuthorize hook: ${err.stack || err.toString()}`);
return res.status(500).send('Internal Server Error');
if (!skip) res.status(500).send('Internal Server Error');
return;
}
if (skip) return;
if (staticPathsRE.test(req.path)) results.push(true);
if (requireAdmin) {
// Filter out all 'true' entries to prevent plugin authors from accidentally granting admin

15
src/tests/backend/specs/webaccess.js
View file

@ -135,7 +135,7 @@ describe(__filename, function () {
assert(!this.called);
this.called = true;
callOrder.push(this.id);
return cb(this.innerHandle(context.req));
return cb(this.innerHandle(context));
}
};
const handlers = {};
@ -179,6 +179,13 @@ describe(__filename, function () {
await agent.get('/').expect(403);
assert.deepEqual(callOrder, ['preAuthorize_0']);
});
it('bypasses authenticate and authorize hooks when next is called', async function () {
settings.requireAuthentication = true;
settings.requireAuthorization = true;
handlers.preAuthorize[0].innerHandle = ({next}) => next();
await agent.get('/').expect(200);
assert.deepEqual(callOrder, ['preAuthorize_0']);
});
it('bypasses authenticate and authorize hooks for static content, defers', async function () {
settings.requireAuthentication = true;
settings.requireAuthorization = true;
@ -251,13 +258,13 @@ describe(__filename, function () {
'authenticate_1']);
});
it('does not defer if return [true], 200', async function () {
handlers.authenticate[0].innerHandle = (req) => { req.session.user = {}; return [true]; };
handlers.authenticate[0].innerHandle = ({req}) => { req.session.user = {}; return [true]; };
await agent.get('/').expect(200);
// Note: authenticate_1 was not called because authenticate_0 handled it.
assert.deepEqual(callOrder, ['preAuthorize_0', 'preAuthorize_1', 'authenticate_0']);
});
it('does not defer if return [false], 401', async function () {
handlers.authenticate[0].innerHandle = (req) => [false];
handlers.authenticate[0].innerHandle = () => [false];
await agent.get('/').expect(401);
// Note: authenticate_1 was not called because authenticate_0 handled it.
assert.deepEqual(callOrder, ['preAuthorize_0', 'preAuthorize_1', 'authenticate_0']);
@ -355,7 +362,7 @@ describe(__filename, function () {
'authorize_0']);
});
it('does not defer if return [false], 403', async function () {
handlers.authorize[0].innerHandle = (req) => [false];
handlers.authorize[0].innerHandle = () => [false];
await agent.get('/').auth('user', 'user-password').expect(403);
// Note: authorize_1 was not called because authorize_0 handled it.
assert.deepEqual(callOrder, ['preAuthorize_0',