From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 457871FF16B for ; Fri, 10 Oct 2025 14:36:55 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 86E191422A; Fri, 10 Oct 2025 14:37:02 +0200 (CEST) Mime-Version: 1.0 Date: Fri, 10 Oct 2025 14:36:59 +0200 Message-Id: To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20251003142108.352525-1-s.sterz@proxmox.com> <20251003142108.352525-6-s.sterz@proxmox.com> <9c4d7d25-4343-4465-99d6-d4046658d521@proxmox.com> In-Reply-To: <9c4d7d25-4343-4465-99d6-d4046658d521@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760099786249 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pdm-devel] [PATCH yew-comp 2/2] token panel: improve token secret dialog layout and hide password X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Fri Oct 10, 2025 at 2:16 PM CEST, Dominik Csapak wrote: > one tiny nit, no blocker from my side though > > otherwise consider this: > > Reviewed-by: Dominik Csapak > > On 10/3/25 4:21 PM, 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 >> --- >> 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, 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) > > i'd probably just use a `.gap(2)` on the row above > >> + .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") > > would prefer to have that written as > > .class(ColorScheme::WarningContainer) > .class("pwt-default-colors") // this should also be a rust helper > > it's not your issue to fix, since you just moved the code here, but i'd > like to reduce the 'hand-written' css classes as much as possible. > > it's not an issue here, just noting it for future patches > that's fair and i wanted to use `ColorScheme::WarningContainer` here anyway, but didn't know about `pwt-default-colors` which sets the css variables in the correct places. so it just didn't work. thanks for that hint! grepped over our code, but it only seems to be used in pmg-yew-quarantine so far, so that would explain why i couldn't find it. can send i v2 with those two changes folded in if desired, though. >> + .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 mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel