[pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient

[pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient

[Dominik Csapak]

by packing the auth into a RwLock and starting a background
future that renews the ticket every 15 minutes

we still use the BroadcastFuture for the first ticket and only
if that is finished we start the scheduled future

we have to store an abort handle for the renewal future and abort it when
the http client is dropped, so we do not request new tickets forever

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/client/http_client.rs | 67 +++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index 3fd9fb7f..692b2c3e 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -1,6 +1,7 @@
 use std::io::Write;
 use std::task::{Context, Poll};
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, RwLock};
+use std::time::Duration;
 
 use anyhow::{bail, format_err, Error};
 use futures::*;
@@ -29,7 +30,7 @@ use crate::tools::{self, BroadcastFuture, DEFAULT_ENCODE_SET};
 
 #[derive(Clone)]
 pub struct AuthInfo {
-    pub username: String,
+    pub userid: Userid,
     pub ticket: String,
     pub token: String,
 }
@@ -99,7 +100,9 @@ pub struct HttpClient {
     client: Client<HttpsConnector>,
     server: String,
     fingerprint: Arc<Mutex<Option<String>>>,
-    auth: BroadcastFuture<AuthInfo>,
+    first_auth: BroadcastFuture<()>,
+    auth: Arc<RwLock<AuthInfo>>,
+    ticket_abort: futures::future::AbortHandle,
     _options: HttpClientOptions,
 }
 
@@ -317,6 +320,41 @@ impl HttpClient {
             }
         };
 
+        let auth = Arc::new(RwLock::new(AuthInfo {
+            userid: userid.clone(),
+            ticket: password.clone(),
+            token: "".to_string(),
+        }));
+
+        let server2 = server.to_string();
+        let client2 = client.clone();
+        let auth2 = auth.clone();
+        let prefix2 = options.prefix.clone();
+
+        let renewal_future = async move {
+            loop {
+                tokio::time::delay_for(Duration::new(60*15,  0)).await; // 15 minutes
+                let (userid, ticket) = {
+                    let authinfo = auth2.read().unwrap().clone();
+                    (authinfo.userid, authinfo.ticket)
+                };
+                match Self::credentials(client2.clone(), server2.clone(), userid, ticket).await {
+                    Ok(auth) => {
+                        if use_ticket_cache & &prefix2.is_some() {
+                            let _ = store_ticket_info(prefix2.as_ref().unwrap(), &server2, &auth.userid.to_string(), &auth.ticket, &auth.token);
+                        }
+                        *auth2.write().unwrap() = auth;
+                    },
+                    Err(err) => {
+                        eprintln!("re-authentication failed: {}", err);
+                        return;
+                    }
+                }
+            }
+        };
+
+        let (renewal_future, ticket_abort) = futures::future::abortable(renewal_future);
+
         let login_future = Self::credentials(
             client.clone(),
             server.to_owned(),
@@ -325,13 +363,14 @@ impl HttpClient {
         ).map_ok({
             let server = server.to_string();
             let prefix = options.prefix.clone();
+            let authinfo = auth.clone();
 
             move |auth| {
                 if use_ticket_cache & &prefix.is_some() {
-                    let _ = store_ticket_info(prefix.as_ref().unwrap(), &server, &auth.username, &auth.ticket, &auth.token);
+                    let _ = store_ticket_info(prefix.as_ref().unwrap(), &server, &auth.userid.to_string(), &auth.ticket, &auth.token);
                 }
-
-                auth
+                *authinfo.write().unwrap() = auth;
+                tokio::spawn(renewal_future);
             }
         });
 
@@ -339,7 +378,9 @@ impl HttpClient {
             client,
             server: String::from(server),
             fingerprint: verified_fingerprint,
-            auth: BroadcastFuture::new(Box::new(login_future)),
+            auth,
+            ticket_abort,
+            first_auth: BroadcastFuture::new(Box::new(login_future)),
             _options: options,
         })
     }
@@ -349,7 +390,9 @@ impl HttpClient {
     /// Login is done on demand, so this is only required if you need
     /// access to authentication data in 'AuthInfo'.
     pub async fn login(&self) -> Result<AuthInfo, Error> {
-        self.auth.listen().await
+        self.first_auth.listen().await?;
+        let authinfo = self.auth.read().unwrap();
+        Ok(authinfo.clone())
     }
 
     /// Returns the optional fingerprint passed to the new() constructor.
@@ -588,7 +631,7 @@ impl HttpClient {
         let req = Self::request_builder(&server, "POST", "/api2/json/access/ticket", Some(data)).unwrap();
         let cred = Self::api_request(client, req).await?;
         let auth = AuthInfo {
-            username: cred["data"]["username"].as_str().unwrap().to_owned(),
+            userid: cred["data"]["username"].as_str().unwrap().parse()?,
             ticket: cred["data"]["ticket"].as_str().unwrap().to_owned(),
             token: cred["data"]["CSRFPreventionToken"].as_str().unwrap().to_owned(),
         };
@@ -666,6 +709,12 @@ impl HttpClient {
     }
 }
 
+impl Drop for HttpClient {
+    fn drop(&mut self) {
+        self.ticket_abort.abort();
+    }
+}
+
 
 #[derive(Clone)]
 pub struct H2Client {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable

[Dominik Csapak]

by selecting between the pull_future and the abort future

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/pull.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index fc13cf40..09e27a17 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -176,7 +176,13 @@ async fn pull (
 
         worker.log(format!("sync datastore '{}' start", store));
 
-        pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, userid).await?;
+        let pull_future = pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, userid);
+        let future = select!{
+            success = pull_future.fuse() => success,
+            abort = worker.abort_future().map(|_| Err(format_err!("pull aborted"))) => abort,
+        };
+
+        let _ = future?;
 
         worker.log(format!("sync datastore '{}' end", store));
 
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient

[Dietmar Maurer]

applied

[pbs-devel] applied: [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable

[Dietmar Maurer]

applied