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

[pbs-devel] [PATCH proxmox-backup v3 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.

v3, thanks @Chris:
- fix problem in where user/acl config wasn't saved on user deletion

v2:
- rebase onto master

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

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        | 83 ++++++++++++++++++++++++++++------
 www/config/TokenView.js        | 29 ++++++++++++
 4 files changed, 107 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 v3 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 v3 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 v3 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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index bba5bb2c0..fd3092244 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,22 @@ 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)?;
+        }
+    }
+
+    pbs_config::user::save_config(&config)?;
+    pbs_config::acl::save_config(&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

[pbs-devel] [PATCH proxmox-backup v3 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 fd3092244..a8dd4c0d0 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};
 
@@ -561,6 +562,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,
@@ -574,6 +579,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(&[
@@ -589,9 +604,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()?;
@@ -631,11 +647,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 v3 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

[pbs-devel] applied-series: [PATCH proxmox-backup v3 0/5] ACL removal on user/token deletion + token regeneration

[Thomas Lamprecht]

Am 04.04.25 um 17:32 schrieb 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.
> 
> v3, thanks @Chris:
> - fix problem in where user/acl config wasn't saved on user deletion
> 
> v2:
> - rebase onto master
> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4382
> [2] https://bugzilla.proxmox.com/show_bug.cgi?id=3887
> 
> 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        | 83 ++++++++++++++++++++++++++++------
>  www/config/TokenView.js        | 29 ++++++++++++
>  4 files changed, 107 insertions(+), 16 deletions(-)
> 


applied series, re-ordered the regenerate button in the top-bar to
be near other buttons that change state and added a separator to
add some more visual distinction, thanks!


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