mirror of
https://github.com/n8n-io/n8n.git
synced 2024-12-24 20:24:05 -08:00
fix(core): Ensure OAuth token data is not stubbed in source control (#10302)
This commit is contained in:
parent
e3edeaa035
commit
98115e95df
|
@ -0,0 +1,85 @@
|
||||||
|
import mock from 'jest-mock-extended/lib/Mock';
|
||||||
|
import { SourceControlExportService } from '../sourceControlExport.service.ee';
|
||||||
|
import type { SourceControlledFile } from '../types/sourceControlledFile';
|
||||||
|
import { Cipher, type InstanceSettings } from 'n8n-core';
|
||||||
|
import { SharedCredentialsRepository } from '@/databases/repositories/sharedCredentials.repository';
|
||||||
|
import { mockInstance } from '@test/mocking';
|
||||||
|
|
||||||
|
import type { SharedCredentials } from '@/databases/entities/SharedCredentials';
|
||||||
|
import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity';
|
||||||
|
import Container from 'typedi';
|
||||||
|
import { ApplicationError, deepCopy } from 'n8n-workflow';
|
||||||
|
|
||||||
|
// https://github.com/jestjs/jest/issues/4715
|
||||||
|
function deepSpyOn<O extends object, M extends keyof O>(object: O, methodName: M) {
|
||||||
|
const spy = jest.fn();
|
||||||
|
// eslint-disable-next-line @typescript-eslint/ban-types
|
||||||
|
const originalMethod = object[methodName];
|
||||||
|
|
||||||
|
if (typeof originalMethod !== 'function') {
|
||||||
|
throw new ApplicationError(`${methodName.toString()} is not a function`, { level: 'warning' });
|
||||||
|
}
|
||||||
|
|
||||||
|
object[methodName] = function (...args: unknown[]) {
|
||||||
|
const clonedArgs = deepCopy(args);
|
||||||
|
spy(...clonedArgs);
|
||||||
|
return originalMethod.apply(this, args);
|
||||||
|
} as O[M];
|
||||||
|
|
||||||
|
return spy;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('SourceControlExportService', () => {
|
||||||
|
const service = new SourceControlExportService(
|
||||||
|
mock(),
|
||||||
|
mock(),
|
||||||
|
mock(),
|
||||||
|
mock<InstanceSettings>({ n8nFolder: '' }),
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('exportCredentialsToWorkFolder', () => {
|
||||||
|
it('should export credentials to work folder', async () => {
|
||||||
|
/**
|
||||||
|
* Arrange
|
||||||
|
*/
|
||||||
|
// @ts-expect-error Private method
|
||||||
|
const replaceSpy = deepSpyOn(service, 'replaceCredentialData');
|
||||||
|
|
||||||
|
mockInstance(SharedCredentialsRepository).findByCredentialIds.mockResolvedValue([
|
||||||
|
mock<SharedCredentials>({
|
||||||
|
credentials: mock<CredentialsEntity>({
|
||||||
|
data: Container.get(Cipher).encrypt(
|
||||||
|
JSON.stringify({
|
||||||
|
authUrl: 'test',
|
||||||
|
accessTokenUrl: 'test',
|
||||||
|
clientId: 'test',
|
||||||
|
clientSecret: 'test',
|
||||||
|
oauthTokenData: {
|
||||||
|
access_token: 'test',
|
||||||
|
token_type: 'test',
|
||||||
|
expires_in: 123,
|
||||||
|
refresh_token: 'test',
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Act
|
||||||
|
*/
|
||||||
|
await service.exportCredentialsToWorkFolder([mock<SourceControlledFile>()]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Assert
|
||||||
|
*/
|
||||||
|
expect(replaceSpy).toHaveBeenCalledWith({
|
||||||
|
authUrl: 'test',
|
||||||
|
accessTokenUrl: 'test',
|
||||||
|
clientId: 'test',
|
||||||
|
clientSecret: 'test',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
|
@ -293,11 +293,18 @@ export class SourceControlExportService {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Edge case: Do not export `oauthTokenData`, so that that the
|
||||||
|
* pulling instance reconnects instead of trying to use stubbed values.
|
||||||
|
*/
|
||||||
|
const credentialData = credentials.getData();
|
||||||
|
const { oauthTokenData, ...rest } = credentialData;
|
||||||
|
|
||||||
const stub: ExportableCredential = {
|
const stub: ExportableCredential = {
|
||||||
id,
|
id,
|
||||||
name,
|
name,
|
||||||
type,
|
type,
|
||||||
data: this.replaceCredentialData(credentials.getData()),
|
data: this.replaceCredentialData(rest),
|
||||||
ownedBy: owner,
|
ownedBy: owner,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -326,7 +326,12 @@ export class SourceControlImportService {
|
||||||
if (existingCredential?.data) {
|
if (existingCredential?.data) {
|
||||||
newCredentialObject.data = existingCredential.data;
|
newCredentialObject.data = existingCredential.data;
|
||||||
} else {
|
} else {
|
||||||
newCredentialObject.setData(data);
|
/**
|
||||||
|
* Edge case: Do not import `oauthTokenData`, so that that the
|
||||||
|
* pulling instance reconnects instead of trying to use stubbed values.
|
||||||
|
*/
|
||||||
|
const { oauthTokenData, ...rest } = data;
|
||||||
|
newCredentialObject.setData(rest);
|
||||||
}
|
}
|
||||||
|
|
||||||
this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`);
|
this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`);
|
||||||
|
|
|
@ -1342,7 +1342,9 @@ export async function requestOAuth2(
|
||||||
// if it's the first time using the credentials, get the access token and save it into the DB.
|
// if it's the first time using the credentials, get the access token and save it into the DB.
|
||||||
if (
|
if (
|
||||||
credentials.grantType === 'clientCredentials' &&
|
credentials.grantType === 'clientCredentials' &&
|
||||||
(oauthTokenData === undefined || Object.keys(oauthTokenData).length === 0)
|
(oauthTokenData === undefined ||
|
||||||
|
Object.keys(oauthTokenData).length === 0 ||
|
||||||
|
oauthTokenData.access_token === '') // stub
|
||||||
) {
|
) {
|
||||||
const { data } = await oAuthClient.credentials.getToken();
|
const { data } = await oAuthClient.credentials.getToken();
|
||||||
// Find the credentials
|
// Find the credentials
|
||||||
|
|
Loading…
Reference in a new issue