From: Shannon Sterz <s.sterz@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH datacenter-manager v2 1/2] server: use proxmox-access-control api implementations
Date: Fri, 11 Apr 2025 15:44:34 +0200 [thread overview]
Message-ID: <20250411134435.269524-11-s.sterz@proxmox.com> (raw)
In-Reply-To: <20250411134435.269524-1-s.sterz@proxmox.com>
instead of re-implementing most of the code here, take advantage of
the new `api` feature that allows for sharing the common api
implementations.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/Cargo.toml | 2 +-
server/src/acl.rs | 102 +++++++++-
server/src/api/access/acl.rs | 357 -----------------------------------
server/src/api/access/mod.rs | 4 +-
4 files changed, 100 insertions(+), 365 deletions(-)
delete mode 100644 server/src/api/access/acl.rs
diff --git a/server/Cargo.toml b/server/Cargo.toml
index 7b0058e..2d58cea 100644
--- a/server/Cargo.toml
+++ b/server/Cargo.toml
@@ -32,7 +32,7 @@ tokio-stream.workspace = true
tracing.workspace = true
url.workspace = true
-proxmox-access-control = { workspace = true, features = [ "impl" ] }
+proxmox-access-control = { workspace = true, features = [ "api" ] }
proxmox-async.workspace = true
proxmox-auth-api = { workspace = true, features = [ "api", "ticket", "pam-authenticator", "password-authenticator" ] }
proxmox-daemon.workspace = true
diff --git a/server/src/acl.rs b/server/src/acl.rs
index a99eede..52a1f97 100644
--- a/server/src/acl.rs
+++ b/server/src/acl.rs
@@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::sync::OnceLock;
-use anyhow::{Context as _, Error};
+use anyhow::{format_err, Context as _, Error};
use proxmox_access_control::types::User;
use proxmox_auth_api::types::Authid;
@@ -10,7 +10,7 @@ use proxmox_section_config::SectionConfigData;
struct AccessControlConfig;
static PRIVILEGES: OnceLock<HashMap<&str, u64>> = OnceLock::new();
-static ROLES: OnceLock<HashMap<&str, u64>> = OnceLock::new();
+static ROLES: OnceLock<HashMap<&str, (u64, &str)>> = OnceLock::new();
impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
fn privileges(&self) -> &HashMap<&str, u64> {
@@ -18,11 +18,11 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
}
#[rustfmt::skip]
- fn roles(&self) -> &HashMap<&str, u64> {
+ fn roles(&self) -> &HashMap<&str, (u64, &str)> {
ROLES.get_or_init(|| {
[
- ("Administrator", pdm_api_types::ROLE_ADMINISTRATOR),
- ("Auditor", pdm_api_types::ROLE_AUDITOR),
+ ("Administrator", (pdm_api_types::ROLE_ADMINISTRATOR, "Administrators can inspect and modify the system.")),
+ ("Auditor", (pdm_api_types::ROLE_AUDITOR, "An Auditor can inspect many aspects of the system, but not change them.")),
//("SystemAdministrator", pdm_api_types::ROLE_SYS_ADMINISTRATOR),
//("SystemAuditor", pdm_api_types::ROLE_SYS_AUDITOR),
//("ResourceAdministrator", pdm_api_types::ROLE_RESOURCE_ADMINISTRATOR),
@@ -63,6 +63,98 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
Ok(())
}
+
+ fn acl_audit_privileges(&self) -> u64 {
+ pdm_api_types::PRIV_ACCESS_AUDIT
+ }
+
+ fn acl_modify_privileges(&self) -> u64 {
+ pdm_api_types::PRIV_ACCESS_MODIFY
+ }
+
+ fn check_acl_path(&self, path: &str) -> Result<(), Error> {
+ let components = proxmox_access_control::acl::split_acl_path(path);
+
+ let components_len = components.len();
+
+ if components_len == 0 {
+ return Ok(());
+ }
+ match components[0] {
+ "access" => {
+ if components_len == 1 {
+ return Ok(());
+ }
+ match components[1] {
+ "acl" | "users" | "realm" => {
+ if components_len == 2 {
+ return Ok(());
+ }
+ }
+ _ => {}
+ }
+ }
+ "resource" => {
+ // `/resource` and `/resource/{remote}`
+ if components_len <= 2 {
+ return Ok(());
+ }
+ // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
+ match components[2] {
+ "guest" | "storage" => {
+ // /resource/{remote-id}/{resource-type}
+ // /resource/{remote-id}/{resource-type}/{resource-id}
+ if components_len <= 4 {
+ return Ok(());
+ }
+ }
+ _ => {}
+ }
+ }
+ "system" => {
+ if components_len == 1 {
+ return Ok(());
+ }
+ match components[1] {
+ "certificates" | "disks" | "log" | "notifications" | "status" | "tasks"
+ | "time" => {
+ if components_len == 2 {
+ return Ok(());
+ }
+ }
+ "services" => {
+ // /system/services/{service}
+ if components_len <= 3 {
+ return Ok(());
+ }
+ }
+ "network" => {
+ if components_len == 2 {
+ return Ok(());
+ }
+ match components[2] {
+ "dns" => {
+ if components_len == 3 {
+ return Ok(());
+ }
+ }
+ "interfaces" => {
+ // /system/network/interfaces/{iface}
+ if components_len <= 4 {
+ return Ok(());
+ }
+ }
+ _ => {}
+ }
+ }
+ _ => {}
+ }
+ }
+ _ => {}
+ }
+
+ Err(format_err!("invalid acl path '{}'.", path))
+ }
}
pub(crate) fn init() {
diff --git a/server/src/api/access/acl.rs b/server/src/api/access/acl.rs
deleted file mode 100644
index a51899b..0000000
--- a/server/src/api/access/acl.rs
+++ /dev/null
@@ -1,357 +0,0 @@
-use anyhow::{bail, Error};
-
-use proxmox_access_control::acl::AclTreeNode;
-use proxmox_access_control::CachedUserInfo;
-use proxmox_config_digest::{ConfigDigest, PROXMOX_CONFIG_DIGEST_SCHEMA};
-use proxmox_router::{Permission, Router, RpcEnvironment};
-use proxmox_schema::api;
-
-use pdm_api_types::{
- AclListItem, AclUgidType, Authid, Role, ACL_PATH_SCHEMA, ACL_PROPAGATE_SCHEMA,
- PRIV_ACCESS_AUDIT, PRIV_ACCESS_MODIFY, PROXMOX_GROUP_ID_SCHEMA,
-};
-
-pub(super) const ROUTER: Router = Router::new()
- .get(&API_METHOD_READ_ACL)
- .put(&API_METHOD_UPDATE_ACL);
-
-// FIXME: copied from PBS
-fn extract_acl_node_data(
- node: &AclTreeNode,
- path: &str,
- list: &mut Vec<AclListItem>,
- exact: bool,
- auth_id_filter: &Option<Authid>,
-) {
- // tokens can't have tokens, so we can early return
- if let Some(auth_id_filter) = auth_id_filter {
- if auth_id_filter.is_token() {
- return;
- }
- }
-
- for (user, roles) in &node.users {
- if let Some(auth_id_filter) = auth_id_filter {
- if !user.is_token() || user.user() != auth_id_filter.user() {
- continue;
- }
- }
-
- for (role, propagate) in roles {
- list.push(AclListItem {
- path: if path.is_empty() {
- String::from("/")
- } else {
- path.to_string()
- },
- propagate: *propagate,
- ugid_type: AclUgidType::User,
- ugid: user.to_string(),
- roleid: role.to_string(),
- });
- }
- }
- for (group, roles) in &node.groups {
- if auth_id_filter.is_some() {
- continue;
- }
-
- for (role, propagate) in roles {
- list.push(AclListItem {
- path: if path.is_empty() {
- String::from("/")
- } else {
- path.to_string()
- },
- propagate: *propagate,
- ugid_type: AclUgidType::Group,
- ugid: group.to_string(),
- roleid: role.to_string(),
- });
- }
- }
- if exact {
- return;
- }
- for (comp, child) in &node.children {
- let new_path = format!("{}/{}", path, comp);
- extract_acl_node_data(child, &new_path, list, exact, auth_id_filter);
- }
-}
-
-#[api(
- input: {
- properties: {
- path: {
- schema: ACL_PATH_SCHEMA,
- optional: true,
- },
- exact: {
- description: "If set, returns only ACL for the exact path.",
- type: bool,
- optional: true,
- default: false,
- },
- },
- },
- returns: {
- description: "ACL entry list.",
- type: Array,
- items: {
- type: AclListItem,
- }
- },
- access: {
- permission: &Permission::Anybody,
- description: "Returns all ACLs if user has Sys.Audit on '/access/acl', or just the ACLs containing the user's API tokens.",
- },
-)]
-/// Read Access Control List (ACLs).
-fn read_acl(
- path: Option<String>,
- exact: bool,
- rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<AclListItem>, Error> {
- let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
-
- let user_info = CachedUserInfo::new()?;
-
- let top_level_privs = user_info.lookup_privs(&auth_id, &["access", "acl"]);
- let auth_id_filter = if (top_level_privs & PRIV_ACCESS_AUDIT) == 0 {
- Some(auth_id)
- } else {
- None
- };
-
- let (mut tree, digest) = proxmox_access_control::acl::config()?;
-
- let mut list: Vec<AclListItem> = Vec::new();
- if let Some(path) = &path {
- if let Some(node) = &tree.find_node(path) {
- extract_acl_node_data(node, path, &mut list, exact, &auth_id_filter);
- }
- } else {
- extract_acl_node_data(&tree.root, "", &mut list, exact, &auth_id_filter);
- }
-
- rpcenv["digest"] = hex::encode(digest).into();
-
- Ok(list)
-}
-
-#[api(
- protected: true,
- input: {
- properties: {
- path: {
- schema: ACL_PATH_SCHEMA,
- },
- role: {
- type: Role,
- },
- propagate: {
- optional: true,
- schema: ACL_PROPAGATE_SCHEMA,
- },
- "auth-id": {
- optional: true,
- type: Authid,
- },
- group: {
- optional: true,
- schema: PROXMOX_GROUP_ID_SCHEMA,
- },
- delete: {
- optional: true,
- description: "Remove permissions (instead of adding it).",
- type: bool,
- default: false,
- },
- digest: {
- optional: true,
- schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
- },
- },
- },
- access: {
- permission: &Permission::Anybody,
- description: "Requires Permissions.Modify on '/access/acl', limited to updating ACLs of the user's API tokens otherwise."
- },
-)]
-/// Update Access Control List (ACLs).
-#[allow(clippy::too_many_arguments)]
-fn update_acl(
- path: String,
- role: String,
- propagate: Option<bool>,
- auth_id: Option<Authid>,
- group: Option<String>,
- delete: bool,
- digest: Option<ConfigDigest>,
- rpcenv: &mut dyn RpcEnvironment,
-) -> Result<(), Error> {
- let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
-
- let user_info = CachedUserInfo::new()?;
-
- let top_level_privs = user_info.lookup_privs(¤t_auth_id, &["access", "acl"]);
- if top_level_privs & PRIV_ACCESS_MODIFY == 0 {
- if group.is_some() {
- bail!("Unprivileged users are not allowed to create group ACL item.");
- }
-
- match &auth_id {
- Some(auth_id) => {
- if current_auth_id.is_token() {
- bail!("Unprivileged API tokens can't set ACL items.");
- } else if !auth_id.is_token() {
- bail!("Unprivileged users can only set ACL items for API tokens.");
- } else if auth_id.user() != current_auth_id.user() {
- bail!("Unprivileged users can only set ACL items for their own API tokens.");
- }
- }
- None => {
- bail!("Unprivileged user needs to provide auth_id to update ACL item.");
- }
- };
- }
-
- let _lock = proxmox_access_control::acl::lock_config()?;
-
- let (mut tree, expected_digest) = proxmox_access_control::acl::config()?;
-
- expected_digest.detect_modification(digest.as_ref())?;
-
- let propagate = propagate.unwrap_or(true);
-
- if let Some(ref _group) = group {
- bail!("parameter 'group' - groups are currently not supported.");
- } else if let Some(ref auth_id) = auth_id {
- if !delete {
- // Note: we allow to delete non-existent users
- let user_cfg = proxmox_access_control::user::cached_config()?;
- if !user_cfg.sections.contains_key(&auth_id.to_string()) {
- bail!(format!(
- "no such {}.",
- if auth_id.is_token() {
- "API token"
- } else {
- "user"
- }
- ));
- }
- }
- } else {
- bail!("missing 'userid' or 'group' parameter.");
- }
-
- if !delete {
- // Note: we allow to delete entries with invalid path
- check_acl_path(&path)?;
- }
-
- if let Some(auth_id) = auth_id {
- if delete {
- tree.delete_user_role(&path, &auth_id, &role);
- } else {
- tree.insert_user_role(&path, &auth_id, &role, propagate);
- }
- } else if let Some(group) = group {
- if delete {
- tree.delete_group_role(&path, &group, &role);
- } else {
- tree.insert_group_role(&path, &group, &role, propagate);
- }
- }
-
- proxmox_access_control::acl::save_config(&tree)?;
-
- Ok(())
-}
-///
-/// Check whether a given ACL `path` conforms to the expected schema.
-///
-/// Currently this just checks for the number of components for various sub-trees.
-fn check_acl_path(path: &str) -> Result<(), Error> {
- let components = proxmox_access_control::acl::split_acl_path(path);
-
- let components_len = components.len();
-
- if components_len == 0 {
- return Ok(());
- }
- match components[0] {
- "access" => {
- if components_len == 1 {
- return Ok(());
- }
- match components[1] {
- "acl" | "users" | "realm" => {
- if components_len == 2 {
- return Ok(());
- }
- }
- _ => {}
- }
- }
- "resource" => {
- // `/resource` and `/resource/{remote}`
- if components_len <= 2 {
- return Ok(());
- }
- // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
- match components[2] {
- "guest" | "storage" => {
- // /resource/{remote-id}/{resource-type}
- // /resource/{remote-id}/{resource-type}/{resource-id}
- if components_len <= 4 {
- return Ok(());
- }
- }
- _ => {}
- }
- }
- "system" => {
- if components_len == 1 {
- return Ok(());
- }
- match components[1] {
- "certificates" | "disks" | "log" | "notifications" | "status" | "tasks"
- | "time" => {
- if components_len == 2 {
- return Ok(());
- }
- }
- "services" => {
- // /system/services/{service}
- if components_len <= 3 {
- return Ok(());
- }
- }
- "network" => {
- if components_len == 2 {
- return Ok(());
- }
- match components[2] {
- "dns" => {
- if components_len == 3 {
- return Ok(());
- }
- }
- "interfaces" => {
- // /system/network/interfaces/{iface}
- if components_len <= 4 {
- return Ok(());
- }
- }
- _ => {}
- }
- }
- _ => {}
- }
- }
- _ => {}
- }
-
- bail!("invalid acl path '{}'.", path);
-}
diff --git a/server/src/api/access/mod.rs b/server/src/api/access/mod.rs
index befe2f1..345b22f 100644
--- a/server/src/api/access/mod.rs
+++ b/server/src/api/access/mod.rs
@@ -13,19 +13,19 @@ use proxmox_sortable_macro::sortable;
use pdm_api_types::{Authid, ACL_PATH_SCHEMA, PRIVILEGES, PRIV_ACCESS_AUDIT};
-mod acl;
mod domains;
mod tfa;
mod users;
#[sortable]
const SUBDIRS: SubdirMap = &sorted!([
- ("acl", &acl::ROUTER),
+ ("acl", &proxmox_access_control::api::ACL_ROUTER),
("domains", &domains::ROUTER),
(
"permissions",
&Router::new().get(&API_METHOD_LIST_PERMISSIONS)
),
+ ("roles", &proxmox_access_control::api::ROLE_ROUTER),
("tfa", &tfa::ROUTER),
(
"ticket",
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
next prev parent reply other threads:[~2025-04-11 13:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-11 13:44 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v2 00/11] ACL edit api and ui components Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 1/6] access-control: add more types to prepare for api feature Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 2/6] access-control: add acl " Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 3/6] access-control: add comments to roles function of AccessControlConfig Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 4/6] access-control: add generic roles endpoint to `api` feature Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 5/6] access-control: api: refactor validation checks to re-use existing code Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH proxmox v2 6/6] access-control: api: refactor extract_acl_node_data to be non-recursive Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH yew-comp v2 1/3] api-types/role_selector: depend on common `RoleInfo` type Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH yew-comp v2 2/3] acl: add a view and semi-generic `EditWindow` for acl entries Shannon Sterz
2025-04-11 13:44 ` [pdm-devel] [PATCH yew-comp v2 3/3] role_selector/acl_edit: make api endpoint and default role configurable Shannon Sterz
2025-04-11 13:44 ` Shannon Sterz [this message]
2025-04-11 13:44 ` [pdm-devel] [PATCH datacenter-manager v2 2/2] ui: configuration: add panel for viewing and editing acl entries Shannon Sterz
2025-04-17 15:46 ` [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v2 00/11] ACL edit api and ui components Thomas Lamprecht
2025-04-22 8:12 ` Shannon Sterz
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=20250411134435.269524-11-s.sterz@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=pdm-devel@lists.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 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