[pbs-devel] [PATCH v6 proxmox-backup 19/29] api: sync jobs: expose optional `sync-direction` parameter
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Nov 6 16:20:35 CET 2024
Quoting Christian Ebner (2024-10-31 13:15:09)
> Exposes and switch the config type for sync job operations based
> on the `sync-direction` parameter, exposed on required api endpoints.
>
> If not set, the default config type is `sync` and the default sync
> direction is `pull` for full backwards compatibility. Whenever
> possible, deterimne the sync direction and config type from the sync
typo "determine"
> job config directly rather than requiring it as optional api
> parameter.
>
> Further, extend read and modify access checks by sync direction to
> conditionally check for the required permissions in pull and push
> direction.
>
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> changes since version 5:
> - Squashed permission check patches into this one, as they make not much
> sense without this
> - Only expose optional sync-direction parameter for api endpoints which
> require them, use the job config to determine sync-direction and/or
> config-type otherwise.
>
> src/api2/admin/sync.rs | 34 ++--
> src/api2/config/datastore.rs | 11 +-
> src/api2/config/notifications/mod.rs | 19 +-
> src/api2/config/sync.rs | 280 ++++++++++++++++++++-------
> src/bin/proxmox-backup-proxy.rs | 11 +-
> 5 files changed, 261 insertions(+), 94 deletions(-)
>
> diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
> index be324564c..8a242b1c3 100644
> --- a/src/api2/admin/sync.rs
> +++ b/src/api2/admin/sync.rs
> @@ -1,6 +1,7 @@
> //! Datastore Synchronization Job Management
>
> use anyhow::{bail, format_err, Error};
> +use serde::Deserialize;
> use serde_json::Value;
>
> use proxmox_router::{
> @@ -29,6 +30,10 @@ use crate::{
> schema: DATASTORE_SCHEMA,
> optional: true,
> },
> + "sync-direction": {
> + type: SyncDirection,
> + optional: true,
> + },
> },
> },
> returns: {
> @@ -44,6 +49,7 @@ use crate::{
> /// List all sync jobs
> pub fn list_sync_jobs(
> store: Option<String>,
> + sync_direction: Option<SyncDirection>,
> _param: Value,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<SyncJobStatus>, Error> {
> @@ -52,8 +58,9 @@ pub fn list_sync_jobs(
>
> let (config, digest) = sync::config()?;
>
> + let sync_direction = sync_direction.unwrap_or_default();
> let job_config_iter = config
> - .convert_to_typed_array("sync")?
> + .convert_to_typed_array(sync_direction.as_config_type_str())?
> .into_iter()
> .filter(|job: &SyncJobConfig| {
> if let Some(store) = &store {
> @@ -62,7 +69,9 @@ pub fn list_sync_jobs(
> true
> }
> })
> - .filter(|job: &SyncJobConfig| check_sync_job_read_access(&user_info, &auth_id, job));
> + .filter(|job: &SyncJobConfig| {
> + check_sync_job_read_access(&user_info, &auth_id, job, sync_direction)
> + });
>
> let mut list = Vec::new();
>
> @@ -106,24 +115,23 @@ pub fn run_sync_job(
> let user_info = CachedUserInfo::new()?;
>
> let (config, _digest) = sync::config()?;
> - let sync_job: SyncJobConfig = config.lookup("sync", &id)?;
> + let (config_type, config_section) = config
> + .sections
> + .get(&id)
> + .ok_or_else(|| format_err!("No sync job with id '{id}' found in config"))?;
> +
> + let sync_direction = SyncDirection::from_config_type_str(config_type)?;
> + let sync_job = SyncJobConfig::deserialize(config_section)?;
>
> - if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) {
> - bail!("permission check failed");
> + if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job, sync_direction) {
> + bail!("permission check failed, '{auth_id}' is missing access");
> }
>
> let job = Job::new("syncjob", &id)?;
>
> let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>
> - let upid_str = do_sync_job(
> - job,
> - sync_job,
> - &auth_id,
> - None,
> - SyncDirection::Pull,
> - to_stdout,
> - )?;
> + let upid_str = do_sync_job(job, sync_job, &auth_id, None, sync_direction, to_stdout)?;
>
> Ok(upid_str)
> }
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index ca6edf05a..c151eda10 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -13,8 +13,9 @@ use proxmox_uuid::Uuid;
>
> use pbs_api_types::{
> Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning, KeepOptions,
> - MaintenanceMode, PruneJobConfig, PruneJobOptions, DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE,
> - PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
> + MaintenanceMode, PruneJobConfig, PruneJobOptions, SyncDirection, DATASTORE_SCHEMA,
> + PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
> + PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
> };
> use pbs_config::BackupLockGuard;
> use pbs_datastore::chunk_store::ChunkStore;
> @@ -498,8 +499,10 @@ pub async fn delete_datastore(
> for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
> delete_verification_job(job.config.id, None, rpcenv)?
> }
> - for job in list_sync_jobs(Some(name.clone()), Value::Null, rpcenv)? {
> - delete_sync_job(job.config.id, None, rpcenv)?
> + for direction in [SyncDirection::Pull, SyncDirection::Push] {
> + for job in list_sync_jobs(Some(name.clone()), Some(direction), Value::Null, rpcenv)? {
> + delete_sync_job(job.config.id, None, rpcenv)?
> + }
> }
> for job in list_prune_jobs(Some(name.clone()), Value::Null, rpcenv)? {
> delete_prune_job(job.config.id, None, rpcenv)?
> diff --git a/src/api2/config/notifications/mod.rs b/src/api2/config/notifications/mod.rs
> index dfe82ed03..31c4851c1 100644
> --- a/src/api2/config/notifications/mod.rs
> +++ b/src/api2/config/notifications/mod.rs
> @@ -9,7 +9,7 @@ use proxmox_schema::api;
> use proxmox_sortable_macro::sortable;
>
> use crate::api2::admin::datastore::get_datastore_list;
> -use pbs_api_types::PRIV_SYS_AUDIT;
> +use pbs_api_types::{SyncDirection, PRIV_SYS_AUDIT};
>
> use crate::api2::admin::prune::list_prune_jobs;
> use crate::api2::admin::sync::list_sync_jobs;
> @@ -154,13 +154,15 @@ pub fn get_values(
> });
> }
>
> - let sync_jobs = list_sync_jobs(None, param.clone(), rpcenv)?;
> - for job in sync_jobs {
> - values.push(MatchableValue {
> - field: "job-id".into(),
> - value: job.config.id,
> - comment: job.config.comment,
> - });
> + for direction in [SyncDirection::Pull, SyncDirection::Push] {
> + let sync_jobs = list_sync_jobs(None, Some(direction), param.clone(), rpcenv)?;
> + for job in sync_jobs {
> + values.push(MatchableValue {
> + field: "job-id".into(),
> + value: job.config.id,
> + comment: job.config.comment,
> + });
> + }
> }
>
> let verify_jobs = list_verification_jobs(None, param.clone(), rpcenv)?;
> @@ -184,6 +186,7 @@ pub fn get_values(
> "package-updates",
> "prune",
> "sync",
> + "sync-push",
> "system-mail",
> "tape-backup",
> "tape-load",
> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
> index 3963049e9..2f32aaccb 100644
> --- a/src/api2/config/sync.rs
> +++ b/src/api2/config/sync.rs
> @@ -1,6 +1,7 @@
> use ::serde::{Deserialize, Serialize};
> use anyhow::{bail, Error};
> use hex::FromHex;
> +use pbs_api_types::SyncDirection;
> use serde_json::Value;
>
> use proxmox_router::{http_bail, Permission, Router, RpcEnvironment};
> @@ -8,8 +9,9 @@ use proxmox_schema::{api, param_bail};
>
> use pbs_api_types::{
> Authid, SyncJobConfig, SyncJobConfigUpdater, JOB_ID_SCHEMA, PRIV_DATASTORE_AUDIT,
> - PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_AUDIT,
> - PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
> + PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
> + PRIV_REMOTE_AUDIT, PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_MODIFY,
> + PRIV_REMOTE_DATASTORE_PRUNE, PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
> };
> use pbs_config::sync;
>
> @@ -20,18 +22,35 @@ pub fn check_sync_job_read_access(
> user_info: &CachedUserInfo,
> auth_id: &Authid,
> job: &SyncJobConfig,
> + sync_direction: SyncDirection,
> ) -> bool {
> + // check for audit access on datastore/namespace, applies for pull and push direction
> let ns_anchor_privs = user_info.lookup_privs(auth_id, &job.acl_path());
> if ns_anchor_privs & PRIV_DATASTORE_AUDIT == 0 {
> return false;
> }
>
> - if let Some(remote) = &job.remote {
> - let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote]);
> - remote_privs & PRIV_REMOTE_AUDIT != 0
> - } else {
> - let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
> - source_ds_privs & PRIV_DATASTORE_AUDIT != 0
> + match sync_direction {
> + SyncDirection::Pull => {
> + if let Some(remote) = &job.remote {
> + let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote]);
> + remote_privs & PRIV_REMOTE_AUDIT != 0
> + } else {
> + let source_ds_privs =
> + user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
> + source_ds_privs & PRIV_DATASTORE_AUDIT != 0
> + }
> + }
> + SyncDirection::Push => {
> + // check for audit access on remote/datastore/namespace
> + if let Some(target_acl_path) = job.remote_acl_path() {
> + let remote_privs = user_info.lookup_privs(auth_id, &target_acl_path);
> + remote_privs & PRIV_REMOTE_AUDIT != 0
the other two checks above check the source side, this checks the the target
side.. should we check both here?
> + } else {
> + // Remote must always be present for sync in push direction, fail otherwise
> + false
> + }
> + }
> }
> }
>
> @@ -43,41 +62,93 @@ fn is_correct_owner(auth_id: &Authid, job: &SyncJobConfig) -> bool {
> }
> }
>
> -/// checks whether user can run the corresponding pull job
> +/// checks whether user can run the corresponding sync job, depending on sync direction
> ///
> -/// namespace creation/deletion ACL and backup group ownership checks happen in the pull code directly.
> +/// namespace creation/deletion ACL and backup group ownership checks happen in the pull/push code
> +/// directly.
> /// remote side checks/filters remote datastore/namespace/group access.
> pub fn check_sync_job_modify_access(
> user_info: &CachedUserInfo,
> auth_id: &Authid,
> job: &SyncJobConfig,
> + sync_direction: SyncDirection,
> ) -> bool {
> - let ns_anchor_privs = user_info.lookup_privs(auth_id, &job.acl_path());
> - if ns_anchor_privs & PRIV_DATASTORE_BACKUP == 0 || ns_anchor_privs & PRIV_DATASTORE_AUDIT == 0 {
> - return false;
> - }
> + match sync_direction {
> + SyncDirection::Pull => {
> + let ns_anchor_privs = user_info.lookup_privs(auth_id, &job.acl_path());
> + if ns_anchor_privs & PRIV_DATASTORE_BACKUP == 0
> + || ns_anchor_privs & PRIV_DATASTORE_AUDIT == 0
> + {
> + return false;
> + }
> +
> + if let Some(true) = job.remove_vanished {
> + if ns_anchor_privs & PRIV_DATASTORE_PRUNE == 0 {
> + return false;
> + }
> + }
>
> - if let Some(true) = job.remove_vanished {
> - if ns_anchor_privs & PRIV_DATASTORE_PRUNE == 0 {
> - return false;
> + // same permission as changing ownership after syncing
> + if !is_correct_owner(auth_id, job) && ns_anchor_privs & PRIV_DATASTORE_MODIFY == 0 {
> + return false;
> + }
> +
> + if let Some(remote) = &job.remote {
> + let remote_privs =
> + user_info.lookup_privs(auth_id, &["remote", remote, &job.remote_store]);
> + return remote_privs & PRIV_REMOTE_READ != 0;
> + }
> + true
> }
> - }
> + SyncDirection::Push => {
> + // Remote must always be present for sync in push direction, fail otherwise
> + let target_privs = if let Some(target_acl_path) = job.remote_acl_path() {
> + user_info.lookup_privs(auth_id, &target_acl_path)
> + } else {
> + return false;
> + };
> +
> + // check user is allowed to create backups on remote datastore
> + if target_privs & PRIV_REMOTE_DATASTORE_BACKUP == 0 {
> + return false;
> + }
>
> - // same permission as changing ownership after syncing
> - if !is_correct_owner(auth_id, job) && ns_anchor_privs & PRIV_DATASTORE_MODIFY == 0 {
> - return false;
> - }
> + if let Some(true) = job.remove_vanished {
> + // check user is allowed to prune backup snapshots on remote datastore
> + if target_privs & PRIV_REMOTE_DATASTORE_PRUNE == 0 {
> + return false;
> + }
> + }
> +
> + // check user is not the owner of the sync job, but has remote datastore modify permissions
> + if !is_correct_owner(auth_id, job) && target_privs & PRIV_REMOTE_DATASTORE_MODIFY == 0 {
> + return false;
> + }
isn't this wrong? if I am modifying/running a sync job "owned" by somebody
else, then I need to have Datastore.Read or Datastore.Modify on the *local*
source datastore+namespace.. else I could use such a sync job to exfiltrate
backups I wouldn't otherwise have access to..
> +
> + // check user is allowed to read from (local) source datastore/namespace
> + let source_privs = user_info.lookup_privs(auth_id, &job.acl_path());
> + if source_privs & PRIV_DATASTORE_AUDIT == 0 {
> + return false;
> + }
>
> - if let Some(remote) = &job.remote {
> - let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote, &job.remote_store]);
> - return remote_privs & PRIV_REMOTE_READ != 0;
> + // check for either datastore read or datastore backup access
> + // (the later implying read access for owned snapshot groups)
> + if source_privs & PRIV_DATASTORE_READ != 0 {
> + return true;
> + }
> + source_privs & PRIV_DATASTORE_BACKUP != 0
> + }
> }
> - true
> }
>
> #[api(
> input: {
> - properties: {},
> + properties: {
> + "sync-direction": {
> + type: SyncDirection,
> + optional: true,
> + },
> + },
> },
> returns: {
> description: "List configured jobs.",
> @@ -92,6 +163,7 @@ pub fn check_sync_job_modify_access(
> /// List all sync jobs
> pub fn list_sync_jobs(
> _param: Value,
> + sync_direction: Option<SyncDirection>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<SyncJobConfig>, Error> {
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> @@ -99,13 +171,16 @@ pub fn list_sync_jobs(
>
> let (config, digest) = sync::config()?;
>
> - let list = config.convert_to_typed_array("sync")?;
> + let sync_direction = sync_direction.unwrap_or_default();
> + let list = config.convert_to_typed_array(sync_direction.as_config_type_str())?;
>
> rpcenv["digest"] = hex::encode(digest).into();
>
> let list = list
> .into_iter()
> - .filter(|sync_job| check_sync_job_read_access(&user_info, &auth_id, sync_job))
> + .filter(|sync_job| {
> + check_sync_job_read_access(&user_info, &auth_id, sync_job, sync_direction)
> + })
> .collect();
> Ok(list)
> }
> @@ -118,6 +193,10 @@ pub fn list_sync_jobs(
> type: SyncJobConfig,
> flatten: true,
> },
> + "sync-direction": {
> + type: SyncDirection,
> + optional: true,
> + },
> },
> },
> access: {
> @@ -128,14 +207,16 @@ pub fn list_sync_jobs(
> /// Create a new sync job.
> pub fn create_sync_job(
> config: SyncJobConfig,
> + sync_direction: Option<SyncDirection>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
> + let sync_direction = sync_direction.unwrap_or_default();
>
> let _lock = sync::lock_config()?;
>
> - if !check_sync_job_modify_access(&user_info, &auth_id, &config) {
> + if !check_sync_job_modify_access(&user_info, &auth_id, &config, sync_direction) {
> bail!("permission check failed");
> }
>
> @@ -158,7 +239,7 @@ pub fn create_sync_job(
> param_bail!("id", "job '{}' already exists.", config.id);
> }
>
> - section_config.set_data(&config.id, "sync", &config)?;
> + section_config.set_data(&config.id, sync_direction.as_config_type_str(), &config)?;
>
> sync::save_config(§ion_config)?;
>
> @@ -188,8 +269,17 @@ pub fn read_sync_job(id: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Sync
>
> let (config, digest) = sync::config()?;
>
> - let sync_job = config.lookup("sync", &id)?;
> - if !check_sync_job_read_access(&user_info, &auth_id, &sync_job) {
> + let (sync_job, sync_direction) =
> + if let Some((config_type, config_section)) = config.sections.get(&id) {
> + (
> + SyncJobConfig::deserialize(config_section)?,
> + SyncDirection::from_config_type_str(config_type)?,
> + )
> + } else {
> + http_bail!(NOT_FOUND, "job '{id}' does not exist.")
> + };
> +
> + if !check_sync_job_read_access(&user_info, &auth_id, &sync_job, sync_direction) {
> bail!("permission check failed");
> }
>
> @@ -284,7 +374,15 @@ pub fn update_sync_job(
> crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> }
>
> - let mut data: SyncJobConfig = config.lookup("sync", &id)?;
> + let (mut data, sync_direction) =
> + if let Some((config_type, config_section)) = config.sections.get(&id) {
> + (
> + SyncJobConfig::deserialize(config_section)?,
> + SyncDirection::from_config_type_str(config_type)?,
> + )
> + } else {
> + http_bail!(NOT_FOUND, "job '{id}' does not exist.")
> + };
>
> if let Some(delete) = delete {
> for delete_prop in delete {
> @@ -405,11 +503,11 @@ pub fn update_sync_job(
> }
> }
>
> - if !check_sync_job_modify_access(&user_info, &auth_id, &data) {
> + if !check_sync_job_modify_access(&user_info, &auth_id, &data, sync_direction) {
> bail!("permission check failed");
> }
>
> - config.set_data(&id, "sync", &data)?;
> + config.set_data(&id, sync_direction.as_config_type_str(), &data)?;
>
> sync::save_config(&config)?;
>
> @@ -456,17 +554,16 @@ pub fn delete_sync_job(
> crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> }
>
> - match config.lookup("sync", &id) {
> - Ok(job) => {
> - if !check_sync_job_modify_access(&user_info, &auth_id, &job) {
> - bail!("permission check failed");
> - }
> - config.sections.remove(&id);
> - }
> - Err(_) => {
> - http_bail!(NOT_FOUND, "job '{}' does not exist.", id)
> + if let Some((config_type, config_section)) = config.sections.get(&id) {
> + let sync_direction = SyncDirection::from_config_type_str(config_type)?;
> + let job = SyncJobConfig::deserialize(config_section)?;
> + if !check_sync_job_modify_access(&user_info, &auth_id, &job, sync_direction) {
> + bail!("permission check failed");
> }
> - };
> + config.sections.remove(&id);
> + } else {
> + http_bail!(NOT_FOUND, "job '{}' does not exist.", id)
> + }
>
> sync::save_config(&config)?;
>
> @@ -536,39 +633,67 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> };
>
> // should work without ACLs
> - assert!(check_sync_job_read_access(&user_info, root_auth_id, &job));
> - assert!(check_sync_job_modify_access(&user_info, root_auth_id, &job));
> + assert!(check_sync_job_read_access(
> + &user_info,
> + root_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
> + assert!(check_sync_job_modify_access(
> + &user_info,
> + root_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
>
> // user without permissions must fail
> assert!(!check_sync_job_read_access(
> &user_info,
> &no_perm_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
> assert!(!check_sync_job_modify_access(
> &user_info,
> &no_perm_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // reading without proper read permissions on either remote or local must fail
> - assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
> + assert!(!check_sync_job_read_access(
> + &user_info,
> + &read_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
>
> // reading without proper read permissions on local end must fail
> job.remote = Some("remote1".to_string());
> - assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
> + assert!(!check_sync_job_read_access(
> + &user_info,
> + &read_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
>
> // reading without proper read permissions on remote end must fail
> job.remote = Some("remote0".to_string());
> job.store = "localstore1".to_string();
> - assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
> + assert!(!check_sync_job_read_access(
> + &user_info,
> + &read_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
>
> // writing without proper write permissions on either end must fail
> job.store = "localstore0".to_string();
> assert!(!check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // writing without proper write permissions on local end must fail
> @@ -580,39 +705,54 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(!check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // reset remote to one where users have access
> job.remote = Some("remote1".to_string());
>
> // user with read permission can only read, but not modify/run
> - assert!(check_sync_job_read_access(&user_info, &read_auth_id, &job));
> + assert!(check_sync_job_read_access(
> + &user_info,
> + &read_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
> job.owner = Some(read_auth_id.clone());
> assert!(!check_sync_job_modify_access(
> &user_info,
> &read_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
> job.owner = None;
> assert!(!check_sync_job_modify_access(
> &user_info,
> &read_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
> job.owner = Some(write_auth_id.clone());
> assert!(!check_sync_job_modify_access(
> &user_info,
> &read_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // user with simple write permission can modify/run
> - assert!(check_sync_job_read_access(&user_info, &write_auth_id, &job));
> + assert!(check_sync_job_read_access(
> + &user_info,
> + &write_auth_id,
> + &job,
> + SyncDirection::Pull,
> + ));
> assert!(check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // but can't modify/run with deletion
> @@ -620,7 +760,8 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(!check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // unless they have Datastore.Prune as well
> @@ -628,7 +769,8 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // changing owner is not possible
> @@ -636,7 +778,8 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(!check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // also not to the default 'root at pam'
> @@ -644,7 +787,8 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(!check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> // unless they have Datastore.Modify as well
> @@ -653,13 +797,15 @@ acl:1:/remote/remote1/remotestore1:write at pbs:RemoteSyncOperator
> assert!(check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
> job.owner = None;
> assert!(check_sync_job_modify_access(
> &user_info,
> &write_auth_id,
> - &job
> + &job,
> + SyncDirection::Pull,
> ));
>
> Ok(())
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 6f19a3fbd..70283510d 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -589,7 +589,14 @@ async fn schedule_datastore_sync_jobs() {
> Ok((config, _digest)) => config,
> };
>
> - for (job_id, (_, job_config)) in config.sections {
> + for (job_id, (job_type, job_config)) in config.sections {
> + let sync_direction = match SyncDirection::from_config_type_str(&job_type) {
> + Ok(direction) => direction,
> + Err(err) => {
> + eprintln!("unexpected config type in sync job config - {err}");
> + continue;
> + }
> + };
> let job_config: SyncJobConfig = match serde_json::from_value(job_config) {
> Ok(c) => c,
> Err(err) => {
> @@ -616,7 +623,7 @@ async fn schedule_datastore_sync_jobs() {
> job_config,
> &auth_id,
> Some(event_str),
> - SyncDirection::Pull,
> + sync_direction,
> false,
> ) {
> eprintln!("unable to start datastore sync job {job_id} - {err}");
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
More information about the pbs-devel
mailing list