[pve-devel] [PATCH manager 1/3] ui: ldap: ad: support 'mode' paramter, replacing 'secure'

[pve-devel] [PATCH manager 1/3] ui: ldap: ad: support 'mode' paramter, replacing 'secure'

[Lukas Wagner]

The backend has supported the 'mode' parameter for quite a while,
however it has not yet been exposed in the GUI, contrary to PMG
and PBS.

The benefit of 'mode' is that it supports LDAP, LDAPS and LDAP via
STARTTLS, compared to just LDAP/LDAPS for the 'secure' parameter.

The modified AuthEdit{LDAP,AD} panel will now automatically migrate
to the new paramter by hooking into onGetValues/onSetValues.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/AuthEditAD.js   | 43 +++++++++++++++++++++++++++------
 www/manager6/dc/AuthEditLDAP.js | 42 ++++++++++++++++++++++++++------
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/www/manager6/dc/AuthEditAD.js b/www/manager6/dc/AuthEditAD.js
index a1999cb7..bd46faaa 100644
--- a/www/manager6/dc/AuthEditAD.js
+++ b/www/manager6/dc/AuthEditAD.js
@@ -49,18 +49,26 @@ Ext.define('PVE.panel.ADInputPanel', {
 		submitEmptyText: false,
 	    },
 	    {
-		xtype: 'proxmoxcheckbox',
-		fieldLabel: 'SSL',
-		name: 'secure',
-		uncheckedValue: 0,
+		xtype: 'proxmoxKVComboBox',
+		name: 'mode',
+		fieldLabel: gettext('Mode'),
+		editable: false,
+		comboItems: [
+		    ['__default__', Proxmox.Utils.defaultText + ' (LDAP)'],
+		    ['ldap', 'LDAP'],
+		    ['ldap+starttls', 'STARTTLS'],
+		    ['ldaps', 'LDAPS'],
+		],
+		value: '__default__',
+		deleteEmpty: !me.isCreate,
 		listeners: {
 		    change: function(field, newValue) {
 			let verifyCheckbox = field.nextSibling('proxmoxcheckbox[name=verify]');
-			if (newValue === true) {
-			    verifyCheckbox.enable();
-			} else {
+			if (newValue === 'ldap' || newValue === '__default__') {
 			    verifyCheckbox.disable();
 			    verifyCheckbox.setValue(0);
+			} else {
+			    verifyCheckbox.enable();
 			}
 		    },
 		},
@@ -91,6 +99,27 @@ Ext.define('PVE.panel.ADInputPanel', {
 	    delete values.verify;
 	}
 
+	if (!me.isCreate) {
+	    // Delete old `secure` parameter. It has been deprecated in favor to the
+	    // `mode` parameter. Migration happens automatically in `onSetValues`.
+	    Proxmox.Utils.assemble_field_data(values, { 'delete': 'secure' });
+	}
+
+
+	return me.callParent([values]);
+    },
+
+    onSetValues(values) {
+	let me = this;
+
+	if (values.secure !== undefined && !values.mode) {
+	    // If `secure` is set, use it to determine the correct setting for `mode`
+	    // `secure` is later deleted by `onSetValues` .
+	    // In case *both* are set, we simply ignore `secure` and use
+	    // whatever `mode` is set to.
+	    values.mode = values.secure ? 'ldaps' : 'ldap';
+	}
+
 	return me.callParent([values]);
     },
 });
diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
index 2ce16e58..721ea971 100644
--- a/www/manager6/dc/AuthEditLDAP.js
+++ b/www/manager6/dc/AuthEditLDAP.js
@@ -49,18 +49,26 @@ Ext.define('PVE.panel.LDAPInputPanel', {
 		submitEmptyText: false,
 	    },
 	    {
-		xtype: 'proxmoxcheckbox',
-		fieldLabel: 'SSL',
-		name: 'secure',
-		uncheckedValue: 0,
+		xtype: 'proxmoxKVComboBox',
+		name: 'mode',
+		fieldLabel: gettext('Mode'),
+		editable: false,
+		comboItems: [
+		    ['__default__', Proxmox.Utils.defaultText + ' (LDAP)'],
+		    ['ldap', 'LDAP'],
+		    ['ldap+starttls', 'STARTTLS'],
+		    ['ldaps', 'LDAPS'],
+		],
+		value: '__default__',
+		deleteEmpty: !me.isCreate,
 		listeners: {
 		    change: function(field, newValue) {
 			let verifyCheckbox = field.nextSibling('proxmoxcheckbox[name=verify]');
-			if (newValue === true) {
-			    verifyCheckbox.enable();
-			} else {
+			if (newValue === 'ldap' || newValue === '__default__') {
 			    verifyCheckbox.disable();
 			    verifyCheckbox.setValue(0);
+			} else {
+			    verifyCheckbox.enable();
 			}
 		    },
 		},
@@ -91,6 +99,26 @@ Ext.define('PVE.panel.LDAPInputPanel', {
 	    delete values.verify;
 	}
 
+	if (!me.isCreate) {
+	    // Delete old `secure` parameter. It has been deprecated in favor to the
+	    // `mode` parameter. Migration happens automatically in `onSetValues`.
+	    Proxmox.Utils.assemble_field_data(values, { 'delete': 'secure' });
+	}
+
+	return me.callParent([values]);
+    },
+
+    onSetValues(values) {
+	let me = this;
+
+	if (values.secure !== undefined && !values.mode) {
+	    // If `secure` is set, use it to determine the correct setting for `mode`
+	    // `secure` is later deleted by `onSetValues` .
+	    // In case *both* are set, we simply ignore `secure` and use
+	    // whatever `mode` is set to.
+	    values.mode = values.secure ? 'ldaps' : 'ldap';
+	}
+
 	return me.callParent([values]);
     },
 });
-- 
2.39.2

[pve-devel] [PATCH manager 2/3] ui: ldap: ad: fix typo for verify certificate combobox

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/AuthEditAD.js   | 2 +-
 www/manager6/dc/AuthEditLDAP.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/dc/AuthEditAD.js b/www/manager6/dc/AuthEditAD.js
index bd46faaa..41390a47 100644
--- a/www/manager6/dc/AuthEditAD.js
+++ b/www/manager6/dc/AuthEditAD.js
@@ -77,7 +77,7 @@ Ext.define('PVE.panel.ADInputPanel', {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: gettext('Verify Certificate'),
 		name: 'verify',
-		unceckedValue: 0,
+		uncheckedValue: 0,
 		disabled: true,
 		checked: false,
 		autoEl: {
diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
index 721ea971..3c9f1cbc 100644
--- a/www/manager6/dc/AuthEditLDAP.js
+++ b/www/manager6/dc/AuthEditLDAP.js
@@ -77,7 +77,7 @@ Ext.define('PVE.panel.LDAPInputPanel', {
 		xtype: 'proxmoxcheckbox',
 		fieldLabel: gettext('Verify Certificate'),
 		name: 'verify',
-		unceckedValue: 0,
+		uncheckedValue: 0,
 		disabled: true,
 		checked: false,
 		autoEl: {
-- 
2.39.2

[pve-devel] [PATCH manager 3/3] ui: ldap: ad: replace occurences of SSL with TLS

[Lukas Wagner]

Although 'SSL' is used colloquially, the proper term is 'TLS'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/AuthEditAD.js   | 2 +-
 www/manager6/dc/AuthEditLDAP.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/dc/AuthEditAD.js b/www/manager6/dc/AuthEditAD.js
index 41390a47..e74192ee 100644
--- a/www/manager6/dc/AuthEditAD.js
+++ b/www/manager6/dc/AuthEditAD.js
@@ -82,7 +82,7 @@ Ext.define('PVE.panel.ADInputPanel', {
 		checked: false,
 		autoEl: {
 		    tag: 'div',
-		    'data-qtip': gettext('Verify SSL certificate of the server'),
+		    'data-qtip': gettext('Verify TLS certificate of the server'),
 		},
 	    },
 	];
diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
index 3c9f1cbc..87e9fd1e 100644
--- a/www/manager6/dc/AuthEditLDAP.js
+++ b/www/manager6/dc/AuthEditLDAP.js
@@ -82,7 +82,7 @@ Ext.define('PVE.panel.LDAPInputPanel', {
 		checked: false,
 		autoEl: {
 		    tag: 'div',
-		    'data-qtip': gettext('Verify SSL certificate of the server'),
+		    'data-qtip': gettext('Verify TLS certificate of the server'),
 		},
 	    },
 	];
-- 
2.39.2

Re: [pve-devel] [PATCH manager 1/3] ui: ldap: ad: support 'mode' paramter, replacing 'secure'

[Lukas Wagner]

Ping. Still applies on master.

On 7/27/23 10:57, Lukas Wagner wrote:
> The backend has supported the 'mode' parameter for quite a while,
> however it has not yet been exposed in the GUI, contrary to PMG
> and PBS.
> 
> The benefit of 'mode' is that it supports LDAP, LDAPS and LDAP via
> STARTTLS, compared to just LDAP/LDAPS for the 'secure' parameter.
> 
> The modified AuthEdit{LDAP,AD} panel will now automatically migrate
> to the new paramter by hooking into onGetValues/onSetValues.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   www/manager6/dc/AuthEditAD.js   | 43 +++++++++++++++++++++++++++------
>   www/manager6/dc/AuthEditLDAP.js | 42 ++++++++++++++++++++++++++------
>   2 files changed, 71 insertions(+), 14 deletions(-)
> 
> diff --git a/www/manager6/dc/AuthEditAD.js b/www/manager6/dc/AuthEditAD.js
> index a1999cb7..bd46faaa 100644
> --- a/www/manager6/dc/AuthEditAD.js
> +++ b/www/manager6/dc/AuthEditAD.js
> @@ -49,18 +49,26 @@ Ext.define('PVE.panel.ADInputPanel', {
>   		submitEmptyText: false,
>   	    },
>   	    {
> -		xtype: 'proxmoxcheckbox',
> -		fieldLabel: 'SSL',
> -		name: 'secure',
> -		uncheckedValue: 0,
> +		xtype: 'proxmoxKVComboBox',
> +		name: 'mode',
> +		fieldLabel: gettext('Mode'),
> +		editable: false,
> +		comboItems: [
> +		    ['__default__', Proxmox.Utils.defaultText + ' (LDAP)'],
> +		    ['ldap', 'LDAP'],
> +		    ['ldap+starttls', 'STARTTLS'],
> +		    ['ldaps', 'LDAPS'],
> +		],
> +		value: '__default__',
> +		deleteEmpty: !me.isCreate,
>   		listeners: {
>   		    change: function(field, newValue) {
>   			let verifyCheckbox = field.nextSibling('proxmoxcheckbox[name=verify]');
> -			if (newValue === true) {
> -			    verifyCheckbox.enable();
> -			} else {
> +			if (newValue === 'ldap' || newValue === '__default__') {
>   			    verifyCheckbox.disable();
>   			    verifyCheckbox.setValue(0);
> +			} else {
> +			    verifyCheckbox.enable();
>   			}
>   		    },
>   		},
> @@ -91,6 +99,27 @@ Ext.define('PVE.panel.ADInputPanel', {
>   	    delete values.verify;
>   	}
>   
> +	if (!me.isCreate) {
> +	    // Delete old `secure` parameter. It has been deprecated in favor to the
> +	    // `mode` parameter. Migration happens automatically in `onSetValues`.
> +	    Proxmox.Utils.assemble_field_data(values, { 'delete': 'secure' });
> +	}
> +
> +
> +	return me.callParent([values]);
> +    },
> +
> +    onSetValues(values) {
> +	let me = this;
> +
> +	if (values.secure !== undefined && !values.mode) {
> +	    // If `secure` is set, use it to determine the correct setting for `mode`
> +	    // `secure` is later deleted by `onSetValues` .
> +	    // In case *both* are set, we simply ignore `secure` and use
> +	    // whatever `mode` is set to.
> +	    values.mode = values.secure ? 'ldaps' : 'ldap';
> +	}
> +
>   	return me.callParent([values]);
>       },
>   });
> diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
> index 2ce16e58..721ea971 100644
> --- a/www/manager6/dc/AuthEditLDAP.js
> +++ b/www/manager6/dc/AuthEditLDAP.js
> @@ -49,18 +49,26 @@ Ext.define('PVE.panel.LDAPInputPanel', {
>   		submitEmptyText: false,
>   	    },
>   	    {
> -		xtype: 'proxmoxcheckbox',
> -		fieldLabel: 'SSL',
> -		name: 'secure',
> -		uncheckedValue: 0,
> +		xtype: 'proxmoxKVComboBox',
> +		name: 'mode',
> +		fieldLabel: gettext('Mode'),
> +		editable: false,
> +		comboItems: [
> +		    ['__default__', Proxmox.Utils.defaultText + ' (LDAP)'],
> +		    ['ldap', 'LDAP'],
> +		    ['ldap+starttls', 'STARTTLS'],
> +		    ['ldaps', 'LDAPS'],
> +		],
> +		value: '__default__',
> +		deleteEmpty: !me.isCreate,
>   		listeners: {
>   		    change: function(field, newValue) {
>   			let verifyCheckbox = field.nextSibling('proxmoxcheckbox[name=verify]');
> -			if (newValue === true) {
> -			    verifyCheckbox.enable();
> -			} else {
> +			if (newValue === 'ldap' || newValue === '__default__') {
>   			    verifyCheckbox.disable();
>   			    verifyCheckbox.setValue(0);
> +			} else {
> +			    verifyCheckbox.enable();
>   			}
>   		    },
>   		},
> @@ -91,6 +99,26 @@ Ext.define('PVE.panel.LDAPInputPanel', {
>   	    delete values.verify;
>   	}
>   
> +	if (!me.isCreate) {
> +	    // Delete old `secure` parameter. It has been deprecated in favor to the
> +	    // `mode` parameter. Migration happens automatically in `onSetValues`.
> +	    Proxmox.Utils.assemble_field_data(values, { 'delete': 'secure' });
> +	}
> +
> +	return me.callParent([values]);
> +    },
> +
> +    onSetValues(values) {
> +	let me = this;
> +
> +	if (values.secure !== undefined && !values.mode) {
> +	    // If `secure` is set, use it to determine the correct setting for `mode`
> +	    // `secure` is later deleted by `onSetValues` .
> +	    // In case *both* are set, we simply ignore `secure` and use
> +	    // whatever `mode` is set to.
> +	    values.mode = values.secure ? 'ldaps' : 'ldap';
> +	}
> +
>   	return me.callParent([values]);
>       },
>   });

-- 
- Lukas

[pve-devel] applied: [PATCH manager 1/3] ui: ldap: ad: support 'mode' paramter, replacing 'secure'

[Thomas Lamprecht]

Am 27/07/2023 um 10:57 schrieb Lukas Wagner:
> The backend has supported the 'mode' parameter for quite a while,
> however it has not yet been exposed in the GUI, contrary to PMG
> and PBS.
> 
> The benefit of 'mode' is that it supports LDAP, LDAPS and LDAP via
> STARTTLS, compared to just LDAP/LDAPS for the 'secure' parameter.
> 
> The modified AuthEdit{LDAP,AD} panel will now automatically migrate
> to the new paramter by hooking into onGetValues/onSetValues.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  www/manager6/dc/AuthEditAD.js   | 43 +++++++++++++++++++++++++++------
>  www/manager6/dc/AuthEditLDAP.js | 42 ++++++++++++++++++++++++++------
>  2 files changed, 71 insertions(+), 14 deletions(-)
> 
>

applied series, thanks!

FWIW, I prefer having trivial stuff (typos, uncontroversial renames like
SSL -> TLS) upfront, as then one can apply them immediately even if there
would be something left to adapt on the actual "meat" of the series.
Anyway, it didn't really matter here, so just mentioning to avoid that
others copy this for when it does matter.