feature: New user-specific readOnly and canCreate settings (#4370)

Also:
  * Group the tests for readability.
  * Factor out some common test setup.
This commit is contained in:
Richard Hansen 2020-09-28 06:22:06 -04:00 committed by GitHub
parent 7bd5435f50
commit bf9d613e95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 260 additions and 155 deletions

View file

@ -288,10 +288,13 @@ set in the settings file or by the authenticate hook.
You can pass the following values to the provided callback: You can pass the following values to the provided callback:
* `[true]` or `['create']` will grant access to modify or create the pad if the * `[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 request is for a pad, otherwise access is simply granted. Access to a pad will
downgraded to modify-only if `settings.editOnly` is true.) be downgraded to modify-only if `settings.editOnly` is true or the user's
* `['modify']` will grant access to modify but not create the pad if the `canCreate` setting is set to `false`, and downgraded to read-only if the
request is for a pad, otherwise access is simply granted. 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. * `['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

View file

@ -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. * User properties:
* If you do not uncomment this, /admin will not be available! * - 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. * WARNING: passwords should not be stored in plaintext in this file.
* If you want to mitigate this, please install ep_hash_auth and * If you want to mitigate this, please install ep_hash_auth and

View file

@ -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. * User properties:
* If you do not uncomment this, /admin will not be available! * - 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. * WARNING: passwords should not be stored in plaintext in this file.
* If you want to mitigate this, please install ep_hash_auth and * If you want to mitigate this, please install ep_hash_auth and

View file

@ -67,8 +67,12 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user
authLogger.debug('access denied: authentication is required'); authLogger.debug('access denied: authentication is required');
return DENY; 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 padAuthzs = userSettings.padAuthorizations || {};
const level = webaccess.normalizeAuthzLevel(padAuthzs[padID]); const level = webaccess.normalizeAuthzLevel(padAuthzs[padID]);
if (!level) { if (!level) {

View file

@ -30,6 +30,7 @@ exports.userCanModify = (padId, req) => {
if (!settings.requireAuthentication) return true; if (!settings.requireAuthentication) return true;
const {session: {user} = {}} = req; const {session: {user} = {}} = req;
assert(user); // If authn required and user == null, the request should have already been denied. 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. assert(user.padAuthorizations); // This is populated even if !settings.requireAuthorization.
const level = exports.normalizeAuthzLevel(user.padAuthorizations[padId]); const level = exports.normalizeAuthzLevel(user.padAuthorizations[padId]);
assert(level); // If !level, the request should have already been denied. assert(level); // If !level, the request should have already been denied.

View file

@ -152,7 +152,7 @@ describe('socket.io access checks', function() {
await cleanUpPads(); await cleanUpPads();
}); });
// Normal accesses. describe('Normal accesses', function() {
it('!authn anonymous cookie /p/pad -> 200, ok', async function() { it('!authn anonymous cookie /p/pad -> 200, ok', async function() {
const res = await agent.get('/p/pad').expect(200); const res = await agent.get('/p/pad').expect(200);
// Should not throw. // Should not throw.
@ -193,9 +193,9 @@ describe('socket.io access checks', function() {
it('supports pad names with characters that must be percent-encoded', async function() { it('supports pad names with characters that must be percent-encoded', async function() {
settings.requireAuthentication = true; settings.requireAuthentication = true;
// requireAuthorization is set to true here to guarantee that the user's padAuthorizations // 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 // object is populated. Technically this isn't necessary because the user's padAuthorizations
// currently populated even if requireAuthorization is false, but setting this to true ensures // is currently populated even if requireAuthorization is false, but setting this to true
// the test remains useful if the implementation ever changes. // ensures the test remains useful if the implementation ever changes.
settings.requireAuthorization = true; settings.requireAuthorization = true;
const encodedPadId = encodeURIComponent('päd'); const encodedPadId = encodeURIComponent('päd');
const res = await agent.get(`/p/${encodedPadId}`).auth('user', 'user-password').expect(200); const res = await agent.get(`/p/${encodedPadId}`).auth('user', 'user-password').expect(200);
@ -204,8 +204,9 @@ describe('socket.io access checks', function() {
const clientVars = await handshake(socket, 'päd'); const clientVars = await handshake(socket, 'päd');
assert.equal(clientVars.type, 'CLIENT_VARS'); assert.equal(clientVars.type, 'CLIENT_VARS');
}); });
});
// Abnormal access attempts. describe('Abnormal access attempts', function() {
it('authn anonymous /p/pad -> 401, error', async function() { it('authn anonymous /p/pad -> 401, error', async function() {
settings.requireAuthentication = true; settings.requireAuthentication = true;
const res = await agent.get('/p/pad').expect(401); const res = await agent.get('/p/pad').expect(401);
@ -229,73 +230,65 @@ describe('socket.io access checks', function() {
const message = await handshake(socket, 'other-pad'); const message = await handshake(socket, 'other-pad');
assert.equal(message.accessStatus, 'deny'); assert.equal(message.accessStatus, 'deny');
}); });
});
// Authorization levels via authorize hook describe('Authorization levels via authorize hook', function() {
it("level='create' -> can create", async () => { beforeEach(async function() {
settings.requireAuthentication = true;
settings.requireAuthorization = true;
});
it("level='create' -> can create", async function() {
authorize = () => 'create'; authorize = () => 'create';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
const clientVars = await handshake(socket, 'pad'); const clientVars = await handshake(socket, 'pad');
assert.equal(clientVars.type, 'CLIENT_VARS'); assert.equal(clientVars.type, 'CLIENT_VARS');
assert.equal(clientVars.data.readonly, false); assert.equal(clientVars.data.readonly, false);
}); });
it('level=true -> can create', async () => { it('level=true -> can create', async function() {
authorize = () => true; authorize = () => true;
settings.requireAuthentication = true;
settings.requireAuthorization = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
const clientVars = await handshake(socket, 'pad'); const clientVars = await handshake(socket, 'pad');
assert.equal(clientVars.type, 'CLIENT_VARS'); assert.equal(clientVars.type, 'CLIENT_VARS');
assert.equal(clientVars.data.readonly, false); assert.equal(clientVars.data.readonly, false);
}); });
it("level='modify' -> can modify", async () => { it("level='modify' -> can modify", async function() {
await padManager.getPad('pad'); // Create the pad. await padManager.getPad('pad'); // Create the pad.
authorize = () => 'modify'; authorize = () => 'modify';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
const clientVars = await handshake(socket, 'pad'); const clientVars = await handshake(socket, 'pad');
assert.equal(clientVars.type, 'CLIENT_VARS'); assert.equal(clientVars.type, 'CLIENT_VARS');
assert.equal(clientVars.data.readonly, false); assert.equal(clientVars.data.readonly, false);
}); });
it("level='create' settings.editOnly=true -> unable to create", async () => { it("level='create' settings.editOnly=true -> unable to create", async function() {
authorize = () => 'create'; authorize = () => 'create';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
settings.editOnly = true; settings.editOnly = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
const message = await handshake(socket, 'pad'); const message = await handshake(socket, 'pad');
assert.equal(message.accessStatus, 'deny'); assert.equal(message.accessStatus, 'deny');
}); });
it("level='modify' settings.editOnly=false -> unable to create", async () => { it("level='modify' settings.editOnly=false -> unable to create", async function() {
authorize = () => 'modify'; authorize = () => 'modify';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
settings.editOnly = false; settings.editOnly = false;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
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 () => { it("level='readOnly' -> unable to create", async function() {
authorize = () => 'readOnly'; authorize = () => 'readOnly';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
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 modify", async () => { it("level='readOnly' -> unable to modify", async function() {
await padManager.getPad('pad'); // Create the pad. await padManager.getPad('pad'); // Create the pad.
authorize = () => 'readOnly'; authorize = () => 'readOnly';
settings.requireAuthentication = true;
settings.requireAuthorization = true;
const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200); const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
socket = await connect(res); socket = await connect(res);
const clientVars = await handshake(socket, 'pad'); const clientVars = await handshake(socket, 'pad');
@ -303,3 +296,83 @@ describe('socket.io access checks', function() {
assert.equal(clientVars.data.readonly, true); assert.equal(clientVars.data.readonly, true);
}); });
}); });
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');
});
});
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');
});
});
});