Add possibility to set "required" RBAC conditions (#10185)

* Add optional property 'required' to rbac conditions

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

* Fix tests, remove object handler support & fix bug (pm.queryFrom)

* Remove required property, handle required conditions at the engine level (raw)

* Update EE snapshots

* Add hasSuperAdminRole util
This commit is contained in:
Jean-Sébastien Herbaux 2021-05-10 11:24:45 +02:00 committed by GitHub
parent 81a9a63cad
commit 934a47eb34
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 305 additions and 148 deletions

View File

@ -48,9 +48,14 @@ describe('Condition Domain', () => {
describe('create', () => {
test('Should register a condition with the minimum amount of information', () => {
const condition = { handler: { foo: 'bar' }, name: 'foo', displayName: 'Foo' };
const handler = jest.fn(() => ({ foo: 'bar' }));
const condition = {
handler,
name: 'foo',
displayName: 'Foo',
};
const expected = {
handler: { foo: 'bar' },
handler,
id: 'application::foo',
displayName: 'Foo',
category: 'default',
@ -62,11 +67,12 @@ describe('Condition Domain', () => {
});
test('Should handle multiple step of transformation', () => {
const handler = jest.fn(() => ({ foo: 'bar' }));
const condition = {
name: 'foo',
plugin: 'bar',
displayName: 'Foo',
handler: { foo: 'bar' },
handler,
invalidAttribute: 'foobar',
};
@ -75,7 +81,7 @@ describe('Condition Domain', () => {
category: 'default',
plugin: 'bar',
displayName: 'Foo',
handler: { foo: 'bar' },
handler,
};
const result = domain.create(condition);

View File

@ -4,7 +4,7 @@ const { pipe, merge, set, pick } = require('lodash/fp');
/**
* The handler of a {@link Condition}
* @typedef {Object | (function(user: Object, options: Object): Object | boolean)} ConditionHandler
* @typedef {(function(user: Object, options: Object): Object | boolean)} ConditionHandler
*/
/**
* Domain representation of a Condition (RBAC)

View File

@ -1,9 +1,40 @@
'use strict';
const { defineAbility } = require('@casl/ability');
const { AbilityBuilder, Ability } = require('@casl/ability');
const { pick } = require('lodash/fp');
const sift = require('sift');
const { buildStrapiQuery } = require('../permission/permissions-manager/query-builers');
const createPermissionsManager = require('../permission/permissions-manager');
const allowedOperations = [
'$or',
'$and',
'$eq',
'$ne',
'$in',
'$nin',
'$lt',
'$lte',
'$gt',
'$gte',
'$exists',
'$elemMatch',
];
const operations = pick(allowedOperations, sift);
const conditionsMatcher = conditions => {
return sift.createQueryTester(conditions, { operations });
};
const defineAbility = register => {
const { can, build } = new AbilityBuilder(Ability);
register(can);
return build({ conditionsMatcher });
};
describe('Permissions Manager', () => {
describe('get Query', () => {
test('It should returns an empty query when no conditions are defined', async () => {
@ -18,14 +49,14 @@ describe('Permissions Manager', () => {
});
test('It should returns a valid query from the ability', () => {
const ability = defineAbility(can => can('read', 'foo', ['bar'], { kai: 'doe' }));
const ability = defineAbility(can => can('read', 'foo', ['bar'], { $and: [{ kai: 'doe' }] }));
const pm = createPermissionsManager({
ability,
action: 'read',
model: 'foo',
});
const expected = { _or: [{ kai: 'doe' }] };
const expected = [{ kai: 'doe' }];
expect(pm.getQuery()).toStrictEqual(expected);
});
@ -149,18 +180,20 @@ describe('Permissions Manager', () => {
});
describe('queryFrom', () => {
const ability = defineAbility(can => can('read', 'article', ['title'], { title: 'foo' }));
const ability = defineAbility(can =>
can('read', 'article', ['title'], { $and: [{ title: 'foo' }] })
);
const pm = createPermissionsManager({
ability,
action: 'read',
model: 'article',
});
const pmQuery = { _or: [{ title: 'foo' }] };
const pmQuery = [{ title: 'foo' }];
test('Create query from simple object', () => {
const query = { _limit: 100 };
const expected = { _limit: 100, _where: [pmQuery] };
const expected = { _limit: 100, _where: pmQuery };
const res = pm.queryFrom(query);
@ -171,7 +204,7 @@ describe('Permissions Manager', () => {
const query = { _limit: 100, _where: [{ a: 'b' }, { c: 'd' }] };
const expected = {
_limit: 100,
_where: [pmQuery, { a: 'b' }, { c: 'd' }],
_where: [{ a: 'b' }, { c: 'd' }, ...pmQuery],
};
const res = pm.queryFrom(query);

View File

@ -102,7 +102,7 @@ describe('Permissions Engine', () => {
plugin: 'test',
name: 'isContainedIn',
category: 'default',
handler: { firstname: { $in: ['Alice', 'Foo'] } },
handler: () => ({ firstname: { $in: ['Alice', 'Foo'] } }),
},
],
};
@ -268,7 +268,7 @@ describe('Permissions Engine', () => {
});
describe('Evaluate', () => {
test('It should register the permission (no conditions / true result)', async () => {
test('It should register the permission (no conditions)', async () => {
const permission = { action: 'read', subject: 'article', properties: { fields: ['title'] } };
const user = getUser('alice');
const registerFn = jest.fn();
@ -278,11 +278,10 @@ describe('Permissions Engine', () => {
expect(registerFn).toHaveBeenCalledWith({
..._.pick(permission, ['action', 'subject']),
fields: permission.properties.fields,
condition: true,
});
});
test('It should register the permission (conditions / true result)', async () => {
test('It should register the permission without a condition (non required true result)', async () => {
const permission = {
action: 'read',
subject: 'article',
@ -297,7 +296,6 @@ describe('Permissions Engine', () => {
const expected = {
..._.omit(permission, ['conditions', 'properties']),
fields: permission.properties.fields,
condition: true,
};
expect(registerFn).toHaveBeenCalledWith(expected);
@ -318,7 +316,7 @@ describe('Permissions Engine', () => {
expect(registerFn).not.toHaveBeenCalled();
});
test('It should register the permission (conditions / object result)', async () => {
test('It should register the permission (non required object result)', async () => {
const permission = {
action: 'read',
subject: 'article',
@ -338,7 +336,13 @@ describe('Permissions Engine', () => {
const expected = {
..._.omit(permission, ['conditions', 'properties']),
fields: permission.properties.fields,
condition: { created_by: user.firstname },
condition: {
$and: [
{
$or: [{ created_by: user.firstname }],
},
],
},
};
expect(registerFn).toHaveBeenCalledWith(expected);
@ -351,18 +355,23 @@ describe('Permissions Engine', () => {
beforeEach(() => {
can = jest.fn();
registerFn = engine.createRegisterFunction(can);
registerFn = engine.createRegisterFunction(can, {}, {});
});
test('It should calls the can function without any condition', () => {
registerFn({ action: 'read', subject: 'article', fields: '*', condition: true });
test('It should calls the can function without any condition', async () => {
await registerFn({ action: 'read', subject: 'article', fields: '*', condition: true });
expect(can).toHaveBeenCalledTimes(1);
expect(can).toHaveBeenCalledWith('read', 'article', '*', undefined);
});
test('It should calls the can function with a condition', () => {
registerFn({ action: 'read', subject: 'article', fields: '*', condition: { created_by: 1 } });
test('It should calls the can function with a condition', async () => {
await registerFn({
action: 'read',
subject: 'article',
fields: '*',
condition: { created_by: 1 },
});
expect(can).toHaveBeenCalledTimes(1);
expect(can).toHaveBeenCalledWith('read', 'article', '*', { created_by: 1 });

View File

@ -0,0 +1,82 @@
'use strict';
const { cloneDeep, has } = require('lodash/fp');
const { hooks } = require('strapi-utils');
const permissionDomain = require('../../domain/permission');
/**
* Create a hook map used by the permission Engine
*/
const createEngineHooks = () => ({
willEvaluatePermission: hooks.createAsyncSeriesHook(),
willRegisterPermission: hooks.createAsyncSeriesHook(),
});
/**
* Create a context from a domain {@link Permission} used by the WillEvaluate hook
* @param {Permission} permission
* @return {{readonly permission: Permission, addCondition(string): this}}
*/
const createWillEvaluateContext = permission => ({
get permission() {
return cloneDeep(permission);
},
addCondition(condition) {
Object.assign(permission, permissionDomain.addCondition(condition, permission));
return this;
},
});
/**
* Create a context from a casl Permission & some options
* @param caslPermission
* @param {object} options
* @param {Permission} options.permission
* @param {object} options.user
*/
const createWillRegisterContext = (caslPermission, { permission, user }) => ({
get permission() {
return cloneDeep(permission);
},
get user() {
return cloneDeep(user);
},
condition: {
and(rawConditionObject) {
if (!caslPermission.condition) {
Object.assign(caslPermission, { condition: { $and: [] } });
}
caslPermission.condition.$and.push(rawConditionObject);
return this;
},
or(rawConditionObject) {
if (!caslPermission.condition) {
Object.assign(caslPermission, { condition: { $and: [] } });
}
const orClause = caslPermission.condition.$and.find(has('$or'));
if (orClause) {
orClause.$or.push(rawConditionObject);
} else {
caslPermission.condition.$and.push({ $or: [rawConditionObject] });
}
return this;
},
},
});
module.exports = {
createEngineHooks,
createWillEvaluateContext,
createWillRegisterContext,
};

View File

@ -4,8 +4,9 @@ const {
curry,
map,
filter,
each,
propEq,
isFunction,
isBoolean,
isArray,
isEmpty,
isObject,
@ -17,12 +18,17 @@ const {
} = require('lodash/fp');
const { AbilityBuilder, Ability } = require('@casl/ability');
const sift = require('sift');
const { hooks } = require('strapi-utils');
const permissionDomain = require('../../domain/permission/index');
const { getService } = require('../../utils');
const {
createEngineHooks,
createWillEvaluateContext,
createWillRegisterContext,
} = require('./engine-hooks');
const allowedOperations = [
'$or',
'$and',
'$eq',
'$ne',
'$in',
@ -40,23 +46,9 @@ const conditionsMatcher = conditions => {
return sift.createQueryTester(conditions, { operations });
};
const createBoundAbstractPermissionDomain = permission => ({
get permission() {
return cloneDeep(permission);
},
addCondition(condition) {
Object.assign(permission, permissionDomain.addCondition(condition, permission));
return this;
},
});
module.exports = conditionProvider => {
const state = {
hooks: {
willEvaluatePermission: hooks.createAsyncSeriesHook(),
},
hooks: createEngineHooks(),
};
return {
@ -83,9 +75,10 @@ module.exports = conditionProvider => {
generateAbilityCreatorFor(user) {
return async (permissions, options) => {
const { can, build } = new AbilityBuilder(Ability);
const registerFn = this.createRegisterFunction(can);
for (const permission of permissions) {
const registerFn = this.createRegisterFunction(can, permission, user);
await this.evaluate({ permission, user, options, registerFn });
}
@ -138,7 +131,7 @@ module.exports = conditionProvider => {
* @returns {Promise<void>}
*/
async applyPermissionProcessors(permission) {
const context = createBoundAbstractPermissionDomain(permission);
const context = createWillEvaluateContext(permission);
// 1. Trigger willEvaluatePermission hook and await transformation operated on the permission
await state.hooks.willEvaluatePermission.call(context);
@ -171,7 +164,7 @@ module.exports = conditionProvider => {
// Register the permission if there is no condition
if (isEmpty(conditions)) {
return registerFn({ action, subject, fields: properties.fields, condition: true });
return registerFn({ action, subject, fields: properties.fields });
}
/** Set of functions used to resolve + evaluate conditions & register the permission if allowed */
@ -179,58 +172,90 @@ module.exports = conditionProvider => {
// 1. Replace each condition name by its associated value
const resolveConditions = map(conditionProvider.get);
// 2. Only keep the handler of each condition
const pickHandlers = map(prop('handler'));
// 2. Filter conditions, only keep those whose handler is a function
const filterValidConditions = filter(condition => isFunction(condition.handler));
// 3. Filter conditions, only keep objects and functions
const filterValidConditions = filter(isObject);
// 4. Evaluate the conditions if they're a function, returns the object otherwise
// 3. Evaluate the conditions handler and returns an object
// containing both the original condition and its result
const evaluateConditions = conditions => {
return Promise.all(
conditions.map(cond =>
isFunction(cond)
? cond(user, merge(conditionOptions, { permission: cloneDeep(permission) }))
: cond
)
conditions.map(async condition => ({
condition,
result: await condition.handler(
user,
merge(conditionOptions, { permission: cloneDeep(permission) })
),
}))
);
};
// 5. Only keeps 'true' booleans or objects as condition's result
const filterValidResults = filter(result => result === true || isObject(result));
// 6. Transform each result into registerFn options
const transformToRegisterOptions = map(result => ({
action,
subject,
fields: properties.fields,
condition: result,
}));
// 7. Register each result using the registerFn
const registerResults = each(registerFn);
// 4. Only keeps booleans or objects as condition's result
const filterValidResults = filter(({ result }) => isBoolean(result) || isObject(result));
/**/
// Execute all the steps needed to register the permission with its associated conditions
await Promise.resolve(conditions)
const evaluatedConditions = await Promise.resolve(conditions)
.then(resolveConditions)
.then(pickHandlers)
.then(filterValidConditions)
.then(evaluateConditions)
.then(filterValidResults)
.then(transformToRegisterOptions)
.then(registerResults);
.then(filterValidResults);
// Utils
const resultPropEq = propEq('result');
const pickResults = map(prop('result'));
if (evaluatedConditions.every(resultPropEq(false))) {
return;
}
// If there is no condition or if one of them return true, register the permission as is
if (isEmpty(evaluatedConditions) || evaluatedConditions.some(resultPropEq(true))) {
return registerFn({ action, subject, fields: properties.fields });
}
const results = pickResults(evaluatedConditions).filter(isObject);
if (isEmpty(results)) {
return registerFn({ action, subject, fields: properties.fields });
}
// Register the permission
return registerFn({
action,
subject,
fields: properties.fields,
condition: { $and: [{ $or: results }] },
});
},
/**
* Encapsulate a register function with custom params to fit `evaluatePermission`'s syntax
* @param can
* @returns {function({action?: *, subject?: *, fields?: *, condition?: *}): *}
* @param {Permission} permission
* @param {object} user
* @returns {function}
*/
createRegisterFunction(can) {
return ({ action, subject, fields, condition }) => {
return can(action, subject, fields, isObject(condition) ? condition : undefined);
createRegisterFunction(can, permission, user) {
const registerToCasl = caslPermission => {
const { action, subject, fields, condition } = caslPermission;
can(action, subject, fields, isObject(condition) ? condition : undefined);
};
const runWillRegisterHook = async caslPermission => {
const hookContext = createWillRegisterContext(caslPermission, {
permission,
user,
});
await state.hooks.willRegisterPermission.call(hookContext);
return caslPermission;
};
return async caslPermission => {
await runWillRegisterHook(caslPermission);
registerToCasl(caslPermission);
};
},

View File

@ -1,6 +1,7 @@
'use strict';
const _ = require('lodash');
const { cloneDeep, isObject, set, isArray } = require('lodash/fp');
const { subject: asSubject } = require('@casl/ability');
const { permittedFieldsOf } = require('@casl/ability/extra');
const {
@ -36,10 +37,19 @@ module.exports = ({ ability, action, model }) => ({
queryFrom(query = {}, action) {
const permissionQuery = this.getQuery(action);
return {
...query,
_where: query._where ? _.concat(permissionQuery, query._where) : [permissionQuery],
};
const newQuery = cloneDeep(query);
const { _where } = query;
if (isObject(_where) && !isArray(_where)) {
Object.assign(newQuery, { _where: [_where] });
}
if (!_where) {
Object.assign(newQuery, { _where: [] });
}
return set('_where', newQuery._where.concat(permissionQuery), newQuery);
},
sanitize(data, options = {}) {

View File

@ -6,13 +6,13 @@ const { VALID_REST_OPERATORS } = require('strapi-utils');
const ops = {
common: VALID_REST_OPERATORS.map(op => `$${op}`),
boolean: ['$or'],
boolean: ['$or', '$and'],
cleanable: ['$elemMatch'],
};
const buildCaslQuery = (ability, action, model) => {
const query = rulesToQuery(ability, action, model, o => o.conditions);
return query && _.has(query, '$or') ? _.pick(query, '$or') : {};
return _.get(query, '$or[0].$and', {});
};
const buildStrapiQuery = caslQuery => {

View File

@ -430,6 +430,17 @@ const resetSuperAdminPermissions = async () => {
await assignPermissions(superAdminRole.id, transformedPermissions);
};
/**
* Check if a user object includes the super admin role
* @param {object} user
* @return {boolean}
*/
const hasSuperAdminRole = user => {
const roles = _.get(user, 'roles', []);
return roles.map(prop('code')).includes(SUPER_ADMIN_CODE);
};
module.exports = {
hooks,
sanitizeRole,
@ -448,6 +459,7 @@ module.exports = {
createRolesIfNoneExist,
displayWarningIfNoSuperAdmin,
addPermissions,
hasSuperAdminRole,
assignPermissions,
resetSuperAdminPermissions,
checkRolesIdForDeletion,

View File

@ -39,11 +39,6 @@ describe('Role CRUD End to End', () => {
expect(sortedData).toMatchInlineSnapshot(`
Object {
"conditions": Array [
Object {
"category": "default",
"displayName": "Has Locale Access",
"id": "plugins::i18n.has-locale-access",
},
Object {
"category": "default",
"displayName": "Is creator",
@ -493,11 +488,6 @@ describe('Role CRUD End to End', () => {
expect(sortedData).toMatchInlineSnapshot(`
Object {
"conditions": Array [
Object {
"category": "default",
"displayName": "Has Locale Access",
"id": "plugins::i18n.has-locale-access",
},
Object {
"category": "default",
"displayName": "Is creator",

View File

@ -5,7 +5,7 @@ const { keys, each, prop, isEmpty } = require('lodash/fp');
const { singular } = require('pluralize');
const { toQueries, runPopulateQueries } = require('./utils/populate-queries');
const BOOLEAN_OPERATORS = ['or'];
const BOOLEAN_OPERATORS = ['or', 'and'];
/**
* Build filters on a bookshelf query
@ -296,7 +296,7 @@ const buildJoinsAndFilter = (qb, model, filters) => {
* @param {Object} options.value - Filter value
*/
const buildWhereClause = ({ qb, field, operator, value }) => {
if (Array.isArray(value) && !['or', 'in', 'nin'].includes(operator)) {
if (Array.isArray(value) && !['and', 'or', 'in', 'nin'].includes(operator)) {
return qb.where(subQb => {
for (let val of value) {
subQb.orWhere(q => buildWhereClause({ qb: q, field, operator, value: val }));
@ -305,6 +305,20 @@ const buildWhereClause = ({ qb, field, operator, value }) => {
}
switch (operator) {
case 'and':
return qb.where(andQb => {
value.forEach(andClause => {
andQb.where(subQb => {
if (Array.isArray(andClause)) {
andClause.forEach(clause =>
subQb.where(andQb => buildWhereClause({ qb: andQb, ...clause }))
);
} else {
buildWhereClause({ qb: subQb, ...andClause });
}
});
});
});
case 'or':
return qb.where(orQb => {
value.forEach(orClause => {

View File

@ -6,7 +6,7 @@ module.exports = async () => {
const { sendDidInitializeEvent } = getService('metrics');
const { decorator } = getService('entity-service-decorator');
const { initDefaultLocale } = getService('locales');
const { sectionsBuilder, actions, conditions, engine } = getService('permissions');
const { sectionsBuilder, actions, engine } = getService('permissions');
// Entity Service
strapi.entityService.decorate(decorator);
@ -22,9 +22,6 @@ module.exports = async () => {
actions.registerI18nActionsHooks();
actions.updateActionsProperties();
// Conditions
await conditions.registerI18nConditions();
// Engine/Permissions
engine.registerI18nPermissionsHandlers();

View File

@ -1,13 +1,11 @@
'use strict';
const i18nActionsService = require('./permissions/actions');
const i18nConditionsService = require('./permissions/conditions');
const sectionsBuilderService = require('./permissions/sections-builder');
const engineService = require('./permissions/engine');
module.exports = {
actions: i18nActionsService,
conditions: i18nConditionsService,
sectionsBuilder: sectionsBuilderService,
engine: engineService,
};

View File

@ -1,36 +0,0 @@
'use strict';
const conditions = [
{
displayName: 'Has Locale Access',
name: 'has-locale-access',
plugin: 'i18n',
handler: (user, options) => {
const { locales } = options.permission.properties || {};
const { superAdminCode } = strapi.admin.services.role.constants;
const isSuperAdmin = user.roles.some(role => role.code === superAdminCode);
if (isSuperAdmin) {
return true;
}
return {
locale: {
$in: locales || [],
},
};
},
},
];
const registerI18nConditions = async () => {
const { conditionProvider } = strapi.admin.services.permission;
await conditionProvider.registerMany(conditions);
};
module.exports = {
conditions,
registerI18nConditions,
};

View File

@ -2,16 +2,29 @@
const { getService } = require('../../utils');
/**
* @typedef {object} WillRegisterPermissionContext
* @property {Permission} permission
* @property {object} user
* @property {object} condition
*/
/**
* Locales property handler for the permission engine
* Add the has-locale-access condition if the locales property is defined
* @param {Permission} permission
* @param {function(string)} addCondition
* @param {WillRegisterPermissionContext} context
*/
const willEvaluatePermissionHandler = ({ permission, addCondition }) => {
const willRegisterPermission = context => {
const { permission, condition, user } = context;
const { subject, properties } = permission;
const { locales } = properties || {};
const isSuperAdmin = strapi.admin.services.role.hasSuperAdminRole(user);
if (isSuperAdmin) {
return;
}
const { locales } = properties || {};
const { isLocalizedContentType } = getService('content-types');
// If there is no subject defined, ignore the permission
@ -31,16 +44,20 @@ const willEvaluatePermissionHandler = ({ permission, addCondition }) => {
return;
}
addCondition('plugins::i18n.has-locale-access');
condition.and({
locale: {
$in: locales || [],
},
});
};
const registerI18nPermissionsHandlers = () => {
const { engine } = strapi.admin.services.permission;
engine.hooks.willEvaluatePermission.register(willEvaluatePermissionHandler);
engine.hooks.willRegisterPermission.register(willRegisterPermission);
};
module.exports = {
willEvaluatePermissionHandler,
willRegisterPermission,
registerI18nPermissionsHandlers,
};

View File

@ -90,7 +90,7 @@ const normalizeFieldName = ({ model, field }) => {
: fieldPath.join('.');
};
const BOOLEAN_OPERATORS = ['or'];
const BOOLEAN_OPERATORS = ['or', 'and'];
const hasDeepFilters = ({ where = [], sort = [] }, { minDepth = 1 } = {}) => {
// A query uses deep filtering if some of the clauses contains a sort or a match expression on a field of a relation

View File

@ -10,8 +10,8 @@ const {
constants: { DP_PUB_STATES },
} = require('./content-types');
const BOOLEAN_OPERATORS = ['or'];
const QUERY_OPERATORS = ['_where', '_or'];
const BOOLEAN_OPERATORS = ['or', 'and'];
const QUERY_OPERATORS = ['_where', '_or', '_and'];
/**
* Global converter