Skip to content

fix: Dropitem permission #3955

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
merged 1 commit into from
Aug 28, 2025
Merged

fix: Dropitem permission #3955

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
9 changes: 8 additions & 1 deletion ui/src/permission/application/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ const systemManage = {
'OR'
),
folderDelete: () => false,
auth: () => false,
auth: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_APPLICATION_AUTH
],
'OR'
),
overview_embed: () =>
hasPermission(
[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code seems to contain a few issues:

  1. Unnecessary Parentheses: The function parameters in the auth method are enclosed in parentheses unnecessarily. This is not required in JavaScript.

  2. Redundant Spaces and Lines: There are multiple spaces between words and lines that can be removed for better readability.

  3. Indentation: Ensure consistent indentation throughout the code for clarity.

Here's a revised version of the code with these improvements applied:

const systemManage = {
    and: (a, b) => !!a && !!b,
    or: (a, b) => !!a || !!b,

    folderDelete: () => false,

    auth: function() {
        return hasPermission([
            RoleConst.ADMIN,
            PermissionConst.RESOURCE_APPLICATION_AUTH
        ], 'OR' );
    },

    overview_embed: function() {
        return hasPermission([
               // Other permissions here
           ],
       'AND'
   )
};

Key Changes:

  • Removed unnecessary parentheses from the and and or functions.
  • Fixed line spacing and padding.
  • Consistently used lowercase letters for object properties and functions to follow JavaScript naming conventions.

These adjustments make the code more readable and maintainable.

Expand Down
7 changes: 6 additions & 1 deletion ui/src/permission/knowledge/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,12 @@ const systemManage = {
chat_user_edit: () =>false,


auth: () => false,
auth: () =>
hasPermission([
RoleConst.ADMIN,
PermissionConst.RESOURCE_KNOWLEDGE_AUTH
],'OR'
),
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet seems to be setting up routes for an application with different permissions for users. However, there are several points that could be improved:

  1. Lack of Proper Routing Logic: The function names chat_user_edit, folderCreate, etc., should ideally correspond to actual API endpoints or route paths.

  2. Permission Handling:

    • The authorization method uses the hasPermission function from a module (likely named roleUtils.js). This function is not defined in the snippet.
    • Ensure that RoleConst and PermissionConst are properly imported and defined before using them in hasPermission.

  3. Code Readability: Adding comments can help clarify what each part of the code does, especially when integrating with an authentication system.

  4. Potential Errors: If hasPermission does not correctly handle multiple roles or conditions (like 'OR'), it might incorrectly allow unauthorized access unless explicitly checked elsewhere.

Here's a potential revision based on these considerations:

const systemManage = {
  // Simulated user edit feature; replace with real endpoint logic if necessary
  chat_user_edit: () => { return true; }, // Replace with real implementation

  // Authentication check based on role and permission requirements
  auth: () => (
    hasPermission(['ADMIN', 'RESOURCE_KNOWLEDGE_AUTH'], 'OR')
  ),

  // Folder management functions
  folderCreate: () => false,
  folderEdit: () =>false,
  folderDelete: () => false,

  // Additional fields can include more routing and functionality
};

Make sure hasPermission is properly implemented in your project to handle the specified checks effectively. Adjust the implementation per your specific use case and environment constraints.

Expand Down
3 changes: 2 additions & 1 deletion ui/src/permission/model/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const systemManage = {
delete: () =>
hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_MODEL_DELETE], 'OR'),

auth: () => false,
auth: () =>
hasPermission([RoleConst.ADMIN, PermissionConst.RESOURCE_MODEL_AUTH], 'OR'),
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Expand Down
9 changes: 8 additions & 1 deletion ui/src/permission/tool/system-manage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,14 @@ const systemManage = {
'OR',
),

auth: () => false,
auth: () =>
hasPermission(
[
RoleConst.ADMIN,
PermissionConst.RESOURCE_TOOL_AUTH,
],
'OR',
),
folderCreate: () => false,
folderEdit: () => false,
folderDelete: () => false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided JavaScript function checks if a user is authenticated based on their permissions in the RoleConst and PermissionConst. The main issue with this approach is that it does not explicitly check if the user is authorized to perform specific actions like creating, editing, or deleting folders. In an ideal setup, these actions should also be checked against the correct permission sets.

Here’s a suggested revision:

@@ -57,13 +62,26 @@ const systemManage = {
     'OR',
   ),
 
-  auth: () => false,
+  auth: (requiredActions) => {
+    // Example of checking if user has required roles/permissions
+    return hasPermission(
+      [
+        RoleConst.ADMIN,
+        PermissionConst.ACCESS_FOLDERS,
+        ...requiredActions.map(action => PermissionConst[`${action.toUpperCase}_AUTH`]),
+      ],
+      'OR',
+    );
+  },
   
   folderCreate: () => systemManage.auth(['CREATE']), 
   folderEdit: () => systemManage.auth(['EDIT']),
   folderDelete: () => systemManage.auth(['DELETE']), 
 };

Key Changes:

  1. Function Accepts Required Actions: Updated the auth method to accept an array of strings (requiredActions) representing the actions the user needs to have.
  2. Generates Permissions Based on Action Names: Inside the auth method, generates a list of permission constants associated with each action name using template literals.
  3. Consistent Use of Arrays: Ensures consistency by returning arrays for both the permitted methods and the required permissions.

This refactoring will allow more flexibility and clarity in determining authentication status for various actions related to managing files/folders. Remember to adjust the PermissionConst and RoleConst according to your API's requirements.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/utils/permission/data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ const PermissionConst = {

RESOURCE_MODEL_AUTH: new Permission('SYSTEM_RESOURCE_MODEL:READ+AUTH'),
RESOURCE_APPLICATION_AUTH: new Permission('SYSTEM_RESOURCE_APPLICATION:READ+AUTH'),
RESOURCE_KNOWLEDGE_AUTH: new Permission('SYSTEM_RESOURCE_AUTH:READ+AUTH'),
RESOURCE_KNOWLEDGE_AUTH: new Permission('SYSTEM_RESOURCE_KNOWLEDGE:READ+AUTH'),
RESOURCE_TOOL_AUTH: new Permission('SYSTEM_RESOURCE_TOOL:READ+AUTH'),

APPEARANCE_SETTINGS_READ: new Permission('APPEARANCE_SETTINGS:READ'),
Expand Down
1 change: 1 addition & 0 deletions ui/src/views/knowledge/component/KnowledgeListContainer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ const MoreFilledPermission = (item: any) => {
permissionPrecise.value.generate(item.id) ||
(permissionPrecise.value.edit(item.id) && apiType.value) === 'workspace' ||
permissionPrecise.value.export(item.id) ||
permissionPrecise.value.auth(item.id) ||
permissionPrecise.value.delete(item.id) ||
isSystemShare.value
)
Expand Down
5 changes: 4 additions & 1 deletion ui/src/views/model/component/ModelCard.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ const permissionPrecise = computed(() => {

const MoreFilledPermission = (id: any) => {
return (
permissionPrecise.value.modify(id) || permissionPrecise.value.delete(id) || isSystemShare.value
permissionPrecise.value.modify(id) ||
permissionPrecise.value.delete(id) ||
permissionPrecise.value.auth(id) ||
isSystemShare.value
)
}

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/system-resource-management/ModelResourceIndex.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ const paginationConfig = reactive({

// sync generete edit export delete
const MoreFilledPermission = () => {
return permissionPrecise.value.delete() || permissionPrecise.value.auth()
return permissionPrecise.value.delete() || permissionPrecise.value.modify()
}

const ResourceAuthorizationDrawerRef = ref()
Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/system-resource-management/ToolResourceIndex.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@
<AppIcon iconName="app-export" class="color-secondary"></AppIcon>
{{ $t('common.export') }}
</el-dropdown-item>
<el-dropdown-item v-if="row.tool_type === 'MCP'" @click.stop="showMcpConfig(row)">
<el-dropdown-item v-if="row.tool_type === 'MCP' && permissionPrecise.edit()" @click.stop="showMcpConfig(row)">
<AppIcon iconName="app-operate-log" class="color-secondary"></AppIcon>
{{ $t('views.tool.mcpConfig') }}
</el-dropdown-item>
Expand Down
1 change: 1 addition & 0 deletions ui/src/views/tool/component/ToolListContainer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ const MoreFieldPermission = (id: any) => {
permissionPrecise.value.edit(id) ||
permissionPrecise.value.export(id) ||
permissionPrecise.value.delete(id) ||
permissionPrecise.value.auth(id) ||
isSystemShare.value
)
}
Expand Down
Loading