diff --git a/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts b/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts index 88781e300b0..8eb8ee9c256 100644 --- a/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts +++ b/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts @@ -3,7 +3,6 @@ import { mockInstance } from '@n8n/backend-test-utils'; import { type CredentialsEntity, type User } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; -import axios from 'axios'; import type { Response } from 'express'; import { OAuth1CredentialController } from '@/controllers/oauth/oauth1-credential.controller'; import { EventService } from '@/events/event.service'; @@ -11,8 +10,6 @@ import type { OAuthRequest } from '@/requests'; import { OauthService } from '@/oauth/oauth.service'; import { ExternalHooks } from '@/external-hooks'; -jest.mock('axios'); - describe('OAuth1CredentialController', () => { const oauthService = mockInstance(OauthService); const eventService = mockInstance(EventService); @@ -25,6 +22,8 @@ describe('OAuth1CredentialController', () => { const timestamp = 1706750625678; jest.useFakeTimers({ advanceTimers: true }); + const accessTokenData = { oauth_token: 'token', oauth_token_secret: 'secret' }; + beforeEach(() => { jest.setSystemTime(new Date(timestamp)); jest.clearAllMocks(); @@ -88,7 +87,36 @@ describe('OAuth1CredentialController', () => { ); }); - it('should exchange the code for a valid token, and save it to DB for static credential', async () => { + it('should sign the access token request with the stored request token secret', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'static-credential' as const, + createdAt: timestamp, + data: 'encrypted-data', + }; + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid', oauth_token_secret: 'request-token-secret' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); + + await controller.handleCallback(req, res); + + expect(oauthService.getOAuth1AccessToken).toHaveBeenCalledWith( + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + { + oauthToken: 'token', + oauthVerifier: 'verifier', + oauthTokenSecret: 'request-token-secret', + }, + ); + }); + + it('should exchange the verifier for a valid token, and save it to DB for static credential', async () => { const mockResolvedCredential = mock({ id: '1' }); const mockState = { token: 'token', @@ -97,26 +125,16 @@ describe('OAuth1CredentialController', () => { createdAt: timestamp, data: 'encrypted-data', }; - oauthService.getCredentialForUpdate.mockResolvedValueOnce(mockResolvedCredential); - // @ts-ignore - oauthService.getDecryptedData.mockResolvedValue({ csrfSecret: 'invalid' }); - oauthService.getOAuthCredentials.mockResolvedValueOnce({ - requestTokenUrl: 'https://example.domain/oauth/request_token', - accessTokenUrl: 'https://example.domain/oauth/access_token', - signatureMethod: 'HMAC-SHA1', - }); oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'invalid' }, { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); await controller.handleCallback(req, res); - // @ts-ignore + expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith( mockResolvedCredential, expect.objectContaining({ @@ -125,7 +143,7 @@ describe('OAuth1CredentialController', () => { oauth_token_secret: 'secret', }), }), - ['csrfSecret'], + ['csrfSecret', 'oauth_token_secret'], ); expect(res.render).toHaveBeenCalledWith('oauth-callback'); }); @@ -156,9 +174,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); oauthService.saveDynamicCredential.mockResolvedValueOnce(undefined); await controller.handleCallback(dynamicReq, res); @@ -211,9 +227,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); oauthService.saveDynamicCredential.mockResolvedValueOnce(undefined); await controller.handleCallback(dynamicReq, res); @@ -250,9 +264,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); await controller.handleCallback(dynamicReq, res); @@ -288,9 +300,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); await controller.handleCallback(dynamicReq, res); @@ -327,9 +337,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); await controller.handleCallback(dynamicReq, res); @@ -364,9 +372,7 @@ describe('OAuth1CredentialController', () => { { accessTokenUrl: 'https://example.domain/oauth/access_token' }, mockState, ]); - jest - .mocked(axios.post) - .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.getOAuth1AccessToken.mockResolvedValueOnce(accessTokenData); await controller.handleCallback(undefinedOriginReq, res); @@ -378,7 +384,7 @@ describe('OAuth1CredentialController', () => { oauth_token_secret: 'secret', }), }), - ['csrfSecret'], + ['csrfSecret', 'oauth_token_secret'], ); expect(res.render).toHaveBeenCalledWith('oauth-callback'); }); diff --git a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts index 65116458eb5..d56e7551960 100644 --- a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts @@ -1,6 +1,5 @@ import { Logger } from '@n8n/backend-common'; import { Get, RestController } from '@n8n/decorators'; -import axios from 'axios'; import { Response } from 'express'; import { ensureError, jsonStringify } from 'n8n-workflow'; @@ -46,24 +45,25 @@ export class OAuth1CredentialController { ); } - const [credential, _, oauthCredentials, state] = + const [credential, decryptedData, oauthCredentials, state] = await this.oauthService.resolveCredential(req); - // Form URL encoded body https://datatracker.ietf.org/doc/html/rfc5849#section-3.5.2 - const oauthToken = await axios.post( - oauthCredentials.accessTokenUrl, - { oauth_token, oauth_verifier }, - { headers: { 'content-type': 'application/x-www-form-urlencoded' } }, - ); + const oauthTokenSecret = + typeof decryptedData.oauth_token_secret === 'string' + ? decryptedData.oauth_token_secret + : ''; - // Response comes as x-www-form-urlencoded string so convert it to JSON - - const paramParser = new URLSearchParams(oauthToken.data); - - const oauthTokenData = Object.fromEntries(paramParser.entries()); + const oauthTokenData = await this.oauthService.getOAuth1AccessToken(oauthCredentials, { + oauthToken: oauth_token, + oauthVerifier: oauth_verifier, + oauthTokenSecret, + }); if (!state.origin || state.origin === 'static-credential') { - await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']); + await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, [ + 'csrfSecret', + 'oauth_token_secret', + ]); this.logger.debug('OAuth1 callback successful for new credential', { credentialId: credential.id, diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index 04e88219378..b4b4d04dac7 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -3276,7 +3276,10 @@ describe('OauthService', () => { expect(authUri).toContain('https://example.domain/oauth/authorize?oauth_token=random-token'); expect(service.encryptAndSaveData).toHaveBeenCalledWith( credential, - expect.objectContaining({ csrfSecret: expect.any(String) }), + expect.objectContaining({ + csrfSecret: expect.any(String), + oauth_token_secret: 'random-secret', + }), [], ); expect(externalHooks.run).toHaveBeenCalledWith('oauth1.authenticate', expect.any(Array)); @@ -3355,6 +3358,92 @@ describe('OauthService', () => { }), ).rejects.toThrow('Request token failed'); }); + + it('should preserve pre-existing query params on the authorization URL', async () => { + const axios = require('axios'); + const credential = mock({ id: '1', type: 'trelloOAuth1Api' }); + const oauthCredentials: OAuth1CredentialData = { + consumerKey: 'consumer_key', + consumerSecret: 'consumer_secret', + requestTokenUrl: 'https://trello.com/1/OAuthGetRequestToken', + authUrl: + 'https://trello.com/1/OAuthAuthorizeToken?scope=read,write,account&expiration=never&name=n8n', + accessTokenUrl: 'https://trello.com/1/OAuthGetAccessToken', + signatureMethod: 'HMAC-SHA1', + }; + + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue(oauthCredentials); + jest.mocked(axios.request).mockResolvedValue({ + data: 'oauth_token=random-token&oauth_token_secret=random-secret', + }); + jest.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined); + + const authUri = await service.generateAOauth1AuthUri(credential, { + cid: credential.id, + origin: 'static-credential', + userId: 'user-id', + }); + + const parsed = new URL(authUri); + expect(parsed.searchParams.get('scope')).toBe('read,write,account'); + expect(parsed.searchParams.get('expiration')).toBe('never'); + expect(parsed.searchParams.get('name')).toBe('n8n'); + expect(parsed.searchParams.get('oauth_token')).toBe('random-token'); + }); + }); + + describe('getOAuth1AccessToken', () => { + const oauthCredentials: OAuth1CredentialData = { + consumerKey: 'consumer_key', + consumerSecret: 'consumer_secret', + requestTokenUrl: 'https://trello.com/1/OAuthGetRequestToken', + authUrl: 'https://trello.com/1/OAuthAuthorizeToken', + accessTokenUrl: 'https://trello.com/1/OAuthGetAccessToken', + signatureMethod: 'HMAC-SHA1', + }; + + it('should send a signed request to the access token endpoint and parse the response', async () => { + const axios = require('axios'); + jest.mocked(axios.request).mockResolvedValue({ + data: 'oauth_token=access-token&oauth_token_secret=access-secret', + }); + + const result = await service.getOAuth1AccessToken(oauthCredentials, { + oauthToken: 'request-token', + oauthVerifier: 'verifier', + oauthTokenSecret: 'request-secret', + }); + + expect(result).toEqual({ + oauth_token: 'access-token', + oauth_token_secret: 'access-secret', + }); + + const requestConfig = jest.mocked(axios.request).mock.calls.at(-1)?.[0]; + expect(requestConfig.method).toBe('POST'); + expect(requestConfig.url).toBe('https://trello.com/1/OAuthGetAccessToken'); + // The request must carry an OAuth1 signature and the request token in the + // Authorization header. + expect(requestConfig.headers.Authorization).toMatch(/^OAuth /); + expect(requestConfig.headers.Authorization).toContain('oauth_signature'); + expect(requestConfig.headers.Authorization).toContain('oauth_token'); + // The verifier travels in the form-encoded body. + expect(requestConfig.headers['content-type']).toBe('application/x-www-form-urlencoded'); + expect(requestConfig.data).toBe('oauth_verifier=verifier'); + }); + + it('should throw when the access token endpoint returns a non-string response', async () => { + const axios = require('axios'); + jest.mocked(axios.request).mockResolvedValue({ data: { not: 'a string' } }); + + await expect( + service.getOAuth1AccessToken(oauthCredentials, { + oauthToken: 'request-token', + oauthVerifier: 'verifier', + oauthTokenSecret: 'request-secret', + }), + ).rejects.toThrow(BadRequestError); + }); }); describe('extractAccountIdentifier', () => { diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index 4e7adca1d9c..08db38a7f1d 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -838,9 +838,17 @@ export class OauthService { ); } - const returnUri = `${oauthCredentials.authUrl}?oauth_token=${responseJson.oauth_token}`; + const returnUriUrl = new URL(oauthCredentials.authUrl); + returnUriUrl.searchParams.set('oauth_token', responseJson.oauth_token); + const returnUri = returnUriUrl.toString(); - await this.encryptAndSaveData(credential, { csrfSecret }, []); + // The request token secret is required to sign the later access token + // request, so it must be persisted until the callback completes. + await this.encryptAndSaveData( + credential, + { csrfSecret, oauth_token_secret: responseJson.oauth_token_secret ?? '' }, + [], + ); this.logger.debug('OAuth1 authorization url created for credential', { csrfData, @@ -850,6 +858,63 @@ export class OauthService { return returnUri; } + /** + * Exchanges an authorized OAuth1 request token for an access token. + * + * The access token request must be signed with the consumer credentials and + * the request token key/secret obtained during {@link generateAOauth1AuthUri}. + * Returns the parsed token data from the (x-www-form-urlencoded) response. + */ + async getOAuth1AccessToken( + oauthCredentials: OAuth1CredentialData, + params: { oauthToken: string; oauthVerifier: string; oauthTokenSecret: string }, + ): Promise> { + const { signatureMethod } = oauthCredentials; + + const oauth = new clientOAuth1({ + consumer: { + key: oauthCredentials.consumerKey, + secret: oauthCredentials.consumerSecret, + }, + signature_method: signatureMethod, + hash_function(base, key) { + const algorithm = algorithmMap[signatureMethod] ?? 'sha1'; + return createHmac(algorithm, key).update(base).digest('base64'); + }, + }); + + const requestData: RequestOptions = { + method: 'POST', + url: oauthCredentials.accessTokenUrl, + data: { oauth_verifier: params.oauthVerifier }, + }; + + const token = { key: params.oauthToken, secret: params.oauthTokenSecret }; + const headers = oauth.toHeader(oauth.authorize(requestData, token)); + + // `oauth_verifier` is part of the signature base string but is not emitted + // into the Authorization header by `toHeader`, so it must travel in the + // form-encoded body for the server to receive and verify it. + const { data: response } = await axios.request({ + method: 'POST', + url: oauthCredentials.accessTokenUrl, + data: new URLSearchParams({ oauth_verifier: params.oauthVerifier }).toString(), + headers: { + ...headers, + 'content-type': 'application/x-www-form-urlencoded', + }, + }); + + // Response comes as x-www-form-urlencoded string so convert it to JSON + if (typeof response !== 'string') { + throw new BadRequestError( + 'Expected string response from OAuth1 access token endpoint, but received invalid response type', + ); + } + + return Object.fromEntries(new URLSearchParams(response).entries()); + } + private convertCredentialToOptions(credential: OAuth2CredentialData): ClientOAuth2Options { const options: ClientOAuth2Options = { clientId: credential.clientId, diff --git a/packages/nodes-base/credentials/TrelloOAuth1Api.credentials.ts b/packages/nodes-base/credentials/TrelloOAuth1Api.credentials.ts new file mode 100644 index 00000000000..6d04e898a77 --- /dev/null +++ b/packages/nodes-base/credentials/TrelloOAuth1Api.credentials.ts @@ -0,0 +1,39 @@ +import type { ICredentialType, INodeProperties } from 'n8n-workflow'; + +export class TrelloOAuth1Api implements ICredentialType { + name = 'trelloOAuth1Api'; + + extends = ['oAuth1Api']; + + displayName = 'Trello OAuth1 API'; + + documentationUrl = 'trello'; + + properties: INodeProperties[] = [ + { + displayName: 'Request Token URL', + name: 'requestTokenUrl', + type: 'hidden', + default: 'https://trello.com/1/OAuthGetRequestToken', + }, + { + displayName: 'Authorization URL', + name: 'authUrl', + type: 'hidden', + default: + 'https://trello.com/1/OAuthAuthorizeToken?scope=read,write,account&expiration=never&name=n8n', + }, + { + displayName: 'Access Token URL', + name: 'accessTokenUrl', + type: 'hidden', + default: 'https://trello.com/1/OAuthGetAccessToken', + }, + { + displayName: 'Signature Method', + name: 'signatureMethod', + type: 'hidden', + default: 'HMAC-SHA1', + }, + ]; +} diff --git a/packages/nodes-base/credentials/test/TrelloOAuth1Api.credentials.test.ts b/packages/nodes-base/credentials/test/TrelloOAuth1Api.credentials.test.ts new file mode 100644 index 00000000000..c6464124e95 --- /dev/null +++ b/packages/nodes-base/credentials/test/TrelloOAuth1Api.credentials.test.ts @@ -0,0 +1,28 @@ +import { TrelloOAuth1Api } from '../TrelloOAuth1Api.credentials'; + +describe('TrelloOAuth1Api credentials', () => { + const credential = new TrelloOAuth1Api(); + + const getDefault = (name: string) => credential.properties.find((p) => p.name === name)?.default; + + it('extends the generic oAuth1Api credential', () => { + expect(credential.name).toBe('trelloOAuth1Api'); + expect(credential.extends).toEqual(['oAuth1Api']); + }); + + it('uses Trello OAuth1 endpoints', () => { + expect(getDefault('requestTokenUrl')).toBe('https://trello.com/1/OAuthGetRequestToken'); + expect(getDefault('accessTokenUrl')).toBe('https://trello.com/1/OAuthGetAccessToken'); + expect(getDefault('signatureMethod')).toBe('HMAC-SHA1'); + }); + + it('embeds required Trello query params on the authorization URL', () => { + const authUrl = getDefault('authUrl') as string; + expect(authUrl.startsWith('https://trello.com/1/OAuthAuthorizeToken')).toBe(true); + + const params = new URL(authUrl).searchParams; + expect(params.get('scope')).toBe('read,write,account'); + expect(params.get('expiration')).toBe('never'); + expect(params.get('name')).toBe('n8n'); + }); +}); diff --git a/packages/nodes-base/nodes/Trello/GenericFunctions.ts b/packages/nodes-base/nodes/Trello/GenericFunctions.ts index 91f90be4b88..fbef62e68d8 100644 --- a/packages/nodes-base/nodes/Trello/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Trello/GenericFunctions.ts @@ -32,6 +32,14 @@ export async function apiRequest( delete options.body; } + const authentication = this.getNodeParameter('authentication', 0, 'apiKey') as + | 'apiKey' + | 'oAuth1'; + + if (authentication === 'oAuth1') { + return await this.helpers.requestOAuth1.call(this, 'trelloOAuth1Api', options); + } + return await this.helpers.requestWithAuthentication.call(this, 'trelloApi', options); } diff --git a/packages/nodes-base/nodes/Trello/Trello.node.ts b/packages/nodes-base/nodes/Trello/Trello.node.ts index 4a596522b55..63fd7cba103 100644 --- a/packages/nodes-base/nodes/Trello/Trello.node.ts +++ b/packages/nodes-base/nodes/Trello/Trello.node.ts @@ -46,9 +46,25 @@ export class Trello implements INodeType { { name: 'trelloApi', required: true, + displayOptions: { show: { authentication: ['apiKey'] } }, + }, + { + name: 'trelloOAuth1Api', + required: true, + displayOptions: { show: { authentication: ['oAuth1'] } }, }, ], properties: [ + { + displayName: 'Authentication', + name: 'authentication', + type: 'options', + options: [ + { name: 'API Key', value: 'apiKey' }, + { name: 'OAuth1', value: 'oAuth1' }, + ], + default: 'apiKey', + }, { displayName: 'Resource', name: 'resource', diff --git a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts index 6f34979afb3..9ea9aa11611 100644 --- a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts +++ b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts @@ -27,6 +27,12 @@ export class TrelloTrigger implements INodeType { { name: 'trelloApi', required: true, + displayOptions: { show: { authentication: ['apiKey'] } }, + }, + { + name: 'trelloOAuth1Api', + required: true, + displayOptions: { show: { authentication: ['oAuth1'] } }, }, ], webhooks: [ @@ -44,6 +50,16 @@ export class TrelloTrigger implements INodeType { }, ], properties: [ + { + displayName: 'Authentication', + name: 'authentication', + type: 'options', + options: [ + { name: 'API Key', value: 'apiKey' }, + { name: 'OAuth1', value: 'oAuth1' }, + ], + default: 'apiKey', + }, { displayName: 'Model ID', name: 'id', @@ -59,6 +75,43 @@ export class TrelloTrigger implements INodeType { webhookMethods = { default: { async checkExists(this: IHookFunctions): Promise { + const authentication = this.getNodeParameter('authentication', 'apiKey') as + | 'apiKey' + | 'oAuth1'; + const idModel = this.getNodeParameter('id') as string; + const webhookUrl = this.getNodeWebhookUrl('default'); + const webhookData = this.getWorkflowStaticData('node'); + + if (authentication === 'oAuth1') { + // OAuth1 has no apiToken to list webhooks against; rely on the stored + // webhookId from the previous activation. + if (!webhookData.webhookId) { + return false; + } + try { + const webhook = await apiRequest.call( + this, + 'GET', + `webhooks/${webhookData.webhookId}`, + {}, + ); + + if (webhook.idModel === idModel && webhook.callbackURL === webhookUrl) { + return true; + } + + // The stored webhook no longer matches the current configuration, so + // remove the stale registration and report it as missing to trigger + // a fresh creation. + await apiRequest.call(this, 'DELETE', `webhooks/${webhookData.webhookId}`, {}); + delete webhookData.webhookId; + return false; + } catch (error) { + delete webhookData.webhookId; + return false; + } + } + const credentials = await this.getCredentials('trelloApi'); // Check all the webhooks which exist already if it is identical to the @@ -67,13 +120,9 @@ export class TrelloTrigger implements INodeType { const responseData = await apiRequest.call(this, 'GET', endpoint, {}); - const idModel = this.getNodeParameter('id') as string; - const webhookUrl = this.getNodeWebhookUrl('default'); - for (const webhook of responseData) { if (webhook.idModel === idModel && webhook.callbackURL === webhookUrl) { // Set webhook-id to be sure that it can be deleted - const webhookData = this.getWorkflowStaticData('node'); webhookData.webhookId = webhook.id as string; return true; } @@ -82,20 +131,26 @@ export class TrelloTrigger implements INodeType { return false; }, async create(this: IHookFunctions): Promise { + const authentication = this.getNodeParameter('authentication', 'apiKey') as + | 'apiKey' + | 'oAuth1'; const webhookUrl = this.getNodeWebhookUrl('default'); - - const credentials = await this.getCredentials('trelloApi'); - const idModel = this.getNodeParameter('id') as string; - const endpoint = `tokens/${credentials.apiToken}/webhooks`; - const body = { description: `n8n Webhook - ${idModel}`, callbackURL: webhookUrl, idModel, }; + let endpoint: string; + if (authentication === 'oAuth1') { + endpoint = 'webhooks'; + } else { + const credentials = await this.getCredentials('trelloApi'); + endpoint = `tokens/${credentials.apiToken}/webhooks`; + } + const responseData = await apiRequest.call(this, 'POST', endpoint, body); if (responseData.id === undefined) { @@ -112,14 +167,20 @@ export class TrelloTrigger implements INodeType { const webhookData = this.getWorkflowStaticData('node'); if (webhookData.webhookId !== undefined) { - const credentials = await this.getCredentials('trelloApi'); + const authentication = this.getNodeParameter('authentication', 'apiKey') as + | 'apiKey' + | 'oAuth1'; - const endpoint = `tokens/${credentials.apiToken}/webhooks/${webhookData.webhookId}`; - - const body = {}; + let endpoint: string; + if (authentication === 'oAuth1') { + endpoint = `webhooks/${webhookData.webhookId}`; + } else { + const credentials = await this.getCredentials('trelloApi'); + endpoint = `tokens/${credentials.apiToken}/webhooks/${webhookData.webhookId}`; + } try { - await apiRequest.call(this, 'DELETE', endpoint, body); + await apiRequest.call(this, 'DELETE', endpoint, {}); } catch (error) { return false; } diff --git a/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts b/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts index b3fccaa37c4..4d920a1595b 100644 --- a/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts +++ b/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts @@ -4,9 +4,16 @@ import type { IWebhookFunctions } from 'n8n-workflow'; import { verifySignature as verifySignatureGeneric } from '../../utils/webhook-signature-verification'; export async function verifySignature(this: IWebhookFunctions): Promise { - const credential = await this.getCredentials('trelloApi'); + const authentication = this.getNodeParameter('authentication', 'apiKey') as 'apiKey' | 'oAuth1'; + const credential = + authentication === 'oAuth1' + ? await this.getCredentials('trelloOAuth1Api') + : await this.getCredentials('trelloApi'); const req = this.getRequestObject(); - const secret = credential.oauthSecret; + // Trello signs webhooks with the consumer (application) secret. For the + // OAuth1 credential that is `consumerSecret`; for the API-key credential the + // user pastes the same value into `oauthSecret`. + const secret = authentication === 'oAuth1' ? credential.consumerSecret : credential.oauthSecret; return verifySignatureGeneric({ getExpectedSignature: () => { diff --git a/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts b/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts index 1aac008c84e..0d9589f4b0a 100644 --- a/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts +++ b/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts @@ -4,9 +4,15 @@ jest.mock('../TrelloTriggerHelpers', () => ({ verifySignature: jest.fn(), })); +jest.mock('../GenericFunctions', () => ({ + apiRequest: jest.fn(), +})); + +import { apiRequest } from '../GenericFunctions'; import { verifySignature } from '../TrelloTriggerHelpers'; const mockedVerifySignature = jest.mocked(verifySignature); +const mockedApiRequest = jest.mocked(apiRequest); describe('TrelloTrigger', () => { let trelloTrigger: TrelloTrigger; @@ -73,4 +79,86 @@ describe('TrelloTrigger', () => { expect(result).toHaveProperty('workflowData'); }); }); + + describe('webhookMethods.checkExists (OAuth1)', () => { + const idModel = '4d5ea62fd76aa1136000000c'; + const webhookUrl = 'https://n8n.example.com/webhook/trello'; + + let mockHookFunctions: any; + let staticData: { webhookId?: string }; + + beforeEach(() => { + staticData = { webhookId: 'existing-webhook-id' }; + + mockHookFunctions = { + getNodeParameter: jest.fn((name: string) => { + if (name === 'authentication') return 'oAuth1'; + if (name === 'id') return idModel; + return undefined; + }), + getNodeWebhookUrl: jest.fn().mockReturnValue(webhookUrl), + getWorkflowStaticData: jest.fn().mockReturnValue(staticData), + }; + }); + + it('should return true when the stored webhook matches idModel and callbackURL', async () => { + mockedApiRequest.mockResolvedValue({ + id: 'existing-webhook-id', + idModel, + callbackURL: webhookUrl, + }); + + const result = await trelloTrigger.webhookMethods.default.checkExists.call(mockHookFunctions); + + expect(result).toBe(true); + expect(mockedApiRequest).toHaveBeenCalledTimes(1); + expect(mockedApiRequest).toHaveBeenCalledWith('GET', 'webhooks/existing-webhook-id', {}); + expect(staticData.webhookId).toBe('existing-webhook-id'); + }); + + it('should delete the stale webhook and return false when config no longer matches', async () => { + mockedApiRequest + .mockResolvedValueOnce({ + id: 'existing-webhook-id', + idModel: 'a-different-model', + callbackURL: 'https://old.example.com/webhook/trello', + }) + .mockResolvedValueOnce(undefined); + + const result = await trelloTrigger.webhookMethods.default.checkExists.call(mockHookFunctions); + + expect(result).toBe(false); + expect(mockedApiRequest).toHaveBeenNthCalledWith( + 1, + 'GET', + 'webhooks/existing-webhook-id', + {}, + ); + expect(mockedApiRequest).toHaveBeenNthCalledWith( + 2, + 'DELETE', + 'webhooks/existing-webhook-id', + {}, + ); + expect(staticData.webhookId).toBeUndefined(); + }); + + it('should return false and clear the stored webhookId when the lookup throws', async () => { + mockedApiRequest.mockRejectedValue(new Error('404 Not Found')); + + const result = await trelloTrigger.webhookMethods.default.checkExists.call(mockHookFunctions); + + expect(result).toBe(false); + expect(staticData.webhookId).toBeUndefined(); + }); + + it('should return false without calling the API when no webhookId is stored', async () => { + delete staticData.webhookId; + + const result = await trelloTrigger.webhookMethods.default.checkExists.call(mockHookFunctions); + + expect(result).toBe(false); + expect(mockedApiRequest).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts b/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts index ec6a814276d..e8496f70e59 100644 --- a/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts +++ b/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts @@ -15,6 +15,7 @@ describe('TrelloTriggerHelpers', () => { jest.clearAllMocks(); mockWebhookFunctions = { + getNodeParameter: jest.fn().mockReturnValue('apiKey'), getCredentials: jest.fn().mockResolvedValue({ oauthSecret: testSecret, }), @@ -110,5 +111,31 @@ describe('TrelloTriggerHelpers', () => { expect(result).toBe(false); }); + + describe('OAuth1 authentication', () => { + beforeEach(() => { + mockWebhookFunctions.getNodeParameter.mockReturnValue('oAuth1'); + mockWebhookFunctions.getCredentials.mockResolvedValue({ + consumerSecret: testSecret, + }); + }); + + it('should read the OAuth1 credential and accept a valid signature', async () => { + const result = await verifySignature.call(mockWebhookFunctions); + + expect(mockWebhookFunctions.getCredentials).toHaveBeenCalledWith('trelloOAuth1Api'); + expect(result).toBe(true); + }); + + it('should reject when consumer secret does not match', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + consumerSecret: 'wrong-secret', + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + }); }); }); diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 087421b03e3..e7f88a524e0 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -380,6 +380,7 @@ "dist/credentials/TravisCiApi.credentials.js", "dist/credentials/TrellixEpoApi.credentials.js", "dist/credentials/TrelloApi.credentials.js", + "dist/credentials/TrelloOAuth1Api.credentials.js", "dist/credentials/TwakeCloudApi.credentials.js", "dist/credentials/TwakeServerApi.credentials.js", "dist/credentials/TwilioApi.credentials.js",