[pbs-devel] [PATCH proxmox-backup v2 0/5] ACL removal on user/token deletion + token regeneration

[pbs-devel] [PATCH proxmox-backup v2 0/5] ACL removal on user/token deletion + token regeneration

[Hannes Laimer]

If a user is deleted, all its permissions and tokens
will now be deleted with it. If a token is deleted
all its permissions will now be deleted.
Until now neither of those two happened[1].
The last two commits add the possibility to regenerate
tokens, basically revoking the old and generating a
new secret while keeping all the set permissions.

This is all in the same series since just adding the
removal of permissions would kill the currently only
way to keep the permissions but change the secret of
a token(deleting it and creating it again with the
same name[2]).

-> pbs-api-types dep has to be bumped since we need the schema added in #4
for #5.

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4382
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=3887

v2:
- rebase onto master

Hannes Laimer (5):
  pbs-config: move secret generation into token_shadow
  fix #4382: api: access: remove permissions of token on deletion
  fix #4382: api: remove permissions and tokens of user on deletion
  fix #3887: api: access: allow secret regeneration
  fix #3887: ui: add regenerate token button

 pbs-config/Cargo.toml          |  1 +
 pbs-config/src/token_shadow.rs | 10 ++++-
 src/api2/access/user.rs        | 80 +++++++++++++++++++++++++++-------
 www/config/TokenView.js        | 29 ++++++++++++
 4 files changed, 104 insertions(+), 16 deletions(-)

-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v2 1/5] pbs-config: move secret generation into token_shadow

[Hannes Laimer]

so we have only one place where we generate secrets.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-config/Cargo.toml          |  1 +
 pbs-config/src/token_shadow.rs | 10 +++++++++-
 src/api2/access/user.rs        |  3 +--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml
index 12d0eb3da..284149658 100644
--- a/pbs-config/Cargo.toml
+++ b/pbs-config/Cargo.toml
@@ -24,6 +24,7 @@ proxmox-section-config.workspace = true
 proxmox-shared-memory.workspace = true
 proxmox-sys = { workspace = true, features = [ "acl", "crypt", "timer" ] }
 proxmox-time.workspace = true
+proxmox-uuid.workspace = true
 
 pbs-api-types.workspace = true
 pbs-buildcfg.workspace = true
diff --git a/pbs-config/src/token_shadow.rs b/pbs-config/src/token_shadow.rs
index 2efb187ef..640fabbfe 100644
--- a/pbs-config/src/token_shadow.rs
+++ b/pbs-config/src/token_shadow.rs
@@ -61,8 +61,16 @@ pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
     }
 }
 
+/// Generates a new secret for the given tokenid / API token, sets it then returns it.
+/// The secret is stored as salted hash.
+pub fn generate_and_set_secret(tokenid: &Authid) -> Result<String, Error> {
+    let secret = format!("{:x}", proxmox_uuid::Uuid::generate());
+    set_secret(tokenid, &secret)?;
+    Ok(secret)
+}
+
 /// Adds a new entry for the given tokenid / API token secret. The secret is stored as salted hash.
-pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
+fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> {
     if !tokenid.is_token() {
         bail!("not an API token ID");
     }
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 031f84caa..8a5afb7b0 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -495,8 +495,7 @@ pub fn generate_token(
         );
     }
 
-    let secret = format!("{:x}", proxmox_uuid::Uuid::generate());
-    token_shadow::set_secret(&tokenid, &secret)?;
+    let secret = token_shadow::generate_and_set_secret(&tokenid)?;
 
     let token = ApiToken {
         tokenid,
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v2 2/5] fix #4382: api: access: remove permissions of token on deletion

[Hannes Laimer]

... and move token deletion into new `do_delete_token` function.
Since it'll be resued later on user deletion.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/access/user.rs | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 8a5afb7b0..bba5bb2c0 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -8,6 +8,7 @@ use std::collections::HashMap;
 
 use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment, SubdirMap};
 use proxmox_schema::api;
+use proxmox_section_config::SectionConfigData;
 use proxmox_tfa::api::TfaConfig;
 
 use pbs_api_types::{
@@ -15,9 +16,7 @@ use pbs_api_types::{
     EXPIRE_USER_SCHEMA, PASSWORD_FORMAT, PBS_PASSWORD_SCHEMA, PRIV_PERMISSIONS_MODIFY,
     PRIV_SYS_AUDIT, PROXMOX_CONFIG_DIGEST_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA,
 };
-use pbs_config::token_shadow;
-
-use pbs_config::CachedUserInfo;
+use pbs_config::{acl::AclTree, token_shadow, CachedUserInfo};
 
 fn new_user_with_tokens(user: User, tfa: &TfaConfig) -> UserWithTokens {
     UserWithTokens {
@@ -651,29 +650,41 @@ pub fn delete_token(
     token_name: Tokenname,
     digest: Option<String>,
 ) -> Result<(), Error> {
-    let _lock = pbs_config::user::lock_config()?;
+    let _acl_lock = pbs_config::acl::lock_config()?;
+    let _user_lock = pbs_config::user::lock_config()?;
 
-    let (mut config, expected_digest) = pbs_config::user::config()?;
+    let (mut user_config, expected_digest) = pbs_config::user::config()?;
 
     if let Some(ref digest) = digest {
         let digest = <[u8; 32]>::from_hex(digest)?;
         crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
     }
 
+    let (mut acl_config, _digest) = pbs_config::acl::config()?;
+    do_delete_token(token_name, &userid, &mut user_config, &mut acl_config)?;
+
+    pbs_config::user::save_config(&user_config)?;
+    pbs_config::acl::save_config(&acl_config)?;
+    Ok(())
+}
+
+fn do_delete_token(
+    token_name: Tokenname,
+    userid: &Userid,
+    user_config: &mut SectionConfigData,
+    acl_config: &mut AclTree,
+) -> Result<(), Error> {
     let tokenid = Authid::from((userid.clone(), Some(token_name.clone())));
     let tokenid_string = tokenid.to_string();
-
-    if config.sections.remove(&tokenid_string).is_none() {
+    if user_config.sections.remove(&tokenid_string).is_none() {
         bail!(
             "token '{}' of user '{}' does not exist.",
             token_name.as_str(),
             userid
         );
     }
-
     token_shadow::delete_secret(&tokenid)?;
-
-    pbs_config::user::save_config(&config)?;
+    acl_config.delete_authid(&tokenid);
 
     Ok(())
 }
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v2 3/5] fix #4382: api: remove permissions and tokens of user on deletion

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/access/user.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index bba5bb2c0..a51ee8f3f 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -353,6 +353,7 @@ pub async fn update_user(
 pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> {
     let _lock = pbs_config::user::lock_config()?;
     let _tfa_lock = crate::config::tfa::write_lock()?;
+    let _acl_lock = pbs_config::acl::lock_config()?;
 
     let (mut config, expected_digest) = pbs_config::user::config()?;
 
@@ -380,6 +381,19 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error>
         eprintln!("error updating TFA config after deleting user {userid:?} {err}",);
     }
 
+    let user_tokens: Vec<ApiToken> = config
+        .convert_to_typed_array::<ApiToken>("token")?
+        .into_iter()
+        .filter(|token| token.tokenid.user().eq(&userid))
+        .collect();
+
+    let (mut acl_tree, _digest) = pbs_config::acl::config()?;
+    for token in user_tokens {
+        if let Some(name) = token.tokenid.tokenname() {
+            do_delete_token(name.to_owned(), &userid, &mut config, &mut acl_tree)?;
+        }
+    }
+
     Ok(())
 }
 
-- 
2.39.5



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

Re: [pbs-devel] [PATCH proxmox-backup v2 3/5] fix #4382: api: remove permissions and tokens of user on deletion

[Christian Ebner]

On 4/4/25 17:10, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/api2/access/user.rs | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
> index bba5bb2c0..a51ee8f3f 100644
> --- a/src/api2/access/user.rs
> +++ b/src/api2/access/user.rs
> @@ -353,6 +353,7 @@ pub async fn update_user(
>   pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> {
>       let _lock = pbs_config::user::lock_config()?;
>       let _tfa_lock = crate::config::tfa::write_lock()?;
> +    let _acl_lock = pbs_config::acl::lock_config()?;
>   
>       let (mut config, expected_digest) = pbs_config::user::config()?;
>   
> @@ -380,6 +381,19 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error>
>           eprintln!("error updating TFA config after deleting user {userid:?} {err}",);
>       }
>   
> +    let user_tokens: Vec<ApiToken> = config
> +        .convert_to_typed_array::<ApiToken>("token")?
> +        .into_iter()
> +        .filter(|token| token.tokenid.user().eq(&userid))
> +        .collect();
> +
> +    let (mut acl_tree, _digest) = pbs_config::acl::config()?;
> +    for token in user_tokens {
> +        if let Some(name) = token.tokenid.tokenname() {
> +            do_delete_token(name.to_owned(), &userid, &mut config, &mut acl_tree)?;
> +        }
> +    }

Unfortunately you were faster with the rebase an probably missed my 
previous comment. Referencing it for this series, so it does get 
considered: 
https://lore.proxmox.com/pbs-devel/20fe5ca6-889d-4930-ae18-457bc3abf5a5@proxmox.com/

>       Ok(())
>   }
>   



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

[pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3887: api: access: allow secret regeneration

[Hannes Laimer]

... through the token PUT endpoint by adding a new `regenerate` bool
parameter.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/access/user.rs | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index a51ee8f3f..478bb799b 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -14,7 +14,8 @@ use proxmox_tfa::api::TfaConfig;
 use pbs_api_types::{
     ApiToken, Authid, Tokenname, User, UserUpdater, UserWithTokens, Userid, ENABLE_USER_SCHEMA,
     EXPIRE_USER_SCHEMA, PASSWORD_FORMAT, PBS_PASSWORD_SCHEMA, PRIV_PERMISSIONS_MODIFY,
-    PRIV_SYS_AUDIT, PROXMOX_CONFIG_DIGEST_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA,
+    PRIV_SYS_AUDIT, PROXMOX_CONFIG_DIGEST_SCHEMA, REGENERATE_TOKEN_SCHEMA,
+    SINGLE_LINE_COMMENT_SCHEMA,
 };
 use pbs_config::{acl::AclTree, token_shadow, CachedUserInfo};
 
@@ -558,6 +559,10 @@ pub enum DeletableTokenProperty {
                 schema: EXPIRE_USER_SCHEMA,
                 optional: true,
             },
+            regenerate: {
+                schema: REGENERATE_TOKEN_SCHEMA,
+                optional: true,
+            },
             delete: {
                 description: "List of properties to delete.",
                 type: Array,
@@ -571,6 +576,16 @@ pub enum DeletableTokenProperty {
                 schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
             },
         },
+    },
+        returns: {
+        description: "Regenerated secret, if regenerate is set.",
+        properties: {
+            secret: {
+                type: String,
+                optional: true,
+                description: "The new API token secret",
+            },
+        },
     },
     access: {
         permission: &Permission::Or(&[
@@ -586,9 +601,10 @@ pub fn update_token(
     comment: Option<String>,
     enable: Option<bool>,
     expire: Option<i64>,
+    regenerate: Option<bool>,
     delete: Option<Vec<DeletableTokenProperty>>,
     digest: Option<String>,
-) -> Result<(), Error> {
+) -> Result<Value, Error> {
     let _lock = pbs_config::user::lock_config()?;
 
     let (mut config, expected_digest) = pbs_config::user::config()?;
@@ -628,11 +644,21 @@ pub fn update_token(
         data.expire = if expire > 0 { Some(expire) } else { None };
     }
 
+    let new_secret = if regenerate.unwrap_or_default() {
+        Some(token_shadow::generate_and_set_secret(&tokenid)?)
+    } else {
+        None
+    };
+
     config.set_data(&tokenid_string, "token", &data)?;
 
     pbs_config::user::save_config(&config)?;
 
-    Ok(())
+    if let Some(secret) = new_secret {
+        Ok(json!({"secret": secret}))
+    } else {
+        Ok(Value::Null)
+    }
 }
 
 #[api(
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3887: ui: add regenerate token button

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/config/TokenView.js | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/www/config/TokenView.js b/www/config/TokenView.js
index 6282a4d44..340698ad4 100644
--- a/www/config/TokenView.js
+++ b/www/config/TokenView.js
@@ -100,6 +100,25 @@ Ext.define('PBS.config.TokenView', {
 	    }).show();
 	},
 
+	regenerateToken: function(_button, _event, record) {
+	    let tokenid = record.data.tokenid;
+	    let user = PBS.Utils.extractTokenUser(tokenid);
+	    let tokenname = PBS.Utils.extractTokenName(tokenid);
+	    Proxmox.Utils.API2Request({
+	        method: 'PUT',
+	        url: `/access/users/${user}/token/${tokenname}`,
+	        params: { 'regenerate': true },
+	        success: function(res, opt) {
+		    Ext.create("PBS.window.TokenShow", {
+			autoShow: true,
+			tokenid: tokenid,
+			secret: res.result.data.secret,
+		    });
+	        },
+	        failure: res => Ext.Msg.alert(gettext("Error"), res.htmlStatus),
+	    });
+	},
+
 	showPermissions: function() {
 	    let me = this;
 	    let view = me.getView();
@@ -174,6 +193,16 @@ Ext.define('PBS.config.TokenView', {
 	    handler: 'showPermissions',
 	    disabled: true,
 	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Regenerate'),
+	    handler: 'regenerateToken',
+	    dangerous: true,
+	    confirmMsg: rec => Ext.String.format(
+		gettext("Regenerate the secret of the API token '{0}'? All current use-sites will loose access!"),
+		rec.data.tokenid,
+	    ),
+	},
     ],
 
     viewConfig: {
-- 
2.39.5



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