all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard
@ 2025-10-07 11:28 Shan Shaji
  2025-10-10  7:56 ` Shannon Sterz
  0 siblings, 1 reply; 4+ messages in thread
From: Shan Shaji @ 2025-10-07 11:28 UTC (permalink / raw)
  To: pdm-devel; +Cc: Shan Shaji

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 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..1e82c48 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 `PRIV_RESOURCE_AUDIT` on one resources under `/resource`.
+        Only resources for which the user has `PRIV_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)
         {
-- 
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] 4+ messages in thread

* Re: [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard
  2025-10-07 11:28 [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard Shan Shaji
@ 2025-10-10  7:56 ` Shannon Sterz
  2025-10-10 10:44   ` Shan Shaji
  0 siblings, 1 reply; 4+ messages in thread
From: Shannon Sterz @ 2025-10-10  7:56 UTC (permalink / raw)
  To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion

On Tue Oct 7, 2025 at 1:28 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 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..1e82c48 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 `PRIV_RESOURCE_AUDIT` on one resources under `/resource`.
> +        Only resources for which the user has `PRIV_RESOURCE_AUDIT` on `/resource/{remote_name}` will be
> +        considered when calculating the top entities."
> +    },

ah sorry this is my mistaken in the original review, PRIV_RESOURCE_AUDIT
is the internal constant name of that privilege. the user-facing
privilege is called `Resource.Audit`. so this might be nicer here and
more consistent with other api endpoints, sorry for that.

>  )]
>  /// 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)
>          {


other than that, looks good to me so 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard
  2025-10-10  7:56 ` Shannon Sterz
@ 2025-10-10 10:44   ` Shan Shaji
  2025-10-10 10:56     ` Shan Shaji
  0 siblings, 1 reply; 4+ messages in thread
From: Shan Shaji @ 2025-10-10 10:44 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: Proxmox Datacenter Manager development discussion

Thanks Shannon for catching that. Will send an updated patch. 


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard
  2025-10-10 10:44   ` Shan Shaji
@ 2025-10-10 10:56     ` Shan Shaji
  0 siblings, 0 replies; 4+ messages in thread
From: Shan Shaji @ 2025-10-10 10:56 UTC (permalink / raw)
  To: Shan Shaji, Shannon Sterz
  Cc: Proxmox Datacenter Manager development discussion

Superseded by v3: https://lore.proxmox.com/pdm-devel/20251010105518.18543-1-s.shaji@proxmox.com/T/#u


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-10 11:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-07 11:28 [pdm-devel] [PATCH datacenter-manager v2] fix #6794: api: allow non-root users to view stats on dashboard Shan Shaji
2025-10-10  7:56 ` Shannon Sterz
2025-10-10 10:44   ` Shan Shaji
2025-10-10 10:56     ` Shan Shaji

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal