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

@@ -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();
+  });
 });

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();
+    });
   });
 });

packages/core/acl/src/acl-role.ts

@@ -107,6 +107,7 @@ export class ACLRole {
 
   public effectiveSnippets(): { allowed: Array<string>; rejected: Array<string> } {
     const currentParams = this._serializeSet(this.snippets);
+
     if (this._snippetCache.params === currentParams) {
       return this._snippetCache.result;
     }

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<any>;
 
+  protected strategyResources: Set<string> | null = null;
+
   constructor() {
     super();
 
@@ -124,6 +127,25 @@ export class ACL extends EventEmitter {
     this.addCoreMiddleware();
   }
 
+  setStrategyResources(resources: Array<string> | 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();

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';

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 {}

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';

packages/core/acl/src/snippet-manager.ts

@@ -30,6 +30,12 @@ class SnippetManager {
   public snippets: Map<string, Snippet> = 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);
   }
 

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');
+  });
 });

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: {},