public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [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] [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] [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

* 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

* [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] 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

* [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] 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

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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal