* [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
* 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 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
* [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 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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.