Merge pull request #334 from thomastaylor312/feat/bindle-keys

feat(*): Adds bindle-keys endpoint support

bin/client/main.rs

@@ -440,6 +440,40 @@ async fn run() -> std::result::Result<(), ClientError> {
                         .map_err(|e| ClientError::Other(e.to_string()))?;
                     println!("Wrote key to keyring file at {}", keyring_path.display())
                 }
+                Keys::Fetch(opts) => {
+                    let new_keys = match opts.key_server {
+                        Some(url) if !opts.use_host => {
+                            println!("Fetching host keys from {}", url);
+                            get_host_keys(url).await?
+                        }
+                        _ => {
+                            println!("Fetching host keys from bindle server");
+                            bindle_client.get_host_keys().await?
+                        }
+                    };
+
+                    let mut keyring = keyring_path
+                        .load()
+                        .await
+                        .unwrap_or_else(|_| KeyRing::default());
+                    let orig_len = keyring.key.len();
+                    // Have to filter before extending so we finish with the borrow of the current keyring
+                    let filtered_keys: Vec<KeyEntry> = new_keys
+                        .key
+                        .into_iter()
+                        .filter(|k| !keyring.key.iter().any(|current| current.key == k.key))
+                        .collect();
+                    keyring.key.extend(filtered_keys);
+                    keyring_path
+                        .save(&keyring)
+                        .await
+                        .map_err(|e| ClientError::Other(e.to_string()))?;
+                    println!(
+                        "Wrote {} keys to keyring file at {}",
+                        keyring.key.len() - orig_len,
+                        keyring_path.display()
+                    )
+                }
             }
         }
         SubCommand::Clean(_clean_opts) => {
@@ -711,3 +745,16 @@ fn parse_roles(roles: String) -> Result<Vec<SignatureRole>> {
         })
         .collect()
 }
+
+async fn get_host_keys(url: url::Url) -> Result<KeyRing> {
+    let resp = reqwest::get(url).await?;
+    if resp.status() != reqwest::StatusCode::OK {
+        return Err(ClientError::Other(format!(
+            "Unable to fetch host keys. Got status code {} with body content:\n{}",
+            resp.status(),
+            String::from_utf8_lossy(&resp.bytes().await?)
+        )));
+    }
+
+    toml::from_slice(&resp.bytes().await?).map_err(ClientError::from)
+}

bin/client/opts.rs

@@ -153,7 +153,8 @@ pub enum Keys {
         about = "Print the public key entries for keys from the secret key file. If no '--label' is supplied, public keys for all secret keys are returned."
     )]
     Print(PrintKey),
-    // TODO(thomastaylor312): We should probably add an endpoint to bindle servers that allow you to download a host key and add a subcommand to help with it
+    #[clap(name = "fetch", about = "Fetch keys from a /bindle-keys endpoint")]
+    Fetch(FetchKeys),
 }
 
 #[derive(Parser)]
@@ -426,6 +427,25 @@ pub struct PrintKey {
     pub label_matching: Option<String>,
 }
 
+#[derive(Parser)]
+pub struct FetchKeys {
+    #[clap(
+        long = "key-server",
+        value_name = "KEY_SERVER",
+        env = "BINDLE_KEY_SERVER",
+        conflicts_with = "use-host",
+        help = "Sets the server address and path to use for fetching keys. Should be a full url path (e.g. https://my.server.com/api/v1/bindle-keys). If this is not set, --host is implied"
+    )]
+    pub key_server: Option<url::Url>,
+    #[clap(
+        long = "host",
+        value_name = "USE_HOST",
+        id = "use-host",
+        help = "Fetches keys from the bindle server set by BINDLE_URL. This is mutually exclusive with --key-server"
+    )]
+    pub use_host: bool,
+}
+
 #[derive(Parser)]
 pub struct SignInvoice {
     #[clap(

src/client/mod.rs

@@ -30,6 +30,7 @@ pub const INVOICE_ENDPOINT: &str = "_i";
 pub const QUERY_ENDPOINT: &str = "_q";
 pub const RELATIONSHIP_ENDPOINT: &str = "_r";
 pub const LOGIN_ENDPOINT: &str = "login";
+pub const BINDLE_KEYS_ENDPOINT: &str = "bindle-keys";
 const TOML_MIME_TYPE: &str = "application/toml";
 
 /// A client type for interacting with a Bindle server
@@ -43,16 +44,6 @@ pub struct Client<T> {
     keyring: Arc<KeyRing>,
 }
 
-#[derive(Debug)]
-/// The operation being performed against a Bindle server.
-enum Operation {
-    Create,
-    Yank,
-    Get,
-    Query,
-    Login,
-}
-
 /// A builder for for setting up a `Client`. Created using `Client::builder`
 pub struct ClientBuilder {
     http2_prior_knowledge: bool,
@@ -175,7 +166,7 @@ impl<T: tokens::TokenManager> Client<T> {
         method: reqwest::Method,
         path: &str,
         body: Option<impl Into<reqwest::Body>>,
-    ) -> anyhow::Result<reqwest::Response> {
+    ) -> Result<reqwest::Response> {
         let req = self.client.request(method, self.base_url.join(path)?);
         let req = self.token_manager.apply_auth_header(req).await?;
         let req = match body {
@@ -514,6 +505,18 @@ impl<T: tokens::TokenManager> Client<T> {
         let resp = unwrap_status(resp, Endpoint::Invoice, Operation::Get).await?;
         Ok(toml::from_slice::<crate::MissingParcelsResponse>(&resp.bytes().await?)?.missing)
     }
+
+    //////////////// Bindle Keys Endpoints ////////////////
+
+    /// Fetches all the host public keys specified for the bindle server
+    #[instrument(level = "trace", skip(self))]
+    pub async fn get_host_keys(&self) -> Result<KeyRing> {
+        let resp = self
+            .raw(reqwest::Method::GET, BINDLE_KEYS_ENDPOINT, None::<&str>)
+            .await?;
+        let resp = unwrap_status(resp, Endpoint::BindleKeys, Operation::Get).await?;
+        Ok(toml::from_slice::<KeyRing>(&resp.bytes().await?)?)
+    }
 }
 
 // We implement provider for client because often times (such as in the CLI) we are composing the
@@ -622,13 +625,24 @@ impl<T: tokens::TokenManager + Send + Sync + 'static> Provider for Client<T> {
 
 // A helper function and related enum to make some reusable code for unwrapping a status code and returning the right error
 
+#[derive(Debug)]
+/// The operation being performed against a Bindle server.
+enum Operation {
+    Create,
+    Yank,
+    Get,
+    Query,
+    Login,
+}
+
 enum Endpoint {
     Invoice,
     Parcel,
     Query,
     // NOTE: This endpoint currently does nothing, but if we need more specific errors, we can use
     // this down the line
     Login,
+    BindleKeys,
 }
 
 async fn unwrap_status(
@@ -653,6 +667,10 @@ async fn unwrap_status(
         (StatusCode::CONFLICT, Endpoint::Invoice) => Err(ClientError::InvoiceAlreadyExists),
         (StatusCode::CONFLICT, Endpoint::Parcel) => Err(ClientError::ParcelAlreadyExists),
         (StatusCode::UNAUTHORIZED, _) => Err(ClientError::Unauthorized),
+        (StatusCode::BAD_REQUEST, Endpoint::BindleKeys) => Err(ClientError::InvalidRequest {
+            status_code: resp.status(),
+            message: parse_error_from_body(resp).await,
+        }),
         (StatusCode::BAD_REQUEST, _) => Err(ClientError::ServerError(Some(format!(
             "Bad request: {}",
             parse_error_from_body(resp).await.unwrap_or_default()
@@ -668,7 +686,7 @@ async fn unwrap_status(
         _ => Err(ClientError::Other(format!(
             "Unknown error response: {:?} to {} returned status {}: {}",
             operation,
-            resp.url().to_string(),
+            resp.url().to_owned(),
             resp.status(),
             parse_error_from_body(resp)
                 .await

src/invoice/api.rs

@@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::invoice::{Invoice, Label};
 use crate::search::SearchOptions;
+use crate::SignatureRole;
 
 /// A custom type for responding to invoice creation requests. Because invoices can be created
 /// before parcels are uploaded, this allows the API to inform the user if there are missing parcels
@@ -58,6 +59,47 @@ impl From<QueryOptions> for SearchOptions {
     }
 }
 
+/// Available query string options for the keyring API
+#[derive(Debug, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct KeyOptions {
+    #[serde(default)]
+    #[serde(deserialize_with = "parse_role_list")]
+    pub roles: Vec<SignatureRole>,
+}
+
+struct RoleVisitor(std::marker::PhantomData<fn() -> Vec<SignatureRole>>);
+
+impl<'de> serde::de::Visitor<'de> for RoleVisitor {
+    type Value = Vec<SignatureRole>;
+
+    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+        formatter.write_str("a comma delimited list of SignatureRoles")
+    }
+
+    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        let roles = v
+            .split(',')
+            .map(|s| {
+                s.parse::<SignatureRole>()
+                    .map_err(|e| serde::de::Error::custom(e))
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok(roles)
+    }
+}
+
+fn parse_role_list<'de, D>(deserializer: D) -> Result<Vec<SignatureRole>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    let visitor = RoleVisitor(std::marker::PhantomData);
+    deserializer.deserialize_str(visitor)
+}
+
 // Keeping these types private for now until we stabilize exactly how we want to handle it
 
 #[derive(Deserialize, Serialize, Debug)]

src/invoice/mod.rs

@@ -18,7 +18,9 @@ pub(crate) use api::DeviceAuthorizationExtraFields;
 #[doc(inline)]
 pub(crate) use api::LoginParams;
 #[doc(inline)]
-pub use api::{ErrorResponse, InvoiceCreateResponse, MissingParcelsResponse, QueryOptions};
+pub use api::{
+    ErrorResponse, InvoiceCreateResponse, KeyOptions, MissingParcelsResponse, QueryOptions,
+};
 #[doc(inline)]
 pub use bindle_spec::BindleSpec;
 #[doc(inline)]

src/invoice/signature.rs

@@ -365,16 +365,24 @@ impl SecretKeyEntry {
 /// all of them must provide a way for the system to fetch a key matching the
 /// desired role.
 pub trait SecretKeyStorage {
-    /// Get a key appropriate for signing with the given role and optional match criteria with LabelMatch enum.
+    /// Get a key appropriate for signing with the given role and optional match criteria with
+    /// LabelMatch enum.
     ///
-    /// If no key is found, this will return a None.
-    /// In general, if multiple keys match, the implementation chooses the "best fit"
-    /// and returns that key.
+    /// If no key is found, this will return a None. In general, if multiple keys match, the
+    /// implementation chooses the "best fit" and returns that key.
     fn get_first_matching(
         &self,
         role: &SignatureRole,
         label_match: Option<&LabelMatch>,
     ) -> Option<&SecretKeyEntry>;
+
+    /// Similar to [`get_first_matching`](get_first_matching), but returns all matches rather than
+    /// just the best fit
+    fn get_all_matching(
+        &self,
+        role: &SignatureRole,
+        label_match: Option<&LabelMatch>,
+    ) -> Vec<&SecretKeyEntry>;
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
@@ -445,6 +453,24 @@ impl SecretKeyStorage for SecretKeyFile {
                 }
         })
     }
+
+    fn get_all_matching(
+        &self,
+        role: &SignatureRole,
+        label_match: Option<&LabelMatch>,
+    ) -> Vec<&SecretKeyEntry> {
+        self.key
+            .iter()
+            .filter(|k| {
+                k.roles.contains(role)
+                    && match label_match {
+                        Some(LabelMatch::FullMatch(label)) => k.label.eq(label),
+                        Some(LabelMatch::PartialMatch(label)) => k.label.contains(label),
+                        None => true,
+                    }
+            })
+            .collect()
+    }
 }
 
 #[cfg(test)]

src/provider/embedded.rs

@@ -442,7 +442,7 @@ where
         remaining_permits = semaphore.available_permits(),
         "Successfully acquired spawn_blocking permit"
     );
-    Ok(tokio::task::spawn_blocking(f)
+    tokio::task::spawn_blocking(f)
         .await
-        .map_err(|_| ProviderError::Other("Internal error: unable to lock task".into()))?)
+        .map_err(|_| ProviderError::Other("Internal error: unable to lock task".into()))
 }