[pve-devel] [PATCH pve-access-control 0/2] api: token: fix #6890: allow comment removal via delete parameter or empty string

[pve-devel] [PATCH pve-access-control 0/2] api: token: fix #6890: allow comment removal via delete parameter or empty string

[Nicolas Frey]

this patch series fixes #6890, which reported that removal of the
comment property on tokens was not possible. Also adds a delete param
which behaves the same as if the comment was set to an empty string.

Nicolas Frey (2):
  api: token: fix #6890: add ability to remove comment with empty string
  api: token: add delete parameter for comment

 src/PVE/API2/User.pm | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
2.47.3


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

[pve-devel] [PATCH pve-access-control 1/2] api: token: fix #6890: add ability to remove comment with empty string

[Nicolas Frey]

Beforehand, if the comment was an empty string, it would not update
the token to have it's comment deleted. Adds missing defined check
instead of checking truthy value (i.e. an empty string being falsy).
The comment is also explicitly deleted, because the response would
otherwise return the comment as empty, not deleted:

┌─────────┬───────┐
│ key     │ value │
╞═════════╪═══════╡
│ comment │       │
├─────────┼───────┤
│ expire  │ 0     │
├─────────┼───────┤
│ privsep │ 1     │
└─────────┴───────┘

instead of:

┌─────────┬───────┐
│ key     │ value │
╞═════════╪═══════╡
│ expire  │ 0     │
├─────────┼───────┤
│ privsep │ 1     │
└─────────┴───────┘

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6890
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/PVE/API2/User.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/User.pm b/src/PVE/API2/User.pm
index 5041fbe..e7c895e 100644
--- a/src/PVE/API2/User.pm
+++ b/src/PVE/API2/User.pm
@@ -870,7 +870,8 @@ __PACKAGE__->register_method({
 
                 $token->{privsep} = $param->{privsep} if defined($param->{privsep});
                 $token->{expire} = $param->{expire} if defined($param->{expire});
-                $token->{comment} = $param->{comment} if $param->{comment};
+                $token->{comment} = $param->{comment} if defined($param->{comment});
+                delete $token->{comment} if (!length $token->{comment});
 
                 $usercfg->{users}->{$userid}->{tokens}->{$tokenid} = $token;
                 cfs_write_file("user.cfg", $usercfg);
-- 
2.47.3


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

[pve-devel] [PATCH pve-access-control 2/2] api: token: add delete parameter for comment

[Nicolas Frey]

Adds delete parameter to the endpoint, which was suggested by Fabian
Grünbichler in the bug report.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/PVE/API2/User.pm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/PVE/API2/User.pm b/src/PVE/API2/User.pm
index e7c895e..4742f17 100644
--- a/src/PVE/API2/User.pm
+++ b/src/PVE/API2/User.pm
@@ -848,6 +848,12 @@ __PACKAGE__->register_method({
             expire => get_standard_option('token-expire'),
             privsep => get_standard_option('token-privsep'),
             comment => get_standard_option('token-comment'),
+            delete => {
+                type => 'string',
+                format => 'pve-configid-list',
+                description => "A list of settings you want to delete.",
+                optional => 1,
+            },
         },
     },
     returns =>
@@ -861,6 +867,9 @@ __PACKAGE__->register_method({
         my $usercfg = cfs_read_file("user.cfg");
         my $token = PVE::AccessControl::check_token_exist($usercfg, $userid, $tokenid);
 
+        my $delete = extract_param($param, 'delete');
+        $delete = { map { $_ => 1 } PVE::Tools::split_list($delete) } if $delete;
+
         PVE::AccessControl::lock_user_config(
             sub {
                 $usercfg = cfs_read_file("user.cfg");
@@ -873,6 +882,18 @@ __PACKAGE__->register_method({
                 $token->{comment} = $param->{comment} if defined($param->{comment});
                 delete $token->{comment} if (!length $token->{comment});
 
+                my $deletable = {
+                    comment => 1,
+                };
+
+                for my $k (keys $delete->%*) {
+                    if (!$deletable->{$k}) {
+                        raise_param_exc({ delete => "unknown option '$k'" });
+                    }
+
+                    delete $token->{$k};
+                }
+
                 $usercfg->{users}->{$userid}->{tokens}->{$tokenid} = $token;
                 cfs_write_file("user.cfg", $usercfg);
             },
-- 
2.47.3


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