Re: [pbs-devel] [PATCH proxmox-datacenter-manager v3 1/2] pdm-config: implement token.shadow generation

["Shannon Sterz" <s.sterz@proxmox.com>]

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@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.

>>> +    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.

>>> +
>>> +    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@lists.proxmox.com
>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel



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