Skip to content

Commit

Permalink
fix(secret-store): into_iter should output String, avoid a breaking c…
Browse files Browse the repository at this point in the history
…hange
  • Loading branch information
Kazy committed Oct 23, 2023
1 parent afba827 commit c26649f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 64 deletions.
50 changes: 3 additions & 47 deletions common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ pub mod models;
pub mod project;
pub mod resource;
pub mod secrets;
pub use secrets::{Secret, SecretStore};
#[cfg(feature = "tracing")]
pub mod tracing;
#[cfg(feature = "wasm")]
pub mod wasm;

use std::{collections::BTreeMap, fmt::Debug};
use std::fmt::Debug;

use anyhow::bail;
use secrets::Secret;
use serde::{Deserialize, Serialize};
#[cfg(feature = "openapi")]
use utoipa::openapi::{Object, ObjectBuilder};
Expand Down Expand Up @@ -145,38 +145,14 @@ impl DatabaseReadyInfo {
"{}://{}:{}@{}:{}/{}",
self.engine,
self.role_name,
self.role_password,
self.role_password.redacted(),
self.address_public,
self.port,
self.database_name
)
}
}

/// Store that holds all the secrets available to a deployment
#[derive(Deserialize, Serialize, Clone)]
pub struct SecretStore {
pub(crate) secrets: BTreeMap<String, Secret<String>>,
}

impl SecretStore {
pub fn new(secrets: BTreeMap<String, Secret<String>>) -> Self {
Self { secrets }
}

pub fn get(&self, key: &str) -> Option<String> {
self.secrets.get(key).map(|k| ToOwned::to_owned(k.expose()))
}
}

impl IntoIterator for SecretStore {
type Item = (String, Secret<String>);
type IntoIter = <BTreeMap<String, Secret<String>> as IntoIterator>::IntoIter;
fn into_iter(self) -> Self::IntoIter {
self.secrets.into_iter()
}
}

#[cfg(feature = "openapi")]
pub fn ulid_type() -> Object {
ObjectBuilder::new()
Expand Down Expand Up @@ -245,26 +221,6 @@ mod tests {
ApiKey::parse("dh9z58jttoes3qv@").unwrap();
}

#[test]
fn secretstore_intoiter() {
let bt = BTreeMap::from([
("1".to_owned(), "2".to_owned().into()),
("3".to_owned(), "4".to_owned().into()),
]);
let ss = SecretStore::new(bt);

let mut iter = ss.into_iter();
assert_eq!(
iter.next(),
Some(("1".to_owned().into(), "2".to_owned().into()))
);
assert_eq!(
iter.next(),
Some(("3".to_owned().into(), "4".to_owned().into()))
);
assert_eq!(iter.next(), None);
}

#[test]
fn semver_compatibility_check_works() {
let semver_tests = &[
Expand Down
51 changes: 36 additions & 15 deletions common/src/secrets.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use serde::{Deserialize, Serialize};
use std::{
collections::BTreeMap,
fmt::{Debug, Display},
};
use std::{collections::BTreeMap, fmt::Debug};
use zeroize::Zeroize;

/// Wrapper type for secret values such as passwords or authentication keys.
Expand All @@ -23,12 +20,6 @@ impl<T: Zeroize> Debug for Secret<T> {
}
}

impl<T: Zeroize> Display for Secret<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "********")
}
}

impl<T: Zeroize> Drop for Secret<T> {
fn drop(&mut self) {
self.0.zeroize();
Expand All @@ -46,9 +37,15 @@ impl<T: Zeroize> Secret<T> {
Self(secret)
}

/// Expose the underlying value of the secret
pub fn expose(&self) -> &T {
&self.0
}

/// Display a placeholder for the secret
pub fn redacted(&self) -> &str {
"********"
}
}

/// Store that holds all the secrets available to a deployment
Expand All @@ -63,9 +60,20 @@ impl SecretStore {
}

pub fn get(&self, key: &str) -> Option<String> {
self.secrets.get(key).map(|s| s.expose().to_owned())
}
}

impl IntoIterator for SecretStore {
type Item = (String, String);
type IntoIter = <BTreeMap<String, String> as IntoIterator>::IntoIter;

fn into_iter(self) -> Self::IntoIter {
self.secrets
.get(key)
.map(|secret| secret.expose().to_owned())
.into_iter()
.map(|(k, s)| (k, s.expose().to_owned()))
.collect::<BTreeMap<String, String>>()
.into_iter()
}
}

Expand All @@ -75,11 +83,10 @@ mod secrets_tests {
use super::*;

#[test]
fn display() {
fn redacted() {
let password_string = String::from("VERYSECRET");
let secret = Secret::new(password_string);
let printed = format!("{}", secret);
assert_eq!(printed, "[REDACTED \"alloc::string::String\"]");
assert_eq!(secret.redacted(), "********");
}

#[test]
Expand Down Expand Up @@ -114,4 +121,18 @@ mod secrets_tests {
"Wrapper { password: [REDACTED \"alloc::string::String\"] }"
);
}

#[test]
fn secretstore_intoiter() {
let bt = BTreeMap::from([
("1".to_owned(), "2".to_owned().into()),
("3".to_owned(), "4".to_owned().into()),
]);
let ss = SecretStore::new(bt);

let mut iter = ss.into_iter();
assert_eq!(iter.next(), Some(("1".to_owned(), "2".to_owned())));
assert_eq!(iter.next(), Some(("3".to_owned(), "4".to_owned())));
assert_eq!(iter.next(), None);
}
}
4 changes: 3 additions & 1 deletion resources/turso/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ impl ResourceBuilder<Client> for Turso {
mod test {
use super::*;
use std::str::FromStr;
use shuttle_service::Secret;

struct MockFactory {
pub environment: Environment,
Expand All @@ -163,7 +164,8 @@ mod test {

async fn get_secrets(
&mut self,
) -> Result<std::collections::BTreeMap<String, String>, shuttle_service::Error> {
) -> Result<std::collections::BTreeMap<String, Secret<String>>, shuttle_service::Error>
{
panic!("no turso test should try to get secrets")
}

Expand Down
2 changes: 1 addition & 1 deletion service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::net::SocketAddr;

use async_trait::async_trait;
use serde::{de::DeserializeOwned, Serialize};
use shuttle_common::secrets::Secret;
pub use shuttle_common::secrets::Secret;
pub use shuttle_common::{
database,
deployment::{DeploymentMetadata, Environment},
Expand Down

0 comments on commit c26649f

Please sign in to comment.