From 4eddcc43d4259cdbf0ba69f43137c03cf7cda40a Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 23 Sep 2026 10:38:21 +0000 Subject: [PATCH] refactor(Microsoft Teams Node): Centralize the Service Principal guard and share the chat member binding (no-changelog) (#38928) --- .../test/v2/node/chatMember/add.body.test.ts | 23 ++++ .../test/v2/node/servicePrincipal.test.ts | 105 ++++-------------- .../servicePrincipalStrippedOperation.test.ts | 2 +- .../Teams/v2/actions/chat/create.operation.ts | 17 +-- .../Teams/v2/actions/chat/get.operation.ts | 4 - .../Teams/v2/actions/chat/getAll.operation.ts | 4 - .../Teams/v2/actions/chat/sharedGuard.ts | 24 ---- .../v2/actions/chatMember/add.operation.ts | 16 ++- .../v2/actions/chatMember/getAll.operation.ts | 4 - .../v2/actions/chatMember/remove.operation.ts | 4 - .../v2/actions/chatMember/sharedGuard.ts | 25 ----- .../actions/chatMessage/create.operation.ts | 4 - .../v2/actions/chatMessage/get.operation.ts | 5 - .../actions/chatMessage/getAll.operation.ts | 4 - .../v2/actions/chatMessage/messageActions.ts | 3 - .../chatMessage/sendAndWait.operation.ts | 5 - .../v2/actions/chatMessage/sharedGuard.ts | 24 ---- .../Microsoft/Teams/v2/actions/router.ts | 49 +++++--- .../nodes/Microsoft/Teams/v2/helpers/utils.ts | 10 ++ .../utils/microsoft/test/transport.test.ts | 8 ++ .../nodes-base/utils/microsoft/transport.ts | 7 ++ 21 files changed, 124 insertions(+), 223 deletions(-) delete mode 100644 packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/sharedGuard.ts delete mode 100644 packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/sharedGuard.ts delete mode 100644 packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sharedGuard.ts diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMember/add.body.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMember/add.body.test.ts index eb23f1162f3..85438103e0e 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMember/add.body.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMember/add.body.test.ts @@ -149,6 +149,29 @@ describe('Microsoft Teams V2 - chatMember:add request body', () => { expect(apiRequest).not.toHaveBeenCalled(); }); + // Teams deep links carry the UPN percent-encoded. The validator decodes it, so the bind + // carries the plain UPN in the slash form the add-member docs show. + it('decodes a URL-copied UPN, so the bind carries the plain UPN', async () => { + setParams(addParams({ userId: 'jacob%40contoso.com', options: {} })); + + await node.execute.call(ctx); + + expect(apiRequest).toHaveBeenCalledWith('POST', '/v1.0/chats/19:abc@thread.v2/members', { + '@odata.type': '#microsoft.graph.aadUserConversationMember', + 'user@odata.bind': 'https://graph.microsoft.com/v1.0/users/jacob@contoso.com', + roles: ['owner'], + }); + }); + + // A lone surrogate is never a valid Graph id; the validator rejects it with the node's own + // error instead of letting it reach the request. + it('rejects a lone surrogate in the user id before any request', async () => { + setParams(addParams({ userId: `${userId}\uD800`, options: {} })); + + await expect(node.execute.call(ctx)).rejects.toThrow('The ID is not valid'); + expect(apiRequest).not.toHaveBeenCalled(); + }); + it('uses the credential graphApiBaseUrl in user@odata.bind (sovereign cloud)', async () => { ctx.getCredentials.mockResolvedValue({ graphApiBaseUrl: 'https://graph.microsoft.us' }); setParams(addParams({ options: {} })); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipal.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipal.test.ts index 68761c9fbe7..53feda771dd 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipal.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipal.test.ts @@ -26,7 +26,7 @@ vi.mock('../../../v2/transport', async () => { }; }); -describe('Microsoft Teams V2 — Service Principal runtime guards', () => { +describe('Microsoft Teams V2, Service Principal runtime guards', () => { let node: MicrosoftTeamsV2; let ctx: MockProxy; @@ -58,71 +58,31 @@ describe('Microsoft Teams V2 — Service Principal runtime guards', () => { ); }; + // One case per resource: the router throws before it reads `operation`, so every operation + // of a hidden resource takes the same path. The `description` is pinned too, so a swapped + // entry in the router's guard map fails here. it.each([ - ['create', 'create'], - ['get', 'get'], - ['getAll', 'getAll'], - ])('chatMessage:%s throws a static error and issues no request under SP', async (_label, op) => { - selectSp({ - resource: 'chatMessage', - operation: op, - chatId: 'chatID', - returnAll: true, - // a mention row, so moving the guard below resolveMentions fires a GET /v1.0/users/... - 'mentions.mention': [ - { userId: { __rl: true, mode: 'id', value: '714c1202-cbac-40ff-9160-53ab5c4df9b8' } }, - ], - }); - - await expect(node.execute.call(ctx)).rejects.toThrow( + [ + 'chat', + 'Chats are not available with the Service Principal credential', + 'App-only Microsoft Graph has no signed-in user to read or create chats for. Use an OAuth2 credential for chat actions.', + ], + [ + 'chatMember', + 'Chat members are not available with the Service Principal credential', + 'The chat picker cannot list chats app-only. Use an OAuth2 credential for chat actions.', + ], + [ + 'chatMessage', 'Chat messages are not available with the Service Principal credential', - ); - expect(transport.microsoftApiRequest).not.toHaveBeenCalled(); - expect(transport.microsoftApiRequestAllItems).not.toHaveBeenCalled(); - }); + 'App-only Microsoft Graph has no signed-in user. Use an OAuth2 credential for chat actions.', + ], + ])( + '%s throws a static error and issues no request under SP', + async (resource, message, description) => { + selectSp({ resource, operation: 'create' }); - it.each(['create', 'get', 'getAll'])( - 'chat:%s throws a static error and issues no request under SP', - async (operation) => { - selectSp({ - resource: 'chat', - operation, - chatId: 'chatID', - returnAll: true, - chatType: 'oneOnOne', - // a participant row, so moving the guard below the row loop fires a GET /v1.0/users/... - 'members.member': [ - { userId: { __rl: true, mode: 'id', value: '714c1202-cbac-40ff-9160-53ab5c4df9b8' } }, - ], - 'members.member[0].userId': '714c1202-cbac-40ff-9160-53ab5c4df9b8', - }); - - await expect(node.execute.call(ctx)).rejects.toThrow( - 'Chats are not available with the Service Principal credential', - ); - expect(transport.microsoftApiRequest).not.toHaveBeenCalled(); - expect(transport.microsoftApiRequestAllItems).not.toHaveBeenCalled(); - // the guard reads only the `authentication` parameter, never the credential itself - expect(ctx.getCredentials).not.toHaveBeenCalled(); - }, - ); - - it.each(['add', 'getAll', 'remove'])( - 'chatMember:%s throws a static error and issues no request under SP', - async (operation) => { - selectSp({ - resource: 'chatMember', - operation, - chatId: 'chatID', - userId: 'e76f456f-5c3f-4f1e-9d5e-4d8f0f6ab111', - membershipId: 'MCMjMiMj', - returnAll: true, - options: {}, - }); - - await expect(node.execute.call(ctx)).rejects.toThrow( - 'Chat members are not available with the Service Principal credential', - ); + await expect(node.execute.call(ctx)).rejects.toMatchObject({ message, description }); expect(transport.microsoftApiRequest).not.toHaveBeenCalled(); expect(transport.microsoftApiRequestAllItems).not.toHaveBeenCalled(); // the guard reads only the `authentication` parameter, never the credential itself @@ -436,25 +396,6 @@ describe('Microsoft Teams V2 — Service Principal runtime guards', () => { }); }); - describe('chatMessage delete actions under SP', () => { - it.each(['softDeleteMessage', 'undoSoftDeleteMessage'])( - 'chatMessage:%s throws a static error and issues no request under SP', - async (operation) => { - selectSp({ - resource: 'chatMessage', - operation, - chatId: 'chatID', - messageId: '1698378560692', - }); - - await expect(node.execute.call(ctx)).rejects.toThrow( - 'Chat messages are not available with the Service Principal credential', - ); - expect(transport.microsoftApiRequest).not.toHaveBeenCalled(); - }, - ); - }); - describe('channelMessage delete actions under SP', () => { it.each(['softDeleteMessage', 'undoSoftDeleteMessage'])( 'channelMessage:%s throws a static error and issues no request under SP', diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipalStrippedOperation.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipalStrippedOperation.test.ts index 9e76eee3605..b664acd5650 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipalStrippedOperation.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/servicePrincipalStrippedOperation.test.ts @@ -20,7 +20,7 @@ vi.mock('../../../v2/transport', async () => { }; }); -const SP_UNSUPPORTED = /is not available with the Service Principal credential|are not available/; +const SP_UNSUPPORTED = /(is|are) not available with the Service Principal credential/; /** * `Workflow` drops a hidden parameter before execution, so a parameter behind `SP_HIDE` is diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/create.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/create.operation.ts index 75b53394936..4bec1276275 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/create.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/create.operation.ts @@ -8,10 +8,13 @@ import { import { escapeODataValue } from '@utils/query-escaping'; import { updateDisplayOptions } from '@utils/utilities'; -import { throwIfChatUnsupported } from './sharedGuard'; import { stampItemIndexOnError, validateUserTargetId } from '../../../../GenericFunctions'; import { userRLC } from '../../descriptions'; -import { resolveUserTarget, userTargetMessages } from '../../helpers/utils'; +import { + aadUserConversationMember, + resolveUserTarget, + userTargetMessages, +} from '../../helpers/utils'; import { getGraphBaseUrl, microsoftApiRequest, SP_HIDE } from '../../transport'; const properties: INodeProperties[] = [ @@ -116,9 +119,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-post?view=graph-rest-1.0 - // App-only Graph has no signed-in user to create a chat for; fail before any request. - throwIfChatUnsupported.call(this); - const node = this.getNode(); const chatType = this.getNodeParameter('chatType', i) as 'oneOnOne' | 'group'; // The `''` fallback is required: `Workflow` strips hidden parameters before execution, @@ -239,11 +239,12 @@ export async function execute(this: IExecuteFunctions, i: number) { const baseUrl = await getGraphBaseUrl.call(this); const toMember = (member: { id: string; role: string; tenantId?: string }) => ({ - '@odata.type': '#microsoft.graph.aadUserConversationMember', - roles: [member.role], // Two escaping layers hold this bind: percent-encode the id for the URL (a B2B guest UPN // truncates at its `#` otherwise), then double any quote for the OData literal. - 'user@odata.bind': `${baseUrl}/v1.0/users('${escapeODataValue(encodeURIComponent(member.id))}')`, + ...aadUserConversationMember( + `${baseUrl}/v1.0/users('${escapeODataValue(encodeURIComponent(member.id))}')`, + member.role, + ), ...(member.tenantId ? { tenantId: member.tenantId } : {}), }); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/get.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/get.operation.ts index 1a0643e27e5..b6249d740a2 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/get.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/get.operation.ts @@ -2,7 +2,6 @@ import type { INodeProperties, IExecuteFunctions } from 'n8n-workflow'; import { updateDisplayOptions } from '@utils/utilities'; -import { throwIfChatUnsupported } from './sharedGuard'; import { chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequest, rewriteNotFound, SP_HIDE } from '../../transport'; @@ -23,9 +22,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-get?view=graph-rest-1.0 - // App-only Graph cannot read chats; fail before any request. - throwIfChatUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; // Kept outside the try for clarity: it is a validation step, not a request. const endpoint = buildTeamsPath.call(this, ['/v1.0/chats/', { id: chatId }]); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/getAll.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/getAll.operation.ts index aa35bf32047..50c964a1ff1 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/getAll.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/getAll.operation.ts @@ -3,7 +3,6 @@ import type { INodeProperties, IExecuteFunctions } from 'n8n-workflow'; import { returnAllOrLimit } from '@utils/descriptions'; import { updateDisplayOptions } from '@utils/utilities'; -import { throwIfChatUnsupported } from './sharedGuard'; import { microsoftApiRequestAllItems, SP_HIDE } from '../../transport'; /** Graph caps `$top` on `GET /chats` at 50 and 400s above it. */ @@ -26,9 +25,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-list?view=graph-rest-1.0 - // App-only Graph cannot list chats; fail before any request. - throwIfChatUnsupported.call(this); - const returnAll = this.getNodeParameter('returnAll', i); // Clamp low on `limit`, not on `$top`: a 0 limit is falsy for the early return in // `microsoftApiRequestAllItems`, so `$top: 1` alone would still walk every page. diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/sharedGuard.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/sharedGuard.ts deleted file mode 100644 index e2755d4a0f7..00000000000 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chat/sharedGuard.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { IExecuteFunctions } from 'n8n-workflow'; -import { NodeOperationError } from 'n8n-workflow'; - -import { getTeamsCredentialType, SERVICE_PRINCIPAL_AUTH } from '../../transport'; - -/** - * Throws a static `NodeOperationError` when the node is configured with the - * app-only (Service Principal) credential. `GET /chats` and `POST /chats` are - * delegated-only (app-only must go through `GET /users/{id}/chats` and has no - * signed-in user to add to a new chat), so every chat operation guards on this - * BEFORE any request - covering hand-edited workflows that bypass the hidden UI. - */ -export function throwIfChatUnsupported(this: IExecuteFunctions): void { - if (getTeamsCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH) { - throw new NodeOperationError( - this.getNode(), - 'Chats are not available with the Service Principal credential', - { - description: - 'App-only Microsoft Graph has no signed-in user to read or create chats for. Use an OAuth2 credential for chat actions.', - }, - ); - } -} diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/add.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/add.operation.ts index 64a3772e8b5..e442df03cd2 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/add.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/add.operation.ts @@ -9,6 +9,7 @@ import { import { updateDisplayOptions } from '@utils/utilities'; import { chatRLC, userRLC } from '../../descriptions'; +import { aadUserConversationMember } from '../../helpers/utils'; import { buildTeamsPath, getGraphBaseUrl, @@ -16,7 +17,6 @@ import { SP_HIDE, validateTeamsId, } from '../../transport'; -import { throwIfChatMemberUnsupported } from './sharedGuard'; const properties: INodeProperties[] = [ { @@ -104,9 +104,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-post-members?view=graph-rest-1.0 - // The chat picker cannot list chats app-only; fail before any request. - throwIfChatMemberUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; // Direct validator call rather than buildTeamsPath: this id is interpolated into // the body, and RLC `validation` is UI-only, so an expression can still supply @@ -117,11 +114,12 @@ export async function execute(this: IExecuteFunctions, i: number) { ); const options = this.getNodeParameter('options', i, {}); - const body: IDataObject = { - '@odata.type': '#microsoft.graph.aadUserConversationMember', - 'user@odata.bind': `${await getGraphBaseUrl.call(this)}/v1.0/users/${userId}`, - roles: [options.role === 'guest' ? 'guest' : 'owner'], - }; + // Slash key form, as the add-member docs show. Raw is safe: the validator above rejects + // `/ \ ? # %`, so the id cannot break out of the path. + const body: IDataObject = aadUserConversationMember( + `${await getGraphBaseUrl.call(this)}/v1.0/users/${userId}`, + options.role === 'guest' ? 'guest' : 'owner', + ); // Not inverted: omitting the field shares NO history, and the 0001-01-01 sentinel // shares the WHOLE history. diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/getAll.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/getAll.operation.ts index 8571acbd9e9..1425e1057d0 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/getAll.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/getAll.operation.ts @@ -5,7 +5,6 @@ import { updateDisplayOptions } from '@utils/utilities'; import { chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequestAllItems, SP_HIDE } from '../../transport'; -import { throwIfChatMemberUnsupported } from './sharedGuard'; const properties: INodeProperties[] = [chatRLC, ...returnAllOrLimit]; @@ -24,9 +23,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-list-members?view=graph-rest-1.0 - // The chat picker cannot list chats app-only; fail before any request. - throwIfChatMemberUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const endpoint = buildTeamsPath.call(this, ['/v1.0/chats/', { id: chatId }, '/members']); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/remove.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/remove.operation.ts index ff0905e9cfc..228cabdc552 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/remove.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/remove.operation.ts @@ -7,7 +7,6 @@ import { import { updateDisplayOptions } from '@utils/utilities'; -import { throwIfChatMemberUnsupported } from './sharedGuard'; import { chatMemberRLC, chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequest, SP_HIDE } from '../../transport'; @@ -28,9 +27,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://learn.microsoft.com/en-us/graph/api/chat-delete-members?view=graph-rest-1.0 - // The chat picker cannot list chats app-only; fail before any request. - throwIfChatMemberUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const membershipId = this.getNodeParameter('membershipId', i, '', { extractValue: true, diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/sharedGuard.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/sharedGuard.ts deleted file mode 100644 index 149222d5fd3..00000000000 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMember/sharedGuard.ts +++ /dev/null @@ -1,25 +0,0 @@ -import type { IExecuteFunctions } from 'n8n-workflow'; -import { NodeOperationError } from 'n8n-workflow'; - -import { getTeamsCredentialType, SERVICE_PRINCIPAL_AUTH } from '../../transport'; - -/** - * Throws a static `NodeOperationError` when the node is configured with the - * app-only (Service Principal) credential. The endpoints themselves have - * application permissions, but the chat picker does not: `GET /chats` is - * delegated-only (app-only must go through `GET /users/{id}/chats`), so every - * chatMember operation guards on this BEFORE any request - covering - * hand-edited workflows that bypass the hidden UI. - */ -export function throwIfChatMemberUnsupported(this: IExecuteFunctions): void { - if (getTeamsCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH) { - throw new NodeOperationError( - this.getNode(), - 'Chat members are not available with the Service Principal credential', - { - description: - 'The chat picker cannot list chats app-only. Use an OAuth2 credential for chat actions.', - }, - ); - } -} diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/create.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/create.operation.ts index 45f09aecc9b..07d41c8b4b0 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/create.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/create.operation.ts @@ -10,7 +10,6 @@ import { } from '../../descriptions'; import { prepareMessage, resolveMentions } from '../../helpers/utils'; import { buildTeamsPath, microsoftApiRequest, SP_HIDE } from '../../transport'; -import { throwIfChatMessageUnsupported } from './sharedGuard'; const properties: INodeProperties[] = [ chatRLC, @@ -70,9 +69,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number, instanceId: string) { // https://docs.microsoft.com/en-us/graph/api/channel-post-messages?view=graph-rest-1.0&tabs=http - // App-only Graph cannot post chat messages; fail before any request. - throwIfChatMessageUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const contentType = this.getNodeParameter('contentType', i) as string; const message = this.getNodeParameter('message', i) as string; diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/get.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/get.operation.ts index ea511d40551..c674b43d4e2 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/get.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/get.operation.ts @@ -4,7 +4,6 @@ import { updateDisplayOptions } from '@utils/utilities'; import { chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequest, SP_HIDE } from '../../transport'; -import { throwIfChatMessageUnsupported } from './sharedGuard'; const properties: INodeProperties[] = [ chatRLC, @@ -34,10 +33,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://docs.microsoft.com/en-us/graph/api/chat-list-messages?view=graph-rest-1.0&tabs=http - // App-only Graph cannot read chats; fail before the request (and before the - // catch below, so the static SP message is surfaced, not the generic one). - throwIfChatMessageUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const messageId = this.getNodeParameter('messageId', i) as string; // Built outside the try so an invalid id surfaces as its own validation diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/getAll.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/getAll.operation.ts index e208f9957c7..38fe6cda906 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/getAll.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/getAll.operation.ts @@ -5,7 +5,6 @@ import { updateDisplayOptions } from '@utils/utilities'; import { chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequestAllItems, SP_HIDE } from '../../transport'; -import { throwIfChatMessageUnsupported } from './sharedGuard'; const properties: INodeProperties[] = [chatRLC, ...returnAllOrLimit]; @@ -24,9 +23,6 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number) { // https://docs.microsoft.com/en-us/graph/api/chat-list-messages?view=graph-rest-1.0&tabs=http - // App-only Graph cannot read chats; fail before any request. - throwIfChatMessageUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const returnAll = this.getNodeParameter('returnAll', i); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/messageActions.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/messageActions.ts index 13d6dbc1435..c34694f3837 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/messageActions.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/messageActions.ts @@ -1,7 +1,6 @@ import type { IExecuteFunctions, INodeProperties } from 'n8n-workflow'; import { NodeOperationError } from 'n8n-workflow'; -import { throwIfChatMessageUnsupported } from './sharedGuard'; import { chatRLC } from '../../descriptions'; import { readTextParameter } from '../../helpers/parameters'; import { buildTeamsPath, microsoftApiRequest, SP_HIDE } from '../../transport'; @@ -78,8 +77,6 @@ export async function runMessageAction( i: number, action: ChatMessageAction, ) { - throwIfChatMessageUnsupported.call(this); - const chatId = readTextParameter.call(this, 'chatId', i, true); const messageId = readTextParameter.call(this, 'messageId', i); const messagePath = buildTeamsPath.call(this, [ diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts index f250799beea..ef362220b57 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts @@ -7,7 +7,6 @@ import { import { createUtmCampaignLink } from '../../../../../../utils/utilities'; import { chatRLC } from '../../descriptions'; import { buildTeamsPath, microsoftApiRequest, SP_HIDE } from '../../transport'; -import { throwIfChatMessageUnsupported } from './sharedGuard'; export const description: INodeProperties[] = getSendAndWaitProperties( [chatRLC], @@ -32,10 +31,6 @@ export const description: INodeProperties[] = getSendAndWaitProperties( })); export async function execute(this: IExecuteFunctions, i: number, instanceId: string) { - // App-only Graph cannot post chat messages. Dispatched from the router before the - // item loop, so this guard fires before any putExecutionToWait. - throwIfChatMessageUnsupported.call(this); - const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const config = getSendAndWaitConfig(this); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sharedGuard.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sharedGuard.ts deleted file mode 100644 index c9e92fb1fea..00000000000 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sharedGuard.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { IExecuteFunctions } from 'n8n-workflow'; -import { NodeOperationError } from 'n8n-workflow'; - -import { getTeamsCredentialType, SERVICE_PRINCIPAL_AUTH } from '../../transport'; - -/** - * Throws a static `NodeOperationError` when the node is configured with the - * app-only (Service Principal) credential. Chat messages have no usable app-only - * form (app-only Graph has no signed-in user), so every chatMessage operation - * guards on this BEFORE any request — covering hand-edited workflows that bypass - * the hidden UI. For `sendAndWait` it fires before any `putExecutionToWait`. - */ -export function throwIfChatMessageUnsupported(this: IExecuteFunctions): void { - if (getTeamsCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH) { - throw new NodeOperationError( - this.getNode(), - 'Chat messages are not available with the Service Principal credential', - { - description: - 'App-only Microsoft Graph has no signed-in user. Use an OAuth2 credential for chat actions.', - }, - ); - } -} diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/router.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/router.ts index 595a33a5ca5..90dd2d4d800 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/router.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/router.ts @@ -11,33 +11,51 @@ import * as activityNotification from './activityNotification'; import * as channel from './channel'; import * as channelMessage from './channelMessage'; import * as chat from './chat'; -import { throwIfChatUnsupported } from './chat/sharedGuard'; import * as chatMember from './chatMember'; -import { throwIfChatMemberUnsupported } from './chatMember/sharedGuard'; import * as chatMessage from './chatMessage'; -import { throwIfChatMessageUnsupported } from './chatMessage/sharedGuard'; import type { MicrosoftTeamsType } from './node.type'; import * as onlineMeeting from './onlineMeeting'; import * as task from './task'; import { configureWaitTillDate } from '../../../../../utils/sendAndWait/configureWaitTillDate.util'; import { stampItemIndexOnError } from '../../../GenericFunctions'; +import { getTeamsCredentialType, SERVICE_PRINCIPAL_AUTH } from '../transport'; /** * The resources whose operation selector is hidden under the Service Principal credential. * * `Workflow` drops a hidden parameter before execution, so `operation` is absent for these - * and the read below fails with `Could not get parameter "operation"` before any operation - * can run its own guard. Guard on the resource first, so the user gets the real reason. - * Each entry is the operation-level guard itself, so the message has one source. + * and the read below fails with `Could not get parameter "operation"`. Guard on the resource + * first, so the user gets the real reason. Each entry holds the error for one resource. * * A `Map`, not an object: `resource` is a stored parameter, and a plain object walks - * `Object.prototype`, so a resource of `__proto__` returns a non-nullish value and fails - * with a raw `TypeError` instead of the node's own unsupported-operation error. + * `Object.prototype`, so a resource of `__proto__` would match the guard, and the error + * constructor would fail with a raw `TypeError` on the `undefined` message. */ -const SERVICE_PRINCIPAL_RESOURCE_GUARDS = new Map void>([ - ['chat', throwIfChatUnsupported], - ['chatMember', throwIfChatMemberUnsupported], - ['chatMessage', throwIfChatMessageUnsupported], +const SERVICE_PRINCIPAL_RESOURCE_GUARDS = new Map([ + [ + 'chat', + { + message: 'Chats are not available with the Service Principal credential', + description: + 'App-only Microsoft Graph has no signed-in user to read or create chats for. Use an OAuth2 credential for chat actions.', + }, + ], + [ + 'chatMember', + { + message: 'Chat members are not available with the Service Principal credential', + description: + 'The chat picker cannot list chats app-only. Use an OAuth2 credential for chat actions.', + }, + ], + [ + 'chatMessage', + { + message: 'Chat messages are not available with the Service Principal credential', + description: + 'App-only Microsoft Graph has no signed-in user. Use an OAuth2 credential for chat actions.', + }, + ], ]); export async function router(this: IExecuteFunctions): Promise { @@ -46,7 +64,12 @@ export async function router(this: IExecuteFunctions): Promise('resource', 0); - SERVICE_PRINCIPAL_RESOURCE_GUARDS.get(String(resource))?.call(this); + const unsupported = SERVICE_PRINCIPAL_RESOURCE_GUARDS.get(resource); + if (unsupported && getTeamsCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH) { + throw new NodeOperationError(this.getNode(), unsupported.message, { + description: unsupported.description, + }); + } const operation = this.getNodeParameter('operation', 0); const nodeVersion = this.getNode().typeVersion; diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils.ts index db91aea7d1b..721e79216e0 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils.ts @@ -272,6 +272,16 @@ export async function resolveUserTarget( } } +/** + * Builds one `aadUserConversationMember` entry. `userRef` is the full `user@odata.bind` URL: + * the two endpoints document different key forms, so each call site composes its own. + */ +export const aadUserConversationMember = (userRef: string, role: string) => ({ + '@odata.type': '#microsoft.graph.aadUserConversationMember', + roles: [role], + 'user@odata.bind': userRef, +}); + /** * Resolves every mention row to a Graph user or team tag. Graph stores `mentions[].mentioned` * verbatim and resolves nothing: a UPN, a well-formed but nonexistent GUID or a bogus tag ID is diff --git a/packages/nodes-base/utils/microsoft/test/transport.test.ts b/packages/nodes-base/utils/microsoft/test/transport.test.ts index d99df287a3e..5903dbb7c52 100644 --- a/packages/nodes-base/utils/microsoft/test/transport.test.ts +++ b/packages/nodes-base/utils/microsoft/test/transport.test.ts @@ -535,6 +535,14 @@ describe('Microsoft Graph transport kernel', () => { expect(() => validateMicrosoftGraphId(id, mockNode)).toThrow(); }); + it.each(['\uD800', 'e76f456f\uDC00'])('rejects an id with a lone surrogate (%j)', (id) => { + expect(() => validateMicrosoftGraphId(id, mockNode)).toThrow('The ID is not valid'); + }); + + it('accepts a well-formed astral character (a surrogate pair is not a lone surrogate)', () => { + expect(validateMicrosoftGraphId('team-\u{1F600}', mockNode)).toBe('team-\u{1F600}'); + }); + it.each([ 'a/b', 'a\\b', diff --git a/packages/nodes-base/utils/microsoft/transport.ts b/packages/nodes-base/utils/microsoft/transport.ts index 3a882cc4f3e..6297b39b654 100644 --- a/packages/nodes-base/utils/microsoft/transport.ts +++ b/packages/nodes-base/utils/microsoft/transport.ts @@ -96,6 +96,13 @@ export function validateMicrosoftGraphId(id: string, node: INode): string { description: 'The ID contains a malformed percent-encoding. Copy it again and try again.', }); } + // A lone surrogate is not valid text. `decodeURIComponent` passes it through, and the + // `encodeURIComponent` on a body value (`@odata.bind`) then throws a raw `URIError`. + if (/\p{Surrogate}/u.test(value)) { + throw new NodeOperationError(node, 'The ID is not valid', { + description: 'The ID contains an invalid character. Copy it again and try again.', + }); + } if (/^\.+$/.test(value)) { throw new NodeOperationError(node, 'The ID is not valid', { description: 'An ID cannot consist only of dots.',