Skip to content

fix: Folder authorization of common user #4261

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
Oct 27, 2025
Merged

fix: Folder authorization of common user #4261

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
4 changes: 2 additions & 2 deletions 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 @@ -409,13 +409,13 @@ def get_has_manage_permission_resource_under_folders(self, current_user_id, fold
if workspace_manage:
current_user_managed_resources_ids = QuerySet(resource_model).filter(workspace_id=workspace_id, folder__in=folder_ids).annotate(
id_str=Cast('id', TextField())
).values_list("id", flat=True)
).values_list("id_str", flat=True)
else:
current_user_managed_resources_ids = QuerySet(WorkspaceUserResourcePermission).filter(
workspace_id=workspace_id, user_id=current_user_id, auth_target_type=auth_target_type,
target__in=QuerySet(resource_model).filter(workspace_id=workspace_id, folder__in=folder_ids).annotate(
id_str=Cast('id', TextField())
).values_list("id", flat=True),
).values_list("id_str", flat=True),
permission_list__contains=['MANAGE']).values_list('target', flat=True)

return current_user_managed_resources_ids
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 code has several issues:

  1. Potential SQL Injection: The use of string formatting can introduce SQL injection vulnerabilities if not handled properly.

  2. Unnecessary Annotation and Values List Calls: These steps are redundant since you are only interested in the id or id_str. You only need to filter based on criteria without annotating fields unless there's a specific reason related to later processing.

  3. Code Duplication: There is repetition in filtering for both managed resources (QuerySet(resource_model)) and those with manage permissions (WorkspaceUserResourcePermission). This could be consolidated using Django ORM queries to avoid duplication.

  4. Lack of Comments: While comments improve readability, they might slow down performance slightly depending on complexity. If the intention is clear from context, comments can be removed.

Here is an optimized version of the code:

def get_has_manage_permission_resource_under_folders(current_user_id, folder_ids):
    workspace_manage = True  # Assuming this flag determines where to fetch data

    # Simplified query to select all IDs under specified folders (assuming these exist in resource model)
    resource_queryset = ResourceModel.objects.filter(workspace_id=self.workspace_id, folder__in=folder_ids).annotate(
        id_str=Cast('id', TextField()))  # Using annotation to include ID as a field if needed

    if workspace_manage:
        current_user_managed_resources_ids = resource_queryset.values_list(
            'id_str',
            flat=True
        )
    else:
        # Filter WorkspaceUserResourcePermissions to find resources user manages
        current_user_managed_resources_ids = (
            WorkspaceUserResourcePermission.objects
            .filter(
                workspace_id=self.workspace_id,
                user_id=current_user_id,
                auth_target_type='RESOURCE_PERMISSION',
                target__in=resource_queryset
            )
            .values_list('target', flat=True)
        )

    return current_user_managed_resources_ids

Key Changes:

  • Removed Redundant Queries: By combining the two main parts into one operation, we reduce redundancy and potential inefficiencies.
  • Simplified Logic: Directly fetching IDs instead of filtering after selecting multiple items reduces unnecessary operations.
  • Consolidated Filtering: Combining conditions within a single queryset makes the logic more concise and easier to understand.

Expand Down
Loading