[pbs-devel] [PATCH proxmox-datacenter-manager v3 1/2] pdm-config: implement token.shadow generation
Samuel Rufinatscha
s.rufinatscha at proxmox.com
Mon Jan 19 08:56:14 CET 2026
comments inline
On 1/16/26 5:47 PM, Shannon Sterz wrote:
> On Fri Jan 16, 2026 at 5:28 PM CET, Samuel Rufinatscha wrote:
>> On 1/14/26 11:44 AM, Fabian Grünbichler wrote:
>>> On January 2, 2026 5:07 pm, Samuel Rufinatscha wrote:
>>>> PDM depends on the shared proxmox/proxmox-access-control crate for
>>>> token.shadow handling, which expects the product to provide a
>>>> cross-process invalidation signal so it can safely cache verified API
>>>> token secrets and invalidate them when token.shadow is changed.
>>>>
>>>> This patch
>>>>
>>>> * adds a token_shadow_generation to PDM’s shared-memory
>>>> ConfigVersionCache
>>>> * implements proxmox_access_control::init::AccessControlConfig
>>>> for pdm_config::AccessControlConfig, which
>>>> - delegates roles/privs/path checks to the existing
>>>> pdm_api_types::AccessControlConfig implementation
>>>> - implements the shadow cache generation trait functions
>>>> * switches the AccessControlConfig init paths (server + CLI) to use
>>>> pdm_config::AccessControlConfig instead of
>>>> pdm_api_types::AccessControlConfig
>>>>
>>>> This patch is part of the series which fixes bug #7017 [1].
>>>>
>>>> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017
>>>>
>>>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha at proxmox.com>
>>>> ---
>>>> cli/admin/src/main.rs | 2 +-
>>>> lib/pdm-config/Cargo.toml | 1 +
>>>> lib/pdm-config/src/access_control_config.rs | 73 +++++++++++++++++++++
>>>> lib/pdm-config/src/config_version_cache.rs | 18 +++++
>>>> lib/pdm-config/src/lib.rs | 2 +
>>>> server/src/acl.rs | 3 +-
>>>> 6 files changed, 96 insertions(+), 3 deletions(-)
>>>> create mode 100644 lib/pdm-config/src/access_control_config.rs
>>>>
>>>> diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
>>>> index f698fa2..916c633 100644
>>>> --- a/cli/admin/src/main.rs
>>>> +++ b/cli/admin/src/main.rs
>>>> @@ -19,7 +19,7 @@ fn main() {
>>>> proxmox_product_config::init(api_user, priv_user);
>>>>
>>>> proxmox_access_control::init::init(
>>>> - &pdm_api_types::AccessControlConfig,
>>>> + &pdm_config::AccessControlConfig,
>>>> pdm_buildcfg::configdir!("/access"),
>>>> )
>>>> .expect("failed to setup access control config");
>>>> diff --git a/lib/pdm-config/Cargo.toml b/lib/pdm-config/Cargo.toml
>>>> index d39c2ad..19781d2 100644
>>>> --- a/lib/pdm-config/Cargo.toml
>>>> +++ b/lib/pdm-config/Cargo.toml
>>>> @@ -13,6 +13,7 @@ once_cell.workspace = true
>>>> openssl.workspace = true
>>>> serde.workspace = true
>>>>
>>>> +proxmox-access-control.workspace = true
>>>> proxmox-config-digest = { workspace = true, features = [ "openssl" ] }
>>>> proxmox-http = { workspace = true, features = [ "http-helpers" ] }
>>>> proxmox-ldap = { workspace = true, features = [ "types" ]}
>>>> diff --git a/lib/pdm-config/src/access_control_config.rs b/lib/pdm-config/src/access_control_config.rs
>>>> new file mode 100644
>>>> index 0000000..6f2e6b3
>>>> --- /dev/null
>>>> +++ b/lib/pdm-config/src/access_control_config.rs
>>>> @@ -0,0 +1,73 @@
>>>> +// e.g. in src/main.rs or server::context mod, wherever convenient
>>>> +
>>>> +use anyhow::Error;
>>>> +use pdm_api_types::{Authid, Userid};
>>>> +use proxmox_section_config::SectionConfigData;
>>>> +use std::collections::HashMap;
>>>> +
>>>> +pub struct AccessControlConfig;
>>>> +
>>>> +impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
>>>
>>> should we then remove the impl from the api type?
>>>
>>
>> Thanks for pointing this out Fabian! Currently, /ui/src/main.rs still
>> makes use of pdm_api_types::AccessControlConfig. This looks like a WASM
>> module, and is based on ticket based auth
>> (proxmox_login::Authentication) as far as I can see. Do you maybe know
>> if it actually requires the token cache / can work with CVC? If it does
>> not, then I think we should keep the API impl. I left this unchanged
>> and only touched server and CLI call sites.
>
> i mostly exposed that there to get access to the privileges, roles, and
> is_superuser functions. they are needed in the ui to selectively render
> ui elements depending on a users privileges.
>
> this should probably be factored out though and shared differently if we
> want to extend this trait with more caching functions.
>
Good point.
>>>> + fn privileges(&self) -> &HashMap<&str, u64> {
>>>> + pdm_api_types::AccessControlConfig.privileges()
>>>> + }
>>>> +
>>>> + fn roles(&self) -> &HashMap<&str, (u64, &str)> {
>>>> + pdm_api_types::AccessControlConfig.roles()
>>>> + }
>>>> +
>>>> + fn is_superuser(&self, auth_id: &Authid) -> bool {
>>>> + pdm_api_types::AccessControlConfig.is_superuser(auth_id)
>>>> + }
>>>> +
>>>> + fn is_group_member(&self, user_id: &Userid, group: &str) -> bool {
>>>> + pdm_api_types::AccessControlConfig.is_group_member(user_id, group)
>>>> + }
>>>> +
>>>> + fn role_admin(&self) -> Option<&str> {
>>>> + pdm_api_types::AccessControlConfig.role_admin()
>>>> + }
>>>> +
>>>> + fn role_no_access(&self) -> Option<&str> {
>>>> + pdm_api_types::AccessControlConfig.role_no_access()
>>>> + }
>>>> +
>>>> + fn init_user_config(&self, config: &mut SectionConfigData) -> Result<(), Error> {
>>>> + pdm_api_types::AccessControlConfig.init_user_config(config)
>>>> + }
>>>> +
>>>> + fn acl_audit_privileges(&self) -> u64 {
>>>> + pdm_api_types::AccessControlConfig.acl_audit_privileges()
>>>> + }
>>>> +
>>>> + fn acl_modify_privileges(&self) -> u64 {
>>>> + pdm_api_types::AccessControlConfig.acl_modify_privileges()
>>>> + }
>>>> +
>>>> + fn check_acl_path(&self, path: &str) -> Result<(), Error> {
>>>> + pdm_api_types::AccessControlConfig.check_acl_path(path)
>>>> + }
>>>> +
>>>> + fn allow_partial_permission_match(&self) -> bool {
>>>> + pdm_api_types::AccessControlConfig.allow_partial_permission_match()
>>>> + }
>>>> +
>>>> + fn cache_generation(&self) -> Option<usize> {
>>>> + pdm_api_types::AccessControlConfig.cache_generation()
>>>> + }
>>>
>>> shouldn't this be wired up to the ConfigVersionCache?
>>>
>>
>> If I understand correctly, cache_generation() and the
>> increment_cache_generation() below do not appear to have been wired
>> so far, meaning that caches were not enabled. To enable them,
>> a PDM AccessControlConfig implementation would probably be required
>> (as suggested in this patch) in order to be able integrate with
>> ConfigVersionCache.
>>
>> I think these two functions should be checked, if we want to enabled
>> them or not, probably best as part of a dedicated scope? I can create a
>> bug report for this.
>>
>
> sure, i think it's not too much effort, though. if you split out the
> caching parts, the ui should be fine without them. it really has no need
> for them afair.
If the UI doesnt make use of it maybe it would be simply best to keep
two different impls? One to keep it minimal, also since not all parts
might be WASM compatible, and one impl as proposed to wire-up CVC (and
maybe other things in the future..).
And will wire CVC for the other two existing caching functions as part
of this series.
>
>>>> +
>>>> + fn increment_cache_generation(&self) -> Result<(), Error> {
>>>> + pdm_api_types::AccessControlConfig.increment_cache_generation()
>>>
>>> shouldn't this be wired up to the ConfigVersionCache?
>>>
>>>> + }
>>>> +
>>>> + fn token_shadow_cache_generation(&self) -> Option<usize> {
>>>> + crate::ConfigVersionCache::new()
>>>> + .ok()
>>>> + .map(|c| c.token_shadow_generation())
>>>> + }
>>>> +
>>>> + fn increment_token_shadow_cache_generation(&self) -> Result<usize, Error> {
>>>> + let c = crate::ConfigVersionCache::new()?;
>>>> + Ok(c.increase_token_shadow_generation())
>>>> + }
>>>> +}
>>>> diff --git a/lib/pdm-config/src/config_version_cache.rs b/lib/pdm-config/src/config_version_cache.rs
>>>> index 36a6a77..933140c 100644
>>>> --- a/lib/pdm-config/src/config_version_cache.rs
>>>> +++ b/lib/pdm-config/src/config_version_cache.rs
>>>> @@ -27,6 +27,8 @@ struct ConfigVersionCacheDataInner {
>>>> traffic_control_generation: AtomicUsize,
>>>> // Tracks updates to the remote/hostname/nodename mapping cache.
>>>> remote_mapping_cache: AtomicUsize,
>>>> + // Token shadow (token.shadow) generation/version.
>>>> + token_shadow_generation: AtomicUsize,
>>>
>>> explanation why this is safe for the commit message would be nice ;)
>>>
>>
>> Will add :)
>>
>>>> // Add further atomics here
>>>> }
>>>>
>>>> @@ -172,4 +174,20 @@ impl ConfigVersionCache {
>>>> .fetch_add(1, Ordering::Relaxed)
>>>> + 1
>>>> }
>>>> +
>>>> + /// Returns the token shadow generation number.
>>>> + pub fn token_shadow_generation(&self) -> usize {
>>>> + self.shmem
>>>> + .data()
>>>> + .token_shadow_generation
>>>> + .load(Ordering::Acquire)
>>>> + }
>>>> +
>>>> + /// Increase the token shadow generation number.
>>>> + pub fn increase_token_shadow_generation(&self) -> usize {
>>>> + self.shmem
>>>> + .data()
>>>> + .token_shadow_generation
>>>> + .fetch_add(1, Ordering::AcqRel)
>>>> + }
>>>> }
>>>> diff --git a/lib/pdm-config/src/lib.rs b/lib/pdm-config/src/lib.rs
>>>> index 4c49054..a15a006 100644
>>>> --- a/lib/pdm-config/src/lib.rs
>>>> +++ b/lib/pdm-config/src/lib.rs
>>>> @@ -9,6 +9,8 @@ pub mod remotes;
>>>> pub mod setup;
>>>> pub mod views;
>>>>
>>>> +mod access_control_config;
>>>> +pub use access_control_config::AccessControlConfig;
>>>> mod config_version_cache;
>>>> pub use config_version_cache::ConfigVersionCache;
>>>>
>>>> diff --git a/server/src/acl.rs b/server/src/acl.rs
>>>> index f421814..e6e007b 100644
>>>> --- a/server/src/acl.rs
>>>> +++ b/server/src/acl.rs
>>>> @@ -1,6 +1,5 @@
>>>> pub(crate) fn init() {
>>>> - static ACCESS_CONTROL_CONFIG: pdm_api_types::AccessControlConfig =
>>>> - pdm_api_types::AccessControlConfig;
>>>> + static ACCESS_CONTROL_CONFIG: pdm_config::AccessControlConfig = pdm_config::AccessControlConfig;
>>>>
>>>> proxmox_access_control::init::init(&ACCESS_CONTROL_CONFIG, pdm_buildcfg::configdir!("/access"))
>>>> .expect("failed to setup access control config");
>>>> --
>>>> 2.47.3
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> pbs-devel mailing list
>>>> pbs-devel at lists.proxmox.com
>>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>>>
>>>
>>>
>>> _______________________________________________
>>> pbs-devel mailing list
>>> pbs-devel at lists.proxmox.com
>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
More information about the pbs-devel
mailing list