Skip to content

fix: Resource authorization #3906

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.

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 21, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

fix: Resource authorization

Copy link

f2c-ci-robot bot commented Aug 21, 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.

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.

Copy link

f2c-ci-robot bot commented Aug 21, 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.

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

if workspace_user_role_mapping_model:
user_query_set=user_query_set.filter(
id__in=QuerySet(workspace_user_role_mapping_model).filter(
workspace_id=self.data.get('workspace_id')).values("user_id"))

return {
'workspace_user_resource_permission_query_set': workspace_user_resource_permission_query_set,
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 looks mostly correct in structure and functionality. However, I can offer a few minor improvements:

  1. Consistent Key Names: Ensure that all keys used in get_queryset are consistent. Currently, 'nick_name', 'username', and 'permission' are all present, but it might be clearer to use the same keys throughout.

  2. Optional Parameters: Consider adding optional parameters for fields like 'workspace_id' or 'auth_target_type' in case they might not always be included in the input data.

  3. Comments: Add comments to explain complex sections of the code, such as how the query sets are filtered based on workspace roles.

Here's an updated version with some of these considerations:

class UserResourcePermissionUserListRequest(serializers.Serializer):
    name = serializers.CharField(required=False, allow_null=True, allow_blank=True, label=_('resource name'))
    permission = serializers.MultipleChoiceField(required=False, allow_null=True, allow_blank=True,
                                                 choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'],
                                                 label=_('permission'))


class UserResourcePermissionSerializer(serializers.Serializer):
    user_id = serializers.CharField(required=True, label=_('user ID'))
    permission = serializers.ChoiceField(required=True, choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'],
                                                 label=_('permission'))


class ResourceUserPermissionUserListRequest(serializers.Serializer):
    user_id = serializers.CharField(required=True, label=_('user ID'))
    permission = serializers.ChoiceField(required=True, choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'],
                                                 label=_('permission'))

class ResourceUserPermissionEditRequest(serializers.Serializer):
    # Optional parameters can be added here if needed
    user_id = serializers.CharField(required=True, label=_('user ID'))
    permission = serializers.ChoiceField(required=True, choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'],
                                                   label=_('permission'))


# Example usage of the get_queryset method
def get_queryset(instance):
    user_query_set = QuerySet(model=get_dynamics_model({
        'nick_name': models.CharField(),
        'username': models.CharField(),
        "permission": models.CharField(),
        "id": models.UUIDField()
    }))

    nick_name = instance.get('nick_name')
    username = instance.get('username')

    if nickname:
        user_query_set = user_query_set.filter(nick_name=nickname)

    if username:
        user_query_set = user_query_set.filter(username=username)

    # Apply permission filter
    permission = instance.get('permission')
    if permission:
        query_p_list = [perm.upper() for perm in permissions.split(',')]
        if len(query_p_list) == 1:
            user_query_set = user_query_set.filter(permission=permissions)
        else:
            user_query_set = user_query_set.filter(permission__in=query_p_list)

    # Filter users who have access through workspace roles (optional)
    workspace_user_role_mapping_model = DatabaseModelManage.get_model("workspace_user_role_mapping")
    if workspace_user_role_mapping_model:
        user_ids_with_workspace_roles = set(QuerySet(workspace_user_role_mapping_model).filter(
            workspace_id=self.data.get('workspace_id')).values("user_id"))
        user_query_set = user_query_set.filter(id__in=user_ids_with_workspace_roles)

    return {
        'workspace_user_resource_permission_query_set': workspace_user_resource_permission_query_set,
        # Optionally add other relevant data
    }

These changes help improve readability and potentially add flexibility and robustness to the code.

@@ -297,7 +297,7 @@ def get_parameters():
required=False
),
OpenApiParameter(
name="permission",
name="permission[]",
description="权限",
type=OpenApiTypes.STR,
location='query',
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 line name="permission" should be changed to name="permissions[]" in the provided code snippet. The change allows for multiple values of "permission" can be sent through the query string.

@openapi.api(tags=['User Management'], summary='获取用户信息')
@api.doc(responses={
    200: {'description': '成功'},
    403: {'description': '无权限'})
})
def get_user_info(**kwargs):
    username = kwargs.get("username", None)
    
    # 检查用户名是否是有效的非空字符串
    if not isinstance(username, str) or len(username.strip()) == 0:
        return responses.bad_request('用户名不能为空并只能包含字母和数字')

    params = {
        'page_index': request.args.get('page_index', 1),
        'per_page': request.args.get('per_page', 10),
        'sort_by': request.args.get('sort_by', 'id'),
        'descending': bool(request.args.get('descending', False)),
    }

    response = requests.get(API_URL + '/users/' + username, params=params)

    try:
        result = response.json()
    except Exception as e:
        raise InternalServerError(str(e))

    if not isinstance(result, dict) or 'data' not in result or 'meta' not in result['data']:
        logging.error(f"Invalid JSON data received from server ({response.text})")
        return responses.server_error('非法的JSON数据返回')

    total_pages = int((int(result['data']['total_count']) / float(params['per_page']))+0.999999)

Here is an updated version of the function that now correctly handles the optional array parameter for permissions:

from flask import request, jsonify

API_URL = 'https://example-api.com/api'

@app.route('/get-user-info', methods=['GET'])
def get_user_info():
    username = request.args.get('username', None)

    if not isinstance(username, str) or len(username.strip()) == 0:
        return jsonify({'message': '用户名不能为空并只能包含字母和数字'}), 400

    permissions = ','.join(map(str, request.args.getlist('permission[]'))).strip()

    params = {
        'per_page': request.args.get('per_page', 10),
        'pagination_enabled': True,
    }

    response = requests.get(API_URL + '/users/' + username, params=params)

    try:
        users_data = response.json()
    except ValueError:
        logging.error(f"Received invalid JSON from API endpoint {response.url}")
        return jsonify({'error': '无法解析JSON响应'}), 500

    pagination = {"items": [], "next_url": ""}

    if "links" in users_data["data"] and users_data["data"]["links"].get("last"):
        if "href" in users_data["data"]["links"]["last"]:
            next_page_response = requests.get(users_data["data"]["links"]["last"].get("_links").get("next"))
            pagination = json.loads(next_page_response.text)
            

    return jsonify({
        "user": users_data,
        "pagination": pagination,
    }), 200
}

@@ -197,5 +197,5 @@ def get(self, request: Request, workspace_id: str, target: str, resource: str, c
data={'workspace_id': workspace_id, "target": target, 'auth_target_type': resource, }
).page({'username': request.query_params.get("username"),
'nick_name': request.query_params.get("nick_name"),
'permission': request.query_params.getlist("permission")}, current_page, page_size,
'permission': request.query_params.getlist("permission[]")}, current_page, page_size,
))
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 line appears to be incorrect due to an empty argument list [] being passed to the get method call within the function get. The request.query_params.getlist() should likely use quotes around "permission" instead of parentheses. Here's the corrected version:

).page({'username': request.query_params.get("username"),
            'nick_name': request.query_params.get("nick_name"),
            'permission': request.query_params.getlist("permission")},
       current_page, page_size)

By ensuring proper quoting around "permission", this should resolve any syntax errors in the code.

@zhanweizhang7 zhanweizhang7 merged commit 452fd53 into v2 Aug 21, 2025
3 of 6 checks passed
@zhanweizhang7 zhanweizhang7 deleted the pr@v2@fix_resource_authorization branch August 21, 2025 06:12
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.

2 participants