Merge pull request #17699 from strapi/feature/stage-rbac-partial-permission-updates

This commit is contained in:
Marc Roig 2023-08-30 09:41:01 +02:00 committed by GitHub
commit 84c0b9f710
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 231 additions and 124 deletions

View File

@ -32,17 +32,17 @@ If a user has several role, the user will be allowed to perform an action if at
An action contains the following information:
| key | description | type | required | Default value | example |
| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -------- | ------------- | -------------------------- |
| **uid** | An id that has to be unique within the plugin. | string | required | - | `'audit-logs.read'` |
| **pluginName** | Name of the plugin registrering the action. | string | - | `'api'` | `'admin'` |
| **section** | Name of the section among `contentTypes`, `plugins` and `settings`. It will define in which permission tab the action will appear | string | required | - | `'settings'` |
| **category** | Name of the category. It will define in which category the action will appear. _Only for the plugins and settings section_. | string | - | - | `'audit logs'` |
| **subCategory** | Name of the subcategory. It will define in which subcategory or the category the action will appear. _Only for the plugins and settings section_. | string | - | - | `'options'` |
| **displayName** | Human name of the action. | string | required | - | `'Read'` |
| **subjects** | List of subjects the action can be applied to. _Only for the contentTypes section_. | array | - | - | `['api::article.article']` |
| **options** | Option object | object | - | `{}` | `{}` |
| **options.applyToProperties** | List of properties the action can be applied to. _Only for the contentTypes section_. | - | - | `[]` | `['fields', 'locale']` |
| key | description | type | required | Default value | example |
| ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -------- | ------------- | -------------------------- |
| **uid** | An id that has to be unique within the plugin. | string | required | - | `'audit-logs.read'` |
| **pluginName** | Name of the plugin registrering the action. | string | - | `'api'` | `'admin'` |
| **section** | Name of the section among `contentTypes`, `plugins`, `settings` and `internal`. It will define in which permission tab the action will appear. <br/> `internal` is not displayed in any tab but used for internal purposes. | string | required | - | `'settings'` |
| **category** | Name of the category. It will define in which category the action will appear. _Only for the plugins and settings section_. | string | - | - | `'audit logs'` |
| **subCategory** | Name of the subcategory. It will define in which subcategory or the category the action will appear. _Only for the plugins and settings section_. | string | - | - | `'options'` |
| **displayName** | Human name of the action. | string | required | - | `'Read'` |
| **subjects** | List of subjects the action can be applied to. _Only for the contentTypes section_. | array | - | - | `['api::article.article']` |
| **options** | Option object | object | - | `{}` | `{}` |
| **options.applyToProperties** | List of properties the action can be applied to. _Only for the contentTypes section_. | - | - | `[]` | `['fields', 'locale']` |
import rbacEditPageImage from '@site/static/img/permissions/rbac-edit-page.png';
@ -86,6 +86,16 @@ module.exports = async () => {
};
```
### Parametrize actions
Actions can be parametrized with the `actionParameters` property. This property is an object that can contain any key/value pair.
An example of the feature:
- The action `review-workflows.stage.transition` can be parametrized with the `from` and `to` parameters, which are the ids of the stages a user can transition from and to.
At the moment only Review Workflows use this feature internally.
## Conditions
### Condition definition
@ -138,11 +148,12 @@ More information on how the handler works [here](https://docs.strapi.io/develope
The permissions are modified in the admin panel (on the [edit page](http://localhost:1337/admin/settings/roles/2) of a role) and stored in the database with the following information:
| key | description | type | required | example |
| -------------- | -------------------------------------------------------------------------------------------------------------------------- | ------ | -------- | ------------------------------------------------------------- |
| **action** | Id of the action that will be permitted. | string | required | `'plugin::content-manager.explorer.update'` |
| **subject** | Id of the subject on which the action will be permitted. | string | - | `'api::article.article'` |
| **properties** | List of the properties of the subject on which the action will be permitted | object | - | `{ fields: ['title', 'description'], locales: ['en', 'fr'] }` |
| **conditions** | List of the conditions that will be ran against an entry to determine whether the action on this entry is permitted or not | array | - | `['admin::is-creator']` |
| key | description | type | required | example |
| -------------------- | -------------------------------------------------------------------------------------------------------------------------- | ------ | -------- | ------------------------------------------------------------- |
| **action** | Id of the action that will be permitted. | string | required | `'plugin::content-manager.explorer.update'` |
| **actionParameters** | Object to parametrize actions. | string | - | `{}` |
| **subject** | Id of the subject on which the action will be permitted. | string | - | `'api::article.article'` |
| **properties** | List of the properties of the subject on which the action will be permitted | object | - | `{ fields: ['title', 'description'], locales: ['en', 'fr'] }` |
| **conditions** | List of the conditions that will be ran against an entry to determine whether the action on this entry is permitted or not | array | - | `['admin::is-creator']` |
A permission contains all needed information for the backend and the frontend to prevent users to perform non-permitted action.

View File

@ -812,6 +812,108 @@ describe('Role', () => {
},
]);
});
test('Filter internal permissions on create', async () => {
const permissions = [
{ action: `action-0` },
{ action: `action-1` },
{ action: 'action-internal' },
];
const createMany = jest.fn(() => Promise.resolve([]));
const getSuperAdmin = jest.fn(() => Promise.resolve({ id: 0 }));
const sendDidUpdateRolePermissions = jest.fn();
const findMany = jest.fn(() => Promise.resolve([]));
const values = jest.fn(() => [
{ actionId: 'action-0', section: 'plugins' },
{ actionId: 'action-1', section: 'plugins' },
{ actionId: 'action-internal', section: 'internal' },
]);
const conditionProviderHas = jest.fn((cond) => cond === 'cond');
global.strapi = {
admin: {
services: {
metrics: { sendDidUpdateRolePermissions },
role: { getSuperAdmin },
permission: {
findMany,
createMany,
actionProvider: { values },
conditionProvider: {
has: conditionProviderHas,
values: jest.fn(() => [{ id: 'admin::is-creator' }]),
},
},
},
},
eventHub: createEventHub(),
};
await roleService.assignPermissions(1, permissions);
expect(createMany).toHaveBeenCalledTimes(1);
expect(createMany).toHaveBeenCalledWith([
{
action: 'action-0',
actionParameters: {},
conditions: [],
properties: {},
role: 1,
subject: null,
},
{
action: 'action-1',
actionParameters: {},
conditions: [],
properties: {},
role: 1,
subject: null,
},
]);
});
test('Filter internal permissions on delete', async () => {
const permissions = [{ action: `action-0` }, { action: `action-1` }];
const createMany = jest.fn(() => Promise.resolve(permissions));
const getSuperAdmin = jest.fn(() => Promise.resolve({ id: 0 }));
const sendDidUpdateRolePermissions = jest.fn();
const findMany = jest.fn(() =>
Promise.resolve([{ action: 'action-internal', id: 1, properties: {} }])
);
const deleteByIds = jest.fn();
const values = jest.fn(() => [
{ actionId: 'action-0', section: 'plugins' },
{ actionId: 'action-1', section: 'plugins' },
{ actionId: 'action-internal', section: 'internal' },
]);
const conditionProviderHas = jest.fn((cond) => cond === 'cond');
global.strapi = {
admin: {
services: {
metrics: { sendDidUpdateRolePermissions },
role: { getSuperAdmin },
permission: {
deleteByIds,
findMany,
createMany,
actionProvider: { values },
conditionProvider: {
has: conditionProviderHas,
values: jest.fn(() => [{ id: 'admin::is-creator' }]),
},
},
},
},
eventHub: createEventHub(),
};
const returnedPermissions = await roleService.assignPermissions(1, permissions);
expect(deleteByIds).toHaveBeenCalledTimes(0);
expect(returnedPermissions).toEqual(permissions);
});
});
describe('addPermissions', () => {

View File

@ -323,6 +323,13 @@ const displayWarningIfNoSuperAdmin = async () => {
const assignPermissions = async (roleId, permissions = []) => {
await validatePermissionsExist(permissions);
// Internal actions are not handled by the role service, so any permission
// with an internal action is filtered out
const internalActions = getService('permission')
.actionProvider.values()
.filter((action) => action.section === 'internal')
.map((action) => action.actionId);
const superAdmin = await getService('role').getSuperAdmin();
const isSuperAdmin = superAdmin && superAdmin.id === roleId;
const assignRole = set('role', roleId);
@ -342,13 +349,13 @@ const assignPermissions = async (roleId, permissions = []) => {
arePermissionsEqual,
permissionsWithRole,
existingPermissions
);
).filter((permission) => !internalActions.includes(permission.action));
const permissionsToDelete = differenceWith(
arePermissionsEqual,
existingPermissions,
permissionsWithRole
);
).filter((permission) => !internalActions.includes(permission.action));
const permissionsToReturn = differenceBy('id', permissionsToDelete, existingPermissions);
@ -368,7 +375,6 @@ const assignPermissions = async (roleId, permissions = []) => {
return permissionsToReturn;
};
const addPermissions = async (roleId, permissions) => {
const { conditionProvider, createMany } = getService('permission');
const { sanitizeConditions } = permissionDomain;
@ -381,7 +387,6 @@ const addPermissions = async (roleId, permissions) => {
return createMany(permissionsWithRole);
};
const isContentTypeAction = (action) => action.section === CONTENT_TYPE_SECTION;
/**

View File

@ -89,112 +89,103 @@ const fieldsPropertyValidation = (action) =>
checkNilFields(action)
);
const permission = yup
.object()
.shape({
action: yup
.string()
.required()
.test('action-validity', 'action is not an existing permission action', function (actionId) {
// If the action field is Nil, ignore the test and let the required check handle the error
if (isNil(actionId)) {
return true;
}
return !!getActionFromProvider(actionId);
}),
actionParameters: yup.object().nullable(),
subject: yup
.string()
.nullable()
.test('subject-validity', 'Invalid subject submitted', function (subject) {
const action = getActionFromProvider(this.options.parent.action);
if (!action) {
return true;
}
if (isNil(action.subjects)) {
return isNil(subject);
}
if (isArray(action.subjects)) {
return action.subjects.includes(subject);
}
return false;
}),
properties: yup
.object()
.test('properties-structure', 'Invalid property set at ${path}', function (properties) {
const action = getActionFromProvider(this.options.parent.action);
const hasNoProperties = isEmpty(properties) || isNil(properties);
if (!has('options.applyToProperties', action)) {
return hasNoProperties;
}
if (hasNoProperties) {
return true;
}
const { applyToProperties } = action.options;
if (!isArray(applyToProperties)) {
return false;
}
return Object.keys(properties).every((property) => applyToProperties.includes(property));
})
.test(
'fields-property',
'Invalid fields property at ${path}',
async function (properties = {}) {
const action = getActionFromProvider(this.options.parent.action);
if (!action || !properties) {
return true;
}
if (!actionDomain.appliesToProperty('fields', action)) {
return true;
}
try {
await fieldsPropertyValidation(action).validate(properties.fields, {
strict: true,
abortEarly: false,
});
return true;
} catch (e) {
// Propagate fieldsPropertyValidation error with updated path
throw this.createError({
message: e.message,
path: `${this.path}.fields`,
});
}
}
),
conditions: yup.array().of(yup.string()),
})
.noUnknown();
const updatePermissions = yup
.object()
.shape({
permissions: yup
.array()
.required()
.of(
yup
.object()
.shape({
action: yup
.string()
.required()
.test(
'action-validity',
'action is not an existing permission action',
function (actionId) {
// If the action field is Nil, ignore the test and let the required check handle the error
if (isNil(actionId)) {
return true;
}
return !!getActionFromProvider(actionId);
}
),
subject: yup
.string()
.nullable()
.test('subject-validity', 'Invalid subject submitted', function (subject) {
const action = getActionFromProvider(this.options.parent.action);
if (!action) {
return true;
}
if (isNil(action.subjects)) {
return isNil(subject);
}
if (isArray(action.subjects)) {
return action.subjects.includes(subject);
}
return false;
}),
properties: yup
.object()
.test(
'properties-structure',
'Invalid property set at ${path}',
function (properties) {
const action = getActionFromProvider(this.options.parent.action);
const hasNoProperties = isEmpty(properties) || isNil(properties);
if (!has('options.applyToProperties', action)) {
return hasNoProperties;
}
if (hasNoProperties) {
return true;
}
const { applyToProperties } = action.options;
if (!isArray(applyToProperties)) {
return false;
}
return Object.keys(properties).every((property) =>
applyToProperties.includes(property)
);
}
)
.test(
'fields-property',
'Invalid fields property at ${path}',
async function (properties = {}) {
const action = getActionFromProvider(this.options.parent.action);
if (!action || !properties) {
return true;
}
if (!actionDomain.appliesToProperty('fields', action)) {
return true;
}
try {
await fieldsPropertyValidation(action).validate(properties.fields, {
strict: true,
abortEarly: false,
});
return true;
} catch (e) {
// Propagate fieldsPropertyValidation error with updated path
throw this.createError({
message: e.message,
path: `${this.path}.fields`,
});
}
}
),
conditions: yup.array().of(yup.string()),
})
.noUnknown()
)
.of(permission)
.test(
'duplicated-permissions',
'Some permissions are duplicated (same action and subject)',
@ -213,5 +204,6 @@ module.exports = {
roles,
isAPluginName,
arrayOfConditionNames,
permission,
updatePermissions,
};

View File

@ -17,8 +17,6 @@ const checkPermissionsSchema = yup.object().shape({
),
});
// validatePermissionsExist
const checkPermissionsExist = function (permissions) {
const existingActions = getService('permission').actionProvider.values();
const failIndex = permissions.findIndex(
@ -48,7 +46,6 @@ const actionsExistSchema = yup
.test('actions-exist', '', checkPermissionsExist);
// exports
module.exports = {
validatedUpdatePermissionsInput: validateYupSchema(validators.updatePermissions),
validatePermissionsExist: validateYupSchema(actionsExistSchema),