Skip to content

feat: system chat user permission #7802

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 10, 2026
Merged

feat: system chat user permission #7802

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/chat-management/users/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ onMounted(() => loadChatUsers())
</el-button>
</MkTooltip>
<!-- 修改用户密码 -->
<ButtonChangeUserPassword :user="row" @refresh="loadChatUsers(false)" />
<ButtonChangeUserPassword v-if="perm.system.chatUser.edit()" :user="row" @refresh="loadChatUsers(false)" />
<!-- 更多 -->
<MkTableMoreDropdown>
<!-- 配额设置 -->

@shaohuzhang1 shaohuzhang1 Nov 10, 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.

Review of the diff

-<ButtonChangeUserPassword :user="row" @refresh="loadChatUsers(false)" />
+<ButtonChangeUserPassword v-if="perm.system.chatUser.edit()" :user="row" @refresh="loadChatUsers(false)" />

1. Correctness / API assumptions

  • edit() is being invoked per row render. If perm.system.chatUser.edit is a boolean/computed ref rather than a function, this will throw (... is not a function) or silently misbehave. Confirm it is a function everywhere else (other files/lines in this component) — the same permission is likely used as perm.system.chatUser.edit (no parentheses) elsewhere, which would be an inconsistency.
  • perm must be reactive (e.g. a reactive/computed store) for the v-if to re-evaluate after permissions are loaded asynchronously. If it's a plain object populated after mount, rows rendered before permission load will keep showing/hiding the button incorrectly.
  • v-if on a component is fine (it unmounts the component, avoids the :user prop binding and internal watchers). Prefer v-if over v-show for permission gating. No key needed.

2. Permission semantics (most important)

  • Using chatUser.edit for "change user password" may be wrong. Resetting/changing another user's password is a more sensitive operation and often has its own permission (e.g. chatUser.resetPassword, chatUser.changePassword, or admin-only). Using the generic edit permission can both over-grant (editors can reset passwords) and under-grant (a support role that may reset passwords but can't edit profiles).
  • Front-end v-if is UX only, not security. Verify the backend endpoint enforcing this action also checks the same permission; otherwise this change is cosmetic.
  • Inconsistency within the action column. The sibling MkTableMoreDropdown (quota settings, etc.) presumably also needs permission gating. If only this button is gated, the column becomes inconsistent — audit the whole cell, and consider hiding the entire "操作" (actions) column when the user has none of the permissions.

3. Performance / readability

  • The predicate is global, not per-row, yet it is evaluated for every row on every re-render. Hoist it: 
    const canEditChatUser = computed(() => perm.system.chatUser.edit())
    <ButtonChangeUserPassword
  v-if="canEditChatUser"
  :user="row"
  @refresh="loadChatUsers(false)"
/>
  • Alternatively, if this pattern repeats, introduce a v-permission="'system:chatUser:edit'" directive or a hasPerm() helper for consistency across the codebase.

4. Line length / lint

  • The new line is ~123 characters including indentation; if the project uses Prettier/ESLint with a 120-char limit (common), CI will fail formatting. The multi-line form above avoids this.

5. Minor

  • The hunk header (-256,7 +256,7) matches the shown context (7 old / 7 new lines) — no count mismatch.
  • @refresh="loadChatUsers(false)" is an inline handler expression: it works, but a clearer form is @refresh="() => loadChatUsers(false)" (same behavior, no ambiguity about the arguments).
  • The Chinese comments are fine for the codebase, but ensure the <!-- 修改用户密码 --> comment is updated/kept adjacent to the gated element (it is).

Suggested final version

<!-- 修改用户密码 -->
<ButtonChangeUserPassword
  v-if="canEditChatUser"
  :user="row"
  @refresh="loadChatUsers(false)"
/>

plus

const canEditChatUser = computed(() => perm.system.chatUser.edit())

and confirm (a) edit is a function, (b) it's the intended permission for password changes, and (c) the backend enforces it.

Expand Down
Loading