Use secrecy for more explicit usage of the access token

Cargo.toml

@@ -26,13 +26,14 @@ fern = "0.6"
 humantime = "2.1.0"
 log = { version = "0.4.14", features = ["std", "serde"] }
 log-reroute = { version = "0.1.6" }
+native-tls = { version = "0.2.7", features = ["vendored"]}
 reqwest = { version = "0.11", features = ["blocking", "json"] }
+secrecy = "0.7.0"
 serde = { version = "1.0.123", features = ["derive"] }
 serde_json = { version = "1.0.62" }
 signal-hook = { version = "0.3.4", features = ["extended-siginfo"] }
 toml = "0.5.8"
 trust-dns-resolver = "0.20.0"
-native-tls = { version = "0.2.7", features = ["vendored"]}
 
 [dev-dependencies]
 tempfile = "3.2.0"

TODO.md

@@ -1,5 +1,4 @@
 * Add better stats collection
 * Add support for detecting systemd notify socket to remove timestamp from logs
-* Use secrecy for access token
 * Deduplicate `from_x` code in ConfigValueBuilder
 * Add systemd sample configuration

src/config.rs

@@ -1,3 +1,4 @@
+use crate::token::SecretDigitalOceanToken;
 use anyhow::Result;
 use humantime::Duration;
 use std::time::Duration as StdDuration;
@@ -8,7 +9,7 @@ pub struct Config {
     pub domain_root: String,
     pub subdomain_to_update: String,
     pub update_interval: UpdateInterval,
-    pub digital_ocean_token: String,
+    pub digital_ocean_token: SecretDigitalOceanToken,
     pub log_level: log::LevelFilter,
     pub dry_run: bool,
 }

src/config_builder.rs

@@ -1,5 +1,7 @@
 use crate::config::{Config, UpdateInterval};
 use crate::config_consts::*;
+use crate::token::SecretDigitalOceanToken;
+use crate::types::ValueFromStr;
 use anyhow::{anyhow, bail, Context, Result};
 use clap::ArgMatches;
 
@@ -75,7 +77,7 @@ pub struct ValueBuilder<'clap, 'toml, T> {
     default_value: Option<T>,
 }
 
-impl<'clap, 'toml, T: std::str::FromStr> ValueBuilder<'clap, 'toml, T> {
+impl<'clap, 'toml, T: ValueFromStr> ValueBuilder<'clap, 'toml, T> {
     pub fn new(key: &str) -> Self {
         ValueBuilder {
             key: key.to_owned(),
@@ -143,7 +145,7 @@ impl<'clap, 'toml, T: std::str::FromStr> ValueBuilder<'clap, 'toml, T> {
         if let Some(ref env_var_name) = self.env_var_name {
             let env_res = std::env::var(env_var_name);
             if let Ok(value) = env_res {
-                let parsed_res = value.parse::<T>();
+                let parsed_res = ValueFromStr::from_str(value.as_ref());
                 if let Ok(value) = parsed_res {
                     self.value = Some(value);
                 }
@@ -161,7 +163,7 @@ impl<'clap, 'toml, T: std::str::FromStr> ValueBuilder<'clap, 'toml, T> {
         if let Some((arg_matches, ref option_name)) = self.clap_option {
             let clap_value = arg_matches.value_of(option_name);
             if let Some(value) = clap_value {
-                let parsed_res = value.parse::<T>();
+                let parsed_res = ValueFromStr::from_str(value);
                 if let Ok(value) = parsed_res {
                     self.value = Some(value);
                 }
@@ -194,7 +196,7 @@ impl<'clap, 'toml, T: std::str::FromStr> ValueBuilder<'clap, 'toml, T> {
             let line = std::fs::read_to_string(file_path);
             if let Ok(line) = line {
                 let value = line.trim_end();
-                let parsed_res = value.parse::<T>();
+                let parsed_res = ValueFromStr::from_str(value);
                 if let Ok(value) = parsed_res {
                     self.value = Some(value);
                 }
@@ -214,7 +216,7 @@ impl<'clap, 'toml, T: std::str::FromStr> ValueBuilder<'clap, 'toml, T> {
             if let Some(toml_value) = toml_value {
                 if let Some(toml_str) = toml_value.as_str() {
                     let value = toml_str;
-                    let parsed_res = value.parse::<T>();
+                    let parsed_res = ValueFromStr::from_str(value);
                     if let Ok(value) = parsed_res {
                         self.value = Some(value);
                     }
@@ -256,7 +258,7 @@ pub struct Builder<'clap> {
     domain_root: Option<String>,
     subdomain_to_update: Option<String>,
     update_interval: Option<UpdateInterval>,
-    digital_ocean_token: Option<String>,
+    digital_ocean_token: Option<SecretDigitalOceanToken>,
     log_level: Option<log::LevelFilter>,
     dry_run: Option<bool>,
 }
@@ -314,7 +316,7 @@ impl<'clap> Builder<'clap> {
         self
     }
 
-    pub fn set_digital_ocean_token(&mut self, value: String) -> &mut Self {
+    pub fn set_digital_ocean_token(&mut self, value: SecretDigitalOceanToken) -> &mut Self {
         self.digital_ocean_token = Some(value);
         self
     }
@@ -365,7 +367,7 @@ impl<'clap> Builder<'clap> {
             }
         }
 
-        let digital_ocean_token: String = builder.build()?;
+        let digital_ocean_token: SecretDigitalOceanToken = builder.build()?;
 
         let log_level = ValueBuilder::new(SERVICE_LOG_LEVEL)
             .with_value(self.log_level)

src/domain_updater.rs

@@ -2,6 +2,7 @@ use anyhow::{anyhow, bail, Context, Result};
 use humantime::format_duration;
 use log::{error, info, trace, warn};
 use reqwest::blocking::Client;
+use secrecy::ExposeSecret;
 use std::net::IpAddr;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -142,7 +143,7 @@ impl DomainRecordUpdater for DigitalOceanUpdater {
         let response = self
             .request_client
             .get(&request_url)
-            .bearer_auth(access_token)
+            .bearer_auth(access_token.expose_secret().as_str())
             .query(&[("name", &subdomain_filter)])
             .send()
             .context("Failed to query DO for domain records")?;
@@ -169,7 +170,7 @@ impl DomainRecordUpdater for DigitalOceanUpdater {
         body.insert("data", new_ip.to_string());
         let response = client
             .put(&request_url)
-            .bearer_auth(access_token)
+            .bearer_auth(access_token.expose_secret().as_str())
             .json(&body)
             .send()
             .context(format!(
@@ -287,12 +288,14 @@ mod tests {
 
     #[test]
     fn test_basic() {
+        use crate::types::ValueFromStr;
+
         let mut config_builder =
             crate::config_builder::Builder::new(None, Err(anyhow!("No config")));
         config_builder
             .set_subdomain_to_update("home".to_owned())
             .set_domain_root("site.com".to_owned())
-            .set_digital_ocean_token("123".to_owned())
+            .set_digital_ocean_token(ValueFromStr::from_str("123").unwrap())
             .set_log_level(log::LevelFilter::Info)
             .set_update_interval(crate::config::UpdateInterval(
                 std::time::Duration::from_secs(5).into(),

src/lib.rs

@@ -7,4 +7,5 @@ pub mod humantime_wrapper_serde;
 pub mod ip_fetcher;
 pub mod logger;
 pub mod signal_handlers;
+pub mod token;
 pub mod types;

src/token.rs

@@ -0,0 +1,39 @@
+use crate::types::ValueFromStr;
+use anyhow::Error;
+use secrecy::Secret;
+
+#[derive(Clone)]
+pub struct DigitalOceanToken(String);
+
+pub type SecretDigitalOceanToken = Secret<DigitalOceanToken>;
+
+impl DigitalOceanToken {
+    pub fn as_str(&self) -> &str {
+        self.0.as_str()
+    }
+}
+
+impl secrecy::Zeroize for DigitalOceanToken {
+    fn zeroize(&mut self) {
+        self.0.zeroize();
+    }
+}
+
+impl secrecy::CloneableSecret for DigitalOceanToken {}
+impl secrecy::DebugSecret for DigitalOceanToken {}
+
+impl std::str::FromStr for DigitalOceanToken {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(DigitalOceanToken(s.to_owned()))
+    }
+}
+
+impl ValueFromStr for Secret<DigitalOceanToken> {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Secret::new(s.parse::<DigitalOceanToken>()?))
+    }
+}

src/types.rs

@@ -1,5 +1,5 @@
+use anyhow::Error;
 use serde::Deserialize;
-
 #[derive(Deserialize, Debug)]
 pub struct DomainRecord {
     pub id: u64,
@@ -18,3 +18,41 @@ pub struct DomainRecords {
 pub struct UpdateDomainRecordResponse {
     pub domain_record: DomainRecord,
 }
+pub trait ValueFromStr: Sized {
+    type Err;
+    fn from_str(s: &str) -> Result<Self, Self::Err>;
+}
+
+// This doesn't work with the following error:
+//
+// conflicting implementations of trait `types::ValueFromStr` for type 'x'
+// upstream crates may add a new impl of trait `std::str::FromStr` for type 'x' in future versions
+//
+// Apparently the recommended workaround is to use macros and be explicit
+// about types.
+// impl<T> ValueFromStr for T
+// where
+//     T: std::str::FromStr,
+// {
+//     type Err = T::Err;
+//     fn from_str(s: &str) -> Result<Self, Self::Err> {
+//         s.parse::<T>()
+//     }
+// }
+
+macro_rules! impl_value_from_str {
+    ( $($t:ty),* ) => {
+        $( impl ValueFromStr for $t {
+            type Err = Error;
+            fn from_str(s: &str) -> Result<Self, Self::Err>
+            {
+                match s.parse::<$t>() {
+                    Ok(val) => Ok(val),
+                    Err(e) => Err(e.into()),
+                }
+            }
+        })*
+    }
+}
+
+impl_value_from_str! { String, bool, crate::config::UpdateInterval, log::LevelFilter }