-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Folder authorization backend #4185
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 |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| import uuid_utils.compat as uuid | ||
| from django.db import transaction | ||
| from django.db.models import QuerySet, Q | ||
| from django.db.models import QuerySet, Q, Func, F | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from rest_framework import serializers | ||
|
|
||
|
|
@@ -269,7 +269,8 @@ def _check_tree_integrity(queryset): | |
| return True # 需要重建 | ||
| return False | ||
|
|
||
| def get_folder_tree(self, name=None): | ||
| def get_folder_tree(self, | ||
| current_user, name=None): | ||
| self.is_valid(raise_exception=True) | ||
| Folder = get_folder_type(self.data.get('source')) # noqa | ||
|
|
||
|
|
@@ -280,15 +281,21 @@ def get_folder_tree(self, name=None): | |
| if self._check_tree_integrity(workspace_folders): | ||
| Folder.objects.rebuild() | ||
|
|
||
| workspace_manage = is_workspace_manage(current_user.id, self.data.get('workspace_id')) | ||
|
|
||
| base_q = Q(workspace_id=self.data.get('workspace_id')) | ||
|
|
||
| if name is not None: | ||
| nodes = Folder.objects.filter( | ||
| Q(workspace_id=self.data.get('workspace_id')) & | ||
| Q(name__contains=name) | ||
| ).get_cached_trees() | ||
| else: | ||
| nodes = Folder.objects.filter( | ||
| Q(workspace_id=self.data.get('workspace_id')) | ||
| ).get_cached_trees() | ||
| base_q &= Q(name__contains=name) | ||
| if not workspace_manage: | ||
| base_q &= Q(id__in=WorkspaceUserResourcePermission.objects.filter(user_id=current_user.id, | ||
| auth_target_type=self.data.get('source'), | ||
| workspace_id=self.data.get('workspace_id'), | ||
| permission_list__contains=['VIEW']) | ||
| .values_list( | ||
| 'target', flat=True)) | ||
|
|
||
| nodes = Folder.objects.filter(base_q).get_cached_trees() | ||
|
|
||
| TreeSerializer = get_folder_tree_serializer(self.data.get('source')) # noqa | ||
| serializer = TreeSerializer(nodes, many=True) | ||
|
Contributor
Author
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. There are several improvements and optimizations that can be made to the provided code. Here is a detailed analysis: Irregularities and Potential Issues
Optimization Suggestions
Here is an optimized version of the code: import uuid_utils.compat as uuid
from django.db import transaction
from django.db.models import QuerySet, Func, F, Q, OuterRef, Exists
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers
def get_folder_types(source):
# Define your folder types here
pass
def get_folder_type(source):
# Retrieve folder type based on source
return get_folder_types(source)[0]
def _is_workspace_manage(user_id, workspace_id):
# Implement user management check
pass
class ResourceNodeNotFoundError(Exception):
pass
class WorkspaceTreeFilterMixin:
def filter_queryset_for_node_search(self, qs, current_user):
base_q = Q(workspace_id=OuterRef('workspace_id'))
if 'name' in self.request.query_params:
base_q &= Q(name__icontains=self.request.query_params['name'])
if not _is_workspace_manage(current_user.id, self.data.get('workspace_id')):
base_q &= ~Exists(WorkspaceUserResourcePermission.objects.filter(
user_id=current_user.id,
auth_target_type=get_folder_type(self.data.get('source')),
workspace_id=self.data.get('workspace_id'),
permission_list__contains=['VIEW']
))
return qs.annotate(has_view_permission=~base_q)
class FolderViewSet(WorkspaceTreeFilterMixin, GenericAPIView):
queryset = Folder.objects.order_by('-created_at')
renderer_classes = [JSONRenderer]
parser_classes = [CustomFormParser, FormParser, JSONParser]
swagger_schema = None
def is_valid(self, raise_exception=False):
super().is_valid(raise_exception)
def get_folder_tree(self, request, *args, **kwargs):
kwargs.setdefault('current_user', request.user)
queryset = self.get_queryset()
try:
tree = list(get_cached_trees(filter_func=lambda node,
current_user: node.has_view_permission.current_value))
serialize_class = get_folder_tree_serializer(self.data.get('source')) # noqa
serializer = serialize_class(tree, many=True, context={'request': request})
response_data = {
"node": {"count": len(tree)},
self.source_name_attribute(): list(serializer.data),
}
return Response(response_data, status=status.HTTP_200_OK)
except ResourceNodeNotFoundError as e:
return JsonResponse({'error': str(e)}, status=status.HTTP_404_NOT_FOUND)Changes Made:
This refactored code should be more efficient, readable, and maintainable. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,8 @@ | |
|
|
||
| from common.auth import TokenAuth | ||
| from common.auth.authentication import has_permissions | ||
| from common.constants.permission_constants import Permission, Group, Operate, RoleConstants | ||
| from common.constants.permission_constants import Permission, Group, Operate, RoleConstants, ViewPermission, \ | ||
| PermissionConstants, CompareConstants | ||
| from common.log.log import log | ||
| from common.result import result | ||
| from folders.api.folder import FolderCreateAPI, FolderEditAPI, FolderReadAPI, FolderTreeReadAPI, FolderDeleteAPI | ||
|
|
@@ -37,9 +38,17 @@ class FolderView(APIView): | |
| tags=[_('Folder')] # type: ignore | ||
| ) | ||
| @has_permissions( | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.CREATE, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}"), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role(), RoleConstants.USER.get_workspace_role() | ||
| lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}"), | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/ROLE/WORKSPACE_MANAGE" | ||
| ), | ||
| lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()], | ||
| [Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), | ||
| operate=Operate.SELF, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}" | ||
| )], CompareConstants.AND), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role() | ||
| ) | ||
| @log( | ||
| menu='folder', operate='Create folder', | ||
|
|
@@ -63,7 +72,8 @@ def post(self, request: Request, workspace_id: str, source: str): | |
| tags=[_('Folder')] # type: ignore | ||
| ) | ||
| @has_permissions( | ||
| lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_WORKSPACE_USER_RESOURCE_PERMISSION"), operate= Operate.READ, | ||
| lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_WORKSPACE_USER_RESOURCE_PERMISSION"), | ||
| operate=Operate.READ, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}"), | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.READ, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}"), | ||
|
|
@@ -73,7 +83,7 @@ def post(self, request: Request, workspace_id: str, source: str): | |
| def get(self, request: Request, workspace_id: str, source: str): | ||
| return result.success(FolderTreeSerializer( | ||
| data={'workspace_id': workspace_id, 'source': source} | ||
| ).get_folder_tree(request.query_params.get('name'))) | ||
| ).get_folder_tree(request.user, request.query_params.get('name'))) | ||
|
|
||
| class Operate(APIView): | ||
| authentication_classes = [TokenAuth] | ||
|
|
@@ -90,8 +100,17 @@ class Operate(APIView): | |
| ) | ||
| @has_permissions( | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}"), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role(), RoleConstants.USER.get_workspace_role() | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/ROLE/WORKSPACE_MANAGE" | ||
| ), | ||
| lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}" | ||
| ), | ||
| lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()], | ||
| [Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), | ||
| operate=Operate.SELF, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}" | ||
| )], CompareConstants.AND), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role() | ||
| ) | ||
| @log( | ||
| menu='folder', operate='Edit folder', | ||
|
|
@@ -132,9 +151,18 @@ def get(self, request: Request, workspace_id: str, source: str, folder_id: str): | |
| tags=[_('Folder')] # type: ignore | ||
| ) | ||
| @has_permissions( | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.DELETE, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}"), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role(), RoleConstants.USER.get_workspace_role() | ||
| lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/ROLE/WORKSPACE_MANAGE" | ||
| ), | ||
| lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), operate=Operate.EDIT, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}" | ||
| ), | ||
| lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()], | ||
| [Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), | ||
| operate=Operate.SELF, | ||
| resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{kwargs.get('folder_id')}" | ||
| )], CompareConstants.AND), | ||
| RoleConstants.WORKSPACE_MANAGE.get_workspace_role() | ||
| ) | ||
| @log( | ||
| menu='folder', operate='Delete folder', | ||
|
Contributor
Author
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. There are several areas of concern and some optimizations that can be made in your code:
Here’s an optimized version of the code incorporating these remarks: from common.auth import TokenAuth
from common.auth.authentication import has_permissions
from common.constants.permission_constants import Permission, Group, Operate, RoleConstants, ViewPermission
from common.log.log import log
from common.result import result
from folders.api.folder import FolderCreateAPI, FolderEditAPI, FolderReadAPI, FolderTreeReadAPI, FolderDeleteAPI
class FolderListView(APIView):
authentication_classes = [TokenAuth]
@action(methods=['GET'], name=_('List Folders'), detail=False)
@has_permissions(
lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/ROLE/WORKSPACE_MANAGE"
),
lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}"
),
lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()],
[Permission(group=Group(f"{kwargs.get('source')}_FOLDER"),
operate=Operate.SELF,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/SOURCE/PARENT_ID"
)], CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
)
@log(menu='folder', operate='Manage folders')
def get(self, request: Request, workspace_id: str, source: str):
filter_parameters = {'source': source} # Add any query parameters here you want to filter on
return result.success(FolderTreeSerializer(data={'workspace_id': workspace_id, 'filter': filter_parameters}).list_folders())
class FolderAPIView(APIView):
authentication_classes = [TokenAuth]
@action(methods=['POST'], name=_('Create Folder'), detail=False)
@has_permissions(
lambda r, kwargs: Permission(group=Group(f"{kwargs.get('source')}_FOLDER"), operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/ROLE/WORKSPACE_MANAGE"
),
lambda r, kwargs: Permission(group=Group(kwargs.get('source')), operate=Operate.EDIT,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}")
),
lambda r, kwargs: ViewPermission([RoleConstants.USER.get_workspace_role()],
[Permission(group=Group(f"{kwargs.get('source')}_FOLDER"),
operate=Operate.SELF,
resource_path=f"/WORKSPACE/{kwargs.get('workspace_id')}/{kwargs.get('source')}/{r.data.get('parent_id')}")
]), CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
)
@log(menu='folder', operate='Create folder')
def post(self, request: Request, workspace_id: str, source: str):
folder_data = request.data.dict() # Assuming input is JSON
return result.success(FolderCreateAPI().create_folder(workspace_id, source, folder_data))
# Similarly, update other view functions similarly to include logic inside their respective classes or functionsKey Changes Made:
|
||
|
|
||
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 code provided is generally well-formed, but there are a few areas that could be improved:
valuefrom the Enum can make some parts more readable and concise.KNOWLEDGEtwice. Ensure you only assign once if it's correct to do so in each context (e.g., knowledge vs. knowledge folder).group,operate, etc.By making these adjustments, you improve readability, reduce redundancy, and maintain consistency across different sections of your codebase. These changes will also help clarify responsibilities and roles better throughout the app.