all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal