Skip to content

fix: Application log permission error #4087

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
merged 1 commit into from
Sep 23, 2025
Merged

fix: Application log permission error #4087

Show file tree
Hide file tree
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
15 changes: 13 additions & 2 deletions apps/application/views/application_version_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ class Page(APIView):
ApplicationVersionApi.Query.get_request_params_api()),
responses=result.get_page_api_response(ApplicationVersionApi.get_response_body_api()),
tags=[_('Application/Version')])
@has_permissions(PermissionConstants.APPLICATION_READ, compare=CompareConstants.AND)
@has_permissions(PermissionConstants.APPLICATION_READ,
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND), compare=CompareConstants.AND)
def get(self, request: Request, application_id: str, current_page: int, page_size: int):
return result.success(
ApplicationVersionSerializer.Query(
Expand All @@ -65,7 +69,14 @@ class Operate(APIView):
manual_parameters=ApplicationVersionApi.Operate.get_request_params_api(),
responses=result.get_api_response(ApplicationVersionApi.get_response_body_api()),
tags=[_('Application/Version')])
@has_permissions(PermissionConstants.APPLICATION_READ, compare=CompareConstants.AND)
@has_permissions(PermissionConstants.APPLICATION_READ, ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(
group=Group.APPLICATION,
operate=Operate.USE,
dynamic_tag=keywords.get(
'application_id'))],
compare=CompareConstants.AND),
compare=CompareConstants.AND)
def get(self, request: Request, application_id: str, work_flow_version_id: str):
return result.success(
ApplicationVersionSerializer.Operate(
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 code snippet is mostly correct but can be improved for clarity and robustness:

Issues Identified:

  1. Redundant compare=CompareConstants.AND:

    • The second @has_permissions decorator contains an unnecessary compare=CompareConstants.AND.

  2. Lack of Consistency in Method Naming:

    • The method names should match the endpoint they are associated with (e.g., get) to maintain consistency.

Optimization Suggestions:

  1. Parameter Validation:

    • Add input validation to ensure that application_id, current_page, and page_size are valid integers before processing.

  2. Use a More Concise Permission Check Setup:

    • Combine similar permissions checks into a single instance or use context managers for better readability and efficiency.

Here's the revised version:

from rest_framework import APIView
from rest_framework.request import Request
from .models import ApplicationVersionApi, OperateAPI, Group, OperationalType, \
                       Permission, CompareConstants, has_permissions

class Page(APIView):
    @has_permissions(PermissionConstants.APPLICATION_READ)
    def get(self, request: Request, application_id: str, current_page: int, page_size: int):
        # Validate inputs
        if not application_id.isdigit() or not page_size.isdigit():
            return result.failure("Invalid parameters")

        try:
            response_data = ApplicationVersionSerializer.Query(
                filter_params=(
                    {'application_id': application_id},
                    *result.get_query_params(request.query_params),
                ),
                pagination_info={'limit': page_size, 'offset': (int(page_size) * (int(current_page) - 1))}
            )
        except Exception as e:
            return result.exception(e)

        return result.success(response_data)


class Operate(APIView):
    @has_permissions(PermissionConstants.APPLICATION_READ)
    def get(self, request: Request, application_id: str, workflow_version_id: str):
        # Validate inputs
        if not application_id.isdigit() or not workflow_version_id.isdigit():
            return result.failure("Invalid parameters")

        try:
            response_data = ApplicationVersionSerializer.Operate(
                filter_params={
                    'workflow_version_id': workflow_version_id
                }
            )
        except Exception as e:
            return result.exception(e)

        return result.success(response_data)

Key Improvements:

  • Consistent Method Names: Ensures that each method name corresponds to its endpoint.
  • Input Validation: Added basic validation to check if application_id and page_size are digits.
  • Error Handling: Wrapped exception handling around operations to provide more meaningful error messages.
  • Reduced Redundancy: Eliminated redundant compare=CompareConstants.AND.
  • Readability: Improved variable naming and structure for better readability.

Expand Down
40 changes: 26 additions & 14 deletions apps/application/views/chat_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ class Export(APIView):
@has_permissions(
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
@log(menu='Conversation Log', operate="Export conversation",
get_operation_object=lambda r, k: get_application_operation_object(k.get('application_id')))
Expand Down Expand Up @@ -164,7 +165,9 @@ def post(self, request: Request, chat_id: str):
@has_permissions(
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND
)
)
def get(self, request: Request, application_id: str):
return result.success(ChatSerializers.Query(
Expand All @@ -182,8 +185,7 @@ class Operate(APIView):
[RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.MANAGE,
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND),
compare=CompareConstants.AND)
compare=CompareConstants.AND))
@log(menu='Conversation Log', operate="Delete a conversation",
get_operation_object=lambda r, k: get_application_operation_object(k.get('application_id')))
def delete(self, request: Request, application_id: str, chat_id: str):
Expand All @@ -206,7 +208,8 @@ class ClientChatHistoryPage(APIView):
@has_permissions(
ViewPermission([RoleConstants.APPLICATION_ACCESS_TOKEN],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
def get(self, request: Request, application_id: str, current_page: int, page_size: int):
return result.success(ChatSerializers.ClientChatHistory(
Expand Down Expand Up @@ -267,7 +270,8 @@ class Page(APIView):
@has_permissions(
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
def get(self, request: Request, application_id: str, current_page: int, page_size: int):
return result.success(ChatSerializers.Query(
Expand All @@ -292,7 +296,8 @@ class Operate(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY,
RoleConstants.APPLICATION_ACCESS_TOKEN],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
def get(self, request: Request, application_id: str, chat_id: str, chat_record_id: str):
return result.success(ChatRecordSerializer.Operate(
Expand All @@ -310,7 +315,8 @@ def get(self, request: Request, application_id: str, chat_id: str, chat_record_i
@has_permissions(
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
def get(self, request: Request, application_id: str, chat_id: str):
return result.success(ChatRecordSerializer.Query(
Expand All @@ -329,9 +335,11 @@ class Page(APIView):
tags=[_("Application/Conversation Log")]
)
@has_permissions(
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY],
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY,
RoleConstants.APPLICATION_ACCESS_TOKEN],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
def get(self, request: Request, application_id: str, chat_id: str, current_page: int, page_size: int):
return result.success(ChatRecordSerializer.Query(
Expand All @@ -354,7 +362,8 @@ class Vote(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY,
RoleConstants.APPLICATION_ACCESS_TOKEN],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND)
)
@log(menu='Conversation Log', operate="Like, Dislike",
get_operation_object=lambda r, k: get_application_operation_object(k.get('application_id')))
Expand All @@ -377,7 +386,7 @@ class ChatRecordImprove(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))]
))
, compare=CompareConstants.AND))
def get(self, request: Request, application_id: str, chat_id: str, chat_record_id: str):
return result.success(ChatRecordSerializer.ChatRecordImprove(
data={'chat_id': chat_id, 'chat_record_id': chat_record_id}).get())
Expand All @@ -397,7 +406,7 @@ class Improve(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))],

compare=CompareConstants.AND
), ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.DATASET,
operate=Operate.MANAGE,
Expand All @@ -424,6 +433,7 @@ def put(self, request: Request, application_id: str, chat_id: str, chat_record_i
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND

), ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.DATASET,
Expand Down Expand Up @@ -451,6 +461,7 @@ class Operate(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))],
compare=CompareConstants.AND

), ViewPermission([RoleConstants.ADMIN, RoleConstants.USER],
[lambda r, keywords: Permission(group=Group.DATASET,
Expand Down Expand Up @@ -499,7 +510,8 @@ class UploadFile(APIView):
ViewPermission([RoleConstants.ADMIN, RoleConstants.USER, RoleConstants.APPLICATION_KEY,
RoleConstants.APPLICATION_ACCESS_TOKEN],
[lambda r, keywords: Permission(group=Group.APPLICATION, operate=Operate.USE,
dynamic_tag=keywords.get('application_id'))])
dynamic_tag=keywords.get('application_id'))]
, compare=CompareConstants.AND)
)
def post(self, request: Request, application_id: str, chat_id: str):
files = request.FILES.getlist('file')
Expand Down
Loading