[PATCH proxmox/yew-comp 0/2] fix #8175: allow LDAP alias in pmgs mobile quarantine interface

[PATCH proxmox/yew-comp 0/2] fix #8175: allow LDAP alias in pmgs mobile quarantine interface

[Dominik Csapak]

See the patches commit messages for details.

Note that we need to bump proxmox-login for this, proxmox-yew-comp needs a
bumped dep on proxmox-login, and we have to rebuild pmg-yew-quarantine-gui
with the new proxmox-yew-comp for this to take effect.


proxmox:

Dominik Csapak (1):
  login: add option to allow arbitrary userids in tickets

 proxmox-login/src/lib.rs | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)


proxmox-yew-comp:

Dominik Csapak (1):
  fix #8175: http client: allow different userids in tickets for PMG

 src/http_client_wasm.rs | 10 ++++++++--
 src/proxmox_product.rs  | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)


Summary over all repositories:
  3 files changed, 39 insertions(+), 20 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH proxmox 1/2] login: add option to allow arbitrary userids in tickets

[Dominik Csapak]

In some cases the ticket does not contain the userid given to the
backend, but a different one. This happens for example in PMG's
quarantine with LDAP login. There a user is allowed to enter an alias to
his mail address, but the ticket still contains the primary e-mail as
userid.

To allow this, add an option to the `Login` struct to disable the userid
check and expose a setter for that.

With this, we can remove the `@quarantine` workaround here since we
don't check the username at all anymore for the PMG quarantine.

This should not have any impact on security, since the backend must not
give out tickets to wrong users and any client could choose to ignore
this anyway.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 proxmox-login/src/lib.rs | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 0b290d20..f460b48c 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -50,6 +50,7 @@ pub struct Login {
     userid: String,
     password: Option<String>,
     pve_compat: bool,
+    ignore_userid: bool,
 }
 
 fn normalize_url(mut api_url: String) -> String {
@@ -58,9 +59,7 @@ fn normalize_url(mut api_url: String) -> String {
 }
 
 fn check_ticket_userid(ticket_userid: &str, expected_userid: &str) -> Result<(), ResponseError> {
-    if ticket_userid.trim_end_matches("@quarantine")
-        != expected_userid.trim_end_matches("@quarantine")
-    {
+    if ticket_userid != expected_userid {
         return Err("returned ticket contained unexpected userid".into());
     }
     Ok(())
@@ -90,6 +89,12 @@ impl Login {
         &self.api_url
     }
 
+    /// When calling this, the ticket check allows a different userid to be returned than
+    /// what was originally supplied.
+    pub fn ignore_userid(&mut self) {
+        self.ignore_userid = true;
+    }
+
     /// Prepare a request given an already parsed ticket.
     pub fn renew_ticket(api_url: impl Into<String>, ticket: Ticket) -> Self {
         Self {
@@ -97,6 +102,7 @@ impl Login {
             pve_compat: ticket.product() == "PVE",
             userid: ticket.userid().to_string(),
             password: Some(ticket.into()),
+            ignore_userid: false,
         }
     }
 
@@ -108,6 +114,7 @@ impl Login {
             pve_compat: false,
             userid: userid.into(),
             password: None,
+            ignore_userid: false,
         }
     }
 
@@ -122,6 +129,7 @@ impl Login {
             userid: userid.into(),
             password: Some(password.into()),
             pve_compat: false,
+            ignore_userid: false,
         }
     }
 
@@ -188,11 +196,11 @@ impl Login {
         let response: api::ApiResponse<api::CreateTicketResponse> = serde_json::from_slice(body)?;
         let response = response.data.ok_or("missing response data")?;
 
-        check_ticket_userid(&response.username, &self.userid)?;
+        self.check_ticket_userid(&response.username)?;
 
         // if a ticket was provided via a cookie, use it like a normal ticket
         if let Some(ticket) = cookie_ticket {
-            check_ticket_userid(ticket.userid(), &self.userid)?;
+            self.check_ticket_userid(ticket.userid())?;
             return Ok(TicketResult::Full(
                 self.authentication_for(ticket, response)?,
             ));
@@ -220,7 +228,7 @@ impl Login {
 
         Ok(match ticket {
             TicketResponse::Full(ticket) => {
-                check_ticket_userid(ticket.userid(), &self.userid)?;
+                self.check_ticket_userid(ticket.userid())?;
                 TicketResult::Full(self.authentication_for(ticket, response)?)
             }
 
@@ -251,6 +259,13 @@ impl Login {
             ticket,
         })
     }
+
+    fn check_ticket_userid(&self, ticket_userid: &str) -> Result<(), ResponseError> {
+        if self.ignore_userid {
+            return Ok(());
+        }
+        check_ticket_userid(ticket_userid, &self.userid)
+    }
 }
 
 /// This is the result of a ticket call. It will either yield a final ticket, or a TFA challenge.
@@ -420,18 +435,6 @@ mod test {
         assert!(check_ticket_userid("root@pam", "root@pam").is_ok()); // trivially equal
 
         assert!(check_ticket_userid("root@pam", "test@pam").is_err()); // trivially non-equal
-
-        // some special cases for PMG quarantine
-        assert!(check_ticket_userid("user@ldap@quarantine", "user@ldap@quarantine").is_ok());
-        assert!(check_ticket_userid("user@ldap@quarantine", "user@ldap").is_ok());
-        assert!(check_ticket_userid("user@ldap", "user@ldap@quarantine").is_ok());
-
-        // test odd realm name
-        assert!(
-            check_ticket_userid("user@quarantine@quarantine", "user@quarantine@quarantine").is_ok()
-        );
-        assert!(check_ticket_userid("user@quarantine", "user@quarantine@quarantine").is_ok());
-        assert!(check_ticket_userid("user@quarantine", "user@ldap@quarantine").is_err());
     }
 
     #[test]
-- 
2.47.3

[PATCH yew-comp 2/2] fix #8175: http client: allow different userids in tickets for PMG

[Dominik Csapak]

In PMG's quarantine, the userid supplied is not the one returned by the
ticket (e.g. an alias in LDAP or simply the '@quarantine' suffix). Use
the new 'ignore_userid' flag in proxmox-login to disable the userid
check there, but only for PMG.

By providing a default implementation (that keeps the default behavior),
it isn't necessary to adapt any other projects.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/http_client_wasm.rs | 10 ++++++++--
 src/proxmox_product.rs  | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/http_client_wasm.rs b/src/http_client_wasm.rs
index c8bdecf..53423d4 100644
--- a/src/http_client_wasm.rs
+++ b/src/http_client_wasm.rs
@@ -134,7 +134,10 @@ impl HttpClientWasm {
         let username = username.into();
         let password = password.into();
 
-        let login = Login::new("", username, password);
+        let mut login = Login::new("", username, password);
+        if self.project.ticket_ignore_userid() {
+            login.ignore_userid();
+        }
         let request = login.request();
         let request =
             Self::post_request_builder(&request.url, request.content_type, &request.body)?;
@@ -146,7 +149,10 @@ impl HttpClientWasm {
     pub async fn refresh(&self, username: impl Into<String>) -> Result<TicketResult, Error> {
         let username = username.into();
 
-        let login = Login::renew_with_cookie("", username);
+        let mut login = Login::renew_with_cookie("", username);
+        if self.project.ticket_ignore_userid() {
+            login.ignore_userid();
+        }
         let request = login.request();
         let request =
             Self::post_request_builder(&request.url, request.content_type, &request.body)?;
diff --git a/src/proxmox_product.rs b/src/proxmox_product.rs
index 0e81cc8..8267039 100644
--- a/src/proxmox_product.rs
+++ b/src/proxmox_product.rs
@@ -20,6 +20,12 @@ pub trait ProjectInfo {
     fn subscription_url(&self) -> &'static str {
         ""
     }
+
+    /// Returns if the product needs to ignore mismatching userids in tickets.
+    /// This is useful when doing LDAP login with email aliases (like in the PMG quarantine)
+    fn ticket_ignore_userid(&self) -> bool {
+        false
+    }
 }
 
 /// Enumerate the different Proxmox products.
@@ -76,4 +82,8 @@ impl ProjectInfo for ExistingProduct {
     fn subscription_url(&self) -> &'static str {
         "/nodes/localhost/subscription"
     }
+
+    fn ticket_ignore_userid(&self) -> bool {
+        matches!(self, ExistingProduct::PMG)
+    }
 }
-- 
2.47.3