Skip to content

fix: Application read permission #4338

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
zhanweizhang7 merged 1 commit into from
Nov 10, 2025
Merged

fix: Application read permission #4338

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
8 changes: 4 additions & 4 deletions apps/application/views/application_chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ class OpenView(APIView):
responses=None,
tags=[_('Application')] # type: ignore
)
@has_permissions(PermissionConstants.APPLICATION_EDIT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EDIT.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.APPLICATION_READ.get_workspace_application_permission(),
PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.APPLICATION.get_workspace_application_permission()],
CompareConstants.AND),
Expand Down Expand Up @@ -167,8 +167,8 @@ class PromptGenerateView(APIView):
responses=None,
tags=[_('Application')] # type: ignore
)
@has_permissions(PermissionConstants.APPLICATION_EDIT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EDIT.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.APPLICATION_READ.get_workspace_application_permission(),
PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.APPLICATION.get_workspace_application_permission()],
CompareConstants.AND),
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code appears to be part of Django REST Framework views that handle CRUD (Create, Read, Update, Delete) operations on application prompts and applications within a workspace. Here are some observations:

Code Snippets Analysis

  1. @has_permissions() Usage:

    • Both PromptGenerateView and OpenView use @has_permissions() decorator with similar logic but differing permission types.
    • The first argument to has_permissions() is for the edit operation (APPLICATION_EDIT), while the second argument covers manage role permissions.

  2. Permission Constants:

    • Replace ApplicationConstants.APPLICATION_EDIT.get_workspace_application_permission() with ApplicationConstants.APPLICATION_READ.get_workspace_application_permission().
    • Similarly, replace ApplicationConstants.APPLICATION_EDIT.get_workspace_permission_workspace_manage_role() with ApplicationConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role().

  3. Tags in API Routes:

    • Both routes have the same tag "_("Application")", which might not be necessary given their context. Consider using more specific tags.

  4. Role Permissions:

    • There's no change needed in the role-based access control aspects since both views operate under the user being able to read or edit the workspace roles and app related permissions.

Summary

  • Minor Error: Change the permissions from EDIT_APPLICATION to READ_APPLICATION.
  • Suggested Improvement: Remove redundant WorkspaceManageRole checks if they don't add any additional security.
  • Additional Enhancements: Ensure that the tags align with their purpose and make them less generic.

Updated Code Example

class OpenView(APIView):
    @swagger_auto_schema(responses={}, tags=[_('Application')])
    @has_permissions(
        ApplicationConstants.APPLICATION_READ.get_workspace_application_permission(),
        PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role()
    )
    def get(self, request):
        # Implement your get logic here
        pass

class PromptGenerateView(APIView):
    @swagger_auto_schema(responses={}, tags=[_('Application')])
    @has_permissions(
        ApplicationConstants.APPLICATION_READ.get_workspace_application_permission(),
        PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role()
    )
    def get(self, request):
        # Implement your get logic here
        pass

This revised code addresses the minor issue with permission constants and ensures clarity in the usage of the tags.

Expand Down
4 changes: 2 additions & 2 deletions apps/application/views/application_chat_record.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class ApplicationChatRecordOperateAPI(APIView):
)
@has_permissions(PermissionConstants.APPLICATION_CHAT_LOG_READ.get_workspace_application_permission(),
PermissionConstants.APPLICATION_CHAT_LOG_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.APPLICATION_EDIT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EDIT.get_workspace_permission_workspace_manage_role(),
PermissionConstants.APPLICATION_READ.get_workspace_application_permission(),
PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.APPLICATION.get_workspace_application_permission()],
CompareConstants.AND),
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one issue or change proposed in this code snippet:

Original line:

PermissionConstants.APPLICATION_EDIT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EDIT.get_workspace_permission_workspace_manage_role()

Modified line:

PermissionConstants.APPLICATION_READ.get_workspace_application_permission(),
PermissionConstants.APPLICATION_READ.get_workspace_permission_workspace_manage_role()

Change Description:
The permissions APPLICATION_EDIT have been replaced with APPLICATION_READ, which could lead to fewer access rights being granted compared to previously.

Optimization/Suggestion:
Consider if there was a reason for initially giving edit permissions (e.g., due to system requirements or specific business logic). If not, it may be better to adjust the permissions accordingly.

However, as of now, the current modification removes edit capabilities from users who would typically need that level of permission.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/permission/application/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ const workspace = {
[
new ComplexPermission([RoleConst.USER],[PermissionConst.APPLICATION.getApplicationWorkspaceResourcePermission(source_id)],[],'AND'),
RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
PermissionConst.APPLICATION_EDIT.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_EDIT.getApplicationWorkspaceResourcePermission(source_id)
PermissionConst.APPLICATION_READ.getWorkspacePermissionWorkspaceManageRole,
PermissionConst.APPLICATION_READ.getApplicationWorkspaceResourcePermission(source_id)
],
'OR'
),
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code provided has a couple of minor issues:

  1. Duplicate Permissions: The PermissionConst.APPLICATION_EDIT is included twice: once for creating the permission (PermissionConst.APPLICATION Edit.getWorkspacePermissionWorkspaceManageRole) and again with the same conditions but under a different source ID.

    Suggestion:

    new ComplexPermission(
    [
        // Removed duplicate PermissionConst.APPLICATION_EDIT due to OR condition
        RoleConst.WORKSPACE_MANAGE.getWorkspaceRole,
        PermissionConst.APPLICATION_READ.getWorkspacePermissionWorkspaceManageRole,
        PermissionConst.APPLICATION_READ.getApplicationWorkspaceResourcePermission(source_id)
    ],
    'OR'
)

  2. Consistency in Code Formatting: The code could benefit from better formatting consistency. However, this issue might be subjective.

  3. Code Readability: Ensure that each section of the list follows a consistent format for readability. For example, all roles should have their own entry within brackets like [RoleConst.WORKSPACE_MANAGE] instead of just using the role name directly without brackets.

These changes will improve the clarity and maintainability of the code while resolving the identified issues.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/application-workflow/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
<AppIcon iconName="app-debug-outlined" class="mr-4"></AppIcon>
{{ $t('views.applicationWorkflow.setting.debug') }}
</el-button>
<el-button @click="saveApplication(true)">
<el-button @click="saveApplication(true)" v-if="permissionPrecise.edit(id)">
<AppIcon iconName="app-save-outlined" class="mr-4"></AppIcon>
{{ $t('common.save') }}
</el-button>
<el-button type="primary" @click="publish">
<el-button type="primary" @click="publish" v-if="permissionPrecise.edit(id)">
{{ $t('views.application.operation.publish') }}
</el-button>

Expand Down
Loading