Skip to content

feat: Resource permission in system-manage #3768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhanweizhang7 merged 1 commit into from
Jul 29, 2025
Merged

feat: Resource permission in system-manage #3768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ui/src/permission/application/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {hasPermission} from '@/utils/permission/index'
import {ComplexPermission} from '@/utils/permission/type'
import {EditionConst, PermissionConst, RoleConst} from '@/utils/permission/data'
import {PermissionConst, RoleConst} from '@/utils/permission/data'

const systemManage = {
create: () => false,
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code has several issues that need to be addressed:

  1. Removed Import Statement: The ComplexPermission type is required for permission checks but is removed from the import statement at line 3.

    To fix: Add back the import { ComplexPermission } from '@/utils/permission/type';

  2. Unused Function Call: There's a commented-out function call create => false within the systemManage object literal at lines 4-5. Since there aren't any other calls to this, you might want to remove it if not needed.

  3. Code Structure: While minor, consider organizing related functions or constants together rather than scattering them throughout the file.

Here’s the corrected version of your code with these suggested changes:

@@ -1,7 +1,8 @@
 import {hasPermission} from '@/utils/permission/index'
+import {ComplexPermission} from '@/utils/permission/type'

 const systemManage = {
     create() => false
 }

 // Add more functionalities here

 export default systemManage;

This should address the specified issues and improve the readability and functionality of your code.

Expand Down
141 changes: 120 additions & 21 deletions ui/src/permission/knowledge/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,128 @@ const systemManage = {
'OR',
),
create: () => false,
sync: () => false,
vector: () => false,
generate: () => false,
edit: () => false,
export: () => false,
delete: () => false,
sync: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_SYNC
],'OR'
),
vector: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_VECTOR
],'OR'
),
generate: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_GENERATE
],'OR'
),
edit: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_EDIT
],'OR'
),
export: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_EXPORT
],'OR'
),
delete: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DELETE
],'OR'
),
// 文档
doc_create: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_CREATE
],'OR'
),
doc_vector: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_VECTOR
],'OR'
),
doc_generate: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_GENERATE
],'OR'
),
doc_migrate: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_MIGRATE
],'OR'
),
doc_edit: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_EDIT
],'OR'
),
doc_sync: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_SYNC
],'OR'
),
doc_delete: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_DELETE
],'OR'
),
doc_export: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_EXPORT
],'OR'
),
doc_download: () => hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_DOWNLOAD_SOURCE_FILE
],'OR'
),

doc_create: () => false,
doc_vector: () => false,
doc_generate: () => false,
doc_migrate: () => false,
doc_edit: () => false,
doc_sync: () => false,
doc_delete: () => false,
doc_export: () => false,
doc_download: () => false,
knowledge_chat_user_edit: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_CHAT_USER_EDIT
],'OR'),

knowledge_chat_user_edit: () => false,

problem_create: () => false,
problem_relate: () => false,
problem_delete: () => false,
problem_edit: () => false,
problem_create: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_CREATE
],'OR'
),
problem_relate: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_RELATE
],'OR'
),
problem_delete: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_DELETE
],'OR'
),
problem_edit: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_EDIT
],'OR'
),

folderCreate: () => false,
folderEdit: () => false,
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks well-designed overall but can be optimized in several ways:

  1. Use of Arrays: The presence of multiple conditional checks for different functionalities can be consolidated into arrays, making it easier to manage and extend.

  2. Reusability: Create helper functions to handle permission logic consistently across different endpoints or features.

    function getPermissions(type) {
  return hasPermission([
    RoleConst.ADMIN,
    `RESOURCE_KNOWLEDGE_${type.toUpperCase()}` // Assuming type is like "chat"
  ],'OR');
}

  3. Duplicated Code: Consider extracting common parts of each endpoint's permissions to improve maintainability.

  4. Documentation Commenting: Add detailed comments explaining the purpose of complex blocks or sections.

  5. Simplify Imports/Exports: If these methods are part of larger modules, ensure they don't unnecessarily import/export every time.

  6. Error Handling: Implement basic error handling if necessary when calling the hasPermission function.

Here’s an example of how you might refactor some of this:

const systemManage = {
  OR: (...args) => args.every(Boolean),
};

function hasPermission(neededRoles, joinTypeOrArray = 'OR') {
  let result;
  
  switch (joinTypeOrArray.toLowerCase()) {
    case 'and':
      result = neededRoles.every((role) => roleExists(role));
      break;
    default:
      result = neededRoles.some((role) => roleExists(role));
      break;
  }
  
  return result;
}

// Helper function to check if a role exists
function roleExists(roleName) {
  // Implementation depends on where roles are defined or loaded
  // Here we just assume it's available globally
  try {
    return global.roles.includes(roleName);
  } catch (err) {
    console.error("Error checking role existence:", err);
    return false;
  }
}

function initializePermissionsMap() {
  const permissionsMap = {};

  // Initialize specific permissions based on their types
  const permissionTypes = ['sync', 'vector', 'generate', 'edit', 'export', 'delete'];
  permissionTypes.forEach(type => {
    const key = `${type}_create`;
    permissionsMap[key] = (role) =>
      hasPermission([RoleConst.ADMIN, `RESOURCE_KNOWLEDGE_${key.split('_')[0].toUpperCase()}`]);
  });

  // Additional document-specific permissions...
  permissionsMap.doc_create = (role) =>
    hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_DOCUMENT_CREATE]);

  permissionsMap.knowledge_chat_user_edit = (role) =>
    hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_CHAT_USER_EDIT]);

  // Problem-related permissions...
  permissionsMap.problem_create = (role) =>
    hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_KNOWLEDGE_PROBLEM_CREATE]);

  // Document-migration etc...

  // Return full map or individual objects as needed
  return permissionsMap;
}

This approach separates functionality into reusable components, which makes maintenance easier, and ensures that all similar permissions are handled similarly throughout the application.

Expand Down
18 changes: 15 additions & 3 deletions ui/src/permission/model/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,21 @@ const systemManage = {
'OR',
),
create: () => false,
modify: () => false,
paramSetting: () => false,
delete: () => false,
modify: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_MODEL_EDIT
],'OR'),
paramSetting: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_MODEL_EDIT
],'OR'),
delete: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_MODEL_DELETE
],'OR'),

folderCreate: () => false,
folderEdit: () => false,
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided JavaScript function const systemManage contains several improvements to enhance readability, maintainability, and correctness:

  1. Separate Functions: Each action method should be defined independently within an object for better organization.

  2. Inline Permission Check: The permission checking logic can be extracted into a standalone function named hasPermission.

  3. Consistent Return Type: Ensure that all actions return either truthy or falsy values consistently.

Here's the improved version of the code:

const roleConst = {
  ADMIN: 'ADMIN',
};

const permissionConst = {
  RESOURCE_MODEL_EDIT: 'RESOURCE_MODEL_EDIT',
  RESOURCE_MODEL_DELETE: 'RESOURCE_MODEL DELETE',
};

function hasPermission(requiredRoles, operator) {
  if (operator === 'AND') {
    return requiredRoles.every(role => user.hasRole(role));
  } else if (operator === 'OR') {
    return requiredRoles.some(role => user.hasRole(role)); // Adjust depending on actual data structure
  }
}

const systemManage = {
  folderCreate: () => false,
  folderEdit: () => false,

  create: () =>
    hasPermission([roleConst.ADMIN], 'AND'),

  modify: () =>
    hasPermission([roleConst.ADMIN, permissionConst.RESOURCE_MODEL_EDIT], 'OR'),

  paramSetting: () =>
    hasPermission([roleConst.ADMIN, permissionConst.RESOURCE_MODEL_EDIT], 'OR'),

  delete: () =>
    hasPermission([roleConst.ADMIN, permissionConst.RESOURCE_MODEL_DELETE], 'OR'),
};

Key Improvements:

  • Modularization: Each action method is now separated into its own variable.

  • Function Extraction: Moved permission-checking logic inside the hasPermission function for clarity and reusability.

  • Consistency: All methods in the systemManage object use consistent naming conventions and types (false returns).

Ensure you have access to the necessary utility functions such as user.hasRole() in this context based on how your application manages roles and permissions.

Expand Down
Loading