Skip to content

fix: [Portal] If you sign out of the portal while logged in and using an application that requires authentication, the system will trigger three "Access Denied" error messages. #7804

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

Closed
shaohuzhang1 wants to merge 1 commit into from
Closed

fix: [Portal] If you sign out of the portal while logged in and using an application that requires authentication, the system will trigger three "Access Denied" error messages. #7804

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
10 changes: 8 additions & 2 deletions ui/src/conversation-panel/left-sidebar/account-menu/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ const tokenPercentage = computed(() => {
const userPwdDialogRef = useTemplateRef<InstanceType<typeof UserPwdDialog>>('userPwdDialogRef')
const apiKeyDialogRef = useTemplateRef<InstanceType<typeof ApiKeyDialog>>('apiKeyDialogRef')

// 退出对话账号后重新导航，由守卫处理后续认证。
// 退出对话账号后回到当前场景首页，不停留在上一用户的对话；由守卫决定匿名认证或跳转登录页。
const handleLogout = () => {
return auth
.logout()
.then(() => router.replace({ path: router.currentRoute.value.fullPath, force: true }))
.then(() =>
router.replace(
route.meta.portal
? { name: 'portal-application-list', force: true }
: { name: 'chat-home', params: { accessToken: route.params.accessToken }, force: true },
),
)
.catch(() => {})
}

@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 the review of the code changes:

1. Potential Issue: Stale Closure of route

Problem: The handleLogout function uses router.replace. Inside the callback, it accesses route.meta and route.params.
If this is a Vue component where route comes from useRoute(), route is a reactive object. However, in a .then() promise callback, if the user navigates away before the logout API resolves, or if the component unmounts, relying on the current reactive route state might be unreliable depending on how Vue handles reactivity during async callbacks.
More importantly, using route inside the async callback risks capturing the wrong context if the logic isn't carefully bound. While Vue's ref/reactive objects generally persist their properties even after navigation events trigger updates, it is clearer and safer to capture the necessary parts (portal status, accessToken) before the promise chain or ensure they are accessed reliably.

However, the bigger issue is UX and Logic:

  • If the user is on a chat screen and logs out, route.params.accessToken might no longer be valid or relevant for the new anonymous session if chat-home expects an active access token. Passing a stale/invalid accessToken as a param could cause immediate errors upon navigation if the next page guards or composables try to use it immediately.

2. Irregularity: Silent Error Handling

The .catch(() => {}) silently swallows all errors, including network failures from auth.logout() or router errors. This makes debugging extremely difficult.
Suggestion: At minimum, log the error, or show a toast notification if logout failed (though usually, local cleanup should happen regardless).

3. Optimization/Best Practice: Explicit vs. Implicit Navigation

Using route.name === 'portal-application-list' checks route.meta.portal. This couples routing logic with view metadata. It is generally fine but ensure meta.portal is strictly typed and always present when expected.

4. Code Style & Readability

  • The ternary operator inside router.replace() is visually cluttered. Extracting the target route into a variable improves readability.
  • force: true is passed in both branches. Ensure this is actually needed. Forcing a replace might reset scroll positions unnecessarily if the content is similar, but usually good for login/logout.

✅ Recommended Improved Version

import { useRoute } from 'vue-router' // Ensure these imports exist elsewhere
import type { RouteLocationRaw } from 'vue-router'

// Assuming `route` is from useRoute() and `router` is from useRouter()

const handleLogout = () => {
  // Capture needed values BEFORE async operation starts to avoid subtle reactivity issues 
  // and clarify intent. Also handles cases where route might change during async wait.
  const isPortal = !!route.meta.portal
  const accessToken = route.params.accessToken

  return auth.logout().then(() => {
    let targetRoute: RouteLocationRaw;
    
    if (isPortal) {
      targetRoute = { name: 'portal-application-list', force: true }
    } else {
      // Note: Consider if `accessToken` is still needed/safe to pass here.
      // If navigating to 'chat-home' without login, usually you don't pass old tokens.
      // Verify business logic: Does 'chat-home' require an accessToken param?
      targetRoute = { 
        name: 'chat-home', 
        params: { accessToken }, 
        force: true 
      }
    }
    
    return router.replace(targetRoute)
  }).catch((error) => {
    // DO NOT swallow errors completely in production.
    console.error('Logout failed:', error)
    // Optional: Navigate anyway even if logout API failed? Depends on security requirements.
    // If strict, do nothing. If lenient, you might still want to clear local state and redirect.
    if (!router.currentRoute.value.meta.requiresAuth) {
       // Fallback redirect if appropriate
    }
  })
}

Key Recommendations:

  1. Remove Silenced Catch: Log errors or notify users.
  2. Verify Param Necessity: Check if chat-home really needs accessToken param after logout. Usually, post-logout routes are public or require new auth initiation, not old tokens.
  3. Readability: Split the ternary logic into if/else before calling router.replace for cleaner code.

Expand Down
Loading