* [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
@ 2025-10-14 14:37 ` Shannon Sterz
2025-10-17 9:35 ` Lukas Wagner
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password Shannon Sterz
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-14 14:37 UTC (permalink / raw)
To: pdm-devel
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>
Reviewed-by: Dominik Csapak <d.csapak@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 0504047..d02eb66 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(©_ref)
+ move |_| {
+ copy_text_to_clipboard(
+ secret["value"].as_str().unwrap_or(""),
+ )
+ }
}),
),
),
diff --git a/src/utils.rs b/src/utils.rs
index 3dfc696..f4db098 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function Shannon Sterz
@ 2025-10-17 9:35 ` Lukas Wagner
2025-10-17 10:43 ` Shannon Sterz
0 siblings, 1 reply; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 9:35 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
The 'utils' module is getting quite large by now with many different
kinds of helpers, maybe it is time to actually start splitting these out
into distinct modules? For instance, in this patch series,
you could create a new 'clipboard' module, moving *both* functions there
and then adding a 'pub use' in 'utils' for the old, deprecated function
for compatibility?
Apart from this and the minor nit below, consider this:
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
> diff --git a/src/utils.rs b/src/utils.rs
> index 3dfc696..f4db098 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) {
> }
> }
>
Missing doc comments for a `pub` function.
> +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();
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function
2025-10-17 9:35 ` Lukas Wagner
@ 2025-10-17 10:43 ` Shannon Sterz
0 siblings, 0 replies; 15+ messages in thread
From: Shannon Sterz @ 2025-10-17 10:43 UTC (permalink / raw)
To: Lukas Wagner; +Cc: Proxmox Datacenter Manager development discussion
On Fri Oct 17, 2025 at 11:35 AM CEST, Lukas Wagner wrote:
> The 'utils' module is getting quite large by now with many different
> kinds of helpers, maybe it is time to actually start splitting these out
> into distinct modules? For instance, in this patch series,
> you could create a new 'clipboard' module, moving *both* functions there
> and then adding a 'pub use' in 'utils' for the old, deprecated function
> for compatibility?
not opposed to that, but we might want to think about how to do that a
bit more in-depth. not sure how we want to split those up exactly.
might be better handled as a separate clean-up series for yew-comp?
> Apart from this and the minor nit below, consider this:
>
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
>
> On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
>> diff --git a/src/utils.rs b/src/utils.rs
>> index 3dfc696..f4db098 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) {
>> }
>> }
>>
>
> Missing doc comments for a `pub` function.
>
ah true, should be
/// Copies `text` to the user's clipboard via the `Clipboard` API.
>> +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();
>
>
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function Shannon Sterz
@ 2025-10-14 14:37 ` Shannon Sterz
2025-10-17 9:36 ` Lukas Wagner
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 1/3] ui: add a token panel and a token acl edit menu in the permissions panel Shannon Sterz
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-14 14:37 UTC (permalink / raw)
To: pdm-devel
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..5c8fa23 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")
+ .gap(2)
+ .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)
+ .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(ColorScheme::WarningContainer)
+ .class("pwt-default-colors")
+ .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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password Shannon Sterz
@ 2025-10-17 9:36 ` Lukas Wagner
2025-10-17 10:04 ` Shannon Sterz
0 siblings, 1 reply; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 9:36 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
Looks good to me.
One comment inline, but nothing that prohibits applying this patch.
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
> 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..5c8fa23 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")
> + .gap(2)
> + .with_child(
> + Field::new()
> + .input_type(InputType::Password)
> + .class(pwt::css::FlexFit)
> + .value(secret.clone())
> + .name("secret")
> + .disabled(true)
When clicking the 'show secret' button, it is not possible to select the
secret via the cursor and copy it by 'hand' (Ctrl-C/right click context
menu), which is due to this field being disabled. I know there is a
'copy secret' button, but this tripped me up for a moment when trying
this out.
Was this an intentional decision, e.g. to avoid mistakes when copying
the value or to discourage users from using the 'show secret' button
altogether?
> + .opacity(100),
> + )
> + .with_child(
> + Tooltip::new(
> + Button::new_icon("fa fa-clipboard")
> + .class(ColorScheme::Primary)
> + .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(ColorScheme::WarningContainer)
> + .class("pwt-default-colors")
> + .with_child(tr!(
> + "Please record the API token secret - it will only be displayed now."
> + )),
> )
> .on_close(ctx.link().change_view_callback(|_| None))
> .into()
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password
2025-10-17 9:36 ` Lukas Wagner
@ 2025-10-17 10:04 ` Shannon Sterz
2025-10-17 11:03 ` Lukas Wagner
0 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-17 10:04 UTC (permalink / raw)
To: Lukas Wagner; +Cc: Proxmox Datacenter Manager development discussion
On Fri Oct 17, 2025 at 11:36 AM CEST, Lukas Wagner wrote:
> Looks good to me.
> One comment inline, but nothing that prohibits applying this patch.
>
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
>
> On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
>> 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..5c8fa23 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")
>> + .gap(2)
>> + .with_child(
>> + Field::new()
>> + .input_type(InputType::Password)
>> + .class(pwt::css::FlexFit)
>> + .value(secret.clone())
>> + .name("secret")
>> + .disabled(true)
>
> When clicking the 'show secret' button, it is not possible to select the
> secret via the cursor and copy it by 'hand' (Ctrl-C/right click context
> menu), which is due to this field being disabled. I know there is a
> 'copy secret' button, but this tripped me up for a moment when trying
> this out.
> Was this an intentional decision, e.g. to avoid mistakes when copying
> the value or to discourage users from using the 'show secret' button
> altogether?
thanks for the review, this was actually discussed in a previous review.
there i used a DisplayField, but that doesn't allow for hiding the
contents like password fields do.
i decided to mark this as `disabled` so that users won't edit it on
accident, but allowing users to select it manually certainly makes
sense.
>> + .opacity(100),
>> + )
>> + .with_child(
>> + Tooltip::new(
>> + Button::new_icon("fa fa-clipboard")
>> + .class(ColorScheme::Primary)
>> + .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(ColorScheme::WarningContainer)
>> + .class("pwt-default-colors")
>> + .with_child(tr!(
>> + "Please record the API token secret - it will only be displayed now."
>> + )),
>> )
>> .on_close(ctx.link().change_view_callback(|_| None))
>> .into()
>
>
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password
2025-10-17 10:04 ` Shannon Sterz
@ 2025-10-17 11:03 ` Lukas Wagner
0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 11:03 UTC (permalink / raw)
To: Shannon Sterz, Lukas Wagner
Cc: Proxmox Datacenter Manager development discussion
On Fri Oct 17, 2025 at 12:04 PM CEST, Shannon Sterz wrote:
> On Fri Oct 17, 2025 at 11:36 AM CEST, Lukas Wagner wrote:
>> Looks good to me.
>> One comment inline, but nothing that prohibits applying this patch.
>>
>> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
>> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
>>
>> On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
>>> 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..5c8fa23 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")
>>> + .gap(2)
>>> + .with_child(
>>> + Field::new()
>>> + .input_type(InputType::Password)
>>> + .class(pwt::css::FlexFit)
>>> + .value(secret.clone())
>>> + .name("secret")
>>> + .disabled(true)
>>
>> When clicking the 'show secret' button, it is not possible to select the
>> secret via the cursor and copy it by 'hand' (Ctrl-C/right click context
>> menu), which is due to this field being disabled. I know there is a
>> 'copy secret' button, but this tripped me up for a moment when trying
>> this out.
>> Was this an intentional decision, e.g. to avoid mistakes when copying
>> the value or to discourage users from using the 'show secret' button
>> altogether?
>
> thanks for the review, this was actually discussed in a previous review.
> there i used a DisplayField, but that doesn't allow for hiding the
> contents like password fields do.
>
> i decided to mark this as `disabled` so that users won't edit it on
> accident, but allowing users to select it manually certainly makes
> sense.
>
Ah, alright, thanks for the explanation!
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 1/3] ui: add a token panel and a token acl edit menu in the permissions panel
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function Shannon Sterz
2025-10-14 14:37 ` [pdm-devel] [PATCH yew-comp v2 2/2] token panel: improve token secret dialog layout and hide password Shannon Sterz
@ 2025-10-14 14:37 ` Shannon Sterz
2025-10-17 9:36 ` Lukas Wagner
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 2/3] server: access: use token endpoints from proxmox-access-control Shannon Sterz
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-14 14:37 UTC (permalink / raw)
To: pdm-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager v2 1/3] ui: add a token panel and a token acl edit menu in the permissions panel
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 1/3] ui: add a token panel and a token acl edit menu in the permissions panel Shannon Sterz
@ 2025-10-17 9:36 ` Lukas Wagner
0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 9:36 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
Looks good to me:
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
> 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()
> },
> )
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 2/3] server: access: use token endpoints from proxmox-access-control
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
` (2 preceding siblings ...)
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 1/3] ui: add a token panel and a token acl edit menu in the permissions panel Shannon Sterz
@ 2025-10-14 14:37 ` Shannon Sterz
2025-10-17 9:36 ` Lukas Wagner
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 3/3] server: clean up acl tree entries and api tokens when deleting users Shannon Sterz
2025-10-17 12:48 ` [pdm-devel] Superseded: Re: [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
5 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-14 14:37 UTC (permalink / raw)
To: pdm-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager v2 2/3] server: access: use token endpoints from proxmox-access-control
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 2/3] server: access: use token endpoints from proxmox-access-control Shannon Sterz
@ 2025-10-17 9:36 ` Lukas Wagner
0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 9:36 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
LGTM!
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
> 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)];
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 3/3] server: clean up acl tree entries and api tokens when deleting users
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
` (3 preceding siblings ...)
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 2/3] server: access: use token endpoints from proxmox-access-control Shannon Sterz
@ 2025-10-14 14:37 ` Shannon Sterz
2025-10-17 9:36 ` Lukas Wagner
2025-10-17 12:48 ` [pdm-devel] Superseded: Re: [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
5 siblings, 1 reply; 15+ messages in thread
From: Shannon Sterz @ 2025-10-14 14:37 UTC (permalink / raw)
To: pdm-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager v2 3/3] server: clean up acl tree entries and api tokens when deleting users
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 3/3] server: clean up acl tree entries and api tokens when deleting users Shannon Sterz
@ 2025-10-17 9:36 ` Lukas Wagner
0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2025-10-17 9:36 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
Looks good to me:
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
On Tue Oct 14, 2025 at 4:37 PM CEST, 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();
> +
> + 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(())
> }
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pdm-devel] Superseded: Re: [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm
2025-10-14 14:37 [pdm-devel] [PATCH datacenter-manager/yew-comp v2 0/5] add better token support for pdm Shannon Sterz
` (4 preceding siblings ...)
2025-10-14 14:37 ` [pdm-devel] [PATCH datacenter-manager v2 3/3] server: clean up acl tree entries and api tokens when deleting users Shannon Sterz
@ 2025-10-17 12:48 ` Shannon Sterz
5 siblings, 0 replies; 15+ messages in thread
From: Shannon Sterz @ 2025-10-17 12:48 UTC (permalink / raw)
To: Shannon Sterz; +Cc: pdm-devel
On Tue Oct 14, 2025 at 4:37 PM CEST, Shannon Sterz wrote:
> 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 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 v1:
> - the patches for proxmox-access-control got applied (thanks @ Thomas
> Lamprecht)
> - use `.gap(2)` instead of `.margin_start(2)` (thanks @ Dominik Csapak)
> - use `ColorScheme::WarningContainer` and `pwt-default-color` instead of
> manually setting background and color classes (thanks @ Dominik
> Csapak)
> - rebased on current master branches
>
> 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-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:
> 6 files changed, 184 insertions(+), 395 deletions(-)
>
> --
> Generated by git-murpp 0.8.1
Superseded-by: https://lore.proxmox.com/pdm-devel/20251017124646.294343-2-s.sterz@proxmox.com/T/#t
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 15+ messages in thread