Re: [pdm-devel] [PATCH datacenter-manager 3/3] server: clean up acl tree entries and api tokens when deleting users

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

On Fri Sep 26, 2025 at 11:18 AM CEST, Fabian Grünbichler wrote:
> On September 24, 2025 4:51 pm, Shannon Sterz wrote:
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>  server/src/api/access/users.rs | 39 +++++++++++++++++++++++++++++-----
>>  1 file changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/server/src/api/access/users.rs b/server/src/api/access/users.rs
>> index da598d8..1d1accb 100644
>> --- a/server/src/api/access/users.rs
>> +++ b/server/src/api/access/users.rs
>> @@ -334,20 +334,19 @@ pub fn update_user(
>>  /// Remove a user from the configuration file.
>>  pub fn delete_user(userid: Userid, digest: Option<ConfigDigest>) -> Result<(), Error> {
>>      let _lock = proxmox_access_control::user::lock_config()?;
>> +    let _acl_lock = proxmox_access_control::acl::lock_config()?;
>>      let _tfa_lock = crate::auth::tfa::write_lock()?;
>>
>> -    let (mut config, config_digest) = proxmox_access_control::user::config()?;
>> +    let (mut user_config, config_digest) = proxmox_access_control::user::config()?;
>>      config_digest.detect_modification(digest.as_ref())?;
>>
>> -    match config.sections.get(userid.as_str()) {
>> +    match user_config.sections.get(userid.as_str()) {
>>          Some(_) => {
>> -            config.sections.remove(userid.as_str());
>> +            user_config.sections.remove(userid.as_str());
>>          }
>>          None => bail!("user '{}' does not exist.", userid),
>>      }
>>
>> -    proxmox_access_control::user::save_config(&config)?;
>> -
>>      let authenticator = crate::auth::lookup_authenticator(userid.realm())?;
>>      match authenticator.remove_password(userid.name()) {
>>          Ok(()) => {}
>> @@ -375,6 +374,36 @@ pub fn delete_user(userid: Userid, digest: Option<ConfigDigest>) -> Result<(), E
>>          }
>>      }
>>
>> +    let user_tokens: Vec<ApiToken> = user_config
>> +        .convert_to_typed_array::<ApiToken>("token")?
>> +        .into_iter()
>> +        .filter(|token| token.tokenid.user().eq(&userid))
>> +        .collect();
>
> do we have any consistency checks between ACLs and users/tokens? if not,
> then..
>
>> +
>> +    let (mut acl_config, _digest) = proxmox_access_control::acl::config()?;
>> +
>> +    let auth_id = userid.clone().into();
>> +    acl_config.delete_authid(&auth_id);
>> +
>> +    for token in user_tokens {
>> +        if let Some(token_name) = token.tokenid.tokenname() {
>> +            let tokenid = Authid::from((userid.clone(), Some(token_name.to_owned())));
>> +            let tokenid_string = tokenid.to_string();
>> +            if user_config.sections.remove(&tokenid_string).is_none() {
>> +                bail!(
>> +                    "token '{}' of user '{userid}' does not exist.",
>> +                    token_name.as_str()
>> +                );
>> +            }
>> +
>> +            proxmox_access_control::token_shadow::delete_secret(&tokenid)?;
>> +            acl_config.delete_authid(&tokenid);
>
> this is not enough to remove all ACLs, since removing a token via the
> token API currently does not clean up its ACL entries..

i added the clean up for tokens in a v2.

but im not entirely sure if i understand you here correctly. are you
worried about token acls not getting cleaned up if only the token is
deleted? because then yeah, that part was missing as you pointed out
there correctly. or is there another scenario you have in mind that i
currently don't see?

>> +        }
>> +    }
>> +
>> +    proxmox_access_control::user::save_config(&user_config)?;
>> +    proxmox_access_control::acl::save_config(&acl_config)?;
>> +
>>      Ok(())
>>  }
>>
>> --
>> 2.47.3
>>
>>
>>
>> _______________________________________________
>> pdm-devel mailing list
>> pdm-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
>>
>>
>>
>
>
> _______________________________________________
> pdm-devel mailing list
> pdm-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel



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