diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index fb7b706df..57ae1c117 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -288,10 +288,13 @@ set in the settings file or by the authenticate hook. You can pass the following values to the provided callback: * `[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. + request is for a pad, otherwise access is simply granted. Access to a pad will + be downgraded to modify-only if `settings.editOnly` is true or the user's + `canCreate` setting is set to `false`, and downgraded to read-only if the + user's `readOnly` setting 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. Access to a pad will be + downgraded to read-only if the user's `readOnly` setting is `true`. * `['readOnly']` will grant read-only access. * `[false]` will deny access. * `[]` or `undefined` will defer the authorization decision to the next diff --git a/settings.json.docker b/settings.json.docker index 332e06ac4..8bbdedb40 100644 --- a/settings.json.docker +++ b/settings.json.docker @@ -391,10 +391,22 @@ }, /* - * Users for basic authentication. + * User accounts. These accounts are used by: + * - default HTTP basic authentication if no plugin handles authentication + * - some but not all authentication plugins + * - some but not all authorization plugins * - * is_admin = true gives access to /admin. - * If you do not uncomment this, /admin will not be available! + * User properties: + * - password: The user's password. Some authentication plugins will ignore + * this. + * - is_admin: true gives access to /admin. Defaults to false. If you do not + * uncomment this, /admin will not be available! + * - readOnly: If true, this user will not be able to create new pads or + * modify existing pads. Defaults to false. + * - canCreate: If this is true and readOnly is false, this user can create + * new pads. Defaults to true. + * + * Authentication and authorization plugins may define additional properties. * * WARNING: passwords should not be stored in plaintext in this file. * If you want to mitigate this, please install ep_hash_auth and diff --git a/settings.json.template b/settings.json.template index c6b7f394f..880310919 100644 --- a/settings.json.template +++ b/settings.json.template @@ -394,10 +394,22 @@ }, /* - * Users for basic authentication. + * User accounts. These accounts are used by: + * - default HTTP basic authentication if no plugin handles authentication + * - some but not all authentication plugins + * - some but not all authorization plugins * - * is_admin = true gives access to /admin. - * If you do not uncomment this, /admin will not be available! + * User properties: + * - password: The user's password. Some authentication plugins will ignore + * this. + * - is_admin: true gives access to /admin. Defaults to false. If you do not + * uncomment this, /admin will not be available! + * - readOnly: If true, this user will not be able to create new pads or + * modify existing pads. Defaults to false. + * - canCreate: If this is true and readOnly is false, this user can create + * new pads. Defaults to true. + * + * Authentication and authorization plugins may define additional properties. * * WARNING: passwords should not be stored in plaintext in this file. * If you want to mitigate this, please install ep_hash_auth and diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index ff28ff8fe..ad802f327 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -67,8 +67,12 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user authLogger.debug('access denied: authentication is required'); return DENY; } - // Check whether the user is authorized. Note that userSettings.padAuthorizations will still be - // populated even if settings.requireAuthorization is false. + + // Check whether the user is authorized to create the pad if it doesn't exist. + if (userSettings.canCreate != null && !userSettings.canCreate) canCreate = false; + if (userSettings.readOnly) canCreate = false; + // Note: userSettings.padAuthorizations should still be populated even if + // settings.requireAuthorization is false. const padAuthzs = userSettings.padAuthorizations || {}; const level = webaccess.normalizeAuthzLevel(padAuthzs[padID]); if (!level) { diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index a2662a8cc..9cb5f4570 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -30,6 +30,7 @@ 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. + if (user.readOnly) return false; 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. diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js index b22d8cd5a..e89d48725 100644 --- a/tests/backend/specs/socketio.js +++ b/tests/backend/specs/socketio.js @@ -152,154 +152,227 @@ describe('socket.io access checks', function() { await cleanUpPads(); }); - // Normal accesses. - it('!authn anonymous cookie /p/pad -> 200, ok', async function() { - const res = await agent.get('/p/pad').expect(200); - // Should not throw. - socket = await connect(res); - const clientVars = await handshake(socket, 'pad'); - assert.equal(clientVars.type, 'CLIENT_VARS'); - }); - it('!authn !cookie -> ok', async function() { - // Should not throw. - socket = await connect(null); - const clientVars = await handshake(socket, 'pad'); - assert.equal(clientVars.type, 'CLIENT_VARS'); - }); - it('!authn user /p/pad -> 200, ok', async function() { - const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); - // Should not throw. - socket = await connect(res); - const clientVars = await handshake(socket, 'pad'); - assert.equal(clientVars.type, 'CLIENT_VARS'); - }); - it('authn user /p/pad -> 200, ok', async function() { - settings.requireAuthentication = true; - const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); - // Should not throw. - socket = await connect(res); - const clientVars = await handshake(socket, 'pad'); - assert.equal(clientVars.type, 'CLIENT_VARS'); - }); - it('authz user /p/pad -> 200, ok', async function() { - settings.requireAuthentication = true; - settings.requireAuthorization = true; - const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); - // Should not throw. - socket = await connect(res); - const clientVars = await handshake(socket, 'pad'); - assert.equal(clientVars.type, 'CLIENT_VARS'); - }); - it('supports pad names with characters that must be percent-encoded', async function() { - settings.requireAuthentication = true; - // requireAuthorization is set to true here to guarantee that the user's padAuthorizations - // object is populated. Technically this isn't necessary because the user's padAuthorizations is - // currently populated even if requireAuthorization is false, but setting this to true ensures - // the test remains useful if the implementation ever changes. - settings.requireAuthorization = true; - const encodedPadId = encodeURIComponent('päd'); - const res = await agent.get(`/p/${encodedPadId}`).auth('user', 'user-password').expect(200); - // Should not throw. - socket = await connect(res); - const clientVars = await handshake(socket, 'päd'); - assert.equal(clientVars.type, 'CLIENT_VARS'); + describe('Normal accesses', function() { + it('!authn anonymous cookie /p/pad -> 200, ok', async function() { + const res = await agent.get('/p/pad').expect(200); + // Should not throw. + socket = await connect(res); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); + it('!authn !cookie -> ok', async function() { + // Should not throw. + socket = await connect(null); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); + it('!authn user /p/pad -> 200, ok', async function() { + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + // Should not throw. + socket = await connect(res); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); + it('authn user /p/pad -> 200, ok', async function() { + settings.requireAuthentication = true; + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + // Should not throw. + socket = await connect(res); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); + it('authz user /p/pad -> 200, ok', async function() { + settings.requireAuthentication = true; + settings.requireAuthorization = true; + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + // Should not throw. + socket = await connect(res); + const clientVars = await handshake(socket, 'pad'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); + it('supports pad names with characters that must be percent-encoded', async function() { + settings.requireAuthentication = true; + // requireAuthorization is set to true here to guarantee that the user's padAuthorizations + // object is populated. Technically this isn't necessary because the user's padAuthorizations + // is currently populated even if requireAuthorization is false, but setting this to true + // ensures the test remains useful if the implementation ever changes. + settings.requireAuthorization = true; + const encodedPadId = encodeURIComponent('päd'); + const res = await agent.get(`/p/${encodedPadId}`).auth('user', 'user-password').expect(200); + // Should not throw. + socket = await connect(res); + const clientVars = await handshake(socket, 'päd'); + assert.equal(clientVars.type, 'CLIENT_VARS'); + }); }); - // Abnormal access attempts. - it('authn anonymous /p/pad -> 401, error', async function() { - settings.requireAuthentication = true; - const res = await agent.get('/p/pad').expect(401); - // Despite the 401, try to create the pad via a socket.io connection anyway. - await assert.rejects(connect(res), {message: /authentication required/i}); - }); - it('authn !cookie -> error', async function() { - settings.requireAuthentication = true; - await assert.rejects(connect(null), {message: /signed express_sid cookie is required/i}); - }); - it('authorization bypass attempt -> error', async function() { - // Only allowed to access /p/pad. - authorize = (req) => req.path === '/p/pad'; - settings.requireAuthentication = true; - settings.requireAuthorization = true; - // First authenticate and establish a session. - const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); - // Connecting should work because the user successfully authenticated. - socket = await connect(res); - // Accessing /p/other-pad should fail, despite the successful fetch of /p/pad. - const message = await handshake(socket, 'other-pad'); - assert.equal(message.accessStatus, 'deny'); + describe('Abnormal access attempts', function() { + it('authn anonymous /p/pad -> 401, error', async function() { + settings.requireAuthentication = true; + const res = await agent.get('/p/pad').expect(401); + // Despite the 401, try to create the pad via a socket.io connection anyway. + await assert.rejects(connect(res), {message: /authentication required/i}); + }); + it('authn !cookie -> error', async function() { + settings.requireAuthentication = true; + await assert.rejects(connect(null), {message: /signed express_sid cookie is required/i}); + }); + it('authorization bypass attempt -> error', async function() { + // Only allowed to access /p/pad. + authorize = (req) => req.path === '/p/pad'; + settings.requireAuthentication = true; + settings.requireAuthorization = true; + // First authenticate and establish a session. + const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); + // Connecting should work because the user successfully authenticated. + socket = await connect(res); + // Accessing /p/other-pad should fail, despite the successful fetch of /p/pad. + 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); + describe('Authorization levels via authorize hook', function() { + beforeEach(async function() { + settings.requireAuthentication = true; + settings.requireAuthorization = true; + }); + + it("level='create' -> can create", async function() { + authorize = () => 'create'; + 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 function() { + authorize = () => 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 function() { + await padManager.getPad('pad'); // Create the pad. + authorize = () => 'modify'; + 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 function() { + authorize = () => 'create'; + 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 function() { + authorize = () => 'modify'; + 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'); + }); + it("level='readOnly' -> unable to create", async function() { + authorize = () => 'readOnly'; + 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 function() { + await padManager.getPad('pad'); // Create the pad. + authorize = () => 'readOnly'; + 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); + }); }); - 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); + + describe('Authorization levels via user settings', function() { + beforeEach(async function() { + settings.requireAuthentication = true; + }); + + it('user.canCreate = true -> can create and modify', async function() { + settings.users.user.canCreate = 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('user.canCreate = false -> unable to create', async function() { + settings.users.user.canCreate = 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'); + }); + it('user.readOnly = true -> unable to create', async function() { + settings.users.user.readOnly = 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('user.readOnly = true -> unable to modify', async function() { + await padManager.getPad('pad'); // Create the pad. + settings.users.user.readOnly = 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); + }); + it('user.readOnly = false -> can create and modify', async function() { + settings.users.user.readOnly = false; + 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('user.readOnly = true, user.canCreate = true -> unable to create', async function() { + settings.users.user.canCreate = true; + settings.users.user.readOnly = 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' -> can modify", async () => { - 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'); - }); - 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); + + describe('Authorization level interaction between authorize hook and user settings', function() { + beforeEach(async function() { + settings.requireAuthentication = true; + settings.requireAuthorization = true; + }); + + it('authorize hook does not elevate level from user settings', async function() { + settings.users.user.readOnly = true; + authorize = () => 'create'; + 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('user settings does not elevate level from authorize hook', async function() { + settings.users.user.readOnly = false; + settings.users.user.canCreate = true; + authorize = () => 'readOnly'; + 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'); + }); }); });