[pdm-devel] [PATCH datacenter-manager 1/2] fix #6901: api: add permission checks for PBS rrd endpoints

[Shan Shaji]

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 at 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(