diff --git a/packages/nodes-base/nodes/Microsoft/Teams/MicrosoftTeamsTrigger.node.ts b/packages/nodes-base/nodes/Microsoft/Teams/MicrosoftTeamsTrigger.node.ts index 3cfa229f302..4a9f03e6eba 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/MicrosoftTeamsTrigger.node.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/MicrosoftTeamsTrigger.node.ts @@ -13,7 +13,12 @@ import type { import { NodeApiError, NodeConnectionTypes } from 'n8n-workflow'; import type { WebhookNotification, SubscriptionResponse } from './v2/helpers/types'; -import { createSubscription, getResourcePath } from './v2/helpers/utils-trigger'; +import { + createSubscription, + generateClientState, + getResourcePath, + verifyWebhook, +} from './v2/helpers/utils-trigger'; import { listSearch } from './v2/methods'; import { microsoftApiRequest, microsoftApiRequestAllItems } from './v2/transport'; @@ -319,13 +324,19 @@ export class MicrosoftTeamsTrigger implements INodeType { }); } + const clientState = generateClientState(); const resourcePaths = await getResourcePath.call(this, event); const subscriptionIds: string[] = []; if (Array.isArray(resourcePaths)) { await Promise.all( resourcePaths.map(async (resource) => { - const subscription = await createSubscription.call(this, webhookUrl, resource); + const subscription = await createSubscription.call( + this, + webhookUrl, + resource, + clientState, + ); subscriptionIds.push(subscription.id); return subscription; }), @@ -333,10 +344,16 @@ export class MicrosoftTeamsTrigger implements INodeType { webhookData.subscriptionIds = subscriptionIds; } else { - const subscription = await createSubscription.call(this, webhookUrl, resourcePaths); + const subscription = await createSubscription.call( + this, + webhookUrl, + resourcePaths, + clientState, + ); webhookData.subscriptionIds = [subscription.id]; } + webhookData.webhookSecret = clientState; return true; }, @@ -366,6 +383,7 @@ export class MicrosoftTeamsTrigger implements INodeType { ); delete webhookData.subscriptionIds; + delete webhookData.webhookSecret; return true; } catch (error) { return false; @@ -384,6 +402,11 @@ export class MicrosoftTeamsTrigger implements INodeType { return { noWebhookResponse: true }; } + if (!verifyWebhook.call(this)) { + res.status(401).send('Unauthorized').end(); + return { noWebhookResponse: true }; + } + const eventNotifications = req.body.value as WebhookNotification[]; const response: IWebhookResponseData = { workflowData: eventNotifications.map((event) => [ diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/MicrosoftTeamTrigger.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/MicrosoftTeamTrigger.test.ts index ac5b2ef6abc..1641918f421 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/MicrosoftTeamTrigger.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/MicrosoftTeamTrigger.test.ts @@ -113,10 +113,31 @@ describe('Microsoft Teams Trigger Node', () => { expirationDateTime: expect.any(String), latestSupportedTlsVersion: 'v1_2', lifecycleNotificationUrl: 'https://webhook.url', + clientState: expect.any(String), }), ); }); + it('should persist a clientState secret on the workflow static data', async () => { + (microsoftApiRequest.call as jest.Mock).mockResolvedValue({ id: 'subscription123' }); + + const staticData: { subscriptionIds?: string[]; webhookSecret?: string } = {}; + mockWebhookFunctions.getNodeWebhookUrl.mockReturnValue('https://webhook.url'); + mockWebhookFunctions.getNodeParameter.mockReturnValue('newChat'); + mockWebhookFunctions.getWorkflowStaticData.mockReturnValue(staticData); + + await new MicrosoftTeamsTrigger().webhookMethods.default.create.call(mockWebhookFunctions); + + expect(typeof staticData.webhookSecret).toBe('string'); + expect((staticData.webhookSecret as string).length).toBeGreaterThan(0); + + const requestBody = (microsoftApiRequest.call as jest.Mock).mock.calls[0][3] as Record< + string, + unknown + >; + expect(requestBody.clientState).toBe(staticData.webhookSecret); + }); + it('should throw an error if the URL is invalid', async () => { mockWebhookFunctions.getNodeWebhookUrl.mockReturnValue('invalid-url'); await expect( @@ -127,8 +148,12 @@ describe('Microsoft Teams Trigger Node', () => { describe('delete', () => { it('should delete subscriptions using stored IDs and clean static data', async () => { - const mockWebhookData = { + const mockWebhookData: { + subscriptionIds?: string[]; + webhookSecret?: string; + } = { subscriptionIds: ['subscription123'], + webhookSecret: 'stored-secret', }; mockWebhookFunctions.getWorkflowStaticData.mockReturnValue(mockWebhookData); @@ -145,6 +170,7 @@ describe('Microsoft Teams Trigger Node', () => { '/v1.0/subscriptions/subscription123', ); expect(mockWebhookData.subscriptionIds).toBeUndefined(); + expect(mockWebhookData.webhookSecret).toBeUndefined(); }); it('should return false if no subscription matches', async () => { @@ -217,6 +243,7 @@ describe('Microsoft Teams Trigger Node', () => { mockWebhookFunctions.getRequestObject.mockReturnValue(mockRequest); mockWebhookFunctions.getResponseObject.mockReturnValue(mockResponse); + mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({}); const result = await new MicrosoftTeamsTrigger().webhook.call(mockWebhookFunctions); @@ -228,5 +255,91 @@ describe('Microsoft Teams Trigger Node', () => { ], ]); }); + + it('should process notifications when stored secret matches clientState', async () => { + const mockRequest = { + body: { + value: [ + { + clientState: 'expected-secret', + resourceData: { message: 'test message' }, + }, + ], + }, + query: {}, + }; + const mockResponse = { + status: jest.fn().mockReturnThis(), + send: jest.fn(), + end: jest.fn(), + }; + + mockWebhookFunctions.getRequestObject.mockReturnValue(mockRequest); + mockWebhookFunctions.getResponseObject.mockReturnValue(mockResponse); + mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = await new MicrosoftTeamsTrigger().webhook.call(mockWebhookFunctions); + + expect(mockResponse.status).not.toHaveBeenCalledWith(401); + expect(result.workflowData).toEqual([ + [ + { + json: { message: 'test message' }, + }, + ], + ]); + }); + + it('should return 401 when clientState does not match stored secret', async () => { + const mockRequest = { + body: { + value: [{ clientState: 'wrong-secret-aa' }], + }, + query: {}, + }; + const mockResponse = { + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + }; + + mockWebhookFunctions.getRequestObject.mockReturnValue(mockRequest); + mockWebhookFunctions.getResponseObject.mockReturnValue(mockResponse); + mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = await new MicrosoftTeamsTrigger().webhook.call(mockWebhookFunctions); + + expect(mockResponse.status).toHaveBeenCalledWith(401); + expect(mockResponse.send).toHaveBeenCalledWith('Unauthorized'); + expect(result.noWebhookResponse).toBe(true); + expect(result.workflowData).toBeUndefined(); + }); + + it('should process notifications when no secret is stored (backward compatibility)', async () => { + const mockRequest = { + body: { + value: [{ clientState: 'anything', resourceData: { id: '1' } }], + }, + query: {}, + }; + const mockResponse = { + status: jest.fn().mockReturnThis(), + send: jest.fn(), + end: jest.fn(), + }; + + mockWebhookFunctions.getRequestObject.mockReturnValue(mockRequest); + mockWebhookFunctions.getResponseObject.mockReturnValue(mockResponse); + mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({}); + + const result = await new MicrosoftTeamsTrigger().webhook.call(mockWebhookFunctions); + + expect(mockResponse.status).not.toHaveBeenCalledWith(401); + expect(result.workflowData).toEqual([[{ json: { id: '1' } }]]); + }); }); }); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/utiles-trigger.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/utiles-trigger.test.ts index 8ab5a8ce06e..1bec7dd0dda 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/utiles-trigger.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/trigger/utiles-trigger.test.ts @@ -1,11 +1,14 @@ import { mock } from 'jest-mock-extended'; +import type { IWebhookFunctions } from 'n8n-workflow'; import { NodeApiError } from 'n8n-workflow'; import { fetchAllTeams, fetchAllChannels, createSubscription, + generateClientState, getResourcePath, + verifyWebhook, } from '../../v2/helpers/utils-trigger'; import { microsoftApiRequest } from '../../v2/transport'; @@ -112,6 +115,46 @@ describe('Microsoft Teams Helpers Functions', () => { }); }); + it('should include clientState in the subscription body when provided', async () => { + (microsoftApiRequest.call as jest.Mock).mockResolvedValue({ + id: 'subscription123', + resource: '/resource/path', + notificationUrl: 'https://webhook.url', + expirationDateTime: '2024-01-01T00:00:00Z', + }); + + await createSubscription.call( + mockHookFunctions, + 'https://webhook.url', + '/resource/path', + 'my-secret-state', + ); + + expect(microsoftApiRequest.call).toHaveBeenCalledWith( + mockHookFunctions, + 'POST', + '/v1.0/subscriptions', + expect.objectContaining({ clientState: 'my-secret-state' }), + ); + }); + + it('should omit clientState from the subscription body when not provided', async () => { + (microsoftApiRequest.call as jest.Mock).mockResolvedValue({ + id: 'subscription123', + resource: '/resource/path', + notificationUrl: 'https://webhook.url', + expirationDateTime: '2024-01-01T00:00:00Z', + }); + + await createSubscription.call(mockHookFunctions, 'https://webhook.url', '/resource/path'); + + const requestBody = (microsoftApiRequest.call as jest.Mock).mock.calls[0][3] as Record< + string, + unknown + >; + expect(requestBody.clientState).toBeUndefined(); + }); + it('should throw a NodeApiError if the API request fails', async () => { const error = new NodeApiError(mockHookFunctions.getNode(), { message: 'API request failed', @@ -125,6 +168,129 @@ describe('Microsoft Teams Helpers Functions', () => { }); }); + describe('generateClientState', () => { + it('should generate a 64-character hex string', () => { + const state = generateClientState(); + expect(state).toHaveLength(64); + expect(/^[0-9a-f]+$/.test(state)).toBe(true); + }); + + it('should generate unique values', () => { + const a = generateClientState(); + const b = generateClientState(); + expect(a).not.toBe(b); + }); + }); + + describe('verifyWebhook', () => { + let mockWebhookFunctions: Partial; + + beforeEach(() => { + mockWebhookFunctions = { + getRequestObject: jest.fn(), + getWorkflowStaticData: jest.fn(), + }; + }); + + it('should return true when no secret is stored (backward compatibility)', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { value: [{ clientState: 'anything' }] }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({}); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(true); + }); + + it('should return true when clientState matches the stored secret', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { + value: [{ clientState: 'expected-secret' }], + }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(true); + }); + + it('should return false when clientState does not match the stored secret', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { value: [{ clientState: 'wrong-secret-aa' }] }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(false); + }); + + it('should return false when clientState is missing from the notification', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { value: [{}] }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(false); + }); + + it('should return true when all notifications in a batch have matching clientState', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { + value: [ + { clientState: 'expected-secret' }, + { clientState: 'expected-secret' }, + { clientState: 'expected-secret' }, + ], + }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(true); + }); + + it('should return false when any notification in a batch has mismatched clientState', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { + value: [ + { clientState: 'expected-secret' }, + { clientState: 'wrong-secret' }, + { clientState: 'expected-secret' }, + ], + }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(false); + }); + + it('should return false when any notification in a batch is missing clientState', () => { + (mockWebhookFunctions.getRequestObject as jest.Mock).mockReturnValue({ + body: { + value: [{ clientState: 'expected-secret' }, {}, { clientState: 'expected-secret' }], + }, + }); + (mockWebhookFunctions.getWorkflowStaticData as jest.Mock).mockReturnValue({ + webhookSecret: 'expected-secret', + }); + + const result = verifyWebhook.call(mockWebhookFunctions as IWebhookFunctions); + expect(result).toBe(false); + }); + }); + describe('getResourcePath', () => { it('should return the correct resource path for newChat event', async () => { const result = await getResourcePath.call(mockHookFunctions, 'newChat'); diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/types.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/types.ts index 8322c17f7c1..74414cfa742 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/types.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/types.ts @@ -14,6 +14,7 @@ export interface WebhookNotification { resourceData: ResourceData; tenantId: string; subscriptionExpirationDateTime: string; + clientState?: string; } export interface ResourceData { diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils-trigger.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils-trigger.ts index 39b09f0733f..141dc0159c4 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils-trigger.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/helpers/utils-trigger.ts @@ -1,9 +1,15 @@ -import type { IHookFunctions, IDataObject } from 'n8n-workflow'; +import { randomBytes } from 'crypto'; +import type { IHookFunctions, IDataObject, IWebhookFunctions } from 'n8n-workflow'; import { NodeOperationError } from 'n8n-workflow'; import type { TeamResponse, ChannelResponse, SubscriptionResponse } from './types'; +import { verifySignature as verifySignatureGeneric } from '../../../../../utils/webhook-signature-verification'; import { microsoftApiRequest } from '../transport'; +export function generateClientState(): string { + return randomBytes(32).toString('hex'); +} + export async function fetchAllTeams(this: IHookFunctions): Promise { const { value: teams } = (await microsoftApiRequest.call( this, @@ -29,6 +35,7 @@ export async function createSubscription( this: IHookFunctions, webhookUrl: string, resourcePath: string, + clientState?: string, ): Promise { const expirationTime = new Date(Date.now() + 4318 * 60 * 1000).toISOString(); const body: IDataObject = { @@ -40,6 +47,10 @@ export async function createSubscription( lifecycleNotificationUrl: webhookUrl, }; + if (clientState) { + body.clientState = clientState; + } + const response = (await microsoftApiRequest.call( this, 'POST', @@ -50,6 +61,32 @@ export async function createSubscription( return response; } +export function verifyWebhook(this: IWebhookFunctions): boolean { + const req = this.getRequestObject(); + const webhookData = this.getWorkflowStaticData('node'); + const expectedSecret = webhookData.webhookSecret; + + const body = req.body as { value?: unknown } | undefined; + const notifications = body?.value; + + if (!Array.isArray(notifications) || notifications.length === 0) { + return false; + } + + return (notifications as unknown[]).every((notification) => { + const actualClientState = + typeof notification === 'object' && notification !== null + ? (notification as { clientState?: unknown }).clientState + : undefined; + return verifySignatureGeneric({ + getExpectedSignature: () => + typeof expectedSecret === 'string' && expectedSecret.length > 0 ? expectedSecret : null, + skipIfNoExpectedSignature: true, + getActualSignature: () => (typeof actualClientState === 'string' ? actualClientState : null), + }); + }); +} + export async function getResourcePath( this: IHookFunctions, event: string,