diff --git a/packages/twenty-client-sdk/src/metadata/generated/schema.graphql b/packages/twenty-client-sdk/src/metadata/generated/schema.graphql index 09f35091ad..ee4b7d8a01 100644 --- a/packages/twenty-client-sdk/src/metadata/generated/schema.graphql +++ b/packages/twenty-client-sdk/src/metadata/generated/schema.graphql @@ -2987,6 +2987,7 @@ type Query { ): IndexConnection! findManyAgents: [Agent!]! findOneAgent(input: AgentIdInput!): Agent! + myConnectedAccounts: [ConnectedAccountPublicDTO!]! getRoles: [Role!]! getToolIndex: [ToolIndexEntry!]! getToolInputSchema(toolName: String!): JSON @@ -3007,7 +3008,6 @@ type Query { getViewGroup(id: String!): ViewGroup myMessageFolders(messageChannelId: UUID): [MessageFolder!]! myMessageChannels(connectedAccountId: UUID): [MessageChannel!]! - myConnectedAccounts: [ConnectedAccountPublicDTO!]! myCalendarChannels(connectedAccountId: UUID): [CalendarChannel!]! minimalMetadata: MinimalMetadata! findWorkspaceAiStats: WorkspaceAiStats! @@ -3226,6 +3226,7 @@ type Mutation { createOneAgent(input: CreateAgentInput!): Agent! updateOneAgent(input: UpdateAgentInput!): Agent! deleteOneAgent(input: AgentIdInput!): Agent! + deleteConnectedAccount(id: UUID!): ConnectedAccountPublicDTO! updateWorkspaceMemberRole(workspaceMemberId: UUID!, roleId: UUID!): WorkspaceMember! createOneRole(createRoleInput: CreateRoleInput!): Role! updateOneRole(updateRoleInput: UpdateRoleInput!): Role! @@ -3253,7 +3254,6 @@ type Mutation { updateMessageChannel(input: UpdateMessageChannelInput!): MessageChannel! createEmailGroupChannel(input: CreateEmailGroupChannelInput!): CreateEmailGroupChannelOutput! deleteEmailGroupChannel(id: UUID!): MessageChannel! - deleteConnectedAccount(id: UUID!): ConnectedAccountPublicDTO! updateCalendarChannel(input: UpdateCalendarChannelInput!): CalendarChannel! createChatThread: AgentChatThread! sendChatMessage(threadId: UUID!, text: String!, messageId: UUID!, browsingContext: JSON, modelId: String, fileAttachments: [FileAttachmentInput!]): SendChatMessageResult! diff --git a/packages/twenty-client-sdk/src/metadata/generated/schema.ts b/packages/twenty-client-sdk/src/metadata/generated/schema.ts index e2bda1803c..dfb6062d4e 100644 --- a/packages/twenty-client-sdk/src/metadata/generated/schema.ts +++ b/packages/twenty-client-sdk/src/metadata/generated/schema.ts @@ -2593,6 +2593,7 @@ export interface Query { indexMetadatas: IndexConnection findManyAgents: Agent[] findOneAgent: Agent + myConnectedAccounts: ConnectedAccountPublicDTO[] getRoles: Role[] getToolIndex: ToolIndexEntry[] getToolInputSchema?: Scalars['JSON'] @@ -2604,7 +2605,6 @@ export interface Query { getViewGroup?: ViewGroup myMessageFolders: MessageFolder[] myMessageChannels: MessageChannel[] - myConnectedAccounts: ConnectedAccountPublicDTO[] myCalendarChannels: CalendarChannel[] minimalMetadata: MinimalMetadata findWorkspaceAiStats: WorkspaceAiStats @@ -2756,6 +2756,7 @@ export interface Mutation { createOneAgent: Agent updateOneAgent: Agent deleteOneAgent: Agent + deleteConnectedAccount: ConnectedAccountPublicDTO updateWorkspaceMemberRole: WorkspaceMember createOneRole: Role updateOneRole: Role @@ -2783,7 +2784,6 @@ export interface Mutation { updateMessageChannel: MessageChannel createEmailGroupChannel: CreateEmailGroupChannelOutput deleteEmailGroupChannel: MessageChannel - deleteConnectedAccount: ConnectedAccountPublicDTO updateCalendarChannel: CalendarChannel createChatThread: AgentChatThread sendChatMessage: SendChatMessageResult @@ -5626,6 +5626,7 @@ export interface QueryGenqlSelection{ filter: IndexFilter} }) findManyAgents?: AgentGenqlSelection findOneAgent?: (AgentGenqlSelection & { __args: {input: AgentIdInput} }) + myConnectedAccounts?: ConnectedAccountPublicDTOGenqlSelection getRoles?: RoleGenqlSelection getToolIndex?: ToolIndexEntryGenqlSelection getToolInputSchema?: { __args: {toolName: Scalars['String']} } @@ -5643,7 +5644,6 @@ export interface QueryGenqlSelection{ getViewGroup?: (ViewGroupGenqlSelection & { __args: {id: Scalars['String']} }) myMessageFolders?: (MessageFolderGenqlSelection & { __args?: {messageChannelId?: (Scalars['UUID'] | null)} }) myMessageChannels?: (MessageChannelGenqlSelection & { __args?: {connectedAccountId?: (Scalars['UUID'] | null)} }) - myConnectedAccounts?: ConnectedAccountPublicDTOGenqlSelection myCalendarChannels?: (CalendarChannelGenqlSelection & { __args?: {connectedAccountId?: (Scalars['UUID'] | null)} }) minimalMetadata?: MinimalMetadataGenqlSelection findWorkspaceAiStats?: WorkspaceAiStatsGenqlSelection @@ -5816,6 +5816,7 @@ export interface MutationGenqlSelection{ createOneAgent?: (AgentGenqlSelection & { __args: {input: CreateAgentInput} }) updateOneAgent?: (AgentGenqlSelection & { __args: {input: UpdateAgentInput} }) deleteOneAgent?: (AgentGenqlSelection & { __args: {input: AgentIdInput} }) + deleteConnectedAccount?: (ConnectedAccountPublicDTOGenqlSelection & { __args: {id: Scalars['UUID']} }) updateWorkspaceMemberRole?: (WorkspaceMemberGenqlSelection & { __args: {workspaceMemberId: Scalars['UUID'], roleId: Scalars['UUID']} }) createOneRole?: (RoleGenqlSelection & { __args: {createRoleInput: CreateRoleInput} }) updateOneRole?: (RoleGenqlSelection & { __args: {updateRoleInput: UpdateRoleInput} }) @@ -5843,7 +5844,6 @@ export interface MutationGenqlSelection{ updateMessageChannel?: (MessageChannelGenqlSelection & { __args: {input: UpdateMessageChannelInput} }) createEmailGroupChannel?: (CreateEmailGroupChannelOutputGenqlSelection & { __args: {input: CreateEmailGroupChannelInput} }) deleteEmailGroupChannel?: (MessageChannelGenqlSelection & { __args: {id: Scalars['UUID']} }) - deleteConnectedAccount?: (ConnectedAccountPublicDTOGenqlSelection & { __args: {id: Scalars['UUID']} }) updateCalendarChannel?: (CalendarChannelGenqlSelection & { __args: {input: UpdateCalendarChannelInput} }) createChatThread?: AgentChatThreadGenqlSelection sendChatMessage?: (SendChatMessageResultGenqlSelection & { __args: {threadId: Scalars['UUID'], text: Scalars['String'], messageId: Scalars['UUID'], browsingContext?: (Scalars['JSON'] | null), modelId?: (Scalars['String'] | null), fileAttachments?: (FileAttachmentInput[] | null)} }) diff --git a/packages/twenty-client-sdk/src/metadata/generated/types.ts b/packages/twenty-client-sdk/src/metadata/generated/types.ts index b1330e150b..c676ed29bf 100644 --- a/packages/twenty-client-sdk/src/metadata/generated/types.ts +++ b/packages/twenty-client-sdk/src/metadata/generated/types.ts @@ -6104,6 +6104,9 @@ export default { ] } ], + "myConnectedAccounts": [ + 270 + ], "getRoles": [ 29 ], @@ -6186,9 +6189,6 @@ export default { ] } ], - "myConnectedAccounts": [ - 270 - ], "myCalendarChannels": [ 303, { @@ -7577,6 +7577,15 @@ export default { ] } ], + "deleteConnectedAccount": [ + 270, + { + "id": [ + 3, + "UUID!" + ] + } + ], "updateWorkspaceMemberRole": [ 20, { @@ -7828,15 +7837,6 @@ export default { ] } ], - "deleteConnectedAccount": [ - 270, - { - "id": [ - 3, - "UUID!" - ] - } - ], "updateCalendarChannel": [ 303, { diff --git a/packages/twenty-server/src/engine/core-modules/tool/tool.module.ts b/packages/twenty-server/src/engine/core-modules/tool/tool.module.ts index 333ae79821..6c458885d4 100644 --- a/packages/twenty-server/src/engine/core-modules/tool/tool.module.ts +++ b/packages/twenty-server/src/engine/core-modules/tool/tool.module.ts @@ -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, diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts new file mode 100644 index 0000000000..cc2e6c5146 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/__tests__/email-composer.service.spec.ts @@ -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 & { 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 => ({ + recipients: { to: 'recipient@example.com' }, + subject: 'Hello', + body: '

Hello

', + ...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); + }); + + 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); + } + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts index 29f3f90cd8..7c1c012b3a 100644 --- a/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts +++ b/packages/twenty-server/src/engine/core-modules/tool/tools/email-tool/email-composer.service.ts @@ -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, + private readonly connectedAccountMetadataService: ConnectedAccountMetadataService, @InjectWorkspaceScopedRepository(FileEntity) private readonly fileRepository: WorkspaceScopedRepository, 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 { - 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 { - 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, ); } diff --git a/packages/twenty-server/src/engine/metadata-modules/connected-account/__tests__/connected-account-metadata.service.spec.ts b/packages/twenty-server/src/engine/metadata-modules/connected-account/__tests__/connected-account-metadata.service.spec.ts new file mode 100644 index 0000000000..43df2b3611 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/connected-account/__tests__/connected-account-metadata.service.spec.ts @@ -0,0 +1,214 @@ +import { randomUUID } from 'node:crypto'; + +import { Test, type TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; + +import { AppOAuthRevokeService } from 'src/engine/core-modules/application/connection-provider/refresh/services/app-oauth-revoke.service'; +import { CalendarChannelEntity } from 'src/engine/metadata-modules/calendar-channel/entities/calendar-channel.entity'; +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 { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity'; + +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(); + +type FakeAccount = Partial & { id: string }; + +const aliceUserPrivateAccount: FakeAccount = { + id: ALICE_ACCOUNT_ID, + workspaceId: WORKSPACE_ID, + userWorkspaceId: ALICE_USER_WORKSPACE_ID, + visibility: 'user', +}; + +const bobUserPrivateAccount: FakeAccount = { + id: BOB_ACCOUNT_ID, + workspaceId: WORKSPACE_ID, + userWorkspaceId: BOB_USER_WORKSPACE_ID, + visibility: 'user', +}; + +const sharedWorkspaceAccount: FakeAccount = { + id: SHARED_ACCOUNT_ID, + workspaceId: WORKSPACE_ID, + userWorkspaceId: ALICE_USER_WORKSPACE_ID, + visibility: 'workspace', +}; + +const matchesWhere = ( + account: FakeAccount, + where: Record | Record[], +): boolean => { + const conditions = Array.isArray(where) ? where : [where]; + + return conditions.some((condition) => + Object.entries(condition).every( + ([key, value]) => account[key as keyof FakeAccount] === value, + ), + ); +}; + +describe('ConnectedAccountMetadataService - user-workspace visibility scoping', () => { + let service: ConnectedAccountMetadataService; + let accounts: FakeAccount[]; + + beforeEach(async () => { + accounts = [ + aliceUserPrivateAccount, + bobUserPrivateAccount, + sharedWorkspaceAccount, + ]; + + const mockConnectedAccountRepository = { + findOne: jest.fn(({ where }) => + Promise.resolve( + accounts.find((account) => matchesWhere(account, where)) ?? null, + ), + ), + find: jest.fn(({ where }) => + Promise.resolve( + accounts.filter((account) => matchesWhere(account, where)), + ), + ), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + ConnectedAccountMetadataService, + { + provide: getRepositoryToken(ConnectedAccountEntity), + useValue: mockConnectedAccountRepository, + }, + { + provide: getRepositoryToken(CalendarChannelEntity), + useValue: {}, + }, + { + provide: getRepositoryToken(MessageChannelEntity), + useValue: {}, + }, + { + provide: AppOAuthRevokeService, + useValue: {}, + }, + ], + }).compile(); + + service = module.get( + ConnectedAccountMetadataService, + ); + }); + + describe('findAccessibleConnectedAccountById', () => { + it("should not return another member's user-private account", async () => { + const result = await service.findAccessibleConnectedAccountById({ + id: ALICE_ACCOUNT_ID, + userWorkspaceId: BOB_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + expect(result).toBeNull(); + }); + + it('should return the caller own user-private account', async () => { + const result = await service.findAccessibleConnectedAccountById({ + id: BOB_ACCOUNT_ID, + userWorkspaceId: BOB_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + expect(result?.id).toBe(BOB_ACCOUNT_ID); + }); + + it('should return a workspace-visibility account for any member', async () => { + const result = await service.findAccessibleConnectedAccountById({ + id: SHARED_ACCOUNT_ID, + userWorkspaceId: BOB_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + expect(result?.id).toBe(SHARED_ACCOUNT_ID); + }); + + it('should reject a user-private account when no userWorkspaceId is present', async () => { + const result = await service.findAccessibleConnectedAccountById({ + id: ALICE_ACCOUNT_ID, + userWorkspaceId: undefined, + workspaceId: WORKSPACE_ID, + }); + + expect(result).toBeNull(); + }); + + it('should allow a workspace-visibility account when no userWorkspaceId is present', async () => { + const result = await service.findAccessibleConnectedAccountById({ + id: SHARED_ACCOUNT_ID, + userWorkspaceId: undefined, + workspaceId: WORKSPACE_ID, + }); + + expect(result?.id).toBe(SHARED_ACCOUNT_ID); + }); + }); + + describe('findAccessibleConnectedAccounts', () => { + it("should split the caller own accounts from workspace-shared ones, never exposing another member's private account", async () => { + const { userConnectedAccounts, workspaceSharedConnectedAccounts } = + await service.findAccessibleConnectedAccounts({ + userWorkspaceId: BOB_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + expect(userConnectedAccounts.map((account) => account.id)).toEqual([ + BOB_ACCOUNT_ID, + ]); + expect( + workspaceSharedConnectedAccounts.map((account) => account.id), + ).toEqual([SHARED_ACCOUNT_ID]); + }); + + it('should return only workspace-shared accounts when no userWorkspaceId is present', async () => { + const { userConnectedAccounts, workspaceSharedConnectedAccounts } = + await service.findAccessibleConnectedAccounts({ + userWorkspaceId: undefined, + workspaceId: WORKSPACE_ID, + }); + + expect(userConnectedAccounts).toEqual([]); + expect( + workspaceSharedConnectedAccounts.map((account) => account.id), + ).toEqual([SHARED_ACCOUNT_ID]); + }); + }); + + describe('findByUserWorkspaceId', () => { + it('should return only the caller own accounts, including their shared ones', async () => { + const result = await service.findByUserWorkspaceId({ + userWorkspaceId: ALICE_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + const ids = result.map((account) => account.id); + + expect(ids).toContain(ALICE_ACCOUNT_ID); + expect(ids).toContain(SHARED_ACCOUNT_ID); + expect(ids).not.toContain(BOB_ACCOUNT_ID); + }); + + it('should not return another member workspace-shared account', async () => { + const result = await service.findByUserWorkspaceId({ + userWorkspaceId: BOB_USER_WORKSPACE_ID, + workspaceId: WORKSPACE_ID, + }); + + const ids = result.map((account) => account.id); + + expect(ids).toEqual([BOB_ACCOUNT_ID]); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts index 1f19c81466..2ef3663322 100644 --- a/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/connected-account/connected-account-metadata.service.ts @@ -1,7 +1,13 @@ import { Injectable, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { + type FindOptionsRelations, + type FindOptionsWhere, + Repository, +} from 'typeorm'; + +import { isDefined } from 'twenty-shared/utils'; import { AppOAuthRevokeService } from 'src/engine/core-modules/application/connection-provider/refresh/services/app-oauth-revoke.service'; import { CalendarChannelEntity } from 'src/engine/metadata-modules/calendar-channel/entities/calendar-channel.entity'; @@ -34,7 +40,7 @@ export class ConnectedAccountMetadataService { workspaceId: string; }): Promise { return this.repository.find({ - where: { userWorkspaceId, workspaceId }, + where: this.getUserConditions({ userWorkspaceId, workspaceId }), }); } @@ -62,6 +68,105 @@ export class ConnectedAccountMetadataService { }); } + private getUserConditions({ + id, + userWorkspaceId, + workspaceId, + }: { + id?: string; + userWorkspaceId: string; + workspaceId: string; + }): FindOptionsWhere { + return { + ...(isDefined(id) ? { id } : {}), + workspaceId, + userWorkspaceId, + }; + } + + private getWorkspaceSharedConditions({ + id, + workspaceId, + }: { + id?: string; + workspaceId: string; + }): FindOptionsWhere { + return { + ...(isDefined(id) ? { id } : {}), + workspaceId, + visibility: 'workspace', + }; + } + + private getAccessibleConditions({ + id, + userWorkspaceId, + workspaceId, + }: { + id?: string; + userWorkspaceId: string | undefined; + workspaceId: string; + }): FindOptionsWhere[] { + const workspaceSharedConditions = this.getWorkspaceSharedConditions({ + id, + workspaceId, + }); + + if (!isDefined(userWorkspaceId)) { + return [workspaceSharedConditions]; + } + + return [ + this.getUserConditions({ id, userWorkspaceId, workspaceId }), + workspaceSharedConditions, + ]; + } + + async findAccessibleConnectedAccounts({ + userWorkspaceId, + workspaceId, + relations, + }: { + userWorkspaceId: string | undefined; + workspaceId: string; + relations?: FindOptionsRelations; + }): Promise<{ + userConnectedAccounts: ConnectedAccountEntity[]; + workspaceSharedConnectedAccounts: ConnectedAccountEntity[]; + }> { + const accounts = await this.repository.find({ + where: this.getAccessibleConditions({ workspaceId, userWorkspaceId }), + relations, + order: { createdAt: 'ASC' }, + }); + + return { + userConnectedAccounts: accounts.filter( + (account) => account.userWorkspaceId === userWorkspaceId, + ), + workspaceSharedConnectedAccounts: accounts.filter( + (account) => account.userWorkspaceId !== userWorkspaceId, + ), + }; + } + + async findAccessibleConnectedAccountById({ + id, + userWorkspaceId, + workspaceId, + relations, + }: { + id: string; + userWorkspaceId: string | undefined; + workspaceId: string; + relations?: FindOptionsRelations; + }): Promise { + return this.repository.findOne({ + where: this.getAccessibleConditions({ workspaceId, userWorkspaceId, id }), + relations, + }); + } + async verifyOwnership({ id, userWorkspaceId, @@ -103,7 +208,7 @@ export class ConnectedAccountMetadataService { workspaceId: string; }): Promise { const accounts = await this.repository.find({ - where: { userWorkspaceId, workspaceId }, + where: this.getUserConditions({ userWorkspaceId, workspaceId }), select: ['id'], }); @@ -116,7 +221,7 @@ export class ConnectedAccountMetadataService { workspaceId: string; }): Promise { const accounts = await this.repository.find({ - where: { workspaceId, visibility: 'workspace' }, + where: this.getWorkspaceSharedConditions({ workspaceId }), select: ['id'], }); diff --git a/packages/twenty-server/src/modules/messaging/message-outbound-manager/resolvers/send-email.resolver.ts b/packages/twenty-server/src/modules/messaging/message-outbound-manager/resolvers/send-email.resolver.ts index 19bd41d51d..4f76ff881c 100644 --- a/packages/twenty-server/src/modules/messaging/message-outbound-manager/resolvers/send-email.resolver.ts +++ b/packages/twenty-server/src/modules/messaging/message-outbound-manager/resolvers/send-email.resolver.ts @@ -69,7 +69,7 @@ export class SendEmailResolver { files: input.files ?? [], inReplyTo: input.inReplyTo, }, - { workspaceId: workspace.id }, + { workspaceId: workspace.id, userWorkspaceId }, { attachmentsFileFolder: FileFolder.EmailAttachment }, ); diff --git a/packages/twenty-server/test/integration/metadata/suites/connected-account/connected-account-resolver.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/connected-account/connected-account-resolver.integration-spec.ts index 8a56501677..5d9695fe4a 100644 --- a/packages/twenty-server/test/integration/metadata/suites/connected-account/connected-account-resolver.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/connected-account/connected-account-resolver.integration-spec.ts @@ -118,4 +118,37 @@ describe('connectedAccountResolver (e2e)', () => { expect(response.body.errors?.[0]?.extensions?.code).toBe('FORBIDDEN'); }); }); + + describe('sendEmail', () => { + it("should reject sending from another member's connected account", async () => { + const response = await makeMetadataAPIRequest({ + query: gql` + mutation SendEmail($input: SendEmailInput!) { + sendEmail(input: $input) { + success + error + } + } + `, + variables: { + input: { + connectedAccountId: CONNECTED_ACCOUNT_DATA_SEED_IDS.JONY, + to: 'recipient@example.com', + subject: 'Should never be sent', + body: '

Should never be sent

', + }, + }, + }); + + expect(response.status).toBe(200); + expect(response.body.errors).toBeUndefined(); + + const result = response.body.data.sendEmail; + + expect(result.success).toBe(false); + expect(result.error).toMatchInlineSnapshot( + `"Connected account 20202020-0cc8-4d60-a3a4-803245698908 does not belong to user workspace 20202020-1e7c-43d9-a5db-685b5069d816"`, + ); + }); + }); });