From 360ec09cf2a32da71dc595be2618a3e2d834dac8 Mon Sep 17 00:00:00 2001 From: Savelii Date: Mon, 5 Oct 2026 11:23:06 +0000 Subject: [PATCH] fix(editor): Enable Save when SSO role provisioning switches back to manual (#39925) Co-authored-by: Claude Fable 5.1 --- .../frontend/@n8n/i18n/src/locales/en.json | 3 +- .../ConfirmProvisioningDialog.test.ts | 187 ++++++++++++++---- .../components/ConfirmProvisioningDialog.vue | 25 +-- 3 files changed, 159 insertions(+), 56 deletions(-) diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 3cd9fc81f06..44f94283369 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -4279,7 +4279,8 @@ "settings.provisioningConfirmDialog.button.downloadProjectRolesCsv": "Existing project role access settings CSV", "settings.provisioningConfirmDialog.button.downloadInstanceRolesCsv": "Existing instance role settings CSV", "settings.provisioningConfirmDialog.projectRulesDeletion.warning": "Existing project mapping rules will be permanently deleted.", - "settings.provisioningConfirmDialog.projectRulesDeletion.description": "Saving this change removes all project role mapping rules. Download the project roles CSV above before saving so you can recreate them if needed.", + "settings.provisioningConfirmDialog.projectRulesDeletion.description.enable": "Saving this change removes all project role mapping rules. Download the project roles CSV above before saving so you can recreate them if needed.", + "settings.provisioningConfirmDialog.projectRulesDeletion.description.disable": "Saving this change removes all project role mapping rules. If you switch back to project role provisioning later, you will need to create them again.", "settings.provisioningInstanceRolesHandledBySsoProvider.description": "User management and instance roles are controlled by your SSO provider. Contact your n8n instance owner or admin to make changes.", "settings.provisioningInstanceRolesHandledByExpressionMapping.description": "Instance roles are managed automatically by expression-based role mapping rules. Manual role changes are disabled.", "settings.projectRolesManaged.description": "Project roles are managed automatically. Manual role changes are disabled.", diff --git a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.test.ts b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.test.ts index cb9b1f92020..3063e3054a5 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.test.ts +++ b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.test.ts @@ -1,19 +1,23 @@ import { createTestingPinia } from '@pinia/testing'; import { screen } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import { nextTick } from 'vue'; import { createComponentRenderer } from '@/__tests__/render'; import ConfirmProvisioningDialog from './ConfirmProvisioningDialog.vue'; -const downloadInstanceRolesCsv = vi.fn().mockResolvedValue(undefined); -const downloadProjectRolesCsv = vi.fn().mockResolvedValue(undefined); +const csvExport = await vi.hoisted(async () => { + const { ref } = await import('vue'); + return { + hasDownloadedInstanceRoleCsv: ref(false), + hasDownloadedProjectRoleCsv: ref(false), + downloadInstanceRolesCsv: vi.fn().mockResolvedValue(undefined), + downloadProjectRolesCsv: vi.fn().mockResolvedValue(undefined), + accessSettingsCsvExportOnModalClose: vi.fn(), + }; +}); vi.mock('@/features/settings/sso/provisioning/composables/useAccessSettingsCsvExport', () => ({ - useAccessSettingsCsvExport: () => ({ - hasDownloadedInstanceRoleCsv: false, - hasDownloadedProjectRoleCsv: false, - downloadInstanceRolesCsv, - downloadProjectRolesCsv, - accessSettingsCsvExportOnModalClose: vi.fn(), - }), + useAccessSettingsCsvExport: () => csvExport, })); const renderDialog = createComponentRenderer(ConfirmProvisioningDialog, { @@ -27,47 +31,144 @@ const baseProps = { authProtocol: 'saml' as const, }; -describe('ConfirmProvisioningDialog — project rules deletion warning', () => { +describe('ConfirmProvisioningDialog', () => { + beforeEach(() => { + csvExport.hasDownloadedInstanceRoleCsv.value = false; + csvExport.hasDownloadedProjectRoleCsv.value = false; + }); + afterEach(() => { // ElDialog teleports content to body — clean up between tests. document.body.innerHTML = ''; }); - it('does not render the deletion warning when willDeleteProjectRules is absent (defaults to false)', async () => { - renderDialog({ props: baseProps }); - await screen.findByTestId('provisioning-confirmation-checkbox'); + describe('project rules deletion warning', () => { + it('does not render the deletion warning when willDeleteProjectRules is absent (defaults to false)', async () => { + renderDialog({ props: baseProps }); + await screen.findByTestId('provisioning-confirmation-checkbox'); - expect(screen.queryByTestId('provisioning-project-rules-deletion-warning')).toBeNull(); - }); - - it('does not render the deletion warning when willDeleteProjectRules is explicitly false', async () => { - renderDialog({ props: { ...baseProps, willDeleteProjectRules: false } }); - await screen.findByTestId('provisioning-confirmation-checkbox'); - - expect(screen.queryByTestId('provisioning-project-rules-deletion-warning')).toBeNull(); - }); - - it('renders the deletion warning callout when willDeleteProjectRules is true', async () => { - renderDialog({ props: { ...baseProps, willDeleteProjectRules: true } }); - - const warning = await screen.findByTestId('provisioning-project-rules-deletion-warning'); - expect(warning).toBeInTheDocument(); - expect(warning).toHaveTextContent( - 'Existing project mapping rules will be permanently deleted.', - ); - }); - - it('renders the deletion warning in the switchToManual flow too (disabling SSO with project rules)', async () => { - renderDialog({ - props: { - ...baseProps, - transitionType: 'switchToManual' as const, - willDeleteProjectRules: true, - }, + expect(screen.queryByTestId('provisioning-project-rules-deletion-warning')).toBeNull(); }); - expect( - await screen.findByTestId('provisioning-project-rules-deletion-warning'), - ).toBeInTheDocument(); + it('does not render the deletion warning when willDeleteProjectRules is explicitly false', async () => { + renderDialog({ props: { ...baseProps, willDeleteProjectRules: false } }); + await screen.findByTestId('provisioning-confirmation-checkbox'); + + expect(screen.queryByTestId('provisioning-project-rules-deletion-warning')).toBeNull(); + }); + + it('renders the deletion warning callout when willDeleteProjectRules is true', async () => { + renderDialog({ props: { ...baseProps, willDeleteProjectRules: true } }); + + const warning = await screen.findByTestId('provisioning-project-rules-deletion-warning'); + expect(warning).toBeInTheDocument(); + expect(warning).toHaveTextContent( + 'Existing project mapping rules will be permanently deleted.', + ); + }); + + it('points to the CSV download control in the backup flow', async () => { + renderDialog({ props: { ...baseProps, willDeleteProjectRules: true } }); + + const warning = await screen.findByTestId('provisioning-project-rules-deletion-warning'); + expect(warning).toHaveTextContent('Download the project roles CSV above'); + expect(screen.getByTestId('provisioning-download-project-roles-csv-button')).toBeVisible(); + }); + + it('renders the deletion warning in the switchToManual flow too (disabling SSO with project rules)', async () => { + renderDialog({ + props: { + ...baseProps, + transitionType: 'switchToManual' as const, + willDeleteProjectRules: true, + }, + }); + + expect( + await screen.findByTestId('provisioning-project-rules-deletion-warning'), + ).toBeInTheDocument(); + }); + + it('does not point to a CSV control in the switchToManual flow, where none is rendered', async () => { + renderDialog({ + props: { + ...baseProps, + transitionType: 'switchToManual' as const, + willDeleteProjectRules: true, + }, + }); + + const warning = await screen.findByTestId('provisioning-project-rules-deletion-warning'); + expect(warning).not.toHaveTextContent('CSV'); + expect(warning).toHaveTextContent( + 'If you switch back to project role provisioning later, you will need to create them again.', + ); + expect(screen.queryByTestId('provisioning-download-project-roles-csv-button')).toBeNull(); + }); + }); + + describe('save button gating', () => { + it('switchToManual: enables Save on the confirmation checkbox alone, even when project roles were stored', async () => { + const { emitted } = renderDialog({ + props: { + ...baseProps, + transitionType: 'switchToManual' as const, + showProjectRolesCsv: true, + willDeleteProjectRules: true, + }, + }); + + const checkbox = await screen.findByTestId('provisioning-confirmation-checkbox'); + const saveButton = screen.getByTestId('provisioning-confirm-button'); + + expect(screen.queryByTestId('provisioning-download-instance-roles-csv-button')).toBeNull(); + expect(screen.queryByTestId('provisioning-download-project-roles-csv-button')).toBeNull(); + expect(checkbox).toBeEnabled(); + expect(saveButton).toBeDisabled(); + + await userEvent.click(checkbox); + expect(saveButton).toBeEnabled(); + + await userEvent.click(saveButton); + expect(emitted('confirmProvisioning')).toHaveLength(1); + }); + + it('backup with project roles: keeps Save disabled until both CSVs are downloaded', async () => { + renderDialog({ props: baseProps }); + + const checkbox = await screen.findByTestId('provisioning-confirmation-checkbox'); + const saveButton = screen.getByTestId('provisioning-confirm-button'); + expect(checkbox).toBeDisabled(); + expect(saveButton).toBeDisabled(); + + csvExport.hasDownloadedInstanceRoleCsv.value = true; + await nextTick(); + expect(checkbox).toBeDisabled(); + expect(saveButton).toBeDisabled(); + + csvExport.hasDownloadedProjectRoleCsv.value = true; + await nextTick(); + expect(checkbox).toBeEnabled(); + expect(saveButton).toBeDisabled(); + + await userEvent.click(checkbox); + expect(saveButton).toBeEnabled(); + }); + + it('backup without project roles: needs the instance CSV only', async () => { + renderDialog({ props: { ...baseProps, showProjectRolesCsv: false } }); + + const checkbox = await screen.findByTestId('provisioning-confirmation-checkbox'); + const saveButton = screen.getByTestId('provisioning-confirm-button'); + expect(screen.queryByTestId('provisioning-download-project-roles-csv-button')).toBeNull(); + expect(checkbox).toBeDisabled(); + + csvExport.hasDownloadedInstanceRoleCsv.value = true; + await nextTick(); + expect(checkbox).toBeEnabled(); + + await userEvent.click(checkbox); + expect(saveButton).toBeEnabled(); + }); }); }); diff --git a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.vue b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.vue index 45ca0f9382f..cb1b012ccb6 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.vue +++ b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/components/ConfirmProvisioningDialog.vue @@ -43,6 +43,14 @@ const messagingKey = computed(() => (isSwitchingToManual.value ? 'disable' : 'en const shouldShowProjectRolesCsv = computed(() => props.showProjectRolesCsv); +// The manual flow renders no CSV controls, so it must never wait for a download. +const isCsvBackupPending = computed( + () => + !isSwitchingToManual.value && + (!hasDownloadedInstanceRoleCsv.value || + (shouldShowProjectRolesCsv.value && !hasDownloadedProjectRoleCsv.value)), +); + watch(visible, () => { loading.value = false; confirmationChecked.value = false; @@ -157,7 +165,9 @@ const onConfirmProvisioningSetting = () => { }}
{{ - locale.baseText('settings.provisioningConfirmDialog.projectRulesDeletion.description') + locale.baseText( + `settings.provisioningConfirmDialog.projectRulesDeletion.description.${messagingKey}`, + ) }} @@ -165,11 +175,7 @@ const onConfirmProvisioningSetting = () => {