[pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Christian Ebner]

This patches fix the ticket parsing when login to instances of PBS version 3
or lower. For this, the current pve_compat flags for `Login` and `Client`
are refactored to be an extendable enum variant instead, adding the ticket
parsing backwards compatibility. In that compatibility mode, response parsing
of the ticket does not interpret the presence of the `ticket-info` field as the
ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
compatible parsing, as the client never used `http-only` tickets.

Since this fixes token generation and ACLs setting via the remote add wizard,
revert the now outdated warning of PBS v4 minimum requirement.

Changes since version 1:
- Fix check for CompatMode::Pbs3Ticket in response ticket parsing, must skip the
  http-only path.
- Set the compatibility mode on the client, not the login. The client propagates
  the compat mode to the login on login() calls. Since that incorrectly set the
  state, the PBS logic worked nevertheless during testing, but also affected the
  PVE state, which was not intended.

proxmox:

Christian Ebner (3):
  proxmox-login: refactor PVE TFA compat mode
  proxmox-client: adapt to new compat mode introduced for proxmox-login
  proxmox-login: add compat mode to fallback to PBS3 ticket parsing

 proxmox-client/src/client.rs | 12 ++++----
 proxmox-client/src/lib.rs    |  2 +-
 proxmox-login/src/lib.rs     | 55 +++++++++++++++++++++++++-----------
 3 files changed, 45 insertions(+), 24 deletions(-)


datacenter-manager:

Christian Ebner (3):
  server: adapt to proxmox-client compat mode changes
  server: pbs-client: check and fallback to PBS v3 ticket compat mode
  Revert "ui: add wizard: note that login currently only works for PBS
    4"

 server/src/connection.rs           | 31 +++++++++++++++++++-----------
 ui/src/remotes/wizard_page_info.rs | 15 ---------------
 2 files changed, 20 insertions(+), 26 deletions(-)


Summary over all repositories:
  5 files changed, 65 insertions(+), 50 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pdm-devel] [PATCH proxmox v2 1/3] proxmox-login: refactor PVE TFA compat mode

[Christian Ebner]

In preparation for extending the compat mode by Proxmox Backup Server
version 3 API compatibility for login ticket parsing.

Instead of storing the compat mode as simple boolean flag and only
considering PVE, use a `CompatMode` enum instead. Further, make the
variable and method names more generic.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

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

diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 4482f2e4..e7b8e023 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -49,7 +49,15 @@ pub struct Login {
     api_url: String,
     userid: String,
     password: Option<String>,
-    pve_compat: bool,
+    compat_mode: CompatMode,
+}
+
+/// Compatibility mode to use for login or ticket renewal.
+#[derive(Copy, Clone, Debug, Default, PartialEq)]
+pub enum CompatMode {
+    #[default]
+    Generic,
+    PveTfa,
 }
 
 fn normalize_url(mut api_url: String) -> String {
@@ -90,9 +98,14 @@ impl Login {
 
     /// Prepare a request given an already parsed ticket.
     pub fn renew_ticket(api_url: impl Into<String>, ticket: Ticket) -> Self {
+        let compat_mode = if ticket.product() == "PVE" {
+            CompatMode::PveTfa
+        } else {
+            CompatMode::default()
+        };
         Self {
             api_url: normalize_url(api_url.into()),
-            pve_compat: ticket.product() == "PVE",
+            compat_mode,
             userid: ticket.userid().to_string(),
             password: Some(ticket.into()),
         }
@@ -103,7 +116,7 @@ impl Login {
     pub fn renew_with_cookie(api_url: impl Into<String>, userid: impl Into<String>) -> Self {
         Self {
             api_url: normalize_url(api_url.into()),
-            pve_compat: false,
+            compat_mode: CompatMode::default(),
             userid: userid.into(),
             password: None,
         }
@@ -119,13 +132,13 @@ impl Login {
             api_url: normalize_url(api_url.into()),
             userid: userid.into(),
             password: Some(password.into()),
-            pve_compat: false,
+            compat_mode: CompatMode::default(),
         }
     }
 
     /// Set the Proxmox VE compatibility parameter for Two-Factor-Authentication support.
-    pub fn pve_compatibility(mut self, compatibility: bool) -> Self {
-        self.pve_compat = compatibility;
+    pub fn set_compatibility(mut self, compat_mode: CompatMode) -> Self {
+        self.compat_mode = compat_mode;
         self
     }
 
@@ -135,8 +148,13 @@ impl Login {
     /// [`response`](Login::response) method in order to extract the validated ticket or
     /// Two-Factor-Authentication challenge.
     pub fn request(&self) -> Request {
+        let new_format = if self.compat_mode == CompatMode::PveTfa {
+            Some(true)
+        } else {
+            None
+        };
         let request = api::CreateTicket {
-            new_format: self.pve_compat.then_some(true),
+            new_format,
             username: self.userid.clone(),
             password: self.password.clone(),
             ..Default::default()
@@ -225,7 +243,7 @@ impl Login {
             TicketResponse::Tfa(ticket, challenge) => {
                 TicketResult::TfaRequired(SecondFactorChallenge {
                     api_url: self.api_url.clone(),
-                    pve_compat: self.pve_compat,
+                    pve_compat: self.compat_mode == CompatMode::PveTfa,
                     userid: response.username,
                     ticket,
                     challenge,
-- 
2.47.3



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

[pdm-devel] [PATCH proxmox v2 2/3] proxmox-client: adapt to new compat mode introduced for proxmox-login

[Christian Ebner]

The API for setting the compatibility changed to allow for easier
extension. Adapt the client to the new interface and expose the same
changes to the client.

Also, re-export the `CompatMode` so users do not need to directly
depend on the implementation in proxmox-login.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 proxmox-client/src/client.rs | 12 ++++++------
 proxmox-client/src/lib.rs    |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/proxmox-client/src/client.rs b/proxmox-client/src/client.rs
index da2c5c59..894d9216 100644
--- a/proxmox-client/src/client.rs
+++ b/proxmox-client/src/client.rs
@@ -17,7 +17,7 @@ use serde::Serialize;
 
 use proxmox_http::Body;
 use proxmox_login::ticket::Validity;
-use proxmox_login::{Login, SecondFactorChallenge, TicketResult};
+use proxmox_login::{CompatMode, Login, SecondFactorChallenge, TicketResult};
 
 use crate::auth::AuthenticationKind;
 use crate::error::ParseFingerprintError;
@@ -68,7 +68,7 @@ pub struct Client {
     api_url: Uri,
     auth: Mutex<Option<Arc<AuthenticationKind>>>,
     client: Arc<proxmox_http::client::Client>,
-    pve_compat: bool,
+    compat_mode: CompatMode,
     cookie_name: Option<String>,
 }
 
@@ -91,7 +91,7 @@ impl Client {
             api_url,
             auth: Mutex::new(None),
             client,
-            pve_compat: false,
+            compat_mode: CompatMode::default(),
             cookie_name: None,
         }
     }
@@ -181,8 +181,8 @@ impl Client {
 
     /// Enable Proxmox VE login API compatibility. This is required to support TFA authentication
     /// on Proxmox VE APIs which require the `new-format` option.
-    pub fn set_pve_compatibility(&mut self, compatibility: bool) {
-        self.pve_compat = compatibility;
+    pub fn set_compatibility(&mut self, compat_mode: CompatMode) {
+        self.compat_mode = compat_mode;
     }
 
     pub fn set_cookie_name(&mut self, cookie_name: &str) {
@@ -418,7 +418,7 @@ impl Client {
     /// If the authentication is complete, `None` is returned and the authentication state updated.
     /// If a 2nd factor is required, `Some` is returned.
     pub async fn login(&self, login: Login) -> Result<Option<SecondFactorChallenge>, Error> {
-        let login = login.pve_compatibility(self.pve_compat);
+        let login = login.set_compatibility(self.compat_mode);
 
         let (ticket, api_response) = self.do_login_request(login.request()).await?;
 
diff --git a/proxmox-client/src/lib.rs b/proxmox-client/src/lib.rs
index f1df1e1d..3bac1333 100644
--- a/proxmox-client/src/lib.rs
+++ b/proxmox-client/src/lib.rs
@@ -12,7 +12,7 @@ mod error;
 pub use error::Error;
 
 pub use proxmox_login::tfa::TfaChallenge;
-pub use proxmox_login::{Authentication, Ticket};
+pub use proxmox_login::{CompatMode, Authentication, Ticket};
 
 mod api_path_builder;
 pub use api_path_builder::ApiPathBuilder;
-- 
2.47.3



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

[pdm-devel] [PATCH proxmox v2 3/3] proxmox-login: add compat mode to fallback to PBS3 ticket parsing

[Christian Ebner]

The ticket info field is present for Proxmox Backup Server version 3
login response data, the client however switches to http-only in this
case.

Allow to use the old authentication workflow instead by extending the
compat mode by `Pbs3Ticket`, resulting in a full ticket instead of
the http-only one.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- fix check for CompatMode::Pbs3Ticket not being set

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

diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index e7b8e023..c919c6a0 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -58,6 +58,7 @@ pub enum CompatMode {
     #[default]
     Generic,
     PveTfa,
+    Pbs3Ticket,
 }
 
 fn normalize_url(mut api_url: String) -> String {
@@ -216,15 +217,17 @@ 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)?,
-            ));
+        if self.compat_mode != CompatMode::Pbs3Ticket {
+            // `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
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v2 1/3] server: adapt to proxmox-client compat mode changes

[Christian Ebner]

Adapt to the method name and type changes for setting the client
login compatibility mode.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 server/src/connection.rs | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/server/src/connection.rs b/server/src/connection.rs
index 2eda452..5530812 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -19,7 +19,9 @@ use openssl::x509::X509StoreContextRef;
 use serde::Serialize;
 
 use proxmox_acme_api::CertificateInfo;
-use proxmox_client::{Client, HttpApiClient, HttpApiResponse, HttpApiResponseStream, TlsOptions};
+use proxmox_client::{
+    Client, CompatMode, HttpApiClient, HttpApiResponse, HttpApiResponseStream, TlsOptions,
+};
 
 use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, TlsProbeOutcome};
 use pve_api_types::client::PveClientImpl;
@@ -32,21 +34,21 @@ static INSTANCE: OnceLock<Box<dyn ClientFactory + Send + Sync>> = OnceLock::new(
 struct ConnectInfo {
     prefix: String,
     perl_compat: bool,
-    pve_compat: bool,
+    compat_mode: CompatMode,
     default_port: u16,
 }
 
 impl ConnectInfo {
     fn for_remote(remote: &Remote) -> Self {
-        let (prefix, perl_compat, pve_compat) = match remote.ty {
-            RemoteType::Pve => ("PVEAPIToken".to_string(), true, true),
-            RemoteType::Pbs => ("PBSAPIToken".to_string(), false, false),
+        let (prefix, perl_compat, compat_mode) = match remote.ty {
+            RemoteType::Pve => ("PVEAPIToken".to_string(), true, CompatMode::PveTfa),
+            RemoteType::Pbs => ("PBSAPIToken".to_string(), false, CompatMode::Generic),
         };
 
         ConnectInfo {
             prefix,
             perl_compat,
-            pve_compat,
+            compat_mode,
             default_port: remote.ty.default_port(),
         }
     }
@@ -56,7 +58,7 @@ impl ConnectInfo {
 fn prepare_connect_client_to_node(
     node: &NodeUrl,
     default_port: u16,
-    pve_compat: bool,
+    compat_mode: CompatMode,
 ) -> Result<Client, Error> {
     let mut options = TlsOptions::default();
 
@@ -75,7 +77,7 @@ fn prepare_connect_client_to_node(
 
     let mut client =
         proxmox_client::Client::with_options(uri.clone(), options, Default::default())?;
-    client.set_pve_compatibility(pve_compat);
+    client.set_compatibility(compat_mode);
     Ok(client)
 }
 
@@ -99,7 +101,7 @@ fn prepare_connect_client(
 
     let info = ConnectInfo::for_remote(remote);
 
-    let client = prepare_connect_client_to_node(node, info.default_port, info.pve_compat)?;
+    let client = prepare_connect_client_to_node(node, info.default_port, info.compat_mode)?;
 
     Ok((client, info))
 }
@@ -135,7 +137,7 @@ fn prepare_connect_multi_client(remote: &Remote) -> Result<(MultiClient, Connect
             client: Arc::new(prepare_connect_client_to_node(
                 node,
                 info.default_port,
-                info.pve_compat,
+                info.compat_mode,
             )?),
             hostname: node.hostname.clone(),
         });
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v2 2/3] server: pbs-client: check and fallback to PBS v3 ticket compat mode

[Christian Ebner]

Since the proxmox-login ticket parsing assumes the ticket to be
http-only if it contains the ticket-info field, but the PBS v3 API
does return that in any case, signal the client to fallback to the
old authentication flow.

This is currently only used during adding of a new remote, namely to
scan the remote and login for PBS API token creation/setting of its
ACLs.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- set the compat mode on the client, not the login, as the client will
  propagate it to the login not vice versa.

 server/src/connection.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/server/src/connection.rs b/server/src/connection.rs
index 5530812..980ef01 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -178,7 +178,14 @@ async fn connect_or_login(
     if remote.authid.is_token() {
         connect(remote, target_endpoint)
     } else {
-        let (client, _info) = prepare_connect_client(remote, target_endpoint)?;
+        let (mut client, _info) = prepare_connect_client(remote, target_endpoint)?;
+
+        //FIXME: drop once PBS3 is EOL
+        if remote.ty == RemoteType::Pbs {
+            // Forces both, PBSv4 and PBSv3 to use the same PBS3 compat login ticket parsing
+            client.set_compatibility(CompatMode::Pbs3Ticket);
+        }
+
         match client
             .login(proxmox_login::Login::new(
                 client.api_url().to_string(),
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v2 3/3] Revert "ui: add wizard: note that login currently only works for PBS 4"

[Christian Ebner]

This reverts commit f9b6ba357f52a2cfd122e1318dc547b742c88541.

By setting the `Pbs3Ticket` compat mode when login to PBS version 3
instances, the token creation and setting of permissions now works as
intended also for older versions.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 ui/src/remotes/wizard_page_info.rs | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index b6ad5b3..7fffdd1 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -379,21 +379,6 @@ impl Component for PdmWizardPageInfo {
         let content = Column::new()
             .class(FlexFit)
             .with_child(Row::new().with_child(input_panel))
-            .with_child(
-                Row::new()
-                    .padding(2)
-                    .gap(2)
-                    .class(css::AlignItems::Center)
-                    .with_optional_child(
-                        if props.remote_type == RemoteType::Pbs && self.user_mode {
-                            Some(tr!(
-                                "Note: Login currently requires Proxmox Backup Server version 4."
-                            ))
-                        } else {
-                            None
-                        },
-                    ),
-            )
             .with_child(
                 Row::new()
                     .padding(2)
-- 
2.47.3



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

Re: [pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Wolfgang Bumiller]

On Tue, Sep 30, 2025 at 10:02:01AM +0200, Christian Ebner wrote:
> This patches fix the ticket parsing when login to instances of PBS version 3
> or lower. For this, the current pve_compat flags for `Login` and `Client`
> are refactored to be an extendable enum variant instead, adding the ticket
> parsing backwards compatibility. In that compatibility mode, response parsing
> of the ticket does not interpret the presence of the `ticket-info` field as the
> ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
> compatible parsing, as the client never used `http-only` tickets.

I thought this field was introduced only with http-only tickets, so why
is it not a reliable indicator and shouldn't *that* be fixed instead?

> 
> Since this fixes token generation and ACLs setting via the remote add wizard,
> revert the now outdated warning of PBS v4 minimum requirement.
> 
> Changes since version 1:
> - Fix check for CompatMode::Pbs3Ticket in response ticket parsing, must skip the
>   http-only path.
> - Set the compatibility mode on the client, not the login. The client propagates
>   the compat mode to the login on login() calls. Since that incorrectly set the
>   state, the PBS logic worked nevertheless during testing, but also affected the
>   PVE state, which was not intended.
> 
> proxmox:
> 
> Christian Ebner (3):
>   proxmox-login: refactor PVE TFA compat mode
>   proxmox-client: adapt to new compat mode introduced for proxmox-login
>   proxmox-login: add compat mode to fallback to PBS3 ticket parsing
> 
>  proxmox-client/src/client.rs | 12 ++++----
>  proxmox-client/src/lib.rs    |  2 +-
>  proxmox-login/src/lib.rs     | 55 +++++++++++++++++++++++++-----------
>  3 files changed, 45 insertions(+), 24 deletions(-)
> 
> 
> datacenter-manager:
> 
> Christian Ebner (3):
>   server: adapt to proxmox-client compat mode changes
>   server: pbs-client: check and fallback to PBS v3 ticket compat mode
>   Revert "ui: add wizard: note that login currently only works for PBS
>     4"
> 
>  server/src/connection.rs           | 31 +++++++++++++++++++-----------
>  ui/src/remotes/wizard_page_info.rs | 15 ---------------
>  2 files changed, 20 insertions(+), 26 deletions(-)
> 
> 
> Summary over all repositories:
>   5 files changed, 65 insertions(+), 50 deletions(-)
> 
> -- 
> Generated by git-murpp 0.8.1


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

Re: [pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Christian Ebner]

On 9/30/25 1:49 PM, Wolfgang Bumiller wrote:
> On Tue, Sep 30, 2025 at 10:02:01AM +0200, Christian Ebner wrote:
>> This patches fix the ticket parsing when login to instances of PBS version 3
>> or lower. For this, the current pve_compat flags for `Login` and `Client`
>> are refactored to be an extendable enum variant instead, adding the ticket
>> parsing backwards compatibility. In that compatibility mode, response parsing
>> of the ticket does not interpret the presence of the `ticket-info` field as the
>> ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
>> compatible parsing, as the client never used `http-only` tickets.
> 
> I thought this field was introduced only with http-only tickets, so why
> is it not a reliable indicator and shouldn't *that* be fixed instead?

I'm not so familiar with the actual code and the changes over time here, 
so that might as well be the case.

AFAIK there are however PBS 3.x versions out there which do return the 
field also on non http-only tickets, the changes introduced by [0] back 
in March. So it only would be required to backport/build based on this 
[1] patch for stable-3 then?

However, older patch level versions would still not work regardless.

[0] 
https://git.proxmox.com/?p=proxmox.git;a=commit;h=6a7f6317093285a1494c4ebace71c71697058587
[1] 
https://git.proxmox.com/?p=proxmox.git;a=commit;h=8d01d274231f5b7172b519104a8042f92ace8e1e


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

Re: [pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Thomas Lamprecht]

Am 30.09.25 um 14:19 schrieb Christian Ebner:
> On 9/30/25 1:49 PM, Wolfgang Bumiller wrote:
>> On Tue, Sep 30, 2025 at 10:02:01AM +0200, Christian Ebner wrote:
>>> This patches fix the ticket parsing when login to instances of PBS version 3
>>> or lower. For this, the current pve_compat flags for `Login` and `Client`
>>> are refactored to be an extendable enum variant instead, adding the ticket
>>> parsing backwards compatibility. In that compatibility mode, response parsing
>>> of the ticket does not interpret the presence of the `ticket-info` field as the
>>> ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
>>> compatible parsing, as the client never used `http-only` tickets.
>>
>> I thought this field was introduced only with http-only tickets, so why
>> is it not a reliable indicator and shouldn't *that* be fixed instead?
> 
> I'm not so familiar with the actual code and the changes over time here, so that might as well be the case.

FYI: Shannon is back at work tomorrow and this is not really _that_
pressing, so I'd wait at least until tomorrow to hopefully get some
better rationale.



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

Re: [pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Shannon Sterz]

On Tue Sep 30, 2025 at 2:34 PM CEST, Thomas Lamprecht wrote:
> Am 30.09.25 um 14:19 schrieb Christian Ebner:
>> On 9/30/25 1:49 PM, Wolfgang Bumiller wrote:
>>> On Tue, Sep 30, 2025 at 10:02:01AM +0200, Christian Ebner wrote:
>>>> This patches fix the ticket parsing when login to instances of PBS version 3
>>>> or lower. For this, the current pve_compat flags for `Login` and `Client`
>>>> are refactored to be an extendable enum variant instead, adding the ticket
>>>> parsing backwards compatibility. In that compatibility mode, response parsing
>>>> of the ticket does not interpret the presence of the `ticket-info` field as the
>>>> ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
>>>> compatible parsing, as the client never used `http-only` tickets.
>>>
>>> I thought this field was introduced only with http-only tickets, so why
>>> is it not a reliable indicator and shouldn't *that* be fixed instead?
>>
>> I'm not so familiar with the actual code and the changes over time here, so that might as well be the case.
>
> FYI: Shannon is back at work tomorrow and this is not really _that_
> pressing, so I'd wait at least until tomorrow to hopefully get some
> better rationale.

hi everyone and sorry if i am missing something (still catching up on
mail), but i think this is basically a more in-depth approach to a fix i
send a while back:

https://lore.proxmox.com/pbs-devel/20250520085549.56525-1-s.sterz@proxmox.com/

as chris has already pointed out in chat, there was a mishap on my end
when refactoring the auth api and the old authentication flow would
still send a `ticket-info` field alongside the `ticket` field.
proxmox-login would then think it is in the new HttpOnly flow and not
use the `ticket` field, even though it should.

this was already fixed in commit f7d8b8f682 (auth-api: remove ticket
info in old create ticket endpoint) [1], but it seems at least
proxmox-backup-server 3.4.6-1 still sends the `ticket-info` as well. i
rebuild pbs from latest stable-3 and stable-bookworm and there the
parameter is correctly dropped.

imo clients *should* use a ticket if they are provided with one and not
get confused with additional parameters. which is what the patch linked
above does. however, chris' approach to compatibility is a lot more
extensible, so we could go down that road too.

i'll check if my patch above works as inteded still and can resend a
rebased version later today.

[1]: https://git.proxmox.com/?p=proxmox.git;a=commitdiff;h=f7d8b8f682370cf0d8c3a0a238c958ceda2b8f7b


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

Re: [pdm-devel] [PATCH datacenter-manager/proxmox v2 0/6] pbs client: fix PBS version 3 login ticket parsing compatibility

[Shannon Sterz]

On Wed Oct 1, 2025 at 11:33 AM CEST, Shannon Sterz wrote:
> On Tue Sep 30, 2025 at 2:34 PM CEST, Thomas Lamprecht wrote:
>> Am 30.09.25 um 14:19 schrieb Christian Ebner:
>>> On 9/30/25 1:49 PM, Wolfgang Bumiller wrote:
>>>> On Tue, Sep 30, 2025 at 10:02:01AM +0200, Christian Ebner wrote:
>>>>> This patches fix the ticket parsing when login to instances of PBS version 3
>>>>> or lower. For this, the current pve_compat flags for `Login` and `Client`
>>>>> are refactored to be an extendable enum variant instead, adding the ticket
>>>>> parsing backwards compatibility. In that compatibility mode, response parsing
>>>>> of the ticket does not interpret the presence of the `ticket-info` field as the
>>>>> ticket being a `http-only` ticket and fallsback to the PBS version 3 and 4
>>>>> compatible parsing, as the client never used `http-only` tickets.
>>>>
>>>> I thought this field was introduced only with http-only tickets, so why
>>>> is it not a reliable indicator and shouldn't *that* be fixed instead?
>>>
>>> I'm not so familiar with the actual code and the changes over time here, so that might as well be the case.
>>
>> FYI: Shannon is back at work tomorrow and this is not really _that_
>> pressing, so I'd wait at least until tomorrow to hopefully get some
>> better rationale.
>
> hi everyone and sorry if i am missing something (still catching up on
> mail), but i think this is basically a more in-depth approach to a fix i
> send a while back:
>
> https://lore.proxmox.com/pbs-devel/20250520085549.56525-1-s.sterz@proxmox.com/
>
> as chris has already pointed out in chat, there was a mishap on my end
> when refactoring the auth api and the old authentication flow would
> still send a `ticket-info` field alongside the `ticket` field.
> proxmox-login would then think it is in the new HttpOnly flow and not
> use the `ticket` field, even though it should.
>
> this was already fixed in commit f7d8b8f682 (auth-api: remove ticket
> info in old create ticket endpoint) [1], but it seems at least
> proxmox-backup-server 3.4.6-1 still sends the `ticket-info` as well. i
> rebuild pbs from latest stable-3 and stable-bookworm and there the
> parameter is correctly dropped.
>
> imo clients *should* use a ticket if they are provided with one and not
> get confused with additional parameters. which is what the patch linked
> above does. however, chris' approach to compatibility is a lot more
> extensible, so we could go down that road too.
>
> i'll check if my patch above works as inteded still and can resend a
> rebased version later today.
>
> [1]: https://git.proxmox.com/?p=proxmox.git;a=commitdiff;h=f7d8b8f682370cf0d8c3a0a238c958ceda2b8f7b

send the updated patch now:

https://lore.proxmox.com/all/20251001131102.266920-1-s.sterz@proxmox.com/T/#u


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