rename permissions to actions, inverted params of provider.get, separated formatter, add possibility to not specigy pluginName for ::application

Signed-off-by: Pierre Noël <petersg83@gmail.com>
This commit is contained in:
Pierre Noël 2020-06-08 11:01:20 +02:00 committed by Alexandre Bodin
parent 707746ef45
commit e078c0b022
20 changed files with 330 additions and 329 deletions

View File

@ -1,5 +1,5 @@
module.exports = {
permissions: [
actions: [
{
uid: 'marketplace.read',
displayName: 'Can access to the marketplace',

View File

@ -1,6 +1,6 @@
const adminPermissions = require('../admin-permissions');
const adminActions = require('../admin-actions');
module.exports = async () => {
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(adminPermissions.permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(adminActions.actions);
};

View File

@ -0,0 +1,30 @@
const formatActionsBySections = actions =>
actions.reduce((result, p) => {
const checkboxItem = {
displayName: p.displayName,
action: p.actionId,
};
switch (p.section) {
case 'contentTypes':
checkboxItem.subjects = p.subjects;
break;
case 'plugins':
checkboxItem.subCategory = p.subCategory;
checkboxItem.plugin = `plugin::${p.pluginName}`;
break;
case 'settings':
checkboxItem.category = p.category;
checkboxItem.subCategory = p.subCategory;
break;
case 'default':
throw new Error(`Unknown section ${p.section}`);
}
result[p.section] = result[p.section] || [];
result[p.section].push(checkboxItem);
return result;
}, {});
module.exports = formatActionsBySections;

View File

@ -0,0 +1,5 @@
const formatActionsBySections = require('./formatActionsBySections');
module.exports = {
formatActionsBySections,
};

View File

@ -1,17 +1,20 @@
'use strict';
const { formatActionsBySections } = require('./formatters');
module.exports = {
/**
* Returns every permissions, in nested format
* @param {KoaContext} ctx - koa context
*/
async getAll(ctx) {
const allWithNestedFormat = strapi.admin.services[
'permission-provider'
].getAllWithNestedFormat();
const allActions = strapi.admin.services.permission.provider.getAll();
ctx.body = {
data: allWithNestedFormat,
data: {
conditions: [],
sections: formatActionsBySections(allActions),
},
};
},
};

View File

@ -0,0 +1,52 @@
'use strict';
const _ = require('lodash');
const actionFields = [
'section',
'displayName',
'category',
'subCategory',
'pluginName',
'subjects',
'conditions',
];
/**
* Return a prefixed id that depends on the pluginName
* @param {Object} params
* @param {Object} params.pluginName - pluginName on which the action is related
* @param {Object} params.uid - uid defined by the developer
*/
const getActionId = ({ pluginName, uid }) => {
let id = '';
if (pluginName === 'admin') {
id = `admin::${uid}`;
} else if (_.isNil(pluginName)) {
id = `plugins::application.${uid}`;
} else {
id = `plugins::${pluginName}.${uid}`;
}
return id;
};
/**
* Create a permission action
* @param {Object} attributes - action attributes
*/
function createAction(attributes) {
const action = _.cloneDeep(_.pick(attributes, actionFields));
action.actionId = getActionId(attributes);
action.conditions = action.conditions || [];
if (['settings', 'plugins'].includes(attributes.section)) {
action.subCategory = attributes.subCategory || 'general';
}
return action;
}
module.exports = {
getActionId,
createAction,
};

View File

@ -0,0 +1,147 @@
'use strict';
const _ = require('lodash');
const actionProviderService = require('../action-provider');
describe('Action Provider Service', () => {
const createdActions = [];
beforeEach(() => {
global.strapi = {
plugins: { aPlugin: {} },
};
});
describe('settings', () => {
test('Can register a settings action', async () => {
const action = {
uid: 'marketplace.read',
displayName: 'Can read',
pluginName: 'admin',
section: 'settings',
category: 'plugins and marketplace',
subCategory: 'marketplace',
};
await actionProviderService.register([action]);
const createdAction = actionProviderService.get(action.uid, action.pluginName);
expect(createdAction).toMatchObject({
..._.omit(action, ['uid']),
actionId: 'admin::marketplace.read',
conditions: [],
});
createdActions.push(createdAction);
});
test('Can register a settings action without subCategory', async () => {
const action = {
uid: 'marketplace.create',
displayName: 'Can create',
pluginName: 'admin',
section: 'settings',
category: 'plugins and marketplace',
};
await actionProviderService.register([action]);
const createdAction = actionProviderService.get(action.uid, action.pluginName);
expect(createdAction).toMatchObject({
..._.omit(action, ['uid']),
actionId: 'admin::marketplace.create',
subCategory: 'general',
conditions: [],
});
createdActions.push(createdAction);
});
test('Can register a settings action with a pluginName other than "admin"', async () => {
const action = {
uid: 'marketplace.update',
displayName: 'Can update',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
await actionProviderService.register([action]);
const createdAction = actionProviderService.get(action.uid, action.pluginName);
expect(createdAction).toMatchObject({
..._.omit(action, ['uid']),
actionId: 'plugins::aPlugin.marketplace.update',
conditions: [],
});
});
test('Cannot register a settings action with a non standard name', async () => {
const action = {
uid: 'Marketplace Read',
displayName: 'Can access to the marketplace',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
expect(() => actionProviderService.register([action])).toThrow(
'[0].uid: The id can only contain lowercase letters, dots and hyphens.'
);
});
test('Cannot register actions with same actionId', async () => {
global.strapi.stopWithError = jest.fn(() => {});
const action1 = {
uid: 'marketplace.delete',
displayName: 'Can delete',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
const action2 = {
uid: action1.uid,
displayName: 'delete',
pluginName: 'aPlugin',
section: 'plugins',
};
await actionProviderService.register([action1, action2]);
expect(global.strapi.stopWithError).toHaveBeenCalledWith(
expect.objectContaining({
name: 'ValidationError',
message:
'Duplicated action keys: plugins::aPlugin.marketplace.delete. You may want to change the actions name.',
})
);
});
test("Cannot register a settings action with a pluginName that doesn't exist", async () => {
const action = {
uid: 'marketplace.read',
displayName: 'Can access to the marketplace',
pluginName: 'plugin-name-that-doesnt-exist',
section: 'settings',
category: 'plugins and marketplace',
};
expect(() => actionProviderService.register([action])).toThrow(
'[0].pluginName is not an existing plugin'
);
});
test('Cannot register a settings action without category', async () => {
const action = {
uid: 'marketplace.read',
displayName: 'Can access to the marketplace',
pluginName: 'admin',
section: 'settings',
};
expect(() => actionProviderService.register([action])).toThrow(
'[0].category is a required field'
);
});
});
});

View File

@ -1,156 +0,0 @@
'use strict';
const _ = require('lodash');
const permissionProviderService = require('../permission-provider');
describe('Permission Provider Service', () => {
const createdPermissions = [];
beforeEach(() => {
global.strapi = {
plugins: { aPlugin: {} },
};
});
describe('settings', () => {
test('Can register a settings permission', async () => {
const permission = {
uid: 'marketplace.read',
displayName: 'Can read',
pluginName: 'admin',
section: 'settings',
category: 'plugins and marketplace',
subCategory: 'marketplace',
};
await permissionProviderService.register([permission]);
const createdPermission = permissionProviderService.get(
permission.pluginName,
permission.uid
);
expect(createdPermission).toMatchObject({
..._.omit(permission, ['uid']),
permissionId: 'admin::marketplace.read',
conditions: [],
});
createdPermissions.push(createdPermission);
});
test('Can register a settings permission without subCategory', async () => {
const permission = {
uid: 'marketplace.create',
displayName: 'Can create',
pluginName: 'admin',
section: 'settings',
category: 'plugins and marketplace',
};
await permissionProviderService.register([permission]);
const createdPermission = permissionProviderService.get(
permission.pluginName,
permission.uid
);
expect(createdPermission).toMatchObject({
..._.omit(permission, ['uid']),
permissionId: 'admin::marketplace.create',
subCategory: 'general',
conditions: [],
});
createdPermissions.push(createdPermission);
});
test('Can register a settings permission with a pluginName other than "admin"', async () => {
const permission = {
uid: 'marketplace.update',
displayName: 'Can update',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
await permissionProviderService.register([permission]);
const createdPermission = permissionProviderService.get(
permission.pluginName,
permission.uid
);
expect(createdPermission).toMatchObject({
..._.omit(permission, ['uid']),
permissionId: 'plugins::aPlugin.marketplace.update',
conditions: [],
});
});
test('Cannot register a settings permission with a non standard name', async () => {
const permission = {
uid: 'Marketplace Read',
displayName: 'Can access to the marketplace',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
expect(() => permissionProviderService.register([permission])).toThrow(
'[0].uid: The id can only contain lowercase letters, dots and hyphens.'
);
});
test('Cannot register permissions with same permissionId', async () => {
global.strapi.stopWithError = jest.fn(() => {});
const permission1 = {
uid: 'marketplace.delete',
displayName: 'Can delete',
pluginName: 'aPlugin',
section: 'settings',
category: 'plugins and marketplace',
};
const permission2 = {
uid: permission1.uid,
displayName: 'delete',
pluginName: 'aPlugin',
section: 'plugins',
};
await permissionProviderService.register([permission1, permission2]);
expect(global.strapi.stopWithError).toHaveBeenCalledWith(
expect.objectContaining({
name: 'ValidationError',
message:
'Duplicated permission keys: plugins::aPlugin.marketplace.delete. You may want to change the permissions name.',
})
);
});
test("Cannot register a settings permission with a pluginName that doesn't exist", async () => {
const permission = {
uid: 'marketplace.read',
displayName: 'Can access to the marketplace',
pluginName: 'plugin-name-that-doesnt-exist',
section: 'settings',
category: 'plugins and marketplace',
};
expect(() => permissionProviderService.register([permission])).toThrow(
'[0].pluginName is not an existing plugin'
);
});
test('Cannot register a settings permission without category', async () => {
const permission = {
uid: 'marketplace.read',
displayName: 'Can access to the marketplace',
pluginName: 'admin',
section: 'settings',
};
expect(() => permissionProviderService.register([permission])).toThrow(
'[0].category is a required field'
);
});
});
});

View File

@ -0,0 +1,32 @@
const { yup } = require('strapi-utils');
const { validateRegisterProviderAction } = require('../validation/action-provider');
const { getActionId, createAction } = require('../domain/action');
const actionProviderFactory = () => {
const actions = new Map();
return {
get(uid, pluginName) {
const actionId = getActionId({ pluginName, uid });
return actions.find(p => p.actionId === actionId);
},
getAll() {
return Array.from(actions.values());
},
register(newActions) {
validateRegisterProviderAction(newActions);
newActions.forEach(newAction => {
const actionId = getActionId(newAction);
if (actions.has(actionId)) {
throw new yup.ValidationError(
`Duplicated action id: ${actionId}. You may want to change the actions name.`
);
}
actions.set(actionId, createAction(newAction));
});
},
};
};
module.exports = actionProviderFactory();

View File

@ -1,126 +0,0 @@
const _ = require('lodash');
const { yup } = require('strapi-utils');
const { validateRegisterProviderPermission } = require('../validation/permission-provider');
// Utils
const prefixId = ({ pluginName, uid }) => {
let id = '';
if (pluginName === 'admin') {
id = `admin::${uid}`;
} else {
id = `plugins::${pluginName}.${uid}`;
}
return id;
};
const formattedPermissionFields = [
'section',
'displayName',
'category',
'subCategory',
'pluginName',
'subjects',
'conditions',
];
const formatPermissionToBeRegistered = permission => {
const formattedPermission = _.cloneDeep(_.pick(permission, formattedPermissionFields));
formattedPermission.permissionId = prefixId(permission);
formattedPermission.conditions = formattedPermission.conditions || [];
if (['settings', 'plugins'].includes(permission.section)) {
formattedPermission.subCategory = permission.subCategory || 'general';
}
return formattedPermission;
};
const getDuplicatedIds = permissions => {
const duplicatedIds = [];
const ids = [];
permissions.forEach(p => {
if (ids.includes(p.permissionId)) {
duplicatedIds.push(p.permissionId);
} else {
ids.push(p.permissionId);
}
});
return duplicatedIds;
};
const formatPermissionsToNestedFormat = formattedPermissions => {
const sections = formattedPermissions.reduce((result, p) => {
const checkboxItem = {
displayName: p.displayName,
action: p.permissionId,
};
switch (p.section) {
case 'contentTypes':
checkboxItem.subjects = p.subjects;
break;
case 'plugins':
checkboxItem.subCategory = p.subCategory;
checkboxItem.plugin = `plugin::${p.pluginName}`;
break;
case 'settings':
checkboxItem.category = p.category;
checkboxItem.subCategory = p.subCategory;
break;
case 'default':
throw new Error(`Unknown section ${p.section}`);
}
result[p.section] = result[p.section] || [];
result[p.section].push(checkboxItem);
return result;
}, {});
return {
sections,
conditions: [],
};
};
// Private variables
let _permissions = [];
let _permissionsWithNestedFormat = {};
// Exported functions
const get = (pluginName, uid) => {
const permissionId = prefixId({ pluginName, uid });
return _permissions.find(p => p.permissionId === permissionId);
};
const getAll = () => _.cloneDeep(_permissions);
const getAllWithNestedFormat = () => _.cloneDeep(_permissionsWithNestedFormat);
const register = newPermissions => {
validateRegisterProviderPermission(newPermissions);
const newPermissionsWithIds = newPermissions.map(formatPermissionToBeRegistered);
const mergedPermissions = [..._permissions, ...newPermissionsWithIds];
const duplicatedIds = getDuplicatedIds(mergedPermissions);
if (duplicatedIds.length > 0) {
strapi.stopWithError(
new yup.ValidationError(
`Duplicated permission keys: ${duplicatedIds.join(
', '
)}. You may want to change the permissions name.`
)
);
}
_permissions = mergedPermissions;
_permissionsWithNestedFormat = formatPermissionsToNestedFormat(mergedPermissions);
};
module.exports = {
get,
getAll,
getAllWithNestedFormat,
register,
};

View File

@ -1,6 +1,7 @@
'use strict';
const { createPermission } = require('../domain/permission');
const actionProvider = require('./action-provider');
/**
* Delete permissions of roles in database
@ -26,16 +27,16 @@ const find = (params = {}) => {
* @param {Array<Permission{action,subject,fields,conditions}>} permissions - permissions to assign to the role
*/
const assign = async (roleID, permissions = []) => {
const existingPermissions = strapi.admin.services['permission-provider'].getAll();
const existingActions = strapi.admin.services.permission.provider.getAll();
for (let permission of permissions) {
const permissionExists = existingPermissions.find(
ep =>
ep.permissionId === permission.action &&
(ep.section !== 'contentTypes' || ep.subjects.includes(permission.subject))
const actionExists = existingActions.find(
ea =>
ea.actionId === permission.action &&
(ea.section !== 'contentTypes' || ea.subjects.includes(permission.subject))
);
if (!permissionExists) {
if (!actionExists) {
throw strapi.errors.badRequest(
`ValidationError', 'This permission doesn't exist: ${JSON.stringify(permission)}`
`ValidationError', 'This action doesn't exist: ${JSON.stringify(permission)}`
);
}
}
@ -59,4 +60,5 @@ module.exports = {
find,
deleteByRolesIds,
assign,
provider: actionProvider,
};

View File

@ -398,12 +398,11 @@ describe('Role CRUD End to End', () => {
});
expect(res.statusCode).toBe(400);
console.log('res.body', res.body);
expect(res.body).toMatchObject({
statusCode: 400,
error: 'Bad Request',
message:
'ValidationError\', \'This permission doesn\'t exist: {"action":"non.existing.action"}',
'ValidationError\', \'This action doesn\'t exist: {"action":"non.existing.action"}',
});
});

View File

@ -2,7 +2,7 @@
const { yup } = require('strapi-utils');
const registerProviderPermissionSchema = yup
const registerProviderActionSchema = yup
.array()
.requiredAllowEmpty()
.required()
@ -21,27 +21,40 @@ const registerProviderPermissionSchema = yup
.string()
.oneOf(['contentTypes', 'plugins', 'settings'])
.required(),
pluginName: yup
.string()
.required()
.isAPluginName(),
pluginName: yup.mixed().when('section', {
is: 'plugins',
then: yup
.string()
.isAPluginName()
.required(),
otherwise: yup.string().isAPluginName(),
}),
subjects: yup.mixed().when('section', {
is: 'contentTypes',
then: yup
.array()
.of(yup.string().isAContentTypeId())
.required(),
otherwise: yup.mixed().oneOf([undefined]),
otherwise: yup
.mixed()
.oneOf([undefined], 'subjects should only be defined for the "contentTypes" section'),
}),
displayName: yup.string().required(),
category: yup.mixed().when('section', {
is: val => ['plugins', 'contentTypes'].includes(val),
then: yup.mixed().oneOf([undefined]),
then: yup
.mixed()
.oneOf([undefined], 'category should only be defined for the "settings" section'),
otherwise: yup.string().required(),
}),
subCategory: yup.mixed().when('section', {
is: 'contentTypes',
then: yup.mixed().oneOf([undefined]),
then: yup
.mixed()
.oneOf(
[undefined],
'subCategory should only be defined for "plugins" and "settings" sections'
),
otherwise: yup.string(),
}),
conditions: yup.array().of(yup.string()),
@ -49,9 +62,9 @@ const registerProviderPermissionSchema = yup
.noUnknown()
);
const validateRegisterProviderPermission = data => {
const validateRegisterProviderAction = data => {
try {
registerProviderPermissionSchema.validateSync(data, { strict: true, abortEarly: false });
registerProviderActionSchema.validateSync(data, { strict: true, abortEarly: false });
} catch (e) {
if (e.errors.length > 0) {
throw new yup.ValidationError(e.errors.join(', '));
@ -62,5 +75,5 @@ const validateRegisterProviderPermission = data => {
};
module.exports = {
validateRegisterProviderPermission,
validateRegisterProviderAction,
};

View File

@ -111,7 +111,7 @@ async function syncComponentsSchemas() {
function registerPermissions() {
const contentTypesUids = Object.keys(strapi.contentTypes); // TODO: filter to not have internal contentTypes
const permissions = [
const actions = [
{
section: 'contentTypes',
displayName: 'Create',
@ -163,6 +163,6 @@ function registerPermissions() {
},
];
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(actions);
}

View File

@ -1,7 +1,7 @@
'use strict';
module.exports = () => {
const permissions = [
const actions = [
{
section: 'plugins',
displayName: 'Read',
@ -10,6 +10,6 @@ module.exports = () => {
},
];
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(actions);
};

View File

@ -109,7 +109,7 @@ module.exports = async () => {
}
// Add permissions
const permissions = [
const actions = [
{
section: 'plugins',
displayName: 'Can access to the Documentation',
@ -132,6 +132,6 @@ module.exports = async () => {
},
];
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(actions);
};

View File

@ -28,7 +28,7 @@ module.exports = async () => {
}
await pruneObsoleteRelations();
registerPermissions();
registerPermissionActions();
};
const createProvider = ({ provider, providerOptions }) => {
@ -81,8 +81,8 @@ const pruneObsoleteRelationsQuery = ({ model }) => {
);
};
const registerPermissions = () => {
const permissions = [
const registerPermissionActions = () => {
const actions = [
{
section: 'plugins',
displayName: 'Can access to the Media Library',
@ -119,6 +119,6 @@ const registerPermissions = () => {
},
];
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(actions);
};

View File

@ -10,7 +10,7 @@
const _ = require('lodash');
const uuid = require('uuid/v4');
const usersPermissionsPermissions = require('../users-permissions-permissions');
const usersPermissionsActions = require('../users-permissions-actions');
module.exports = async () => {
const pluginStore = strapi.store({
@ -183,6 +183,6 @@ module.exports = async () => {
strapi.reload.isWatching = true;
}
const permissionProvider = strapi.admin.services['permission-provider'];
permissionProvider.register(usersPermissionsPermissions.permissions);
const actionProvider = strapi.admin.services.permission.provider;
actionProvider.register(usersPermissionsActions.actions);
};

View File

@ -22,7 +22,7 @@ function arrayRequiredAllowEmpty(message) {
function isAPluginName(message) {
return this.test('is not a plugin name', message, function(value) {
return ['admin', ...Object.keys(strapi.plugins)].includes(value)
return [undefined, 'admin', ...Object.keys(strapi.plugins)].includes(value)
? true
: this.createError({ path: this.path, message: `${this.path} is not an existing plugin` });
});