[pbs-devel] [PATCH proxmox v5 1/4] acme: reduce visibility of Request type

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

Currently, the low-level ACME Request type is publicly exposed, even
though users are expected to go through AcmeClient and
proxmox-acme-api handlers. This patch reduces visibility so that
the Request type and related fields/methods are crate-internal only.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
 proxmox-acme/src/account.rs       | 94 ++-----------------------------
 proxmox-acme/src/async_client.rs  |  2 +-
 proxmox-acme/src/authorization.rs | 30 ----------
 proxmox-acme/src/client.rs        |  6 +-
 proxmox-acme/src/lib.rs           |  4 --
 proxmox-acme/src/order.rs         |  2 +-
 proxmox-acme/src/request.rs       | 12 ++--
 7 files changed, 16 insertions(+), 134 deletions(-)

diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
index f763c1e9..d8eb3e73 100644
--- a/proxmox-acme/src/account.rs
+++ b/proxmox-acme/src/account.rs
@@ -8,12 +8,11 @@ use openssl::pkey::{PKey, Private};
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 
-use crate::authorization::{Authorization, GetAuthorization};
 use crate::b64u;
 use crate::directory::Directory;
 use crate::jws::Jws;
 use crate::key::{Jwk, PublicKey};
-use crate::order::{NewOrder, Order, OrderData};
+use crate::order::{NewOrder, OrderData};
 use crate::request::Request;
 use crate::types::{AccountData, AccountStatus, ExternalAccountBinding};
 use crate::Error;
@@ -92,7 +91,7 @@ impl Account {
     }
 
     /// Prepare a "POST-as-GET" request to fetch data. Low level helper.
-    pub fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
+    pub(crate) fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
         let key = PKey::private_key_from_pem(self.private_key.as_bytes())?;
         let body = serde_json::to_string(&Jws::new_full(
             &key,
@@ -112,7 +111,7 @@ impl Account {
     }
 
     /// Prepare a JSON POST request. Low level helper.
-    pub fn post_request<T: Serialize>(
+    pub(crate) fn post_request<T: Serialize>(
         &self,
         url: &str,
         nonce: &str,
@@ -136,31 +135,6 @@ impl Account {
         })
     }
 
-    /// Prepare a JSON POST request.
-    fn post_request_raw_payload(
-        &self,
-        url: &str,
-        nonce: &str,
-        payload: String,
-    ) -> Result<Request, Error> {
-        let key = PKey::private_key_from_pem(self.private_key.as_bytes())?;
-        let body = serde_json::to_string(&Jws::new_full(
-            &key,
-            Some(self.location.clone()),
-            url.to_owned(),
-            nonce.to_owned(),
-            payload,
-        )?)?;
-
-        Ok(Request {
-            url: url.to_owned(),
-            method: "POST",
-            content_type: crate::request::JSON_CONTENT_TYPE,
-            body,
-            expected: 200,
-        })
-    }
-
     /// Get the "key authorization" for a token.
     pub fn key_authorization(&self, token: &str) -> Result<String, Error> {
         let key = PKey::private_key_from_pem(self.private_key.as_bytes())?;
@@ -176,64 +150,6 @@ impl Account {
         Ok(b64u::encode(digest))
     }
 
-    /// Prepare a request to update account data.
-    ///
-    /// This is a rather low level interface. You should know what you're doing.
-    pub fn update_account_request<T: Serialize>(
-        &self,
-        nonce: &str,
-        data: &T,
-    ) -> Result<Request, Error> {
-        self.post_request(&self.location, nonce, data)
-    }
-
-    /// Prepare a request to deactivate this account.
-    pub fn deactivate_account_request<T: Serialize>(&self, nonce: &str) -> Result<Request, Error> {
-        self.post_request_raw_payload(
-            &self.location,
-            nonce,
-            r#"{"status":"deactivated"}"#.to_string(),
-        )
-    }
-
-    /// Prepare a request to query an Authorization for an Order.
-    ///
-    /// Returns `Ok(None)` if `auth_index` is out of out of range. You can query the number of
-    /// authorizations from via [`Order::authorization_len`] or by manually inspecting its
-    /// `.data.authorization` vector.
-    pub fn get_authorization(
-        &self,
-        order: &Order,
-        auth_index: usize,
-        nonce: &str,
-    ) -> Result<Option<GetAuthorization>, Error> {
-        match order.authorization(auth_index) {
-            None => Ok(None),
-            Some(url) => Ok(Some(GetAuthorization::new(self.get_request(url, nonce)?))),
-        }
-    }
-
-    /// Prepare a request to validate a Challenge from an Authorization.
-    ///
-    /// Returns `Ok(None)` if `challenge_index` is out of out of range. The challenge count is
-    /// available by inspecting the [`Authorization::challenges`] vector.
-    ///
-    /// This returns a raw `Request` since validation takes some time and the `Authorization`
-    /// object has to be re-queried and its `status` inspected.
-    pub fn validate_challenge(
-        &self,
-        authorization: &Authorization,
-        challenge_index: usize,
-        nonce: &str,
-    ) -> Result<Option<Request>, Error> {
-        match authorization.challenges.get(challenge_index) {
-            None => Ok(None),
-            Some(challenge) => self
-                .post_request_raw_payload(&challenge.url, nonce, "{}".to_string())
-                .map(Some),
-        }
-    }
-
     /// Prepare a request to revoke a certificate.
     ///
     /// The certificate can be either PEM or DER formatted.
@@ -274,7 +190,7 @@ pub struct CertificateRevocation<'a> {
 
 impl CertificateRevocation<'_> {
     /// Create the revocation request using the specified nonce for the given directory.
-    pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
+    pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
         let revoke_cert = directory.data.revoke_cert.as_ref().ok_or_else(|| {
             Error::Custom("no 'revokeCert' URL specified by provider".to_string())
         })?;
@@ -364,7 +280,7 @@ impl AccountCreator {
     /// the resulting request.
     /// Changing the private key between using the request and passing the response to
     /// [`response`](AccountCreator::response()) will render the account unusable!
-    pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
+    pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
         let key = self.key.as_deref().ok_or(Error::MissingKey)?;
         let url = directory.new_account_url().ok_or_else(|| {
             Error::Custom("no 'newAccount' URL specified by provider".to_string())
diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
index dc755fb9..2ff3ba22 100644
--- a/proxmox-acme/src/async_client.rs
+++ b/proxmox-acme/src/async_client.rs
@@ -10,7 +10,7 @@ use proxmox_http::{client::Client, Body};
 
 use crate::account::AccountCreator;
 use crate::order::{Order, OrderData};
-use crate::Request as AcmeRequest;
+use crate::request::Request as AcmeRequest;
 use crate::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
 
 /// A non-blocking Acme client using tokio/hyper.
diff --git a/proxmox-acme/src/authorization.rs b/proxmox-acme/src/authorization.rs
index 28bc1b4b..7027381a 100644
--- a/proxmox-acme/src/authorization.rs
+++ b/proxmox-acme/src/authorization.rs
@@ -6,8 +6,6 @@ use serde::{Deserialize, Serialize};
 use serde_json::Value;
 
 use crate::order::Identifier;
-use crate::request::Request;
-use crate::Error;
 
 /// Status of an [`Authorization`].
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)]
@@ -132,31 +130,3 @@ impl Challenge {
 fn is_false(b: &bool) -> bool {
     !*b
 }
-
-/// Represents an in-flight query for an authorization.
-///
-/// This is created via [`Account::get_authorization`](crate::Account::get_authorization()).
-pub struct GetAuthorization {
-    //order: OrderData,
-    /// The request to send to the ACME provider. This is wrapped in an option in order to allow
-    /// moving it out instead of copying the contents.
-    ///
-    /// When generated via [`Account::get_authorization`](crate::Account::get_authorization()),
-    /// this is guaranteed to be `Some`.
-    ///
-    /// The response should be passed to the the [`response`](GetAuthorization::response()) method.
-    pub request: Option<Request>,
-}
-
-impl GetAuthorization {
-    pub(crate) fn new(request: Request) -> Self {
-        Self {
-            request: Some(request),
-        }
-    }
-
-    /// Deal with the response we got from the server.
-    pub fn response(self, response_body: &[u8]) -> Result<Authorization, Error> {
-        Ok(serde_json::from_slice(response_body)?)
-    }
-}
diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
index 931f7245..5c812567 100644
--- a/proxmox-acme/src/client.rs
+++ b/proxmox-acme/src/client.rs
@@ -7,8 +7,8 @@ use serde::{Deserialize, Serialize};
 use crate::b64u;
 use crate::error;
 use crate::order::OrderData;
-use crate::request::ErrorResponse;
-use crate::{Account, Authorization, Challenge, Directory, Error, Order, Request};
+use crate::request::{ErrorResponse, Request};
+use crate::{Account, Authorization, Challenge, Directory, Error, Order};
 
 macro_rules! format_err {
     ($($fmt:tt)*) => { Error::Client(format!($($fmt)*)) };
@@ -564,7 +564,7 @@ impl Client {
     }
 
     /// Low-level API to run an n API request. This automatically updates the current nonce!
-    pub fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
+    pub(crate) fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
         self.inner.run_request(request)
     }
 
diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
index df722629..6722030c 100644
--- a/proxmox-acme/src/lib.rs
+++ b/proxmox-acme/src/lib.rs
@@ -66,10 +66,6 @@ pub use error::Error;
 #[doc(inline)]
 pub use order::Order;
 
-#[cfg(feature = "impl")]
-#[doc(inline)]
-pub use request::Request;
-
 // we don't inline these:
 #[cfg(feature = "impl")]
 pub use order::NewOrder;
diff --git a/proxmox-acme/src/order.rs b/proxmox-acme/src/order.rs
index b6551004..432a81a4 100644
--- a/proxmox-acme/src/order.rs
+++ b/proxmox-acme/src/order.rs
@@ -153,7 +153,7 @@ pub struct NewOrder {
     //order: OrderData,
     /// The request to execute to place the order. When creating a [`NewOrder`] via
     /// [`Account::new_order`](crate::Account::new_order) this is guaranteed to be `Some`.
-    pub request: Option<Request>,
+    pub(crate) request: Option<Request>,
 }
 
 impl NewOrder {
diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
index 78a90913..dadfc5af 100644
--- a/proxmox-acme/src/request.rs
+++ b/proxmox-acme/src/request.rs
@@ -4,21 +4,21 @@ pub(crate) const JSON_CONTENT_TYPE: &str = "application/jose+json";
 pub(crate) const CREATED: u16 = 201;
 
 /// A request which should be performed on the ACME provider.
-pub struct Request {
+pub(crate) struct Request {
     /// The complete URL to send the request to.
-    pub url: String,
+    pub(crate) url: String,
 
     /// The HTTP method name to use.
-    pub method: &'static str,
+    pub(crate) method: &'static str,
 
     /// The `Content-Type` header to pass along.
-    pub content_type: &'static str,
+    pub(crate) content_type: &'static str,
 
     /// The body to pass along with request, or an empty string.
-    pub body: String,
+    pub(crate) body: String,
 
     /// The expected status code a compliant ACME provider will return on success.
-    pub expected: u16,
+    pub(crate) expected: u16,
 }
 
 /// An ACME error response contains a specially formatted type string, and can optionally
-- 
2.47.3



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