From 5f5d3f3d90f97f4fca3172c29a48267945f8f70b Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Sat, 11 May 2024 23:08:50 +0800 Subject: [PATCH] feat: strategy with resources list (#4312) * chore: strategy with resources list * chore: append strategy resource when collection loaded * chore: test * chore: no permission error * chore: test * fix: update strategy resources after update collection * fix: test * fix: snippet name * chore: error class import --- packages/core/acl/src/__tests__/acl.test.ts | 24 +++++ .../core/acl/src/__tests__/snippet.test.ts | 93 +++++++++++++++++++ packages/core/acl/src/acl-role.ts | 1 + packages/core/acl/src/acl.ts | 77 ++++++++++----- packages/core/acl/src/errors/index.ts | 10 ++ .../acl/src/errors/no-permission-error.ts | 10 ++ packages/core/acl/src/index.ts | 1 + packages/core/acl/src/snippet-manager.ts | 6 ++ .../src/server/__tests__/acl.test.ts | 24 +++++ .../src/server/__tests__/middleware.test.ts | 40 ++++++++ .../src/server/__tests__/own.test.ts | 2 + .../src/server/__tests__/prepare.ts | 1 + .../src/server/__tests__/role.test.ts | 22 +++++ .../src/server/middlewares/with-acl-meta.ts | 3 +- .../@nocobase/plugin-acl/src/server/server.ts | 18 ++++ .../repositories/collection-repository.ts | 1 + .../plugin-users/src/server/server.ts | 4 +- .../plugin-workflow/src/server/Plugin.ts | 2 +- 18 files changed, 314 insertions(+), 25 deletions(-) create mode 100644 packages/core/acl/src/errors/index.ts create mode 100644 packages/core/acl/src/errors/no-permission-error.ts diff --git a/packages/core/acl/src/__tests__/acl.test.ts b/packages/core/acl/src/__tests__/acl.test.ts index ce40c22978f0f..fdfb4a1c0491d 100644 --- a/packages/core/acl/src/__tests__/acl.test.ts +++ b/packages/core/acl/src/__tests__/acl.test.ts @@ -415,4 +415,28 @@ describe('acl', () => { ctx1.permission.can.params.fields.push('createdById'); expect(ctx2.permission.can.params.fields).toEqual([]); }); + + it('should not allow when strategyResources is set', async () => { + acl.setAvailableAction('create', { + displayName: 'create', + type: 'new-data', + }); + + const role = acl.define({ + role: 'admin', + strategy: { + actions: ['create'], + }, + }); + + expect(acl.can({ role: 'admin', resource: 'users', action: 'create' })).toBeTruthy(); + + acl.setStrategyResources(['posts']); + + expect(acl.can({ role: 'admin', resource: 'users', action: 'create' })).toBeNull(); + + acl.setStrategyResources(['posts', 'users']); + + expect(acl.can({ role: 'admin', resource: 'users', action: 'create' })).toBeTruthy(); + }); }); diff --git a/packages/core/acl/src/__tests__/snippet.test.ts b/packages/core/acl/src/__tests__/snippet.test.ts index 17747932da4ac..9feaac13f77b4 100644 --- a/packages/core/acl/src/__tests__/snippet.test.ts +++ b/packages/core/acl/src/__tests__/snippet.test.ts @@ -7,8 +7,56 @@ * For more information, please refer to: https://www.nocobase.com/agreement. */ +import { MockServer, createMockServer } from '@nocobase/test'; import { ACL } from '..'; import SnippetManager from '../snippet-manager'; +describe('nocobase snippet', () => { + let app: MockServer; + + beforeEach(async () => { + app = await createMockServer({ + plugins: ['nocobase'], + }); + }); + + afterEach(async () => { + await app.destroy(); + }); + + test('snippet allowed', async () => { + const testRole = app.acl.define({ + role: 'test', + }); + + testRole.snippets.add('!pm.users'); + testRole.snippets.add('pm.*'); + + expect( + app.acl.can({ + role: 'test', + resource: 'users', + action: 'list', + }), + ).toBeNull(); + }); + + it('should allow all snippets', async () => { + const testRole = app.acl.define({ + role: 'test', + }); + + testRole.snippets.add('!pm.acl.roles'); + testRole.snippets.add('pm.*'); + + expect( + app.acl.can({ + role: 'test', + resource: 'users', + action: 'list', + }), + ).toBeTruthy(); + }); +}); describe('acl snippet', () => { let acl: ACL; @@ -86,6 +134,34 @@ describe('acl snippet', () => { expect(adminRole.snippetAllowed('other:list')).toBeNull(); }); + + it('should return true when last rule allowd', () => { + acl.registerSnippet({ + name: 'sc.collection-manager.fields', + actions: ['fields:list'], + }); + + acl.registerSnippet({ + name: 'sc.collection-manager.gi', + actions: ['fields:list'], + }); + + acl.registerSnippet({ + name: 'sc.users', + actions: ['users:*'], + }); + + const adminRole = acl.define({ + role: 'admin', + }); + + adminRole.snippets.add('!sc.collection-manager.gi'); + adminRole.snippets.add('!sc.users'); + adminRole.snippets.add('sc.*'); + + expect(acl.can({ role: 'admin', resource: 'fields', action: 'list' })).toBeTruthy(); + expect(acl.can({ role: 'admin', resource: 'users', action: 'list' })).toBeNull(); + }); }); describe('snippet manager', () => { @@ -135,5 +211,22 @@ describe('snippet manager', () => { expect(snippetManager.allow('fields:list', 'sc.collection-manager.fields')).toBeNull(); }); + + it('should not register snippet named with *', async () => { + const snippetManager = new SnippetManager(); + + let error; + + try { + snippetManager.register({ + name: 'sc.collection-manager.*', + actions: ['collections:*'], + }); + } catch (e) { + error = e; + } + + expect(error).toBeDefined(); + }); }); }); diff --git a/packages/core/acl/src/acl-role.ts b/packages/core/acl/src/acl-role.ts index 54d2d94944036..e2f4b53a03e4e 100644 --- a/packages/core/acl/src/acl-role.ts +++ b/packages/core/acl/src/acl-role.ts @@ -107,6 +107,7 @@ export class ACLRole { public effectiveSnippets(): { allowed: Array; rejected: Array } { const currentParams = this._serializeSet(this.snippets); + if (this._snippetCache.params === currentParams) { return this._snippetCache.result; } diff --git a/packages/core/acl/src/acl.ts b/packages/core/acl/src/acl.ts index 74089a6ef07eb..c23e9b853aee0 100644 --- a/packages/core/acl/src/acl.ts +++ b/packages/core/acl/src/acl.ts @@ -18,6 +18,7 @@ import { ACLRole, ResourceActionsOptions, RoleActionParams } from './acl-role'; import { AllowManager, ConditionFunc } from './allow-manager'; import FixedParamsManager, { Merger } from './fixed-params-manager'; import SnippetManager, { SnippetOptions } from './snippet-manager'; +import { NoPermissionError } from './errors/no-permission-error'; interface CanResult { role: string; @@ -92,6 +93,8 @@ export class ACL extends EventEmitter { protected middlewares: Toposort; + protected strategyResources: Set | null = null; + constructor() { super(); @@ -124,6 +127,25 @@ export class ACL extends EventEmitter { this.addCoreMiddleware(); } + setStrategyResources(resources: Array | null) { + this.strategyResources = new Set(resources); + } + + getStrategyResources() { + return this.strategyResources ? [...this.strategyResources] : null; + } + + appendStrategyResource(resource: string) { + if (!this.strategyResources) { + this.strategyResources = new Set(); + } + this.strategyResources.add(resource); + } + + removeStrategyResource(resource: string) { + this.strategyResources.delete(resource); + } + define(options: DefineOptions): ACLRole { const roleName = options.role; const role = new ACLRole(this, roleName); @@ -230,7 +252,11 @@ export class ACL extends EventEmitter { return null; } - let roleStrategyParams = roleStrategy?.allow(resource, this.resolveActionAlias(action)); + let roleStrategyParams; + + if (this.strategyResources === null || this.strategyResources.has(resource)) { + roleStrategyParams = roleStrategy?.allow(resource, this.resolveActionAlias(action)); + } if (!roleStrategyParams && snippetAllowed) { roleStrategyParams = {}; @@ -391,7 +417,7 @@ export class ACL extends EventEmitter { if (params?.filter?.createdById) { const collection = ctx.db.getCollection(resourceName); if (!collection || !collection.getField('createdById')) { - return lodash.omit(params, 'filter.createdById'); + throw new NoPermissionError('createdById field not found'); } } @@ -419,25 +445,34 @@ export class ACL extends EventEmitter { ctx.log?.debug && ctx.log.debug('acl params', params); - if (params && resourcerAction.mergeParams) { - const filteredParams = acl.filterParams(ctx, resourceName, params); - const parsedParams = await acl.parseJsonTemplate(filteredParams, ctx); - - ctx.permission.parsedParams = parsedParams; - ctx.log?.debug && ctx.log.debug('acl parsedParams', parsedParams); - ctx.permission.rawParams = lodash.cloneDeep(resourcerAction.params); - resourcerAction.mergeParams(parsedParams, { - appends: (x, y) => { - if (!x) { - return []; - } - if (!y) { - return x; - } - return (x as any[]).filter((i) => y.includes(i.split('.').shift())); - }, - }); - ctx.permission.mergedParams = lodash.cloneDeep(resourcerAction.params); + try { + if (params && resourcerAction.mergeParams) { + const filteredParams = acl.filterParams(ctx, resourceName, params); + const parsedParams = await acl.parseJsonTemplate(filteredParams, ctx); + + ctx.permission.parsedParams = parsedParams; + ctx.log?.debug && ctx.log.debug('acl parsedParams', parsedParams); + ctx.permission.rawParams = lodash.cloneDeep(resourcerAction.params); + resourcerAction.mergeParams(parsedParams, { + appends: (x, y) => { + if (!x) { + return []; + } + if (!y) { + return x; + } + return (x as any[]).filter((i) => y.includes(i.split('.').shift())); + }, + }); + ctx.permission.mergedParams = lodash.cloneDeep(resourcerAction.params); + } + } catch (e) { + if (e instanceof NoPermissionError) { + ctx.throw(403, 'No permissions'); + return; + } + + throw e; } await next(); diff --git a/packages/core/acl/src/errors/index.ts b/packages/core/acl/src/errors/index.ts new file mode 100644 index 0000000000000..9ce036b718322 --- /dev/null +++ b/packages/core/acl/src/errors/index.ts @@ -0,0 +1,10 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +export * from './no-permission-error'; diff --git a/packages/core/acl/src/errors/no-permission-error.ts b/packages/core/acl/src/errors/no-permission-error.ts new file mode 100644 index 0000000000000..76575306846b1 --- /dev/null +++ b/packages/core/acl/src/errors/no-permission-error.ts @@ -0,0 +1,10 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +export class NoPermissionError extends Error {} diff --git a/packages/core/acl/src/index.ts b/packages/core/acl/src/index.ts index 42f6b12f951d1..269a8f6678c1f 100644 --- a/packages/core/acl/src/index.ts +++ b/packages/core/acl/src/index.ts @@ -13,3 +13,4 @@ export * from './acl-available-strategy'; export * from './acl-resource'; export * from './acl-role'; export * from './skip-middleware'; +export * from './errors'; diff --git a/packages/core/acl/src/snippet-manager.ts b/packages/core/acl/src/snippet-manager.ts index 68083738b3ac6..0ed92f3b3ca5f 100644 --- a/packages/core/acl/src/snippet-manager.ts +++ b/packages/core/acl/src/snippet-manager.ts @@ -30,6 +30,12 @@ class SnippetManager { public snippets: Map = new Map(); register(snippet: SnippetOptions) { + const name = snippet.name; + // throw error if name include * or end with dot + if (name.includes('*') || name.endsWith('.')) { + throw new Error(`Invalid snippet name: ${name}, name should not include * or end with dot.`); + } + this.snippets.set(snippet.name, snippet); } diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/acl.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/acl.test.ts index 8c5404b91f118..3830d0cbac767 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/acl.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/acl.test.ts @@ -337,6 +337,7 @@ describe('acl', () => { forceUpdate: true, }); + app.acl.appendStrategyResource('posts'); expect( acl.can({ role: 'new', @@ -887,4 +888,27 @@ describe('acl', () => { expect(destroyResponse.statusCode).toEqual(200); expect(await db.getRepository('roles').findOne({ filterByTk: 'testRole' })).toBeNull(); }); + + it('should set acl strategy resources', async () => { + await db.getRepository('collections').create({ + values: { + name: 'posts', + fields: [ + { + name: 'title', + type: 'string', + }, + ], + }, + context: {}, + }); + + expect(app.acl.getStrategyResources()).toContain('posts'); + + await db.getRepository('collections').destroy({ + filterByTk: 'posts', + }); + + expect(app.acl.getStrategyResources()).not.toContain('posts'); + }); }); diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/middleware.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/middleware.test.ts index 1114b4381fa40..13678491d86c5 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/middleware.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/middleware.test.ts @@ -78,6 +78,46 @@ describe('middleware', () => { await app.destroy(); }); + it('should no permission when createdById field not exists in collection', async () => { + await db.getRepository('collections').create({ + values: { + name: 'foos', + autoGenId: false, + fields: [ + { + type: 'string', + name: 'name', + primaryKey: true, + }, + ], + }, + context: {}, + }); + + await db.getRepository('roles').update({ + filterByTk: 'admin', + values: { + strategy: { + actions: ['create', 'update:own'], + }, + }, + }); + + const response = await adminAgent.resource('foos').create({ + values: { + name: 'foo-name', + }, + }); + + expect(response.statusCode).toEqual(200); + + const updateRes = await adminAgent.resource('foos').update({ + filterByTk: response.body.data.name, + }); + + expect(updateRes.statusCode).toEqual(403); + }); + it('should throw 403 when no permission', async () => { const response = await app.agent().resource('posts').create({ values: {}, diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/own.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/own.test.ts index d8a70670e2566..9e5aba8951ff6 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/own.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/own.test.ts @@ -99,6 +99,7 @@ describe('own test', () => { }) .set({ Authorization: 'Bearer ' + adminToken }); + acl.appendStrategyResource('tests'); const response = await userAgent.get('/tests:list'); expect(response.statusCode).toEqual(200); }); @@ -113,6 +114,7 @@ describe('own test', () => { }, }); + acl.appendStrategyResource('posts'); let response = await userAgent.resource('posts').create({ values: { title: 't1', diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/prepare.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/prepare.ts index ea20d143f96fa..6afb546a0a8d8 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/prepare.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/prepare.ts @@ -15,5 +15,6 @@ export async function prepareApp(): Promise { acl: true, plugins: ['acl', 'error-handler', 'users', 'ui-schema-storage', 'data-source-main', 'auth', 'data-source-manager'], }); + return app; } diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/role.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/role.test.ts index 15c8c9bd1ecdb..33d5ba71e4e7c 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/role.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/role.test.ts @@ -48,6 +48,28 @@ describe('role api', () => { adminAgent = app.agent().login(admin); }); + it('should have permission to users collection with strategy', async () => { + await db.getRepository('roles').create({ + values: { + name: 'tests', + strategy: { + actions: ['view'], + }, + }, + }); + + const user1 = await db.getRepository('users').create({ + values: { + roles: ['tests'], + }, + }); + + const userAgent = app.agent().login(user1); + + const response = await userAgent.resource('users').list(); + expect(response.statusCode).toBe(200); + }); + it('should list actions', async () => { const response = await adminAgent.resource('availableActions').list(); expect(response.statusCode).toEqual(200); diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/with-acl-meta.ts b/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/with-acl-meta.ts index d0d5b4266a315..dee4ef4a35873 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/with-acl-meta.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/with-acl-meta.ts @@ -9,8 +9,7 @@ import lodash from 'lodash'; import { snakeCase } from '@nocobase/database'; - -class NoPermissionError extends Error {} +import { NoPermissionError } from '@nocobase/acl'; function createWithACLMetaMiddleware() { return async (ctx: any, next) => { diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/server.ts b/packages/plugins/@nocobase/plugin-acl/src/server/server.ts index a73d184645137..5a1149ac516eb 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/server.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/server.ts @@ -106,6 +106,8 @@ export class PluginACLServer extends Plugin { 'dataSources:list', 'roles.dataSourcesCollections:*', 'roles.dataSourceResources:*', + 'dataSourcesRolesResourcesScopes:*', + 'rolesResourcesScopes:*', ], }); @@ -576,6 +578,22 @@ export class PluginACLServer extends Plugin { }, { after: 'dataSource', group: 'with-acl-meta' }, ); + + this.db.on('afterUpdateCollection', async (collection) => { + if (collection.options.loadedFromCollectionManager) { + this.app.acl.appendStrategyResource(collection.name); + } + }); + + this.db.on('afterDefineCollection', async (collection) => { + if (collection.options.loadedFromCollectionManager) { + this.app.acl.appendStrategyResource(collection.name); + } + }); + + this.db.on('afterRemoveCollection', (collection) => { + this.app.acl.removeStrategyResource(collection.name); + }); } async install() { diff --git a/packages/plugins/@nocobase/plugin-data-source-main/src/server/repositories/collection-repository.ts b/packages/plugins/@nocobase/plugin-data-source-main/src/server/repositories/collection-repository.ts index b1042816f3075..a8161a409eecd 100644 --- a/packages/plugins/@nocobase/plugin-data-source-main/src/server/repositories/collection-repository.ts +++ b/packages/plugins/@nocobase/plugin-data-source-main/src/server/repositories/collection-repository.ts @@ -173,6 +173,7 @@ export class CollectionRepository extends Repository { const options = collection.options; const fields = []; + for (const [name, field] of collection.fields) { fields.push({ name, diff --git a/packages/plugins/@nocobase/plugin-users/src/server/server.ts b/packages/plugins/@nocobase/plugin-users/src/server/server.ts index f8339eb79146b..54aa0f0c44f09 100644 --- a/packages/plugins/@nocobase/plugin-users/src/server/server.ts +++ b/packages/plugins/@nocobase/plugin-users/src/server/server.ts @@ -148,7 +148,7 @@ export default class PluginUsersServer extends Plugin { loggedInActions.forEach((action) => this.app.acl.allow('users', action, 'loggedIn')); this.app.acl.registerSnippet({ - name: `pm.${this.name}.*`, + name: `pm.${this.name}`, actions: ['users:*'], }); } @@ -200,6 +200,7 @@ export default class PluginUsersServer extends Plugin { async install(options) { const { rootNickname, rootPassword, rootEmail, rootUsername } = this.getInstallingData(options); const User = this.db.getCollection('users'); + if (await User.repository.findOne({ filter: { email: rootEmail } })) { return; } @@ -214,6 +215,7 @@ export default class PluginUsersServer extends Plugin { }); const repo = this.db.getRepository('collections'); + if (repo) { await repo.db2cm('users'); } diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts index 5107ea734ee4f..2a638225c1f91 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts @@ -229,7 +229,7 @@ export default class PluginWorkflowServer extends Plugin { }); this.app.acl.registerSnippet({ - name: 'ui.*', + name: 'ui.workflows', actions: ['workflows:list'], });