Skip to content

fix(admin): always open impersonation in a new tab to prevent SSE connection failure#19056

Open
u2giants wants to merge 1 commit intotwentyhq:mainfrom
u2giants:feature/impersonation-always-new-tab
Open

fix(admin): always open impersonation in a new tab to prevent SSE connection failure#19056
u2giants wants to merge 1 commit intotwentyhq:mainfrom
u2giants:feature/impersonation-always-new-tab

Conversation

@u2giants
Copy link
Copy Markdown

Problem

When an admin impersonates a user in the same workspace, the current code calls executeImpersonationAuth, which does an in-session token swap:

if (isCurrentWorkspace) {
  await executeImpersonationAuth(loginToken.token);  // ← breaks SSE
  return;
}

executeImpersonationAuth calls clearSseClient() and then getAuthTokensFromLoginToken, but the SSE connection is not successfully re-established under the impersonated session. The result is that every view load immediately after impersonating shows "An error occurred", because the server rejects the impersonated session's attempt to subscribe to the event stream:

InternalServerError: You are not authorized to add a query to this event stream
  subCode: 'NOT_AUTHORIZED'

This makes same-workspace impersonation effectively broken for any data view.

Fix

Remove the isCurrentWorkspace special case and always use executeImpersonationRedirect with '_blank':

// Always open in a new tab. The in-session token swap breaks the
// SSE event stream connection, causing "An error occurred" on any
// view load immediately after impersonating.
return executeImpersonationRedirect(
  workspace.workspaceUrls,
  loginToken.token,
  '_blank',
);

The different-workspace path was already doing this — this change unifies both. The /verify route handles token exchange cleanly in a new tab with no SSE state to tear down in the current session.

What changes

  • SettingsAdminWorkspaceContent.tsx: remove isCurrentWorkspace branch, remove now-unused useImpersonationAuth hook and currentWorkspaceState atom
  • No behaviour change for different-workspace impersonation (was already _blank)
  • Same-workspace impersonation now opens a clean new tab instead of trying to hot-swap tokens in the current session

Testing

Tested on a self-hosted Twenty instance. Before this change: impersonating a same-workspace user resulted in "An error occurred" on every view. After: the new tab opens a fully functional session as the impersonated user with no errors.

Why not fix the SSE reconnection instead?

The SSE stream is tied to the original user's auth context. Reconnecting it under the impersonated token requires deeper changes to the SSE client lifecycle. Opening a new tab is the simpler, safer, and arguably better UX — it also matches what already happens for cross-workspace impersonation, and leaves the admin's own session undisturbed.

The in-session token swap path (executeImpersonationAuth) broke the SSE
event stream connection for same-workspace impersonation, causing "An
error occurred" on every view load immediately after impersonating.

Removing the isCurrentWorkspace branch and always using
executeImpersonationRedirect with '_blank' means the /verify route
handles token exchange in a clean new tab, with no SSE state to tear
down in the current session.

The different-workspace path was already '_blank' — this unifies both.
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@github-actions
Copy link
Copy Markdown
Contributor

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against dbf32f9

Comment on lines +87 to 90
// Always open in a new tab. The in-session token swap breaks the
// SSE event stream connection, causing "An error occurred" on any
// view load immediately after impersonating.
return executeImpersonationRedirect(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: User impersonation silently fails on self-hosted instances because executeImpersonationRedirect doesn't work when IS_MULTIWORKSPACE_ENABLED is false, which is the default.
Severity: CRITICAL

Suggested Fix

Restore the logic to handle same-workspace impersonation for instances where IS_MULTIWORKSPACE_ENABLED is false. This could involve reintroducing the call to executeImpersonationAuth as a fallback or modifying redirectToWorkspaceDomain to handle the single-workspace case correctly instead of silently returning.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-front/src/modules/settings/admin-panel/components/SettingsAdminWorkspaceContent.tsx#L87-L90

Potential issue: The user impersonation feature will silently fail on default
self-hosted instances where `IS_MULTIWORKSPACE_ENABLED` is `false`. The pull request
removes the `executeImpersonationAuth` function, which handled same-workspace
impersonation, and now exclusively uses `executeImpersonationRedirect`. This function
calls `redirectToWorkspaceDomain`, which contains a guard that causes it to return early
without performing a redirect if multi-workspace is disabled. As a result, when an admin
attempts to impersonate a user, the action appears to complete but no redirect or
impersonation occurs, and no error is shown.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant