public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate
@ 2024-06-10 15:42 Shannon Sterz
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees Shannon Sterz
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

the idea behind the `proxmox-access` crate is to enable us to easily
re-use the acl tree, user config and token shadow implementation of
proxmox backup server across multiple projects. this series factors out
the implementations from proxmox-backup's `pbs-config` crate.

to use this crate, a client simply needs to implement the `AcmConfig`
(Access Control Management Config) trait and providing a path to store
the configuration files in. for the cached configs, one also needs to
provide the product name and the location of the shared memory that will
be used to keep track of cache generations.

currently, i did not move proxmox-backup itself over to this crate, as
`pbs-config` has quite a few interdependencies within `proxmox-backup`
that would take some detangling. however, i can also get started on that
if desired.

proxmox:

Shannon Sterz (5):
  access: add the proxmox-access crate to reuse acl trees
  access: define shared `User`, `UserWithTokens` and `ApiTokens types
  access: make token shadow implementation re-usable
  access: factor out user config and cache handling
  access: increment user cache generation when saving acl config

 Cargo.toml                                 |    3 +
 proxmox-access/Cargo.toml                  |   30 +
 proxmox-access/src/acl.rs                  | 1008 ++++++++++++++++++++
 proxmox-access/src/cached_user_info.rs     |  242 +++++
 proxmox-access/src/config_version_cache.rs |  113 +++
 proxmox-access/src/init.rs                 |  139 +++
 proxmox-access/src/lib.rs                  |   11 +
 proxmox-access/src/token_shadow.rs         |   84 ++
 proxmox-access/src/types.rs                |  228 +++++
 proxmox-access/src/user.rs                 |  182 ++++
 10 files changed, 2040 insertions(+)
 create mode 100644 proxmox-access/Cargo.toml
 create mode 100644 proxmox-access/src/acl.rs
 create mode 100644 proxmox-access/src/cached_user_info.rs
 create mode 100644 proxmox-access/src/config_version_cache.rs
 create mode 100644 proxmox-access/src/init.rs
 create mode 100644 proxmox-access/src/lib.rs
 create mode 100644 proxmox-access/src/token_shadow.rs
 create mode 100644 proxmox-access/src/types.rs
 create mode 100644 proxmox-access/src/user.rs


Summary over all repositories:
  10 files changed, 2040 insertions(+), 0 deletions(-)

--
Generated by git-murpp 0.5.0


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


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

* [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
@ 2024-06-10 15:42 ` Shannon Sterz
  2024-06-11 12:53   ` Wolfgang Bumiller
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types Shannon Sterz
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

this commit factors out the acl tree from proxmox-backup so we can
re-use it accross other products. to use it, the product needs to
implement the `AcmConfig` trait and provide this crate with a
location to safe its configuration files.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 Cargo.toml                 |   2 +
 proxmox-access/Cargo.toml  |  22 +
 proxmox-access/src/acl.rs  | 999 +++++++++++++++++++++++++++++++++++++
 proxmox-access/src/init.rs |  73 +++
 proxmox-access/src/lib.rs  |   2 +
 5 files changed, 1098 insertions(+)
 create mode 100644 proxmox-access/Cargo.toml
 create mode 100644 proxmox-access/src/acl.rs
 create mode 100644 proxmox-access/src/init.rs
 create mode 100644 proxmox-access/src/lib.rs

diff --git a/Cargo.toml b/Cargo.toml
index b3c97808..6b17dd4d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,5 +1,6 @@
 [workspace]
 members = [
+    "proxmox-access",
     "proxmox-acme",
     "proxmox-acme-api",
     "proxmox-api-macro",
@@ -110,6 +111,7 @@ zstd = { version = "0.12", features = [ "bindgen" ] }
 # workspace dependencies
 proxmox-acme = {  version = "0.5.2", path = "proxmox-acme", default-features = false }
 proxmox-api-macro = { version = "1.0.8", path = "proxmox-api-macro" }
+proxmox-auth-api = { version = "0.4.0", path = "proxmox-auth-api" }
 proxmox-async = { version = "0.4.1", path = "proxmox-async" }
 proxmox-compression = { version = "0.2.0", path = "proxmox-compression" }
 proxmox-http = { version = "0.9.0", path = "proxmox-http" }
diff --git a/proxmox-access/Cargo.toml b/proxmox-access/Cargo.toml
new file mode 100644
index 00000000..51e97304
--- /dev/null
+++ b/proxmox-access/Cargo.toml
@@ -0,0 +1,22 @@
+[package]
+name = "proxmox-access"
+version = "0.1.0"
+authors.workspace = true
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+homepage.workspace = true
+exclude.workspace = true
+rust-version.workspace = true
+description = "A collection of utilities to implement access control management."
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+anyhow.workspace = true
+nix.workspace = true
+openssl.workspace = true
+
+# proxmox-notify.workspace = true
+proxmox-auth-api = { workspace = true, features = [ "api-types" ] }
+proxmox-product-config.workspace = true
diff --git a/proxmox-access/src/acl.rs b/proxmox-access/src/acl.rs
new file mode 100644
index 00000000..26f639ae
--- /dev/null
+++ b/proxmox-access/src/acl.rs
@@ -0,0 +1,999 @@
+use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::io::Write;
+use std::path::Path;
+use std::sync::{Arc, OnceLock, RwLock};
+
+use anyhow::{bail, Error};
+
+use proxmox_auth_api::types::{Authid, Userid};
+use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLockGuard};
+
+use crate::init::{acl_config, acl_config_lock, acm_conf};
+
+pub fn split_acl_path(path: &str) -> Vec<&str> {
+    let items = path.split('/');
+
+    let mut components = vec![];
+
+    for name in items {
+        if name.is_empty() {
+            continue;
+        }
+        components.push(name);
+    }
+
+    components
+}
+
+/// Tree representing a parsed acl.cfg
+#[derive(Default)]
+pub struct AclTree {
+    /// Root node of the tree.
+    ///
+    /// The rest of the tree is available via [find_node()](AclTree::find_node()) or an
+    /// [`AclTreeNode`]'s [children](AclTreeNode::children) member.
+    pub root: AclTreeNode,
+}
+
+/// Node representing ACLs for a certain ACL path.
+#[derive(Default)]
+pub struct AclTreeNode {
+    /// [User](pbs_api_types::User) or
+    /// [Token](pbs_api_types::ApiToken) ACLs for this node.
+    pub users: HashMap<Authid, HashMap<String, bool>>,
+    /// `Group` ACLs for this node (not yet implemented)
+    pub groups: HashMap<String, HashMap<String, bool>>,
+    /// `AclTreeNodes` representing ACL paths directly below the current one.
+    pub children: BTreeMap<String, AclTreeNode>,
+}
+
+impl AclTreeNode {
+    /// Creates a new, empty AclTreeNode.
+    pub fn new() -> Self {
+        Self {
+            users: HashMap::new(),
+            groups: HashMap::new(),
+            children: BTreeMap::new(),
+        }
+    }
+
+    /// Returns applicable role and their propagation status for a given [Authid].
+    ///
+    /// If the `Authid` is a [User](pbs_api_types::User) that has no specific `Roles` configured on
+    /// this node, applicable `Group` roles will be returned instead.
+    ///
+    /// If `leaf` is `false`, only those roles where the propagate flag in the ACL is set to `true`
+    /// are returned. Otherwise, all roles will be returned.
+    pub fn extract_roles(&self, auth_id: &Authid, leaf: bool) -> HashMap<String, bool> {
+        let user_roles = self.extract_user_roles(auth_id, leaf);
+        if !user_roles.is_empty() || auth_id.is_token() {
+            // user privs always override group privs
+            return user_roles;
+        };
+
+        self.extract_group_roles(auth_id.user(), leaf)
+    }
+
+    fn extract_user_roles(&self, auth_id: &Authid, leaf: bool) -> HashMap<String, bool> {
+        let mut map = HashMap::new();
+
+        let roles = match self.users.get(auth_id) {
+            Some(m) => m,
+            None => return map,
+        };
+
+        for (role, propagate) in roles {
+            if *propagate || leaf {
+                if acm_conf()
+                    .role_no_access()
+                    .map_or_else(|| false, |r| r == role)
+                {
+                    // return a map with a single role 'NoAccess'
+                    let mut map = HashMap::new();
+                    map.insert(role.to_string(), false);
+                    return map;
+                }
+                map.insert(role.to_string(), *propagate);
+            }
+        }
+
+        map
+    }
+
+    fn extract_group_roles(&self, _user: &Userid, leaf: bool) -> HashMap<String, bool> {
+        let mut map = HashMap::new();
+
+        #[allow(clippy::for_kv_map)]
+        for (_group, roles) in &self.groups {
+            let is_member = false; // fixme: check if user is member of the group
+            if !is_member {
+                continue;
+            }
+
+            for (role, propagate) in roles {
+                if *propagate || leaf {
+                    if acm_conf()
+                        .role_no_access()
+                        .map_or_else(|| false, |r| r == role)
+                    {
+                        // return a map with a single role 'NoAccess'
+                        let mut map = HashMap::new();
+                        map.insert(role.to_string(), false);
+                        return map;
+                    }
+                    map.insert(role.to_string(), *propagate);
+                }
+            }
+        }
+
+        map
+    }
+
+    fn delete_group_role(&mut self, group: &str, role: &str) {
+        let roles = match self.groups.get_mut(group) {
+            Some(r) => r,
+            None => return,
+        };
+        roles.remove(role);
+    }
+
+    fn delete_user_role(&mut self, auth_id: &Authid, role: &str) {
+        let roles = match self.users.get_mut(auth_id) {
+            Some(r) => r,
+            None => return,
+        };
+        roles.remove(role);
+    }
+
+    fn delete_authid(&mut self, auth_id: &Authid) {
+        for node in self.children.values_mut() {
+            node.delete_authid(auth_id);
+        }
+        self.users.remove(auth_id);
+    }
+
+    fn insert_group_role(&mut self, group: String, role: String, propagate: bool) {
+        let map = self.groups.entry(group).or_default();
+        if let Some(no_access) = acm_conf().role_no_access() {
+            if role == no_access {
+                map.clear();
+            } else {
+                map.remove(no_access);
+            }
+        }
+
+        map.insert(role, propagate);
+    }
+
+    fn insert_user_role(&mut self, auth_id: Authid, role: String, propagate: bool) {
+        let map = self.users.entry(auth_id).or_default();
+        if let Some(no_access) = acm_conf().role_no_access() {
+            if role == no_access {
+                map.clear();
+            } else {
+                map.remove(no_access);
+            }
+        }
+
+        map.insert(role, propagate);
+    }
+
+    fn get_child_paths(
+        &self,
+        path: String,
+        auth_id: &Authid,
+        paths: &mut Vec<String>,
+    ) -> Result<(), Error> {
+        for (sub_comp, child_node) in &self.children {
+            let roles = child_node.extract_roles(auth_id, true);
+            let child_path = format!("{path}/{sub_comp}");
+            if !roles.is_empty() {
+                paths.push(child_path.clone());
+            }
+            child_node.get_child_paths(child_path, auth_id, paths)?;
+        }
+        Ok(())
+    }
+}
+
+impl AclTree {
+    /// Create a new, empty ACL tree with a single, empty root [node](AclTreeNode)
+    pub fn new() -> Self {
+        Self {
+            root: AclTreeNode::new(),
+        }
+    }
+
+    /// Iterates over the tree looking for a node matching `path`.
+    pub fn find_node(&mut self, path: &str) -> Option<&mut AclTreeNode> {
+        let path = split_acl_path(path);
+        self.get_node_mut(&path)
+    }
+
+    fn get_node(&self, path: &[&str]) -> Option<&AclTreeNode> {
+        let mut node = &self.root;
+        for outer in path {
+            for comp in outer.split('/') {
+                node = match node.children.get(comp) {
+                    Some(n) => n,
+                    None => return None,
+                };
+            }
+        }
+        Some(node)
+    }
+
+    fn get_node_mut(&mut self, path: &[&str]) -> Option<&mut AclTreeNode> {
+        let mut node = &mut self.root;
+        for outer in path {
+            for comp in outer.split('/') {
+                node = match node.children.get_mut(comp) {
+                    Some(n) => n,
+                    None => return None,
+                };
+            }
+        }
+        Some(node)
+    }
+
+    fn get_or_insert_node(&mut self, path: &[&str]) -> &mut AclTreeNode {
+        let mut node = &mut self.root;
+        for outer in path {
+            for comp in outer.split('/') {
+                node = node.children.entry(String::from(comp)).or_default();
+            }
+        }
+        node
+    }
+
+    /// Deletes the specified `role` from the `group`'s ACL on `path`.
+    ///
+    /// Never fails, even if the `path` has no ACLs configured, or the `group`/`role` combination
+    /// does not exist on `path`.
+    pub fn delete_group_role(&mut self, path: &str, group: &str, role: &str) {
+        let path = split_acl_path(path);
+        let node = match self.get_node_mut(&path) {
+            Some(n) => n,
+            None => return,
+        };
+        node.delete_group_role(group, role);
+    }
+
+    /// Deletes the specified `role` from the `user`'s ACL on `path`.
+    ///
+    /// Never fails, even if the `path` has no ACLs configured, or the `user`/`role` combination
+    /// does not exist on `path`.
+    pub fn delete_user_role(&mut self, path: &str, auth_id: &Authid, role: &str) {
+        let path = split_acl_path(path);
+        let node = match self.get_node_mut(&path) {
+            Some(n) => n,
+            None => return,
+        };
+        node.delete_user_role(auth_id, role);
+    }
+
+    /// Deletes the [`AclTreeNode`] at the specified patth
+    ///
+    /// Never fails, deletes a node iff the specified path exists.
+    pub fn delete_node(&mut self, path: &str) {
+        let mut path = split_acl_path(path);
+        let last = path.pop();
+        let parent = match self.get_node_mut(&path) {
+            Some(n) => n,
+            None => return,
+        };
+        if let Some(name) = last {
+            parent.children.remove(name);
+        }
+    }
+
+    /// Deletes a user or token from the ACL-tree
+    ///
+    /// Traverses the tree in-order and removes the given user/token by their Authid
+    /// from every node in the tree.
+    pub fn delete_authid(&mut self, auth_id: &Authid) {
+        self.root.delete_authid(auth_id);
+    }
+
+    /// Inserts the specified `role` into the `group` ACL on `path`.
+    ///
+    /// The [`AclTreeNode`] representing `path` will be created and inserted into the tree if
+    /// necessary.
+    pub fn insert_group_role(&mut self, path: &str, group: &str, role: &str, propagate: bool) {
+        let path = split_acl_path(path);
+        let node = self.get_or_insert_node(&path);
+        node.insert_group_role(group.to_string(), role.to_string(), propagate);
+    }
+
+    /// Inserts the specified `role` into the `user` ACL on `path`.
+    ///
+    /// The [`AclTreeNode`] representing `path` will be created and inserted into the tree if
+    /// necessary.
+    pub fn insert_user_role(&mut self, path: &str, auth_id: &Authid, role: &str, propagate: bool) {
+        let path = split_acl_path(path);
+        let node = self.get_or_insert_node(&path);
+        node.insert_user_role(auth_id.to_owned(), role.to_string(), propagate);
+    }
+
+    fn write_node_config(node: &AclTreeNode, path: &str, w: &mut dyn Write) -> Result<(), Error> {
+        let mut role_ug_map0: HashMap<_, BTreeSet<_>> = HashMap::new();
+        let mut role_ug_map1: HashMap<_, BTreeSet<_>> = HashMap::new();
+
+        for (auth_id, roles) in &node.users {
+            // no need to save, because root is always 'Administrator'
+            if !auth_id.is_token() && auth_id.user() == "root@pam" {
+                continue;
+            }
+            for (role, propagate) in roles {
+                let role = role.as_str();
+                let auth_id = auth_id.to_string();
+                if *propagate {
+                    role_ug_map1.entry(role).or_default().insert(auth_id);
+                } else {
+                    role_ug_map0.entry(role).or_default().insert(auth_id);
+                }
+            }
+        }
+
+        for (group, roles) in &node.groups {
+            for (role, propagate) in roles {
+                let group = format!("@{}", group);
+                if *propagate {
+                    role_ug_map1.entry(role).or_default().insert(group);
+                } else {
+                    role_ug_map0.entry(role).or_default().insert(group);
+                }
+            }
+        }
+
+        fn group_by_property_list(
+            item_property_map: &HashMap<&str, BTreeSet<String>>,
+        ) -> BTreeMap<String, BTreeSet<String>> {
+            let mut result_map: BTreeMap<_, BTreeSet<_>> = BTreeMap::new();
+            for (item, property_map) in item_property_map {
+                let item_list = property_map.iter().fold(String::new(), |mut acc, v| {
+                    if !acc.is_empty() {
+                        acc.push(',');
+                    }
+                    acc.push_str(v);
+                    acc
+                });
+                result_map
+                    .entry(item_list)
+                    .or_default()
+                    .insert(item.to_string());
+            }
+            result_map
+        }
+
+        let uglist_role_map0 = group_by_property_list(&role_ug_map0);
+        let uglist_role_map1 = group_by_property_list(&role_ug_map1);
+
+        fn role_list(roles: &BTreeSet<String>) -> String {
+            if let Some(no_access) = acm_conf().role_no_access() {
+                if roles.contains(no_access) {
+                    return String::from(no_access);
+                }
+            }
+
+            roles.iter().fold(String::new(), |mut acc, v| {
+                if !acc.is_empty() {
+                    acc.push(',');
+                }
+                acc.push_str(v);
+                acc
+            })
+        }
+
+        for (uglist, roles) in &uglist_role_map0 {
+            let role_list = role_list(roles);
+            writeln!(
+                w,
+                "acl:0:{}:{}:{}",
+                if path.is_empty() { "/" } else { path },
+                uglist,
+                role_list
+            )?;
+        }
+
+        for (uglist, roles) in &uglist_role_map1 {
+            let role_list = role_list(roles);
+            writeln!(
+                w,
+                "acl:1:{}:{}:{}",
+                if path.is_empty() { "/" } else { path },
+                uglist,
+                role_list
+            )?;
+        }
+
+        for (name, child) in node.children.iter() {
+            let child_path = format!("{}/{}", path, name);
+            Self::write_node_config(child, &child_path, w)?;
+        }
+
+        Ok(())
+    }
+
+    fn write_config(&self, w: &mut dyn Write) -> Result<(), Error> {
+        Self::write_node_config(&self.root, "", w)
+    }
+
+    fn parse_acl_line(&mut self, line: &str) -> Result<(), Error> {
+        let items: Vec<&str> = line.split(':').collect();
+
+        if items.len() != 5 {
+            bail!("wrong number of items.");
+        }
+
+        if items[0] != "acl" {
+            bail!("line does not start with 'acl'.");
+        }
+
+        let propagate = if items[1] == "0" {
+            false
+        } else if items[1] == "1" {
+            true
+        } else {
+            bail!("expected '0' or '1' for propagate flag.");
+        };
+
+        let path_str = items[2];
+        let path = split_acl_path(path_str);
+        let node = self.get_or_insert_node(&path);
+
+        let uglist: Vec<&str> = items[3].split(',').map(|v| v.trim()).collect();
+
+        let rolelist: Vec<&str> = items[4].split(',').map(|v| v.trim()).collect();
+
+        for user_or_group in &uglist {
+            for role in &rolelist {
+                if !acm_conf().roles().contains_key(role) {
+                    bail!("unknown role '{}'", role);
+                }
+                if let Some(group) = user_or_group.strip_prefix('@') {
+                    node.insert_group_role(group.to_string(), role.to_string(), propagate);
+                } else {
+                    node.insert_user_role(user_or_group.parse()?, role.to_string(), propagate);
+                }
+            }
+        }
+
+        Ok(())
+    }
+
+    fn load(filename: &Path) -> Result<(Self, [u8; 32]), Error> {
+        let mut tree = Self::new();
+
+        let raw = match std::fs::read_to_string(filename) {
+            Ok(v) => v,
+            Err(err) => {
+                if err.kind() == std::io::ErrorKind::NotFound {
+                    String::new()
+                } else {
+                    bail!("unable to read acl config {:?} - {}", filename, err);
+                }
+            }
+        };
+
+        let digest = openssl::sha::sha256(raw.as_bytes());
+
+        for (linenr, line) in raw.lines().enumerate() {
+            let line = line.trim();
+            if line.is_empty() {
+                continue;
+            }
+            if let Err(err) = tree.parse_acl_line(line) {
+                bail!(
+                    "unable to parse acl config {:?}, line {} - {}",
+                    filename,
+                    linenr + 1,
+                    err
+                );
+            }
+        }
+
+        Ok((tree, digest))
+    }
+
+    /// This is used for testing
+    pub fn from_raw(raw: &str) -> Result<Self, Error> {
+        let mut tree = Self::new();
+        for (linenr, line) in raw.lines().enumerate() {
+            let line = line.trim();
+            if line.is_empty() {
+                continue;
+            }
+            if let Err(err) = tree.parse_acl_line(line) {
+                bail!(
+                    "unable to parse acl config data, line {} - {}",
+                    linenr + 1,
+                    err
+                );
+            }
+        }
+        Ok(tree)
+    }
+
+    /// Returns a map of role name and propagation status for a given `auth_id` and `path`.
+    ///
+    /// This will collect role mappings according to the following algorithm:
+    /// - iterate over all intermediate nodes along `path` and collect roles with `propagate` set
+    /// - get all (propagating and non-propagating) roles for last component of path
+    /// - more specific role maps replace less specific role maps
+    /// -- user/token is more specific than group at each level
+    /// -- roles lower in the tree are more specific than those higher up along the path
+    pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashMap<String, bool> {
+        let mut node = &self.root;
+        let mut role_map = node.extract_roles(auth_id, path.is_empty());
+
+        let mut comp_iter = path.iter().peekable();
+
+        while let Some(comp) = comp_iter.next() {
+            let last_comp = comp_iter.peek().is_none();
+
+            let mut sub_comp_iter = comp.split('/').peekable();
+
+            while let Some(sub_comp) = sub_comp_iter.next() {
+                let last_sub_comp = last_comp && sub_comp_iter.peek().is_none();
+
+                node = match node.children.get(sub_comp) {
+                    Some(n) => n,
+                    None => return role_map, // path not found
+                };
+
+                let new_map = node.extract_roles(auth_id, last_sub_comp);
+                if !new_map.is_empty() {
+                    // overwrite previous mappings
+                    role_map = new_map;
+                }
+            }
+        }
+
+        role_map
+    }
+
+    pub fn get_child_paths(&self, auth_id: &Authid, path: &[&str]) -> Result<Vec<String>, Error> {
+        let mut res = Vec::new();
+
+        if let Some(node) = self.get_node(path) {
+            let path = path.join("/");
+            node.get_child_paths(path, auth_id, &mut res)?;
+        }
+
+        Ok(res)
+    }
+}
+
+/// Get exclusive lock
+pub fn lock_config() -> Result<ApiLockGuard, Error> {
+    open_api_lockfile(acl_config_lock(), None, true)
+}
+
+/// Reads the [`AclTree`] from the [default path](ACL_CFG_FILENAME).
+pub fn config() -> Result<(AclTree, [u8; 32]), Error> {
+    let path = acl_config();
+    AclTree::load(&path)
+}
+
+/// Returns a cached [`AclTree`] or fresh copy read directly from the [default
+/// path](ACL_CFG_FILENAME)
+///
+/// Since the AclTree is used for every API request's permission check, this caching mechanism
+/// allows to skip reading and parsing the file again if it is unchanged.
+pub fn cached_config() -> Result<Arc<AclTree>, Error> {
+    struct ConfigCache {
+        data: Option<Arc<AclTree>>,
+        last_mtime: i64,
+        last_mtime_nsec: i64,
+    }
+
+    static CACHED_CONFIG: OnceLock<RwLock<ConfigCache>> = OnceLock::new();
+    let cached_conf = CACHED_CONFIG.get_or_init(|| {
+        RwLock::new(ConfigCache {
+            data: None,
+            last_mtime: 0,
+            last_mtime_nsec: 0,
+        })
+    });
+
+    let conf = acl_config();
+    let stat = match nix::sys::stat::stat(&conf) {
+        Ok(stat) => Some(stat),
+        Err(nix::errno::Errno::ENOENT) => None,
+        Err(err) => bail!("unable to stat '{}' - {err}", conf.display()),
+    };
+
+    {
+        // limit scope
+        let cache = cached_conf.read().unwrap();
+        if let Some(ref config) = cache.data {
+            if let Some(stat) = stat {
+                if stat.st_mtime == cache.last_mtime && stat.st_mtime_nsec == cache.last_mtime_nsec
+                {
+                    return Ok(config.clone());
+                }
+            } else if cache.last_mtime == 0 && cache.last_mtime_nsec == 0 {
+                return Ok(config.clone());
+            }
+        }
+    }
+
+    let (config, _digest) = config()?;
+    let config = Arc::new(config);
+
+    let mut cache = cached_conf.write().unwrap();
+    if let Some(stat) = stat {
+        cache.last_mtime = stat.st_mtime;
+        cache.last_mtime_nsec = stat.st_mtime_nsec;
+    }
+    cache.data = Some(config.clone());
+
+    Ok(config)
+}
+
+/// Saves an [`AclTree`] to the [default path](ACL_CFG_FILENAME), ensuring proper ownership and
+/// file permissions.
+pub fn save_config(acl: &AclTree) -> Result<(), Error> {
+    let mut raw: Vec<u8> = Vec::new();
+    acl.write_config(&mut raw)?;
+
+    let conf = acl_config();
+    replace_privileged_config(conf, &raw)?;
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod test {
+    use std::{collections::HashMap, sync::OnceLock};
+
+    use crate::init::{init_acm_config, AcmConfig};
+
+    use super::AclTree;
+    use anyhow::Error;
+
+    use proxmox_auth_api::types::Authid;
+
+    #[derive(Debug)]
+    struct TestAcmConfig<'a> {
+        roles: HashMap<&'a str, u64>,
+    }
+
+    impl AcmConfig for TestAcmConfig<'_> {
+        fn roles(&self) -> &HashMap<&str, u64> {
+            &self.roles
+        }
+
+        fn role_no_access(&self) -> Option<&'static str> {
+            Some("NoAccess")
+        }
+
+        fn role_admin(&self) -> Option<&'static str> {
+            Some("Admin")
+        }
+    }
+
+    fn setup_acl_tree_config() {
+        static ACL_CONFIG: OnceLock<TestAcmConfig> = OnceLock::new();
+        let config = ACL_CONFIG.get_or_init(|| {
+            let mut roles = HashMap::new();
+            roles.insert("NoAccess", 0);
+            roles.insert("Admin", u64::MAX);
+            roles.insert("DatastoreBackup", 4);
+            roles.insert("DatastoreReader", 8);
+
+            let config = TestAcmConfig { roles };
+            config
+        });
+
+        // ignore errors here, we don't care if it's initialized already
+        let _ = init_acm_config(config);
+    }
+
+    fn check_roles(tree: &AclTree, auth_id: &Authid, path: &str, expected_roles: &str) {
+        let path_vec = super::split_acl_path(path);
+        let mut roles = tree
+            .roles(auth_id, &path_vec)
+            .keys()
+            .cloned()
+            .collect::<Vec<String>>();
+        roles.sort();
+        let roles = roles.join(",");
+
+        assert_eq!(
+            roles, expected_roles,
+            "\nat check_roles for '{}' on '{}'",
+            auth_id, path
+        );
+    }
+
+    #[test]
+    fn test_acl_line_compression() {
+        setup_acl_tree_config();
+
+        let tree = AclTree::from_raw(
+            "\
+            acl:0:/store/store2:user1@pbs:Admin\n\
+            acl:0:/store/store2:user2@pbs:Admin\n\
+            acl:0:/store/store2:user1@pbs:DatastoreBackup\n\
+            acl:0:/store/store2:user2@pbs:DatastoreBackup\n\
+            ",
+        )
+        .expect("failed to parse acl tree");
+
+        let mut raw: Vec<u8> = Vec::new();
+        tree.write_config(&mut raw)
+            .expect("failed to write acl tree");
+        let raw = std::str::from_utf8(&raw).expect("acl tree is not valid utf8");
+
+        assert_eq!(
+            raw,
+            "acl:0:/store/store2:user1@pbs,user2@pbs:Admin,DatastoreBackup\n"
+        );
+    }
+
+    #[test]
+    fn test_roles_1() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let tree = AclTree::from_raw(
+            "\
+            acl:1:/storage:user1@pbs:Admin\n\
+            acl:1:/storage/store1:user1@pbs:DatastoreBackup\n\
+            acl:1:/storage/store2:user2@pbs:DatastoreBackup\n\
+            ",
+        )?;
+        let user1: Authid = "user1@pbs".parse()?;
+        check_roles(&tree, &user1, "/", "");
+        check_roles(&tree, &user1, "/storage", "Admin");
+        check_roles(&tree, &user1, "/storage/store1", "DatastoreBackup");
+        check_roles(&tree, &user1, "/storage/store2", "Admin");
+
+        let user2: Authid = "user2@pbs".parse()?;
+        check_roles(&tree, &user2, "/", "");
+        check_roles(&tree, &user2, "/storage", "");
+        check_roles(&tree, &user2, "/storage/store1", "");
+        check_roles(&tree, &user2, "/storage/store2", "DatastoreBackup");
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_role_no_access() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let tree = AclTree::from_raw(
+            "\
+            acl:1:/:user1@pbs:Admin\n\
+            acl:1:/storage:user1@pbs:NoAccess\n\
+            acl:1:/storage/store1:user1@pbs:DatastoreBackup\n\
+            ",
+        )?;
+        let user1: Authid = "user1@pbs".parse()?;
+        check_roles(&tree, &user1, "/", "Admin");
+        check_roles(&tree, &user1, "/storage", "NoAccess");
+        check_roles(&tree, &user1, "/storage/store1", "DatastoreBackup");
+        check_roles(&tree, &user1, "/storage/store2", "NoAccess");
+        check_roles(&tree, &user1, "/system", "Admin");
+
+        let tree = AclTree::from_raw(
+            "\
+            acl:1:/:user1@pbs:Admin\n\
+            acl:0:/storage:user1@pbs:NoAccess\n\
+            acl:1:/storage/store1:user1@pbs:DatastoreBackup\n\
+            ",
+        )?;
+        check_roles(&tree, &user1, "/", "Admin");
+        check_roles(&tree, &user1, "/storage", "NoAccess");
+        check_roles(&tree, &user1, "/storage/store1", "DatastoreBackup");
+        check_roles(&tree, &user1, "/storage/store2", "Admin");
+        check_roles(&tree, &user1, "/system", "Admin");
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_role_add_delete() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let mut tree = AclTree::new();
+
+        let user1: Authid = "user1@pbs".parse()?;
+
+        tree.insert_user_role("/", &user1, "Admin", true);
+        tree.insert_user_role("/", &user1, "Audit", true);
+
+        check_roles(&tree, &user1, "/", "Admin,Audit");
+
+        tree.insert_user_role("/", &user1, "NoAccess", true);
+        check_roles(&tree, &user1, "/", "NoAccess");
+
+        let mut raw: Vec<u8> = Vec::new();
+        tree.write_config(&mut raw)?;
+        let raw = std::str::from_utf8(&raw)?;
+
+        assert_eq!(raw, "acl:1:/:user1@pbs:NoAccess\n");
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_no_access_overwrite() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let mut tree = AclTree::new();
+
+        let user1: Authid = "user1@pbs".parse()?;
+
+        tree.insert_user_role("/storage", &user1, "NoAccess", true);
+
+        check_roles(&tree, &user1, "/storage", "NoAccess");
+
+        tree.insert_user_role("/storage", &user1, "Admin", true);
+        tree.insert_user_role("/storage", &user1, "Audit", true);
+
+        check_roles(&tree, &user1, "/storage", "Admin,Audit");
+
+        tree.insert_user_role("/storage", &user1, "NoAccess", true);
+
+        check_roles(&tree, &user1, "/storage", "NoAccess");
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_get_child_paths() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let tree = AclTree::from_raw(
+            "\
+            acl:0:/store/store2:user1@pbs:Admin\n\
+            acl:1:/store/store2/store31/store4/store6:user2@pbs:DatastoreReader\n\
+            acl:0:/store/store2/store3:user1@pbs:Admin\n\
+            ",
+        )
+        .expect("failed to parse acl tree");
+
+        let user1: Authid = "user1@pbs".parse()?;
+        let user2: Authid = "user2@pbs".parse()?;
+
+        // user1 has admin on "/store/store2/store3" -> return paths
+        let paths = tree.get_child_paths(&user1, &["store"])?;
+        assert!(
+            paths.len() == 2
+                && paths.contains(&"store/store2".to_string())
+                && paths.contains(&"store/store2/store3".to_string())
+        );
+
+        // user2 has no privileges under "/store/store2/store3" --> return empty
+        assert!(tree
+            .get_child_paths(&user2, &["store", "store2", "store3"],)?
+            .is_empty());
+
+        // user2 has DatastoreReader privileges under "/store/store2/store31" --> return paths
+        let paths = tree.get_child_paths(&user2, &["store/store2/store31"])?;
+        assert!(
+            paths.len() == 1 && paths.contains(&"store/store2/store31/store4/store6".to_string())
+        );
+
+        // user2 has no privileges under "/store/store2/foo/bar/baz"
+        assert!(tree
+            .get_child_paths(&user2, &["store", "store2", "foo/bar/baz"])?
+            .is_empty());
+
+        // user2 has DatastoreReader privileges on "/store/store2/store31/store4/store6", but not
+        // on any child paths --> return empty
+        assert!(tree
+            .get_child_paths(&user2, &["store/store2/store31/store4/store6"],)?
+            .is_empty());
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_delete_node() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let mut tree = AclTree::new();
+
+        let user1: Authid = "user1@pbs".parse()?;
+
+        tree.insert_user_role("/storage", &user1, "NoAccess", true);
+        tree.insert_user_role("/storage/a", &user1, "NoAccess", true);
+        tree.insert_user_role("/storage/b", &user1, "NoAccess", true);
+        tree.insert_user_role("/storage/b/a", &user1, "NoAccess", true);
+        tree.insert_user_role("/storage/b/b", &user1, "NoAccess", true);
+        tree.insert_user_role("/datastore/c", &user1, "NoAccess", true);
+        tree.insert_user_role("/datastore/d", &user1, "NoAccess", true);
+
+        assert!(tree.find_node("/storage/b/a").is_some());
+        tree.delete_node("/storage/b/a");
+        assert!(tree.find_node("/storage/b/a").is_none());
+
+        assert!(tree.find_node("/storage/b/b").is_some());
+        assert!(tree.find_node("/storage/b").is_some());
+        tree.delete_node("/storage/b");
+        assert!(tree.find_node("/storage/b/b").is_none());
+        assert!(tree.find_node("/storage/b").is_none());
+
+        assert!(tree.find_node("/storage").is_some());
+        assert!(tree.find_node("/storage/a").is_some());
+        tree.delete_node("/storage");
+        assert!(tree.find_node("/storage").is_none());
+        assert!(tree.find_node("/storage/a").is_none());
+
+        assert!(tree.find_node("/datastore/c").is_some());
+        tree.delete_node("/datastore/c");
+        assert!(tree.find_node("/datastore/c").is_none());
+
+        assert!(tree.find_node("/datastore/d").is_some());
+        tree.delete_node("/datastore/d");
+        assert!(tree.find_node("/datastore/d").is_none());
+
+        // '/' should not be deletable
+        assert!(tree.find_node("/").is_some());
+        tree.delete_node("/");
+        assert!(tree.find_node("/").is_some());
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_delete_authid() -> Result<(), Error> {
+        setup_acl_tree_config();
+
+        let mut tree = AclTree::new();
+
+        let user1: Authid = "user1@pbs".parse()?;
+        let user2: Authid = "user2@pbs".parse()?;
+
+        let user1_paths = vec![
+            "/",
+            "/storage",
+            "/storage/a",
+            "/storage/a/b",
+            "/storage/b",
+            "/storage/b/a",
+            "/storage/b/b",
+            "/storage/a/a",
+        ];
+        let user2_paths = vec!["/", "/storage", "/storage/a/b", "/storage/a/a"];
+
+        for path in &user1_paths {
+            tree.insert_user_role(path, &user1, "NoAccess", true);
+        }
+        for path in &user2_paths {
+            tree.insert_user_role(path, &user2, "NoAccess", true);
+        }
+
+        tree.delete_authid(&user1);
+
+        for path in &user1_paths {
+            let node = tree.find_node(path);
+            assert!(node.is_some());
+            if let Some(node) = node {
+                assert!(node.users.get(&user1).is_none());
+            }
+        }
+        for path in &user2_paths {
+            let node = tree.find_node(path);
+            assert!(node.is_some());
+            if let Some(node) = node {
+                assert!(node.users.get(&user2).is_some());
+            }
+        }
+
+        tree.delete_authid(&user2);
+
+        for path in &user2_paths {
+            let node = tree.find_node(path);
+            assert!(node.is_some());
+            if let Some(node) = node {
+                assert!(node.users.get(&user2).is_none());
+            }
+        }
+
+        Ok(())
+    }
+}
diff --git a/proxmox-access/src/init.rs b/proxmox-access/src/init.rs
new file mode 100644
index 00000000..71f2f8fc
--- /dev/null
+++ b/proxmox-access/src/init.rs
@@ -0,0 +1,73 @@
+use anyhow::{format_err, Error};
+use std::{
+    collections::HashMap,
+    path::{Path, PathBuf},
+    sync::OnceLock,
+};
+
+static ACM_CONF: OnceLock<&'static dyn AcmConfig> = OnceLock::new();
+static ACM_CONF_DIR: OnceLock<PathBuf> = OnceLock::new();
+
+/// This trait specifies the functions a product needs to implement to get ACL tree based access
+/// control management from this plugin.
+pub trait AcmConfig: Send + Sync {
+    /// Returns a mapping of all recognized roles and their corresponding `u64` value.
+    fn roles(&self) -> &HashMap<&str, u64>;
+
+    /// Optionally returns a role that has no access to any resource.
+    ///
+    /// Default: Returns `None`.
+    fn role_no_access(&self) -> Option<&str> {
+        None
+    }
+
+    /// Optionally returns a role that is allowed to access all resources.
+    ///
+    /// Default: Returns `None`.
+    fn role_admin(&self) -> Option<&str> {
+        None
+    }
+}
+
+pub fn init<P: AsRef<Path>>(
+    acm_config: &'static dyn AcmConfig,
+    config_dir: P,
+) -> Result<(), Error> {
+    init_acm_config(acm_config)?;
+    init_acm_config_dir(config_dir)
+}
+
+pub fn init_acm_config_dir<P: AsRef<Path>>(config_dir: P) -> Result<(), Error> {
+    ACM_CONF_DIR
+        .set(config_dir.as_ref().to_owned())
+        .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
+}
+
+pub(crate) fn init_acm_config(config: &'static dyn AcmConfig) -> Result<(), Error> {
+    ACM_CONF
+        .set(config)
+        .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
+}
+
+
+pub(crate) fn acm_conf() -> &'static dyn AcmConfig {
+    *ACM_CONF
+        .get()
+        .expect("please initialize the acm config before using it!")
+}
+
+
+fn conf_dir() -> &'static PathBuf {
+    ACM_CONF_DIR
+        .get()
+        .expect("please initialize acm config dir before using it!")
+}
+
+pub(crate) fn acl_config() -> PathBuf {
+    conf_dir().with_file_name("acl.cfg")
+}
+
+pub(crate) fn acl_config_lock() -> PathBuf {
+    conf_dir().with_file_name(".acl.lck")
+}
+
diff --git a/proxmox-access/src/lib.rs b/proxmox-access/src/lib.rs
new file mode 100644
index 00000000..8ad2c83d
--- /dev/null
+++ b/proxmox-access/src/lib.rs
@@ -0,0 +1,2 @@
+pub mod acl;
+pub mod init;
-- 
2.39.2



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


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

* [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees Shannon Sterz
@ 2024-06-10 15:42 ` Shannon Sterz
  2024-06-11 12:51   ` Wolfgang Bumiller
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 3/5] access: make token shadow implementation re-usable Shannon Sterz
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

these types are used by the user config in `proxmox-backup` server.
this commit factors them out so we can re-use them in other products
as well as this crate.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access/Cargo.toml   |   3 +
 proxmox-access/src/lib.rs   |   1 +
 proxmox-access/src/types.rs | 228 ++++++++++++++++++++++++++++++++++++
 3 files changed, 232 insertions(+)
 create mode 100644 proxmox-access/src/types.rs

diff --git a/proxmox-access/Cargo.toml b/proxmox-access/Cargo.toml
index 51e97304..70f863f5 100644
--- a/proxmox-access/Cargo.toml
+++ b/proxmox-access/Cargo.toml
@@ -16,7 +16,10 @@ description = "A collection of utilities to implement access control management.
 anyhow.workspace = true
 nix.workspace = true
 openssl.workspace = true
+serde.workspace = true
 
 # proxmox-notify.workspace = true
 proxmox-auth-api = { workspace = true, features = [ "api-types" ] }
+proxmox-schema.workspace = true
 proxmox-product-config.workspace = true
+proxmox-time.workspace = true
diff --git a/proxmox-access/src/lib.rs b/proxmox-access/src/lib.rs
index 8ad2c83d..edb42568 100644
--- a/proxmox-access/src/lib.rs
+++ b/proxmox-access/src/lib.rs
@@ -1,2 +1,3 @@
 pub mod acl;
 pub mod init;
+pub mod types;
diff --git a/proxmox-access/src/types.rs b/proxmox-access/src/types.rs
new file mode 100644
index 00000000..9ed4e9cd
--- /dev/null
+++ b/proxmox-access/src/types.rs
@@ -0,0 +1,228 @@
+use proxmox_auth_api::types::{Authid, Userid, PROXMOX_TOKEN_ID_SCHEMA};
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::{
+    api,
+    api_types::{COMMENT_SCHEMA, SINGLE_LINE_COMMENT_FORMAT},
+    BooleanSchema, IntegerSchema, Schema, StringSchema, Updater,
+};
+
+pub const ENABLE_USER_SCHEMA: Schema = BooleanSchema::new(
+    "Enable the account (default). You can set this to '0' to disable the account.",
+)
+.default(true)
+.schema();
+
+pub const EXPIRE_USER_SCHEMA: Schema = IntegerSchema::new(
+    "Account expiration date (seconds since epoch). '0' means no expiration date.",
+)
+.default(0)
+.minimum(0)
+.schema();
+
+pub const FIRST_NAME_SCHEMA: Schema = StringSchema::new("First name.")
+    .format(&SINGLE_LINE_COMMENT_FORMAT)
+    .min_length(2)
+    .max_length(64)
+    .schema();
+
+pub const LAST_NAME_SCHEMA: Schema = StringSchema::new("Last name.")
+    .format(&SINGLE_LINE_COMMENT_FORMAT)
+    .min_length(2)
+    .max_length(64)
+    .schema();
+
+pub const EMAIL_SCHEMA: Schema = StringSchema::new("E-Mail Address.")
+    .format(&SINGLE_LINE_COMMENT_FORMAT)
+    .min_length(2)
+    .max_length(64)
+    .schema();
+
+#[api(
+    properties: {
+        userid: {
+            type: Userid,
+        },
+        comment: {
+            optional: true,
+            schema: COMMENT_SCHEMA,
+        },
+        enable: {
+            optional: true,
+            schema: ENABLE_USER_SCHEMA,
+        },
+        expire: {
+            optional: true,
+            schema: EXPIRE_USER_SCHEMA,
+        },
+        firstname: {
+            optional: true,
+            schema: FIRST_NAME_SCHEMA,
+        },
+        lastname: {
+            schema: LAST_NAME_SCHEMA,
+            optional: true,
+         },
+        email: {
+            schema: EMAIL_SCHEMA,
+            optional: true,
+        },
+        tokens: {
+            type: Array,
+            optional: true,
+            description: "List of user's API tokens.",
+            items: {
+                type: ApiToken
+            },
+        },
+        "totp-locked": {
+            type: bool,
+            optional: true,
+            default: false,
+            description: "True if the user is currently locked out of TOTP factors",
+        },
+        "tfa-locked-until": {
+            optional: true,
+            description: "Contains a timestamp until when a user is locked out of 2nd factors",
+        },
+    }
+)]
+#[derive(Serialize, Deserialize, Clone, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// User properties with added list of ApiTokens
+pub struct UserWithTokens {
+    pub userid: Userid,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enable: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub expire: Option<i64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub firstname: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub lastname: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub email: Option<String>,
+    #[serde(skip_serializing_if = "Vec::is_empty", default)]
+    pub tokens: Vec<ApiToken>,
+    #[serde(skip_serializing_if = "bool_is_false", default)]
+    pub totp_locked: bool,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub tfa_locked_until: Option<i64>,
+}
+
+fn bool_is_false(b: &bool) -> bool {
+    !b
+}
+
+#[api(
+    properties: {
+        tokenid: {
+            schema: PROXMOX_TOKEN_ID_SCHEMA,
+        },
+        comment: {
+            optional: true,
+            schema: COMMENT_SCHEMA,
+        },
+        enable: {
+            optional: true,
+            schema: ENABLE_USER_SCHEMA,
+        },
+        expire: {
+            optional: true,
+            schema: EXPIRE_USER_SCHEMA,
+        },
+    }
+)]
+#[derive(Serialize, Deserialize, Clone, PartialEq)]
+/// ApiToken properties.
+pub struct ApiToken {
+    pub tokenid: Authid,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enable: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub expire: Option<i64>,
+}
+
+impl ApiToken {
+    pub fn is_active(&self) -> bool {
+        if !self.enable.unwrap_or(true) {
+            return false;
+        }
+        if let Some(expire) = self.expire {
+            let now = proxmox_time::epoch_i64();
+            if expire > 0 && expire <= now {
+                return false;
+            }
+        }
+        true
+    }
+}
+
+#[api(
+    properties: {
+        userid: {
+            type: Userid,
+        },
+        comment: {
+            optional: true,
+            schema: COMMENT_SCHEMA,
+        },
+        enable: {
+            optional: true,
+            schema: ENABLE_USER_SCHEMA,
+        },
+        expire: {
+            optional: true,
+            schema: EXPIRE_USER_SCHEMA,
+        },
+        firstname: {
+            optional: true,
+            schema: FIRST_NAME_SCHEMA,
+        },
+        lastname: {
+            schema: LAST_NAME_SCHEMA,
+            optional: true,
+         },
+        email: {
+            schema: EMAIL_SCHEMA,
+            optional: true,
+        },
+    }
+)]
+#[derive(Serialize, Deserialize, Updater, PartialEq, Eq)]
+/// User properties.
+pub struct User {
+    #[updater(skip)]
+    pub userid: Userid,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enable: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub expire: Option<i64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub firstname: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub lastname: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub email: Option<String>,
+}
+
+impl User {
+    pub fn is_active(&self) -> bool {
+        if !self.enable.unwrap_or(true) {
+            return false;
+        }
+        if let Some(expire) = self.expire {
+            let now = proxmox_time::epoch_i64();
+            if expire > 0 && expire <= now {
+                return false;
+            }
+        }
+        true
+    }
+}
-- 
2.39.2



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


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

* [pbs-devel] [PATCH proxmox 3/5] access: make token shadow implementation re-usable
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees Shannon Sterz
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types Shannon Sterz
@ 2024-06-10 15:42 ` Shannon Sterz
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling Shannon Sterz
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

this commit factors out the token shadow implementation from
`proxmox-backup` so it can be used in other products.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access/Cargo.toml          |  2 +
 proxmox-access/src/init.rs         |  7 +++
 proxmox-access/src/lib.rs          |  1 +
 proxmox-access/src/token_shadow.rs | 84 ++++++++++++++++++++++++++++++
 4 files changed, 94 insertions(+)
 create mode 100644 proxmox-access/src/token_shadow.rs

diff --git a/proxmox-access/Cargo.toml b/proxmox-access/Cargo.toml
index 70f863f5..0949e44c 100644
--- a/proxmox-access/Cargo.toml
+++ b/proxmox-access/Cargo.toml
@@ -17,9 +17,11 @@ anyhow.workspace = true
 nix.workspace = true
 openssl.workspace = true
 serde.workspace = true
+serde_json.workspace = true
 
 # proxmox-notify.workspace = true
 proxmox-auth-api = { workspace = true, features = [ "api-types" ] }
 proxmox-schema.workspace = true
 proxmox-product-config.workspace = true
+proxmox-sys = { workspace = true, features = [ "crypt" ] }
 proxmox-time.workspace = true
diff --git a/proxmox-access/src/init.rs b/proxmox-access/src/init.rs
index 71f2f8fc..4bcfbd87 100644
--- a/proxmox-access/src/init.rs
+++ b/proxmox-access/src/init.rs
@@ -71,3 +71,10 @@ pub(crate) fn acl_config_lock() -> PathBuf {
     conf_dir().with_file_name(".acl.lck")
 }
 
+pub(crate) fn token_shadow() -> PathBuf {
+    conf_dir().with_file_name("token.shadow")
+}
+
+pub(crate) fn token_shadow_lock() -> PathBuf {
+    conf_dir().with_file_name("token.shadow.lock")
+}
diff --git a/proxmox-access/src/lib.rs b/proxmox-access/src/lib.rs
index edb42568..524b0e60 100644
--- a/proxmox-access/src/lib.rs
+++ b/proxmox-access/src/lib.rs
@@ -1,3 +1,4 @@
 pub mod acl;
 pub mod init;
+pub mod token_shadow;
 pub mod types;
diff --git a/proxmox-access/src/token_shadow.rs b/proxmox-access/src/token_shadow.rs
new file mode 100644
index 00000000..ab8925b7
--- /dev/null
+++ b/proxmox-access/src/token_shadow.rs
@@ -0,0 +1,84 @@
+use std::collections::HashMap;
+
+use anyhow::{bail, format_err, Error};
+use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard};
+use serde::{Deserialize, Serialize};
+use serde_json::{from_value, Value};
+
+use proxmox_auth_api::types::Authid;
+
+use crate::init::{token_shadow, token_shadow_lock};
+
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// ApiToken id / secret pair
+pub struct ApiTokenSecret {
+    pub tokenid: Authid,
+    pub secret: String,
+}
+
+// Get exclusive lock
+fn lock_config() -> Result<ApiLockGuard, Error> {
+    open_api_lockfile(token_shadow_lock(), None, true)
+}
+
+fn read_file() -> Result<HashMap<Authid, String>, Error> {
+    let json = proxmox_sys::fs::file_get_json(token_shadow(), Some(Value::Null))?;
+
+    if json == Value::Null {
+        Ok(HashMap::new())
+    } else {
+        // swallow serde error which might contain sensitive data
+        from_value(json)
+            .map_err(|_err| format_err!("unable to parse '{}'", token_shadow().display()))
+    }
+}
+
+fn write_file(data: HashMap<Authid, String>) -> Result<(), Error> {
+    let json = serde_json::to_vec(&data)?;
+    replace_config(token_shadow(), &json)
+}
+
+/// Verifies that an entry for given tokenid / API token secret exists
+pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
+    if !tokenid.is_token() {
+        bail!("not an API token ID");
+    }
+
+    let data = read_file()?;
+    match data.get(tokenid) {
+        Some(hashed_secret) => proxmox_sys::crypt::verify_crypt_pw(secret, hashed_secret),
+        None => bail!("invalid API token"),
+    }
+}
+
+/// Adds a new entry for the given tokenid / API token secret. The secret is stored as salted hash.
+pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
+    if !tokenid.is_token() {
+        bail!("not an API token ID");
+    }
+
+    let _guard = lock_config()?;
+
+    let mut data = read_file()?;
+    let hashed_secret = proxmox_sys::crypt::encrypt_pw(secret)?;
+    data.insert(tokenid.clone(), hashed_secret);
+    write_file(data)?;
+
+    Ok(())
+}
+
+/// Deletes the entry for the given tokenid.
+pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
+    if !tokenid.is_token() {
+        bail!("not an API token ID");
+    }
+
+    let _guard = lock_config()?;
+
+    let mut data = read_file()?;
+    data.remove(tokenid);
+    write_file(data)?;
+
+    Ok(())
+}
-- 
2.39.2



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


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

* [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
                   ` (2 preceding siblings ...)
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 3/5] access: make token shadow implementation re-usable Shannon Sterz
@ 2024-06-10 15:42 ` Shannon Sterz
  2024-06-11 13:07   ` Wolfgang Bumiller
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 5/5] access: increment user cache generation when saving acl config Shannon Sterz
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

this commit factors out the user config as well as the shared memory
based config version caching mechanism. this makes it necessary that
users of this crate provide the product name and a reference to a
shared memory location as well.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 Cargo.toml                                 |   1 +
 proxmox-access/Cargo.toml                  |   3 +
 proxmox-access/src/acl.rs                  |   4 +
 proxmox-access/src/cached_user_info.rs     | 242 +++++++++++++++++++++
 proxmox-access/src/config_version_cache.rs | 113 ++++++++++
 proxmox-access/src/init.rs                 |  61 +++++-
 proxmox-access/src/lib.rs                  |   7 +
 proxmox-access/src/user.rs                 | 182 ++++++++++++++++
 8 files changed, 612 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-access/src/cached_user_info.rs
 create mode 100644 proxmox-access/src/config_version_cache.rs
 create mode 100644 proxmox-access/src/user.rs

diff --git a/Cargo.toml b/Cargo.toml
index 6b17dd4d..32d05d20 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -127,6 +127,7 @@ proxmox-router = { version = "2.1.3", path = "proxmox-router" }
 proxmox-schema = { version = "3.1.1", path = "proxmox-schema" }
 proxmox-section-config = { version = "2.0.0", path = "proxmox-section-config" }
 proxmox-serde = { version = "0.1.1", path = "proxmox-serde", features = [ "serde_json" ] }
+proxmox-shared-memory = { version = "0.3.0", path = "proxmox-shared-memory" }
 proxmox-sortable-macro = { version = "0.1.3", path = "proxmox-sortable-macro" }
 proxmox-sys = { version = "0.5.5", path = "proxmox-sys" }
 proxmox-tfa = { version = "4.0.4", path = "proxmox-tfa" }
diff --git a/proxmox-access/Cargo.toml b/proxmox-access/Cargo.toml
index 0949e44c..6d28445b 100644
--- a/proxmox-access/Cargo.toml
+++ b/proxmox-access/Cargo.toml
@@ -21,7 +21,10 @@ serde_json.workspace = true
 
 # proxmox-notify.workspace = true
 proxmox-auth-api = { workspace = true, features = [ "api-types" ] }
+proxmox-router = { workspace = true }
 proxmox-schema.workspace = true
+proxmox-section-config.workspace = true
 proxmox-product-config.workspace = true
+proxmox-shared-memory.workspace = true
 proxmox-sys = { workspace = true, features = [ "crypt" ] }
 proxmox-time.workspace = true
diff --git a/proxmox-access/src/acl.rs b/proxmox-access/src/acl.rs
index 26f639ae..8cfb4fe7 100644
--- a/proxmox-access/src/acl.rs
+++ b/proxmox-access/src/acl.rs
@@ -665,6 +665,10 @@ mod test {
             &self.roles
         }
 
+        fn privileges(&self) -> &HashMap<&str, u64> {
+            unreachable!("acl tests don't need privileges")
+        }
+
         fn role_no_access(&self) -> Option<&'static str> {
             Some("NoAccess")
         }
diff --git a/proxmox-access/src/cached_user_info.rs b/proxmox-access/src/cached_user_info.rs
new file mode 100644
index 00000000..e647b618
--- /dev/null
+++ b/proxmox-access/src/cached_user_info.rs
@@ -0,0 +1,242 @@
+//! Cached user info for fast ACL permission checks
+
+use std::sync::{Arc, OnceLock, RwLock};
+
+use anyhow::{bail, Error};
+
+use proxmox_auth_api::types::{Authid, Userid};
+use proxmox_router::UserInformation;
+use proxmox_section_config::SectionConfigData;
+use proxmox_time::epoch_i64;
+
+use crate::acl::AclTree;
+use crate::init::acm_conf;
+use crate::types::{ApiToken, User};
+use crate::ConfigVersionCache;
+
+/// Cache User/Group/Token/Acl configuration data for fast permission tests
+pub struct CachedUserInfo {
+    user_cfg: Arc<SectionConfigData>,
+    acl_tree: Arc<AclTree>,
+}
+
+struct ConfigCache {
+    data: Option<Arc<CachedUserInfo>>,
+    last_update: i64,
+    last_user_cache_generation: usize,
+}
+
+impl CachedUserInfo {
+    /// Returns a cached instance (up to 5 seconds old).
+    pub fn new() -> Result<Arc<Self>, Error> {
+        let now = epoch_i64();
+
+        let version_cache = ConfigVersionCache::new()?;
+        let user_cache_generation = version_cache.user_cache_generation();
+
+        static CACHED_CONFIG: OnceLock<RwLock<ConfigCache>> = OnceLock::new();
+        let cached_config = CACHED_CONFIG.get_or_init(|| {
+            RwLock::new(ConfigCache {
+                data: None,
+                last_update: 0,
+                last_user_cache_generation: 0,
+            })
+        });
+
+        {
+            // limit scope
+            let cache = cached_config.read().unwrap();
+            if (user_cache_generation == cache.last_user_cache_generation)
+                && ((now - cache.last_update) < 5)
+            {
+                if let Some(ref config) = cache.data {
+                    return Ok(config.clone());
+                }
+            }
+        }
+
+        let config = Arc::new(CachedUserInfo {
+            user_cfg: crate::user::cached_config()?,
+            acl_tree: crate::acl::cached_config()?,
+        });
+
+        let mut cache = cached_config.write().unwrap();
+        cache.last_update = now;
+        cache.last_user_cache_generation = user_cache_generation;
+        cache.data = Some(config.clone());
+
+        Ok(config)
+    }
+
+    pub fn is_superuser(&self, auth_id: &Authid) -> bool {
+        acm_conf().is_superuser(auth_id)
+    }
+
+    pub fn is_group_member(&self, user_id: &Userid, group: &str) -> bool {
+        acm_conf().is_group_member(user_id, group)
+    }
+
+    /// Test if a user_id is enabled and not expired
+    pub fn is_active_user_id(&self, userid: &Userid) -> bool {
+        if let Ok(info) = self.user_cfg.lookup::<User>("user", userid.as_str()) {
+            info.is_active()
+        } else {
+            false
+        }
+    }
+
+    /// Test if a authentication id is enabled and not expired
+    pub fn is_active_auth_id(&self, auth_id: &Authid) -> bool {
+        let userid = auth_id.user();
+
+        if !self.is_active_user_id(userid) {
+            return false;
+        }
+
+        if auth_id.is_token() {
+            if let Ok(info) = self
+                .user_cfg
+                .lookup::<ApiToken>("token", &auth_id.to_string())
+            {
+                return info.is_active();
+            } else {
+                return false;
+            }
+        }
+
+        true
+    }
+
+    pub fn check_privs(
+        &self,
+        auth_id: &Authid,
+        path: &[&str],
+        required_privs: u64,
+        partial: bool,
+    ) -> Result<(), Error> {
+        let privs = self.lookup_privs(auth_id, path);
+        let allowed = if partial {
+            (privs & required_privs) != 0
+        } else {
+            (privs & required_privs) == required_privs
+        };
+        if !allowed {
+            // printing the path doesn't leak any information as long as we
+            // always check privilege before resource existence
+            let priv_names = privs_to_priv_names(required_privs);
+            let priv_names = if partial {
+                priv_names.join("|")
+            } else {
+                priv_names.join("&")
+            };
+            bail!(
+                "missing permissions '{priv_names}' on '/{}'",
+                path.join("/")
+            );
+        }
+        Ok(())
+    }
+
+    pub fn lookup_privs(&self, auth_id: &Authid, path: &[&str]) -> u64 {
+        let (privs, _) = self.lookup_privs_details(auth_id, path);
+        privs
+    }
+
+    pub fn lookup_privs_details(&self, auth_id: &Authid, path: &[&str]) -> (u64, u64) {
+        if self.is_superuser(auth_id) {
+            let acm_config = acm_conf();
+            if let Some(admin) = acm_config.role_admin() {
+                if let Some(admin) = acm_config.roles().get(admin) {
+                    return (*admin, *admin);
+                }
+            }
+        }
+
+        let roles = self.acl_tree.roles(auth_id, path);
+        let mut privs: u64 = 0;
+        let mut propagated_privs: u64 = 0;
+        for (role, propagate) in roles {
+            if let Some(role_privs) = acm_conf().roles().get(role.as_str()) {
+                if propagate {
+                    propagated_privs |= role_privs;
+                }
+                privs |= role_privs;
+            }
+        }
+
+        if auth_id.is_token() {
+            // limit privs to that of owning user
+            let user_auth_id = Authid::from(auth_id.user().clone());
+            let (owner_privs, owner_propagated_privs) =
+                self.lookup_privs_details(&user_auth_id, path);
+            privs &= owner_privs;
+            propagated_privs &= owner_propagated_privs;
+        }
+
+        (privs, propagated_privs)
+    }
+
+    /// Checks whether the `auth_id` has any of the privilegs `privs` on any object below `path`.
+    pub fn any_privs_below(
+        &self,
+        auth_id: &Authid,
+        path: &[&str],
+        privs: u64,
+    ) -> Result<bool, Error> {
+        // if the anchor path itself has matching propagated privs, we skip checking children
+        let (_privs, propagated_privs) = self.lookup_privs_details(auth_id, path);
+        if propagated_privs & privs != 0 {
+            return Ok(true);
+        }
+
+        // get all sub-paths with roles defined for `auth_id`
+        let paths = self.acl_tree.get_child_paths(auth_id, path)?;
+
+        for path in paths.iter() {
+            // early return if any sub-path has any of the privs we are looking for
+            if privs & self.lookup_privs(auth_id, &[path.as_str()]) != 0 {
+                return Ok(true);
+            }
+        }
+
+        // no paths or no matching paths
+        Ok(false)
+    }
+}
+
+impl UserInformation for CachedUserInfo {
+    fn is_superuser(&self, userid: &str) -> bool {
+        if let Ok(authid) = userid.parse() {
+            return self.is_superuser(&authid);
+        }
+
+        false
+    }
+
+    fn is_group_member(&self, userid: &str, group: &str) -> bool {
+        if let Ok(userid) = userid.parse() {
+            return self.is_group_member(&userid, group);
+        }
+
+        false
+    }
+
+    fn lookup_privs(&self, auth_id: &str, path: &[&str]) -> u64 {
+        match auth_id.parse::<Authid>() {
+            Ok(auth_id) => Self::lookup_privs(self, &auth_id, path),
+            Err(_) => 0,
+        }
+    }
+}
+
+pub fn privs_to_priv_names(privs: u64) -> Vec<&'static str> {
+    acm_conf()
+        .privileges()
+        .iter()
+        .fold(Vec::new(), |mut priv_names, (name, value)| {
+            if value & privs != 0 {
+                priv_names.push(name);
+            }
+            priv_names
+        })
+}
diff --git a/proxmox-access/src/config_version_cache.rs b/proxmox-access/src/config_version_cache.rs
new file mode 100644
index 00000000..62effab7
--- /dev/null
+++ b/proxmox-access/src/config_version_cache.rs
@@ -0,0 +1,113 @@
+use std::mem::{ManuallyDrop, MaybeUninit};
+use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::{Arc, OnceLock};
+
+use anyhow::{bail, Error};
+use nix::sys::stat::Mode;
+
+use proxmox_product_config::default_create_options;
+use proxmox_sys::fs::create_path;
+
+use proxmox_shared_memory::*;
+
+use crate::init::{shmem_file, shmem_magic};
+
+#[derive(Debug)]
+#[repr(C)]
+struct ConfigVersionCacheDataInner {
+    magic: [u8; 8],
+    // User (user.cfg) cache generation/version.
+    user_cache_generation: AtomicUsize,
+}
+
+#[repr(C)]
+union ConfigVersionCacheData {
+    data: ManuallyDrop<ConfigVersionCacheDataInner>,
+    _padding: [u8; 4096],
+}
+
+#[test]
+fn assert_cache_size() {
+    assert_eq!(std::mem::size_of::<ConfigVersionCacheData>(), 4096);
+}
+
+impl std::ops::Deref for ConfigVersionCacheData {
+    type Target = ConfigVersionCacheDataInner;
+
+    #[inline]
+    fn deref(&self) -> &ConfigVersionCacheDataInner {
+        unsafe { &self.data }
+    }
+}
+
+impl std::ops::DerefMut for ConfigVersionCacheData {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut ConfigVersionCacheDataInner {
+        unsafe { &mut self.data }
+    }
+}
+
+impl Init for ConfigVersionCacheData {
+    fn initialize(this: &mut MaybeUninit<Self>) {
+        unsafe {
+            let me = &mut *this.as_mut_ptr();
+            me.magic = *shmem_magic();
+        }
+    }
+
+    fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
+        unsafe {
+            let me = &*this.as_ptr();
+            if me.magic != *shmem_magic() {
+                bail!("ConfigVersionCache: wrong magic number");
+            }
+            Ok(())
+        }
+    }
+}
+
+pub struct ConfigVersionCache {
+    shmem: SharedMemory<ConfigVersionCacheData>,
+}
+
+static INSTANCE: OnceLock<Arc<ConfigVersionCache>> = OnceLock::new();
+
+impl ConfigVersionCache {
+    /// Open the memory based communication channel singleton.
+    pub fn new() -> Result<Arc<Self>, Error> {
+        Ok(INSTANCE
+            .get_or_init(|| Self::open().expect("could not open config version cache!"))
+            .clone())
+    }
+
+    // Actual work of `new`:
+    fn open() -> Result<Arc<Self>, Error> {
+        let dir_opts = default_create_options().perm(Mode::from_bits_truncate(0o770));
+
+        let file_path = shmem_file();
+        let dir_path = file_path.parent().unwrap();
+
+        create_path(dir_path, Some(dir_opts.clone()), Some(dir_opts))?;
+
+        let file_opts = default_create_options().perm(Mode::from_bits_truncate(0o660));
+        let shmem: SharedMemory<ConfigVersionCacheData> = SharedMemory::open(file_path, file_opts)?;
+
+        Ok(Arc::new(Self { shmem }))
+    }
+
+    /// Returns the user cache generation number.
+    pub fn user_cache_generation(&self) -> usize {
+        self.shmem
+            .data()
+            .user_cache_generation
+            .load(Ordering::Acquire)
+    }
+
+    /// Increase the user cache generation number.
+    pub fn increase_user_cache_generation(&self) {
+        self.shmem
+            .data()
+            .user_cache_generation
+            .fetch_add(1, Ordering::AcqRel);
+    }
+}
diff --git a/proxmox-access/src/init.rs b/proxmox-access/src/init.rs
index 4bcfbd87..ad8584ae 100644
--- a/proxmox-access/src/init.rs
+++ b/proxmox-access/src/init.rs
@@ -1,4 +1,5 @@
 use anyhow::{format_err, Error};
+use proxmox_auth_api::types::{Authid, Userid};
 use std::{
     collections::HashMap,
     path::{Path, PathBuf},
@@ -7,13 +8,32 @@ use std::{
 
 static ACM_CONF: OnceLock<&'static dyn AcmConfig> = OnceLock::new();
 static ACM_CONF_DIR: OnceLock<PathBuf> = OnceLock::new();
+static SHMEM_FILE: OnceLock<PathBuf> = OnceLock::new();
+static PROXMOX_CONFIG_VERSION_CACHE_MAGIC: OnceLock<[u8; 8]> = OnceLock::new();
 
 /// This trait specifies the functions a product needs to implement to get ACL tree based access
 /// control management from this plugin.
 pub trait AcmConfig: Send + Sync {
+    /// Returns a mapping of all recognized privileges and their corresponding `u64` value.
+    fn privileges(&self) -> &HashMap<&str, u64>;
+
     /// Returns a mapping of all recognized roles and their corresponding `u64` value.
     fn roles(&self) -> &HashMap<&str, u64>;
 
+    /// Checks whether an `Authid` has super user privileges or not.
+    ///
+    /// Default: Always returns `false`.
+    fn is_superuser(&self, _auth_id: &Authid) -> bool {
+        false
+    }
+
+    /// Checks whether a user is part of a group.
+    ///
+    /// Default: Always returns `false`.
+    fn is_group_member(&self, _user_id: &Userid, _group: &str) -> bool {
+        false
+    }
+
     /// Optionally returns a role that has no access to any resource.
     ///
     /// Default: Returns `None`.
@@ -32,9 +52,12 @@ pub trait AcmConfig: Send + Sync {
 pub fn init<P: AsRef<Path>>(
     acm_config: &'static dyn AcmConfig,
     config_dir: P,
+    shmem_path: P,
+    product_name: &str,
 ) -> Result<(), Error> {
     init_acm_config(acm_config)?;
-    init_acm_config_dir(config_dir)
+    init_acm_config_dir(config_dir)?;
+    init_shmem_cache(shmem_path, product_name)
 }
 
 pub fn init_acm_config_dir<P: AsRef<Path>>(config_dir: P) -> Result<(), Error> {
@@ -49,6 +72,23 @@ pub(crate) fn init_acm_config(config: &'static dyn AcmConfig) -> Result<(), Erro
         .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
 }
 
+fn init_shmem_cache<P: AsRef<Path>>(shmem_path: P, product_name: &str) -> Result<(), Error> {
+    let magic = openssl::sha::sha256(product_name.as_bytes());
+
+    PROXMOX_CONFIG_VERSION_CACHE_MAGIC
+        .set(
+            magic[0..8]
+                .try_into()
+                // this will never fail because sha256() always returns a [u8, 32] from which we
+                // can always get a [u8, 8].
+                .unwrap(),
+        )
+        .map_err(|_e| format_err!("shared memory cache cannot be initialized twice!"))?;
+
+    SHMEM_FILE
+        .set(shmem_path.as_ref().to_owned())
+        .map_err(|_e| format_err!("shared memory cache cannot be initialized twice!"))
+}
 
 pub(crate) fn acm_conf() -> &'static dyn AcmConfig {
     *ACM_CONF
@@ -56,6 +96,17 @@ pub(crate) fn acm_conf() -> &'static dyn AcmConfig {
         .expect("please initialize the acm config before using it!")
 }
 
+pub(crate) fn shmem_magic() -> &'static [u8; 8] {
+    PROXMOX_CONFIG_VERSION_CACHE_MAGIC
+        .get()
+        .expect("shared memory cache magic wasn't initialized before using it!")
+}
+
+pub(crate) fn shmem_file() -> &'static PathBuf {
+    SHMEM_FILE
+        .get()
+        .expect("shared memory cache magic wasn't initialized before using it!")
+}
 
 fn conf_dir() -> &'static PathBuf {
     ACM_CONF_DIR
@@ -71,6 +122,14 @@ pub(crate) fn acl_config_lock() -> PathBuf {
     conf_dir().with_file_name(".acl.lck")
 }
 
+pub(crate) fn user_config() -> PathBuf {
+    conf_dir().with_file_name("user.cfg")
+}
+
+pub(crate) fn user_config_lock() -> PathBuf {
+    conf_dir().with_file_name(".user.lck")
+}
+
 pub(crate) fn token_shadow() -> PathBuf {
     conf_dir().with_file_name("token.shadow")
 }
diff --git a/proxmox-access/src/lib.rs b/proxmox-access/src/lib.rs
index 524b0e60..7d3af46c 100644
--- a/proxmox-access/src/lib.rs
+++ b/proxmox-access/src/lib.rs
@@ -2,3 +2,10 @@ pub mod acl;
 pub mod init;
 pub mod token_shadow;
 pub mod types;
+pub mod user;
+
+mod cached_user_info;
+pub use cached_user_info::CachedUserInfo;
+
+mod config_version_cache;
+pub use config_version_cache::ConfigVersionCache;
diff --git a/proxmox-access/src/user.rs b/proxmox-access/src/user.rs
new file mode 100644
index 00000000..c2263c9e
--- /dev/null
+++ b/proxmox-access/src/user.rs
@@ -0,0 +1,182 @@
+use std::collections::HashMap;
+use std::sync::{Arc, OnceLock, RwLock};
+
+use anyhow::{bail, Error};
+
+use proxmox_auth_api::types::Authid;
+use proxmox_product_config::{open_api_lockfile, replace_privileged_config, ApiLockGuard};
+use proxmox_schema::*;
+use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
+
+use crate::init::{user_config, user_config_lock};
+use crate::types::{ApiToken, User};
+
+fn get_or_init_config() -> &'static SectionConfig {
+    static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
+    CONFIG.get_or_init(|| {
+        let mut config = SectionConfig::new(&Authid::API_SCHEMA);
+
+        let user_schema = match User::API_SCHEMA {
+            Schema::Object(ref user_schema) => user_schema,
+            _ => unreachable!(),
+        };
+        let user_plugin =
+            SectionConfigPlugin::new("user".to_string(), Some("userid".to_string()), user_schema);
+        config.register_plugin(user_plugin);
+
+        let token_schema = match ApiToken::API_SCHEMA {
+            Schema::Object(ref token_schema) => token_schema,
+            _ => unreachable!(),
+        };
+        let token_plugin = SectionConfigPlugin::new(
+            "token".to_string(),
+            Some("tokenid".to_string()),
+            token_schema,
+        );
+        config.register_plugin(token_plugin);
+
+        config
+    })
+}
+
+/// Get exclusive lock
+pub fn lock_config() -> Result<ApiLockGuard, Error> {
+    open_api_lockfile(user_config_lock(), None, true)
+}
+
+pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
+    let content = proxmox_sys::fs::file_read_optional_string(user_config())?.unwrap_or_default();
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = get_or_init_config().parse(user_config(), &content)?;
+
+    Ok((data, digest))
+}
+
+pub fn cached_config() -> Result<Arc<SectionConfigData>, Error> {
+    struct ConfigCache {
+        data: Option<Arc<SectionConfigData>>,
+        last_mtime: i64,
+        last_mtime_nsec: i64,
+    }
+
+    static CACHED_CONFIG: OnceLock<RwLock<ConfigCache>> = OnceLock::new();
+    let cached_config = CACHED_CONFIG.get_or_init(|| {
+        RwLock::new(ConfigCache {
+            data: None,
+            last_mtime: 0,
+            last_mtime_nsec: 0,
+        })
+    });
+
+    let stat = match nix::sys::stat::stat(&user_config()) {
+        Ok(stat) => Some(stat),
+        Err(nix::errno::Errno::ENOENT) => None,
+        Err(err) => bail!("unable to stat '{}' - {err}", user_config().display()),
+    };
+
+    {
+        // limit scope
+        let cache = cached_config.read().unwrap();
+        if let Some(ref config) = cache.data {
+            if let Some(stat) = stat {
+                if stat.st_mtime == cache.last_mtime && stat.st_mtime_nsec == cache.last_mtime_nsec
+                {
+                    return Ok(config.clone());
+                }
+            } else if cache.last_mtime == 0 && cache.last_mtime_nsec == 0 {
+                return Ok(config.clone());
+            }
+        }
+    }
+
+    let (config, _digest) = config()?;
+    let config = Arc::new(config);
+
+    let mut cache = cached_config.write().unwrap();
+    if let Some(stat) = stat {
+        cache.last_mtime = stat.st_mtime;
+        cache.last_mtime_nsec = stat.st_mtime_nsec;
+    }
+    cache.data = Some(config.clone());
+
+    Ok(config)
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let config_file = user_config();
+    let raw = get_or_init_config().write(&config_file, config)?;
+    replace_privileged_config(config_file, raw.as_bytes())?;
+
+    // increase user version
+    // We use this in CachedUserInfo
+    let version_cache = crate::ConfigVersionCache::new()?;
+    version_cache.increase_user_cache_generation();
+
+    Ok(())
+}
+
+/// Only exposed for testing
+#[doc(hidden)]
+pub fn test_cfg_from_str(raw: &str) -> Result<(SectionConfigData, [u8; 32]), Error> {
+    let cfg = get_or_init_config();
+    let parsed = cfg.parse("test_user_cfg", raw)?;
+
+    Ok((parsed, [0; 32]))
+}
+
+// shell completion helper
+pub fn complete_userid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data
+            .sections
+            .iter()
+            .filter_map(|(id, (section_type, _))| {
+                if section_type == "user" {
+                    Some(id.to_string())
+                } else {
+                    None
+                }
+            })
+            .collect(),
+        Err(_) => Vec::new(),
+    }
+}
+
+// shell completion helper
+pub fn complete_authid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data.sections.keys().map(|id| id.to_string()).collect(),
+        Err(_) => vec![],
+    }
+}
+
+// shell completion helper
+pub fn complete_token_name(_arg: &str, param: &HashMap<String, String>) -> Vec<String> {
+    let data = match config() {
+        Ok((data, _digest)) => data,
+        Err(_) => return Vec::new(),
+    };
+
+    match param.get("userid") {
+        Some(userid) => {
+            let user = data.lookup::<User>("user", userid);
+            let tokens = data.convert_to_typed_array("token");
+            match (user, tokens) {
+                (Ok(_), Ok(tokens)) => tokens
+                    .into_iter()
+                    .filter_map(|token: ApiToken| {
+                        let tokenid = token.tokenid;
+                        if tokenid.is_token() && tokenid.user() == userid {
+                            Some(tokenid.tokenname().unwrap().as_str().to_string())
+                        } else {
+                            None
+                        }
+                    })
+                    .collect(),
+                _ => vec![],
+            }
+        }
+        None => vec![],
+    }
+}
-- 
2.39.2



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


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

* [pbs-devel] [PATCH proxmox 5/5] access: increment user cache generation when saving acl config
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
                   ` (3 preceding siblings ...)
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling Shannon Sterz
@ 2024-06-10 15:42 ` Shannon Sterz
  2024-06-11 17:28 ` [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Thomas Lamprecht
  2024-06-13 12:54 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2024-06-10 15:42 UTC (permalink / raw)
  To: pbs-devel

since `CachedUserInfo` takes care of both, the user config and the acl
config, we need to also bump the cache generation when storing the
acl config.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access/src/acl.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/proxmox-access/src/acl.rs b/proxmox-access/src/acl.rs
index 8cfb4fe7..6489a1a1 100644
--- a/proxmox-access/src/acl.rs
+++ b/proxmox-access/src/acl.rs
@@ -641,6 +641,11 @@ pub fn save_config(acl: &AclTree) -> Result<(), Error> {
     let conf = acl_config();
     replace_privileged_config(conf, &raw)?;
 
+    // increase user version
+    // We use this in CachedUserInfo
+    let version_cache = crate::ConfigVersionCache::new()?;
+    version_cache.increase_user_cache_generation();
+
     Ok(())
 }
 
-- 
2.39.2



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


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

* Re: [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types Shannon Sterz
@ 2024-06-11 12:51   ` Wolfgang Bumiller
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Bumiller @ 2024-06-11 12:51 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

On Mon, Jun 10, 2024 at 05:42:11PM GMT, Shannon Sterz wrote:
> +#[api(
> +    properties: {
> +        userid: {
> +            type: Userid,
> +        },
> +        comment: {
> +            optional: true,
> +            schema: COMMENT_SCHEMA,
> +        },
> +        enable: {
> +            optional: true,
> +            schema: ENABLE_USER_SCHEMA,
> +        },
> +        expire: {
> +            optional: true,
> +            schema: EXPIRE_USER_SCHEMA,
> +        },
> +        firstname: {
> +            optional: true,
> +            schema: FIRST_NAME_SCHEMA,
> +        },
> +        lastname: {
> +            schema: LAST_NAME_SCHEMA,
> +            optional: true,
> +         },
> +        email: {
> +            schema: EMAIL_SCHEMA,
> +            optional: true,
> +        },
> +        tokens: {
> +            type: Array,
> +            optional: true,
> +            description: "List of user's API tokens.",
> +            items: {
> +                type: ApiToken
> +            },
> +        },
> +        "totp-locked": {
> +            type: bool,
> +            optional: true,
> +            default: false,
> +            description: "True if the user is currently locked out of TOTP factors",
> +        },
> +        "tfa-locked-until": {
> +            optional: true,
> +            description: "Contains a timestamp until when a user is locked out of 2nd factors",
> +        },
> +    }
> +)]
> +#[derive(Serialize, Deserialize, Clone, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +/// User properties with added list of ApiTokens
> +pub struct UserWithTokens {

While already moving things around, can we make this just contain a
nested `User` with `#[serde(flatten)]`?

> +    pub userid: Userid,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub comment: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub enable: Option<bool>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub expire: Option<i64>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub firstname: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub lastname: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub email: Option<String>,
> +    #[serde(skip_serializing_if = "Vec::is_empty", default)]
> +    pub tokens: Vec<ApiToken>,
> +    #[serde(skip_serializing_if = "bool_is_false", default)]
> +    pub totp_locked: bool,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub tfa_locked_until: Option<i64>,
> +}
> +
> +fn bool_is_false(b: &bool) -> bool {
> +    !b
> +}
> +
> +#[api(
> +    properties: {
> +        tokenid: {
> +            schema: PROXMOX_TOKEN_ID_SCHEMA,
> +        },
> +        comment: {
> +            optional: true,
> +            schema: COMMENT_SCHEMA,
> +        },
> +        enable: {
> +            optional: true,
> +            schema: ENABLE_USER_SCHEMA,
> +        },
> +        expire: {
> +            optional: true,
> +            schema: EXPIRE_USER_SCHEMA,
> +        },
> +    }
> +)]
> +#[derive(Serialize, Deserialize, Clone, PartialEq)]
> +/// ApiToken properties.
> +pub struct ApiToken {
> +    pub tokenid: Authid,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub comment: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub enable: Option<bool>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub expire: Option<i64>,
> +}
> +
> +impl ApiToken {
> +    pub fn is_active(&self) -> bool {
> +        if !self.enable.unwrap_or(true) {
> +            return false;
> +        }
> +        if let Some(expire) = self.expire {
> +            let now = proxmox_time::epoch_i64();
> +            if expire > 0 && expire <= now {
> +                return false;
> +            }
> +        }
> +        true
> +    }
> +}
> +
> +#[api(
> +    properties: {
> +        userid: {
> +            type: Userid,
> +        },
> +        comment: {
> +            optional: true,
> +            schema: COMMENT_SCHEMA,
> +        },
> +        enable: {
> +            optional: true,
> +            schema: ENABLE_USER_SCHEMA,
> +        },
> +        expire: {
> +            optional: true,
> +            schema: EXPIRE_USER_SCHEMA,
> +        },
> +        firstname: {
> +            optional: true,
> +            schema: FIRST_NAME_SCHEMA,
> +        },
> +        lastname: {
> +            schema: LAST_NAME_SCHEMA,
> +            optional: true,
> +         },
> +        email: {
> +            schema: EMAIL_SCHEMA,
> +            optional: true,
> +        },
> +    }
> +)]
> +#[derive(Serialize, Deserialize, Updater, PartialEq, Eq)]
> +/// User properties.
> +pub struct User {
> +    #[updater(skip)]
> +    pub userid: Userid,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub comment: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub enable: Option<bool>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub expire: Option<i64>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub firstname: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub lastname: Option<String>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub email: Option<String>,
> +}
> +
> +impl User {
> +    pub fn is_active(&self) -> bool {
> +        if !self.enable.unwrap_or(true) {
> +            return false;
> +        }
> +        if let Some(expire) = self.expire {
> +            let now = proxmox_time::epoch_i64();
> +            if expire > 0 && expire <= now {
> +                return false;
> +            }
> +        }
> +        true
> +    }
> +}
> -- 
> 2.39.2


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


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

* Re: [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees Shannon Sterz
@ 2024-06-11 12:53   ` Wolfgang Bumiller
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Bumiller @ 2024-06-11 12:53 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

On Mon, Jun 10, 2024 at 05:42:10PM GMT, Shannon Sterz wrote:
> diff --git a/proxmox-access/src/init.rs b/proxmox-access/src/init.rs
> new file mode 100644
> index 00000000..71f2f8fc
> --- /dev/null
> +++ b/proxmox-access/src/init.rs
> @@ -0,0 +1,73 @@
> +use anyhow::{format_err, Error};
> +use std::{
> +    collections::HashMap,
> +    path::{Path, PathBuf},
> +    sync::OnceLock,
> +};
> +
> +static ACM_CONF: OnceLock<&'static dyn AcmConfig> = OnceLock::new();
> +static ACM_CONF_DIR: OnceLock<PathBuf> = OnceLock::new();
> +
> +/// This trait specifies the functions a product needs to implement to get ACL tree based access
> +/// control management from this plugin.
> +pub trait AcmConfig: Send + Sync {

This is a terrible name ;-)
Given the methods defined here, we could just call it `RoleSetup`?

> +    /// Returns a mapping of all recognized roles and their corresponding `u64` value.
> +    fn roles(&self) -> &HashMap<&str, u64>;
> +
> +    /// Optionally returns a role that has no access to any resource.
> +    ///
> +    /// Default: Returns `None`.
> +    fn role_no_access(&self) -> Option<&str> {
> +        None
> +    }
> +
> +    /// Optionally returns a role that is allowed to access all resources.
> +    ///
> +    /// Default: Returns `None`.
> +    fn role_admin(&self) -> Option<&str> {
> +        None
> +    }
> +}
> +
> +pub fn init<P: AsRef<Path>>(
> +    acm_config: &'static dyn AcmConfig,
> +    config_dir: P,
> +) -> Result<(), Error> {
> +    init_acm_config(acm_config)?;
> +    init_acm_config_dir(config_dir)
> +}
> +
> +pub fn init_acm_config_dir<P: AsRef<Path>>(config_dir: P) -> Result<(), Error> {

^ pub(crate) ?

> +    ACM_CONF_DIR
> +        .set(config_dir.as_ref().to_owned())
> +        .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
> +}
> +
> +pub(crate) fn init_acm_config(config: &'static dyn AcmConfig) -> Result<(), Error> {
> +    ACM_CONF
> +        .set(config)
> +        .map_err(|_e| format_err!("cannot initialize acl tree config twice!"))
> +}
> +
> +
> +pub(crate) fn acm_conf() -> &'static dyn AcmConfig {
> +    *ACM_CONF
> +        .get()
> +        .expect("please initialize the acm config before using it!")
> +}
> +
> +
> +fn conf_dir() -> &'static PathBuf {
> +    ACM_CONF_DIR
> +        .get()
> +        .expect("please initialize acm config dir before using it!")
> +}
> +
> +pub(crate) fn acl_config() -> PathBuf {
> +    conf_dir().with_file_name("acl.cfg")
> +}
> +
> +pub(crate) fn acl_config_lock() -> PathBuf {
> +    conf_dir().with_file_name(".acl.lck")
> +}
> +
> diff --git a/proxmox-access/src/lib.rs b/proxmox-access/src/lib.rs
> new file mode 100644
> index 00000000..8ad2c83d
> --- /dev/null
> +++ b/proxmox-access/src/lib.rs
> @@ -0,0 +1,2 @@
> +pub mod acl;
> +pub mod init;
> -- 
> 2.39.2


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


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

* Re: [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling Shannon Sterz
@ 2024-06-11 13:07   ` Wolfgang Bumiller
  2024-06-11 14:30     ` Shannon Sterz
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Bumiller @ 2024-06-11 13:07 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

On Mon, Jun 10, 2024 at 05:42:13PM GMT, Shannon Sterz wrote:
> this commit factors out the user config as well as the shared memory
> based config version caching mechanism. this makes it necessary that
> users of this crate provide the product name and a reference to a
> shared memory location as well.

Given that we have more values in the shared memory in PBS I wonder if
we should instead provide the `get` and `increment` methods to init
(directly as `fn` or via a trait).


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


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

* Re: [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling
  2024-06-11 13:07   ` Wolfgang Bumiller
@ 2024-06-11 14:30     ` Shannon Sterz
  2024-06-12 12:49       ` Wolfgang Bumiller
  0 siblings, 1 reply; 13+ messages in thread
From: Shannon Sterz @ 2024-06-11 14:30 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pbs-devel

On Tue Jun 11, 2024 at 3:07 PM CEST, Wolfgang Bumiller wrote:
> On Mon, Jun 10, 2024 at 05:42:13PM GMT, Shannon Sterz wrote:
> > this commit factors out the user config as well as the shared memory
> > based config version caching mechanism. this makes it necessary that
> > users of this crate provide the product name and a reference to a
> > shared memory location as well.
>
> Given that we have more values in the shared memory in PBS I wonder if
> we should instead provide the `get` and `increment` methods to init
> (directly as `fn` or via a trait).

yeah, that might make sense to hand that over via a trait and implement
the actually shared memory portion either in the project itself or have
a generic implementation that we can adapt to each project. not entirely
sure what the best way is, though.

if we do implement some kind of generic shared memory cache generation
counter, where would live ideal?


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


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

* Re: [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
                   ` (4 preceding siblings ...)
  2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 5/5] access: increment user cache generation when saving acl config Shannon Sterz
@ 2024-06-11 17:28 ` Thomas Lamprecht
  2024-06-13 12:54 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2024-06-11 17:28 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Shannon Sterz

Am 10/06/2024 um 17:42 schrieb Shannon Sterz:
> the idea behind the `proxmox-access` crate is to enable us to easily
> re-use the acl tree, user config and token shadow implementation of
> proxmox backup server across multiple projects. this series factors out
> the implementations from proxmox-backup's `pbs-config` crate.

High level comment:

"access" is a bit too generic for my taste, "access-control" would be slightly
better, that is also used on the PVE Perl side, i.e., it gas a long history in
Proxmox projects.
While surely not perfect, it would IMO still better describe what that crate
might provide (i.e., not bindings for MS Access ;-P), maybe someone else
has a better idea; but if not I'd do /access/access-control/.


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


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

* Re: [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling
  2024-06-11 14:30     ` Shannon Sterz
@ 2024-06-12 12:49       ` Wolfgang Bumiller
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Bumiller @ 2024-06-12 12:49 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

On Tue, Jun 11, 2024 at 04:30:25PM GMT, Shannon Sterz wrote:
> On Tue Jun 11, 2024 at 3:07 PM CEST, Wolfgang Bumiller wrote:
> > On Mon, Jun 10, 2024 at 05:42:13PM GMT, Shannon Sterz wrote:
> > > this commit factors out the user config as well as the shared memory
> > > based config version caching mechanism. this makes it necessary that
> > > users of this crate provide the product name and a reference to a
> > > shared memory location as well.
> >
> > Given that we have more values in the shared memory in PBS I wonder if
> > we should instead provide the `get` and `increment` methods to init
> > (directly as `fn` or via a trait).
> 
> yeah, that might make sense to hand that over via a trait and implement
> the actually shared memory portion either in the project itself or have
> a generic implementation that we can adapt to each project. not entirely
> sure what the best way is, though.
> 
> if we do implement some kind of generic shared memory cache generation
> counter, where would live ideal?

Depends on what it does.
As long as we only need simple counters, the crate could just expose
them as an array filling the allocated 4k with the users defining
indices for their counters. I doubt we'll need more than what we can fit
in there for now ;-)


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


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

* Re: [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate
  2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
                   ` (5 preceding siblings ...)
  2024-06-11 17:28 ` [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Thomas Lamprecht
@ 2024-06-13 12:54 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2024-06-13 12:54 UTC (permalink / raw)
  To: Shannon Sterz, pbs-devel

send a v2: https://lists.proxmox.com/pipermail/pbs-devel/2024-June/009857.html


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


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

end of thread, other threads:[~2024-06-13 12:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-10 15:42 [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Shannon Sterz
2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 1/5] access: add the proxmox-access crate to reuse acl trees Shannon Sterz
2024-06-11 12:53   ` Wolfgang Bumiller
2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 2/5] access: define shared `User`, `UserWithTokens` and `ApiTokens types Shannon Sterz
2024-06-11 12:51   ` Wolfgang Bumiller
2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 3/5] access: make token shadow implementation re-usable Shannon Sterz
2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 4/5] access: factor out user config and cache handling Shannon Sterz
2024-06-11 13:07   ` Wolfgang Bumiller
2024-06-11 14:30     ` Shannon Sterz
2024-06-12 12:49       ` Wolfgang Bumiller
2024-06-10 15:42 ` [pbs-devel] [PATCH proxmox 5/5] access: increment user cache generation when saving acl config Shannon Sterz
2024-06-11 17:28 ` [pbs-devel] [PATCH proxmox 0/5] add proxmox-access crate Thomas Lamprecht
2024-06-13 12:54 ` Shannon Sterz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal