[pve-devel] [PATCH-SERIES manager] ui: user edit: improve 'keys' field

[pve-devel] [PATCH-SERIES manager] ui: user edit: improve 'keys' field

[Fiona Ebner]

Nowadays, TFA is configured in the respective window, so only legacy
values require some special treatment. The only real use case for
actual legacy keys is syncing them from LDAP. I wasn't sure if it
still makes sense to keep those editable, so the series is doing the
changes towards a more restricted field gradually.

Fiona Ebner (3):
  ui: user edit: protect user's TFA settings again
  ui: user edit: hide key field except for legacy values
  ui: user edit: prohibit editing keys option

 www/manager6/dc/UserEdit.js | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH manager 1/3] ui: user edit: protect user's TFA settings again

[Fiona Ebner]

Same rationale as in 5b25580d ("Protect the user's tfa key setting."):
it should not be possible to change the value when it's not an actual
secret but a reference to what TFA method is used or, in case of 'x',
whether TFA is used.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/UserEdit.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js
index b637cd53..ad52edf0 100644
--- a/www/manager6/dc/UserEdit.js
+++ b/www/manager6/dc/UserEdit.js
@@ -162,7 +162,10 @@ Ext.define('PVE.dc.UserEdit', {
 		    var data = response.result.data;
 		    me.setValues(data);
 		    if (data.keys) {
-			if (data.keys === 'x!oath' || data.keys === 'x!u2f') {
+			if (data.keys === 'x' ||
+			    data.keys === 'x!oath' ||
+			    data.keys === 'x!u2f' ||
+			    data.keys === 'x!yubico') {
 			    me.down('[name="keys"]').setDisabled(1);
 			}
 		    }
-- 
2.39.2

[pve-devel] applied: [PATCH manager 1/3] ui: user edit: protect user's TFA settings again

[Thomas Lamprecht]

Am 09/02/2024 um 14:08 schrieb Fiona Ebner:
> Same rationale as in 5b25580d ("Protect the user's tfa key setting."):
> it should not be possible to change the value when it's not an actual
> secret but a reference to what TFA method is used or, in case of 'x',
> whether TFA is used.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/dc/UserEdit.js | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
>

applied this one for now, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 2/3] ui: user edit: hide key field except for legacy values

[Fiona Ebner]

It should not be possible to set new legacy values via the UI, the TFA
configuration window should be used to set second factors.

Existing values are only interesting to show in case it's one of the
legacy values 'x!<TYPE>' or a legacy secret e.g. synced from LDAP. In
the latter case it's kept editable like it previously was.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/UserEdit.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js
index ad52edf0..ca701312 100644
--- a/www/manager6/dc/UserEdit.js
+++ b/www/manager6/dc/UserEdit.js
@@ -168,6 +168,12 @@ Ext.define('PVE.dc.UserEdit', {
 			    data.keys === 'x!yubico') {
 			    me.down('[name="keys"]').setDisabled(1);
 			}
+			if (data.keys === 'x') {
+			    me.down('[name="keys"]').setHidden(true);
+			}
+		    } else {
+			me.down('[name="keys"]').setDisabled(true);
+			me.down('[name="keys"]').setHidden(true);
 		    }
 		},
 	    });
-- 
2.39.2

[pve-devel] [PATCH manager 3/3] ui: user edit: prohibit editing keys option

[Fiona Ebner]

by turning the field into a displayfield. The TFA configuration
window should be used to set second factors. It's still worth showing
the field in case it's a legacy value.

Editing a secret that was originally synced from LDAP is a use case
where editing the field might've still made sense, but it's arguably
better to do that on the LDAP side then and re-sync.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/dc/UserEdit.js | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js
index ca701312..c9b32773 100644
--- a/www/manager6/dc/UserEdit.js
+++ b/www/manager6/dc/UserEdit.js
@@ -128,7 +128,7 @@ Ext.define('PVE.dc.UserEdit', {
 	    ],
 	    advancedItems: [
 		{
-		    xtype: 'textfield',
+		    xtype: 'displayfield',
 		    name: 'keys',
 		    fieldLabel: gettext('Key IDs'),
 		},
@@ -161,18 +161,7 @@ Ext.define('PVE.dc.UserEdit', {
 		success: function(response, options) {
 		    var data = response.result.data;
 		    me.setValues(data);
-		    if (data.keys) {
-			if (data.keys === 'x' ||
-			    data.keys === 'x!oath' ||
-			    data.keys === 'x!u2f' ||
-			    data.keys === 'x!yubico') {
-			    me.down('[name="keys"]').setDisabled(1);
-			}
-			if (data.keys === 'x') {
-			    me.down('[name="keys"]').setHidden(true);
-			}
-		    } else {
-			me.down('[name="keys"]').setDisabled(true);
+		    if (!data.keys || data.keys === 'x') {
 			me.down('[name="keys"]').setHidden(true);
 		    }
 		},
-- 
2.39.2