From: Dietmar Maurer <dietmar@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>, Dylan Whyte <d.whyte@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #2847: api: datastore: change backup owner
Date: Wed, 14 Oct 2020 08:33:29 +0200 (CEST) [thread overview]
Message-ID: <1757107249.88.1602657210365@webmail.proxmox.com> (raw)
In-Reply-To: <20201013085841.22773-1-d.whyte@proxmox.com>
applied a modified version, see comments inline:
> On 10/13/2020 10:58 AM Dylan Whyte <d.whyte@proxmox.com> wrote:
>
>
> This adds an api method to change the owner of
> a backup-group.
>
> Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
> ---
> src/api2/admin/datastore.rs | 56 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index c260b62d..f4c4e2de 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -1492,6 +1492,57 @@ fn set_notes(
> Ok(())
> }
>
> +#[api(
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + group: {
> + description: "Backup group.",
> + },
All others method in this api path uses "backup-type" and "backup-id", so I prefer to use
that here too.
> + "new-owner": {
> + description: "Userid of new owner.",
> + },
Using "String" as type is much too generic. This should be:
"new-owner": {
type: Userid,
},
> + },
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> + },
> +)]
> +/// Change owner of a backup group
> +fn set_backup_owner(
> + store: String,
> + group: String,
> + new_owner: String,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> +
> + let datastore = DataStore::lookup_datastore(&store)?;
> +
> + // user requesting change of owner
> + let userid: Userid = rpcenv.get_user().unwrap().parse()?;
> + let user_info = CachedUserInfo::new()?;
> + let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
> +
> + let backup_group: BackupGroup = group.parse()?;
> +
> + let new_owner: Userid = new_owner.parse()?;
> + let new_owner_info = CachedUserInfo::new()?;
There is no need to get CachedUserInfo::new() twice!
> +
> + if new_owner_info.is_active_user(&new_owner) {
> + let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0;
> + if !allowed { check_backup_owner(&datastore, &backup_group, &userid)?; }
Also, this check is redundant, because the rest server already verifies the "access" permissions.
I removed that for now. Fabian will extend this when he add the api token patches.
> +
> + datastore.set_owner(&backup_group, &new_owner, true)?;
> +
> + } else {
> + bail!("user {} is inactive or non-existent", new_owner);
> + }
> +
> + Ok(())
> +}
> +
> #[sortable]
> const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> (
> @@ -1499,6 +1550,11 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> &Router::new()
> .get(&API_METHOD_CATALOG)
> ),
> + (
> + "change-owner",
> + &Router::new()
> + .post(&API_METHOD_SET_BACKUP_OWNER)
> + ),
> (
> "download",
> &Router::new()
> --
> 2.20.1
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
prev parent reply other threads:[~2020-10-14 6:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-13 8:58 Dylan Whyte
2020-10-13 8:58 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #2847: proxmox-backup-client: add change-owner cmd Dylan Whyte
2020-10-14 6:56 ` Dietmar Maurer
2020-10-14 6:33 ` Dietmar Maurer [this message]
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=1757107249.88.1602657210365@webmail.proxmox.com \
--to=dietmar@proxmox.com \
--cc=d.whyte@proxmox.com \
--cc=pbs-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