fix: correct role union calculation logic (#6605)

* refactor: enhance role merging logic: prioritize strategy actions over existing actions * refactor: union role acl * refactor: union role acl * refactor: union role acl * fix: assign getKeys * fix: assign * refactor: switch role set default role record * refactor: switch role set default role record * refactor: switch role set default role record * refactor: switch role set default role record * refactor: add setCurrentRole cache * fix: e2e test * fix: setCurrentRole * refactor: switch role --------- Co-authored-by: chenos <chenlinxh@gmail.com>
2025-05-05 13:39:24 +08:00 · 2025-04-09 19:06:05 +08:00 · 2025-04-09 19:06:05 +08:00 · 41961fb923
commit 41961fb923
parent 2d9545a719
9 changed files with 403 additions and 31 deletions
--- a/packages/core/acl/src/utils/acl-role.ts
+++ b/packages/core/acl/src/utils/acl-role.ts
@ -30,9 +30,65 @@ 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': {...},
+ *    'users:view': {} // all view
+ * },
+ * {
+ *  strategy: {
+ *    actions: ['view']
+ * }]
+ **/
+function adjustActionByStrategy(
+  roles,
+  result: {
+    actions?: Record<string, object>;
+    strategy?: { actions?: string[] };
+    resources?: string[];
+  },
+) {
+  const { actions, strategy } = result;
+  const actionSet = getAdjustActions(roles);
+  if (!_.isEmpty(actions) && !_.isEmpty(strategy?.actions) && !_.isEmpty(result.resources)) {
+    for (const resource of result.resources) {
+      for (const action of strategy.actions) {
+        if (actionSet.has(action)) {
+          actions[`${resource}:${action}`] = {};
+        }
+      }
+    }
+  }
+}
+
+function getAdjustActions(roles: ACLRole[]) {
+  const actionSet = new Set<string>();
+  for (const role of roles) {
+    const jsonRole = role.toJSON();
+    // Within the same role, actions have higher priority than strategy.actions.
+    if (!_.isEmpty(jsonRole.strategy?.['actions']) && _.isEmpty(jsonRole.actions)) {
+      jsonRole.strategy['actions'].forEach((x) => !x.includes('own') && actionSet.add(x));
+    }
+  }
+  return actionSet;
+}
+
 function mergeRoleNames(sourceRoleNames, newRoleName) {
  return newRoleName ? sourceRoleNames.concat(newRoleName) : sourceRoleNames;
 }
--- a/packages/core/utils/src/tests/assign.test.ts
+++ b/packages/core/utils/src/tests/assign.test.ts
@ -315,6 +315,34 @@ describe('merge strategy', () => {
        key1: 'val1 + val2',
      });
    });
+    it('case 2', () => {
+      const obj = assign(
+        {
+          filter: { a: 'a2' },
+        },
+        {},
+        {
+          filter: () => '123',
+        },
+      );
+      expect(obj).toMatchObject({
+        filter: '123',
+      });
+    });
+    it('case 3', () => {
+      const obj = assign(
+        {},
+        {
+          filter: { a: 'a2' },
+        },
+        {
+          filter: () => '123',
+        },
+      );
+      expect(obj).toMatchObject({
+        filter: '123',
+      });
+    });
  });

  describe('merge', () => {
@ -416,4 +444,32 @@ describe('merge strategy', () => {
      });
    });
  });
+
+  describe('source is empty', () => {
+    it('case 1', () => {
+      const obj = assign(
+        {
+          resourceName: 'uiSchemas',
+          resourceIndex: 'n0jylid5rqa',
+          actionName: 'getJsonSchema',
+          values: {},
+        },
+        {},
+        {
+          filter: 'andMerge',
+          fields: 'intersect',
+          except: 'union',
+          whitelist: 'intersect',
+          blacklist: 'intersect',
+          sort: 'overwrite',
+        },
+      );
+      expect(obj).toMatchObject({
+        resourceName: 'uiSchemas',
+        resourceIndex: 'n0jylid5rqa',
+        actionName: 'getJsonSchema',
+        values: {},
+      });
+    });
+  });
 });
--- a/packages/core/utils/src/assign.ts
+++ b/packages/core/utils/src/assign.ts
@ -8,7 +8,7 @@
 */

 import deepmerge from 'deepmerge';
-import lodash from 'lodash';
+import _ from 'lodash';
 import { isPlainObject } from './common';

 type MergeStrategyType = 'merge' | 'deepMerge' | 'overwrite' | 'andMerge' | 'orMerge' | 'intersect' | 'union';
@ -88,7 +88,7 @@ mergeStrategies.set('union', (x, y) => {
  if (typeof y === 'string') {
    y = y.split(',');
  }
-  return lodash.uniq((x || []).concat(y || [])).filter(Boolean);
+  return _.uniq((x || []).concat(y || [])).filter(Boolean);
 });

 mergeStrategies.set('intersect', (x, y) =>
@ -110,15 +110,22 @@ mergeStrategies.set('intersect', (x, y) =>
 );

 export function assign(target: any, source: any, strategies: MergeStrategies = {}) {
-  getKeys(source).forEach((sourceKey) => {
+  const sourceKeys = getKeys(source);
+  const targetKeys = getKeys(target);
+  _.uniq([...sourceKeys, ...targetKeys]).forEach((sourceKey) => {
    const strategy = strategies[sourceKey];
-    let func = mergeStrategies.get('deepMerge');
+    let func: any;
    if (typeof strategy === 'function') {
      func = strategy;
    } else if (typeof strategy === 'string' && mergeStrategies.has(strategy as any)) {
      func = mergeStrategies.get(strategy as any);
    }
-    target[sourceKey] = func(target[sourceKey], source[sourceKey]);
+    if (func) {
+      target[sourceKey] = func(target[sourceKey], source[sourceKey]);
+    } else if (sourceKeys.includes(sourceKey)) {
+      const func = mergeStrategies.get('deepMerge');
+      target[sourceKey] = func(target[sourceKey], source[sourceKey]);
+    }
  });
  return target;
 }
--- a/packages/plugins/@nocobase/plugin-acl/src/client/SwitchRole.tsx
+++ b/packages/plugins/@nocobase/plugin-acl/src/client/SwitchRole.tsx
@ -17,14 +17,20 @@ import {
  useCurrentRoleMode,
 } from '@nocobase/client';
 import { Divider } from 'antd';
+import _ from 'lodash';

 export const SwitchRole = () => {
  const { t } = useTranslation();
  const api = useAPIClient();
-  const roles = useCurrentRoles();
+  const roles = _.cloneDeep(useCurrentRoles());
  const roleMode = useCurrentRoleMode();
  const currentRole = roles.find((role) => role.name === api.auth.role)?.name;
-
+  if (roleMode === 'allow-use-union') {
+    roles.unshift({
+      name: '__union__',
+      title: t('Full permissions', { ns: 'acl' }),
+    });
+  }
  // 当角色数量小于等于1 或者 是仅使用合并角色模式时，不显示切换角色选项
  if (roles.length <= 1 || roleMode === 'only-use-union') {
    return null;
--- a/packages/plugins/@nocobase/plugin-acl/src/server/tests/role-user.test.ts
+++ b/packages/plugins/@nocobase/plugin-acl/src/server/tests/role-user.test.ts
@ -11,6 +11,8 @@ import Database, { BelongsToManyRepository } from '@nocobase/database';
 import UsersPlugin from '@nocobase/plugin-users';
 import { createMockServer, MockServer } from '@nocobase/test';
 import jwt from 'jsonwebtoken';
+import { SystemRoleMode } from '../enum';
+import { UNION_ROLE_KEY } from '../constants';

 describe('role', () => {
  let api: MockServer;
@ -20,7 +22,7 @@ describe('role', () => {

  beforeEach(async () => {
    api = await createMockServer({
-      plugins: ['field-sort', 'users', 'acl', 'auth', 'data-source-manager', 'system-settings'],
+      plugins: ['field-sort', 'users', 'acl', 'auth', 'data-source-manager', 'system-settings', 'ui-schema-storage'],
    });
    db = api.db;
    usersPlugin = api.getPlugin('users');
@ -163,4 +165,77 @@ describe('role', () => {

    expect(defaultRole['name']).toEqual('test2');
  });
+
+  it('should set users default role is __union__', async () => {
+    const role = await db.getRepository('roles').create({
+      values: {
+        name: 'test',
+        title: 'test user',
+        allowConfigure: true,
+      },
+    });
+    const user = await db.getRepository('users').create({
+      values: {
+        token: '123',
+        roles: [role.name],
+      },
+    });
+
+    const repo = db.getRepository('systemSettings');
+    await repo.update({
+      filter: { id: 1 },
+      values: {
+        roleMode: SystemRoleMode.allowUseUnion,
+      },
+    });
+    const userToken = jwt.sign({ userId: user.get('id') }, 'test-key');
+    const response = await api
+      .agent()
+      .post('/users:setDefaultRole')
+      .send({
+        roleName: UNION_ROLE_KEY,
+      })
+      .set({
+        Authorization: `Bearer ${userToken}`,
+        'X-Authenticator': 'basic',
+      });
+
+    expect(response.statusCode).toEqual(200);
+    let userRole = await db.getRepository('rolesUsers').findOne({ where: { userId: user.get('id'), default: true } });
+    expect(userRole.roleName).toEqual(UNION_ROLE_KEY);
+
+    // switch
+    const response1 = await api
+      .agent()
+      .post('/users:setDefaultRole')
+      .send({
+        roleName: role.name,
+      })
+      .set({
+        Authorization: `Bearer ${userToken}`,
+        'X-Authenticator': 'basic',
+      });
+
+    expect(response1.statusCode).toEqual(200);
+    userRole = await db.getRepository('rolesUsers').findOne({ where: { userId: user.get('id'), default: true } });
+    expect(userRole.roleName).toEqual(role.name);
+
+    const response2 = await api
+      .agent()
+      .post('/users:setDefaultRole')
+      .send({
+        roleName: UNION_ROLE_KEY,
+      })
+      .set({
+        Authorization: `Bearer ${userToken}`,
+        'X-Authenticator': 'basic',
+      });
+
+    expect(response2.statusCode).toEqual(200);
+    userRole = await db.getRepository('rolesUsers').findOne({ where: { userId: user.get('id'), default: true } });
+    expect(userRole.roleName).toEqual(UNION_ROLE_KEY);
+    const agent = await api.agent().login(user);
+    const response3 = await agent.resource('roles').check();
+    expect(response3.statusCode).toEqual(200);
+  });
 });
--- a/packages/plugins/@nocobase/plugin-acl/src/server/tests/union-role.test.ts
+++ b/packages/plugins/@nocobase/plugin-acl/src/server/tests/union-role.test.ts
@ -530,4 +530,150 @@ 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['users:view']).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:own', 'update'],
+        },
+        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).toHaveProperty('users:view');
+    expect(rolesResponse.body.data.actions['users:view']).toStrictEqual({});
+    expect(rolesResponse.body.data.actions).not.toHaveProperty('users:create:own');
+    expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('filter');
+    expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('whitelist');
+    expect(rolesResponse.body.data.actions['users:update']).toStrictEqual({});
+  });
 });
--- a/packages/plugins/@nocobase/plugin-acl/src/server/actions/user-setDefaultRole.ts
+++ b/packages/plugins/@nocobase/plugin-acl/src/server/actions/user-setDefaultRole.ts
@ -29,25 +29,47 @@ export async function setDefaultRole(ctx: Context, next: Next) {
  const repository = db.getRepository('rolesUsers');

  await db.sequelize.transaction(async (transaction) => {
-    await repository.update({
+    const currentUserDefaultRole = await repository.findOne({
      filter: {
        userId: currentUser.id,
-      },
-      values: {
-        default: false,
-      },
-      transaction,
-    });
-    await repository.update({
-      filter: {
-        userId: currentUser.id,
-        roleName,
-      },
-      values: {
        default: true,
      },
      transaction,
    });
+
+    if (currentUserDefaultRole?.roleName === roleName) {
+      return;
+    }
+
+    if (currentUserDefaultRole) {
+      await repository.model.update(
+        { default: false },
+        { where: { userId: currentUser.id, roleName: currentUserDefaultRole.roleName }, transaction },
+      );
+    }
+
+    const targetUserRole = await repository.findOne({
+      filter: {
+        userId: currentUser.id,
+        roleName,
+      },
+      transaction,
+    });
+    let model;
+    if (targetUserRole) {
+      await repository.model.update({ default: true }, { where: { userId: currentUser.id, roleName }, transaction });
+      model = targetUserRole.set('default', true);
+    } else {
+      model = await repository.create({
+        values: {
+          userId: currentUser.id,
+          roleName,
+          default: true,
+        },
+        transaction,
+      });
+    }
+    db.emitAsync('rolesUsers.afterSave', model);
  });

  ctx.body = 'ok';
--- a/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts
+++ b/packages/plugins/@nocobase/plugin-acl/src/server/middlewares/setCurrentRole.ts
@ -47,7 +47,9 @@ export async function setCurrentRole(ctx: Context, next) {
  roles.forEach((role: any) => rolesMap.set(role.name, role));
  const userRoles = Array.from(rolesMap.values());
  ctx.state.currentUser.roles = userRoles;
-  const systemSettings = await ctx.db.getRepository('systemSettings').findOne();
+  const systemSettings = (await cache.wrap(`app:systemSettings`, () =>
+    ctx.db.getRepository('systemSettings').findOne(),
+  )) as Model;
  const roleMode = systemSettings?.get('roleMode') || SystemRoleMode.default;
  if ([currentRole, ctx.state.currentRole].includes(UNION_ROLE_KEY) && roleMode === SystemRoleMode.default) {
    currentRole = userRoles[0].name;
@ -58,12 +60,6 @@ export async function setCurrentRole(ctx: Context, next) {
    ctx.headers['x-role'] = UNION_ROLE_KEY;
    ctx.state.currentRoles = userRoles.map((role) => role.name);
    return next();
-  } else if (roleMode === SystemRoleMode.allowUseUnion) {
-    userRoles.unshift({
-      name: UNION_ROLE_KEY,
-      title: ctx.t('Full permissions', { ns: 'acl' }),
-    });
-    ctx.state.currentUser.roles = userRoles;
  }

  if (currentRole === UNION_ROLE_KEY) {
@ -85,11 +81,13 @@ export async function setCurrentRole(ctx: Context, next) {
  }
  // 2. If the X-Role is not set, or the X-Role does not belong to the user, use the default role
  if (!role) {
-    const defaultRole = userRoles.find((role) => role?.rolesUsers?.default);
-    role = (defaultRole || userRoles.find((x) => x.name !== UNION_ROLE_KEY))?.name;
+    const defaultRoleModel = await cache.wrap(`roles:${ctx.state.currentUser.id}:defaultRole`, () =>
+      ctx.db.getRepository('rolesUsers').findOne({ where: { userId: ctx.state.currentUser.id, default: true } }),
+    );
+    role = defaultRoleModel?.roleName || userRoles[0]?.name;
  }
  ctx.state.currentRole = role;
-  ctx.state.currentRoles = [role];
+  ctx.state.currentRoles = role === UNION_ROLE_KEY ? [userRoles[0]?.name] : [role];
  if (!ctx.state.currentRoles.length) {
    return ctx.throw(401, {
      code: 'ROLE_NOT_FOUND_ERR',
--- a/packages/plugins/@nocobase/plugin-acl/src/server/server.ts
+++ b/packages/plugins/@nocobase/plugin-acl/src/server/server.ts
@ -341,10 +341,16 @@ export class PluginACLServer extends Plugin {
    this.app.db.on('rolesUsers.afterSave', async (model) => {
      const cache = this.app.cache as Cache;
      await cache.del(`roles:${model.get('userId')}`);
+      await cache.del(`roles:${model.get('userId')}:defaultRole`);
+    });
+    this.app.db.on('systemSettings.afterSave', async (model) => {
+      const cache = this.app.cache as Cache;
+      await cache.del(`app:systemSettings`);
    });
    this.app.db.on('rolesUsers.afterDestroy', async (model) => {
      const cache = this.app.cache as Cache;
      await cache.del(`roles:${model.get('userId')}`);
+      await cache.del(`roles:${model.get('userId')}:defaultRole`);
    });

    const writeRolesToACL = async (app, options) => {