Re: [PATCH proxmox 9/9] access-control: enforce token policy on token create and update

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

On Wed Sep 23, 2026 at 10:59 PM CEST, Thomas Lamprecht wrote:
> An update only checks the expiration date if its value actually
> changes, so unrelated edits or clients resubmitting unchanged fields
> are not rejected for tokens that predate the policy. The check runs
> before the secret is generated, or rotated on update, to avoid
> leaving an orphaned entry in the token shadow file, or losing the
> old secret, for a rejected request.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>  proxmox-access-control/src/api/tokens.rs | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/proxmox-access-control/src/api/tokens.rs b/proxmox-access-control/src/api/tokens.rs
> index f7417934..58295793 100644
> --- a/proxmox-access-control/src/api/tokens.rs
> +++ b/proxmox-access-control/src/api/tokens.rs
> @@ -97,6 +97,12 @@ pub fn generate_token(
>          );
>      }
>
> +    // check the policy before generating the secret to avoid leaving an orphaned entry in the
> +    // token shadow file on rejection
> +    if let Some(policy) = crate::init::access_conf().token_policy()? {
> +        policy.check_expiry_change(expire.unwrap_or(0), false, proxmox_time::epoch_i64())?;
> +    }
> +
>      let secret = token_shadow::generate_and_set_secret(&tokenid)?;
>
>      let token = ApiToken {
> @@ -180,6 +186,7 @@ pub fn update_token(
>      let tokenid_string = tokenid.to_string();
>
>      let mut data: ApiToken = config.lookup("token", &tokenid_string)?;
> +    let old_expire = data.expire.unwrap_or(0);
>
>      if let Some(delete) = delete {
>          for delete_prop in delete {
> @@ -206,6 +213,16 @@ pub fn update_token(
>          data.expire = if expire > 0 { Some(expire) } else { None };
>      }
>
> +    // enforce the policy only if this update actually changes the expiration date, so unrelated
> +    // edits do not retroactively reject tokens that predate the policy; check before a requested
> +    // secret regeneration so a rejected update does not rotate the secret either
> +    let new_expire = data.expire.unwrap_or(0);
> +    if new_expire != old_expire {
> +        if let Some(policy) = crate::init::access_conf().token_policy()? {
> +            policy.check_expiry_change(new_expire, true, proxmox_time::epoch_i64())?;
> +        }
> +    }

nit: moving this check in the previous if-let clause that checks whether
the expiry is to be updated would make it a bit cleaner imo.

note that there is no way besides setting `expire = 0` to remove an
expiry.

> +
>      let new_secret = if regenerate == Some(true) {
>          let secret = token_shadow::generate_and_set_secret(&tokenid)?;
>          Some(ApiTokenSecret { tokenid, secret })