mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2025-02-21 02:56:40 -08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(core): Allow secrets manager secrets to be used in credentials (#13110)

Browse source
This commit is contained in:
Marc Littlemore 2025-02-07 16:43:00 +00:00 committed by GitHub
parent 4577ce0846
commit cae98e733d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 75 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts
View file

@ -4,6 +4,7 @@ import type { Response } from 'express';
import { mock } from 'jest-mock-extended';
import { Cipher } from 'n8n-core';
import { Logger } from 'n8n-core';
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import nock from 'nock';
import { Time } from '@/constants';
@ -19,8 +20,11 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { ExternalHooks } from '@/external-hooks';
import type { OAuthRequest } from '@/requests';
import { SecretsHelper } from '@/secrets-helpers.ee';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { mockInstance } from '@test/mocking';
jest.mock('@/workflow-execute-additional-data');
describe('OAuth1CredentialController', () => {
mockInstance(Logger);
mockInstance(ExternalHooks);
@ -28,6 +32,9 @@ describe('OAuth1CredentialController', () => {
mockInstance(VariablesService, {
getAllCached: async () => [],
});
const additionalData = mock<IWorkflowExecuteAdditionalData>();
(WorkflowExecuteAdditionalData.getBase as jest.Mock).mockReturnValue(additionalData);
const cipher = mockInstance(Cipher);
const credentialsHelper = mockInstance(CredentialsHelper);
const credentialsRepository = mockInstance(CredentialsRepository);
@ -106,6 +113,14 @@ describe('OAuth1CredentialController', () => {
}),
);
expect(cipher.encrypt).toHaveBeenCalledWith({ csrfSecret });
expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
additionalData,
credential,
credential.type,
'internal',
undefined,
false,
);
});
});
@ -235,6 +250,14 @@ describe('OAuth1CredentialController', () => {
}),
);
expect(res.render).toHaveBeenCalledWith('oauth-callback');
expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
additionalData,
credential,
credential.type,
'internal',
undefined,
true,
);
});
});
});

23
packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts
View file

@ -4,6 +4,7 @@ import { type Response } from 'express';
import { mock } from 'jest-mock-extended';
import { Cipher } from 'n8n-core';
import { Logger } from 'n8n-core';
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import nock from 'nock';
import { CREDENTIAL_BLANKING_VALUE, Time } from '@/constants';
@ -19,14 +20,20 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { ExternalHooks } from '@/external-hooks';
import type { OAuthRequest } from '@/requests';
import { SecretsHelper } from '@/secrets-helpers.ee';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { mockInstance } from '@test/mocking';
jest.mock('@/workflow-execute-additional-data');
describe('OAuth2CredentialController', () => {
mockInstance(Logger);
mockInstance(SecretsHelper);
mockInstance(VariablesService, {
getAllCached: async () => [],
});
const additionalData = mock<IWorkflowExecuteAdditionalData>();
(WorkflowExecuteAdditionalData.getBase as jest.Mock).mockReturnValue(additionalData);
const cipher = mockInstance(Cipher);
const externalHooks = mockInstance(ExternalHooks);
const credentialsHelper = mockInstance(CredentialsHelper);
@ -108,6 +115,14 @@ describe('OAuth2CredentialController', () => {
type: 'oAuth2Api',
}),
);
expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
additionalData,
credential,
credential.type,
'internal',
undefined,
false,
);
});
});
@ -256,6 +271,14 @@ describe('OAuth2CredentialController', () => {
}),
);
expect(res.render).toHaveBeenCalledWith('oauth-callback');
expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
additionalData,
credential,
credential.type,
'internal',
undefined,
true,
);
});
it('merges oauthTokenData if it already exists', async () => {

30
packages/cli/src/controllers/oauth/abstract-oauth.controller.ts
View file

@ -86,9 +86,30 @@ export abstract class AbstractOAuthController {
return await WorkflowExecuteAdditionalData.getBase();
}
protected async getDecryptedData(
/**
* Allow decrypted data to evaluate expressions that include $secrets and apply overwrites
*/
protected async getDecryptedDataForAuthUri(
credential: ICredentialsDb,
additionalData: IWorkflowExecuteAdditionalData,
) {
return await this.getDecryptedData(credential, additionalData, false);
}
/**
* Do not apply overwrites here because that removes the CSRF state, and breaks the oauth flow
*/
protected async getDecryptedDataForCallback(
credential: ICredentialsDb,
additionalData: IWorkflowExecuteAdditionalData,
) {
return await this.getDecryptedData(credential, additionalData, true);
}
private async getDecryptedData(
credential: ICredentialsDb,
additionalData: IWorkflowExecuteAdditionalData,
raw: boolean,
) {
return await this.credentialsHelper.getDecrypted(
additionalData,
@ -96,7 +117,7 @@ export abstract class AbstractOAuthController {
credential.type,
'internal',
undefined,
true,
raw,
);
}
@ -183,7 +204,10 @@ export abstract class AbstractOAuthController {
}
const additionalData = await this.getAdditionalData();
const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData);
const decryptedDataOriginal = await this.getDecryptedDataForCallback(
credential,
additionalData,
);
const oauthCredentials = this.applyDefaultsAndOverwrites<T>(
credential,
decryptedDataOriginal,

2
packages/cli/src/controllers/oauth/oauth1-credential.controller.ts
View file

@ -35,7 +35,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
async getAuthUri(req: OAuthRequest.OAuth1Credential.Auth): Promise<string> {
const credential = await this.getCredential(req);
const additionalData = await this.getAdditionalData();
const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData);
const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
const oauthCredentials = this.applyDefaultsAndOverwrites<OAuth1CredentialData>(
credential,
decryptedDataOriginal,

2
packages/cli/src/controllers/oauth/oauth2-credential.controller.ts
View file

@ -23,7 +23,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
async getAuthUri(req: OAuthRequest.OAuth2Credential.Auth): Promise<string> {
const credential = await this.getCredential(req);
const additionalData = await this.getAdditionalData();
const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData);
const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
// At some point in the past we saved hidden scopes to credentials (but shouldn't)
// Delete scope before applying defaults to make sure new scopes are present on reconnect