public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient
@ 2020-09-15  9:15 Dominik Csapak
  2020-09-15  9:15 ` [pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable Dominik Csapak
  2020-09-17  4:11 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dietmar Maurer
  0 siblings, 2 replies; 4+ messages in thread
From: Dominik Csapak @ 2020-09-15  9:15 UTC (permalink / raw)
  To: pbs-devel

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





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

* [pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable
  2020-09-15  9:15 [pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dominik Csapak
@ 2020-09-15  9:15 ` Dominik Csapak
  2020-09-17  4:11   ` [pbs-devel] applied: " Dietmar Maurer
  2020-09-17  4:11 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dietmar Maurer
  1 sibling, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2020-09-15  9:15 UTC (permalink / raw)
  To: pbs-devel

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





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

* [pbs-devel] applied: [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient
  2020-09-15  9:15 [pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dominik Csapak
  2020-09-15  9:15 ` [pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable Dominik Csapak
@ 2020-09-17  4:11 ` Dietmar Maurer
  1 sibling, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2020-09-17  4:11 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

applied




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

* [pbs-devel] applied: [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable
  2020-09-15  9:15 ` [pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable Dominik Csapak
@ 2020-09-17  4:11   ` Dietmar Maurer
  0 siblings, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2020-09-17  4:11 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

applied




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

end of thread, other threads:[~2020-09-17  4:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15  9:15 [pbs-devel] [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dominik Csapak
2020-09-15  9:15 ` [pbs-devel] [PATCH proxmox-backup 2/2] api2/pull: make pull worker abortable Dominik Csapak
2020-09-17  4:11   ` [pbs-devel] applied: " Dietmar Maurer
2020-09-17  4:11 ` [pbs-devel] applied: [PATCH proxmox-backup 1/2] fix #2870: renew tickets in HttpClient Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal