From b63ecf5fdd6099c17a24756bcd2db9a7e550f530 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?=
 =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?=
 =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?=
 <aditya@netroy.in>
Date: Fri, 3 May 2024 13:59:05 +0200
Subject: [PATCH] fix(editor): Show MFA section to instance owner, even when
 external auth is enabled

---
 .../src/views/SettingsPersonalView.vue        | 27 +++++++------
 .../__tests__/SettingsPersonalView.test.ts    | 39 ++++++++++++++-----
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/packages/editor-ui/src/views/SettingsPersonalView.vue b/packages/editor-ui/src/views/SettingsPersonalView.vue
index c985ee613980a..d2248b39d8179 100644
--- 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
index 09df7dfc216f3..7236800010e07 100644
--- 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();
+		});
 	});
 });