fix(editor): Only show role assignment warning modal when value actually changed (#28387)

This commit is contained in:
Csaba Tuncsik 2026-04-14 15:32:44 +02:00 committed by GitHub
parent ac41112731
commit 9c97931ca0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 135 additions and 157 deletions

View file

@ -3115,22 +3115,18 @@
"settings.provisioning.scopesProjectsRolesClaimName.help": "The claim name used to provision projects and their roles from Oauth. For SAML / LDAP, this will be the attribute name checked.",
"settings.provisioning.toggle": "Provision instance and project roles",
"settings.provisioning.toggle.help": "Project access can only be defined on external provider. Any existing project access configured in n8n, but not on the provider, will be removed once a user logs in.",
"settings.provisioningConfirmDialog.enable.title": "Enable user role provisioning",
"settings.provisioningConfirmDialog.enable.title": "Change role assignment method",
"settings.provisioningConfirmDialog.enable.description": "You're changing how roles are assigned for SSO users. Download your current role and access mappings as a backup before saving to ensure your SSO provider is configured correctly.",
"settings.provisioningConfirmDialog.enable.checkbox": "I have downloaded and reviewed the CSV export. My SSO provider is correctly configured for the new assignment method.",
"settings.provisioningConfirmDialog.disable.title": "Manage user role provisioning in n8n only",
"settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partOne": "Your SSO provider will control all user roles in n8n.",
"settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partOne.withProjectRoles": "Your SSO provider will control all user and project roles in n8n.",
"settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partTwo": "Roles not assigned by your SSO provider will default to global:member.",
"settings.provisioningConfirmDialog.breakingChangeDescription.secondLine": "<b>Before enabling:</b> Download and review your current access settings below to ensure your SSO provider is configured correctly.",
"settings.provisioningConfirmDialog.disable.description": "You're switching instance role management from SSO back to n8n.",
"settings.provisioningConfirmDialog.enable.checkbox": "I have downloaded and reviewed the CSV export. My SSO provider is correctly configured to control user roles on this n8n instance.",
"settings.provisioningConfirmDialog.disable.checkbox": "I confirm that I want to no longer provision user roles from my SSO provider.",
"settings.provisioningConfirmDialog.disable.description": "Roles and project access for SSO users will no longer be provided by your IdP. Users will be assigned roles and project access directly in n8n.",
"settings.provisioningConfirmDialog.disable.checkbox": "I confirm that I want to manage user roles and access directly in n8n, and no longer provision them from my SSO provider.",
"settings.provisioningConfirmDialog.link.docs": "Link to docs",
"settings.provisioningConfirmDialog.button.enable.confirm": "Save and enable",
"settings.provisioningConfirmDialog.button.enable.confirm": "Save changes",
"settings.provisioningConfirmDialog.button.disable.confirm": "Save",
"settings.provisioningConfirmDialog.button.cancel": "Cancel",
"settings.provisioningConfirmDialog.button.generateCsvExport": "Generate access settings CSV export",
"settings.provisioningConfirmDialog.button.downloadProjectRolesCsv": "Existing project access settings csv",
"settings.provisioningConfirmDialog.button.downloadInstanceRolesCsv": "Existing instance role settings csv",
"settings.provisioningConfirmDialog.button.downloadProjectRolesCsv": "Existing project role access settings CSV",
"settings.provisioningConfirmDialog.button.downloadInstanceRolesCsv": "Existing instance role settings CSV",
"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.provisioningProjectRolesHandledBySsoProvider.description": "User management and project roles are controlled by your SSO provider. Contact your n8n instance owner or admin to make changes.",

View file

@ -38,7 +38,9 @@ const {
formValue: userRoleProvisioning,
isUserRoleProvisioningChanged,
saveProvisioningConfig,
shouldPromptUserToConfirmUserRoleProvisioningChange,
roleAssignmentTransition,
storedHasProjectRoles,
revertRoleAssignment,
} = useUserRoleProvisioningForm(SupportedProtocols.OIDC);
type PromptType = 'login' | 'none' | 'consent' | 'select_account' | 'create';
@ -114,13 +116,7 @@ const cannotSaveOidcSettings = computed(() => {
});
async function onOidcSettingsSave(provisioningChangesConfirmed: boolean = false) {
if (
!provisioningChangesConfirmed &&
shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: !!ssoStore.oidcConfig?.loginEnabled,
loginEnabledFormValue: ssoStore.isOidcLoginEnabled,
})
) {
if (!provisioningChangesConfirmed && roleAssignmentTransition.value !== 'none') {
showUserRoleProvisioningDialog.value = true;
return;
}
@ -314,10 +310,14 @@ onMounted(async () => {
/>
<ConfirmProvisioningDialog
v-model="showUserRoleProvisioningDialog"
:new-provisioning-setting="userRoleProvisioning"
:transition-type="roleAssignmentTransition"
:show-project-roles-csv="storedHasProjectRoles || roleAssignment === 'instance_and_project'"
auth-protocol="oidc"
@confirm-provisioning="onOidcSettingsSave(true)"
@cancel="showUserRoleProvisioningDialog = false"
@cancel="
revertRoleAssignment();
showUserRoleProvisioningDialog = false;
"
/>
<div :class="$style.group">
<label>Authentication Context Class Reference</label>

View file

@ -73,7 +73,9 @@ const {
formValue: userRoleProvisioning,
isUserRoleProvisioningChanged,
saveProvisioningConfig,
shouldPromptUserToConfirmUserRoleProvisioningChange,
roleAssignmentTransition,
storedHasProjectRoles,
revertRoleAssignment,
} = useUserRoleProvisioningForm(SupportedProtocols.SAML);
async function loadSamlConfig() {
@ -212,13 +214,7 @@ const onSave = async (provisioningChangesConfirmed: boolean = false) => {
}
}
if (
!provisioningChangesConfirmed &&
shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: !!ssoStore.isSamlLoginEnabled,
loginEnabledFormValue: samlLoginEnabled.value,
})
) {
if (!provisioningChangesConfirmed && roleAssignmentTransition.value !== 'none') {
showUserRoleProvisioningDialog.value = true;
return;
}
@ -405,10 +401,14 @@ onMounted(async () => {
/>
<ConfirmProvisioningDialog
v-model="showUserRoleProvisioningDialog"
:new-provisioning-setting="userRoleProvisioning"
:transition-type="roleAssignmentTransition"
:show-project-roles-csv="storedHasProjectRoles || roleAssignment === 'instance_and_project'"
auth-protocol="saml"
@confirm-provisioning="onSave(true)"
@cancel="showUserRoleProvisioningDialog = false"
@cancel="
revertRoleAssignment();
showUserRoleProvisioningDialog = false;
"
/>
</div>

View file

@ -4,13 +4,14 @@ import { ElDialog } from 'element-plus';
import { N8nButton, N8nCard, N8nCheckbox, N8nIcon, N8nText } from '@n8n/design-system';
import { ref, watch, computed } from 'vue';
import { useAccessSettingsCsvExport } from '@/features/settings/sso/provisioning/composables/useAccessSettingsCsvExport';
import type { UserRoleProvisioningSetting } from './UserRoleProvisioningDropdown.vue';
import type { RoleAssignmentTransitionType } from '../composables/useUserRoleProvisioningForm';
import type { SupportedProtocolType } from '../../sso.store';
const visible = defineModel<boolean>();
const props = defineProps<{
newProvisioningSetting: UserRoleProvisioningSetting;
transitionType: RoleAssignmentTransitionType;
showProjectRolesCsv: boolean;
authProtocol: SupportedProtocolType;
}>();
@ -32,13 +33,11 @@ const {
accessSettingsCsvExportOnModalClose,
} = useAccessSettingsCsvExport();
const isDisablingProvisioning = computed(() => props.newProvisioningSetting === 'disabled');
const isSwitchingToManual = computed(() => props.transitionType === 'switchToManual');
const messagingKey = computed(() => (isDisablingProvisioning.value ? 'disable' : 'enable'));
const messagingKey = computed(() => (isSwitchingToManual.value ? 'disable' : 'enable'));
const shouldShowProjectRolesCsv = computed(
() => props.newProvisioningSetting === 'instance_and_project_roles',
);
const shouldShowProjectRolesCsv = computed(() => props.showProjectRolesCsv);
watch(visible, () => {
loading.value = false;
@ -75,24 +74,11 @@ const onConfirmProvisioningSetting = () => {
:title="locale.baseText(`settings.provisioningConfirmDialog.${messagingKey}.title`)"
width="650"
>
<template v-if="!isDisablingProvisioning">
<template v-if="!isSwitchingToManual">
<div class="mb-s">
<N8nText color="text-base"
>{{
locale.baseText(
newProvisioningSetting === 'instance_and_project_roles'
? 'settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partOne.withProjectRoles'
: 'settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partOne',
)
}}
</N8nText>
<N8nText :class="$style.descriptionTextPartTwo" color="text-base">
{{
locale.baseText(
'settings.provisioningConfirmDialog.breakingChangeDescription.firstSentence.partTwo',
)
}}</N8nText
>
<N8nText color="text-base">{{
locale.baseText('settings.provisioningConfirmDialog.enable.description')
}}</N8nText>
</div>
<div class="mb-s">
<N8nText color="text-base"
@ -103,16 +89,6 @@ const onConfirmProvisioningSetting = () => {
></N8nText
>
</div>
<div class="mb-s">
<N8nText
v-n8n-html="
locale.baseText(
'settings.provisioningConfirmDialog.breakingChangeDescription.secondLine',
)
"
color="text-base"
></N8nText>
</div>
<ul :class="$style.list" class="mb-s">
<li>
<N8nText color="text-base">{{
@ -171,7 +147,7 @@ const onConfirmProvisioningSetting = () => {
<N8nCheckbox
v-model="confirmationChecked"
:disabled="
!isDisablingProvisioning &&
!isSwitchingToManual &&
(!hasDownloadedInstanceRoleCsv ||
(shouldShowProjectRolesCsv && !hasDownloadedProjectRoleCsv))
"
@ -201,7 +177,7 @@ const onConfirmProvisioningSetting = () => {
:disabled="
loading ||
!confirmationChecked ||
(!isDisablingProvisioning && !hasDownloadedInstanceRoleCsv) ||
(!isSwitchingToManual && !hasDownloadedInstanceRoleCsv) ||
(shouldShowProjectRolesCsv && !hasDownloadedProjectRoleCsv)
"
data-test-id="provisioning-confirm-button"
@ -230,10 +206,6 @@ const onConfirmProvisioningSetting = () => {
background-color: var(--color--background--light-1);
}
.descriptionTextPartTwo {
margin-left: 4px;
}
.icon {
height: 32px; // to match height of download button
margin: 0 var(--spacing--lg);

View file

@ -307,97 +307,92 @@ describe('useUserRoleProvisioningForm', () => {
});
});
describe('shouldPromptUserToConfirmUserRoleProvisioningChange', () => {
it('should return false when disabling SSO', async () => {
describe('roleAssignmentTransition', () => {
it('should return none when dropdown has not changed', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const { shouldPromptUserToConfirmUserRoleProvisioningChange } =
useUserRoleProvisioningForm('oidc');
const result = shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: true,
loginEnabledFormValue: false,
});
expect(result).toEqual(false);
});
it('should return false when enabling SSO without provisioning enabled', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: false }),
);
const { shouldPromptUserToConfirmUserRoleProvisioningChange } =
useUserRoleProvisioningForm('oidc');
const result = shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: false,
loginEnabledFormValue: true,
});
expect(result).toEqual(false);
});
it('should return true when enabling SSO with user role provisioning enabled', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const { formValue, shouldPromptUserToConfirmUserRoleProvisioningChange } =
useUserRoleProvisioningForm('oidc');
const { formValue, roleAssignmentTransition } = useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('instance_role'));
const result = shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: false,
loginEnabledFormValue: true,
});
expect(result).toEqual(true);
expect(roleAssignmentTransition.value).toBe('none');
});
it('should return true when changing provisioning setting while SSO is active', async () => {
it('should return backup when changing from manual to SSO option', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({}),
);
const { formValue, roleAssignment, roleAssignmentTransition } =
useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('disabled'));
roleAssignment.value = 'instance';
expect(roleAssignmentTransition.value).toBe('backup');
});
it('should return backup when changing between SSO options', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const { roleAssignment, formValue, shouldPromptUserToConfirmUserRoleProvisioningChange } =
const { formValue, roleAssignment, roleAssignmentTransition } =
useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('instance_role'));
roleAssignment.value = 'instance_and_project';
const result = shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: true,
loginEnabledFormValue: true,
});
expect(result).toEqual(true);
expect(roleAssignmentTransition.value).toBe('backup');
});
it('should return true when enabling SSO with expression_based provisioning enabled', async () => {
it('should return backup when changing mapping method', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({
scopesProvisionInstanceRole: false,
scopesProvisionProjectRoles: false,
}),
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const {
roleAssignment,
mappingMethod,
formValue,
shouldPromptUserToConfirmUserRoleProvisioningChange,
} = useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('disabled'));
const { formValue, mappingMethod, roleAssignmentTransition } =
useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('instance_role'));
mappingMethod.value = 'rules_in_n8n';
expect(roleAssignmentTransition.value).toBe('backup');
});
it('should return switchToManual when changing from SSO option to manual', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const { formValue, roleAssignment, roleAssignmentTransition } =
useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('instance_role'));
roleAssignment.value = 'manual';
expect(roleAssignmentTransition.value).toBe('switchToManual');
});
});
describe('revertRoleAssignment', () => {
it('should reset dropdown to stored values', async () => {
vi.mocked(provisioningApi.getProvisioningConfig).mockResolvedValue(
mockProvisioningConfig({ scopesProvisionInstanceRole: true }),
);
const { formValue, roleAssignment, mappingMethod, revertRoleAssignment } =
useUserRoleProvisioningForm('oidc');
await vi.waitFor(() => expect(formValue.value).toBe('instance_role'));
roleAssignment.value = 'instance_and_project';
mappingMethod.value = 'rules_in_n8n';
const result = shouldPromptUserToConfirmUserRoleProvisioningChange({
currentLoginEnabled: false,
loginEnabledFormValue: true,
});
revertRoleAssignment();
expect(result).toEqual(true);
expect(roleAssignment.value).toBe('instance');
expect(mappingMethod.value).toBe('idp');
});
});
});

View file

@ -11,6 +11,8 @@ import { type SupportedProtocolType } from '../../sso.store';
import { useTelemetry } from '@/app/composables/useTelemetry';
import { useRootStore } from '@n8n/stores/useRootStore';
export type RoleAssignmentTransitionType = 'none' | 'backup' | 'switchToManual';
type DropdownValues = {
roleAssignment: RoleAssignmentSetting;
mappingMethod: RoleMappingMethodSetting;
@ -87,6 +89,11 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
const roleAssignment = ref<RoleAssignmentSetting>('manual');
const mappingMethod = ref<RoleMappingMethodSetting>('idp');
const storedHasProjectRules = ref(false);
const storedValues = computed(() =>
getDropdownValuesFromConfig(provisioningStore.provisioningConfig, storedHasProjectRules.value),
);
const formValue = computed<UserRoleProvisioningSetting>({
get: () => toLegacyValue(roleAssignment.value, mappingMethod.value),
@ -98,7 +105,7 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
});
const isUserRoleProvisioningChanged = computed<boolean>(() => {
const stored = getDropdownValuesFromConfig(provisioningStore.provisioningConfig);
const stored = storedValues.value;
return (
stored.roleAssignment !== roleAssignment.value || stored.mappingMethod !== mappingMethod.value
);
@ -120,7 +127,7 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
? 'idp'
: mappingMethod.value;
const stored = getDropdownValuesFromConfig(provisioningStore.provisioningConfig);
const stored = storedValues.value;
if (
effectiveRoleAssignment === stored.roleAssignment &&
effectiveMappingMethod === stored.mappingMethod
@ -140,23 +147,28 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
);
};
const shouldPromptUserToConfirmUserRoleProvisioningChange = ({
currentLoginEnabled,
loginEnabledFormValue,
}: {
currentLoginEnabled: boolean;
loginEnabledFormValue: boolean;
}) => {
const isLoginEnabledChanged = currentLoginEnabled !== loginEnabledFormValue;
const isEnablingSsoLogin = isLoginEnabledChanged && !currentLoginEnabled;
const isDisablingSsoLogin = isLoginEnabledChanged && currentLoginEnabled;
const isEnablingSsoAlongSideProvisioning = isEnablingSsoLogin && formValue.value !== 'disabled';
const isChangingProvisioningSettingWhileLoginWasAlreadyEnabled =
isUserRoleProvisioningChanged.value && currentLoginEnabled && !isDisablingSsoLogin;
const roleAssignmentTransition = computed<RoleAssignmentTransitionType>(() => {
const stored = storedValues.value;
if (
stored.roleAssignment === roleAssignment.value &&
stored.mappingMethod === mappingMethod.value
) {
return 'none';
}
if (roleAssignment.value === 'manual') {
return 'switchToManual';
}
return 'backup';
});
return (
isEnablingSsoAlongSideProvisioning || isChangingProvisioningSettingWhileLoginWasAlreadyEnabled
);
const storedHasProjectRoles = computed(
() => storedValues.value.roleAssignment === 'instance_and_project',
);
const revertRoleAssignment = () => {
const stored = storedValues.value;
roleAssignment.value = stored.roleAssignment;
mappingMethod.value = stored.mappingMethod;
};
const initFormValue = () => {
@ -170,6 +182,7 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
hasProjectRules = rules.some((r) => r.type === 'project');
}
storedHasProjectRules.value = hasProjectRules;
const values = getDropdownValuesFromConfig(config, hasProjectRules);
roleAssignment.value = values.roleAssignment;
mappingMethod.value = values.mappingMethod;
@ -184,6 +197,8 @@ export function useUserRoleProvisioningForm(protocol: SupportedProtocolType) {
formValue,
isUserRoleProvisioningChanged,
saveProvisioningConfig,
shouldPromptUserToConfirmUserRoleProvisioningChange,
roleAssignmentTransition,
storedHasProjectRoles,
revertRoleAssignment,
};
}