* [pbs-devel] [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors
@ 2020-11-02 10:48 Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 2/3] sync: add access check tests Fabian Grünbichler
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2020-11-02 10:48 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/config/cached_user_info.rs | 8 ++++++++
src/config/user.rs | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs
index f56c07a8..518cf050 100644
--- a/src/config/cached_user_info.rs
+++ b/src/config/cached_user_info.rs
@@ -57,6 +57,14 @@ impl CachedUserInfo {
Ok(config)
}
+ #[cfg(test)]
+ pub(crate) fn test_new(user_cfg: SectionConfigData, acl_tree: AclTree) -> Self {
+ Self {
+ user_cfg: Arc::new(user_cfg),
+ acl_tree: Arc::new(acl_tree),
+ }
+ }
+
/// Test if a authentication id is enabled and not expired
pub fn is_active_auth_id(&self, auth_id: &Authid) -> bool {
let userid = auth_id.user();
diff --git a/src/config/user.rs b/src/config/user.rs
index 5966c96d..3254183b 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -241,6 +241,14 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
Ok(())
}
+#[cfg(test)]
+pub(crate) fn test_cfg_from_str(raw: &str) -> Result<(SectionConfigData, [u8;32]), Error> {
+ let cfg = init();
+ let parsed = cfg.parse("test_user_cfg", raw)?;
+
+ Ok((parsed, [0;32]))
+}
+
// shell completion helper
pub fn complete_userid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
match config() {
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/3] sync: add access check tests
2020-11-02 10:48 [pbs-devel] [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Fabian Grünbichler
@ 2020-11-02 10:48 ` Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: extend managing remotes Fabian Grünbichler
2020-11-02 20:17 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2020-11-02 10:48 UTC (permalink / raw)
To: pbs-devel
should cover all the current scenarios. remote server-side checks can't
be meaningfully unit-tested, but they are simple enough so should
hopefully never break.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/config/sync.rs | 114 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 0d67b033..f9810369 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -32,6 +32,7 @@ pub fn check_sync_job_read_access(
let remote_privs = user_info.lookup_privs(&auth_id, &["remote", &job.remote]);
remote_privs & PRIV_REMOTE_AUDIT != 0
}
+
// user can run the corresponding pull job
pub fn check_sync_job_modify_access(
user_info: &CachedUserInfo,
@@ -410,3 +411,116 @@ pub const ROUTER: Router = Router::new()
.get(&API_METHOD_LIST_SYNC_JOBS)
.post(&API_METHOD_CREATE_SYNC_JOB)
.match_all("id", &ITEM_ROUTER);
+
+
+#[test]
+fn sync_job_access_test() -> Result<(), Error> {
+ let (user_cfg, _) = crate::config::user::test_cfg_from_str(r###"
+user: noperm@pbs
+
+user: read@pbs
+
+user: write@pbs
+
+"###).expect("test user.cfg is not parsable");
+ let acl_tree = crate::config::acl::AclTree::from_raw(r###"
+acl:1:/datastore/localstore1:read@pbs,write@pbs:DatastoreAudit
+acl:1:/datastore/localstore1:write@pbs:DatastoreBackup
+acl:1:/datastore/localstore2:write@pbs:DatastorePowerUser
+acl:1:/datastore/localstore3:write@pbs:DatastoreAdmin
+acl:1:/remote/remote1:read@pbs,write@pbs:RemoteAudit
+acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
+"###).expect("test acl.cfg is not parsable");
+
+ let user_info = CachedUserInfo::test_new(user_cfg, acl_tree);
+
+ let root_auth_id = Authid::root_auth_id();
+
+ let no_perm_auth_id: Authid = "noperm@pbs".parse()?;
+ let read_auth_id: Authid = "read@pbs".parse()?;
+ let write_auth_id: Authid = "write@pbs".parse()?;
+
+ let mut job = SyncJobConfig {
+ id: "regular".to_string(),
+ remote: "remote0".to_string(),
+ remote_store: "remotestore1".to_string(),
+ store: "localstore0".to_string(),
+ owner: Some(write_auth_id.clone()),
+ comment: None,
+ remove_vanished: None,
+ schedule: None,
+ };
+
+ // should work without ACLs
+ assert_eq!(check_sync_job_read_access(&user_info, &root_auth_id, &job), true);
+ assert_eq!(check_sync_job_modify_access(&user_info, &root_auth_id, &job), true);
+
+ // user without permissions must fail
+ assert_eq!(check_sync_job_read_access(&user_info, &no_perm_auth_id, &job), false);
+ assert_eq!(check_sync_job_modify_access(&user_info, &no_perm_auth_id, &job), false);
+
+ // reading without proper read permissions on either remote or local must fail
+ assert_eq!(check_sync_job_read_access(&user_info, &read_auth_id, &job), false);
+
+ // reading without proper read permissions on local end must fail
+ job.remote = "remote1".to_string();
+ assert_eq!(check_sync_job_read_access(&user_info, &read_auth_id, &job), false);
+
+ // reading without proper read permissions on remote end must fail
+ job.remote = "remote0".to_string();
+ job.store = "localstore1".to_string();
+ assert_eq!(check_sync_job_read_access(&user_info, &read_auth_id, &job), false);
+
+ // writing without proper write permissions on either end must fail
+ job.store = "localstore0".to_string();
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), false);
+
+ // writing without proper write permissions on local end must fail
+ job.remote = "remote1".to_string();
+
+ // writing without proper write permissions on remote end must fail
+ job.remote = "remote0".to_string();
+ job.store = "localstore1".to_string();
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), false);
+
+ // reset remote to one where users have access
+ job.remote = "remote1".to_string();
+
+ // user with read permission can only read, but not modify/run
+ assert_eq!(check_sync_job_read_access(&user_info, &read_auth_id, &job), true);
+ job.owner = Some(read_auth_id.clone());
+ assert_eq!(check_sync_job_modify_access(&user_info, &read_auth_id, &job), false);
+ job.owner = None;
+ assert_eq!(check_sync_job_modify_access(&user_info, &read_auth_id, &job), false);
+ job.owner = Some(write_auth_id.clone());
+ assert_eq!(check_sync_job_modify_access(&user_info, &read_auth_id, &job), false);
+
+ // user with simple write permission can modify/run
+ assert_eq!(check_sync_job_read_access(&user_info, &write_auth_id, &job), true);
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), true);
+
+ // but can't modify/run with deletion
+ job.remove_vanished = Some(true);
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), false);
+
+ // unless they have Datastore.Prune as well
+ job.store = "localstore2".to_string();
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), true);
+
+ // changing owner is not possible
+ job.owner = Some(read_auth_id.clone());
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), false);
+
+ // also not to the default 'backup@pam'
+ job.owner = None;
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), false);
+
+ // unless they have Datastore.Modify as well
+ job.store = "localstore3".to_string();
+ job.owner = Some(read_auth_id.clone());
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), true);
+ job.owner = None;
+ assert_eq!(check_sync_job_modify_access(&user_info, &write_auth_id, &job), true);
+
+ Ok(())
+}
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/3] docs: extend managing remotes
2020-11-02 10:48 [pbs-devel] [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 2/3] sync: add access check tests Fabian Grünbichler
@ 2020-11-02 10:48 ` Fabian Grünbichler
2020-11-02 20:17 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2020-11-02 10:48 UTC (permalink / raw)
To: pbs-devel
with information about required privileges and limitations
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
docs/managing-remotes.rst | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
index e8495db1..382ca84d 100644
--- a/docs/managing-remotes.rst
+++ b/docs/managing-remotes.rst
@@ -79,4 +79,17 @@ either start it manually on the GUI or provide it with a schedule (see
└────────────┴───────┴────────┴──────────────┴───────────┴─────────┘
# proxmox-backup-manager sync-job remove pbs2-local
+For setting up sync jobs, the configuring user needs the following permissions:
+#. ``Remote.Read`` on the ``/remote/{remote}/{remote-store}`` path
+#. at least ``Datastore.Backup`` on the local target datastore (``/datastore/{store}``)
+
+If the ``remove-vanished`` option is set, ``Datastore.Prune`` is required on
+the local datastore as well. If the ``owner`` option is not set (defaulting to
+``backup@pam``) or set to something other than the configuring user,
+``Datastore.Modify`` is required as well.
+
+.. note:: A sync job can only sync backup groups that the configured remote's
+ user/API token can read. If a remote is configured with a user/API token that
+ only has ``Datastore.Backup`` privileges, only the limited set of accessible
+ snapshots owned by that user/API token can be synced.
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] applied-series: [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors
2020-11-02 10:48 [pbs-devel] [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 2/3] sync: add access check tests Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: extend managing remotes Fabian Grünbichler
@ 2020-11-02 20:17 ` Thomas Lamprecht
2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2020-11-02 20:17 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 02.11.20 11:48, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> src/config/cached_user_info.rs | 8 ++++++++
> src/config/user.rs | 8 ++++++++
> 2 files changed, 16 insertions(+)
>
>
applied series, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-02 20:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 10:48 [pbs-devel] [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 2/3] sync: add access check tests Fabian Grünbichler
2020-11-02 10:48 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: extend managing remotes Fabian Grünbichler
2020-11-02 20:17 ` [pbs-devel] applied-series: [PATCH proxmox-backup 1/3] user.cfg/user info: add test constructors Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox