Merge pull request #56726 from nextcloud/artonge/fix/user_ldap/deleting_and_initial_config

fix(user_ldap): Improve case where selected config is undefined

Author: artonge

apps/user_ldap/lib/Controller/ConfigAPIController.php

@@ -250,7 +250,7 @@ public function testConfiguration(string $configID) {
 			$this->ensureConfigIDExists($configID);
 			$connection = $this->connectionFactory->get($configID);
 			$conf = $connection->getConfiguration();
-			if ($conf['ldap_configuration_active'] === '0') {
+			if ($conf['ldap_configuration_active'] !== '1') {
 				//needs to be true, otherwise it will also fail with an irritating message
 				$conf['ldap_configuration_active'] = '1';
 			}

apps/user_ldap/src/components/SettingsTabs/ServerTab.vue

@@ -47,7 +47,7 @@
 					type="number"
 					autocomplete="off"
 					@change="(event) => ldapConfigProxy.ldapPort = event.target.value" />
-				<NcButton :disabled="loadingGuessPortAndTLS" @click="guessPortAndTLS">
+				<NcButton :disabled="loadingGuessPortAndTLS || ldapConfigProxy.ldapHost === ''" @click="guessPortAndTLS">
 					{{ t('user_ldap', 'Detect Port') }}
 				</NcButton>
 			</div>
@@ -83,7 +83,7 @@
 				:helper-text="t('user_ldap', 'You can specify Base DN for users and groups in the Advanced tab')"
 				@change="(event) => ldapConfigProxy.ldapBase = event.target.value" />
 
-			<NcButton :disabled="loadingGuessBaseDN" @click="guessBaseDN">
+			<NcButton :disabled="loadingGuessBaseDN || needsToSaveCredentials" @click="guessBaseDN">
 				{{ t('user_ldap', 'Detect Base DN') }}
 			</NcButton>
 			<NcButton :disabled="loadingCountInBaseDN || ldapConfigProxy.ldapBase === ''" @click="countInBaseDN">
@@ -98,7 +98,7 @@ import { showInfo } from '@nextcloud/dialogs'
 import { n, t } from '@nextcloud/l10n'
 import { NcButton, NcCheckboxRadioSwitch, NcTextArea, NcTextField } from '@nextcloud/vue'
 import { storeToRefs } from 'pinia'
-import { computed, ref } from 'vue'
+import { computed, ref, watch } from 'vue'
 import ContentCopy from 'vue-material-design-icons/ContentCopy.vue'
 import Delete from 'vue-material-design-icons/Delete.vue'
 import { callWizard } from '../../services/ldapConfigService.ts'
@@ -120,6 +120,18 @@ const needsToSaveCredentials = computed(() => {
 	return ldapConfigProxy.value.ldapAgentName !== localLdapAgentName.value || ldapConfigProxy.value.ldapAgentPassword !== localLdapAgentPassword.value
 })
 
+watch(
+	ldapConfigProxy,
+	(newVal) => {
+		localLdapAgentName.value = newVal.ldapAgentName
+		if (newVal.ldapAgentPassword === '***') {
+			localLdapAgentPassword.value = ''
+		} else {
+			localLdapAgentPassword.value = newVal.ldapAgentPassword
+		}
+	},
+)
+
 /**
  *
  */
@@ -136,7 +148,7 @@ async function guessPortAndTLS() {
 		loadingGuessPortAndTLS.value = true
 		const { changes } = await callWizard('guessPortAndTLS', props.configId)
 		// Not using ldapConfigProxy to avoid triggering the save logic.
-		ldapConfigs.value[props.configId].ldapPort = (changes!.ldap_port as string) ?? ''
+		ldapConfigs.value[props.configId]!.ldapPort = (changes!.ldap_port as string) ?? ''
 	} finally {
 		loadingGuessPortAndTLS.value = false
 	}

apps/user_ldap/src/services/ldapConfigService.ts

@@ -174,11 +174,16 @@ export async function callWizard(action: WizardAction, configId: string, extraPa
 
 		return response.data.ocs.data
 	} catch (error) {
+		let message = t('user_ldap', 'An error occurred')
+
 		if (isAxiosError(error) && error.response?.data.ocs.meta.status === 'failure') {
-			const message = error.response.data.ocs.meta.message ?? t('user_ldap', 'An error occurred')
-			showError(message)
+			if (error.response.data.ocs.meta.message !== '' && error.response.data.ocs.meta.message !== undefined) {
+				message = error.response.data.ocs.meta.message
+			}
 		}
 
+		showError(message)
+
 		throw error
 	}
 }

apps/user_ldap/src/store/configs.ts

@@ -80,11 +80,21 @@ export const useLDAPConfigsStore = defineStore('ldap-configs', () => {
 	 */
 	async function removeConfig(configId: string) {
 		const result = await deleteConfig(configId)
+
 		if (result === true) {
-			delete ldapConfigs.value[configId]
+			if (Object.keys(ldapConfigs.value).length === 1) {
+				// Ensure at least one config exists before deleting the last one
+				selectedConfigId.value = await create()
+				// The new config id could be the same as the deleted one, so only delete if different
+				if (selectedConfigId.value !== configId) {
+					delete ldapConfigs.value[configId]
+				}
+			} else {
+				// Select the first config that is not the deleted one
+				selectedConfigId.value = Object.keys(ldapConfigs.value).filter((_configId) => configId !== _configId)[0]
+				delete ldapConfigs.value[configId]
+			}
 		}
-
-		selectedConfigId.value = Object.keys(ldapConfigs.value)[0] ?? await create()
 	}
 
 	return {

apps/user_ldap/src/views/Settings.vue

@@ -19,10 +19,10 @@
 					:options="Object.keys(ldapConfigs)"
 					:input-label="t('user_ldap', 'Select LDAP Config')">
 					<template #option="{ label: configId }">
-						{{ `${configId}: ${ldapConfigs[configId].ldapHost}` }}
+						{{ `${configId}: ${ldapConfigs[configId]?.ldapHost ?? ''}` }}
 					</template>
 					<template #selected-option="{ label: configId }">
-						{{ `${configId}: ${ldapConfigs[configId].ldapHost}` }}
+						{{ `${configId}: ${ldapConfigs[configId]?.ldapHost ?? ''}` }}
 					</template>
 				</NcSelect>
 				<NcButton
@@ -123,7 +123,8 @@ const selectedTab = ref('server')
 const clearMappingLoading = ref(false)
 
 const selectedConfigHasServerInfo = computed(() => {
-	return selectedConfig.value.ldapHost !== ''
+	return selectedConfig.value !== undefined
+		&& selectedConfig.value.ldapHost !== ''
 		&& selectedConfig.value.ldapPort !== ''
 		&& selectedConfig.value.ldapBase !== ''
 		&& selectedConfig.value.ldapAgentName !== ''

dist/user_ldap-settings-admin.css

@@ -1,4 +1,4 @@
 /* extracted by css-entry-points-plugin */
-@import './user_ldap-user_ldap-settings-admin-1lfli3rS.chunk.css';
+@import './user_ldap-user_ldap-settings-admin-BM0QsD47.chunk.css';
 @import './TrashCanOutline-BE4hS1RR.chunk.css';
 @import './Plus-BhZ0LWiU.chunk.css';