Skip to content

feat: system scan login permission #7783

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 scan login permission #7783

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
5 changes: 3 additions & 2 deletions ui/src/views/system/chat-management/authentication/components/SCANLogin.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import dingtalkLogo from '@/assets/logo/logo_dingtalk.svg'
import enterpriseWechatLogo from '@/assets/logo/logo_enterprise-wechat.svg'
import larkLogo from '@/assets/logo/logo_lark.svg'
import EditSCANDrawer from '../EditSCANDrawer.vue'
import perm from '@/permission'

interface QrLoginPlatformView extends QrLoginPlatformPayload {
isValid: boolean
Expand Down Expand Up @@ -88,12 +89,12 @@ onMounted(loadQrPlatforms)

<div class="flex-align-center gap-4">
<!-- 配置扫码登录平台 -->
<el-button plain @click.stop="handleEditPlatform(platform)">{{ platform.isValid ? '配置' : '未配置' }}</el-button>
<el-button :disabled="!perm.system.chatAuth.edit()" plain @click.stop="handleEditPlatform(platform)">{{ platform.isValid ? '配置' : '未配置' }}</el-button>
<template v-if="platform.isValid">
<el-divider direction="vertical" />
<!-- 启停扫码登录，阻止点击打开配置 -->
<span class="inline-flex" @click.stop>
<el-switch v-model="platform.isActive" @change="handlePlatformStatusChange(platform)" />
<el-switch :disabled="!perm.system.chatAuth.edit()" v-model="platform.isActive" @change="handlePlatformStatusChange(platform)" />
</span>
</template>
</div>

@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.

Here is an analysis of the provided code diff:

✅ Positive Aspects

  1. Consistent Permission Check: You correctly added perm.system.chatAuth.edit() checks to both the el-button and the el-switch, which is good for security and UX consistency.
  2. UX Improvement: Disabling the controls based on permissions prevents users from attempting actions they are not authorized for.

⚠️ Potential Issues & Irregularities

1. Performance: Repeated Function Calls in Template

Issue: You are calling !perm.system.chatAuth.edit() directly in the template for every platform item in a v-for loop (implied by the context).
Risk: If the list of platforms is long, this function might be called excessively during re-renders, causing unnecessary computation.
Suggestion: Compute the permission once before rendering.

2. Logic Redundancy / Edge Case

Issue: The el-switch has @click.stop on its wrapper span (<span class="inline-flex" @click.stop>). When disabled is true, Element Plus usually handles interaction prevention internally. However, relying solely on visual disable without ensuring the underlying logic respects the permission in the event handler (handlePlatformStatusChange) can be risky if someone bypasses UI restrictions (e.g., via console or API).
Suggestion: Ensure handleEditPlatform and handlePlatformStatusChange also check permissions internally, even if the UI is disabled.

3. Import Path & Naming Convention

Issue: import perm from '@/permission' suggests a default export. Ensure this aligns with your project’s convention. Also, chatAuth should ideally reflect a clear domain boundary.

4. Accessibility (a11y)

Issue: Disabled buttons don’t communicate why they are disabled.
Suggestion: Consider adding a tooltip or note when the button is disabled due to lack of permission.

💡 Optimized Code Suggestion

<script setup lang="ts">
import { computed } from 'vue'; // Assuming Vue 3 script setup
import perm from '@/permission';

// ... existing imports and code ...

// Define a computed property to avoid repeated function calls
const canEditChatAuth = computed(() => perm.system.chatAuth.edit());

// Optional: Add internal permission check in handlers for defense-in-depth
const handleEditPlatform = (platform: QrLoginPlatformView) => {
  if (!canEditChatAuth.value) return;
  // ... existing logic ...
};

const handlePlatformStatusChange = async (platform: QrLoginPlatformView) => {
  if (!canEditChatAuth.value) return;
  // ... existing logic ...
};

onMounted(loadQrPlatforms);
</script>

<template>
  <!-- ... -->
  <div class="flex-align-center gap-4">
    <!-- 配置扫码登录平台 -->
    <el-button 
      :disabled="!canEditChatAuth" 
      plain 
      @click.stop="handleEditPlatform(platform)"
    >
      {{ platform.isValid ? '配置' : '未配置' }}
    </el-button>

    <template v-if="platform.isValid">
      <el-divider direction="vertical" />
      <!-- 启停扫码登录，阻止点击打开配置 -->
      <span class="inline-flex" @click.stop>
        <el-switch 
          :disabled="!canEditChatAuth" 
          v-model="platform.isActive" 
          @change="handlePlatformStatusChange(platform)" 
        />
      </span>
    </template>
  </div>
  <!-- ... -->
</template>

Summary of Recommendations

  1. Extract Permission Check: Use a computed property (canEditChatAuth) to cache the permission result.
  2. Defense in Depth: Add permission checks inside handleEditPlatform and handlePlatformStatusChange functions, not just in the UI.
  3. Optional UX Enhancement: Add tooltips to disabled elements explaining why action is unavailable.

Expand Down
5 changes: 3 additions & 2 deletions ui/src/views/system/settings/authentication/components/SCANLogin.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import dingtalkLogo from '@/assets/logo/logo_dingtalk.svg'
import enterpriseWechatLogo from '@/assets/logo/logo_enterprise-wechat.svg'
import larkLogo from '@/assets/logo/logo_lark.svg'
import EditSCANDrawer from '../EditSCANDrawer.vue'
import perm from '@/permission'

interface QrLoginPlatformView extends QrLoginPlatformPayload {
isValid: boolean
Expand Down Expand Up @@ -88,12 +89,12 @@ onMounted(loadQrPlatforms)

<div class="flex-align-center gap-4">
<!-- 配置扫码登录平台 -->
<el-button plain @click.stop="handleEditPlatform(platform)">{{ platform.isValid ? '配置' : '未配置' }}</el-button>
<el-button :disabled="!perm.system.loginAuth.edit()" plain @click.stop="handleEditPlatform(platform)">{{ platform.isValid ? '配置' : '未配置' }}</el-button>
<template v-if="platform.isValid">
<el-divider direction="vertical" />
<!-- 启停扫码登录，阻止点击打开配置 -->
<span class="inline-flex" @click.stop>
<el-switch v-model="platform.isActive" @change="handlePlatformStatusChange(platform)" />
<el-switch :disabled="!perm.system.loginAuth.edit()" v-model="platform.isActive" @change="handlePlatformStatusChange(platform)" />
</span>
</template>
</div>

@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.

Here is a review of the provided code diff with potential issues and optimization suggestions:

1. Naming Convention & Readability

  • Issue: The imported variable perm is too generic. A single-word prefix like perm can conflict with other variables (e.g., permissions, permanent states) or lack clarity in larger scopes.
  • Suggestion: Rename it to something more descriptive, such as permissions, authGuard, accessControl, or loginPerm. 
    // Before
import perm from '@/permission'
// After
import permissionHelper from '@/permission' // Or const { system } = permissionHelper if destructuring is needed

2. Performance / Reactivity

  • Potential Issue: Calling perm.system.loginAuth.edit() directly in the template (:disabled="!perm.system.loginAuth.edit()") executes this function on every component re-render. If edit() involves synchronous logic checks, this is okay. However, if it involves any asynchronous logic or heavy computation, this will cause performance issues.
  • Suggestion:
    • If edit() is a simple boolean check function, consider using Vue's computed properties to cache the result.
    • Better yet, if @/permission exports reactive state or pre-computed access flags, bind to those directly instead of calling functions in the template.
    • If you must call a function, ensure it is lightweight. Example improvement using computed:
    <script setup lang="ts">
  // ... other imports
  import permissionApi from '@/permission'

  const canEditLoginAuth = computed(() => permissionApi.system.loginAuth.edit())
</script>

<template>
  <!-- Use the computed ref -->
  <el-button :disabled="!canEditLoginAuth" ... />
  <el-switch :disabled="!canEditLoginAuth" ... />
</template>

3. Code Duplication

  • Issue: The same permission check !perm.system.loginAuth.edit() is repeated for both the el-button and the el-switch.
  • Suggestion: As mentioned above, extract this into a single computed property (canEditLoginAuth) to avoid duplication and ensure consistent state across both UI elements.

4. Type Safety / API Contract

  • Check: Ensure that perm.system.loginAuth.edit() returns a boolean. If it returns undefined or null (e.g., due to missing permissions object), !perm... might behave unexpectedly. Using Vue's optional chaining or nullish coalescing might be safer depending on your permission logic implementation: 
    // Safer check if structure might be incomplete
!perm?.system?.loginAuth?.edit?.()
    (Note: Only apply this if your permission module doesn't guarantee full initialization).

5. Accessibility / UX

  • Observation: Disabling buttons/switches is good for preventing invalid actions. However, consider adding a tooltip when disabled to explain why the action is unavailable (e.g., "No permission to edit login authentication"). This improves UX significantly.

✅ Recommended Refactored Code Snippet

// Script setup section
<script setup lang="ts">
import { computed } from 'vue'
// Assuming '@/permission' defaults export an object or named exports. 
// Adjust import based on your actual module structure.
import permissionApi from '@/permission' 

// Extract permission state to a computed property for better readability and caching
const canEditLoginAuth = computed(() => {
  // Safely access nested properties if necessary
  return !!permissionApi?.system?.loginAuth?.edit?.() 
})
</script>

<template>
  <div class="flex-align-center gap-4">
    <!-- Configuration Button -->
    <el-button 
      :disabled="!canEditLoginAuth" 
      plain 
      @click.stop="handleEditPlatform(platform)"
    >
      {{ platform.isValid ? '配置' : '未配置' }}
    </el-button>

    <template v-if="platform.isValid">
      <el-divider direction="vertical" />
      
      <!-- Status Switch -->
      <span class="inline-flex" @click.stop>
        <el-switch 
          :disabled="!canEditLoginAuth" 
          v-model="platform.isActive" 
          @change="handlePlatformStatusChange(platform)" 
          <!-- Optional: Add tooltip for accessibility when disabled -->
          :title="!canEditLoginAuth ? '权限不足，无法启停平台' : ''"
        />
      </span>
    </template>
  </div>
</template>

This approach improves maintainability, reduces redundant logic, and provides clearer intent.

Expand Down
Loading