Skip to content

Conversation

etiennejouan
Copy link
Contributor

Related to #14360

Screenshot 2025-09-26 at 12 16 46

@Bonapara, let you tell me if wording and banner color are ok. This banner is displayed when a user is impersonating an other user

Copy link
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.

Greptile Overview

Summary

This PR adds an impersonation banner feature to provide visual feedback when administrators are viewing the application as another user. The implementation introduces a new Recoil selector that detects impersonation status from JWT tokens and displays a banner with user context and an option to stop impersonating.

Key Changes:

  • New State Management: Created isImpersonatingState selector that decodes JWT tokens to detect impersonation status
  • Visual Banner: Added InformationBannerIsImpersonating component that shows current impersonated user details
  • UX Improvements: Modified dropdown menu to prevent nested impersonation by disabling the option when already impersonating
  • Integration: Properly integrated banner into existing information banner system

Technical Implementation:

  • Uses Recoil for reactive state management tied to JWT token changes
  • Follows existing codebase patterns for information banners
  • Includes proper internationalization with Lingui
  • Implements null safety checks and error handling

The code is well-structured and follows the project's architectural patterns. One minor style improvement was suggested regarding string interpolation within translation templates.

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - implements a defensive security feature with proper state management
  • Score reflects well-structured code following project patterns with one minor style suggestion. The feature enhances security by providing clear visual feedback during impersonation sessions.
  • No files require special attention - all changes follow established patterns and include proper error handling

Important Files Changed

File Analysis

Filename        Score        Overview
packages/twenty-front/src/modules/auth/states/isImpersonatingState.ts 5/5 New Recoil selector to detect impersonation status from JWT token - clean implementation with proper error handling
packages/twenty-front/src/modules/information-banner/components/impersonate/InformationBannerIsImpersonating.tsx 4/5 New impersonation banner component with proper null checking and i18n support - potential string concatenation issue
packages/twenty-front/src/modules/information-banner/components/InformationBannerWrapper.tsx 5/5 Integrates impersonation banner into wrapper component - clean addition following existing patterns
packages/twenty-front/src/modules/settings/members/ManageMembersDropdownMenu.tsx 5/5 Prevents impersonation when already impersonating by updating permission logic - good UX improvement

Sequence Diagram

sequenceDiagram
    participant User as Admin User
    participant Dropdown as ManageMembersDropdownMenu
    participant Auth as Auth System
    participant JWT as JWT Token
    participant State as isImpersonatingState
    participant Banner as InformationBannerWrapper
    participant ImpBanner as InformationBannerIsImpersonating

    User->>Dropdown: Click impersonate option
    Dropdown->>State: Check isImpersonating
    State->>JWT: Decode JWT token
    JWT-->>State: Return isImpersonating flag
    State-->>Dropdown: Return current status
    alt If not already impersonating
        Dropdown->>Auth: Start impersonation
        Auth->>JWT: Update token with isImpersonating: true
        JWT-->>State: Token change triggers recomputation
        State-->>Banner: New impersonation status
        Banner->>ImpBanner: Render impersonation banner
        ImpBanner-->>User: Show "View as [User]" banner
    else If already impersonating
        Note over Dropdown: Impersonate option disabled
    end

    User->>ImpBanner: Click "Stop impersonating"
    ImpBanner->>Auth: Call signOut()
    Auth->>JWT: Clear impersonation token
    JWT-->>State: Token change triggers recomputation
    State-->>Banner: Impersonation ended
    Banner-->>User: Hide impersonation banner
Loading

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

github-actions bot commented Sep 26, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:51493

This environment will automatically shut down when the PR is closed or after 5 hours.

get: ({ get }) => {
const tokenPair = get(tokenPairState);

if (!isDefined(tokenPair?.accessOrWorkspaceAgnosticToken?.token))
Copy link
Member

Choose a reason for hiding this comment

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

nit: prefer block {}

const { closeDropdown } = useCloseDropdown();
const canImpersonate = useHasPermissionFlag(PermissionFlagType.IMPERSONATE);
const isImpersonating = useRecoilValue(isImpersonatingState);
const canImpersonate =
Copy link
Member

Choose a reason for hiding this comment

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

Are we not looking at canImpersonate from user state as well?

Copy link
Contributor Author

@etiennejouan etiennejouan Sep 29, 2025

Choose a reason for hiding this comment

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

It has been approved in previous PR like that. And I think I'm ok with it because impersonation can be done at two levels, workspace and server.
Workspace impersonation is enabled via permission and can be started from settings/member page (here).
Server impersonation is enabled via 'canImpersonate from user' and can be started from admin panel.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, makes sense, ok SGDM!

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

@etiennejouan etiennejouan merged commit b74ed36 into main Sep 29, 2025
54 checks passed
@etiennejouan etiennejouan deleted the ej/impersonate-banner branch September 29, 2025 15:10
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.

2 participants