refactor: enhance role merging logic: prioritize strategy actions over existing actions

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

@@ -30,9 +30,71 @@ export function mergeRole(roles: ACLRole[]) {
     }
   }
   result.snippets = mergeRoleSnippets(allSnippets);
+  adjustActionByStrategy(roles, result);
   return result;
 }
 
+/**
+ * When merging permissions from multiple roles, if strategy.actions allows certain actions, then those actions have higher priority.
+ * For example, [
+ * {
+ *  actions: {
+ *    'users:view': {...},
+ *    'users:create': {...}
+ *  },
+ *  strategy: {
+ *    actions: ['view']
+ *  }
+ * }]
+ * finally result: [{
+ *  actions: {
+ *    'users:create': {...},
+ * },
+ * {
+ *  strategy: {
+ *    actions: ['view']
+ * }]
+ **/
+function adjustActionByStrategy(
+  roles,
+  result: { actions?: Record<string, object>; strategy?: { actions?: string[] } },
+) {
+  const { actions, strategy } = result;
+  if (_.isEmpty(actions) || _.isEmpty(strategy?.actions)) {
+    return;
+  }
+  const adjustActions = calcAdjustActions(roles);
+  if (!adjustActions.length) {
+    return;
+  }
+  for (const key of Object.keys(actions)) {
+    const needRemove = adjustActions.includes(key.split(':')[1]);
+    if (needRemove) {
+      delete actions[key];
+    }
+  }
+}
+
+function calcAdjustActions(roles) {
+  const adjustActionSet = new Set();
+  for (const role of roles) {
+    const r = role.toJSON();
+    if (_.isEmpty(r.actions) && r.strategy?.actions?.length) {
+      r.strategy.actions.forEach((x) => adjustActionSet.add(x));
+      continue;
+    }
+    if (!_.isEmpty(r.actions) && r.strategy?.actions?.length) {
+      for (const action of r.strategy.actions) {
+        const exist = Object.keys(r.actions).some((key) => key.split(':')[1] === action);
+        if (!exist) {
+          adjustActionSet.add(action);
+        }
+      }
+    }
+  }
+  return [...adjustActionSet];
+}
+
 function mergeRoleNames(sourceRoleNames, newRoleName) {
   return newRoleName ? sourceRoleNames.concat(newRoleName) : sourceRoleNames;
 }

packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts

@@ -530,4 +530,147 @@ describe('union role: full permissions', async () => {
     expect(createRoleResponse.statusCode).toBe(200);
     expect(createRoleResponse.body.data.role).not.toBe(UNION_ROLE_KEY);
   });
+
+  it('should general action permissions override specific resource permissions when using union role #1924', async () => {
+    const rootAgent = await app.agent().login(rootUser);
+    await rootAgent
+      .post(`/dataSources/main/roles:update`)
+      .query({
+        filterByTk: role1.name,
+      })
+      .send({
+        roleName: role1.name,
+        strategy: {
+          actions: ['view'],
+        },
+        dataSourceKey: 'main',
+      });
+
+    const ownDataSourceScopeRole = await db.getRepository('dataSourcesRolesResourcesScopes').findOne({
+      where: {
+        key: 'own',
+        dataSourceKey: 'main',
+      },
+    });
+    const scopeFields = ['id', 'createdBy', 'createdById'];
+    const dataSourceResourcesResponse = await rootAgent
+      .post(`/roles/${role2.name}/dataSourceResources:create`)
+      .query({
+        filterByTk: 'users',
+        filter: {
+          dataSourceKey: 'main',
+          name: 'users',
+        },
+      })
+      .send({
+        usingActionsConfig: true,
+        actions: [
+          {
+            name: 'view',
+            fields: scopeFields,
+            scope: {
+              id: ownDataSourceScopeRole.id,
+              createdAt: '2025-02-19T08:57:17.385Z',
+              updatedAt: '2025-02-19T08:57:17.385Z',
+              key: 'own',
+              dataSourceKey: 'main',
+              name: '{{t("Own records")}}',
+              resourceName: null,
+              scope: {
+                createdById: '{{ ctx.state.currentUser.id }}',
+              },
+            },
+          },
+        ],
+        name: 'users',
+        dataSourceKey: 'main',
+      });
+    expect(dataSourceResourcesResponse.statusCode).toBe(200);
+
+    agent = await app.agent().login(user, UNION_ROLE_KEY);
+    const rolesResponse = await agent.resource('roles').check();
+    expect(rolesResponse.status).toBe(200);
+    expect(rolesResponse.body.data.actions).toStrictEqual({});
+  });
+
+  it('should verify actions configuration for union role with specific scopes', async () => {
+    const rootAgent = await app.agent().login(rootUser);
+    await rootAgent
+      .post(`/dataSources/main/roles:update`)
+      .query({
+        filterByTk: role1.name,
+      })
+      .send({
+        roleName: role1.name,
+        strategy: {
+          actions: ['view', 'create:own1'],
+        },
+        dataSourceKey: 'main',
+      });
+
+    const ownDataSourceScopeRole = await db.getRepository('dataSourcesRolesResourcesScopes').findOne({
+      where: {
+        key: 'own',
+        dataSourceKey: 'main',
+      },
+    });
+    const scopeFields = ['id', 'createdBy', 'createdById'];
+    const dataSourceResourcesResponse = await rootAgent
+      .post(`/roles/${role2.name}/dataSourceResources:create`)
+      .query({
+        filterByTk: 'users',
+        filter: {
+          dataSourceKey: 'main',
+          name: 'users',
+        },
+      })
+      .send({
+        usingActionsConfig: true,
+        actions: [
+          {
+            name: 'view',
+            fields: scopeFields,
+            scope: {
+              id: ownDataSourceScopeRole.id,
+              createdAt: '2025-02-19T08:57:17.385Z',
+              updatedAt: '2025-02-19T08:57:17.385Z',
+              key: 'own',
+              dataSourceKey: 'main',
+              name: '{{t("Own records")}}',
+              resourceName: null,
+              scope: {
+                createdById: '{{ ctx.state.currentUser.id }}',
+              },
+            },
+          },
+          {
+            name: 'create',
+            fields: scopeFields,
+            scope: {
+              id: ownDataSourceScopeRole.id,
+              createdAt: '2025-02-19T08:57:17.385Z',
+              updatedAt: '2025-02-19T08:57:17.385Z',
+              key: 'own',
+              dataSourceKey: 'main',
+              name: '{{t("Own records")}}',
+              resourceName: null,
+              scope: {
+                createdById: '{{ ctx.state.currentUser.id }}',
+              },
+            },
+          },
+        ],
+        name: 'users',
+        dataSourceKey: 'main',
+      });
+    expect(dataSourceResourcesResponse.statusCode).toBe(200);
+
+    agent = await app.agent().login(user, UNION_ROLE_KEY);
+    const rolesResponse = await agent.resource('roles').check();
+    expect(rolesResponse.status).toBe(200);
+    expect(rolesResponse.body.data.actions).toHaveProperty('users:create');
+    expect(rolesResponse.body.data.actions).not.toHaveProperty('users:view');
+    expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('filter');
+    expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('whitelist');
+  });
 });