diff --git a/packages/@n8n/api-types/src/dto/credential-resolver/update-credential-resolver.dto.ts b/packages/@n8n/api-types/src/dto/credential-resolver/update-credential-resolver.dto.ts index 6a89b608c53..8cb4b4533f4 100644 --- a/packages/@n8n/api-types/src/dto/credential-resolver/update-credential-resolver.dto.ts +++ b/packages/@n8n/api-types/src/dto/credential-resolver/update-credential-resolver.dto.ts @@ -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(), }) {} diff --git a/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts b/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts index c3a84246544..a0c9e4401a1 100644 --- a/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts +++ b/packages/@n8n/decorators/src/credential-resolver/credential-resolver.ts @@ -74,6 +74,13 @@ export interface ICredentialResolver { handle: CredentialResolverHandle, ): Promise; + /** + * Deletes all credential data for the resolver. + * Optional - not all resolvers support deletion. + * @throws {CredentialResolverError} When deletion operation fails + */ + deleteAllSecrets?(handle: CredentialResolverHandle): Promise; + /** * Validates resolver configuration before saving. * Should verify connectivity, authentication, and configuration structure. diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers.controller.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers.controller.ts index 28f4b9c3dd0..de9865f939d 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers.controller.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers.controller.ts @@ -119,6 +119,7 @@ export class CredentialResolversController { type: dto.type, name: dto.name, config: dto.config, + clearCredentials: dto.clearCredentials, user: req.user, }), ); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/oauth-credential-resolver.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/oauth-credential-resolver.ts index 7b32178036e..d2e2f3fa598 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/oauth-credential-resolver.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/oauth-credential-resolver.ts @@ -182,6 +182,10 @@ export class OAuthCredentialResolver implements ICredentialResolver { await this.storage.deleteCredentialData(credentialId, key, handle.resolverId, parsedOptions); } + async deleteAllSecrets(handle: CredentialResolverHandle): Promise { + await this.storage.deleteAllCredentialData(handle); + } + private async parseOptions(options: CredentialResolverConfiguration) { const result = await OAuthCredentialResolverOptionsSchema.safeParseAsync(options); if (result.error) { diff --git a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/storage/dynamic-credential-entry-storage.ts b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/storage/dynamic-credential-entry-storage.ts index 45bcfc82964..bcc719a2910 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/storage/dynamic-credential-entry-storage.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers/storage/dynamic-credential-entry-storage.ts @@ -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 { + await this.dynamicCredentialEntryRepository.delete({ resolverId: handle.resolverId }); + } } diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-resolver.service.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-resolver.service.test.ts index 56cdeccfbed..07c25198094 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-resolver.service.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-resolver.service.test.ts @@ -23,7 +23,7 @@ describe('DynamicCredentialResolverService', () => { let mockCipher: jest.Mocked; let mockExpressionService: jest.Mocked; - const mockResolverImplementation: jest.Mocked = { + 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; const createMockEntity = ( overrides: Partial = {}, @@ -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, + ); + 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, + ); + 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', () => { diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/credential-resolver.service.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/credential-resolver.service.ts index 569a7f23776..ecd3aeca130 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/credential-resolver.service.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/credential-resolver.service.ts @@ -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})`); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 8e4edaa952d..72fdcfd10db 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -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.", diff --git a/packages/frontend/@n8n/rest-api-client/src/api/credentialResolvers.ts b/packages/frontend/@n8n/rest-api-client/src/api/credentialResolvers.ts index b0b158680b1..72a3610ebfe 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/credentialResolvers.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/credentialResolvers.ts @@ -32,7 +32,12 @@ export async function createCredentialResolver( export async function updateCredentialResolver( context: IRestApiContext, resolverId: string, - payload: { name: string; type: string; config: Record }, + payload: { + name: string; + type: string; + config: Record; + clearCredentials?: boolean; + }, ): Promise { return await makeRestApiRequest(context, 'PATCH', `/credential-resolvers/${resolverId}`, payload); } diff --git a/packages/frontend/editor-ui/src/app/components/CredentialResolverEditModal.vue b/packages/frontend/editor-ui/src/app/components/CredentialResolverEditModal.vue index 917066b73e6..c76bf40ddbb 100644 --- a/packages/frontend/editor-ui/src/app/components/CredentialResolverEditModal.vue +++ b/packages/frontend/editor-ui/src/app/components/CredentialResolverEditModal.vue @@ -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>({}); +const clearCredentials = ref(null); const hasUnsavedChanges = ref(false); const errorMessage = ref(''); const mainContentRef = ref(); +// Store original values to detect non-name changes +const originalResolverName = ref(''); +const originalResolverType = ref(''); +const originalResolverConfig = ref>({}); +// 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(() => [ @@ -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; + 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 }} +
+ + + + + + + +
+ + + {{ i18n.baseText('credentialResolverEdit.clearCredentials.warning') }} + +