Fix acl target action error (#311)

* fix: field association resource name

* chore: resourceCollection fields unique index

* fix: test

* feat: allowConfigure permission skip

* feat: skip with array type actionNames

* chore: rename acl skip to allow

* fix: type

* chore: rename SkipManager to AllowManager

packages/core/acl/src/__tests__/allow.test.ts

@@ -26,7 +26,7 @@ describe('skip', () => {
     await middlewareFunc(ctx, nextFunc);
     expect(nextFunc).toHaveBeenCalledTimes(0);
 
-    acl.skip('users', 'login');
+    acl.allow('users', 'login');
 
     await middlewareFunc(ctx, nextFunc);
     expect(nextFunc).toHaveBeenCalledTimes(1);
@@ -49,7 +49,7 @@ describe('skip', () => {
     const nextFunc = jest.fn();
 
     let skip = false;
-    acl.skip('users', 'login', (ctx) => {
+    acl.allow('users', 'login', (ctx) => {
       return skip;
     });
 
@@ -65,7 +65,7 @@ describe('skip', () => {
     const middlewareFunc = acl.middleware();
 
     const conditionFn = jest.fn();
-    acl.skipManager.registerSkipCondition('superUser', () => {
+    acl.allowManager.registerAllowCondition('superUser', async () => {
       conditionFn();
       return true;
     });
@@ -84,7 +84,7 @@ describe('skip', () => {
 
     const nextFunc = jest.fn();
 
-    acl.skip('users', 'login', 'superUser');
+    acl.allow('users', 'login', 'superUser');
 
     await middlewareFunc(ctx, nextFunc);
     expect(nextFunc).toHaveBeenCalledTimes(1);

packages/core/acl/src/acl.ts

@@ -5,7 +5,7 @@ import lodash from 'lodash';
 import { AclAvailableAction, AvailableActionOptions } from './acl-available-action';
 import { ACLAvailableStrategy, AvailableStrategyOptions, predicate } from './acl-available-strategy';
 import { ACLRole, RoleActionParams } from './acl-role';
-import { SkipManager } from './skip-manager';
+import { AllowManager } from './allow-manager';
 const parse = require('json-templates');
 
 interface CanResult {
@@ -47,7 +47,7 @@ export class ACL extends EventEmitter {
   protected availableStrategy = new Map<string, ACLAvailableStrategy>();
   protected middlewares = [];
 
-  public skipManager = new SkipManager(this);
+  public allowManager = new AllowManager(this);
 
   roles = new Map<string, ACLRole>();
 
@@ -85,7 +85,7 @@ export class ACL extends EventEmitter {
       }
     });
 
-    this.middlewares.push(this.skipManager.aclMiddleware());
+    this.middlewares.push(this.allowManager.aclMiddleware());
   }
 
   define(options: DefineOptions): ACLRole {
@@ -217,8 +217,14 @@ export class ACL extends EventEmitter {
     this.middlewares.push(fn);
   }
 
-  skip(resourceName: string, actionName: string, condition?: any) {
+  allow(resourceName: string, actionNames: string[] | string, condition?: any) {
-    this.skipManager.skip(resourceName, actionName, condition);
+    if (!Array.isArray(actionNames)) {
+      actionNames = [actionNames];
+    }
+
+    for (const actionName of actionNames) {
+      this.allowManager.allow(resourceName, actionName, condition);
+    }
   }
 
   parseJsonTemplate(json: any, ctx: any) {
@@ -243,12 +249,6 @@ export class ACL extends EventEmitter {
       return params;
     };
 
-    const ctxToObject = (ctx) => {
-      return {
-        state: JSON.parse(JSON.stringify(ctx.state)),
-      };
-    };
-
     return async function ACLMiddleware(ctx, next) {
       const roleName = ctx.state.currentRole || 'anonymous';
       const { resourceName, actionName } = ctx.action;

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

@@ -1,26 +1,39 @@
 import { ACL } from './acl';
 
-type ConditionFunc = (ctx: any) => boolean;
+type ConditionFunc = (ctx: any) => Promise<boolean>;
 
-export class SkipManager {
+export class AllowManager {
   protected skipActions = new Map<string, Map<string, string | ConditionFunc | true>>();
 
   protected registeredCondition = new Map<string, ConditionFunc>();
 
   constructor(public acl: ACL) {
-    this.registerSkipCondition('logged-in', (ctx) => {
+    this.registerAllowCondition('loggedIn', (ctx) => {
       return ctx.state.currentUser;
     });
+
+    this.registerAllowCondition('allowConfigure', async (ctx) => {
+      const roleName = ctx.state.currentRole;
+      if (!roleName) {
+        return false;
+      }
+
+      const roleInstance = await ctx.db.getRepository('roles').findOne({
+        name: roleName,
+      });
+
+      return roleInstance?.get('allowConfigure');
+    });
   }
 
-  skip(resourceName: string, actionName: string, condition?: string | ConditionFunc) {
+  allow(resourceName: string, actionName: string, condition?: string | ConditionFunc) {
     const actionMap = this.skipActions.get(resourceName) || new Map<string, string | ConditionFunc>();
     actionMap.set(actionName, condition || true);
 
     this.skipActions.set(resourceName, actionMap);
   }
 
-  getSkippedConditions(resourceName: string, actionName: string): Array<ConditionFunc | true> {
+  getAllowedConditions(resourceName: string, actionName: string): Array<ConditionFunc | true> {
     const fetchActionSteps: string[] = ['*', resourceName];
 
     const results = [];
@@ -38,14 +51,14 @@ export class SkipManager {
     return results;
   }
 
-  registerSkipCondition(name: string, condition: ConditionFunc) {
+  registerAllowCondition(name: string, condition: ConditionFunc) {
     this.registeredCondition.set(name, condition);
   }
 
   aclMiddleware() {
     return async (ctx, next) => {
       const { resourceName, actionName } = ctx.action;
-      const skippedConditions = ctx.app.acl.skipManager.getSkippedConditions(resourceName, actionName);
+      const skippedConditions = ctx.app.acl.allowManager.getAllowedConditions(resourceName, actionName);
       let skip = false;
 
       for (const skippedCondition of skippedConditions) {
@@ -53,7 +66,7 @@ export class SkipManager {
           let skipResult = false;
 
           if (typeof skippedCondition === 'function') {
-            skipResult = skippedCondition(ctx);
+            skipResult = await skippedCondition(ctx);
           } else if (skippedCondition) {
             skipResult = true;
           }

packages/plugins/acl/src/__tests__/configuration.test.ts

@@ -0,0 +1,45 @@
+import { MockServer } from '@nocobase/test';
+import { Database } from '@nocobase/database';
+import { ACL } from '@nocobase/acl';
+import { UiSchemaRepository } from '@nocobase/plugin-ui-schema-storage';
+import { changeMockRole, prepareApp } from './prepare';
+
+describe('configuration', () => {
+  let app: MockServer;
+  let db: Database;
+  let acl: ACL;
+
+  let uiSchemaRepository: UiSchemaRepository;
+
+  afterEach(async () => {
+    await app.destroy();
+  });
+
+  beforeEach(async () => {
+    app = await prepareApp();
+    db = app.db;
+    acl = app.acl;
+
+    uiSchemaRepository = db.getRepository('uiSchemas');
+  });
+
+  it('should list collections', async () => {
+    expect((await app.agent().resource('collections').create()).statusCode).toEqual(403);
+    expect((await app.agent().resource('collections').list()).statusCode).toEqual(200);
+  });
+
+  it('should allow when role has allowConfigure with true value', async () => {
+    await db.getRepository('roles').create({
+      values: {
+        name: 'admin1',
+        title: 'admin allowConfigure',
+        allowConfigure: true,
+      },
+    });
+
+    changeMockRole('admin1');
+
+    expect((await app.agent().resource('collections').create()).statusCode).toEqual(200);
+    expect((await app.agent().resource('collections').list()).statusCode).toEqual(200);
+  });
+});

packages/plugins/acl/src/collections/rolesResources.ts

@@ -3,6 +3,12 @@ import { CollectionOptions } from '@nocobase/database';
 export default {
   name: 'rolesResources',
   model: 'RoleResourceModel',
+  indexes: [
+    {
+      unique: true,
+      fields: ['roleName', 'name'],
+    },
+  ],
   fields: [
     {
       type: 'belongsTo',

packages/plugins/acl/src/model/RoleResourceActionModel.ts

@@ -14,6 +14,7 @@ export class RoleResourceActionModel extends Model {
     const db: Database = this.constructor.database;
 
     const { resourceName, role, acl, associationFieldsActions, grantHelper } = options;
+
     const actionName = this.get('name') as string;
 
     const fields = this.get('fields') as any;
@@ -47,17 +48,19 @@ export class RoleResourceActionModel extends Model {
 
       const fieldActions: AssociationFieldAction = associationFieldsActions?.[fieldType]?.[availableAction];
 
+      const fieldTarget = collectionField.get('target');
+
       if (fieldActions) {
         const associationActions = fieldActions.associationActions || [];
         associationActions.forEach((associationAction) => {
-          const actionName = `${resourceName}.${field}:${associationAction}`;
+          const actionName = `${resourceName}.${fieldTarget}:${associationAction}`;
           role.grantAction(actionName);
         });
 
         const targetActions = fieldActions.targetActions || [];
 
         targetActions.forEach((targetAction) => {
-          const targetActionPath = `${field}:${targetAction}`;
+          const targetActionPath = `${fieldTarget}:${targetAction}`;
 
           grantHelper.resourceTargetActionMap.set(resourceName, [
             ...(grantHelper.resourceTargetActionMap.get(resourceName) || []),

packages/plugins/acl/src/model/RoleResourceModel.ts

@@ -35,6 +35,7 @@ export class RoleResourceModel extends Model {
     const roleName = this.get('roleName') as string;
     const role = acl.getRole(roleName);
 
+    // revoke resource of role
     await this.revoke({ role, resourceName, grantHelper });
 
     // @ts-ignore

packages/plugins/acl/src/server.ts

@@ -293,11 +293,13 @@ export class PluginACL extends Plugin {
         ],
       });
     });
-    this.app.acl.skip('roles.menuUiSchemas', 'set', 'logged-in');
+
-    this.app.acl.skip('roles.menuUiSchemas', 'toggle', 'logged-in');
+    this.app.acl.allow('roles', 'check', 'loggedIn');
-    this.app.acl.skip('roles.menuUiSchemas', 'list', 'logged-in');
+    this.app.acl.allow('roles', ['create', 'update', 'destroy'], 'allowConfigure');
-    this.app.acl.skip('roles', 'check', 'logged-in');
+
-    this.app.acl.skip('*', '*', (ctx) => {
+    this.app.acl.allow('roles.menuUiSchemas', ['set', 'toggle', 'list'], 'allowConfigure');
+
+    this.app.acl.allow('*', '*', (ctx) => {
       return ctx.state.currentRole === 'root';
     });
 

packages/plugins/china-region/src/server.ts

@@ -11,7 +11,7 @@ export class ChinaRegionPlugin extends Plugin {
     await this.db.import({
       directory: resolve(__dirname, 'collections'),
     });
-    this.app.acl.skip('chinaRegions', 'list', 'logged-in');
+    this.app.acl.allow('chinaRegions', 'list', 'loggedIn');
   }
 
   async importData() {

packages/plugins/collection-manager/src/plugin.ts

@@ -235,7 +235,8 @@ export class CollectionManagerPlugin extends Plugin {
       await next();
     });
 
-    this.app.acl.skip('collections', 'list', 'logged-in');
+    this.app.acl.allow('collections', 'list', 'loggedIn');
+    this.app.acl.allow('collections', ['create', 'update', 'destroy'], 'allowConfigure');
   }
 
   async load() {

packages/plugins/file-manager/src/server.ts

@@ -36,7 +36,7 @@ export default class PluginFileManager extends Plugin {
       await getStorageConfig(STORAGE_TYPE_LOCAL).middleware(this.app);
     }
 
-    this.app.acl.skip('attachments', 'upload', 'logged-in');
+    this.app.acl.allow('attachments', 'upload', 'loggedIn');
   }
 
   getName(): string {

packages/plugins/ui-schema-storage/src/server.ts

@@ -61,8 +61,8 @@ export class UiSchemaStoragePlugin extends Plugin {
       actions: uiSchemaActions,
     });
 
-    this.app.acl.skip('uiSchemas', '*', 'logged-in');
+    this.app.acl.allow('uiSchemas', '*', 'loggedIn');
-    this.app.acl.skip('uiSchemaTemplates', '*', 'logged-in');
+    this.app.acl.allow('uiSchemaTemplates', '*', 'loggedIn');
   }
 
   async load() {

packages/plugins/users/src/__tests__/fields.test.ts

@@ -11,6 +11,7 @@ describe('createdBy/updatedBy', () => {
     api = mockServer();
     api.plugin(UsersPlugin, userPluginConfig);
     api.plugin(PluginACL);
+    await api.cleanDb();
     await api.loadAndInstall();
     db = api.db;
   });

packages/plugins/users/src/server.ts

@@ -96,8 +96,8 @@ export default class UsersPlugin extends Plugin<UserPluginConfig> {
     const publicActions = ['check', 'signin', 'signup', 'lostpassword', 'resetpassword', 'getUserByResetToken'];
     const loggedInActions = ['signout', 'updateProfile', 'changePassword', 'setDefaultRole'];
 
-    publicActions.forEach((action) => this.app.acl.skip('users', action));
+    publicActions.forEach((action) => this.app.acl.allow('users', action));
-    loggedInActions.forEach((action) => this.app.acl.skip('users', action, 'logged-in'));
+    loggedInActions.forEach((action) => this.app.acl.allow('users', action, 'loggedIn'));
   }
 
   async load() {