mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
feat: Add feature for clearing credentials on resolver update (#24169)
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import { z } from 'zod';
|
||||
import { Z } from 'zod-class';
|
||||
|
||||
import {
|
||||
@@ -10,4 +11,5 @@ export class UpdateCredentialResolverDto extends Z.class({
|
||||
type: credentialResolverTypeNameSchema.optional(),
|
||||
name: credentialResolverNameSchema.optional(),
|
||||
config: credentialResolverConfigSchema.optional(),
|
||||
clearCredentials: z.boolean().optional(),
|
||||
}) {}
|
||||
|
||||
@@ -74,6 +74,13 @@ export interface ICredentialResolver {
|
||||
handle: CredentialResolverHandle,
|
||||
): Promise<void>;
|
||||
|
||||
/**
|
||||
* Deletes all credential data for the resolver.
|
||||
* Optional - not all resolvers support deletion.
|
||||
* @throws {CredentialResolverError} When deletion operation fails
|
||||
*/
|
||||
deleteAllSecrets?(handle: CredentialResolverHandle): Promise<void>;
|
||||
|
||||
/**
|
||||
* Validates resolver configuration before saving.
|
||||
* Should verify connectivity, authentication, and configuration structure.
|
||||
|
||||
@@ -119,6 +119,7 @@ export class CredentialResolversController {
|
||||
type: dto.type,
|
||||
name: dto.name,
|
||||
config: dto.config,
|
||||
clearCredentials: dto.clearCredentials,
|
||||
user: req.user,
|
||||
}),
|
||||
);
|
||||
|
||||
+4
@@ -182,6 +182,10 @@ export class OAuthCredentialResolver implements ICredentialResolver {
|
||||
await this.storage.deleteCredentialData(credentialId, key, handle.resolverId, parsedOptions);
|
||||
}
|
||||
|
||||
async deleteAllSecrets(handle: CredentialResolverHandle): Promise<void> {
|
||||
await this.storage.deleteAllCredentialData(handle);
|
||||
}
|
||||
|
||||
private async parseOptions(options: CredentialResolverConfiguration) {
|
||||
const result = await OAuthCredentialResolverOptionsSchema.safeParseAsync(options);
|
||||
if (result.error) {
|
||||
|
||||
+5
@@ -3,6 +3,7 @@ import { Service } from '@n8n/di';
|
||||
import { ICredentialEntriesStorage } from './storage-interface';
|
||||
import { DynamicCredentialEntry } from '../../database/entities/dynamic-credential-entry';
|
||||
import { DynamicCredentialEntryRepository } from '../../database/repositories/dynamic-credential-entry.repository';
|
||||
import { CredentialResolverHandle } from '@n8n/decorators';
|
||||
|
||||
@Service()
|
||||
export class DynamicCredentialEntryStorage implements ICredentialEntriesStorage {
|
||||
@@ -61,4 +62,8 @@ export class DynamicCredentialEntryStorage implements ICredentialEntriesStorage
|
||||
resolverId,
|
||||
});
|
||||
}
|
||||
|
||||
async deleteAllCredentialData(handle: CredentialResolverHandle): Promise<void> {
|
||||
await this.dynamicCredentialEntryRepository.delete({ resolverId: handle.resolverId });
|
||||
}
|
||||
}
|
||||
|
||||
+112
-2
@@ -23,7 +23,7 @@ describe('DynamicCredentialResolverService', () => {
|
||||
let mockCipher: jest.Mocked<Cipher>;
|
||||
let mockExpressionService: jest.Mocked<ResolverConfigExpressionService>;
|
||||
|
||||
const mockResolverImplementation: jest.Mocked<ICredentialResolver> = {
|
||||
const mockResolverImplementation = {
|
||||
metadata: {
|
||||
name: 'test.resolver',
|
||||
description: 'A test resolver',
|
||||
@@ -31,7 +31,8 @@ describe('DynamicCredentialResolverService', () => {
|
||||
getSecret: jest.fn(),
|
||||
setSecret: jest.fn(),
|
||||
validateOptions: jest.fn(),
|
||||
};
|
||||
deleteAllSecrets: jest.fn(),
|
||||
} as jest.Mocked<ICredentialResolver>;
|
||||
|
||||
const createMockEntity = (
|
||||
overrides: Partial<DynamicCredentialResolver> = {},
|
||||
@@ -428,6 +429,115 @@ describe('DynamicCredentialResolverService', () => {
|
||||
// Verify expression service was called with canUseExternalSecrets=false
|
||||
expect(mockExpressionService.resolve).toHaveBeenCalledWith(newConfig, false);
|
||||
});
|
||||
|
||||
it('should call deleteAllSecrets when clearCredentials is true', async () => {
|
||||
const entity = createMockEntity();
|
||||
const updatedEntity = createMockEntity();
|
||||
const mockUser = createMockUser();
|
||||
const decryptedConfig = { prefix: 'test' };
|
||||
const resolverWithDeleteAllSecrets = {
|
||||
...mockResolverImplementation,
|
||||
deleteAllSecrets: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
mockRepository.findOneBy.mockResolvedValue(entity);
|
||||
mockRegistry.getResolverByTypename.mockReturnValue(
|
||||
resolverWithDeleteAllSecrets as jest.Mocked<ICredentialResolver>,
|
||||
);
|
||||
mockRepository.save.mockResolvedValue(updatedEntity);
|
||||
mockCipher.decrypt.mockReturnValue(JSON.stringify(decryptedConfig));
|
||||
|
||||
await service.update('resolver-id-123', {
|
||||
clearCredentials: true,
|
||||
user: mockUser,
|
||||
});
|
||||
|
||||
expect(mockRegistry.getResolverByTypename).toHaveBeenCalledWith('test.resolver');
|
||||
expect(resolverWithDeleteAllSecrets.deleteAllSecrets).toHaveBeenCalledWith({
|
||||
resolverId: 'resolver-id-123',
|
||||
resolverName: 'test.resolver',
|
||||
configuration: decryptedConfig,
|
||||
});
|
||||
expect(mockRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not call deleteAllSecrets when clearCredentials is false', async () => {
|
||||
const entity = createMockEntity();
|
||||
const updatedEntity = createMockEntity();
|
||||
const mockUser = createMockUser();
|
||||
const decryptedConfig = { prefix: 'test' };
|
||||
|
||||
mockRepository.findOneBy.mockResolvedValue(entity);
|
||||
mockRepository.save.mockResolvedValue(updatedEntity);
|
||||
mockCipher.decrypt.mockReturnValue(JSON.stringify(decryptedConfig));
|
||||
|
||||
await service.update('resolver-id-123', {
|
||||
clearCredentials: false,
|
||||
user: mockUser,
|
||||
});
|
||||
|
||||
expect(mockRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not call deleteAllSecrets when clearCredentials is undefined', async () => {
|
||||
const entity = createMockEntity();
|
||||
const updatedEntity = createMockEntity();
|
||||
const mockUser = createMockUser();
|
||||
const decryptedConfig = { prefix: 'test' };
|
||||
|
||||
mockRepository.findOneBy.mockResolvedValue(entity);
|
||||
mockRepository.save.mockResolvedValue(updatedEntity);
|
||||
mockCipher.decrypt.mockReturnValue(JSON.stringify(decryptedConfig));
|
||||
|
||||
await service.update('resolver-id-123', {
|
||||
name: 'Updated Name',
|
||||
user: mockUser,
|
||||
});
|
||||
|
||||
expect(mockRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw CredentialResolverValidationError when resolver type is unknown and clearCredentials is true', async () => {
|
||||
const entity = createMockEntity();
|
||||
const mockUser = createMockUser();
|
||||
|
||||
mockRepository.findOneBy.mockResolvedValue(entity);
|
||||
mockRegistry.getResolverByTypename.mockReturnValue(undefined);
|
||||
|
||||
await expect(
|
||||
service.update('resolver-id-123', {
|
||||
clearCredentials: true,
|
||||
user: mockUser,
|
||||
}),
|
||||
).rejects.toThrow(CredentialResolverValidationError);
|
||||
|
||||
expect(mockRepository.save).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle resolver without deleteAllSecrets method gracefully', async () => {
|
||||
const entity = createMockEntity();
|
||||
const updatedEntity = createMockEntity();
|
||||
const mockUser = createMockUser();
|
||||
const decryptedConfig = { prefix: 'test' };
|
||||
const resolverWithoutDeleteAllSecrets = {
|
||||
...mockResolverImplementation,
|
||||
deleteAllSecrets: undefined,
|
||||
};
|
||||
|
||||
mockRepository.findOneBy.mockResolvedValue(entity);
|
||||
mockRegistry.getResolverByTypename.mockReturnValue(
|
||||
resolverWithoutDeleteAllSecrets as jest.Mocked<ICredentialResolver>,
|
||||
);
|
||||
mockRepository.save.mockResolvedValue(updatedEntity);
|
||||
mockCipher.decrypt.mockReturnValue(JSON.stringify(decryptedConfig));
|
||||
|
||||
await service.update('resolver-id-123', {
|
||||
clearCredentials: true,
|
||||
user: mockUser,
|
||||
});
|
||||
|
||||
expect(mockRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('delete', () => {
|
||||
|
||||
+17
@@ -27,6 +27,7 @@ export interface UpdateResolverParams {
|
||||
name?: string;
|
||||
type?: string;
|
||||
config?: CredentialResolverConfiguration;
|
||||
clearCredentials?: boolean;
|
||||
user: User;
|
||||
}
|
||||
|
||||
@@ -132,6 +133,22 @@ export class DynamicCredentialResolverService {
|
||||
existing.name = params.name;
|
||||
}
|
||||
|
||||
if (params.clearCredentials === true) {
|
||||
const resolver = this.registry.getResolverByTypename(existing.type);
|
||||
|
||||
if (!resolver) {
|
||||
throw new CredentialResolverValidationError(`Unknown resolver type: ${existing.type}`);
|
||||
}
|
||||
|
||||
if ('deleteAllSecrets' in resolver && typeof resolver.deleteAllSecrets === 'function') {
|
||||
await resolver.deleteAllSecrets({
|
||||
resolverId: id,
|
||||
resolverName: resolver.metadata.name,
|
||||
configuration: this.decryptConfig(existing.config),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const saved = await this.repository.save(existing);
|
||||
this.logger.debug(`Updated credential resolver "${saved.name}" (${saved.id})`);
|
||||
|
||||
|
||||
@@ -3259,6 +3259,12 @@
|
||||
"credentialResolverEdit.sidebar.details": "Details",
|
||||
"credentialResolverEdit.details.id": "ID",
|
||||
"credentialResolverEdit.details.notSaved": "Not saved yet",
|
||||
"credentialResolverEdit.clearCredentials.label": "Clear credentials",
|
||||
"credentialResolverEdit.clearCredentials.placeholder": "Select an option",
|
||||
"credentialResolverEdit.clearCredentials.yes": "Yes",
|
||||
"credentialResolverEdit.clearCredentials.no": "No",
|
||||
"credentialResolverEdit.clearCredentials.error.required": "Clear credentials selection is required",
|
||||
"credentialResolverEdit.clearCredentials.warning": "When making changes to a resolver, it is advised to clear the existing credentials for that resolver. Please choose whether to take that action.",
|
||||
"workflowSettings.timeSavedPerExecution": "Estimated time saved",
|
||||
"workflowSettings.timeSavedPerExecution.hint": "Minutes per production execution",
|
||||
"workflowSettings.timeSavedPerExecution.tooltip": "Total time savings are summarised in the Overview page.",
|
||||
|
||||
@@ -32,7 +32,12 @@ export async function createCredentialResolver(
|
||||
export async function updateCredentialResolver(
|
||||
context: IRestApiContext,
|
||||
resolverId: string,
|
||||
payload: { name: string; type: string; config: Record<string, unknown> },
|
||||
payload: {
|
||||
name: string;
|
||||
type: string;
|
||||
config: Record<string, unknown>;
|
||||
clearCredentials?: boolean;
|
||||
},
|
||||
): Promise<CredentialResolver> {
|
||||
return await makeRestApiRequest(context, 'PATCH', `/credential-resolvers/${resolverId}`, payload);
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ import type {
|
||||
ICredentialDataDecryptedObject,
|
||||
CredentialInformation,
|
||||
} from 'n8n-workflow';
|
||||
import { deepCopy } from 'n8n-workflow';
|
||||
import type { IUpdateInformation } from '@/Interface';
|
||||
import CredentialInputs from '@/features/credentials/components/CredentialEdit/CredentialInputs.vue';
|
||||
|
||||
@@ -52,10 +53,18 @@ const isSaving = ref(false);
|
||||
const resolverName = ref('');
|
||||
const resolverType = ref('');
|
||||
const resolverConfig = ref<Record<string, unknown>>({});
|
||||
const clearCredentials = ref<boolean | null>(null);
|
||||
const hasUnsavedChanges = ref(false);
|
||||
const errorMessage = ref<string>('');
|
||||
const mainContentRef = ref<HTMLElement>();
|
||||
|
||||
// Store original values to detect non-name changes
|
||||
const originalResolverName = ref('');
|
||||
const originalResolverType = ref('');
|
||||
const originalResolverConfig = ref<Record<string, unknown>>({});
|
||||
// Track if user has ever made a non-name change (stays true even if reverted)
|
||||
const hasEverMadeNonNameChange = ref(false);
|
||||
|
||||
const {
|
||||
resolverTypes: availableTypes,
|
||||
fetchResolverTypes: loadResolverTypes,
|
||||
@@ -159,8 +168,29 @@ const requiredPropertiesFilled = computed(() => {
|
||||
return true;
|
||||
});
|
||||
|
||||
// Check if non-name fields have changed
|
||||
const hasNonNameChanges = computed(() => {
|
||||
if (!isEditMode.value) return false;
|
||||
|
||||
const typeChanged = originalResolverType.value !== resolverType.value;
|
||||
const configChanged =
|
||||
JSON.stringify(originalResolverConfig.value) !== JSON.stringify(resolverConfig.value);
|
||||
|
||||
// Update the flag if there's a current change
|
||||
if (typeChanged || configChanged) {
|
||||
hasEverMadeNonNameChange.value = true;
|
||||
}
|
||||
|
||||
// Show dropdown if user has ever made a non-name change
|
||||
return hasEverMadeNonNameChange.value;
|
||||
});
|
||||
|
||||
const canSave = computed(() => {
|
||||
return resolverName.value.trim() !== '' && resolverType.value !== '';
|
||||
const baseCheck = resolverName.value.trim() !== '' && resolverType.value !== '';
|
||||
if (isEditMode.value && hasNonNameChanges.value) {
|
||||
return baseCheck && clearCredentials.value !== null;
|
||||
}
|
||||
return baseCheck;
|
||||
});
|
||||
|
||||
const sidebarItems = computed<IMenuItem[]>(() => [
|
||||
@@ -184,6 +214,15 @@ const loadResolver = async () => {
|
||||
resolverName.value = resolver.name;
|
||||
resolverType.value = resolver.type;
|
||||
resolverConfig.value = resolver.decryptedConfig || {};
|
||||
|
||||
// Store original values for change detection
|
||||
originalResolverName.value = resolver.name;
|
||||
originalResolverType.value = resolver.type;
|
||||
originalResolverConfig.value = deepCopy(resolver.decryptedConfig || {});
|
||||
|
||||
// Reset the flag when loading a resolver
|
||||
hasEverMadeNonNameChange.value = false;
|
||||
clearCredentials.value = null;
|
||||
} catch (error) {
|
||||
toast.showError(error, i18n.baseText('credentialResolverEdit.error.save'));
|
||||
}
|
||||
@@ -208,13 +247,34 @@ const save = async () => {
|
||||
return;
|
||||
}
|
||||
|
||||
if (isEditMode.value && hasNonNameChanges.value && clearCredentials.value === null) {
|
||||
errorMessage.value = i18n.baseText('credentialResolverEdit.clearCredentials.error.required');
|
||||
isSaving.value = false;
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const payload = {
|
||||
const payload: {
|
||||
name: string;
|
||||
type: string;
|
||||
config: Record<string, unknown>;
|
||||
clearCredentials?: boolean;
|
||||
} = {
|
||||
name: resolverName.value.trim(),
|
||||
type: resolverType.value,
|
||||
config: resolverConfig.value,
|
||||
};
|
||||
|
||||
// Include clearCredentials if non-name fields changed, otherwise default to false
|
||||
if (isEditMode.value) {
|
||||
if (hasNonNameChanges.value && clearCredentials.value !== null) {
|
||||
payload.clearCredentials = clearCredentials.value;
|
||||
} else if (!hasNonNameChanges.value) {
|
||||
// Only name changed, default to false
|
||||
payload.clearCredentials = false;
|
||||
}
|
||||
}
|
||||
|
||||
let savedResolver: CredentialResolver;
|
||||
if (isEditMode.value && props.data?.resolverId) {
|
||||
savedResolver = await updateCredentialResolver(
|
||||
@@ -255,6 +315,10 @@ const onConfigUpdate = (updateData: IUpdateInformation) => {
|
||||
};
|
||||
hasUnsavedChanges.value = true;
|
||||
errorMessage.value = '';
|
||||
// Reset clearCredentials when config changes to force user to make a choice
|
||||
if (isEditMode.value) {
|
||||
clearCredentials.value = null;
|
||||
}
|
||||
};
|
||||
|
||||
const onNameEdit = (newName: string) => {
|
||||
@@ -299,6 +363,7 @@ onMounted(async () => {
|
||||
} else {
|
||||
// Set default name for new resolvers
|
||||
resolverName.value = i18n.baseText('credentialResolverEdit.defaultName');
|
||||
clearCredentials.value = null;
|
||||
}
|
||||
isLoading.value = false;
|
||||
});
|
||||
@@ -374,6 +439,43 @@ onMounted(async () => {
|
||||
{{ errorMessage }}
|
||||
</N8nCallout>
|
||||
|
||||
<div v-if="isEditMode && hasNonNameChanges" :class="$style.formGroup">
|
||||
<label :class="$style.label">
|
||||
{{ i18n.baseText('credentialResolverEdit.clearCredentials.label') }}
|
||||
</label>
|
||||
<N8nSelect
|
||||
v-model="clearCredentials"
|
||||
:placeholder="i18n.baseText('credentialResolverEdit.clearCredentials.placeholder')"
|
||||
data-test-id="credential-resolver-clear-credentials-select"
|
||||
@update:model-value="
|
||||
() => {
|
||||
hasUnsavedChanges = true;
|
||||
errorMessage = '';
|
||||
}
|
||||
"
|
||||
>
|
||||
<N8nOption
|
||||
:label="i18n.baseText('credentialResolverEdit.clearCredentials.yes')"
|
||||
:value="true"
|
||||
>
|
||||
</N8nOption>
|
||||
<N8nOption
|
||||
:label="i18n.baseText('credentialResolverEdit.clearCredentials.no')"
|
||||
:value="false"
|
||||
>
|
||||
</N8nOption>
|
||||
</N8nSelect>
|
||||
</div>
|
||||
|
||||
<N8nCallout
|
||||
v-if="isEditMode && hasNonNameChanges"
|
||||
theme="warning"
|
||||
:class="$style.warningAlert"
|
||||
data-test-id="credential-resolver-clear-credentials-warning"
|
||||
>
|
||||
{{ i18n.baseText('credentialResolverEdit.clearCredentials.warning') }}
|
||||
</N8nCallout>
|
||||
|
||||
<div :class="$style.formGroup">
|
||||
<label :class="$style.label">
|
||||
{{ i18n.baseText('credentialResolverEdit.type.label') }}
|
||||
@@ -386,6 +488,7 @@ onMounted(async () => {
|
||||
() => {
|
||||
hasUnsavedChanges = true;
|
||||
errorMessage = '';
|
||||
clearCredentials = null;
|
||||
}
|
||||
"
|
||||
>
|
||||
@@ -523,4 +626,8 @@ onMounted(async () => {
|
||||
.errorAlert {
|
||||
margin-bottom: var(--spacing--md);
|
||||
}
|
||||
|
||||
.warningAlert {
|
||||
margin-bottom: var(--spacing--md);
|
||||
}
|
||||
</style>
|
||||
|
||||
Reference in New Issue
Block a user