Re: [PATCH datacenter-manager v3 1/6] server: pbs-client: add delete admin token method

["Lukas Wagner" <l.wagner@proxmox.com>]

Hi Shan,

thanks for the patch!

On Wed Feb 11, 2026 at 4:20 PM CET, Shan Shaji wrote:
> Inorder to allow deleting the generated token of PBS from PDM, add
> method inside the PbsClient to delete the admin token.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>
>  changes since v2: No changes
>  changes since v1:
>  - Revert the dropping of '/' from the comment.
>  - Remove unnecessary colon.
>  
>  server/src/pbs_client.rs | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/server/src/pbs_client.rs b/server/src/pbs_client.rs
> index f4f1f82..31befdd 100644
> --- a/server/src/pbs_client.rs
> +++ b/server/src/pbs_client.rs
> @@ -260,6 +260,13 @@ impl PbsClient {
>          Ok(token)
>      }
>  
> +    /// Delete API-Token from the PBS remote.

tiny nit: I'd spell it 'API token'

> +    pub async fn delete_admin_token(&self, userid: &Userid, tokenid: &str) -> Result<(), Error> {

This function should take &TokenameRef, I think. After all, it's what
you have at the callsite before you you turn it into a &str.

Also, I think the function should just be called `delete_token`, since
nothing of it is specific to the token being an admin token (it can
delete *any* token, not just an admin one). That being said, I
understand why you named it this way, it's the symmetry with
`create_admin_token`. The difference is that `create_admin_token` does
indeed create a new token *and* also assigns admin permissions to it.

> +        let path = format!("/api2/extjs/access/users/{userid}/token/{tokenid}");

Technically, the userid parameter should be percent-encoded - the @ sign
in "root@pam" would usually be reserved to separate credentials and the
host in a URL, see [1]. Apparently its not an issue with the
`create_admin_token` function so far, but still a good practice, even if
we don't really have issues with it now.

[1] https://docs.mapp.com/docs/url-encoding-and-what-characters-are-valid-in-a-uri

> +        self.0.delete(&path).await?.nodata()?;
> +        Ok(())
> +    }
> +
>      /// Return the status the Proxmox Backup Server instance
>      pub async fn node_status(&self) -> Result<pbs_api_types::NodeStatus, Error> {
>          let path = "/api2/extjs/nodes/localhost/status";