* [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs
@ 2023-11-28 14:16 Hannes Laimer
2023-11-28 14:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job Hannes Laimer
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Hannes Laimer @ 2023-11-28 14:16 UTC (permalink / raw)
To: pbs-devel
Reported-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/config/sync.rs | 10 ++++------
src/server/pull.rs | 6 +++---
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index ea0e08f1..8809465c 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -8,8 +8,8 @@ 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_DATASTORE_READ,
- PRIV_REMOTE_AUDIT, PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
+ PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_AUDIT,
+ PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
};
use pbs_config::sync;
@@ -70,11 +70,9 @@ pub fn check_sync_job_modify_access(
if let Some(remote) = &job.remote {
let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote, &job.remote_store]);
- remote_privs & PRIV_REMOTE_READ != 0
- } else {
- let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
- source_ds_privs & PRIV_DATASTORE_READ != 0
+ return remote_privs & PRIV_REMOTE_READ != 0;
}
+ true
}
#[api(
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 1403c7a7..66ef333a 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -17,7 +17,7 @@ use serde_json::json;
use pbs_api_types::{
print_store_and_ns, Authid, BackupDir, BackupGroup, BackupNamespace, CryptMode, GroupFilter,
GroupListItem, Operation, RateLimitConfig, Remote, SnapshotListItem, MAX_NAMESPACE_DEPTH,
- PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
+ PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ,
};
use pbs_client::{BackupReader, BackupRepository, HttpClient, RemoteChunkReader};
use pbs_config::CachedUserInfo;
@@ -271,8 +271,8 @@ impl PullSource for LocalSource {
&self.store,
namespace.clone(),
0,
- None,
- None,
+ Some(PRIV_DATASTORE_READ),
+ Some(PRIV_DATASTORE_BACKUP),
Some(owner),
)?
.filter_map(Result::ok)
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job
2023-11-28 14:16 [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Hannes Laimer
@ 2023-11-28 14:16 ` Hannes Laimer
2023-11-28 14:40 ` Dominik Csapak
2023-11-28 14:39 ` [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Dominik Csapak
2023-11-28 16:38 ` [pbs-devel] applied: " Thomas Lamprecht
2 siblings, 1 reply; 5+ messages in thread
From: Hannes Laimer @ 2023-11-28 14:16 UTC (permalink / raw)
To: pbs-devel
Reported-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/SyncJobEdit.js | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index d20def74..58c8fb5c 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -73,6 +73,7 @@ Ext.define('PBS.window.SyncJobEdit', {
}
if (!me.isCreate) {
PBS.Utils.delete_if_default(values, 'rate-in');
+ PBS.Utils.delete_if_default(values, 'remote');
if (typeof values.delete === 'string') {
values.delete = values.delete.split(',');
}
@@ -200,9 +201,6 @@ Ext.define('PBS.window.SyncJobEdit', {
xtype: 'pbsRemoteSelector',
allowBlank: false,
name: 'remote',
- cbind: {
- deleteEmpty: '{!isCreate}',
- },
skipEmptyText: true,
listeners: {
change: function(f, value) {
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job
2023-11-28 14:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job Hannes Laimer
@ 2023-11-28 14:40 ` Dominik Csapak
0 siblings, 0 replies; 5+ messages in thread
From: Dominik Csapak @ 2023-11-28 14:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Can now convert a remote sync job to a local sync job
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/28/23 15:16, Hannes Laimer wrote:
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> www/window/SyncJobEdit.js | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index d20def74..58c8fb5c 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -73,6 +73,7 @@ Ext.define('PBS.window.SyncJobEdit', {
> }
> if (!me.isCreate) {
> PBS.Utils.delete_if_default(values, 'rate-in');
> + PBS.Utils.delete_if_default(values, 'remote');
> if (typeof values.delete === 'string') {
> values.delete = values.delete.split(',');
> }
> @@ -200,9 +201,6 @@ Ext.define('PBS.window.SyncJobEdit', {
> xtype: 'pbsRemoteSelector',
> allowBlank: false,
> name: 'remote',
> - cbind: {
> - deleteEmpty: '{!isCreate}',
> - },
> skipEmptyText: true,
> listeners: {
> change: function(f, value) {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs
2023-11-28 14:16 [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Hannes Laimer
2023-11-28 14:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job Hannes Laimer
@ 2023-11-28 14:39 ` Dominik Csapak
2023-11-28 16:38 ` [pbs-devel] applied: " Thomas Lamprecht
2 siblings, 0 replies; 5+ messages in thread
From: Dominik Csapak @ 2023-11-28 14:39 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
with that i can now sync snapshots that can be read from the local user
code seems fine to me but i'm not sure about the PRIV_DATASTORE_READ and
PRIV_DATASTORE_BACKUP use in the last hunk (i don't have a deep understanding
of the code but i couldn't find an issue by testing), so it's only:
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/28/23 15:16, Hannes Laimer wrote:
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/api2/config/sync.rs | 10 ++++------
> src/server/pull.rs | 6 +++---
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
> index ea0e08f1..8809465c 100644
> --- a/src/api2/config/sync.rs
> +++ b/src/api2/config/sync.rs
> @@ -8,8 +8,8 @@ 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_DATASTORE_READ,
> - PRIV_REMOTE_AUDIT, PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
> + PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_AUDIT,
> + PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA,
> };
> use pbs_config::sync;
>
> @@ -70,11 +70,9 @@ pub fn check_sync_job_modify_access(
>
> if let Some(remote) = &job.remote {
> let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote, &job.remote_store]);
> - remote_privs & PRIV_REMOTE_READ != 0
> - } else {
> - let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
> - source_ds_privs & PRIV_DATASTORE_READ != 0
> + return remote_privs & PRIV_REMOTE_READ != 0;
> }
> + true
> }
>
> #[api(
> diff --git a/src/server/pull.rs b/src/server/pull.rs
> index 1403c7a7..66ef333a 100644
> --- a/src/server/pull.rs
> +++ b/src/server/pull.rs
> @@ -17,7 +17,7 @@ use serde_json::json;
> use pbs_api_types::{
> print_store_and_ns, Authid, BackupDir, BackupGroup, BackupNamespace, CryptMode, GroupFilter,
> GroupListItem, Operation, RateLimitConfig, Remote, SnapshotListItem, MAX_NAMESPACE_DEPTH,
> - PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
> + PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ,
> };
> use pbs_client::{BackupReader, BackupRepository, HttpClient, RemoteChunkReader};
> use pbs_config::CachedUserInfo;
> @@ -271,8 +271,8 @@ impl PullSource for LocalSource {
> &self.store,
> namespace.clone(),
> 0,
> - None,
> - None,
> + Some(PRIV_DATASTORE_READ),
> + Some(PRIV_DATASTORE_BACKUP),
> Some(owner),
> )?
> .filter_map(Result::ok)
^ permalink raw reply [flat|nested] 5+ messages in thread* [pbs-devel] applied: [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs
2023-11-28 14:16 [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Hannes Laimer
2023-11-28 14:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job Hannes Laimer
2023-11-28 14:39 ` [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Dominik Csapak
@ 2023-11-28 16:38 ` Thomas Lamprecht
2 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2023-11-28 16:38 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Am 28/11/2023 um 15:16 schrieb Hannes Laimer:
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/api2/config/sync.rs | 10 ++++------
> src/server/pull.rs | 6 +++---
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
>
applied series, with Dominik's R-b and T-b, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-28 16:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-28 14:16 [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Hannes Laimer
2023-11-28 14:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: fix changing remote to local sync job Hannes Laimer
2023-11-28 14:40 ` Dominik Csapak
2023-11-28 14:39 ` [pbs-devel] [PATCH proxmox-backup 1/2] pull: fix permission checks for local syncs Dominik Csapak
2023-11-28 16:38 ` [pbs-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox