Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

fix: Folder authorization of common user --bug=1062968 --user=张展玮 【应用】普通用户对自己管理的文件夹进行资源授权,生效资源选择所有子资源,授权报错 https://www.tapd.cn/62980211/s/1790231

--bug=1062968 --user=张展玮 【应用】普通用户对自己管理的文件夹进行资源授权,生效资源选择所有子资源,授权报错 https://www.tapd.cn/62980211/s/1790231
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 27, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 27, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

).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

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.

@zhanweizhang7 zhanweizhang7 merged commit 1174ee1 into v2 Oct 27, 2025
4 of 6 checks passed
@zhanweizhang7 zhanweizhang7 deleted the pr@v2@fix_folder_authorization branch October 27, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants