public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager] ui: ldap: fix `Parameter verification error`
@ 2023-02-14 13:35 Lukas Wagner
  2023-02-15  8:29 ` Friedrich Weber
  0 siblings, 1 reply; 3+ messages in thread
From: Lukas Wagner @ 2023-02-14 13:35 UTC (permalink / raw)
  To: pve-devel

...when editing LDAP realm sync settings and only a single property is
empty and thus to be deleted (e.g. values.delete = "filter").

If `delete` is a simple string and not an array,
`PVE.Utils.delete_if_default` simply creates a comma-separated list,
(e.g. value.delete = "filter,sync-attributes").

When the properties from the other panel are evaluated and added to the
the `delete` property, comma-separated list format is not considered,
leading to a final value for `delete` that could look like this:
  value.delete = {
    "server2",
    "comment",
    "filter,sync-attributes"
  }

This commit fixes this by splitting `delete` in case it is a string.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 Same fix as in my LDAP widget-toolkit patch series:
 https://lists.proxmox.com/pipermail/pbs-devel/2023-February/005952.html

 www/manager6/dc/AuthEditLDAP.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
index 96ef77b9..2ce16e58 100644
--- a/www/manager6/dc/AuthEditLDAP.js
+++ b/www/manager6/dc/AuthEditLDAP.js
@@ -139,6 +139,11 @@ Ext.define('PVE.panel.LDAPSyncInputPanel', {
 	PVE.Utils.delete_if_default(values, 'sync-defaults-options');
 	PVE.Utils.delete_if_default(values, 'sync_attributes');
 
+	// Force values.delete to be an array
+	if (typeof values.delete === 'string') {
+	   values.delete = values.delete.split(',');
+	}
+
 	if (me.isCreate) {
 	    delete values.delete; // on create we cannot delete values
 	}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH manager] ui: ldap: fix `Parameter verification error`
  2023-02-14 13:35 [pve-devel] [PATCH manager] ui: ldap: fix `Parameter verification error` Lukas Wagner
@ 2023-02-15  8:29 ` Friedrich Weber
  2023-02-15  9:15   ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Friedrich Weber @ 2023-02-15  8:29 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pve-devel

Can confirm this patch fixes the issue, so

Tested-by: Friedrich Weber <f.weber@proxmox.com>

Steps to reproduce:

1) Create LDAP realm with default sync settings
2) Edit LDAP realm, go to sync settings tab, enter values in all text 
boxes except one (e.g. all except "Group filter")
3) Click OK

On current master, this shows "Parameter verification failed. (400) / 
delete: invalid format - invalid configuration ID 
'group_filter,sync-defaults-options'".

With this patch, the error does not appear, and the settings are saved 
properly.

On 14/02/2023 14:35, Lukas Wagner wrote:
> ...when editing LDAP realm sync settings and only a single property is
> empty and thus to be deleted (e.g. values.delete = "filter").
> 
> If `delete` is a simple string and not an array,
> `PVE.Utils.delete_if_default` simply creates a comma-separated list,
> (e.g. value.delete = "filter,sync-attributes").
> 
> When the properties from the other panel are evaluated and added to the
> the `delete` property, comma-separated list format is not considered,
> leading to a final value for `delete` that could look like this:
>    value.delete = {
>      "server2",
>      "comment",
>      "filter,sync-attributes"
>    }
> 
> This commit fixes this by splitting `delete` in case it is a string.
> 
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   Same fix as in my LDAP widget-toolkit patch series:
>   https://lists.proxmox.com/pipermail/pbs-devel/2023-February/005952.html
> 
>   www/manager6/dc/AuthEditLDAP.js | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/www/manager6/dc/AuthEditLDAP.js b/www/manager6/dc/AuthEditLDAP.js
> index 96ef77b9..2ce16e58 100644
> --- a/www/manager6/dc/AuthEditLDAP.js
> +++ b/www/manager6/dc/AuthEditLDAP.js
> @@ -139,6 +139,11 @@ Ext.define('PVE.panel.LDAPSyncInputPanel', {
>   	PVE.Utils.delete_if_default(values, 'sync-defaults-options');
>   	PVE.Utils.delete_if_default(values, 'sync_attributes');
>   
> +	// Force values.delete to be an array
> +	if (typeof values.delete === 'string') {
> +	   values.delete = values.delete.split(',');
> +	}
> +
>   	if (me.isCreate) {
>   	    delete values.delete; // on create we cannot delete values
>   	}




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] applied: [PATCH manager] ui: ldap: fix `Parameter verification error`
  2023-02-15  8:29 ` Friedrich Weber
@ 2023-02-15  9:15   ` Thomas Lamprecht
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2023-02-15  9:15 UTC (permalink / raw)
  To: Proxmox VE development discussion, Friedrich Weber, Lukas Wagner

Am 15/02/2023 um 09:29 schrieb Friedrich Weber:
> Tested-by: Friedrich Weber <f.weber@proxmox.com>

with above: applied, thanks to both of you!




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-15  9:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 13:35 [pve-devel] [PATCH manager] ui: ldap: fix `Parameter verification error` Lukas Wagner
2023-02-15  8:29 ` Friedrich Weber
2023-02-15  9:15   ` [pve-devel] applied: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal