Skip to content

Commit

Permalink
feat(*): Adds bindle keys endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
thomastaylor312 committed Jul 7, 2022
1 parent d7edfa8 commit de685f1
Show file tree
Hide file tree
Showing 11 changed files with 253 additions and 46 deletions.
2 changes: 1 addition & 1 deletion src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,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
Expand Down
42 changes: 42 additions & 0 deletions src/invoice/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)]
Expand Down
4 changes: 3 additions & 1 deletion src/invoice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
34 changes: 30 additions & 4 deletions src/invoice/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions src/provider/embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
36 changes: 6 additions & 30 deletions src/provider/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,11 +682,7 @@ mod test {
// Create a temporary directory
let root = tempdir().unwrap();
let scaffold = testing::Scaffold::load("valid_v1").await;
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;
let inv_name = scaffold.invoice.canonical_name();

let signed = NoopSigned(NoopVerified(scaffold.invoice.clone()));
Expand Down Expand Up @@ -720,11 +716,7 @@ mod test {
let root = tempdir().unwrap();
let mut scaffold = testing::Scaffold::load("valid_v1").await;
scaffold.invoice.yanked = Some(true);
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;

let signed = NoopSigned(NoopVerified(scaffold.invoice.clone()));
assert!(store.create_invoice(signed).await.is_err());
Expand All @@ -735,11 +727,7 @@ mod test {
let scaffold = testing::Scaffold::load("valid_v1").await;
let parcel = scaffold.parcel_files.get("parcel").unwrap();
let root = tempdir().expect("create tempdir");
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;

let signed = NoopSigned(NoopVerified(scaffold.invoice.clone()));
// Create the invoice so we can create a parcel
Expand Down Expand Up @@ -785,11 +773,7 @@ mod test {
#[tokio::test]
async fn test_should_store_and_retrieve_bindle() {
let root = tempdir().expect("create tempdir");
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;

let scaffold = testing::Scaffold::load("valid_v1").await;

Expand Down Expand Up @@ -834,11 +818,7 @@ mod test {
// Completely invalid size
parcels[0].label.size = 100000;
scaffold.invoice.parcel = Some(parcels);
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;

let signed = NoopSigned(NoopVerified(scaffold.invoice.clone()));
store
Expand Down Expand Up @@ -867,11 +847,7 @@ mod test {
// Create a temporary directory
let root = tempdir().unwrap();
let scaffold = testing::Scaffold::load("valid_v1").await;
let store = FileProvider::new(
root.path().to_owned(),
crate::search::StrictEngine::default(),
)
.await;
let store = FileProvider::new(root.path(), crate::search::StrictEngine::default()).await;

// We want two copies to try and write at the same time
let signed1 = NoopSigned(NoopVerified(scaffold.invoice.clone()));
Expand Down
57 changes: 53 additions & 4 deletions src/server/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ pub mod v1 {
use super::*;

use crate::{
signature::{KeyRing, SecretKeyStorage},
LoginParams, QueryOptions, SignatureError,
signature::{KeyEntry, KeyRing, SecretKeyStorage},
KeyOptions, LoginParams, QueryOptions, SignatureError,
};

use oauth2::reqwest::async_http_client;
Expand Down Expand Up @@ -439,6 +439,56 @@ pub mod v1 {
))
}

//////////// Bindle Key Functions ////////////

/// Returns public keys with a `host` label from the keychain. Returns a bad request error if
/// other key roles are requested
pub async fn bindle_keys<S: SecretKeyStorage>(
opts: KeyOptions,
secret_store: S,
accept_header: Option<String>,
) -> Result<impl warp::Reply, Infallible> {
// If we have more than one role or the role is not a host role, let the user know so they
// aren't confused when we only return a list of host public keys
if opts.roles.len() > 1
|| (!opts.roles.is_empty() && !opts.roles.contains(&SignatureRole::Host))
{
return Ok(reply::reply_from_error(
"This bindle server implementation only supports returning keys with the role of `host`. Please only specify `host` as a role (or omit the parameter)",
StatusCode::BAD_REQUEST,
));
}

let key_entries = match secret_store
.get_all_matching(&SignatureRole::Host, None)
.into_iter()
.map(|s| {
let mut entry = KeyEntry::try_from(s)?;
// Explicitly set the roles to just contain host as this is likely being added to the
// consumer's keychain and we don't want to give it a role it shouldn't have
entry.roles = vec![SignatureRole::Host];
Ok(entry)
})
.collect::<Result<Vec<_>, SignatureError>>()
{
Ok(entries) => entries,
Err(e) => {
return Ok(reply::reply_from_error(
e,
// This means something is wrong with the encoding of the server's keys
StatusCode::INTERNAL_SERVER_ERROR,
));
}
};

let keyring = KeyRing::new(key_entries);

Ok(warp::reply::with_status(
reply::serialized_data(&keyring, accept_header.unwrap_or_default()),
warp::http::StatusCode::OK,
))
}

//////////// Helper Functions ////////////

/// Fetches an invoice from the given store and checks that the given SHA exists within that
Expand All @@ -461,8 +511,7 @@ pub mod v1 {
// Make sure the sha exists in the list
let label = inv
.parcel
.map(|parcels| parcels.into_iter().find(|p| p.label.sha256 == sha))
.flatten()
.and_then(|parcels| parcels.into_iter().find(|p| p.label.sha256 == sha))
.map(|p| p.label);

match label {
Expand Down
81 changes: 81 additions & 0 deletions src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,4 +822,85 @@ mod test {
String::from_utf8_lossy(res.body())
);
}

#[tokio::test]
async fn test_bindle_keys() {
let (store, index, keystore) = testing::setup_embedded().await;
let api = super::routes::api(
store.clone(),
index,
AlwaysAuthenticate,
AlwaysAuthorize,
keystore.clone(),
VerificationStrategy::default(),
KeyRing::default(),
);

// Creating the invoice without a token should fail
let res = warp::test::request()
.method("GET")
.header("Content-Type", "application/toml")
.path("/v1/bindle-keys")
.reply(&api)
.await;

assert_eq!(
res.status(),
warp::http::StatusCode::OK,
"A get request with no query params should succeed. Body: {}",
String::from_utf8_lossy(res.body())
);

let keyring: crate::invoice::signature::KeyRing =
toml::from_slice(res.body()).expect("should be valid keyring response TOML");

// Sanity check that it just creates the 1 key and it has the right type
assert_eq!(keyring.key.len(), 1, "Should only return 1 host key");
assert_eq!(
keyring.key[0].roles,
vec![SignatureRole::Host],
"Returned keys should only have host roles"
);

// Now assert the same thing when specifying a query param
let res = warp::test::request()
.method("GET")
.header("Content-Type", "application/toml")
.path("/v1/bindle-keys?roles=host")
.reply(&api)
.await;

assert_eq!(
res.status(),
warp::http::StatusCode::OK,
"A get request with query params should succeed. Body: {}",
String::from_utf8_lossy(res.body())
);

let keyring: crate::invoice::signature::KeyRing =
toml::from_slice(res.body()).expect("should be valid keyring response TOML");

// Sanity check that it just creates the 1 key and it has the right type
assert_eq!(keyring.key.len(), 1, "Should only return 1 host key");
assert_eq!(
keyring.key[0].roles,
vec![SignatureRole::Host],
"Returned keys should only have host roles"
);

// And now make sure we get an error if non-host roles are specified
let res = warp::test::request()
.method("GET")
.header("Content-Type", "application/toml")
.path("/v1/bindle-keys?roles=host,creator")
.reply(&api)
.await;

assert_eq!(
res.status(),
warp::http::StatusCode::BAD_REQUEST,
"A get request with non host roles should fail. Body: {}",
String::from_utf8_lossy(res.body())
);
}
}
Loading

0 comments on commit de685f1

Please sign in to comment.