Skip to content
Closed
Changes from all commits
Commits
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
51 changes: 29 additions & 22 deletions ui/src/views/application/application-card/ApplicationCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,44 @@
import type { ApplicationDetail } from '@/api/types'
import { isWorkFlow } from '@/utils/common'
import { dateFormat } from '@/utils/time'

import applicationAPI from "@/api/admin/workspace/application/application"
import { useRoute, useRouter } from 'vue-router';
import { computed } from 'vue';
defineOptions({ name: 'ApplicationCard' })

const router = useRouter()
const props = defineProps<{ application: ApplicationDetail; selectable?: boolean; selected?: boolean }>()

import { CHAT_API_BASE_PATH } from '@/api/constants'
const emit = defineEmits<{ click: []; selected: [selected: boolean] }>()

defineSlots<{ 'action-dropdown'?: () => unknown }>()
const apiQuery = computed(() => {
const query = props.application?.work_flow?.nodes?.find(n => n.id === 'base-node')?.properties?.api_input_field_list.map((f: any) => {
return `${f.variable}=${f.default_value}`
}).join('&')
if (query) {
return '?' + query
}
return ''

})

const chatBasePath = CHAT_API_BASE_PATH.replace(/\/api$/, '')

function handleOpen() {
if (!props.selectable) emit('click')
}
const toChat = () => {
applicationAPI.getAccessToken(props.application.id).then(ok => {
const accessUrl = ok.access_token ? `${window.location.origin}${chatBasePath}/${ok.access_token}/${apiQuery.value}` : ''
window.open(accessUrl, '_blank')
})
}
</script>

<template>
<MkResourceCard
:create_time="application.create_time"
:nick_name="application.nick_name || '-'"
:selectable="selectable"
:selected="selected"
:title="application.name"
:class="{ 'cursor-pointer': !selectable }"
@click="handleOpen"
@selected="emit('selected', $event)"
>
<MkResourceCard :create_time="application.create_time" :nick_name="application.nick_name || '-'"
:selectable="selectable" :selected="selected" :title="application.name" :class="{ 'cursor-pointer': !selectable }"
@click="handleOpen" @selected="emit('selected', $event)">
<template #icon>
<ApplicationIcon :icon="application.icon" />
</template>
Expand All @@ -42,14 +55,8 @@ function handleOpen() {

<template #footer="{ Action, ActionDropdown }">
<MkStatusLabel v-if="application.is_portal" type="custom" label="已上架" icon="icon_launch" class="text-N600!" />
<MkStatusLabel
v-else
:active="application.is_publish"
active-text="已发布"
inactive-text="未发布"
:icon="application.is_publish ? undefined : 'icon_time_filled'"
class="text-N600!"
/>
<MkStatusLabel v-else :active="application.is_publish" active-text="已发布" inactive-text="未发布"
:icon="application.is_publish ? undefined : 'icon_time_filled'" class="text-N600!" />
<template v-if="application.is_publish">
<el-divider direction="vertical" />
<span class="flex-align-center gap-2">
Expand All @@ -61,7 +68,7 @@ function handleOpen() {
<component :is="Action">
<!-- 去对话 -->
<MkTooltip content="去对话" placement="top">
<el-button text>
<el-button text @click="toChat">
<MkIcon name="icon_new-chat_outlined" />
</el-button>
</MkTooltip>

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, highlighting irregularities, potential issues, and optimization suggestions.

1. Irregularities & Best Practices

A. Import Placement

  • Issue: Imports are mixed throughout the script setup block. import applicationAPI, useRoute/useRouter, computed, and CHAT_API_BASE_PATH are inserted interspersed with variable definitions and defineOptions.
  • Recommendation: Move all import statements to the very top of the <script setup> block. This improves readability and follows standard ESLint/Vue formatting rules (e.g., import/first).

B. Unused useRoute

  • Issue: const router = useRouter() is initialized, but router is never used in the logic. toChat uses window.open. useRoute is imported but not utilized either.
  • Recommendation: Remove unused imports and variables. If navigation is intended via Vue Router, use router.push(). If opening in a new tab via URL construction is intended, remove the Vue Router imports.

C. Template Formatting

  • Issue: The template changes involve collapsing multi-line component attributes into single lines. While valid, this reduces readability for long attribute lists and makes git diffs harder to read when attributes change later.
  • Recommendation: Maintain consistent formatting. Ideally, one attribute per line when there are more than 3-4 attributes, or stick to a prettier/eslint-config-vue style guide.

2. Potential Issues

A. Security: Open Redirect / Command Injection

  • Issue: apiQuery is constructed using unvalidated data from props.application.work_flow.nodes...default_value. These values are joined directly into a query string (?key=value) and passed as a fragment/path part of access_token.
    • If variable or default_value contain characters like &, #, ?, or newlines, they can break the URL structure or inject unintended parameters.
    • The access_token is appended before the query string. Ensure that chatBasePath + access_token expects a query string immediately after, and that access_token itself is URL-encoded if it contains special chars.
  • Recommendation:
    1. Use URLSearchParams to safely encode query parameters.
    2. Validate/sanitize inputs from application.work_flow.
    const apiQuery = computed(() => {
      const nodes = props.application?.work_flow?.nodes;
      const baseNode = nodes?.find(n => n.id === 'base-node');
      const fieldList = baseNode?.properties?.api_input_field_list;
    
      if (!fieldList || !Array.isArray(fieldList)) return '';
    
      const params = new URLSearchParams();
      fieldList.forEach((f: any) => {
        // Optional: Validate that f.variable is safe
        if (f.variable && f.default_value !== undefined) {
          params.append(f.variable, String(f.default_value));
        }
      });
    
      return params.toString() ? '?' + params.toString() : '';
    });

B. Race Condition / Context Loss on Window.open

  • Issue: window.open might be blocked by popup blockers if not triggered directly by a user gesture. Currently, it's inside a .then(()) callback. While generally okay if the promise resolves immediately, if there's network latency, some browsers might flag this as non-user-triggered.
  • Recommendation: This is usually an acceptable pattern for "open in new tab" APIs, but ensure the click handler calls a function that might open synchronously, or accept that popup blockers might apply. Given it's a button click, it's likely fine, but be aware of edge cases where async operations delay window.open.

C. Fragile Data Selection

  • Issue: The code uses optional chaining heavily on deeply nested objects: props.application?.work_flow?.nodes?.find(...). If the structure changes slightly, this silently fails returning empty strings.
  • Recommendation: This is defensive coding and good, but ensure apiQuery.value is always safe to append. The current implementation handles missing data correctly by returning ''.

3. Optimization Suggestions

A. Computed Property Dependency

  • The apiQuery computed property depends on props.application. This is correct and efficient as Vue will recompute only when props.application changes.

B. String Construction for URL

  • Instead of manual string concatenation:
    `${window.location.origin}${chatBasePath}/${ok.access_token}/${apiQuery.value}`
    Consider creating a clean builder function or ensuring access_token is URL-safe. If access_token contains slashes or spaces, this URL breaks.

C. Template Performance

  • The MkStatusLabel v-else block has inline logic :icon="application.is_publish ? undefined : 'icon_time_filled'". This is minor but fine. Ensure MkStatusLabel is optimized for frequent re-renders if this list is large.

Refactored Script Segment Example

<script setup lang="ts">
import type { ApplicationDetail } from '@/api/types'
import { isWorkFlow } from '@/utils/common'
import { dateFormat } from '@/utils/time'
import applicationAPI from "@/api/admin/workspace/application/application"
import { CHAT_API_BASE_PATH } from '@/api/constants'
// Only import useRouter if you plan to use router.push later. 
// Since we are using window.open, remove useRouter/useRoute unless needed elsewhere.
// import { useRoute, useRouter } from 'vue-router'; 
import { computed } from 'vue';

defineOptions({ name: 'ApplicationCard' })

const props = defineProps<{
  application: ApplicationDetail;
  selectable?: boolean;
  selected?: boolean;
}>()

const emit = defineEmits<{
  click: [];
  selected: [selected: boolean];
}>()

defineSlots<{ 'action-dropdown'?: () => unknown }>()

const chatBasePath = CHAT_API_BASE_PATH.replace(/\/api$/, '')

// Use URLSearchParams for safer query string generation
const apiQuery = computed(() => {
  try {
    const baseNode = props.application.work_flow?.nodes?.find(
      (n: any) => n.id === 'base-node'
    );
    const fieldList = baseNode?.properties?.api_input_field_list;

    if (!fieldList || !Array.isArray(fieldList)) {
      return '';
    }

    const params = new URLSearchParams();
    fieldList.forEach((f: any) => {
      if (f.variable !== undefined && f.default_value !== undefined) {
        params.append(String(f.variable), String(f.default_value));
      }
    });

    const queryString = params.toString();
    return queryString ? `?${queryString}` : '';
  } catch (e) {
    console.error("Error generating API query", e);
    return '';
  }
});

function handleOpen() {
  if (!props.selectable) emit('click');
}

const toChat = async () => {
  try {
    // Wait for token
    const response = await applicationAPI.getAccessToken(props.application.id);
    
    if (!response.access_token) {
      console.warn('No access token received');
      return;
    }

    // Construct URL safely
    // Note: Ensure access_token is URL-safe or encode it if necessary
    const tokenPath = `/token-${encodeURIComponent(response.access_token)}`; // Example if path-based security requires it
    
    const fullUrl = `${window.location.origin}${chatBasePath}/${response.access_token}${apiQuery.value}`;
    
    window.open(fullUrl, '_blank', 'noopener,noreferrer');
  } catch (error) {
    console.error('Failed to get access token for chat', error);
  }
};
</script>

Summary of Actions

  1. Move imports to the top.
  2. Remove unused useRouter/useRoute imports/variables.
  3. Use URLSearchParams for building the query string to prevent malformed URLs.
  4. Add error handling (try/catch or .catch) around the async getAccessToken call.
  5. Review template formatting to match project standards (likely multi-line for complex components).

Expand Down
Loading