feat: add API Token #1119
feat: add API Token #1119
Conversation
github-actions
bot
added
enhancement
WalkthroughThis pull request introduces significant changes to the AI chat interface by replacing the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
xuanlid
force-pushed
the
feat/apiToken
branch
from
ef9715e to
750d6ac
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/plugins/robot/src/Main.vue (2)
165-166: Potential security concern: token in local storage.
Storing any sensitive token in local storage can increase XSS attack risk. If feasible, consider more secure alternatives (e.g., short-lived tokens in memory or secure cookies).
371-374: Closing the popover is straightforward.
Hiding the popover viashowPopover.value = falseis valid. If a reset of form data is required whenever the popover closes, consider adding it here.packages/plugins/robot/src/robotSettingPopover.vue (1)
27-50: Prop definitions are clear, but consider watch for later updates.
WhiletypeValueandtokenValueare accurately exposed, any future change in parent props won't automatically updateformData. If you need that reactivity, consider a watcher or using computed props.+ // Example approach: + watch( + () => props.typeValue.value, + (newVal) => { + formData.type = newVal + } + ) + watch( + () => props.tokenValue, + (newVal) => { + formData.tokenVal = newVal + } + )
🛑 Comments failed to post (2)
packages/plugins/robot/src/Main.vue (2)
317-317:
⚠️ Potential issueRef usage bug: reassigning the ref variable.
Currently,tokenValue = JSON.parse(aiSession)?.foundationModel?.tokenreassigns the entire ref variable, breaking reactivity. Replace withtokenValue.value = ...to preserve correct reactive behavior.- tokenValue = JSON.parse(aiSession)?.foundationModel?.token + tokenValue.value = JSON.parse(aiSession)?.foundationModel?.token📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.tokenValue.value = JSON.parse(aiSession)?.foundationModel?.token
353-364:
⚠️ Potential issueLogical errors in model switching.
- Line 359:
item.value = model.typeis an assignment, not a comparison. Replace=with===to locate the correct model.- Both the confirm callback (line 360) and line 364 call
endContent(), causing the conversation to be ended unconditionally. Consider moving the second call inside the confirm callback'sexecor removing it if redundant.- selectedModel.value = AIModelOptions.find((item) => (item.value = model.type)) + selectedModel.value = AIModelOptions.find((item) => item.value === model.type) - // Remove or relocate the unconditional endContent() call at line 364Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/plugins/robot/src/Main.vue (1)
158-172: 💡 Verification agent🧩 Analysis chain
Verify token security in local storage.
Storing API tokens in localStorage poses security risks as it's vulnerable to XSS attacks.
Consider these security implications:
- The token is stored in plain text
- localStorage is accessible via JavaScript
- No encryption is implemented
🌐 Web query:
What are the best practices for storing API tokens in frontend applications?💡 Result:
Securing API tokens in frontend applications requires balancing accessibility and protection against common attack vectors like XSS and CSRF. Based on industry best practices from leading security authorities, here are the recommended approaches:
Core Best Practices
1. Prefer Browser Memory Storage
- Store access tokens in-memory using JavaScript closures or Web Workers (via libraries like Auth0 SPA SDK)[4][7].
- Tokens persist only during the active session and are cleared on tab/window closure[4].
- Example implementation:
let token = null; function setToken(newToken) { token = newToken; } function getToken() { return token; }2. Use Backend-for-Frontend (BFF) Architecture
- Deploy a lightweight backend component (e.g., OAuth Agent/Proxy) to handle token acquisition/storage[2][5][8].
- The BFF:
- Issues secure
HttpOnlycookies to the frontend[2][3].- Proxies API requests to avoid exposing tokens to browser JavaScript[5].
- Rotates refresh tokens automatically[2][8].
3. Secure Cookies When Required
If using cookies:
- Enable
Secure,HttpOnly, andSameSite=Strictflags[1][3][7].- Pair with CSRF tokens via headers like
X-CSRF-Token[3][5].
Storage Options Comparison
Method Security Against XSS Security Against CSRF Persistence Implementation Complexity Browser Memory High High None Low BFF + Cookies High Medium (with CSRF) Session High Encrypted LocalStorage Medium (if encrypted)[1] High Persistent Medium
Additional Safeguards
- Token Encryption: Encrypt JWT payloads if stored client-side (e.g., AES-256)[1][5].
- Short Lifespans: Set access token expiration ≤1 hour and refresh token ≤24 hours[1][4][7].
- Refresh Token Rotation: Invalidate previous refresh tokens after each use[3][8].
- HTTPS Enforcement: Transmit tokens only over encrypted channels[7][9].
Architecture Recommendations
For Single-Page Apps (SPAs):
This pattern eliminates client-side token exposure while maintaining SPA usability[2][5][8].
For Traditional Web Apps:
- Store tokens server-side using frameworks like Express/ASP.NET sessions[3][7].
By prioritizing memory/BFF storage, enforcing HTTPS, and adopting defense-in-depth measures like encryption and token rotation, developers can mitigate most frontend token security risks[1][2][4][8].
Citations:
- 1: http://www.cyberchief.ai/2023/05/secure-jwt-token-storage.html
- 2: https://curity.io/resources/learn/the-token-handler-pattern/
- 3: https://docs.cyberark.com/identity/latest/en/content/developer/oidc/tokens/token-storage.htm?TocPath=Add+and+manage+users%7CUse+access+tokens%7C_____3
- 4: https://auth0.com/docs/secure/security-guidance/data-security/token-storage
- 5: https://learn.microsoft.com/en-us/azure/architecture/web-apps/guides/security/secure-single-page-application-authorization
- 6: https://42crunch.com/token-management-best-practices/
- 7: https://auth0.com/docs/secure/tokens/token-best-practices
- 8: https://curity.io/resources/learn/api-security-best-practices/
- 9: https://jsramblings.com/best-practices-for-working-with-api-keys-in-the-frontend/
- 10: https://docs.cyberark.com/wpm/latest/en/content/developer/oidc/tokens/token-storage.htm
Security Alert: Insecure API Token Storage in Local Storage
The file
packages/plugins/robot/src/Main.vue(lines 158–172) currently stores API tokens in plain text within localStorage. As highlighted by industry best practices, saving tokens this way exposes them to XSS risks because localStorage is accessible via JavaScript and lacks inherent encryption.Recommendations:
- In-Memory Storage: Consider holding tokens in browser memory during the session. This limits persistence to the active session and reduces exposure.
- BFF Approach: Evaluate using a Backend-for-Frontend architecture that leverages secure, HttpOnly cookies. This method minimizes token exposure to client-side scripts.
- Encryption Safeguards: If localStorage must be used, implement robust encryption and ensure tokens are accompanied by additional safeguards (e.g., HTTPS, short lifespans, CSRF protection).
Addressing these concerns will help align the implementation with current security best practices.
🧹 Nitpick comments (2)
packages/plugins/robot/src/Main.vue (2)
20-41: Consider adding accessibility attributes to the popover.The popover implementation could benefit from improved accessibility.
Add ARIA attributes to make the popover more accessible:
<tiny-popover ref="popoverRef" width="270" trigger="manual" v-model="showPopover" :visible-arrow="false" popper-class="chat-popover" + role="dialog" + aria-label="AI Model Settings" >
371-373: Add error handling to activeSetting method.The
activeSettingmethod lacks error handling for edge cases.Consider adding error handling:
const activeSetting = () => { + try { showPopover.value = false + } catch (error) { + console.error('Failed to close settings popover:', error) + // Optionally show user feedback + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/robot/src/Main.vue(9 hunks)packages/plugins/robot/src/robotSettingPopover.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/robot/src/robotSettingPopover.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/plugins/robot/src/Main.vue (1)
157-167: 🛠️ Refactor suggestionAdd token validation before storing in session.
The token is being stored in the session without any validation.
Apply this diff to add basic validation:
setContextSession = () => { + const sanitizedToken = tokenValue.value?.trim() || '' + if (sanitizedToken && !/^[A-Za-z0-9-._~+/]+=*$/.test(sanitizedToken)) { + console.error('Invalid token format') + return + } localStorage.setItem( 'aiChat', sessionProcess ? JSON.stringify(sessionProcess) : JSON.stringify({ foundationModel: { manufacturer: selectedModel.value.manufacturer, model: selectedModel.value.value, - token: tokenValue.value + token: sanitizedToken }, messages: [], displayMessages: [] }) ) }
🧹 Nitpick comments (1)
packages/plugins/robot/src/Main.vue (1)
371-373: Consider adding debounce to activeSetting.The
activeSettingfunction directly modifies the popover visibility. Consider adding debounce to prevent rapid toggling.Apply this diff to add debounce:
+import { debounce } from '@opentiny/vue-renderless/common/deps/lodash' + const activeSetting = () => { showPopover.value = false } + +const debouncedActiveSetting = debounce(activeSetting, 300)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/robot/src/Main.vue(9 hunks)
🔇 Additional comments (4)
packages/plugins/robot/src/Main.vue (4)
20-41: LGTM! The popover implementation looks good.The replacement of dropdown with popover improves the UI/UX by providing a dedicated space for model settings.
147-148: Useref()consistently for reactive variables.The variables are declared with
refbut usinglet. This mixing of styles could lead to reactivity issues.
314-318: Add null check in initCurrentModel.The function assumes the localStorage data structure is valid without proper validation.
352-365: Fix potential bug in model change handler.The model change handler has several issues:
- Assignment operator used instead of comparison
- Redundant
endContent()call
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/plugins/robot/src/Main.vue (3)
147-148:⚠️ Potential issueUse
constfor reactive variables.The variables are declared with
letbut initialized withref(). This mixing of styles could lead to reactivity issues.Apply this fix:
- const tokenValue = ref('') - const showPopover = ref(false) + const tokenValue = ref('') + const showPopover = ref(false)
319-323:⚠️ Potential issueAdd null checks in initCurrentModel.
The function assumes the localStorage data structure is valid without proper validation.
Apply this fix:
const initCurrentModel = (aiSession) => { - const currentModelValue = JSON.parse(aiSession)?.foundationModel?.model - selectedModel.value = AIModelOptions.find((item) => item.value === currentModelValue) - tokenValue.value = JSON.parse(aiSession)?.foundationModel?.token + try { + const parsedSession = JSON.parse(aiSession) + if (parsedSession?.foundationModel) { + const currentModelValue = parsedSession.foundationModel.model + selectedModel.value = AIModelOptions.find((item) => item.value === currentModelValue) || AIModelOptions[0] + tokenValue.value = parsedSession.foundationModel.token || '' + } + } catch (error) { + console.error('Failed to initialize model:', error) + selectedModel.value = AIModelOptions[0] + tokenValue.value = '' + } }
364-379:⚠️ Potential issueFix potential bug in model change handler.
The function has several issues:
- Redundant token check could be simplified
- Inconsistent token handling
Apply these fixes:
const changeModel = (model) => { if (selectedModel.value.value !== model.type) { confirm({ title: '切换AI大模型', message: '切换AI大模型将导致当前会话被清空，重新开启新会话，是否继续？', exec() { selectedModel.value = AIModelOptions.find((item) => item.value === model.type) tokenValue.value = model.tokenVal endContent() } }) - } else if (tokenValue.value !== model.tokenVal && selectedModel.value.value === model.type) { + } else if (tokenValue.value !== model.tokenVal) { tokenValue.value = model.tokenVal changeTokenValue() } }
🧹 Nitpick comments (3)
packages/plugins/robot/src/Main.vue (3)
307-311: Add error handling for empty token.The token validation is good, but the error message could be more actionable.
Apply this diff to improve the error message:
if (!tokenValue.value) { - messages.value.push({ role: 'assistant', content: '当前会话未设置API Token，请设置后再试！', name: 'AI' }) + messages.value.push({ + role: 'assistant', + content: '当前会话未设置API Token。请点击右上角的设置图标来配置API Token，然后重试。', + name: 'AI' + }) inProcesing.value = false return }
225-227: Add timeout handling for long-running requests.While a 10-minute timeout is set, there's no user feedback during this period.
Consider adding a loading indicator or progress updates for long-running requests:
+ const loadingInstance = Loading.service({ + text: '正在处理请求，请稍等...', + customClass: 'chat-loading', + background: 'rgba(0, 0, 0, 0.15)', + target: '#bind-chatgpt' + }) getMetaApi(META_SERVICE.Http) .post('/app-center/api/ai/chat', getSendSeesionProcess(), { timeout: 600000 }) .then((res) => { + loadingInstance.close() const { originalResponse, schema, replyWithoutCode } = res
241-246: Improve error handling in catch block.The error handling could be more informative for users.
Apply this diff to improve error handling:
- .catch(() => { - messages.value[messages.value.length - 1].content = '连接失败' + .catch((error) => { + const errorMessage = error.response?.data?.message || '连接失败，请稍后重试' + messages.value[messages.value.length - 1].content = errorMessage localStorage.removeItem('aiChat') inProcesing.value = false connectedFailed.value = false + console.error('AI chat request failed:', error) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/robot/src/Main.vue(10 hunks)packages/plugins/robot/src/robotSettingPopover.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/robot/src/robotSettingPopover.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/plugins/robot/src/Main.vue (1)
20-41: LGTM! The popover implementation enhances user experience.The replacement of dropdown with popover provides a better interface for managing both model selection and API token configuration.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/plugins/robot/src/Main.vue (3)
307-311: Add error handling for missing API token.The token validation is implemented correctly, but the error message could be more informative.
- messages.value.push({ role: 'assistant', content: '当前会话未设置API Token，请设置后再试！', name: 'AI' }) + messages.value.push({ + role: 'assistant', + content: '请在设置中配置API Token。点击右上角的设置图标，在弹出的窗口中输入有效的API Token。', + name: 'AI' + })
364-379: Improve model change handler logic.The function has been improved but could be more robust with additional error handling.
const changeModel = (model) => { + if (!model?.type) { + console.error('Invalid model configuration') + return + } if (selectedModel.value.value !== model.type) { confirm({ title: '切换AI大模型', message: '切换AI大模型将导致当前会话被清空，重新开启新会话，是否继续？', exec() { selectedModel.value = AIModelOptions.find((item) => item.value === model.type) tokenValue.value = model.tokenVal endContent() + }, + cancel() { + showPopover.value = false } }) } else if (tokenValue.value !== model.tokenVal && selectedModel.value.value === model.type) { tokenValue.value = model.tokenVal changeTokenValue() } }
385-387: Add documentation for activeSetting function.The function's purpose and usage context should be documented.
+// Closes the settings popover. Called after model settings are updated or when clicking outside the popover const activeSetting = () => { showPopover.value = false }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/robot/src/Main.vue(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/plugins/robot/src/Main.vue (3)
20-41: LGTM! Improved UI interaction with popover.The replacement of dropdown with popover provides better user experience for model settings.
147-148: Useconstfor reactive variables.The variables are declared with
letbut initialized withref(). This mixing of styles could lead to reactivity issues.
319-323: Add null check in initCurrentModel.The function assumes the localStorage data structure is valid without proper validation.
| const changeTokenValue = () => { | ||
| localStorage.removeItem('aiChat') | ||
| sessionProcess = null | ||
| setContextSession() | ||
| sessionProcess = JSON.parse(localStorage.getItem('aiChat')) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling in changeTokenValue.
The function performs critical operations without error handling.
const changeTokenValue = () => {
+ try {
localStorage.removeItem('aiChat')
sessionProcess = null
setContextSession()
sessionProcess = JSON.parse(localStorage.getItem('aiChat'))
+ } catch (error) {
+ console.error('Failed to update token:', error)
+ Notify({
+ type: 'error',
+ message: '更新Token失败，请重试',
+ position: 'top-right',
+ duration: 5000
+ })
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const changeTokenValue = () => { | |
| localStorage.removeItem('aiChat') | |
| sessionProcess = null | |
| setContextSession() | |
| sessionProcess = JSON.parse(localStorage.getItem('aiChat')) | |
| } | |
| const changeTokenValue = () => { | |
| try { | |
| localStorage.removeItem('aiChat') | |
| sessionProcess = null | |
| setContextSession() | |
| sessionProcess = JSON.parse(localStorage.getItem('aiChat')) | |
| } catch (error) { | |
| console.error('Failed to update token:', error) | |
| Notify({ | |
| type: 'error', | |
| message: '更新Token失败，请重试', | |
| position: 'top-right', | |
| duration: 5000 | |
| }) | |
| } | |
| } |
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
ai对话框增加API Token
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor