-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,8 +96,8 @@ def is_valid(self, *, auth_target_type=None, workspace_id=None, raise_exception= | |
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')) | ||
choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'], | ||
label=_('permission')) | ||
|
||
|
||
class UserResourcePermissionSerializer(serializers.Serializer): | ||
|
@@ -304,7 +304,7 @@ class ResourceUserPermissionUserListRequest(serializers.Serializer): | |
class ResourceUserPermissionEditRequest(serializers.Serializer): | ||
user_id = serializers.CharField(required=True, label=_('workspace id')) | ||
permission = serializers.ChoiceField(required=True, choices=['NOT_AUTH', 'MANAGE', 'VIEW', 'ROLE'], | ||
label=_('permission')) | ||
label=_('permission')) | ||
|
||
|
||
permission_map = { | ||
|
@@ -326,7 +326,8 @@ def get_queryset(self, instance): | |
user_query_set = QuerySet(model=get_dynamics_model({ | ||
'nick_name': models.CharField(), | ||
'username': models.CharField(), | ||
"permission": models.CharField() | ||
"permission": models.CharField(), | ||
"id": models.UUIDField(), | ||
})) | ||
nick_name = instance.get('nick_name') | ||
username = instance.get('username') | ||
|
@@ -352,6 +353,11 @@ def get_queryset(self, instance): | |
else: | ||
user_query_set = user_query_set.filter( | ||
permission__in=query_p_list) | ||
workspace_user_role_mapping_model = DatabaseModelManage.get_model("workspace_user_role_mapping") | ||
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ).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 |
There was a problem hiding this comment.
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 toname="permissions[]"
in the provided code snippet. The change allows for multiple values of "permission" can be sent through the query string.Here is an updated version of the function that now correctly handles the optional array parameter for permissions: