fix(core): Fix using secrets for credentials on oauth callback (#14711)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
This commit is contained in:
Guillaume Jacquart
2025-04-18 10:29:04 +02:00
committed by GitHub
parent 3641c1fb87
commit 09806c36ae
8 changed files with 23 additions and 20 deletions

View File

@@ -86,7 +86,7 @@ describe('OAuth1CredentialController', () => {
jest.spyOn(Csrf.prototype, 'create').mockReturnValueOnce('token'); jest.spyOn(Csrf.prototype, 'create').mockReturnValueOnce('token');
sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(credential); sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(credential);
credentialsHelper.getDecrypted.mockResolvedValueOnce({}); credentialsHelper.getDecrypted.mockResolvedValueOnce({});
credentialsHelper.applyDefaultsAndOverwrites.mockReturnValueOnce({ credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValueOnce({
requestTokenUrl: 'https://example.domain/oauth/request_token', requestTokenUrl: 'https://example.domain/oauth/request_token',
authUrl: 'https://example.domain/oauth/authorize', authUrl: 'https://example.domain/oauth/authorize',
accessTokenUrl: 'https://example.domain/oauth/access_token', accessTokenUrl: 'https://example.domain/oauth/access_token',
@@ -223,7 +223,7 @@ describe('OAuth1CredentialController', () => {
it('should exchange the code for a valid token, and save it to DB', async () => { it('should exchange the code for a valid token, and save it to DB', async () => {
credentialsRepository.findOneBy.mockResolvedValue(credential); credentialsRepository.findOneBy.mockResolvedValue(credential);
credentialsHelper.getDecrypted.mockResolvedValue({ csrfSecret }); credentialsHelper.getDecrypted.mockResolvedValue({ csrfSecret });
credentialsHelper.applyDefaultsAndOverwrites.mockReturnValueOnce({ credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValueOnce({
requestTokenUrl: 'https://example.domain/oauth/request_token', requestTokenUrl: 'https://example.domain/oauth/request_token',
accessTokenUrl: 'https://example.domain/oauth/access_token', accessTokenUrl: 'https://example.domain/oauth/access_token',
signatureMethod: 'HMAC-SHA1', signatureMethod: 'HMAC-SHA1',

View File

@@ -64,7 +64,7 @@ describe('OAuth2CredentialController', () => {
jest.setSystemTime(new Date(timestamp)); jest.setSystemTime(new Date(timestamp));
jest.clearAllMocks(); jest.clearAllMocks();
credentialsHelper.applyDefaultsAndOverwrites.mockReturnValue({ credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue({
clientId: 'test-client-id', clientId: 'test-client-id',
clientSecret: 'oauth-secret', clientSecret: 'oauth-secret',
authUrl: 'https://example.domain/o/oauth2/v2/auth', authUrl: 'https://example.domain/o/oauth2/v2/auth',

View File

@@ -121,17 +121,20 @@ export abstract class AbstractOAuthController {
); );
} }
protected applyDefaultsAndOverwrites<T>( protected async applyDefaultsAndOverwrites<T>(
credential: ICredentialsDb, credential: ICredentialsDb,
decryptedData: ICredentialDataDecryptedObject, decryptedData: ICredentialDataDecryptedObject,
additionalData: IWorkflowExecuteAdditionalData, additionalData: IWorkflowExecuteAdditionalData,
) { ) {
return this.credentialsHelper.applyDefaultsAndOverwrites( return (await this.credentialsHelper.applyDefaultsAndOverwrites(
additionalData, additionalData,
decryptedData, decryptedData,
credential,
credential.type, credential.type,
'internal', 'internal',
) as unknown as T; undefined,
undefined,
)) as unknown as T;
} }
protected async encryptAndSaveData( protected async encryptAndSaveData(
@@ -209,7 +212,8 @@ export abstract class AbstractOAuthController {
credential, credential,
additionalData, additionalData,
); );
const oauthCredentials = this.applyDefaultsAndOverwrites<T>(
const oauthCredentials = await this.applyDefaultsAndOverwrites<T>(
credential, credential,
decryptedDataOriginal, decryptedDataOriginal,
additionalData, additionalData,

View File

@@ -36,7 +36,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
const credential = await this.getCredential(req); const credential = await this.getCredential(req);
const additionalData = await this.getAdditionalData(); const additionalData = await this.getAdditionalData();
const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData); const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
const oauthCredentials = this.applyDefaultsAndOverwrites<OAuth1CredentialData>( const oauthCredentials = await this.applyDefaultsAndOverwrites<OAuth1CredentialData>(
credential, credential,
decryptedDataOriginal, decryptedDataOriginal,
additionalData, additionalData,

View File

@@ -37,7 +37,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
delete decryptedDataOriginal.scope; delete decryptedDataOriginal.scope;
} }
const oauthCredentials = this.applyDefaultsAndOverwrites<OAuth2CredentialData>( const oauthCredentials = await this.applyDefaultsAndOverwrites<OAuth2CredentialData>(
credential, credential,
decryptedDataOriginal, decryptedDataOriginal,
additionalData, additionalData,

View File

@@ -331,31 +331,29 @@ export class CredentialsHelper extends ICredentialsHelper {
await additionalData?.secretsHelpers?.waitForInit(); await additionalData?.secretsHelpers?.waitForInit();
const canUseSecrets = await this.credentialCanUseExternalSecrets(nodeCredentials); return await this.applyDefaultsAndOverwrites(
return this.applyDefaultsAndOverwrites(
additionalData, additionalData,
decryptedDataOriginal, decryptedDataOriginal,
nodeCredentials,
type, type,
mode, mode,
executeData, executeData,
expressionResolveValues, expressionResolveValues,
canUseSecrets,
); );
} }
/** /**
* Applies credential default data and overwrites * Applies credential default data and overwrites
*/ */
applyDefaultsAndOverwrites( async applyDefaultsAndOverwrites(
additionalData: IWorkflowExecuteAdditionalData, additionalData: IWorkflowExecuteAdditionalData,
decryptedDataOriginal: ICredentialDataDecryptedObject, decryptedDataOriginal: ICredentialDataDecryptedObject,
credential: INodeCredentialsDetails,
type: string, type: string,
mode: WorkflowExecuteMode, mode: WorkflowExecuteMode,
executeData?: IExecuteData, executeData?: IExecuteData,
expressionResolveValues?: ICredentialsExpressionResolveValues, expressionResolveValues?: ICredentialsExpressionResolveValues,
canUseSecrets?: boolean, ): Promise<ICredentialDataDecryptedObject> {
): ICredentialDataDecryptedObject {
const credentialsProperties = this.getCredentialsProperties(type); const credentialsProperties = this.getCredentialsProperties(type);
// Load and apply the credentials overwrites if any exist // Load and apply the credentials overwrites if any exist
@@ -380,8 +378,9 @@ export class CredentialsHelper extends ICredentialsHelper {
decryptedData.oauthTokenData = decryptedDataOriginal.oauthTokenData; decryptedData.oauthTokenData = decryptedDataOriginal.oauthTokenData;
} }
const canUseExternalSecrets = await this.credentialCanUseExternalSecrets(credential);
const additionalKeys = getAdditionalKeys(additionalData, mode, null, { const additionalKeys = getAdditionalKeys(additionalData, mode, null, {
secretsEnabled: canUseSecrets, secretsEnabled: canUseExternalSecrets,
}); });
if (expressionResolveValues) { if (expressionResolveValues) {

View File

@@ -187,14 +187,14 @@ export class CredentialsTester {
if (credentialsDecrypted.data) { if (credentialsDecrypted.data) {
try { try {
const additionalData = await WorkflowExecuteAdditionalData.getBase(userId); const additionalData = await WorkflowExecuteAdditionalData.getBase(userId);
credentialsDecrypted.data = this.credentialsHelper.applyDefaultsAndOverwrites( credentialsDecrypted.data = await this.credentialsHelper.applyDefaultsAndOverwrites(
additionalData, additionalData,
credentialsDecrypted.data, credentialsDecrypted.data,
credentialsDecrypted,
credentialType, credentialType,
'internal' as WorkflowExecuteMode, 'internal' as WorkflowExecuteMode,
undefined, undefined,
undefined, undefined,
await this.credentialsHelper.credentialCanUseExternalSecrets(credentialsDecrypted),
); );
} catch (error) { } catch (error) {
this.logger.debug('Credential test failed', error); this.logger.debug('Credential test failed', error);

View File

@@ -28,7 +28,7 @@ describe('OAuth2 API', () => {
authQueryParameters: 'access_type=offline', authQueryParameters: 'access_type=offline',
}; };
CredentialsHelper.prototype.applyDefaultsAndOverwrites = (_, decryptedDataOriginal) => CredentialsHelper.prototype.applyDefaultsAndOverwrites = async (_, decryptedDataOriginal) =>
decryptedDataOriginal; decryptedDataOriginal;
beforeAll(async () => { beforeAll(async () => {