fix(editor): Show MFA section to instance owner, even when external a…

…uth is enabled (#9301)
n8n-io · May 3, 2024 · b65e0e2 · b65e0e2
1 parent d92f994
commit b65e0e2
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 23 deletions.
diff --git a/packages/editor-ui/src/views/SettingsPersonalView.vue b/packages/editor-ui/src/views/SettingsPersonalView.vue
@@ -32,7 +32,7 @@
 				/>
 			</div>
 		</div>
-		<div v-if="!signInWithLdap && !signInWithSaml">
+		<div v-if="isPersonalSecurityEnabled">
 			<div class="mb-s">
 				<n8n-heading size="large">{{ i18n.baseText('settings.personal.security') }}</n8n-heading>
 			</div>
@@ -43,7 +43,7 @@
 					}}</n8n-link>
 				</n8n-input-label>
 			</div>
-			<div v-if="isMfaFeatureEnabled">
+			<div v-if="isMfaFeatureEnabled" data-test-id="mfa-section">
 				<div class="mb-xs">
 					<n8n-input-label :label="$locale.baseText('settings.personal.mfa.section.title')" />
 					<n8n-text :bold="false" :class="$style.infoText">
@@ -171,7 +171,7 @@ export default defineComponent({
 					required: true,
 					autocomplete: 'given-name',
 					capitalize: true,
-					disabled: this.isLDAPFeatureEnabled && this.signInWithLdap,
+					disabled: this.isExternalAuthEnabled,
 				},
 			},
 			{
@@ -183,7 +183,7 @@ export default defineComponent({
 					required: true,
 					autocomplete: 'family-name',
 					capitalize: true,
-					disabled: this.isLDAPFeatureEnabled && this.signInWithLdap,
+					disabled: this.isExternalAuthEnabled,
 				},
 			},
 			{
@@ -196,7 +196,7 @@ export default defineComponent({
 					validationRules: [{ name: 'VALID_EMAIL' }],
 					autocomplete: 'email',
 					capitalize: true,
-					disabled: (this.isLDAPFeatureEnabled && this.signInWithLdap) || this.signInWithSaml,
+					disabled: !this.isPersonalSecurityEnabled,
 				},
 			},
 		];
@@ -206,16 +206,15 @@ export default defineComponent({
 		currentUser(): IUser | null {
 			return this.usersStore.currentUser;
 		},
-		signInWithLdap(): boolean {
-			return this.currentUser?.signInType === 'ldap';
+		isExternalAuthEnabled(): boolean {
+			const isLdapEnabled =
+				this.settingsStore.settings.enterprise.ldap && this.currentUser?.signInType === 'ldap';
+			const isSamlEnabled =
+				this.settingsStore.isSamlLoginEnabled && this.settingsStore.isDefaultAuthenticationSaml;
+			return isLdapEnabled || isSamlEnabled;
 		},
-		isLDAPFeatureEnabled(): boolean {
-			return this.settingsStore.settings.enterprise.ldap;
-		},
-		signInWithSaml(): boolean {
-			return (
-				this.settingsStore.isSamlLoginEnabled && this.settingsStore.isDefaultAuthenticationSaml
-			);
+		isPersonalSecurityEnabled(): boolean {
+			return this.usersStore.isInstanceOwner || !this.isExternalAuthEnabled;
 		},
 		mfaDisabled(): boolean {
 			return !this.usersStore.mfaEnabled;

diff --git a/packages/editor-ui/src/views/__tests__/SettingsPersonalView.test.ts b/packages/editor-ui/src/views/__tests__/SettingsPersonalView.test.ts
@@ -57,16 +57,37 @@ describe('SettingsPersonalView', () => {
 		expect(getByTestId('change-password-link')).toBeInTheDocument();
 	});
 
-	it('should disable email and pw change when SAML login is enabled', async () => {
-		vi.spyOn(settingsStore, 'isSamlLoginEnabled', 'get').mockReturnValue(true);
-		vi.spyOn(settingsStore, 'isDefaultAuthenticationSaml', 'get').mockReturnValue(true);
+	describe('when external auth is enabled, email and password change', () => {
+		beforeEach(() => {
+			vi.spyOn(settingsStore, 'isSamlLoginEnabled', 'get').mockReturnValue(true);
+			vi.spyOn(settingsStore, 'isDefaultAuthenticationSaml', 'get').mockReturnValue(true);
+			vi.spyOn(settingsStore, 'isMfaFeatureEnabled', 'get').mockReturnValue(true);
+		});
 
-		const { queryByTestId, getAllByRole } = renderComponent({ pinia });
-		await waitAllPromises();
+		it('should not be disabled for the instance owner', async () => {
+			vi.spyOn(usersStore, 'isInstanceOwner', 'get').mockReturnValue(true);
+
+			const { queryByTestId, getAllByRole } = renderComponent({ pinia });
+			await waitAllPromises();
+
+			expect(
+				getAllByRole('textbox').find((el) => el.getAttribute('type') === 'email'),
+			).toBeEnabled();
+			expect(queryByTestId('change-password-link')).toBeInTheDocument();
+			expect(queryByTestId('mfa-section')).toBeInTheDocument();
+		});
+
+		it('should be disabled for members', async () => {
+			vi.spyOn(usersStore, 'isInstanceOwner', 'get').mockReturnValue(false);
+
+			const { queryByTestId, getAllByRole } = renderComponent({ pinia });
+			await waitAllPromises();
 
-		expect(
-			getAllByRole('textbox').find((el) => el.getAttribute('type') === 'email'),
-		).toBeDisabled();
-		expect(queryByTestId('change-password-link')).not.toBeInTheDocument();
+			expect(
+				getAllByRole('textbox').find((el) => el.getAttribute('type') === 'email'),
+			).toBeDisabled();
+			expect(queryByTestId('change-password-link')).not.toBeInTheDocument();
+			expect(queryByTestId('mfa-section')).not.toBeInTheDocument();
+		});
 	});
 });