public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] applied: [PATCH 1/2] disk api: avoid using unrelated Datastore.Audit priv for disk management
Date: Sat, 17 Jun 2023 15:32:40 +0200	[thread overview]
Message-ID: <20230617133241.3052198-1-t.lamprecht@proxmox.com> (raw)

Local disk and storage creation and listing is something rather
different than the Proxmox VE storage client ABI that provides an
abstract access to a variety of storage types, specifically targeted
to virtual guests images, templates and backups.

The Datastore.* privilege group is specifically made for auditing the
abstract configuration, here the name must be interpreted in context
and not just assumed that due to "datastore" sounding like it could
have to do something with disks or creation of local storage it just
must be a good fit.

Luckily, Sys.Audit was already used too, which is the correct one
here, this is for node specific (HW) details, not some config for
accessing datastore in a restricted way.

This is a step in splitting the disk manage code out of the
pve-storage package, and maybe even repository.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/API2/Disks.pm           | 7 ++-----
 src/PVE/API2/Disks/Directory.pm | 2 +-
 src/PVE/API2/Disks/LVM.pm       | 2 +-
 src/PVE/API2/Disks/LVMThin.pm   | 2 +-
 src/PVE/API2/Disks/ZFS.pm       | 4 ++--
 5 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/PVE/API2/Disks.pm b/src/PVE/API2/Disks.pm
index bde6132..f0e3fc4 100644
--- a/src/PVE/API2/Disks.pm
+++ b/src/PVE/API2/Disks.pm
@@ -85,10 +85,7 @@ __PACKAGE__->register_method ({
     protected => 1,
     proxyto => 'node',
     permissions => {
-	check => ['or',
-	    ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
-	    ['perm', '/nodes/{node}', ['Sys.Audit', 'Datastore.Audit'], any => 1],
-	],
+	check => ['or', ['perm', '/', ['Sys.Audit']], ['perm', '/nodes/{node}', ['Sys.Audit']]],
     },
     parameters => {
 	additionalProperties => 0,
@@ -183,7 +180,7 @@ __PACKAGE__->register_method ({
     protected => 1,
     proxyto => "node",
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     parameters => {
 	additionalProperties => 0,
diff --git a/src/PVE/API2/Disks/Directory.pm b/src/PVE/API2/Disks/Directory.pm
index efc701a..5cddddb 100644
--- a/src/PVE/API2/Disks/Directory.pm
+++ b/src/PVE/API2/Disks/Directory.pm
@@ -97,7 +97,7 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     description => "PVE Managed Directory storages.",
     parameters => {
diff --git a/src/PVE/API2/Disks/LVM.pm b/src/PVE/API2/Disks/LVM.pm
index c091e37..f0419f6 100644
--- a/src/PVE/API2/Disks/LVM.pm
+++ b/src/PVE/API2/Disks/LVM.pm
@@ -21,7 +21,7 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     description => "List LVM Volume Groups",
     parameters => {
diff --git a/src/PVE/API2/Disks/LVMThin.pm b/src/PVE/API2/Disks/LVMThin.pm
index 89b6401..bd6d47b 100644
--- a/src/PVE/API2/Disks/LVMThin.pm
+++ b/src/PVE/API2/Disks/LVMThin.pm
@@ -22,7 +22,7 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     description => "List LVM thinpools",
     parameters => {
diff --git a/src/PVE/API2/Disks/ZFS.pm b/src/PVE/API2/Disks/ZFS.pm
index afd9a3e..6a4381c 100644
--- a/src/PVE/API2/Disks/ZFS.pm
+++ b/src/PVE/API2/Disks/ZFS.pm
@@ -57,7 +57,7 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     description => "List Zpools.",
     parameters => {
@@ -130,7 +130,7 @@ __PACKAGE__->register_method ({
     proxyto => 'node',
     protected => 1,
     permissions => {
-	check => ['perm', '/', ['Sys.Audit', 'Datastore.Audit'], any => 1],
+	check => ['perm', '/', ['Sys.Audit']],
     },
     description => "Get details about a zpool.",
     parameters => {
-- 
2.39.2





             reply	other threads:[~2023-06-17 13:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-17 13:32 Thomas Lamprecht [this message]
2023-06-17 13:32 ` [pve-devel] applied: [PATCH 2/2] disk api: only ask for Datastore.Allocate if adding to storage config Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230617133241.3052198-1-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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