Re: [PATCH proxmox-backup 14/28] sync: pull: allow pulling for user/token with append permission

[Robert Obkircher <r.obkircher@proxmox.com>]

> Extends the access checks for syncs in pull direction to allow
> appending contents when user has Datastore.Append permissions.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>
> diff --git a/src/api2/pull.rs b/src/api2/pull.rs
> index 17c1e4f34..6b53c55b7 100644
> --- a/src/api2/pull.rs
> +++ b/src/api2/pull.rs
> @@ -8,9 +8,9 @@ use proxmox_schema::api;
>  
>  use pbs_api_types::{
>      Authid, BackupNamespace, CRYPT_KEY_ID_SCHEMA, DATASTORE_SCHEMA, GROUP_FILTER_LIST_SCHEMA,
> -    GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_PRUNE,
> -    PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, RESYNC_CORRUPT_SCHEMA,
> -    RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
> +    GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_BACKUP,
> +    PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA,
> +    RESYNC_CORRUPT_SCHEMA, RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
>      SYNC_WORKER_THREADS_SCHEMA, SyncJobConfig, TRANSFER_LAST_SCHEMA,
>  };
>  use pbs_config::CachedUserInfo;
> @@ -36,8 +36,8 @@ pub fn check_pull_privs(
>      user_info.check_privs(
>          auth_id,
>          &local_store_ns_acl_path,
> -        PRIV_DATASTORE_BACKUP,
> -        false,
> +        PRIV_DATASTORE_APPEND | PRIV_DATASTORE_BACKUP,
> +        true,
>      )?;
>  
>      if let Some(remote) = remote {
> diff --git a/src/server/pull.rs b/src/server/pull.rs
> index 8c6253604..856e0fc5a 100644
> --- a/src/server/pull.rs
> +++ b/src/server/pull.rs
> @@ -1442,13 +1442,16 @@ fn check_and_create_ns(params: &PullParameters, ns: &BackupNamespace) -> Result<
>          created = true;
>      }
>  
> -    check_ns_privs(
> -        params.target.store.name(),
> -        ns,
> -        &params.owner,
> -        PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_APPEND,
> -    )
> -    .map_err(|err| format_err!("sync into {store_ns_str} not allowed - {err}"))?;
> +    let privs = if params.resync_corrupt {
> +        PRIV_DATASTORE_BACKUP
Should we really skip the entire namespace if we only have append
permission? We could still pull the snapshots that aren't corrupt.

-- 
Robert Obkircher <r.obkircher@proxmox.com>