Skip to content

feat: Tag permission interface #4225

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
Oct 22, 2025
Merged

feat: Tag permission interface #4225

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
22 changes: 20 additions & 2 deletions apps/common/constants/permission_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ class Operate(Enum):
TO_CHAT = "READ+TO_CHAT" # 去对话
SETTING = "READ+SETTING" # 管理
DOWNLOAD = "READ+DOWNLOAD" # 下载
AUTH = "READ+AUTH"
AUTH = "READ+AUTH" # 资源授权
TAG = "READ+TAG" # 标签设置


class RoleGroup(Enum):
Expand Down Expand Up @@ -325,6 +326,7 @@ def get_workspace_role(self):
Group.APPLICATION.value: _("Application"),
Group.KNOWLEDGE.value: _("Knowledge"),
Group.KNOWLEDGE_DOCUMENT.value: _("Document"),
Group.KNOWLEDGE_TAG.value: _("Tag"),
Group.KNOWLEDGE_PROBLEM.value: _("Problem"),
Group.KNOWLEDGE_HIT_TEST.value: _("Hit-Test"),
Operate.IMPORT.value: _("Import"),
Expand All @@ -350,6 +352,7 @@ def get_workspace_role(self):
Operate.WEIXIN_PUBLIC_ACCOUNT.value: _('Weixin Public Account'),
Operate.ADD_KNOWLEDGE.value: _('Add to Knowledge Base'),
Operate.AUTH.value: _('resource authorization'),
Operate.TAG.value: _('Tag Setting'),
Group.APPLICATION_OVERVIEW.value: _('Overview'),
Group.APPLICATION_ACCESS.value: _('Application Access'),
Group.APPLICATION_CHAT_USER.value: _('Dialogue users'),
Expand All @@ -368,13 +371,15 @@ def get_workspace_role(self):
Group.SYSTEM_MODEL.value: _("Model"),
Group.SYSTEM_KNOWLEDGE.value: _("Knowledge"),
Group.SYSTEM_KNOWLEDGE_DOCUMENT.value: _("Document"),
Group.SYSTEM_KNOWLEDGE_TAG.value: _("Tag"),
Group.SYSTEM_KNOWLEDGE_PROBLEM.value: _("Problem"),
Group.SYSTEM_KNOWLEDGE_HIT_TEST.value: _("Hit-Test"),
Group.SYSTEM_KNOWLEDGE_CHAT_USER.value: _("Dialogue users"),
Group.SYSTEM_RES_TOOL.value: _("Tool"),
Group.SYSTEM_RES_MODEL.value: _("Model"),
Group.SYSTEM_RES_KNOWLEDGE.value: _("Knowledge"),
Group.SYSTEM_RES_KNOWLEDGE_DOCUMENT.value: _("Document"),
Group.SYSTEM_RES_KNOWLEDGE_TAG.value: _("Tag"),
Group.SYSTEM_RES_KNOWLEDGE_PROBLEM.value: _("Problem"),
Group.SYSTEM_RES_KNOWLEDGE_HIT_TEST.value: _("Hit-Test"),
Group.SYSTEM_RES_KNOWLEDGE_CHAT_USER.value: _("Dialogue users"),
Expand All @@ -394,7 +399,6 @@ def get_workspace_role(self):
# SystemGroup.RESOURCE.value: _("Resource"),
}


class Permission:
"""
权限信息
Expand Down Expand Up @@ -665,6 +669,12 @@ class PermissionConstants(Enum):
resource_permission_group_list=[ResourcePermissionConst.KNOWLEDGE_MANGE],
parent_group=[WorkspaceGroup.KNOWLEDGE, UserGroup.KNOWLEDGE]
)
KNOWLEDGE_DOCUMENT_TAG = Permission(
group=Group.KNOWLEDGE_DOCUMENT, operate=Operate.TAG,
role_list=[RoleConstants.ADMIN, RoleConstants.USER],
resource_permission_group_list=[ResourcePermissionConst.KNOWLEDGE_MANGE],
parent_group=[WorkspaceGroup.KNOWLEDGE, UserGroup.KNOWLEDGE]
)
KNOWLEDGE_HIT_TEST = Permission(
group=Group.KNOWLEDGE_HIT_TEST, operate=Operate.READ,
role_list=[RoleConstants.ADMIN, RoleConstants.USER],
Expand Down Expand Up @@ -1224,6 +1234,10 @@ class PermissionConstants(Enum):
group=Group.SYSTEM_KNOWLEDGE_DOCUMENT, operate=Operate.MIGRATE, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.SHARED_KNOWLEDGE], is_ee=settings.edition == "EE"
)
SHARED_KNOWLEDGE_DOCUMENT_TAG = Permission(
group=Group.SYSTEM_KNOWLEDGE_DOCUMENT, operate=Operate.TAG, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.SHARED_KNOWLEDGE], is_ee=settings.edition == "EE"
)
SHARED_KNOWLEDGE_TAG_READ = Permission(
group=Group.SYSTEM_KNOWLEDGE_TAG, operate=Operate.READ, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.SHARED_KNOWLEDGE], is_ee=settings.edition == "EE"
Expand Down Expand Up @@ -1444,6 +1458,10 @@ class PermissionConstants(Enum):
group=Group.SYSTEM_RES_KNOWLEDGE_DOCUMENT, operate=Operate.MIGRATE, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
)
RESOURCE_KNOWLEDGE_DOCUMENT_TAG = Permission(
group=Group.SYSTEM_RES_KNOWLEDGE_DOCUMENT, operate=Operate.TAG, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
)
RESOURCE_KNOWLEDGE_HIT_TEST = Permission(
group=Group.SYSTEM_RES_KNOWLEDGE_HIT_TEST, operate=Operate.READ, role_list=[RoleConstants.ADMIN],
parent_group=[SystemGroup.RESOURCE_KNOWLEDGE], is_ee=settings.edition == "EE"
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 22, 2025

Choose a reason for hiding this comment

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

In this review of the provided Python code, I identify several areas that could be improved:

  1. Duplicate Tag Entries: The Operate.TAG enumeration has two entries with identical values (READ+TAG). This redundancy can lead to confusion and unexpected behavior throughout the application.

  2. Documentation Issues:

    • Some enumerations lack proper documentation comments to explain what each value represents.
    • The function get_workspace_role uses strings like 'resource authorization', which do not match the Enum names used in other parts of the code (e.g., Auth.value.

  3. Suggestion: Consistency with casing should be standardized across all references within the same file. For example, using lower-case naming consistently for enums might improve readability and maintainability.

  4. Enum Naming Conventions: It's recommended to follow PEP 8 guidelines for variable and function naming consistency and clarity.

  5. Comments: Adding more detailed comments about the purpose or intent behind certain actions could help clarify the logic flow.

Suggestions

Suggestion 1: Remove Duplicates

Remove the duplicate entry for OPERATE.TAG. You can choose either one based on which fits better with your design choices.

class Operate(Enum):
    ...
    TAG = "READ+TAG"

Alternatively, combine the meaning into a singular tag-related operation if applicable instead:

class Operate(Enum):
    # existing operations
    TAG_SET = "SET+TAG"  # Example new operation combining setting an attribute
    ```

#### Suggestion 2: Improve Docstrings and Consistency
Ensure each enum and function/variable name follows consistent naming conventions. Update docstrings accordingly where necessary to reflect changes in naming conventions.

Example update:

```python
class Operate(Enum):
    """
    Types of operation permitted by members to access resources.
    
    READ: Allow access to information without modifying.
    WRITE: Modify the data.
    DELETE: Delete the record permanently.
    """

    TO_CHAT = "READ+TO_CHAT"  # Request assistance via a dialogue channel
    SETTING = "READ+SETTING"  # Set preferences related to specific functionalities
    DOWNLOAD = "READ+DOWNLOAD"  # Download a document or asset associated with the project
    AUTH = "READ+AUTH"  # Resource Authorization Operation

# Further updates... similar to above

By implementing these changes, you enhance code clarity, maintainability, and reduce potential errors introduced by redundant or inconsistent elements.

Expand Down
16 changes: 8 additions & 8 deletions apps/knowledge/views/document.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,8 @@ class BatchAddTag(APIView):
tags=[_('Knowledge Base/Documentation')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_DOCUMENT_EDIT.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_EDIT.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.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),
Expand Down Expand Up @@ -724,8 +724,8 @@ class Tags(APIView):
tags=[_('Knowledge Base/Documentation')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.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),
Expand All @@ -745,8 +745,8 @@ def get(self, request: Request, workspace_id: str, knowledge_id: str, document_i
tags=[_('Knowledge Base/Documentation')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.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),
Expand All @@ -770,8 +770,8 @@ class BatchDelete(APIView):
tags=[_('Knowledge Base/Documentation')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_permission_workspace_manage_role(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 22, 2025

Choose a reason for hiding this comment

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

The code provided seems to be related to permissions management for tagging knowledge documents and tags within a system like Knowledge Base Documentation. Here are some observations:

  1. Variable Naming: The variable permission_constants is used multiple times, but it's unclear what kind of constants this represents without additional context. Ensure that this is correctly imported or defined.

  2. Duplicate Permissions: There seems to be two instances where similar permissions (READ, EDIT) are referenced twice with different permission methods (e.g., using get_workspace_knowledge_permission() and get_workspace_permission_workspace_manage_role(). This might lead to confusion if there aren't clear differences between them based on the intended functionality.

  3. Consistency with Permissions: Some functions use one permission method while others use another. Make sure these consistent across the codebase to avoid errors during runtime when permissions need to be checked consistently.

  4. ViewPermission Usage: It’s noted that roles can restrict access to certain operations through views. Ensure that every function checks required capabilities using can_user_view_document_with_permissions.

  5. Logging: Consider adding logging statements at strategic points in your APIs to help with debugging issues. This could involve capturing input parameters, output results, or error messages.

  6. Error Handling: Implement proper exception handling to manage errors gracefully rather than simply failing silently. You should catch exceptions such as permissionDenied errors, invalid inputs, network issues, etc., and handle them appropriately with informative error messages.

  7. Code Review: Conducting a review process among other developers can help identify patterns not immediately apparent by oneself or highlight inefficiencies in structure and logic.

Overall, ensure clarity in terms of role permissions, consistency in implementation practices, and thorough testing across all user interactions involved in managing annotations and tag associations for documentation items.

Expand Down
32 changes: 21 additions & 11 deletions apps/knowledge/views/tag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from common.auth import TokenAuth
from common.auth.authentication import has_permissions
from common.constants.permission_constants import PermissionConstants, RoleConstants
from common.constants.permission_constants import PermissionConstants, RoleConstants, ViewPermission, CompareConstants
from common.log.log import log
from common.result import result
from knowledge.api.tag import TagCreateAPI, TagDeleteAPI, TagEditAPI
Expand All @@ -25,9 +25,11 @@ class KnowledgeTagView(APIView):
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_CREATE.get_workspace_permission(),
PermissionConstants.KNOWLEDGE_TAG_CREATE.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_CREATE.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
RoleConstants.USER.get_workspace_role()
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], CompareConstants.AND),
)
@log(
menu='tag', operate="Create a knowledge tag",
Expand All @@ -47,9 +49,11 @@ def post(self, request: Request, workspace_id: str, knowledge_id: str):
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_permission(),
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
RoleConstants.USER.get_workspace_role()
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], CompareConstants.AND),
)
@log(
menu='tag', operate="Create a knowledge tag",
Expand All @@ -74,9 +78,11 @@ class Operate(APIView):
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_EDIT.get_workspace_permission(),
PermissionConstants.KNOWLEDGE_TAG_EDIT.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_EDIT.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
RoleConstants.USER.get_workspace_role()
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], CompareConstants.AND),
)
@log(
menu='tag', operate="Update a knowledge tag",
Expand All @@ -99,9 +105,11 @@ class Delete(APIView):
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_permission(),
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
RoleConstants.USER.get_workspace_role()
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], CompareConstants.AND),
)
@log(
menu='tag', operate="Delete a knowledge tag",
Expand All @@ -124,9 +132,11 @@ class BatchDelete(APIView):
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_permission(),
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_TAG_DELETE.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
RoleConstants.USER.get_workspace_role()
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], CompareConstants.AND),
)
@log(
menu='tag', operate="Batch Delete knowledge tag",
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 22, 2025

Choose a reason for hiding this comment

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

There are several issues and optimizations that can be addressed in this Python code:

  1. Imports Optimization: Some imports are redundant. For example, ViewPermission is re-imported after being imported earlier. This can be reduced.

  2. Consistent Parameter Passing: Ensure consistent use of string templates for translations to avoid errors.

  3. Redundant Permissions Checks: If you want both WorkspaceManage-role and specific role permissions (like USER) for certain operations, it might make sense to combine them into a single permission check where feasible.

  4. Comments Clarification: Add comments if they're not already present to clarify the purpose of each method or function call.

Here's an optimized version with some corrections:

from common.auth import TokenAuth
from common.auth.authentication import has_permissions
from common.constants.permission_constants import (
    PermissionConstants,
    RoleConstants,
    ViewPermission,
    CompareConstants
)
from common.log.log import log
from common.result import result

class KnowledgeTagView(APIView):
    @method_decorator(has_permissions([
        PermissionConstants.KNOWLEDGE_TAG_CREATE.get_workspace_knowledge_permission(),
        RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
        ViewPermission(RoleConstants.USER.get_workspace_role(), 
                      [PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], 
                      CompareConstants.AND)
    ]))
    @method_decorator(log(menu='tag', operate="Create a knowledge tag"))
    def post(self, request: Request, workspace_id: str, knowledge_id: str) -> Response:
        return success()

def handle_tag_operations():
    """
    Handle various tag-related operations
    """

@handle_tag_operations.method_decorator(has_permissions([
    PermissionConstants.KNOWLEDGE_TAG_READ.get_workspace_knowledge_permission(),
    RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
    ViewPermission(RoleConstants.USER.get_workspace_role(), 
                  [PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], 
                  CompareConstants.AND)
]))
@handle_tag_operations.method_decorator(log(menu='tag', operate="Read a knowledge tag"))
def get_tags(request: Request, workspace_id: str, knowledge_id: str) -> Response:
    return success()

@handle_tag_operations.method_decorator(has_permissions([
    PermissionConstants.KNOWLEDGE_TAG_EDIT.get_workspace_knowledge_permission(),
    RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
    ViewPermission(RoleConstants.USER.get_workspace_role(), 
                  [PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()], 
                  CompareConstants.AND)
]))
@handle_tag_operations.method_decorator(log(menu='tag', operate="Edit a knowledge tag"))
def edit_tags(request: Request, workspace_id: str, knowledge_id: str) -> Response:
    return success()

# Similar functions will be created similarly...

Key Changes Made:

  • Reduced redundancy in imports.
  • Consolidated identical logic across methods using decorators in Django viewset-style methods (APIView.post, etc.).
  • Combined similar checks into a single decorator call when applicable.

This refactoring improves maintainability and reduces chances for errors caused by duplicated logic.

Expand Down
6 changes: 6 additions & 0 deletions apps/locales/en_US/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8730,4 +8730,10 @@ msgid "Sample Rate"
msgstr ""

msgid "Captcha is required"
msgstr ""

msgid "Tag"
msgstr ""

msgid "Tag Setting"
msgstr ""
8 changes: 7 additions & 1 deletion apps/locales/zh_CN/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8856,4 +8856,10 @@ msgid "Sample Rate"
msgstr "采样率"

msgid "Captcha is required"
msgstr "验证码是必填项"
msgstr "验证码是必填项"

msgid "Tag"
msgstr "标签管理"

msgid "Tag Setting"
msgstr "标签设置"
8 changes: 7 additions & 1 deletion apps/locales/zh_Hant/LC_MESSAGES/django.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8856,4 +8856,10 @@ msgid "Sample Rate"
msgstr "採樣率"

msgid "Captcha is required"
msgstr "驗證碼是必填項"
msgstr "驗證碼是必填項"

msgid "Tag"
msgstr "標籤管理"

msgid "Tag Setting"
msgstr "標籤設定"
Loading