client: drop environment and login methods

The environment trait was useful on the CLI, but does not really
translate well to eg. the wasm ui (or pdm for that matter), so drop it
and instead have `.login` and `.login_tfa` just take the
`proxmox_login` type and handle the updating of authentication data.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

proxmox-client/src/client.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 use std::fmt;
 use std::future::Future;
 use std::sync::Arc;
-use std::sync::Mutex as StdMutex;
+use std::sync::Mutex;
 
 use http::request::Request;
 use http::response::Response;
@@ -10,10 +10,10 @@ use http::uri::PathAndQuery;
 use http::{StatusCode, Uri};
 use serde_json::Value;
 
-use proxmox_login::{Login, TicketResult};
+use proxmox_login::{Login, SecondFactorChallenge, TicketResult};
 
 use crate::auth::AuthenticationKind;
-use crate::{Authentication, Environment, Error, Token};
+use crate::{Error, Token};
 
 /// HTTP client backend trait.
 ///
@@ -25,18 +25,14 @@ pub trait HttpClient: Send + Sync {
 }
 
 /// Proxmox VE high level API client.
-pub struct Client<C, E: Environment> {
+pub struct Client<C> {
-    env: E,
     api_url: Uri,
-    auth: StdMutex<Option<Arc<AuthenticationKind>>>,
+    auth: Mutex<Option<Arc<AuthenticationKind>>>,
     client: C,
     pve_compat: bool,
 }
 
-impl<C, E> Client<C, E>
+impl<C> Client<C> {
-where
-    E: Environment,
-{
     /// Get the underlying client object.
     pub fn inner(&self) -> &C {
         &self.client
@@ -70,7 +66,7 @@ fn to_request(request: proxmox_login::Request) -> Result<http::Request<Vec<u8>>,
         .map_err(|err| Error::internal("error building login http request", err))
 }
 
-impl<C, E: Environment> Client<C, E> {
+impl<C> Client<C> {
     /// 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) {
@@ -78,29 +74,28 @@ impl<C, E: Environment> Client<C, E> {
     }
 }
 
-impl<C, E> Client<C, E>
+impl<C> Client<C>
 where
-    E: Environment,
     C: HttpClient,
 {
-    /// Instantiate a client for an API with a given environment and HTTP client instance.
+    /// Instantiate a client for an API with a given HTTP client instance.
-    pub fn with_client(api_url: Uri, environment: E, client: C) -> Self {
+    pub fn with_client(api_url: Uri, client: C) -> Self {
         Self {
-            env: environment,
             api_url,
-            auth: StdMutex::new(None),
+            auth: Mutex::new(None),
             client,
             pve_compat: false,
         }
     }
 
-    pub async fn login_auth(&self) -> Result<Arc<AuthenticationKind>, Error> {
+    /// Assert that we are authenticated and return the `AuthenticationKind`.
-        self.login().await?;
+    /// Otherwise returns `Error::Unauthenticated`.
+    pub fn login_auth(&self) -> Result<Arc<AuthenticationKind>, Error> {
         self.auth
             .lock()
             .unwrap()
             .clone()
-            .ok_or_else(|| Error::Other("login failed to set authentication information"))
+            .ok_or_else(|| Error::Unauthorized)
     }
 
     /// If currently logged in, this will fill in the auth cookie and CSRFPreventionToken header
@@ -122,113 +117,54 @@ where
         &self,
         request: http::request::Builder,
     ) -> Result<http::request::Builder, Error> {
-        Ok(self.login_auth().await?.set_auth_headers(request))
+        Ok(self.login_auth()?.set_auth_headers(request))
     }
 
-    /// Ensure that we have a valid ticket.
+    /// Attempt to login.
     ///
-    /// This will first attempt to load a ticket from the provided [`Environment`]. If successful,
+    /// This will propagate the PVE compatibility state and then perform the `Login` request via
-    /// its expiration time will be verified.
+    /// the inner http client.
     ///
-    /// If no valid ticket is available already, this will connect to the PVE API and perform
+    /// If the authentication is complete, `None` is returned and the authentication state updated.
-    /// authentication.
+    /// If a 2nd factor is required, `Some` is returned.
-    pub async fn login(&self) -> Result<(), Error> {
+    pub async fn login(&self, login: Login) -> Result<Option<SecondFactorChallenge>, Error> {
-        let (userid, login) = self.need_login().await?;
-        let Some(login) = login else { return Ok(()) };
-
         let login = login.pve_compatibility(self.pve_compat);
 
-        let response = self.client.request(to_request(login.request())?).await?;
+        let api_response = self.client.request(to_request(login.request())?).await?;
 
-        if !response.status().is_success() {
+        if !api_response.status().is_success() {
             // FIXME: does `http` somehow expose the status string?
-            return Err(Error::api(response.status(), "authentication failed"));
+            return Err(Error::api(api_response.status(), "authentication failed"));
         }
 
-        let challenge = match login.response(response.body())? {
+        Ok(match login.response(api_response.body())? {
-            TicketResult::Full(auth) => return self.finish_auth(&userid, auth).await,
+            TicketResult::TfaRequired(challenge) => Some(challenge),
-            TicketResult::TfaRequired(challenge) => challenge,
+            TicketResult::Full(auth) => {
-        };
+                *self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
-
+                None
-        let response = self
-            .env
-            .query_second_factor_async(&self.api_url, &userid, &challenge.challenge)
-            .await?;
-
-        let response = self
-            .client
-            .request(to_request(challenge.respond_raw(&response))?)
-            .await?;
-
-        let status = response.status();
-        if !status.is_success() {
-            return Err(Error::api(status, "authentication failed"));
-        }
-
-        let auth = challenge.response(response.body())?;
-
-        self.finish_auth(&userid, auth).await
-    }
-
-    /// Get the current username and, if required, a `Login` request.
-    async fn need_login(&self) -> Result<(String, Option<Login>), Error> {
-        use proxmox_login::ticket::Validity;
-
-        let (userid, auth) = self.current_auth().await?;
-
-        let authkind = match auth {
-            None => {
-                let password = self
-                    .env
-                    .query_password_async(&self.api_url, &userid)
-                    .await?;
-                return Ok((
-                    userid.clone(),
-                    Some(Login::new(self.api_url.to_string(), userid, password)),
-                ));
-            }
-            Some(authkind) => authkind,
-        };
-
-        let auth = match &*authkind {
-            AuthenticationKind::Token(_) => return Ok((userid, None)),
-            AuthenticationKind::Ticket(auth) => auth,
-        };
-
-        Ok(match auth.ticket.validity() {
-            Validity::Valid => {
-                *self.auth.lock().unwrap() = Some(authkind);
-                (userid, None)
-            }
-            Validity::Refresh => (
-                userid,
-                Some(
-                    Login::renew(self.api_url.to_string(), auth.ticket.to_string())
-                        .map_err(Error::Ticket)?,
-                ),
-            ),
-
-            Validity::Expired => {
-                let password = self
-                    .env
-                    .query_password_async(&self.api_url, &userid)
-                    .await?;
-                (
-                    userid.clone(),
-                    Some(Login::new(self.api_url.to_string(), userid, password)),
-                )
             }
         })
     }
 
-    /// Store the authentication info in our `auth` field and notify the environment.
+    /// Attempt to finish a 2nd factor login.
-    async fn finish_auth(&self, userid: &str, auth: Authentication) -> Result<(), Error> {
+    ///
-        let auth_string = serde_json::to_string(&auth)
+    /// This will propagate the PVE compatibility state and then perform the `Login` request via
-            .map_err(|err| Error::internal("failed to serialize authentication info", err))?;
+    /// the inner http client.
+    pub async fn login_tfa(
+        &self,
+        challenge: SecondFactorChallenge,
+        challenge_response: proxmox_login::Request,
+    ) -> Result<(), Error> {
+        let api_response = self.client.request(to_request(challenge_response)?).await?;
+
+        if !api_response.status().is_success() {
+            // FIXME: does `http` somehow expose the status string?
+            return Err(Error::api(api_response.status(), "authentication failed"));
+        }
+
+        let auth = challenge.response(api_response.body())?;
         *self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
-        self.env
+        Ok(())
-            .store_ticket_async(&self.api_url, userid, auth_string.as_bytes())
-            .await
     }
 
     /// Get the currently used API url.
@@ -236,46 +172,6 @@ where
         &self.api_url
     }
 
-    /// Get the current user id and a reference to the current authentication method.
-    /// If not authenticated yet, authenticate.
-    ///
-    /// This may cause the environment to be queried for user ids/passwords/FIDO/...
-    async fn current_auth(&self) -> Result<(String, Option<Arc<AuthenticationKind>>), Error> {
-        let auth = self.auth.lock().unwrap().clone();
-
-        let userid;
-        let auth = match auth {
-            Some(auth) => {
-                userid = auth.userid().to_owned();
-                Some(auth)
-            }
-            None => {
-                userid = self.env.query_userid_async(&self.api_url).await?;
-                self.reload_existing_ticket(&userid).await?
-            }
-        };
-
-        Ok((userid, auth))
-    }
-
-    /// Attempt to load an existing ticket from the environment.
-    async fn reload_existing_ticket(
-        &self,
-        userid: &str,
-    ) -> Result<Option<Arc<AuthenticationKind>>, Error> {
-        let ticket = match self.env.load_ticket_async(&self.api_url, userid).await? {
-            Some(auth) => auth,
-            None => return Ok(None),
-        };
-
-        let auth: Authentication = serde_json::from_slice(&ticket)
-            .map_err(|err| Error::internal("loaded bad ticket from environment", err))?;
-
-        let auth = Arc::new(auth.into());
-        *self.auth.lock().unwrap() = Some(Arc::clone(&auth));
-        Ok(Some(auth))
-    }
-
     /// Build a URI relative to the current API endpoint.
     fn build_uri(&self, path: &str) -> Result<Uri, Error> {
         let parts = self.api_url.clone().into_parts();
@@ -300,8 +196,6 @@ where
     where
         R: serde::de::DeserializeOwned,
     {
-        self.login().await?;
-
         let request = self
             .set_auth_headers(Request::get(self.build_uri(uri)?))
             .await?
@@ -321,7 +215,7 @@ where
         B: serde::Serialize,
         R: serde::de::DeserializeOwned,
     {
-        let auth = self.login_auth().await?;
+        let auth = self.login_auth()?;
         self.json_request(&auth, http::Method::GET, uri, body).await
     }
 
@@ -331,7 +225,7 @@ where
         B: serde::Serialize,
         R: serde::de::DeserializeOwned,
     {
-        let auth = self.login_auth().await?;
+        let auth = self.login_auth()?;
         self.json_request(&auth, http::Method::PUT, uri, body).await
     }
 
@@ -341,7 +235,7 @@ where
         B: serde::Serialize,
         R: serde::de::DeserializeOwned,
     {
-        let auth = self.login_auth().await?;
+        let auth = self.login_auth()?;
         self.json_request(&auth, http::Method::POST, uri, body)
             .await
     }
@@ -351,8 +245,6 @@ where
     where
         R: serde::de::DeserializeOwned,
     {
-        self.login().await?;
-
         let request = self
             .set_auth_headers(Request::delete(self.build_uri(uri)?))
             .await?
@@ -372,7 +264,7 @@ where
         B: serde::Serialize,
         R: serde::de::DeserializeOwned,
     {
-        let auth = self.login_auth().await?;
+        let auth = self.login_auth()?;
         self.json_request(&auth, http::Method::DELETE, uri, body)
             .await
     }
@@ -547,20 +439,13 @@ impl<T> RawApiResponse<T> {
 }
 
 #[cfg(feature = "hyper-client")]
-pub type HyperClient<E> = Client<Arc<proxmox_http::client::Client>, E>;
+pub type HyperClient = Client<Arc<proxmox_http::client::Client>>;
 
 #[cfg(feature = "hyper-client")]
-impl<C, E> Client<C, E>
+impl<C> Client<C> {
-where
-    E: Environment,
-{
     /// Create a new client instance which will connect to the provided endpoint.
-    pub fn new(api_url: Uri, environment: E) -> HyperClient<E> {
+    pub fn new(api_url: Uri) -> HyperClient {
-        Client::with_client(
+        Client::with_client(api_url, Arc::new(proxmox_http::client::Client::new()))
-            api_url,
-            environment,
-            Arc::new(proxmox_http::client::Client::new()),
-        )
     }
 }
 
@@ -579,7 +464,7 @@ mod hyper_client_extras {
     use proxmox_http::client::Client as ProxmoxClient;
 
     use super::{Client, HyperClient};
-    use crate::{Environment, Error};
+    use crate::Error;
 
     #[derive(Default)]
     pub enum TlsOptions {
@@ -636,17 +521,13 @@ mod hyper_client_extras {
         true
     }
 
-    impl<C, E> Client<C, E>
+    impl<C> Client<C> {
-    where
-        E: Environment,
-    {
         /// Create a new client instance which will connect to the provided endpoint.
         pub fn with_options(
             api_url: Uri,
-            environment: E,
             tls_options: TlsOptions,
             http_options: proxmox_http::HttpOptions,
-        ) -> Result<HyperClient<E>, Error> {
+        ) -> Result<HyperClient, Error> {
             let mut connector = SslConnector::builder(SslMethod::tls_client())
                 .map_err(|err| Error::internal("failed to create ssl connector builder", err))?;
 
@@ -680,7 +561,7 @@ mod hyper_client_extras {
 
             let client = ProxmoxClient::with_ssl_connector(connector.build(), http_options);
 
-            Ok(Client::with_client(api_url, environment, Arc::new(client)))
+            Ok(Client::with_client(api_url, Arc::new(client)))
         }
     }
 

proxmox-client/src/environment.rs

@@ -1,130 +0,0 @@
-use std::future::Future;
-use std::pin::Pin;
-use std::time::Duration;
-
-use http::Uri;
-
-use proxmox_login::tfa::TfaChallenge;
-
-use crate::Error;
-
-/// Provide input from the environment for storing/loading tickets or tokens and querying the user
-/// for passwords or 2nd factors.
-pub trait Environment: Send + Sync {
-    /// Store a ticket belonging to a user of an API.
-    ///
-    /// This is only used if `store_ticket_async` is not overwritten and may be left unimplemented
-    /// in async code. By default it will just return an error.
-    ///
-    /// [`store_ticket_async`]: Environment::store_ticket_async
-    fn store_ticket(&self, api_url: &Uri, userid: &str, ticket: &[u8]) -> Result<(), Error> {
-        let _ = (api_url, userid, ticket);
-        Err(Error::Other("missing store_ticket(_async) implementation"))
-    }
-
-    /// Load a user's cached ticket for an API url.
-    ///
-    /// This is only used if [`load_ticket_async`] is not overwritten and may be left unimplemented
-    /// in async code. By default it will just return an error.
-    ///
-    /// [`load_ticket_async`]: Environment::load_ticket_async
-    fn load_ticket(&self, api_url: &Uri, userid: &str) -> Result<Option<Vec<u8>>, Error> {
-        let _ = (api_url, userid);
-        Err(Error::Other("missing load_ticket(_async) implementation"))
-    }
-
-    /// Query for a userid (name and realm).
-    ///
-    /// This is only used if [`query_userid_async`] is not overwritten and may be left
-    /// unimplemented in async code. By default it will just return an error.
-    ///
-    /// [`query_userid_async`]: Environment::query_userid_async
-    fn query_userid(&self, api_url: &Uri) -> Result<String, Error> {
-        let _ = api_url;
-        Err(Error::Other("missing query_userid(_async) implementation"))
-    }
-
-    /// Query for a password.
-    ///
-    /// This is only used if [`query_password_async`] is not overwritten and may be left
-    /// unimplemented in async code. By default it will just return an error.
-    ///
-    /// [`query_password_async`]: Environment::query_password_async
-    fn query_password(&self, api_url: &Uri, userid: &str) -> Result<String, Error> {
-        let _ = (api_url, userid);
-        Err(Error::Other(
-            "missing query_password(_async) implementation",
-        ))
-    }
-
-    /// Query for a second factor. The default implementation is to not support 2nd factors.
-    ///
-    /// This is only used if [`query_second_factor_async`] is not overwritten and may be left
-    /// unimplemented in async code. By default it will just return an error.
-    ///
-    /// [`query_second_factor_async`]: Environment::query_second_factor_async
-    fn query_second_factor(
-        &self,
-        api_url: &Uri,
-        userid: &str,
-        challenge: &TfaChallenge,
-    ) -> Result<String, Error> {
-        let _ = (api_url, userid, challenge);
-        Err(Error::TfaNotSupported)
-    }
-
-    /// The client code uses async rust and it is fine to implement this instead of `store_ticket`.
-    fn store_ticket_async<'a>(
-        &'a self,
-        api_url: &'a Uri,
-        userid: &'a str,
-        ticket: &'a [u8],
-    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'a>> {
-        Box::pin(async move { self.store_ticket(api_url, userid, ticket) })
-    }
-
-    #[allow(clippy::type_complexity)]
-    fn load_ticket_async<'a>(
-        &'a self,
-        api_url: &'a Uri,
-        userid: &'a str,
-    ) -> Pin<Box<dyn Future<Output = Result<Option<Vec<u8>>, Error>> + Send + 'a>> {
-        Box::pin(async move { self.load_ticket(api_url, userid) })
-    }
-
-    fn query_userid_async<'a>(
-        &'a self,
-        api_url: &'a Uri,
-    ) -> Pin<Box<dyn Future<Output = Result<String, Error>> + Send + 'a>> {
-        Box::pin(async move { self.query_userid(api_url) })
-    }
-
-    fn query_password_async<'a>(
-        &'a self,
-        api_url: &'a Uri,
-        userid: &'a str,
-    ) -> Pin<Box<dyn Future<Output = Result<String, Error>> + Send + 'a>> {
-        Box::pin(async move { self.query_password(api_url, userid) })
-    }
-
-    fn query_second_factor_async<'a>(
-        &'a self,
-        api_url: &'a Uri,
-        userid: &'a str,
-        challenge: &'a TfaChallenge,
-    ) -> Pin<Box<dyn Future<Output = Result<String, Error>> + Send + 'a>> {
-        Box::pin(async move { self.query_second_factor(api_url, userid, challenge) })
-    }
-
-    /// In order to allow the polling based task API to function, we need a way to sleep in async
-    /// context.
-    /// This will likely be removed when the streaming tasks API is available.
-    ///
-    /// # Panics
-    ///
-    /// The default implementation simply panics.
-    fn sleep(time: Duration) -> Result<Pin<Box<dyn Future<Output = ()> + Send + 'static>>, Error> {
-        let _ = time;
-        Err(Error::SleepNotSupported)
-    }
-}

proxmox-client/src/error.rs

@@ -11,7 +11,7 @@ pub enum Error {
     /// to sleep. This signals that the environment does not support that.
     SleepNotSupported,
 
-    /// Tried to make an API call without a ticket which requires ones.
+    /// Tried to make an API call without a ticket.
     Unauthorized,
 
     /// The API responded with an error code.

proxmox-client/src/lib.rs

@@ -1,7 +1,5 @@
-mod environment;
 mod error;
 
-pub use environment::Environment;
 pub use error::Error;
 
 pub use proxmox_login::tfa::TfaChallenge;