Skip to content

fix: Tool permission #4271

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 2 commits into from
Oct 27, 2025
Merged

fix: Tool permission #4271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abc695c
fix: Locales
zhanweizhang7 Oct 27, 2025
6cd5cfa
fix: Tool permission
zhanweizhang7 Oct 27, 2025
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: 3 additions & 0 deletions apps/locales/en_US/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8760,4 +8760,7 @@ msgid "Tag key already exists"
msgstr ""

msgid "Tag value already exists"
msgstr ""

msgid "Non-existent id"
msgstr ""
3 changes: 3 additions & 0 deletions apps/locales/zh_CN/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8887,3 +8887,6 @@ msgstr "标签已存在"

msgid "Tag value already exists"
msgstr "标签值已存在"

msgid "Non-existent id"
msgstr "不存在的ID"
3 changes: 3 additions & 0 deletions apps/locales/zh_Hant/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8887,3 +8887,6 @@ msgstr "標籤已存在"

msgid "Tag value already exists"
msgstr "標籤值已存在"

msgid "Non-existent id"
msgstr "不存在的ID"
2 changes: 1 addition & 1 deletion apps/system_manage/serializers/user_resource_permission.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def is_valid(self, *, auth_target_type=None, workspace_id=None, raise_exception=
workspace_id, workspace_id, workspace_id])
if illegal_target_id_list is not None and len(illegal_target_id_list) > 0:
raise AppApiException(500,
_('Non-existent id[') + str(illegal_target_id_list) + ']')
_('Non-existent id')+'[' + str(illegal_target_id_list) + ']')


m_map = {
Expand Down
4 changes: 2 additions & 2 deletions ui/src/permission/tool/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ const workspace = {
[
new ComplexPermission([RoleConst.USER],[PermissionConst.TOOL.getToolWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.TOOL_EXPORT.getToolWorkspaceResourcePermission(source_id),
PermissionConst.TOOL_EXPORT.getWorkspacePermissionWorkspaceManageRole
PermissionConst.TOOL_EDIT.getToolWorkspaceResourcePermission(source_id),
PermissionConst.TOOL_EDIT.getWorkspacePermissionWorkspaceManageRole
],
'OR'
),
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 27, 2025

Choose a reason for hiding this comment

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

The provided TypeScript code snippet appears to be defining permission checks in a complex workflow system. Here are some observations:

  1. Permission Changes:
    The original line has two different export permissions being added:

    PermissionConst.TOOL_EXPORT.getToolWorkspaceResourcePermission(source_id)

    This was changed to use an edit instead of an export:

    PermissionConst.TOOL_EDIT.getToolWorkspaceResourcePermission(source_id)

  2. Security Concerns:
    Using export can potentially expose sensitive or critical data to un authorized users, especially if source_id is not properly secured.

  3. Readability:
    The addition of duplicate workspace.manage-role roles could cause confusion and redundancy. It might be better to consolidate these into fewer roles if possible.

  4. Code Structure:
    There aren't any syntax errors but it's good practice to ensure the structure remains consistent and efficient.

Optimization Suggestions:

  1. Consolidate Permissions: If there aren't specific reasons to keep both an export and edit role, consider removing one based on the intended function.

  2. Secure Source ID Usage: Ensure that source_id is sanitized and appropriately checked during runtime before passing it to methods like getting tool permissions.

Here’s a revised version with those considerations:

const workspace: {
    allowedRolesAndPermissions: Array<{
        conditions: ({ UserRole, WorkspaceRole }: typeof Role) => boolean[],
    }>,
} = {
    [
        new ComplexPermission([Role.User], [Permission.EditToolWorkspaceResourcePermission(source_id)], [], 'AND'),
        Role.WorkspaceManage,
        Permission.EditToolWorkspaceResourcePermission(source_id),
        Permission.EditToolWorkspacePermission(Role.Manage)
    ],
};

Note: Adjusted method names according to updated references, assuming they exist within the context of your codebase.

Expand Down
Loading