From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6E90C1FF161 for ; Wed, 6 Nov 2024 16:20:38 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4A08811878; Wed, 6 Nov 2024 16:20:48 +0100 (CET) MIME-Version: 1.0 In-Reply-To: <20241031121519.434337-20-c.ebner@proxmox.com> References: <20241031121519.434337-1-c.ebner@proxmox.com> <20241031121519.434337-20-c.ebner@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= To: Christian Ebner , pbs-devel@lists.proxmox.com Date: Wed, 06 Nov 2024 16:20:35 +0100 Message-ID: <173090643519.79072.2923413753129715762@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, datastore.rs, datastore.read, job.store, sync.rs, mod.rs, proxmox-backup-proxy.rs] Subject: Re: [pbs-devel] [PATCH v6 proxmox-backup 19/29] api: sync jobs: expose optional `sync-direction` parameter X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 > --- > 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, > + sync_direction: Option, > _param: Value, > rpcenv: &mut dyn RpcEnvironment, > ) -> Result, 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, > rpcenv: &mut dyn RpcEnvironment, > ) -> Result, 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, > 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 > 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@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@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@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@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@pbs:RemoteSyncOperator > assert!(!check_sync_job_modify_access( > &user_info, > &write_auth_id, > - &job > + &job, > + SyncDirection::Pull, > )); > > // also not to the default 'root@pam' > @@ -644,7 +787,8 @@ acl:1:/remote/remote1/remotestore1:write@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@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@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel