[pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp 0/8] add better token support for pdm

[pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp 0/8] add better token support for pdm

[Shannon Sterz]

this series aims to add a ui to the pre-existing token support in pdm.
it also aims to get it more in-line with what other proxmox products
provide in terms of functionality.

the first two patches prepaer proxmox-access-control by refactoring it a
little bit. the next commit adds api endpoints for handling tokens.
these api endpoints are intentionally not providing any access control
permissions, so that users of this crate can set them themselves.

the next two commits improve the token panel in proxmox-yew-comp by
using the Clipboard API when copying values. the layout and
functionality of the dialog displaying the token secret is also
improved.

the final three patches integrate the token panel in pdm's ui. they also
refactor the api endpoints related token handling to use the new
endpoints from proxmxo-access-control and make sure the user delete
endpoint cleans up acls and tokens too.

Changelog
---------

changes since rfc:
- the commits implementing the basic token panel have already been
  applied by Thomas Lamprecht, thanks!
- moved adding `use` and `mod` statements for the token module to the
  right commit in the series (thanks @ Dominik Csapak)
- generate token secrets in the `token_shadow` module instead of in the
  token endpoints themselves (thanks @ Fabian Grünbichler)
- use a schema for the `regenerate` parameter of the update token
  endpoint (thanks @ Fabian Grünbichler)
- allow deleting comments via a `delete` property (thanks @ Fabian
  Grünbichler)
- make the token delete endpoint clean up token acls (thanks @ Fabian
  Grünbichler)
- improve copy to clipboard functionality to use the new Clipboard API
- improve the layout of the token secret dialog (thanks @ Thomas
  Lamprecht)


proxmox:

Shannon Sterz (3):
  access-control: refactor api module to be more hirachical
  access-control: move `ApiTokenSecret` to types module
  access-control: add api endpoints for handling tokens

 proxmox-access-control/Cargo.toml             |   1 +
 .../src/{api.rs => api/acl.rs}                |   0
 proxmox-access-control/src/api/mod.rs         |   8 +
 proxmox-access-control/src/api/tokens.rs      | 310 ++++++++++++++++++
 proxmox-access-control/src/token_shadow.rs    |  17 +-
 proxmox-access-control/src/types.rs           |  43 ++-
 6 files changed, 369 insertions(+), 10 deletions(-)
 rename proxmox-access-control/src/{api.rs => api/acl.rs} (100%)
 create mode 100644 proxmox-access-control/src/api/mod.rs
 create mode 100644 proxmox-access-control/src/api/tokens.rs


proxmox-yew-comp:

Shannon Sterz (2):
  utils/tfa add recover/token panel: add copy_text_to_clipboard function
  token panel: improve token secret dialog layout and hide password

 Cargo.toml                  |   2 +
 src/tfa/tfa_add_recovery.rs |  17 ++----
 src/token_panel.rs          | 117 ++++++++++++++++++------------------
 src/utils.rs                |  22 +++++++
 4 files changed, 89 insertions(+), 69 deletions(-)


proxmox-datacenter-manager:

Shannon Sterz (3):
  ui: add a token panel and a token acl edit menu in the permissions
    panel
  server: access: use token endpoints from proxmox-access-control
  server: clean up acl tree entries and api tokens when deleting users

 server/src/api/access/users.rs | 388 ++++++---------------------------
 ui/src/configuration/mod.rs    |  33 ++-
 2 files changed, 95 insertions(+), 326 deletions(-)


Summary over all repositories:
  12 files changed, 553 insertions(+), 405 deletions(-)

--
Generated by git-murpp 0.8.1


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

[pdm-devel] [PATCH proxmox 1/3] access-control: refactor api module to be more hirachical

[Shannon Sterz]

this is mainly in preparation of factoring out more api endpoints
related to access control. the refactoring is done in a way where
users should not need to adapt.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access-control/src/{api.rs => api/acl.rs} | 0
 proxmox-access-control/src/api/mod.rs             | 2 ++
 2 files changed, 2 insertions(+)
 rename proxmox-access-control/src/{api.rs => api/acl.rs} (100%)
 create mode 100644 proxmox-access-control/src/api/mod.rs

diff --git a/proxmox-access-control/src/api.rs b/proxmox-access-control/src/api/acl.rs
similarity index 100%
rename from proxmox-access-control/src/api.rs
rename to proxmox-access-control/src/api/acl.rs
diff --git a/proxmox-access-control/src/api/mod.rs b/proxmox-access-control/src/api/mod.rs
new file mode 100644
index 00000000..5e17dc76
--- /dev/null
+++ b/proxmox-access-control/src/api/mod.rs
@@ -0,0 +1,2 @@
+mod acl;
+pub use acl::{ACL_ROUTER, API_METHOD_READ_ACL, API_METHOD_UPDATE_ACL, ROLE_ROUTER};
-- 
2.47.3



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

[pdm-devel] [PATCH proxmox 2/3] access-control: move `ApiTokenSecret` to types module

[Shannon Sterz]

this is technically a breaking change, but so far this type has no
users

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access-control/src/token_shadow.rs |  9 ---------
 proxmox-access-control/src/types.rs        | 10 ++++++++++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs
index 60b71ac9..46397edb 100644
--- a/proxmox-access-control/src/token_shadow.rs
+++ b/proxmox-access-control/src/token_shadow.rs
@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 
 use anyhow::{bail, format_err, Error};
-use serde::{Deserialize, Serialize};
 use serde_json::{from_value, Value};
 
 use proxmox_auth_api::types::Authid;
@@ -9,14 +8,6 @@ use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard};
 
 use crate::init::{token_shadow, token_shadow_lock};
 
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// ApiToken id / secret pair
-pub struct ApiTokenSecret {
-    pub tokenid: Authid,
-    pub secret: String,
-}
-
 // Get exclusive lock
 fn lock_config() -> Result<ApiLockGuard, Error> {
     open_api_lockfile(token_shadow_lock(), None, true)
diff --git a/proxmox-access-control/src/types.rs b/proxmox-access-control/src/types.rs
index ea64d333..9c539f7b 100644
--- a/proxmox-access-control/src/types.rs
+++ b/proxmox-access-control/src/types.rs
@@ -147,6 +147,16 @@ impl ApiToken {
     }
 }
 
+#[api]
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// ApiToken id / secret pair
+pub struct ApiTokenSecret {
+    pub tokenid: Authid,
+    /// The secret associated with the token.
+    pub secret: String,
+}
+
 #[api(
     properties: {
         userid: {
-- 
2.47.3



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

[pdm-devel] [PATCH proxmox 3/3] access-control: add api endpoints for handling tokens

[Shannon Sterz]

by adding most of the logic of token creation, updating and deleting
here, users can more easily add api tokens to their api. this requires
the api feature.

users of these api endpoints are expected to set the `access`
parameters according to their needs. by default only super users can
access any of them.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-access-control/Cargo.toml          |   1 +
 proxmox-access-control/src/api/mod.rs      |   6 +
 proxmox-access-control/src/api/tokens.rs   | 310 +++++++++++++++++++++
 proxmox-access-control/src/token_shadow.rs |   8 +
 proxmox-access-control/src/types.rs        |  33 ++-
 5 files changed, 357 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-access-control/src/api/tokens.rs

diff --git a/proxmox-access-control/Cargo.toml b/proxmox-access-control/Cargo.toml
index dbcbeced..2c4e0628 100644
--- a/proxmox-access-control/Cargo.toml
+++ b/proxmox-access-control/Cargo.toml
@@ -31,6 +31,7 @@ proxmox-section-config = { workspace = true, optional = true }
 proxmox-shared-memory = { workspace = true, optional = true }
 proxmox-sys = { workspace = true, features = [ "crypt" ], optional = true }
 proxmox-time = { workspace = true }
+proxmox-uuid = { workspace = true }
 
 [features]
 default = []
diff --git a/proxmox-access-control/src/api/mod.rs b/proxmox-access-control/src/api/mod.rs
index 5e17dc76..59dc32e2 100644
--- a/proxmox-access-control/src/api/mod.rs
+++ b/proxmox-access-control/src/api/mod.rs
@@ -1,2 +1,8 @@
+mod tokens;
+pub use tokens::{
+    API_METHOD_DELETE_TOKEN, API_METHOD_GENERATE_TOKEN, API_METHOD_LIST_TOKENS,
+    API_METHOD_READ_TOKEN, API_METHOD_UPDATE_TOKEN,
+};
+
 mod acl;
 pub use acl::{ACL_ROUTER, API_METHOD_READ_ACL, API_METHOD_UPDATE_ACL, ROLE_ROUTER};
diff --git a/proxmox-access-control/src/api/tokens.rs b/proxmox-access-control/src/api/tokens.rs
new file mode 100644
index 00000000..e86b26b7
--- /dev/null
+++ b/proxmox-access-control/src/api/tokens.rs
@@ -0,0 +1,310 @@
+//! User Management
+
+use anyhow::{bail, Error};
+use proxmox_config_digest::ConfigDigest;
+use proxmox_schema::api_types::COMMENT_SCHEMA;
+
+use proxmox_auth_api::types::{Authid, Tokenname, Userid};
+use proxmox_router::{ApiMethod, RpcEnvironment};
+use proxmox_schema::api;
+
+use crate::acl;
+use crate::token_shadow::{self};
+use crate::types::{
+    ApiToken, ApiTokenSecret, DeletableTokenProperty, TokenApiEntry, ENABLE_USER_SCHEMA,
+    EXPIRE_USER_SCHEMA, REGENERATE_TOKEN_SCHEMA,
+};
+
+#[api(
+    input: {
+        properties: {
+            userid: {
+                type: Userid,
+            },
+            "token-name": {
+                type: Tokenname,
+            },
+        },
+    },
+    returns: { type: ApiToken },
+)]
+/// Read user's API token metadata
+pub fn read_token(
+    userid: Userid,
+    token_name: Tokenname,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<ApiToken, Error> {
+    let (config, digest) = crate::user::config()?;
+
+    let tokenid = Authid::from((userid, Some(token_name)));
+
+    rpcenv["digest"] = hex::encode(digest).into();
+    config.lookup("token", &tokenid.to_string())
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: {
+                type: Userid,
+            },
+            "token-name": {
+                type: Tokenname,
+            },
+            comment: {
+                optional: true,
+                schema: COMMENT_SCHEMA,
+            },
+            enable: {
+                schema: ENABLE_USER_SCHEMA,
+                optional: true,
+            },
+            expire: {
+                schema: EXPIRE_USER_SCHEMA,
+                optional: true,
+            },
+            digest: {
+                optional: true,
+                type: ConfigDigest,
+            },
+        },
+    },
+    returns: { type: ApiTokenSecret },
+)]
+/// Generate a new API token with given metadata
+pub fn generate_token(
+    userid: Userid,
+    token_name: Tokenname,
+    comment: Option<String>,
+    enable: Option<bool>,
+    expire: Option<i64>,
+    digest: Option<ConfigDigest>,
+) -> Result<ApiTokenSecret, Error> {
+    let _lock = crate::user::lock_config()?;
+    let (mut config, config_digest) = crate::user::config()?;
+
+    config_digest.detect_modification(digest.as_ref())?;
+
+    let tokenid = Authid::from((userid.clone(), Some(token_name.clone())));
+    let tokenid_string = tokenid.to_string();
+
+    if config.sections.contains_key(&tokenid_string) {
+        bail!(
+            "token '{}' for user '{userid}' already exists.",
+            token_name.as_str(),
+        );
+    }
+
+    let secret = token_shadow::generate_and_set_secret(&tokenid)?;
+
+    let token = ApiToken {
+        tokenid: tokenid.clone(),
+        comment,
+        enable,
+        expire,
+    };
+
+    config.set_data(&tokenid_string, "token", &token)?;
+
+    crate::user::save_config(&config)?;
+
+    Ok(ApiTokenSecret { tokenid, secret })
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: {
+                type: Userid,
+            },
+            "token-name": {
+                type: Tokenname,
+            },
+            comment: {
+                optional: true,
+                schema: COMMENT_SCHEMA,
+            },
+            enable: {
+                schema: ENABLE_USER_SCHEMA,
+                optional: true,
+            },
+            expire: {
+                schema: EXPIRE_USER_SCHEMA,
+                optional: true,
+            },
+            regenerate: {
+                schema: REGENERATE_TOKEN_SCHEMA,
+                optional: true,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableTokenProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                type: ConfigDigest,
+            },
+        },
+    },
+    returns: {
+        type: ApiTokenSecret,
+        optional: true
+    }
+)]
+/// Update user's API token metadata. If regenerate is set to true, the token and it's new secret
+/// will be returned.
+#[allow(clippy::too_many_arguments)]
+
+pub fn update_token(
+    userid: Userid,
+    token_name: Tokenname,
+    comment: Option<String>,
+    enable: Option<bool>,
+    expire: Option<i64>,
+    regenerate: Option<bool>,
+    delete: Option<Vec<DeletableTokenProperty>>,
+    digest: Option<ConfigDigest>,
+) -> Result<Option<ApiTokenSecret>, Error> {
+    let _lock = crate::user::lock_config()?;
+
+    let (mut config, config_digest) = crate::user::config()?;
+    config_digest.detect_modification(digest.as_ref())?;
+
+    let tokenid = Authid::from((userid, Some(token_name)));
+    let tokenid_string = tokenid.to_string();
+
+    let mut data: ApiToken = config.lookup("token", &tokenid_string)?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableTokenProperty::Comment => data.comment = None,
+            }
+        }
+    }
+
+    if let Some(comment) = comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            data.comment = None;
+        } else {
+            data.comment = Some(comment);
+        }
+    }
+
+    if let Some(enable) = enable {
+        data.enable = if enable { None } else { Some(false) };
+    }
+
+    if let Some(expire) = expire {
+        data.expire = if expire > 0 { Some(expire) } else { None };
+    }
+
+    let new_secret = if regenerate == Some(true) {
+        let secret = token_shadow::generate_and_set_secret(&tokenid)?;
+        Some(ApiTokenSecret { tokenid, secret })
+    } else {
+        None
+    };
+
+    config.set_data(&tokenid_string, "token", &data)?;
+
+    crate::user::save_config(&config)?;
+
+    Ok(new_secret)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: {
+                type: Userid,
+            },
+            "token-name": {
+                type: Tokenname,
+            },
+            digest: {
+                optional: true,
+                type: ConfigDigest,
+            },
+        },
+    },
+)]
+/// Delete a user's API token
+pub fn delete_token(
+    userid: Userid,
+    token_name: Tokenname,
+    digest: Option<ConfigDigest>,
+) -> Result<(), Error> {
+    let _acl_lock = acl::lock_config()?;
+    let _user_lock = crate::user::lock_config()?;
+
+    let (mut user_config, config_digest) = crate::user::config()?;
+    config_digest.detect_modification(digest.as_ref())?;
+    let (mut acl_config, _digest) = crate::acl::config()?;
+
+    let tokenid = Authid::from((userid.clone(), Some(token_name.clone())));
+    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(),
+        );
+    }
+
+    token_shadow::delete_secret(&tokenid)?;
+    acl_config.delete_authid(&tokenid);
+    crate::user::save_config(&user_config)?;
+    crate::acl::save_config(&acl_config)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            userid: {
+                type: Userid,
+            },
+        },
+    },
+    returns: {
+        description: "List user's API tokens (with config digest).",
+        type: Array,
+        items: { type: TokenApiEntry },
+    },
+)]
+/// List user's API tokens
+pub fn list_tokens(
+    userid: Userid,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<TokenApiEntry>, Error> {
+    let (config, digest) = crate::user::config()?;
+
+    let list: Vec<ApiToken> = config.convert_to_typed_array("token")?;
+
+    rpcenv["digest"] = hex::encode(digest).into();
+
+    let filter_by_owner = |token: ApiToken| {
+        if token.tokenid.is_token() && token.tokenid.user() == &userid {
+            let token_name = token.tokenid.tokenname().unwrap().to_owned();
+            Some(TokenApiEntry { token_name, token })
+        } else {
+            None
+        }
+    };
+
+    let res = list.into_iter().filter_map(filter_by_owner).collect();
+
+    Ok(res)
+}
diff --git a/proxmox-access-control/src/token_shadow.rs b/proxmox-access-control/src/token_shadow.rs
index 46397edb..373910f3 100644
--- a/proxmox-access-control/src/token_shadow.rs
+++ b/proxmox-access-control/src/token_shadow.rs
@@ -73,3 +73,11 @@ pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> {
 
     Ok(())
 }
+
+/// 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)
+}
diff --git a/proxmox-access-control/src/types.rs b/proxmox-access-control/src/types.rs
index 9c539f7b..85b1f106 100644
--- a/proxmox-access-control/src/types.rs
+++ b/proxmox-access-control/src/types.rs
@@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize};
 
 use const_format::concatcp;
 
-use proxmox_auth_api::types::{Authid, Userid, PROXMOX_TOKEN_ID_SCHEMA};
+use proxmox_auth_api::types::{Authid, Tokenname, Userid, PROXMOX_TOKEN_ID_SCHEMA};
 use proxmox_schema::{
     api,
     api_types::{COMMENT_SCHEMA, SAFE_ID_REGEX_STR, SINGLE_LINE_COMMENT_FORMAT},
@@ -154,9 +154,26 @@ impl ApiToken {
 pub struct ApiTokenSecret {
     pub tokenid: Authid,
     /// The secret associated with the token.
+    // rename to `value` as that is what it is called in the api
+    #[serde(rename = "value")]
     pub secret: String,
 }
 
+#[api(
+    properties: {
+        token: { type: ApiToken },
+    }
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// A Token Entry that contains the token-name
+pub struct TokenApiEntry {
+    /// The Token name
+    pub token_name: Tokenname,
+    #[serde(flatten)]
+    pub token: ApiToken,
+}
+
 #[api(
     properties: {
         userid: {
@@ -285,3 +302,17 @@ pub struct RoleInfo {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
 }
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// The set of properties that can be deleted from a token.
+pub enum DeletableTokenProperty {
+    /// Delete the comment property.
+    Comment,
+}
+
+pub const REGENERATE_TOKEN_SCHEMA: Schema =
+    BooleanSchema::new("Regenerate token secret while keeping permissions.")
+        .default(false)
+        .schema();
-- 
2.47.3



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

[pdm-devel] [PATCH yew-comp 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function

[Shannon Sterz]

this also adapts all use sites of `copy_to_clipboard` to use this new
function instead and marks the old function as deprecated.
`copy_to_clipboard` is based on the `document.execCommand()` method
that is deprecated and might not be supported in the future [1].

`copy_text_to_clipboard` is based on the new `Clipboard` API that is
now in baseline and, thus, widely available. it should also be
somewhat more ergonomic to use. users don't need to handle a `NodeRef`
in multiple places, but can just pass text to be copied to the
function directly.

this requires the web_sys features `Clipboard` and `Navigator`.

[1]:
https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand
[2]:
https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 Cargo.toml                  |  2 ++
 src/tfa/tfa_add_recovery.rs | 17 ++++++-----------
 src/token_panel.rs          | 16 +++++++++-------
 src/utils.rs                | 22 ++++++++++++++++++++++
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 9abb8d3..9fe242f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -18,6 +18,7 @@ web-sys = { version = "0.3", features = [
   "AbortController",
   "AbortSignal",
   "Attr",
+  "Clipboard",
   "Crypto",
   "Document",
   "DomParser",
@@ -26,6 +27,7 @@ web-sys = { version = "0.3", features = [
   "HtmlDocument",
   "HtmlElement",
   "NamedNodeMap",
+  "Navigator",
   "Node",
   "Range",
   "ReadableStreamDefaultReader",
diff --git a/src/tfa/tfa_add_recovery.rs b/src/tfa/tfa_add_recovery.rs
index b24e73c..7c0e9d6 100644
--- a/src/tfa/tfa_add_recovery.rs
+++ b/src/tfa/tfa_add_recovery.rs
@@ -14,7 +14,7 @@ use crate::percent_encoding::percent_encode_component;
 
 use pwt_macros::builder;
 
-use crate::utils::copy_to_clipboard;
+use crate::utils::copy_text_to_clipboard;
 use crate::{AuthidSelector, EditWindow};
 
 #[derive(Debug, Deserialize)]
@@ -81,7 +81,6 @@ pub enum Msg {
 #[doc(hidden)]
 pub struct ProxmoxTfaAddRecovery {
     recovery_keys: Option<RecoveryKeyInfo>,
-    container_ref: NodeRef,
     print_counter: usize,
     print_portal: Option<Html>,
 }
@@ -107,14 +106,15 @@ fn render_input_form(_form_ctx: FormContext) -> Html {
 impl ProxmoxTfaAddRecovery {
     fn recovery_keys_dialog(&self, ctx: &Context<Self>, data: &RecoveryKeyInfo) -> Html {
         use std::fmt::Write;
-        let text: String = data
+        let text: AttrValue = data
             .keys
             .iter()
             .enumerate()
             .fold(String::new(), |mut acc, (i, key)| {
                 let _ = writeln!(acc, "{i}: {key}\n");
                 acc
-            });
+            })
+            .into();
 
         Dialog::new(tr!("Recovery Keys for user '{}'", data.userid))
             .on_close(ctx.props().on_close.clone())
@@ -128,8 +128,7 @@ impl ProxmoxTfaAddRecovery {
                                     .class("pwt-font-monospace")
                                     .padding(2)
                                     .border(true)
-                                    .with_child(text)
-                                    .into_html_with_ref(self.container_ref.clone()),
+                                    .with_child(text.clone()),
                             )
                             .with_child(
                                 Container::new()
@@ -147,10 +146,7 @@ impl ProxmoxTfaAddRecovery {
                                 Button::new(tr!("Copy Recovery Keys"))
                                     .icon_class("fa fa-clipboard")
                                     .class("pwt-scheme-primary")
-                                    .onclick({
-                                        let container_ref = self.container_ref.clone();
-                                        move |_| copy_to_clipboard(&container_ref)
-                                    }),
+                                    .on_activate(move |_| copy_text_to_clipboard(&text)),
                             )
                             .with_child(
                                 Button::new(tr!("Print Recovery Keys"))
@@ -172,7 +168,6 @@ impl Component for ProxmoxTfaAddRecovery {
     fn create(_ctx: &Context<Self>) -> Self {
         Self {
             recovery_keys: None,
-            container_ref: NodeRef::default(),
             print_portal: None,
             print_counter: 0,
         }
diff --git a/src/token_panel.rs b/src/token_panel.rs
index c70adb2..c027a32 100644
--- a/src/token_panel.rs
+++ b/src/token_panel.rs
@@ -17,7 +17,9 @@ use pwt::widget::form::{Checkbox, DisplayField, Field, FormContext, InputType};
 use pwt::widget::{Button, Column, Container, Dialog, InputPanel, Toolbar};
 
 use crate::percent_encoding::percent_encode_component;
-use crate::utils::{copy_to_clipboard, epoch_to_input_value, render_boolean, render_epoch_short};
+use crate::utils::{
+    copy_text_to_clipboard, epoch_to_input_value, render_boolean, render_epoch_short,
+};
 use crate::{
     AuthidSelector, ConfirmButton, EditWindow, LoadableComponent, LoadableComponentContext,
     LoadableComponentLink, LoadableComponentMaster, PermissionPanel,
@@ -121,7 +123,6 @@ enum Msg {
 struct ProxmoxTokenView {
     selection: Selection,
     store: Store<ApiToken>,
-    secret_node_ref: NodeRef,
     columns: Rc<Vec<DataTableHeader<ApiToken>>>,
 }
 
@@ -149,7 +150,6 @@ impl LoadableComponent for ProxmoxTokenView {
         Self {
             selection,
             store,
-            secret_node_ref: NodeRef::default(),
             columns: columns(),
         }
     }
@@ -351,8 +351,7 @@ impl ProxmoxTokenView {
                             .style("opacity", "0")
                             .with_child(AttrValue::from(
                                 secret["value"].as_str().unwrap_or("").to_owned(),
-                            ))
-                            .into_html_with_ref(self.secret_node_ref.clone()),
+                            )),
                     )
                     .with_child(
                         Container::new()
@@ -373,8 +372,11 @@ impl ProxmoxTokenView {
                                     .icon_class("fa fa-clipboard")
                                     .class("pwt-scheme-primary")
                                     .on_activate({
-                                        let copy_ref = self.secret_node_ref.clone();
-                                        move |_| copy_to_clipboard(&copy_ref)
+                                        move |_| {
+                                            copy_text_to_clipboard(
+                                                secret["value"].as_str().unwrap_or(""),
+                                            )
+                                        }
                                     }),
                             ),
                     ),
diff --git a/src/utils.rs b/src/utils.rs
index 79b7ad7..23794b9 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use std::fmt::Display;
 use std::sync::Mutex;
 
+use pwt::convert_js_error;
 use serde_json::Value;
 use wasm_bindgen::JsCast;
 use yew::prelude::*;
@@ -338,6 +339,9 @@ pub fn json_array_to_flat_string(list: &[Value]) -> String {
     list.join(" ")
 }
 
+#[deprecated(
+    note = "This relies on the deprecated `execCommand` method. Please use `utils::copy_text_to_clipboard` instead."
+)]
 pub fn copy_to_clipboard(node_ref: &NodeRef) {
     if let Some(el) = node_ref.cast::<web_sys::HtmlInputElement>() {
         let window = gloo_utils::window();
@@ -356,6 +360,24 @@ pub fn copy_to_clipboard(node_ref: &NodeRef) {
     }
 }
 
+pub fn copy_text_to_clipboard(text: &str) {
+    let text = text.to_owned();
+
+    wasm_bindgen_futures::spawn_local(async move {
+        let future: wasm_bindgen_futures::JsFuture = gloo_utils::window()
+            .navigator()
+            .clipboard()
+            .write_text(&text)
+            .into();
+
+        let res = future.await.map_err(convert_js_error);
+
+        if let Err(e) = res {
+            log::error!("could not copy to clipboard: {e:#}");
+        }
+    });
+}
+
 /// Set the browser window.location.href
 pub fn set_location_href(href: &str) {
     let window = gloo_utils::window();
-- 
2.47.3



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

[pdm-devel] [PATCH yew-comp 2/2] token panel: improve token secret dialog layout and hide password

[Shannon Sterz]

the dialog showing the token secret now displays it like any other
password. meaning that the actual secret is covered up at first but
can be displayed if required. this adds protections against should
surfing attacks.

the copy secret button has also been moved next to the input field and
is now an icon buton with a tool tip.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/token_panel.rs | 111 ++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 56 deletions(-)

diff --git a/src/token_panel.rs b/src/token_panel.rs
index c027a32..049aaa0 100644
--- a/src/token_panel.rs
+++ b/src/token_panel.rs
@@ -6,6 +6,7 @@ use anyhow::Error;
 use proxmox_access_control::types::{ApiToken, UserWithTokens};
 use proxmox_auth_api::types::Authid;
 use proxmox_client::ApiResponseData;
+use pwt::css::ColorScheme;
 use serde_json::{json, Value};

 use yew::virtual_dom::{Key, VComp, VNode};
@@ -14,7 +15,9 @@ use pwt::prelude::*;
 use pwt::state::{Selection, Store};
 use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
 use pwt::widget::form::{Checkbox, DisplayField, Field, FormContext, InputType};
-use pwt::widget::{Button, Column, Container, Dialog, InputPanel, Toolbar};
+use pwt::widget::{
+    Button, Column, Container, Dialog, FieldLabel, InputPanel, Row, Toolbar, Tooltip,
+};

 use crate::percent_encoding::percent_encode_component;
 use crate::utils::{
@@ -321,65 +324,61 @@ impl ProxmoxTokenView {
     }

     fn show_secret_dialog(&self, ctx: &LoadableComponentContext<Self>, secret: &Value) -> Html {
-        let secret = secret.clone();
+        let tokenid = AttrValue::from(secret["tokenid"].as_str().unwrap_or("").to_owned());
+        let secret = AttrValue::from(secret["value"].as_str().unwrap_or("").to_owned());

         Dialog::new(tr!("Token Secret"))
             .with_child(
-                Column::new()
-                    .with_child(
-                        InputPanel::new()
-                            .padding(4)
-                            .with_large_field(
-                                tr!("Token ID"),
-                                DisplayField::new()
-                                    .value(AttrValue::from(
-                                        secret["tokenid"].as_str().unwrap_or("").to_owned(),
-                                    ))
-                                    .border(true),
-                            )
-                            .with_large_field(
-                                tr!("Secret"),
-                                DisplayField::new()
-                                    .value(AttrValue::from(
-                                        secret["value"].as_str().unwrap_or("").to_owned(),
-                                    ))
-                                    .border(true),
-                            ),
-                    )
-                    .with_child(
-                        Container::new()
-                            .style("opacity", "0")
-                            .with_child(AttrValue::from(
-                                secret["value"].as_str().unwrap_or("").to_owned(),
-                            )),
-                    )
-                    .with_child(
-                        Container::new()
-                            .padding(4)
-                            .class(pwt::css::FlexFit)
-                            .class("pwt-bg-color-warning-container")
-                            .class("pwt-color-on-warning-container")
-                            .with_child(tr!(
-                                "Please record the API token secret - it will only be displayed now"
-                            )),
-                    )
-                    .with_child(
-                        Toolbar::new()
-                            .class("pwt-bg-color-surface")
-                            .with_flex_spacer()
-                            .with_child(
-                                Button::new(tr!("Copy Secret Value"))
-                                    .icon_class("fa fa-clipboard")
-                                    .class("pwt-scheme-primary")
-                                    .on_activate({
-                                        move |_| {
-                                            copy_text_to_clipboard(
-                                                secret["value"].as_str().unwrap_or(""),
+                Column::new().with_child(
+                    InputPanel::new()
+                        .padding(4)
+                        .with_large_field(
+                            tr!("Token ID"),
+                            DisplayField::new()
+                                .value(tokenid)
+                                .name("tokenid")
+                                .border(true),
+                        )
+                        .with_large_custom_child(
+                            Container::new()
+                                .class("pwt-form-grid-col4")
+                                .with_child(FieldLabel::new(tr!("Secret")))
+                                .with_child(
+                                    Row::new()
+                                        .class("pwt-fill-grid-row")
+                                        .with_child(
+                                            Field::new()
+                                                .input_type(InputType::Password)
+                                                .class(pwt::css::FlexFit)
+                                                .value(secret.clone())
+                                                .name("secret")
+                                                .disabled(true)
+                                                .opacity(100),
+                                        )
+                                        .with_child(
+                                            Tooltip::new(
+                                                Button::new_icon("fa fa-clipboard")
+                                                    .class(ColorScheme::Primary)
+                                                    .margin_start(2)
+                                                    .on_activate(move |_| {
+                                                        copy_text_to_clipboard(&secret)
+                                                    }),
                                             )
-                                        }
-                                    }),
-                            ),
-                    ),
+                                            .tip(tr!("Copy token secret to clipboard.")),
+                                        ),
+                                ),
+                        ),
+                ),
+            )
+            .with_child(
+                Container::new()
+                    .padding(4)
+                    .class(pwt::css::FlexFit)
+                    .class("pwt-bg-color-warning-container")
+                    .class("pwt-color-on-warning-container")
+                    .with_child(tr!(
+                        "Please record the API token secret - it will only be displayed now."
+                    )),
             )
             .on_close(ctx.link().change_view_callback(|_| None))
             .into()
--
2.47.3



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

[pdm-devel] [PATCH datacenter-manager 1/3] ui: add a token panel and a token acl edit menu in the permissions panel

[Shannon Sterz]

this allows users to create and manage api tokens.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 ui/src/configuration/mod.rs | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/ui/src/configuration/mod.rs b/ui/src/configuration/mod.rs
index 0a92796..4110f39 100644
--- a/ui/src/configuration/mod.rs
+++ b/ui/src/configuration/mod.rs
@@ -7,7 +7,7 @@ use pwt::widget::{Container, MiniScrollMode, Panel, TabBarItem, TabPanel};
 use proxmox_yew_comp::configuration::TimePanel;
 use proxmox_yew_comp::configuration::{DnsPanel, NetworkView};
 use proxmox_yew_comp::tfa::TfaView;
-use proxmox_yew_comp::{AclEdit, AclView, AuthView, UserPanel};
+use proxmox_yew_comp::{AclEdit, AclView, AuthView, TokenPanel, UserPanel};
 
 mod permission_path_selector;
 mod webauthn;
@@ -73,6 +73,19 @@ pub fn access_control() -> Html {
                     .into()
             },
         )
+        .with_item_builder(
+            TabBarItem::new()
+                .key("api-tokens")
+                .icon_class("fa fa-user-o")
+                .label(tr!("API Token")),
+            |_| {
+                Container::new()
+                    .class("pwt-content-spacer")
+                    .class(pwt::css::FlexFit)
+                    .with_child(TokenPanel::new())
+                    .into()
+            },
+        )
         .with_item_builder(
             TabBarItem::new()
                 .key("two-factor")
@@ -95,11 +108,19 @@ pub fn access_control() -> Html {
                 Container::new()
                     .class("pwt-content-spacer")
                     .class(pwt::css::FlexFit)
-                    .with_child(AclView::new().with_acl_edit_menu_entry(
-                        tr!("User Permission"),
-                        "fa fa-fw fa-user",
-                        acl_edit.clone().use_tokens(false),
-                    ))
+                    .with_child(
+                        AclView::new()
+                            .with_acl_edit_menu_entry(
+                                tr!("User Permission"),
+                                "fa fa-fw fa-user",
+                                acl_edit.clone().use_tokens(false),
+                            )
+                            .with_acl_edit_menu_entry(
+                                tr!("Token Permission"),
+                                "fa fa-fw fa-user-o",
+                                acl_edit.clone().use_tokens(true),
+                            ),
+                    )
                     .into()
             },
         )
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager 2/3] server: access: use token endpoints from proxmox-access-control

[Shannon Sterz]

this allows keeping this logic in one place accross products

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 server/src/api/access/users.rs | 349 ++++-----------------------------
 1 file changed, 34 insertions(+), 315 deletions(-)

diff --git a/server/src/api/access/users.rs b/server/src/api/access/users.rs
index 7b6839e..da598d8 100644
--- a/server/src/api/access/users.rs
+++ b/server/src/api/access/users.rs
@@ -1,20 +1,16 @@
 //! User Management
 
 use anyhow::{bail, format_err, Error};
-use serde::{Deserialize, Serialize};
-use serde_json::{json, Value};
 use std::collections::HashMap;
 
-use proxmox_access_control::types::{
-    ApiToken, User, UserUpdater, UserWithTokens, ENABLE_USER_SCHEMA, EXPIRE_USER_SCHEMA,
-};
-use proxmox_access_control::{token_shadow, CachedUserInfo};
+use proxmox_access_control::types::{ApiToken, User, UserUpdater, UserWithTokens};
+use proxmox_access_control::CachedUserInfo;
 use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment, SubdirMap};
 use proxmox_schema::api;
 
 use pdm_api_types::{
-    Authid, ConfigDigest, DeletableUserProperty, Tokenname, Userid, PDM_PASSWORD_SCHEMA,
-    PRIV_ACCESS_MODIFY, PRIV_SYS_AUDIT, SINGLE_LINE_COMMENT_SCHEMA,
+    Authid, ConfigDigest, DeletableUserProperty, Userid, PDM_PASSWORD_SCHEMA, PRIV_ACCESS_MODIFY,
+    PRIV_SYS_AUDIT,
 };
 
 fn new_user_with_tokens(user: User) -> UserWithTokens {
@@ -382,336 +378,59 @@ pub fn delete_user(userid: Userid, digest: Option<ConfigDigest>) -> Result<(), E
     Ok(())
 }
 
-#[api(
-    input: {
-        properties: {
-            userid: {
-                type: Userid,
-            },
-            "token-name": {
-                type: Tokenname,
-            },
-        },
-    },
-    returns: { type: ApiToken },
-    access: {
-        permission: &Permission::Or(&[
+const API_METHOD_READ_TOKEN_WITH_ACCESS: ApiMethod =
+    proxmox_access_control::api::API_METHOD_READ_TOKEN.access(
+        None,
+        &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
             &Permission::UserParam("userid"),
         ]),
-    },
-)]
-/// Read user's API token metadata
-pub fn read_token(
-    userid: Userid,
-    token_name: Tokenname,
-    _info: &ApiMethod,
-    rpcenv: &mut dyn RpcEnvironment,
-) -> Result<ApiToken, Error> {
-    let (config, digest) = proxmox_access_control::user::config()?;
+    );
 
-    let tokenid = Authid::from((userid, Some(token_name)));
-
-    rpcenv["digest"] = hex::encode(digest).into();
-    config.lookup("token", &tokenid.to_string())
-}
-
-#[api(
-    protected: true,
-    input: {
-        properties: {
-            userid: {
-                type: Userid,
-            },
-            "token-name": {
-                type: Tokenname,
-            },
-            comment: {
-                optional: true,
-                schema: SINGLE_LINE_COMMENT_SCHEMA,
-            },
-            enable: {
-                schema: ENABLE_USER_SCHEMA,
-                optional: true,
-            },
-            expire: {
-                schema: EXPIRE_USER_SCHEMA,
-                optional: true,
-            },
-            digest: {
-                optional: true,
-                type: ConfigDigest,
-            },
-        },
-    },
-    access: {
-        permission: &Permission::Or(&[
+const API_METHOD_UPDATE_TOKEN_WITH_ACCESS: ApiMethod =
+    proxmox_access_control::api::API_METHOD_UPDATE_TOKEN.access(
+        None,
+        &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_ACCESS_MODIFY, false),
             &Permission::UserParam("userid"),
         ]),
-    },
-    returns: {
-        description: "API token identifier + generated secret.",
-        properties: {
-            value: {
-                type: String,
-                description: "The API token secret",
-            },
-            tokenid: {
-                type: String,
-                description: "The API token identifier",
-            },
-        },
-    },
-)]
-/// Generate a new API token with given metadata
-pub fn generate_token(
-    userid: Userid,
-    token_name: Tokenname,
-    comment: Option<String>,
-    enable: Option<bool>,
-    expire: Option<i64>,
-    digest: Option<ConfigDigest>,
-) -> Result<Value, Error> {
-    let _lock = proxmox_access_control::user::lock_config()?;
+    );
 
-    let (mut config, config_digest) = proxmox_access_control::user::config()?;
-    config_digest.detect_modification(digest.as_ref())?;
-
-    let tokenid = Authid::from((userid.clone(), Some(token_name.clone())));
-    let tokenid_string = tokenid.to_string();
-
-    if config.sections.contains_key(&tokenid_string) {
-        bail!(
-            "token '{}' for user '{}' already exists.",
-            token_name.as_str(),
-            userid
-        );
-    }
-
-    let secret = format!("{:x}", proxmox_uuid::Uuid::generate());
-    token_shadow::set_secret(&tokenid, &secret)?;
-
-    let token = ApiToken {
-        tokenid,
-        comment,
-        enable,
-        expire,
-    };
-
-    config.set_data(&tokenid_string, "token", &token)?;
-
-    proxmox_access_control::user::save_config(&config)?;
-
-    Ok(json!({
-        "tokenid": tokenid_string,
-        "value": secret
-    }))
-}
-
-#[api(
-    protected: true,
-    input: {
-        properties: {
-            userid: {
-                type: Userid,
-            },
-            "token-name": {
-                type: Tokenname,
-            },
-            comment: {
-                optional: true,
-                schema: SINGLE_LINE_COMMENT_SCHEMA,
-            },
-            enable: {
-                schema: ENABLE_USER_SCHEMA,
-                optional: true,
-            },
-            expire: {
-                schema: EXPIRE_USER_SCHEMA,
-                optional: true,
-            },
-            digest: {
-                optional: true,
-                type: ConfigDigest,
-            },
-        },
-    },
-    access: {
-        permission: &Permission::Or(&[
+const API_METHOD_GENERATE_TOKEN_WITH_ACCESS: ApiMethod =
+    proxmox_access_control::api::API_METHOD_GENERATE_TOKEN.access(
+        None,
+        &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_ACCESS_MODIFY, false),
             &Permission::UserParam("userid"),
         ]),
-    },
-)]
-/// Update user's API token metadata
-pub fn update_token(
-    userid: Userid,
-    token_name: Tokenname,
-    comment: Option<String>,
-    enable: Option<bool>,
-    expire: Option<i64>,
-    digest: Option<ConfigDigest>,
-) -> Result<(), Error> {
-    let _lock = proxmox_access_control::user::lock_config()?;
+    );
 
-    let (mut config, config_digest) = proxmox_access_control::user::config()?;
-    config_digest.detect_modification(digest.as_ref())?;
-
-    let tokenid = Authid::from((userid, Some(token_name)));
-    let tokenid_string = tokenid.to_string();
-
-    let mut data: ApiToken = config.lookup("token", &tokenid_string)?;
-
-    if let Some(comment) = comment {
-        let comment = comment.trim().to_string();
-        if comment.is_empty() {
-            data.comment = None;
-        } else {
-            data.comment = Some(comment);
-        }
-    }
-
-    if let Some(enable) = enable {
-        data.enable = if enable { None } else { Some(false) };
-    }
-
-    if let Some(expire) = expire {
-        data.expire = if expire > 0 { Some(expire) } else { None };
-    }
-
-    config.set_data(&tokenid_string, "token", &data)?;
-
-    proxmox_access_control::user::save_config(&config)?;
-
-    Ok(())
-}
-
-#[api(
-    protected: true,
-    input: {
-        properties: {
-            userid: {
-                type: Userid,
-            },
-            "token-name": {
-                type: Tokenname,
-            },
-            digest: {
-                optional: true,
-                type: ConfigDigest,
-            },
-        },
-    },
-    access: {
-        permission: &Permission::Or(&[
+const API_METHOD_DELETE_TOKEN_WITH_ACCESS: ApiMethod =
+    proxmox_access_control::api::API_METHOD_DELETE_TOKEN.access(
+        None,
+        &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_ACCESS_MODIFY, false),
             &Permission::UserParam("userid"),
         ]),
-    },
-)]
-/// Delete a user's API token
-pub fn delete_token(
-    userid: Userid,
-    token_name: Tokenname,
-    digest: Option<ConfigDigest>,
-) -> Result<(), Error> {
-    let _lock = proxmox_access_control::user::lock_config()?;
+    );
 
-    let (mut config, config_digest) = proxmox_access_control::user::config()?;
-    config_digest.detect_modification(digest.as_ref())?;
-
-    let tokenid = Authid::from((userid.clone(), Some(token_name.clone())));
-    let tokenid_string = tokenid.to_string();
-
-    match config.sections.get(&tokenid_string) {
-        Some(_) => {
-            config.sections.remove(&tokenid_string);
-        }
-        None => bail!(
-            "token '{}' of user '{}' does not exist.",
-            token_name.as_str(),
-            userid
-        ),
-    }
-
-    token_shadow::delete_secret(&tokenid)?;
-
-    proxmox_access_control::user::save_config(&config)?;
-
-    Ok(())
-}
-
-#[api(
-    properties: {
-        "token-name": { type: Tokenname },
-        token: { type: ApiToken },
-    }
-)]
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// A Token Entry that contains the token-name
-pub struct TokenApiEntry {
-    /// The Token name
-    pub token_name: Tokenname,
-    #[serde(flatten)]
-    pub token: ApiToken,
-}
-
-#[api(
-    input: {
-        properties: {
-            userid: {
-                type: Userid,
-            },
-        },
-    },
-    returns: {
-        description: "List user's API tokens (with config digest).",
-        type: Array,
-        items: { type: TokenApiEntry },
-    },
-    access: {
-        permission: &Permission::Or(&[
+const API_METHOD_LIST_TOKENS_WITH_ACCESS: ApiMethod =
+    proxmox_access_control::api::API_METHOD_LIST_TOKENS.access(
+        None,
+        &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
             &Permission::UserParam("userid"),
         ]),
-    },
-)]
-/// List user's API tokens
-pub fn list_tokens(
-    userid: Userid,
-    _info: &ApiMethod,
-    rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<TokenApiEntry>, Error> {
-    let (config, digest) = proxmox_access_control::user::config()?;
-
-    let list: Vec<ApiToken> = config.convert_to_typed_array("token")?;
-
-    rpcenv["digest"] = hex::encode(digest).into();
-
-    let filter_by_owner = |token: ApiToken| {
-        if token.tokenid.is_token() && token.tokenid.user() == &userid {
-            let token_name = token.tokenid.tokenname().unwrap().to_owned();
-            Some(TokenApiEntry { token_name, token })
-        } else {
-            None
-        }
-    };
-
-    let res = list.into_iter().filter_map(filter_by_owner).collect();
-
-    Ok(res)
-}
+    );
 
 const TOKEN_ITEM_ROUTER: Router = Router::new()
-    .get(&API_METHOD_READ_TOKEN)
-    .put(&API_METHOD_UPDATE_TOKEN)
-    .post(&API_METHOD_GENERATE_TOKEN)
-    .delete(&API_METHOD_DELETE_TOKEN);
+    .get(&API_METHOD_READ_TOKEN_WITH_ACCESS)
+    .put(&API_METHOD_UPDATE_TOKEN_WITH_ACCESS)
+    .post(&API_METHOD_GENERATE_TOKEN_WITH_ACCESS)
+    .delete(&API_METHOD_DELETE_TOKEN_WITH_ACCESS);
 
 const TOKEN_ROUTER: Router = Router::new()
-    .get(&API_METHOD_LIST_TOKENS)
+    .get(&API_METHOD_LIST_TOKENS_WITH_ACCESS)
     .match_all("token-name", &TOKEN_ITEM_ROUTER);
 
 const USER_SUBDIRS: SubdirMap = &[("token", &TOKEN_ROUTER)];
-- 
2.47.3



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

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

[Shannon Sterz]

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();
+
+    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);
+        }
+    }
+
+    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] [RFC datacenter-manager/proxmox/yew-comp 0/8] token support for pdm

[Shannon Sterz]

this series adds a ui for adding, editing, removing and regenarating api
tokens to proxmox-yew-comp and integrates it into
proxmox-datacenter-manager. it also allows for adding acl entries for
them in the permissions panel.

sending this as an rfc as this series also factors out the token related
api endpoints into proxmox-access-control and i would like some feedback
on the approach there. i didn't want to add even more methods to the
AccessControlConfig here in order to not clutter it too much.

proxmox:

Shannon Sterz (3):
  access-control: refactor api module to be more hirachical
  access-control: move `ApiTokenSecret` to types module
  access-control: add api endpoints for handling tokens

 proxmox-access-control/Cargo.toml             |   2 +
 .../src/{api.rs => api/acl.rs}                |   0
 proxmox-access-control/src/api/mod.rs         |   8 +
 proxmox-access-control/src/api/tokens.rs      | 306 ++++++++++++++++++
 proxmox-access-control/src/token_shadow.rs    |   9 -
 proxmox-access-control/src/types.rs           |  29 +-
 6 files changed, 344 insertions(+), 10 deletions(-)
 rename proxmox-access-control/src/{api.rs => api/acl.rs} (100%)
 create mode 100644 proxmox-access-control/src/api/mod.rs
 create mode 100644 proxmox-access-control/src/api/tokens.rs


proxmox-yew-comp:

Shannon Sterz (2):
  utils/user_panel: factor out epoch_to_input_value helper
  token_panel: implement a token panel

 src/lib.rs         |   3 +
 src/token_panel.rs | 569 +++++++++++++++++++++++++++++++++++++++++++++
 src/user_panel.rs  |  21 +-
 src/utils.rs       |  19 ++
 4 files changed, 592 insertions(+), 20 deletions(-)
 create mode 100644 src/token_panel.rs


proxmox-datacenter-manager:

Shannon Sterz (3):
  ui: add a token panel and a token acl edit menu in the permissions
    panel
  server: access: use token endpoints from proxmox-access-control
  server: clean up acl tree entries and api tokens when deleting users

 server/src/api/access/users.rs | 388 ++++++---------------------------
 ui/src/configuration/mod.rs    |  33 ++-
 2 files changed, 95 insertions(+), 326 deletions(-)


Summary over all repositories:
  12 files changed, 1031 insertions(+), 356 deletions(-)

--
Generated by git-murpp 0.8.1


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

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

[Shannon Sterz]

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();
+
+    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);
+        }
+    }
+
+    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

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

[Fabian Grünbichler]

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..

> +        }
> +    }
> +
> +    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

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

[Shannon Sterz]

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

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

[Fabian Grünbichler]

On October 1, 2025 5:29 pm, Shannon Sterz wrote:
> 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?

no, that was exactly the issue I was worried about!

delete token => ACL remains
delete user => token doesn't exist, so ACL is not removed either here

in PVE we just drop ACLs on parsing that reference invalid authids..


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