Refactor email composer (#21177)

# Introduction Gate what connected account can be ingested in case of ai mcp user workspace agnostic funnel to only the workspace shared connected account Added a quick win intregration tests on seeded connected accounts ( that wasn't covered but already protected fix impacts only the mcp ) Refactored the API slightly too ## Notice This mean there's a breaking change in the product behavior Whereas before a non user workspace related mcp interaction would might have fallback on any private user connected account it will now only search for workspace visible listed ones
2026-06-03 17:15:51 +02:00
parent 8c3a93871e
commit 164a5b1e8d
10 changed files with 736 additions and 84 deletions
@@ -15,7 +15,7 @@ import { HttpTool } from 'src/engine/core-modules/tool/tools/http-tool/http-tool
 import { NavigateAppTool } from 'src/engine/core-modules/tool/tools/navigate-tool/navigate-app-tool';
 import { SearchHelpCenterTool } from 'src/engine/core-modules/tool/tools/search-help-center-tool/search-help-center-tool';
 import { WorkspaceManyOrAllFlatEntityMapsCacheModule } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.module';
-import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { ConnectedAccountMetadataModule } from 'src/engine/metadata-modules/connected-account/connected-account-metadata.module';
 import { NavigationMenuItemModule } from 'src/engine/metadata-modules/navigation-menu-item/navigation-menu-item.module';
 import { ObjectMetadataModule } from 'src/engine/metadata-modules/object-metadata/object-metadata.module';
 import { ViewModule } from 'src/engine/metadata-modules/view/view.module';
@@ -26,7 +26,8 @@ import { provideWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspac
  imports: [
    MessagingImportManagerModule,
    MessagingSendManagerModule,
-    TypeOrmModule.forFeature([FileEntity, ConnectedAccountEntity]),
+    TypeOrmModule.forFeature([FileEntity]),
+    ConnectedAccountMetadataModule,
    ApplicationModule,
    FeatureFlagModule,
    FileModule,
@@ -0,0 +1,287 @@
+import { randomUUID } from 'node:crypto';
+
+import { Test, type TestingModule } from '@nestjs/testing';
+
+import { ConnectedAccountProvider, FileFolder } from 'twenty-shared/types';
+import { isDefined } from 'twenty-shared/utils';
+
+import { EmailComposerService } from 'src/engine/core-modules/tool/tools/email-tool/email-composer.service';
+import { type ComposeEmailParams } from 'src/engine/core-modules/tool/tools/email-tool/types/compose-email-params.type';
+import { type ToolExecutionContext } from 'src/engine/core-modules/tool/types/tool-execution-context.type';
+import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
+import { FileService } from 'src/engine/core-modules/file/services/file.service';
+import { ConnectedAccountMetadataService } from 'src/engine/metadata-modules/connected-account/connected-account-metadata.service';
+import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
+import { getWorkspaceScopedRepositoryToken } from 'src/engine/twenty-orm/workspace-scoped-repository/get-workspace-scoped-repository-token.util';
+
+const WORKSPACE_ID = randomUUID();
+const ALICE_USER_WORKSPACE_ID = randomUUID();
+const BOB_USER_WORKSPACE_ID = randomUUID();
+
+const ALICE_ACCOUNT_ID = randomUUID();
+const BOB_ACCOUNT_ID = randomUUID();
+const SHARED_ACCOUNT_ID = randomUUID();
+
+// In-memory connected accounts that mimic the rows TypeORM would return.
+type FakeAccount = Partial<ConnectedAccountEntity> & { id: string };
+
+const aliceUserPrivateAccount: FakeAccount = {
+  id: ALICE_ACCOUNT_ID,
+  workspaceId: WORKSPACE_ID,
+  userWorkspaceId: ALICE_USER_WORKSPACE_ID,
+  visibility: 'user',
+  handle: 'alice@example.com',
+  provider: ConnectedAccountProvider.GOOGLE,
+  connectionParameters: null,
+  messageChannels: [{ id: 'mc-alice', handle: 'alice@example.com' }] as never,
+};
+
+const bobUserPrivateAccount: FakeAccount = {
+  id: BOB_ACCOUNT_ID,
+  workspaceId: WORKSPACE_ID,
+  userWorkspaceId: BOB_USER_WORKSPACE_ID,
+  visibility: 'user',
+  handle: 'bob@example.com',
+  provider: ConnectedAccountProvider.GOOGLE,
+  connectionParameters: null,
+  messageChannels: [{ id: 'mc-bob', handle: 'bob@example.com' }] as never,
+};
+
+// Workspace-visibility account (owned by Alice but shared with the workspace).
+const sharedWorkspaceAccount: FakeAccount = {
+  id: SHARED_ACCOUNT_ID,
+  workspaceId: WORKSPACE_ID,
+  userWorkspaceId: ALICE_USER_WORKSPACE_ID,
+  visibility: 'workspace',
+  handle: 'team@example.com',
+  provider: ConnectedAccountProvider.GOOGLE,
+  connectionParameters: null,
+  messageChannels: [{ id: 'mc-shared', handle: 'team@example.com' }] as never,
+};
+
+// Mirrors ConnectedAccountMetadataService's visibility rule: an account is
+// usable by a caller when it is workspace-shared, or it belongs to the caller's
+// own user workspace. The authoritative scoping is proven directly against the
+// repository in connected-account-metadata.service.spec.ts; here we stub the
+// finders so these tests focus on the composer's own selection/rejection logic.
+const isVisibleToCaller = (
+  account: FakeAccount,
+  userWorkspaceId: string | undefined,
+): boolean =>
+  account.visibility === 'workspace' ||
+  (isDefined(userWorkspaceId) && account.userWorkspaceId === userWorkspaceId);
+
+const buildComposeParams = (
+  overrides: Partial<ComposeEmailParams> = {},
+): ComposeEmailParams => ({
+  recipients: { to: 'recipient@example.com' },
+  subject: 'Hello',
+  body: '<p>Hello</p>',
+  ...overrides,
+});
+
+describe('EmailComposerService - connected account authorization', () => {
+  let service: EmailComposerService;
+  let accounts: FakeAccount[];
+
+  beforeEach(async () => {
+    accounts = [
+      aliceUserPrivateAccount,
+      bobUserPrivateAccount,
+      sharedWorkspaceAccount,
+    ];
+
+    const mockConnectedAccountMetadataService = {
+      findAccessibleConnectedAccountById: jest.fn(
+        ({ id, userWorkspaceId, workspaceId }) =>
+          Promise.resolve(
+            accounts.find(
+              (account) =>
+                account.id === id &&
+                account.workspaceId === workspaceId &&
+                isVisibleToCaller(account, userWorkspaceId),
+            ) ?? null,
+          ),
+      ),
+      findAccessibleConnectedAccounts: jest.fn(
+        ({ userWorkspaceId, workspaceId }) => {
+          const accessibleAccounts = accounts.filter(
+            (account) =>
+              account.workspaceId === workspaceId &&
+              isVisibleToCaller(account, userWorkspaceId),
+          );
+
+          return Promise.resolve({
+            userConnectedAccounts: accessibleAccounts.filter(
+              (account) => account.userWorkspaceId === userWorkspaceId,
+            ),
+            workspaceSharedConnectedAccounts: accessibleAccounts.filter(
+              (account) => account.userWorkspaceId !== userWorkspaceId,
+            ),
+          });
+        },
+      ),
+    };
+
+    const mockGlobalWorkspaceOrmManager = {
+      executeInWorkspaceContext: jest
+        .fn()
+        .mockImplementation((fn: () => unknown) => fn()),
+      getRepository: jest.fn(),
+    };
+
+    const mockFileRepository = {
+      find: jest.fn().mockResolvedValue([]),
+    };
+
+    const mockFileService = {
+      getFileStreamById: jest.fn(),
+    };
+
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        EmailComposerService,
+        {
+          provide: GlobalWorkspaceOrmManager,
+          useValue: mockGlobalWorkspaceOrmManager,
+        },
+        {
+          provide: ConnectedAccountMetadataService,
+          useValue: mockConnectedAccountMetadataService,
+        },
+        {
+          provide: getWorkspaceScopedRepositoryToken(FileEntity),
+          useValue: mockFileRepository,
+        },
+        {
+          provide: FileService,
+          useValue: mockFileService,
+        },
+      ],
+    }).compile();
+
+    service = module.get<EmailComposerService>(EmailComposerService);
+  });
+
+  const compose = (params: ComposeEmailParams, context: ToolExecutionContext) =>
+    service.composeEmail(params, context, {
+      attachmentsFileFolder: FileFolder.Workflow,
+    });
+
+  describe('explicit connectedAccountId', () => {
+    it("should reject sending from another member's user-private account (impersonation)", async () => {
+      const bobContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: BOB_USER_WORKSPACE_ID,
+      };
+
+      // Bob asks to send FROM Alice's private account.
+      await expect(
+        compose(
+          buildComposeParams({
+            connectedAccountId: aliceUserPrivateAccount.id,
+          }),
+          bobContext,
+        ),
+      ).rejects.toThrow();
+    });
+
+    it('should allow a member to use their own user-private account', async () => {
+      const bobContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: BOB_USER_WORKSPACE_ID,
+      };
+
+      const result = await compose(
+        buildComposeParams({ connectedAccountId: bobUserPrivateAccount.id }),
+        bobContext,
+      );
+
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.connectedAccount.id).toBe(bobUserPrivateAccount.id);
+      }
+    });
+
+    it('should allow any member to use a workspace-visibility account', async () => {
+      const bobContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: BOB_USER_WORKSPACE_ID,
+      };
+
+      const result = await compose(
+        buildComposeParams({ connectedAccountId: sharedWorkspaceAccount.id }),
+        bobContext,
+      );
+
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.connectedAccount.id).toBe(sharedWorkspaceAccount.id);
+      }
+    });
+  });
+
+  describe('omitted connectedAccountId (default selection)', () => {
+    it("should not silently default to another member's user-private account", async () => {
+      // Only Alice's user-private account exists; Bob has none of his own.
+      accounts = [aliceUserPrivateAccount];
+
+      const bobContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: BOB_USER_WORKSPACE_ID,
+      };
+
+      await expect(compose(buildComposeParams(), bobContext)).rejects.toThrow();
+    });
+
+    it('should prefer the caller own account over a workspace-visibility account', async () => {
+      const bobContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+        userWorkspaceId: BOB_USER_WORKSPACE_ID,
+      };
+
+      const result = await compose(buildComposeParams(), bobContext);
+
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.connectedAccount.userWorkspaceId).toBe(
+          BOB_USER_WORKSPACE_ID,
+        );
+      }
+    });
+  });
+
+  describe('system/workflow execution without a user identity', () => {
+    it('should reject a user-private account when no userWorkspaceId is present', async () => {
+      const systemContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+      };
+
+      await expect(
+        compose(
+          buildComposeParams({
+            connectedAccountId: aliceUserPrivateAccount.id,
+          }),
+          systemContext,
+        ),
+      ).rejects.toThrow();
+    });
+
+    it('should allow a workspace-visibility account when no userWorkspaceId is present', async () => {
+      const systemContext: ToolExecutionContext = {
+        workspaceId: WORKSPACE_ID,
+      };
+
+      const result = await compose(
+        buildComposeParams({ connectedAccountId: sharedWorkspaceAccount.id }),
+        systemContext,
+      );
+
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.connectedAccount.id).toBe(sharedWorkspaceAccount.id);
+      }
+    });
+  });
+});
@@ -1,5 +1,4 @@
 import { Injectable, Logger } from '@nestjs/common';
-import { InjectRepository } from '@nestjs/typeorm';

 import { toPlainText } from '@react-email/render';
 import { isNonEmptyString } from '@sniptt/guards';
@@ -11,7 +10,7 @@ import {
  FileFolder,
 } from 'twenty-shared/types';
 import { isDefined, isValidUuid } from 'twenty-shared/utils';
-import { In, LessThanOrEqual, type Repository } from 'typeorm';
+import { In, LessThanOrEqual } from 'typeorm';
 import { z } from 'zod';

 import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
@@ -24,7 +23,7 @@ import { type ComposeEmailParams } from 'src/engine/core-modules/tool/tools/emai
 import { EmailComposerResult } from 'src/engine/core-modules/tool/tools/email-tool/types/email-composer-result.type';
 import { parseCommaSeparatedEmails } from 'src/engine/core-modules/tool/tools/email-tool/utils/parse-comma-separated-emails.util';
 import { type ToolExecutionContext } from 'src/engine/core-modules/tool/types/tool-execution-context.type';
-import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-account/entities/connected-account.entity';
+import { ConnectedAccountMetadataService } from 'src/engine/metadata-modules/connected-account/connected-account-metadata.service';
 import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
 import { buildSystemAuthContext } from 'src/engine/twenty-orm/utils/build-system-auth-context.util';
 import { InjectWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/inject-workspace-scoped-repository.decorator';
@@ -45,17 +44,21 @@ export class EmailComposerService {

  constructor(
    private readonly globalWorkspaceOrmManager: GlobalWorkspaceOrmManager,
-    @InjectRepository(ConnectedAccountEntity)
-    private readonly connectedAccountRepository: Repository<ConnectedAccountEntity>,
+    private readonly connectedAccountMetadataService: ConnectedAccountMetadataService,
    @InjectWorkspaceScopedRepository(FileEntity)
    private readonly fileRepository: WorkspaceScopedRepository<FileEntity>,
    private readonly fileService: FileService,
  ) {}

-  private async getConnectedAccount(
-    connectedAccountId: string,
-    workspaceId: string,
-  ) {
+  private async getConnectedAccount({
+    connectedAccountId,
+    workspaceId,
+    userWorkspaceId,
+  }: {
+    connectedAccountId: string;
+    workspaceId: string;
+    userWorkspaceId: string | undefined;
+  }) {
    if (!isValidUuid(connectedAccountId)) {
      throw new EmailToolException(
        `Connected Account ID is not a valid UUID`,
@@ -63,54 +66,63 @@ export class EmailComposerService {
      );
    }

-    const authContext = buildSystemAuthContext(workspaceId);
-
-    return this.globalWorkspaceOrmManager.executeInWorkspaceContext(
-      async () => {
-        const connectedAccount = await this.connectedAccountRepository.findOne({
-          where: { id: connectedAccountId, workspaceId },
+    const connectedAccount =
+      await this.connectedAccountMetadataService.findAccessibleConnectedAccountById(
+        {
+          id: connectedAccountId,
+          userWorkspaceId,
+          workspaceId,
          relations: {
            messageChannels: {
              messageFolders: true,
            },
          },
-        });
+        },
+      );

-        if (!isDefined(connectedAccount)) {
-          throw new EmailToolException(
-            `Connected Account '${connectedAccountId}' not found`,
-            EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
-          );
-        }
+    if (!isDefined(connectedAccount)) {
+      throw new EmailToolException(
+        `Connected Account '${connectedAccountId}' not found`,
+        EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
+      );
+    }

-        return connectedAccount;
-      },
-      authContext,
-    );
+    return connectedAccount;
  }

-  private async getOrThrowFirstConnectedAccountId(
-    workspaceId: string,
-  ): Promise<string> {
-    const authContext = buildSystemAuthContext(workspaceId);
+  private async getDefaultConnectedAccountOrThrow({
+    workspaceId,
+    userWorkspaceId,
+  }: {
+    workspaceId: string;
+    userWorkspaceId: string | undefined;
+  }) {
+    const { userConnectedAccounts, workspaceSharedConnectedAccounts } =
+      await this.connectedAccountMetadataService.findAccessibleConnectedAccounts(
+        {
+          userWorkspaceId,
+          workspaceId,
+          relations: {
+            messageChannels: {
+              messageFolders: true,
+            },
+          },
+        },
+      );

-    return this.globalWorkspaceOrmManager.executeInWorkspaceContext(
-      async () => {
-        const allAccounts = await this.connectedAccountRepository.find({
-          where: { workspaceId },
-        });
+    // Prefer the caller's own account; fall back to a workspace-shared one, but
+    // never silently default to another member's private account.
+    const connectedAccount =
+      userConnectedAccounts[0] ?? workspaceSharedConnectedAccounts[0];

-        if (!allAccounts || allAccounts.length === 0) {
-          throw new EmailToolException(
-            'No connected accounts found for this workspace',
-            EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
-          );
-        }
+    if (!isDefined(connectedAccount)) {
+      throw new EmailToolException(
+        'No connected accounts found for this workspace',
+        EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
+      );
+    }

-        return allAccounts[0].id;
-      },
-      authContext,
-    );
+    return connectedAccount;
  }

  private normalizeRecipients(parameters: ComposeEmailParams): {
@@ -314,9 +326,8 @@ export class EmailComposerService {
    context: ToolExecutionContext,
    options: { attachmentsFileFolder: FileFolder },
  ): Promise<EmailComposerResult> {
-    const { workspaceId } = context;
-    const { subject, body, files, inReplyTo } = parameters;
-    let { connectedAccountId } = parameters;
+    const { workspaceId, userWorkspaceId } = context;
+    const { subject, body, files, inReplyTo, connectedAccountId } = parameters;

    let recipients: { to: string[]; cc: string[]; bcc: string[] };

@@ -352,15 +363,16 @@ export class EmailComposerService {

    const toRecipientsDisplay = recipients.to.join(', ');

-    if (!connectedAccountId) {
-      connectedAccountId =
-        await this.getOrThrowFirstConnectedAccountId(workspaceId);
-    }
-
-    const connectedAccount = await this.getConnectedAccount(
-      connectedAccountId,
-      workspaceId,
-    );
+    const connectedAccount = isNonEmptyString(connectedAccountId)
+      ? await this.getConnectedAccount({
+          connectedAccountId,
+          workspaceId,
+          userWorkspaceId,
+        })
+      : await this.getDefaultConnectedAccountOrThrow({
+          workspaceId,
+          userWorkspaceId,
+        });

    const messageChannel = connectedAccount.messageChannels.find(
      (channel) => channel.handle === connectedAccount.handle,
@@ -375,14 +387,14 @@ export class EmailComposerService {
      !isDefined(connectedAccount.connectionParameters?.SMTP)
    ) {
      throw new EmailToolException(
-        `SMTP is not configured for connected account '${connectedAccountId}'`,
+        `SMTP is not configured for connected account '${connectedAccount.id}'`,
        EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
      );
    }

    if (!isSmtpOnlyAccount && !isDefined(messageChannel)) {
      throw new EmailToolException(
-        `No message channel found for connected account '${connectedAccountId}'`,
+        `No message channel found for connected account '${connectedAccount.id}'`,
        EmailToolExceptionCode.CONNECTED_ACCOUNT_NOT_FOUND,
      );
    }