* [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine
@ 2025-10-15 14:22 Dominik Csapak
2025-10-15 14:22 ` [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets Dominik Csapak
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Dominik Csapak @ 2025-10-15 14:22 UTC (permalink / raw)
To: pmg-devel
Fixes the login for ldap users of the quarantine by disabling the
realm selector on the login page, and by fixing the userid checking
in proxmox-login
note that the pmg-yew-quarantine-gui patch depends on both a bumped
proxmox-login and yew-comp for this to work correctly
proxmox:
Dominik Csapak (1):
login: fix userid check for '@quarantine' user tickets
proxmox-login/src/lib.rs | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
proxmox-yew-comp:
Dominik Csapak (1):
login panel: make realm selector optional
src/login_panel.rs | 52 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 10 deletions(-)
pmg-yew-quarantine-gui:
Dominik Csapak (1):
login page: disable realm selection
src/page_login.rs | 1 +
1 file changed, 1 insertion(+)
Summary over all repositories:
3 files changed, 48 insertions(+), 17 deletions(-)
--
Generated by git-murpp 0.8.1
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets 2025-10-15 14:22 [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Dominik Csapak @ 2025-10-15 14:22 ` Dominik Csapak 2025-10-21 18:33 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-15 14:22 ` [pmg-devel] [PATCH yew-comp 1/1] login panel: make realm selector optional Dominik Csapak ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2025-10-15 14:22 UTC (permalink / raw) To: pmg-devel when logging into the pmg quarantine via LDAP, the user typically enters a userid like 'foo@bar.com'. When receiving a valid ticket, this contains a userid like 'foo@bar.com@quarantine'. To check if that's correct, use our helper instead of manually checking for equality. That helper also needs fixing: while it should be (optionally) possible to enter the username with 'foo@bar.com@quarantine' (so we have to strip the quarantine part from the expected userid), we also have to strip the '@quarantine' part from the ticket response, since it contains that too. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- proxmox-login/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs index 4b2869a7..c67cd70b 100644 --- a/proxmox-login/src/lib.rs +++ b/proxmox-login/src/lib.rs @@ -58,7 +58,9 @@ fn normalize_url(mut api_url: String) -> String { } fn check_ticket_userid(ticket_userid: &str, expected_userid: &str) -> Result<(), ResponseError> { - if ticket_userid != expected_userid.trim_end_matches("@quarantine") { + if ticket_userid.trim_end_matches("@quarantine") + != expected_userid.trim_end_matches("@quarantine") + { return Err("returned ticket contained unexpected userid".into()); } Ok(()) @@ -186,9 +188,7 @@ impl Login { let response: api::ApiResponse<api::CreateTicketResponse> = serde_json::from_slice(body)?; let response = response.data.ok_or("missing response data")?; - if response.username != self.userid { - return Err("ticket response contained unexpected userid".into()); - } + check_ticket_userid(&response.username, &self.userid)?; // if a ticket was provided via a cookie, use it like a normal ticket if let Some(ticket) = cookie_ticket { @@ -380,9 +380,7 @@ impl SecondFactorChallenge { let response: api::ApiResponse<api::CreateTicketResponse> = serde_json::from_slice(body)?; let response = response.data.ok_or("missing response data")?; - if response.username != self.userid { - return Err("ticket response contained unexpected userid".into()); - } + check_ticket_userid(&response.username, &self.userid)?; // get the ticket from: // 1. the cookie if possible -> new HttpOnly authentication outside of the browser -- 2.47.3 _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pmg-devel] applied: [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets 2025-10-15 14:22 ` [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets Dominik Csapak @ 2025-10-21 18:33 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2025-10-21 18:33 UTC (permalink / raw) To: pmg-devel, Dominik Csapak On Wed, 15 Oct 2025 16:22:58 +0200, Dominik Csapak wrote: > when logging into the pmg quarantine via LDAP, the user typically > enters a userid like 'foo@bar.com'. When receiving a valid ticket, > this contains a userid like 'foo@bar.com@quarantine'. To check if that's > correct, use our helper instead of manually checking for equality. > > That helper also needs fixing: while it should be (optionally) possible > to enter the username with 'foo@bar.com@quarantine' (so we have to strip > the quarantine part from the expected userid), we also have to strip the > '@quarantine' part from the ticket response, since it contains that too. > > [...] Applied, with some actual unit tests added as follow-up as rust makes doing that so easy that it's hard to argue for not having them, thanks! [1/1] login: fix userid check for '@quarantine' user tickets commit: 7e23aa421fa5d7177fd2ff58d6d217ab6e7c8d80 _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pmg-devel] [PATCH yew-comp 1/1] login panel: make realm selector optional 2025-10-15 14:22 [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Dominik Csapak 2025-10-15 14:22 ` [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets Dominik Csapak @ 2025-10-15 14:22 ` Dominik Csapak 2025-10-22 17:03 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-15 14:23 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection Dominik Csapak 2025-10-20 15:03 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Stoiko Ivanov 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2025-10-15 14:22 UTC (permalink / raw) To: pmg-devel in some cases, we don't want to have a realm selector because the user has to enter e.g. an e-mail address instead (like the PMG mobile quarantine gui). Add a property for that that defaults to the current behavior and add a validator for the username field that checks for an '@' in the middle. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- src/login_panel.rs | 52 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/src/login_panel.rs b/src/login_panel.rs index f958871..8e6caa2 100644 --- a/src/login_panel.rs +++ b/src/login_panel.rs @@ -1,5 +1,7 @@ use std::rc::Rc; +use anyhow::bail; + use pwt::props::PwtSpace; use pwt::state::PersistentState; use pwt::touch::{SnackBar, SnackBarContextExt}; @@ -33,6 +35,11 @@ pub struct LoginPanel { #[builder] pub default_realm: AttrValue, + /// Determines if the realm box is shown/used + #[prop_or(true)] + #[builder] + pub realm_selectable: bool, + /// Mobile Layout /// /// Use special layout for mobile apps. For example shows error in a [SnackBar] @@ -176,6 +183,18 @@ impl ProxmoxLoginPanel { .label_id(username_label_id) .default(default_username) .required(true) + .validate({ + let realm_selectable = props.realm_selectable; + move |value: &String| { + if realm_selectable { + return Ok(()); + } else if let Some((user, realm)) = value.rsplit_once('@') { + if !user.is_empty() && !realm.is_empty() { + return Ok(()); + } + } + bail!("{}", tr!("invalid username")); + }}) .autofocus(true), ) .with_child( @@ -191,19 +210,19 @@ impl ProxmoxLoginPanel { .required(true) .input_type(InputType::Password), ) - .with_child( + .with_optional_child(props.realm_selectable.then_some( FieldLabel::new(tr!("Realm")) .id(realm_label_id.clone()) .padding_top(1) .padding_bottom(PwtSpace::Em(0.25)), - ) - .with_child( + )) + .with_optional_child(props.realm_selectable.then_some( RealmSelector::new() .name("realm") .label_id(realm_label_id) .path(props.domain_path.clone()) - .default(default_realm), - ) + .default(default_realm) + )) .with_child( SubmitButton::new() .class("pwt-scheme-primary") @@ -244,7 +263,7 @@ impl ProxmoxLoginPanel { let (default_username, default_realm) = self.get_defaults(props); - let input_panel = InputPanel::new() + let mut input_panel = InputPanel::new() .class(pwt::css::Overflow::Auto) .width("initial") // don't try to minimize size .padding(4) @@ -262,14 +281,17 @@ impl ProxmoxLoginPanel { .name("password") .required(true) .input_type(InputType::Password), - ) - .with_field( + ); + + if props.realm_selectable { + input_panel.add_field( tr!("Realm"), RealmSelector::new() .name("realm") .path(props.domain_path.clone()) .default(default_realm), ); + } let tfa_dialog = self.challenge.as_ref().map(|challenge| { TfaDialog::new(challenge.clone()) @@ -450,9 +472,19 @@ impl Component for ProxmoxLoginPanel { Msg::Submit => { self.loading = true; - let username = self.form_ctx.read().get_field_text("username"); let password = self.form_ctx.read().get_field_text("password"); - let realm = self.form_ctx.read().get_field_text("realm"); + let (username, realm) = if props.realm_selectable { + let username = self.form_ctx.read().get_field_text("username"); + let realm = self.form_ctx.read().get_field_text("realm"); + (username, realm) + } else { + self.form_ctx + .read() + .get_field_text("username") + .rsplit_once('@') + .map(|(user, realm)| (user.to_string(), realm.to_string())) + .unwrap_or_default() + }; self.send_login(ctx, username, password, realm); if let (true, Some(controller)) = (props.mobile, ctx.link().snackbar_controller()) { -- 2.47.3 _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pmg-devel] applied: [PATCH yew-comp 1/1] login panel: make realm selector optional 2025-10-15 14:22 ` [pmg-devel] [PATCH yew-comp 1/1] login panel: make realm selector optional Dominik Csapak @ 2025-10-22 17:03 ` Thomas Lamprecht 2025-10-23 6:54 ` Dominik Csapak 0 siblings, 1 reply; 9+ messages in thread From: Thomas Lamprecht @ 2025-10-22 17:03 UTC (permalink / raw) To: pmg-devel, Dominik Csapak On Wed, 15 Oct 2025 16:22:59 +0200, Dominik Csapak wrote: > in some cases, we don't want to have a realm selector because the user > has to enter e.g. an e-mail address instead (like the PMG mobile > quarantine gui). > > Add a property for that that defaults to the current behavior and add a > validator for the username field that checks for an '@' in the middle. > > [...] Applied but on top of Shannon's patches for which I had to solve some merge conflict, so would be great if you could recheck the changes, thanks! [1/1] login panel: make realm selector optional commit: 54c57fae4ce1e9db0b87faed02e46e3b5fa43a2f _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pmg-devel] applied: [PATCH yew-comp 1/1] login panel: make realm selector optional 2025-10-22 17:03 ` [pmg-devel] applied: " Thomas Lamprecht @ 2025-10-23 6:54 ` Dominik Csapak 0 siblings, 0 replies; 9+ messages in thread From: Dominik Csapak @ 2025-10-23 6:54 UTC (permalink / raw) To: Thomas Lamprecht, pmg-devel On 10/22/25 7:04 PM, Thomas Lamprecht wrote: > On Wed, 15 Oct 2025 16:22:59 +0200, Dominik Csapak wrote: >> in some cases, we don't want to have a realm selector because the user >> has to enter e.g. an e-mail address instead (like the PMG mobile >> quarantine gui). >> >> Add a property for that that defaults to the current behavior and add a >> validator for the username field that checks for an '@' in the middle. >> >> [...] > > Applied but on top of Shannon's patches for which I had to solve some merge > conflict, so would be great if you could recheck the changes, thanks! FYI: changes look good to me and tests fine here > > [1/1] login panel: make realm selector optional > commit: 54c57fae4ce1e9db0b87faed02e46e3b5fa43a2f _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pmg-devel] [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection 2025-10-15 14:22 [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Dominik Csapak 2025-10-15 14:22 ` [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets Dominik Csapak 2025-10-15 14:22 ` [pmg-devel] [PATCH yew-comp 1/1] login panel: make realm selector optional Dominik Csapak @ 2025-10-15 14:23 ` Dominik Csapak 2025-10-22 17:35 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-20 15:03 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Stoiko Ivanov 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2025-10-15 14:23 UTC (permalink / raw) To: pmg-devel since the user should not be able to select a realm (e.g. for ldap logins the e-mail address should be entered), disable that for the login panel. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- src/page_login.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/page_login.rs b/src/page_login.rs index 6449202..d285209 100644 --- a/src/page_login.rs +++ b/src/page_login.rs @@ -135,6 +135,7 @@ impl Component for PmgPageLogin { .with_child( LoginPanel::new() .mobile(true) + .realm_selectable(false) .domain_path("/access/auth-realm") .on_login(ctx.link().callback(Msg::Login)), ) -- 2.47.3 _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pmg-devel] applied: [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection 2025-10-15 14:23 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection Dominik Csapak @ 2025-10-22 17:35 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2025-10-22 17:35 UTC (permalink / raw) To: pmg-devel, Dominik Csapak On Wed, 15 Oct 2025 16:23:00 +0200, Dominik Csapak wrote: > since the user should not be able to select a realm (e.g. for ldap > logins the e-mail address should be entered), disable that for the login > panel. > > Applied, thanks! [1/1] login page: disable realm selection commit: 77631248f94ca03f17ac36c8307d082c65b98386 _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine 2025-10-15 14:22 [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Dominik Csapak ` (2 preceding siblings ...) 2025-10-15 14:23 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection Dominik Csapak @ 2025-10-20 15:03 ` Stoiko Ivanov 3 siblings, 0 replies; 9+ messages in thread From: Stoiko Ivanov @ 2025-10-20 15:03 UTC (permalink / raw) To: Dominik Csapak; +Cc: pmg-devel Thanks for the quick patch! I gave it a spin - and after our talk off-list I agree that the fix as is makes sense: Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com> Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com> On Wed, 15 Oct 2025 16:22:57 +0200 Dominik Csapak <d.csapak@proxmox.com> wrote: > Fixes the login for ldap users of the quarantine by disabling the > realm selector on the login page, and by fixing the userid checking > in proxmox-login > > note that the pmg-yew-quarantine-gui patch depends on both a bumped > proxmox-login and yew-comp for this to work correctly > > proxmox: > > Dominik Csapak (1): > login: fix userid check for '@quarantine' user tickets > > proxmox-login/src/lib.rs | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > > proxmox-yew-comp: > > Dominik Csapak (1): > login panel: make realm selector optional > > src/login_panel.rs | 52 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 10 deletions(-) > > > pmg-yew-quarantine-gui: > > Dominik Csapak (1): > login page: disable realm selection > > src/page_login.rs | 1 + > 1 file changed, 1 insertion(+) > > > Summary over all repositories: > 3 files changed, 48 insertions(+), 17 deletions(-) > _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-10-23 6:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-15 14:22 [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Dominik Csapak 2025-10-15 14:22 ` [pmg-devel] [PATCH proxmox 1/1] login: fix userid check for '@quarantine' user tickets Dominik Csapak 2025-10-21 18:33 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-15 14:22 ` [pmg-devel] [PATCH yew-comp 1/1] login panel: make realm selector optional Dominik Csapak 2025-10-22 17:03 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-23 6:54 ` Dominik Csapak 2025-10-15 14:23 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui 1/1] login page: disable realm selection Dominik Csapak 2025-10-22 17:35 ` [pmg-devel] applied: " Thomas Lamprecht 2025-10-20 15:03 ` [pmg-devel] [PATCH pmg-yew-quarantine-gui/proxmox/yew-comp 0/3] fix ldap login for pmg mobile quarantine Stoiko Ivanov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox