RBAC Action Aliases - v5 (#20878)

This commit is contained in:
Jean-Sébastien Herbaux 2024-08-06 15:16:25 +02:00 committed by GitHub
parent 2205943267
commit 3ad6bddd75
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 373 additions and 129 deletions

View File

@ -50,6 +50,12 @@ export const actions = [
section: 'settings',
category: 'users and roles',
subCategory: 'users',
aliases: [
{
actionId: 'plugin::content-manager.explorer.read',
subjects: ['admin::user'],
},
],
},
{
uid: 'users.update',
@ -82,6 +88,12 @@ export const actions = [
section: 'settings',
category: 'users and roles',
subCategory: 'roles',
aliases: [
{
actionId: 'plugin::content-manager.explorer.read',
subjects: ['admin::role'],
},
],
},
{
uid: 'roles.update',

View File

@ -8,12 +8,12 @@ const providerMethods = [
'appliesToProperty',
'delete',
'get',
'getWhere',
'values',
'keys',
'has',
'size',
'clear',
'unstable_aliases',
];
describe('Action Provider', () => {
@ -371,5 +371,145 @@ describe('Action Provider', () => {
);
});
});
describe('Aliases', () => {
test('Return an empty array when there is no alias pointing to the given action', async () => {
const provider = createActionProvider();
const actions = [
{
section: 'settings',
displayName: 'Foo bar',
uid: 'foo',
pluginName: 'bar',
category: 'category',
subCategory: 'subcategory',
},
{
section: 'settings',
displayName: 'Bar foo',
category: 'category',
uid: 'bar',
},
];
await provider.registerMany(actions);
const aliases = provider.unstable_aliases('plugin::bar.foo');
expect(aliases).toStrictEqual([]);
});
test(`Return an empty array when there is an alias but the subject don't match`, async () => {
const provider = createActionProvider();
const actions = [
{
section: 'settings',
displayName: 'Foo bar',
uid: 'foo',
pluginName: 'bar',
category: 'category',
subCategory: 'subcategory',
},
{
section: 'settings',
displayName: 'Bar foo',
category: 'category',
uid: 'bar',
aliases: [{ actionId: 'plugin::bar.foo', subjects: ['baz'] }],
},
];
await provider.registerMany(actions);
const aliases = provider.unstable_aliases('plugin::bar.foo', 'bar');
expect(aliases).toStrictEqual([]);
expect(aliases).toHaveLength(0);
});
test(`Return an empty array when there is an alias but no subject is passed (when required)`, async () => {
const provider = createActionProvider();
const actions = [
{
section: 'settings',
displayName: 'Foo bar',
uid: 'foo',
pluginName: 'bar',
category: 'category',
subCategory: 'subcategory',
},
{
section: 'settings',
displayName: 'Bar foo',
category: 'category',
uid: 'bar',
aliases: [{ actionId: 'plugin::bar.foo', subjects: ['baz'] }],
},
];
await provider.registerMany(actions);
const aliases = provider.unstable_aliases('plugin::bar.foo');
expect(aliases).toStrictEqual([]);
expect(aliases).toHaveLength(0);
});
test('Return 1 result when there is a valid alias pointing to the given action', async () => {
const provider = createActionProvider();
const actions = [
{
section: 'settings',
displayName: 'Foo bar',
uid: 'foo',
pluginName: 'bar',
category: 'category',
subCategory: 'subcategory',
},
{
section: 'settings',
displayName: 'Bar foo',
category: 'category',
uid: 'bar',
aliases: [{ actionId: 'plugin::bar.foo' }],
},
];
await provider.registerMany(actions);
const aliases = provider.unstable_aliases('plugin::bar.foo');
expect(aliases).toHaveLength(1);
expect(aliases).toStrictEqual(['api::bar']);
});
test('Return 1 result when there is a valid alias pointing to the given action with a valid subject', async () => {
const provider = createActionProvider();
const actions = [
{
section: 'settings',
displayName: 'Foo bar',
uid: 'foo',
pluginName: 'bar',
category: 'category',
subCategory: 'subcategory',
},
{
section: 'settings',
displayName: 'Bar foo',
category: 'category',
uid: 'bar',
aliases: [{ actionId: 'plugin::bar.foo', subjects: ['baz'] }],
},
];
await provider.registerMany(actions);
const aliases = provider.unstable_aliases('plugin::bar.foo', 'baz');
expect(aliases).toHaveLength(1);
expect(aliases).toStrictEqual(['api::bar']);
});
});
});
});

View File

@ -2,18 +2,77 @@ import type { Utils } from '@strapi/types';
import { curry, pipe, merge, set, pick, omit, includes, isArray, prop } from 'lodash/fp';
export interface ActionAlias {
/**
* The action ID to alias
*/
actionId: string;
/**
* An optional array of subject to restrict the alias usage
*/
subjects?: string[];
}
export type Action = {
actionId: string; // The unique identifier of the action
section: string; // The section linked to the action - These can be 'contentTypes' | 'plugins' | 'settings' | 'internal'
displayName: string; // The human readable name of an action
category: string; // The main category of an action
subCategory?: string; // The secondary category of an action (only for settings and plugins section)
pluginName?: string; // The plugin which provide the action
subjects?: string[]; // A list of subjects on which the action can be applied
/**
* The unique identifier of the action
*/
actionId: string;
/**
* The section linked to the action - These can be 'contentTypes' | 'plugins' | 'settings' | 'internal'
*/
section: string;
/**
* The human readable name of an action
*/
displayName: string;
/**
* The main category of an action
*/
category: string;
/**
* The secondary category of an action (only for settings and plugins section)
*/
subCategory?: string;
/**
* The plugin that provides the action
*/
pluginName?: string;
/**
* A list of subjects on which the action can be applied
*/
subjects?: string[];
/**
* The options of an action
*/
options: {
// The options of an action
applyToProperties: string[] | null; // The list of properties that can be associated with an action
/**
* The list of properties that can be associated with an action
*/
applyToProperties: string[] | null;
};
/**
* An optional array of @see {@link ActionAlias}.
*
* It represents the possible aliases for the current action.
*
* Aliases are unidirectional.
*
* Note: This is an internal property and probably shouldn't be used outside Strapi core features.
* Its behavior might change at any time without notice.
*
* @internal
*/
aliases?: ActionAlias[];
};
/**
@ -54,6 +113,7 @@ const actionFields = [
'subjects',
'options',
'actionId',
'aliases',
] as const;
/**

View File

@ -13,7 +13,7 @@ const { ApplicationError } = errors;
* Creates a new instance of an action provider
*/
const createActionProvider = (options?: Options) => {
const provider = providerFactory(options);
const provider = providerFactory<Action>(options);
const actionHooks = {
appliesPropertyToSubject: hooks.createAsyncParallelHook(),
};
@ -79,6 +79,41 @@ const createActionProvider = (options?: Options) => {
return results.every((result) => result !== false);
},
/**
* @experimental
*/
unstable_aliases(actionId: string, subject?: string | null): string[] {
const isRegistered = this.has(actionId);
if (!isRegistered) {
return [];
}
return this.values()
.filter((action) =>
action.aliases?.some((alias) => {
// Only look at alias with the correct actionId
if (alias.actionId !== actionId) {
return false;
}
// If the alias don't have a list of required subjects, keep it
if (!Array.isArray(alias.subjects)) {
return true;
}
// If the alias require specific subjects but none is provided, skip it
if (!subject) {
return false;
}
// Else, make sure the given subject is allowed
return alias.subjects.includes(subject);
})
)
.map((action) => action.actionId);
},
};
};

View File

@ -7,7 +7,6 @@ const providerMethods = [
'registerMany',
'delete',
'get',
'getWhere',
'values',
'keys',
'has',

View File

@ -56,6 +56,14 @@ const registerProviderActionSchema = yup
options: yup.object({
applyToProperties: yup.array().of(yup.string()),
}),
aliases: yup
.array(
yup.object({
actionId: yup.string(),
subjects: yup.array(yup.string()).nullable(),
})
)
.nullable(),
})
.noUnknown()
);

View File

@ -118,7 +118,7 @@ export const permission = yup
return isNil(subject);
}
if (isArray(action.subjects)) {
if (isArray(action.subjects) && !isNil(subject)) {
return action.subjects.includes(subject);
}

View File

@ -5,4 +5,3 @@
"project": ["./server/tsconfig.eslint.json"]
}
}

View File

@ -29,24 +29,21 @@ const sanitizeMainField = (model: any, mainField: any, userAbility: any) => {
model: model.uid,
});
if (!isListable(model, mainField)) {
// Whether the main field can be displayed or not, regardless of permissions.
const isMainFieldListable = isListable(model, mainField);
// Whether the user has the permission to access the model's main field (using RBAC abilities)
const canReadMainField = permissionChecker.can.read(null, mainField);
if (!isMainFieldListable || !canReadMainField) {
// Default to 'id' if the actual main field shouldn't be displayed
return 'id';
}
if (permissionChecker.cannot.read(null, mainField)) {
// Allow reading role name if user can read the user model
if (model.uid === 'plugin::users-permissions.role') {
const userPermissionChecker = getService('permission-checker').create({
userAbility,
model: 'plugin::users-permissions.user',
});
// Edge cases
if (userPermissionChecker.can.read()) {
return 'name';
}
}
return 'id';
// 1. Enforce 'name' as the main field for users and permissions' roles
if (model.uid === 'plugin::users-permissions.role') {
return 'name';
}
return mainField;

View File

@ -7,6 +7,11 @@ const { createPolicy } = policy;
export default createPolicy({
name: 'plugin::content-manager.hasPermissions',
validator: validateHasPermissionsInput,
/**
* NOTE: Action aliases are currently not checked at this level (policy).
* This is currently the intended behavior to avoid changing the behavior of API related permissions.
* If you want to add support for it, please create a dedicated RFC with a list of potential side effect this could have.
*/
handler(ctx: Context, config = {}) {
const { actions = [], hasAtLeastOne = false }: { actions: string[]; hasAtLeastOne: boolean } =
config;

View File

@ -26,19 +26,38 @@ const createPermissionChecker =
model,
});
const toSubject = (entity?: Entity) =>
entity ? permissionsManager.toSubject(entity, model) : model;
const { actionProvider } = strapi.service('admin::permission');
const toSubject = (entity?: Entity) => {
return entity ? permissionsManager.toSubject(entity, model) : model;
};
// @ts-expect-error preserve the parameter order
// eslint-disable-next-line @typescript-eslint/default-param-last
const can = (action: string, entity?: Entity, field: string) => {
return userAbility.can(action, toSubject(entity), field);
const subject = toSubject(entity);
const aliases = actionProvider.unstable_aliases(action, model) as string[];
return (
// Test the original action to see if it passes
userAbility.can(action, subject, field) ||
// Else try every known alias if at least one of them succeed, then the user "can"
aliases.some((alias) => userAbility.can(alias, subject, field))
);
};
// @ts-expect-error preserve the parameter order
// eslint-disable-next-line @typescript-eslint/default-param-last
const cannot = (action: string, entity?: Entity, field: string) => {
return userAbility.cannot(action, toSubject(entity), field);
const subject = toSubject(entity);
const aliases = actionProvider.unstable_aliases(action, model) as string[];
return (
// Test both the original action
userAbility.cannot(action, subject, field) &&
// and every known alias, if all of them fail (cannot), then the user truly "cannot"
aliases.every((alias) => userAbility.cannot(alias, subject, field))
);
};
const sanitizeOutput = (data: Entity, { action = ACTIONS.read }: { action?: string } = {}) => {

View File

@ -48,5 +48,5 @@ export const query: Core.MiddlewareFactory = (
config: Partial<Config>,
{ strapi }: { strapi: Core.Strapi }
) => {
addQsParser(strapi.server.app, { ...defaults, ...config });
addQsParser(strapi.server.app, { ...defaults, ...config } as Config);
};

View File

@ -1,16 +1,6 @@
import providerFactory from '../provider-factory';
const providerMethods = [
'register',
'delete',
'get',
'getWhere',
'values',
'keys',
'has',
'size',
'clear',
];
const providerMethods = ['register', 'delete', 'get', 'values', 'keys', 'has', 'size', 'clear'];
describe('Provider Factory', () => {
describe('Core', () => {
@ -204,50 +194,6 @@ describe('Provider Factory', () => {
});
});
describe('GetWhere', () => {
const items = [
{ key: 'keyA', value: { foo: 'barA', bar: 'foo1' } },
{ key: 'keyB', value: { foo: 'barB', bar: 'foo2' } },
{ key: 'keyC', value: { foo: 'barC', bar: 'foo1' } },
{ key: 'keyD', value: { foo: 'barD', bar: 'foo2' } },
];
const provider = providerFactory();
const sortItems = (a, b) => (a.key < b.key ? -1 : 1);
const pickItems = (...indexes) => indexes.map((index) => items[index].value);
beforeAll(async () => {
for (const item of items) {
await provider.register(item.key, item.value);
}
});
test('Calling getWhere without filters returns every registered items', async () => {
const expected = items.map((i) => i.value).sort(sortItems);
const results = provider.getWhere();
expect(results).toStrictEqual(expect.any(Array));
expect(results).toHaveLength(items.length);
expect(results.sort(sortItems)).toEqual(expected);
});
test.each([
[{ foo: 'barA' }, pickItems(0)],
[{ bar: 'foo1' }, pickItems(0, 2)],
[{ bar: 'foo2' }, pickItems(1, 3)],
[{ foo: 'barD', bar: 'foo2' }, pickItems(3)],
[{ foo: 'barC', bar: 'foo2' }, []],
[{ foobar: 'foobar' }, []],
[{}, pickItems(0, 1, 2, 3)],
])('Filters %s', (filters, expected) => {
const results = provider.getWhere(filters);
expect(results).toStrictEqual(expect.any(Array));
expect(results).toStrictEqual(expected);
});
});
describe('Values', () => {
test('Returns an empty array when there is no registered item', async () => {
const provider = providerFactory();

View File

@ -31,36 +31,35 @@ export interface Options {
type Item = Record<string, unknown>;
export interface Provider {
export interface Provider<T = unknown> {
hooks: ProviderHooksMap;
register(key: string, item: Item): Promise<Provider>;
register(key: string, item: T): Promise<Provider>;
delete(key: string): Promise<Provider>;
get(key: string): Item | undefined;
getWhere(filters?: Record<string, unknown>): Item[];
values(): Item[];
get(key: string): T | undefined;
values(): T[];
keys(): string[];
has(key: string): boolean;
size(): number;
clear(): Promise<Provider>;
}
export type ProviderFactory = (options?: Options) => Provider;
export type ProviderFactory<T> = (options?: Options) => Provider<T>;
/**
* A Provider factory
*/
const providerFactory: ProviderFactory = (options = {}) => {
const providerFactory = <T = Item>(options: Options = {}): Provider<T> => {
const { throwOnDuplicates = true } = options;
const state = {
hooks: createProviderHooksMap(),
registry: new Map<string, Item>(),
registry: new Map<string, T>(),
};
return {
hooks: state.hooks,
async register(key: string, item: Item) {
async register(key: string, item: T) {
if (throwOnDuplicates && this.has(key)) {
throw new Error(`Duplicated item key: ${key}`);
}
@ -92,19 +91,6 @@ const providerFactory: ProviderFactory = (options = {}) => {
return state.registry.get(key);
},
getWhere(filters = {}) {
const items = this.values();
const filtersEntries = Object.entries(filters);
if (filtersEntries.length === 0) {
return items;
}
return items.filter((item) => {
return filtersEntries.every(([key, value]) => item[key] === value);
});
},
values() {
return Array.from(state.registry.values());
},

View File

@ -1,14 +1,43 @@
import { capitalize, isArray, getOr, prop } from 'lodash/fp';
import { isArray, getOr, prop } from 'lodash/fp';
import { getService } from '../../utils';
const actions = ['create', 'read', 'update', 'delete'].map((uid) => ({
section: 'settings',
category: 'Internationalization',
subCategory: 'Locales',
pluginName: 'i18n',
displayName: capitalize(uid),
uid: `locale.${uid}`,
}));
const actions = [
{
section: 'settings',
category: 'Internationalization',
subCategory: 'Locales',
pluginName: 'i18n',
displayName: 'Create',
uid: 'locale.create',
},
{
section: 'settings',
category: 'Internationalization',
subCategory: 'Locales',
pluginName: 'i18n',
displayName: 'Read',
uid: 'locale.read',
aliases: [
{ actionId: 'plugin::content-manager.explorer.read', subjects: ['plugin::i18n.locale'] },
],
},
{
section: 'settings',
category: 'Internationalization',
subCategory: 'Locales',
pluginName: 'i18n',
displayName: 'Update',
uid: 'locale.update',
},
{
section: 'settings',
category: 'Internationalization',
subCategory: 'Locales',
pluginName: 'i18n',
displayName: 'Delete',
uid: 'locale.delete',
},
];
const addLocalesPropertyIfNeeded = ({ value: action }: any) => {
const {

View File

@ -16,6 +16,12 @@ module.exports = {
uid: 'roles.read',
subCategory: 'roles',
pluginName: 'users-permissions',
aliases: [
{
actionId: 'plugin::content-manager.explorer.read',
subjects: ['plugin::users-permissions.role'],
},
],
},
{
section: 'plugins',

View File

@ -55,16 +55,15 @@ const createEntry = async (model, data, populate) => {
body: data,
qs: { populate },
});
return body;
};
const getRelations = async (rq, uid, field, id) => {
const res = await rq({
return rq({
method: 'GET',
url: `/content-manager/relations/${uid}/${id}/${field}`,
});
return res;
};
const createUserAndReq = async (userName, permissions) => {
@ -83,9 +82,7 @@ const createUserAndReq = async (userName, permissions) => {
roles: [role.id],
});
const rq = await createAuthRequest({ strapi, userInfo: user });
return rq;
return createAuthRequest({ strapi, userInfo: user });
};
describe('Relation permissions', () => {
@ -102,6 +99,9 @@ describe('Relation permissions', () => {
action: 'plugin::content-manager.explorer.read',
subject: 'plugin::users-permissions.user',
},
{
action: 'plugin::users-permissions.roles.read',
},
]);
rqPermissive = await createUserAndReq('permissive', [
// Can read shops and products
@ -117,6 +117,9 @@ describe('Relation permissions', () => {
action: 'plugin::content-manager.explorer.read',
subject: 'plugin::users-permissions.user',
},
{
action: 'plugin::users-permissions.roles.read',
},
]);
};
@ -160,9 +163,9 @@ describe('Relation permissions', () => {
shop.documentId
);
// Main field should be in here
// The main field should be in here
expect(products.results).toHaveLength(1);
// Main field should be visible
// The main field should be visible
expect(products.results[0].name).toBe(product.name);
});
@ -175,7 +178,7 @@ describe('Relation permissions', () => {
);
expect(products.results).toHaveLength(1);
// Main field should not be visible
// The main field should not be visible
expect(products.results[0].name).toBeUndefined();
expect(products.results[0].id).toBe(product.id);
});
@ -192,7 +195,7 @@ describe('Relation permissions', () => {
user.documentId
);
// Main field should be visible
// The main field should be visible
expect(role.results?.[0].name).toBe('Authenticated');
});
});