fix: block self-impersonation in admin panel (#21130)
## Issue - From Settings -> Admin Panel -> Workspace -> Members, impersonating the currently logged-in user still issued an impersonation login token. Token exchange produced invalid impersonation JWTs (`impersonatorUserWorkspaceId === impersonatedUserWorkspaceId`). JWT validation then failed with `User cannot impersonate themselves`, leaving the app in an endless loading state until cookies were cleared. - Closes #21086 ## Approach I was first thinking of to only hide the impersonate button for the logged-in user in the admin, since they can not click what isn’t shown (as I thought it was just a frontend issue). But that was not enough: - The `impersonate` mutation can still be called directly (GraphQL client, scripts, devtools). - Before this fix, the mutation could succeed and only fail later at JWT validation, which led to invalid tokens and a broken session. So the PR does both: - Frontend: hide/disable self-impersonation in the UI and avoid reloading on failed token exchange (UX). - Backend: reject self-impersonation in `ImpersonationService` and at token exchange (enforcement, fail fast before bad tokens). Hiding the button is the right product behavior; the backend change is what makes the rule real and safe. ## How to test Manual: - Log in as a user with admin impersonation. - Go to Settings -> Admin panel -> Workspace -> open your workspace -> members. - Confirm your row has no Impersonate button; other members still do. - Open Admin Panel -> User for yourself -> confirm no impersonate button. - Open Settings -> Members -> your own member profile -> confirm no Impersonate action. - Impersonate another member -> should work as before Automated: `npx jest impersonation.service.spec` ### Before: <img width="830" height="413" alt="Screenshot 2026-06-02 122224" src="https://github.com/user-attachments/assets/46f38a74-8bd6-4ffa-b749-500ce18314f1" /> ### After: <img width="795" height="369" alt="Screenshot 2026-06-02 122333" src="https://github.com/user-attachments/assets/62ece4a8-d38b-4f91-817f-792ff49b146b" /> --------- Signed-off-by: Parship Chowdhury <parshipchowdhury@gmail.com> Co-authored-by: Etienne <45695613+etiennejouan@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
4d520a312f
commit
120793f69f
@@ -272,6 +272,7 @@ export const useAuth = () => {
|
||||
handleSetLoginToken(loginToken);
|
||||
navigate(AppPath.SignInUp);
|
||||
setSignInUpStep(SignInUpStep.TwoFactorAuthenticationProvision);
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
@@ -283,7 +284,9 @@ export const useAuth = () => {
|
||||
handleSetLoginToken(loginToken);
|
||||
navigate(AppPath.SignInUp);
|
||||
setSignInUpStep(SignInUpStep.TwoFactorAuthenticationVerification);
|
||||
return;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
[
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback } from 'react';
|
||||
import { useStore } from 'jotai';
|
||||
import { useCallback } from 'react';
|
||||
|
||||
import { useAuth } from '@/auth/hooks/useAuth';
|
||||
import { tokenPairState } from '@/auth/states/tokenPairState';
|
||||
@@ -40,7 +40,13 @@ export const useImpersonationSession = () => {
|
||||
);
|
||||
}
|
||||
|
||||
await getAuthTokensFromLoginToken(loginToken);
|
||||
try {
|
||||
await getAuthTokensFromLoginToken(loginToken);
|
||||
} catch (error) {
|
||||
sessionStorage.removeItem(IMPERSONATION_SESSION_KEY);
|
||||
throw error;
|
||||
}
|
||||
|
||||
reloadWithSession(targetPath);
|
||||
},
|
||||
[store, getAuthTokensFromLoginToken],
|
||||
|
||||
+13
-1
@@ -1,10 +1,13 @@
|
||||
import { useState } from 'react';
|
||||
|
||||
import { useMutation } from '@apollo/client/react';
|
||||
import { t } from '@lingui/core/macro';
|
||||
import { AppPath } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState';
|
||||
import { useImpersonationSession } from '@/auth/hooks/useImpersonationSession';
|
||||
import { currentUserState } from '@/auth/states/currentUserState';
|
||||
import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState';
|
||||
import { useRedirectToWorkspaceDomain } from '@/domain-manager/hooks/useRedirectToWorkspaceDomain';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
@@ -12,6 +15,7 @@ import { ImpersonateDocument } from '~/generated-metadata/graphql';
|
||||
import { getWorkspaceUrl } from '~/utils/getWorkspaceUrl';
|
||||
|
||||
export const useHandleImpersonate = () => {
|
||||
const currentUser = useAtomStateValue(currentUserState);
|
||||
const currentWorkspace = useAtomStateValue(currentWorkspaceState);
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
const { startImpersonating } = useImpersonationSession();
|
||||
@@ -22,6 +26,14 @@ export const useHandleImpersonate = () => {
|
||||
);
|
||||
|
||||
const handleImpersonate = async (userId: string, workspaceId: string) => {
|
||||
if (!isDefined(currentUser?.id) || userId === currentUser.id) {
|
||||
enqueueErrorSnackBar({
|
||||
message: t`You cannot impersonate your own account`,
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
setImpersonatingUserId(userId);
|
||||
|
||||
await impersonate({
|
||||
|
||||
@@ -166,7 +166,8 @@ export const SettingsAdminUserDetail = () => {
|
||||
/>
|
||||
{currentUser?.canImpersonate &&
|
||||
activeWorkspace &&
|
||||
isDefined(user) && (
|
||||
isDefined(user) &&
|
||||
user.id !== currentUser.id && (
|
||||
<StyledButtonContainer>
|
||||
<Button
|
||||
Icon={IconEyeShare}
|
||||
|
||||
+16
-14
@@ -285,20 +285,22 @@ export const SettingsAdminWorkspaceDetail = () => {
|
||||
</TableCell>
|
||||
<TableCell>{user.email}</TableCell>
|
||||
<TableCell align="right">
|
||||
{workspace.allowImpersonation && (
|
||||
<Button
|
||||
Icon={IconEyeShare}
|
||||
variant="secondary"
|
||||
size="small"
|
||||
title={t`Impersonate`}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
handleImpersonate(userId, workspaceId!);
|
||||
}}
|
||||
disabled={impersonatingUserId === userId}
|
||||
/>
|
||||
)}
|
||||
{workspace.allowImpersonation &&
|
||||
isDefined(currentUser?.id) &&
|
||||
userId !== currentUser.id && (
|
||||
<Button
|
||||
Icon={IconEyeShare}
|
||||
variant="secondary"
|
||||
size="small"
|
||||
title={t`Impersonate`}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
handleImpersonate(userId, workspaceId!);
|
||||
}}
|
||||
disabled={impersonatingUserId === userId}
|
||||
/>
|
||||
)}
|
||||
</TableCell>
|
||||
</TableRow>
|
||||
);
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
import { useParams } from 'react-router-dom';
|
||||
import { useDebouncedCallback } from 'use-debounce';
|
||||
|
||||
import { CoreObjectNameSingular, SettingsPath } from 'twenty-shared/types';
|
||||
import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord';
|
||||
import { useImpersonationSession } from '@/auth/hooks/useImpersonationSession';
|
||||
import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord';
|
||||
import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer';
|
||||
import { SettingsRolesQueryEffect } from '@/settings/roles/components/SettingsRolesQueryEffect';
|
||||
import { useHasPermissionFlag } from '@/settings/roles/hooks/useHasPermissionFlag';
|
||||
@@ -14,12 +13,14 @@ import { SettingsPageLayout } from '@/settings/components/layout/SettingsPageLay
|
||||
import { TabList } from '@/ui/layout/tab-list/components/TabList';
|
||||
import { activeTabIdComponentState } from '@/ui/layout/tab-list/states/activeTabIdComponentState';
|
||||
import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue';
|
||||
import { t } from '@lingui/core/macro';
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { getSettingsPath } from 'twenty-shared/utils';
|
||||
import { t } from '@lingui/core/macro';
|
||||
import { CoreObjectNameSingular, SettingsPath } from 'twenty-shared/types';
|
||||
import { getSettingsPath, isDefined } from 'twenty-shared/utils';
|
||||
import { IconInfoCircle, IconLockOpen } from 'twenty-ui/display';
|
||||
import { useNavigateSettings } from '~/hooks/useNavigateSettings';
|
||||
|
||||
import { currentUserState } from '@/auth/states/currentUserState';
|
||||
import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState';
|
||||
import { isImpersonatingState } from '@/auth/states/isImpersonatingState';
|
||||
import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord';
|
||||
@@ -29,9 +30,9 @@ import { useWorkspaceMemberRoles } from '@/settings/members/hooks/useWorkspaceMe
|
||||
import { type WorkspaceMember } from '@/workspace-member/types/WorkspaceMember';
|
||||
import { useMutation } from '@apollo/client/react';
|
||||
import {
|
||||
PermissionFlagType,
|
||||
DeleteUserWorkspaceDocument,
|
||||
ImpersonateDocument,
|
||||
PermissionFlagType,
|
||||
} from '~/generated-metadata/graphql';
|
||||
|
||||
const SETTINGS_WORKSPACE_MEMBER_TABS = {
|
||||
@@ -49,6 +50,7 @@ export const SettingsWorkspaceMember = () => {
|
||||
const navigateSettings = useNavigateSettings();
|
||||
const { enqueueErrorSnackBar, enqueueSuccessSnackBar } = useSnackBar();
|
||||
const { openModal, closeModal } = useModal();
|
||||
const currentUser = useAtomStateValue(currentUserState);
|
||||
const currentWorkspace = useAtomStateValue(currentWorkspaceState);
|
||||
const { startImpersonating } = useImpersonationSession();
|
||||
const [impersonate] = useMutation(ImpersonateDocument);
|
||||
@@ -143,6 +145,15 @@ export const SettingsWorkspaceMember = () => {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!isDefined(currentUser?.id) || member.userId === currentUser.id) {
|
||||
enqueueErrorSnackBar({
|
||||
message: t`You cannot impersonate your own account`,
|
||||
options: { duration: 2000 },
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
await impersonate({
|
||||
variables: {
|
||||
userId: member.userId,
|
||||
@@ -150,6 +161,7 @@ export const SettingsWorkspaceMember = () => {
|
||||
},
|
||||
onCompleted: async (data) => {
|
||||
const { loginToken } = data.impersonate;
|
||||
|
||||
await startImpersonating(loginToken.token);
|
||||
},
|
||||
onError: () => {
|
||||
@@ -203,7 +215,14 @@ export const SettingsWorkspaceMember = () => {
|
||||
{activeTabId === SETTINGS_WORKSPACE_MEMBER_TABS.TABS_IDS.INFOS && (
|
||||
<MemberInfosTab
|
||||
member={member}
|
||||
onImpersonate={canImpersonate ? handleImpersonate : undefined}
|
||||
onImpersonate={
|
||||
canImpersonate &&
|
||||
isDefined(member.userId) &&
|
||||
isDefined(currentUser?.id) &&
|
||||
member.userId !== currentUser.id
|
||||
? handleImpersonate
|
||||
: undefined
|
||||
}
|
||||
onNameChange={debouncedUpdateName}
|
||||
onDelete={() => openModal(DELETE_MEMBER_MODAL_ID)}
|
||||
/>
|
||||
|
||||
@@ -704,6 +704,15 @@ export class AuthResolver {
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
impersonatorUserWorkspace.userId === toImpersonateUserWorkspace.userId
|
||||
) {
|
||||
throw new AuthException(
|
||||
'User cannot impersonate themselves',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
);
|
||||
}
|
||||
|
||||
const isServerLevelImpersonation =
|
||||
toImpersonateUserWorkspace.workspace.id !==
|
||||
impersonatorUserWorkspace.workspace.id;
|
||||
|
||||
+37
@@ -297,6 +297,43 @@ describe('ImpersonationService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw an error when impersonating the same user', async () => {
|
||||
const sameUserWorkspace = {
|
||||
id: 'same-user-workspace-id',
|
||||
userId: 'same-user-id',
|
||||
workspaceId: 'workspace-id',
|
||||
user: {
|
||||
id: 'same-user-id',
|
||||
email: 'same@example.com',
|
||||
canImpersonate: true,
|
||||
canAccessFullAdminPanel: false,
|
||||
},
|
||||
workspace: {
|
||||
id: 'workspace-id',
|
||||
allowImpersonation: true,
|
||||
},
|
||||
twoFactorAuthenticationMethods: [],
|
||||
};
|
||||
|
||||
UserWorkspaceFindOneMock.mockResolvedValueOnce(sameUserWorkspace);
|
||||
UserWorkspaceFindOneMock.mockResolvedValueOnce(sameUserWorkspace);
|
||||
|
||||
await expect(
|
||||
service.impersonate(
|
||||
'same-user-id',
|
||||
'workspace-id',
|
||||
'same-user-workspace-id',
|
||||
),
|
||||
).rejects.toThrow(
|
||||
new AuthException(
|
||||
'User cannot impersonate themselves',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
),
|
||||
);
|
||||
|
||||
expect(LoginTokenServiceGenerateLoginTokenMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw an error when impersonation is not enabled for the workspace', async () => {
|
||||
const mockToImpersonateUserWorkspace = {
|
||||
userId: 'target-user-id',
|
||||
|
||||
+9
@@ -63,6 +63,15 @@ export class ImpersonationService {
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
toImpersonateUserWorkspace.userId === impersonatorUserWorkspace.userId
|
||||
) {
|
||||
throw new AuthException(
|
||||
'User cannot impersonate themselves',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
);
|
||||
}
|
||||
|
||||
const isServerLevelImpersonation =
|
||||
toImpersonateUserWorkspace.workspace.id !==
|
||||
impersonatorUserWorkspace.workspace.id;
|
||||
|
||||
Reference in New Issue
Block a user