From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id D65A6B9A7
 for <pve-devel@lists.proxmox.com>; Thu, 10 Aug 2023 09:17:09 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id B263E1D2EB
 for <pve-devel@lists.proxmox.com>; Thu, 10 Aug 2023 09:16:39 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Thu, 10 Aug 2023 09:16:39 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id DBEC144752
 for <pve-devel@lists.proxmox.com>; Thu, 10 Aug 2023 09:16:38 +0200 (CEST)
Message-ID: <dcf64dc1-a392-d1c1-952c-4a0b4e9ab009@proxmox.com>
Date: Thu, 10 Aug 2023 09:16:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.1
To: Philipp Hufnagl <p.hufnagl@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20230808091342.637190-1-p.hufnagl@proxmox.com>
 <a3ea6ca3-9e48-9b93-7c9e-4ba137880275@proxmox.com>
 <22eba78f-dec2-42ff-9d75-3107aecdd981@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <22eba78f-dec2-42ff-9d75-3107aecdd981@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 2.014 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A            -4.14 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH manager] fix #474: allow transfer from
 container/vms
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 10 Aug 2023 07:17:09 -0000

Am 09.08.23 um 16:20 schrieb Philipp Hufnagl:
> On 8/9/23 13:32, Fiona Ebner wrote:
> 
>> The permission for the original pool should be checked here?! Or is
>> that already done somewhere? 
> 
> The permission of the original pool does not matter.

But it should. After all, the operation is modifying the original pool,
so the user better have an appropriate permission to do so.

> The permission of the VM is important
> (maybe the original pool granting the user permission on the VM).
> Hovever I tested it with granting the
> user merely audit permissions on the VM and admin permissions on the
> target pool and still got the
> correct permission error so I don't think the permission checks have to
> be modified at all
> 

Currently, Permissions.Modify|VM.Allocate on the VM and Pool.Allocate on
the target pool would be enough to "steal" the guest, no permissions
required on the original pool at all. IMHO, the user really should have
a Pool.Allocate on the original pool as well.

Since I noticed it in v3: we usually use "api:" and "ui:" as prefixes
rather than "backend:" and "frontend:". Would be nice if you could use
them too for consistency.