fix: Server-level impersonation doesn't bypass 2FA when enabled (#14340)

## Description

- This PR solves the a sub-issue from
https://github.com/twentyhq/core-team-issues/issues/1421
- impersonation tokens bypasses 2FA as intended 
- Added Audit trails to cover all impersonation events
- Added Proper testing coverage

---------

Co-authored-by: Félix Malfait <felix@twenty.com>
This commit is contained in:
Harshit Singh
2025-09-07 19:23:23 +05:30
committed by GitHub
parent 214375556a
commit b78d139db5
22 changed files with 365 additions and 61 deletions
@@ -8,6 +8,7 @@ import { ApiKey } from 'src/engine/core-modules/api-key/api-key.entity';
import { ApiKeyModule } from 'src/engine/core-modules/api-key/api-key.module';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { AppTokenService } from 'src/engine/core-modules/app-token/services/app-token.service';
import { AuditModule } from 'src/engine/core-modules/audit/audit.module';
import { GoogleAPIsAuthController } from 'src/engine/core-modules/auth/controllers/google-apis-auth.controller';
import { GoogleAuthController } from 'src/engine/core-modules/auth/controllers/google-auth.controller';
import { MicrosoftAPIsAuthController } from 'src/engine/core-modules/auth/controllers/microsoft-apis-auth.controller';
@@ -107,6 +108,7 @@ import { JwtAuthStrategy } from './strategies/jwt.auth.strategy';
UserRoleModule,
TwoFactorAuthenticationModule,
ApiKeyModule,
AuditModule,
],
controllers: [
GoogleAuthController,
@@ -4,6 +4,7 @@ import { getRepositoryToken } from '@nestjs/typeorm';
import { ApiKeyService } from 'src/engine/core-modules/api-key/api-key.service';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { AuditService } from 'src/engine/core-modules/audit/services/audit.service';
import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { RefreshTokenService } from 'src/engine/core-modules/auth/token/services/refresh-token.service';
import { WorkspaceAgnosticTokenService } from 'src/engine/core-modules/auth/token/services/workspace-agnostic-token.service';
@@ -125,6 +126,14 @@ describe('AuthResolver', () => {
provide: TwentyConfigService,
useValue: {},
},
{
provide: AuditService,
useValue: {
createContext: jest.fn().mockReturnValue({
insertWorkspaceEvent: jest.fn(),
}),
},
},
// {
// provide: OAuthService,
// useValue: {},
@@ -22,6 +22,8 @@ import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filt
// import { OAuthService } from 'src/engine/core-modules/auth/services/oauth.service';
import { ApiKeyService } from 'src/engine/core-modules/api-key/api-key.service';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { AuditService } from 'src/engine/core-modules/audit/services/audit.service';
import { MONITORING_EVENT } from 'src/engine/core-modules/audit/utils/events/workspace-event/monitoring/monitoring';
import {
AuthException,
AuthExceptionCode,
@@ -40,7 +42,10 @@ import { RefreshTokenService } from 'src/engine/core-modules/auth/token/services
import { RenewTokenService } from 'src/engine/core-modules/auth/token/services/renew-token.service';
import { TransientTokenService } from 'src/engine/core-modules/auth/token/services/transient-token.service';
import { WorkspaceAgnosticTokenService } from 'src/engine/core-modules/auth/token/services/workspace-agnostic-token.service';
import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type';
import {
JwtTokenTypeEnum,
LoginTokenJwtPayload,
} from 'src/engine/core-modules/auth/types/auth-context.type';
import { CaptchaGuard } from 'src/engine/core-modules/captcha/captcha.guard';
import { CaptchaGraphqlApiExceptionFilter } from 'src/engine/core-modules/captcha/filters/captcha-graphql-api-exception.filter';
import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service';
@@ -53,6 +58,7 @@ import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
import { TwoFactorAuthenticationVerificationInput } from 'src/engine/core-modules/two-factor-authentication/dto/two-factor-authentication-verification.input';
import { TwoFactorAuthenticationExceptionFilter } from 'src/engine/core-modules/two-factor-authentication/two-factor-authentication-exception.filter';
import { TwoFactorAuthenticationService } from 'src/engine/core-modules/two-factor-authentication/two-factor-authentication.service';
import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity';
import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
import { UserService } from 'src/engine/core-modules/user/services/user.service';
import { User } from 'src/engine/core-modules/user/user.entity';
@@ -113,6 +119,7 @@ export class AuthResolver {
private userWorkspaceService: UserWorkspaceService,
private emailVerificationTokenService: EmailVerificationTokenService,
private sSOService: SSOService,
private readonly auditService: AuditService,
) {}
@UseGuards(CaptchaGuard, PublicEndpointGuard)
@@ -536,44 +543,174 @@ export class AuthResolver {
@Args() getAuthTokensFromLoginTokenInput: GetAuthTokensFromLoginTokenInput,
@Args('origin') origin: string,
): Promise<AuthTokens> {
const {
sub: email,
workspaceId,
authProvider,
} = await this.loginTokenService.verifyLoginToken(
const tokenPayload = await this.validateAndDecodeLoginToken(
getAuthTokensFromLoginTokenInput.loginToken,
);
const workspace = await this.validateWorkspaceAccess(
origin,
tokenPayload.workspaceId,
);
const { user, userWorkspace } = await this.validateUserAccess(
tokenPayload.sub,
tokenPayload.workspaceId,
);
if (tokenPayload.authProvider === AuthProviderEnum.Impersonation) {
await this.validateAndLogImpersonation(
tokenPayload,
workspace,
user.email,
);
} else {
await this.validateRegularAuthentication(workspace, userWorkspace);
}
return await this.authService.verify(
user.email,
workspace.id,
tokenPayload.authProvider,
);
}
private async validateAndDecodeLoginToken(
loginToken: string,
): Promise<LoginTokenJwtPayload> {
return await this.loginTokenService.verifyLoginToken(loginToken);
}
private async validateWorkspaceAccess(
origin: string,
tokenWorkspaceId: string,
): Promise<Workspace> {
const workspace =
await this.domainManagerService.getWorkspaceByOriginOrDefaultWorkspace(
origin,
);
workspaceValidator.assertIsDefinedOrThrow(workspace);
workspaceValidator.assertIsDefinedOrThrow(
workspace,
new AuthException(
'Workspace not found',
AuthExceptionCode.WORKSPACE_NOT_FOUND,
),
);
if (workspaceId !== workspace.id) {
if (tokenWorkspaceId !== workspace.id) {
throw new AuthException(
'Token is not valid for this workspace',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
);
}
return workspace;
}
private async validateUserAccess(
email: string,
workspaceId: string,
): Promise<{ user: User; userWorkspace: UserWorkspace }> {
const user = await this.userService.getUserByEmail(email);
await this.authService.checkIsEmailVerified(user.isEmailVerified);
const currentUserWorkspace =
const userWorkspace =
await this.userWorkspaceService.getUserWorkspaceForUserOrThrow({
userId: user.id,
workspaceId,
});
return { user, userWorkspace };
}
private async validateRegularAuthentication(
workspace: Workspace,
userWorkspace: UserWorkspace,
): Promise<void> {
await this.twoFactorAuthenticationService.validateTwoFactorAuthenticationRequirement(
workspace,
currentUserWorkspace.twoFactorAuthenticationMethods,
userWorkspace.twoFactorAuthenticationMethods,
);
}
return await this.authService.verify(email, workspace.id, authProvider);
private async validateAndLogImpersonation(
tokenPayload: LoginTokenJwtPayload,
workspace: Workspace,
targetUserEmail: string,
): Promise<void> {
const { impersonatorUserId } = tokenPayload;
const auditService = this.auditService.createContext({
workspaceId: workspace.id,
userId: impersonatorUserId,
});
await auditService.insertWorkspaceEvent(MONITORING_EVENT, {
eventName: 'server.impersonation.token_exchange_attempt',
message: `Impersonation token exchange attempt for ${targetUserEmail} by ${impersonatorUserId}`,
});
if (workspace.allowImpersonation !== true) {
throw new AuthException(
'Impersonation not allowed on this workspace',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
);
}
if (!impersonatorUserId) {
await auditService.insertWorkspaceEvent(MONITORING_EVENT, {
eventName: 'server.impersonation.token_exchange_failed',
message: `Invalid impersonation token (missing impersonator user ID) for ${targetUserEmail}`,
});
throw new AuthException(
'Invalid impersonation token (missing impersonator user ID)',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
);
}
const impersonatorUser = await this.userRepository.findOne({
where: { id: impersonatorUserId },
});
if (!impersonatorUser) {
await auditService.insertWorkspaceEvent(MONITORING_EVENT, {
eventName: 'server.impersonation.token_exchange_failed',
message: `Impersonator user not found: ${impersonatorUserId} for ${targetUserEmail}`,
});
throw new AuthException(
'Impersonator user not found',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
);
}
if (impersonatorUser.canImpersonate !== true) {
await auditService.insertWorkspaceEvent(MONITORING_EVENT, {
eventName: 'server.impersonation.token_exchange_failed',
message: `User not authorized to impersonate: ${impersonatorUserId} for ${targetUserEmail}`,
});
throw new AuthException(
'User not authorized to impersonate',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
);
}
await this.logImpersonationEvent(workspace.id, impersonatorUserId);
}
private async logImpersonationEvent(
workspaceId: string,
impersonatorUserId: string,
): Promise<void> {
const auditService = this.auditService.createContext({
workspaceId,
userId: impersonatorUserId,
});
await auditService.insertWorkspaceEvent(MONITORING_EVENT, {
eventName: 'server.impersonation.login_token_exchanged',
message: 'Impersonation token exchanged',
});
}
@Mutation(() => AuthorizeApp)
@@ -268,7 +268,7 @@ export class AuthService {
async verify(
email: string,
workspaceId: string,
authProvider?: AuthProviderEnum,
authProvider: AuthProviderEnum,
): Promise<AuthTokens> {
if (!email) {
throw new AuthException(
@@ -13,6 +13,7 @@ import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrap
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity';
import { User } from 'src/engine/core-modules/user/user.entity';
import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
@@ -131,7 +132,11 @@ describe('AccessTokenService', () => {
} as any);
jest.spyOn(jwtWrapperService, 'sign').mockReturnValue(mockToken);
const result = await service.generateAccessToken({ userId, workspaceId });
const result = await service.generateAccessToken({
userId,
workspaceId,
authProvider: AuthProviderEnum.Password,
});
expect(result).toEqual({
token: mockToken,
@@ -155,6 +160,7 @@ describe('AccessTokenService', () => {
service.generateAccessToken({
userId: 'non-existent-user',
workspaceId: 'workspace-id',
authProvider: AuthProviderEnum.Password,
}),
).rejects.toThrow(AuthException);
});
@@ -2,6 +2,7 @@ import { Test, type TestingModule } from '@nestjs/testing';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
import { LoginTokenService } from './login-token.service';
@@ -55,7 +56,11 @@ describe('LoginTokenService', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue(mockExpiresIn);
jest.spyOn(jwtWrapperService, 'sign').mockReturnValue(mockToken);
const result = await service.generateLoginToken(email, workspaceId);
const result = await service.generateLoginToken(
email,
workspaceId,
AuthProviderEnum.Password,
);
expect(result).toEqual({
token: mockToken,
@@ -69,12 +74,60 @@ describe('LoginTokenService', () => {
'LOGIN_TOKEN_EXPIRES_IN',
);
expect(jwtWrapperService.sign).toHaveBeenCalledWith(
{ sub: email, workspaceId, type: 'LOGIN' },
{
sub: email,
workspaceId,
type: 'LOGIN',
authProvider: AuthProviderEnum.Password,
impersonatorUserId: undefined,
},
{ secret: mockSecret, expiresIn: mockExpiresIn },
);
});
});
describe('generateLoginToken with impersonation', () => {
it('should include impersonatorUserId in JWT payload when using Impersonation auth provider', async () => {
const email = 'test@example.com';
const mockSecret = 'mock-secret';
const mockToken = 'mock-token';
const workspaceId = 'workspace-id';
const impersonatorUserId = 'impersonator-id';
jest
.spyOn(jwtWrapperService, 'generateAppSecret')
.mockReturnValue(mockSecret);
jest.spyOn(twentyConfigService, 'get').mockReturnValue('1h');
jest.spyOn(jwtWrapperService, 'sign').mockReturnValue(mockToken);
const result = await service.generateLoginToken(
email,
workspaceId,
AuthProviderEnum.Impersonation,
{ impersonatorUserId },
);
expect(result).toEqual({
token: mockToken,
expiresAt: expect.any(Date),
});
expect(jwtWrapperService.generateAppSecret).toHaveBeenCalledWith(
'LOGIN',
workspaceId,
);
expect(jwtWrapperService.sign).toHaveBeenCalledWith(
{
sub: email,
workspaceId,
type: 'LOGIN',
authProvider: AuthProviderEnum.Impersonation,
impersonatorUserId,
},
{ secret: mockSecret, expiresIn: expect.any(String) },
);
});
});
describe('verifyLoginToken', () => {
it('should verify a login token successfully', async () => {
const mockToken = 'valid-token';
@@ -4,12 +4,12 @@ import { addMilliseconds } from 'date-fns';
import ms from 'ms';
import { type AuthToken } from 'src/engine/core-modules/auth/dto/token.entity';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import {
type LoginTokenJwtPayload,
JwtTokenTypeEnum,
} from 'src/engine/core-modules/auth/types/auth-context.type';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { type AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
@Injectable()
@@ -22,13 +22,15 @@ export class LoginTokenService {
async generateLoginToken(
email: string,
workspaceId: string,
authProvider?: AuthProviderEnum,
authProvider: AuthProviderEnum,
options?: { impersonatorUserId?: string },
): Promise<AuthToken> {
const jwtPayload: LoginTokenJwtPayload = {
type: JwtTokenTypeEnum.LOGIN,
sub: email,
workspaceId,
authProvider,
impersonatorUserId: options?.impersonatorUserId,
};
const secret = this.jwtWrapperService.generateAppSecret(
@@ -10,6 +10,7 @@ import { RefreshTokenService } from 'src/engine/core-modules/auth/token/services
import { WorkspaceAgnosticTokenService } from 'src/engine/core-modules/auth/token/services/workspace-agnostic-token.service';
import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type';
import { type User } from 'src/engine/core-modules/user/user.entity';
import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
import { RenewTokenService } from './renew-token.service';
@@ -86,7 +87,7 @@ describe('RenewTokenService', () => {
jest.spyOn(refreshTokenService, 'verifyRefreshToken').mockResolvedValue({
user: mockUser,
token: mockAppToken as AppToken,
authProvider: undefined,
authProvider: AuthProviderEnum.Password,
targetedTokenType: JwtTokenTypeEnum.ACCESS,
});
jest.spyOn(appTokenRepository, 'update').mockResolvedValue({} as any);
@@ -114,9 +115,10 @@ describe('RenewTokenService', () => {
expect(accessTokenService.generateAccessToken).toHaveBeenCalledWith({
userId: mockUser.id,
workspaceId: mockWorkspaceId,
authProvider: AuthProviderEnum.Password,
});
expect(refreshTokenService.generateRefreshToken).toHaveBeenCalledWith({
authProvider: undefined,
authProvider: AuthProviderEnum.Password,
targetedTokenType: JwtTokenTypeEnum.ACCESS,
userId: mockUser.id,
workspaceId: mockWorkspaceId,
@@ -14,6 +14,7 @@ import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/
import { RefreshTokenService } from 'src/engine/core-modules/auth/token/services/refresh-token.service';
import { WorkspaceAgnosticTokenService } from 'src/engine/core-modules/auth/token/services/workspace-agnostic-token.service';
import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type';
import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
@Injectable()
export class RenewTokenService {
@@ -57,6 +58,10 @@ export class RenewTokenService {
const targetedTokenType =
targetedTokenTypeFromPayload ?? JwtTokenTypeEnum.ACCESS;
// Support legacy tokens where authProvider might be undefined
// TODO: remove in November 2025
const resolvedAuthProvider = authProvider ?? AuthProviderEnum.Password;
const accessToken =
isDefined(authProvider) &&
targetedTokenType === JwtTokenTypeEnum.WORKSPACE_AGNOSTIC
@@ -69,13 +74,13 @@ export class RenewTokenService {
: await this.accessTokenService.generateAccessToken({
userId: user.id,
workspaceId,
authProvider,
authProvider: resolvedAuthProvider,
});
const refreshToken = await this.refreshTokenService.generateRefreshToken({
userId: user.id,
workspaceId,
authProvider,
authProvider: resolvedAuthProvider,
targetedTokenType,
});
@@ -43,7 +43,8 @@ export type FileTokenJwtPayload = CommonPropertiesJwtPayload & {
export type LoginTokenJwtPayload = CommonPropertiesJwtPayload & {
type: JwtTokenTypeEnum.LOGIN;
workspaceId: string;
authProvider?: AuthProviderEnum;
authProvider: AuthProviderEnum;
impersonatorUserId?: string;
};
export type TransientTokenJwtPayload = CommonPropertiesJwtPayload & {
@@ -81,7 +82,7 @@ export type AccessTokenJwtPayload = CommonPropertiesJwtPayload & {
userId: string;
workspaceMemberId?: string;
userWorkspaceId: string;
authProvider?: AuthProviderEnum;
authProvider: AuthProviderEnum;
};
export type PostgresProxyTokenJwtPayload = CommonPropertiesJwtPayload & {