[pve-devel] [PATCH manager] ui: acl add: show warning if root@pam is selected

[pve-devel] [PATCH manager] ui: acl add: show warning if root@pam is selected

[Lukas Wagner]

Currently, users are able to add ACL entries for the root@pam user.
Since this user always has full permissions, no entry in the ACL
tree will be saved, and consequently no new entry shows up in the UI
after pressing 'Add' in the dialog. This can be irritating if the
user does not know about this 'implementation detail'.

This commit adds a little warning that pops up if root@pam is
selected:

  'root@pam always has full permissions. No entry will be added.'

The same problem also exists for API token permissions. Here it is
not really easy to add the warning though, since we do not know if
the token has separated privileges enable or not.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/ACLView.js | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/www/manager6/dc/ACLView.js b/www/manager6/dc/ACLView.js
index 79f900cd..ec81a487 100644
--- a/www/manager6/dc/ACLView.js
+++ b/www/manager6/dc/ACLView.js
@@ -35,6 +35,20 @@ Ext.define('PVE.dc.ACLAdd', {
 		xtype: 'pmxUserSelector',
 		name: 'users',
 		fieldLabel: gettext('User'),
+		listeners: {
+		    change: function(field, newVal) {
+			this.nextSibling('displayfield[reference=root-selected-warning]')
+			    .setVisible(newVal === 'root@pam');
+		    }
+		},
+	    });
+	    items.push({
+		    xtype: 'displayfield',
+		    reference: 'root-selected-warning',
+		    userCls: 'pmx-hint',
+		    hidden: true,
+		    value: '\'root@pam\' ' +
+			gettext('always has full permissions. No entry will be added.'),
 	    });
 	} else if (me.aclType === 'token') {
 	    me.subject = gettext("API Token Permission");
-- 
2.39.2

Re: [pve-devel] [PATCH manager] ui: acl add: show warning if root@pam is selected

[Fiona Ebner]

Am 26.07.23 um 15:41 schrieb Lukas Wagner:
> Currently, users are able to add ACL entries for the root@pam user.
> Since this user always has full permissions, no entry in the ACL
> tree will be saved, and consequently no new entry shows up in the UI
> after pressing 'Add' in the dialog. This can be irritating if the
> user does not know about this 'implementation detail'.
> 

Should we filter out the root@pam user from the selection dropdown
altogether? Or maybe disable the Add button when root@pam is selected
(and reword the warning appropriately)?

> This commit adds a little warning that pops up if root@pam is
> selected:
> 
>   'root@pam always has full permissions. No entry will be added.'
> 
> The same problem also exists for API token permissions. Here it is
> not really easy to add the warning though, since we do not know if
> the token has separated privileges enable or not.
> 

It seems we do have that information available as a result of the
/access/users?full=1 API call, or?

> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  www/manager6/dc/ACLView.js | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/www/manager6/dc/ACLView.js b/www/manager6/dc/ACLView.js
> index 79f900cd..ec81a487 100644
> --- a/www/manager6/dc/ACLView.js
> +++ b/www/manager6/dc/ACLView.js
> @@ -35,6 +35,20 @@ Ext.define('PVE.dc.ACLAdd', {
>  		xtype: 'pmxUserSelector',
>  		name: 'users',
>  		fieldLabel: gettext('User'),
> +		listeners: {
> +		    change: function(field, newVal) {
> +			this.nextSibling('displayfield[reference=root-selected-warning]')
> +			    .setVisible(newVal === 'root@pam');
> +		    }

eslint complains about a missing trailing comma here

> +		},
> +	    });
> +	    items.push({
> +		    xtype: 'displayfield',
> +		    reference: 'root-selected-warning',
> +		    userCls: 'pmx-hint',
> +		    hidden: true,
> +		    value: '\'root@pam\' ' +
> +			gettext('always has full permissions. No entry will be added.'),
>  	    });
>  	} else if (me.aclType === 'token') {
>  	    me.subject = gettext("API Token Permission");

Re: [pve-devel] [PATCH manager] ui: acl add: show warning if root@pam is selected

[Lukas Wagner]

On 10/10/23 14:10, Fiona Ebner wrote:
> Am 26.07.23 um 15:41 schrieb Lukas Wagner:
>> Currently, users are able to add ACL entries for the root@pam user.
>> Since this user always has full permissions, no entry in the ACL
>> tree will be saved, and consequently no new entry shows up in the UI
>> after pressing 'Add' in the dialog. This can be irritating if the
>> user does not know about this 'implementation detail'.
>>
> 
> Should we filter out the root@pam user from the selection dropdown
> altogether? Or maybe disable the Add button when root@pam is selected
> (and reword the warning appropriately)?
I think the second approach might be good idea, I'll try that.

> 
>> This commit adds a little warning that pops up if root@pam is
>> selected:
>>
>>    'root@pam always has full permissions. No entry will be added.'
>>
>> The same problem also exists for API token permissions. Here it is
>> not really easy to add the warning though, since we do not know if
>> the token has separated privileges enable or not.
>>
> 
> It seems we do have that information available as a result of the
> /access/users?full=1 API call, or?

You are right, I missed that because I did not check the code for
pmxUserSelector.

I'll send a v2 with the suggested improvements.


-- 
- Lukas