From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Shan Shaji" <s.shaji@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager v3] fix #6794: api: allow non-root users to view stats on dashboard
Date: Fri, 10 Oct 2025 13:11:35 +0200 [thread overview]
Message-ID: <DDELHWPGPECJ.2DE2ZLYJ2TZ3B@proxmox.com> (raw)
In-Reply-To: <20251010105518.18543-1-s.shaji@proxmox.com>
On Fri Oct 10, 2025 at 12:55 PM CEST, Shan Shaji wrote:
> When a non-root user logged into PDM, the Top Entities section was
> not visible, even though the required privileges had been assigned.
>
> To fix it allow all authenticated users to access the `/top-entities`
> endpoint. The returned top entities will be filtered by the user's
> permission on what resources they have access to.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
> changes since v2: Thanks @Shannon Sterz
> - changed PRIV_RESOURCE_AUDIT to Resource.Audit in the access
> description.
>
> changes since v1: Thanks @Shannon Sterz
> - Updated commit message.
> - Added access description.
> - Replaced `UNAUTHORIZED` with `FORBIDDEN`.
> - Removed "FIXME" comment.
>
> server/src/api/resources.rs | 27 +++++++++++++++++---
> server/src/metric_collection/top_entities.rs | 5 ++++
> 2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index a7c3881..f4f56bc 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -480,7 +480,6 @@ pub async fn get_subscription_status(
> }
>
> // FIXME: make timeframe and count parameters?
> -// FIXME: permissions?
> #[api(
> input: {
> properties: {
> @@ -490,13 +489,35 @@ pub async fn get_subscription_status(
> }
> }
> },
> + access: {
> + permission: &Permission::Anybody,
> + description: "The user needs to have at least `Resource.Audit` on one resources under `/resource`.
> + Only resources for which the user has `Resource.Audit` on `/resource/{remote_name}` will be
> + considered when calculating the top entities."
> + },
> )]
> /// Returns the top X entities regarding the chosen type
> -async fn get_top_entities(timeframe: Option<RrdTimeframe>) -> Result<TopEntities, Error> {
> +async fn get_top_entities(
> + timeframe: Option<RrdTimeframe>,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<TopEntities, Error> {
> + let user_info = CachedUserInfo::new()?;
> + let auth_id: Authid = rpcenv
> + .get_auth_id()
> + .ok_or_else(|| format_err!("no authid available"))?
> + .parse()?;
> +
> + if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
> + http_bail!(FORBIDDEN, "user has no access to resources");
> + }
> +
> let (remotes_config, _) = pdm_config::remotes::config()?;
> + let check_remote_privs = |remote_name: &str| {
> + user_info.lookup_privs(&auth_id, &["resource", remote_name]) & PRIV_RESOURCE_AUDIT != 0
> + };
>
> let timeframe = timeframe.unwrap_or(RrdTimeframe::Day);
> - let res = top_entities::calculate_top(&remotes_config, timeframe, 10);
> + let res = top_entities::calculate_top(&remotes_config, timeframe, 10, check_remote_privs);
> Ok(res)
> }
>
> diff --git a/server/src/metric_collection/top_entities.rs b/server/src/metric_collection/top_entities.rs
> index 47fda24..990189a 100644
> --- a/server/src/metric_collection/top_entities.rs
> +++ b/server/src/metric_collection/top_entities.rs
> @@ -36,12 +36,17 @@ pub fn calculate_top(
> remotes: &HashMap<String, pdm_api_types::remotes::Remote>,
> timeframe: proxmox_rrd_api_types::RrdTimeframe,
> num: usize,
> + check_remote_privs: impl Fn(&str) -> bool
> ) -> TopEntities {
> let mut guest_cpu = Vec::new();
> let mut node_cpu = Vec::new();
> let mut node_memory = Vec::new();
>
> for remote_name in remotes.keys() {
> + if !check_remote_privs(remote_name) {
> + continue;
> + }
> +
> if let Some(data) =
> crate::api::resources::get_cached_resources(remote_name, i64::MAX as u64)
> {
alright, thanks for incorporating my last notes there. looks good now,
consider this:
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
prev parent reply other threads:[~2025-10-10 11:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-10 10:55 Shan Shaji
2025-10-10 11:11 ` Shannon Sterz [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DDELHWPGPECJ.2DE2ZLYJ2TZ3B@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=pdm-devel@lists.proxmox.com \
--cc=s.shaji@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox