Skip to content

fix: system shared knowledge setting permission #7761

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
peslyn merged 1 commit into from
Nov 9, 2026
Merged

fix: system shared knowledge setting permission #7761

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 ui/src/views/system/shared-resources/knowledge/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function handleSearchChange(query?: Dict<unknown>) {
v-if="perm.knowledge.share.generate()"
/>
<!-- 设置 -->
<MkDropdownItem v-if="perm.knowledge.share.edit()" @click.stop="handleOpenKnowledgeSetting(knowledge)">
<MkDropdownItem v-if="perm.knowledge.share.settingRead()" @click.stop="handleOpenKnowledgeSetting(knowledge)">
<template #icon><MkIcon name="icon_setting" /></template>
设置
</MkDropdownItem>

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The change looks reasonable at a glance, but here are things to verify:

  1. Permission method name mismatch risk – Confirm that perm.knowledge.share.settingRead() actually exists in the perm object/module. If it's a newly added method, ensure it's defined and exported; otherwise this will throw at runtime (or be undefined → falsy → menu item silently hidden).

  2. Naming semantics vs. action – The handler is handleOpenKnowledgeSetting (an edit action). If settingRead() only grants read permission for settings, opening the settings dialog may still allow edits inside it — check that the inner settings UI also enforces write permissions. Otherwise you've only changed the visibility of the entry point, not the actual authorization.

  3. Consistency with sibling items – The item above uses perm.knowledge.share.generate() (a verb). Mixing generate() / edit() / settingRead() naming styles is inconsistent; consider a uniform convention (e.g., canEditSettings vs settingsReadable) to avoid confusion later.

  4. v-if vs v-show – Fine as-is if this is a dropdown that conditionally renders; no issue.

  5. Hardening suggestion – For defense-in-depth, add a guard inside handleOpenKnowledgeSetting:

    if (!perm.knowledge.share.settingRead()) return;

    so permission is enforced even if the template check is accidentally removed or the method is called from elsewhere.

  6. Test coverage – Add/verify unit tests for: user with read-only settings permission (item visible, edit controls disabled), user without any permission (item hidden), and admin (full access).

  7. i18n – The label 设置 is hardcoded Chinese; if the project uses i18n elsewhere in this component, wrap it ({{ $t('...') }}) for consistency. (Pre-existing issue, not introduced by this diff.)

No functional bug is evident in the diff itself, but points 1 and 2 are the ones most likely to bite you.

Expand Down
Loading