Skip to content

fix: Knowledge workflow permission #4434

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
Dec 4, 2025
Merged

fix: Knowledge workflow permission #4434

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
2 changes: 1 addition & 1 deletion apps/application/flow/step_node/data_source_web_node/impl/base_data_source_web_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def execute(self, **kwargs) -> NodeResult:
try:
ForkManage(source_url, selector.split(" ") if selector is not None else []).fork(3, set(), collect_handler)

return NodeResult({'document_list': document_list},
return NodeResult({'document_list': document_list,'source_url': source_url, 'selector': selector},
self.workflow_manage.params.get('knowledge_base') or {})

except Exception as e:
Expand Down
2 changes: 1 addition & 1 deletion apps/application/flow/step_node/knowledge_write_node/impl/base_knowledge_write_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def execute(self, documents, **kwargs) -> NodeResult:
"paragraphs": [{
"title": p.get("title"),
"content": p.get("content"),
} for p in document.get("paragraphs")[0:4]]
} for p in document.get("paragraphs")[0:5]]
} for document in documents]

return NodeResult({'write_content': write_content_list}, {})
Expand Down
42 changes: 32 additions & 10 deletions apps/knowledge/views/knowledge_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,34 @@
class KnowledgeDatasourceFormListView(APIView):
authentication_classes = [TokenAuth]

@has_permissions(
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND
),
)
def post(self, request: Request, workspace_id: str, knowledge_id: str, type: str, id: str):
return result.success(KnowledgeWorkflowSerializer.Datasource(
data={'type': type, 'id': id, 'params': request.data, 'function_name': 'get_form_list'}).action())


class KnowledgeDatasourceView(APIView):
authentication_classes = [TokenAuth]

@has_permissions(
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND
),
)
def post(self, request: Request, workspace_id: str, knowledge_id: str, type: str, id: str, function_name: str):
return result.success(KnowledgeWorkflowSerializer.Datasource(
data={'type': type, 'id': id, 'params': request.data, 'function_name': function_name}).action())
Expand All @@ -45,8 +67,8 @@ class KnowledgeWorkflowUploadDocumentView(APIView):
tags=[_('Knowledge Base')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
Expand Down Expand Up @@ -99,8 +121,8 @@ class Operate(APIView):
tags=[_('Knowledge Base')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
Expand Down Expand Up @@ -204,8 +226,8 @@ def put(self, request: Request, workspace_id: str, knowledge_id: str):
tags=[_('Knowledge Base')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
Expand All @@ -232,8 +254,8 @@ class KnowledgeWorkflowVersionView(APIView):
tags=[_('Knowledge Base')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
Expand All @@ -260,8 +282,8 @@ class McpServers(APIView):
responses=SpeechToTextAPI.get_response(),
tags=[_('Knowledge Base')] # type: ignore
)
@has_permissions(PermissionConstants.KNOWLEDGE_READ.get_workspace_application_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_application_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_application_permission()],
CompareConstants.AND),
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

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

The code snippet you provided is primarily a series of views defined in an API using Django Rest Framework (DRF). I've made a brief analysis to identify any irregularities, issues, or optimizations:

Issues

  1. Duplicated authentication_classes: Each view has the same authentication_classes, which can be redundant unless there's specific context where each view should have different authentication strategies.

  2. Unused Import Statements:

    from rest_framework.decorators import api_view, permission_classes

    These imports might not actually be used in this section if they're intended for other parts of the project. They could be removed without affecting functionality.

  3. Lack of Type Hinting on Return Types:
    While Python supports type hinting through PEP 484 and later versions, these details are not shown here. It's advisable to add explicit typing if available.

  4. Potential Duplicates:
    The last @has_permissions decorator call seems identical to one further up, possibly indicating a copy-paste error when copying permissions across methods.

Suggestions

  1. Consistent Authentication Classes:
    If all views need similar types of authentication, consider centralizing these at either the application level (in settings) or using middleware.

    authentication_classes = [TokenAuth]  # Consider whether this should vary per view

  2. Remove Unused Imports:
    Review the entire file for unused imports and remove them to keep it clean and efficient.

  3. Type Hints:
    If you have access to type hints, add them for better readability and maintainability.

  4. Review Permissions Across Methods:
    Ensure that permission logic is consistent within each method, especially when handling similar functionalities.

Here's a simplified version with some suggested improvements:

from django.utils.translation import gettext_lazy as _
from drf_yasg.utils import swagger_auto_schema
from rest_framework.authentication import TokenAuthentication
from rest_framework.permissions import IsAuthenticated, AllowAny
from rest_framework.response import Response
from rest_framework.views import GenericAPIView

class KnowledgeWorkflowFormListView(GenericAPIView):
    authentication_classes = [TokenAuthentication]
    permission_classes = [IsAuthenticated]

    @swagger_autoSchema(
        methods=['post'],
        tags=[_('Knowledge Base')]
    )
    def post(self, request, workspace_id, knowledge_id, type, id):
        return Response({'ok': True})

# Similar patterns apply to other views...

class McpServers(GenericAPIView):
    authentication_classes = [TokenAuthentication]
    permission_classes = [IsAuthenticated]

    @swagger_autoSchema(
        methods=['get'],
        responses={
            status.HTTP_200_OK: {'application/json', SpeechToTextAPI.get_response()}
        },
        tags=[_('Knowledge Base')]
    )
    def get(self, request, workspace_id, application_id):
        return Response({'ok': True})

This example removes unnecessary imports, standardizes authentication classes, adds basic comments, and includes simple response structures for demonstration purposes.

Expand Down
16 changes: 8 additions & 8 deletions apps/knowledge/views/knowledge_workflow_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class KnowledgeWorkflowVersionView(APIView):
responses=KnowledgeVersionListAPI.get_response(),
tags=[_('Knowledge/Version')] # type: ignore
)
@has_permissions(PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND),
Expand All @@ -68,8 +68,8 @@ class Page(APIView):
responses=KnowledgeVersionPageAPI.get_response(),
tags=[_('Knowledge/Version')] # type: ignore
)
@has_permissions(PermissionConstants.KNOWLEDGE_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_READ.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND),
Expand All @@ -93,8 +93,8 @@ class Operate(APIView):
responses=KnowledgeVersionOperateAPI.get_response(),
tags=[_('Knowledge/Version')] # type: ignore
)
@has_permissions(PermissionConstants.KNOWLEDGE_EDIT.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_EDIT.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_READ.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND),
Expand All @@ -115,8 +115,8 @@ def get(self, request: Request, workspace_id: str, knowledge_id: str, knowledge_
responses=KnowledgeVersionOperateAPI.get_response(),
tags=[_('Knowledge/Version')] # type: ignore
)
@has_permissions(PermissionConstants.KNOWLEDGE_EDIT.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_EDIT.get_workspace_permission_workspace_manage_role(),
@has_permissions(PermissionConstants.KNOWLEDGE_WORKFLOW_EDIT.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_WORKFLOW_EDIT.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND),
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

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

There do not appear to be obvious issues or opportunities for optimization in this code snippet related to permissions handling. Each view is correctly decorated with @has_permissions using specific permission constants. The use of ViewPermission([RoleConstants.USER.get_workspace_role()], ...) ensures that only users with the appropriate roles can access these endpoints.

However, there are some areas where you might want additional validation or clarification:

  1. Workspace IDs Check: Ensure that workspace_id and knowledge_id are provided as query parameters rather than being hard-coded into the URL. This prevents unauthorized modification through the URL structure.

  2. Type Hints: While type hints are used, they aren't explicitly checked during runtime. Consider adding type checking logic if needed.

  3. Response Types: Verify that the response types returned by each method match the expected responses defined elsewhere in the codebase (e.g., KnowledgeVersionListAPI, KnowledgeVersionPageAPI, etc.).

  4. Logging and Debugging: Add logging statements within methods to debug if necessary, especially around where permissions are applied.

Overall, the code looks functionally correct assuming it aligns well with the broader application's security policies regarding permission management.

Expand Down
13 changes: 11 additions & 2 deletions ui/src/views/knowledge-workflow/component/PublishHistory.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@
</el-button>
<template #dropdown>
<el-dropdown-menu>
<el-dropdown-item @click.stop="openEditVersion(row)">
<el-dropdown-item
v-if="permissionPrecise.workflow_edit(id)"
@click.stop="openEditVersion(row)"
>
<AppIcon iconName="app-edit" class="color-secondary"></AppIcon>
{{ $t('common.edit') }}
</el-dropdown-item>
Expand Down Expand Up @@ -75,9 +78,11 @@ import { datetimeFormat } from '@/utils/time'
import { MsgSuccess, MsgError } from '@/utils/message'
import { t } from '@/locales'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import permissionMap from '@/permission'

const route = useRoute()
const {
params: { id },
params: { id, folderId },
} = route as any
const apiType = computed(() => {
if (route.path.includes('shared')) {
Expand All @@ -89,6 +94,10 @@ const apiType = computed(() => {
}
})

const permissionPrecise = computed(() => {
return permissionMap['knowledge'][apiType.value]
})

const emit = defineEmits(['click', 'refreshVersion'])
const loading = ref(false)
const LogData = ref<any[]>([])
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

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

This code is generally well-written and follows standard practices. However, there are a few minor adjustments and optimizations you might consider:

  1. Remove Unused Variables: You are passing params to the useRef hook within an as any assertion, which can introduce type safety risks. Ensure that id and folderId are correctly typed in your component's interface.

  2. Use Computed Properties Efficiently: The logic inside permissionPrecise could benefit from caching since it doesn't change based on reactive state. Consider using computed() instead of reassigning in the setup function.

Here’s how you can revise the code with these considerations:

// Import necessary components and dependencies at the top

const route = useRoute()
const {
  params: { id, folderId },
} = route as { params: { id?: string; folderId?: string } };

const apiType = computed(() => {
  if (route.path.includes('shared')) {
    return 'shared';
  }
});

const permissionMap = {
  knowledge: new Map([
    ['published', () => true], // Example map value
    ['draft', () => false],
    // Add other rules as needed
  ]),
};

const permissionPrecise = computed(() => {
  return permissionMap[knowledge][apiType.value];
});

const emit = defineEmits(['click', 'refreshVersion']);
const loading = ref(false);
const LogData = ref<any[]>([]);

Key Adjustments:

  • Removed the unnecessary params cast to {} and used an explicit TypeScript definition for destructuring.
  • Simplified the logic inside permissionPrecise to avoid repeated computations.
  • Used a simple mapping structure (Map) to store permissions for different types, making it easier to manage and extend later.

These changes improve clarity, maintainability, and reduce potential pitfalls such as runtime errors due to incorrect type assumptions.

Expand Down
Loading