[pbs-devel] [PATCH proxmox] login: use `ticket` if both it and `ticket_info` are provided

* [pbs-devel] [PATCH proxmox] login: use `ticket` if both it and `ticket_info` are provided
@ 2025-05-20  8:55 Shannon Sterz
  2025-10-01 13:28 ` [pbs-devel] Superseded: " Shannon Sterz
  0 siblings, 1 reply; 2+ messages in thread
From: Shannon Sterz @ 2025-05-20  8:55 UTC (permalink / raw)
  To: pbs-devel

previously the precense of `ticket_info` was assumed to indicate the
HTTPOnly authentication flow. the `ticket` field was ignore in that
case, because the client has no way of validating a ticket anyway.

this commit changes the behaviour to assume that the server is not
trying to "trick us" and that the presence of a `ticket` field
indicates that this value should be used for authentication. if the
`ticket_info` field is also present, the field will be ignored.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---

this is basically a different approach to fixing an issue that occured
when a server also returned a `ticket_info` field but was still using
the non-HTTPOnly auth flow [1]. while this shouldn't be strictly
necessary, it shouldn't hurt either.

[1]: https://lore.proxmox.com/all/20250516131122.276231-1-s.sterz@proxmox.com/

 proxmox-login/src/lib.rs | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index e97ece7b..710434d6 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -200,22 +200,24 @@ impl Login {
             ));
         }

-        // `ticket_info` is set when the server sets the ticket via an HttpOnly cookie. this also
-        // means we do not have access to the cookie itself which happens for example in a browser.
-        // assume that the cookie is handled properly by the context (browser) and don't worry
-        // about handling it ourselves.
-        if let Some(ref ticket) = response.ticket_info {
-            let ticket = ticket.parse()?;
-            return Ok(TicketResult::HttpOnly(
-                self.authentication_for(ticket, response)?,
-            ));
-        }
-
         // old authentication flow where we needed to handle the ticket ourselves even in the
         // browser etc.
         let ticket: TicketResponse = match response.ticket {
             Some(ref ticket) => ticket.parse()?,
-            None => return Err("no ticket information in response".into()),
+            None => {
+                // `ticket_info` is set when the server sets the ticket via a HttpOnly cookie. this
+                // also means we do not have access to the cookie itself which happens for example
+                // in a browser. assume that the cookie is handled properly by the context
+                // (browser) and don't worry about handling it ourselves.
+                if let Some(ref ticket) = response.ticket_info {
+                    let ticket = ticket.parse()?;
+                    return Ok(TicketResult::HttpOnly(
+                        self.authentication_for(ticket, response)?,
+                    ));
+                }
+
+                return Err("no ticket information in response".into());
+            }
         };

         Ok(match ticket {
--
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread