* [pdm-devel] [PATCH datacenter-manager 0/2] fix #6901: add explicit permissions for PBS status and RRD endpoints
@ 2025-10-10 15:18 Shan Shaji
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints Shan Shaji
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path Shan Shaji
0 siblings, 2 replies; 7+ messages in thread
From: Shan Shaji @ 2025-10-10 15:18 UTC (permalink / raw)
To: pdm-devel
If a non-root user tried to view the overview of a PBS, a
"403: permission check failed" error was shown. Additionally,
the RRD data for the node and datastores were not visible.
To fix the issue, explicit permission checks were added for
the PBS RRD endpoints and the PBS status endpoint.
Ticket #6901 also reports a similar issue in the EVPN panel,
which will be addressed in a separate patch.
Shan Shaji (2):
fix #6901: api: add permission checks for PBS rrd endpoints
fix #6901: api: remove `node` reference from templated privilege path
server/src/api/pbs/mod.rs | 3 ++-
server/src/api/pbs/rrddata.rs | 11 ++++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints
2025-10-10 15:18 [pdm-devel] [PATCH datacenter-manager 0/2] fix #6901: add explicit permissions for PBS status and RRD endpoints Shan Shaji
@ 2025-10-10 15:18 ` Shan Shaji
2025-10-13 8:41 ` Shannon Sterz
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path Shan Shaji
1 sibling, 1 reply; 7+ messages in thread
From: Shan Shaji @ 2025-10-10 15:18 UTC (permalink / raw)
To: pdm-devel
When a non-root user tried to view the RRD data of the PBS node or
datastores, even with Administrator privileges, the API was
returning a "403: permission check failed" error. This occured
because the access property was not defined inside the `api` macro.
To fix the issue, a resource level permission check was added.
Now if a user has atleast the `Resource.Audit` permission, then they
can access the RRD data of the node and datastores.
Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
server/src/api/pbs/rrddata.rs | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/server/src/api/pbs/rrddata.rs b/server/src/api/pbs/rrddata.rs
index aa980d4..c8885de 100644
--- a/server/src/api/pbs/rrddata.rs
+++ b/server/src/api/pbs/rrddata.rs
@@ -2,8 +2,9 @@ use anyhow::Error;
use pdm_api_types::{
remotes::REMOTE_ID_SCHEMA,
rrddata::{PbsDatastoreDataPoint, PbsNodeDataPoint},
+ PRIV_RESOURCE_AUDIT,
};
-use proxmox_router::Router;
+use proxmox_router::{Permission, Router};
use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
use proxmox_schema::api;
use serde_json::Value;
@@ -100,6 +101,10 @@ impl DataPoint for PbsDatastoreDataPoint {
},
},
},
+ access: {
+ permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
+ description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
+ }
)]
/// Read PBS node stats
async fn get_pbs_node_rrd_data(
@@ -125,6 +130,10 @@ async fn get_pbs_node_rrd_data(
},
},
},
+ access: {
+ permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
+ description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
+ }
)]
/// Read PBS datastore stats
async fn get_pbs_datastore_rrd_data(
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path
2025-10-10 15:18 [pdm-devel] [PATCH datacenter-manager 0/2] fix #6901: add explicit permissions for PBS status and RRD endpoints Shan Shaji
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints Shan Shaji
@ 2025-10-10 15:18 ` Shan Shaji
2025-10-13 8:41 ` Shannon Sterz
1 sibling, 1 reply; 7+ messages in thread
From: Shan Shaji @ 2025-10-10 15:18 UTC (permalink / raw)
To: pdm-devel
If a non root user tried to view the overview of a PBS, it was
showing "403: permission check failed" error. This occured because the
privilege path included the "node" object which is neither accepted
as a parameter in the endpoint nor passed from the UI.
To fix the issue removed the "node" reference. Now if the user has
atleast the `Resource.Audit` permission, they can view PBS status under
the overview panel.
Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
server/src/api/pbs/mod.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
index dc31f62..65a2e43 100644
--- a/server/src/api/pbs/mod.rs
+++ b/server/src/api/pbs/mod.rs
@@ -272,7 +272,8 @@ pub async fn scan_remote_pbs(
},
},
access: {
- permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
+ permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
+ description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
},
)]
/// Get status for the PBS remote
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints Shan Shaji
@ 2025-10-13 8:41 ` Shannon Sterz
2025-10-14 7:59 ` Shan Shaji
0 siblings, 1 reply; 7+ messages in thread
From: Shannon Sterz @ 2025-10-13 8:41 UTC (permalink / raw)
To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion
On Fri Oct 10, 2025 at 5:18 PM CEST, Shan Shaji wrote:
> When a non-root user tried to view the RRD data of the PBS node or
> datastores, even with Administrator privileges, the API was
> returning a "403: permission check failed" error. This occured
> because the access property was not defined inside the `api` macro.
>
> To fix the issue, a resource level permission check was added.
> Now if a user has atleast the `Resource.Audit` permission, then they
> can access the RRD data of the node and datastores.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
> server/src/api/pbs/rrddata.rs | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/server/src/api/pbs/rrddata.rs b/server/src/api/pbs/rrddata.rs
> index aa980d4..c8885de 100644
> --- a/server/src/api/pbs/rrddata.rs
> +++ b/server/src/api/pbs/rrddata.rs
> @@ -2,8 +2,9 @@ use anyhow::Error;
> use pdm_api_types::{
> remotes::REMOTE_ID_SCHEMA,
> rrddata::{PbsDatastoreDataPoint, PbsNodeDataPoint},
> + PRIV_RESOURCE_AUDIT,
> };
> -use proxmox_router::Router;
> +use proxmox_router::{Permission, Router};
> use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
> use proxmox_schema::api;
> use serde_json::Value;
> @@ -100,6 +101,10 @@ impl DataPoint for PbsDatastoreDataPoint {
> },
> },
> },
> + access: {
> + permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
> + description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
nit: a typo snuck in here, this should be `at least`
also correct me if im wrong, but this is wrong? for the user to be able
to access this endpoint, they would need `Resource.Audit` permissions on
the specific remote that they are trying to get the RRD data for.
otherwise they'll get a 403 error.
by default permissions propagate down the the acl tree, but that can be
disabled. so `Resource.Audit` on `/resource` does not always imply
`Resource.Audit` on `/resource/my-pbs-remote` etc. hence, this should
probably be:
The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}`.
note that for pbs we autogenerate descriptions for endpoints that only
set "permission" but not "description" [1]. while this has the upside of
always being in-line with the actual code, the descriptions might be a
bit difficult to parse especially for newbies to our codebase.
[1]: https://pbs.proxmox.com/docs/api-viewer/index.html#/tape/drive/{drive}/clean
> + }
> )]
> /// Read PBS node stats
> async fn get_pbs_node_rrd_data(
> @@ -125,6 +130,10 @@ async fn get_pbs_node_rrd_data(
> },
> },
> },
> + access: {
> + permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
> + description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
nit: same as above, should probably be:
The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}/datastore/{datastore}`.
> + }
> )]
> /// Read PBS datastore stats
> async fn get_pbs_datastore_rrd_data(
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path Shan Shaji
@ 2025-10-13 8:41 ` Shannon Sterz
0 siblings, 0 replies; 7+ messages in thread
From: Shannon Sterz @ 2025-10-13 8:41 UTC (permalink / raw)
To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion
On Fri Oct 10, 2025 at 5:18 PM CEST, Shan Shaji wrote:
> If a non root user tried to view the overview of a PBS, it was
> showing "403: permission check failed" error. This occured because the
> privilege path included the "node" object which is neither accepted
> as a parameter in the endpoint nor passed from the UI.
>
> To fix the issue removed the "node" reference. Now if the user has
> atleast the `Resource.Audit` permission, they can view PBS status under
> the overview panel.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
> server/src/api/pbs/mod.rs | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
> index dc31f62..65a2e43 100644
> --- a/server/src/api/pbs/mod.rs
> +++ b/server/src/api/pbs/mod.rs
> @@ -272,7 +272,8 @@ pub async fn scan_remote_pbs(
> },
> },
> access: {
> - permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
> + permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
> + description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
nit: same as previous patch, should be:
The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}/node/{node}`.
> },
> )]
> /// Get status for the PBS remote
other than this and the comments on the previous patch
Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints
2025-10-13 8:41 ` Shannon Sterz
@ 2025-10-14 7:59 ` Shan Shaji
2025-10-14 8:59 ` Shan Shaji
0 siblings, 1 reply; 7+ messages in thread
From: Shan Shaji @ 2025-10-14 7:59 UTC (permalink / raw)
To: Shannon Sterz; +Cc: Proxmox Datacenter Manager development discussion
On Mon Oct 13, 2025 at 10:41 AM CEST, Shannon Sterz wrote:
> On Fri Oct 10, 2025 at 5:18 PM CEST, Shan Shaji wrote:
>> When a non-root user tried to view the RRD data of the PBS node or
>> datastores, even with Administrator privileges, the API was
>> returning a "403: permission check failed" error. This occured
>> because the access property was not defined inside the `api` macro.
>>
>> To fix the issue, a resource level permission check was added.
>> Now if a user has atleast the `Resource.Audit` permission, then they
>> can access the RRD data of the node and datastores.
>>
>> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
>> ---
>> server/src/api/pbs/rrddata.rs | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/server/src/api/pbs/rrddata.rs b/server/src/api/pbs/rrddata.rs
>> index aa980d4..c8885de 100644
>> --- a/server/src/api/pbs/rrddata.rs
>> +++ b/server/src/api/pbs/rrddata.rs
>> @@ -2,8 +2,9 @@ use anyhow::Error;
>> use pdm_api_types::{
>> remotes::REMOTE_ID_SCHEMA,
>> rrddata::{PbsDatastoreDataPoint, PbsNodeDataPoint},
>> + PRIV_RESOURCE_AUDIT,
>> };
>> -use proxmox_router::Router;
>> +use proxmox_router::{Permission, Router};
>> use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
>> use proxmox_schema::api;
>> use serde_json::Value;
>> @@ -100,6 +101,10 @@ impl DataPoint for PbsDatastoreDataPoint {
>> },
>> },
>> },
>> + access: {
>> + permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
>> + description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
>
> nit: a typo snuck in here, this should be `at least`
Thank You! Will update it.
> also correct me if im wrong, but this is wrong? for the user to be able
> to access this endpoint, they would need `Resource.Audit` permissions on
> the specific remote that they are trying to get the RRD data for.
> otherwise they'll get a 403 error.
>
yes exactly, AFAIU the user needs to have `Resource.Audit` permission on
the specific remote to get the RRD data. Sorry my description was not well written.
> by default permissions propagate down the the acl tree, but that can be
> disabled. so `Resource.Audit` on `/resource` does not always imply
> `Resource.Audit` on `/resource/my-pbs-remote` etc. hence, this should
> probably be:
>
> The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}`.
Makes sense and will update to this one. Thank you!
> note that for pbs we autogenerate descriptions for endpoints that only
> set "permission" but not "description" [1]. while this has the upside of
> always being in-line with the actual code, the descriptions might be a
> bit difficult to parse especially for newbies to our codebase.
>
> [1]: https://pbs.proxmox.com/docs/api-viewer/index.html#/tape/drive/{drive}/clean
Understood, thank you!
>> + }
>> )]
>> /// Read PBS node stats
>> async fn get_pbs_node_rrd_data(
>> @@ -125,6 +130,10 @@ async fn get_pbs_node_rrd_data(
>> },
>> },
>> },
>> + access: {
>> + permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
>> + description: "The user needs to have atleast `Resource.Audit` privilege under `/resource`."
>
> nit: same as above, should probably be:
>
> The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}/datastore/{datastore}`.
>
>> + }
>> )]
>> /// Read PBS datastore stats
>> async fn get_pbs_datastore_rrd_data(
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints
2025-10-14 7:59 ` Shan Shaji
@ 2025-10-14 8:59 ` Shan Shaji
0 siblings, 0 replies; 7+ messages in thread
From: Shan Shaji @ 2025-10-14 8:59 UTC (permalink / raw)
To: Shan Shaji, Shannon Sterz
Cc: Proxmox Datacenter Manager development discussion
Superseded by v2: https://lore.proxmox.com/pdm-devel/20251014085651.73407-1-s.shaji@proxmox.com/T/#t
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-14 8:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-10 15:18 [pdm-devel] [PATCH datacenter-manager 0/2] fix #6901: add explicit permissions for PBS status and RRD endpoints Shan Shaji
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints Shan Shaji
2025-10-13 8:41 ` Shannon Sterz
2025-10-14 7:59 ` Shan Shaji
2025-10-14 8:59 ` Shan Shaji
2025-10-10 15:18 ` [pdm-devel] [PATCH datacenter-manager 2/2] fix #6901: api: remove `node` reference from templated privilege path Shan Shaji
2025-10-13 8:41 ` Shannon Sterz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox