From 02757079c0c2d19a612b61a11a054a86faa66ab4 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 11 Sep 2020 19:46:47 -0400 Subject: [PATCH] security: Enable authorize plugins to grant modify-only access --- doc/api/hooks_server-side.md | 10 ++++-- src/node/db/SecurityManager.js | 5 ++- src/node/hooks/express/webaccess.js | 1 + tests/backend/specs/socketio.js | 54 +++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 370e782ed..95ad6c1c9 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -253,8 +253,8 @@ following are true: For pre-authentication invocations of your authorize function, you can pass the following values to the provided callback: -* `[true]` or `['create']` will immediately grant access without requiring the - user to authenticate. +* `[true]`, `['create']`, or `['modify']` will immediately grant access without + requiring the user to authenticate. * `[false]` will trigger authentication unless authentication is not required. * `[]` or `undefined` will defer the decision to the next authorization plugin (if any, otherwise it is the same as calling with `[false]`). @@ -267,7 +267,11 @@ public. For post-authentication invocations of your authorize function, you can pass the following values to the provided callback: -* `[true]` or `['create']` will grant access. +* `[true]` or `['create']` will grant access to modify or create the pad if the + request is for a pad, otherwise access is simply granted. (Access will be + 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. * `[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/db/SecurityManager.js b/src/node/db/SecurityManager.js index 781f47e85..19fc23c26 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -58,6 +58,8 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user return DENY; } + let canCreate = !settings.editOnly; + if (settings.requireAuthentication) { // Make sure the user has authenticated if authentication is required. The caller should have // already performed this check, but it is repeated here just in case. @@ -73,6 +75,7 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user authLogger.debug('access denied: unauthorized'); return DENY; } + if (level !== 'create') canCreate = false; } // allow plugins to deny access @@ -88,7 +91,7 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user const p_padExists = padManager.doesPadExist(padID); const padExists = await p_padExists; - if (!padExists && settings.editOnly) { + if (!padExists && !canCreate) { authLogger.debug('access denied: user attempted to create a pad, which is prohibited'); return DENY; } diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index fd8c935e6..01ba8a926 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -13,6 +13,7 @@ exports.normalizeAuthzLevel = (level) => { switch (level) { case true: return 'create'; + case 'modify': case 'create': return level; default: diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js index b685e1f51..f6cd25a63 100644 --- a/tests/backend/specs/socketio.js +++ b/tests/backend/specs/socketio.js @@ -125,6 +125,7 @@ describe('socket.io access checks', function() { beforeEach(async function() { Object.assign(settingsBackup, settings); assert(socket == null); + settings.editOnly = false; settings.requireAuthentication = false; settings.requireAuthorization = false; settings.users = { @@ -224,4 +225,57 @@ describe('socket.io access checks', function() { const message = await handshake(socket, 'other-pad'); assert.equal(message.accessStatus, 'deny'); }); + + // Authorization levels via authorize hook + it("level='create' -> can create", async () => { + authorize = () => 'create'; + 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, false); + }); + it('level=true -> can create', async () => { + authorize = () => true; + 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, false); + }); + it("level='modify' -> can modify", async () => { + const pad = await padManager.getPad('pad'); // Create the pad. + authorize = () => 'modify'; + 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, false); + }); + it("level='create' settings.editOnly=true -> unable to create", async () => { + authorize = () => 'create'; + settings.requireAuthentication = true; + settings.requireAuthorization = true; + settings.editOnly = 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='modify' settings.editOnly=false -> unable to create", async () => { + authorize = () => 'modify'; + settings.requireAuthentication = true; + settings.requireAuthorization = true; + settings.editOnly = false; + 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'); + }); });