mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-30 08:17:06 +02:00
feat(Microsoft Teams Trigger Node): Add webhook request verification (#29490)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
853ad40e08
commit
8ef505dc82
|
|
@ -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) => [
|
||||
|
|
|
|||
|
|
@ -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' } }]]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<IWebhookFunctions>;
|
||||
|
||||
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');
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ export interface WebhookNotification {
|
|||
resourceData: ResourceData;
|
||||
tenantId: string;
|
||||
subscriptionExpirationDateTime: string;
|
||||
clientState?: string;
|
||||
}
|
||||
|
||||
export interface ResourceData {
|
||||
|
|
|
|||
|
|
@ -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<TeamResponse[]> {
|
||||
const { value: teams } = (await microsoftApiRequest.call(
|
||||
this,
|
||||
|
|
@ -29,6 +35,7 @@ export async function createSubscription(
|
|||
this: IHookFunctions,
|
||||
webhookUrl: string,
|
||||
resourcePath: string,
|
||||
clientState?: string,
|
||||
): Promise<SubscriptionResponse> {
|
||||
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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user