diff --git a/packages/@n8n/client-oauth2/src/client-assertion.ts b/packages/@n8n/client-oauth2/src/client-assertion.ts index 4e6dabb1d13..176d877c2e7 100644 --- a/packages/@n8n/client-oauth2/src/client-assertion.ts +++ b/packages/@n8n/client-oauth2/src/client-assertion.ts @@ -1,5 +1,11 @@ import { formatPemBlock } from '@n8n/utils/format-pem-block'; -import { createPrivateKey, createSign, randomUUID, X509Certificate } from 'node:crypto'; +import { + createPrivateKey, + createSign, + randomUUID, + X509Certificate, + type KeyObject, +} from 'node:crypto'; // private_key_jwt (RFC 7521/7523): the client proves its identity with a JWT // signed by its private key instead of a shared secret. The `x5t` header (SHA-1 @@ -13,8 +19,16 @@ function base64url(input: Buffer | string): string { } function certificateThumbprint(certificate: string): string { - const fingerprint = new X509Certificate(formatPemBlock(certificate)).fingerprint; - return Buffer.from(fingerprint.replace(/:/g, ''), 'hex').toString('base64url'); + let parsed: X509Certificate; + try { + parsed = new X509Certificate(formatPemBlock(certificate)); + } catch (error) { + throw new Error( + 'The Certificate field must contain a PEM certificate (-----BEGIN CERTIFICATE-----).', + { cause: error }, + ); + } + return Buffer.from(parsed.fingerprint.replace(/:/g, ''), 'hex').toString('base64url'); } export interface BuildClientAssertionOptions { @@ -39,9 +53,18 @@ export function buildClientAssertion(options: BuildClientAssertionOptions): stri exp: now + ASSERTION_TTL_SECONDS, }; + let privateKey: KeyObject; + try { + privateKey = createPrivateKey(formatPemBlock(options.privateKey)); + } catch (error) { + throw new Error( + 'The Private Key field must contain a PEM private key (-----BEGIN PRIVATE KEY-----).', + { cause: error }, + ); + } + // `createSign('RSA-SHA256')` also signs EC/Ed25519 keys, producing a signature // that contradicts the pinned `alg: RS256` header. Reject non-RSA keys up front. - const privateKey = createPrivateKey(formatPemBlock(options.privateKey)); if (privateKey.asymmetricKeyType !== 'rsa') { throw new Error('Certificate authentication requires an RSA private key'); } diff --git a/packages/@n8n/client-oauth2/src/credential-options.ts b/packages/@n8n/client-oauth2/src/credential-options.ts new file mode 100644 index 00000000000..35dbc479cf0 --- /dev/null +++ b/packages/@n8n/client-oauth2/src/credential-options.ts @@ -0,0 +1,40 @@ +import type { ClientCertificate, OAuth2ClientCredentialType } from './types'; + +interface ClientAuthCredential { + clientCredentialType?: OAuth2ClientCredentialType; + clientSecret?: string; + privateKey?: string; + certificate?: string; +} + +interface ClientAuthOptions { + clientCredentialType?: OAuth2ClientCredentialType; + clientSecret?: string; + clientCertificate?: ClientCertificate; +} + +/** + * Resolves the client-authentication options for `ClientOAuth2` from credential data. + * In certificate mode the client uses a signed assertion, so the (possibly stale) secret is + * dropped; the certificate is attached only when both PEMs are present (otherwise the token + * request fails with a clear "missing certificate" error from the flow). + */ +export function resolveClientAuthOptions(credential: ClientAuthCredential): ClientAuthOptions { + if (credential.clientCredentialType === 'certificate') { + const usesCertificate = !!credential.privateKey && !!credential.certificate; + return { + clientCredentialType: 'certificate', + ...(usesCertificate && { + clientCertificate: { + privateKey: credential.privateKey as string, + certificate: credential.certificate as string, + }, + }), + }; + } + + return { + clientCredentialType: credential.clientCredentialType, + clientSecret: credential.clientSecret, + }; +} diff --git a/packages/@n8n/client-oauth2/src/index.ts b/packages/@n8n/client-oauth2/src/index.ts index feed92f42b3..3f40260cead 100644 --- a/packages/@n8n/client-oauth2/src/index.ts +++ b/packages/@n8n/client-oauth2/src/index.ts @@ -3,4 +3,5 @@ export { ClientOAuth2 } from './client-oauth2'; export type { ClientOAuth2TokenData } from './client-oauth2-token'; export { ClientOAuth2Token } from './client-oauth2-token'; export { AuthError } from './utils'; +export { resolveClientAuthOptions } from './credential-options'; export type * from './types'; diff --git a/packages/@n8n/client-oauth2/test/client-assertion.test.ts b/packages/@n8n/client-oauth2/test/client-assertion.test.ts index a7f96dced7c..44a98551678 100644 --- a/packages/@n8n/client-oauth2/test/client-assertion.test.ts +++ b/packages/@n8n/client-oauth2/test/client-assertion.test.ts @@ -43,7 +43,7 @@ describe('buildClientAssertion', () => { expect(verified).toBe(true); }); - it('throws when the certificate is not a valid X.509 certificate', () => { + it('throws a clear error when the certificate is not a valid X.509 certificate', () => { expect(() => buildClientAssertion({ clientId: config.clientId, @@ -51,10 +51,10 @@ describe('buildClientAssertion', () => { privateKey: config.privateKey, certificate: 'not-a-valid-certificate', }), - ).toThrow(); + ).toThrow('The Certificate field must contain a PEM certificate'); }); - it('throws when the private key is invalid', () => { + it('throws a clear error when the private key is invalid', () => { expect(() => buildClientAssertion({ clientId: config.clientId, @@ -62,7 +62,18 @@ describe('buildClientAssertion', () => { privateKey: 'not-a-valid-key', certificate: config.certificate, }), - ).toThrow(); + ).toThrow('The Private Key field must contain a PEM private key'); + }); + + it('throws a clear error when the certificate and private key fields are swapped', () => { + expect(() => + buildClientAssertion({ + clientId: config.clientId, + accessTokenUri: config.accessTokenUri, + privateKey: config.certificate, + certificate: config.privateKey, + }), + ).toThrow('The Certificate field must contain a PEM certificate'); }); it('throws when a well-formed non-RSA (EC) private key is provided', () => { diff --git a/packages/@n8n/client-oauth2/test/credential-options.test.ts b/packages/@n8n/client-oauth2/test/credential-options.test.ts new file mode 100644 index 00000000000..01d59a61241 --- /dev/null +++ b/packages/@n8n/client-oauth2/test/credential-options.test.ts @@ -0,0 +1,34 @@ +import { resolveClientAuthOptions } from '@/credential-options'; + +describe('resolveClientAuthOptions', () => { + it('returns the client secret in secret mode', () => { + expect( + resolveClientAuthOptions({ clientCredentialType: 'clientSecret', clientSecret: 'secret' }), + ).toEqual({ clientCredentialType: 'clientSecret', clientSecret: 'secret' }); + }); + + it('attaches the certificate and drops the secret in certificate mode', () => { + expect( + resolveClientAuthOptions({ + clientCredentialType: 'certificate', + clientSecret: 'stale-secret', + privateKey: 'pk', + certificate: 'cert', + }), + ).toEqual({ + clientCredentialType: 'certificate', + clientCertificate: { privateKey: 'pk', certificate: 'cert' }, + }); + }); + + it('omits the certificate (and the secret) when a PEM is missing in certificate mode', () => { + expect( + resolveClientAuthOptions({ + clientCredentialType: 'certificate', + clientSecret: 'stale-secret', + privateKey: 'pk', + certificate: '', + }), + ).toEqual({ clientCredentialType: 'certificate' }); + }); +}); diff --git a/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts b/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts index 11062b505a1..df0b8601299 100644 --- a/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts +++ b/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts @@ -1,5 +1,6 @@ import { Logger } from '@n8n/backend-common'; import { mockInstance } from '@n8n/backend-test-utils'; +import type { ClientOAuth2 } from '@n8n/client-oauth2'; import { type CredentialsEntity, type User } from '@n8n/db'; import { Container } from '@n8n/di'; import type { Response } from 'express'; @@ -14,7 +15,10 @@ import { OauthService } from '@/oauth/oauth.service'; import type { OAuthRequest } from '@/requests'; vi.mock('axios'); -vi.mock('@n8n/client-oauth2'); +vi.mock('@n8n/client-oauth2', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ClientOAuth2: vi.fn() }; +}); vi.mock('pkce-challenge'); describe('OAuth2CredentialController', () => { @@ -144,6 +148,128 @@ describe('OAuth2CredentialController', () => { expect(externalHooks.run).toHaveBeenCalledWith('oauth2.callback', expect.any(Array)); }); + it('should build the client with a certificate when certificate authentication is selected', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = vi.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + vi.mocked(ClientOAuth2).mockImplementation(function () { + return { code: { getToken: mockGetToken } } as unknown as ClientOAuth2; + }); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + data: 'encrypted-data', + }; + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'body', + }, + mockState, + { csrfSecret: 'csrf-secret' }, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: validState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + expect(ClientOAuth2).toHaveBeenCalledWith( + expect.objectContaining({ + clientCertificate: { privateKey: 'private-key-pem', certificate: 'certificate-pem' }, + }), + ); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + + it('should not send a client secret when certificate authentication is selected but a stale secret is stored', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = vi.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + vi.mocked(ClientOAuth2).mockImplementation(function () { + return { code: { getToken: mockGetToken } } as unknown as ClientOAuth2; + }); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + data: 'encrypted-data', + }; + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + // Leftover from before the credential was switched to certificate auth. + clientSecret: 'stale-secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'body', + }, + mockState, + { csrfSecret: 'csrf-secret' }, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: validState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + // The body-auth path must not forward the stale secret alongside the client assertion. + expect(mockGetToken).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: expect.not.objectContaining({ client_secret: expect.anything() }), + }), + ); + // The client itself is still built for certificate auth, without a secret. + expect(ClientOAuth2).toHaveBeenCalledWith( + expect.objectContaining({ + clientCredentialType: 'certificate', + clientCertificate: { privateKey: 'private-key-pem', certificate: 'certificate-pem' }, + }), + ); + }); + it('should pass state resource to token exchange and store it for static credentials', async () => { const { ClientOAuth2 } = await import('@n8n/client-oauth2'); const mockGetToken = vi.fn().mockResolvedValue({ diff --git a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts index 1aab0ddcd08..0783c2fe5db 100644 --- a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts @@ -1,6 +1,6 @@ import { Logger } from '@n8n/backend-common'; import type { ClientOAuth2Options, OAuth2CredentialData } from '@n8n/client-oauth2'; -import { ClientOAuth2 } from '@n8n/client-oauth2'; +import { ClientOAuth2, resolveClientAuthOptions } from '@n8n/client-oauth2'; import { Get, RestController } from '@n8n/decorators'; import { Response } from 'express'; import omit from 'lodash/omit'; @@ -166,7 +166,7 @@ export class OAuth2CredentialController { private convertCredentialToOptions(credential: OAuth2CredentialData): ClientOAuth2Options { const options: ClientOAuth2Options = { clientId: credential.clientId, - clientSecret: credential.clientSecret ?? '', + ...resolveClientAuthOptions(credential), accessTokenUri: credential.accessTokenUrl ?? '', authorizationUri: credential.authUrl ?? '', authentication: credential.authentication ?? 'header', diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index 424bac1db43..073019b37ac 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -1,4 +1,3 @@ -import type { Mock } from 'vitest'; import { Logger } from '@n8n/backend-common'; import { OutboundHttp, SsrfProtectionService, type HttpRequestClient } from '@n8n/backend-network'; import { mockInstance } from '@n8n/backend-test-utils'; @@ -8,11 +7,12 @@ import { Time } from '@n8n/constants'; import type { AuthenticatedRequest, CredentialsEntity, ICredentialsDb, User } from '@n8n/db'; import { CredentialsRepository } from '@n8n/db'; import type { Request, Response } from 'express'; -import { mock } from 'vitest-mock-extended'; import type { Cipher } from 'n8n-core'; import { Credentials } from 'n8n-core'; import type { IHttpRequestOptions, IWorkflowExecuteAdditionalData } from 'n8n-workflow'; import { UnexpectedError } from 'n8n-workflow'; +import type { Mock } from 'vitest'; +import { mock } from 'vitest-mock-extended'; import { AuthService } from '@/auth/auth.service'; import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; @@ -40,7 +40,10 @@ import { UrlService } from '@/services/url.service'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; vi.mock('@/workflow-execute-additional-data'); -vi.mock('@n8n/client-oauth2'); +vi.mock('@n8n/client-oauth2', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ClientOAuth2: vi.fn() }; +}); vi.mock('pkce-challenge'); /** @@ -5039,6 +5042,50 @@ describe('OauthService', () => { expect(mockToken.refresh).toHaveBeenCalledTimes(1); }); + it('builds the client with a certificate when certificate authentication is selected', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + let capturedOptions: unknown; + const refreshed = { + data: { access_token: 'new-token', token_type: 'bearer' }, + accessToken: 'new-token', + }; + const mockToken = { refresh: vi.fn().mockResolvedValue(refreshed), client: {} }; + vi.mocked(ClientOAuth2).mockImplementation(function (options) { + capturedOptions = options; + return { createToken: vi.fn().mockReturnValue(mockToken) } as never; + }); + + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + vi.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + clientSecret: 'stale-secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + oauthTokenData: { + access_token: 'stale', + refresh_token: 'refresh-tok', + token_type: 'bearer', + }, + } as unknown as OAuth2CredentialData); + vi.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toEqual({ Authorization: 'Bearer new-token' }); + expect(capturedOptions).toEqual( + expect.objectContaining({ + clientCredentialType: 'certificate', + clientCertificate: { privateKey: 'private-key-pem', certificate: 'certificate-pem' }, + }), + ); + // The stale secret must not be carried into the client options in certificate mode. + expect(capturedOptions).not.toHaveProperty('clientSecret', 'stale-secret'); + }); + it('persists the refreshed token data after a successful refresh', async () => { const { ClientOAuth2 } = await import('@n8n/client-oauth2'); const refreshedData = { access_token: 'new-token', token_type: 'bearer' }; diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index 826953920da..1c2c7489a7f 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -26,6 +26,7 @@ import { UrlService } from '@/services/url.service'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import { ClientOAuth2, + resolveClientAuthOptions, type ClientOAuth2Options, type ClientOAuth2TokenData, type OAuth2AuthenticationMethod, @@ -688,7 +689,7 @@ export class OauthService { const oAuthClient = new ClientOAuth2({ clientId: oauthCredentials.clientId, - clientSecret: oauthCredentials.clientSecret, + ...resolveClientAuthOptions(oauthCredentials), accessTokenUri: oauthCredentials.accessTokenUrl, scopes: scopes?.length ? scopes : undefined, ignoreSSLIssues: oauthCredentials.ignoreSSLIssues, @@ -1208,6 +1209,11 @@ export class OauthService { return Object.fromEntries(new URLSearchParams(response).entries()); } + // Builds options for the authorization-redirect leg, consumed by the `oauth2.authenticate` + // hook and `code.getUri()`. Neither authenticates the client. The certificate is deliberately + // not mapped here — it would only leak the private key to the hook with no benefit. `clientSecret` + // is also unused by `getUri()` and reaches only the hook (pre-existing). The resulting asymmetry + // (secret reaches the hook, certificate does not) is intentional. private convertCredentialToOptions(credential: OAuth2CredentialData): ClientOAuth2Options { const options: ClientOAuth2Options = { clientId: credential.clientId, diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/create-oauth2-client.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/create-oauth2-client.test.ts index 84f664ba7b7..114b648fe56 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/create-oauth2-client.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/create-oauth2-client.test.ts @@ -1,7 +1,8 @@ +import type * as ClientOAuth2Module from '@n8n/client-oauth2'; import type { IAllExecuteFunctions, INode, IWorkflowExecuteAdditionalData } from 'n8n-workflow'; import { mockDeep } from 'vitest-mock-extended'; -import { requestOAuth2 } from '../request-helpers/oauth'; +import { refreshOAuth2Token, requestOAuth2 } from '../request-helpers/oauth'; const { mockGetToken, mockSign, mockCreateToken, MockClientOAuth2 } = vi.hoisted(() => ({ mockGetToken: vi.fn(), @@ -10,9 +11,10 @@ const { mockGetToken, mockSign, mockCreateToken, MockClientOAuth2 } = vi.hoisted MockClientOAuth2: vi.fn(), })); -vi.mock('@n8n/client-oauth2', () => ({ - ClientOAuth2: MockClientOAuth2, -})); +vi.mock('@n8n/client-oauth2', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ClientOAuth2: MockClientOAuth2 }; +}); describe('createOAuth2Client - scope handling', () => { const mockThis = mockDeep(); @@ -113,4 +115,101 @@ describe('createOAuth2Client - scope handling', () => { expect.objectContaining({ scopes: ['read', 'write'] }), ); }); + + test('should pass clientCertificate when certificate authentication is selected', async () => { + mockThis.getCredentials.mockResolvedValue({ + ...baseCredentials, + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + }); + + await call(); + + expect(MockClientOAuth2).toHaveBeenCalledWith( + expect.objectContaining({ + clientCertificate: { privateKey: 'private-key-pem', certificate: 'certificate-pem' }, + }), + ); + }); + + test('should not pass clientCertificate when authentication is the default client secret', async () => { + mockThis.getCredentials.mockResolvedValue({ ...baseCredentials }); + + await call(); + + expect(MockClientOAuth2).toHaveBeenCalledWith( + expect.not.objectContaining({ clientCertificate: expect.anything() }), + ); + }); + + test('should not pass clientCertificate when certificate is selected but PEMs are missing', async () => { + mockThis.getCredentials.mockResolvedValue({ + ...baseCredentials, + clientCredentialType: 'certificate', + privateKey: '', + certificate: '', + }); + + await call(); + + expect(MockClientOAuth2).toHaveBeenCalledWith( + expect.not.objectContaining({ clientCertificate: expect.anything() }), + ); + }); + + test('should not pass clientCertificate when only one PEM field is provided', async () => { + mockThis.getCredentials.mockResolvedValue({ + ...baseCredentials, + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: '', + }); + + await call(); + + expect(MockClientOAuth2).toHaveBeenCalledWith( + expect.not.objectContaining({ clientCertificate: expect.anything() }), + ); + }); + + test('should not carry the client secret when certificate authentication is selected', async () => { + mockThis.getCredentials.mockResolvedValue({ + ...baseCredentials, + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + clientSecret: 'stale-secret', + }); + + await call(); + + const passedOptions = MockClientOAuth2.mock.calls[0][0]; + expect(passedOptions.clientSecret).toBeUndefined(); + expect(passedOptions.clientCertificate).toEqual({ + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + }); + }); + + test('should build the client with a certificate on the token-refresh path', async () => { + mockThis.getCredentials.mockResolvedValue({ + ...baseCredentials, + grantType: 'authorizationCode', + clientCredentialType: 'certificate', + privateKey: 'private-key-pem', + certificate: 'certificate-pem', + oauthTokenData: { access_token: 'stale', refresh_token: 'refresh-tok' }, + }); + + await refreshOAuth2Token + .call(mockThis, 'testOAuth2', mockNode, mockAdditionalData) + .catch(() => {}); + + expect(MockClientOAuth2).toHaveBeenCalledWith( + expect.objectContaining({ + clientCertificate: { privateKey: 'private-key-pem', certificate: 'certificate-pem' }, + }), + ); + }); }); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/request-helpers/oauth.ts b/packages/core/src/execution-engine/node-execution-context/utils/request-helpers/oauth.ts index cef5f26e3bd..870e709555d 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/request-helpers/oauth.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/request-helpers/oauth.ts @@ -13,7 +13,7 @@ import type { ClientOAuth2TokenData, OAuth2CredentialData, } from '@n8n/client-oauth2'; -import { AuthError, ClientOAuth2 } from '@n8n/client-oauth2'; +import { AuthError, ClientOAuth2, resolveClientAuthOptions } from '@n8n/client-oauth2'; import { Container } from '@n8n/di'; import type { AxiosError } from 'axios'; import { createHmac } from 'crypto'; @@ -50,7 +50,7 @@ function createOAuth2Client(credentials: OAuth2CredentialData): ClientOAuth2 { .filter(Boolean); return new ClientOAuth2({ clientId: credentials.clientId, - clientSecret: credentials.clientSecret, + ...resolveClientAuthOptions(credentials), accessTokenUri: credentials.accessTokenUrl, scopes: scopes?.length ? scopes : undefined, ignoreSSLIssues: credentials.ignoreSSLIssues, @@ -185,9 +185,10 @@ async function refreshOrFetchToken(ctx: RefreshOAuth2TokenContext): Promise-----BEGIN PRIVATE KEY-----
KEY DATA GOES HERE
-----END PRIVATE KEY-----', + }, + { + displayName: 'Certificate', + name: 'certificate', + type: 'string', + typeOptions: { + password: true, + rows: 4, + }, + default: '', + required: true, + displayOptions: { + show: { + clientCredentialType: ['certificate'], + }, + }, + description: + 'PEM-encoded public certificate registered on the Entra app registration (Certificates & secrets). Used to derive the x5t thumbprint that tells Entra which key verifies the assertion.', + }, // Info about the tenantID // https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-v2-protocols#endpoints // Endpoints `/common` can only be used for multitenant apps diff --git a/packages/nodes-base/credentials/OAuth2Api.credentials.ts b/packages/nodes-base/credentials/OAuth2Api.credentials.ts index 2899200ae69..1138434aff8 100644 --- a/packages/nodes-base/credentials/OAuth2Api.credentials.ts +++ b/packages/nodes-base/credentials/OAuth2Api.credentials.ts @@ -90,6 +90,14 @@ export class OAuth2Api implements ICredentialType { default: '', required: true, }, + { + // Hidden ordering anchor: lets extending credentials (e.g. microsoftOAuth2Api) render a + // secret/certificate selector right after Client ID by overriding this field. Inert here. + displayName: 'Authentication', + name: 'clientCredentialType', + type: 'hidden', + default: 'clientSecret', + }, { displayName: 'Client Secret', name: 'clientSecret', @@ -105,6 +113,20 @@ export class OAuth2Api implements ICredentialType { default: '', required: true, }, + { + // Hidden ordering anchors (see `clientCredentialType` above): let extending credentials + // render the certificate fields right after Client Secret instead of appended at the end. + displayName: 'Private Key', + name: 'privateKey', + type: 'hidden', + default: '', + }, + { + displayName: 'Certificate', + name: 'certificate', + type: 'hidden', + default: '', + }, // WARNING: if you are extending from this credentials and allow user to set their own scopes // you HAVE TO add it to GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE in packages/cli/src/constants.ts // track any updates to this behavior in N8N-7424 diff --git a/packages/nodes-base/credentials/test/MicrosoftOAuth2Api.credentials.test.ts b/packages/nodes-base/credentials/test/MicrosoftOAuth2Api.credentials.test.ts index 7afa8bca622..5cf9122513d 100644 --- a/packages/nodes-base/credentials/test/MicrosoftOAuth2Api.credentials.test.ts +++ b/packages/nodes-base/credentials/test/MicrosoftOAuth2Api.credentials.test.ts @@ -1,4 +1,7 @@ +import { NodeHelpers, type INodeProperties } from 'n8n-workflow'; + import { MicrosoftOAuth2Api } from '../MicrosoftOAuth2Api.credentials'; +import { OAuth2Api } from '../OAuth2Api.credentials'; describe('MicrosoftOAuth2Api Credential', () => { const microsoftOAuth2Api = new MicrosoftOAuth2Api(); @@ -28,4 +31,50 @@ describe('MicrosoftOAuth2Api Credential', () => { expect(authQueryParamsProperty?.type).toBe('hidden'); expect(authQueryParamsProperty?.default).toBe('response_mode=query&prompt=select_account'); }); + + describe('certificate authentication', () => { + const getProperty = (name: string) => + microsoftOAuth2Api.properties.find((p) => p.name === name); + + it('should offer a client secret / certificate selector defaulting to client secret', () => { + const clientCredentialType = getProperty('clientCredentialType'); + expect(clientCredentialType?.type).toBe('options'); + expect(clientCredentialType?.default).toBe('clientSecret'); + expect(clientCredentialType?.options).toEqual([ + { name: 'Client Secret', value: 'clientSecret' }, + { name: 'Certificate', value: 'certificate' }, + ]); + }); + + it('should only show the client secret when client secret authentication is selected', () => { + const clientSecret = getProperty('clientSecret'); + expect(clientSecret?.displayOptions?.show?.clientCredentialType).toEqual(['clientSecret']); + }); + + it('should show the private key and certificate only for certificate authentication', () => { + for (const name of ['privateKey', 'certificate']) { + const property = getProperty(name); + expect(property?.required).toBe(true); + expect(property?.displayOptions?.show?.clientCredentialType).toEqual(['certificate']); + } + }); + + it('should render the auth block contiguously after Client ID once merged with the base', () => { + const merged: INodeProperties[] = []; + NodeHelpers.mergeNodeProperties(merged, new OAuth2Api().properties); + NodeHelpers.mergeNodeProperties(merged, microsoftOAuth2Api.properties); + + const names = merged.map((p) => p.name); + const start = names.indexOf('clientId'); + + // Selector → secret → key → cert render together right after Client ID, not appended last. + expect(names.slice(start + 1, start + 5)).toEqual([ + 'clientCredentialType', + 'clientSecret', + 'privateKey', + 'certificate', + ]); + expect(merged[start + 1].type).toBe('options'); + }); + }); });